enforce the 1 IP per type and reject all other IPs of the same type
Please ensure your pull request adheres to the following guidelines:
- [x] For first time contributors, read Submitting a pull request
- [x] All code is covered by unit and/or runtime tests where feasible.
- [x] All commits contain a well written commit description including a title,
description and a
Fixes: #XXXline if the commit addresses a particular GitHub issue. - [x] All commits are signed off. See the section Developer’s Certificate of Origin
- [x] Provide a title or release-note blurb suitable for the release notes.
- [x] Thanks for contributing!
This PR is made to handle when K8s Nodes have more than one IP address per address type in their status field. Fixes: #20787
k8s: Enforce the 1 IP per type
Commit ade978a489747f96c2cfda1d31173c9eaaf4000c does not contain "Signed-off-by".
Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin
@tommyp1ckles Made some changes, could you please take a look.
Also is there any make rule for the repo to format the code or to point out the mistakes in the code. I tried running make lint but it is taking very it is stuck for a very long time.
@NikhilSharmaWe General approach looks ok, looks like the build is failing though. Couple of comments as well.
@christarazi Can you take a look to see if this adequately solves the issue at hand.
@NikhilSharmaWe not sure what the problem with lint is, maybe check https://docs.cilium.io/en/stable/contributing/development/dev_setup/ to make sure you have the right dependencies installed etc.
The branch CI will also do lint checks and post comments if it finds any issues.
@NikhilSharmaWe If you rebase the CI, that should fix the current build failure.
@tommyp1ckles Rebased the branch
c/c @aditighag
@NikhilSharmaWe You can ping @cilium/tophat, or ping in the #development Slack channel if you need someone to trigger the tests.
/test
Job 'Cilium-PR-K8s-1.23-kernel-4.19' failed:
Click to show.
Test Name
K8sChaosTest Connectivity demo application Endpoint can still connect while Cilium is not running
Failure Output
If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.23-kernel-4.19 so I can create one.
What if this is a dual-stack cluster? Will we do the right thing there?
(It may be that cilium was wrong before, and this code isn't at fault. But we should find out)
What if this is a dual-stack cluster? Will we do the right thing there?
What would dual-stack have to do with this code? This code is checking for address types such as InternalIP or ExternalIP, not family JFYI in case that's what might have caused confusion.
What would dual-stack have to do with this code?
Right, but dual-stack nodes have multiple entries for the same address type. Like I said, it is possible that the existing code is making a bad assumption.
For example, this is from a dual-stack kind cluster on my laptop:
spec:
podCIDR: 10.244.0.0/24
podCIDRs:
- 10.244.0.0/24
- fd00:10:244::/64
providerID: kind://docker/ovn/ovn-control-plane
status:
addresses:
- address: 172.18.0.2
type: InternalIP
- address: fc00:f853:ccd:e793::2
type: InternalIP
- address: ovn-control-plane
type: Hostname
...
Ah, gotcha. Understood. Thanks Casey!
@NikhilSharmaWe could you account for IP family as well in this logic?
Thanks for the contribution! The PR has been inactive for some time, so I'm turning it into a draft until feedback is addressed.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
This pull request has not seen any activity since it was marked stale. Closing.