From 9859bbc7f2b1d9006fd137e6ed1db807cbfe86a0 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Mon, 24 Apr 2023 10:40:15 +0200 Subject: [PATCH] Use build.Version to check if resource version is a mismatch (#2521) Co-authored-by: Bassem Dghaidi <568794+Link-@users.noreply.github.com> --- .github/workflows/e2e-test-linux-vm.yaml | 2 +- .../autoscalingrunnerset_controller.go | 17 ++ .../autoscalingrunnerset_controller_test.go | 173 +++++++++++++++++- 3 files changed, 184 insertions(+), 8 deletions(-) diff --git a/.github/workflows/e2e-test-linux-vm.yaml b/.github/workflows/e2e-test-linux-vm.yaml index 6dd39fb4..35dafedd 100644 --- a/.github/workflows/e2e-test-linux-vm.yaml +++ b/.github/workflows/e2e-test-linux-vm.yaml @@ -16,7 +16,7 @@ env: TARGET_ORG: actions-runner-controller TARGET_REPO: arc_e2e_test_dummy IMAGE_NAME: "arc-test-image" - IMAGE_VERSION: "dev" + IMAGE_VERSION: "0.4.0" jobs: default-setup: diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go index 16c20442..79e28df9 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -24,6 +24,7 @@ import ( "strings" "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" + "github.com/actions/actions-runner-controller/build" "github.com/actions/actions-runner-controller/github/actions" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" @@ -136,6 +137,22 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl return ctrl.Result{}, nil } + if autoscalingRunnerSet.Labels[LabelKeyKubernetesVersion] != build.Version { + if err := r.Delete(ctx, autoscalingRunnerSet); err != nil { + log.Error(err, "Failed to delete autoscaling runner set on version mismatch", + "targetVersion", build.Version, + "actualVersion", autoscalingRunnerSet.Labels[LabelKeyKubernetesVersion], + ) + return ctrl.Result{}, nil + } + + log.Info("Autoscaling runner set version doesn't match the build version. Deleting the resource.", + "targetVersion", build.Version, + "actualVersion", autoscalingRunnerSet.Labels[LabelKeyKubernetesVersion], + ) + return ctrl.Result{}, nil + } + if !controllerutil.ContainsFinalizer(autoscalingRunnerSet, autoscalingRunnerSetFinalizerName) { log.Info("Adding finalizer") if err := patch(ctx, r.Client, autoscalingRunnerSet, func(obj *v1alpha1.AutoscalingRunnerSet) { diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go index a53732e3..390511e6 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/types" "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" + "github.com/actions/actions-runner-controller/build" "github.com/actions/actions-runner-controller/github/actions" "github.com/actions/actions-runner-controller/github/actions/fake" "github.com/actions/actions-runner-controller/github/actions/testserver" @@ -38,13 +39,25 @@ const ( autoscalingRunnerSetTestGitHubToken = "gh_token" ) -var _ = Describe("Test AutoScalingRunnerSet controller", func() { +var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { var ctx context.Context var mgr ctrl.Manager var autoscalingNS *corev1.Namespace var autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet var configSecret *corev1.Secret + var originalBuildVersion string + buildVersion := "0.1.0" + + BeforeAll(func() { + originalBuildVersion = build.Version + build.Version = buildVersion + }) + + AfterAll(func() { + build.Version = originalBuildVersion + }) + BeforeEach(func() { ctx = context.Background() autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient) @@ -67,6 +80,9 @@ var _ = Describe("Test AutoScalingRunnerSet controller", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-asrs", Namespace: autoscalingNS.Name, + Labels: map[string]string{ + LabelKeyKubernetesVersion: buildVersion, + }, }, Spec: v1alpha1.AutoscalingRunnerSetSpec{ GitHubConfigUrl: "https://github.com/owner/repo", @@ -474,7 +490,19 @@ var _ = Describe("Test AutoScalingRunnerSet controller", func() { }) }) -var _ = Describe("Test AutoScalingController updates", func() { +var _ = Describe("Test AutoScalingController updates", Ordered, func() { + var originalBuildVersion string + buildVersion := "0.1.0" + + BeforeAll(func() { + originalBuildVersion = build.Version + build.Version = buildVersion + }) + + AfterAll(func() { + build.Version = originalBuildVersion + }) + Context("Creating autoscaling runner set with RunnerScaleSetName set", func() { var ctx context.Context var mgr ctrl.Manager @@ -483,6 +511,7 @@ var _ = Describe("Test AutoScalingController updates", func() { var configSecret *corev1.Secret BeforeEach(func() { + originalBuildVersion = build.Version ctx = context.Background() autoscalingNS, mgr = createNamespace(GinkgoT(), k8sClient) configSecret = createDefaultSecret(GinkgoT(), k8sClient, autoscalingNS.Name) @@ -528,6 +557,9 @@ var _ = Describe("Test AutoScalingController updates", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-asrs", Namespace: autoscalingNS.Name, + Labels: map[string]string{ + LabelKeyKubernetesVersion: buildVersion, + }, }, Spec: v1alpha1.AutoscalingRunnerSetSpec{ GitHubConfigUrl: "https://github.com/owner/repo", @@ -598,7 +630,18 @@ var _ = Describe("Test AutoScalingController updates", func() { }) }) -var _ = Describe("Test AutoscalingController creation failures", func() { +var _ = Describe("Test AutoscalingController creation failures", Ordered, func() { + var originalBuildVersion string + buildVersion := "0.1.0" + + BeforeAll(func() { + originalBuildVersion = build.Version + build.Version = buildVersion + }) + + AfterAll(func() { + build.Version = originalBuildVersion + }) Context("When autoscaling runner set creation fails on the client", func() { var ctx context.Context var mgr ctrl.Manager @@ -629,6 +672,9 @@ var _ = Describe("Test AutoscalingController creation failures", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-asrs", Namespace: autoscalingNS.Name, + Labels: map[string]string{ + LabelKeyKubernetesVersion: buildVersion, + }, }, Spec: v1alpha1.AutoscalingRunnerSetSpec{ GitHubConfigUrl: "https://github.com/owner/repo", @@ -707,7 +753,18 @@ var _ = Describe("Test AutoscalingController creation failures", func() { }) }) -var _ = Describe("Test Client optional configuration", func() { +var _ = Describe("Test client optional configuration", Ordered, func() { + var originalBuildVersion string + buildVersion := "0.1.0" + + BeforeAll(func() { + originalBuildVersion = build.Version + build.Version = buildVersion + }) + + AfterAll(func() { + build.Version = originalBuildVersion + }) Context("When specifying a proxy", func() { var ctx context.Context var mgr ctrl.Manager @@ -747,6 +804,9 @@ var _ = Describe("Test Client optional configuration", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-asrs", Namespace: autoscalingNS.Name, + Labels: map[string]string{ + LabelKeyKubernetesVersion: buildVersion, + }, }, Spec: v1alpha1.AutoscalingRunnerSetSpec{ GitHubConfigUrl: "http://example.com/org/repo", @@ -823,6 +883,9 @@ var _ = Describe("Test Client optional configuration", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-asrs", Namespace: autoscalingNS.Name, + Labels: map[string]string{ + LabelKeyKubernetesVersion: buildVersion, + }, }, Spec: v1alpha1.AutoscalingRunnerSetSpec{ GitHubConfigUrl: "http://example.com/org/repo", @@ -939,6 +1002,9 @@ var _ = Describe("Test Client optional configuration", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-asrs", Namespace: autoscalingNS.Name, + Labels: map[string]string{ + LabelKeyKubernetesVersion: buildVersion, + }, }, Spec: v1alpha1.AutoscalingRunnerSetSpec{ GitHubConfigUrl: server.ConfigURLForOrg("my-org"), @@ -989,6 +1055,9 @@ var _ = Describe("Test Client optional configuration", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-asrs", Namespace: autoscalingNS.Name, + Labels: map[string]string{ + LabelKeyKubernetesVersion: buildVersion, + }, }, Spec: v1alpha1.AutoscalingRunnerSetSpec{ GitHubConfigUrl: "https://github.com/owner/repo", @@ -1050,6 +1119,9 @@ var _ = Describe("Test Client optional configuration", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-asrs", Namespace: autoscalingNS.Name, + Labels: map[string]string{ + LabelKeyKubernetesVersion: buildVersion, + }, }, Spec: v1alpha1.AutoscalingRunnerSetSpec{ GitHubConfigUrl: "https://github.com/owner/repo", @@ -1102,7 +1174,19 @@ var _ = Describe("Test Client optional configuration", func() { }) }) -var _ = Describe("Test external permissions cleanup", func() { +var _ = Describe("Test external permissions cleanup", Ordered, func() { + var originalBuildVersion string + buildVersion := "0.1.0" + + BeforeAll(func() { + originalBuildVersion = build.Version + build.Version = buildVersion + }) + + AfterAll(func() { + build.Version = originalBuildVersion + }) + It("Should clean up kubernetes mode permissions", func() { ctx := context.Background() autoscalingNS, mgr := createNamespace(GinkgoT(), k8sClient) @@ -1129,7 +1213,8 @@ var _ = Describe("Test external permissions cleanup", func() { Name: "test-asrs", Namespace: autoscalingNS.Name, Labels: map[string]string{ - "app.kubernetes.io/name": "gha-runner-scale-set", + "app.kubernetes.io/name": "gha-runner-scale-set", + LabelKeyKubernetesVersion: buildVersion, }, Annotations: map[string]string{ AnnotationKeyKubernetesModeRoleBindingName: "kube-mode-role-binding", @@ -1286,7 +1371,8 @@ var _ = Describe("Test external permissions cleanup", func() { Name: "test-asrs", Namespace: autoscalingNS.Name, Labels: map[string]string{ - "app.kubernetes.io/name": "gha-runner-scale-set", + "app.kubernetes.io/name": "gha-runner-scale-set", + LabelKeyKubernetesVersion: buildVersion, }, Annotations: map[string]string{ AnnotationKeyManagerRoleName: "manager-role", @@ -1465,3 +1551,76 @@ var _ = Describe("Test external permissions cleanup", func() { ).Should(BeTrue(), "Expected role to be cleaned up") }) }) + +var _ = Describe("Test resource version and build version mismatch", func() { + It("Should delete and recreate the autoscaling runner set to match the build version", 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(), + } + err := controller.SetupWithManager(mgr) + Expect(err).NotTo(HaveOccurred(), "failed to setup controller") + + originalVersion := build.Version + defer func() { + build.Version = originalVersion + }() + build.Version = "0.2.0" + + min := 1 + max := 10 + autoscalingRunnerSet := &v1alpha1.AutoscalingRunnerSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-asrs", + Namespace: autoscalingNS.Name, + Labels: map[string]string{ + "app.kubernetes.io/name": "gha-runner-scale-set", + "app.kubernetes.io/version": "0.1.0", + }, + Annotations: map[string]string{ + AnnotationKeyKubernetesModeRoleBindingName: "kube-mode-role-binding", + AnnotationKeyKubernetesModeRoleName: "kube-mode-role", + AnnotationKeyKubernetesModeServiceAccountName: "kube-mode-service-account", + }, + }, + Spec: v1alpha1.AutoscalingRunnerSetSpec{ + GitHubConfigUrl: "https://github.com/owner/repo", + GitHubConfigSecret: configSecret.Name, + MaxRunners: &max, + MinRunners: &min, + RunnerGroup: "testgroup", + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "runner", + Image: "ghcr.io/actions/runner", + }, + }, + }, + }, + }, + } + + // create autoscaling runner set before starting a manager + err = k8sClient.Create(ctx, autoscalingRunnerSet) + Expect(err).NotTo(HaveOccurred()) + + startManagers(GinkgoT(), mgr) + + Eventually(func() bool { + ars := new(v1alpha1.AutoscalingRunnerSet) + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: autoscalingRunnerSet.Namespace, Name: autoscalingRunnerSet.Name}, ars) + return errors.IsNotFound(err) + }).Should(BeTrue()) + }) +})