operator/clustermesh: add endpoint slice synchronization #28440

MrFreezeex · 2023-10-06T11:01:18Z

Please ensure your pull request adheres to the following guidelines:

For first time contributors, read Submitting a pull request
All code is covered by unit and/or runtime tests where feasible.
All commits contain a well written commit description including a title,
description and a Fixes: #XXX line if the commit addresses a particular
GitHub issue.
If your commit description contains a Fixes: <commit-id> tag, then
please add the commit author[s] as reviewer[s] to this issue.
All commits are signed off. See the section Developer’s Certificate of Origin
Provide a title or release-note blurb suitable for the release notes.
Are you a user of Cilium? Please add yourself to the Users doc
Thanks for contributing!

This commit adds a new clustermesh module inside the operator which pull data from Cilium's clustermesh components and synchronize Endpoint Slices from remote cluster. See commits description for more details.

Related to: #27902

Add Kubernetes EndpointSlice synchronization from Cilium clustermesh

MrFreezeex · 2023-10-06T11:04:59Z

Note that I'm creating this PR early but there are many things to iron out like helm chart integration, probably integration tests, documentation. I have not really tested it so far, I can just say that the reconciler unit tests succeed and the code compile. Will try to address those ~ next week or so.

MrFreezeex · 2023-10-22T17:48:19Z

It seems to work fairly well locally:

❯ k get endpointslices.discovery.k8s.io --context kind-clustermesh2
NAME                            ADDRESSTYPE   PORTS   ENDPOINTS               AGE
kubernetes                      IPv4          6443    172.20.0.5              20h
rebel-base-2sjcn                IPv4          80      10.2.0.196,10.2.1.175   20h
rebel-base-clustermesh1-6lwsj   IPv4          80      10.1.0.122,10.1.1.202   11m
❯ k get endpointslices.discovery.k8s.io --context kind-clustermesh1
NAME                            ADDRESSTYPE   PORTS   ENDPOINTS               AGE
kubernetes                      IPv4          6443    172.20.0.3              20h
rebel-base-7tllq                IPv4          80      10.1.1.202,10.1.0.122   20h
rebel-base-clustermesh2-86vjj   IPv4          80      10.2.1.175,10.2.0.196   11m

I was hoping to add an integration tests but IIUC there is no test for clustermesh so it would be probably a bit harder but I can check if needed. I could add mcs-api conformance test which covers endpointslice mirroring from remote cluster as well apparently. However those need Cilium support for ServiceImport/Export so that should probably be done in a future PR I think 🤔.

examples/kubernetes/clustermesh/global-service-example/cluster1.yaml

pippolo84 · 2024-03-21T11:10:16Z

Hi @pippolo84 many thanks for the review! :D

I updated a bit the last commit do you think you could take another quick look on it if it looks all sane to you 🙏? I was a bit updating this at the same time you reviewed 😅.

It's linked to the suggestion you made about headless service exclusion. There was a bug on the label selector for instance, like I assumed the label selector were merged with the functionnal pattern but not at all, it's overriding the previous value. So now I have to parse the labels and add the headless exclusion and also did some other small fixes on this commit.

Sure, I re-requested a review to myself, I'll take a look ASAP.

MrFreezeex · 2024-03-21T18:35:56Z

/test

pippolo84

This looks cleaner, thanks! 🚀
I like that we are not changing the operator and clustermesh-apiserver resources anymore, just customizing the agent ones as needed. 👍

I've just left a comment regarding a unit test that I'm afraid might become flaky. Let me know what you think.

pkg/k8s/utils/utils_test.go

pippolo84

Approving one more time after latest changes. 🥇

Thanks again!

marseel

Looks good!
I left two small non-blocking nits.

install/kubernetes/cilium/templates/cilium-operator/deployment.yaml

pkg/clustermesh/endpointslicesync/clustermesh.go

