[DrFaust92]

The checksum/* pod annotations on the operator Deployment hash the entire rendered ConfigMap, and the manifest's metadata.labels include helm.sh/chart. That value changes on every chart version bump, so the operator pod is restarted on every helm upgrade even when none of the config files changed.

This adds an altinity-clickhouse-operator.configMapContentHash helper that hashes only the data/binaryData sections, emits it from dev/generate_helm_chart.sh, and regenerates Deployment-clickhouse-operator.yaml — all nine annotations (files, confd-files, configd-files, templatesd-files, usersd-files and the four keeper-* ones).

Running the generator before and after the change produces a diff limited to those nine lines. Rendered against a bumped chart version the checksums are unchanged, and adding a file to configs.configdFiles still changes the matching checksum. helm lint passes.

No chart version bump here, since the chart version tracks the operator release.

The same fix was recently made in the argo-cd chart (argoproj/argo-helm#4044); the loki chart uses the same data-only hashing.

[sunsingerus]

Thanks for this — the diagnosis is right, the helper is well built, and updating dev/generate_helm_chart.sh alongside the generated template is exactly the convention here. I reproduced your result: at chart 0.27.4 → 0.27.5 with identical config, all 9 checksums move today and none move with your patch. Regenerating with your generator change reproduces the committed Deployment-clickhouse-operator.yaml byte-for-byte, and helm lint / helm template / dev/test_helm_chart.sh (21/21) all pass. I also checked the failure mode I was most worried about — a silent fromYaml failure freezing the hash — and it cannot ship: malformed YAML aborts the whole render before any checksum is computed, and a bad template path hard-fails on include.

Two things before this can land.

1. The pod still restarts — the chart version is also in the pod template labels

templates/generated/Deployment-clickhouse-operator.yaml:25 renders the pod template labels through the full labels helper, which emits helm.sh/chart and app.kubernetes.io/version:

  template:
    metadata:
      labels:
        helm.sh/chart: altinity-clickhouse-operator-0.27.4     # changes on every chart bump
        app.kubernetes.io/name: altinity-clickhouse-operator
        app.kubernetes.io/instance: t
        app.kubernetes.io/version: "0.27.4"                    # changes on every appVersion bump
        app.kubernetes.io/managed-by: Helm

Those live inside spec.template.metadata.labels, so they feed pod-template-hash and roll the Deployment on their own. Rendering 0.27.4 vs 0.27.5 with your patch applied and the image tag pinned, the 9 checksums are now identical — but the render still differs by 4 lines (helm.sh/chart and app.kubernetes.io/version, once in metadata.labels and once in the pod template), and the operator still restarts. So the PR as it stands is necessary but not sufficient for its stated goal.

Note dev/generate_helm_chart.sh:53-57 sets .version and .appVersion from the same release file, so they always move together. With an unpinned tag the image tag changes too and a restart is intended — the churn only ever mattered for users pinning image.tag, and the labels defeat the fix for them as well.

Could you include the label fix here? The line is generator-owned, so it needs the same two-place treatment you already used for the checksums: dev/generate_helm_chart.sh:208 and the regenerated template.

Two shapes, and I'd suggest the second:

• selectorLabels + podLabels — simplest, but it also drops app.kubernetes.io/managed-by and any user-supplied commonLabels from the pods. commonLabels is a documented values knob that users reasonably expect on pods, so this trades one regression for another.
• A dedicated helper — say altinity-clickhouse-operator.podTemplateLabels — that is the labels helper minus helm.sh/chart and app.kubernetes.io/version. That removes exactly the two churning labels and keeps managed-by and commonLabels.

Either is safe with respect to the selector: spec.selector.matchLabels (line 22) uses selectorLabels only, and both shapes keep those.

2. Please add a regression guard to dev/test_helm_chart.sh

That script has no checksum coverage at all today, so it passes equally before and after your change — and it would not catch a revert to whole-manifest hashing, nor the nastier failure where the hash freezes and the operator silently stops restarting on real config changes. Since the operator reads its config once at startup and has no file watcher, a missed restart strands stale configd/usersd/templatesd config, so this behaviour is worth pinning.

The assertion is the contract itself: render at two chart versions, require the 9 checksum/* annotations to be equal, and require that a real data change still moves the matching one. Chart version is not settable via --set, so it needs a temp copy of the chart with Chart.yaml edited — roughly:

# checksum/* must be stable across a chart version bump ...
tmp=$(mktemp -d); cp -r "${chart_dir}" "${tmp}/c"
a=$(helm template t "${tmp}/c" | grep -E '^\s+checksum/')
yq e -i '.version = "99.99.99"' "${tmp}/c/Chart.yaml"
b=$(helm template t "${tmp}/c" | grep -E '^\s+checksum/')
[ "${a}" = "${b}" ] || fail "checksum/* changed on a chart version bump"

# ... but must still move when the mounted data changes
c=$(helm template t "${chart_dir}" --set 'watchNamespaces={ns1,ns2}' | grep -E '^\s+checksum/files:')
[ "${c}" != "$(printf '%s' "${a}" | grep 'checksum/files:')" ] || fail "checksum/files did not track a data change"

Smaller points, no action needed unless you want to

• Three annotations now collapse to the same value (4ca13a59…) because the confd, keeper-confd and keeper-usersd ConfigMaps render with no data keys. Correct, but it looks odd at a glance.
• fromYaml silently decodes only the first document of a multi-document file and does not report an error. None of the nine generated ConfigMaps is multi-document, so this is latent rather than a live bug, but the raw-include form did cover it — worth a line in the helper comment.
• Worth a release-note line once the label fix is in: adopting this recomputes all nine annotations once, so the operator pod restarts a single time on the upgrade that lands it. ClickHouse clusters are unaffected — only reconciliation pauses.

[DrFaust92]

Both addressed, and thanks for the thorough review — particularly for confirming the fromYaml failure mode can't ship silently, which I hadn't checked.

1. The chart version in the pod template labels

You were right that the PR was necessary but not sufficient. Reproduced your measurement first — pod template across a chart+appVersion bump with image tags pinned:

-        helm.sh/chart: altinity-clickhouse-operator-0.27.3
+        helm.sh/chart: altinity-clickhouse-operator-99.99.99
-        app.kubernetes.io/version: "0.27.3"
+        app.kubernetes.io/version: "99.99.99"

Went with your second option, the dedicated helper, for the reason you gave — selectorLabels + podLabels would have dropped managed-by and user commonLabels, trading one regression for another. altinity-clickhouse-operator.podTemplateLabels is the labels helper minus helm.sh/chart and app.kubernetes.io/version.

Two-place treatment as you asked: dev/generate_helm_chart.sh:208 and the regenerated Deployment.

After:

	result
pod template across a version bump	identical
pod labels	name, instance, managed-by
commonLabels.team=obs	reaches the pod template
podLabels.extra=yes	still applied
spec.selector.matchLabels	unchanged
object labels	still carry helm.sh/chart and app.kubernetes.io/version

A note on regenerating: running the generator with my local yq/jq also reformatted the four CRDs and stripped trailing newlines from Chart.yaml, ServiceAccount and ClusterRoleBinding — ~330 lines of unrelated churn. I reverted all of it and kept only the single intended line in the Deployment, so the diff stays reviewable. Worth knowing the generator isn't reproducible across tool versions.

2. Regression guard in dev/test_helm_chart.sh

Added, using your approach of a temp chart copy since the chart version isn't settable via --set. Two assertions:

• pod template is unchanged by a version bump — byte-compares the template across a chart+appVersion bump with image tags pinned.
• checksum/files tracks a data change — the other half, so a frozen hash can't pass silently.

dev/test_helm_chart.sh --skip-docs → PASSED 16 checks.

I verified both are real guards rather than assuming:

• Reverting the pod-label fix → FAIL pod template is unchanged by a version bump.
• Reverting the content-hash helper to whole-manifest hashing → the same assertion fails, reporting the moved checksum/files value. So the one check covers both halves of the fix, which is what you wanted pinned given the operator reads its config once at startup.

3. Smaller points

Added your fromYaml note to the helper comment — it decodes only the first document of a multi-document file without erroring, latent here since none of the nine generated ConfigMaps is multi-document.

The three identical 4ca13a59… values and the one-time restart on the upgrade that lands this are both expected; happy to add the release-note line wherever you'd like it.

[DrFaust92]

Run Tests didn't fail on anything in this change — the runner was reclaimed mid-run:

2026-10-23T09:01:05Z ##[error]The runner has received a shutdown signal. This can happen when
                     the runner service is stopped, or a manually started runner is canceled.
2026-10-23T09:01:05Z ##[error]The operation was canceled.

It died at 27m56s during minikube image load, before any test executed — the last real log line is done: altinity/clickhouse-backup:latest-fips. Go unit tests (race) and Test Helm Chart both passed, and Test Helm Chart is the job that actually covers this change.

Nothing to rebase: against the base branch the branch is ahead_by: 2, behind_by: 0, and the diff is still just the four intended files:

deploy/helm/clickhouse-operator/templates/_helpers.tpl                      +31/-0
deploy/helm/clickhouse-operator/templates/generated/Deployment-...yaml      +10/-10
dev/generate_helm_chart.sh                                                   +2/-2
dev/test_helm_chart.sh                                                      +32/-0

Could someone with write access re-run the failed job? gh run rerun is admin-only, so I can't trigger it, and I'd rather not push a no-op commit just to kick CI while the review is open.