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

helm: don't expose kvstoremesh metrics if not enabled #28481

Merged
merged 1 commit into from Oct 16, 2023

Conversation

giorio94
Copy link
Member

Currently, the kvstoremesh metrics entry inside the dedicated Service and ServiceMonitor is always inserted if the corresponding setting is enabled (true by default), regardless of whether kvstoremesh is actually enabled or not. Yet, this would create problems when Prometheus attempts to scrape it, as the target port would be closed. Hence, let's update the helm chart so that these entries are inserted only if kvstoremesh is enabled.

Fixes: #issue-number

Fix issue causing KVStoreMesh metrics to be included in the dedicated Service/ServiceMonitor when KVStoreMesh is disabled 

Currently, the kvstoremesh metrics entry inside the dedicated
Service and ServiceMonitor is always inserted if the corresponding
setting is enabled (true by default), regardless of whether
kvstoremesh is actually enabled or not. Yet, this would create
problems when Prometheus attempts to scrape it, as the target
port would be closed. Hence, let's update the helm chart so that
these entries are inserted only if kvstoremesh is enabled.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
@giorio94 giorio94 added kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium. area/clustermesh Relates to multi-cluster routing functionality in Cilium. area/helm Impacts helm charts and user deployment experience needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Oct 10, 2023
@giorio94 giorio94 requested review from a team as code owners October 10, 2023 09:10
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.3 Oct 10, 2023
@giorio94
Copy link
Member Author

/test

@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 10, 2023
@squeed squeed merged commit e1b69e7 into cilium:main Oct 16, 2023
62 of 63 checks passed
@jrajahalme jrajahalme added this to Needs backport from main in 1.14.4 Oct 18, 2023
@jrajahalme jrajahalme removed this from Needs backport from main in 1.14.3 Oct 18, 2023
@tklauser tklauser added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Oct 24, 2023
@tklauser tklauser mentioned this pull request Oct 24, 2023
14 tasks
@tklauser tklauser added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Oct 25, 2023
@thorn3r thorn3r moved this from Needs backport from main to Backport done to v1.14 in 1.14.4 Nov 9, 2023
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 backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
1.14.4
Backport done to v1.14
Development

Successfully merging this pull request may close these issues.

None yet

5 participants