searcher: accept storageSize in G as well as Gi for cache sizing

charts/sourcegraph/CHANGELOG.md

@@ -8,6 +8,7 @@ Use `**BREAKING**:` to denote a breaking change
 
 ## Unreleased
 
+- `searcher.storageSize` now accepts `G` as well as `Gi` when deriving `SEARCHER_CACHE_SIZE_MB` and `SYMBOLS_CACHE_SIZE_MB`. Previously a value like `150G` silently rendered both as `0`, which disables cache eviction and lets the cache volume fill up. Any other unit now fails the render unless `searcher.autoCacheSize` is enabled
 - 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.

charts/sourcegraph/README.md

@@ -348,7 +348,7 @@ In addition to the documented values, all services also support the following va
 | searcher.serviceAccount.create | bool | `false` | Enable creation of ServiceAccount for `searcher` |
 | searcher.serviceAccount.name | string | `""` | Name of the ServiceAccount to be created or an existing ServiceAccount |
 | searcher.storageAnnotations | object | `{}` | Optional annotations to add to the `searcher` PVC |
-| searcher.storageSize | string | `"26Gi"` | Size of the PVC for searcher pods to store cache data |
+| searcher.storageSize | string | `"26Gi"` | Size of the PVC for searcher pods to store cache data. Unless `autoCacheSize` is enabled, this must be a whole number of `G` or `Gi` (e.g. `150G`, `100Gi`) so the chart can derive `SEARCHER_CACHE_SIZE_MB` and `SYMBOLS_CACHE_SIZE_MB` from it |
 | searcher.storageSubPath | string | `""` | Optional subPath for the `searcher` primary data volume mount |
 | sgTestConnection | object | `{"enabled":true}` | Enable the busybox connection test after deployment |
 | sourcegraph.affinity | object | `{}` | Global Affinity, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#affinity-and-anti-affinity) |

charts/sourcegraph/templates/_helpers.tpl

@@ -187,6 +187,20 @@ app.kubernetes.io/name: jaeger
 {{- end }}
 {{- end }}
 
+{{/*
+~45% of searcher.storageSize in megabytes, for SEARCHER_CACHE_SIZE_MB and
+SYMBOLS_CACHE_SIZE_MB. Accepts a whole number of G or Gi (e.g. 150G, 100Gi).
+Any other unit fails the render: a non-numeric value would otherwise cast to 0,
+which searcher treats as "never evict" and the cache volume fills up.
+*/}}
+{{- define "sourcegraph.searcher.cacheSizeMB" -}}
+{{- $size := .Values.searcher.storageSize | default "26Gi" | toString }}
+{{- if not (regexMatch "^[0-9]+Gi?$" $size) }}
+{{- fail (printf "searcher.storageSize must be a whole number of G or Gi (got %q), or set searcher.autoCacheSize=true" $size) }}
+{{- end }}
+{{- $size | trimSuffix "Gi" | trimSuffix "G" | mul 450 }}
+{{- end }}
+
 {{- define "sourcegraph.databaseAuth" -}}
 {{- $top := index . 0 -}}
 {{- $service := index . 1 -}}

charts/sourcegraph/templates/searcher/searcher.StatefulSet.yaml

@@ -62,24 +62,11 @@ spec:
         # sizes its cache to ~45% of the live cache volume, tracking PVC
         # expansion automatically. SEARCHER_CACHE_SIZE_MB / SYMBOLS_CACHE_SIZE_MB
         # are deprecated in favor of this auto-detection.
+        # Both caches share the PVC below and each gets ~45% of it.
         - name: SEARCHER_CACHE_SIZE_MB
-          # Set the cache size to ~45% of the PVC size
-          {{- if .Values.searcher.storageSize }}
-          value: {{ trimSuffix "Gi" .Values.searcher.storageSize | mul 450 | quote }}
-          {{- else }}
-          # This value is ~45% of the default value for
-          # storageSize in the VolumeClaimTemplate below
-          value: "11700"
-          {{- end }}
+          value: {{ include "sourcegraph.searcher.cacheSizeMB" . | quote }}
         - name: SYMBOLS_CACHE_SIZE_MB
-          # Set the cache size to ~45% of the PVC size
-          {{- if .Values.searcher.storageSize }}
-          value: {{ trimSuffix "Gi" .Values.searcher.storageSize | mul 450 | quote }}
-          {{- else }}
-          # This value is ~45% of the default value for
-          # storageSize in the VolumeClaimTemplate below
-          value: "11700"
-          {{- end }}
+          value: {{ include "sourcegraph.searcher.cacheSizeMB" . | quote }}
         {{- end }}
         - name: POD_NAME
           valueFrom:

charts/sourcegraph/tests/searcherCacheSize_test.yaml

@@ -0,0 +1,72 @@
+suite: searcher cache size
+templates:
+- searcher/searcher.StatefulSet.yaml
+tests:
+- it: should size both caches to ~45% of the default storageSize
+  asserts:
+  - contains:
+      path: spec.template.spec.containers[0].env
+      content:
+        name: SEARCHER_CACHE_SIZE_MB
+        value: "11700"
+  - contains:
+      path: spec.template.spec.containers[0].env
+      content:
+        name: SYMBOLS_CACHE_SIZE_MB
+        value: "11700"
+- it: should accept storageSize in Gi
+  set:
+    searcher:
+      storageSize: 100Gi
+  asserts:
+  - contains:
+      path: spec.template.spec.containers[0].env
+      content:
+        name: SEARCHER_CACHE_SIZE_MB
+        value: "45000"
+  - contains:
+      path: spec.template.spec.containers[0].env
+      content:
+        name: SYMBOLS_CACHE_SIZE_MB
+        value: "45000"
+- it: should accept storageSize in G
+  set:
+    searcher:
+      storageSize: 150G
+  asserts:
+  - contains:
+      path: spec.template.spec.containers[0].env
+      content:
+        name: SEARCHER_CACHE_SIZE_MB
+        value: "67500"
+  - contains:
+      path: spec.template.spec.containers[0].env
+      content:
+        name: SYMBOLS_CACHE_SIZE_MB
+        value: "67500"
+  - equal:
+      path: spec.volumeClaimTemplates[0].spec.resources.requests.storage
+      value: 150G
+- it: should fail on units other than G or Gi instead of disabling eviction
+  set:
+    searcher:
+      storageSize: 1Ti
+  asserts:
+  - failedTemplate:
+      errorMessage: searcher.storageSize must be a whole number of G or Gi (got "1Ti"), or set searcher.autoCacheSize=true
+- it: should not need a G or Gi storageSize when autoCacheSize is enabled
+  set:
+    searcher:
+      autoCacheSize: true
+      storageSize: 1Ti
+  asserts:
+  - notContains:
+      path: spec.template.spec.containers[0].env
+      content:
+        name: SEARCHER_CACHE_SIZE_MB
+      any: true
+  - notContains:
+      path: spec.template.spec.containers[0].env
+      content:
+        name: SYMBOLS_CACHE_SIZE_MB
+      any: true

charts/sourcegraph/values.yaml

@@ -1214,7 +1214,9 @@ searcher:
   # This tracks PVC expansion automatically, whereas the hardcoded env vars are
   # frozen to the initial `storageSize` and do not follow later disk growth.
   autoCacheSize: false
-  # -- Size of the PVC for searcher pods to store cache data
+  # -- Size of the PVC for searcher pods to store cache data. Unless `autoCacheSize`
+  # is enabled, this must be a whole number of `G` or `Gi` (e.g. `150G`, `100Gi`)
+  # so the chart can derive `SEARCHER_CACHE_SIZE_MB` and `SYMBOLS_CACHE_SIZE_MB` from it
   storageSize: 26Gi
   # -- Optional subPath for the `searcher` primary data volume mount
   storageSubPath: ""