etcd backend not passing watcher errors up the stack, and errors not … #1337
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
robbrockbank
force-pushed
the
handle-old-revision
branch
5 times, most recently
from
9326b9e to
eddd417
Compare
…triggering full resync in watcher cache
robbrockbank
force-pushed
the
handle-old-revision
branch
from
eddd417 to
5ff61dc
Compare
doublek
approved these changes
Oct 30, 2020
3 tasks
|
@robbrockbank Thanks for handling this. At the risk of being annoying - since I haven't understood the detail here - can I just ask if the combination of #1247 + this still makes sense? I'm asking because it appears to me that this PR reverts about half of the changes in #1247 . (Also cc @caseydavenport , as the author of #1247 ) |
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.
…triggering full resync in watcher cache
Description
This contains a couple of fixes
In etcd backend, make sure errors due to watches are passed as an event - this is to ensure real errors result in a re-sync. In particular compacted revisions manifest as an error on the watch channel - we were not sending an error and just terminating the channel causing us to loop indefinitely trying to use a compacted revision.
Once in-sync, do not go out of sync in the watcher syncer. There is no point in changing status, we already cache the revisions so that once we are connected again we send the correct updates. Felix was not using the status change, and the status change was actually causing issues with other components that were not expecting the status change.
Remove the ErrorWatchTerminated definition so that we don't try to use it. This forces us to fix up applications that were relying on this error type.
Should fix: projectcalico/calico#4109
Todos
Release Note