[prometheus-modbus-exporter] fix config-reloader-sidecar deployment#6875
Open
stefan-kolev wants to merge 1 commit into
Open
Conversation
The config-reloader-sidecar has never deployed for any chart user
because of two compounding bugs:
1. templates/deployment.yaml:57 reads `.Values.configReloaderSidecar.enable`
while values.yaml provides `.enabled` (with a 'd'). Helm silently
resolves the missing key to nil, so the sidecar block is always
skipped, regardless of the user's value.
2. The config-reloader-sidecar image runs as root (it sends signals to
the modbus_exporter process via the shared PID namespace). With
the chart's default `podSecurityContext.runAsNonRoot: true`, kubelet
refuses to start the container with:
"container has runAsNonRoot and image will run as root"
The chart provided no per-container securityContext override for
the sidecar, so even if the typo is fixed the sidecar would
CrashLoop on the chart defaults.
Changes:
* deployment.yaml: rename `.enable` -> `.enabled` to match values.yaml,
add `imagePullPolicy` (was missing) and a `securityContext` block
templated from `configReloaderSidecar.securityContext`.
* values.yaml: add a default `configReloaderSidecar.securityContext`
with `runAsNonRoot: false`, `runAsUser: 0` so the sidecar starts
on default chart settings, with a comment explaining why.
* Chart.yaml: bump version 0.1.4 -> 0.1.5.
Verified live: with these changes, both containers come up Ready;
the modbus_exporter pod stays non-root (container-level
`securityContext.runAsNonRoot: true` is unchanged) while the sidecar
gets the explicit override it needs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
955a18c to
017bf2d
Compare
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.
Summary
The
config-reloader-sidecarhas never actually deployed for any chart user since it was added, due to two compounding bugs:Typo in template gate.
templates/deployment.yaml:57reads.Values.configReloaderSidecar.enable, butvalues.yamlprovides.enabled(with a 'd'). Helm silently resolves the missing key to nil, so the sidecar block is always skipped — even with the chart's own default value ofenabled: true.Missing securityContext override on the sidecar. The
openenergyprojects/config-reloader-sidecarimage runs as root in order to send signals to themodbus_exporterprocess via the shared PID namespace. The chart's defaultpodSecurityContext.runAsNonRoot: truethen causes kubelet to reject the container with:The chart provided no per-container
securityContextoverride for the sidecar, so even if the typo were fixed in isolation, the sidecar wouldCreateContainerConfigErroron default chart values.Changes
templates/deployment.yaml.Values.configReloaderSidecar.enable→.enabledto matchvalues.yaml.imagePullPolicy(was previously omitted).securityContextblock, sourced fromconfigReloaderSidecar.securityContext.values.yaml: provide a defaultconfigReloaderSidecar.securityContextwithrunAsNonRoot: falseandrunAsUser: 0, with a comment explaining why the override is necessary.Chart.yaml: bump chart version0.1.4→0.1.5.The main
modbus_exportercontainer's container-levelsecurityContext.runAsNonRoot: trueis unchanged — only the sidecar gets the explicit root override it needs.Test plan
Verified live on a Kubernetes cluster running the chart:
helm lintpasses.helm templaterenders both containers, with the sidecar block correctly conditional on.enabled.helm upgradeof an existing release: bothmodbus_exporterandconfig-reloader-sidecarcontainers come up Ready (1/1 → 2/2).modbus_exporterconfig without a pod rollout (which is the whole reason the sidecar exists).0.1.4,helm templateemits no sidecar at all, and explicitly settingconfigReloaderSidecar.enable: true(workaround) makes the sidecar appear but it then fails withCreateContainerConfigError: container has runAsNonRoot and image will run as root.