Let the listener own the EphemeralRunner Running phase transition

apis/actions.github.com/v1alpha1/ephemeralrunner_types.go

@@ -146,6 +146,10 @@ type EphemeralRunnerStatus struct {
 	//
 	// The PodSucceded phase should be set only when confirmed that EphemeralRunner
 	// actually executed the job and has been removed from the service.
+	//
+	// The Running phase is owned by the listener and is set only when a job has
+	// been assigned to this EphemeralRunner. It does not mean the runner is merely
+	// online and waiting for work; an idle registered runner stays Pending.
 	// +optional
 	Phase EphemeralRunnerPhase `json:"phase,omitempty"`
 	// +optional
@@ -185,11 +189,12 @@ type EphemeralRunnerStatus struct {
 type EphemeralRunnerPhase string
 
 const (
-	// EphemeralRunnerPhasePending is a phase set when the ephemeral runner is
-	// being provisioned and is not yet online.
+	// EphemeralRunnerPhasePending is a phase set while no job has been assigned to
+	// the ephemeral runner. It covers both a runner that is still being provisioned
+	// and one that is already online and registered but idle.
 	EphemeralRunnerPhasePending EphemeralRunnerPhase = "Pending"
-	// EphemeralRunnerPhaseRunning is a phase set when the ephemeral runner is online and
-	// waiting for a job to execute.
+	// EphemeralRunnerPhaseRunning is a phase set by the listener when a job has been
+	// assigned to this ephemeral runner and the runner is executing it.
 	EphemeralRunnerPhaseRunning EphemeralRunnerPhase = "Running"
 	// EphemeralRunnerPhaseSucceeded is a phase set when the ephemeral runner
 	// successfully executed the job and has been removed from the service.

cmd/ghalistener/scaler/scaler.go

@@ -15,6 +15,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
+	"k8s.io/client-go/util/retry"
 )
 
 type Option func(*Scaler)
@@ -123,6 +124,7 @@ func (w *Scaler) applyDefaults() error {
 // It takes a context and a jobInfo parameter which contains the details of the started job.
 // This update marks the ephemeral runner so that the controller would have more context
 // about the ephemeral runner that should not be deleted when scaling down.
+// It also transitions the phase to Running if the runner is not in a terminal state.
 // It returns an error if there is any issue with updating the job information.
 func (w *Scaler) HandleJobStarted(ctx context.Context, jobInfo *scaleset.JobStarted) error {
 	w.logger.Info("Updating job info for the runner",
@@ -137,23 +139,61 @@ func (w *Scaler) HandleJobStarted(ctx context.Context, jobInfo *scaleset.JobStar
 
 	w.dirty = true
 
+	// The promotion to Running is guarded by an optimistic lock on the resource version
+	// observed by the GET below, so a terminal phase written between the read and the
+	// patch is never clobbered. Conflicts are retried against freshly read state.
+	return retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		return w.patchJobStarted(ctx, jobInfo)
+	})
+}
+
+func (w *Scaler) patchJobStarted(ctx context.Context, jobInfo *scaleset.JobStarted) error {
+	// Fetch current EphemeralRunner to check phase and deletion status
+	currentRunner := &v1alpha1.EphemeralRunner{}
+	err := w.clientset.RESTClient().
+		Get().
+		Prefix("apis", v1alpha1.GroupVersion.Group, v1alpha1.GroupVersion.Version).
+		Namespace(w.config.EphemeralRunnerSetNamespace).
+		Resource("ephemeralrunners").
+		Name(jobInfo.RunnerName).
+		Do(ctx).
+		Into(currentRunner)
+	if err != nil {
+		if kerrors.IsNotFound(err) {
+			w.logger.Info("Ephemeral runner not found, skipping job info update", "runnerName", jobInfo.RunnerName)
+			return nil
+		}
+		return fmt.Errorf("failed to get ephemeral runner: %w", err)
+	}
+
 	original, err := json.Marshal(&v1alpha1.EphemeralRunner{})
 	if err != nil {
 		return fmt.Errorf("failed to marshal empty ephemeral runner: %w", err)
 	}
 
-	patch, err := json.Marshal(
-		&v1alpha1.EphemeralRunner{
-			Status: v1alpha1.EphemeralRunnerStatus{
-				JobRequestID:      jobInfo.RunnerRequestID,
-				JobRepositoryName: fmt.Sprintf("%s/%s", jobInfo.OwnerName, jobInfo.RepositoryName),
-				JobID:             jobInfo.JobID,
-				WorkflowRunID:     jobInfo.WorkflowRunID,
-				JobWorkflowRef:    jobInfo.JobWorkflowRef,
-				JobDisplayName:    jobInfo.JobDisplayName,
-			},
+	// Build patch with job fields
+	patchRunner := &v1alpha1.EphemeralRunner{
+		Status: v1alpha1.EphemeralRunnerStatus{
+			JobRequestID:      jobInfo.RunnerRequestID,
+			JobRepositoryName: fmt.Sprintf("%s/%s", jobInfo.OwnerName, jobInfo.RepositoryName),
+			JobID:             jobInfo.JobID,
+			WorkflowRunID:     jobInfo.WorkflowRunID,
+			JobWorkflowRef:    jobInfo.JobWorkflowRef,
+			JobDisplayName:    jobInfo.JobDisplayName,
 		},
-	)
+	}
+
+	// Only set Running phase if current phase is not terminal/failure and deletion is not in progress
+	if currentRunner.DeletionTimestamp == nil &&
+		currentRunner.Status.Phase != v1alpha1.EphemeralRunnerPhaseFailed &&
+		currentRunner.Status.Phase != v1alpha1.EphemeralRunnerPhaseSucceeded &&
+		currentRunner.Status.Phase != v1alpha1.EphemeralRunnerPhaseOutdated {
+		patchRunner.Status.Phase = v1alpha1.EphemeralRunnerPhaseRunning
+		// Optimistic lock: reject the promotion if the runner changed since the GET.
+		patchRunner.ResourceVersion = currentRunner.ResourceVersion
+	}
+
+	patch, err := json.Marshal(patchRunner)
 	if err != nil {
 		return fmt.Errorf("failed to marshal ephemeral runner patch: %w", err)
 	}
@@ -170,7 +210,7 @@ func (w *Scaler) HandleJobStarted(ctx context.Context, jobInfo *scaleset.JobStar
 		Patch(types.MergePatchType).
 		Prefix("apis", v1alpha1.GroupVersion.Group, v1alpha1.GroupVersion.Version).
 		Namespace(w.config.EphemeralRunnerSetNamespace).
-		Resource("EphemeralRunners").
+		Resource("ephemeralrunners").
 		Name(jobInfo.RunnerName).
 		SubResource("status").
 		Body(mergePatch).
@@ -181,6 +221,10 @@ func (w *Scaler) HandleJobStarted(ctx context.Context, jobInfo *scaleset.JobStar
 			w.logger.Info("Ephemeral runner not found, skipping patching of ephemeral runner status", "runnerName", jobInfo.RunnerName)
 			return nil
 		}
+		if kerrors.IsConflict(err) {
+			w.logger.Info("Ephemeral runner changed while patching job info, retrying", "runnerName", jobInfo.RunnerName)
+			return err
+		}
 		return fmt.Errorf("could not patch ephemeral runner status, patch JSON: %s, error: %w", string(mergePatch), err)
 	}