Use metrics to display runner statuses instead of status field for EphemeralRunnerSet and AutoscalingRunnerSet (#4557)

controllers/actions.github.com/autoscalingrunnerset_controller.go

@@ -292,18 +292,6 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl
 		}
 
 		if ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash] != desired.Annotations[annotationKeyIntegrityHash] {
-			// 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 {
-				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")
-					return ctrl.Result{}, err
-				}
-				return ctrl.Result{RequeueAfter: 1 * time.Second}, nil
-			}
-
 			original := ephemeralRunnerSet.DeepCopy()
 			ephemeralRunnerSet.Spec.EphemeralRunnerMetadata = desired.Spec.EphemeralRunnerMetadata
 			ephemeralRunnerSet.Spec.EphemeralRunnerSpec = desired.Spec.EphemeralRunnerSpec
@@ -386,7 +374,6 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl
 	if err := r.updateStatus(
 		ctx,
 		&autoscalingRunnerSet,
-		&ephemeralRunnerSet,
 		v1alpha1.AutoscalingRunnerSetPhaseRunning,
 		log,
 	); err != nil {
@@ -432,24 +419,14 @@ 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
+func (r *AutoscalingRunnerSetReconciler) updateStatus(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, phase v1alpha1.AutoscalingRunnerSetPhase, log logr.Logger) error {
 	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")

controllers/actions.github.com/autoscalingrunnerset_controller_test.go

@@ -837,111 +837,6 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() {
 		})
 	})
 
-	Context("When updating an AutoscalingRunnerSet with running or pending jobs", func() {
-		It("It should wait for running and pending jobs to finish before applying the update.", func() {
-			// Wait till the listener is created
-			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")
-
-			// Wait till the ephemeral runner set is created
-			Eventually(
-				func() (int, error) {
-					runnerSetList := new(v1alpha1.EphemeralRunnerSetList)
-					err := k8sClient.List(ctx, runnerSetList, client.InNamespace(autoscalingRunnerSet.Namespace))
-					if err != nil {
-						return 0, err
-					}
-
-					return len(runnerSetList.Items), nil
-				},
-				autoscalingRunnerSetTestTimeout,
-				autoscalingRunnerSetTestInterval,
-			).Should(BeEquivalentTo(1), "Only one EphemeralRunnerSet should be created")
-
-			runnerSetList := new(v1alpha1.EphemeralRunnerSetList)
-			err := k8sClient.List(ctx, runnerSetList, client.InNamespace(autoscalingRunnerSet.Namespace))
-			Expect(err).NotTo(HaveOccurred(), "failed to list EphemeralRunnerSet")
-
-			// 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
-
-			desiredStatus := v1alpha1.AutoscalingRunnerSetStatus{
-				CurrentRunners:          activeRunnerSet.Status.CurrentReplicas,
-				Phase:                   v1alpha1.AutoscalingRunnerSetPhaseRunning,
-				PendingEphemeralRunners: activeRunnerSet.Status.PendingEphemeralRunners,
-				RunningEphemeralRunners: activeRunnerSet.Status.RunningEphemeralRunners,
-				FailedEphemeralRunners:  activeRunnerSet.Status.FailedEphemeralRunners,
-			}
-
-			err = k8sClient.Status().Patch(ctx, activeRunnerSet, 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")
-
-			// Patch the AutoScalingRunnerSet image which should trigger
-			// the recreation of the Listener and EphemeralRunnerSet
-			patched := autoscalingRunnerSet.DeepCopy()
-			if patched.Annotations == nil {
-				patched.Annotations = make(map[string]string)
-			}
-			patched.Annotations[annotationKeyIntegrityHash] = "testgroup2"
-			patched.Spec.Template.Spec = corev1.PodSpec{
-				Containers: []corev1.Container{
-					{
-						Name:  "runner",
-						Image: "ghcr.io/actions/abcd:1.1.1",
-					},
-				},
-			}
-			err = k8sClient.Patch(ctx, patched, client.MergeFrom(autoscalingRunnerSet))
-			Expect(err).NotTo(HaveOccurred(), "failed to patch AutoScalingRunnerSet")
-			autoscalingRunnerSet = patched.DeepCopy()
-
-			// The EphemeralRunnerSet should not be recreated
-			Consistently(
-				func() (string, error) {
-					runnerSetList := new(v1alpha1.EphemeralRunnerSetList)
-					err := k8sClient.List(ctx, runnerSetList, client.InNamespace(autoscalingRunnerSet.Namespace))
-					Expect(err).NotTo(HaveOccurred(), "failed to fetch AutoScalingRunnerSet")
-					return runnerSetList.Items[0].Name, nil
-				},
-				autoscalingRunnerSetTestTimeout,
-				autoscalingRunnerSetTestInterval,
-			).Should(Equal(activeRunnerSet.Name), "The EphemeralRunnerSet should not be recreated")
-
-			// The listener should not be recreated
-			Consistently(
-				func() error {
-					return k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, listener)
-				},
-				autoscalingRunnerSetTestTimeout,
-				autoscalingRunnerSetTestInterval,
-			).ShouldNot(Succeed(), "Listener should not be recreated")
-		})
-	})
-
 	It("Should update Status on EphemeralRunnerSet status Update", func() {
 		ars := new(v1alpha1.AutoscalingRunnerSet)
 		Eventually(
@@ -978,17 +873,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))

controllers/actions.github.com/ephemeralrunner_controller.go

@@ -22,9 +22,12 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
+	"sync"
 	"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/github/actions"
 	"github.com/actions/scaleset"
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
@@ -46,11 +49,19 @@ 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
 }
 
+var ephemeralRunnerPhaseMetrics = struct {
+	sync.Mutex
+	phases map[types.NamespacedName]v1alpha1.EphemeralRunnerPhase
+}{
+	phases: map[types.NamespacedName]v1alpha1.EphemeralRunnerPhase{},
+}
+
 // precompute backoff durations for failed ephemeral runners
 // the len(failedRunnerBackoff) must be equal to maxFailures + 1
 var failedRunnerBackoff = []time.Duration{
@@ -86,6 +97,8 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 	original := ephemeralRunner.DeepCopy()
 
 	if !ephemeralRunner.DeletionTimestamp.IsZero() {
+		r.publishEphemeralRunnerPhaseMetric(&ephemeralRunner, "", log)
+
 		if !controllerutil.ContainsFinalizer(&ephemeralRunner, ephemeralRunnerFinalizerName) {
 			return ctrl.Result{}, nil
 		}
@@ -142,6 +155,8 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return ctrl.Result{}, nil
 	}
 
+	r.publishEphemeralRunnerPhaseMetric(&ephemeralRunner, ephemeralRunner.Status.Phase, log)
+
 	if ephemeralRunner.IsDone() {
 		log.Info("Cleaning up resources after after ephemeral runner termination", "phase", ephemeralRunner.Status.Phase)
 		err := r.cleanupResources(ctx, &ephemeralRunner, log)
@@ -337,6 +352,10 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 			// If the runner container exits with 0, we assume that the runner has finished successfully.
 			// If side-car container exits with non-zero, it shouldn't affect the runner. Runner exit code
 			// drives the controller's inference of whether the job has succeeded or failed.
+			if err := r.markAsSucceeded(ctx, &ephemeralRunner, pod, log); err != nil {
+				log.Error(err, "Failed to set ephemeral runner to phase Succeeded")
+				return ctrl.Result{}, err
+			}
 			if err := r.Delete(ctx, &ephemeralRunner); err != nil {
 				log.Error(err, "Failed to delete ephemeral runner after successful completion")
 				return ctrl.Result{}, err
@@ -390,6 +409,10 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 
 	default: // succeeded
 		log.Info("Ephemeral runner has finished successfully, deleting ephemeral runner", "exitCode", cs.State.Terminated.ExitCode)
+		if err := r.markAsSucceeded(ctx, &ephemeralRunner, pod, log); err != nil {
+			log.Error(err, "Failed to set ephemeral runner to phase Succeeded")
+			return ctrl.Result{}, err
+		}
 		if err := r.Delete(ctx, &ephemeralRunner); err != nil {
 			log.Error(err, "Failed to delete ephemeral runner after successful completion")
 			return ctrl.Result{}, err
@@ -583,6 +606,7 @@ func (r *EphemeralRunnerReconciler) markAsFailed(ctx context.Context, ephemeralR
 	if err := r.Status().Patch(ctx, ephemeralRunner, client.MergeFrom(original)); err != nil {
 		return fmt.Errorf("failed to update ephemeral runner status Phase/Message: %w", err)
 	}
+	r.publishEphemeralRunnerPhaseMetric(ephemeralRunner, ephemeralRunner.Status.Phase, log)
 
 	log.Info("Removing the runner from the service")
 	if err := r.deleteRunnerFromService(ctx, ephemeralRunner, log); err != nil {
@@ -604,6 +628,7 @@ func (r *EphemeralRunnerReconciler) markAsOutdated(ctx context.Context, ephemera
 	if err := r.Status().Patch(ctx, ephemeralRunner, client.MergeFrom(original)); err != nil {
 		return fmt.Errorf("failed to update ephemeral runner status Phase/Message: %w", err)
 	}
+	r.publishEphemeralRunnerPhaseMetric(ephemeralRunner, ephemeralRunner.Status.Phase, log)
 
 	log.Info("Removing the runner from the service")
 	if err := r.deleteRunnerFromService(ctx, ephemeralRunner, log); err != nil {
@@ -612,6 +637,23 @@ func (r *EphemeralRunnerReconciler) markAsOutdated(ctx context.Context, ephemera
 	return nil
 }
 
+func (r *EphemeralRunnerReconciler) markAsSucceeded(ctx context.Context, ephemeralRunner *v1alpha1.EphemeralRunner, pod *corev1.Pod, log logr.Logger) error {
+	log.Info("Updating ephemeral runner status to Succeeded")
+
+	original := ephemeralRunner.DeepCopy()
+	ephemeralRunner.Status.Phase = v1alpha1.EphemeralRunnerPhaseSucceeded
+	ephemeralRunner.Status.Ready = false
+	ephemeralRunner.Status.Reason = pod.Status.Reason
+	ephemeralRunner.Status.Message = pod.Status.Message
+	if err := r.Status().Patch(ctx, ephemeralRunner, client.MergeFrom(original)); err != nil {
+		return fmt.Errorf("failed to update ephemeral runner status Phase/Message: %w", err)
+	}
+	r.publishEphemeralRunnerPhaseMetric(ephemeralRunner, ephemeralRunner.Status.Phase, log)
+
+	log.Info("EphemeralRunner is marked as Succeeded")
+	return nil
+}
+
 // deletePodAsFailed is responsible for deleting the pod and updating the .Status.Failures for tracking failure count.
 // It should not be responsible for setting the status to Failed.
 //
@@ -835,11 +877,61 @@ func (r *EphemeralRunnerReconciler) updateRunStatusFromPod(ctx context.Context,
 	if err := r.Status().Patch(ctx, ephemeralRunner, client.MergeFrom(original)); err != nil {
 		return fmt.Errorf("failed to update runner status for Phase/Reason/Message/Ready: %w", err)
 	}
+	r.publishEphemeralRunnerPhaseMetric(ephemeralRunner, ephemeralRunner.Status.Phase, log)
 
 	log.Info("Updated ephemeral runner status")
 	return nil
 }
 
+func (r *EphemeralRunnerReconciler) publishEphemeralRunnerPhaseMetric(ephemeralRunner *v1alpha1.EphemeralRunner, phase v1alpha1.EphemeralRunnerPhase, log logr.Logger) {
+	if !r.PublishMetrics {
+		return
+	}
+
+	commonLabels, err := ephemeralRunnerMetricLabels(ephemeralRunner)
+	if err != nil {
+		log.Error(err, "Failed to build ephemeral runner metric labels")
+		return
+	}
+
+	key := types.NamespacedName{Namespace: ephemeralRunner.Namespace, Name: ephemeralRunner.Name}
+
+	ephemeralRunnerPhaseMetrics.Lock()
+	defer ephemeralRunnerPhaseMetrics.Unlock()
+
+	previousPhase, ok := ephemeralRunnerPhaseMetrics.phases[key]
+	if ok && previousPhase == phase {
+		return
+	}
+
+	if ok {
+		metrics.SubEphemeralRunner(commonLabels, previousPhase)
+	}
+
+	if phase == "" {
+		delete(ephemeralRunnerPhaseMetrics.phases, key)
+		return
+	}
+
+	metrics.AddEphemeralRunner(commonLabels, phase)
+	ephemeralRunnerPhaseMetrics.phases[key] = phase
+}
+
+func ephemeralRunnerMetricLabels(ephemeralRunner *v1alpha1.EphemeralRunner) (metrics.CommonLabels, error) {
+	parsedURL, err := actions.ParseGitHubConfigFromURL(ephemeralRunner.Spec.GitHubConfigURL)
+	if err != nil {
+		return metrics.CommonLabels{}, fmt.Errorf("github config URL is invalid: %w", err)
+	}
+
+	return metrics.CommonLabels{
+		Name:         ephemeralRunner.Labels[LabelKeyGitHubScaleSetName],
+		Namespace:    ephemeralRunner.Labels[LabelKeyGitHubScaleSetNamespace],
+		Repository:   parsedURL.Repository,
+		Organization: parsedURL.Organization,
+		Enterprise:   parsedURL.Enterprise,
+	}, nil
+}
+
 func (r *EphemeralRunnerReconciler) deleteRunnerFromService(ctx context.Context, ephemeralRunner *v1alpha1.EphemeralRunner, log logr.Logger) error {
 	client, err := r.GetActionsService(ctx, ephemeralRunner)
 	if err != nil {

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"
@@ -51,9 +49,8 @@ const (
 // EphemeralRunnerSetReconciler reconciles a EphemeralRunnerSet object
 type EphemeralRunnerSetReconciler struct {
 	client.Client
-	Log            logr.Logger
-	Scheme         *runtime.Scheme
-	PublishMetrics bool
+	Log    logr.Logger
+	Scheme *runtime.Scheme
 	ResourceBuilder
 }
 
@@ -205,29 +202,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 +246,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 +256,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.

controllers/actions.github.com/ephemeralrunnerset_controller_test.go

@@ -9,14 +9,21 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
+	"github.com/actions/actions-runner-controller/controllers/actions.github.com/metrics"
+	"github.com/actions/actions-runner-controller/github/actions"
+	"github.com/actions/scaleset"
+	prometheusdto "github.com/prometheus/client_model/go"
 	corev1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	controllerMetrics "sigs.k8s.io/controller-runtime/pkg/metrics"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -34,10 +41,92 @@ const (
 	ephemeralRunnerSetTestInterval = time.Millisecond * 250
 )
 
+var registerActionsGithubMetricsForTest sync.Once
+
 func TestPrecomputedConstants(t *testing.T) {
 	require.Equal(t, len(failedRunnerBackoff), maxFailures+1)
 }
 
+func expectEphemeralRunnerPhase(ctx context.Context, ephemeralRunner *v1alpha1.EphemeralRunner, phase v1alpha1.EphemeralRunnerPhase) {
+	updated := new(v1alpha1.EphemeralRunner)
+	err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, updated)
+	Expect(err).NotTo(HaveOccurred(), "failed to get ephemeral runner")
+	Expect(updated.Status.Phase).To(Equal(phase))
+}
+
+func expectEphemeralRunnerPhaseMetric(ephemeralRunner *v1alpha1.EphemeralRunner, phase v1alpha1.EphemeralRunnerPhase, expected float64) {
+	metricName := ephemeralRunnerPhaseMetricName(phase)
+	Expect(metricName).NotTo(BeEmpty(), "unexpected ephemeral runner phase")
+
+	parsedURL, err := actions.ParseGitHubConfigFromURL(ephemeralRunner.Spec.GitHubConfigURL)
+	Expect(err).NotTo(HaveOccurred(), "failed to parse GitHub config URL")
+
+	value, err := ephemeralRunnerPhaseMetricValue(metricName, map[string]string{
+		"name":         ephemeralRunner.Labels[LabelKeyGitHubScaleSetName],
+		"namespace":    ephemeralRunner.Labels[LabelKeyGitHubScaleSetNamespace],
+		"repository":   parsedURL.Repository,
+		"organization": parsedURL.Organization,
+		"enterprise":   parsedURL.Enterprise,
+	})
+	Expect(err).NotTo(HaveOccurred(), "failed to gather ephemeral runner phase metrics")
+	Expect(value).To(Equal(expected))
+}
+
+func ephemeralRunnerPhaseMetricName(phase v1alpha1.EphemeralRunnerPhase) string {
+	switch phase {
+	case v1alpha1.EphemeralRunnerPhasePending:
+		return "gha_controller_pending_ephemeral_runners"
+	case v1alpha1.EphemeralRunnerPhaseRunning:
+		return "gha_controller_running_ephemeral_runners"
+	case v1alpha1.EphemeralRunnerPhaseSucceeded:
+		return "gha_controller_succeeded_ephemeral_runners"
+	case v1alpha1.EphemeralRunnerPhaseFailed:
+		return "gha_controller_failed_ephemeral_runners"
+	case v1alpha1.EphemeralRunnerPhaseOutdated:
+		return "gha_controller_outdated_ephemeral_runners"
+	default:
+		return ""
+	}
+}
+
+func ephemeralRunnerPhaseMetricValue(metricName string, labels map[string]string) (float64, error) {
+	metricFamilies, err := controllerMetrics.Registry.Gather()
+	if err != nil {
+		return 0, err
+	}
+
+	for _, metricFamily := range metricFamilies {
+		if metricFamily.GetName() != metricName {
+			continue
+		}
+
+		for _, metric := range metricFamily.GetMetric() {
+			if metricHasLabels(metric, labels) {
+				if metric.GetGauge() == nil {
+					return 0, nil
+				}
+				return metric.GetGauge().GetValue(), nil
+			}
+		}
+	}
+
+	return 0, nil
+}
+
+func metricHasLabels(metric *prometheusdto.Metric, labels map[string]string) bool {
+	metricLabels := map[string]string{}
+	for _, label := range metric.GetLabel() {
+		metricLabels[label.GetName()] = label.GetValue()
+	}
+
+	for name, value := range labels {
+		if metricLabels[name] != value {
+			return false
+		}
+	}
+	return true
+}
+
 var _ = Describe("Test EphemeralRunnerSet controller", func() {
 	var ctx context.Context
 	var mgr ctrl.Manager
@@ -133,20 +222,20 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
 				ephemeralRunnerSetTestInterval,
 			).Should(BeEquivalentTo(0), "No EphemeralRunner should be created")
 
-			// Check if the status stay 0
+			// Check if the status is initialized
 			Consistently(
-				func() (int, error) {
+				func() (v1alpha1.EphemeralRunnerSetPhase, error) {
 					runnerSet := new(v1alpha1.EphemeralRunnerSet)
 					err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunnerSet.Name, Namespace: ephemeralRunnerSet.Namespace}, runnerSet)
 					if err != nil {
-						return -1, err
+						return "", err
 					}
 
-					return int(runnerSet.Status.CurrentReplicas), nil
+					return runnerSet.Status.Phase, nil
 				},
 				ephemeralRunnerSetTestTimeout,
 				ephemeralRunnerSetTestInterval,
-			).Should(BeEquivalentTo(0), "EphemeralRunnerSet status should be 0")
+			).Should(BeEquivalentTo(v1alpha1.EphemeralRunnerSetPhaseRunning), "EphemeralRunnerSet status should be running")
 
 			// Scaling up the EphemeralRunnerSet
 			updated := created.DeepCopy()
@@ -187,20 +276,20 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
 				ephemeralRunnerSetTestInterval,
 			).Should(BeEquivalentTo(5), "5 EphemeralRunner should be created")
 
-			// Check if the status is updated
+			// Check if the status stays running
 			Eventually(
-				func() (int, error) {
+				func() (v1alpha1.EphemeralRunnerSetPhase, error) {
 					runnerSet := new(v1alpha1.EphemeralRunnerSet)
 					err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunnerSet.Name, Namespace: ephemeralRunnerSet.Namespace}, runnerSet)
 					if err != nil {
-						return -1, err
+						return "", err
 					}
 
-					return int(runnerSet.Status.CurrentReplicas), nil
+					return runnerSet.Status.Phase, nil
 				},
 				ephemeralRunnerSetTestTimeout,
 				ephemeralRunnerSetTestInterval,
-			).Should(BeEquivalentTo(5), "EphemeralRunnerSet status should be 5")
+			).Should(BeEquivalentTo(v1alpha1.EphemeralRunnerSetPhaseRunning), "EphemeralRunnerSet status should be running")
 		})
 	})
 
@@ -1184,11 +1273,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 +1312,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 +1332,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) {
@@ -1273,6 +1350,148 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() {
 	})
 })
 
+var _ = Describe("EphemeralRunner phase metrics", func() {
+	var ctx context.Context
+	var autoscalingNS *corev1.Namespace
+	var mgr ctrl.Manager
+	var configSecret *corev1.Secret
+	var controller *EphemeralRunnerReconciler
+	var ephemeralRunner *v1alpha1.EphemeralRunner
+	var request ctrl.Request
+
+	BeforeEach(func() {
+		registerActionsGithubMetricsForTest.Do(func() {
+			metrics.RegisterMetrics()
+		})
+
+		ephemeralRunnerPhaseMetrics.Lock()
+		ephemeralRunnerPhaseMetrics.phases = map[types.NamespacedName]v1alpha1.EphemeralRunnerPhase{}
+		ephemeralRunnerPhaseMetrics.Unlock()
+
+		ctx = context.Background()
+		autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient)
+		configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name)
+
+		controller = &EphemeralRunnerReconciler{
+			Client:         k8sClient,
+			Scheme:         mgr.GetScheme(),
+			Log:            logf.Log,
+			PublishMetrics: true,
+			ResourceBuilder: ResourceBuilder{
+				SecretResolver: secretresolver.New(k8sClient, fake.NewMultiClient(
+					fake.WithClient(
+						fake.NewClient(
+							fake.WithGenerateJitRunnerConfig(
+								&scaleset.RunnerScaleSetJitRunnerConfig{
+									Runner:           &scaleset.RunnerReference{ID: 100, Name: "test-runner"},
+									EncodedJITConfig: "fake-jit-config",
+								},
+								nil,
+							),
+						),
+					),
+				)),
+			},
+		}
+
+		ephemeralRunner = &v1alpha1.EphemeralRunner{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test-runner",
+				Namespace: autoscalingNS.Name,
+				Labels: map[string]string{
+					LabelKeyGitHubScaleSetName:      "test-scale-set",
+					LabelKeyGitHubScaleSetNamespace: autoscalingNS.Name,
+				},
+			},
+			Spec: v1alpha1.EphemeralRunnerSpec{
+				GitHubConfigURL:    "https://github.com/owner/repo",
+				GitHubConfigSecret: configSecret.Name,
+				RunnerScaleSetID:   100,
+				PodTemplateSpec: corev1.PodTemplateSpec{
+					Spec: corev1.PodSpec{
+						Containers: []corev1.Container{
+							{
+								Name:  v1alpha1.EphemeralRunnerContainerName,
+								Image: "ghcr.io/actions/runner",
+							},
+						},
+					},
+				},
+			},
+		}
+
+		err := k8sClient.Create(ctx, ephemeralRunner)
+		Expect(err).NotTo(HaveOccurred(), "failed to create ephemeral runner")
+
+		request = ctrl.Request{NamespacedName: types.NamespacedName{Namespace: ephemeralRunner.Namespace, Name: ephemeralRunner.Name}}
+	})
+
+	It("publishes pending, running, and succeeded phase transitions", func() {
+		_, err := controller.Reconcile(ctx, request)
+		Expect(err).NotTo(HaveOccurred(), "failed to reconcile ephemeral runner")
+
+		pod := new(corev1.Pod)
+		Eventually(func() error {
+			return k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, pod)
+		}, ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval).Should(Succeed(), "expected ephemeral runner pod to be created")
+
+		podPending := pod.DeepCopy()
+		podPending.Status.Phase = corev1.PodPending
+		podPending.Status.ContainerStatuses = []corev1.ContainerStatus{
+			{
+				Name:  v1alpha1.EphemeralRunnerContainerName,
+				State: corev1.ContainerState{},
+			},
+		}
+		err = k8sClient.Status().Patch(ctx, podPending, client.MergeFrom(pod))
+		Expect(err).NotTo(HaveOccurred(), "failed to patch pod to pending")
+
+		_, err = controller.Reconcile(ctx, request)
+		Expect(err).NotTo(HaveOccurred(), "failed to reconcile pending pod")
+		expectEphemeralRunnerPhase(ctx, ephemeralRunner, v1alpha1.EphemeralRunnerPhasePending)
+		expectEphemeralRunnerPhaseMetric(ephemeralRunner, v1alpha1.EphemeralRunnerPhasePending, 1)
+		expectEphemeralRunnerPhaseMetric(ephemeralRunner, v1alpha1.EphemeralRunnerPhaseRunning, 0)
+
+		podRunning := podPending.DeepCopy()
+		podRunning.Status.Phase = corev1.PodRunning
+		podRunning.Status.ContainerStatuses = []corev1.ContainerStatus{
+			{
+				Name: v1alpha1.EphemeralRunnerContainerName,
+				State: corev1.ContainerState{
+					Running: &corev1.ContainerStateRunning{StartedAt: metav1.Now()},
+				},
+			},
+		}
+		err = k8sClient.Status().Patch(ctx, podRunning, client.MergeFrom(podPending))
+		Expect(err).NotTo(HaveOccurred(), "failed to patch pod to running")
+
+		_, err = controller.Reconcile(ctx, request)
+		Expect(err).NotTo(HaveOccurred(), "failed to reconcile running pod")
+		expectEphemeralRunnerPhase(ctx, ephemeralRunner, v1alpha1.EphemeralRunnerPhaseRunning)
+		expectEphemeralRunnerPhaseMetric(ephemeralRunner, v1alpha1.EphemeralRunnerPhasePending, 0)
+		expectEphemeralRunnerPhaseMetric(ephemeralRunner, v1alpha1.EphemeralRunnerPhaseRunning, 1)
+
+		podSucceeded := podRunning.DeepCopy()
+		podSucceeded.Status.Phase = corev1.PodSucceeded
+		podSucceeded.Status.ContainerStatuses = []corev1.ContainerStatus{
+			{
+				Name: v1alpha1.EphemeralRunnerContainerName,
+				State: corev1.ContainerState{
+					Terminated: &corev1.ContainerStateTerminated{ExitCode: 0},
+				},
+			},
+		}
+		err = k8sClient.Status().Patch(ctx, podSucceeded, client.MergeFrom(podRunning))
+		Expect(err).NotTo(HaveOccurred(), "failed to patch pod to succeeded")
+
+		_, err = controller.Reconcile(ctx, request)
+		Expect(err).NotTo(HaveOccurred(), "failed to reconcile succeeded pod")
+		expectEphemeralRunnerPhase(ctx, ephemeralRunner, v1alpha1.EphemeralRunnerPhaseSucceeded)
+		expectEphemeralRunnerPhaseMetric(ephemeralRunner, v1alpha1.EphemeralRunnerPhaseRunning, 0)
+		expectEphemeralRunnerPhaseMetric(ephemeralRunner, v1alpha1.EphemeralRunnerPhaseSucceeded, 1)
+	})
+})
+
 var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func() {
 	var ctx context.Context
 	var mgr ctrl.Manager

controllers/actions.github.com/metrics/metrics.go

@@ -1,6 +1,7 @@
 package metrics
 
 import (
+	"github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1"
 	"github.com/prometheus/client_golang/prometheus"
 	"sigs.k8s.io/controller-runtime/pkg/metrics"
 )
@@ -50,6 +51,14 @@ var (
 		},
 		labels,
 	)
