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

Implement commands for listing per-cluster CT/SNAT maps #24629

Conversation

YutaroHayakawa
Copy link
Member

Please see individual commits for more details.

Implement commands for listing per-cluster CT/SNAT maps

@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. labels Mar 29, 2023
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/oss/clustermesh-overlapping-podcidr/per-cluster-maps-list branch from 1628142 to 1c7119f Compare March 29, 2023 13:18
@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/oss/clustermesh-overlapping-podcidr/per-cluster-maps-list branch 3 times, most recently from cd2906e to b7ffc59 Compare March 30, 2023 06:32
@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/oss/clustermesh-overlapping-podcidr/per-cluster-maps-list branch from b7ffc59 to 4aef80e Compare April 1, 2023 07:39
@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa YutaroHayakawa marked this pull request as ready for review April 1, 2023 15:25
@YutaroHayakawa YutaroHayakawa requested review from a team as code owners April 1, 2023 15:25
Copy link
Member

@nathanjsweet nathanjsweet left a comment

Choose a reason for hiding this comment

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

nit and validation concern.

cilium/cmd/bpf_ct_list.go Outdated Show resolved Hide resolved
cilium/cmd/bpf_ct_list.go Show resolved Hide resolved
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/oss/clustermesh-overlapping-podcidr/per-cluster-maps-list branch from 4aef80e to 6b98f8f Compare April 7, 2023 05:55
@YutaroHayakawa
Copy link
Member Author

Reflected on Nate's comment. And sorry, I added a commit that implements the missing flag string for from_tunnel (c.f. #24212 (comment)).

@YutaroHayakawa
Copy link
Member Author

YutaroHayakawa commented Apr 7, 2023

Smoke test, Smoke Test with IPv6: #23812

@YutaroHayakawa
Copy link
Member Author

/conformance-test

@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa
Copy link
Member Author

/conformance-test-ipv6

@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/oss/clustermesh-overlapping-podcidr/per-cluster-maps-list branch from 6b98f8f to 410fdc3 Compare April 7, 2023 16:17
@YutaroHayakawa
Copy link
Member Author

YutaroHayakawa commented Apr 7, 2023

/test

Job 'Cilium-PR-K8s-1.25-kernel-4.19' failed:

Click to show.

Test Name

K8sAgentPolicyTest Multi-node policy test with L7 policy using connectivity-check to check datapath

Failure Output

FAIL: cannot install connectivity-check

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.25-kernel-4.19/1680/

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

@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/oss/clustermesh-overlapping-podcidr/per-cluster-maps-list branch from 410fdc3 to 009a73d Compare April 11, 2023 02:00
@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa
Copy link
Member Author

Cilium Datapath is broken by cilium-cli change (cilium/cilium-cli#1498) and now fixed by 0a984a7. I think we don't have to re-run datapath test as it is not related to this change.

@YutaroHayakawa YutaroHayakawa added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. and removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Apr 11, 2023
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Introduce a new method GetClusterCTMaps which gets all CT maps for
specific cluster.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
dumpAndRead takes dump function and argument (args... interface{}) and
calls dump function with args without expand the args slice. Fix it to
expand slice.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
In the current `cilium bpf ct list`, we specify the instance of the CT
maps to dump by reserved word "global" or numeric id of the endpoints.
This means we recognize the "type" of the CT maps by checking if the
first argument is string or int and now we're trying to introduce
another type for per-cluster maps.

It is technically possible to introduce compatible command fixture like
follows, but the parsing is unnecessarily complicated.

Global   : cilium bpf ct list global
Enpoints : cilium bpf ct list 12345
Cluster  : cilium bpf ct list cluster 1

Instead, we can explicitly specify the type by the first argument.

Global   : cilium bpf ct list global
Enpoints : cilium bpf ct list endpoint 12345
Cluster  : cilium bpf ct list cluster 1

This breaks the existing command for per-endpoint CT maps, but we
consider this as an acceptable change since it doesn't change output of
the command and cilium command fixture is considered as an cilium
internal things and not a user interface we maintain.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Introduce a new command `cilium bpf ct list cluster <cluster id>` which
dumps per-cluster CT maps.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Introduce a new command `cilium bpf nat list cluster <cluster id>` which
dumps per-cluster NAT maps.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Handle from_tunnel in flagString. So that we can show from_tunnel value
to CLI output.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/oss/clustermesh-overlapping-podcidr/per-cluster-maps-list branch from 009a73d to 3630bbe Compare April 12, 2023 08:56
@YutaroHayakawa
Copy link
Member Author

Rebasing on the master and re-running the CI.

@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa
Copy link
Member Author

ConformanceK8sKind: #24622

@YutaroHayakawa YutaroHayakawa added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Apr 12, 2023
@dylandreimerink dylandreimerink merged commit d93bee9 into cilium:master Apr 13, 2023
57 checks passed
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

3 participants