From 250bbbbecda7032f3ad78900b08186a92262d073 Mon Sep 17 00:00:00 2001 From: Michael Lin Date: Fri, 14 Aug 2026 20:22:18 -0500 Subject: [PATCH 1/3] redis: vendor redis.conf and auto-size maxmemory from the memory limit `redis-cache` and `redis-store` relied entirely on the image's baked `/etc/redis/redis.conf`, which hardcodes `maxmemory 6gb`. The chart never told redis about the pod memory limit, so lowering `resources.limits.memory` below 6GiB made the kubelet OOM-kill the pod before LRU eviction ran, and the pod restarted into the same wall. Follow the pgsql pattern: vendor the images' config in-repo under `files/redis-{cache,store}/redis.conf`, render it into a `redis-{cache,store}-conf` ConfigMap and mount it over the image path with `subPath`. The image entrypoint (`redis-server /etc/redis/redis.conf`) reads it, so no `command`/`args` override is needed. The vendored files are copied verbatim from the pinned 6.0.0 image tags. The chart appends a `maxmemory` sized at `config.maxmemoryRatio` (0.75) of `resources.limits.memory`. A new `config` block adds `existingConfig`, `maxmemory`, `maxmemoryRatio`, `maxmemoryPolicy` and `additionalConfig` for both services. The quantity parser fails safe: an unset or unrecognised limit emits no `maxmemory` and the vendored default stands. `sourcegraph.redis.assertNoConfClash` fails the render with a migration message when `extraVolumeMounts` already targets `/etc/redis/redis.conf` or an extra volume reuses the reserved `redis-conf` name, instead of letting it surface as an opaque duplicate-mount rejection. Co-Authored-By: Claude Opus 5 (1M context) --- charts/sourcegraph/CHANGELOG.md | 2 + charts/sourcegraph/README.md | 10 + .../sourcegraph/files/redis-cache/redis.conf | 17 ++ .../sourcegraph/files/redis-store/redis.conf | 18 ++ charts/sourcegraph/templates/_helpers.tpl | 57 ++++++ .../redis/redis-cache.ConfigMap.yaml | 35 ++++ .../redis/redis-cache.Deployment.yaml | 14 ++ .../redis/redis-store.ConfigMap.yaml | 35 ++++ .../redis/redis-store.Deployment.yaml | 14 ++ .../tests/redisConfigMount_test.yaml | 118 +++++++++++ .../sourcegraph/tests/redisConfig_test.yaml | 190 ++++++++++++++++++ charts/sourcegraph/values.yaml | 36 ++++ 12 files changed, 546 insertions(+) create mode 100644 charts/sourcegraph/files/redis-cache/redis.conf create mode 100644 charts/sourcegraph/files/redis-store/redis.conf create mode 100644 charts/sourcegraph/templates/redis/redis-cache.ConfigMap.yaml create mode 100644 charts/sourcegraph/templates/redis/redis-store.ConfigMap.yaml create mode 100644 charts/sourcegraph/tests/redisConfigMount_test.yaml create mode 100644 charts/sourcegraph/tests/redisConfig_test.yaml diff --git a/charts/sourcegraph/CHANGELOG.md b/charts/sourcegraph/CHANGELOG.md index 1ed7b2c3e..169ce8c86 100644 --- a/charts/sourcegraph/CHANGELOG.md +++ b/charts/sourcegraph/CHANGELOG.md @@ -8,6 +8,8 @@ Use `**BREAKING**:` to denote a breaking change ## Unreleased +- **BREAKING**: The chart now manages `/etc/redis/redis.conf` for `redis-cache` and `redis-store`. It vendors the config that the images bake in (`charts/sourcegraph/files/redis-{cache,store}/redis.conf`), renders it into a `redis-{cache,store}-conf` ConfigMap and mounts it over the image path. Operators who inject their own `redis.conf` through `redisCache.extraVolumeMounts` / `redisStore.extraVolumeMounts` must move to the new `redisCache.config.existingConfig` / `redisStore.config.existingConfig`; the chart fails the render with a migration message otherwise. +- Added `redisCache.config` and `redisStore.config` (`existingConfig`, `maxmemory`, `maxmemoryRatio`, `maxmemoryPolicy`, `additionalConfig`). By default the chart sizes redis `maxmemory` at `maxmemoryRatio` (0.75) of `resources.limits.memory` instead of leaving the images' hardcoded `maxmemory 6gb`, so lowering the memory limit no longer OOM-kills the pod before eviction runs. At the default 7Gi limit this lowers the effective cap from 6GiB to 5.25GiB: `redis-cache` only evicts earlier, but `redis-store` uses `noeviction`, so its write-error ceiling drops too. Set `redisStore.config.maxmemory: 6gb` or raise `redisStore.config.maxmemoryRatio` to keep the old ceiling. - Added `searcher.autoCacheSize` (default `false`) to omit the `SEARCHER_CACHE_SIZE_MB` and `SYMBOLS_CACHE_SIZE_MB` env vars, letting `searcher` auto-size its cache to ~45% of the live cache volume so it tracks PVC expansion instead of staying frozen to the initial `storageSize` - Added support for ordering trace processors via `openTelemetry.gateway.config.traces.tracePipelineProcessors`, falling back to processors ordered by name when unset - Removed the unused executor controller `/data` PersistentVolumeClaim from the Kubernetes-native executor chart (`sourcegraph-executor/k8s`), along with the now-orphaned `storageClass` and `executor.storageSize` values and the vestigial `EXECUTOR_KUBERNETES_PERSISTENCE_VOLUME_NAME` env var. Since single-job-pod became the only k8s execution mode, job pods use their own ephemeral `emptyDir` volume and the controller writes nothing to `/data`. diff --git a/charts/sourcegraph/README.md b/charts/sourcegraph/README.md index 8a51554ba..61cc0fcc3 100644 --- a/charts/sourcegraph/README.md +++ b/charts/sourcegraph/README.md @@ -289,6 +289,11 @@ In addition to the documented values, all services also support the following va | prometheus.storageAnnotations | object | `{}` | Optional annotations to add to the `prometheus` PVC | | prometheus.storageSize | string | `"200Gi"` | PVC Storage Request for `prometheus` data volume | | prometheus.storageSubPath | string | `""` | Optional subPath for the `prometheus` primary data volume mount | +| redisCache.config.additionalConfig | string | `""` | Additional raw redis directives appended to the vendored `redis-cache` config. Notes: This is expecting a multiline string. It renders into a ConfigMap in plaintext, so do not put secrets such as `requirepass` here; use `existingConfig` instead. | +| redisCache.config.existingConfig | string | `""` | Name of an existing ConfigMap for `redis-cache`. It must contain a `redis.conf` key. When set, the chart-managed ConfigMap is not rendered and this one is mounted instead, so the chart no longer sizes `maxmemory`. Mutually exclusive with `additionalConfig`. | +| redisCache.config.maxmemory | string | `""` | Explicit redis `maxmemory` for `redis-cache` (for example `6gb`). Overrides the auto-computed value. Empty means compute it from the container memory limit. | +| redisCache.config.maxmemoryPolicy | string | `""` | Override the redis `maxmemory-policy` for `redis-cache`. Empty keeps the vendored default (`allkeys-lru`). | +| redisCache.config.maxmemoryRatio | float | `0.75` | Fraction of `redisCache.resources.limits.memory` used for `maxmemory` when `maxmemory` is empty. Ignored when no memory limit is set. | | redisCache.connection.endpoint | string | `"redis-cache:6379"` | Endpoint to use for redis-cache. Supports either host:port or IANA specification | | redisCache.connection.existingSecret | string | `""` | Name of existing secret to use for Redis endpoint The secret must contain the key `endpoint` and should follow IANA specification learn more from the [Helm docs](https://docs.sourcegraph.com/admin/install/kubernetes/helm#using-external-redis-instances) | | redisCache.containerSecurityContext | object | `{"allowPrivilegeEscalation":false,"readOnlyRootFilesystem":true,"runAsGroup":1000,"runAsUser":999}` | Security context for the `redis-cache` container, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-container) | @@ -307,6 +312,11 @@ In addition to the documented values, all services also support the following va | redisExporter.image.defaultTag | string | `"6.0.0@sha256:b2ec48fc6adef31f36d525170138dec303c1c0c20c530d659f1fb7c6c54698af"` | Docker image tag for the `redis-exporter` image | | redisExporter.image.name | string | `"redis_exporter"` | Docker image name for the `redis-exporter` image | | redisExporter.resources | object | `{"limits":{"cpu":"10m","memory":"100Mi"},"requests":{"cpu":"10m","memory":"100Mi"}}` | Resource requests & limits for the `redis-exporter` sidecar container, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/) | +| redisStore.config.additionalConfig | string | `""` | Additional raw redis directives appended to the vendored `redis-store` config. Notes: This is expecting a multiline string. It renders into a ConfigMap in plaintext, so do not put secrets such as `requirepass` here; use `existingConfig` instead. | +| redisStore.config.existingConfig | string | `""` | Name of an existing ConfigMap for `redis-store`. It must contain a `redis.conf` key. When set, the chart-managed ConfigMap is not rendered and this one is mounted instead, so the chart no longer sizes `maxmemory`. Mutually exclusive with `additionalConfig`. | +| redisStore.config.maxmemory | string | `""` | Explicit redis `maxmemory` for `redis-store` (for example `6gb`). Overrides the auto-computed value. Empty means compute it from the container memory limit. | +| redisStore.config.maxmemoryPolicy | string | `""` | Override the redis `maxmemory-policy` for `redis-store`. Empty keeps the vendored default (`noeviction`). | +| redisStore.config.maxmemoryRatio | float | `0.75` | Fraction of `redisStore.resources.limits.memory` used for `maxmemory` when `maxmemory` is empty. Ignored when no memory limit is set. | | redisStore.connection.endpoint | string | `"redis-store:6379"` | Endpoint to use for redis-store. Supports either host:port or IANA specification | | redisStore.connection.existingSecret | string | `""` | Name of existing secret to use for Redis endpoint The secret must contain the key `endpoint` and should follow IANA specification learn more from the [Helm docs](https://docs.sourcegraph.com/admin/install/kubernetes/helm#using-external-redis-instances) | | redisStore.containerSecurityContext | object | `{"allowPrivilegeEscalation":false,"readOnlyRootFilesystem":true,"runAsGroup":1000,"runAsUser":999}` | Security context for the `redis-store` container, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-container) | diff --git a/charts/sourcegraph/files/redis-cache/redis.conf b/charts/sourcegraph/files/redis-cache/redis.conf new file mode 100644 index 000000000..907ac587b --- /dev/null +++ b/charts/sourcegraph/files/redis-cache/redis.conf @@ -0,0 +1,17 @@ +# Vendored from the `redis-cache` image (`/etc/redis/redis.conf`). +# Source: sourcegraph/sourcegraph docker-images/redis-cache/redis.conf +# Seeded from image tag 6.0.0. Keep this file in sync when the image tag changes. +# +# The chart mounts this file over /etc/redis/redis.conf, so the image entrypoint +# (`redis-server /etc/redis/redis.conf`) reads it. The chart appends the computed +# `maxmemory` and any `redisCache.config` overrides after this body. + +# allow access from all instances +protected-mode no +# limit memory usage, discard unused keys when hitting limit +maxmemory 6gb +maxmemory-policy allkeys-lru +# snapshots on disk every minute +dir /redis-data/ +appendonly no +save 60 1 diff --git a/charts/sourcegraph/files/redis-store/redis.conf b/charts/sourcegraph/files/redis-store/redis.conf new file mode 100644 index 000000000..b45fba495 --- /dev/null +++ b/charts/sourcegraph/files/redis-store/redis.conf @@ -0,0 +1,18 @@ +# Vendored from the `redis-store` image (`/etc/redis/redis.conf`). +# Source: sourcegraph/sourcegraph docker-images/redis-store/redis.conf +# Seeded from image tag 6.0.0. Keep this file in sync when the image tag changes. +# +# The chart mounts this file over /etc/redis/redis.conf, so the image entrypoint +# (`redis-server /etc/redis/redis.conf`) reads it. The chart appends the computed +# `maxmemory` and any `redisStore.config` overrides after this body. + +# allow access from all instances +protected-mode no +# limit memory usage, return error when hitting limit +maxmemory 6gb +maxmemory-policy noeviction +# live commit log to disk, additionally snapshot every 5 minutes +dir /redis-data/ +appendonly yes +aof-use-rdb-preamble yes +save 300 1 diff --git a/charts/sourcegraph/templates/_helpers.tpl b/charts/sourcegraph/templates/_helpers.tpl index 5c064c4b4..57b7d9bd8 100644 --- a/charts/sourcegraph/templates/_helpers.tpl +++ b/charts/sourcegraph/templates/_helpers.tpl @@ -312,3 +312,60 @@ checksum/auth: {{ toJson $checksum | sha256sum }} {{- $checksum := append $checksum .Values.redisCache.connection -}} checksum/redis: {{ toJson $checksum | sha256sum }} {{- end -}} + +{{/* +Resolve the redis `maxmemory` directive for a service. +Usage: include "sourcegraph.redis.maxmemory" (list . "redisCache") + +Resolution order: + 1. .config.maxmemory, used verbatim. + 2. floor(.config.maxmemoryRatio * .resources.limits.memory), + rendered as a plain byte count. + 3. Empty string, when there is no memory limit or the quantity is not + recognised. The caller then emits no `maxmemory` and the vendored default + stands. +*/}} +{{- define "sourcegraph.redis.maxmemory" -}} +{{- $top := index . 0 -}} +{{- $service := index . 1 -}} +{{- $values := index $top.Values $service -}} +{{- $config := $values.config | default dict -}} +{{- if $config.maxmemory -}} +{{- $config.maxmemory -}} +{{- else -}} +{{- $limit := dig "resources" "limits" "memory" "" $values | toString -}} +{{- $number := regexReplaceAll "^([0-9]+(\\.[0-9]+)?).*$" $limit "${1}" -}} +{{- $suffix := regexReplaceAll "^[0-9]+(\\.[0-9]+)?" $limit "" -}} +{{- /* Kubernetes quantity suffixes: binary (1024^n) and decimal (1000^n). */ -}} +{{- $units := dict "" 1.0 "k" 1e3 "M" 1e6 "G" 1e9 "T" 1e12 "P" 1e15 "E" 1e18 "Ki" 1024.0 "Mi" 1048576.0 "Gi" 1073741824.0 "Ti" 1099511627776.0 "Pi" 1125899906842624.0 "Ei" 1152921504606846976.0 -}} +{{- if and (regexMatch "^[0-9]+(\\.[0-9]+)?$" $number) (hasKey $units $suffix) -}} +{{- $ratio := $config.maxmemoryRatio | default 0.75 | float64 -}} +{{- $bytes := floor (mulf (float64 $number) (index $units $suffix) $ratio) -}} +{{- if gt $bytes 0.0 -}} +{{- printf "%d" (int64 $bytes) -}} +{{- end -}} +{{- end -}} +{{- end -}} +{{- 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 +opaque "must be unique" rejection from the API server. +Usage: include "sourcegraph.redis.assertNoConfClash" (list . "redisCache") +*/}} +{{- define "sourcegraph.redis.assertNoConfClash" -}} +{{- $top := index . 0 -}} +{{- $service := index . 1 -}} +{{- $values := index $top.Values $service -}} +{{- range ($values.extraVolumeMounts | default list) -}} +{{- if has .mountPath (list "/etc/redis/redis.conf" "/etc/redis" "/etc/redis/") -}} +{{- fail (printf "%s.extraVolumeMounts must not mount over /etc/redis/redis.conf; the chart now manages that file. Move your custom redis config to %s.config.existingConfig or %s.config.additionalConfig." $service $service $service) -}} +{{- end -}} +{{- end -}} +{{- range (concat ($values.extraVolumes | default list) ($values.extraVolumeMounts | default list)) -}} +{{- if eq (.name | toString) "redis-conf" -}} +{{- fail (printf "%s must not define a volume named 'redis-conf'; the chart reserves that name for the redis config mount." $service) -}} +{{- end -}} +{{- end -}} +{{- end -}} diff --git a/charts/sourcegraph/templates/redis/redis-cache.ConfigMap.yaml b/charts/sourcegraph/templates/redis/redis-cache.ConfigMap.yaml new file mode 100644 index 000000000..ffe6cf9b2 --- /dev/null +++ b/charts/sourcegraph/templates/redis/redis-cache.ConfigMap.yaml @@ -0,0 +1,35 @@ +{{- if and .Values.redisCache.enabled (and .Values.redisCache.config.existingConfig .Values.redisCache.config.additionalConfig) -}} +{{- fail "You can only define one of 'redisCache.config.existingConfig' and 'redisCache.config.additionalConfig' at a time" }} +{{- end }} +{{- if and .Values.redisCache.enabled (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: + annotations: + description: Configuration for redis-cache + labels: + {{- include "sourcegraph.labels" . | nindent 4 }} + {{- if .Values.redisCache.labels }} + {{- toYaml .Values.redisCache.labels | nindent 4 }} + {{- end }} + deploy: sourcegraph + app.kubernetes.io/component: redis + name: {{ .Values.redisCache.name }}-conf +data: + redis.conf: | + {{- $body | nindent 4 }} +{{- end }} diff --git a/charts/sourcegraph/templates/redis/redis-cache.Deployment.yaml b/charts/sourcegraph/templates/redis/redis-cache.Deployment.yaml index bb5982328..bade1a6e2 100644 --- a/charts/sourcegraph/templates/redis/redis-cache.Deployment.yaml +++ b/charts/sourcegraph/templates/redis/redis-cache.Deployment.yaml @@ -1,4 +1,5 @@ {{- if .Values.redisCache.enabled -}} +{{- include "sourcegraph.redis.assertNoConfClash" (list . "redisCache") -}} apiVersion: apps/v1 kind: Deployment metadata: @@ -26,6 +27,9 @@ spec: metadata: annotations: kubectl.kubernetes.io/default-container: redis-cache + {{- if not .Values.redisCache.config.existingConfig }} + checksum/redis-config: {{ include (print $.Template.BasePath "/redis/redis-cache.ConfigMap.yaml") . | sha256sum }} + {{- end }} {{- if .Values.sourcegraph.podAnnotations }} {{- toYaml .Values.sourcegraph.podAnnotations | nindent 8 }} {{- end }} @@ -100,6 +104,10 @@ spec: {{- if .Values.redisCache.storageSubPath }} subPath: {{ .Values.redisCache.storageSubPath }} {{- end }} + - mountPath: /etc/redis/redis.conf + name: redis-conf + subPath: redis.conf + readOnly: true {{- if .Values.redisCache.extraVolumeMounts }} {{- toYaml .Values.redisCache.extraVolumeMounts | nindent 8 }} {{- end }} @@ -139,6 +147,12 @@ spec: - name: redis-data persistentVolumeClaim: claimName: redis-cache + - name: redis-conf + configMap: + name: {{ default (print .Values.redisCache.name "-conf") .Values.redisCache.config.existingConfig }} + items: + - key: redis.conf + path: redis.conf {{- if .Values.redisCache.extraVolumes }} {{- toYaml .Values.redisCache.extraVolumes | nindent 6 }} {{- end }} diff --git a/charts/sourcegraph/templates/redis/redis-store.ConfigMap.yaml b/charts/sourcegraph/templates/redis/redis-store.ConfigMap.yaml new file mode 100644 index 000000000..a921fdee8 --- /dev/null +++ b/charts/sourcegraph/templates/redis/redis-store.ConfigMap.yaml @@ -0,0 +1,35 @@ +{{- if and .Values.redisStore.enabled (and .Values.redisStore.config.existingConfig .Values.redisStore.config.additionalConfig) -}} +{{- fail "You can only define one of 'redisStore.config.existingConfig' and 'redisStore.config.additionalConfig' at a time" }} +{{- end }} +{{- if and .Values.redisStore.enabled (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: + annotations: + description: Configuration for redis-store + labels: + {{- include "sourcegraph.labels" . | nindent 4 }} + {{- if .Values.redisStore.labels }} + {{- toYaml .Values.redisStore.labels | nindent 4 }} + {{- end }} + deploy: sourcegraph + app.kubernetes.io/component: redis + name: {{ .Values.redisStore.name }}-conf +data: + redis.conf: | + {{- $body | nindent 4 }} +{{- end }} diff --git a/charts/sourcegraph/templates/redis/redis-store.Deployment.yaml b/charts/sourcegraph/templates/redis/redis-store.Deployment.yaml index 59806df90..9ab9e7c21 100644 --- a/charts/sourcegraph/templates/redis/redis-store.Deployment.yaml +++ b/charts/sourcegraph/templates/redis/redis-store.Deployment.yaml @@ -1,4 +1,5 @@ {{- if .Values.redisStore.enabled -}} +{{- include "sourcegraph.redis.assertNoConfClash" (list . "redisStore") -}} apiVersion: apps/v1 kind: Deployment metadata: @@ -26,6 +27,9 @@ spec: metadata: annotations: kubectl.kubernetes.io/default-container: redis-store + {{- if not .Values.redisStore.config.existingConfig }} + checksum/redis-config: {{ include (print $.Template.BasePath "/redis/redis-store.ConfigMap.yaml") . | sha256sum }} + {{- end }} {{- if .Values.sourcegraph.podAnnotations }} {{- toYaml .Values.sourcegraph.podAnnotations | nindent 8 }} {{- end }} @@ -99,6 +103,10 @@ spec: {{- if .Values.redisStore.storageSubPath }} subPath: {{ .Values.redisStore.storageSubPath }} {{- end }} + - mountPath: /etc/redis/redis.conf + name: redis-conf + subPath: redis.conf + readOnly: true {{- if .Values.redisStore.extraVolumeMounts }} {{- toYaml .Values.redisStore.extraVolumeMounts | nindent 8 }} {{- end }} @@ -138,6 +146,12 @@ spec: - name: redis-data persistentVolumeClaim: claimName: redis-store + - name: redis-conf + configMap: + name: {{ default (print .Values.redisStore.name "-conf") .Values.redisStore.config.existingConfig }} + items: + - key: redis.conf + path: redis.conf {{- if .Values.redisStore.extraVolumes }} {{- toYaml .Values.redisStore.extraVolumes | nindent 6 }} {{- end }} diff --git a/charts/sourcegraph/tests/redisConfigMount_test.yaml b/charts/sourcegraph/tests/redisConfigMount_test.yaml new file mode 100644 index 000000000..01a65fddf --- /dev/null +++ b/charts/sourcegraph/tests/redisConfigMount_test.yaml @@ -0,0 +1,118 @@ +--- +suite: redisConfigMount +# The ConfigMap templates are listed so the Deployments' checksum annotation can +# include them. +templates: +- redis/redis-cache.Deployment.yaml +- redis/redis-store.Deployment.yaml +- redis/redis-cache.ConfigMap.yaml +- redis/redis-store.ConfigMap.yaml +tests: +- it: should mount the chart-managed ConfigMap over the image path for redis-cache + template: redis/redis-cache.Deployment.yaml + asserts: + - contains: + path: spec.template.spec.containers[0].volumeMounts + content: + mountPath: /etc/redis/redis.conf + name: redis-conf + subPath: redis.conf + readOnly: true + - contains: + path: spec.template.spec.volumes + content: + name: redis-conf + configMap: + name: redis-cache-conf + items: + - key: redis.conf + path: redis.conf + - isNotEmpty: + path: spec.template.metadata.annotations["checksum/redis-config"] + - notExists: + path: spec.template.spec.containers[0].command +- it: should mount the chart-managed ConfigMap over the image path for redis-store + template: redis/redis-store.Deployment.yaml + asserts: + - contains: + path: spec.template.spec.containers[0].volumeMounts + content: + mountPath: /etc/redis/redis.conf + name: redis-conf + subPath: redis.conf + readOnly: true + - contains: + path: spec.template.spec.volumes + content: + name: redis-conf + configMap: + name: redis-store-conf + items: + - key: redis.conf + path: redis.conf + - isNotEmpty: + path: spec.template.metadata.annotations["checksum/redis-config"] + - notExists: + path: spec.template.spec.containers[0].command +- it: should mount an existing ConfigMap and drop the checksum annotation + template: redis/redis-cache.Deployment.yaml + set: + redisCache: + config: + existingConfig: my-redis-conf + asserts: + - contains: + path: spec.template.spec.volumes + content: + name: redis-conf + configMap: + name: my-redis-conf + items: + - key: redis.conf + path: redis.conf + - notExists: + path: spec.template.metadata.annotations["checksum/redis-config"] +- it: should fail when redis-cache extraVolumeMounts collide with the chart-managed redis.conf + template: redis/redis-cache.Deployment.yaml + set: + redisCache: + extraVolumeMounts: + - name: my-conf + mountPath: /etc/redis/redis.conf + subPath: redis.conf + asserts: + - failedTemplate: + errorMessage: "redisCache.extraVolumeMounts must not mount over /etc/redis/redis.conf; the chart now manages that file. Move your custom redis config to redisCache.config.existingConfig or redisCache.config.additionalConfig." +- it: should fail when redis-store extraVolumeMounts mount the whole /etc/redis directory + template: redis/redis-store.Deployment.yaml + set: + redisStore: + extraVolumeMounts: + - name: my-conf + mountPath: /etc/redis + asserts: + - failedTemplate: + errorMessage: "redisStore.extraVolumeMounts must not mount over /etc/redis/redis.conf; the chart now manages that file. Move your custom redis config to redisStore.config.existingConfig or redisStore.config.additionalConfig." +- it: should fail when an extra volume reuses the reserved redis-conf name + template: redis/redis-cache.Deployment.yaml + set: + redisCache: + extraVolumes: + - name: redis-conf + emptyDir: {} + asserts: + - failedTemplate: + errorMessage: "redisCache must not define a volume named 'redis-conf'; the chart reserves that name for the redis config mount." +- it: should still allow unrelated extraVolumeMounts + template: redis/redis-cache.Deployment.yaml + set: + redisCache: + extraVolumeMounts: + - name: my-extra + mountPath: /somewhere-else + asserts: + - contains: + path: spec.template.spec.containers[0].volumeMounts + content: + name: my-extra + mountPath: /somewhere-else diff --git a/charts/sourcegraph/tests/redisConfig_test.yaml b/charts/sourcegraph/tests/redisConfig_test.yaml new file mode 100644 index 000000000..84526b953 --- /dev/null +++ b/charts/sourcegraph/tests/redisConfig_test.yaml @@ -0,0 +1,190 @@ +--- +suite: redisConfig +templates: +- redis/redis-cache.ConfigMap.yaml +- redis/redis-store.ConfigMap.yaml +tests: +- it: should render the vendored config with an auto-sized maxmemory by default + asserts: + - matchRegex: + path: data["redis.conf"] + pattern: "\ndir /redis-data/\n" + # 7Gi * 0.75 + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory 5637144576\n?$" +- it: should keep the vendored maxmemory-policy of each service + template: redis/redis-cache.ConfigMap.yaml + asserts: + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory-policy allkeys-lru\n" +- it: should keep the vendored maxmemory-policy for redis-store + template: redis/redis-store.ConfigMap.yaml + asserts: + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory-policy noeviction\n" +- it: should size maxmemory from a binary memory limit + set: + redisCache: + resources: + limits: + memory: 4Gi + redisStore: + resources: + limits: + memory: 512Mi + asserts: + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory (3221225472|402653184)\n?$" +- it: should size maxmemory from a decimal memory limit + set: + redisCache: + resources: + limits: + memory: 2G + redisStore: + resources: + limits: + memory: 2G + asserts: + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory 1500000000\n?$" +- it: should size maxmemory from a suffixless byte count + set: + redisCache: + resources: + limits: + memory: "6442450944" + redisStore: + resources: + limits: + memory: "6442450944" + asserts: + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory 4831838208\n?$" +- it: should size maxmemory from a fractional memory limit + set: + redisCache: + resources: + limits: + memory: 1.5Gi + redisStore: + resources: + limits: + memory: 1.5Gi + asserts: + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory 1207959552\n?$" +- it: should honour maxmemoryRatio + set: + redisCache: + config: + maxmemoryRatio: 0.5 + redisStore: + config: + maxmemoryRatio: 0.5 + asserts: + # 7Gi * 0.5 + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory 3758096384\n?$" +- it: should not append maxmemory when there is no memory limit + set: + redisCache: + resources: + limits: null + redisStore: + resources: + limits: null + asserts: + - notMatchRegex: + path: data["redis.conf"] + pattern: "Chart-managed directives" +- it: should not append maxmemory when the memory limit is unparseable + set: + redisCache: + resources: + limits: + memory: 7Gib + redisStore: + resources: + limits: + memory: notaquantity + asserts: + - notMatchRegex: + path: data["redis.conf"] + pattern: "Chart-managed directives" +- it: should use an explicit maxmemory verbatim + set: + redisCache: + config: + maxmemory: 6gb + redisStore: + config: + maxmemory: 6gb + asserts: + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory 6gb\n?$" +- it: should append maxmemory-policy and additionalConfig overrides + set: + redisCache: + config: + maxmemoryPolicy: volatile-lru + additionalConfig: | + lazyfree-lazy-eviction yes + redisStore: + config: + maxmemoryPolicy: volatile-lru + additionalConfig: | + lazyfree-lazy-eviction yes + asserts: + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory 5637144576\nmaxmemory-policy volatile-lru\nlazyfree-lazy-eviction yes\n?$" +- it: should not render the redis-cache ConfigMap when existingConfig is set + template: redis/redis-cache.ConfigMap.yaml + set: + redisCache: + config: + existingConfig: my-redis-conf + asserts: + - hasDocuments: + count: 0 +- it: should not render the redis-store ConfigMap when existingConfig is set + template: redis/redis-store.ConfigMap.yaml + set: + redisStore: + config: + existingConfig: my-redis-conf + asserts: + - hasDocuments: + count: 0 +- it: should fail when both redisCache existingConfig and additionalConfig are defined + template: redis/redis-cache.ConfigMap.yaml + set: + redisCache: + config: + existingConfig: i-will-break-rendering + additionalConfig: | + maxmemory 1gb + asserts: + - failedTemplate: + errorMessage: You can only define one of 'redisCache.config.existingConfig' and 'redisCache.config.additionalConfig' at a time +- it: should fail when both redisStore existingConfig and additionalConfig are defined + template: redis/redis-store.ConfigMap.yaml + set: + redisStore: + config: + existingConfig: i-will-break-rendering + additionalConfig: | + maxmemory 1gb + asserts: + - failedTemplate: + errorMessage: You can only define one of 'redisStore.config.existingConfig' and 'redisStore.config.additionalConfig' at a time diff --git a/charts/sourcegraph/values.yaml b/charts/sourcegraph/values.yaml index d260444e4..8cb4954ca 100644 --- a/charts/sourcegraph/values.yaml +++ b/charts/sourcegraph/values.yaml @@ -1037,6 +1037,24 @@ redisCache: storageSubPath: "" # -- Optional annotations to add to the `redis-cache` PVC storageAnnotations: {} + config: + # -- Name of an existing ConfigMap for `redis-cache`. It must contain a `redis.conf` key. + # When set, the chart-managed ConfigMap is not rendered and this one is mounted instead, + # so the chart no longer sizes `maxmemory`. Mutually exclusive with `additionalConfig`. + existingConfig: "" + # -- Explicit redis `maxmemory` for `redis-cache` (for example `6gb`). Overrides the + # auto-computed value. Empty means compute it from the container memory limit. + maxmemory: "" + # -- Fraction of `redisCache.resources.limits.memory` used for `maxmemory` when + # `maxmemory` is empty. Ignored when no memory limit is set. + maxmemoryRatio: 0.75 + # -- Override the redis `maxmemory-policy` for `redis-cache`. + # Empty keeps the vendored default (`allkeys-lru`). + maxmemoryPolicy: "" + # -- Additional raw redis directives appended to the vendored `redis-cache` config. + # Notes: This is expecting a multiline string. It renders into a ConfigMap in plaintext, + # so do not put secrets such as `requirepass` here; use `existingConfig` instead. + additionalConfig: "" redisExporter: image: @@ -1110,6 +1128,24 @@ redisStore: storageSubPath: "" # -- Optional annotations to add to the `redis-store` PVC storageAnnotations: {} + config: + # -- Name of an existing ConfigMap for `redis-store`. It must contain a `redis.conf` key. + # When set, the chart-managed ConfigMap is not rendered and this one is mounted instead, + # so the chart no longer sizes `maxmemory`. Mutually exclusive with `additionalConfig`. + existingConfig: "" + # -- Explicit redis `maxmemory` for `redis-store` (for example `6gb`). Overrides the + # auto-computed value. Empty means compute it from the container memory limit. + maxmemory: "" + # -- Fraction of `redisStore.resources.limits.memory` used for `maxmemory` when + # `maxmemory` is empty. Ignored when no memory limit is set. + maxmemoryRatio: 0.75 + # -- Override the redis `maxmemory-policy` for `redis-store`. + # Empty keeps the vendored default (`noeviction`). + maxmemoryPolicy: "" + # -- Additional raw redis directives appended to the vendored `redis-store` config. + # Notes: This is expecting a multiline string. It renders into a ConfigMap in plaintext, + # so do not put secrets such as `requirepass` here; use `existingConfig` instead. + additionalConfig: "" searcher: image: From 4bdb35303f7d528310c0ad3598888506089c7c63 Mon Sep 17 00:00:00 2001 From: Michael Lin Date: Tue, 22 Sep 2026 03:47:08 +0000 Subject: [PATCH 2/3] Finish Redis sizing validation and config migration support Amp-Thread-ID: https://ampcode.com/threads/T-01a0c732-cf8b-71ae-a2af-40664eafab0a Co-authored-by: Amp --- charts/sourcegraph/CHANGELOG.md | 4 +- charts/sourcegraph/README.md | 51 +++++++- charts/sourcegraph/README.md.gotmpl | 41 +++++++ charts/sourcegraph/templates/_helpers.tpl | 14 ++- .../redis/redis-cache.ConfigMap.yaml | 6 +- .../redis/redis-cache.Deployment.yaml | 8 +- .../redis/redis-store.ConfigMap.yaml | 6 +- .../redis/redis-store.Deployment.yaml | 8 +- .../tests/redisConfigMount_test.yaml | 93 ++++++++++++++- .../sourcegraph/tests/redisConfig_test.yaml | 112 +++++++++++++++++- charts/sourcegraph/values.yaml | 22 +++- 11 files changed, 342 insertions(+), 23 deletions(-) diff --git a/charts/sourcegraph/CHANGELOG.md b/charts/sourcegraph/CHANGELOG.md index 69203b8ca..0d04287ab 100644 --- a/charts/sourcegraph/CHANGELOG.md +++ b/charts/sourcegraph/CHANGELOG.md @@ -8,8 +8,8 @@ Use `**BREAKING**:` to denote a breaking change ## Unreleased -- **BREAKING**: The chart now manages `/etc/redis/redis.conf` for `redis-cache` and `redis-store`. It vendors the config that the images bake in (`charts/sourcegraph/files/redis-{cache,store}/redis.conf`), renders it into a `redis-{cache,store}-conf` ConfigMap and mounts it over the image path. Operators who inject their own `redis.conf` through `redisCache.extraVolumeMounts` / `redisStore.extraVolumeMounts` must move to the new `redisCache.config.existingConfig` / `redisStore.config.existingConfig`; the chart fails the render with a migration message otherwise. -- Added `redisCache.config` and `redisStore.config` (`existingConfig`, `maxmemory`, `maxmemoryRatio`, `maxmemoryPolicy`, `additionalConfig`). By default the chart sizes redis `maxmemory` at `maxmemoryRatio` (0.75) of `resources.limits.memory` instead of leaving the images' hardcoded `maxmemory 6gb`, so lowering the memory limit no longer OOM-kills the pod before eviction runs. At the default 7Gi limit this lowers the effective cap from 6GiB to 5.25GiB: `redis-cache` only evicts earlier, but `redis-store` uses `noeviction`, so its write-error ceiling drops too. Set `redisStore.config.maxmemory: 6gb` or raise `redisStore.config.maxmemoryRatio` to keep the old ceiling. +- **BREAKING**: The chart now manages `/etc/redis/redis.conf` for `redis-cache` and `redis-store`, replacing image-baked configuration, including any custom authentication settings. Operators with custom images or config mounts must either set the corresponding `redisCache.config.enabled` / `redisStore.config.enabled` to `false` to preserve the old behavior, or migrate to the new config options. Conflicting `extraVolumeMounts` fail rendering with a migration message. See [Upgrading Redis configuration](README.md#upgrading-redis-configuration), including guidance for Secret-mounted configs. +- Added `redisCache.config` and `redisStore.config` (`enabled`, `existingConfig`, `maxmemory`, `maxmemoryRatio`, `maxmemoryPolicy`, `additionalConfig`). By default the chart sizes Redis `maxmemory` at 75% of `resources.limits.memory` to leave room for overhead; this reduces, but does not eliminate, OOM risk. Ratios must be greater than 0 and less than 1. Auto-sizing is skipped in local development mode or when the limit is absent or unrecognised. At the default 7Gi limit the effective cap drops from 6GiB to 5.25GiB: `redis-cache` evicts earlier, but `redis-store` uses `noeviction`, so its write-error ceiling drops too. Set `redisStore.config.maxmemory: 6gb` to keep the old ceiling if the pod has sufficient memory. - Added a `network-policy` example, which limits Executor and Executor job pods to the frontend API - Corrected the external object storage examples to configure the shared store for frontend, worker, precise code intel, syntactic code intel, gitserver, and searcher, including credentials or workload service accounts as required. - Removed the unused application ports from the precise and syntactic code intel worker Deployments and Services; health checks and Prometheus metrics continue to use the debug server on port 6060. diff --git a/charts/sourcegraph/README.md b/charts/sourcegraph/README.md index 29359820c..ad364757d 100644 --- a/charts/sourcegraph/README.md +++ b/charts/sourcegraph/README.md @@ -7,6 +7,47 @@ Visit the [Helm docs](https://docs.sourcegraph.com/admin/install/kubernetes) for guidance on using this chart. +## Upgrading Redis configuration + +The chart now mounts `/etc/redis/redis.conf` for both Redis services and sizes +`maxmemory` to 75% of each container's memory limit. At the default 7Gi limit, +this changes the cap from 6GiB to 5.25GiB. `redis-cache` evicts earlier; +`redis-store` keeps `noeviction` and rejects writes sooner when full. Set +`redisStore.config.maxmemory: 6gb` to retain its old cap, provided the pod has +enough memory for Redis overhead. Auto-sizing reserves headroom but cannot +guarantee against OOM kills, particularly during persistence operations. + +Before upgrading: + +- **Custom images:** the mounted config replaces the image's config, including + any `requirepass`, ACL, TLS, or persistence settings. Set + `redisCache.config.enabled: false` and/or `redisStore.config.enabled: false` + to preserve the corresponding image config. Otherwise, migrate those settings + explicitly; image-baked authentication is not inherited. +- **Existing config mounts:** either disable config management as above and + keep your `extraVolumes` / `extraVolumeMounts`, or remove those mounts and set + `config.existingConfig` to the name of a ConfigMap with a complete `redis.conf` + key. An existing ConfigMap bypasses all chart sizing and directive overrides. +- **Secrets:** both `additionalConfig` and `existingConfig` use plaintext + ConfigMaps. For a config containing credentials, disable config management + and mount a Secret at `/etc/redis/redis.conf` with `extraVolumeMounts` instead. + Configure the clients' `connection.existingSecret` and the exporter's + `redisExporter.env.REDIS_PASSWORD.valueFrom.secretKeyRef` as appropriate. + Redis readiness alone does not verify exporter authentication; check `redis_up`. + +Disabling config management disables all other `config` options, including +auto-sizing, so you must size Redis memory yourself. With management enabled, +an explicit `config.maxmemory` takes precedence over auto-sizing, and +`additionalConfig` is appended last. Supported memory limits are plain byte +counts or numbers with `k`, `M`, `G`, `T`, `P`, `E`, `Ki`, `Mi`, `Gi`, `Ti`, +`Pi`, or `Ei` suffixes. If the limit is absent or unrecognised, or +`sourcegraph.localDevMode` is enabled, the vendored 6gb default remains unless +overridden explicitly. + +Chart-managed config changes restart the Redis pod through a checksum +annotation. Changes to an existing ConfigMap or Secret require a manual pod +restart because `subPath` mounts do not update in running containers. + ## Configuration Options Reference the below chart for all available configuration parameters. @@ -301,11 +342,12 @@ In addition to the documented values, all services also support the following va | prometheus.storageAnnotations | object | `{}` | Optional annotations to add to the `prometheus` PVC | | prometheus.storageSize | string | `"200Gi"` | PVC Storage Request for `prometheus` data volume | | prometheus.storageSubPath | string | `""` | Optional subPath for the `prometheus` primary data volume mount | -| redisCache.config.additionalConfig | string | `""` | Additional raw redis directives appended to the vendored `redis-cache` config. Notes: This is expecting a multiline string. It renders into a ConfigMap in plaintext, so do not put secrets such as `requirepass` here; use `existingConfig` instead. | +| redisCache.config.additionalConfig | string | `""` | Additional raw redis directives appended to the vendored `redis-cache` config. Notes: This is expecting a multiline string. It renders into a ConfigMap in plaintext, so do not put secrets such as `requirepass` here or in `existingConfig` (also a ConfigMap). For secrets, set `config.enabled: false` and mount a Secret using `extraVolumeMounts`. | +| redisCache.config.enabled | bool | `true` | Mount a chart-managed Redis config. Set to false to preserve image-baked configuration or custom `extraVolumeMounts` (including Secret mounts). When false, all other `config` options are ignored and auto-sizing is disabled. | | redisCache.config.existingConfig | string | `""` | Name of an existing ConfigMap for `redis-cache`. It must contain a `redis.conf` key. When set, the chart-managed ConfigMap is not rendered and this one is mounted instead, so the chart no longer sizes `maxmemory`. Mutually exclusive with `additionalConfig`. | | redisCache.config.maxmemory | string | `""` | Explicit redis `maxmemory` for `redis-cache` (for example `6gb`). Overrides the auto-computed value. Empty means compute it from the container memory limit. | | redisCache.config.maxmemoryPolicy | string | `""` | Override the redis `maxmemory-policy` for `redis-cache`. Empty keeps the vendored default (`allkeys-lru`). | -| redisCache.config.maxmemoryRatio | float | `0.75` | Fraction of `redisCache.resources.limits.memory` used for `maxmemory` when `maxmemory` is empty. Ignored when no memory limit is set. | +| redisCache.config.maxmemoryRatio | float | `0.75` | Fraction of `redisCache.resources.limits.memory` used for `maxmemory` when `maxmemory` is empty. Must be greater than 0 and less than 1. Ignored when no memory limit is set or `sourcegraph.localDevMode` is enabled. An absent or unrecognised limit keeps the vendored 6gb default; use an explicit maxmemory if needed. | | redisCache.connection.endpoint | string | `"redis-cache:6379"` | Endpoint to use for redis-cache. Supports either host:port or IANA specification | | redisCache.connection.existingSecret | string | `""` | Name of existing secret to use for Redis endpoint The secret must contain the key `endpoint` and should follow IANA specification learn more from the [Helm docs](https://docs.sourcegraph.com/admin/install/kubernetes/helm#using-external-redis-instances) | | redisCache.containerSecurityContext | object | `{"allowPrivilegeEscalation":false,"readOnlyRootFilesystem":true,"runAsGroup":1000,"runAsUser":999}` | Security context for the `redis-cache` container, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-container) | @@ -325,11 +367,12 @@ In addition to the documented values, all services also support the following va | redisExporter.image.defaultTag | string | `"6.0.0@sha256:b2ec48fc6adef31f36d525170138dec303c1c0c20c530d659f1fb7c6c54698af"` | Docker image tag for the `redis-exporter` image | | redisExporter.image.name | string | `"redis_exporter"` | Docker image name for the `redis-exporter` image | | redisExporter.resources | object | `{"limits":{"cpu":"10m","memory":"100Mi"},"requests":{"cpu":"10m","memory":"100Mi"}}` | Resource requests & limits for the `redis-exporter` sidecar container, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/) | -| redisStore.config.additionalConfig | string | `""` | Additional raw redis directives appended to the vendored `redis-store` config. Notes: This is expecting a multiline string. It renders into a ConfigMap in plaintext, so do not put secrets such as `requirepass` here; use `existingConfig` instead. | +| redisStore.config.additionalConfig | string | `""` | Additional raw redis directives appended to the vendored `redis-store` config. Notes: This is expecting a multiline string. It renders into a ConfigMap in plaintext, so do not put secrets such as `requirepass` here or in `existingConfig` (also a ConfigMap). For secrets, set `config.enabled: false` and mount a Secret using `extraVolumeMounts`. | +| redisStore.config.enabled | bool | `true` | Mount a chart-managed Redis config. Set to false to preserve image-baked configuration or custom `extraVolumeMounts` (including Secret mounts). When false, all other `config` options are ignored and auto-sizing is disabled. | | redisStore.config.existingConfig | string | `""` | Name of an existing ConfigMap for `redis-store`. It must contain a `redis.conf` key. When set, the chart-managed ConfigMap is not rendered and this one is mounted instead, so the chart no longer sizes `maxmemory`. Mutually exclusive with `additionalConfig`. | | redisStore.config.maxmemory | string | `""` | Explicit redis `maxmemory` for `redis-store` (for example `6gb`). Overrides the auto-computed value. Empty means compute it from the container memory limit. | | redisStore.config.maxmemoryPolicy | string | `""` | Override the redis `maxmemory-policy` for `redis-store`. Empty keeps the vendored default (`noeviction`). | -| redisStore.config.maxmemoryRatio | float | `0.75` | Fraction of `redisStore.resources.limits.memory` used for `maxmemory` when `maxmemory` is empty. Ignored when no memory limit is set. | +| redisStore.config.maxmemoryRatio | float | `0.75` | Fraction of `redisStore.resources.limits.memory` used for `maxmemory` when `maxmemory` is empty. Must be greater than 0 and less than 1. Ignored when no memory limit is set or `sourcegraph.localDevMode` is enabled. An absent or unrecognised limit keeps the vendored 6gb default; use an explicit maxmemory if needed. | | redisStore.connection.endpoint | string | `"redis-store:6379"` | Endpoint to use for redis-store. Supports either host:port or IANA specification | | redisStore.connection.existingSecret | string | `""` | Name of existing secret to use for Redis endpoint The secret must contain the key `endpoint` and should follow IANA specification learn more from the [Helm docs](https://docs.sourcegraph.com/admin/install/kubernetes/helm#using-external-redis-instances) | | redisStore.containerSecurityContext | object | `{"allowPrivilegeEscalation":false,"readOnlyRootFilesystem":true,"runAsGroup":1000,"runAsUser":999}` | Security context for the `redis-store` container, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-container) | diff --git a/charts/sourcegraph/README.md.gotmpl b/charts/sourcegraph/README.md.gotmpl index 06ed43c99..2e2dfa37e 100644 --- a/charts/sourcegraph/README.md.gotmpl +++ b/charts/sourcegraph/README.md.gotmpl @@ -7,6 +7,47 @@ Visit the [Helm docs](https://docs.sourcegraph.com/admin/install/kubernetes) for guidance on using this chart. +## Upgrading Redis configuration + +The chart now mounts `/etc/redis/redis.conf` for both Redis services and sizes +`maxmemory` to 75% of each container's memory limit. At the default 7Gi limit, +this changes the cap from 6GiB to 5.25GiB. `redis-cache` evicts earlier; +`redis-store` keeps `noeviction` and rejects writes sooner when full. Set +`redisStore.config.maxmemory: 6gb` to retain its old cap, provided the pod has +enough memory for Redis overhead. Auto-sizing reserves headroom but cannot +guarantee against OOM kills, particularly during persistence operations. + +Before upgrading: + +- **Custom images:** the mounted config replaces the image's config, including + any `requirepass`, ACL, TLS, or persistence settings. Set + `redisCache.config.enabled: false` and/or `redisStore.config.enabled: false` + to preserve the corresponding image config. Otherwise, migrate those settings + explicitly; image-baked authentication is not inherited. +- **Existing config mounts:** either disable config management as above and + keep your `extraVolumes` / `extraVolumeMounts`, or remove those mounts and set + `config.existingConfig` to the name of a ConfigMap with a complete `redis.conf` + key. An existing ConfigMap bypasses all chart sizing and directive overrides. +- **Secrets:** both `additionalConfig` and `existingConfig` use plaintext + ConfigMaps. For a config containing credentials, disable config management + and mount a Secret at `/etc/redis/redis.conf` with `extraVolumeMounts` instead. + Configure the clients' `connection.existingSecret` and the exporter's + `redisExporter.env.REDIS_PASSWORD.valueFrom.secretKeyRef` as appropriate. + Redis readiness alone does not verify exporter authentication; check `redis_up`. + +Disabling config management disables all other `config` options, including +auto-sizing, so you must size Redis memory yourself. With management enabled, +an explicit `config.maxmemory` takes precedence over auto-sizing, and +`additionalConfig` is appended last. Supported memory limits are plain byte +counts or numbers with `k`, `M`, `G`, `T`, `P`, `E`, `Ki`, `Mi`, `Gi`, `Ti`, +`Pi`, or `Ei` suffixes. If the limit is absent or unrecognised, or +`sourcegraph.localDevMode` is enabled, the vendored 6gb default remains unless +overridden explicitly. + +Chart-managed config changes restart the Redis pod through a checksum +annotation. Changes to an existing ConfigMap or Secret require a manual pod +restart because `subPath` mounts do not update in running containers. + ## Configuration Options Reference the below chart for all available configuration parameters. diff --git a/charts/sourcegraph/templates/_helpers.tpl b/charts/sourcegraph/templates/_helpers.tpl index 57b7d9bd8..fa1af3b4e 100644 --- a/charts/sourcegraph/templates/_helpers.tpl +++ b/charts/sourcegraph/templates/_helpers.tpl @@ -320,7 +320,7 @@ Usage: include "sourcegraph.redis.maxmemory" (list . "redisCache") Resolution order: 1. .config.maxmemory, used verbatim. 2. floor(.config.maxmemoryRatio * .resources.limits.memory), - rendered as a plain byte count. + rendered as a plain byte count, unless localDevMode removes the limit. 3. Empty string, when there is no memory limit or the quantity is not recognised. The caller then emits no `maxmemory` and the vendored default stands. @@ -332,14 +332,20 @@ Resolution order: {{- $config := $values.config | default dict -}} {{- if $config.maxmemory -}} {{- $config.maxmemory -}} -{{- else -}} +{{- else if not $top.Values.sourcegraph.localDevMode -}} {{- $limit := dig "resources" "limits" "memory" "" $values | toString -}} {{- $number := regexReplaceAll "^([0-9]+(\\.[0-9]+)?).*$" $limit "${1}" -}} {{- $suffix := regexReplaceAll "^[0-9]+(\\.[0-9]+)?" $limit "" -}} {{- /* Kubernetes quantity suffixes: binary (1024^n) and decimal (1000^n). */ -}} {{- $units := dict "" 1.0 "k" 1e3 "M" 1e6 "G" 1e9 "T" 1e12 "P" 1e15 "E" 1e18 "Ki" 1024.0 "Mi" 1048576.0 "Gi" 1073741824.0 "Ti" 1099511627776.0 "Pi" 1125899906842624.0 "Ei" 1152921504606846976.0 -}} {{- if and (regexMatch "^[0-9]+(\\.[0-9]+)?$" $number) (hasKey $units $suffix) -}} -{{- $ratio := $config.maxmemoryRatio | default 0.75 | float64 -}} +{{- $ratio := 0.75 -}} +{{- if hasKey $config "maxmemoryRatio" -}} +{{- $ratio = float64 $config.maxmemoryRatio -}} +{{- end -}} +{{- if not (and (gt $ratio 0.0) (lt $ratio 1.0)) -}} +{{- fail (printf "%s.config.maxmemoryRatio must be greater than 0 and less than 1" $service) -}} +{{- end -}} {{- $bytes := floor (mulf (float64 $number) (index $units $suffix) $ratio) -}} {{- if gt $bytes 0.0 -}} {{- printf "%d" (int64 $bytes) -}} @@ -360,7 +366,7 @@ Usage: include "sourcegraph.redis.assertNoConfClash" (list . "redisCache") {{- $values := index $top.Values $service -}} {{- range ($values.extraVolumeMounts | default list) -}} {{- if has .mountPath (list "/etc/redis/redis.conf" "/etc/redis" "/etc/redis/") -}} -{{- fail (printf "%s.extraVolumeMounts must not mount over /etc/redis/redis.conf; the chart now manages that file. Move your custom redis config to %s.config.existingConfig or %s.config.additionalConfig." $service $service $service) -}} +{{- fail (printf "%s.extraVolumeMounts must not mount over /etc/redis/redis.conf; the chart now manages that file. Move your custom redis config to %s.config.existingConfig or %s.config.additionalConfig, or set %s.config.enabled=false to preserve your existing mount." $service $service $service $service) -}} {{- end -}} {{- end -}} {{- range (concat ($values.extraVolumes | default list) ($values.extraVolumeMounts | default list)) -}} diff --git a/charts/sourcegraph/templates/redis/redis-cache.ConfigMap.yaml b/charts/sourcegraph/templates/redis/redis-cache.ConfigMap.yaml index ffe6cf9b2..bfecbc7e4 100644 --- a/charts/sourcegraph/templates/redis/redis-cache.ConfigMap.yaml +++ b/charts/sourcegraph/templates/redis/redis-cache.ConfigMap.yaml @@ -1,7 +1,8 @@ -{{- if and .Values.redisCache.enabled (and .Values.redisCache.config.existingConfig .Values.redisCache.config.additionalConfig) -}} +{{- if and .Values.redisCache.enabled .Values.redisCache.config.enabled -}} +{{- if and .Values.redisCache.config.existingConfig .Values.redisCache.config.additionalConfig -}} {{- fail "You can only define one of 'redisCache.config.existingConfig' and 'redisCache.config.additionalConfig' at a time" }} {{- end }} -{{- if and .Values.redisCache.enabled (not .Values.redisCache.config.existingConfig) -}} +{{- 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") -}} @@ -33,3 +34,4 @@ data: redis.conf: | {{- $body | nindent 4 }} {{- end }} +{{- end }} diff --git a/charts/sourcegraph/templates/redis/redis-cache.Deployment.yaml b/charts/sourcegraph/templates/redis/redis-cache.Deployment.yaml index bade1a6e2..1ea57e114 100644 --- a/charts/sourcegraph/templates/redis/redis-cache.Deployment.yaml +++ b/charts/sourcegraph/templates/redis/redis-cache.Deployment.yaml @@ -1,5 +1,7 @@ {{- if .Values.redisCache.enabled -}} +{{- if .Values.redisCache.config.enabled -}} {{- include "sourcegraph.redis.assertNoConfClash" (list . "redisCache") -}} +{{- end -}} apiVersion: apps/v1 kind: Deployment metadata: @@ -27,7 +29,7 @@ spec: metadata: annotations: kubectl.kubernetes.io/default-container: redis-cache - {{- if not .Values.redisCache.config.existingConfig }} + {{- if and .Values.redisCache.config.enabled (not .Values.redisCache.config.existingConfig) }} checksum/redis-config: {{ include (print $.Template.BasePath "/redis/redis-cache.ConfigMap.yaml") . | sha256sum }} {{- end }} {{- if .Values.sourcegraph.podAnnotations }} @@ -104,10 +106,12 @@ spec: {{- if .Values.redisCache.storageSubPath }} subPath: {{ .Values.redisCache.storageSubPath }} {{- end }} + {{- if .Values.redisCache.config.enabled }} - mountPath: /etc/redis/redis.conf name: redis-conf subPath: redis.conf readOnly: true + {{- end }} {{- if .Values.redisCache.extraVolumeMounts }} {{- toYaml .Values.redisCache.extraVolumeMounts | nindent 8 }} {{- end }} @@ -147,12 +151,14 @@ spec: - name: redis-data persistentVolumeClaim: claimName: redis-cache + {{- if .Values.redisCache.config.enabled }} - name: redis-conf configMap: name: {{ default (print .Values.redisCache.name "-conf") .Values.redisCache.config.existingConfig }} items: - key: redis.conf path: redis.conf + {{- end }} {{- if .Values.redisCache.extraVolumes }} {{- toYaml .Values.redisCache.extraVolumes | nindent 6 }} {{- end }} diff --git a/charts/sourcegraph/templates/redis/redis-store.ConfigMap.yaml b/charts/sourcegraph/templates/redis/redis-store.ConfigMap.yaml index a921fdee8..6cf4246c7 100644 --- a/charts/sourcegraph/templates/redis/redis-store.ConfigMap.yaml +++ b/charts/sourcegraph/templates/redis/redis-store.ConfigMap.yaml @@ -1,7 +1,8 @@ -{{- if and .Values.redisStore.enabled (and .Values.redisStore.config.existingConfig .Values.redisStore.config.additionalConfig) -}} +{{- if and .Values.redisStore.enabled .Values.redisStore.config.enabled -}} +{{- if and .Values.redisStore.config.existingConfig .Values.redisStore.config.additionalConfig -}} {{- fail "You can only define one of 'redisStore.config.existingConfig' and 'redisStore.config.additionalConfig' at a time" }} {{- end }} -{{- if and .Values.redisStore.enabled (not .Values.redisStore.config.existingConfig) -}} +{{- 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") -}} @@ -33,3 +34,4 @@ data: redis.conf: | {{- $body | nindent 4 }} {{- end }} +{{- end }} diff --git a/charts/sourcegraph/templates/redis/redis-store.Deployment.yaml b/charts/sourcegraph/templates/redis/redis-store.Deployment.yaml index 9ab9e7c21..e557c4ebb 100644 --- a/charts/sourcegraph/templates/redis/redis-store.Deployment.yaml +++ b/charts/sourcegraph/templates/redis/redis-store.Deployment.yaml @@ -1,5 +1,7 @@ {{- if .Values.redisStore.enabled -}} +{{- if .Values.redisStore.config.enabled -}} {{- include "sourcegraph.redis.assertNoConfClash" (list . "redisStore") -}} +{{- end -}} apiVersion: apps/v1 kind: Deployment metadata: @@ -27,7 +29,7 @@ spec: metadata: annotations: kubectl.kubernetes.io/default-container: redis-store - {{- if not .Values.redisStore.config.existingConfig }} + {{- if and .Values.redisStore.config.enabled (not .Values.redisStore.config.existingConfig) }} checksum/redis-config: {{ include (print $.Template.BasePath "/redis/redis-store.ConfigMap.yaml") . | sha256sum }} {{- end }} {{- if .Values.sourcegraph.podAnnotations }} @@ -103,10 +105,12 @@ spec: {{- if .Values.redisStore.storageSubPath }} subPath: {{ .Values.redisStore.storageSubPath }} {{- end }} + {{- if .Values.redisStore.config.enabled }} - mountPath: /etc/redis/redis.conf name: redis-conf subPath: redis.conf readOnly: true + {{- end }} {{- if .Values.redisStore.extraVolumeMounts }} {{- toYaml .Values.redisStore.extraVolumeMounts | nindent 8 }} {{- end }} @@ -146,12 +150,14 @@ spec: - name: redis-data persistentVolumeClaim: claimName: redis-store + {{- if .Values.redisStore.config.enabled }} - name: redis-conf configMap: name: {{ default (print .Values.redisStore.name "-conf") .Values.redisStore.config.existingConfig }} items: - key: redis.conf path: redis.conf + {{- end }} {{- if .Values.redisStore.extraVolumes }} {{- toYaml .Values.redisStore.extraVolumes | nindent 6 }} {{- end }} diff --git a/charts/sourcegraph/tests/redisConfigMount_test.yaml b/charts/sourcegraph/tests/redisConfigMount_test.yaml index 01a65fddf..b2557a011 100644 --- a/charts/sourcegraph/tests/redisConfigMount_test.yaml +++ b/charts/sourcegraph/tests/redisConfigMount_test.yaml @@ -82,7 +82,7 @@ tests: subPath: redis.conf asserts: - failedTemplate: - errorMessage: "redisCache.extraVolumeMounts must not mount over /etc/redis/redis.conf; the chart now manages that file. Move your custom redis config to redisCache.config.existingConfig or redisCache.config.additionalConfig." + errorMessage: "redisCache.extraVolumeMounts must not mount over /etc/redis/redis.conf; the chart now manages that file. Move your custom redis config to redisCache.config.existingConfig or redisCache.config.additionalConfig, or set redisCache.config.enabled=false to preserve your existing mount." - it: should fail when redis-store extraVolumeMounts mount the whole /etc/redis directory template: redis/redis-store.Deployment.yaml set: @@ -92,7 +92,7 @@ tests: mountPath: /etc/redis asserts: - failedTemplate: - errorMessage: "redisStore.extraVolumeMounts must not mount over /etc/redis/redis.conf; the chart now manages that file. Move your custom redis config to redisStore.config.existingConfig or redisStore.config.additionalConfig." + errorMessage: "redisStore.extraVolumeMounts must not mount over /etc/redis/redis.conf; the chart now manages that file. Move your custom redis config to redisStore.config.existingConfig or redisStore.config.additionalConfig, or set redisStore.config.enabled=false to preserve your existing mount." - it: should fail when an extra volume reuses the reserved redis-conf name template: redis/redis-cache.Deployment.yaml set: @@ -116,3 +116,92 @@ tests: content: name: my-extra mountPath: /somewhere-else +- it: should preserve the image config when management is disabled + set: + redisCache.config.enabled: false + redisStore.config.enabled: false + asserts: + - equal: + path: spec.template.spec.containers[0].volumeMounts + value: + - mountPath: /redis-data + name: redis-data + template: redis/redis-cache.Deployment.yaml + - equal: + path: spec.template.spec.containers[0].volumeMounts + value: + - mountPath: /redis-data + name: redis-data + template: redis/redis-store.Deployment.yaml + - lengthEqual: + path: spec.template.spec.volumes + count: 1 + template: redis/redis-cache.Deployment.yaml + - lengthEqual: + path: spec.template.spec.volumes + count: 1 + template: redis/redis-store.Deployment.yaml + - notExists: + path: spec.template.metadata.annotations["checksum/redis-config"] + template: redis/redis-cache.Deployment.yaml + - notExists: + path: spec.template.metadata.annotations["checksum/redis-config"] + template: redis/redis-store.Deployment.yaml +- it: should preserve a Secret-mounted config when management is disabled + set: + redisCache.config.enabled: false + redisStore.config.enabled: false + redisCache.extraVolumeMounts: &mounts + - name: redis-conf + mountPath: /etc/redis/redis.conf + subPath: redis.conf + readOnly: true + redisStore.extraVolumeMounts: *mounts + redisCache.extraVolumes: &volumes + - name: redis-conf + secret: + secretName: redis-auth-config + redisStore.extraVolumes: *volumes + asserts: + - equal: + path: spec.template.spec.containers[0].volumeMounts + value: + - mountPath: /redis-data + name: redis-data + - name: redis-conf + mountPath: /etc/redis/redis.conf + subPath: redis.conf + readOnly: true + template: redis/redis-cache.Deployment.yaml + - equal: + path: spec.template.spec.containers[0].volumeMounts + value: + - mountPath: /redis-data + name: redis-data + - name: redis-conf + mountPath: /etc/redis/redis.conf + subPath: redis.conf + readOnly: true + template: redis/redis-store.Deployment.yaml + - equal: + path: spec.template.spec.volumes[1] + value: + name: redis-conf + secret: + secretName: redis-auth-config + template: redis/redis-cache.Deployment.yaml + - equal: + path: spec.template.spec.volumes[1] + value: + name: redis-conf + secret: + secretName: redis-auth-config + template: redis/redis-store.Deployment.yaml + - lengthEqual: + path: spec.template.spec.volumes + count: 2 + template: redis/redis-cache.Deployment.yaml + - lengthEqual: + path: spec.template.spec.volumes + count: 2 + template: redis/redis-store.Deployment.yaml diff --git a/charts/sourcegraph/tests/redisConfig_test.yaml b/charts/sourcegraph/tests/redisConfig_test.yaml index 84526b953..0450fa26f 100644 --- a/charts/sourcegraph/tests/redisConfig_test.yaml +++ b/charts/sourcegraph/tests/redisConfig_test.yaml @@ -38,7 +38,12 @@ tests: asserts: - matchRegex: path: data["redis.conf"] - pattern: "\nmaxmemory (3221225472|402653184)\n?$" + pattern: "\nmaxmemory 3221225472\n?$" + template: redis/redis-cache.ConfigMap.yaml + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory 402653184\n?$" + template: redis/redis-store.ConfigMap.yaml - it: should size maxmemory from a decimal memory limit set: redisCache: @@ -188,3 +193,108 @@ tests: asserts: - failedTemplate: errorMessage: You can only define one of 'redisStore.config.existingConfig' and 'redisStore.config.additionalConfig' at a time +- it: should reject a zero ratio instead of silently defaulting it + template: redis/redis-cache.ConfigMap.yaml + set: + redisCache.config.maxmemoryRatio: 0 + asserts: + - failedTemplate: + errorMessage: redisCache.config.maxmemoryRatio must be greater than 0 and less than 1 +- it: should reject a negative ratio instead of reverting to 6gb + template: redis/redis-store.ConfigMap.yaml + set: + redisStore.config.maxmemoryRatio: -0.5 + asserts: + - failedTemplate: + errorMessage: redisStore.config.maxmemoryRatio must be greater than 0 and less than 1 +- it: should reject a ratio of one to reserve memory for Redis overhead + template: redis/redis-cache.ConfigMap.yaml + set: + redisCache.config.maxmemoryRatio: 1 + asserts: + - failedTemplate: + errorMessage: redisCache.config.maxmemoryRatio must be greater than 0 and less than 1 +- it: should reject a ratio greater than one + template: redis/redis-store.ConfigMap.yaml + set: + redisStore.config.maxmemoryRatio: 1.5 + asserts: + - failedTemplate: + errorMessage: redisStore.config.maxmemoryRatio must be greater than 0 and less than 1 +- it: should reject a nonnumeric ratio + template: redis/redis-cache.ConfigMap.yaml + set: + redisCache.config.maxmemoryRatio: invalid + asserts: + - failedTemplate: + errorMessage: redisCache.config.maxmemoryRatio must be greater than 0 and less than 1 +- it: should floor fractional bytes and accept ratios near either boundary + set: + redisCache.resources.limits.memory: "1001" + redisStore.resources.limits.memory: "1001" + redisCache.config.maxmemoryRatio: 0.01 + redisStore.config.maxmemoryRatio: 0.99 + asserts: + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory 10\n?$" + template: redis/redis-cache.ConfigMap.yaml + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory 990\n?$" + template: redis/redis-store.ConfigMap.yaml +- it: should keep the vendored limit in local development mode + set: + sourcegraph.localDevMode: true + redisCache.resources.limits.memory: 512Mi + redisStore.resources.limits.memory: 1Gi + asserts: + - notMatchRegex: + path: data["redis.conf"] + pattern: "Chart-managed directives" + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory 6gb\n" +- it: should honour an explicit limit even in local development mode + set: + sourcegraph.localDevMode: true + redisCache.config.maxmemory: 256mb + redisStore.config.maxmemory: 256mb + asserts: + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory 256mb\n?$" +- it: should ignore an unused ratio when an explicit limit is set + set: + redisCache.config.maxmemory: 300mb + redisStore.config.maxmemory: 300mb + redisCache.config.maxmemoryRatio: 0 + redisStore.config.maxmemoryRatio: 2 + asserts: + - matchRegex: + path: data["redis.conf"] + pattern: "\nmaxmemory 300mb\n?$" +- it: should skip config rendering and validation when config management is disabled + set: + redisCache.config: + enabled: false + existingConfig: ignored + additionalConfig: ignored + maxmemoryRatio: 0 + redisStore.config: + enabled: false + existingConfig: ignored + additionalConfig: ignored + maxmemoryRatio: 0 + asserts: + - hasDocuments: + count: 0 +- it: should skip config rendering and validation for external Redis + set: + redisCache.enabled: false + redisStore.enabled: false + redisCache.config.maxmemoryRatio: 0 + redisStore.config.maxmemoryRatio: 0 + asserts: + - hasDocuments: + count: 0 diff --git a/charts/sourcegraph/values.yaml b/charts/sourcegraph/values.yaml index a7beaef88..606e95669 100644 --- a/charts/sourcegraph/values.yaml +++ b/charts/sourcegraph/values.yaml @@ -1090,6 +1090,10 @@ redisCache: # -- Optional annotations to add to the `redis-cache` PVC storageAnnotations: {} config: + # -- Mount a chart-managed Redis config. Set to false to preserve image-baked + # configuration or custom `extraVolumeMounts` (including Secret mounts). + # When false, all other `config` options are ignored and auto-sizing is disabled. + enabled: true # -- Name of an existing ConfigMap for `redis-cache`. It must contain a `redis.conf` key. # When set, the chart-managed ConfigMap is not rendered and this one is mounted instead, # so the chart no longer sizes `maxmemory`. Mutually exclusive with `additionalConfig`. @@ -1098,14 +1102,17 @@ redisCache: # auto-computed value. Empty means compute it from the container memory limit. maxmemory: "" # -- Fraction of `redisCache.resources.limits.memory` used for `maxmemory` when - # `maxmemory` is empty. Ignored when no memory limit is set. + # `maxmemory` is empty. Must be greater than 0 and less than 1. Ignored when no + # memory limit is set or `sourcegraph.localDevMode` is enabled. An absent or + # unrecognised limit keeps the vendored 6gb default; use an explicit maxmemory if needed. maxmemoryRatio: 0.75 # -- Override the redis `maxmemory-policy` for `redis-cache`. # Empty keeps the vendored default (`allkeys-lru`). maxmemoryPolicy: "" # -- Additional raw redis directives appended to the vendored `redis-cache` config. # Notes: This is expecting a multiline string. It renders into a ConfigMap in plaintext, - # so do not put secrets such as `requirepass` here; use `existingConfig` instead. + # so do not put secrets such as `requirepass` here or in `existingConfig` (also a ConfigMap). + # For secrets, set `config.enabled: false` and mount a Secret using `extraVolumeMounts`. additionalConfig: "" redisExporter: @@ -1183,6 +1190,10 @@ redisStore: # -- Optional annotations to add to the `redis-store` PVC storageAnnotations: {} config: + # -- Mount a chart-managed Redis config. Set to false to preserve image-baked + # configuration or custom `extraVolumeMounts` (including Secret mounts). + # When false, all other `config` options are ignored and auto-sizing is disabled. + enabled: true # -- Name of an existing ConfigMap for `redis-store`. It must contain a `redis.conf` key. # When set, the chart-managed ConfigMap is not rendered and this one is mounted instead, # so the chart no longer sizes `maxmemory`. Mutually exclusive with `additionalConfig`. @@ -1191,14 +1202,17 @@ redisStore: # auto-computed value. Empty means compute it from the container memory limit. maxmemory: "" # -- Fraction of `redisStore.resources.limits.memory` used for `maxmemory` when - # `maxmemory` is empty. Ignored when no memory limit is set. + # `maxmemory` is empty. Must be greater than 0 and less than 1. Ignored when no + # memory limit is set or `sourcegraph.localDevMode` is enabled. An absent or + # unrecognised limit keeps the vendored 6gb default; use an explicit maxmemory if needed. maxmemoryRatio: 0.75 # -- Override the redis `maxmemory-policy` for `redis-store`. # Empty keeps the vendored default (`noeviction`). maxmemoryPolicy: "" # -- Additional raw redis directives appended to the vendored `redis-store` config. # Notes: This is expecting a multiline string. It renders into a ConfigMap in plaintext, - # so do not put secrets such as `requirepass` here; use `existingConfig` instead. + # so do not put secrets such as `requirepass` here or in `existingConfig` (also a ConfigMap). + # For secrets, set `config.enabled: false` and mount a Secret using `extraVolumeMounts`. additionalConfig: "" searcher: From 5307f4c73cef810ccb7a08a5fe544693b27edfb0 Mon Sep 17 00:00:00 2001 From: Michael Lin Date: Fri, 25 Sep 2026 00:17:00 +0000 Subject: [PATCH 3/3] Make Redis config management opt-in Amp-Thread-ID: https://ampcode.com/threads/T-01a0c732-cf8b-71ae-a2af-40664eafab0a Co-authored-by: Amp --- charts/sourcegraph/CHANGELOG.md | 4 +- charts/sourcegraph/README.md | 43 ++++++++---- charts/sourcegraph/README.md.gotmpl | 39 +++++++---- .../tests/redisConfigMount_test.yaml | 66 ++++++++++++++++--- .../sourcegraph/tests/redisConfig_test.yaml | 5 +- charts/sourcegraph/values.yaml | 14 ++-- 6 files changed, 127 insertions(+), 44 deletions(-) diff --git a/charts/sourcegraph/CHANGELOG.md b/charts/sourcegraph/CHANGELOG.md index 0d04287ab..e01868129 100644 --- a/charts/sourcegraph/CHANGELOG.md +++ b/charts/sourcegraph/CHANGELOG.md @@ -8,8 +8,8 @@ Use `**BREAKING**:` to denote a breaking change ## Unreleased -- **BREAKING**: The chart now manages `/etc/redis/redis.conf` for `redis-cache` and `redis-store`, replacing image-baked configuration, including any custom authentication settings. Operators with custom images or config mounts must either set the corresponding `redisCache.config.enabled` / `redisStore.config.enabled` to `false` to preserve the old behavior, or migrate to the new config options. Conflicting `extraVolumeMounts` fail rendering with a migration message. See [Upgrading Redis configuration](README.md#upgrading-redis-configuration), including guidance for Secret-mounted configs. -- Added `redisCache.config` and `redisStore.config` (`enabled`, `existingConfig`, `maxmemory`, `maxmemoryRatio`, `maxmemoryPolicy`, `additionalConfig`). By default the chart sizes Redis `maxmemory` at 75% of `resources.limits.memory` to leave room for overhead; this reduces, but does not eliminate, OOM risk. Ratios must be greater than 0 and less than 1. Auto-sizing is skipped in local development mode or when the limit is absent or unrecognised. At the default 7Gi limit the effective cap drops from 6GiB to 5.25GiB: `redis-cache` evicts earlier, but `redis-store` uses `noeviction`, so its write-error ceiling drops too. Set `redisStore.config.maxmemory: 6gb` to keep the old ceiling if the pod has sufficient memory. +- Added opt-in Redis config management through `redisCache.config.enabled` and `redisStore.config.enabled` (both default to `false`). Default deployments retain their image configs, custom mounts, authentication, and memory settings. When enabled, the chart mounts `/etc/redis/redis.conf` and supports `existingConfig`, `maxmemory`, `maxmemoryRatio`, `maxmemoryPolicy`, and `additionalConfig`. See [Enabling Redis config management](README.md#enabling-redis-config-management) before opting in, especially when using custom images or Secret-mounted configs. +- With Redis config management enabled, the chart sizes `maxmemory` at 75% of `resources.limits.memory` to leave room for overhead; this reduces, but does not eliminate, OOM risk. Ratios must be greater than 0 and less than 1. Auto-sizing is skipped in local development mode or when the limit is absent or unrecognised. At the default 7Gi limit, opting in lowers the effective cap from 6GiB to 5.25GiB: `redis-cache` evicts earlier, but `redis-store` uses `noeviction`, so its write-error ceiling drops too. Set `redisStore.config.maxmemory: 6gb` to keep the old ceiling if the pod has sufficient memory. - Added a `network-policy` example, which limits Executor and Executor job pods to the frontend API - Corrected the external object storage examples to configure the shared store for frontend, worker, precise code intel, syntactic code intel, gitserver, and searcher, including credentials or workload service accounts as required. - Removed the unused application ports from the precise and syntactic code intel worker Deployments and Services; health checks and Prometheus metrics continue to use the debug server on port 6060. diff --git a/charts/sourcegraph/README.md b/charts/sourcegraph/README.md index ad364757d..0f22286af 100644 --- a/charts/sourcegraph/README.md +++ b/charts/sourcegraph/README.md @@ -7,36 +7,51 @@ Visit the [Helm docs](https://docs.sourcegraph.com/admin/install/kubernetes) for guidance on using this chart. -## Upgrading Redis configuration +## Enabling Redis config management -The chart now mounts `/etc/redis/redis.conf` for both Redis services and sizes -`maxmemory` to 75% of each container's memory limit. At the default 7Gi limit, -this changes the cap from 6GiB to 5.25GiB. `redis-cache` evicts earlier; +Redis config management is **disabled by default**. Upgrading the chart preserves +existing image configs, custom mounts, authentication, and memory settings. +The standard images retain their 6GiB cap, which can cause OOM kills if the pod +memory limit is lower. Opt in for either service independently, or both: + +```yaml +redisCache: + config: + enabled: true +redisStore: + config: + enabled: true +``` + +For each enabled service, the chart mounts `/etc/redis/redis.conf` and sizes +`maxmemory` to 75% of the container's memory limit. At the default 7Gi limit, +opting in changes the cap from 6GiB to 5.25GiB. `redis-cache` evicts earlier; `redis-store` keeps `noeviction` and rejects writes sooner when full. Set `redisStore.config.maxmemory: 6gb` to retain its old cap, provided the pod has enough memory for Redis overhead. Auto-sizing reserves headroom but cannot guarantee against OOM kills, particularly during persistence operations. -Before upgrading: +Before opting in: - **Custom images:** the mounted config replaces the image's config, including - any `requirepass`, ACL, TLS, or persistence settings. Set + any `requirepass`, ACL, TLS, or persistence settings. Leave `redisCache.config.enabled: false` and/or `redisStore.config.enabled: false` to preserve the corresponding image config. Otherwise, migrate those settings explicitly; image-baked authentication is not inherited. -- **Existing config mounts:** either disable config management as above and +- **Existing config mounts:** either leave config management disabled and keep your `extraVolumes` / `extraVolumeMounts`, or remove those mounts and set - `config.existingConfig` to the name of a ConfigMap with a complete `redis.conf` - key. An existing ConfigMap bypasses all chart sizing and directive overrides. + `config.enabled: true` and `config.existingConfig` to the name of a ConfigMap + with a complete `redis.conf` key. An existing ConfigMap bypasses all chart + sizing and directive overrides. - **Secrets:** both `additionalConfig` and `existingConfig` use plaintext - ConfigMaps. For a config containing credentials, disable config management + ConfigMaps. For a config containing credentials, leave config management disabled and mount a Secret at `/etc/redis/redis.conf` with `extraVolumeMounts` instead. Configure the clients' `connection.existingSecret` and the exporter's `redisExporter.env.REDIS_PASSWORD.valueFrom.secretKeyRef` as appropriate. Redis readiness alone does not verify exporter authentication; check `redis_up`. -Disabling config management disables all other `config` options, including -auto-sizing, so you must size Redis memory yourself. With management enabled, +While config management is disabled, all other `config` options are ignored, +including auto-sizing, so you must size Redis memory yourself. With management enabled, an explicit `config.maxmemory` takes precedence over auto-sizing, and `additionalConfig` is appended last. Supported memory limits are plain byte counts or numbers with `k`, `M`, `G`, `T`, `P`, `E`, `Ki`, `Mi`, `Gi`, `Ti`, @@ -343,7 +358,7 @@ In addition to the documented values, all services also support the following va | prometheus.storageSize | string | `"200Gi"` | PVC Storage Request for `prometheus` data volume | | prometheus.storageSubPath | string | `""` | Optional subPath for the `prometheus` primary data volume mount | | redisCache.config.additionalConfig | string | `""` | Additional raw redis directives appended to the vendored `redis-cache` config. Notes: This is expecting a multiline string. It renders into a ConfigMap in plaintext, so do not put secrets such as `requirepass` here or in `existingConfig` (also a ConfigMap). For secrets, set `config.enabled: false` and mount a Secret using `extraVolumeMounts`. | -| redisCache.config.enabled | bool | `true` | Mount a chart-managed Redis config. Set to false to preserve image-baked configuration or custom `extraVolumeMounts` (including Secret mounts). When false, all other `config` options are ignored and auto-sizing is disabled. | +| redisCache.config.enabled | bool | `false` | Opt in to a chart-managed Redis config and automatic memory sizing. Disabled by default to preserve image-baked configuration and custom `extraVolumeMounts` (including Secret mounts). When false, all other `config` options are ignored and auto-sizing is disabled. | | redisCache.config.existingConfig | string | `""` | Name of an existing ConfigMap for `redis-cache`. It must contain a `redis.conf` key. When set, the chart-managed ConfigMap is not rendered and this one is mounted instead, so the chart no longer sizes `maxmemory`. Mutually exclusive with `additionalConfig`. | | redisCache.config.maxmemory | string | `""` | Explicit redis `maxmemory` for `redis-cache` (for example `6gb`). Overrides the auto-computed value. Empty means compute it from the container memory limit. | | redisCache.config.maxmemoryPolicy | string | `""` | Override the redis `maxmemory-policy` for `redis-cache`. Empty keeps the vendored default (`allkeys-lru`). | @@ -368,7 +383,7 @@ In addition to the documented values, all services also support the following va | redisExporter.image.name | string | `"redis_exporter"` | Docker image name for the `redis-exporter` image | | redisExporter.resources | object | `{"limits":{"cpu":"10m","memory":"100Mi"},"requests":{"cpu":"10m","memory":"100Mi"}}` | Resource requests & limits for the `redis-exporter` sidecar container, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/) | | redisStore.config.additionalConfig | string | `""` | Additional raw redis directives appended to the vendored `redis-store` config. Notes: This is expecting a multiline string. It renders into a ConfigMap in plaintext, so do not put secrets such as `requirepass` here or in `existingConfig` (also a ConfigMap). For secrets, set `config.enabled: false` and mount a Secret using `extraVolumeMounts`. | -| redisStore.config.enabled | bool | `true` | Mount a chart-managed Redis config. Set to false to preserve image-baked configuration or custom `extraVolumeMounts` (including Secret mounts). When false, all other `config` options are ignored and auto-sizing is disabled. | +| redisStore.config.enabled | bool | `false` | Opt in to a chart-managed Redis config and automatic memory sizing. Disabled by default to preserve image-baked configuration and custom `extraVolumeMounts` (including Secret mounts). When false, all other `config` options are ignored and auto-sizing is disabled. | | redisStore.config.existingConfig | string | `""` | Name of an existing ConfigMap for `redis-store`. It must contain a `redis.conf` key. When set, the chart-managed ConfigMap is not rendered and this one is mounted instead, so the chart no longer sizes `maxmemory`. Mutually exclusive with `additionalConfig`. | | redisStore.config.maxmemory | string | `""` | Explicit redis `maxmemory` for `redis-store` (for example `6gb`). Overrides the auto-computed value. Empty means compute it from the container memory limit. | | redisStore.config.maxmemoryPolicy | string | `""` | Override the redis `maxmemory-policy` for `redis-store`. Empty keeps the vendored default (`noeviction`). | diff --git a/charts/sourcegraph/README.md.gotmpl b/charts/sourcegraph/README.md.gotmpl index 2e2dfa37e..b2fb8bba1 100644 --- a/charts/sourcegraph/README.md.gotmpl +++ b/charts/sourcegraph/README.md.gotmpl @@ -7,36 +7,51 @@ Visit the [Helm docs](https://docs.sourcegraph.com/admin/install/kubernetes) for guidance on using this chart. -## Upgrading Redis configuration +## Enabling Redis config management -The chart now mounts `/etc/redis/redis.conf` for both Redis services and sizes -`maxmemory` to 75% of each container's memory limit. At the default 7Gi limit, -this changes the cap from 6GiB to 5.25GiB. `redis-cache` evicts earlier; +Redis config management is **disabled by default**. Upgrading the chart preserves +existing image configs, custom mounts, authentication, and memory settings. +The standard images retain their 6GiB cap, which can cause OOM kills if the pod +memory limit is lower. Opt in for either service independently, or both: + +```yaml +redisCache: + config: + enabled: true +redisStore: + config: + enabled: true +``` + +For each enabled service, the chart mounts `/etc/redis/redis.conf` and sizes +`maxmemory` to 75% of the container's memory limit. At the default 7Gi limit, +opting in changes the cap from 6GiB to 5.25GiB. `redis-cache` evicts earlier; `redis-store` keeps `noeviction` and rejects writes sooner when full. Set `redisStore.config.maxmemory: 6gb` to retain its old cap, provided the pod has enough memory for Redis overhead. Auto-sizing reserves headroom but cannot guarantee against OOM kills, particularly during persistence operations. -Before upgrading: +Before opting in: - **Custom images:** the mounted config replaces the image's config, including - any `requirepass`, ACL, TLS, or persistence settings. Set + any `requirepass`, ACL, TLS, or persistence settings. Leave `redisCache.config.enabled: false` and/or `redisStore.config.enabled: false` to preserve the corresponding image config. Otherwise, migrate those settings explicitly; image-baked authentication is not inherited. -- **Existing config mounts:** either disable config management as above and +- **Existing config mounts:** either leave config management disabled and keep your `extraVolumes` / `extraVolumeMounts`, or remove those mounts and set - `config.existingConfig` to the name of a ConfigMap with a complete `redis.conf` - key. An existing ConfigMap bypasses all chart sizing and directive overrides. + `config.enabled: true` and `config.existingConfig` to the name of a ConfigMap + with a complete `redis.conf` key. An existing ConfigMap bypasses all chart + sizing and directive overrides. - **Secrets:** both `additionalConfig` and `existingConfig` use plaintext - ConfigMaps. For a config containing credentials, disable config management + ConfigMaps. For a config containing credentials, leave config management disabled and mount a Secret at `/etc/redis/redis.conf` with `extraVolumeMounts` instead. Configure the clients' `connection.existingSecret` and the exporter's `redisExporter.env.REDIS_PASSWORD.valueFrom.secretKeyRef` as appropriate. Redis readiness alone does not verify exporter authentication; check `redis_up`. -Disabling config management disables all other `config` options, including -auto-sizing, so you must size Redis memory yourself. With management enabled, +While config management is disabled, all other `config` options are ignored, +including auto-sizing, so you must size Redis memory yourself. With management enabled, an explicit `config.maxmemory` takes precedence over auto-sizing, and `additionalConfig` is appended last. Supported memory limits are plain byte counts or numbers with `k`, `M`, `G`, `T`, `P`, `E`, `Ki`, `Mi`, `Gi`, `Ti`, diff --git a/charts/sourcegraph/tests/redisConfigMount_test.yaml b/charts/sourcegraph/tests/redisConfigMount_test.yaml index b2557a011..78dab75c8 100644 --- a/charts/sourcegraph/tests/redisConfigMount_test.yaml +++ b/charts/sourcegraph/tests/redisConfigMount_test.yaml @@ -8,8 +8,10 @@ templates: - redis/redis-cache.ConfigMap.yaml - redis/redis-store.ConfigMap.yaml tests: -- it: should mount the chart-managed ConfigMap over the image path for redis-cache +- it: should enable config management for redis-cache independently template: redis/redis-cache.Deployment.yaml + set: + redisCache.config.enabled: true asserts: - contains: path: spec.template.spec.containers[0].volumeMounts @@ -31,8 +33,27 @@ tests: path: spec.template.metadata.annotations["checksum/redis-config"] - notExists: path: spec.template.spec.containers[0].command -- it: should mount the chart-managed ConfigMap over the image path for redis-store + - hasDocuments: + count: 1 + template: redis/redis-cache.ConfigMap.yaml + - hasDocuments: + count: 0 + template: redis/redis-store.ConfigMap.yaml + - lengthEqual: + path: spec.template.spec.containers[0].volumeMounts + count: 1 + template: redis/redis-store.Deployment.yaml + - lengthEqual: + path: spec.template.spec.volumes + count: 1 + template: redis/redis-store.Deployment.yaml + - notExists: + path: spec.template.metadata.annotations["checksum/redis-config"] + template: redis/redis-store.Deployment.yaml +- it: should enable config management for redis-store independently template: redis/redis-store.Deployment.yaml + set: + redisStore.config.enabled: true asserts: - contains: path: spec.template.spec.containers[0].volumeMounts @@ -54,11 +75,29 @@ tests: path: spec.template.metadata.annotations["checksum/redis-config"] - notExists: path: spec.template.spec.containers[0].command + - hasDocuments: + count: 1 + template: redis/redis-store.ConfigMap.yaml + - hasDocuments: + count: 0 + template: redis/redis-cache.ConfigMap.yaml + - lengthEqual: + path: spec.template.spec.containers[0].volumeMounts + count: 1 + template: redis/redis-cache.Deployment.yaml + - lengthEqual: + path: spec.template.spec.volumes + count: 1 + template: redis/redis-cache.Deployment.yaml + - notExists: + path: spec.template.metadata.annotations["checksum/redis-config"] + template: redis/redis-cache.Deployment.yaml - it: should mount an existing ConfigMap and drop the checksum annotation template: redis/redis-cache.Deployment.yaml set: redisCache: config: + enabled: true existingConfig: my-redis-conf asserts: - contains: @@ -76,6 +115,8 @@ tests: template: redis/redis-cache.Deployment.yaml set: redisCache: + config: + enabled: true extraVolumeMounts: - name: my-conf mountPath: /etc/redis/redis.conf @@ -87,6 +128,8 @@ tests: template: redis/redis-store.Deployment.yaml set: redisStore: + config: + enabled: true extraVolumeMounts: - name: my-conf mountPath: /etc/redis @@ -97,6 +140,8 @@ tests: template: redis/redis-cache.Deployment.yaml set: redisCache: + config: + enabled: true extraVolumes: - name: redis-conf emptyDir: {} @@ -107,6 +152,8 @@ tests: template: redis/redis-cache.Deployment.yaml set: redisCache: + config: + enabled: true extraVolumeMounts: - name: my-extra mountPath: /somewhere-else @@ -116,11 +163,14 @@ tests: content: name: my-extra mountPath: /somewhere-else -- it: should preserve the image config when management is disabled - set: - redisCache.config.enabled: false - redisStore.config.enabled: false +- it: should preserve the image config by default asserts: + - hasDocuments: + count: 0 + template: redis/redis-cache.ConfigMap.yaml + - hasDocuments: + count: 0 + template: redis/redis-store.ConfigMap.yaml - equal: path: spec.template.spec.containers[0].volumeMounts value: @@ -147,10 +197,8 @@ tests: - notExists: path: spec.template.metadata.annotations["checksum/redis-config"] template: redis/redis-store.Deployment.yaml -- it: should preserve a Secret-mounted config when management is disabled +- it: should preserve a Secret-mounted config by default set: - redisCache.config.enabled: false - redisStore.config.enabled: false redisCache.extraVolumeMounts: &mounts - name: redis-conf mountPath: /etc/redis/redis.conf diff --git a/charts/sourcegraph/tests/redisConfig_test.yaml b/charts/sourcegraph/tests/redisConfig_test.yaml index 0450fa26f..671d349c0 100644 --- a/charts/sourcegraph/tests/redisConfig_test.yaml +++ b/charts/sourcegraph/tests/redisConfig_test.yaml @@ -3,8 +3,11 @@ suite: redisConfig templates: - redis/redis-cache.ConfigMap.yaml - redis/redis-store.ConfigMap.yaml +set: + redisCache.config.enabled: true + redisStore.config.enabled: true tests: -- it: should render the vendored config with an auto-sized maxmemory by default +- it: should render the vendored config with an auto-sized maxmemory when enabled asserts: - matchRegex: path: data["redis.conf"] diff --git a/charts/sourcegraph/values.yaml b/charts/sourcegraph/values.yaml index 606e95669..74a1395e0 100644 --- a/charts/sourcegraph/values.yaml +++ b/charts/sourcegraph/values.yaml @@ -1090,10 +1090,11 @@ redisCache: # -- Optional annotations to add to the `redis-cache` PVC storageAnnotations: {} config: - # -- Mount a chart-managed Redis config. Set to false to preserve image-baked - # configuration or custom `extraVolumeMounts` (including Secret mounts). + # -- Opt in to a chart-managed Redis config and automatic memory sizing. + # Disabled by default to preserve image-baked configuration and custom + # `extraVolumeMounts` (including Secret mounts). # When false, all other `config` options are ignored and auto-sizing is disabled. - enabled: true + enabled: false # -- Name of an existing ConfigMap for `redis-cache`. It must contain a `redis.conf` key. # When set, the chart-managed ConfigMap is not rendered and this one is mounted instead, # so the chart no longer sizes `maxmemory`. Mutually exclusive with `additionalConfig`. @@ -1190,10 +1191,11 @@ redisStore: # -- Optional annotations to add to the `redis-store` PVC storageAnnotations: {} config: - # -- Mount a chart-managed Redis config. Set to false to preserve image-baked - # configuration or custom `extraVolumeMounts` (including Secret mounts). + # -- Opt in to a chart-managed Redis config and automatic memory sizing. + # Disabled by default to preserve image-baked configuration and custom + # `extraVolumeMounts` (including Secret mounts). # When false, all other `config` options are ignored and auto-sizing is disabled. - enabled: true + enabled: false # -- Name of an existing ConfigMap for `redis-store`. It must contain a `redis.conf` key. # When set, the chart-managed ConfigMap is not rendered and this one is mounted instead, # so the chart no longer sizes `maxmemory`. Mutually exclusive with `additionalConfig`.