Skip to content

Hash only the ConfigMap data in the checksum/* pod annotations #2079

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.

Sign up for GitHub

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

Open
DrFaust92 wants to merge 4 commits into
base: 0.28.0
Choose a base branch
from
+86 −12
Open

Hash only the ConfigMap data in the checksum/* pod annotations #2079

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
23527a2
Hash only the ConfigMap data in the checksum/* pod annotations
DrFaust92 Sep 11, 2026
328a93a
Keep the chart version out of the pod template labels too
DrFaust92 Oct 23, 2026
e9fe7b1
Add a CHANGELOG entry for the checksum and pod label fix
Slach Oct 25, 2026
2836c6a
Make the checksum/files data-change check able to fail
Slach Oct 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions deploy/helm/clickhouse-operator/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,24 @@ app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
{{- end -}}

{{/*
Pod template labels

The common labels minus helm.sh/chart and app.kubernetes.io/version. Both of those
change on a version bump, and anything inside the pod template feeds
pod-template-hash, so leaving them there rolls the Deployment on a chart bump even
when the image and the configuration are identical. They stay on the objects
themselves. commonLabels and managed-by are kept, since a user who sets
commonLabels reasonably expects them on the pods.
*/}}
{{- define "altinity-clickhouse-operator.podTemplateLabels" -}}
{{ include "altinity-clickhouse-operator.selectorLabels" . }}
{{- if .Values.commonLabels }}
{{ toYaml .Values.commonLabels }}
{{- end }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
{{- end -}}

{{/*
Selector labels
*/}}
Expand Down Expand Up @@ -148,3 +166,16 @@ Arguments (list): root context, configs.files, watchNamespaces list
{{- end -}}
{{- include "altinity-clickhouse-operator.configmap-data" (list $root $files) -}}
{{- end -}}

{{/*
Compute a ConfigMap checksum from its data only, for the checksum/* pod annotations.
Hashing the whole manifest includes the helm.sh/chart label, which changes on every chart version
bump and would restart the operator when no configuration changed.

fromYaml decodes only the first document of a multi-document file, without reporting an error.
None of the generated ConfigMaps is multi-document, so this is latent rather than a live bug,
but it is a difference from the raw-include form this replaced.
*/}}
{{- define "altinity-clickhouse-operator.configMapContentHash" -}}
{{ pick (include (print .ctx.Template.BasePath .name) .ctx | fromYaml) "data" "binaryData" | toYaml | sha256sum }}
{{- end -}}
20 changes: 10 additions & 10 deletions deploy/helm/clickhouse-operator/templates/generated/Deployment-clickhouse-operator.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,18 @@ spec:
matchLabels: {{ include "altinity-clickhouse-operator.selectorLabels" . | nindent 6 }}
template:
metadata:
labels: {{ include "altinity-clickhouse-operator.labels" . | nindent 8 }}{{ if .Values.podLabels }}{{ toYaml .Values.podLabels | nindent 8 }}{{ end }}
labels: {{ include "altinity-clickhouse-operator.podTemplateLabels" . | nindent 8 }}{{ if .Values.podLabels }}{{ toYaml .Values.podLabels | nindent 8 }}{{ end }}
annotations:
{{ if .Values.podAnnotations }}{{ toYaml .Values.podAnnotations | nindent 8 }}{{ end }}
checksum/files: {{ include (print $.Template.BasePath "/generated/ConfigMap-etc-clickhouse-operator-files.yaml") . | sha256sum }}
checksum/confd-files: {{ include (print $.Template.BasePath "/generated/ConfigMap-etc-clickhouse-operator-confd-files.yaml") . | sha256sum }}
checksum/configd-files: {{ include (print $.Template.BasePath "/generated/ConfigMap-etc-clickhouse-operator-configd-files.yaml") . | sha256sum }}
checksum/templatesd-files: {{ include (print $.Template.BasePath "/generated/ConfigMap-etc-clickhouse-operator-templatesd-files.yaml") . | sha256sum }}
checksum/usersd-files: {{ include (print $.Template.BasePath "/generated/ConfigMap-etc-clickhouse-operator-usersd-files.yaml") . | sha256sum }}
checksum/keeper-confd-files: {{ include (print $.Template.BasePath "/generated/ConfigMap-etc-keeper-operator-confd-files.yaml") . | sha256sum }}
checksum/keeper-configd-files: {{ include (print $.Template.BasePath "/generated/ConfigMap-etc-keeper-operator-configd-files.yaml") . | sha256sum }}
checksum/keeper-templatesd-files: {{ include (print $.Template.BasePath "/generated/ConfigMap-etc-keeper-operator-templatesd-files.yaml") . | sha256sum }}
checksum/keeper-usersd-files: {{ include (print $.Template.BasePath "/generated/ConfigMap-etc-keeper-operator-usersd-files.yaml") . | sha256sum }}
checksum/files: {{ include "altinity-clickhouse-operator.configMapContentHash" (dict "ctx" . "name" "/generated/ConfigMap-etc-clickhouse-operator-files.yaml") }}
checksum/confd-files: {{ include "altinity-clickhouse-operator.configMapContentHash" (dict "ctx" . "name" "/generated/ConfigMap-etc-clickhouse-operator-confd-files.yaml") }}
checksum/configd-files: {{ include "altinity-clickhouse-operator.configMapContentHash" (dict "ctx" . "name" "/generated/ConfigMap-etc-clickhouse-operator-configd-files.yaml") }}
checksum/templatesd-files: {{ include "altinity-clickhouse-operator.configMapContentHash" (dict "ctx" . "name" "/generated/ConfigMap-etc-clickhouse-operator-templatesd-files.yaml") }}
checksum/usersd-files: {{ include "altinity-clickhouse-operator.configMapContentHash" (dict "ctx" . "name" "/generated/ConfigMap-etc-clickhouse-operator-usersd-files.yaml") }}
checksum/keeper-confd-files: {{ include "altinity-clickhouse-operator.configMapContentHash" (dict "ctx" . "name" "/generated/ConfigMap-etc-keeper-operator-confd-files.yaml") }}
checksum/keeper-configd-files: {{ include "altinity-clickhouse-operator.configMapContentHash" (dict "ctx" . "name" "/generated/ConfigMap-etc-keeper-operator-configd-files.yaml") }}
checksum/keeper-templatesd-files: {{ include "altinity-clickhouse-operator.configMapContentHash" (dict "ctx" . "name" "/generated/ConfigMap-etc-keeper-operator-templatesd-files.yaml") }}
checksum/keeper-usersd-files: {{ include "altinity-clickhouse-operator.configMapContentHash" (dict "ctx" . "name" "/generated/ConfigMap-etc-keeper-operator-usersd-files.yaml") }}
spec:
serviceAccountName: {{ include "altinity-clickhouse-operator.serviceAccountName" . }}
volumes:
Expand Down
4 changes: 2 additions & 2 deletions dev/generate_helm_chart.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ function update_deployment_resource() {
a_data="${annotations}" yq e -i '.podAnnotations |= env(a_data)' "${values_yaml}"
yq e -i '.spec.template.metadata.annotations = {}' "${file}"

yq e -i '.spec.template.metadata.labels |= "{{ include \"altinity-clickhouse-operator.labels\" . | nindent 8 }}{{ if .Values.podLabels }}{{ toYaml .Values.podLabels | nindent 8 }}{{ end }}"' "${file}"
yq e -i '.spec.template.metadata.labels |= "{{ include \"altinity-clickhouse-operator.podTemplateLabels\" . | nindent 8 }}{{ if .Values.podLabels }}{{ toYaml .Values.podLabels | nindent 8 }}{{ end }}"' "${file}"
yq e -i '.spec.template.metadata.annotations += {"{{ if .Values.podAnnotations }}{{ toYaml .Values.podAnnotations | nindent 8 }}{{ end }}": null}' "${file}"
yq e -i '.spec.template.spec.imagePullSecrets |= "{{ toYaml .Values.imagePullSecrets | nindent 8 }}"' "${file}"
yq e -i '.spec.template.spec.serviceAccountName |= "{{ include \"altinity-clickhouse-operator.serviceAccountName\" . }}"' "${file}"
Expand All @@ -230,7 +230,7 @@ function update_deployment_resource() {
yq e -i '(.spec.template.spec.volumes[] | select(.configMap != null) | .configMap.name | select(. == "'"${cm}"'")) |= "'"${newCm}"'"' "${file}"
local cmName="${cm/etc-clickhouse-operator-/}"
cmName="${cmName/etc-keeper-operator-/keeper-}"
yq e -i '.spec.template.metadata.annotations += {"checksum/'"${cmName}"'": "{{ include (print $.Template.BasePath \"/generated/ConfigMap-'"${cm}"'.yaml\") . | sha256sum }}"}' "${file}"
yq e -i '.spec.template.metadata.annotations += {"checksum/'"${cmName}"'": "{{ include \"altinity-clickhouse-operator.configMapContentHash\" (dict \"ctx\" . \"name\" \"/generated/ConfigMap-'"${cm}"'.yaml\") }}"}' "${file}"
done

yq e -i '.spec.template.spec.containers[0].name |= "{{ .Chart.Name }}"' "${file}"
Expand Down
32 changes: 32 additions & 0 deletions dev/test_helm_chart.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,38 @@ else
report "ch-metrics is gated on metrics.enabled" yes
fi

# The operator reads its configuration once at startup and has no file watcher, so a missed
# restart strands stale confd/configd/templatesd/usersd config. These two assertions are the
# contract: nothing version-derived may sit in the pod template, and a real data change must
# still move the matching checksum. Chart version is not settable via --set, so the first one
# renders a copy of the chart with Chart.yaml edited.
pod_template() {
render "$@" | awk '/^ template:/{t=1} t{print} t&&/^ containers:/{exit}'
}

tmp_chart="$(mktemp -d)"
cp -R "${CHART}" "${tmp_chart}/chart"
pinned=(--set operator.image.tag=pinned --set metrics.image.tag=pinned)
before="$(pod_template "${pinned[@]}")"
yq e -i '.version = "99.99.99" | .appVersion = "99.99.99"' "${tmp_chart}/chart/Chart.yaml"
after="$(helm template "${RELEASE}" "${tmp_chart}/chart" -n "${NS}" "${pinned[@]}" 2>&1 | awk '/^ template:/{t=1} t{print} t&&/^ containers:/{exit}')"
rm -rf "${tmp_chart}"
if [[ "${before}" == "${after}" ]]; then
report "pod template is unchanged by a version bump" yes
else
report "pod template is unchanged by a version bump" no \
"$(diff <(printf '%s\n' "${before}") <(printf '%s\n' "${after}") | head -6 | tr '\n' ' ')"
fi

# ... but a real change to the mounted data must still roll the pods.
base_files="$(render | grep -E '^\s+checksum/files:')"
changed_files="$(render --set 'configs.files.config\.yaml=# changed' | grep -E '^\s+checksum/files:')"
if [[ -n "${base_files}" && "${base_files}" != "${changed_files}" ]]; then
report "checksum/files tracks a data change" yes
else
report "checksum/files tracks a data change" no "checksum/files did not move when the mounted data changed"
fi

echo
echo "crd-install hook"
# The hook runs at pre-install/pre-upgrade, so anything wrong here fails the whole release with
Expand Down
Loading