OCPBUGS-87249 - clustermanager: re-evaluate egress IPs when cloud egress IP config changes - #3269
OCPBUGS-87249 - clustermanager: re-evaluate egress IPs when cloud egress IP config changes#3269jechen0648 wants to merge 1 commit into
Conversation
…anges When cloud-network-config-controller updates the cloud.network.openshift.io/egress-ipconfig annotation (e.g., adding an IPv6 subnet that was initially missing because the AWS subnet's IPv6 CIDR block was not yet in 'associated' state), the node update handler returned early without re-triggering egress IP assignment. Add CloudEgressIPConfigAnnotationChanged() and include it in the early-return guard so that addEgressNode() is called whenever the annotation changes, allowing previously-unassigned IPv6 egress IPs to be reconsidered. Signed-off-by: Jean Chen <jechen@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
WalkthroughNode update reconciliation now treats cloud egress IP config annotation changes as meaningful. A new node annotation comparison helper feeds the egress IP event handler, which no longer short-circuits those updates and logs a different message before continuing. ChangesCloud egress IP annotation handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jechen0648 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
Status, support, documentation and community
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
jechen0648
changed the title
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
/retest |
|
@sadasu: This PR was included in a payload test run from openshift/cloud-network-config-controller#228
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/91d917a0-8087-11f1-886f-7fdbe052f05d-0 |
|
/retest |
|
@jechen0648: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@sadasu: This PR was included in a payload test run from openshift/cloud-network-config-controller#228
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/aaa99e80-90d6-11f1-9bba-3026d0a99078-0 |
2 participants
📑 Description
Problem
On dualstack AWS clusters, egressIPv6 was never assigned even though nodes correctly report IPv6 subnet and capacity in the cloud.network.openshift.io/egress-ipconfig annotation.
The underlying cause lives in two places. The primary cause is in cloud-network-config-controller (CNCC): a bug in getSubnet could cause it to initially write the annotation with no IPv6 subnet (or the wrong one) by unconditionally picking the first entry in Ipv6CidrBlockAssociationSet regardless of its state. This is fixed in a companion CNCC PR.
However, even with the CNCC fix in place, there is a secondary failure mode in OVN-K's node UpdateResource handler. Consider this sequence:
A node comes up; CNCC has not yet annotated it, so initEgressIPAllocator fails. The node's egressIPConfig in the allocator cache has V6.Net == nil.
CNCC annotates the node (possibly with no IPv6 subnet initially, due to the CNCC bug).
OVN-K receives the node UPDATE, calls initEgressIPAllocator (cache updated), then hits the early-return guard:
if isOldReady == isNewReady && !isHostCIDRsAltered {
return nil // ← exits here; addEgressNode() is never called
}
Because only the CNCC annotation changed — not readiness and not host-cidrs — the handler returns early without calling addEgressNode().
Any EgressIPs whose IPv6 address could not be placed (no node had a matching V6.Net) remain permanently unassigned — addEgressNode() is the only code path that re-evaluates them.
Later, CNCC corrects the annotation (adding the IPv6 subnet). OVN-K again receives the UPDATE and again exits early, never reconsidering the unassigned IPv6 EgressIPs.
The net effect: even after both nodes report i.e. "ipv6":"2600:1f18:1816:ac01::/64" with capacity 14 in their annotation, the EgressIP status shows the IPv6 address as permanently unassigned.
Fix
pkg/util/node_annotations.go
Add CloudEgressIPConfigAnnotationChanged(), analogous to the existing NodeHostCIDRsAnnotationChanged()
pkg/clustermanager/egressip_event_handler.go
Add isCloudEgressIPConfigAltered to the early-return guard so that annotation changes from CNCC trigger addEgressNode(), giving previously-unassigned EgressIPs another chance to be reconciled with the updated node cache. A distinct log line is emitted when the trigger is an annotation change rather than a readiness transition.
Impact
Cloud platforms only (PlatformTypeIsEgressIPCloudProvider()): CloudEgressIPConfigAnnotationChanged only fires on nodes that CNCC manages, so bare-metal is unaffected.
Low frequency: CNCC typically writes the annotation once at node startup and rarely updates it, so addEgressNode() is not called more often than necessary.
No functional regression for IPv4: The change is additive — it only allows addEgressNode() to run in a case where it previously did not.
Related
Companion CNCC fix: [openshift/cloud-network-config-controller# ](openshift/cloud-network-config-controller#228) — fixes the root cause where getSubnet picks a stale disassociated IPv6 CIDR block association instead of the current associated one.
Testing
Pre-merge testing was performed with openshift/cloud-network-config-controller#228 on dualstack AWS IPv4 primary and dualstack AWS IPv6 primary cluster
Regression testing with openshift/cloud-network-config-controller#228 was performed on dualstack BM cluster
Fixes #
Additional Information for reviewers
✅ Checks
How to verify it
Summary by CodeRabbit