fix: don't render PodMonitor and HPA for disabled components - #717
Open
mouchar wants to merge 1 commit into
Open
fix: don't render PodMonitor and HPA for disabled components#717mouchar wants to merge 1 commit into
mouchar wants to merge 1 commit into
Conversation
The PodMonitor (or VMPodScrape) for proxy, broker, bookkeeper and autorecovery was guarded only by `<component>.podMonitor.enabled`, which defaults to true, so it was rendered even when the component itself was disabled via `components.*=false`. This left scrape configs whose selectors match no pods: harmless to Prometheus, but permanent drift in GitOps setups and empty targets in dashboards. `proxy-hpa.yaml` and `broker-hpa.yaml` had the same missing guard. That case is rarer, since it also requires `autoscaling.enabled=true`, but the result is worse: the HPA targets a StatefulSet that was never created, so the controller sits at ScalingActive=False / FailedGetScale and emits warning events continuously. Add the missing `components.*` condition to all six templates, keeping the existing `standalone.enabled` checks. This matches what the zookeeper, function-worker and oxia pod monitors already do, and what every other template belonging to these components already does. Fixes apache#716
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #716
Motivation
The
PodMonitor(orVMPodScrape, whenvictoria-metrics-k8s-stackis enabled) for the proxy, broker, bookkeeper and autorecovery components was guarded only by<component>.podMonitor.enabled, which defaults totrue. As a result it was rendered even when the component itself was disabled viacomponents.*=false, leaving scrape configs whose selectors match no pods — harmless to Prometheus, but permanent drift in GitOps setups and empty targets/series in dashboards. Disabling a component required disabling it twice.proxy-hpa.yamlandbroker-hpa.yamlhad the same missing guard. That case is rarer, since it also requiresautoscaling.enabled=true, but the consequence is worse: the HPA targets a StatefulSet that was never created, so the HPA controller sits atScalingActive=False/FailedGetScaleand emits warning events continuously.The zookeeper, function-worker and oxia pod monitors already check
components.*, as does every other template belonging to these four components (statefulset, service, configmap, pdb, service-account) — these six were the outliers.Modifications
Added the missing
components.*condition to six templates, keeping the existingstandalone.enabledchecks where present:proxy-podmonitor.yamland $.Values.components.proxy $.Values.proxy.podMonitor.enabledbroker-podmonitor.yamland $.Values.components.broker $.Values.broker.podMonitor.enabled (not $.Values.standalone.enabled)bookkeeper-podmonitor.yamland $.Values.components.bookkeeper $.Values.bookkeeper.podMonitor.enabled (not $.Values.standalone.enabled)autorecovery-podmonitor.yamland $.Values.components.autorecovery $.Values.autorecovery.podMonitor.enabled (not $.Values.standalone.enabled)proxy-hpa.yamland .Values.components.proxy .Values.proxy.autoscaling.enabledbroker-hpa.yamland .Values.components.broker .Values.broker.autoscaling.enabled (not .Values.standalone.enabled)The now-stale
# deploy <component> PodMonitor only when ... is truecomment above each pod monitor guard was updated to match.No new tests: the chart has no harness that can assert a resource is absent (the template checks in CI are kubeconform schema validation over rendered output), and adding one would be a larger change than the fix itself.
Verifying this change
Before the change, both of these rendered resources for components that were switched off; after it, both render nothing:
No behavior change with default values — all five pod monitors still render, and both HPAs still render with
autoscaling.enabled=true.Also verified locally:
helm lintclean; the CI "all features enabled" renders (.ci/templates-all-values.yaml, and with thepatch1overlay, at--kube-version 1.36.0) and the.ci/clusters/values-standalone.yamlscenario all render successfully.Note this is technically a behavior change for anyone who sets
components.<component>=falseand relies on the stray PodMonitor/HPA being present — unlikely, but may be worth a line in the release notes.