[ipam] Skip redundant CiliumNode status writes in refreshNode - #671
Closed
jaredledvina wants to merge 1 commit into
Closed
[ipam] Skip redundant CiliumNode status writes in refreshNode#671jaredledvina wants to merge 1 commit into
jaredledvina wants to merge 1 commit into
Conversation
Signed-off-by: Jared Ledvina <jared.ledvina@datadoghq.com>
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
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.
refreshNode()inpkg/ipam/crd.goissued a full-objectUpdateStatus()on the CiliumNode custom resource on every allocate/release/retry trigger, even when the node's IPAM status was unchanged. Since the agent and the operator both write the same CiliumNode object (and its spec/status share a single resourceVersion), these unconditional no-op writes add avoidable apiserver write load and resourceVersion churn, which also increases the chance of update conflicts.This adds a no-op guard to
refreshNode():UpdateStatus()call when the freshly rebuiltStatus.IPAM.Usedmap is unchanged from the last value successfully written to the apiserver.Usedis compared by content, since the agent is its sole writer.Status.IPAM.ReleaseIPswith an explicit dirty flag set byupdateLocalNodeResource()rather than a content comparison.ReleaseIPsis shared with the operator (the release ACK/NACK handshake), so value-equality against the agent's own last write is not a reliable no-op signal: the operator can re-mark an IP with a value the agent previously wrote.The write is still performed whenever
Usedactually changes or a release ACK/NACK is pending, so status propagation is unaffected.Adds unit tests in
pkg/ipam/crd_test.gocovering the skip/no-skip cases, theReleaseIPsdirty-flag path, and the UID-reset edge case.