Repository navigation
Let a PodDisruptionBudget protect busy ephemeral runners - #4710
Open
aandreev-akamai wants to merge 1 commit into
Open
aandreev-akamai wants to merge 1 commit into
aandreev-akamai wants to merge 1 commit into
Conversation
A PDB selecting runner pods never works today: the pod's controller owner is an EphemeralRunner, which has no scale subresource, so the disruption controller reports SyncFailed (expectedPods: 0). Voluntary evictions such as node drains and managed node auto-upgrades therefore kill in-flight jobs. - Serve a scale subresource on EphemeralRunner (spec/status.replicas, fixed at 1) so the disruption controller can resolve a budget. - Label the runner pod actions.github.com/runner-busy=true once a job is assigned, so a PDB can select busy runners only and idle runners stay evictable. Runner pods are single-use, so the label is never removed. - Ignore replicas when deciding whether the EphemeralRunnerSet template drifted. The CRD defaults it to 1 and older CRDs prune it, so comparing it would bump the revision and delete idle runners. - Grant the controller patch on pods in the scale set manager roles. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
aandreev-akamai
requested review from
a team,
Steve-Glass,
mumoshu,
nikola-jokic,
rentziass and
toast-gear
as code owners
October 9, 2026 14:40
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The scale endpoint and drift-normalization behavior need direct regression coverage.
2 open findings
What changed in this PR
Adds PDB support for busy ephemeral runners to protect in-flight jobs during voluntary disruptions.
Changes:
- Adds a fixed-size
scalesubresource toEphemeralRunner. - Labels busy runner pods and grants required patch permissions.
- Prevents replica defaulting from triggering runner-set drift.
| File | Description |
|---|---|
controllers/actions.github.com/predicates_test.go |
Includes replica status in projection coverage. |
controllers/actions.github.com/helpers.go |
Ignores replicas during drift detection. |
controllers/actions.github.com/ephemeralrunner_controller.go |
Labels busy pods and reports replicas. |
controllers/actions.github.com/ephemeralrunner_controller_test.go |
Tests replica defaults and busy labeling. |
controllers/actions.github.com/constants.go |
Defines the busy-runner label. |
config/crd/bases/actions.github.com_ephemeralrunnersets.yaml |
Adds the embedded replica schema. |
config/crd/bases/actions.github.com_ephemeralrunners.yaml |
Adds replica fields and scale subresource. |
charts/gha-runner-scale-set/templates/manager_role.yaml |
Grants pod patch permission. |
charts/gha-runner-scale-set-experimental/templates/manager_role.yaml |
Grants experimental pod patch permission. |
charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml |
Packages the runner-set schema update. |
charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml |
Packages the scale subresource. |
charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunnersets.yaml |
Updates the experimental runner-set CRD. |
charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunners.yaml |
Updates the experimental runner CRD. |
apis/actions.github.com/v1alpha1/ephemeralrunner_types.go |
Defines replica fields and scale markers. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+244
to
+254
| Eventually( | ||
| func() (int32, error) { | ||
| current := new(v1alpha1.EphemeralRunner) | ||
| if err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, current); err != nil { | ||
| return 0, err | ||
| } | ||
| return current.Status.Replicas, nil | ||
| }, | ||
| ephemeralRunnerTimeout, | ||
| ephemeralRunnerInterval, | ||
| ).Should(BeEquivalentTo(1), "status.replicas should be reported through the scale subresource") |
Comment on lines
+34
to
+35
| currentSpec, desiredSpec := current.Spec.EphemeralRunnerSpec, desired.Spec.EphemeralRunnerSpec | ||
| currentSpec.Replicas, desiredSpec.Replicas = 0, 0 |
This branch has not been deployed
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
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.
Fixes #4618
Problem
A
PodDisruptionBudgetselectinggha-runner-scale-setrunner pods never works. The runner pod's controller owner is anEphemeralRunner, which only has astatussubresource. The disruption controller resolves expected pods through the owner'sscalesubresource, so the PDB reportsSyncFailed(expectedPods: 0, currentHealthy: 0). Voluntary evictions that only honor PDBs (kubectl drain, AKS/EKS/GKE node auto-upgrade) therefore kill in-flight jobs withThe runner has received a shutdown signal.safe-to-evict/do-not-disruptdon't help, they are only honored by the Cluster Autoscaler / Karpenter. See #4493, #2562 and #4618.Change
scalesubresource onEphemeralRunner. Addsspec.replicasandstatus.replicas, both fixed at 1 (spec.replicasis an enum of1, sokubectl scaleis rejected). Existing objects pick up the default on read, so no migration is needed. The controller also reportsstatus.replicas = 1.Busy label on the runner pod. Once a job is assigned, the
EphemeralRunnercontroller setsactions.github.com/runner-busy: "true"on the pod. Runner pods are single-use, so the label is never removed. A PDB can then protect busy runners only, and idle runners stay evictable so drains and upgrades are not blocked by idle capacity:Drift check ignores
replicas. TheEphemeralRunnerSettemplate embedsEphemeralRunnerSpec, so the CRD default would make the stored template differ from the desired one and bump the revision, deleting idle runners. Older CRDs prune the field, so the comparison ignores it instead of asserting a value. This keeps a controller upgrade without a CRD upgrade safe.RBAC. The
gha-runner-scale-setandgha-runner-scale-set-experimentalmanager Roles gainpatchonpods.Upgrade notes
EphemeralRunner/EphemeralRunnerSetCRDs for the PDB to start working. Without them the controller keeps working as before, only without PDB protection.Evidence
runner-busy=truereportsexpectedPods: 1, currentHealthy: 1instead ofSyncFailed. Evicting a busy pod is rejected withTooManyRequests, while evicting an idle pod succeeds. Before the change the same PDB reportsSyncFailedwithexpectedPods: 0.spec.replicasdefaults to 1,status.replicasis reported, an idle pod is not labelled, and the pod is labelled once a job is assigned. The full suite passes (187/187), includingTestPredicateProjectionsCoverEveryStatusField, which now listsReplicas.go test ./...passes excepttest/e2eandtest_e2e_arc, which needGITHUB_TOKENand a live cluster.🤖 Generated with Claude Code