From a8b1b0e61827e0e025838ed4d7e64082d9c30237 Mon Sep 17 00:00:00 2001
From: Asaf Haim <asaf.haim@retrain.ai>
Date: Sat, 8 Jun 2024 23:51:25 +0300
Subject: [PATCH] refactor: combine both status updates under one call

---
 .../v1alpha1/autoscalingrunnerset_types.go    |  2 +-
 .../autoscalingrunnerset_controller.go        | 67 +++++++++----------
 2 files changed, 34 insertions(+), 35 deletions(-)

diff --git a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go
index d50ca6e3..237c0493 100644
--- a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go
+++ b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go
@@ -294,7 +294,7 @@ type AutoscalingRunnerSetStatus struct {
 
 	// +optional
 	// +kubebuilder:validation:Minimum:=0
-	DesiredMinRunners int `json:"desiredMinRunners,omitempty"`
+	DesiredMinRunners int `json:"desiredMinRunners"`
 	// +optional
 	ScheduledOverridesSummary *string `json:"scheduledOverridesSummary,omitempty"`
 }
diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go
index eb0db3d9..2398a454 100644
--- a/controllers/actions.github.com/autoscalingrunnerset_controller.go
+++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go
@@ -318,52 +318,51 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl
 
 	var overridesSummary string
 
-	// Update the status of the desired min runners and scheduled overrides summary
-	if autoscalingRunnerSet.Status.DesiredMinRunners != minRunners {
+	currentReplicasOutOfDate := latestRunnerSet.Status.CurrentReplicas != autoscalingRunnerSet.Status.CurrentRunners
+	minReplicasOutOfDate := autoscalingRunnerSet.Status.DesiredMinRunners != minRunners
+
+	// Update the status of autoscaling runner set.
+	if minReplicasOutOfDate || currentReplicasOutOfDate {
 		if err := patchSubResource(ctx, r.Status(), autoscalingRunnerSet, func(obj *v1alpha1.AutoscalingRunnerSet) {
-			obj.Status.DesiredMinRunners = minRunners
-
-			if (active != nil && upcoming == nil) || (active != nil && upcoming != nil && active.Period.EndTime.Before(upcoming.Period.StartTime)) {
-				var after int
-				if obj.Status.DesiredMinRunners >= 0 {
-					after = obj.Status.DesiredMinRunners
-				}
-
-				overridesSummary = fmt.Sprintf("min=%d status=active endTime=%s", after, active.Period.EndTime)
+			if currentReplicasOutOfDate {
+				obj.Status.CurrentRunners = latestRunnerSet.Status.CurrentReplicas
+				obj.Status.PendingEphemeralRunners = latestRunnerSet.Status.PendingEphemeralRunners
+				obj.Status.RunningEphemeralRunners = latestRunnerSet.Status.RunningEphemeralRunners
+				obj.Status.FailedEphemeralRunners = latestRunnerSet.Status.FailedEphemeralRunners
 			}
 
-			if active == nil && upcoming != nil || (active != nil && upcoming != nil && active.Period.EndTime.After(upcoming.Period.StartTime)) {
-				if upcoming.ScheduledOverride.MinRunners != nil {
-					overridesSummary = fmt.Sprintf("min=%d status=upcoming startTime=%s", *upcoming.ScheduledOverride.MinRunners, upcoming.Period.StartTime)
-				}
-			}
+			if minReplicasOutOfDate {
+				obj.Status.DesiredMinRunners = minRunners
 
-			if overridesSummary != "" {
-				obj.Status.ScheduledOverridesSummary = &overridesSummary
-			} else {
-				obj.Status.ScheduledOverridesSummary = nil
+				if (active != nil && upcoming == nil) || (active != nil && upcoming != nil && active.Period.EndTime.Before(upcoming.Period.StartTime)) {
+					var after int
+					if obj.Status.DesiredMinRunners >= 0 {
+						after = obj.Status.DesiredMinRunners
+					}
+
+					overridesSummary = fmt.Sprintf("min=%d status=active endTime=%s", after, active.Period.EndTime)
+				}
+
+				if active == nil && upcoming != nil || (active != nil && upcoming != nil && active.Period.EndTime.After(upcoming.Period.StartTime)) {
+					if upcoming.ScheduledOverride.MinRunners != nil {
+						overridesSummary = fmt.Sprintf("min=%d status=upcoming startTime=%s", *upcoming.ScheduledOverride.MinRunners, upcoming.Period.StartTime)
+					}
+				}
+
+				if overridesSummary != "" {
+					obj.Status.ScheduledOverridesSummary = &overridesSummary
+				} else {
+					obj.Status.ScheduledOverridesSummary = nil
+				}
 			}
 		}); err != nil {
-			log.Error(err, "Failed to update autoscaling runner set status with desired min runners")
+			log.Error(err, "Failed to update autoscaling runner set status")
 			return ctrl.Result{}, err
 		}
 
 		return ctrl.Result{}, nil
 	}
 
-	// Update the status of autoscaling runner set.
-	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
-		}
-	}
-
 	return ctrl.Result{RequeueAfter: DefaultScaleSetHealthyRequeueAfter}, nil
 }