Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Ready from the pod conditions when setting it to the EphemeralRunner #3891

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions controllers/actions.github.com/ephemeralrunner_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
104 changes: 99 additions & 5 deletions controllers/actions.github.com/ephemeralrunner_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down
Loading