sourcegraph-executor/k8s: opt-in CronJob to clean up orphaned executor job resources - #923
Open
marcleblanc2 wants to merge 1 commit into
Open
sourcegraph-executor/k8s: opt-in CronJob to clean up orphaned executor job resources#923marcleblanc2 wants to merge 1 commit into
marcleblanc2 wants to merge 1 commit into
Conversation
… 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 <amp@ampcode.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The executor deletes each job's Job, Secret, and (optional) PVC in
Teardown(kubernetes.go), butTeardownnever 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 noownerReferencesand the Jobs nottlSecondsAfterFinished, so they accumulate forever. One real cluster reached 699 orphanedsg-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
executor.cleanup.*values deploying a CronJob that deletessg-executor-job-*Jobs, Secrets, and PersistentVolumeClaims older thanexecutor.cleanup.minimumAgeSeconds(default 3600s, every 30 min).minimumAgeSecondsdoes not exceedexecutor.kubernetesJob.deadline(default 1200s), so resources of running jobs are never touched — any executor job older than the deadline has already been terminated byactiveDeadlineSeconds.sg-executorServiceAccount; the executor Role gainsget/listonjobs(it already had list on secrets/PVCs and delete on all three).alpine/kubectl(kubectl + shell;bitnami/kubectlis deprecated andregistry.k8s.io/kubectlis distroless, no shell). Fully overridable for air-gapped mirrors.Testing
helm lintandhelm templatecleanhelm unittest: 10/10 pass, including newtests/cleanup_test.yaml(default off, renders when enabled, fails render on unsafeminimumAgeSeconds)alpine/kubectl:1.33.4on a live EKS cluster: busyboxdate -DparsescreationTimestampcorrectly and kubectl is presenthelm-docsregenerated (README diff contains only the four new value rows)