diff --git a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go index afcb4c93ed..018f71c914 100644 --- a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go @@ -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 diff --git a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml index 8b78ce2430..386284273f 100644 --- a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml @@ -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 diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml index 8b78ce2430..386284273f 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml @@ -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 diff --git a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml index 8b78ce2430..386284273f 100644 --- a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml +++ b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml @@ -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 diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go index ab44125f65..d4404243f5 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -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") diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go index e122cb737e..10cf20eed7 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go @@ -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") +}