This commit adds a new clustermesh module inside the operator which pull data from Cilium's clustermesh components and unlike the Cilium agent will act on the whole cluster level instead of the node level. This adds optional endpoint slice synchronization from remote clusters. Having endpoint slices of others clusters is useful for non Cilium controllers that would need this information (for instance CoreDNS, non Cilium ingress controller). This is also a base to support the endpoint slice synchronization from Multi-Cluster Services API (KEP-1645) and it is actually already using a mcs-api label to identify the source cluster. The endpoint slice synchronization is based on existing upstream Kubernetes endpoint slice controller/reconciler but adapted. Thanks to this we do not have to write ourselves some of the extensive logis to split the EndpointSlices, reuse them, handle dual stack and all the extensive logic there... As we don't deal with Pods there is some logic to "mock" the pod informer passed to the controller and based on the global services info in the cluster mesh return "fake" pods. There is also some similar mocking done to service informer and the client to create EndpointSlice as there should be one set of EndpointSlice per cluster. There is also a fake node informer that essentially return a fake "node" per cluster. The endpoint slice synchronization is guarded by a new annotation `cilium.io/global-sync-endpoint-slices` as enabling it on global services with high churn may be a scalability concern on all the etcd and/or kube-apiserver of all the Kubernetes cluster which involve the same global services with the endpoint slice sync enabled. Note that, the upstream controller is already optimized to do a low number of requests to kube-apiserver to alleviate some of the scalability concerns. Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>

- Allow operator write access to endpointslices - Allow operator to write events as the Kubernetes controller write some in case of errors - Add similar configuration as the cilium agent regarding clustermesh Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>

This commit allow Cilium watchers to ignore EndpointSlices mirrored by the Cilium clustermesh EndpointSlice controller. We do not need to watch those EndpointSlices as Cilium has special datapath logic for those already. Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>

Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>

Now that we synchronize EndpointSlice we also need to synchronize headless services so that we can synchronize their EndpointSlice too. We implement this by removing the exclusion on regular Endpoints/Service resources of headless Services and adding it back inside the daemon. As a result clustermesh-api-server and the operator will now watch headless Services/Endpoints as well. Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>

aanm · 2024-04-02T14:59:51Z

/test

sayboras

Look good for helm changes

aanm

Great work 🎉

aanm · 2024-04-03T14:38:20Z

Documentation/network/clustermesh/services.rst

+By default Kubernetes EndpointSlice synchronization is disabled on Global services.
+To have Cilium discover remote clusters endpoints of a Global Service
+from DNS or any third party controllers, enable synchronization by adding
+the annotation ``service.cilium.io/global-sync-endpoint-slices: "true"``.


I will not block the PR on this. @MrFreezeex can we have a follow up PR with the followin sentence:

the annotation ``service.cilium.io/global-sync-endpoint-slices: "true"``. This will allow Cilium to create k8s endpoint slices that belong a remote cluster for services that have that annotation.

Sure 👍, I will try to include that change in a separate commit in one of my followup PRs that I intend to do to address the hostname sync limitation.

MrFreezeex · 2024-04-03T16:12:35Z

Huge thanks to all the people involved here ❤️. Many of you provided super insightful reviews that made the code a lot better, wrote encouraging messages or even provided direct help!

Now I am excited to to have other users trying this, annnd see some of you around for the next followups/related PRs :D.

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 Oct 6, 2023

github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Oct 6, 2023

MrFreezeex force-pushed the add-sync-endpoints branch from 21b8b27 to 8597a35 Compare October 7, 2023 16:09

MrFreezeex force-pushed the add-sync-endpoints branch 12 times, most recently from 63f61d4 to 009bd60 Compare October 22, 2023 17:46

MrFreezeex commented Oct 22, 2023

View reviewed changes

examples/kubernetes/clustermesh/global-service-example/cluster1.yaml Show resolved Hide resolved

MrFreezeex force-pushed the add-sync-endpoints branch from 229ef8d to ea327b0 Compare October 22, 2023 18:30

MrFreezeex marked this pull request as ready for review October 22, 2023 19:00

MrFreezeex requested review from a team as code owners October 22, 2023 19:00

MrFreezeex requested review from sayboras, pippolo84 and qmonnet October 22, 2023 19:00

MrFreezeex force-pushed the add-sync-endpoints branch from ea327b0 to 8ae4747 Compare October 22, 2023 19:47

MrFreezeex force-pushed the add-sync-endpoints branch from 142c20f to 04037b8 Compare March 21, 2024 18:21

pippolo84 reviewed Mar 21, 2024

View reviewed changes

pkg/k8s/utils/utils_test.go Outdated Show resolved Hide resolved

MrFreezeex force-pushed the add-sync-endpoints branch 3 times, most recently from 43bdd4f to ba3c01b Compare March 21, 2024 20:00

giorio94 mentioned this pull request Mar 22, 2024

Test endpoint slice synchronization as part of the Conformance Cluster Mesh workflow #31551

Merged

pippolo84 approved these changes Mar 22, 2024

View reviewed changes

marseel approved these changes Apr 2, 2024

View reviewed changes

install/kubernetes/cilium/templates/cilium-operator/deployment.yaml Outdated Show resolved Hide resolved

pkg/clustermesh/endpointslicesync/clustermesh.go Outdated Show resolved Hide resolved

MrFreezeex force-pushed the add-sync-endpoints branch 2 times, most recently from b1fd0df to 64c3361 Compare April 2, 2024 11:10

MrFreezeex added 5 commits April 2, 2024 13:27

docs: explain endpoint slice synchronization and add an example

e0f0493

Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>

MrFreezeex force-pushed the add-sync-endpoints branch from 64c3361 to 944f7ba Compare April 2, 2024 11:28

sayboras approved these changes Apr 3, 2024

View reviewed changes

aanm approved these changes Apr 3, 2024

View reviewed changes

aanm enabled auto-merge April 3, 2024 14:38

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 Apr 3, 2024

aanm added this pull request to the merge queue Apr 3, 2024

Merged via the queue into cilium:main with commit 99268e7 Apr 3, 2024
61 of 62 checks passed

aanm added release-note/major This PR introduces major new functionality to Cilium. and removed release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Apr 3, 2024

giorio94 mentioned this pull request May 9, 2024

Operator: expose remote clusters information through dedicated CLI command, and introduce troubleshoot commands #32436

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

operator/clustermesh: add endpoint slice synchronization #28440

operator/clustermesh: add endpoint slice synchronization #28440

MrFreezeex commented Oct 6, 2023 •

edited

Loading

MrFreezeex commented Oct 6, 2023 •

edited

Loading

MrFreezeex commented Oct 22, 2023 •

edited

Loading

pippolo84 commented Mar 21, 2024

MrFreezeex commented Mar 21, 2024

pippolo84 left a comment

pippolo84 left a comment

marseel left a comment

aanm commented Apr 2, 2024

sayboras left a comment

aanm left a comment

aanm Apr 3, 2024

MrFreezeex Apr 3, 2024 •

edited

Loading

MrFreezeex commented Apr 3, 2024 •

edited

Loading

operator/clustermesh: add endpoint slice synchronization #28440

operator/clustermesh: add endpoint slice synchronization #28440

Conversation

MrFreezeex commented Oct 6, 2023 • edited Loading

MrFreezeex commented Oct 6, 2023 • edited Loading

MrFreezeex commented Oct 22, 2023 • edited Loading

pippolo84 commented Mar 21, 2024

MrFreezeex commented Mar 21, 2024

pippolo84 left a comment

Choose a reason for hiding this comment

pippolo84 left a comment

Choose a reason for hiding this comment

marseel left a comment

Choose a reason for hiding this comment

aanm commented Apr 2, 2024

sayboras left a comment

Choose a reason for hiding this comment

aanm left a comment

Choose a reason for hiding this comment

aanm Apr 3, 2024

Choose a reason for hiding this comment

MrFreezeex Apr 3, 2024 • edited Loading

Choose a reason for hiding this comment

MrFreezeex commented Apr 3, 2024 • edited Loading

MrFreezeex commented Oct 6, 2023 •

edited

Loading

MrFreezeex commented Oct 6, 2023 •

edited

Loading

MrFreezeex commented Oct 22, 2023 •

edited

Loading

MrFreezeex Apr 3, 2024 •

edited

Loading

MrFreezeex commented Apr 3, 2024 •

edited

Loading