Skip to content

redis: keep config checksum stable across chart versions - #989

Merged
devdinu merged 2 commits into
mainfrom
redis-config-checksum-ignore-chart-version
Oct 30, 2026
Merged

devdinu merged 2 commits into
mainfrom
redis-config-checksum-ignore-chart-version

Conversation

@devdinu

@devdinu devdinu commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

With redisCache.config.enabled / redisStore.config.enabled, redis-cache and redis-store restart on every Sourcegraph upgrade, even when redis.conf and the redis images are unchanged. Sourcegraph Cloud enabled this fleet-wide on 2026-09-27 (sourcegraph/cloud#64455), and since then every deployment has restarted redis despite the frozen redis image tags.

Cause

The checksum/redis-config pod annotation hashed the whole rendered ConfigMap:

checksum/redis-config: {{ include (print $.Template.BasePath "/redis/redis-cache.ConfigMap.yaml") . | sha256sum }}

That rendering includes the helm.sh/chart: sourcegraph-<version> and app.kubernetes.io/version labels. The chart version is bumped with every Sourcegraph release, so the hash changed every upgrade, the pod template changed, and the Recreate strategy restarted redis.

Fix

Move the redis.conf rendering (vendored file plus maxmemory, maxmemory-policy, additionalConfig) into a sourcegraph.redis.conf helper. The ConfigMaps render their data from it, and the Deployments hash only its output. Redis now restarts only when its configuration changes, such as a memory limit change that resizes maxmemory.

Upgrading to a chart with this fix changes the annotation value once, so redis restarts one last time on that upgrade.

Checklist

ref PLAT-910

Test plan

  • helm unittest charts/sourcegraph: 209/209 pass. The new cases in redisConfigMount_test.yaml assert the checksum is identical at chart versions 8.0.0 and 9.9.9 and changes when maxmemory changes; the two version cases fail against the old templates.
  • helm template before and after for defaults, all overrides (including tpl in additionalConfig), no memory limit, localDevMode, and existingConfig: rendered ConfigMaps are byte-identical; only the checksum values differ.
  • The new checksum equals the sha256 of the expected redis.conf text, computed independently.

The redis Deployments' checksum/redis-config annotation hashed the whole
rendered ConfigMap, including its helm.sh/chart and app.kubernetes.io/version
labels. With config.enabled, every Sourcegraph upgrade changed the hash and
restarted redis-cache and redis-store even when redis.conf was unchanged.

Move the redis.conf rendering into a sourcegraph.redis.conf helper that both
the ConfigMaps and the checksum use, so the checksum covers only redis.conf.

Amp-Thread-ID: https://ampcode.com/threads/T-01a18a84-419f-703b-a77d-635921732235
Co-authored-by: Dinesh Kumar <dinesh.kumar@sourcegraph.com>
@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

The checksum tests pinned a hash for redis-cache only, so a regression
confined to redis-store would not have failed them. The cases also sat
in redisConfigMount_test.yaml, which covers the volume mount rather than
the annotation.

Move them into a redisConfigChecksum_test.yaml suite, pin a hash for
both services, and assert the hash ignores unrelated input (global
labels, image tag) while still changing for maxmemory, maxmemoryPolicy
and additionalConfig. The helper now fails the render when a vendored
redis.conf is missing instead of silently hashing an empty body.
@devdinu
devdinu requested a review from a team October 30, 2026 20:38
@devdinu
devdinu marked this pull request as ready for review October 30, 2026 20:38

@filiphaftek filiphaftek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good finding 👍

@devdinu
devdinu merged commit a6d7ab7 into main Oct 30, 2026
7 checks passed
@devdinu
devdinu deleted the redis-config-checksum-ignore-chart-version branch October 30, 2026 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants