From 6d07b8d853378575f70c8f4fe6a5fbb21974c93f Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Thu, 20 Nov 2025 23:06:27 +0100 Subject: [PATCH] Add ephemeral runner finalizer during creation and check finalizer without requeue (#4320) --- .github/workflows/gha-e2e-tests.yaml | 53 +++----- .../ephemeralrunner_controller.go | 24 +--- .../ephemeralrunnerset_controller_test.go | 118 +++++++++++------- .../actions.github.com/resourcebuilder.go | 5 +- hack/e2e-test.sh | 7 +- .../anonymous-proxy-setup.test.sh | 2 +- 6 files changed, 104 insertions(+), 105 deletions(-) diff --git a/.github/workflows/gha-e2e-tests.yaml b/.github/workflows/gha-e2e-tests.yaml index f35c7a2e..5fd3af44 100644 --- a/.github/workflows/gha-e2e-tests.yaml +++ b/.github/workflows/gha-e2e-tests.yaml @@ -30,9 +30,6 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.id == github.repository_id - env: - TARGET_ORG: ${{ env.TARGET_ORG }} - TARGET_REPO: ${{ env.TARGET_REPO }} steps: - uses: actions/checkout@v5 with: @@ -48,6 +45,8 @@ jobs: - name: Run default setup test run: hack/e2e-test.sh default-setup + env: + GITHUB_TOKEN: "${{steps.config-token.outputs.token}}" shell: bash default-setup: @@ -145,10 +144,6 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.id == github.repository_id - env: - TARGET_ORG: ${{ env.TARGET_ORG }} - TARGET_REPO: ${{ env.TARGET_REPO }} - steps: - uses: actions/checkout@v5 with: @@ -164,6 +159,8 @@ jobs: - name: Run single namespace setup test run: hack/e2e-test.sh single-namespace-setup + env: + GITHUB_TOKEN: "${{steps.config-token.outputs.token}}" shell: bash single-namespace-setup: @@ -263,10 +260,6 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.id == github.repository_id - env: - TARGET_ORG: ${{ env.TARGET_ORG }} - TARGET_REPO: ${{ env.TARGET_REPO }} - steps: - uses: actions/checkout@v5 with: @@ -282,6 +275,8 @@ jobs: - name: Run dind mode setup test run: hack/e2e-test.sh dind-mode-setup + env: + GITHUB_TOKEN: "${{steps.config-token.outputs.token}}" shell: bash dind-mode-setup: @@ -380,10 +375,6 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.id == github.repository_id - env: - TARGET_ORG: ${{ env.TARGET_ORG }} - TARGET_REPO: ${{ env.TARGET_REPO }} - steps: - uses: actions/checkout@v5 with: @@ -399,6 +390,8 @@ jobs: - name: Run kubernetes mode setup test run: hack/e2e-test.sh kubernetes-mode-setup + env: + GITHUB_TOKEN: "${{steps.config-token.outputs.token}}" shell: bash kubernetes-mode-setup: @@ -506,10 +499,6 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.id == github.repository_id - env: - TARGET_ORG: ${{ env.TARGET_ORG }} - TARGET_REPO: ${{ env.TARGET_REPO }} - steps: - uses: actions/checkout@v5 with: @@ -525,6 +514,8 @@ jobs: - name: Run single namespace setup test run: hack/e2e-test.sh single-namespace-setup + env: + GITHUB_TOKEN: "${{steps.config-token.outputs.token}}" shell: bash auth-proxy-setup: @@ -634,10 +625,6 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.id == github.repository_id - env: - TARGET_ORG: ${{ env.TARGET_ORG }} - TARGET_REPO: ${{ env.TARGET_REPO }} - steps: - uses: actions/checkout@v5 with: @@ -653,6 +640,8 @@ jobs: - name: Run anonymous proxy setup test run: hack/e2e-test.sh anonymous-proxy-setup + env: + GITHUB_TOKEN: "${{steps.config-token.outputs.token}}" shell: bash anonymous-proxy-setup: @@ -756,10 +745,6 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.id == github.repository_id - env: - TARGET_ORG: ${{ env.TARGET_ORG }} - TARGET_REPO: ${{ env.TARGET_REPO }} - steps: - uses: actions/checkout@v5 with: @@ -775,6 +760,8 @@ jobs: - name: Run self signed CA setup test run: hack/e2e-test.sh self-signed-ca-setup + env: + GITHUB_TOKEN: "${{steps.config-token.outputs.token}}" shell: bash self-signed-ca-setup: @@ -903,10 +890,6 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.id == github.repository_id - env: - TARGET_ORG: ${{ env.TARGET_ORG }} - TARGET_REPO: ${{ env.TARGET_REPO }} - steps: - uses: actions/checkout@v5 with: @@ -922,6 +905,8 @@ jobs: - name: Run update strategy test run: hack/e2e-test.sh update-strategy + env: + GITHUB_TOKEN: "${{steps.config-token.outputs.token}}" shell: bash update-strategy-tests: @@ -1100,10 +1085,6 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.id == github.repository_id - env: - TARGET_ORG: ${{ env.TARGET_ORG }} - TARGET_REPO: ${{ env.TARGET_REPO }} - steps: - uses: actions/checkout@v5 with: @@ -1119,6 +1100,8 @@ jobs: - name: Run init with min runners test run: hack/e2e-test.sh init-with-min-runners + env: + GITHUB_TOKEN: "${{steps.config-token.outputs.token}}" shell: bash init-with-min-runners: diff --git a/controllers/actions.github.com/ephemeralrunner_controller.go b/controllers/actions.github.com/ephemeralrunner_controller.go index 48185320..ec67b833 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller.go +++ b/controllers/actions.github.com/ephemeralrunner_controller.go @@ -154,31 +154,17 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, nil } - if !controllerutil.ContainsFinalizer(ephemeralRunner, ephemeralRunnerFinalizerName) { - log.Info("Adding finalizer") + addFinalizers := !controllerutil.ContainsFinalizer(ephemeralRunner, ephemeralRunnerFinalizerName) || !controllerutil.ContainsFinalizer(ephemeralRunner, ephemeralRunnerActionsFinalizerName) + if addFinalizers { + log.Info("Adding finalizers") if err := patch(ctx, r.Client, ephemeralRunner, func(obj *v1alpha1.EphemeralRunner) { controllerutil.AddFinalizer(obj, ephemeralRunnerFinalizerName) + controllerutil.AddFinalizer(obj, ephemeralRunnerActionsFinalizerName) }); err != nil { log.Error(err, "Failed to update with finalizer set") return ctrl.Result{}, err } - - log.Info("Successfully added finalizer") - return ctrl.Result{}, nil - } - - if !controllerutil.ContainsFinalizer(ephemeralRunner, ephemeralRunnerActionsFinalizerName) { - log.Info("Adding runner registration finalizer") - err := patch(ctx, r.Client, ephemeralRunner, func(obj *v1alpha1.EphemeralRunner) { - controllerutil.AddFinalizer(obj, ephemeralRunnerActionsFinalizerName) - }) - if err != nil { - log.Error(err, "Failed to update with runner registration finalizer set") - return ctrl.Result{}, err - } - - log.Info("Successfully added runner registration finalizer") - return ctrl.Result{}, nil + log.Info("Successfully added finalizers") } secret := new(corev1.Secret) diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go index a23d537a..443863eb 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go @@ -32,9 +32,8 @@ import ( ) const ( - ephemeralRunnerSetTestTimeout = time.Second * 20 - ephemeralRunnerSetTestInterval = time.Millisecond * 250 - ephemeralRunnerSetTestGitHubToken = "gh_token" + ephemeralRunnerSetTestTimeout = time.Second * 20 + ephemeralRunnerSetTestInterval = time.Millisecond * 250 ) func TestPrecomputedConstants(t *testing.T) { @@ -119,8 +118,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { Consistently( func() (int, error) { runnerList := new(v1alpha1.EphemeralRunnerList) - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) - if err != nil { + if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil { return -1, err } @@ -153,8 +151,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { Eventually( func() (int, error) { runnerList := new(v1alpha1.EphemeralRunnerList) - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) - if err != nil { + if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil { return -1, err } @@ -172,8 +169,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { } if refetch { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) - if err != nil { + if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil { return -1, err } } @@ -215,8 +211,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { Eventually( func() (int, error) { runnerList := new(v1alpha1.EphemeralRunnerList) - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) - if err != nil { + if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil { return -1, err } @@ -234,8 +229,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { } if refetch { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) - if err != nil { + if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil { return -1, err } } @@ -253,8 +247,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { Eventually( func() (int, error) { runnerList := new(v1alpha1.EphemeralRunnerList) - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) - if err != nil { + if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil { return -1, err } @@ -299,8 +292,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList := new(v1alpha1.EphemeralRunnerList) Eventually( func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) - if err != nil { + if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil { return -1, err } @@ -326,8 +318,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList := new(v1alpha1.EphemeralRunnerList) Eventually( func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) - if err != nil { + if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil { return -1, err } @@ -351,7 +342,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList = new(v1alpha1.EphemeralRunnerList) Eventually( func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -378,7 +369,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList := new(v1alpha1.EphemeralRunnerList) Eventually( func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -403,7 +394,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList = new(v1alpha1.EphemeralRunnerList) Eventually( func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -429,7 +420,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList = new(v1alpha1.EphemeralRunnerList) Eventually( func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -456,7 +447,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList := new(v1alpha1.EphemeralRunnerList) Eventually( func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -481,7 +472,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList = new(v1alpha1.EphemeralRunnerList) Consistently( func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -508,7 +499,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList := new(v1alpha1.EphemeralRunnerList) Eventually( func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -545,7 +536,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { // We should have 3 runners, and have no Succeeded ones Eventually( func() error { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return err } @@ -582,7 +573,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList := new(v1alpha1.EphemeralRunnerList) Eventually( func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -607,7 +598,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList = new(v1alpha1.EphemeralRunnerList) Eventually( func() error { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return err } @@ -648,7 +639,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { // We should have 1 runner up and pending Eventually( func() error { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return err } @@ -675,7 +666,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { Eventually( func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -702,7 +693,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList := new(v1alpha1.EphemeralRunnerList) Eventually( func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -728,7 +719,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList = new(v1alpha1.EphemeralRunnerList) Eventually( func() error { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return err } @@ -772,8 +763,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList = new(v1alpha1.EphemeralRunnerList) Consistently( func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) - if err != nil { + if err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace); err != nil { return -1, err } @@ -799,7 +789,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList = new(v1alpha1.EphemeralRunnerList) Eventually( func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -826,7 +816,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList := new(v1alpha1.EphemeralRunnerList) Eventually( func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -853,7 +843,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList = new(v1alpha1.EphemeralRunnerList) Eventually( func() error { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return err } @@ -897,7 +887,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList = new(v1alpha1.EphemeralRunnerList) Eventually( func() error { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return err } @@ -938,7 +928,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { runnerList := new(v1alpha1.EphemeralRunnerList) Eventually( func() (bool, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return false, err } @@ -1046,7 +1036,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { Eventually( func() (int, error) { runnerList = new(v1alpha1.EphemeralRunnerList) - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -1208,7 +1198,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func( Eventually(func(g Gomega) { runnerList := new(v1alpha1.EphemeralRunnerList) - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) g.Expect(err).NotTo(HaveOccurred(), "failed to list EphemeralRunners") for _, runner := range runnerList.Items { @@ -1226,7 +1216,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func( Eventually( func(g Gomega) (int, error) { runnerList := new(v1alpha1.EphemeralRunnerList) - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -1245,7 +1235,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func( } if refetch { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -1260,6 +1250,18 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func( err = k8sClient.Delete(ctx, ephemeralRunnerSet) Expect(err).NotTo(HaveOccurred(), "failed to delete EphemeralRunnerSet") + Eventually(func(g Gomega) (int, error) { + runnerList := new(v1alpha1.EphemeralRunnerList) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) + if err != nil { + return -1, err + } + return len(runnerList.Items), nil + }, + ephemeralRunnerSetTestTimeout, + ephemeralRunnerSetTestInterval, + ).Should(BeEquivalentTo(0), "EphemeralRunners should be deleted") + // Assert that the proxy secret is deleted Eventually(func(g Gomega) { proxySecret := &corev1.Secret{} @@ -1343,7 +1345,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func( runnerList := new(v1alpha1.EphemeralRunnerList) Eventually(func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -1490,7 +1492,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with custom root CA", func( runnerList := new(v1alpha1.EphemeralRunnerList) Eventually(func() (int, error) { - err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + err := listEphemeralRunnersAndRemoveFinalizers(ctx, k8sClient, runnerList, ephemeralRunnerSet.Namespace) if err != nil { return -1, err } @@ -1529,3 +1531,27 @@ var _ = Describe("Test EphemeralRunnerSet controller with custom root CA", func( ).Should(BeTrue(), "server was not called") }) }) + +// helper function to remove ephemeral runners since in the test, ephemeral runner reconciler is not started +func listEphemeralRunnersAndRemoveFinalizers(ctx context.Context, k8sClient client.Client, list *v1alpha1.EphemeralRunnerList, namespace string) error { + err := k8sClient.List(ctx, list, client.InNamespace(namespace)) + if err != nil { + return err + } + + // Since we are not starting ephemeral runner reconciler, ignore + liveItems := make([]v1alpha1.EphemeralRunner, 0) + for _, item := range list.Items { + if !item.DeletionTimestamp.IsZero() { + if err := patch(ctx, k8sClient, &item, func(runner *v1alpha1.EphemeralRunner) { + runner.Finalizers = []string{} + }); err != nil { + return err + } + continue + } + liveItems = append(liveItems, item) + } + list.Items = liveItems + return nil +} diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index 6b9ef880..bf188d6d 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/controllers/actions.github.com/resourcebuilder.go @@ -572,12 +572,15 @@ func (b *ResourceBuilder) newEphemeralRunner(ephemeralRunnerSet *v1alpha1.Epheme annotations[AnnotationKeyPatchID] = strconv.Itoa(ephemeralRunnerSet.Spec.PatchID) return &v1alpha1.EphemeralRunner{ - TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ GenerateName: ephemeralRunnerSet.Name + "-runner-", Namespace: ephemeralRunnerSet.Namespace, Labels: labels, Annotations: annotations, + Finalizers: []string{ + ephemeralRunnerFinalizerName, + ephemeralRunnerActionsFinalizerName, + }, OwnerReferences: []metav1.OwnerReference{ { APIVersion: ephemeralRunnerSet.GetObjectKind().GroupVersionKind().GroupVersion().String(), diff --git a/hack/e2e-test.sh b/hack/e2e-test.sh index d532662e..fa0b000c 100755 --- a/hack/e2e-test.sh +++ b/hack/e2e-test.sh @@ -11,11 +11,12 @@ export PLATFORMS="linux/amd64" TARGETS=() function set_targets() { - local cases="$(find "${TEST_DIR}" -name '*.test.sh' | sed "s#^${TEST_DIR}/##g" )" + local cases + cases="$(find "${TEST_DIR}" -name '*.test.sh' | sed "s#^${TEST_DIR}/##g")" mapfile -t TARGETS < <(echo "${cases}") - echo $TARGETS + echo "${TARGETS[@]}" } function env_test() { @@ -89,4 +90,4 @@ function main() { fi } -main $@ +main "$@" diff --git a/test/actions.github.com/anonymous-proxy-setup.test.sh b/test/actions.github.com/anonymous-proxy-setup.test.sh index fb79fb10..a60e51c6 100755 --- a/test/actions.github.com/anonymous-proxy-setup.test.sh +++ b/test/actions.github.com/anonymous-proxy-setup.test.sh @@ -41,7 +41,7 @@ function install_squid() { function install_scale_set() { echo "Installing scale set ${SCALE_SET_NAMESPACE}/${SCALE_SET_NAME}" - echo helm install "${SCALE_SET_NAME}" \ + helm install "${SCALE_SET_NAME}" \ --namespace "${SCALE_SET_NAMESPACE}" \ --create-namespace \ --set githubConfigUrl="https://github.com/${TARGET_ORG}/${TARGET_REPO}" \