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

kvstoremesh: don't disable by default #2660

Merged
merged 1 commit into from
Jul 6, 2024

Conversation

marseel
Copy link
Contributor

@marseel marseel commented Jul 4, 2024

In 1.16 we enabled kvstoremesh by default in helm. Currently, cilium-cli was still setting kvstoremesh.enabled to false. Now, if user does not explicitly specify if kvstoremesh should be enabled or disabled, we will rely on default helm value.

In 1.16 we enabled kvstoremesh by default in helm.
Currently, cilium-cli was still setting kvstoremesh.enabled to false.
Now, if user does not explicitly specify if kvstoremesh should be
enabled or disabled, we will rely on default helm value.

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
@marseel marseel requested a review from giorio94 July 5, 2024 09:53
@marseel marseel marked this pull request as ready for review July 5, 2024 09:53
@marseel marseel requested review from a team as code owners July 5, 2024 09:53
@marseel marseel requested a review from squeed July 5, 2024 09:53
Copy link
Member

@giorio94 giorio94 left a comment

Choose a reason for hiding this comment

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

Thanks!

@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 Jul 5, 2024
@michi-covalent michi-covalent merged commit 5a87831 into cilium:main Jul 6, 2024
13 checks passed
marseel added a commit to marseel/cilium that referenced this pull request Jul 10, 2024
With KVStoreMesh enabled by default, we rely on having cluster id.
Currently, external-workloads did not have it set in CI, which was
causing CI failure when trying to update cilium-cli containing change
from cilium/cilium-cli#2660

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
github-merge-queue bot pushed a commit to cilium/cilium that referenced this pull request Jul 10, 2024
With KVStoreMesh enabled by default, we rely on having cluster id.
Currently, external-workloads did not have it set in CI, which was
causing CI failure when trying to update cilium-cli containing change
from cilium/cilium-cli#2660

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
giorio94 pushed a commit to cilium/cilium that referenced this pull request Jul 15, 2024
[ upstream commit 2c7df4f ]

With KVStoreMesh enabled by default, we rely on having cluster id.
Currently, external-workloads did not have it set in CI, which was
causing CI failure when trying to update cilium-cli containing change
from cilium/cilium-cli#2660

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
giorio94 pushed a commit to cilium/cilium that referenced this pull request Jul 15, 2024
[ upstream commit 2c7df4f ]

With KVStoreMesh enabled by default, we rely on having cluster id.
Currently, external-workloads did not have it set in CI, which was
causing CI failure when trying to update cilium-cli containing change
from cilium/cilium-cli#2660

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
aanm pushed a commit to cilium/cilium that referenced this pull request Jul 15, 2024
[ upstream commit 2c7df4f ]

With KVStoreMesh enabled by default, we rely on having cluster id.
Currently, external-workloads did not have it set in CI, which was
causing CI failure when trying to update cilium-cli containing change
from cilium/cilium-cli#2660

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
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
ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants