Fix L0 test to make it more reliable. (#2178)

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
 	}
 

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
+			Eventually(
-			for i := 0; i < 6; i++ {
+				func() (bool, error) {
-				Eventually(
+					err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, updated)
-					func() (bool, error) {
+					if err != nil {
-						if err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, pod); err != nil {
+						return false, err
-							return false, err
+					}
-						}
-						return true, nil
-					},
-					timeout,
-					interval,
-				).Should(BeEquivalentTo(true))
 
+					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 {

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,