redis: keep config checksum stable across chart versions

charts/sourcegraph/CHANGELOG.md

@@ -13,6 +13,7 @@ Use `**BREAKING**:` to denote a breaking change
 - Added `indexedSearch.readinessProbe` (`failureThreshold`, `periodSeconds`, `timeoutSeconds`) to tune the `zoekt-webserver` readiness probe, which was previously hardcoded. Defaults are unchanged (`3`/`5`/`5`). Raise `failureThreshold` on instances with large monorepos, where loading a many-shard repo can stall `/healthz` past `timeoutSeconds` and drop a healthy pod out of the `indexed-search` service.
 - Added `indexedSearch.livenessProbe` (`failureThreshold`, `initialDelaySeconds`, `periodSeconds`, `timeoutSeconds`) to tune the `zoekt-webserver` liveness probe, which was previously hardcoded. Defaults are unchanged (`10`/`120`/`60`/`5`). Raise `failureThreshold` or `timeoutSeconds` where the index greatly exceeds the container memory limit: page-cache reclaim stalls `/healthz` past `timeoutSeconds`, and consecutive failures restart a container that is still serving. Keep the resulting window wider than zoekt's in-process watchdog (`ZOEKT_WATCHDOG_ERRORS` x `ZOEKT_WATCHDOG_TICK`) so the watchdog remains the first line of defence.
 - `searcher.storageSize` now accepts any Kubernetes quantity (e.g. `150G`, `1Ti`, `1.5Ti`, `500000M`) when deriving `SEARCHER_CACHE_SIZE_MB` and `SYMBOLS_CACHE_SIZE_MB`. Previously anything other than `Gi` silently rendered both as `0`, which disables cache eviction and lets the cache volume fill up. Unparseable values, or values below `3M`, now fail the render unless `searcher.autoCacheSize` is enabled. The derived size now matches searcher's own auto-sizing arithmetic (45% of the volume in decimal megabytes), so existing `Gi` deployments see a slightly higher value on upgrade, e.g. the default `26Gi` renders `12562` instead of `11700`.
+- Fixed `redis-cache` and `redis-store` restarting on every upgrade when `config.enabled` is true. The `checksum/redis-config` pod annotation hashed the whole rendered ConfigMap, including its chart-version labels, so it changed with every release even when `redis.conf` did not. It now hashes only `redis.conf`, so redis restarts only when its configuration changes. The annotation value changes once, so redis restarts one last time on the upgrade that picks up this fix.
 
 ## 8.1.0
 

charts/sourcegraph/templates/_helpers.tpl

@@ -368,6 +368,40 @@ Resolution order:
 {{- end -}}
 {{- end -}}
 
