From e863513a33fdc080eb0a3456c5a791d7c2bfb5dc Mon Sep 17 00:00:00 2001
From: Yusuke Kuoka <ykuoka@gmail.com>
Date: Thu, 11 Mar 2021 20:01:56 +0900
Subject: [PATCH] Fix PercentageRunnersBusy scaling not working

PercentageRunnerBusy seems to have regressed since #355 due to that RunnerDeployment.Spec.Selector is empty by default and the HRA controller was using that empty selector to query runners, which somehow returned 0 runners. This fixes that by using the newly added automatic `runner-deployment-name` label for the default runner label and the selector, which avoids querying with empty selector.

Ref https://github.com/summerwind/actions-runner-controller/issues/377#issuecomment-795200205
---
 controllers/autoscaling.go                 | 19 +++++++++----
 controllers/runnerdeployment_controller.go | 32 ++++++++++++++--------
 2 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/controllers/autoscaling.go b/controllers/autoscaling.go
index 8e25c4a7ea..e7dcfbe7fb 100644
--- a/controllers/autoscaling.go
+++ b/controllers/autoscaling.go
@@ -266,17 +266,26 @@ func (r *HorizontalRunnerAutoscalerReconciler) calculateReplicasByPercentageRunn
 		scaleDownFactor = sdf
 	}
 
-	selector, err := metav1.LabelSelectorAsSelector(rd.Spec.Selector)
+	// return the list of runners in namespace. Horizontal Runner Autoscaler should only be responsible for scaling resources in its own ns.
+	var runnerList v1alpha1.RunnerList
+
+	var opts []client.ListOption
+
+	opts = append(opts, client.InNamespace(rd.Namespace))
+
+	selector, err := metav1.LabelSelectorAsSelector(getSelector(&rd))
 	if err != nil {
 		return nil, err
 	}
-	// return the list of runners in namespace. Horizontal Runner Autoscaler should only be responsible for scaling resources in its own ns.
-	var runnerList v1alpha1.RunnerList
+
+	opts = append(opts, client.MatchingLabelsSelector{Selector: selector})
+
+	r.Log.V(2).Info("Finding runners with selector", "ns", rd.Namespace)
+
 	if err := r.List(
 		ctx,
 		&runnerList,
-		client.InNamespace(rd.Namespace),
-		client.MatchingLabelsSelector{Selector: selector},
+		opts...,
 	); err != nil {
 		if !kerrors.IsNotFound(err) {
 			return nil, err
diff --git a/controllers/runnerdeployment_controller.go b/controllers/runnerdeployment_controller.go
index da99622b46..6d4780e6d1 100644
--- a/controllers/runnerdeployment_controller.go
+++ b/controllers/runnerdeployment_controller.go
@@ -41,7 +41,8 @@ import (
 )
 
 const (
-	LabelKeyRunnerTemplateHash = "runner-template-hash"
+	LabelKeyRunnerTemplateHash   = "runner-template-hash"
+	LabelKeyRunnerDeploymentName = "runner-deployment-name"
 
 	runnerSetOwnerKey = ".metadata.controller"
 )
@@ -326,23 +327,32 @@ func (r *RunnerDeploymentReconciler) newRunnerReplicaSet(rd v1alpha1.RunnerDeplo
 	return newRunnerReplicaSet(&rd, r.CommonRunnerLabels, r.Scheme)
 }
 
+func getSelector(rd *v1alpha1.RunnerDeployment) *metav1.LabelSelector {
+	selector := rd.Spec.Selector
+	if selector == nil {
+		selector = &metav1.LabelSelector{MatchLabels: map[string]string{LabelKeyRunnerDeploymentName: rd.Name}}
+	}
+
+	return selector
+}
+
 func newRunnerReplicaSet(rd *v1alpha1.RunnerDeployment, commonRunnerLabels []string, scheme *runtime.Scheme) (*v1alpha1.RunnerReplicaSet, error) {
 	newRSTemplate := *rd.Spec.Template.DeepCopy()
 
-	templateHash := ComputeHash(&newRSTemplate)
-	// Add template hash label to selector.
-	labels := CloneAndAddLabel(rd.Spec.Template.Labels, LabelKeyRunnerTemplateHash, templateHash)
-
 	for _, l := range commonRunnerLabels {
 		newRSTemplate.Spec.Labels = append(newRSTemplate.Spec.Labels, l)
 	}
 
-	newRSTemplate.Labels = labels
+	templateHash := ComputeHash(&newRSTemplate)
+
+	// Add template hash label to selector.
+	newRSTemplate.ObjectMeta.Labels = CloneAndAddLabel(newRSTemplate.ObjectMeta.Labels, LabelKeyRunnerTemplateHash, templateHash)
+
+	// This label selector is used by default when rd.Spec.Selector is empty.
+	newRSTemplate.ObjectMeta.Labels = CloneAndAddLabel(newRSTemplate.ObjectMeta.Labels, LabelKeyRunnerDeploymentName, rd.Name)
+
+	selector := getSelector(rd)
 
-	selector := rd.Spec.Selector
-	if selector == nil {
-		selector = &metav1.LabelSelector{MatchLabels: labels}
-	}
 	newRSSelector := CloneSelectorAndAddLabel(selector, LabelKeyRunnerTemplateHash, templateHash)
 
 	rs := v1alpha1.RunnerReplicaSet{
@@ -350,7 +360,7 @@ func newRunnerReplicaSet(rd *v1alpha1.RunnerDeployment, commonRunnerLabels []str
 		ObjectMeta: metav1.ObjectMeta{
 			GenerateName: rd.ObjectMeta.Name + "-",
 			Namespace:    rd.ObjectMeta.Namespace,
-			Labels:       labels,
+			Labels:       newRSTemplate.ObjectMeta.Labels,
 		},
 		Spec: v1alpha1.RunnerReplicaSetSpec{
 			Replicas: rd.Spec.Replicas,