Allow syntactic-code-intel-worker /tmp scratch on a per-pod PVC

charts/sourcegraph/CHANGELOG.md

@@ -8,6 +8,7 @@ Use `**BREAKING**:` to denote a breaking change
 
 ## Unreleased
 
+- Added `syntacticCodeIntel.storageType`, `syntacticCodeIntel.storageSize`, and `syntacticCodeIntel.storageClassName`, so the `syntactic-code-intel-worker` `/tmp` scratch volume can be a per-pod PVC (generic ephemeral volume) instead of an `emptyDir` on the node boot disk, matching `preciseCodeIntel`. The default remains an unbounded `emptyDir`.
 - Added an `outbound-http-proxy` example, which routes outbound traffic from frontend, worker, and migrator through an HTTP proxy with `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY`
 - Fixed the `codeintel-db` PersistentVolumeClaim being renamed by `codeIntelDB.name`, which left the StatefulSet pointing at a `codeintel-db` claim that no longer existed and stuck the pod in `Pending`. The PVC is now always named `codeintel-db`, matching the other components and the documented behaviour of `*.name`. If you worked around this by creating a `codeintel-db` PVC by hand, label it `app.kubernetes.io/managed-by: Helm` and annotate it with `meta.helm.sh/release-name` and `meta.helm.sh/release-namespace` before upgrading so Helm can adopt it. [#612](https://github.com/sourcegraph/deploy-sourcegraph-helm/issues/612)
 - Added `indexedSearch.readinessProbe` (`failureThreshold`, `periodSeconds`, `timeoutSeconds`) to tune the `zoekt-webserver` readiness probe, which was previously hardcoded. Defaults are unchanged (`3`/`5`/`5`). Raise `failureThreshold` on instances with large monorepos, where loading a many-shard repo can stall `/healthz` past `timeoutSeconds` and drop a healthy pod out of the `indexed-search` service.

charts/sourcegraph/README.md

@@ -460,6 +460,9 @@ In addition to the documented values, all services also support the following va
 | syntacticCodeIntel.resources | object | `{"limits":{"cpu":"2","memory":"4G"},"requests":{"cpu":"500m","memory":"2G"}}` | Resource requests & limits for the `syntactic-code-intel-worker` container, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/) |
 | syntacticCodeIntel.serviceAccount.create | bool | `false` | Enable creation of ServiceAccount for `syntactic-code-intel-worker` |
 | syntacticCodeIntel.serviceAccount.name | string | `""` | Name of the ServiceAccount to be created or an existing ServiceAccount |
+| syntacticCodeIntel.storageClassName | string | `""` | StorageClass for the `/tmp` scratch PVC when storageType is `pvc`. Empty uses the cluster default class. The claim is per-pod and discarded with the pod, so this class must use `reclaimPolicy: Delete`; `storageClass` is `Retain` for stateful data and is not used here. |
+| syntacticCodeIntel.storageSize | string | `""` | Size of the `/tmp` scratch volume. Required when storageType is `pvc`. When storageType is `emptyDir` this sets `sizeLimit` (optional, leave empty for unlimited). |
+| syntacticCodeIntel.storageType | string | `"emptyDir"` | Backing store for the `/tmp` scratch volume, used for indexing. One of: emptyDir, pvc. emptyDir: node ephemeral storage, i.e. the node boot disk. pvc: per-pod PVC, discarded with the pod. The pod `fsGroup` defaults to the container `runAsGroup` so the non-root worker can write to the volume; set `podSecurityContext.fsGroup` to override it. |
 | syntectServer.containerSecurityContext | object | `{"allowPrivilegeEscalation":false,"readOnlyRootFilesystem":true,"runAsGroup":101,"runAsUser":100}` | Security context for the `syntect-server` container, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-container) |
 | syntectServer.env | object | the chart will add some default environment values | Environment variables for the `syntect-server` container |
 | syntectServer.image.defaultTag | string | `<appVersion>@sha256:<digest>`, pinned by each release | Docker image tag for the `syntect-server` image |

charts/sourcegraph/templates/syntactic-code-intel/worker.Deployment.yaml

@@ -46,6 +46,10 @@ spec:
         deploy: sourcegraph
         app: syntactic-code-intel-worker
     spec:
