Code style changes on the controller (#4324)

controllers/actions.github.com/autoscalinglistener_controller_test.go

@@ -26,9 +26,8 @@ import (
 )
 
 const (
-	autoscalingListenerTestTimeout     = time.Second * 20
-	autoscalingListenerTestInterval    = time.Millisecond * 250
-	autoscalingListenerTestGitHubToken = "gh_token"
+	autoscalingListenerTestTimeout  = time.Second * 20
+	autoscalingListenerTestInterval = time.Millisecond * 250
 )
 
 var _ = Describe("Test AutoScalingListener controller", func() {

controllers/actions.github.com/autoscalingrunnerset_controller.go

@@ -48,7 +48,7 @@ const (
 	annotationKeyValuesHash = "actions.github.com/values-hash"
 
 	autoscalingRunnerSetFinalizerName = "autoscalingrunnerset.actions.github.com/finalizer"
-	runnerScaleSetIdAnnotationKey     = "runner-scale-set-id"
+	runnerScaleSetIDAnnotationKey     = "runner-scale-set-id"
 )
 
 type UpdateStrategy string
@@ -180,14 +180,14 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl
 		return ctrl.Result{}, nil
 	}
 
-	scaleSetIdRaw, ok := autoscalingRunnerSet.Annotations[runnerScaleSetIdAnnotationKey]
+	scaleSetIDRaw, ok := autoscalingRunnerSet.Annotations[runnerScaleSetIDAnnotationKey]
 	if !ok {
 		// Need to create a new runner scale set on Actions service
 		log.Info("Runner scale set id annotation does not exist. Creating a new runner scale set.")
 		return r.createRunnerScaleSet(ctx, autoscalingRunnerSet, log)
 	}
 
-	if id, err := strconv.Atoi(scaleSetIdRaw); err != nil || id <= 0 {
+	if id, err := strconv.Atoi(scaleSetIDRaw); err != nil || id <= 0 {
 		log.Info("Runner scale set id annotation is not an id, or is <= 0. Creating a new runner scale set.")
 		// something modified the scaleSetId. Try to create one
 		return r.createRunnerScaleSet(ctx, autoscalingRunnerSet, log)
@@ -403,7 +403,7 @@ func (r *AutoscalingRunnerSetReconciler) createRunnerScaleSet(ctx context.Contex
 		return ctrl.Result{}, err
 	}
 
-	runnerGroupId := 1
+	runnerGroupID := 1
 	if len(autoscalingRunnerSet.Spec.RunnerGroup) > 0 {
 		runnerGroup, err := actionsClient.GetRunnerGroupByName(ctx, autoscalingRunnerSet.Spec.RunnerGroup)
 		if err != nil {
@@ -411,14 +411,14 @@ func (r *AutoscalingRunnerSetReconciler) createRunnerScaleSet(ctx context.Contex
 			return ctrl.Result{}, err
 		}
 
-		runnerGroupId = int(runnerGroup.ID)
+		runnerGroupID = int(runnerGroup.ID)
 	}
 
-	runnerScaleSet, err := actionsClient.GetRunnerScaleSet(ctx, runnerGroupId, autoscalingRunnerSet.Spec.RunnerScaleSetName)
+	runnerScaleSet, err := actionsClient.GetRunnerScaleSet(ctx, runnerGroupID, autoscalingRunnerSet.Spec.RunnerScaleSetName)
 	if err != nil {
 		logger.Error(err, "Failed to get runner scale set from Actions service",
 			"runnerGroupId",
-			strconv.Itoa(runnerGroupId),
+			strconv.Itoa(runnerGroupID),
 			"runnerScaleSetName",
 			autoscalingRunnerSet.Spec.RunnerScaleSetName)
 		return ctrl.Result{}, err
@@ -429,7 +429,7 @@ func (r *AutoscalingRunnerSetReconciler) createRunnerScaleSet(ctx context.Contex
 			ctx,
 			&actions.RunnerScaleSet{
 				Name:          autoscalingRunnerSet.Spec.RunnerScaleSetName,
-				RunnerGroupId: runnerGroupId,
+				RunnerGroupId: runnerGroupID,
 				Labels: []actions.Label{
 					{
 						Name: autoscalingRunnerSet.Spec.RunnerScaleSetName,
@@ -466,7 +466,7 @@ func (r *AutoscalingRunnerSetReconciler) createRunnerScaleSet(ctx context.Contex
 	logger.Info("Adding runner scale set ID, name and runner group name as an annotation and url labels")
 	if err = patch(ctx, r.Client, autoscalingRunnerSet, func(obj *v1alpha1.AutoscalingRunnerSet) {
 		obj.Annotations[AnnotationKeyGitHubRunnerScaleSetName] = runnerScaleSet.Name
-		obj.Annotations[runnerScaleSetIdAnnotationKey] = strconv.Itoa(runnerScaleSet.Id)
+		obj.Annotations[runnerScaleSetIDAnnotationKey] = strconv.Itoa(runnerScaleSet.Id)
 		obj.Annotations[AnnotationKeyGitHubRunnerGroupName] = runnerScaleSet.RunnerGroupName
 		if err := applyGitHubURLLabels(obj.Spec.GitHubConfigUrl, obj.Labels); err != nil { // should never happen
 			logger.Error(err, "Failed to apply GitHub URL labels")
@@ -484,7 +484,7 @@ func (r *AutoscalingRunnerSetReconciler) createRunnerScaleSet(ctx context.Contex
 }
 
 func (r *AutoscalingRunnerSetReconciler) updateRunnerScaleSetRunnerGroup(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, logger logr.Logger) (ctrl.Result, error) {
-	runnerScaleSetId, err := strconv.Atoi(autoscalingRunnerSet.Annotations[runnerScaleSetIdAnnotationKey])
+	runnerScaleSetID, err := strconv.Atoi(autoscalingRunnerSet.Annotations[runnerScaleSetIDAnnotationKey])
 	if err != nil {
 		logger.Error(err, "Failed to parse runner scale set ID")
 		return ctrl.Result{}, err
@@ -496,7 +496,7 @@ func (r *AutoscalingRunnerSetReconciler) updateRunnerScaleSetRunnerGroup(ctx con
 		return ctrl.Result{}, err
 	}
 
-	runnerGroupId := 1
+	runnerGroupID := 1
 	if len(autoscalingRunnerSet.Spec.RunnerGroup) > 0 {
 		runnerGroup, err := actionsClient.GetRunnerGroupByName(ctx, autoscalingRunnerSet.Spec.RunnerGroup)
 		if err != nil {
@@ -504,12 +504,12 @@ func (r *AutoscalingRunnerSetReconciler) updateRunnerScaleSetRunnerGroup(ctx con
 			return ctrl.Result{}, err
 		}
 
-		runnerGroupId = int(runnerGroup.ID)
+		runnerGroupID = int(runnerGroup.ID)
 	}
 
-	updatedRunnerScaleSet, err := actionsClient.UpdateRunnerScaleSet(ctx, runnerScaleSetId, &actions.RunnerScaleSet{RunnerGroupId: runnerGroupId})
+	updatedRunnerScaleSet, err := actionsClient.UpdateRunnerScaleSet(ctx, runnerScaleSetID, &actions.RunnerScaleSet{RunnerGroupId: runnerGroupID})
 	if err != nil {
-		logger.Error(err, "Failed to update runner scale set", "runnerScaleSetId", runnerScaleSetId)
+		logger.Error(err, "Failed to update runner scale set", "runnerScaleSetId", runnerScaleSetID)
 		return ctrl.Result{}, err
 	}
 
@@ -527,7 +527,7 @@ func (r *AutoscalingRunnerSetReconciler) updateRunnerScaleSetRunnerGroup(ctx con
 }
 
 func (r *AutoscalingRunnerSetReconciler) updateRunnerScaleSetName(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, logger logr.Logger) (ctrl.Result, error) {
-	runnerScaleSetId, err := strconv.Atoi(autoscalingRunnerSet.Annotations[runnerScaleSetIdAnnotationKey])
+	runnerScaleSetID, err := strconv.Atoi(autoscalingRunnerSet.Annotations[runnerScaleSetIDAnnotationKey])
 	if err != nil {
 		logger.Error(err, "Failed to parse runner scale set ID")
 		return ctrl.Result{}, err
@@ -544,9 +544,9 @@ func (r *AutoscalingRunnerSetReconciler) updateRunnerScaleSetName(ctx context.Co
 		return ctrl.Result{}, err
 	}
 
-	updatedRunnerScaleSet, err := actionsClient.UpdateRunnerScaleSet(ctx, runnerScaleSetId, &actions.RunnerScaleSet{Name: autoscalingRunnerSet.Spec.RunnerScaleSetName})
+	updatedRunnerScaleSet, err := actionsClient.UpdateRunnerScaleSet(ctx, runnerScaleSetID, &actions.RunnerScaleSet{Name: autoscalingRunnerSet.Spec.RunnerScaleSetName})
 	if err != nil {
-		logger.Error(err, "Failed to update runner scale set", "runnerScaleSetId", runnerScaleSetId)
+		logger.Error(err, "Failed to update runner scale set", "runnerScaleSetId", runnerScaleSetID)
 		return ctrl.Result{}, err
 	}
 
@@ -563,7 +563,7 @@ func (r *AutoscalingRunnerSetReconciler) updateRunnerScaleSetName(ctx context.Co
 }
 
 func (r *AutoscalingRunnerSetReconciler) deleteRunnerScaleSet(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, logger logr.Logger) error {
-	scaleSetId, ok := autoscalingRunnerSet.Annotations[runnerScaleSetIdAnnotationKey]
+	scaleSetID, ok := autoscalingRunnerSet.Annotations[runnerScaleSetIDAnnotationKey]
 	if !ok {
 		// Annotation not being present can occur in 3 scenarios
 		// 1. Scale set is never created.
@@ -580,7 +580,7 @@ func (r *AutoscalingRunnerSetReconciler) deleteRunnerScaleSet(ctx context.Contex
 		return nil
 	}
 	logger.Info("Deleting the runner scale set from Actions service")
-	runnerScaleSetId, err := strconv.Atoi(scaleSetId)
+	runnerScaleSetID, err := strconv.Atoi(scaleSetID)
 	if err != nil {
 		// If the annotation is not set correctly, we are going to get stuck in a loop trying to parse the scale set id.
 		// If the configuration is invalid (secret does not exist for example), we never got to the point to create runner set.
@@ -595,17 +595,17 @@ func (r *AutoscalingRunnerSetReconciler) deleteRunnerScaleSet(ctx context.Contex
 		return err
 	}
 
-	err = actionsClient.DeleteRunnerScaleSet(ctx, runnerScaleSetId)
+	err = actionsClient.DeleteRunnerScaleSet(ctx, runnerScaleSetID)
 	if err != nil {
-		logger.Error(err, "Failed to delete runner scale set", "runnerScaleSetId", runnerScaleSetId)
+		logger.Error(err, "Failed to delete runner scale set", "runnerScaleSetId", runnerScaleSetID)
 		return err
 	}
 
 	err = patch(ctx, r.Client, autoscalingRunnerSet, func(obj *v1alpha1.AutoscalingRunnerSet) {
-		delete(obj.Annotations, runnerScaleSetIdAnnotationKey)
+		delete(obj.Annotations, runnerScaleSetIDAnnotationKey)
 	})
 	if err != nil {
-		logger.Error(err, "Failed to patch autoscaling runner set with annotation removed", "annotation", runnerScaleSetIdAnnotationKey)
+		logger.Error(err, "Failed to patch autoscaling runner set with annotation removed", "annotation", runnerScaleSetIDAnnotationKey)
 		return err
 	}
 
@@ -1006,6 +1006,7 @@ func (c *autoscalingRunnerSetFinalizerDependencyCleaner) removeManagerRoleFinali
 
 // NOTE: if this is logic should be used for other resources,
 // consider using generics
+
 type EphemeralRunnerSets struct {
 	list   *v1alpha1.EphemeralRunnerSetList
 	sorted bool

controllers/actions.github.com/autoscalingrunnerset_controller_test.go

@@ -34,9 +34,8 @@ import (
 )
 
 const (
-	autoscalingRunnerSetTestTimeout     = time.Second * 20
-	autoscalingRunnerSetTestInterval    = time.Millisecond * 250
-	autoscalingRunnerSetTestGitHubToken = "gh_token"
+	autoscalingRunnerSetTestTimeout  = time.Second * 20
+	autoscalingRunnerSetTestInterval = time.Millisecond * 250
 )
 
 var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() {
@@ -141,7 +140,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() {
 						return "", err
 					}
 
-					if _, ok := created.Annotations[runnerScaleSetIdAnnotationKey]; !ok {
+					if _, ok := created.Annotations[runnerScaleSetIDAnnotationKey]; !ok {
 						return "", nil
 					}
 
@@ -149,7 +148,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() {
 						return "", nil
 					}
 
-					return fmt.Sprintf("%s_%s", created.Annotations[runnerScaleSetIdAnnotationKey], created.Annotations[AnnotationKeyGitHubRunnerGroupName]), nil
+					return fmt.Sprintf("%s_%s", created.Annotations[runnerScaleSetIDAnnotationKey], created.Annotations[AnnotationKeyGitHubRunnerGroupName]), nil
 				},
 				autoscalingRunnerSetTestTimeout,
 				autoscalingRunnerSetTestInterval).Should(BeEquivalentTo("1_testgroup"), "RunnerScaleSet should be created/fetched and update the AutoScalingRunnerSet's annotation")

controllers/actions.github.com/constants.go

@@ -36,7 +36,8 @@ const (
 	LabelKeyGitHubRepository        = "actions.github.com/repository"
 )
 
-// Finalizer used to protect resources from deletion while AutoscalingRunnerSet is running
+// AutoscalingRunnerSetCleanupFinalizerName is a finalizer used to protect resources
+// from deletion while AutoscalingRunnerSet is running
 const AutoscalingRunnerSetCleanupFinalizerName = "actions.github.com/cleanup-protection"
 
 const (

controllers/actions.github.com/ephemeralrunner_controller.go

@@ -689,7 +689,7 @@ func (r *EphemeralRunnerReconciler) createPod(ctx context.Context, runner *v1alp
 	}
 
 	log.Info("Creating new pod for ephemeral runner")
-	newPod := r.newEphemeralRunnerPod(ctx, runner, secret, envs...)
+	newPod := r.newEphemeralRunnerPod(runner, secret, envs...)
 
 	if err := ctrl.SetControllerReference(runner, newPod, r.Scheme); err != nil {
 		log.Error(err, "Failed to set controller reference to a new pod")

controllers/actions.github.com/helpers_test.go

@@ -42,11 +42,11 @@ func createNamespace(t ginkgo.GinkgoTInterface, client client.Client) (*corev1.N
 		ObjectMeta: metav1.ObjectMeta{Name: "testns-autoscaling" + RandStringRunes(5)},
 	}
 
-	err := k8sClient.Create(context.Background(), ns)
+	err := client.Create(context.Background(), ns)
 	require.NoError(t, err)
 
 	t.Cleanup(func() {
-		err := k8sClient.Delete(context.Background(), ns)
+		err := client.Delete(context.Background(), ns)
 		require.NoError(t, err)
 	})
 

controllers/actions.github.com/resourcebuilder.go

@@ -2,7 +2,6 @@ package actionsgithubcom
 
 import (
 	"bytes"
-	"context"
 	"encoding/json"
 	"fmt"
 	"maps"
@@ -83,7 +82,7 @@ func boolPtr(v bool) *bool {
 }
 
 func (b *ResourceBuilder) newAutoScalingListener(autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet, namespace, image string, imagePullSecrets []corev1.LocalObjectReference) (*v1alpha1.AutoscalingListener, error) {
-	runnerScaleSetId, err := strconv.Atoi(autoscalingRunnerSet.Annotations[runnerScaleSetIdAnnotationKey])
+	runnerScaleSetID, err := strconv.Atoi(autoscalingRunnerSet.Annotations[runnerScaleSetIDAnnotationKey])
 	if err != nil {
 		return nil, err
 	}
@@ -125,7 +124,7 @@ func (b *ResourceBuilder) newAutoScalingListener(autoscalingRunnerSet *v1alpha1.
 			GitHubConfigUrl:               autoscalingRunnerSet.Spec.GitHubConfigUrl,
 			GitHubConfigSecret:            autoscalingRunnerSet.Spec.GitHubConfigSecret,
 			VaultConfig:                   autoscalingRunnerSet.VaultConfig(),
-			RunnerScaleSetId:              runnerScaleSetId,
+			RunnerScaleSetId:              runnerScaleSetID,
 			AutoscalingRunnerSetNamespace: autoscalingRunnerSet.Namespace,
 			AutoscalingRunnerSetName:      autoscalingRunnerSet.Name,
 			EphemeralRunnerSetName:        ephemeralRunnerSet.Name,
@@ -496,7 +495,7 @@ func (b *ResourceBuilder) newScaleSetListenerRoleBinding(autoscalingListener *v1
 }
 
 func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet) (*v1alpha1.EphemeralRunnerSet, error) {
-	runnerScaleSetId, err := strconv.Atoi(autoscalingRunnerSet.Annotations[runnerScaleSetIdAnnotationKey])
+	runnerScaleSetID, err := strconv.Atoi(autoscalingRunnerSet.Annotations[runnerScaleSetIDAnnotationKey])
 	if err != nil {
 		return nil, err
 	}
@@ -541,7 +540,7 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A
 		Spec: v1alpha1.EphemeralRunnerSetSpec{
 			Replicas: 0,
 			EphemeralRunnerSpec: v1alpha1.EphemeralRunnerSpec{
-				RunnerScaleSetId:   runnerScaleSetId,
+				RunnerScaleSetId:   runnerScaleSetID,
 				GitHubConfigUrl:    autoscalingRunnerSet.Spec.GitHubConfigUrl,
 				GitHubConfigSecret: autoscalingRunnerSet.Spec.GitHubConfigSecret,
 				Proxy:              autoscalingRunnerSet.Spec.Proxy,
@@ -556,20 +555,12 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A
 }
 
 func (b *ResourceBuilder) newEphemeralRunner(ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet) *v1alpha1.EphemeralRunner {
-	labels := make(map[string]string)
-	for k, v := range ephemeralRunnerSet.Labels {
-		if k == LabelKeyKubernetesComponent {
-			labels[k] = "runner"
-		} else {
-			labels[k] = v
-		}
-	}
-
-	annotations := make(map[string]string)
-	for key, val := range ephemeralRunnerSet.Annotations {
-		annotations[key] = val
-	}
+	labels := make(map[string]string, len(ephemeralRunnerSet.Labels))
+	maps.Copy(labels, ephemeralRunnerSet.Labels)
+	labels[LabelKeyKubernetesComponent] = "runner"
 
+	annotations := make(map[string]string, len(ephemeralRunnerSet.Annotations)+1)
+	maps.Copy(annotations, ephemeralRunnerSet.Annotations)
 	annotations[AnnotationKeyPatchID] = strconv.Itoa(ephemeralRunnerSet.Spec.PatchID)
 	return &v1alpha1.EphemeralRunner{
 		ObjectMeta: metav1.ObjectMeta{
@@ -596,27 +587,17 @@ func (b *ResourceBuilder) newEphemeralRunner(ephemeralRunnerSet *v1alpha1.Epheme
 	}
 }
 
-func (b *ResourceBuilder) newEphemeralRunnerPod(ctx context.Context, runner *v1alpha1.EphemeralRunner, secret *corev1.Secret, envs ...corev1.EnvVar) *corev1.Pod {
+func (b *ResourceBuilder) newEphemeralRunnerPod(runner *v1alpha1.EphemeralRunner, secret *corev1.Secret, envs ...corev1.EnvVar) *corev1.Pod {
 	var newPod corev1.Pod
 
-	labels := map[string]string{}
-	annotations := map[string]string{}
+	annotations := make(map[string]string, len(runner.Annotations)+len(runner.Spec.Annotations))
+	maps.Copy(annotations, runner.Annotations)
+	maps.Copy(annotations, runner.Spec.Annotations)
 
-	for k, v := range runner.Labels {
-		labels[k] = v
-	}
-	for k, v := range runner.Spec.Labels {
-		labels[k] = v
-	}
+	labels := make(map[string]string, len(runner.Labels)+len(runner.Spec.Labels)+2)
+	maps.Copy(labels, runner.Labels)
+	maps.Copy(labels, runner.Spec.Labels)
 	labels["actions-ephemeral-runner"] = string(corev1.ConditionTrue)
-
-	for k, v := range runner.Annotations {
-		annotations[k] = v
-	}
-	for k, v := range runner.Spec.Annotations {
-		annotations[k] = v
-	}
-
 	labels[LabelKeyPodTemplateHash] = hash.FNVHashStringObjects(
 		FilterLabels(labels, LabelKeyRunnerTemplateHash),
 		annotations,

controllers/actions.github.com/resourcebuilder_test.go

@@ -1,7 +1,6 @@
 package actionsgithubcom
 
 import (
-	"context"
 	"fmt"
 	"strings"
 	"testing"
@@ -28,7 +27,7 @@ func TestLabelPropagation(t *testing.T) {
 				"directly.excluded.org/arbitrary": "not-excluded-value",
 			},
 			Annotations: map[string]string{
-				runnerScaleSetIdAnnotationKey:         "1",
+				runnerScaleSetIDAnnotationKey:         "1",
 				AnnotationKeyGitHubRunnerGroupName:    "test-group",
 				AnnotationKeyGitHubRunnerScaleSetName: "test-scale-set",
 			},
@@ -104,7 +103,7 @@ func TestLabelPropagation(t *testing.T) {
 			Name: "test",
 		},
 	}
-	pod := b.newEphemeralRunnerPod(context.TODO(), ephemeralRunner, runnerSecret)
+	pod := b.newEphemeralRunnerPod(ephemeralRunner, runnerSecret)
 	for key := range ephemeralRunner.Labels {
 		assert.Equal(t, ephemeralRunner.Labels[key], pod.Labels[key])
 	}
@@ -124,7 +123,7 @@ func TestGitHubURLTrimLabelValues(t *testing.T) {
 				LabelKeyKubernetesVersion: "0.2.0",
 			},
 			Annotations: map[string]string{
-				runnerScaleSetIdAnnotationKey:         "1",
+				runnerScaleSetIDAnnotationKey:         "1",
 				AnnotationKeyGitHubRunnerGroupName:    "test-group",
 				AnnotationKeyGitHubRunnerScaleSetName: "test-scale-set",
 			},
@@ -190,7 +189,7 @@ func TestOwnershipRelationships(t *testing.T) {
 				LabelKeyKubernetesVersion: "0.2.0",
 			},
 			Annotations: map[string]string{
-				runnerScaleSetIdAnnotationKey:         "1",
+				runnerScaleSetIDAnnotationKey:         "1",
 				AnnotationKeyGitHubRunnerGroupName:    "test-group",
 				AnnotationKeyGitHubRunnerScaleSetName: "test-scale-set",
 				annotationKeyValuesHash:               "test-hash",
@@ -233,7 +232,7 @@ func TestOwnershipRelationships(t *testing.T) {
 			Name: "test-secret",
 		},
 	}
-	pod := b.newEphemeralRunnerPod(context.TODO(), ephemeralRunner, runnerSecret)
+	pod := b.newEphemeralRunnerPod(ephemeralRunner, runnerSecret)
 
 	// Test EphemeralRunnerPod ownership
 	require.Len(t, pod.OwnerReferences, 1, "EphemeralRunnerPod should have exactly one owner reference")