zombie-cleanup: protect busy ARC runners, reap only hung ones - #953
Open
jeanschmidt wants to merge 1 commit into
Open
zombie-cleanup: protect busy ARC runners, reap only hung ones#953jeanschmidt wants to merge 1 commit into
jeanschmidt wants to merge 1 commit into
Conversation
**Impact:** ARC runner pods in the `arc-runners` namespace on all clusters **Risk:** medium ## What Teaches the zombie-cleanup CronJob to tell an idle runner apart from one actively running a job. Runners executing a workflow are now protected from age-based reaping until they cross a much larger busy hard cap (48h, presumed hung), and every delete candidate is re-checked against fresh per-pod state immediately before deletion so a runner that picks up a job mid-run is never killed. Also bumps `running_max_age_hours` 12h→24h and adds the `busy_max_age_hours` knob. ## Why The previous logic reaped any runner pod once it had been `Running` past the age threshold, with no notion of whether it was doing work. A long-running-but-legitimate job (up to GHA's 6h max plus prepare time) sitting past the threshold would get its pod deleted out from under it, failing the job. This keeps the GC's value — cleaning up genuinely stuck/orphaned pods — while no longer disrupting live jobs. Busy state is derived from the owning `EphemeralRunner` (`status.jobId`) and from live workflow/step pods that pin a runner via the `runner-pod` label. # Notes - New RBAC: the cleanup ServiceAccount now needs `get`/`list` on `actions.github.com/ephemeralrunners`. - Fail-safe by design: if the EphemeralRunner listing or a per-pod GET fails, runner/job pods are protected rather than deleted; bare pods still clean normally. New gauges/counter (`runner_pods_busy_skipped`, `job_pods_protected`, `ephemeralrunner_read_errors`, `recheck_skipped`, `busy_hardcap_deletions_total`) surface this on the pushgateway. - The pod-age-as-job-runtime proxy holds only because every scale set runs `minRunners: 0` (pod created at job assignment). A warm pool (`minRunners > 0`) would break the busy hard cap — `validate_config` fails closed on unsafe thresholds but cannot detect that. Signed-off-by: Jean Schmidt <contato@jschmidt.me>
tofu plan — meta-prod-aws-ue2✅ Plan succeeded · commit Plan output |
tofu plan — meta-prod-aws-uw1✅ Plan succeeded · commit Plan output |
tofu plan — meta-prod-aws-ue1✅ Plan succeeded · commit Plan output |
tofu plan — lf-prod-aws-ue1✅ Plan succeeded · commit Plan output |
tofu plan — lf-prod-aws-ue2✅ Plan succeeded · commit Plan output |
huydhn
reviewed
Jul 29, 2026
| return None | ||
|
|
||
|
|
||
| def classify_pod( |
Contributor
There was a problem hiding this comment.
This function doesn't classify placeholder pods, asking Claude to walk me through the logic there:
1. is_managed_pod → owner is a ConfigMap, not in {ReplicaSet,DaemonSet,StatefulSet,Job} → not skipped.
2. is_ephemeralrunner_pod → no EphemeralRunner owner → False.
3. get_runner_pod_label → no runner-pod label → None.
4. → falls straight through to _age_decision.
My understand is that keeps the current status quo for placeholder pods. So far so good, but is there any chance for shenanigan here? Like zombie-clean up reaps placeholder runner but not workflow, or vice versa
huydhn
reviewed
Jul 29, 2026
| total_count = 0 | ||
| managed_count = 0 | ||
| busy_skipped = 0 | ||
| job_protected = 0 |
Contributor
There was a problem hiding this comment.
Do we want to keep track of failsafe_protected here too? it looks like an important metric IMO as a high value of failsafe_protected means that zombie-cleanup is having issue doing its job
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
2 participants
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.
Impact: zombie-cleanup
Risk: high
What
Teaches the zombie-cleanup CronJob to tell an idle runner apart from one actively running a job. Runners executing a workflow are now protected from age-based reaping until they cross a much larger busy hard cap (48h, presumed hung), and every delete candidate is re-checked against fresh per-pod state immediately before deletion so a runner that picks up a job mid-run is never killed. Also bumps
running_max_age_hours12h→24h and adds thebusy_max_age_hoursknob.Why
The previous logic reaped any runner pod once it had been
Runningpast the age threshold, with no notion of whether it was doing work. A long-running-but-legitimate job (up to GHA's 6h max plus prepare time) sitting past the threshold would get its pod deleted out from under it, failing the job. This keeps the GC's value — cleaning up genuinely stuck/orphaned pods — while no longer disrupting live jobs. Busy state is derived from the owningEphemeralRunner(status.jobId) and from live workflow/step pods that pin a runner via therunner-podlabel.