+      {{- $storageType := .Values.syntacticCodeIntel.storageType | default "emptyDir" }}
+      {{- if not (has $storageType (list "emptyDir" "pvc")) }}
+      {{- fail (printf "syntacticCodeIntel.storageType must be one of emptyDir, pvc; got %q" $storageType) }}
+      {{- end }}
       {{- include "sourcegraph.terminationGracePeriodSeconds" (list . "syntacticCodeIntel") | nindent 6 }}
       containers:
       - name: syntactic-code-intel-worker
@@ -105,8 +109,13 @@ spec:
       {{- if .Values.syntacticCodeIntel.extraContainers }}
         {{- toYaml .Values.syntacticCodeIntel.extraContainers | nindent 6 }}
       {{- end }}
+      {{- $podSecurityContext := deepCopy .Values.syntacticCodeIntel.podSecurityContext }}
+      {{- if eq $storageType "pvc" }}
+      {{- /* A freshly provisioned volume is root-owned, so the non-root worker needs fsGroup to write /tmp. */}}
+      {{- $podSecurityContext = merge $podSecurityContext (dict "fsGroup" (.Values.syntacticCodeIntel.containerSecurityContext.runAsGroup | default 101) "fsGroupChangePolicy" "OnRootMismatch") }}
+      {{- end }}
       securityContext:
-        {{- toYaml .Values.syntacticCodeIntel.podSecurityContext | nindent 8 }}
+        {{- toYaml $podSecurityContext | nindent 8 }}
       {{- include "sourcegraph.nodeSelector" (list . "syntacticCodeIntel" ) | trim | nindent 6 }}
       {{- include "sourcegraph.affinity" (list . "syntacticCodeIntel" ) | trim | nindent 6 }}
       {{- with include "sourcegraph.priorityClassName" (list . "syntacticCodeIntel" ) | trim }}{{ . | nindent 6 }}{{- end }}
@@ -118,7 +127,24 @@ spec:
       {{- include "sourcegraph.renderServiceAccountName" (list . "syntacticCodeIntel") | trim | nindent 6 }}
       volumes:
       - name: tmpdir
+        {{- if eq $storageType "pvc" }}
+        ephemeral:
+          volumeClaimTemplate:
+            spec:
+              accessModes:
+              - ReadWriteOnce
+              resources:
+                requests:
+                  storage: {{ required "syntacticCodeIntel.storageSize is required when syntacticCodeIntel.storageType is pvc" .Values.syntacticCodeIntel.storageSize }}
+              {{- with .Values.syntacticCodeIntel.storageClassName }}
+              storageClassName: {{ . }}
+              {{- end }}
+        {{- else if .Values.syntacticCodeIntel.storageSize }}
+        emptyDir:
+          sizeLimit: {{ .Values.syntacticCodeIntel.storageSize }}
+        {{- else }}
         emptyDir: {}
+        {{- end }}
       {{- if .Values.syntacticCodeIntel.extraVolumes }}
       {{- toYaml .Values.syntacticCodeIntel.extraVolumes | nindent 6 }}
       {{- end }}

charts/sourcegraph/tests/syntacticCodeIntelStorage_test.yaml

