RunnerSet: Automatic-recovery from registration timeout and deregistr…

…ation on pod termination (#652)

Ref #629
Ref #613
Ref #612

acceptance/testdata/repo.runnerset.hra.yaml

@@ -16,6 +16,8 @@ spec:
   # RunnerSet doesn't support scale from/to zero yet
   minReplicas: 1
   maxReplicas: 5
+  # This should be less than 600(seconds, the default) for faster testing
+  scaleDownDelaySecondsAfterScaleOut: 60
   metrics:
   - type: PercentageRunnersBusy
     scaleUpThreshold: '0.75'

acceptance/testdata/repo.runnerset.yaml

@@ -14,6 +14,10 @@ spec:
   serviceName: example-runnerset
 
   #replicas: 1
+
+  # From my limited testing, `ephemeral: true` is more reliable.
+  # Seomtimes, updating already deployed runners from `ephemeral: false` to `ephemeral: true` seems to
+  # result in queued jobs hanging forever.
   ephemeral: false
 
   repository: ${TEST_REPO}

controllers/pod_runner_token_injector.go

@@ -17,6 +17,10 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
+const (
+	AnnotationKeyTokenExpirationDate = "actions-runner-controller/token-expires-at"
+)
+
 // +kubebuilder:webhook:path=/mutate-runner-set-pod,mutating=true,failurePolicy=ignore,groups="",resources=pods,verbs=create,versions=v1,name=mutate-runner-pod.webhook.actions.summerwind.dev,sideEffects=None,admissionReviewVersions=v1beta1
 
 type PodRunnerTokenInjector struct {
@@ -72,7 +76,7 @@ func (t *PodRunnerTokenInjector) Handle(ctx context.Context, req admission.Reque
 
 	updated := mutatePod(&pod, *rt.Token)
 
-	updated.Annotations["actions-runner-controller/token-expires-at"] = ts
+	updated.Annotations[AnnotationKeyTokenExpirationDate] = ts
 
 	if pod.Spec.RestartPolicy != corev1.RestartPolicyOnFailure {
 		updated.Spec.RestartPolicy = corev1.RestartPolicyOnFailure

controllers/runner_controller.go

@@ -51,6 +51,10 @@ const (
 
 	// This is an annotation internal to actions-runner-controller and can change in backward-incompatible ways
 	annotationKeyRegistrationOnly = "actions-runner-controller/registration-only"
+
+	EnvVarOrg        = "RUNNER_ORG"
+	EnvVarRepo       = "RUNNER_REPO"
+	EnvVarEnterprise = "RUNNER_ENTERPRISE"
 )
 
 // RunnerReconciler reconciles a Runner object
@@ -89,7 +93,7 @@ func (r *RunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 	}
 
 	if runner.ObjectMeta.DeletionTimestamp.IsZero() {
-		finalizers, added := addFinalizer(runner.ObjectMeta.Finalizers)
+		finalizers, added := addFinalizer(runner.ObjectMeta.Finalizers, finalizerName)
 
 		if added {
 			newRunner := runner.DeepCopy()
@@ -103,7 +107,7 @@ func (r *RunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 			return ctrl.Result{}, nil
 		}
 	} else {
-		finalizers, removed := removeFinalizer(runner.ObjectMeta.Finalizers)
+		finalizers, removed := removeFinalizer(runner.ObjectMeta.Finalizers, finalizerName)
 
 		if removed {
 			if len(runner.Status.Registration.Token) > 0 {
@@ -741,15 +745,15 @@ func newRunnerPod(template corev1.Pod, runnerSpec v1alpha1.RunnerConfig, default
 
 	env := []corev1.EnvVar{
 		{
-			Name:  "RUNNER_ORG",
+			Name:  EnvVarOrg,
 			Value: runnerSpec.Organization,
 		},
 		{
-			Name:  "RUNNER_REPO",
+			Name:  EnvVarRepo,
 			Value: runnerSpec.Repository,
 		},
 		{
-			Name:  "RUNNER_ENTERPRISE",
+			Name:  EnvVarEnterprise,
 			Value: runnerSpec.Enterprise,
 		},
 		{
@@ -1029,7 +1033,7 @@ func (r *RunnerReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-func addFinalizer(finalizers []string) ([]string, bool) {
+func addFinalizer(finalizers []string, finalizerName string) ([]string, bool) {
 	exists := false
 	for _, name := range finalizers {
 		if name == finalizerName {
@@ -1044,7 +1048,7 @@ func addFinalizer(finalizers []string) ([]string, bool) {
 	return append(finalizers, finalizerName), true
 }
 
-func removeFinalizer(finalizers []string) ([]string, bool) {
+func removeFinalizer(finalizers []string, finalizerName string) ([]string, bool) {
 	removed := false
 	result := []string{}