fix: Make IPAM default route idempotent on ADD retries - #270
Merged
Conversation
A CNI ADD retried against a netns that a previous, non-DEL'd ADD had already configured failed with "add default route ... file exists" instead of succeeding, leaving pods stuck in ContainerCreating. addAddrAndDefaultRoute now checks existing kernel state before adding: identical state is a no-op, while a default route via a different gateway is a loud conflict error instead of being silently overwritten or masked. DEL's host-device delegation error (the mechanism that flushes the route/address by moving the interface out of the netns) is now logged instead of silently discarded, so a failed cleanup is diagnosable.
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.
Summary
galactic-cni's IPAM step adds a default route toward the attachment's gateway on every CNI ADD. If a caller retries ADD against the same netns without an intervening DEL — which happens in practice when an earlier ADD attempt is aborted before its own rollback runs — the route already exists and ADD fails with
configure IPAM: add default route via <gateway>: file exists, leaving the pod stuck inContainerCreatingon every subsequent retry.The IPAM route/address install now checks existing kernel state first: if the address or default route already match what's being installed, the call is a no-op success. If a default route exists via a different gateway, that's a real misconfiguration and now fails loudly instead of being silently papered over or overwritten.
Also hardened the DEL path: the error from delegating DEL to the host-device plugin (the mechanism that actually flushes the route/address, by moving the guest interface back out of the netns) was previously discarded entirely. It's now logged, so a failed cleanup — the underlying cause of the stale route in the first place — is visible instead of invisible.