Add runnerVariants: many runner pod specs behind one listener and one RBAC bundle - #4614
Open
HarikrishnanBalagopal wants to merge 4 commits into
Open
Add runnerVariants: many runner pod specs behind one listener and one RBAC bundle#4614HarikrishnanBalagopal wants to merge 4 commits into
HarikrishnanBalagopal wants to merge 4 commits into
Conversation
The listener process is hard-wired to one scale set: it opens one message session for one RunnerScaleSetID and drives one EphemeralRunnerSet. This is the first part of the runnerVariants feature, which lets one AutoscalingRunnerSet declare several runner variants served by one listener pod. config: add an optional ScaleSets list to the listener config. When empty the listener behaves exactly as before (single set from the scalar fields). EffectiveScaleSets() returns one synthetic entry from the scalars in that case, so the driver treats both modes the same way. validateScaleSets() checks each entry and rejects duplicate EphemeralRunnerSet names. metrics: split the exporter into a shared server (registry + HTTP endpoint) and per-scale-set recorders. Each recorder stamps its own scale set name, so several variants can export on one metrics port without colliding. The single-set path keeps the same series and labels. main: fan out one session per effective scale set. Single-set mode keeps the original semantics exactly (a listener error propagates and the pod exits so the controller recreates it). Multi-set mode supervises each session with capped backoff so one bad session cannot cancel its siblings; only shutdown ends the run. Tests cover EffectiveScaleSets, list validation, per-scale-set recorders, and the supervisor restart + failure-isolation behaviour.
Add the RunnerVariant type and the optional RunnerVariants list to AutoscalingRunnerSetSpec, plus the ListenerScaleSet tuple type and a runner-variant label and runner-scale-set-ids annotation constant. The types are not wired into the reconciler yet; this commit only introduces the API surface and its generated code so the empty (single-shape) path stays provably byte-for-byte identical. EffectiveVariants resolves the spec into the variants to reconcile, returning one default variant (empty name) carrying the top level values when RunnerVariants is empty, so later commits can loop over the result without a separate default code path. Back-compat is guarded by golden hash tests captured from the pre-runnerVariants HEAD: the EphemeralRunnerSet integrity hash, the AutoscalingListener spec hash, the listener Role hash and the RunnerSetSpecHash are all unchanged for the default path. The AutoscalingRunnerSet level Hash and ListenerSpecHash change once (spew includes the new nil field); that flips the set through Pending on the first reconcile after upgrade but churns no child object, and the post-change values are pinned so the one-time nature stays visible. The Makefile strips the same x-kubernetes-list keys under runnerVariants[].template.spec as it already does for the top level template.spec; the two CRD subtrees are byte-identical after the fix.
Wire the reconciler and resource builder to the runnerVariants CRD field added in the previous commit. When an AutoscalingRunnerSet declares one or more variants it takes a fan-out path: one EphemeralRunnerSet per variant behind a single AutoscalingListener and one RBAC bundle. A set with no variants keeps the classic single-set path byte-for-byte, pinned by the golden hash tests. Reconciler (autoscalingrunnerset_controller.go): - register a runner scale set id per variant; keep the scalar runner-scale-set-id annotation and add a variant name to id map on actions.github.com/runner-scale-set-ids, so single-set stays unchanged - shouldCreateScaleSet re-enters registration when any declared variant still lacks an id, so adding a variant registers it incrementally - reconcileMultiVariant creates the missing EphemeralRunnerSet per variant, patches drifted ones by integrity hash, and deletes orphans left by a removed variant - reconcileMultiVariantListener keeps exactly one listener, built from the first variant's EphemeralRunnerSet, carrying the scale set tuples on the out of band actions.github.com/listener-scale-sets annotation so the listener spec and hash stay stable - run the multi-variant branch before the scalar runner group and name reconciliation, which are single-scale-set concepts - cleanupEphemeralRunnerSet lists owned sets by label so no variant is orphaned on delete; the single set matches the same labels Resource builder (resourcebuilder.go): - newEphemeralRunnerSetForVariant names the set, resolves its scale set id, applies the variant template, and stamps the runner-variant label only for a named variant - the listener Role scopes over every child EphemeralRunnerSet name decoded from the listener annotation; single-set scopes over the one scalar name - newScaleSetListenerConfig appends the extra scale sets to config.json only when the annotation is present, so single-set config bytes are unchanged Tests: table tests for the pure helpers (naming, id resolution, annotation round trips, shouldCreateScaleSet, isMultiVariant) and an envtest suite that creates a two variant set and asserts one EphemeralRunnerSet per variant with its own id and template, a single listener carrying both scale sets, orphan cleanup on variant removal, and full cleanup on delete.
Render an optional runnerVariants list on the AutoscalingRunnerSet chart, with a values example and a helper that rejects duplicate or non-DNS-label variant names and names that would make the EphemeralRunnerSet name longer than 63 characters. Add template tests for the two-variant render and the duplicate-name failure, and an ADR that records the design and its back-compat guarantees.
HarikrishnanBalagopal
requested review from
Steve-Glass,
mumoshu,
rentziass and
toast-gear
as code owners
August 28, 2026 00:13
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.
What this does
Adds an optional
runnerVariantslist toAutoscalingRunnerSet. One scale set can now bring up runner pods of different specs (image, resources) behind a single listener pod and a single RBAC bundle (ServiceAccount, Role, RoleBinding, config Secret).Fixes / addresses #4169.
When
runnerVariantsis empty the scale set behaves exactly as before, byte-for-byte. This is guarded by golden-hash tests.Why
Today each shape of runner needs its own
AutoscalingRunnerSet, and each ARS forces its own always-on listener pod plus its own RBAC bundle. On large clusters this multiplies pods and objects for no functional reason. #4169 reports a fleet of ~88 ARS that differ almost only in image and a small number of resource shapes. Splitting one pod spec per scale set id is a GitHub Actions protocol requirement; the number of listener pods and RBAC bundles is an ARC modeling choice.github.com/actions/scaleset(v0.4.0) already exposes a per-scale-set-id, goroutine-safeMessageSessionClient, so one listener process can drive many scale sets. The whole feature therefore ships from this repo alone, with noscalesetrelease dependency.Design
An ADR is included at
docs/adrs/2026-08-28-runner-variants.md. In short:name, its ownrunnerScaleSetLabels, an optional podtemplate, and optionalminRunners/maxRunnersthat override the ARS-level values.EphemeralRunnerSetper variant, and creates exactly oneAutoscalingListener.actions.github.com/listener-scale-setsannotation, not in the hashed listener spec. Adding a slice field to the hashed spec would change the hash of every existing single-variant listener (spew prints nil slices and the type name) and force a fleet-wide listener pod recreation on upgrade. The annotation is absent for single-variant sets, so their listener spec, Role, andconfig.jsonstay byte-for-byte identical.Commit layout (please review in order)
cmd/ghalistener). OptionalscaleSetsconfig; empty list keeps the original single-session behavior.runner-scale-set-idsannotation map, and the add / remove / delete / orphan flows. Includes a real fix:cleanupEphemeralRunnerSetused to look up only the ERS named after the ARS and would have orphaned variant ERS on delete; it now lists by label and deletes all owned sets.runnerVariants:values example, a template block, a helper that validates variant names, and chart template tests.Ordering note: the reconciler must not emit
scaleSetsto a listener image that predates commit 1. Within one PR / release that is automatic.Back-compat guarantee
For the empty-variant path these are pinned identical to the release tip by golden tests: the ephemeral runner set integrity hash, the listener spec hash, the Role integrity hash, and the runner-set spec hash. The ARS-level
Hash()andListenerSpecHash()change once (the spec gained a field), which is a single Pending-to-Running flip on the ARS on upgrade with no child object churn.Testing
make generate manifestsclean (no strayx-kubernetes-list-*keys under the new subtree).gofmt,go vet,go build ./...clean.BackCompat,ARSLevelHash,EffectiveVariants).AutoscalingListener; remove-variant orphan delete; delete-ARS full cleanup).ok ... 181s, no regression).cmd/ghalistener/...green under-race.kind e2e / acceptance is not included in this PR; happy to add it if you would prefer it in-tree.
Note
#4169 does not have a maintainer response yet and CONTRIBUTING asks to discuss significant changes first. The ADR and this PR are the concrete proposal for that discussion. Merge is entirely at the maintainers' discretion; the design is written to keep the existing single-variant path unchanged so the change is safe to defer or revise.