+	succeededEphemeralRunners = prometheus.NewGaugeVec(
+		prometheus.GaugeOpts{
+			Subsystem: githubScaleSetControllerSubsystem,
+			Name:      "succeeded_ephemeral_runners",
+			Help:      "Number of ephemeral runners in a succeeded state.",
+		},
+		labels,
+	)
 	failedEphemeralRunners = prometheus.NewGaugeVec(
 		prometheus.GaugeOpts{
 			Subsystem: githubScaleSetControllerSubsystem,
@@ -58,6 +67,14 @@ var (
 		},
 		labels,
 	)
+	outdatedEphemeralRunners = prometheus.NewGaugeVec(
+		prometheus.GaugeOpts{
+			Subsystem: githubScaleSetControllerSubsystem,
+			Name:      "outdated_ephemeral_runners",
+			Help:      "Number of ephemeral runners in an outdated state.",
+		},
+		labels,
+	)
 	runningListeners = prometheus.NewGaugeVec(
 		prometheus.GaugeOpts{
 			Subsystem: githubScaleSetControllerSubsystem,
@@ -72,15 +89,34 @@ func RegisterMetrics() {
 	metrics.Registry.MustRegister(
 		pendingEphemeralRunners,
 		runningEphemeralRunners,
+		succeededEphemeralRunners,
 		failedEphemeralRunners,
+		outdatedEphemeralRunners,
 		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))
+func AddEphemeralRunner(commonLabels CommonLabels, phase v1alpha1.EphemeralRunnerPhase) {
+	updateEphemeralRunner(commonLabels, phase, 1)
+}
+
+func SubEphemeralRunner(commonLabels CommonLabels, phase v1alpha1.EphemeralRunnerPhase) {
+	updateEphemeralRunner(commonLabels, phase, -1)
+}
+
+func updateEphemeralRunner(commonLabels CommonLabels, phase v1alpha1.EphemeralRunnerPhase, delta float64) {
+	switch phase {
+	case v1alpha1.EphemeralRunnerPhasePending:
+		pendingEphemeralRunners.With(commonLabels.labels()).Add(delta)
+	case v1alpha1.EphemeralRunnerPhaseRunning:
+		runningEphemeralRunners.With(commonLabels.labels()).Add(delta)
+	case v1alpha1.EphemeralRunnerPhaseSucceeded:
+		succeededEphemeralRunners.With(commonLabels.labels()).Add(delta)
+	case v1alpha1.EphemeralRunnerPhaseFailed:
+		failedEphemeralRunners.With(commonLabels.labels()).Add(delta)
+	case v1alpha1.EphemeralRunnerPhaseOutdated:
+		outdatedEphemeralRunners.With(commonLabels.labels()).Add(delta)
+	}
 }
 
 func AddRunningListener(commonLabels CommonLabels) {