ipam: Warn when failing to update CN status #13197
Merged
Conversation
This file contains hidden or 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
This commit attempts to improve the readability and debugability of (*Node).syncToAPIServer(). The main motivation for this change is to fix the swallowing of the error when UpdateStatus fails, and the subsequent Get succeeds. To that end, this commit adds a warning log msg and propagates the error in case we never succeed, to surface any trouble with updating the status of the CiliumNode (CN) resource. Failures in updating the status can be indicative of general connectivity problems in ENI and Azure IPAM modes. As seen in cilium#13193, it can also lead to Cilium never becoming ready because the agent was never able to retrieve ENI information required to make allocations, etc. (The former was caused by the Operator being blocked from "publishing" the ENI information into the CN resource, which is why the agent was unable to retrieve it.) Signed-off-by: Chris Tarazi <chris@isovalent.com>
aanm
approved these changes
Sep 17, 2020
|
test-me-please |
vadorovsky
approved these changes
Sep 17, 2020
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.
This commit attempts to improve the readability and debugability of
(*Node).syncToAPIServer(). The main motivation for this change is to fix
the swallowing of the error when UpdateStatus fails, and the subsequent
Get succeeds.
To that end, this commit adds a warning log msg and propagates the error
in case we never succeed, to surface any trouble with updating the
status of the CiliumNode (CN) resource. Failures in updating the status
can be indicative of general connectivity problems in ENI and Azure IPAM
modes. As seen in #13193, it can
also lead to Cilium never becoming ready because the agent was never
able to retrieve ENI information required to make allocations, etc. (The
former was caused by the Operator being blocked from "publishing" the
ENI information into the CN resource, which is why the agent was unable
to retrieve it.)