+{{/*
+Render the effective redis.conf for a service: the vendored file plus the
+chart-managed directives.
+Usage: include "sourcegraph.redis.conf" (list . "redisCache")
+
+The redis Deployments hash this output for their checksum/redis-config
+annotation, so it must depend only on the config, never on release metadata
+such as the chart version; otherwise every upgrade restarts redis.
+*/}}
+{{- define "sourcegraph.redis.conf" -}}
+{{- $top := index . 0 -}}
+{{- $service := index . 1 -}}
+{{- $config := (index $top.Values $service).config -}}
+{{- $dir := kebabcase $service -}}
+{{- $body := $top.Files.Get (printf "files/%s/redis.conf" $dir) | trimSuffix "\n" -}}
+{{- if not $body -}}
+{{- fail (printf "no vendored redis.conf found at files/%s/redis.conf for service %s" $dir $service) -}}
+{{- end -}}
+{{- $overrides := list -}}
+{{- with include "sourcegraph.redis.maxmemory" (list $top $service) -}}
+{{- $overrides = append $overrides (printf "maxmemory %s" .) -}}
+{{- end -}}
+{{- with $config.maxmemoryPolicy -}}
+{{- $overrides = append $overrides (printf "maxmemory-policy %s" .) -}}
+{{- end -}}
+{{- with $config.additionalConfig -}}
+{{- $overrides = append $overrides (tpl . $top | trim) -}}
+{{- end -}}
+{{- if $overrides -}}
+{{- $body = printf "%s\n\n# Chart-managed directives. Redis applies the last occurrence of a directive.\n%s" $body (join "\n" $overrides) -}}
+{{- end -}}
+{{- $body -}}
+{{- end -}}
+
 {{/*
 Fail the render when a service's extra volumes or volume mounts collide with the
 chart-managed redis config mount. Without this the collision only surfaces as an

charts/sourcegraph/templates/redis/redis-cache.ConfigMap.yaml

@@ -3,20 +3,6 @@
 {{- fail "You can only define one of 'redisCache.config.existingConfig' and 'redisCache.config.additionalConfig' at a time" }}
 {{- end }}
 {{- if not .Values.redisCache.config.existingConfig -}}
-{{- $body := .Files.Get "files/redis-cache/redis.conf" | trimSuffix "\n" -}}
-{{- $overrides := list -}}
-{{- with include "sourcegraph.redis.maxmemory" (list . "redisCache") -}}
-{{- $overrides = append $overrides (printf "maxmemory %s" .) -}}
-{{- end -}}
-{{- with .Values.redisCache.config.maxmemoryPolicy -}}
-{{- $overrides = append $overrides (printf "maxmemory-policy %s" .) -}}
-{{- end -}}
-{{- with .Values.redisCache.config.additionalConfig -}}
-{{- $overrides = append $overrides (tpl . $ | trim) -}}
-{{- end -}}
-{{- if $overrides -}}
-{{- $body = printf "%s\n\n# Chart-managed directives. Redis applies the last occurrence of a directive.\n%s" $body (join "\n" $overrides) -}}
-{{- end -}}
 apiVersion: v1
 kind: ConfigMap
 metadata:
@@ -32,6 +18,6 @@ metadata:
   name: {{ .Values.redisCache.name }}-conf
 data:
   redis.conf: |
-    {{- $body | nindent 4 }}
+    {{- include "sourcegraph.redis.conf" (list . "redisCache") | nindent 4 }}
 {{- end }}
 {{- end }}

charts/sourcegraph/templates/redis/redis-cache.Deployment.yaml

@@ -30,7 +30,7 @@ spec:
       annotations:
         kubectl.kubernetes.io/default-container: redis-cache
         {{- if and .Values.redisCache.config.enabled (not .Values.redisCache.config.existingConfig) }}
-        checksum/redis-config: {{ include (print $.Template.BasePath "/redis/redis-cache.ConfigMap.yaml") . | sha256sum }}
+        checksum/redis-config: {{ include "sourcegraph.redis.conf" (list . "redisCache") | sha256sum }}
         {{- end }}
       {{- if .Values.sourcegraph.podAnnotations }}
       {{- toYaml .Values.sourcegraph.podAnnotations | nindent 8 }}

charts/sourcegraph/templates/redis/redis-store.ConfigMap.yaml

@@ -3,20 +3,6 @@
 {{- fail "You can only define one of 'redisStore.config.existingConfig' and 'redisStore.config.additionalConfig' at a time" }}
 {{- end }}
 {{- if not .Values.redisStore.config.existingConfig -}}
-{{- $body := .Files.Get "files/redis-store/redis.conf" | trimSuffix "\n" -}}
-{{- $overrides := list -}}
-{{- with include "sourcegraph.redis.maxmemory" (list . "redisStore") -}}
-{{- $overrides = append $overrides (printf "maxmemory %s" .) -}}
-{{- end -}}
-{{- with .Values.redisStore.config.maxmemoryPolicy -}}
-{{- $overrides = append $overrides (printf "maxmemory-policy %s" .) -}}
-{{- end -}}
-{{- with .Values.redisStore.config.additionalConfig -}}
-{{- $overrides = append $overrides (tpl . $ | trim) -}}
-{{- end -}}
-{{- if $overrides -}}
-{{- $body = printf "%s\n\n# Chart-managed directives. Redis applies the last occurrence of a directive.\n%s" $body (join "\n" $overrides) -}}
-{{- end -}}
 apiVersion: v1
 kind: ConfigMap
 metadata:
@@ -32,6 +18,6 @@ metadata:
   name: {{ .Values.redisStore.name }}-conf
 data:
   redis.conf: |
-    {{- $body | nindent 4 }}
+    {{- include "sourcegraph.redis.conf" (list . "redisStore") | nindent 4 }}
 {{- end }}
 {{- end }}

charts/sourcegraph/templates/redis/redis-store.Deployment.yaml

@@ -30,7 +30,7 @@ spec:
       annotations:
         kubectl.kubernetes.io/default-container: redis-store
         {{- if and .Values.redisStore.config.enabled (not .Values.redisStore.config.existingConfig) }}
-        checksum/redis-config: {{ include (print $.Template.BasePath "/redis/redis-store.ConfigMap.yaml") . | sha256sum }}
+        checksum/redis-config: {{ include "sourcegraph.redis.conf" (list . "redisStore") | sha256sum }}
         {{- end }}
       {{- if .Values.sourcegraph.podAnnotations }}
       {{- toYaml .Values.sourcegraph.podAnnotations | nindent 8 }}

charts/sourcegraph/tests/redisConfigChecksum_test.yaml

@@ -0,0 +1,165 @@
+suite: redis config checksum
+templates:
+  - templates/redis/redis-store.Deployment.yaml
+  - templates/redis/redis-cache.Deployment.yaml
+tests:
+  # The checksum must cover the rendered redis.conf body and nothing else.
+  # Hashing the whole ConfigMap manifest pulls in the helm.sh/chart label, so
+  # every chart build changes it and restarts redis even when the config is
+  # byte-identical. These pin the hash for a fixed config, so any change that
+  # lets unrelated input back in fails here.
+  #
+  # Editing a vendored files/redis-*/redis.conf changes these hashes on purpose.
+  # Re-pin by rendering the ConfigMap for the same values and hashing its
+  # redis.conf body, e.g.
+  #   helm template charts/sourcegraph --set redisStore.config.enabled=true \
+  #     --set redisStore.resources.limits.memory=7Gi \
+  #     -s templates/redis/redis-store.ConfigMap.yaml
+  - it: pins the redis-store checksum for a fixed config
+    templates:
+      - templates/redis/redis-store.Deployment.yaml
+    set:
+      redisStore.config.enabled: true
+      redisStore.resources.limits.memory: 7Gi
+    asserts:
+      - equal:
+          path: spec.template.metadata.annotations["checksum/redis-config"]
+          value: &pinnedStoreChecksum f65e9a885e0fd9df5e9f4c3dc6dbcf5b05f11b997d375f5272f7a3c9397f65e5
+
+  - it: pins the redis-cache checksum for a fixed config
+    templates:
+      - templates/redis/redis-cache.Deployment.yaml
+    set:
+      redisCache.config.enabled: true
+      redisCache.resources.limits.memory: 10Gi
+    asserts:
+      - equal:
+          path: spec.template.metadata.annotations["checksum/redis-config"]
+          value: 0a0f0bb343c5d37ed52bcebf58fd3dbe10f54423c9d920aa90ac8504ee108877
+
+  # The chart version is the specific input that caused the fleet-wide restarts:
+  # it is bumped on every release and reached the hash via the ConfigMap labels.
+  - it: does not change when only the chart version changes
+    templates:
+      - templates/redis/redis-cache.Deployment.yaml
+    chart:
+      version: 8.0.0
+      appVersion: 8.0.0
+    set:
+      redisCache.config:
+        enabled: true
+        maxmemory: 1gb
+    asserts:
+      - equal:
+          path: spec.template.metadata.annotations["checksum/redis-config"]
+          value: &pinnedCacheChecksum 57f261f5131adbefd08fe4181b50b898b68f386949f8bbc10f747da37e178f74
+
+  - it: keeps the same checksum on a later chart version
+    templates:
+      - templates/redis/redis-cache.Deployment.yaml
+    chart:
+      version: 9.9.9
+      appVersion: 9.9.9
+    set:
+      redisCache.config:
+        enabled: true
+        maxmemory: 1gb
+    asserts:
+      - equal:
+          path: spec.template.metadata.annotations["checksum/redis-config"]
+          value: *pinnedCacheChecksum
+
+  - it: changes when an explicit maxmemory changes
+    templates:
+      - templates/redis/redis-cache.Deployment.yaml
+    chart:
+      version: 9.9.9
+      appVersion: 9.9.9
+    set:
+      redisCache.config:
+        enabled: true
+        maxmemory: 2gb
+    asserts:
+      - notEqual:
+          path: spec.template.metadata.annotations["checksum/redis-config"]
+          value: *pinnedCacheChecksum
+
+  # Unrelated input must not reach the hash. These are the regressions that
+  # caused redis to roll on every chart build.
+  - it: does not change when an unrelated global label changes
+    templates:
+      - templates/redis/redis-store.Deployment.yaml
+    set:
+      redisStore.config.enabled: true
+      redisStore.resources.limits.memory: 7Gi
+      sourcegraph.labels.team: platform
+    asserts:
+      - equal:
+          path: spec.template.metadata.annotations["checksum/redis-config"]
+          value: *pinnedStoreChecksum
+
+  - it: does not change when the sourcegraph image tag changes
+    templates:
+      - templates/redis/redis-store.Deployment.yaml
+    set:
+      redisStore.config.enabled: true
+      redisStore.resources.limits.memory: 7Gi
+      sourcegraph.image.tag: some-new-build
+    asserts:
+      - equal:
+          path: spec.template.metadata.annotations["checksum/redis-config"]
+          value: *pinnedStoreChecksum
+
+  # Real config changes must still roll redis, otherwise the annotation is useless.
+  - it: changes when the memory limit changes the derived maxmemory
+    templates:
+      - templates/redis/redis-store.Deployment.yaml
+    set:
+      redisStore.config.enabled: true
+      redisStore.resources.limits.memory: 14Gi
+    asserts:
+      - notEqual:
+          path: spec.template.metadata.annotations["checksum/redis-config"]
+          value: *pinnedStoreChecksum
+
+  - it: changes when maxmemoryPolicy is set
+    templates:
+      - templates/redis/redis-store.Deployment.yaml
+    set:
+      redisStore.config.enabled: true
+      redisStore.resources.limits.memory: 7Gi
+      redisStore.config.maxmemoryPolicy: allkeys-lru
+    asserts:
+      - notEqual:
+          path: spec.template.metadata.annotations["checksum/redis-config"]
+          value: *pinnedStoreChecksum
+
+  - it: changes when additionalConfig is set
+    templates:
+      - templates/redis/redis-store.Deployment.yaml
+    set:
+      redisStore.config.enabled: true
+      redisStore.resources.limits.memory: 7Gi
+      redisStore.config.additionalConfig: io-threads 4
+    asserts:
+      - notEqual:
+          path: spec.template.metadata.annotations["checksum/redis-config"]
+          value: *pinnedStoreChecksum
+
+  - it: is absent when the chart-managed config is disabled
+    set:
+      redisStore.config.enabled: false
+      redisCache.config.enabled: false
+    asserts:
+      - notExists:
+          path: spec.template.metadata.annotations["checksum/redis-config"]
+
+  - it: is absent when an existingConfig is supplied
+    set:
+      redisStore.config.enabled: true
+      redisCache.config.enabled: true
+      redisStore.config.existingConfig: my-own-redis-conf
+      redisCache.config.existingConfig: my-own-redis-conf
+    asserts:
+      - notExists:
+          path: spec.template.metadata.annotations["checksum/redis-config"]