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

Fix connectivity issue if nodes share the same name across the clustermesh and wireguard is enabled #24785

Merged

Conversation

giorio94
Copy link
Member

@giorio94 giorio94 commented Apr 6, 2023

Currently, the wireguard subsystem in the cilium agent caches information about the known peers by node name only. This can lead to conflicts in case of clustermesh, if nodes in different clusters have the same name, causing in turn connectivity issues. Hence, let's switch to identify peers by full name (i.e., cluster-name/node-name) to ensure uniqueness.

Fixes: #24227
Reported-by: @oulinbao

Fix connectivity issue if nodes share the same name across the clustermesh and wireguard is enabled

@giorio94 giorio94 added kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium. area/clustermesh Relates to multi-cluster routing functionality in Cilium. area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. needs-backport/1.11 needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Apr 6, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.12.9 Apr 6, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.13.2 Apr 6, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.11.16 Apr 6, 2023
@giorio94 giorio94 force-pushed the mio/clustermesh-wireguard-same-node-name branch 2 times, most recently from 3fea3e9 to e03cff2 Compare April 7, 2023 09:08
@giorio94
Copy link
Member Author

giorio94 commented Apr 7, 2023

/test

@giorio94 giorio94 marked this pull request as ready for review April 7, 2023 09:08
@giorio94 giorio94 requested review from a team as code owners April 7, 2023 09:08
@giorio94 giorio94 requested review from NikAleksandrov, a team and brb and removed request for a team April 7, 2023 09:08
@giorio94
Copy link
Member Author

giorio94 commented Apr 7, 2023

/ci-eks

Hit unrelated flake #24774

@giorio94
Copy link
Member Author

giorio94 commented Apr 7, 2023

The ConformanceEKS test hit again the same unrelated flake. I'm not rerunning it, since it wouldn't test anything wireguard-related in any case.

@michi-covalent michi-covalent added this to Needs backport from master in 1.12.10 Apr 14, 2023
@michi-covalent michi-covalent removed this from Needs backport from master in 1.12.9 Apr 14, 2023
@michi-covalent michi-covalent added this to Needs backport from master in 1.11.17 Apr 14, 2023
@michi-covalent michi-covalent removed this from Needs backport from master in 1.11.16 Apr 14, 2023
@gentoo-root gentoo-root added this to Needs backport from master in 1.13.3 Apr 14, 2023
@gentoo-root gentoo-root removed this from Needs backport from master in 1.13.2 Apr 14, 2023
@pchaigno pchaigno requested review from YutaroHayakawa and removed request for NikAleksandrov April 18, 2023 10:25
Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@giorio94
Copy link
Member Author

test-1.27-net-next failures:

@giorio94
Copy link
Member Author

Marking as ready to merge, since reviews are in, and test failures are due to known flakes (and unrelated, as wireguard is not enabled).

@giorio94 giorio94 added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Apr 20, 2023
@ldelossa ldelossa merged commit 7398de6 into cilium:main Apr 20, 2023
58 of 60 checks passed
@nbusseneau nbusseneau mentioned this pull request Apr 20, 2023
4 tasks
@nbusseneau nbusseneau mentioned this pull request Apr 20, 2023
7 tasks
@nbusseneau nbusseneau mentioned this pull request Apr 20, 2023
15 tasks
@nbusseneau nbusseneau 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 Apr 20, 2023
@sayboras sayboras added backport-done/1.13 The backport for Cilium 1.13.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.11 The backport for Cilium 1.11.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. backport-pending/1.12 labels Apr 26, 2023
@jrajahalme jrajahalme moved this from Needs backport from main to Backport done to v1.11 in 1.11.17 May 12, 2023
@thorn3r thorn3r moved this from Needs backport from main to Backport done to v1.12 in 1.12.10 May 16, 2023
@thorn3r thorn3r moved this from Needs backport from main to Backport done to v1.13 in 1.13.3 May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/clustermesh Relates to multi-cluster routing functionality in Cilium. area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. 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.

cilium agent can not find all wireguard peers in clustermesh context
6 participants