Track AutoscalingRunnerSet updates with metadata.generation (#4636)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Nikola Jokic
2026-09-11 14:38:30 +02:00
committed by GitHub
co-authored by Copilot App
parent db5702bb71
commit 5581cd0c54
6 changed files with 335 additions and 68 deletions
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -378,6 +373,25 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl
if !cmp.Equal(listener.Spec, desired.Spec) ||
!cmp.Equal(listener.Labels, desired.Labels) ||
!cmp.Equal(listener.Annotations, desired.Annotations) {
// The listener is about to be torn down and rebuilt, which is what
// the pending phase means. Report it here rather than relying on the
// generation check above: the desired listener is derived from the
// AutoscalingRunnerSet's labels and annotations as well as its spec,
// and metadata writes do not bump metadata.generation. Without this,
// a label-only edit would leave the scale set claiming to be running
// while it has no listener at all, and it would keep claiming that
// if the rebuild never succeeded.
if err := r.updateStatus(
ctx,
&autoscalingRunnerSet,
v1alpha1.AutoscalingRunnerSetPhasePending,
autoscalingRunnerSet.Status.ObservedGeneration,
log,
); err != nil {
log.Error(err, "Failed to update autoscaling runner set status before re-creating the listener")
return ctrl.Result{}, err
}
log.Info("Deleting AutoscalingListener to re-create with updated spec")
if err := r.Delete(ctx, &listener); err != nil {
log.Error(err, "Failed to delete AutoscalingListener for re-creation")
@@ -393,6 +407,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 +452,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")
@@ -492,6 +492,236 @@ 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())
})
// A label-only edit does not bump metadata.generation, so it never
// advances the observed generation. It does still rebuild the listener,
// because the desired listener labels are derived from the
// AutoscalingRunnerSet's, and the phase has to report that rebuild. So
// the scale set passes through Pending transiently and comes back to
// Running, while the observed generation stays exactly where it was.
It("does not re-observe a generation when only labels change, but still reports the listener rebuild", func() {
listener := new(v1alpha1.AutoscalingListener)
Eventually(
func() error {
return k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, listener)
},
autoscalingRunnerSetTestTimeout,
autoscalingRunnerSetTestInterval,
).Should(Succeed(), "Listener should be created")
originalListenerUID := listener.UID
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")
// Hold the deletion window open. Without this the listener is deleted
// and re-created between two polls, so an implementation that never
// reported Pending would look identical to one that did. The
// AutoscalingListener controller does not run in this suite, so
// nothing else is going to clear this finalizer.
blockDeletion(listener)
defer unblockDeletion(listener)
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())
// The listener is on its way out, so the scale set must not claim to
// be running. This is the assertion that fails if the Pending update
// before the listener delete is removed.
Eventually(
func(g Gomega) {
current := new(v1alpha1.AutoscalingListener)
g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, current)).To(Succeed())
g.Expect(current.DeletionTimestamp).NotTo(BeNil(), "listener should be marked for deletion to pick up the new label")
ars := new(v1alpha1.AutoscalingRunnerSet)
g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(autoscalingRunnerSet), ars)).To(Succeed())
g.Expect(ars.Status.Phase).To(Equal(v1alpha1.AutoscalingRunnerSetPhasePending), "scale set must report Pending while its listener is being rebuilt")
},
autoscalingRunnerSetTestTimeout,
autoscalingRunnerSetTestInterval,
).Should(Succeed())
// It stays Pending for as long as the rebuild has not happened, and
// the observed generation does not drift in the meantime.
Consistently(
func(g Gomega) {
ars := new(v1alpha1.AutoscalingRunnerSet)
g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(autoscalingRunnerSet), ars)).To(Succeed())
g.Expect(ars.Status.Phase).To(Equal(v1alpha1.AutoscalingRunnerSetPhasePending), "scale set must stay Pending until the listener is back")
g.Expect(ars.Status.ObservedGeneration).To(Equal(settledGeneration))
},
3*time.Second,
autoscalingRunnerSetTestInterval,
).Should(Succeed())
// Let the rebuild finish.
unblockDeletion(listener)
Eventually(
func(g Gomega) {
current := new(v1alpha1.AutoscalingListener)
g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, current)).To(Succeed())
g.Expect(current.UID).NotTo(Equal(originalListenerUID), "listener should be re-created to pick up the new label")
g.Expect(current.Labels).To(HaveKeyWithValue("arc.test/label-drift", "updated"), "listener should carry the new label")
},
autoscalingRunnerSetTestTimeout,
autoscalingRunnerSetTestInterval,
).Should(Succeed())
// Once the listener is back, the scale set settles on Running again
// without the observed generation ever moving, because no spec write
// happened.
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))
},
autoscalingRunnerSetTestTimeout,
autoscalingRunnerSetTestInterval,
).Should(Succeed(), "AutoscalingRunnerSet should return to Running once the listener is rebuilt")
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), "a label-only edit must not move the observed generation")
},
3*time.Second,
autoscalingRunnerSetTestInterval,
).Should(Succeed())
})
// The observed generation is only meant to advance once a change has
// actually been applied. A reconcile that cannot finish must leave it
// behind the live generation, so the work is retried rather than being
// mistaken for settled.
It("leaves the observed generation behind until the reconcile completes", func() {
listener := new(v1alpha1.AutoscalingListener)
Eventually(
func() error {
return k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, listener)
},
autoscalingRunnerSetTestTimeout,
autoscalingRunnerSetTestInterval,
).Should(Succeed(), "Listener should be created")
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")
// Stall the reconcile part way through: the max runner change below
// forces the listener to be rebuilt, and the rebuild cannot complete
// while the listener is held in deletion.
blockDeletion(listener)
defer unblockDeletion(listener)
patched := autoscalingRunnerSet.DeepCopy()
updatedMax := 20
patched.Spec.MaxRunners = &updatedMax
Expect(k8sClient.Patch(ctx, patched, client.MergeFrom(autoscalingRunnerSet))).To(Succeed(), "failed to patch AutoScalingRunnerSet max runners")
var liveGeneration int64
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.AutoscalingRunnerSetPhasePending), "the scale set should go Pending while the change is being applied")
liveGeneration = current.Generation
},
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.Status.ObservedGeneration).To(Equal(settledGeneration), "observed generation must not advance while the change is still being applied")
g.Expect(current.Status.ObservedGeneration).To(BeNumerically("<", liveGeneration))
g.Expect(current.Status.Phase).To(Equal(v1alpha1.AutoscalingRunnerSetPhasePending), "the scale set must stay Pending until the change is applied")
},
3*time.Second,
autoscalingRunnerSetTestInterval,
).Should(Succeed())
// Let the reconcile complete, and only now should the marker catch up.
unblockDeletion(listener)
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(liveGeneration), "observed generation must catch up once the change has been applied")
},
autoscalingRunnerSetTestTimeout,
autoscalingRunnerSetTestInterval,
).Should(Succeed())
})
It("updates EphemeralRunnerSet when the runner image changes without touching the Listener", func() {
listener := new(v1alpha1.AutoscalingListener)
Eventually(
@@ -979,10 +1209,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 +1223,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")
})
})
@@ -2447,3 +2676,39 @@ var _ = Describe("Test AutoscalingRunnerSet with a stale runner scale set", Orde
})
})
})
// testHoldFinalizer keeps an AutoscalingListener around after it has been
// deleted, so a test can observe the window during which the scale set has no
// usable listener. The AutoscalingListener controller is not running in this
// suite, so nothing else adds or removes finalizers on these objects.
const testHoldFinalizer = "arc.test/hold-deletion"
func blockDeletion(listener *v1alpha1.AutoscalingListener) {
GinkgoHelper()
current := new(v1alpha1.AutoscalingListener)
Expect(k8sClient.Get(context.Background(), client.ObjectKeyFromObject(listener), current)).To(Succeed(), "failed to get listener to block its deletion")
original := current.DeepCopy()
Expect(controllerutil.AddFinalizer(current, testHoldFinalizer)).To(BeTrue(), "listener should not already hold the test finalizer")
Expect(k8sClient.Patch(context.Background(), current, client.MergeFrom(original))).To(Succeed(), "failed to add the test finalizer to the listener")
}
// unblockDeletion is idempotent so it can be deferred as a safety net and still
// be called explicitly at the point a test wants the rebuild to proceed.
func unblockDeletion(listener *v1alpha1.AutoscalingListener) {
GinkgoHelper()
current := new(v1alpha1.AutoscalingListener)
err := k8sClient.Get(context.Background(), client.ObjectKeyFromObject(listener), current)
if errors.IsNotFound(err) {
return
}
Expect(err).NotTo(HaveOccurred(), "failed to get listener to unblock its deletion")
original := current.DeepCopy()
if !controllerutil.RemoveFinalizer(current, testHoldFinalizer) {
return
}
Expect(k8sClient.Patch(context.Background(), current, client.MergeFrom(original))).To(Succeed(), "failed to remove the test finalizer from the listener")
}