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 tunnel map key with ClusterID #22687

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 the tunnel map to realize that. Note that this PR only adds infrastructure, and currently, we don't "use" that. Please see the commits for more details.

Extend tunnel map key with ClusterID

@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 Dec 12, 2022
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Dec 12, 2022
@YutaroHayakawa YutaroHayakawa added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. area/clustermesh Relates to multi-cluster routing functionality in Cilium. release-note/misc This PR makes changes that have no direct user impact. and removed kind/community-contribution This was a contribution made by a community member. labels Dec 12, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Dec 12, 2022
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/clustermesh-overlapping-podcidr-oss-tunnel-map branch from 44dc08d to 341dfe5 Compare December 13, 2022 09:49
@YutaroHayakawa YutaroHayakawa marked this pull request as ready for review December 13, 2022 10:26
@YutaroHayakawa YutaroHayakawa requested review from a team as code owners December 13, 2022 10:26
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/clustermesh-overlapping-podcidr-oss-tunnel-map branch from 341dfe5 to 066171e Compare January 4, 2023 06:42
@YutaroHayakawa
Copy link
Member Author

Rebased on the latest master.

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.

LGTM.

@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/clustermesh-overlapping-podcidr-oss-tunnel-map branch from 066171e to 164b9d7 Compare January 10, 2023 07:32
@YutaroHayakawa
Copy link
Member Author

Fixed the conflicts with the latest master.

@YutaroHayakawa
Copy link
Member Author

/test

In Cluster Mesh with overlapping PodCIDR, we have to identify remote
endpoint with IP + ClusterID. Thus, we should also lookup tunnel map
using IP + ClusterID.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Implement required helpers to implement tunnel map with cluster-aware
addressing.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Modify tunnel map APIs (Set/Get/Delete/SilentDelete) to be aware of the
cluster-aware addressing. Currently, there's no user interface to set
non-zero ClusterID.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Tunnel map APIs (Set/Get/Delete/SilentDelete) are implemented as a
method of Map (tunnel map) struct. However, they all uses global
TunnelMap valiable internally. Fix for readability and ease of tests.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/clustermesh-overlapping-podcidr-oss-tunnel-map branch from 164b9d7 to c36dad8 Compare January 11, 2023 03:54
@YutaroHayakawa
Copy link
Member Author

YutaroHayakawa commented Jan 11, 2023

Rebased on the latest master. This removes the duplicated code with #22200. Also I renamed the tunnel_privileged_test.go to tunnel_test.go per this comment #22857 (comment).

@YutaroHayakawa
Copy link
Member Author

YutaroHayakawa commented Jan 11, 2023

/test

Job 'Cilium-PR-K8s-1.16-kernel-4.9' failed:

Click to show.

Test Name

K8sDatapathConfig Host firewall With VXLAN and endpoint routes

Failure Output

FAIL: Failed to reach 192.168.56.12:80 from testclient-p4hl6

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

Copy link
Member

@dylandreimerink dylandreimerink left a comment

Choose a reason for hiding this comment

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

LGTM

@YutaroHayakawa
Copy link
Member Author

@YutaroHayakawa
Copy link
Member Author

/test-1.16-4.9

@YutaroHayakawa
Copy link
Member Author

Multicluster / Cluster mesh hit this #23045.

@YutaroHayakawa
Copy link
Member Author

/ci-multicluster

@YutaroHayakawa YutaroHayakawa added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 12, 2023
@ti-mo ti-mo merged commit 1fec8eb into cilium:master Jan 12, 2023
@pchaigno
Copy link
Member

@YutaroHayakawa Please reopen the flake issue in such cases.

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. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants