* Take pointer to map in AsOptionalMap for consistency
- Other `As*` functions take pointers.
- It avoids having to initialize the map passed to the function
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
* Remove double pointer
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
* AsOptionalMap -> CollectMapEntriesWithPrefix
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Add a `map-lease-prefix` prefix for config keys for
`config-leader-election` that is a map from a generated lease prefix
to a new prefix:
```yaml
map-lease-prefix.<component>.<package>.<reconciler_type_name>: <new_prefix>
map-lease-prefix.<component-x>.<package-x>.<reconciler_type_name-x>: <new_prefix>
```
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
* Use TB interface, rather than T pointer when doing test stuff
this will permit passing testing.B everywhere too!
/assign @markusthoemmes @dprotaso
* rm-todo
* Fix race: Make informed watcher start wait for Add event 🏎️
When using the informed watcher to watch a config map, previously add
events were being processed in a goroutine with no syncrhonization
making it so that code may try to access the values backed by the
configmaps before they are initialized.
This commit makes it so that the Start method of the informer will wait
for the add event to occur at least once for all config maps it is
watching.
This commit also undoes the workaround added in #1929 which was working
around the race condition identified in #1907 (and in
https://github.com/tektoncd/pipeline/issues/3720). This means that if
the synchronization was removed, the impacted test would start flaking
again. If we wanted it to reliably fail in that case, we could introduce
a sleep in the callback but that doesn't seem worth it.
I also tested this change by manually patching the changes into my
clone of tektoncd/pipeline and following the repro steps at
https://github.com/tektoncd/pipeline/issues/2815#issuecomment-733207368
Before the change I can reproduce the issue, and after the change, I
can't! :D
Fixes#1960
* Make synced callback and named wait group private 🕵️
These utility objects don't really make sense to expose as part of the
informed watcher package and are only used by the informed watcher.
Writing tests for unexported code makes me a bit :( but hopefully these
will get moved to some other package one day. And there's really no
reason to expose these to users of knative/pkg at the moment.
* Port setup code to pkg
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
* Moved prebuilt setups in config
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
* Removed maybe useless code
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
* Removed hardcoded configs and renamed defaultConfig() to NoopConfig()
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
* Applied suggestions
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
* Applied suggestions
Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>
* Check example-hash label for consistency in ConfigMap webhook.
We've seen users try to edit our configuration and falling into the trap of editing the '_example' block a lot. This attempts at guiding the users to do "the right thing" by checking the ConfigMap's '_example' value (if present) against a precomputed hash of the same value (if present). The idea is that we precompute this has using the tool herein in code generation and thus allow us to easily change the example block automatically while making it hard to change it on ppurpose. If the hashes don't match on an upgrade, the webhook will return an error synchronously, guiding the user to the correct behavior.
* Add tests.
* Add hash check to test utilities.
* Add a bit of coverage.
* Rewrite into a table test.
* Rename.
* Reduce test surface.
* Godoc.
* Code nits.
* Docs.
* Use CRC32.
* Nits.
Generally, the `onAfterStore` function is used to kick off a global resync, which should be a very quick operation. Having `onAfterStore` being called asynchronously on a goroutine makes it very hard or straight out impossible to control it, especially in tests.
If there's a costly operation being done `onAfterStore`, it should be up to the caller to decide to make that asynchronous and thus put it into a goroutine.
* tests: Rename 'cm' vars to 'cmw' for consistency
* Filter tracked sharedmain ConfigMaps based on label selector
The ConfigMap watcher only tracks the ConfigMaps which contain the label
defined by the optional SYSTEM_RESOURCE_LABEL env var if the latter
exists.
* Ignore idempotent updates and other cleanups
This makes sure we don't call the configmap update on the idempotent updates, which we get now and
that causes us lots of global resyncs. (┛◉Д◉)┛彡┻━┻.
In addition I went ahead and applied some nitpicking to the code.
/assign mattmoor
* fix races
* resync to zero
* Profiling support
* Move ProfilingPort to profiling package
* Fix golint errors
* Refactor watcher to accept variable length observers
* Cleaner happy path
* Remove profiling handlers argument
* use :8008 string as const
* Make the profiling port package private
* Make UpdateFromConfigMap member of profiling handler
* use mutex when accessing the enabled flag
* test the server as well
* Fixes
* Initialize profiling from configMap at startup
* Use httptest.ResponseRecorder to make the test more lightweight
* Fixes
* Do not initialize from configmap at startup
Dave Protasowski
Pierangelo Di Pilato
Pierangelo Di Pilato
jwcesign
Matt Moore
Markus Thömmes
Evan Anderson
Victor Agababov
Christie Wilson
Matt Moore
runzexia
Julian Friedman
Francesco Guardiani
Zbynek Roubalik
Yanwei Guo
Weston Haught
Jon Donovan
Slavomir Kaslev
Antoine Cotten
Nima Kaviani
Matthias Wessendorf
Martin Gencur