Surface EphemeralRunnerSet stats to AutoscalingRunnerSet (#2382)

controllers/actions.github.com/autoscalingrunnerset_controller.go

@@ -238,6 +238,9 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl
 	if latestRunnerSet.Status.CurrentReplicas != autoscalingRunnerSet.Status.CurrentRunners {
 		if err := patchSubResource(ctx, r.Status(), autoscalingRunnerSet, func(obj *v1alpha1.AutoscalingRunnerSet) {
 			obj.Status.CurrentRunners = latestRunnerSet.Status.CurrentReplicas
+			obj.Status.PendingEphemeralRunners = latestRunnerSet.Status.PendingEphemeralRunners
+			obj.Status.RunningEphemeralRunners = latestRunnerSet.Status.RunningEphemeralRunners
+			obj.Status.FailedEphemeralRunners = latestRunnerSet.Status.FailedEphemeralRunners
 		}); err != nil {
 			log.Error(err, "Failed to update autoscaling runner set status with current runner count")
 			return ctrl.Result{}, err

controllers/actions.github.com/autoscalingrunnerset_controller_test.go

@@ -157,23 +157,6 @@ var _ = Describe("Test AutoScalingRunnerSet controller", func() {
 			err := k8sClient.List(ctx, runnerSetList, client.InNamespace(autoscalingRunnerSet.Namespace))
 			Expect(err).NotTo(HaveOccurred(), "failed to list EphemeralRunnerSet")
 			Expect(len(runnerSetList.Items)).To(BeEquivalentTo(1), "Only one EphemeralRunnerSet should be created")
-			runnerSet := runnerSetList.Items[0]
-			statusUpdate := runnerSet.DeepCopy()
-			statusUpdate.Status.CurrentReplicas = 100
-			err = k8sClient.Status().Patch(ctx, statusUpdate, client.MergeFrom(&runnerSet))
-			Expect(err).NotTo(HaveOccurred(), "failed to patch EphemeralRunnerSet status")
-
-			Eventually(
-				func() (int, error) {
-					updated := new(v1alpha1.AutoscalingRunnerSet)
-					err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, updated)
-					if err != nil {
-						return 0, fmt.Errorf("failed to get AutoScalingRunnerSet: %w", err)
-					}
-					return updated.Status.CurrentRunners, nil
-				},
-				autoscalingRunnerSetTestTimeout,
-				autoscalingRunnerSetTestInterval).Should(BeEquivalentTo(100), "AutoScalingRunnerSet status should be updated")
 		})
 	})
 
@@ -398,9 +381,75 @@ var _ = Describe("Test AutoScalingRunnerSet controller", func() {
 					return updated.Annotations[runnerScaleSetRunnerGroupNameKey], nil
 				},
 				autoscalingRunnerSetTestTimeout,
-				autoscalingRunnerSetTestInterval).Should(BeEquivalentTo("testgroup2"), "AutoScalingRunnerSet should have the runner group in its annotation")
+				autoscalingRunnerSetTestInterval,
+			).Should(BeEquivalentTo("testgroup2"), "AutoScalingRunnerSet should have the runner group in its annotation")
 		})
 	})
