Skip to content

Commit

Permalink
update TaskRun step states after cancellation or timeout
Browse files Browse the repository at this point in the history
  • Loading branch information
danielhelfand authored and tekton-robot committed Aug 27, 2020
1 parent fb296e6 commit fb613dc
Show file tree
Hide file tree
Showing 4 changed files with 243 additions and 11 deletions.
30 changes: 28 additions & 2 deletions pkg/reconciler/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -498,8 +498,9 @@ func (c *Reconciler) failTaskRun(ctx context.Context, tr *v1beta1.TaskRun, reaso
logger.Warn("stopping task run %q because of %q", tr.Name, reason)
tr.Status.MarkResourceFailed(reason, errors.New(message))

completionTime := metav1.Time{Time: time.Now()}
// update tr completed time
tr.Status.CompletionTime = &metav1.Time{Time: time.Now()}
tr.Status.CompletionTime = &completionTime

if tr.Status.PodName == "" {
logger.Warnf("task run %q has no pod running yet", tr.Name)
Expand All @@ -510,11 +511,36 @@ func (c *Reconciler) failTaskRun(ctx context.Context, tr *v1beta1.TaskRun, reaso
// can be reached, for example, by the pod never being schedulable due to limits imposed by
// a namespace's ResourceQuota.
err := c.KubeClientSet.CoreV1().Pods(tr.Namespace).Delete(tr.Status.PodName, &metav1.DeleteOptions{})

if err != nil && !k8serrors.IsNotFound(err) {
logger.Infof("Failed to terminate pod: %v", err)
return err
}

// Update step states for TaskRun on TaskRun object since pod has been deleted for cancel or timeout
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(),
}
step.Running = nil
tr.Status.Steps[i] = step
}

if step.Waiting != nil {
step.Terminated = &corev1.ContainerStateTerminated{
ExitCode: 1,
FinishedAt: completionTime,
Reason: reason.String(),
}
step.Waiting = nil
tr.Status.Steps[i] = step
}
}

return nil
}

