OCPBUGS-95615: fix(kubevirt): use only first MachineInternalIP per family in EndpointSlice endpoints#8892
Conversation
…tSlice endpoints Since CAPK PR openshift#366 exposes all VMI interface IPs for dual-stack CSR approval, Machine.Status.Addresses includes ovn-k8s-mp0 management port IPs from the guest cluster's OVN overlay alongside the VM's routable pod network IP. The HCCO machine controller was putting all MachineInternalIP addresses into EndpointSlice Endpoint.Addresses, causing MetalLB/kube-proxy to round-robin traffic to non-routable IPs and producing ~50% connection timeouts on LoadBalancer services. The existing link-local filter (OCPBUGS-83711) does not catch these IPs because they are regular RFC1918 addresses (e.g. 10.8.x.2), not link-local. Use only the first MachineInternalIP per IP family, which is the VM's primary pod network IP on the management cluster. This matches the convention already used in e2e tests (firstMachineAddress). Signed-off-by: Divyam Pateriya <dpateriy@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
📝 WalkthroughWalkthroughThis change modifies the address selection logic in the KubeVirt passthrough service reconciler to select only the first IPv4 address and first IPv6 address per machine when multiple addresses of the same family exist, rather than appending all addresses per family. Test data and a test case are added to verify machines with multiple same-family internal IP addresses correctly produce endpoint slices referencing only the first address per family. Changes
Sequence Diagram(s)Not applicable — this is a targeted logic fix within a single function plus associated test additions, not a multi-component feature or workflow change. Estimated code review effort: Medium Suggested labels: bug, needs-tests-review Suggested reviewers: Not enough information available to determine relevant reviewers from the provided summary. 🐰 PoemA rabbit hopped through IPs galore, 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 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: dpateriya 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 |
|
/jira refresh |
|
@dpateriya: No Jira issue is referenced in the title of this pull request. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@dpateriya: No Jira issue is referenced in the title of this pull request. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@dpateriya: No Jira issue is referenced in the title of this pull request. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@dpateriya: This pull request references Jira Issue OCPBUGS-95615, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8892 +/- ##
=======================================
Coverage 43.28% 43.28%
=======================================
Files 771 771
Lines 95503 95503
=======================================
Hits 41335 41335
Misses 51284 51284
Partials 2884 2884
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
@dpateriya: all tests passed! 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. |
|
/Verified by me Env: OCP 4.21.21 Management Cluster Machine addresses (before fix) Machine kubevirt-test-vh8dr-4t9bh reports two non-link-local IPv4 InternalIPs: BEFORE (unpatched HCCO) EndpointSlice contains both addresses — traffic round-robins to the non-routable IP: AFTER (patched HCCO) Patched the HCCO deployment with the fix image, deleted EndpointSlices, and they were recreated with only the first address per family: Only 10.130.2.8 (the routable VM pod IP) is present. The 10.132.0.2 (ovn-k8s-mp0) is correctly filtered. |
|
@dpateriya: This PR has been marked as verified by DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
What this PR does / why we need it:
Since CAPK PR kubernetes-sigs/cluster-api-provider-kubevirt#366 began exposing all VMI interface IPs for dual-stack CSR approval,
Machine.Status.Addressesincludesovn-k8s-mp0management port IPs from the guest cluster's OVN overlay alongside the VM's routable pod network IP.The HCCO machine controller was putting all
MachineInternalIPaddresses intoEndpointSlice.Endpoint.Addresses, causing MetalLB/kube-proxy to round-robin traffic to non-routableovn-k8s-mp0IPs and producing ~50% connection timeouts on LoadBalancer services exposed via MetalLB.The existing link-local filter (OCPBUGS-83711, PR #8264) does not catch these IPs because they are regular RFC1918 addresses (e.g.
10.8.x.2), not link-local.This PR uses only the first
MachineInternalIPper IP family, which is the VM's primary pod network IP on the management cluster. This matches the convention already used in e2e tests (firstMachineAddressinnodepool_kv_advanced_multinet_test.go).Which issue(s) this PR fixes:
Related to OCPBUGS-83711 (link-local filter) and OCPBUGS-84269 / PR #8230 (CIDR conflict false positive) — same root cause (CAPK PR #366), different symptom.
Customer cases: 04485238, 04485252
Checklist:
Made with Cursor
Summary by CodeRabbit