retry: refactor stop channel/WaitGroup usage to reduce dependency on WatchFactory #3297
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
…WatchFactory Instead of having the WatchFactory expose a waitgroup, pass the top-level waitgroup down to the retry code and use it there. The WatchFactory already waits for all handlers to stop, so we don't need to couple both things together. Signed-off-by: Dan Williams <dcbw@redhat.com>
dcbw
force-pushed
the
retry-wait-refactor
branch
from
051f8bf
to
8e5a605
Compare
bpickard22
reviewed
Dec 1, 2022
dcbw
added a commit
to dcbw/ovn-kubernetes-1
that referenced
this pull request
Dec 5, 2022
…WatchFactory Backport of ovn-org/ovn-kubernetes#3297
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.
Instead of having the WatchFactory expose a WaitGroup (which is somewhat of an encapsulation violation), we can pass the top-level WaitGroup from ovnkube.go down to the retry code and use it there. We're already using that WaitGroup many places in the master and node code so we might as well push it down to retry code too. This keeps the factory code somewhat cleaner by not exposing its own internal WaitGroup.
All we really care about is that the goroutine that calls
periodicallyRetryResources()exits when the OvnController or OvnNode is shut down, and adding it to the top-level WaitGroup accomplishes that.As a bonus, consolidate FakeOvnNode shutdown code in the node tests into AfterEach().
@trozet @npinaeva @bpickard22
Reverts part of #3226