From 64409cffc5f6106c686ea8620919ba9aa06c60f8 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Wed, 19 Aug 2026 22:15:23 -0600 Subject: [PATCH 1/2] sourcegraph-executor/k8s: add opt-in CronJob to clean up orphaned job resources The executor deletes each job's Job, Secret, and PVC in Teardown, but Teardown never runs when the executor is killed mid-job (OOM kill, node scale-down, rollout). Nothing garbage-collects those resources: the Secrets have no ownerReferences and the Jobs no ttlSecondsAfterFinished, so they accumulate forever (one real cluster reached 699 orphaned sg-executor-job-* secrets). Add an opt-in CronJob (executor.cleanup.enabled) that deletes sg-executor-job-* Jobs, Secrets, and PersistentVolumeClaims older than executor.cleanup.minimumAgeSeconds. The template fails rendering if that age does not exceed executor.kubernetesJob.deadline, so resources of running jobs are never deleted. Reuses the sg-executor ServiceAccount; the executor Role additionally gets get/list on jobs. Amp-Thread-ID: https://ampcode.com/threads/T-01a01d55-b956-718b-a863-ef9c8c5648be Co-authored-by: Amp --- charts/sourcegraph-executor/k8s/README.md | 4 ++ .../templates/executor.CleanupCronJob.yaml | 72 +++++++++++++++++++ .../k8s/templates/executor.Role.yaml | 2 + .../k8s/tests/cleanup_test.yaml | 40 +++++++++++ charts/sourcegraph-executor/k8s/values.yaml | 11 +++ 5 files changed, 129 insertions(+) create mode 100644 charts/sourcegraph-executor/k8s/templates/executor.CleanupCronJob.yaml create mode 100644 charts/sourcegraph-executor/k8s/tests/cleanup_test.yaml diff --git a/charts/sourcegraph-executor/k8s/README.md b/charts/sourcegraph-executor/k8s/README.md index 1b5905a94..081fa5303 100644 --- a/charts/sourcegraph-executor/k8s/README.md +++ b/charts/sourcegraph-executor/k8s/README.md @@ -57,6 +57,10 @@ In addition to the documented values, the `executor` and `private-docker-registr | Key | Type | Default | Description | |-----|------|---------|-------------| | executor.affinity | object | `{}` | Affinity, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#affinity-and-anti-affinity) | +| executor.cleanup.enabled | bool | `false` | If true, deploy a CronJob that deletes leftover executor job resources (Jobs, Secrets, and PersistentVolumeClaims named `sg-executor-job-*`). The executor deletes these itself after each job, but leaves them behind when it is killed mid-job (e.g. OOM kill, node scale-down, rollout). | +| executor.cleanup.image | string | `"index.docker.io/alpine/kubectl:1.33.4"` | A kubectl image that includes a shell, used by the cleanup CronJob. | +| executor.cleanup.minimumAgeSeconds | string | `"3600"` | Only delete resources older than this many seconds. Must be greater than `executor.kubernetesJob.deadline`, so resources of running jobs are never deleted. | +| executor.cleanup.schedule | string | `"*/30 * * * *"` | Cron schedule of the cleanup CronJob. | | executor.configureRbac | bool | `true` | Whether to configure the necessary RBAC resources. Required only once for all executor deployments. | | executor.containerSecurityContext | object | `{"privileged":false}` | Security context for the container, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-container) | | executor.debug.keepJobs | string | `"false"` | If true, Kubernetes jobs will not be deleted after they complete. Not recommended for production use as it can hit cluster limits. | diff --git a/charts/sourcegraph-executor/k8s/templates/executor.CleanupCronJob.yaml b/charts/sourcegraph-executor/k8s/templates/executor.CleanupCronJob.yaml new file mode 100644 index 000000000..4e5bfe8cc --- /dev/null +++ b/charts/sourcegraph-executor/k8s/templates/executor.CleanupCronJob.yaml @@ -0,0 +1,72 @@ +{{- if .Values.executor.cleanup.enabled }} +{{- if lt (int .Values.executor.cleanup.minimumAgeSeconds) (int .Values.executor.kubernetesJob.deadline) }} +{{- fail "executor.cleanup.minimumAgeSeconds must be greater than executor.kubernetesJob.deadline, so resources of running jobs are never deleted" }} +{{- end }} +apiVersion: batch/v1 +kind: CronJob +metadata: + name: {{ include "executor.name" . }}-cleanup + annotations: + description: Deletes leftover executor job resources, which the executor could not tear down because it was killed mid-job + labels: + {{- include "executor.labels" . | nindent 4 }} +spec: + schedule: {{ .Values.executor.cleanup.schedule | quote }} + concurrencyPolicy: Forbid + successfulJobsHistoryLimit: 1 + failedJobsHistoryLimit: 3 + jobTemplate: + spec: + activeDeadlineSeconds: 600 + template: + metadata: + labels: + {{- include "executor.labels" . | nindent 12 }} + spec: + serviceAccountName: sg-executor + restartPolicy: OnFailure + containers: + - name: cleanup + image: {{ .Values.executor.cleanup.image }} + imagePullPolicy: {{ .Values.sourcegraph.image.pullPolicy }} + env: + - name: NAMESPACE + value: {{ .Values.executor.namespace | quote }} + - name: MINIMUM_AGE_SECONDS + value: {{ .Values.executor.cleanup.minimumAgeSeconds | quote }} + command: + - /bin/sh + - -eu + - -c + - | + now_seconds=$(date -u +%s) + sweep() { + kind=$1 + kubectl get "$kind" --namespace "$NAMESPACE" \ + --output 'jsonpath={range .items[*]}{.metadata.name} {.metadata.creationTimestamp}{"\n"}{end}' \ + | while read -r name created_at; do + case "$name" in sg-executor-job-*) ;; *) continue ;; esac + created_seconds=$(date -u -D '%Y-%m-%dT%H:%M:%SZ' -d "$created_at" +%s) + if [ $((now_seconds - created_seconds)) -ge "$MINIMUM_AGE_SECONDS" ]; then + echo "deleting $kind/$name, created at $created_at" + kubectl delete "$kind" "$name" --namespace "$NAMESPACE" --ignore-not-found + fi + done + } + sweep jobs + sweep secrets + sweep persistentvolumeclaims + {{- if not .Values.sourcegraph.localDevMode }} + resources: + requests: + cpu: 10m + memory: 32Mi + limits: + cpu: 100m + memory: 128Mi + {{- end }} + {{- with .Values.sourcegraph.imagePullSecrets }} + imagePullSecrets: + {{- toYaml . | nindent 12 }} + {{- end }} +{{- end }} diff --git a/charts/sourcegraph-executor/k8s/templates/executor.Role.yaml b/charts/sourcegraph-executor/k8s/templates/executor.Role.yaml index 7a3d1ef4d..cf7f510e8 100644 --- a/charts/sourcegraph-executor/k8s/templates/executor.Role.yaml +++ b/charts/sourcegraph-executor/k8s/templates/executor.Role.yaml @@ -14,6 +14,8 @@ rules: resources: - jobs verbs: + - get + - list - create - delete - apiGroups: diff --git a/charts/sourcegraph-executor/k8s/tests/cleanup_test.yaml b/charts/sourcegraph-executor/k8s/tests/cleanup_test.yaml new file mode 100644 index 000000000..e0abceedd --- /dev/null +++ b/charts/sourcegraph-executor/k8s/tests/cleanup_test.yaml @@ -0,0 +1,40 @@ +suite: executor cleanup +templates: + - executor.CleanupCronJob.yaml +tests: + - it: should not render the CronJob by default + set: + executor: + queueName: "test" + asserts: + - hasDocuments: + count: 0 + + - it: should render the CronJob when cleanup is enabled + set: + executor: + queueName: "test" + cleanup: + enabled: true + asserts: + - containsDocument: + kind: CronJob + apiVersion: batch/v1 + name: executor-test-cleanup + - equal: + path: spec.schedule + value: "*/30 * * * *" + - equal: + path: spec.jobTemplate.spec.template.spec.serviceAccountName + value: sg-executor + + - it: should fail when minimumAgeSeconds does not exceed the job deadline + set: + executor: + queueName: "test" + cleanup: + enabled: true + minimumAgeSeconds: "600" + asserts: + - failedTemplate: + errorMessage: "executor.cleanup.minimumAgeSeconds must be greater than executor.kubernetesJob.deadline, so resources of running jobs are never deleted" diff --git a/charts/sourcegraph-executor/k8s/values.yaml b/charts/sourcegraph-executor/k8s/values.yaml index 45673ee78..af2d1f3dc 100644 --- a/charts/sourcegraph-executor/k8s/values.yaml +++ b/charts/sourcegraph-executor/k8s/values.yaml @@ -135,6 +135,17 @@ executor: keepJobs: "false" keepWorkspaces: "false" + cleanup: + # -- If true, deploy a CronJob that deletes leftover executor job resources (Jobs, Secrets, and PersistentVolumeClaims named `sg-executor-job-*`). + # The executor deletes these itself after each job, but leaves them behind when it is killed mid-job (e.g. OOM kill, node scale-down, rollout). + enabled: false + # -- Cron schedule of the cleanup CronJob. + schedule: "*/30 * * * *" + # -- Only delete resources older than this many seconds. Must be greater than `executor.kubernetesJob.deadline`, so resources of running jobs are never deleted. + minimumAgeSeconds: "3600" + # -- A kubectl image that includes a shell, used by the cleanup CronJob. + image: "index.docker.io/alpine/kubectl:1.33.4" + # -- Affinity, # learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#affinity-and-anti-affinity) affinity: { } From ce2cf932ec92645b4c6e1e3b2dc46db6f4460b75 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Thu, 12 Nov 2026 22:33:09 -0600 Subject: [PATCH 2/2] executor/k8s: grant cleanup RBAC only when cleanup is enabled The cleanup CronJob runs as sg-executor and lists Jobs, Secrets, and PVCs, and kubectl delete gets each one first. #1017 removes get and list on Secrets and PVCs from the executor's Role, since the executor never calls them, which would leave the CronJob with 403s. Grant get and list on all three in a rule rendered only with executor.cleanup.enabled, instead of on Jobs for every install. kubectl delete also waits by default, which lists and watches an object that lingers, such as a PVC held by its protection finalizer. The Role has no watch, so delete with --wait=false. Amp-Thread-ID: https://ampcode.com/threads/T-01a1d269-2788-7075-b66b-b87e3087047d Co-authored-by: Amp --- .../templates/executor.CleanupCronJob.yaml | 2 +- .../k8s/templates/executor.Role.yaml | 20 +++++++++++++-- .../k8s/tests/cleanup_test.yaml | 25 +++++++++++++++++++ 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/charts/sourcegraph-executor/k8s/templates/executor.CleanupCronJob.yaml b/charts/sourcegraph-executor/k8s/templates/executor.CleanupCronJob.yaml index 4e5bfe8cc..6d0ded6a4 100644 --- a/charts/sourcegraph-executor/k8s/templates/executor.CleanupCronJob.yaml +++ b/charts/sourcegraph-executor/k8s/templates/executor.CleanupCronJob.yaml @@ -49,7 +49,7 @@ spec: created_seconds=$(date -u -D '%Y-%m-%dT%H:%M:%SZ' -d "$created_at" +%s) if [ $((now_seconds - created_seconds)) -ge "$MINIMUM_AGE_SECONDS" ]; then echo "deleting $kind/$name, created at $created_at" - kubectl delete "$kind" "$name" --namespace "$NAMESPACE" --ignore-not-found + kubectl delete "$kind" "$name" --namespace "$NAMESPACE" --ignore-not-found --wait=false fi done } diff --git a/charts/sourcegraph-executor/k8s/templates/executor.Role.yaml b/charts/sourcegraph-executor/k8s/templates/executor.Role.yaml index cf7f510e8..54e063938 100644 --- a/charts/sourcegraph-executor/k8s/templates/executor.Role.yaml +++ b/charts/sourcegraph-executor/k8s/templates/executor.Role.yaml @@ -14,10 +14,26 @@ rules: resources: - jobs verbs: - - get - - list - create - delete + {{- if .Values.executor.cleanup.enabled }} + # The cleanup CronJob lists leftover job resources, and kubectl delete gets each one first + - apiGroups: + - batch + resources: + - jobs + verbs: + - get + - list + - apiGroups: + - "" + resources: + - secrets + - persistentvolumeclaims + verbs: + - get + - list + {{- end }} - apiGroups: - "" resources: diff --git a/charts/sourcegraph-executor/k8s/tests/cleanup_test.yaml b/charts/sourcegraph-executor/k8s/tests/cleanup_test.yaml index e0abceedd..fa8f4cd70 100644 --- a/charts/sourcegraph-executor/k8s/tests/cleanup_test.yaml +++ b/charts/sourcegraph-executor/k8s/tests/cleanup_test.yaml @@ -1,8 +1,10 @@ suite: executor cleanup templates: - executor.CleanupCronJob.yaml + - executor.Role.yaml tests: - it: should not render the CronJob by default + template: executor.CleanupCronJob.yaml set: executor: queueName: "test" @@ -11,6 +13,7 @@ tests: count: 0 - it: should render the CronJob when cleanup is enabled + template: executor.CleanupCronJob.yaml set: executor: queueName: "test" @@ -29,6 +32,7 @@ tests: value: sg-executor - it: should fail when minimumAgeSeconds does not exceed the job deadline + template: executor.CleanupCronJob.yaml set: executor: queueName: "test" @@ -38,3 +42,24 @@ tests: asserts: - failedTemplate: errorMessage: "executor.cleanup.minimumAgeSeconds must be greater than executor.kubernetesJob.deadline, so resources of running jobs are never deleted" + + - it: should let the CronJob's ServiceAccount get and list job resources when cleanup is enabled + template: executor.Role.yaml + set: + executor: + queueName: "test" + cleanup: + enabled: true + asserts: + - contains: + path: rules + content: + apiGroups: ["batch"] + resources: ["jobs"] + verbs: ["get", "list"] + - contains: + path: rules + content: + apiGroups: [""] + resources: ["secrets", "persistentvolumeclaims"] + verbs: ["get", "list"]