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

Extend ipcache key with ClusterID #22200

Conversation

YutaroHayakawa
Copy link
Member

This is a part of the Cluster Mesh with overlapping PodCIDR support. When the remote cluster has an overlapping PodCIDR, we must identify the remote endpoint using IP + ClusterID. This PR makes the required changes for ipcache to realize that. Note that this PR only adds infrastructure, and currently, we don't "use" that. Please review commit by commit.

Extend ipcache key with ClusterID

@YutaroHayakawa YutaroHayakawa added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/misc This PR makes changes that have no direct user impact. labels Nov 16, 2022
@YutaroHayakawa YutaroHayakawa requested review from a team as code owners November 16, 2022 04:51
@YutaroHayakawa YutaroHayakawa marked this pull request as draft November 29, 2022 09:13
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/clustermesh-overlapping-podcidr-oss-ipcache branch 7 times, most recently from 4302118 to 40dc1f2 Compare December 1, 2022 07:58
@YutaroHayakawa YutaroHayakawa marked this pull request as ready for review December 2, 2022 02:26
@YutaroHayakawa YutaroHayakawa requested a review from a team as a code owner December 2, 2022 02:26
@joestringer joestringer added upgrade-impact This PR has potential upgrade or downgrade impact. release-blocker/1.13 This issue will prevent the release of the next version of Cilium. labels Dec 6, 2022
@joestringer
Copy link
Member

@YutaroHayakawa will this impact upcoming 1.14 or later features? If yes we may consider for 1.13, otherwise let's remove the 1.13 release blocker label.

@joestringer joestringer removed the release-blocker/1.13 This issue will prevent the release of the next version of Cilium. label Dec 7, 2022
@joestringer
Copy link
Member

Discussed out-of-band with Yutaro, no need for release-blocker label here.

@YutaroHayakawa
Copy link
Member Author

@jrajahalme @NikAleksandrov Hi, I'm glad if I can have some 👀 on this PR 🙏

@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/clustermesh-overlapping-podcidr-oss-ipcache branch from 40dc1f2 to a15271b Compare January 4, 2023 06:40
@YutaroHayakawa
Copy link
Member Author

Rebased on the latest master.

@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa YutaroHayakawa added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 5, 2023
@YutaroHayakawa
Copy link
Member Author

A review is in, and CI is passing. Let me mark this ready-to-merge.

Copy link
Contributor

@ldelossa ldelossa left a comment

Choose a reason for hiding this comment

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

Small nits, but overall things look good.

@@ -218,3 +218,22 @@ func (ac AddrCluster) As20() (ac20 [20]byte) {
func (ac AddrCluster) AsNetIP() net.IP {
return ac.addr.AsSlice()
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: maybe add some comments about what this structure is representing?

pkg/clustermesh/types/addressing.go Show resolved Hide resolved
@maintainer-s-little-helper maintainer-s-little-helper bot removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Jan 5, 2023
Introduce a new type PrefixCluster which holds IP prefix and CluterID.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Consume one padding slot and encode cluster_id into struct ipcache_key.
This fields will be used to lookup tunnel endpoint and security identity
of the remote cluster with overlapping PodCIDR.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
With Cluster Mesh with overlapping PodCIDR, remote endpoints must be
identified using IP + ClusterID. Thus, ipcache lookup must be changed as
well.

This commit changes function signature and doesn't contain any code to
use that.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/clustermesh-overlapping-podcidr-oss-ipcache branch from a15271b to 0c7514a Compare January 6, 2023 15:44
@YutaroHayakawa
Copy link
Member Author

Added comments.

@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa YutaroHayakawa added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 10, 2023
@aditighag aditighag merged commit b3747c8 into cilium:master Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. upgrade-impact This PR has potential upgrade or downgrade impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants