Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linux/node_id: do not attempt to map NoID #25222

Merged
merged 1 commit into from
May 4, 2023

Conversation

bimmlerd
Copy link
Member

@bimmlerd bimmlerd commented May 2, 2023

Noticed during code review for a different issue.

We correctly detect that we failed to allocate a new node ID (due to exhaustion of the idpool), but then still go ahead and map it. This leads to spurious errors which include "Failed to map node IP address to allocated ID".

Instead, don't try to map NoID and return it directly.

Fix spurious errors containing "Failed to map node IP address to allocated ID".

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 2, 2023
@pchaigno pchaigno added kind/bug This is a bug in the Cilium logic. needs-backport/1.11 needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels May 2, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.13.3 May 2, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.12.10 May 2, 2023
@pchaigno pchaigno added the release-note/bug This PR fixes an issue in a previous release of Cilium. label May 2, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.11.17 May 2, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 2, 2023
@bimmlerd
Copy link
Member Author

bimmlerd commented May 2, 2023

/test

@bimmlerd
Copy link
Member Author

bimmlerd commented May 2, 2023

Rerunning the failed (non-required) ConformanceK8sKind / kubernetes-e2e (ipv4).
It failed while installing cilium, though I didn't find much actionable logs. I did see that pulling of the CI image didn't seem to complete for one of the cilium pods, according to k8s events, but not sure whether that's symptom or cause.

@bimmlerd bimmlerd marked this pull request as ready for review May 2, 2023 12:33
@bimmlerd bimmlerd requested a review from a team as a code owner May 2, 2023 12:33
We correctly detect that we failed to allocate a new node ID (due to
exhaustion of the idpool), but then still go ahead and map it. This
leads to spurious errors which include "Failed to map node IP address to
allocated ID".

Instead, don't try to map NoID and return it directly.

Fixes: af88b42 (datapath: Introduce node IDs)

Suggested-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
@bimmlerd bimmlerd force-pushed the pr/bimmlerd/do-not-map-nodeid branch from d9d0a01 to 4378873 Compare May 2, 2023 12:36
@bimmlerd
Copy link
Member Author

bimmlerd commented May 2, 2023

/test

Job 'Cilium-PR-K8s-1.26-kernel-net-next' failed:

Click to show.

Test Name

K8sDatapathServicesTest Checks N/S loadbalancing With host policy Tests NodePort

Failure Output

FAIL: Can not connect to service "http://[fd04::11]:32458" from outside cluster (1/10)

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.26-kernel-net-next/2027/

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.26-kernel-net-next so I can create one.

Then please upload the Jenkins artifacts to that issue.

@bimmlerd
Copy link
Member Author

bimmlerd commented May 3, 2023

CI triage:

@bimmlerd
Copy link
Member Author

bimmlerd commented May 3, 2023

/test-1.26-net-next

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 3, 2023
@tklauser tklauser merged commit 9115e05 into cilium:main May 4, 2023
56 checks passed
@bimmlerd bimmlerd deleted the pr/bimmlerd/do-not-map-nodeid branch May 4, 2023 11:58
@YutaroHayakawa YutaroHayakawa mentioned this pull request May 10, 2023
14 tasks
@YutaroHayakawa YutaroHayakawa added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels May 10, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.13 in 1.13.3 May 10, 2023
@YutaroHayakawa YutaroHayakawa mentioned this pull request May 10, 2023
7 tasks
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.12 in 1.12.10 May 10, 2023
@jrajahalme jrajahalme mentioned this pull request May 11, 2023
5 tasks
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.11 in 1.11.17 May 11, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.11 in 1.11.17 May 11, 2023
@jrajahalme jrajahalme added backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. and removed backport-pending/1.11 labels May 12, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.11 to Backport done to v1.11 in 1.11.17 May 12, 2023
@YutaroHayakawa YutaroHayakawa added backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. and removed backport-pending/1.12 labels May 15, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.12 to Backport done to v1.12 in 1.12.10 May 15, 2023
@julianwiedmann julianwiedmann added backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. labels Jul 10, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.13 to Backport done to v1.13 in 1.13.3 Jul 10, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.13 to Backport done to v1.13 in 1.13.3 Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
1.11.17
Backport done to v1.11
1.12.10
Backport done to v1.12
1.13.3
Backport done to v1.13
Development

Successfully merging this pull request may close these issues.

None yet

7 participants