fix(config): only call run if informer not already synced #33
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
markphelps
approved these changes
Jun 20, 2024
markphelps
added a commit
that referenced
this pull request
Jan 10, 2025
* 'main' of https://github.com/flipt-io/reverst: fix(config): only call run if informer not already synced (#33) fix(config): correctly set token supplied via explicit token field (#32) feat(config): support watching k8s secrets directly as token sources (#31) fix(server): call WriteHeader in status interceptor (#30) fix: correct typo and add cancel context handling in watchFSNotify function (#29) feat: rename reverst to reverstd and add a new tunnel client CLI in its place (#28) feat(gh): tag images pushed to main with SHA as latest (#27) fix(server/client): fix propagation of client errors from server to client (#26) chore(gh): run publish on merge_group event type chore(mod): upgrade quic-go to v0.44.0 (#25) fix(server): stop accumulating times and calculate correct ellapsed fix(roundrobbin): ensure current cannot be out of bounds fix(roundrobbin): ensure that evict is only called once chore(dagger): run unit tests 5 times repeatedly test(roundrobbin): add failing case ensuring one call to evict per unique eviction chore(server): add debug logging around metrics endpoint
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, reverst prints out a warning because informers are getting
Run()called more than once.This is because the shared informer factory caches previously instantiated informers based in the object type (note for future thought, it doesn't take into consideration e.g. list options or namespace filter supplied).
The way the secret source works, it can and will request the same informer, so we will invoke Run on the same instance more than once when secrets are updated.
This change just interogates the informer returned by the factory to see if it has been synced at-least once.
We won't build another informer until we have at-least waited for a single sync, so I am leaning on this invariant to avoid calling Run more than once.
The other alternative is to effectively create our own factory and cache it manually, which we could do.
For now though, this appears to avoid the double Run situation.
p.s. I fixed the race on
t.Logcausing the occassional panic in tests.