Add HRA printer column "SCHEDULE" (#561)

Adds a column to help the operator see if they configured HRA.Spec.ScheduledOverrides correctly, in a form of "next override schedule recognized by the controller":

```
$ k get horizontalrunnerautoscaler
NAME                            MIN   MAX   DESIRED   SCHEDULE
actions-runner-aos-autoscaler   0     5     0
org                             0     5     0         min=0 time=2021-05-21 15:00:00 +0000 UTC
```

Ref https://github.com/actions-runner-controller/actions-runner-controller/issues/484

api/v1alpha1/horizontalrunnerautoscaler_types.go

@@ -201,6 +201,11 @@ type HorizontalRunnerAutoscalerStatus struct {
 
 	// +optional
 	CacheEntries []CacheEntry `json:"cacheEntries,omitempty"`
+
+	// ScheduledOverridesSummary is the summary of active and upcoming scheduled overrides to be shown in e.g. a column of a `kubectl get hra` output
+	// for observability.
+	// +optional
+	ScheduledOverridesSummary *string `json:"scheduledOverridesSummary,omitempty"`
 }
 
 const CacheEntryKeyDesiredReplicas = "desiredReplicas"
@@ -216,6 +221,7 @@ type CacheEntry struct {
 // +kubebuilder:printcolumn:JSONPath=".spec.minReplicas",name=Min,type=number
 // +kubebuilder:printcolumn:JSONPath=".spec.maxReplicas",name=Max,type=number
 // +kubebuilder:printcolumn:JSONPath=".status.desiredReplicas",name=Desired,type=number
+// +kubebuilder:printcolumn:JSONPath=".status.scheduledOverridesSummary",name=Schedule,type=string
 
 // HorizontalRunnerAutoscaler is the Schema for the horizontalrunnerautoscaler API
 type HorizontalRunnerAutoscaler struct {

api/v1alpha1/zz_generated.deepcopy.go

@@ -250,6 +250,11 @@ func (in *HorizontalRunnerAutoscalerStatus) DeepCopyInto(out *HorizontalRunnerAu
 			(*in)[i].DeepCopyInto(&(*out)[i])
 		}
 	}
+	if in.ScheduledOverridesSummary != nil {
+		in, out := &in.ScheduledOverridesSummary, &out.ScheduledOverridesSummary
+		*out = new(string)
+		**out = **in
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HorizontalRunnerAutoscalerStatus.

charts/actions-runner-controller/crds/actions.summerwind.dev_horizontalrunnerautoscalers.yaml

@@ -18,6 +18,9 @@ spec:
   - JSONPath: .status.desiredReplicas
     name: Desired
     type: number
+  - JSONPath: .status.scheduledOverridesSummary
+    name: Schedule
+    type: string
   group: actions.summerwind.dev
   names:
     kind: HorizontalRunnerAutoscaler
@@ -265,6 +268,11 @@ spec:
                 which is updated on mutation by the API Server.
               format: int64
               type: integer
+            scheduledOverridesSummary:
+              description: ScheduledOverridesSummary is the summary of active and
+                upcoming scheduled overrides to be shown in e.g. a column of a `kubectl
+                get hra` output for observability.
+              type: string
           type: object
       type: object
   version: v1alpha1

config/crd/bases/actions.summerwind.dev_horizontalrunnerautoscalers.yaml

@@ -18,6 +18,9 @@ spec:
   - JSONPath: .status.desiredReplicas
     name: Desired
     type: number
+  - JSONPath: .status.scheduledOverridesSummary
+    name: Schedule
+    type: string
   group: actions.summerwind.dev
   names:
     kind: HorizontalRunnerAutoscaler
@@ -265,6 +268,11 @@ spec:
                 which is updated on mutation by the API Server.
               format: int64
               type: integer
+            scheduledOverridesSummary:
+              description: ScheduledOverridesSummary is the summary of active and
+                upcoming scheduled overrides to be shown in e.g. a column of a `kubectl
+                get hra` output for observability.
+              type: string
           type: object
       type: object
   version: v1alpha1

controllers/autoscaling_test.go

@@ -209,7 +209,7 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
 					Replicas: tc.fixed,
 				},
 				Status: v1alpha1.RunnerDeploymentStatus{
-					Replicas: tc.sReplicas,
+					DesiredReplicas: tc.sReplicas,
 				},
 			}
 
@@ -224,7 +224,12 @@ func TestDetermineDesiredReplicas_RepositoryRunner(t *testing.T) {
 				},
 			}
 
