Skip to content

Commit

Permalink
Error sweep: refactor steps termination when failing TaskRun
Browse files Browse the repository at this point in the history
This commit refactors the step termination logics once the pod for the
TaskRun is marked failed. It adds to the container termination message
and complete the prerequisites for differentiating pod termination
reason with TaskRunReasons which are currently used to cover container
termination reasons.
  • Loading branch information
JeromeJu authored and tekton-robot committed Nov 28, 2023
1 parent fa865d8 commit 52bb407
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 9 deletions.
25 changes: 16 additions & 9 deletions pkg/reconciler/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -724,25 +724,32 @@ func (c *Reconciler) failTaskRun(ctx context.Context, tr *v1.TaskRun, reason v1.
var err error
if reason == v1.TaskRunReasonCancelled &&
(config.FromContextOrDefaults(ctx).FeatureFlags.EnableKeepPodOnCancel && config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == config.AlphaAPIFields) {
logger.Infof("canceling task run %q by entrypoint", tr.Name)
logger.Infof("Canceling task run %q by entrypoint", tr.Name)
err = podconvert.CancelPod(ctx, c.KubeClientSet, tr.Namespace, tr.Status.PodName)
} else {
err = c.KubeClientSet.CoreV1().Pods(tr.Namespace).Delete(ctx, tr.Status.PodName, metav1.DeleteOptions{})
}
if err != nil && !k8serrors.IsNotFound(err) {
logger.Infof("Failed to terminate pod: %v", err)
logger.Errorf("Failed to terminate pod %s: %v", tr.Status.PodName, err)
return err
}

// Update step states for TaskRun on TaskRun object since pod has been deleted for cancel or timeout
terminateStepsInPod(tr, reason)
return nil
}

// terminateStepsInPod updates step states for TaskRun on TaskRun object since pod has been deleted for cancel or timeout
func terminateStepsInPod(tr *v1.TaskRun, taskRunReason v1.TaskRunReason) {
for i, step := range tr.Status.Steps {
// If running, include StartedAt for when step began running
if step.Running != nil {
step.Terminated = &corev1.ContainerStateTerminated{
ExitCode: 1,
StartedAt: step.Running.StartedAt,
FinishedAt: completionTime,
Reason: reason.String(),
FinishedAt: *tr.Status.CompletionTime,
// TODO(#7385): replace with more pod/container termination reason instead of overloading taskRunReason
Reason: taskRunReason.String(),
Message: fmt.Sprintf("Step %s terminated as pod %s is terminated", step.Name, tr.Status.PodName),
}
step.Running = nil
tr.Status.Steps[i] = step
Expand All @@ -751,15 +758,15 @@ func (c *Reconciler) failTaskRun(ctx context.Context, tr *v1.TaskRun, reason v1.
if step.Waiting != nil {
step.Terminated = &corev1.ContainerStateTerminated{
ExitCode: 1,
FinishedAt: completionTime,
Reason: reason.String(),
FinishedAt: *tr.Status.CompletionTime,
// TODO(#7385): replace with more pod/container termination reason instead of overloading taskRunReason
Reason: taskRunReason.String(),
Message: fmt.Sprintf("Step %s terminated as pod %s is terminated", step.Name, tr.Status.PodName),
}
step.Waiting = nil
tr.Status.Steps[i] = step
}
}

return nil
}

// createPod creates a Pod based on the Task's configuration, with pvcName as a volumeMount
Expand Down
9 changes: 9 additions & 0 deletions pkg/reconciler/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4218,6 +4218,7 @@ status:
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1.TaskRunReasonCancelled.String(),
Message: "Step terminated as pod foo-is-bar is terminated",
},
},
},
Expand Down Expand Up @@ -4263,6 +4264,7 @@ status:
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1.TaskRunReasonCancelled.String(),
Message: "Step terminated as pod foo-is-bar is terminated",
},
},
},
Expand Down Expand Up @@ -4304,6 +4306,7 @@ status:
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1.TaskRunReasonTimedOut.String(),
Message: "Step terminated as pod foo-is-bar is terminated",
},
},
},
Expand Down Expand Up @@ -4360,6 +4363,7 @@ status:
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1.TaskRunReasonTimedOut.String(),
Message: "Step terminated as pod foo-is-bar is terminated",
},
},
},
Expand All @@ -4368,6 +4372,7 @@ status:
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1.TaskRunReasonTimedOut.String(),
Message: "Step terminated as pod foo-is-bar is terminated",
},
},
},
Expand Down Expand Up @@ -4413,6 +4418,7 @@ status:
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1.TaskRunReasonTimedOut.String(),
Message: "Step terminated as pod foo-is-bar is terminated",
},
},
},
Expand All @@ -4421,6 +4427,7 @@ status:
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1.TaskRunReasonTimedOut.String(),
Message: "Step terminated as pod foo-is-bar is terminated",
},
},
},
Expand All @@ -4429,6 +4436,7 @@ status:
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1.TaskRunReasonTimedOut.String(),
Message: "Step terminated as pod foo-is-bar is terminated",
},
},
},
Expand Down Expand Up @@ -4482,6 +4490,7 @@ status:
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1.TaskRunReasonTimedOut.String(),
Message: "Step terminated as pod foo-is-bar is terminated",
},
},
},
Expand Down

0 comments on commit 52bb407

Please sign in to comment.