Report the pending phase when the listener is rebuilt

Moving update detection to metadata.generation lost a signal. The desired
listener is built from the AutoscalingRunnerSet's labels and annotations as
well as its spec, and any difference there deletes the listener so it can
be re-created. metadata.generation only moves on spec writes, so a
label-only edit still tore the listener down while the scale set kept
reporting Running. If the rebuild then failed, it reported Running with no
listener indefinitely. Under the old label-inclusive hash the phase did
move, so this was a regression.

Mark the resource pending at the point the listener is deleted rather than
trying to infer it from the generation. That is what the phase already
means: its own doc comment says pending is when the listener is not yet
started. It also covers the case properly, because it keys off the actual
decision to rebuild instead of guessing from the trigger.

This does not change when the listener is rebuilt. A label-only edit
recreates it today and still does; only the reported phase changes. The
earlier claim that labels no longer restart anything was wrong: labels are
propagated to the listener, so editing one is a restart. What
metadata.generation changes is narrower than that, and the test now covers
the listener's identity across a label edit rather than implying it is
untouched.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

controllers/actions.github.com/autoscalingrunnerset_controller.go

@@ -373,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")

controllers/actions.github.com/autoscalingrunnerset_controller_test.go

@@ -527,12 +527,27 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() {
 		// 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() {
+		// A label-only edit does not bump metadata.generation, so it is not an
+		// update to apply. It does still rebuild the listener, because the
+		// desired listener labels are derived from the AutoscalingRunnerSet's,
+		// and the phase has to report that.
+		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
 				},
@@ -554,13 +569,38 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() {
 				autoscalingRunnerSetTestInterval,
 			).Should(Succeed())
 
+			// The listener is rebuilt to pick the new label up, so the scale set
+			// must not keep claiming to be running while that happens.
+			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))
-					g.Expect(current.Status.Phase).To(Equal(v1alpha1.AutoscalingRunnerSetPhaseRunning), "a label-only edit must not push the scale set back to Pending")
+					g.Expect(current.Status.ObservedGeneration).To(Equal(settledGeneration), "a label-only edit must not move the observed generation")
 				},
 				3*time.Second,
 				autoscalingRunnerSetTestInterval,