From b6f26aa886c03a6fad0f7d00265e0a93a0cbb532 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Thu, 23 Jan 2025 13:00:08 +0100 Subject: [PATCH] Use ready from runner container in pod container statuses for EphemeralRunner --- .../ephemeralrunner_controller.go | 29 ++++- .../ephemeralrunner_controller_test.go | 104 +++++++++++++++++- 2 files changed, 123 insertions(+), 10 deletions(-) diff --git a/controllers/actions.github.com/ephemeralrunner_controller.go b/controllers/actions.github.com/ephemeralrunner_controller.go index b39d35dafb..005f621c16 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller.go +++ b/controllers/actions.github.com/ephemeralrunner_controller.go @@ -712,22 +712,41 @@ func (r *EphemeralRunnerReconciler) updateRunStatusFromPod(ctx context.Context, if pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed { return nil } - if ephemeralRunner.Status.Phase == pod.Status.Phase { + + var ready bool + var lastTransitionTime time.Time + for _, condition := range pod.Status.Conditions { + if condition.Type == corev1.PodReady && condition.LastTransitionTime.After(lastTransitionTime) { + ready = condition.Status == corev1.ConditionTrue + lastTransitionTime = condition.LastTransitionTime.Time + } + } + + phaseChanged := ephemeralRunner.Status.Phase != pod.Status.Phase + readyChanged := ready != ephemeralRunner.Status.Ready + + if !phaseChanged && !readyChanged { return nil } - log.Info("Updating ephemeral runner status with pod phase", "statusPhase", pod.Status.Phase, "statusReason", pod.Status.Reason, "statusMessage", pod.Status.Message) + log.Info( + "Updating ephemeral runner status", + "statusPhase", pod.Status.Phase, + "statusReason", pod.Status.Reason, + "statusMessage", pod.Status.Message, + "ready", ready, + ) err := patchSubResource(ctx, r.Status(), ephemeralRunner, func(obj *v1alpha1.EphemeralRunner) { obj.Status.Phase = pod.Status.Phase - obj.Status.Ready = obj.Status.Ready || (pod.Status.Phase == corev1.PodRunning) + obj.Status.Ready = ready obj.Status.Reason = pod.Status.Reason obj.Status.Message = pod.Status.Message }) if err != nil { - return fmt.Errorf("failed to update runner status for Phase/Reason/Message: %v", err) + return fmt.Errorf("failed to update runner status for Phase/Reason/Message/Ready: %v", err) } - log.Info("Updated ephemeral runner status with pod phase") + log.Info("Updated ephemeral runner status") return nil } diff --git a/controllers/actions.github.com/ephemeralrunner_controller_test.go b/controllers/actions.github.com/ephemeralrunner_controller_test.go index 14c51d43f1..15f7d8aebc 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunner_controller_test.go @@ -379,16 +379,21 @@ var _ = Describe("EphemeralRunner", func() { podCopy := pod.DeepCopy() pod.Status.Phase = phase // set container state to force status update - pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ - Name: EphemeralRunnerContainerName, - State: corev1.ContainerState{}, - }) + pod.Status.ContainerStatuses = append( + pod.Status.ContainerStatuses, + corev1.ContainerStatus{ + Name: EphemeralRunnerContainerName, + State: corev1.ContainerState{}, + }, + ) + err := k8sClient.Status().Patch(ctx, pod, client.MergeFrom(podCopy)) Expect(err).To(BeNil(), "failed to patch pod status") + var updated *v1alpha1.EphemeralRunner Eventually( func() (corev1.PodPhase, error) { - updated := new(v1alpha1.EphemeralRunner) + updated = new(v1alpha1.EphemeralRunner) err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, updated) if err != nil { return "", err @@ -401,6 +406,95 @@ var _ = Describe("EphemeralRunner", func() { } }) + It("It should update ready based on the latest condition", func() { + pod := new(corev1.Pod) + 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 + }).Should(BeEquivalentTo(true)) + + newPod := pod.DeepCopy() + newPod.Status.Conditions = []corev1.PodCondition{ + { + Type: corev1.PodScheduled, + Status: corev1.ConditionTrue, + LastTransitionTime: metav1.Now(), + }, + { + Type: corev1.PodInitialized, + Status: corev1.ConditionTrue, + LastTransitionTime: metav1.Now(), + }, + { + Type: corev1.ContainersReady, + Status: corev1.ConditionTrue, + LastTransitionTime: metav1.Now(), + }, + { + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + LastTransitionTime: metav1.Now(), + }, + } + newPod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ + Name: EphemeralRunnerContainerName, + State: corev1.ContainerState{}, + }) + err := k8sClient.Status().Patch(ctx, newPod, client.MergeFrom(pod)) + Expect(err).To(BeNil(), "failed to patch pod status") + + var er *v1alpha1.EphemeralRunner + Eventually( + func() (bool, error) { + er = new(v1alpha1.EphemeralRunner) + err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, er) + if err != nil { + return false, err + } + return er.Status.Ready, nil + }, + ephemeralRunnerTimeout, + ephemeralRunnerInterval, + ).Should(BeEquivalentTo(true)) + + // Fetch the pod again + Eventually( + func() (bool, error) { + err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, pod) + if err != nil { + return false, err + } + return true, nil + }, + ephemeralRunnerTimeout, + ephemeralRunnerInterval, + ).Should(BeEquivalentTo(true)) + + newPod = pod.DeepCopy() + newPod.Status.Conditions = append(newPod.Status.Conditions, corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionFalse, + LastTransitionTime: metav1.Time{Time: metav1.Now().Add(1 * time.Second)}, + }) + + err = k8sClient.Status().Patch(ctx, newPod, client.MergeFrom(pod)) + Expect(err).To(BeNil(), "expected no errors when updating new pod status") + + Eventually( + func() (bool, error) { + err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, pod) + if err != nil { + return false, err + } + return ephemeralRunner.Status.Ready, nil + }, + ephemeralRunnerTimeout, + ephemeralRunnerInterval, + ).Should(BeEquivalentTo(false)) + }) + It("It should not update phase if container state does not exist", func() { pod := new(corev1.Pod) Eventually(