Skip to content

executor: remove unused /data PVC and dead replicas value from k8s chart#911

Merged
michaellzc merged 1 commit into
mainfrom
michaellzc/remove-executor-data-pvc
Jul 21, 2026
Merged

executor: remove unused /data PVC and dead replicas value from k8s chart#911
michaellzc merged 1 commit into
mainfrom
michaellzc/remove-executor-data-pvc

Conversation

@michaellzc

@michaellzc michaellzc commented Jul 20, 2026

Copy link
Copy Markdown
Member

Summary

Removes the executor controller's unused /data PersistentVolumeClaim from the Kubernetes-native executor chart (sourcegraph-executor/k8s) — the container volumeMount, the pod-spec volume, the PVC template, and the now-orphaned storageClass / executor.storageSize values / vestigial EXECUTOR_KUBERNETES_PERSISTENCE_VOLUME_NAME env var. Since single-job-pod became the only k8s execution mode (#1163), job pods use their own ephemeral emptyDir at /job and the controller writes nothing to /data. Also removes the non-functional executor.replicas value, since the controller is a singleton that pins Job pods to its own node (replicas > 1 never worked — extra pods contend for the same ReadWriteOnce PVC). RBAC for job PVCs and the separate firecracker/dind chart are intentionally untouched.

Checklist

Test plan

helm lint passes and all 7 helm unittest cases pass. helm template renders with no mountPath: /data, no PVC volume/resource, and no PERSISTENCE_VOLUME env var.

Validated the upgrade path for an existing installation end-to-end on a local kind cluster:

  1. Installed the current main chart (with the /data PVC) → pod 1/1 Running, PVC sg-executor-codeintel Bound (10Gi RWO), Deployment mounting it at /data.
  2. Ran helm upgrade --wait to this branch's chart. Result:
    • Helm release deployed (rev 2), rollout succeeded, new pod 1/1 Running with 0 restarts.
    • PVC and its backing PV deleted cleanly (reclaimPolicy Delete GC'd the PV); the old pod terminated so the PVC finalizer cleared within seconds — no stuck Terminating resources.
    • Deployment volumes/volumeMounts empty; ConfigMap env var removed; no Warning events.

Note: on clusters using a Retain PV reclaim policy, the released PV remains and an admin would delete it manually (the /data volume held only lost+found in k8s mode, so no data loss).

Follow-up (not in this PR): the "Job scheduling on Kubernetes" self-hosted docs page still states jobs must co-locate with the executor to share a Persistent Volume — that requirement died with multi-job mode and should be corrected.

Remove the executor controller's `/data` PersistentVolumeClaim from the
Kubernetes-native executor chart, together with its container volumeMount,
pod-spec volume, the orphaned `storageClass`/`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`.

Also remove the non-functional `executor.replicas` value: the controller is
a singleton that pins Job pods to its own node, so `replicas > 1` never
worked (extra replicas contend for the same ReadWriteOnce PVC).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@michaellzc
michaellzc requested review from a team July 21, 2026 00:02
@marcleblanc2

Copy link
Copy Markdown
Contributor

LGTM.

I'm not familiar with the KUBERNETES_JOB_VOLUME_TYPE=pvc mode, but if it's not in use, then we should also remove the PVC permissions from the Kubernetes RBAC role here. DS thread here.

@michaellzc
michaellzc merged commit d72cf84 into main Jul 21, 2026
5 checks passed
@michaellzc
michaellzc deleted the michaellzc/remove-executor-data-pvc branch July 21, 2026 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants