From 5223a4434dc657b394220e28bd2679698693fc04 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Mon, 6 Jul 2026 14:37:20 +0200 Subject: [PATCH] Use metrics to display runner statuses instead of status field for EphemeralRunnerSet and AutoscalingRunnerSet --- .../v1alpha1/autoscalingrunnerset_types.go | 17 - .../v1alpha1/ephemeralrunnerset_types.go | 13 - ...ions.github.com_autoscalingrunnersets.yaml | 23 -- ...ctions.github.com_ephemeralrunnersets.yaml | 26 -- ...ions.github.com_autoscalingrunnersets.yaml | 23 -- ...ctions.github.com_ephemeralrunnersets.yaml | 26 -- ...ions.github.com_autoscalingrunnersets.yaml | 23 -- ...ctions.github.com_ephemeralrunnersets.yaml | 26 -- .../autoscalingrunnerset_controller.go | 28 +- .../autoscalingrunnerset_controller_test.go | 22 +- .../ephemeralrunner_controller.go | 90 +++++- .../ephemeralrunner_lifecycle.go | 60 ++++ .../ephemeralrunner_lifecycle_test.go | 303 ++++++++++++++++++ .../ephemeralrunnerset_controller.go | 32 +- .../ephemeralrunnerset_controller_test.go | 36 +-- .../lifecycle_metrics_integration_test.go | 115 +++++++ .../actions.github.com/metrics/metrics.go | 48 ++- .../metrics/metrics_test.go | 135 ++++++++ 18 files changed, 776 insertions(+), 270 deletions(-) create mode 100644 controllers/actions.github.com/ephemeralrunner_lifecycle.go create mode 100644 controllers/actions.github.com/ephemeralrunner_lifecycle_test.go create mode 100644 controllers/actions.github.com/metrics/lifecycle_metrics_integration_test.go create mode 100644 controllers/actions.github.com/metrics/metrics_test.go diff --git a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go index 908e1acc..4646384d 100644 --- a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go @@ -36,12 +36,7 @@ import ( // +kubebuilder:subresource:status // +kubebuilder:printcolumn:JSONPath=".spec.minRunners",name=Minimum Runners,type=integer // +kubebuilder:printcolumn:JSONPath=".spec.maxRunners",name=Maximum Runners,type=integer -// +kubebuilder:printcolumn:JSONPath=".status.currentRunners",name=Current Runners,type=integer // +kubebuilder:printcolumn:JSONPath=".status.phase",name=Phase,type=string -// +kubebuilder:printcolumn:JSONPath=".status.pendingEphemeralRunners",name=Pending Runners,type=integer -// +kubebuilder:printcolumn:JSONPath=".status.runningEphemeralRunners",name=Running Runners,type=integer -// +kubebuilder:printcolumn:JSONPath=".status.finishedEphemeralRunners",name=Finished Runners,type=integer -// +kubebuilder:printcolumn:JSONPath=".status.deletingEphemeralRunners",name=Deleting Runners,type=integer // AutoscalingRunnerSet is the Schema for the autoscalingrunnersets API type AutoscalingRunnerSet struct { @@ -314,20 +309,8 @@ type HistogramMetric struct { // AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet type AutoscalingRunnerSetStatus struct { - // +optional - CurrentRunners int `json:"currentRunners"` - // +optional Phase AutoscalingRunnerSetPhase `json:"phase"` - - // EphemeralRunner counts separated by the stage ephemeral runners are in, taken from the EphemeralRunnerSet - - // +optional - PendingEphemeralRunners int `json:"pendingEphemeralRunners"` - // +optional - RunningEphemeralRunners int `json:"runningEphemeralRunners"` - // +optional - FailedEphemeralRunners int `json:"failedEphemeralRunners"` } type AutoscalingRunnerSetPhase string diff --git a/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go b/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go index 9453f8a1..6b0051aa 100644 --- a/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go +++ b/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go @@ -37,14 +37,6 @@ type EphemeralRunnerSetSpec struct { // EphemeralRunnerSetStatus defines the observed state of EphemeralRunnerSet type EphemeralRunnerSetStatus struct { - // CurrentReplicas is the number of currently running EphemeralRunner resources being managed by this EphemeralRunnerSet. - CurrentReplicas int `json:"currentReplicas"` - // +optional - PendingEphemeralRunners int `json:"pendingEphemeralRunners"` - // +optional - RunningEphemeralRunners int `json:"runningEphemeralRunners"` - // +optional - FailedEphemeralRunners int `json:"failedEphemeralRunners"` // +optional Phase EphemeralRunnerSetPhase `json:"phase"` } @@ -62,11 +54,6 @@ const ( // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:printcolumn:JSONPath=".spec.replicas",name="DesiredReplicas",type="integer" -// +kubebuilder:printcolumn:JSONPath=".status.currentReplicas", name="CurrentReplicas",type="integer" -// +kubebuilder:printcolumn:JSONPath=".status.pendingEphemeralRunners",name=Pending Runners,type=integer -// +kubebuilder:printcolumn:JSONPath=".status.runningEphemeralRunners",name=Running Runners,type=integer -// +kubebuilder:printcolumn:JSONPath=".status.finishedEphemeralRunners",name=Finished Runners,type=integer -// +kubebuilder:printcolumn:JSONPath=".status.deletingEphemeralRunners",name=Deleting Runners,type=integer // +kubebuilder:printcolumn:JSONPath=".status.phase",name=Phase,type=string // EphemeralRunnerSet is the Schema for the ephemeralrunnersets API 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 d788cc1c..1f4b63f3 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 @@ -21,24 +21,9 @@ spec: - jsonPath: .spec.maxRunners name: Maximum Runners type: integer - - jsonPath: .status.currentRunners - name: Current Runners - type: integer - jsonPath: .status.phase name: Phase type: string - - jsonPath: .status.pendingEphemeralRunners - name: Pending Runners - type: integer - - jsonPath: .status.runningEphemeralRunners - name: Running Runners - type: integer - - jsonPath: .status.finishedEphemeralRunners - name: Finished Runners - type: integer - - jsonPath: .status.deletingEphemeralRunners - name: Deleting Runners - type: integer name: v1alpha1 schema: openAPIV3Schema: @@ -16543,16 +16528,8 @@ spec: status: description: AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet properties: - currentRunners: - type: integer - failedEphemeralRunners: - type: integer - pendingEphemeralRunners: - type: integer phase: type: string - runningEphemeralRunners: - type: integer type: object type: object served: true diff --git a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunnersets.yaml b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunnersets.yaml index 7e8ade4a..fa706e3b 100644 --- a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunnersets.yaml @@ -18,21 +18,6 @@ spec: - jsonPath: .spec.replicas name: DesiredReplicas type: integer - - jsonPath: .status.currentReplicas - name: CurrentReplicas - type: integer - - jsonPath: .status.pendingEphemeralRunners - name: Pending Runners - type: integer - - jsonPath: .status.runningEphemeralRunners - name: Running Runners - type: integer - - jsonPath: .status.finishedEphemeralRunners - name: Finished Runners - type: integer - - jsonPath: .status.deletingEphemeralRunners - name: Deleting Runners - type: integer - jsonPath: .status.phase name: Phase type: string @@ -8316,20 +8301,9 @@ spec: status: description: EphemeralRunnerSetStatus defines the observed state of EphemeralRunnerSet properties: - currentReplicas: - description: CurrentReplicas is the number of currently running EphemeralRunner resources being managed by this EphemeralRunnerSet. - type: integer - failedEphemeralRunners: - type: integer - pendingEphemeralRunners: - type: integer phase: description: EphemeralRunnerSetPhase is the phase of the ephemeral runner set resource type: string - runningEphemeralRunners: - type: integer - required: - - currentReplicas type: object type: object served: true 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 d788cc1c..1f4b63f3 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 @@ -21,24 +21,9 @@ spec: - jsonPath: .spec.maxRunners name: Maximum Runners type: integer - - jsonPath: .status.currentRunners - name: Current Runners - type: integer - jsonPath: .status.phase name: Phase type: string - - jsonPath: .status.pendingEphemeralRunners - name: Pending Runners - type: integer - - jsonPath: .status.runningEphemeralRunners - name: Running Runners - type: integer - - jsonPath: .status.finishedEphemeralRunners - name: Finished Runners - type: integer - - jsonPath: .status.deletingEphemeralRunners - name: Deleting Runners - type: integer name: v1alpha1 schema: openAPIV3Schema: @@ -16543,16 +16528,8 @@ spec: status: description: AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet properties: - currentRunners: - type: integer - failedEphemeralRunners: - type: integer - pendingEphemeralRunners: - type: integer phase: type: string - runningEphemeralRunners: - type: integer type: object type: object served: true diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml index 7e8ade4a..fa706e3b 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml @@ -18,21 +18,6 @@ spec: - jsonPath: .spec.replicas name: DesiredReplicas type: integer - - jsonPath: .status.currentReplicas - name: CurrentReplicas - type: integer - - jsonPath: .status.pendingEphemeralRunners - name: Pending Runners - type: integer - - jsonPath: .status.runningEphemeralRunners - name: Running Runners - type: integer - - jsonPath: .status.finishedEphemeralRunners - name: Finished Runners - type: integer - - jsonPath: .status.deletingEphemeralRunners - name: Deleting Runners - type: integer - jsonPath: .status.phase name: Phase type: string @@ -8316,20 +8301,9 @@ spec: status: description: EphemeralRunnerSetStatus defines the observed state of EphemeralRunnerSet properties: - currentReplicas: - description: CurrentReplicas is the number of currently running EphemeralRunner resources being managed by this EphemeralRunnerSet. - type: integer - failedEphemeralRunners: - type: integer - pendingEphemeralRunners: - type: integer phase: description: EphemeralRunnerSetPhase is the phase of the ephemeral runner set resource type: string - runningEphemeralRunners: - type: integer - required: - - currentReplicas type: object type: object served: true diff --git a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml index d788cc1c..1f4b63f3 100644 --- a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml +++ b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml @@ -21,24 +21,9 @@ spec: - jsonPath: .spec.maxRunners name: Maximum Runners type: integer - - jsonPath: .status.currentRunners - name: Current Runners - type: integer - jsonPath: .status.phase name: Phase type: string - - jsonPath: .status.pendingEphemeralRunners - name: Pending Runners - type: integer - - jsonPath: .status.runningEphemeralRunners - name: Running Runners - type: integer - - jsonPath: .status.finishedEphemeralRunners - name: Finished Runners - type: integer - - jsonPath: .status.deletingEphemeralRunners - name: Deleting Runners - type: integer name: v1alpha1 schema: openAPIV3Schema: @@ -16543,16 +16528,8 @@ spec: status: description: AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet properties: - currentRunners: - type: integer - failedEphemeralRunners: - type: integer - pendingEphemeralRunners: - type: integer phase: type: string - runningEphemeralRunners: - type: integer type: object type: object served: true diff --git a/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml b/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml index 7e8ade4a..fa706e3b 100644 --- a/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml +++ b/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml @@ -18,21 +18,6 @@ spec: - jsonPath: .spec.replicas name: DesiredReplicas type: integer - - jsonPath: .status.currentReplicas - name: CurrentReplicas - type: integer - - jsonPath: .status.pendingEphemeralRunners - name: Pending Runners - type: integer - - jsonPath: .status.runningEphemeralRunners - name: Running Runners - type: integer - - jsonPath: .status.finishedEphemeralRunners - name: Finished Runners - type: integer - - jsonPath: .status.deletingEphemeralRunners - name: Deleting Runners - type: integer - jsonPath: .status.phase name: Phase type: string @@ -8316,20 +8301,9 @@ spec: status: description: EphemeralRunnerSetStatus defines the observed state of EphemeralRunnerSet properties: - currentReplicas: - description: CurrentReplicas is the number of currently running EphemeralRunner resources being managed by this EphemeralRunnerSet. - type: integer - failedEphemeralRunners: - type: integer - pendingEphemeralRunners: - type: integer phase: description: EphemeralRunnerSetPhase is the phase of the ephemeral runner set resource type: string - runningEphemeralRunners: - type: integer - required: - - currentReplicas type: object type: object served: true diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go index a07cb350..f565a147 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -295,7 +295,19 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl // When runners are actively processing jobs, defer the spec update: // delete the listener to stop accepting new jobs, but leave the ERS // (and its running pods) untouched until all jobs have drained. - if ephemeralRunnerSet.Status.RunningEphemeralRunners+ephemeralRunnerSet.Status.PendingEphemeralRunners > 0 { + var ephemeralRunnerList v1alpha1.EphemeralRunnerList + if err := r.List(ctx, &ephemeralRunnerList, + client.InNamespace(ephemeralRunnerSet.Namespace), + client.MatchingFields{resourceOwnerKey: ephemeralRunnerSet.Name}, + ); err != nil { + log.Error(err, "Failed to list ephemeral runners") + return ctrl.Result{}, err + } + + buckets := AggregateEphemeralRunnerLifecycle(ephemeralRunnerList.Items) + activeRunners := buckets.Pending + buckets.Running + + if activeRunners > 0 { log.Info("Ephemeral runner set spec changed but runners are still active; deleting listener to stop new jobs") if _, err := r.cleanupListener(ctx, &autoscalingRunnerSet, log); err != nil { log.Error(err, "Failed to clean up listener while waiting for runners to drain") @@ -433,23 +445,13 @@ 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, ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet, phase v1alpha1.AutoscalingRunnerSetPhase, log logr.Logger) error { - countDiff := ephemeralRunnerSet != nil && ephemeralRunnerSet.Status.CurrentReplicas != autoscalingRunnerSet.Status.CurrentRunners phaseDiff := phase != autoscalingRunnerSet.Status.Phase - if !countDiff && !phaseDiff { + if !phaseDiff { return nil } original := autoscalingRunnerSet.DeepCopy() - if phaseDiff { - autoscalingRunnerSet.Status.Phase = phase - } - - if countDiff && ephemeralRunnerSet != nil { - autoscalingRunnerSet.Status.CurrentRunners = ephemeralRunnerSet.Status.CurrentReplicas - autoscalingRunnerSet.Status.PendingEphemeralRunners = ephemeralRunnerSet.Status.PendingEphemeralRunners - autoscalingRunnerSet.Status.RunningEphemeralRunners = ephemeralRunnerSet.Status.RunningEphemeralRunners - autoscalingRunnerSet.Status.FailedEphemeralRunners = ephemeralRunnerSet.Status.FailedEphemeralRunners - } + autoscalingRunnerSet.Status.Phase = phase 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 55ebe5c6..0de00581 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go @@ -871,17 +871,10 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { // Emulate running and pending jobs runnerSet := runnerSetList.Items[0] activeRunnerSet := runnerSet.DeepCopy() - activeRunnerSet.Status.CurrentReplicas = 6 - activeRunnerSet.Status.FailedEphemeralRunners = 1 - activeRunnerSet.Status.RunningEphemeralRunners = 2 - activeRunnerSet.Status.PendingEphemeralRunners = 3 + activeRunnerSet.Status.Phase = v1alpha1.EphemeralRunnerSetPhaseRunning desiredStatus := v1alpha1.AutoscalingRunnerSetStatus{ - CurrentRunners: activeRunnerSet.Status.CurrentReplicas, - Phase: v1alpha1.AutoscalingRunnerSetPhaseRunning, - PendingEphemeralRunners: activeRunnerSet.Status.PendingEphemeralRunners, - RunningEphemeralRunners: activeRunnerSet.Status.RunningEphemeralRunners, - FailedEphemeralRunners: activeRunnerSet.Status.FailedEphemeralRunners, + Phase: v1alpha1.AutoscalingRunnerSetPhaseRunning, } err = k8sClient.Status().Patch(ctx, activeRunnerSet, client.MergeFrom(&runnerSet)) @@ -978,17 +971,10 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { runnerSet := runnerSetList.Items[0] statusUpdate := runnerSet.DeepCopy() - statusUpdate.Status.CurrentReplicas = 6 - statusUpdate.Status.FailedEphemeralRunners = 1 - statusUpdate.Status.RunningEphemeralRunners = 2 - statusUpdate.Status.PendingEphemeralRunners = 3 + statusUpdate.Status.Phase = v1alpha1.EphemeralRunnerSetPhaseRunning desiredStatus := v1alpha1.AutoscalingRunnerSetStatus{ - CurrentRunners: statusUpdate.Status.CurrentReplicas, - Phase: v1alpha1.AutoscalingRunnerSetPhaseRunning, - PendingEphemeralRunners: statusUpdate.Status.PendingEphemeralRunners, - RunningEphemeralRunners: statusUpdate.Status.RunningEphemeralRunners, - FailedEphemeralRunners: statusUpdate.Status.FailedEphemeralRunners, + Phase: v1alpha1.AutoscalingRunnerSetPhaseRunning, } err := k8sClient.Status().Patch(ctx, statusUpdate, client.MergeFrom(&runnerSet)) diff --git a/controllers/actions.github.com/ephemeralrunner_controller.go b/controllers/actions.github.com/ephemeralrunner_controller.go index 64b98e27..5ce0c444 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller.go +++ b/controllers/actions.github.com/ephemeralrunner_controller.go @@ -25,6 +25,7 @@ import ( "time" "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" + "github.com/actions/actions-runner-controller/controllers/actions.github.com/metrics" "github.com/actions/scaleset" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" @@ -46,8 +47,9 @@ const ( // EphemeralRunnerReconciler reconciles a EphemeralRunner object type EphemeralRunnerReconciler struct { client.Client - Log logr.Logger - Scheme *runtime.Scheme + Log logr.Logger + Scheme *runtime.Scheme + PublishMetrics bool ResourceBuilder } @@ -86,6 +88,8 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ original := ephemeralRunner.DeepCopy() if !ephemeralRunner.DeletionTimestamp.IsZero() { + r.emitLifecycleMetrics(ctx, &ephemeralRunner, log) + if !controllerutil.ContainsFinalizer(&ephemeralRunner, ephemeralRunnerFinalizerName) { return ctrl.Result{}, nil } @@ -584,6 +588,8 @@ func (r *EphemeralRunnerReconciler) markAsFailed(ctx context.Context, ephemeralR return fmt.Errorf("failed to update ephemeral runner status Phase/Message: %w", err) } + r.emitLifecycleMetrics(ctx, ephemeralRunner, log) + log.Info("Removing the runner from the service") if err := r.deleteRunnerFromService(ctx, ephemeralRunner, log); err != nil { return fmt.Errorf("failed to remove the runner from service: %w", err) @@ -605,6 +611,8 @@ func (r *EphemeralRunnerReconciler) markAsOutdated(ctx context.Context, ephemera return fmt.Errorf("failed to update ephemeral runner status Phase/Message: %w", err) } + r.emitLifecycleMetrics(ctx, ephemeralRunner, log) + log.Info("Removing the runner from the service") if err := r.deleteRunnerFromService(ctx, ephemeralRunner, log); err != nil { return fmt.Errorf("failed to remove the runner from service: %w", err) @@ -638,6 +646,8 @@ func (r *EphemeralRunnerReconciler) deletePodAsFailed(ctx context.Context, ephem return fmt.Errorf("failed to update ephemeral runner status with failure count: %w", err) } + r.emitLifecycleMetrics(ctx, ephemeralRunner, log) + log.Info("EphemeralRunner pod is deleted and status is updated with failure count") return nil } @@ -836,6 +846,8 @@ func (r *EphemeralRunnerReconciler) updateRunStatusFromPod(ctx context.Context, return fmt.Errorf("failed to update runner status for Phase/Reason/Message/Ready: %w", err) } + r.emitLifecycleMetrics(ctx, ephemeralRunner, log) + log.Info("Updated ephemeral runner status") return nil } @@ -888,3 +900,77 @@ func initContainerFailed(pod *corev1.Pod) bool { } return false } + +// emitLifecycleMetrics recomputes and emits lifecycle metrics for all EphemeralRunners +// owned by the same EphemeralRunnerSet as the given runner. This ensures metrics reflect +// the current lifecycle state of all sibling runners under the same AutoscalingRunnerSet. +// +// Label values are derived from the runner's labels (LabelKeyGitHubScaleSetName, etc.). +// If required labels are missing, logs a warning and skips metric emission. +func (r *EphemeralRunnerReconciler) emitLifecycleMetrics(ctx context.Context, ephemeralRunner *v1alpha1.EphemeralRunner, log logr.Logger) { + if !r.PublishMetrics { + return + } + + // Extract owner EphemeralRunnerSet from controller owner reference + ownerRef := metav1.GetControllerOfNoCopy(ephemeralRunner) + if ownerRef == nil || ownerRef.Kind != "EphemeralRunnerSet" { + log.V(1).Info("EphemeralRunner has no EphemeralRunnerSet owner, skipping metric emission") + return + } + + // List all sibling EphemeralRunners owned by the same EphemeralRunnerSet + var runnerList v1alpha1.EphemeralRunnerList + if err := r.List(ctx, &runnerList, + client.InNamespace(ephemeralRunner.Namespace), + client.MatchingFields{resourceOwnerKey: ownerRef.Name}, + ); err != nil { + log.Error(err, "Failed to list sibling EphemeralRunners for metric emission") + return + } + + // Aggregate lifecycle counts using the helper from Task 3 + buckets := AggregateEphemeralRunnerLifecycle(runnerList.Items) + + // Extract label values from the runner (all siblings share the same labels) + name := ephemeralRunner.Labels[LabelKeyGitHubScaleSetName] + namespace := ephemeralRunner.Labels[LabelKeyGitHubScaleSetNamespace] + repository := ephemeralRunner.Labels[LabelKeyGitHubRepository] + organization := ephemeralRunner.Labels[LabelKeyGitHubOrganization] + enterprise := ephemeralRunner.Labels[LabelKeyGitHubEnterprise] + + // Gracefully handle missing labels: log warning and skip if name/namespace empty + if name == "" || namespace == "" { + log.Info("Missing required labels (name/namespace) for metric emission, skipping", + "name", name, + "namespace", namespace, + ) + return + } + + // Emit all six lifecycle metrics + metrics.SetEphemeralRunnerCountsByLifecycle( + metrics.CommonLabels{ + Name: name, + Namespace: namespace, + Repository: repository, + Organization: organization, + Enterprise: enterprise, + }, + buckets.Pending, + buckets.Running, + buckets.Succeeded, + buckets.Failed, + buckets.Outdated, + buckets.Deleting, + ) + + log.V(1).Info("Emitted lifecycle metrics", + "pending", buckets.Pending, + "running", buckets.Running, + "succeeded", buckets.Succeeded, + "failed", buckets.Failed, + "outdated", buckets.Outdated, + "deleting", buckets.Deleting, + ) +} diff --git a/controllers/actions.github.com/ephemeralrunner_lifecycle.go b/controllers/actions.github.com/ephemeralrunner_lifecycle.go new file mode 100644 index 00000000..7c8af4bc --- /dev/null +++ b/controllers/actions.github.com/ephemeralrunner_lifecycle.go @@ -0,0 +1,60 @@ +package actionsgithubcom + +import ( + v1alpha1 "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" +) + +// EphemeralRunnerLifecycleBuckets represents the counts of ephemeral runners +// in each lifecycle state according to the precedence contract: +// 1. deleting if DeletionTimestamp != nil +// 2. Otherwise by Status.Phase (Running/Succeeded/Failed/Outdated) +// 3. Fallback to pending for empty/unset/other +type EphemeralRunnerLifecycleBuckets struct { + Pending int + Running int + Succeeded int + Failed int + Outdated int + Deleting int +} + +// AggregateEphemeralRunnerLifecycle classifies a list of EphemeralRunners into +// lifecycle buckets using deterministic precedence to ensure each runner is counted +// in exactly one bucket. This helper is independent from EphemeralRunnerSet-specific +// structures and can be called directly by any controller managing EphemeralRunners. +// +// Precedence contract: +// 1. deleting if DeletionTimestamp != nil (takes precedence over all phases) +// 2. Otherwise by Status.Phase: Running/Succeeded/Failed/Outdated +// 3. Fallback to pending for empty/unset/other phases +func AggregateEphemeralRunnerLifecycle(runners []v1alpha1.EphemeralRunner) EphemeralRunnerLifecycleBuckets { + var buckets EphemeralRunnerLifecycleBuckets + + for i := range runners { + r := &runners[i] + + // Precedence 1: DeletionTimestamp takes precedence over all phases + if !r.DeletionTimestamp.IsZero() { + buckets.Deleting++ + continue + } + + // Precedence 2: Classify by Status.Phase + switch r.Status.Phase { + case v1alpha1.EphemeralRunnerPhaseRunning: + buckets.Running++ + case v1alpha1.EphemeralRunnerPhaseSucceeded: + buckets.Succeeded++ + case v1alpha1.EphemeralRunnerPhaseFailed: + buckets.Failed++ + case v1alpha1.EphemeralRunnerPhaseOutdated: + buckets.Outdated++ + default: + // Precedence 3: Fallback to pending for empty/unset/other phases + // This includes EphemeralRunnerPhasePending and any unset or unrecognized phases + buckets.Pending++ + } + } + + return buckets +} diff --git a/controllers/actions.github.com/ephemeralrunner_lifecycle_test.go b/controllers/actions.github.com/ephemeralrunner_lifecycle_test.go new file mode 100644 index 00000000..0afb2ade --- /dev/null +++ b/controllers/actions.github.com/ephemeralrunner_lifecycle_test.go @@ -0,0 +1,303 @@ +package actionsgithubcom + +import ( + "testing" + + v1alpha1 "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestAggregateEphemeralRunnerLifecycle(t *testing.T) { + now := metav1.Now() + + tests := []struct { + name string + runners []v1alpha1.EphemeralRunner + expected EphemeralRunnerLifecycleBuckets + }{ + { + name: "empty list", + runners: []v1alpha1.EphemeralRunner{}, + expected: EphemeralRunnerLifecycleBuckets{ + Pending: 0, Running: 0, Succeeded: 0, Failed: 0, Outdated: 0, Deleting: 0, + }, + }, + { + name: "all phases without deletion timestamp", + runners: []v1alpha1.EphemeralRunner{ + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhasePending}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseRunning}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseSucceeded}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseFailed}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseOutdated}}, + }, + expected: EphemeralRunnerLifecycleBuckets{ + Pending: 1, Running: 1, Succeeded: 1, Failed: 1, Outdated: 1, Deleting: 0, + }, + }, + { + name: "empty phase defaults to pending", + runners: []v1alpha1.EphemeralRunner{ + {Status: v1alpha1.EphemeralRunnerStatus{Phase: ""}}, + {Status: v1alpha1.EphemeralRunnerStatus{}}, + }, + expected: EphemeralRunnerLifecycleBuckets{ + Pending: 2, Running: 0, Succeeded: 0, Failed: 0, Outdated: 0, Deleting: 0, + }, + }, + { + name: "unrecognized phase defaults to pending", + runners: []v1alpha1.EphemeralRunner{ + {Status: v1alpha1.EphemeralRunnerStatus{Phase: "UnknownPhase"}}, + }, + expected: EphemeralRunnerLifecycleBuckets{ + Pending: 1, Running: 0, Succeeded: 0, Failed: 0, Outdated: 0, Deleting: 0, + }, + }, + { + name: "deletion timestamp takes precedence over all phases", + runners: []v1alpha1.EphemeralRunner{ + { + ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now}, + Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhasePending}, + }, + { + ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now}, + Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseRunning}, + }, + { + ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now}, + Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseSucceeded}, + }, + { + ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now}, + Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseFailed}, + }, + { + ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now}, + Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseOutdated}, + }, + }, + expected: EphemeralRunnerLifecycleBuckets{ + Pending: 0, Running: 0, Succeeded: 0, Failed: 0, Outdated: 0, Deleting: 5, + }, + }, + { + name: "edge case: running phase with deletion timestamp goes to deleting", + runners: []v1alpha1.EphemeralRunner{ + { + ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now}, + Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseRunning}, + }, + }, + expected: EphemeralRunnerLifecycleBuckets{ + Pending: 0, Running: 0, Succeeded: 0, Failed: 0, Outdated: 0, Deleting: 1, + }, + }, + { + name: "mixed runners with and without deletion timestamp", + runners: []v1alpha1.EphemeralRunner{ + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseRunning}}, + { + ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now}, + Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseRunning}, + }, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseFailed}}, + { + ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now}, + Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseFailed}, + }, + }, + expected: EphemeralRunnerLifecycleBuckets{ + Pending: 0, Running: 1, Succeeded: 0, Failed: 1, Outdated: 0, Deleting: 2, + }, + }, + { + name: "multiple runners in same phase", + runners: []v1alpha1.EphemeralRunner{ + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseRunning}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseRunning}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseRunning}}, + }, + expected: EphemeralRunnerLifecycleBuckets{ + Pending: 0, Running: 3, Succeeded: 0, Failed: 0, Outdated: 0, Deleting: 0, + }, + }, + { + name: "deletion timestamp with empty phase goes to deleting", + runners: []v1alpha1.EphemeralRunner{ + { + ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now}, + Status: v1alpha1.EphemeralRunnerStatus{Phase: ""}, + }, + }, + expected: EphemeralRunnerLifecycleBuckets{ + Pending: 0, Running: 0, Succeeded: 0, Failed: 0, Outdated: 0, Deleting: 1, + }, + }, + { + name: "comprehensive mixed scenario", + runners: []v1alpha1.EphemeralRunner{ + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhasePending}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhasePending}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseRunning}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseRunning}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseRunning}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseSucceeded}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseFailed}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseFailed}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseOutdated}}, + { + ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now}, + Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseRunning}, + }, + { + ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now}, + Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseSucceeded}, + }, + }, + expected: EphemeralRunnerLifecycleBuckets{ + Pending: 2, Running: 3, Succeeded: 1, Failed: 2, Outdated: 1, Deleting: 2, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := AggregateEphemeralRunnerLifecycle(tt.runners) + + if result.Pending != tt.expected.Pending { + t.Errorf("Pending count mismatch: got %d, want %d", result.Pending, tt.expected.Pending) + } + if result.Running != tt.expected.Running { + t.Errorf("Running count mismatch: got %d, want %d", result.Running, tt.expected.Running) + } + if result.Succeeded != tt.expected.Succeeded { + t.Errorf("Succeeded count mismatch: got %d, want %d", result.Succeeded, tt.expected.Succeeded) + } + if result.Failed != tt.expected.Failed { + t.Errorf("Failed count mismatch: got %d, want %d", result.Failed, tt.expected.Failed) + } + if result.Outdated != tt.expected.Outdated { + t.Errorf("Outdated count mismatch: got %d, want %d", result.Outdated, tt.expected.Outdated) + } + if result.Deleting != tt.expected.Deleting { + t.Errorf("Deleting count mismatch: got %d, want %d", result.Deleting, tt.expected.Deleting) + } + + total := result.Pending + result.Running + result.Succeeded + result.Failed + result.Outdated + result.Deleting + if total != len(tt.runners) { + t.Errorf("Total count mismatch: counted %d runners but input had %d runners", total, len(tt.runners)) + } + }) + } +} + +func TestAggregateEphemeralRunnerLifecycle_EdgePrecedence(t *testing.T) { + now := metav1.Now() + + tests := []struct { + name string + phase v1alpha1.EphemeralRunnerPhase + hasDeletionTime bool + expectedBucket string + }{ + { + name: "Running without deletion timestamp", + phase: v1alpha1.EphemeralRunnerPhaseRunning, + hasDeletionTime: false, + expectedBucket: "running", + }, + { + name: "Running with deletion timestamp goes to deleting", + phase: v1alpha1.EphemeralRunnerPhaseRunning, + hasDeletionTime: true, + expectedBucket: "deleting", + }, + { + name: "Succeeded without deletion timestamp", + phase: v1alpha1.EphemeralRunnerPhaseSucceeded, + hasDeletionTime: false, + expectedBucket: "succeeded", + }, + { + name: "Succeeded with deletion timestamp goes to deleting", + phase: v1alpha1.EphemeralRunnerPhaseSucceeded, + hasDeletionTime: true, + expectedBucket: "deleting", + }, + { + name: "Failed without deletion timestamp", + phase: v1alpha1.EphemeralRunnerPhaseFailed, + hasDeletionTime: false, + expectedBucket: "failed", + }, + { + name: "Failed with deletion timestamp goes to deleting", + phase: v1alpha1.EphemeralRunnerPhaseFailed, + hasDeletionTime: true, + expectedBucket: "deleting", + }, + { + name: "Outdated without deletion timestamp", + phase: v1alpha1.EphemeralRunnerPhaseOutdated, + hasDeletionTime: false, + expectedBucket: "outdated", + }, + { + name: "Outdated with deletion timestamp goes to deleting", + phase: v1alpha1.EphemeralRunnerPhaseOutdated, + hasDeletionTime: true, + expectedBucket: "deleting", + }, + { + name: "Pending without deletion timestamp", + phase: v1alpha1.EphemeralRunnerPhasePending, + hasDeletionTime: false, + expectedBucket: "pending", + }, + { + name: "Pending with deletion timestamp goes to deleting", + phase: v1alpha1.EphemeralRunnerPhasePending, + hasDeletionTime: true, + expectedBucket: "deleting", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + runner := v1alpha1.EphemeralRunner{ + Status: v1alpha1.EphemeralRunnerStatus{Phase: tt.phase}, + } + if tt.hasDeletionTime { + runner.DeletionTimestamp = &now + } + + result := AggregateEphemeralRunnerLifecycle([]v1alpha1.EphemeralRunner{runner}) + + var foundBucket string + if result.Pending == 1 { + foundBucket = "pending" + } else if result.Running == 1 { + foundBucket = "running" + } else if result.Succeeded == 1 { + foundBucket = "succeeded" + } else if result.Failed == 1 { + foundBucket = "failed" + } else if result.Outdated == 1 { + foundBucket = "outdated" + } else if result.Deleting == 1 { + foundBucket = "deleting" + } + + if foundBucket != tt.expectedBucket { + t.Errorf("Expected runner to be in %s bucket, but found in %s bucket", tt.expectedBucket, foundBucket) + } + + total := result.Pending + result.Running + result.Succeeded + result.Failed + result.Outdated + result.Deleting + if total != 1 { + t.Errorf("Expected exactly 1 runner to be counted, but got %d", total) + } + }) + } +} diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller.go b/controllers/actions.github.com/ephemeralrunnerset_controller.go index 381c9d29..4a1ed903 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller.go @@ -27,9 +27,7 @@ import ( "time" "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" - "github.com/actions/actions-runner-controller/controllers/actions.github.com/metrics" "github.com/actions/actions-runner-controller/controllers/actions.github.com/multiclient" - "github.com/actions/actions-runner-controller/github/actions" "github.com/actions/scaleset" "github.com/go-logr/logr" "go.uber.org/multierr" @@ -205,29 +203,6 @@ func (r *EphemeralRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl.R "deleting", len(ephemeralRunnersByState.deleting), ) - if r.PublishMetrics { - githubConfigURL := ephemeralRunnerSet.Spec.EphemeralRunnerSpec.GitHubConfigURL - parsedURL, err := actions.ParseGitHubConfigFromURL(githubConfigURL) - if err != nil { - log.Error(err, "Github Config URL is invalid", "URL", githubConfigURL) - // stop reconciling on this object - return ctrl.Result{}, nil - } - - metrics.SetEphemeralRunnerCountsByStatus( - metrics.CommonLabels{ - Name: ephemeralRunnerSet.Labels[LabelKeyGitHubScaleSetName], - Namespace: ephemeralRunnerSet.Labels[LabelKeyGitHubScaleSetNamespace], - Repository: parsedURL.Repository, - Organization: parsedURL.Organization, - Enterprise: parsedURL.Enterprise, - }, - len(ephemeralRunnersByState.pending), - len(ephemeralRunnersByState.running), - len(ephemeralRunnersByState.failed), - ) - } - total := ephemeralRunnersByState.scaleTotal() if ephemeralRunnerSet.Spec.PatchID == 0 || ephemeralRunnerSet.Spec.PatchID != ephemeralRunnersByState.latestPatchID { defer func() { @@ -272,7 +247,6 @@ func (r *EphemeralRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl.R func (r *EphemeralRunnerSetReconciler) updateStatus(ctx context.Context, ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet, state *ephemeralRunnersByState, log logr.Logger) error { original := ephemeralRunnerSet.DeepCopy() - total := state.scaleTotal() var phase v1alpha1.EphemeralRunnerSetPhase switch { case len(state.outdated) > 0: @@ -283,11 +257,7 @@ func (r *EphemeralRunnerSetReconciler) updateStatus(ctx context.Context, ephemer phase = ephemeralRunnerSet.Status.Phase } desiredStatus := v1alpha1.EphemeralRunnerSetStatus{ - CurrentReplicas: total, - Phase: phase, - PendingEphemeralRunners: len(state.pending), - RunningEphemeralRunners: len(state.running), - FailedEphemeralRunners: len(state.failed), + Phase: phase, } // Update the status if needed. diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go index c3459652..8eb485d6 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go @@ -136,17 +136,16 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { // Check if the status stay 0 Consistently( func() (int, error) { - runnerSet := new(v1alpha1.EphemeralRunnerSet) - err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunnerSet.Name, Namespace: ephemeralRunnerSet.Namespace}, runnerSet) + var runnerList v1alpha1.EphemeralRunnerList + err := k8sClient.List(ctx, &runnerList, client.InNamespace(ephemeralRunnerSet.Namespace), client.MatchingFields{resourceOwnerKey: ephemeralRunnerSet.Name}) if err != nil { return -1, err } - - return int(runnerSet.Status.CurrentReplicas), nil + return len(runnerList.Items), nil }, ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval, - ).Should(BeEquivalentTo(0), "EphemeralRunnerSet status should be 0") + ).Should(BeEquivalentTo(0), "EphemeralRunnerSet should have 0 runners") // Scaling up the EphemeralRunnerSet updated := created.DeepCopy() @@ -190,17 +189,16 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { // Check if the status is updated Eventually( func() (int, error) { - runnerSet := new(v1alpha1.EphemeralRunnerSet) - err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunnerSet.Name, Namespace: ephemeralRunnerSet.Namespace}, runnerSet) + var runnerList v1alpha1.EphemeralRunnerList + err := k8sClient.List(ctx, &runnerList, client.InNamespace(ephemeralRunnerSet.Namespace), client.MatchingFields{resourceOwnerKey: ephemeralRunnerSet.Name}) if err != nil { return -1, err } - - return int(runnerSet.Status.CurrentReplicas), nil + return len(runnerList.Items), nil }, ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval, - ).Should(BeEquivalentTo(5), "EphemeralRunnerSet status should be 5") + ).Should(BeEquivalentTo(5), "EphemeralRunnerSet should have 5 runners") }) }) @@ -1184,11 +1182,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { ).Should(BeTrue(), "Failed to eventually update to one pending, one running and one failed") desiredStatus := v1alpha1.EphemeralRunnerSetStatus{ - Phase: v1alpha1.EphemeralRunnerSetPhaseRunning, - CurrentReplicas: 3, - PendingEphemeralRunners: 1, - RunningEphemeralRunners: 1, - FailedEphemeralRunners: 1, + Phase: v1alpha1.EphemeralRunnerSetPhaseRunning, } Eventually( func() (v1alpha1.EphemeralRunnerSetStatus, error) { @@ -1227,11 +1221,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { ).Should(BeEquivalentTo(1), "Failed to eventually scale down") desiredStatus = v1alpha1.EphemeralRunnerSetStatus{ - CurrentReplicas: 1, - PendingEphemeralRunners: 0, - RunningEphemeralRunners: 0, - FailedEphemeralRunners: 1, - Phase: v1alpha1.EphemeralRunnerSetPhaseRunning, + Phase: v1alpha1.EphemeralRunnerSetPhaseRunning, } Eventually( @@ -1251,11 +1241,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { Expect(err).To(BeNil(), "Failed to delete failed ephemeral runner") desiredStatus = v1alpha1.EphemeralRunnerSetStatus{ - CurrentReplicas: 0, - PendingEphemeralRunners: 0, - RunningEphemeralRunners: 0, - FailedEphemeralRunners: 0, - Phase: v1alpha1.EphemeralRunnerSetPhaseRunning, + Phase: v1alpha1.EphemeralRunnerSetPhaseRunning, } Eventually( func() (v1alpha1.EphemeralRunnerSetStatus, error) { diff --git a/controllers/actions.github.com/metrics/lifecycle_metrics_integration_test.go b/controllers/actions.github.com/metrics/lifecycle_metrics_integration_test.go new file mode 100644 index 00000000..d215a775 --- /dev/null +++ b/controllers/actions.github.com/metrics/lifecycle_metrics_integration_test.go @@ -0,0 +1,115 @@ +package metrics_test + +import ( + "fmt" + "sync" + "testing" + + v1alpha1 "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" + actionsgithubcom "github.com/actions/actions-runner-controller/controllers/actions.github.com" + githubmetrics "github.com/actions/actions-runner-controller/controllers/actions.github.com/metrics" + dto "github.com/prometheus/client_model/go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + controllerMetrics "sigs.k8s.io/controller-runtime/pkg/metrics" +) + +var registerMetricsOnce sync.Once + +func TestEphemeralRunnerLifecycleBucketsSetPrometheusMetrics(t *testing.T) { + registerMetricsOnce.Do(githubmetrics.RegisterMetrics) + + now := metav1.Now() + runners := []v1alpha1.EphemeralRunner{ + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhasePending}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseRunning}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseRunning}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseSucceeded}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseFailed}}, + {Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseOutdated}}, + { + ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now}, + Status: v1alpha1.EphemeralRunnerStatus{Phase: v1alpha1.EphemeralRunnerPhaseRunning}, + }, + } + + buckets := actionsgithubcom.AggregateEphemeralRunnerLifecycle(runners) + labels := githubmetrics.CommonLabels{ + Name: "lifecycle-metrics-test", + Namespace: "default", + Repository: "test/repo", + Organization: "test-org", + Enterprise: "test-enterprise", + } + + githubmetrics.SetEphemeralRunnerCountsByLifecycle( + labels, + buckets.Pending, + buckets.Running, + buckets.Succeeded, + buckets.Failed, + buckets.Outdated, + buckets.Deleting, + ) + + metricFamilies, err := controllerMetrics.Registry.Gather() + require.NoError(t, err) + + labelSet := commonLabelSet(labels) + assertGatheredGauge(t, metricFamilies, "gha_controller_pending_ephemeral_runners", labelSet, 1) + assertGatheredGauge(t, metricFamilies, "gha_controller_running_ephemeral_runners", labelSet, 2) + assertGatheredGauge(t, metricFamilies, "gha_controller_succeeded_ephemeral_runners", labelSet, 1) + assertGatheredGauge(t, metricFamilies, "gha_controller_failed_ephemeral_runners", labelSet, 1) + assertGatheredGauge(t, metricFamilies, "gha_controller_outdated_ephemeral_runners", labelSet, 1) + assertGatheredGauge(t, metricFamilies, "gha_controller_deleting_ephemeral_runners", labelSet, 1) +} + +func assertGatheredGauge(t *testing.T, metricFamilies []*dto.MetricFamily, metricName string, expectedLabels map[string]string, expectedValue float64) { + t.Helper() + + for _, family := range metricFamilies { + if family.GetName() != metricName { + continue + } + + for _, metric := range family.GetMetric() { + if !metricHasLabels(metric, expectedLabels) { + continue + } + + require.NotNil(t, metric.Gauge, "metric %q should be a gauge", metricName) + assert.Equal(t, expectedValue, metric.Gauge.GetValue(), "metric %q value mismatch", metricName) + return + } + + require.Fail(t, fmt.Sprintf("metric %q with expected labels was not gathered", metricName)) + } + + require.Fail(t, fmt.Sprintf("metric family %q was not gathered", metricName)) +} + +func metricHasLabels(metric *dto.Metric, expectedLabels map[string]string) bool { + actualLabels := make(map[string]string, len(metric.GetLabel())) + for _, label := range metric.GetLabel() { + actualLabels[label.GetName()] = label.GetValue() + } + + for name, expectedValue := range expectedLabels { + if actualLabels[name] != expectedValue { + return false + } + } + + return true +} + +func commonLabelSet(labels githubmetrics.CommonLabels) map[string]string { + return map[string]string{ + "name": labels.Name, + "namespace": labels.Namespace, + "repository": labels.Repository, + "organization": labels.Organization, + "enterprise": labels.Enterprise, + } +} diff --git a/controllers/actions.github.com/metrics/metrics.go b/controllers/actions.github.com/metrics/metrics.go index 8e137514..a2763dd9 100644 --- a/controllers/actions.github.com/metrics/metrics.go +++ b/controllers/actions.github.com/metrics/metrics.go @@ -58,6 +58,30 @@ var ( }, labels, ) + succeededEphemeralRunners = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Subsystem: githubScaleSetControllerSubsystem, + Name: "succeeded_ephemeral_runners", + Help: "Number of ephemeral runners in a succeeded state.", + }, + labels, + ) + outdatedEphemeralRunners = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Subsystem: githubScaleSetControllerSubsystem, + Name: "outdated_ephemeral_runners", + Help: "Number of ephemeral runners in an outdated state.", + }, + labels, + ) + deletingEphemeralRunners = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Subsystem: githubScaleSetControllerSubsystem, + Name: "deleting_ephemeral_runners", + Help: "Number of ephemeral runners in a deleting state.", + }, + labels, + ) runningListeners = prometheus.NewGaugeVec( prometheus.GaugeOpts{ Subsystem: githubScaleSetControllerSubsystem, @@ -72,15 +96,31 @@ func RegisterMetrics() { metrics.Registry.MustRegister( pendingEphemeralRunners, runningEphemeralRunners, + succeededEphemeralRunners, failedEphemeralRunners, + outdatedEphemeralRunners, + deletingEphemeralRunners, runningListeners, ) } -func SetEphemeralRunnerCountsByStatus(commonLabels CommonLabels, pending, running, failed int) { - pendingEphemeralRunners.With(commonLabels.labels()).Set(float64(pending)) - runningEphemeralRunners.With(commonLabels.labels()).Set(float64(running)) - failedEphemeralRunners.With(commonLabels.labels()).Set(float64(failed)) +// SetEphemeralRunnerCountsByLifecycle sets ephemeral runner counts across all six lifecycle states +// using deterministic bucket assignment with explicit precedence: +// +// Lifecycle Precedence Contract: +// 1. deleting: if runner has DeletionTimestamp set +// 2. explicit phase buckets: if runner.Status.Phase is one of (Running/Succeeded/Failed/Outdated) +// 3. pending (fallback): for empty/unset/other phase values +// +// This ensures each runner maps to exactly one metric bucket (no double-counting). +func SetEphemeralRunnerCountsByLifecycle(commonLabels CommonLabels, pending, running, succeeded, failed, outdated, deleting int) { + labels := commonLabels.labels() + pendingEphemeralRunners.With(labels).Set(float64(pending)) + runningEphemeralRunners.With(labels).Set(float64(running)) + succeededEphemeralRunners.With(labels).Set(float64(succeeded)) + failedEphemeralRunners.With(labels).Set(float64(failed)) + outdatedEphemeralRunners.With(labels).Set(float64(outdated)) + deletingEphemeralRunners.With(labels).Set(float64(deleting)) } func AddRunningListener(commonLabels CommonLabels) { diff --git a/controllers/actions.github.com/metrics/metrics_test.go b/controllers/actions.github.com/metrics/metrics_test.go new file mode 100644 index 00000000..cc0612e2 --- /dev/null +++ b/controllers/actions.github.com/metrics/metrics_test.go @@ -0,0 +1,135 @@ +package metrics + +import ( + "testing" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSetEphemeralRunnerCountsByLifecycle(t *testing.T) { + testCases := []struct { + name string + labels CommonLabels + pending int + running int + succeeded int + failed int + outdated int + deleting int + }{ + { + name: "all zeros", + labels: CommonLabels{ + Name: "test-runner", + Namespace: "default", + Repository: "org/repo", + Organization: "org", + Enterprise: "enterprise1", + }, + pending: 0, + running: 0, + succeeded: 0, + failed: 0, + outdated: 0, + deleting: 0, + }, + { + name: "mixed counts", + labels: CommonLabels{ + Name: "test-runner", + Namespace: "default", + Repository: "org/repo", + Organization: "org", + Enterprise: "enterprise1", + }, + pending: 5, + running: 10, + succeeded: 3, + failed: 2, + outdated: 1, + deleting: 1, + }, + { + name: "all non-zero", + labels: CommonLabels{ + Name: "test-runner-2", + Namespace: "kube-system", + Repository: "other/repo", + Organization: "otherorg", + Enterprise: "enterprise2", + }, + pending: 1, + running: 1, + succeeded: 1, + failed: 1, + outdated: 1, + deleting: 1, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + SetEphemeralRunnerCountsByLifecycle( + tc.labels, + tc.pending, + tc.running, + tc.succeeded, + tc.failed, + tc.outdated, + tc.deleting, + ) + + labelString := testutil.CollectAndCount(pendingEphemeralRunners) + require.Greater(t, labelString, 0, "metrics should be registered") + }) + } +} + +func TestSetEphemeralRunnerCountsByLifecycleValues(t *testing.T) { + lbls := CommonLabels{ + Name: "precedence-test", + Namespace: "default", + Repository: "test/repo", + Organization: "test", + Enterprise: "testent", + } + + SetEphemeralRunnerCountsByLifecycle(lbls, 1, 2, 3, 4, 5, 6) + + assertGaugeValue(t, pendingEphemeralRunners, lbls, 1) + assertGaugeValue(t, runningEphemeralRunners, lbls, 2) + assertGaugeValue(t, succeededEphemeralRunners, lbls, 3) + assertGaugeValue(t, failedEphemeralRunners, lbls, 4) + assertGaugeValue(t, outdatedEphemeralRunners, lbls, 5) + assertGaugeValue(t, deletingEphemeralRunners, lbls, 6) +} + +func assertGaugeValue(t *testing.T, gauge *prometheus.GaugeVec, labels CommonLabels, expected float64) { + t.Helper() + assert.Equal(t, expected, testutil.ToFloat64(gauge.With(labels.labels()))) +} + +func TestCommonLabelsContract(t *testing.T) { + lbls := CommonLabels{ + Name: "test", + Namespace: "ns", + Repository: "repo", + Organization: "org", + Enterprise: "ent", + } + + promLabels := lbls.labels() + + expected := prometheus.Labels{ + "name": "test", + "namespace": "ns", + "repository": "repo", + "organization": "org", + "enterprise": "ent", + } + + assert.Equal(t, expected, promLabels) +}