From 41f2ca3ed922f75d055ffc54ef697176220d46eb Mon Sep 17 00:00:00 2001 From: Chris Patterson Date: Fri, 3 Mar 2023 08:36:14 -0500 Subject: [PATCH] Adding parameter to configure the runner set name. (#2279) Co-authored-by: TingluoHuang --- .../v1alpha1/autoscalingrunnerset_types.go | 5 + ...ions.github.com_autoscalingrunnersets.yaml | 2 + .../templates/autoscalingrunnerset.yaml | 3 + .../tests/template_test.go | 47 +++++++ charts/gha-runner-scale-set/values.yaml | 3 + ...ions.github.com_autoscalingrunnersets.yaml | 2 + .../autoscalinglistener_controller.go | 1 + .../autoscalingrunnerset_controller.go | 66 +++++++++- .../autoscalingrunnerset_controller_test.go | 123 ++++++++++++++++++ github/actions/fake/client.go | 7 + 10 files changed, 252 insertions(+), 7 deletions(-) diff --git a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go index ba8af2fc..8f2bf102 100644 --- a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go @@ -57,6 +57,9 @@ type AutoscalingRunnerSetSpec struct { // +optional RunnerGroup string `json:"runnerGroup,omitempty"` + // +optional + RunnerScaleSetName string `json:"runnerScaleSetName,omitempty"` + // +optional Proxy *ProxyConfig `json:"proxy,omitempty"` @@ -205,6 +208,7 @@ func (ars *AutoscalingRunnerSet) RunnerSetSpecHash() string { GitHubConfigUrl string GitHubConfigSecret string RunnerGroup string + RunnerScaleSetName string Proxy *ProxyConfig GitHubServerTLS *GitHubServerTLSConfig Template corev1.PodTemplateSpec @@ -213,6 +217,7 @@ func (ars *AutoscalingRunnerSet) RunnerSetSpecHash() string { GitHubConfigUrl: ars.Spec.GitHubConfigUrl, GitHubConfigSecret: ars.Spec.GitHubConfigSecret, RunnerGroup: ars.Spec.RunnerGroup, + RunnerScaleSetName: ars.Spec.RunnerScaleSetName, Proxy: ars.Spec.Proxy, GitHubServerTLS: ars.Spec.GitHubServerTLS, Template: ars.Spec.Template, diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml index 00775406..12c4b5b8 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml @@ -86,6 +86,8 @@ spec: type: object runnerGroup: type: string + runnerScaleSetName: + type: string template: description: Required properties: diff --git a/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml b/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml index 478fc775..6ad3c6c8 100644 --- a/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml +++ b/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml @@ -17,6 +17,9 @@ spec: {{- with .Values.runnerGroup }} runnerGroup: {{ . }} {{- end }} + {{- with .Values.runnerScaleSetName }} + runnerScaleSetName: {{ . }} + {{- end }} {{- if .Values.proxy }} proxy: diff --git a/charts/gha-runner-scale-set/tests/template_test.go b/charts/gha-runner-scale-set/tests/template_test.go index c9939415..e6995dff 100644 --- a/charts/gha-runner-scale-set/tests/template_test.go +++ b/charts/gha-runner-scale-set/tests/template_test.go @@ -310,6 +310,53 @@ func TestTemplateRenderedAutoScalingRunnerSet(t *testing.T) { assert.Equal(t, "ghcr.io/actions/actions-runner:latest", ars.Spec.Template.Spec.Containers[0].Image) } +func TestTemplateRenderedAutoScalingRunnerSet_RunnerScaleSetName(t *testing.T) { + t.Parallel() + + // Path to the helm chart we will test + helmChartPath, err := filepath.Abs("../../gha-runner-scale-set") + require.NoError(t, err) + + releaseName := "test-runners" + namespaceName := "test-" + strings.ToLower(random.UniqueId()) + + options := &helm.Options{ + SetValues: map[string]string{ + "githubConfigUrl": "https://github.com/actions", + "githubConfigSecret.github_token": "gh_token12345", + "runnerScaleSetName": "test-runner-scale-set-name", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + } + + output := helm.RenderTemplate(t, options, helmChartPath, releaseName, []string{"templates/autoscalingrunnerset.yaml"}) + + var ars v1alpha1.AutoscalingRunnerSet + helm.UnmarshalK8SYaml(t, output, &ars) + + assert.Equal(t, namespaceName, ars.Namespace) + assert.Equal(t, "test-runners", ars.Name) + + assert.Equal(t, "gha-runner-scale-set", ars.Labels["app.kubernetes.io/name"]) + assert.Equal(t, "test-runners", ars.Labels["app.kubernetes.io/instance"]) + assert.Equal(t, "https://github.com/actions", ars.Spec.GitHubConfigUrl) + assert.Equal(t, "test-runners-gha-runner-scale-set-github-secret", ars.Spec.GitHubConfigSecret) + assert.Equal(t, "test-runner-scale-set-name", ars.Spec.RunnerScaleSetName) + + assert.Empty(t, ars.Spec.RunnerGroup, "RunnerGroup should be empty") + + assert.Nil(t, ars.Spec.MinRunners, "MinRunners should be nil") + assert.Nil(t, ars.Spec.MaxRunners, "MaxRunners should be nil") + assert.Nil(t, ars.Spec.Proxy, "Proxy should be nil") + assert.Nil(t, ars.Spec.GitHubServerTLS, "GitHubServerTLS should be nil") + + assert.NotNil(t, ars.Spec.Template.Spec, "Template.Spec should not be nil") + + assert.Len(t, ars.Spec.Template.Spec.Containers, 1, "Template.Spec should have 1 container") + assert.Equal(t, "runner", ars.Spec.Template.Spec.Containers[0].Name) + assert.Equal(t, "ghcr.io/actions/actions-runner:latest", ars.Spec.Template.Spec.Containers[0].Image) +} + func TestTemplateRenderedAutoScalingRunnerSet_ProvideMetadata(t *testing.T) { t.Parallel() diff --git a/charts/gha-runner-scale-set/values.yaml b/charts/gha-runner-scale-set/values.yaml index f3bbc214..8845daa8 100644 --- a/charts/gha-runner-scale-set/values.yaml +++ b/charts/gha-runner-scale-set/values.yaml @@ -44,6 +44,9 @@ githubConfigSecret: # runnerGroup: "default" +## name of the runner scale set to create. Defaults to the helm release name +# runnerScaleSetName: "" + ## template is the PodSpec for each runner Pod template: spec: diff --git a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml index 00775406..12c4b5b8 100644 --- a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml +++ b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml @@ -86,6 +86,8 @@ spec: type: object runnerGroup: type: string + runnerScaleSetName: + type: string template: description: Required properties: diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index 3110a748..5ad3f0c3 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller.go +++ b/controllers/actions.github.com/autoscalinglistener_controller.go @@ -99,6 +99,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. } log.Info("Successfully removed finalizer after cleanup") + return ctrl.Result{}, nil } if !controllerutil.ContainsFinalizer(autoscalingListener, autoscalingListenerFinalizerName) { diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go index 1c5c32eb..1c77acd9 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -46,6 +46,7 @@ const ( LabelKeyRunnerSpecHash = "runner-spec-hash" autoscalingRunnerSetFinalizerName = "autoscalingrunnerset.actions.github.com/finalizer" runnerScaleSetIdKey = "runner-scale-set-id" + runnerScaleSetNameKey = "runner-scale-set-name" runnerScaleSetRunnerGroupNameKey = "runner-scale-set-runner-group-name" ) @@ -123,6 +124,7 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl } log.Info("Successfully removed finalizer after cleanup") + return ctrl.Result{}, nil } if !controllerutil.ContainsFinalizer(autoscalingRunnerSet, autoscalingRunnerSetFinalizerName) { @@ -158,6 +160,13 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl return r.updateRunnerScaleSetRunnerGroup(ctx, autoscalingRunnerSet, log) } + // Make sure the runner scale set name is up to date + currentRunnerScaleSetName, ok := autoscalingRunnerSet.Annotations[runnerScaleSetNameKey] + if !ok || (len(autoscalingRunnerSet.Spec.RunnerScaleSetName) > 0 && !strings.EqualFold(currentRunnerScaleSetName, autoscalingRunnerSet.Spec.RunnerScaleSetName)) { + log.Info("AutoScalingRunnerSet runner scale set name changed. Updating the runner scale set.") + return r.updateRunnerScaleSetName(ctx, autoscalingRunnerSet, log) + } + secret := new(corev1.Secret) if err := r.Get(ctx, types.NamespacedName{Namespace: autoscalingRunnerSet.Namespace, Name: autoscalingRunnerSet.Spec.GitHubConfigSecret}, secret); err != nil { log.Error(err, "Failed to find GitHub config secret.", @@ -297,11 +306,14 @@ func (r *AutoscalingRunnerSetReconciler) deleteEphemeralRunnerSets(ctx context.C func (r *AutoscalingRunnerSetReconciler) createRunnerScaleSet(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, logger logr.Logger) (ctrl.Result, error) { logger.Info("Creating a new runner scale set") actionsClient, err := r.actionsClientFor(ctx, autoscalingRunnerSet) + if len(autoscalingRunnerSet.Spec.RunnerScaleSetName) == 0 { + autoscalingRunnerSet.Spec.RunnerScaleSetName = autoscalingRunnerSet.Name + } if err != nil { logger.Error(err, "Failed to initialize Actions service client for creating a new runner scale set") return ctrl.Result{}, err } - runnerScaleSet, err := actionsClient.GetRunnerScaleSet(ctx, autoscalingRunnerSet.Name) + runnerScaleSet, err := actionsClient.GetRunnerScaleSet(ctx, autoscalingRunnerSet.Spec.RunnerScaleSetName) if err != nil { logger.Error(err, "Failed to get runner scale set from Actions service") return ctrl.Result{}, err @@ -322,11 +334,11 @@ func (r *AutoscalingRunnerSetReconciler) createRunnerScaleSet(ctx context.Contex runnerScaleSet, err = actionsClient.CreateRunnerScaleSet( ctx, &actions.RunnerScaleSet{ - Name: autoscalingRunnerSet.Name, + Name: autoscalingRunnerSet.Spec.RunnerScaleSetName, RunnerGroupId: runnerGroupId, Labels: []actions.Label{ { - Name: autoscalingRunnerSet.Name, + Name: autoscalingRunnerSet.Spec.RunnerScaleSetName, Type: "System", }, }, @@ -346,16 +358,20 @@ func (r *AutoscalingRunnerSetReconciler) createRunnerScaleSet(ctx context.Contex autoscalingRunnerSet.Annotations = map[string]string{} } - logger.Info("Adding runner scale set ID and runner group name as an annotation") + logger.Info("Adding runner scale set ID, name and runner group name as an annotation") if err = patch(ctx, r.Client, autoscalingRunnerSet, func(obj *v1alpha1.AutoscalingRunnerSet) { + obj.Annotations[runnerScaleSetNameKey] = runnerScaleSet.Name obj.Annotations[runnerScaleSetIdKey] = strconv.Itoa(runnerScaleSet.Id) obj.Annotations[runnerScaleSetRunnerGroupNameKey] = runnerScaleSet.RunnerGroupName }); err != nil { - logger.Error(err, "Failed to add runner scale set ID and runner group name as an annotation") + logger.Error(err, "Failed to add runner scale set ID, name and runner group name as an annotation") return ctrl.Result{}, err } - logger.Info("Updated with runner scale set ID and runner group name as an annotation") + logger.Info("Updated with runner scale set ID, name and runner group name as an annotation", + "id", runnerScaleSet.Id, + "name", runnerScaleSet.Name, + "runnerGroupName", runnerScaleSet.RunnerGroupName) return ctrl.Result{}, nil } @@ -383,7 +399,7 @@ func (r *AutoscalingRunnerSetReconciler) updateRunnerScaleSetRunnerGroup(ctx con runnerGroupId = int(runnerGroup.ID) } - updatedRunnerScaleSet, err := actionsClient.UpdateRunnerScaleSet(ctx, runnerScaleSetId, &actions.RunnerScaleSet{Name: autoscalingRunnerSet.Name, 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) return ctrl.Result{}, err @@ -401,6 +417,42 @@ func (r *AutoscalingRunnerSetReconciler) updateRunnerScaleSetRunnerGroup(ctx con return ctrl.Result{}, nil } +func (r *AutoscalingRunnerSetReconciler) updateRunnerScaleSetName(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, logger logr.Logger) (ctrl.Result, error) { + runnerScaleSetId, err := strconv.Atoi(autoscalingRunnerSet.Annotations[runnerScaleSetIdKey]) + if err != nil { + logger.Error(err, "Failed to parse runner scale set ID") + return ctrl.Result{}, err + } + + if len(autoscalingRunnerSet.Spec.RunnerScaleSetName) == 0 { + logger.Info("Runner scale set name is not specified, skipping") + return ctrl.Result{}, nil + } + + actionsClient, err := r.actionsClientFor(ctx, autoscalingRunnerSet) + if err != nil { + logger.Error(err, "Failed to initialize Actions service client for updating a existing runner scale set") + return ctrl.Result{}, err + } + + 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) + return ctrl.Result{}, err + } + + logger.Info("Updating runner scale set name as an annotation") + if err := patch(ctx, r.Client, autoscalingRunnerSet, func(obj *v1alpha1.AutoscalingRunnerSet) { + obj.Annotations[runnerScaleSetNameKey] = updatedRunnerScaleSet.Name + }); err != nil { + logger.Error(err, "Failed to update runner scale set name annotation") + return ctrl.Result{}, err + } + + logger.Info("Updated runner scale set with match name", "name", updatedRunnerScaleSet.Name) + return ctrl.Result{}, nil +} + func (r *AutoscalingRunnerSetReconciler) deleteRunnerScaleSet(ctx context.Context, autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, logger logr.Logger) error { logger.Info("Deleting the runner scale set from Actions service") runnerScaleSetId, err := strconv.Atoi(autoscalingRunnerSet.Annotations[runnerScaleSetIdKey]) diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go index b0cd4c2c..e999fc7e 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go @@ -400,6 +400,129 @@ var _ = Describe("Test AutoScalingRunnerSet controller", func() { }) }) +var _ = Describe("Test AutoScalingController updates", func() { + Context("Creating autoscaling runner set with RunnerScaleSetName set", func() { + var ctx context.Context + var mgr ctrl.Manager + var autoscalingNS *corev1.Namespace + var autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet + var configSecret *corev1.Secret + + BeforeEach(func() { + ctx = context.Background() + autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient) + configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) + + controller := &AutoscalingRunnerSetReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Log: logf.Log, + ControllerNamespace: autoscalingNS.Name, + DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", + ActionsClient: fake.NewMultiClient( + fake.WithDefaultClient( + fake.NewFakeClient( + fake.WithUpdateRunnerScaleSet( + &actions.RunnerScaleSet{ + Id: 1, + Name: "testset_update", + RunnerGroupId: 1, + RunnerGroupName: "testgroup", + Labels: []actions.Label{{Type: "test", Name: "test"}}, + RunnerSetting: actions.RunnerSetting{}, + CreatedOn: time.Now(), + RunnerJitConfigUrl: "test.test.test", + Statistics: nil, + }, + nil, + ), + ), + nil, + ), + ), + } + err := controller.SetupWithManager(mgr) + Expect(err).NotTo(HaveOccurred(), "failed to setup controller") + + startManagers(GinkgoT(), mgr) + }) + + It("It should be create AutoScalingRunnerSet and has annotation for the RunnerScaleSetName", func() { + min := 1 + max := 10 + autoscalingRunnerSet = &v1alpha1.AutoscalingRunnerSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-asrs", + Namespace: autoscalingNS.Name, + }, + Spec: v1alpha1.AutoscalingRunnerSetSpec{ + GitHubConfigUrl: "https://github.com/owner/repo", + GitHubConfigSecret: configSecret.Name, + MaxRunners: &max, + MinRunners: &min, + RunnerScaleSetName: "testset", + RunnerGroup: "testgroup", + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "runner", + Image: "ghcr.io/actions/runner", + }, + }, + }, + }, + }, + } + + err := k8sClient.Create(ctx, autoscalingRunnerSet) + Expect(err).NotTo(HaveOccurred(), "failed to create AutoScalingRunnerSet") + + // Wait for the AutoScalingRunnerSet to be created with right annotation + ars := new(v1alpha1.AutoscalingRunnerSet) + Eventually( + func() (string, error) { + err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, ars) + if err != nil { + return "", err + } + + if val, ok := ars.Annotations[runnerScaleSetNameKey]; ok { + return val, nil + } + + return "", nil + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(BeEquivalentTo(autoscalingRunnerSet.Spec.RunnerScaleSetName), "AutoScalingRunnerSet should have annotation for the RunnerScaleSetName") + + update := autoscalingRunnerSet.DeepCopy() + update.Spec.RunnerScaleSetName = "testset_update" + err = k8sClient.Patch(ctx, update, client.MergeFrom(autoscalingRunnerSet)) + Expect(err).NotTo(HaveOccurred(), "failed to update AutoScalingRunnerSet") + + // Wait for the AutoScalingRunnerSet to be updated with right annotation + Eventually( + func() (string, error) { + err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, ars) + if err != nil { + return "", err + } + + if val, ok := ars.Annotations[runnerScaleSetNameKey]; ok { + return val, nil + } + + return "", nil + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(BeEquivalentTo(update.Spec.RunnerScaleSetName), "AutoScalingRunnerSet should have a updated annotation for the RunnerScaleSetName") + }) + }) +}) + var _ = Describe("Test AutoscalingController creation failures", func() { Context("When autoscaling runner set creation fails on the client", func() { var ctx context.Context diff --git a/github/actions/fake/client.go b/github/actions/fake/client.go index 5d7e22b7..0729425d 100644 --- a/github/actions/fake/client.go +++ b/github/actions/fake/client.go @@ -38,6 +38,13 @@ func WithCreateRunnerScaleSet(scaleSet *actions.RunnerScaleSet, err error) Optio } } +func WithUpdateRunnerScaleSet(scaleSet *actions.RunnerScaleSet, err error) Option { + return func(f *FakeClient) { + f.updateRunnerScaleSetResult.RunnerScaleSet = scaleSet + f.updateRunnerScaleSetResult.err = err + } +} + var defaultRunnerScaleSet = &actions.RunnerScaleSet{ Id: 1, Name: "testset",