-			got, _, _, err := h.computeReplicasWithCache(log, metav1Now.Time, rd, hra)
+			minReplicas, _, _, err := h.getMinReplicas(log, metav1Now.Time, hra)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			got, _, _, err := h.computeReplicasWithCache(log, metav1Now.Time, rd, hra, minReplicas)
 			if err != nil {
 				if tc.err == "" {
 					t.Fatalf("unexpected error: expected none, got %v", err)
@@ -459,7 +464,7 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
 					Replicas: tc.fixed,
 				},
 				Status: v1alpha1.RunnerDeploymentStatus{
-					Replicas: tc.sReplicas,
+					DesiredReplicas: tc.sReplicas,
 				},
 			}
 
@@ -483,7 +488,12 @@ func TestDetermineDesiredReplicas_OrganizationalRunner(t *testing.T) {
 				},
 			}
 
-			got, _, _, err := h.computeReplicasWithCache(log, metav1Now.Time, rd, hra)
+			minReplicas, _, _, err := h.getMinReplicas(log, metav1Now.Time, hra)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			got, _, _, err := h.computeReplicasWithCache(log, metav1Now.Time, rd, hra, minReplicas)
 			if err != nil {
 				if tc.err == "" {
 					t.Fatalf("unexpected error: expected none, got %v", err)

controllers/horizontalrunnerautoscaler_controller.go

@@ -19,6 +19,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"reflect"
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
@@ -91,7 +92,14 @@ func (r *HorizontalRunnerAutoscalerReconciler) Reconcile(req ctrl.Request) (ctrl
 
 	now := time.Now()
 
-	newDesiredReplicas, computedReplicas, computedReplicasFromCache, err := r.computeReplicasWithCache(log, now, rd, hra)
+	minReplicas, active, upcoming, err := r.getMinReplicas(log, now, hra)
+	if err != nil {
+		log.Error(err, "Could not compute min replicas")
+
+		return ctrl.Result{}, err
+	}
+
+	newDesiredReplicas, computedReplicas, computedReplicasFromCache, err := r.computeReplicasWithCache(log, now, rd, hra, minReplicas)
 	if err != nil {
 		r.Recorder.Event(&hra, corev1.EventTypeNormal, "RunnerAutoscalingFailure", err.Error())
 
@@ -112,11 +120,9 @@ func (r *HorizontalRunnerAutoscalerReconciler) Reconcile(req ctrl.Request) (ctrl
 		}
 	}
 
-	var updated *v1alpha1.HorizontalRunnerAutoscaler
+	updated := hra.DeepCopy()
 
 	if hra.Status.DesiredReplicas == nil || *hra.Status.DesiredReplicas != newDesiredReplicas {
-		updated = hra.DeepCopy()
-
 		if (hra.Status.DesiredReplicas == nil && newDesiredReplicas > 1) ||
 			(hra.Status.DesiredReplicas != nil && newDesiredReplicas > *hra.Status.DesiredReplicas) {
 
@@ -127,10 +133,6 @@ func (r *HorizontalRunnerAutoscalerReconciler) Reconcile(req ctrl.Request) (ctrl
 	}
 
 	if computedReplicasFromCache == nil {
-		if updated == nil {
-			updated = hra.DeepCopy()
-		}
-
 		cacheEntries := getValidCacheEntries(updated, now)
 
 		var cacheDuration time.Duration
@@ -148,11 +150,34 @@ func (r *HorizontalRunnerAutoscalerReconciler) Reconcile(req ctrl.Request) (ctrl
 		})
 	}
 
-	if updated != nil {
+	var overridesSummary string
+
+	if (active != nil && upcoming == nil) || (active != nil && upcoming != nil && active.Period.EndTime.Before(upcoming.Period.StartTime)) {
+		after := defaultReplicas
+		if hra.Spec.MinReplicas != nil && *hra.Spec.MinReplicas >= 0 {
+			after = *hra.Spec.MinReplicas
+		}
+
+		overridesSummary = fmt.Sprintf("min=%d time=%s", after, active.Period.EndTime)
+	}
+
+	if active == nil && upcoming != nil || (active != nil && upcoming != nil && active.Period.EndTime.After(upcoming.Period.StartTime)) {
+		if upcoming.ScheduledOverride.MinReplicas != nil {
+			overridesSummary = fmt.Sprintf("min=%d time=%s", *upcoming.ScheduledOverride.MinReplicas, upcoming.Period.StartTime)
+		}
+	}
+
+	if overridesSummary != "" {
+		updated.Status.ScheduledOverridesSummary = &overridesSummary
+	} else {
+		updated.Status.ScheduledOverridesSummary = nil
+	}
+
+	if !reflect.DeepEqual(hra.Status, updated.Status) {
 		metrics.SetHorizontalRunnerAutoscalerStatus(updated.ObjectMeta, updated.Status)
 
 		if err := r.Status().Patch(ctx, updated, client.MergeFrom(&hra)); err != nil {
-			return ctrl.Result{}, fmt.Errorf("patching horizontalrunnerautoscaler status to add cache entry: %w", err)
+			return ctrl.Result{}, fmt.Errorf("patching horizontalrunnerautoscaler status: %w", err)
 		}
 	}
 
@@ -185,9 +210,14 @@ func (r *HorizontalRunnerAutoscalerReconciler) SetupWithManager(mgr ctrl.Manager
 		Complete(r)
 }
 
-func (r *HorizontalRunnerAutoscalerReconciler) matchScheduledOverrides(log logr.Logger, now time.Time, hra v1alpha1.HorizontalRunnerAutoscaler) (*int, *Period, *Period, error) {
+type Override struct {
+	ScheduledOverride v1alpha1.ScheduledOverride
+	Period            Period
+}
+
+func (r *HorizontalRunnerAutoscalerReconciler) matchScheduledOverrides(log logr.Logger, now time.Time, hra v1alpha1.HorizontalRunnerAutoscaler) (*int, *Override, *Override, error) {
 	var minReplicas *int
-	var active, upcoming *Period
+	var active, upcoming *Override
 
 	for _, o := range hra.Spec.ScheduledOverrides {
 		log.V(1).Info(
@@ -213,7 +243,7 @@ func (r *HorizontalRunnerAutoscalerReconciler) matchScheduledOverrides(log logr.
 		// Use the first when there are two or more active scheduled overrides,
 		// as the spec defines that the earlier scheduled override is prioritized higher than later ones.
 		if a != nil && active == nil {
-			active = a
+			active = &Override{Period: *a, ScheduledOverride: o}
 
 			if o.MinReplicas != nil {
 				minReplicas = o.MinReplicas
@@ -227,8 +257,8 @@ func (r *HorizontalRunnerAutoscalerReconciler) matchScheduledOverrides(log logr.
 			}
 		}
 
-		if u != nil && (upcoming == nil || u.StartTime.Before(upcoming.StartTime)) {
+		if u != nil && (upcoming == nil || u.StartTime.Before(upcoming.Period.StartTime)) {
-			upcoming = u
+			upcoming = &Override{Period: *u, ScheduledOverride: o}
 
 			log.V(1).Info(
 				"Found upcoming scheduled override",
@@ -242,18 +272,23 @@ func (r *HorizontalRunnerAutoscalerReconciler) matchScheduledOverrides(log logr.
 	return minReplicas, active, upcoming, nil
 }
 
-func (r *HorizontalRunnerAutoscalerReconciler) computeReplicasWithCache(log logr.Logger, now time.Time, rd v1alpha1.RunnerDeployment, hra v1alpha1.HorizontalRunnerAutoscaler) (int, int, *int, error) {
+func (r *HorizontalRunnerAutoscalerReconciler) getMinReplicas(log logr.Logger, now time.Time, hra v1alpha1.HorizontalRunnerAutoscaler) (int, *Override, *Override, error) {
 	minReplicas := defaultReplicas
 	if hra.Spec.MinReplicas != nil && *hra.Spec.MinReplicas >= 0 {
 		minReplicas = *hra.Spec.MinReplicas
 	}
 
-	if m, _, _, err := r.matchScheduledOverrides(log, now, hra); err != nil {
+	m, active, upcoming, err := r.matchScheduledOverrides(log, now, hra)
-		return 0, 0, nil, err
+	if err != nil {
+		return 0, nil, nil, err
 	} else if m != nil {
 		minReplicas = *m
 	}
 
+	return minReplicas, active, upcoming, nil
+}
+
+func (r *HorizontalRunnerAutoscalerReconciler) computeReplicasWithCache(log logr.Logger, now time.Time, rd v1alpha1.RunnerDeployment, hra v1alpha1.HorizontalRunnerAutoscaler, minReplicas int) (int, int, *int, error) {
 	var suggestedReplicas int
 
 	suggestedReplicasFromCache := r.fetchSuggestedReplicasFromCache(hra)