+
+	It("Should update Status on EphemeralRunnerSet status Update", func() {
+		ars := new(v1alpha1.AutoscalingRunnerSet)
+		Eventually(
+			func() (bool, error) {
+				err := k8sClient.Get(
+					ctx,
+					client.ObjectKey{
+						Name:      autoscalingRunnerSet.Name,
+						Namespace: autoscalingRunnerSet.Namespace,
+					},
+					ars,
+				)
+				if err != nil {
+					return false, err
+				}
+				return true, nil
+			},
+			autoscalingRunnerSetTestTimeout,
+			autoscalingRunnerSetTestInterval,
+		).Should(BeTrue(), "AutoscalingRunnerSet should be created")
+
+		runnerSetList := new(v1alpha1.EphemeralRunnerSetList)
+		Eventually(func() (int, error) {
+			err := k8sClient.List(ctx, runnerSetList, client.InNamespace(ars.Namespace))
+			if err != nil {
+				return 0, err
+			}
+			return len(runnerSetList.Items), nil
+		},
+			autoscalingRunnerSetTestTimeout,
+			autoscalingRunnerSetTestInterval,
+		).Should(BeEquivalentTo(1), "Failed to fetch runner set list")
+
+		runnerSet := runnerSetList.Items[0]
+		statusUpdate := runnerSet.DeepCopy()
+		statusUpdate.Status.CurrentReplicas = 6
+		statusUpdate.Status.FailedEphemeralRunners = 1
+		statusUpdate.Status.RunningEphemeralRunners = 2
+		statusUpdate.Status.PendingEphemeralRunners = 3
+
+		desiredStatus := v1alpha1.AutoscalingRunnerSetStatus{
+			CurrentRunners:          statusUpdate.Status.CurrentReplicas,
+			State:                   "",
+			PendingEphemeralRunners: statusUpdate.Status.PendingEphemeralRunners,
+			RunningEphemeralRunners: statusUpdate.Status.RunningEphemeralRunners,
+			FailedEphemeralRunners:  statusUpdate.Status.FailedEphemeralRunners,
+		}
+
+		err := k8sClient.Status().Patch(ctx, statusUpdate, client.MergeFrom(&runnerSet))
+		Expect(err).NotTo(HaveOccurred(), "Failed to patch runner set status")
+
+		Eventually(
+			func() (v1alpha1.AutoscalingRunnerSetStatus, error) {
+				updated := new(v1alpha1.AutoscalingRunnerSet)
+				err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, updated)
+				if err != nil {
+					return v1alpha1.AutoscalingRunnerSetStatus{}, fmt.Errorf("failed to get AutoScalingRunnerSet: %w", err)
+				}
+				return updated.Status, nil
+			},
+			autoscalingRunnerSetTestTimeout,
+			autoscalingRunnerSetTestInterval,
+		).Should(BeEquivalentTo(desiredStatus), "AutoScalingRunnerSet status should be updated")
+	})
 })
 
 var _ = Describe("Test AutoScalingController updates", func() {

controllers/actions.github.com/ephemeralrunnerset_controller.go

@@ -200,11 +200,18 @@ func (r *EphemeralRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl.R
 		}
 	}
 
+	desiredStatus := v1alpha1.EphemeralRunnerSetStatus{
+		CurrentReplicas:         total,
+		PendingEphemeralRunners: len(pendingEphemeralRunners),
+		RunningEphemeralRunners: len(runningEphemeralRunners),
+		FailedEphemeralRunners:  len(failedEphemeralRunners),
+	}
+
 	// Update the status if needed.
-	if ephemeralRunnerSet.Status.CurrentReplicas != total {
+	if ephemeralRunnerSet.Status != desiredStatus {
 		log.Info("Updating status with current runners count", "count", total)
 		if err := patchSubResource(ctx, r.Status(), ephemeralRunnerSet, func(obj *v1alpha1.EphemeralRunnerSet) {
-			obj.Status.CurrentReplicas = total
+			obj.Status = desiredStatus
 		}); err != nil {
 			log.Error(err, "Failed to update status with current runners count")
 			return ctrl.Result{}, err

controllers/actions.github.com/ephemeralrunnerset_controller_test.go

@@ -559,6 +559,181 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
 				ephemeralRunnerSetTestTimeout,
 				ephemeralRunnerSetTestInterval).Should(BeEquivalentTo(0), "0 EphemeralRunner should be created")
 		})
+
+		It("Should update status on Ephemeral Runner state changes", func() {
+			created := new(actionsv1alpha1.EphemeralRunnerSet)
+			Eventually(
+				func() error {
+					return k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunnerSet.Name, Namespace: ephemeralRunnerSet.Namespace}, created)
+				},
+				ephemeralRunnerSetTestTimeout,
+				ephemeralRunnerSetTestInterval,
+			).Should(Succeed(), "EphemeralRunnerSet should be created")
+
+			// Scale up the EphemeralRunnerSet
+			updated := created.DeepCopy()
+			updated.Spec.Replicas = 3
+			err := k8sClient.Update(ctx, updated)
+			Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunnerSet replica count")
+
+			runnerList := new(actionsv1alpha1.EphemeralRunnerList)
+			Eventually(
+				func() (bool, error) {
+					err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
+					if err != nil {
+						return false, err
+					}
+
+					if len(runnerList.Items) != 3 {
+						return false, err
+					}
+
+					var pendingOriginal *v1alpha1.EphemeralRunner
+					var runningOriginal *v1alpha1.EphemeralRunner
+					var failedOriginal *v1alpha1.EphemeralRunner
+					var empty []*v1alpha1.EphemeralRunner
+					for _, runner := range runnerList.Items {
+						switch runner.Status.RunnerId {
+						case 101:
+							pendingOriginal = runner.DeepCopy()
+						case 102:
+							runningOriginal = runner.DeepCopy()
+						case 103:
+							failedOriginal = runner.DeepCopy()
+						default:
+							empty = append(empty, runner.DeepCopy())
+						}
+					}
+
+					refetch := false
+					if pendingOriginal == nil { // if NO pending
+						refetch = true
+						pendingOriginal = empty[0]
+						empty = empty[1:]
+
+						pending := pendingOriginal.DeepCopy()
+						pending.Status.RunnerId = 101
+						pending.Status.Phase = corev1.PodPending
+
+						err = k8sClient.Status().Patch(ctx, pending, client.MergeFrom(pendingOriginal))
+						if err != nil {
+							return false, err
+						}
+					}
+
+					if runningOriginal == nil { // if NO running
+						refetch = true
+						runningOriginal = empty[0]
+						empty = empty[1:]
+						running := runningOriginal.DeepCopy()
+						running.Status.RunnerId = 102
+						running.Status.Phase = corev1.PodRunning
+
+						err = k8sClient.Status().Patch(ctx, running, client.MergeFrom(runningOriginal))
+						if err != nil {
+							return false, err
+						}
+					}
+
+					if failedOriginal == nil { // if NO failed
+						refetch = true
+						failedOriginal = empty[0]
+
+						failed := pendingOriginal.DeepCopy()
+						failed.Status.RunnerId = 103
+						failed.Status.Phase = corev1.PodFailed
+
+						err = k8sClient.Status().Patch(ctx, failed, client.MergeFrom(failedOriginal))
+						if err != nil {
+							return false, err
+						}
+					}
+
+					return !refetch, nil
+				},
+				ephemeralRunnerSetTestTimeout,
+				ephemeralRunnerSetTestInterval,
+			).Should(BeTrue(), "Failed to eventually update to one pending, one running and one failed")
+
+			desiredStatus := v1alpha1.EphemeralRunnerSetStatus{
+				CurrentReplicas:         3,
+				PendingEphemeralRunners: 1,
+				RunningEphemeralRunners: 1,
+				FailedEphemeralRunners:  1,
+			}
+			Eventually(
+				func() (v1alpha1.EphemeralRunnerSetStatus, error) {
+					updated := new(v1alpha1.EphemeralRunnerSet)
+					err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunnerSet.Name, Namespace: ephemeralRunnerSet.Namespace}, updated)
+					if err != nil {
+						return v1alpha1.EphemeralRunnerSetStatus{}, err
+					}
+					return updated.Status, nil
+				},
+				ephemeralRunnerSetTestTimeout,
+				ephemeralRunnerSetTestInterval,
+			).Should(BeEquivalentTo(desiredStatus), "Status is not eventually updated to the desired one")
+
+			updated = new(v1alpha1.EphemeralRunnerSet)
+			err = k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunnerSet.Name, Namespace: ephemeralRunnerSet.Namespace}, updated)
+			Expect(err).NotTo(HaveOccurred(), "Failed to fetch ephemeral runner set")
+
+			updatedOriginal := updated.DeepCopy()
+			updated.Spec.Replicas = 0
+
+			err = k8sClient.Patch(ctx, updated, client.MergeFrom(updatedOriginal))
+			Expect(err).NotTo(HaveOccurred(), "Failed to patch ephemeral runner set with 0 replicas")
+
+			Eventually(
+				func() (int, error) {
+					runnerList = new(actionsv1alpha1.EphemeralRunnerList)
+					err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace))
+					if err != nil {
+						return -1, err
+					}
+					return len(runnerList.Items), nil
+				},
+				ephemeralRunnerSetTestTimeout,
+				ephemeralRunnerSetTestInterval,
+			).Should(BeEquivalentTo(1), "Failed to eventually scale down")
+
+			desiredStatus = v1alpha1.EphemeralRunnerSetStatus{
+				CurrentReplicas:         1,
+				PendingEphemeralRunners: 0,
+				RunningEphemeralRunners: 0,
+				FailedEphemeralRunners:  1,
+			}
+
+			Eventually(
+				func() (v1alpha1.EphemeralRunnerSetStatus, error) {
+					updated := new(v1alpha1.EphemeralRunnerSet)
+					err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunnerSet.Name, Namespace: ephemeralRunnerSet.Namespace}, updated)
+					if err != nil {
+						return v1alpha1.EphemeralRunnerSetStatus{}, err
+					}
+					return updated.Status, nil
+				},
+				ephemeralRunnerSetTestTimeout,
+				ephemeralRunnerSetTestInterval,
+			).Should(BeEquivalentTo(desiredStatus), "Status is not eventually updated to the desired one")
+
+			err = k8sClient.Delete(ctx, &runnerList.Items[0])
+			Expect(err).To(BeNil(), "Failed to delete failed ephemeral runner")
+
+			desiredStatus = v1alpha1.EphemeralRunnerSetStatus{} // empty
+			Eventually(
+				func() (v1alpha1.EphemeralRunnerSetStatus, error) {
+					updated := new(v1alpha1.EphemeralRunnerSet)
+					err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunnerSet.Name, Namespace: ephemeralRunnerSet.Namespace}, updated)
+					if err != nil {
+						return v1alpha1.EphemeralRunnerSetStatus{}, err
+					}
+					return updated.Status, nil
+				},
+				ephemeralRunnerSetTestTimeout,
+				ephemeralRunnerSetTestInterval,
+			).Should(BeEquivalentTo(desiredStatus), "Status is not eventually updated to the desired one")
+		})
 	})
 })
 
@@ -821,12 +996,13 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func(
 		err = k8sClient.Status().Patch(ctx, runner, client.MergeFrom(&runnerList.Items[0]))
 		Expect(err).NotTo(HaveOccurred(), "failed to update ephemeral runner status")
 
-		updatedRunnerSet := new(actionsv1alpha1.EphemeralRunnerSet)
-		err = k8sClient.Get(ctx, client.ObjectKey{Namespace: ephemeralRunnerSet.Namespace, Name: ephemeralRunnerSet.Name}, updatedRunnerSet)
+		runnerSet := new(actionsv1alpha1.EphemeralRunnerSet)
+		err = k8sClient.Get(ctx, client.ObjectKey{Namespace: ephemeralRunnerSet.Namespace, Name: ephemeralRunnerSet.Name}, runnerSet)
 		Expect(err).NotTo(HaveOccurred(), "failed to get EphemeralRunnerSet")
 
+		updatedRunnerSet := runnerSet.DeepCopy()
 		updatedRunnerSet.Spec.Replicas = 0
-		err = k8sClient.Update(ctx, updatedRunnerSet)
+		err = k8sClient.Patch(ctx, updatedRunnerSet, client.MergeFrom(runnerSet))
 		Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunnerSet")
 
 		Eventually(