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

clustermesh: Add an infrastructure for connect time parameter exchange and capability negotiation #22553

Conversation

YutaroHayakawa
Copy link
Member

Add a new kvstore object CiliumClusterConfig with path cilium/cluster-config/<cluster name>. The object is created per cluster by clustermesh-apiserver and obtained by cilium-agent at start-up time. The use cases of the objects are

Getting per-cluster parameter
ClusterMesh with overlapping PodCIDR support needs this for getting ClusterID on connect time and setup kvstore observers to annotate incoming objects with remote ClusterID. We can put such parameters to CiliumClusterConfig.

Capability negotiation and configuration validation
When there's a capability mismatch or configuration mismatch between clusters, we may not be able to connect those clusters correctly. Currently, we don't have any mechanism to check such a mismatch. With CiliumClusterConfig, we can check that on connect time. Currently, we only check ClusterID duplication but can add any capabilities/configuration in the future.

clustermesh: Add an infrastructure to connect time parameter exchange and capability negotiation

@YutaroHayakawa YutaroHayakawa added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. area/clustermesh Relates to multi-cluster routing functionality in Cilium. sig/kvstore Impacts the KVStore package interactions. labels Dec 5, 2022
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Dec 5, 2022
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/clustermesh-overlapping-podcidr-oss-cluster-config branch from 49c7848 to 7292a6f Compare December 6, 2022 03:26
@YutaroHayakawa YutaroHayakawa marked this pull request as ready for review December 6, 2022 03:50
@YutaroHayakawa YutaroHayakawa requested review from a team as code owners December 6, 2022 03:51
@YutaroHayakawa YutaroHayakawa removed the kind/community-contribution This was a contribution made by a community member. label Dec 6, 2022
@YutaroHayakawa YutaroHayakawa changed the title clustermesh: Add an infrastructure to connect time parameter exchange and capability negotiation clustermesh: Add an infrastructure for connect time parameter exchange and capability negotiation Dec 6, 2022
@YutaroHayakawa YutaroHayakawa marked this pull request as draft December 6, 2022 05:34
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/clustermesh-overlapping-podcidr-oss-cluster-config branch 2 times, most recently from 7692323 to eff723a Compare December 6, 2022 06:04
@YutaroHayakawa YutaroHayakawa marked this pull request as ready for review December 6, 2022 06:29
pkg/clustermesh/clustermesh.go Outdated Show resolved Hide resolved
pkg/clustermesh/remote_cluster.go Outdated Show resolved Hide resolved
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/clustermesh-overlapping-podcidr-oss-cluster-config branch 3 times, most recently from bc48cb1 to 4c75c5b Compare December 8, 2022 16:46
Copy link
Member

@aanm aanm 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 just a heads-up that since we have branched v1.13, this PR will not be part of the 1.13 unless it is set with the needs-backport/1.13 label.

@YutaroHayakawa
Copy link
Member Author

Ok, let me put it. Thanks for the heads up.

@YutaroHayakawa YutaroHayakawa added the needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch label Dec 9, 2022
@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa
Copy link
Member Author

YutaroHayakawa commented Dec 12, 2022

Failed tests
ci-l4lb: cilium-config-agent issue, need rebasing
ci-aks: cilium-config-agent issue, need rebasing
ci-eks: cilium-config-agent issue, need rebasing
ci-gke: #21652 (GKE IP exhaustion)
ci-multicluster: #21652 (GKE IP exhaustion)

Add a new type CiliumClusterConfig which represents a cluster
configuration. This will be serialized and stored into kvstore during
the clustermesh-apiserver startup time. Later on, cilium-agent on each
node reads it when connecting to new clusters.

The current use case of this is getting ClusterID at connect time, but by
exposing the cluster configuration, we can also do some useful validation
such as

- Make sure the cluster id is not conflicting with existing clusters.
- Make sure the new cluster doesn't have any capability mismatch.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Add hepler functions to set/get cluster information on kvstore.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
Implement a basic connect-time validation using CiliumClusterConfig.
clustermesh-apiserver is modified to set local CiliumClusterConfig on
start-up time and cilium-agent is modified to get CiliumClusterConfig
of remote clusters and validates it.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
For compatibility with an old Cilium that doesn't support cluster
configuration feature, we should be able to connect to the remote
cluster even if the configuration is missing. Test it.

Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
@YutaroHayakawa YutaroHayakawa force-pushed the yutaro/clustermesh-overlapping-podcidr-oss-cluster-config branch from 4c75c5b to d95f03f Compare December 12, 2022 08:03
@YutaroHayakawa
Copy link
Member Author

/test

@YutaroHayakawa
Copy link
Member Author

Failed tests
ci-aks: Seems like a new flake. Filed an issue #22681. See if retrying help.

@YutaroHayakawa
Copy link
Member Author

/ci-aks

@YutaroHayakawa
Copy link
Member Author

YutaroHayakawa commented Dec 12, 2022

All tests have passed. Making this ready-to-merge.

@YutaroHayakawa YutaroHayakawa added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Dec 12, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.13.0-rc4 Dec 12, 2022
@pchaigno pchaigno merged commit 398cf5e into cilium:master Dec 12, 2022
@pchaigno
Copy link
Member

Should @aanm become a member of @cilium/sig-clustermesh?

@joestringer joestringer 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 Dec 21, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.13 in 1.13.0-rc4 Dec 21, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.13 in 1.13.0-rc4 Dec 21, 2022
@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 Dec 21, 2022
@joestringer joestringer added backport-done/1.13 The backport for Cilium 1.13.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. labels Dec 22, 2022
@joestringer joestringer moved this from Backport pending to v1.13 to Backport done to v1.10 in 1.13.0-rc4 Dec 22, 2022
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. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/kvstore Impacts the KVStore package interactions.
Projects
No open projects
1.13.0-rc4
Backport done to v1.13
Development

Successfully merging this pull request may close these issues.

None yet

4 participants