Add node IP to HostNetworkNamespace address_set#3141
Add node IP to HostNetworkNamespace address_set#3141asood-rh wants to merge 2 commits intoopenshift:masterfrom
Conversation
When NoOverlay mode is used for a network, it uses learned route with proto bgp and that sets Node IP as source IP. 10.129.2.0/23 nhid 157 via 192.168.100.100 dev br-ex proto bgp metric 20 So, it is essential to add node IP to HostNetworkNamespace address_set to let host network POD use network-policy while using NoOverlay mode. Signed-off-by: Arnab Ghosh <arnabghosh89@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds logic to include a node's primary interface IPs (from the node annotation) in host-network-namespace address sets when transport mode is set to NoOverlay; includes a unit test validating the behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: asood-rh 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
go-controller/pkg/ovn/master_test.go (1)
1885-1888: Consider adding an IPv6 primary-ifaddr assertion in this test.The production path in
getHostNamespaceAddressesForNodenow handles both IPv4 and IPv6 primary interface CIDRs, but this case validates only IPv4. Adding a dual-stack variant would lock coverage for both parse branches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go-controller/pkg/ovn/master_test.go` around lines 1885 - 1888, Add an IPv6 primary interface address to the test so the getHostNamespaceAddressesForNode IPv6 branch is exercised: in the same test case where ips is appended with "192.168.1.10" also append a representative IPv6 primary-ifaddr like "fd00::10" (or the dual-stack variant used elsewhere) and then call fakeOvn.asf.EventuallyExpectAddressSetWithAddresses(config.Kubernetes.HostNetworkNamespace, ips) so the expected addresses include both the IPv4 and IPv6 primary interface addresses; ensure the ips slice used by the assertion matches the dual-stack inputs for getHostNamespaceAddressesForNode.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@go-controller/pkg/ovn/master_test.go`:
- Around line 1885-1888: Add an IPv6 primary interface address to the test so
the getHostNamespaceAddressesForNode IPv6 branch is exercised: in the same test
case where ips is appended with "192.168.1.10" also append a representative IPv6
primary-ifaddr like "fd00::10" (or the dual-stack variant used elsewhere) and
then call
fakeOvn.asf.EventuallyExpectAddressSetWithAddresses(config.Kubernetes.HostNetworkNamespace,
ips) so the expected addresses include both the IPv4 and IPv6 primary interface
addresses; ensure the ips slice used by the assertion matches the dual-stack
inputs for getHostNamespaceAddressesForNode.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 262325c2-c283-4ea4-84f2-0df676bbf958
📒 Files selected for processing (2)
go-controller/pkg/ovn/master_test.gogo-controller/pkg/ovn/namespace.go
Signed-off-by: Arti Sood <asood@redhat.com>
|
@asood-rh: 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. |
2 participants
When NoOverlay mode is used for a network, it uses learned route with proto bgp and that sets Node IP as source IP.
10.129.2.0/23 nhid 157 via 192.168.100.100 dev br-ex proto bgp metric 20
So, it is essential to add node IP to HostNetworkNamespace address_set to let host network POD use network-policy while using NoOverlay mode.
📑 Description
This PR is just to pre merge upstream commit with CNO PR openshift/cluster-network-operator#2960 for bug https://redhat.atlassian.net/browse/OCPBUGS-83406
Fixes #
Additional Information for reviewers
✅ Checks
How to verify it
Summary by CodeRabbit
Bug Fixes
Tests