sourcegraph-executor/k8s: opt-in CronJob to clean up orphaned executor job resources - #923

Open issues

Found in review on 2026-11-13 (review thread), at ce2cf93. The cleanup works and is off by default, and the chart defaults are safe; this PR is a draft until the first two are fixed.

1. The render check does not keep running jobs safe. The executor puts the deadline on the job pod, not the Job, so it counts from when the pod starts, while the script ages objects from when they were created, and never looks at whether a Job is still active. Any time a pod spends Pending eats the margin. Reproduced on EKS with deadline: 100 and minimumAgeSeconds: 120, which passes the check: a Job whose pod started 63 seconds after the Job was created was deleted 71 seconds into its 100 second run. The check also lets equal values through, against its own error message, and ignores maximumRuntimePerJob, which bounds how long a multi-step job's Secret and PVC live. Fix: skip Jobs that still have active pods and the Secrets and PVCs that belong to them, and make the check strict
2. A cleanup run that is not allowed to list anything reports success. The script has no pipefail, so a failed kubectl get is ignored. Reproduced on EKS: run as a ServiceAccount with no permissions, the pod logged three Forbidden errors, exited 0, and the Job showed Complete. A release with executor.configureRbac: false and cleanup enabled would do this on every run. Fix: fail the script when kubectl get fails
3. The cleanup pod ignores the chart's pod settings. It gets no nodeSelector, tolerations, affinity, security contexts, or pod annotations from values, and the image runs as root (uid 0 on EKS). A namespace enforcing the restricted Pod Security Standard rejects it, and a tainted executor node pool leaves it Pending. Neither was tried
4. The sweep covers the whole namespace, the check covers one release. With two executor releases in one namespace, cleanup enabled on one deletes the other's objects by its own age limit, whatever the other release's deadline is. Not tried
5. No changelog entry in charts/sourcegraph-executor/CHANGELOG.md, and the RBAC table in the chart README does not list the verbs cleanup adds
6. Small ones:
   • alpine/kubectl:1.33.4 is a community image pinned by tag only, in a pod that can read every Secret in the namespace and create Jobs; pin a digest
   • the tests still pass with the render check weakened and with the name filter removed
   • an unquoted minimumAgeSeconds of 1000000 or more renders in exponent form, which the script's age comparison does not handle (rendered only, not run)

Tested in this review on the EKS test instance m5, by upgrading the executor release from main to this branch merged onto main:

• With default values the manifest is identical to main and no CronJob exists
• With cleanup enabled, the upgrade added the CronJob and the Role's get and list rules, and did not restart the executor
• One run started by hand deleted a finished Job, an old Secret, and an old unbound PVC named sg-executor-job-*, and kept a 2 second old Secret with that prefix and an old Secret without it.
• The run in issue 1 and the run in issue 2

Not tested: a scheduled run, a PVC held by its protection finalizer, hundreds of leftovers within the 600 second and 128Mi limits, and the real executor leaving objects behind.

Problem

The executor deletes each job's Job, Secret, and (optional) PVC in Teardown (kubernetes.go), but Teardown never runs when the executor pod is killed mid-job — OOM kill, node scale-down/spot reclaim, or a deployment rollout. Nothing in Kubernetes garbage-collects the leftovers: the Secrets carry no ownerReferences and the Jobs no ttlSecondsAfterFinished, so they accumulate forever. One real cluster reached 699 orphaned sg-executor-job-* Secrets.

The proper fix (ownerReferences from Secret/PVC → Job plus ttlSecondsAfterFinished) belongs in the executor code in sourcegraph/sourcegraph; until then the chart can sweep up.

Change

• New opt-in executor.cleanup.* values deploying a CronJob that deletes sg-executor-job-* Jobs, Secrets, and PersistentVolumeClaims older than executor.cleanup.minimumAgeSeconds (default 3600s, every 30 min).
• Template rendering fails if minimumAgeSeconds does not exceed executor.kubernetesJob.deadline (default 1200s), so resources of running jobs are never touched — any executor job older than the deadline has already been terminated by activeDeadlineSeconds.
• Reuses the sg-executor ServiceAccount. Only with executor.cleanup.enabled, the executor Role gains get and list on jobs, secrets, and persistentvolumeclaims (it already has delete on all three). fix/resources: Drop unused ports, Services, Role verbs, and settings #1016 removes get and list on Secrets and PVCs from the default Role, because the executor never calls them.
• kubectl delete runs with --wait=false: waiting lists and watches objects that linger, like a PVC held by its protection finalizer, and the Role has no watch.
• Image: alpine/kubectl (kubectl + shell; bitnami/kubectl is deprecated and registry.k8s.io/kubectl is distroless, no shell). Fully overridable for air-gapped mirrors.

Testing

• helm lint and helm template clean
• helm unittest: 11/11 pass, including new tests/cleanup_test.yaml (default off, renders when enabled, fails render on unsafe minimumAgeSeconds, Role grants get/list when enabled)
• Merged with fix/resources: Drop unused ports, Services, Role verbs, and settings #1016's head (git merge-tree): no conflict, 11/11 pass, and the Role grants the cleanup verbs only when cleanup is enabled
• Verified alpine/kubectl:1.33.4 on a live EKS cluster: busybox date -D parses creationTimestamp correctly and kubectl is present
• helm-docs regenerated (README diff contains only the four new value rows)