From bc7adc83a711e4bdd7604d6eb527fbe8ca9f9d51 Mon Sep 17 00:00:00 2001 From: Dinesh Kumar <5300055+devdinu@users.noreply.github.com> Date: Wed, 30 Sep 2026 16:30:26 -0700 Subject: [PATCH] precise-code-intel: allow /tmp scratch on a per-pod PVC The worker writes large SCIP uploads to /tmp before processing them in multiple passes, and /tmp was always an unbounded emptyDir. That puts the write on the node boot disk, where a large upload competes with every other pod on the node and can fill the disk. A new storageType value selects the backing store. emptyDir stays the default, so rendered output is unchanged for existing installs. pvc switches /tmp to a generic ephemeral volume, giving each pod its own claim on storageClass.name that is discarded with the pod. storageSize is required for pvc and sets sizeLimit when left on emptyDir. The mount path stays /tmp, so the worker needs no TMPDIR redirect. A freshly provisioned volume is root-owned and the worker runs as a non-root user, so the pvc branch defaults the pod fsGroup to the container runAsGroup (with fsGroupChangePolicy OnRootMismatch) to keep /tmp writable. A user-set podSecurityContext.fsGroup still wins. An unknown storageType now fails rendering instead of silently falling back to emptyDir. Amp-Thread-ID: https://ampcode.com/threads/T-01a0f4b6-78bf-7109-82dc-4545a652ecdf Co-authored-by: Dinesh Kumar --- charts/sourcegraph/README.md | 2 + .../precise-code-intel/worker.Deployment.yaml | 26 +++- .../tests/preciseCodeIntelStorage_test.yaml | 130 ++++++++++++++++++ charts/sourcegraph/values.yaml | 11 ++ 4 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 charts/sourcegraph/tests/preciseCodeIntelStorage_test.yaml diff --git a/charts/sourcegraph/README.md b/charts/sourcegraph/README.md index 4fc444aa..38e5c730 100644 --- a/charts/sourcegraph/README.md +++ b/charts/sourcegraph/README.md @@ -341,6 +341,8 @@ In addition to the documented values, all services also support the following va | preciseCodeIntel.resources | object | `{"limits":{"cpu":"2","memory":"4G"},"requests":{"cpu":"500m","memory":"2G"}}` | Resource requests & limits for the `precise-code-intel-worker` container, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/) | | preciseCodeIntel.serviceAccount.create | bool | `false` | Enable creation of ServiceAccount for `precise-code-intel-worker` | | preciseCodeIntel.serviceAccount.name | string | `""` | Name of the ServiceAccount to be created or an existing ServiceAccount | +| preciseCodeIntel.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). | +| preciseCodeIntel.storageType | string | `"emptyDir"` | Backing store for the `/tmp` scratch volume, used for large SCIP upload processing. One of: emptyDir, pvc. emptyDir: node ephemeral storage, i.e. the node boot disk. pvc: per-pod PVC on `storageClass.name`, 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. | | priorityClasses | list | `[]` | Additional priorityClasses minimize re-scheduling downtime for StatefulSets. Each StatefulSets might use different priority class. learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/scheduling-eviction/pod-priority-preemption/#priorityclass) Sample class definition: - name: gitserver-class value: 100 preemptionPolicy: Never description: "gitserver priority class" | | prometheus.containerSecurityContext | object | `{"allowPrivilegeEscalation":false,"readOnlyRootFilesystem":false,"runAsGroup":100,"runAsUser":100}` | Security context for the `prometheus` container, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-container) | | prometheus.createRoleBinding | bool | `true` | Disable the creation of a RoleBinding object, for customers who block all RBAC resource creation | diff --git a/charts/sourcegraph/templates/precise-code-intel/worker.Deployment.yaml b/charts/sourcegraph/templates/precise-code-intel/worker.Deployment.yaml index 8f290a21..758eaa9e 100644 --- a/charts/sourcegraph/templates/precise-code-intel/worker.Deployment.yaml +++ b/charts/sourcegraph/templates/precise-code-intel/worker.Deployment.yaml @@ -45,6 +45,10 @@ spec: deploy: sourcegraph app: precise-code-intel-worker spec: + {{- $storageType := .Values.preciseCodeIntel.storageType | default "emptyDir" }} + {{- if not (has $storageType (list "emptyDir" "pvc")) }} + {{- fail (printf "preciseCodeIntel.storageType must be one of emptyDir, pvc; got %q" $storageType) }} + {{- end }} {{- include "sourcegraph.terminationGracePeriodSeconds" (list . "preciseCodeIntel") | nindent 6 }} containers: - name: precise-code-intel-worker @@ -104,8 +108,13 @@ spec: {{- if .Values.preciseCodeIntel.extraContainers }} {{- toYaml .Values.preciseCodeIntel.extraContainers | nindent 6 }} {{- end }} + {{- $podSecurityContext := deepCopy .Values.preciseCodeIntel.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.preciseCodeIntel.containerSecurityContext.runAsGroup | default 101) "fsGroupChangePolicy" "OnRootMismatch") }} + {{- end }} securityContext: - {{- toYaml .Values.preciseCodeIntel.podSecurityContext | nindent 8 }} + {{- toYaml $podSecurityContext | nindent 8 }} {{- include "sourcegraph.nodeSelector" (list . "preciseCodeIntel" ) | trim | nindent 6 }} {{- include "sourcegraph.affinity" (list . "preciseCodeIntel" ) | trim | nindent 6 }} {{- with include "sourcegraph.priorityClassName" (list . "preciseCodeIntel" ) | trim }}{{ . | nindent 6 }}{{- end }} @@ -117,7 +126,22 @@ spec: {{- include "sourcegraph.renderServiceAccountName" (list . "preciseCodeIntel") | trim | nindent 6 }} volumes: - name: tmpdir + {{- if eq $storageType "pvc" }} + ephemeral: + volumeClaimTemplate: + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: {{ required "preciseCodeIntel.storageSize is required when preciseCodeIntel.storageType is pvc" .Values.preciseCodeIntel.storageSize }} + storageClassName: {{ .Values.storageClass.name }} + {{- else if .Values.preciseCodeIntel.storageSize }} + emptyDir: + sizeLimit: {{ .Values.preciseCodeIntel.storageSize }} + {{- else }} emptyDir: {} + {{- end }} {{- if .Values.preciseCodeIntel.extraVolumes }} {{- toYaml .Values.preciseCodeIntel.extraVolumes | nindent 6 }} {{- end }} diff --git a/charts/sourcegraph/tests/preciseCodeIntelStorage_test.yaml b/charts/sourcegraph/tests/preciseCodeIntelStorage_test.yaml new file mode 100644 index 00000000..f60651af --- /dev/null +++ b/charts/sourcegraph/tests/preciseCodeIntelStorage_test.yaml @@ -0,0 +1,130 @@ +--- +suite: preciseCodeIntelStorage +templates: +- precise-code-intel/worker.Deployment.yaml +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: + preciseCodeIntel: + 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: + preciseCodeIntel: + storageType: pvc + storageSize: 50Gi + asserts: + - contains: + path: spec.template.spec.volumes + content: + name: tmpdir + ephemeral: + volumeClaimTemplate: + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 50Gi + storageClassName: sourcegraph + # /tmp mount is unchanged, so no TMPDIR redirect is needed + - contains: + path: spec.template.spec.containers[0].volumeMounts + content: + mountPath: /tmp + name: tmpdir + +- it: should follow storageClass.name like every other PVC in the chart + set: + storageClass: + name: custom-ssd + preciseCodeIntel: + storageType: pvc + storageSize: 50Gi + asserts: + - equal: + path: spec.template.spec.volumes[0].ephemeral.volumeClaimTemplate.spec.storageClassName + value: custom-ssd + +- it: should fail when storageSize is omitted on the pvc branch + set: + preciseCodeIntel: + storageType: pvc + asserts: + - failedTemplate: + errorMessage: preciseCodeIntel.storageSize is required when preciseCodeIntel.storageType is pvc + +- it: should fall back to emptyDir when storageType is absent + set: + preciseCodeIntel: + storageType: null + asserts: + - contains: + path: spec.template.spec.volumes + content: + name: tmpdir + emptyDir: {} + +- it: should fail on an unknown storageType + set: + preciseCodeIntel: + storageType: PVC + storageSize: 50Gi + asserts: + - failedTemplate: + errorMessage: 'preciseCodeIntel.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: + preciseCodeIntel: + 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: + preciseCodeIntel: + storageType: pvc + storageSize: 50Gi + podSecurityContext: + fsGroup: 2000 + runAsNonRoot: true + asserts: + - equal: + path: spec.template.spec.securityContext + value: + fsGroup: 2000 + fsGroupChangePolicy: OnRootMismatch + runAsNonRoot: true diff --git a/charts/sourcegraph/values.yaml b/charts/sourcegraph/values.yaml index 74a1395e..73362f75 100644 --- a/charts/sourcegraph/values.yaml +++ b/charts/sourcegraph/values.yaml @@ -979,6 +979,17 @@ preciseCodeIntel: create: false # -- Name of the ServiceAccount to be created or an existing ServiceAccount name: "" + # -- Backing store for the `/tmp` scratch volume, used for large SCIP upload + # processing. One of: emptyDir, pvc. + # emptyDir: node ephemeral storage, i.e. the node boot disk. + # pvc: per-pod PVC on `storageClass.name`, 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: "" prometheus: # -- Enable `prometheus` (recommended)