diff --git a/controllers/actions.github.com/ephemeralrunner_controller.go b/controllers/actions.github.com/ephemeralrunner_controller.go index 3ef0306c..b6945d29 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller.go +++ b/controllers/actions.github.com/ephemeralrunner_controller.go @@ -310,6 +310,7 @@ func (r *EphemeralRunnerReconciler) cleanupRunnerLinkedPods(ctx context.Context, } if len(runnerLinkedPodList.Items) == 0 { + log.Info("Runner-linked pods are deleted") return true, nil } @@ -344,6 +345,7 @@ func (r *EphemeralRunnerReconciler) cleanupRunnerLinkedSecrets(ctx context.Conte } if len(runnerLinkedSecretList.Items) == 0 { + log.Info("Runner-linked secrets are deleted") return true, nil } diff --git a/controllers/actions.github.com/ephemeralrunner_controller_test.go b/controllers/actions.github.com/ephemeralrunner_controller_test.go index 42f80809..7f006454 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunner_controller_test.go @@ -2,6 +2,7 @@ package actionsgithubcom import ( "context" + "fmt" "net/http" "time" @@ -22,7 +23,7 @@ import ( const ( gh_token = "gh_token" - timeout = time.Second * 30 + timeout = time.Second * 10 interval = time.Millisecond * 250 runnerImage = "ghcr.io/actions/actions-runner:latest" ) @@ -344,19 +345,6 @@ var _ = Describe("EphemeralRunner", func() { interval, ).Should(BeEquivalentTo(true)) - Eventually( - func() (bool, error) { - secret := new(corev1.Secret) - err = k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, secret) - if err == nil { - return false, nil - } - return kerrors.IsNotFound(err), nil - }, - timeout, - interval, - ).Should(BeEquivalentTo(true)) - Eventually( func() (bool, error) { updated := new(v1alpha1.EphemeralRunner) @@ -458,20 +446,43 @@ var _ = Describe("EphemeralRunner", func() { }) It("It should not re-create pod indefinitely", func() { + updated := new(v1alpha1.EphemeralRunner) pod := new(corev1.Pod) - failures := 0 - for i := 0; i < 6; i++ { - Eventually( - func() (bool, error) { - if err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, pod); err != nil { - return false, err - } - return true, nil - }, - timeout, - interval, - ).Should(BeEquivalentTo(true)) + Eventually( + func() (bool, error) { + err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, updated) + if err != nil { + return false, err + } + err = k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, pod) + if err != nil { + if kerrors.IsNotFound(err) && len(updated.Status.Failures) > 5 { + return true, nil + } + + return false, err + } + + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ + Name: EphemeralRunnerContainerName, + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + ExitCode: 1, + }, + }, + }) + err = k8sClient.Status().Update(ctx, pod) + Expect(err).To(BeNil(), "Failed to update pod status") + return false, fmt.Errorf("pod haven't failed for 5 times.") + }, + timeout, + interval, + ).Should(BeEquivalentTo(true), "we should stop creating pod after 5 failures") + + // In case we still have pod created due to controller-runtime cache delay, mark the container as exited + err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, pod) + if err == nil { pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: EphemeralRunnerContainerName, State: corev1.ContainerState{ @@ -482,19 +493,18 @@ var _ = Describe("EphemeralRunner", func() { }) err := k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil(), "Failed to update pod status") - - failures++ - - updated := new(v1alpha1.EphemeralRunner) - Eventually(func() (bool, error) { - err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, updated) - if err != nil { - return false, err - } - return len(updated.Status.Failures) == failures, nil - }, timeout, interval).Should(BeEquivalentTo(true)) } + // EphemeralRunner should failed with reason TooManyPodFailures + Eventually(func() (string, error) { + err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, updated) + if err != nil { + return "", err + } + return updated.Status.Reason, nil + }, timeout, interval).Should(BeEquivalentTo("TooManyPodFailures"), "Reason should be TooManyPodFailures") + + // EphemeralRunner should not have any pod Eventually(func() (bool, error) { err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, pod) if err == nil { diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go index aa53504b..817e846f 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go @@ -20,7 +20,7 @@ import ( ) const ( - ephemeralRunnerSetTestTimeout = time.Second * 5 + ephemeralRunnerSetTestTimeout = time.Second * 10 ephemeralRunnerSetTestInterval = time.Millisecond * 250 ephemeralRunnerSetTestGitHubToken = "gh_token" ) @@ -172,6 +172,26 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { return -1, err } + // Set status to simulate a configured EphemeralRunner + refetch := false + for i, runner := range runnerList.Items { + if runner.Status.RunnerId == 0 { + updatedRunner := runner.DeepCopy() + updatedRunner.Status.Phase = corev1.PodRunning + updatedRunner.Status.RunnerId = i + 100 + err = k8sClient.Status().Patch(ctx, updatedRunner, client.MergeFrom(&runner)) + Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunner") + refetch = true + } + } + + if refetch { + err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + if err != nil { + return -1, err + } + } + return len(runnerList.Items), nil }, ephemeralRunnerSetTestTimeout, @@ -214,6 +234,26 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { return -1, err } + // Set status to simulate a configured EphemeralRunner + refetch := false + for i, runner := range runnerList.Items { + if runner.Status.RunnerId == 0 { + updatedRunner := runner.DeepCopy() + updatedRunner.Status.Phase = corev1.PodRunning + updatedRunner.Status.RunnerId = i + 100 + err = k8sClient.Status().Patch(ctx, updatedRunner, client.MergeFrom(&runner)) + Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunner") + refetch = true + } + } + + if refetch { + err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + if err != nil { + return -1, err + } + } + return len(runnerList.Items), nil }, ephemeralRunnerSetTestTimeout, @@ -278,20 +318,31 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { return -1, err } + // Set status to simulate a configured EphemeralRunner + refetch := false + for i, runner := range runnerList.Items { + if runner.Status.RunnerId == 0 { + updatedRunner := runner.DeepCopy() + updatedRunner.Status.Phase = corev1.PodRunning + updatedRunner.Status.RunnerId = i + 100 + err = k8sClient.Status().Patch(ctx, updatedRunner, client.MergeFrom(&runner)) + Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunner") + refetch = true + } + } + + if refetch { + err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + if err != nil { + return -1, err + } + } + return len(runnerList.Items), nil }, ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval).Should(BeEquivalentTo(5), "5 EphemeralRunner should be created") - // Set status to simulate a configured EphemeralRunner - for i, runner := range runnerList.Items { - updatedRunner := runner.DeepCopy() - updatedRunner.Status.Phase = corev1.PodRunning - updatedRunner.Status.RunnerId = i + 100 - err = k8sClient.Status().Patch(ctx, updatedRunner, client.MergeFrom(&runner)) - Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunner") - } - // Mark one of the EphemeralRunner as finished finishedRunner := runnerList.Items[4].DeepCopy() finishedRunner.Status.Phase = corev1.PodSucceeded @@ -327,20 +378,31 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { return -1, err } + // Set status to simulate a configured EphemeralRunner + refetch := false + for i, runner := range runnerList.Items { + if runner.Status.RunnerId == 0 { + updatedRunner := runner.DeepCopy() + updatedRunner.Status.Phase = corev1.PodRunning + updatedRunner.Status.RunnerId = i + 100 + err = k8sClient.Status().Patch(ctx, updatedRunner, client.MergeFrom(&runner)) + Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunner") + refetch = true + } + } + + if refetch { + err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + if err != nil { + return -1, err + } + } + return len(runnerList.Items), nil }, ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval).Should(BeEquivalentTo(5), "5 EphemeralRunner should be created") - // Set status to simulate a configured EphemeralRunner - for i, runner := range runnerList.Items { - updatedRunner := runner.DeepCopy() - updatedRunner.Status.Phase = corev1.PodRunning - updatedRunner.Status.RunnerId = i + 100 - err = k8sClient.Status().Patch(ctx, updatedRunner, client.MergeFrom(&runner)) - Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunner") - } - // Scale down the EphemeralRunnerSet updated = created.DeepCopy() updated.Spec.Replicas = 3 @@ -356,6 +418,26 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { return -1, err } + // Set status to simulate a configured EphemeralRunner + refetch := false + for i, runner := range runnerList.Items { + if runner.Status.RunnerId == 0 { + updatedRunner := runner.DeepCopy() + updatedRunner.Status.Phase = corev1.PodRunning + updatedRunner.Status.RunnerId = i + 100 + err = k8sClient.Status().Patch(ctx, updatedRunner, client.MergeFrom(&runner)) + Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunner") + refetch = true + } + } + + if refetch { + err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + if err != nil { + return -1, err + } + } + return len(runnerList.Items), nil }, ephemeralRunnerSetTestTimeout, @@ -387,6 +469,26 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { return -1, err } + // Set status to simulate a configured EphemeralRunner + refetch := false + for i, runner := range runnerList.Items { + if runner.Status.RunnerId == 0 { + updatedRunner := runner.DeepCopy() + updatedRunner.Status.Phase = corev1.PodRunning + updatedRunner.Status.RunnerId = i + 100 + err = k8sClient.Status().Patch(ctx, updatedRunner, client.MergeFrom(&runner)) + Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunner") + refetch = true + } + } + + if refetch { + err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + if err != nil { + return -1, err + } + } + return len(runnerList.Items), nil }, ephemeralRunnerSetTestTimeout, @@ -413,6 +515,26 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { return -1, err } + // Set status to simulate a configured EphemeralRunner + refetch := false + for i, runner := range runnerList.Items { + if runner.Status.RunnerId == 0 { + updatedRunner := runner.DeepCopy() + updatedRunner.Status.Phase = corev1.PodRunning + updatedRunner.Status.RunnerId = i + 100 + err = k8sClient.Status().Patch(ctx, updatedRunner, client.MergeFrom(&runner)) + Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunner") + refetch = true + } + } + + if refetch { + err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + if err != nil { + return -1, err + } + } + return len(runnerList.Items), nil }, ephemeralRunnerSetTestTimeout, @@ -436,6 +558,26 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { return -1, err } + // Set status to simulate a configured EphemeralRunner + refetch := false + for i, runner := range runnerList.Items { + if runner.Status.RunnerId == 0 { + updatedRunner := runner.DeepCopy() + updatedRunner.Status.Phase = corev1.PodRunning + updatedRunner.Status.RunnerId = i + 100 + err = k8sClient.Status().Patch(ctx, updatedRunner, client.MergeFrom(&runner)) + Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunner") + refetch = true + } + } + + if refetch { + err := k8sClient.List(ctx, runnerList, client.InNamespace(ephemeralRunnerSet.Namespace)) + if err != nil { + return -1, err + } + } + return len(runnerList.Items), nil }, ephemeralRunnerSetTestTimeout,