@@ -0,0 +1,154 @@
+---
+suite: syntacticCodeIntelStorage
+templates:
+- syntactic-code-intel/worker.Deployment.yaml
+set:
+  syntacticCodeIntel:
+    enabled: true
+tests:
+- it: should back tmpdir with a plain emptyDir by default
+  asserts:
+  - contains:
+      path: spec.template.spec.volumes
+      content:
+        name: tmpdir
+        emptyDir: {}
+  - contains:
+      path: spec.template.spec.containers[0].volumeMounts
+      content:
+        mountPath: /tmp
+        name: tmpdir
+
+- it: should set sizeLimit when storageSize is set on the emptyDir branch
+  set:
+    syntacticCodeIntel:
+      storageType: emptyDir
+      storageSize: 20Gi
+  asserts:
+  - contains:
+      path: spec.template.spec.volumes
+      content:
+        name: tmpdir
+        emptyDir:
+          sizeLimit: 20Gi
+
+- it: should back tmpdir with a per-pod PVC when storageType=pvc
+  set:
+    syntacticCodeIntel:
+      storageType: pvc
+      storageSize: 50Gi
+  asserts:
+  - contains:
+      path: spec.template.spec.volumes
+      content:
+        name: tmpdir
+        ephemeral:
+          volumeClaimTemplate:
+            spec:
+              accessModes:
+              - ReadWriteOnce
+              resources:
+                requests:
+                  storage: 50Gi
+  # /tmp mount is unchanged, so no TMPDIR redirect is needed
+  - contains:
+      path: spec.template.spec.containers[0].volumeMounts
+      content:
+        mountPath: /tmp
+        name: tmpdir
+
+# The per-pod claim needs a Delete-policy class; storageClass is Retain.
+- it: should omit storageClassName by default so the cluster default class is used
+  set:
+    syntacticCodeIntel:
+      storageType: pvc
+      storageSize: 50Gi
+  asserts:
+  - notExists:
+      path: spec.template.spec.volumes[0].ephemeral.volumeClaimTemplate.spec.storageClassName
+
+- it: should not inherit storageClass.name
+  set:
+    storageClass:
+      name: custom-ssd
+    syntacticCodeIntel:
+      storageType: pvc
+      storageSize: 50Gi
+  asserts:
+  - notExists:
+      path: spec.template.spec.volumes[0].ephemeral.volumeClaimTemplate.spec.storageClassName
+
+- it: should use syntacticCodeIntel.storageClassName when set
+  set:
+    storageClass:
+      name: custom-ssd
+    syntacticCodeIntel:
+      storageType: pvc
+      storageSize: 50Gi
+      storageClassName: premium-rwo
+  asserts:
+  - equal:
+      path: spec.template.spec.volumes[0].ephemeral.volumeClaimTemplate.spec.storageClassName
+      value: premium-rwo
+
+- it: should fail when storageSize is omitted on the pvc branch
+  set:
+    syntacticCodeIntel:
+      storageType: pvc
+  asserts:
+  - failedTemplate:
+      errorMessage: syntacticCodeIntel.storageSize is required when syntacticCodeIntel.storageType is pvc
+
+- it: should fall back to emptyDir when storageType is absent
+  set:
+    syntacticCodeIntel:
+      storageType: null
+  asserts:
+  - contains:
+      path: spec.template.spec.volumes
+      content:
+        name: tmpdir
+        emptyDir: {}
+
+- it: should fail on an unknown storageType
+  set:
+    syntacticCodeIntel:
+      storageType: PVC
+      storageSize: 50Gi
+  asserts:
+  - failedTemplate:
+      errorMessage: 'syntacticCodeIntel.storageType must be one of emptyDir, pvc; got "PVC"'
+
+- it: should leave the pod securityContext untouched on the emptyDir branch
+  asserts:
+  - equal:
+      path: spec.template.spec.securityContext
+      value: {}
+
+- it: should set fsGroup to the container runAsGroup on the pvc branch so /tmp is writable
+  set:
+    syntacticCodeIntel:
+      storageType: pvc
+      storageSize: 50Gi
+  asserts:
+  - equal:
+      path: spec.template.spec.securityContext
+      value:
+        fsGroup: 101
+        fsGroupChangePolicy: OnRootMismatch
+
+- it: should keep a user-set fsGroup and other podSecurityContext fields on the pvc branch
+  set:
+    syntacticCodeIntel:
+      storageType: pvc
+      storageSize: 50Gi
+      podSecurityContext:
+        fsGroup: 2000
+        runAsNonRoot: true
+  asserts:
+  - equal:
+      path: spec.template.spec.securityContext
+      value:
+        fsGroup: 2000
+        fsGroupChangePolicy: OnRootMismatch
+        runAsNonRoot: true

charts/sourcegraph/values.yaml

@@ -965,6 +965,22 @@ syntacticCodeIntel:
     create: false
     # -- Name of the ServiceAccount to be created or an existing ServiceAccount
     name: ""
+  # -- Backing store for the `/tmp` scratch volume, used for indexing.
+  # One of: emptyDir, pvc.
+  # emptyDir: node ephemeral storage, i.e. the node boot disk.
+  # pvc: per-pod PVC, discarded with the pod. The pod `fsGroup` defaults to the
+  # container `runAsGroup` so the non-root worker can write to the volume; set
+  # `podSecurityContext.fsGroup` to override it.
+  storageType: emptyDir
+  # -- Size of the `/tmp` scratch volume. Required when storageType is `pvc`.
+  # When storageType is `emptyDir` this sets `sizeLimit` (optional, leave empty
+  # for unlimited).
+  storageSize: ""
+  # -- StorageClass for the `/tmp` scratch PVC when storageType is `pvc`.
+  # Empty uses the cluster default class. The claim is per-pod and discarded
+  # with the pod, so this class must use `reclaimPolicy: Delete`; `storageClass`
+  # is `Retain` for stateful data and is not used here.
+  storageClassName: ""
 
 preciseCodeIntel:
   # -- Environment variables for the `precise-code-intel-worker` container