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

Minor helm improvements concerning clustermesh #28763

Merged
merged 2 commits into from
Oct 30, 2023

Conversation

giorio94
Copy link
Member

  • Add validation to explicit clustermesh-apiserver incompatibilities with Cilium running in kvstore mode
  • Allow to create the clustermesh configuration also when clustermesh-apiserver is disabled
Improve helm validation for clustermesh, and allow creating the clustermesh configuration also in kvstore mode

The clustermesh-apiserver cannot be used in combination with Cilium
running in kvstore mode if either identity allocation is set to
kvstore mode or the CiliumEndpoint CRD is disabled. In these cases,
clustermesh can be enabled directly setting the parameters to access
the local kvstore in the remote clustermesh configuration.
Similarly, also external workloads support cannot be enabled in
such configurations. Let's make these constraints more explicit by
adding dedicated validation through Helm.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Modify the helm chart to create the cilium-clustermesh secret (i.e.,
the list of remote clusters to connect to) if the corresponding enable
flag is set, independently of whether the clustermesh-apiserver is
enabled or not. This allows using the same helm settings also in case
Cilium is configured in kvstore mode, and agents can directly connect
to the remote etcd clusters. In this case, it is users' responsibility
to specify the appropriate key, certificate and certificate authority
for each remote etcd cluster.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
@giorio94 giorio94 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. area/helm Impacts helm charts and user deployment experience labels Oct 24, 2023
@giorio94 giorio94 requested review from a team as code owners October 24, 2023 15:00
@giorio94 giorio94 requested review from joamaki, squeed, a team and thorn3r and removed request for a team October 24, 2023 15:00
@giorio94
Copy link
Member Author

/test

Copy link
Contributor

@squeed squeed left a comment

Choose a reason for hiding this comment

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

edit: nevermind, mis-read the PR

Copy link
Contributor

@thorn3r thorn3r left a comment

Choose a reason for hiding this comment

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

Nice 1-up for UX

@maintainer-s-little-helper 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 Oct 30, 2023
@aditighag aditighag merged commit 31bbc8f into cilium:main Oct 30, 2023
62 of 63 checks passed
giorio94 added a commit to giorio94/cilium-cli that referenced this pull request May 10, 2024
cilium/cilium#28763 decoupled the helm settings to enable the
clustermesh-apiserver and provide the list of clusters to connect to.
Let's reflect this change to the 'cilium clustermesh disable' command
as well, explicitly disabling and resetting the remote clusters configs
when invoked, to correctly disconnect from possible leftover clusters.

Fixes: cilium/cilium#32300
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
michi-covalent pushed a commit to cilium/cilium-cli that referenced this pull request May 15, 2024
cilium/cilium#28763 decoupled the helm settings to enable the
clustermesh-apiserver and provide the list of clusters to connect to.
Let's reflect this change to the 'cilium clustermesh disable' command
as well, explicitly disabling and resetting the remote clusters configs
when invoked, to correctly disconnect from possible leftover clusters.

Fixes: cilium/cilium#32300
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
michi-covalent pushed a commit that referenced this pull request Jul 16, 2024
#28763 decoupled the helm settings to enable the
clustermesh-apiserver and provide the list of clusters to connect to.
Let's reflect this change to the 'cilium clustermesh disable' command
as well, explicitly disabling and resetting the remote clusters configs
when invoked, to correctly disconnect from possible leftover clusters.

Fixes: #32300
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
michi-covalent pushed a commit that referenced this pull request Aug 5, 2024
[ cherry-picked from cilium/cilium-cli repository ]

#28763 decoupled the helm settings to enable the
clustermesh-apiserver and provide the list of clusters to connect to.
Let's reflect this change to the 'cilium clustermesh disable' command
as well, explicitly disabling and resetting the remote clusters configs
when invoked, to correctly disconnect from possible leftover clusters.

Fixes: #32300
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
github-merge-queue bot pushed a commit that referenced this pull request Aug 16, 2024
[ cherry-picked from cilium/cilium-cli repository ]

#28763 decoupled the helm settings to enable the
clustermesh-apiserver and provide the list of clusters to connect to.
Let's reflect this change to the 'cilium clustermesh disable' command
as well, explicitly disabling and resetting the remote clusters configs
when invoked, to correctly disconnect from possible leftover clusters.

Fixes: #32300
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
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. area/helm Impacts helm charts and user deployment experience ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants