public_git/actions-runner-controller
1
0
Fork 0
mirror of https://github.com/actions-runner-controller/actions-runner-controller.git synced 2026-09-30 07:28:01 +02:00
Code Issues Projects Releases Wiki Activity

Track AutoscalingRunnerSet updates with metadata.generation

Browse Source 
The AutoscalingRunnerSet controller detected changes by hashing the spec
and the labels on every reconcile and comparing the result against the
actions.github.com/integrity-hash annotation it had written on a previous
pass. That is a hand-rolled version of something the API server already
does for us: it bumps metadata.generation on every spec write, and nothing
else. Recording that value in the status as ObservedGeneration gives the
same "has this changed since I last applied it" signal without recomputing
a hash, without an extra non-status write to the object, and with a value a
user can read and reason about.

updateStatus now takes the observed generation and patches it next to the
phase, returning early only when both are already what we want. When the
generation is ahead of the observed generation we move to Pending but
deliberately leave the observed generation where it is; it only catches up
at the end of a reconcile that actually applied the change, so a reconcile
that fails part way through is retried as pending rather than being
mistaken for settled.

The old code returned immediately after stamping the hash. That was needed
because stamping the annotation was itself a write to the object, so
continuing would have worked from a stale copy. Recording the generation
only touches the status subresource, which does not bump generation, so the
rest of the reconcile can run on the same object and apply the change in
the same pass instead of waiting for the next event.

One user-visible difference: the old hash covered Labels as well as Spec,
and metadata.generation does not move when only labels change. Editing just
a label on an AutoscalingRunnerSet no longer forces it through Pending.
Labels still propagate to the EphemeralRunnerSet; they simply no longer
count as an update that has to be applied.

Hash, ListenerSpecHash and RunnerSetSpecHash are removed. The latter two
already had no callers, and the first has none now.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Nikola Jokic
2026-09-10 22:56:47 +02:00
co-authored by Copilot App
parent 0cfedfbb2c
commit b20df164d6
6 changed files with 125 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go
+4 -43
View File
@@ -23,7 +23,6 @@ import (
"net/url"
"strings"
"github.com/actions/actions-runner-controller/hash"
"github.com/actions/actions-runner-controller/vault"
"golang.org/x/net/http/httpproxy"
corev1 "k8s.io/api/core/v1"
@@ -322,6 +321,10 @@ type HistogramMetric struct {
type AutoscalingRunnerSetStatus struct {
// +optional
Phase AutoscalingRunnerSetPhase `json:"phase"`
// ObservedGeneration tracks the metadata.generation of this ARS at observation time,
// enabling detection of Pending phase when generation differs. Unset defaults to 0.
// +optional
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
}
type AutoscalingRunnerSetPhase string
@@ -334,26 +337,6 @@ const (
AutoscalingRunnerSetPhaseOutdated AutoscalingRunnerSetPhase = "Outdated"
)
func (ars *AutoscalingRunnerSet) Hash() string {
type data struct {
Spec *AutoscalingRunnerSetSpec
Labels map[string]string
}
d := &data{
Spec: ars.Spec.DeepCopy(),
Labels: ars.Labels,
}
return hash.ComputeTemplateHash(d)
}
func (ars *AutoscalingRunnerSet) ListenerSpecHash() string {
arsSpec := ars.Spec.DeepCopy()
spec := arsSpec
return hash.ComputeTemplateHash(&spec)
}
func (ars *AutoscalingRunnerSet) GitHubConfigSecret() string {
return ars.Spec.GitHubConfigSecret
}
@@ -381,28 +364,6 @@ func (ars *AutoscalingRunnerSet) VaultProxy() *ProxyConfig {
return nil
}
func (ars *AutoscalingRunnerSet) RunnerSetSpecHash() string {
type runnerSetSpec struct {
GitHubConfigUrl string
GitHubConfigSecret string
RunnerGroup string
RunnerScaleSetName string
Proxy *ProxyConfig
GitHubServerTLS *TLSConfig
Template corev1.PodTemplateSpec
}
spec := &runnerSetSpec{
GitHubConfigUrl: ars.Spec.GitHubConfigUrl,
GitHubConfigSecret: ars.Spec.GitHubConfigSecret,
RunnerGroup: ars.Spec.RunnerGroup,
RunnerScaleSetName: ars.Spec.RunnerScaleSetName,
Proxy: ars.Spec.Proxy,
GitHubServerTLS: ars.Spec.GitHubServerTLS,
Template: ars.Spec.Template,
}
return hash.ComputeTemplateHash(&spec)
}
// +kubebuilder:object:root=true
// AutoscalingRunnerSetList contains a list of AutoscalingRunnerSet
charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml
Generated
+6
View File
@@ -16555,6 +16555,12 @@ spec:
status:
description: AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet
properties:
observedGeneration:
description: |-
ObservedGeneration tracks the metadata.generation of this ARS at observation time,
enabling detection of Pending phase when generation differs. Unset defaults to 0.
format: int64
type: integer
phase:
type: string
type: object
charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml
Generated
+6
View File
@@ -16555,6 +16555,12 @@ spec:
status:
description: AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet
properties:
observedGeneration:
description: |-
ObservedGeneration tracks the metadata.generation of this ARS at observation time,
enabling detection of Pending phase when generation differs. Unset defaults to 0.
format: int64
type: integer
phase:
type: string
type: object
config/crd/bases/actions.github.com_autoscalingrunnersets.yaml
Generated
+6
View File
@@ -16555,6 +16555,12 @@ spec:
status:
description: AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet
properties:
observedGeneration:
description: |-
ObservedGeneration tracks the metadata.generation of this ARS at observation time,
enabling detection of Pending phase when generation differs. Unset defaults to 0.
format: int64
type: integer
phase:
type: string
type: object
controllers/actions.github.com/autoscalingrunnerset_controller.go
+24 -20
View File
@@ -143,27 +143,22 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl
return ctrl.Result{}, nil
}
// Something has changed, we need to re-apply the pending phase and change hash annotation to trigger the update of runner scale set and listener.
if targetHash := autoscalingRunnerSet.Hash(); autoscalingRunnerSet.Annotations[annotationKeyIntegrityHash] != targetHash {
// TODO: apply the version label
original := autoscalingRunnerSet.DeepCopy()
if autoscalingRunnerSet.Annotations == nil {
autoscalingRunnerSet.Annotations = map[string]string{}
}
autoscalingRunnerSet.Annotations[annotationKeyIntegrityHash] = targetHash
if err := r.Patch(ctx, &autoscalingRunnerSet, client.MergeFrom(original)); err != nil {
log.Error(err, "Failed to update autoscaling runner set with new change hash and pending phase")
return ctrl.Result{}, err
}
original = autoscalingRunnerSet.DeepCopy()
autoscalingRunnerSet.Status.Phase = v1alpha1.AutoscalingRunnerSetPhasePending
if err := r.Status().Patch(ctx, &autoscalingRunnerSet, client.MergeFrom(original)); err != nil {
// The spec changed since we last observed it, so move back to the pending
// phase. The observed generation is deliberately left at its old value here:
// it only catches up at the end of a successful reconcile, so a reconcile
// that fails half way through is retried as pending rather than being
// mistaken for settled.
if autoscalingRunnerSet.Generation > autoscalingRunnerSet.Status.ObservedGeneration {
if err := r.updateStatus(
ctx,
&autoscalingRunnerSet,
v1alpha1.AutoscalingRunnerSetPhasePending,
autoscalingRunnerSet.Status.ObservedGeneration,
log,
); err != nil {
log.Error(err, "Failed to update autoscaling runner set status with pending phase")
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}
outdated := autoscalingRunnerSet.Status.Phase == v1alpha1.AutoscalingRunnerSetPhaseOutdated
@@ -393,6 +388,7 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl
ctx,
&autoscalingRunnerSet,
v1alpha1.AutoscalingRunnerSetPhaseRunning,
autoscalingRunnerSet.Generation,
log,
); err != nil {
log.Error(err, "Failed to update autoscaling runner set status to running")
@@ -437,14 +433,22 @@ func (r *AutoscalingRunnerSetReconciler) cleanUpResources(ctx context.Context, a
}
// Update the status of autoscaling runner set if necessary
func (r *AutoscalingRunnerSetReconciler) updateStatus(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, phase v1alpha1.AutoscalingRunnerSetPhase, log logr.Logger) error {
func (r *AutoscalingRunnerSetReconciler) updateStatus(
ctx context.Context,
autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet,
phase v1alpha1.AutoscalingRunnerSetPhase,
observedGeneration int64,
log logr.Logger,
) error {
phaseDiff := phase != autoscalingRunnerSet.Status.Phase
if !phaseDiff {
observedGenerationDiff := observedGeneration != autoscalingRunnerSet.Status.ObservedGeneration
if !phaseDiff && !observedGenerationDiff {
return nil
}
original := autoscalingRunnerSet.DeepCopy()
autoscalingRunnerSet.Status.Phase = phase
autoscalingRunnerSet.Status.ObservedGeneration = observedGeneration
if err := r.Status().Patch(ctx, autoscalingRunnerSet, client.MergeFrom(original)); err != nil {
log.Error(err, "Failed to patch autoscaling runner set status")
controllers/actions.github.com/autoscalingrunnerset_controller_test.go
+79 -5
View File
@@ -492,6 +492,81 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() {
})
Context("When updating a new AutoScalingRunnerSet", func() {
It("advances the observed generation once a spec change has been applied", func() {
var settledGeneration int64
Eventually(
func(g Gomega) {
current := new(v1alpha1.AutoscalingRunnerSet)
g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(autoscalingRunnerSet), current)).To(Succeed())
g.Expect(current.Status.Phase).To(Equal(v1alpha1.AutoscalingRunnerSetPhaseRunning))
g.Expect(current.Status.ObservedGeneration).To(Equal(current.Generation))
settledGeneration = current.Generation
},
autoscalingRunnerSetTestTimeout,
autoscalingRunnerSetTestInterval,
).Should(Succeed(), "AutoscalingRunnerSet should settle with its generation observed")
patched := autoscalingRunnerSet.DeepCopy()
patched.Spec.Template.Spec.Containers[0].Image = "ghcr.io/actions/runner:updated"
Expect(k8sClient.Patch(ctx, patched, client.MergeFrom(autoscalingRunnerSet))).To(Succeed(), "failed to patch AutoScalingRunnerSet")
Eventually(
func(g Gomega) {
current := new(v1alpha1.AutoscalingRunnerSet)
g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(autoscalingRunnerSet), current)).To(Succeed())
g.Expect(current.Generation).To(BeNumerically(">", settledGeneration), "a spec write must bump metadata.generation")
g.Expect(current.Status.Phase).To(Equal(v1alpha1.AutoscalingRunnerSetPhaseRunning))
g.Expect(current.Status.ObservedGeneration).To(Equal(current.Generation), "observed generation must catch up once the change is applied")
},
autoscalingRunnerSetTestTimeout,
autoscalingRunnerSetTestInterval,
).Should(Succeed())
})
// metadata.generation only tracks spec writes, so a label-only edit no
// longer drags the scale set through the Pending phase the way the old
// label-inclusive hash did. Labels still propagate to the
// EphemeralRunnerSet, they just do not count as an update to apply.
It("does not re-observe a generation when only labels change", func() {
var settledGeneration int64
Eventually(
func(g Gomega) {
current := new(v1alpha1.AutoscalingRunnerSet)
g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(autoscalingRunnerSet), current)).To(Succeed())
g.Expect(current.Status.ObservedGeneration).To(Equal(current.Generation))
settledGeneration = current.Generation
},
autoscalingRunnerSetTestTimeout,
autoscalingRunnerSetTestInterval,
).Should(Succeed(), "AutoscalingRunnerSet should settle with its generation observed")
patched := autoscalingRunnerSet.DeepCopy()
patched.Labels["arc.test/label-drift"] = "updated"
Expect(k8sClient.Patch(ctx, patched, client.MergeFrom(autoscalingRunnerSet))).To(Succeed(), "failed to patch AutoScalingRunnerSet labels")
Eventually(
func(g Gomega) {
current := new(v1alpha1.EphemeralRunnerSet)
g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, current)).To(Succeed())
g.Expect(current.Labels).To(HaveKeyWithValue("arc.test/label-drift", "updated"), "labels should still propagate to the EphemeralRunnerSet")
},
autoscalingRunnerSetTestTimeout,
autoscalingRunnerSetTestInterval,
).Should(Succeed())
Consistently(
func(g Gomega) {
current := new(v1alpha1.AutoscalingRunnerSet)
g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(autoscalingRunnerSet), current)).To(Succeed())
g.Expect(current.Generation).To(Equal(settledGeneration), "a label-only edit must not bump metadata.generation")
g.Expect(current.Status.ObservedGeneration).To(Equal(settledGeneration))
g.Expect(current.Status.Phase).To(Equal(v1alpha1.AutoscalingRunnerSetPhaseRunning), "a label-only edit must not push the scale set back to Pending")
},
3*time.Second,
autoscalingRunnerSetTestInterval,
).Should(Succeed())
})
It("updates EphemeralRunnerSet when the runner image changes without touching the Listener", func() {
listener := new(v1alpha1.AutoscalingListener)
Eventually(
@@ -979,10 +1054,6 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() {
statusUpdate := runnerSet.DeepCopy()
statusUpdate.Status.Phase = v1alpha1.EphemeralRunnerSetPhaseRunning
desiredStatus := v1alpha1.AutoscalingRunnerSetStatus{
Phase: v1alpha1.AutoscalingRunnerSetPhaseRunning,
}
err := k8sClient.Status().Patch(ctx, statusUpdate, client.MergeFrom(&runnerSet))
Expect(err).NotTo(HaveOccurred(), "Failed to patch runner set status")
@@ -997,7 +1068,10 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() {
},
autoscalingRunnerSetTestTimeout,
autoscalingRunnerSetTestInterval,
).Should(BeEquivalentTo(desiredStatus), "AutoScalingRunnerSet status should be updated")
).Should(SatisfyAll(
WithTransform(func(s v1alpha1.AutoscalingRunnerSetStatus) v1alpha1.AutoscalingRunnerSetPhase { return s.Phase }, Equal(v1alpha1.AutoscalingRunnerSetPhaseRunning)),
WithTransform(func(s v1alpha1.AutoscalingRunnerSetStatus) int64 { return s.ObservedGeneration }, BeNumerically(">=", ars.Generation)),
), "AutoScalingRunnerSet status should be updated")
})
})