Expand Down
194 changes: 188 additions & 6 deletions pkg/reconciler/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3076,12 +3076,13 @@ func TestReconcileWorkspaceWithVolumeClaimTemplate(t *testing.T) {

func TestFailTaskRun(t *testing.T) {
testCases := []struct {
name string
taskRun *v1beta1.TaskRun
pod *corev1.Pod
reason v1beta1.TaskRunReason
message string
expectedStatus apis.Condition
name string
taskRun *v1beta1.TaskRun
pod *corev1.Pod
reason v1beta1.TaskRunReason
message string
expectedStatus apis.Condition
expectedStepStates []v1beta1.StepState
}{{
name: "no-pod-scheduled",
taskRun: tb.TaskRun("test-taskrun-run-failed", tb.TaskRunNamespace("foo"), tb.TaskRunSpec(
Expand Down Expand Up @@ -3120,6 +3121,180 @@ func TestFailTaskRun(t *testing.T) {
Reason: "some reason",
Message: "some message",
},
}, {
name: "step-status-update-cancel",
taskRun: tb.TaskRun("test-taskrun-run-cancel", tb.TaskRunNamespace("foo"), tb.TaskRunSpec(
tb.TaskRunTaskRef(simpleTask.Name),
tb.TaskRunCancelled,
), tb.TaskRunStatus(tb.StatusCondition(apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionUnknown,
}), tb.StepState(
tb.SetStepStateRunning(corev1.ContainerStateRunning{StartedAt: metav1.Time{Time: time.Now()}}),
), tb.PodName("foo-is-bar"))),
pod: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "foo-is-bar",
}},
reason: v1beta1.TaskRunReasonCancelled,
message: "TaskRun test-taskrun-run-cancel was cancelled",
expectedStatus: apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionFalse,
Reason: v1beta1.TaskRunReasonCancelled.String(),
Message: "TaskRun test-taskrun-run-cancel was cancelled",
},
expectedStepStates: []v1beta1.StepState{
{
ContainerState: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1beta1.TaskRunReasonCancelled.String(),
},
},
},
},
}, {
name: "step-status-update-timeout",
taskRun: tb.TaskRun("test-taskrun-run-timeout", tb.TaskRunNamespace("foo"), tb.TaskRunSpec(
tb.TaskRunTaskRef(simpleTask.Name),
tb.TaskRunTimeout(time.Duration(10*time.Second)),
), tb.TaskRunStatus(tb.StatusCondition(apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionUnknown,
}), tb.StepState(
tb.SetStepStateRunning(corev1.ContainerStateRunning{StartedAt: metav1.Time{Time: time.Now()}}),
), tb.PodName("foo-is-bar"))),
pod: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "foo-is-bar",
}},
reason: v1beta1.TaskRunReasonTimedOut,
message: "TaskRun test-taskrun-run-timeout failed to finish within 10s",
expectedStatus: apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionFalse,
Reason: v1beta1.TaskRunReasonTimedOut.String(),
Message: "TaskRun test-taskrun-run-timeout failed to finish within 10s",
},
expectedStepStates: []v1beta1.StepState{
{
ContainerState: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1beta1.TaskRunReasonTimedOut.String(),
},
},
},
},
}, {
name: "step-status-update-multiple-steps",
taskRun: tb.TaskRun("test-taskrun-run-timeout-multiple-steps", tb.TaskRunNamespace("foo"), tb.TaskRunSpec(
tb.TaskRunTaskRef(taskMultipleSteps.Name),
tb.TaskRunTimeout(time.Duration(10*time.Second)),
), tb.TaskRunStatus(tb.StatusCondition(apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionUnknown,
}), tb.StepState(
tb.SetStepStateTerminated(corev1.ContainerStateTerminated{StartedAt: metav1.Time{Time: time.Now()}, FinishedAt: metav1.Time{Time: time.Now()}, Reason: "Completed"}),
), tb.StepState(
tb.SetStepStateRunning(corev1.ContainerStateRunning{StartedAt: metav1.Time{Time: time.Now()}}),
), tb.StepState(
tb.SetStepStateRunning(corev1.ContainerStateRunning{StartedAt: metav1.Time{Time: time.Now()}}),
),
tb.PodName("foo-is-bar"))),
pod: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "foo-is-bar",
}},
reason: v1beta1.TaskRunReasonTimedOut,
message: "TaskRun test-taskrun-run-timeout-multiple-steps failed to finish within 10s",
expectedStatus: apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionFalse,
Reason: v1beta1.TaskRunReasonTimedOut.String(),
Message: "TaskRun test-taskrun-run-timeout-multiple-steps failed to finish within 10s",
},
expectedStepStates: []v1beta1.StepState{
{
ContainerState: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 0,
Reason: "Completed",
},
},
},
{
ContainerState: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1beta1.TaskRunReasonTimedOut.String(),
},
},
},
{
ContainerState: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1beta1.TaskRunReasonTimedOut.String(),
},
},
},
},
}, {
name: "step-status-update-multiple-steps-waiting-state",
taskRun: tb.TaskRun("test-taskrun-run-timeout-multiple-steps-waiting", tb.TaskRunNamespace("foo"), tb.TaskRunSpec(
tb.TaskRunTaskRef(taskMultipleSteps.Name),
tb.TaskRunTimeout(time.Duration(10*time.Second)),
), tb.TaskRunStatus(tb.StatusCondition(apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionUnknown,
}), tb.StepState(
tb.SetStepStateWaiting(corev1.ContainerStateWaiting{Reason: "PodInitializing"}),
), tb.StepState(
tb.SetStepStateWaiting(corev1.ContainerStateWaiting{Reason: "PodInitializing"}),
), tb.StepState(
tb.SetStepStateWaiting(corev1.ContainerStateWaiting{Reason: "PodInitializing"}),
),
tb.PodName("foo-is-bar"))),
pod: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "foo-is-bar",
}},
reason: v1beta1.TaskRunReasonTimedOut,
message: "TaskRun test-taskrun-run-timeout-multiple-steps-waiting failed to finish within 10s",
expectedStatus: apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionFalse,
Reason: v1beta1.TaskRunReasonTimedOut.String(),
Message: "TaskRun test-taskrun-run-timeout-multiple-steps-waiting failed to finish within 10s",
},
expectedStepStates: []v1beta1.StepState{
{
ContainerState: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1beta1.TaskRunReasonTimedOut.String(),
},
},
},
{
ContainerState: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1beta1.TaskRunReasonTimedOut.String(),
},
},
},
{
ContainerState: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
ExitCode: 1,
Reason: v1beta1.TaskRunReasonTimedOut.String(),
},
},
},
},
}}

for _, tc := range testCases {
Expand Down Expand Up @@ -3156,6 +3331,13 @@ func TestFailTaskRun(t *testing.T) {
if d := cmp.Diff(tc.taskRun.Status.GetCondition(apis.ConditionSucceeded), &tc.expectedStatus, ignoreLastTransitionTime); d != "" {
t.Fatalf(diff.PrintWantGot(d))
}

if tc.expectedStepStates != nil {
ignoreTerminatedFields := cmpopts.IgnoreFields(corev1.ContainerStateTerminated{}, "StartedAt", "FinishedAt")
if c := cmp.Diff(tc.expectedStepStates, tc.taskRun.Status.Steps, ignoreTerminatedFields); c != "" {
t.Errorf("test %s failed: %s", tc.name, diff.PrintWantGot(c))
}
}
})
}
}
Expand Down
11 changes: 10 additions & 1 deletion test/cancel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

jsonpatch "gomodules.xyz/jsonpatch/v2"

"github.com/google/go-cmp/cmp"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -163,10 +164,18 @@ func TestTaskRunPipelineRunCancel(t *testing.T) {
wg.Add(1)
go func(name string) {
defer wg.Done()
err := WaitForTaskRunState(c, name, FailedWithReason("TaskRunCancelled", name), "TaskRunCancelled")
err := WaitForTaskRunState(c, name, FailedWithReason(v1beta1.TaskRunReasonCancelled.String(), name), v1beta1.TaskRunReasonCancelled.String())
if err != nil {
t.Errorf("Error waiting for TaskRun %s to be finished: %v", name, err)
}
for _, step := range taskrunItem.Status.Steps {
if step.Terminated == nil {
t.Errorf("TaskRun %s step %s does not have a terminated state but should", name, step.Name)
}
if d := cmp.Diff(step.Terminated.Reason, string(v1beta1.TaskRunReasonCancelled)); d != "" {
t.Fatalf("-got, +want: %v", d)
}
}
}(taskrunItem.Name)
}
wg.Wait()
Expand Down
19 changes: 17 additions & 2 deletions test/timeout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -195,13 +196,27 @@ func TestTaskRunTimeout(t *testing.T) {
},
}
if _, err := c.TaskRunClient.Create(taskRun); err != nil {
t.Fatalf("Failed to create TaskRun `%s`: %s", "run-giraffe", err)
t.Fatalf("Failed to create TaskRun `%s`: %s", taskRun.Name, err)
}

t.Logf("Waiting for TaskRun %s in namespace %s to complete", "run-giraffe", namespace)
if err := WaitForTaskRunState(c, "run-giraffe", FailedWithReason("TaskRunTimeout", "run-giraffe"), "TaskRunTimeout"); err != nil {
if err := WaitForTaskRunState(c, taskRun.Name, FailedWithReason(v1beta1.TaskRunReasonTimedOut.String(), taskRun.Name), v1beta1.TaskRunReasonTimedOut.String()); err != nil {
t.Errorf("Error waiting for TaskRun %s to finish: %s", "run-giraffe", err)
}

tr, err := c.TaskRunClient.Get(taskRun.Name, metav1.GetOptions{})
if err != nil {
t.Errorf("Error retrieving TaskRun %s: %v", taskRun.Name, err)
}

for _, step := range tr.Status.Steps {
if step.Terminated == nil {
t.Errorf("TaskRun %s step %s does not have a terminated state but should", taskRun.Name, step.Name)
}
if d := cmp.Diff(step.Terminated.Reason, v1beta1.TaskRunReasonTimedOut.String()); d != "" {
t.Fatalf("-got, +want: %v", d)
}
}
}

func TestPipelineTaskTimeout(t *testing.T) {
Expand Down

0 comments on commit fb613dc

Please sign in to comment.