Skip to content

Commit 8c8b3d0

Browse files
danielhelfanddanielhelfand
committed
update TaskRun step states after cancellation or timeout
1 parent 3a7a693 commit 8c8b3d0

File tree

4 files changed

+242
-10
lines changed

4 files changed

+242
-10
lines changed

‎pkg/reconciler/taskrun/taskrun.go

+28-2
Original file line numberDiff line numberDiff line change
@@ -498,8 +498,9 @@ func (c *Reconciler) failTaskRun(ctx context.Context, tr *v1beta1.TaskRun, reaso
498498
logger.Warn("stopping task run %q because of %q", tr.Name, reason)
499499
tr.Status.MarkResourceFailed(reason, errors.New(message))
500500

501+
completionTime := metav1.Time{Time: time.Now()}
501502
// update tr completed time
502-
tr.Status.CompletionTime = &metav1.Time{Time: time.Now()}
503+
tr.Status.CompletionTime = &completionTime
503504

504505
if tr.Status.PodName == "" {
505506
logger.Warnf("task run %q has no pod running yet", tr.Name)
@@ -510,11 +511,36 @@ func (c *Reconciler) failTaskRun(ctx context.Context, tr *v1beta1.TaskRun, reaso
510511
// can be reached, for example, by the pod never being schedulable due to limits imposed by
511512
// a namespace's ResourceQuota.
512513
err := c.KubeClientSet.CoreV1().Pods(tr.Namespace).Delete(tr.Status.PodName, &metav1.DeleteOptions{})
513-
514514
if err != nil && !k8serrors.IsNotFound(err) {
515515
logger.Infof("Failed to terminate pod: %v", err)
516516
return err
517517
}
518+
519+
// Update step states for TaskRun on TaskRun object since pod has been deleted for cancel or timeout
520+
for i, step := range tr.Status.Steps {
521+
// If running, include StartedAt for when step began running
522+
if step.Running != nil {
523+
step.Terminated = &corev1.ContainerStateTerminated{
524+
ExitCode: 1,
525+
StartedAt: step.Running.StartedAt,
526+
FinishedAt: completionTime,
527+
Reason: string(reason),
528+
}
529+
step.Running = nil
530+
tr.Status.Steps[i] = step
531+
}
532+
533+
if step.Waiting != nil {
534+
step.Terminated = &corev1.ContainerStateTerminated{
535+
ExitCode: 1,
536+
FinishedAt: completionTime,
537+
Reason: string(reason),
538+
}
539+
step.Waiting = nil
540+
tr.Status.Steps[i] = step
541+
}
542+
}
543+
518544
return nil
519545
}
520546

‎pkg/reconciler/taskrun/taskrun_test.go

+188-6
Original file line numberDiff line numberDiff line change
@@ -3076,12 +3076,13 @@ func TestReconcileWorkspaceWithVolumeClaimTemplate(t *testing.T) {
30763076

30773077
func TestFailTaskRun(t *testing.T) {
30783078
testCases := []struct {
3079-
name string
3080-
taskRun *v1beta1.TaskRun
3081-
pod *corev1.Pod
3082-
reason v1beta1.TaskRunReason
3083-
message string
3084-
expectedStatus apis.Condition
3079+
name string
3080+
taskRun *v1beta1.TaskRun
3081+
pod *corev1.Pod
3082+
reason v1beta1.TaskRunReason
3083+
message string
3084+
expectedStatus apis.Condition
3085+
expectedStepStates []v1beta1.StepState
30853086
}{{
30863087
name: "no-pod-scheduled",
30873088
taskRun: tb.TaskRun("test-taskrun-run-failed", tb.TaskRunNamespace("foo"), tb.TaskRunSpec(
@@ -3120,6 +3121,180 @@ func TestFailTaskRun(t *testing.T) {
31203121
Reason: "some reason",
31213122
Message: "some message",
31223123
},
3124+
}, {
3125+
name: "step-status-update-cancel",
3126+
taskRun: tb.TaskRun("test-taskrun-run-cancel", tb.TaskRunNamespace("foo"), tb.TaskRunSpec(
3127+
tb.TaskRunTaskRef(simpleTask.Name),
3128+
tb.TaskRunCancelled,
3129+
), tb.TaskRunStatus(tb.StatusCondition(apis.Condition{
3130+
Type: apis.ConditionSucceeded,
3131+
Status: corev1.ConditionUnknown,
3132+
}), tb.StepState(
3133+
tb.SetStepStateRunning(corev1.ContainerStateRunning{StartedAt: metav1.Time{Time: time.Now()}}),
3134+
), tb.PodName("foo-is-bar"))),
3135+
pod: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
3136+
Namespace: "foo",
3137+
Name: "foo-is-bar",
3138+
}},
3139+
reason: "TaskRunCancelled",
3140+
message: "TaskRun test-taskrun-run-cancel was cancelled",
3141+
expectedStatus: apis.Condition{
3142+
Type: apis.ConditionSucceeded,
3143+
Status: corev1.ConditionFalse,
3144+
Reason: "TaskRunCancelled",
3145+
Message: "TaskRun test-taskrun-run-cancel was cancelled",
3146+
},
3147+
expectedStepStates: []v1beta1.StepState{
3148+
{
3149+
ContainerState: corev1.ContainerState{
3150+
Terminated: &corev1.ContainerStateTerminated{
3151+
ExitCode: 1,
3152+
Reason: "TaskRunCancelled",
3153+
},
3154+
},
3155+
},
3156+
},
3157+
}, {
3158+
name: "step-status-update-timeout",
3159+
taskRun: tb.TaskRun("test-taskrun-run-timeout", tb.TaskRunNamespace("foo"), tb.TaskRunSpec(
3160+
tb.TaskRunTaskRef(simpleTask.Name),
3161+
tb.TaskRunTimeout(time.Duration(10*time.Second)),
3162+
), tb.TaskRunStatus(tb.StatusCondition(apis.Condition{
3163+
Type: apis.ConditionSucceeded,
3164+
Status: corev1.ConditionUnknown,
3165+
}), tb.StepState(
3166+
tb.SetStepStateRunning(corev1.ContainerStateRunning{StartedAt: metav1.Time{Time: time.Now()}}),
3167+
), tb.PodName("foo-is-bar"))),
3168+
pod: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
3169+
Namespace: "foo",
3170+
Name: "foo-is-bar",
3171+
}},
3172+
reason: "TaskRunTimeout",
3173+
message: "TaskRun test-taskrun-run-timeout failed to finish within 10s",
3174+
expectedStatus: apis.Condition{
3175+
Type: apis.ConditionSucceeded,
3176+
Status: corev1.ConditionFalse,
3177+
Reason: "TaskRunTimeout",
3178+
Message: "TaskRun test-taskrun-run-timeout failed to finish within 10s",
3179+
},
3180+
expectedStepStates: []v1beta1.StepState{
3181+
{
3182+
ContainerState: corev1.ContainerState{
3183+
Terminated: &corev1.ContainerStateTerminated{
3184+
ExitCode: 1,
3185+
Reason: "TaskRunTimeout",
3186+
},
3187+
},
3188+
},
3189+
},
3190+
}, {
3191+
name: "step-status-update-multiple-steps",
3192+
taskRun: tb.TaskRun("test-taskrun-run-timeout-multiple-steps", tb.TaskRunNamespace("foo"), tb.TaskRunSpec(
3193+
tb.TaskRunTaskRef(taskMultipleSteps.Name),
3194+
tb.TaskRunTimeout(time.Duration(10*time.Second)),
3195+
), tb.TaskRunStatus(tb.StatusCondition(apis.Condition{
3196+
Type: apis.ConditionSucceeded,
3197+
Status: corev1.ConditionUnknown,
3198+
}), tb.StepState(
3199+
tb.SetStepStateTerminated(corev1.ContainerStateTerminated{StartedAt: metav1.Time{Time: time.Now()}, FinishedAt: metav1.Time{Time: time.Now()}, Reason: "Completed"}),
3200+
), tb.StepState(
3201+
tb.SetStepStateRunning(corev1.ContainerStateRunning{StartedAt: metav1.Time{Time: time.Now()}}),
3202+
), tb.StepState(
3203+
tb.SetStepStateRunning(corev1.ContainerStateRunning{StartedAt: metav1.Time{Time: time.Now()}}),
3204+
),
3205+
tb.PodName("foo-is-bar"))),
3206+
pod: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
3207+
Namespace: "foo",
3208+
Name: "foo-is-bar",
3209+
}},
3210+
reason: "TaskRunTimeout",
3211+
message: "TaskRun test-taskrun-run-timeout-multiple-steps failed to finish within 10s",
3212+
expectedStatus: apis.Condition{
3213+
Type: apis.ConditionSucceeded,
3214+
Status: corev1.ConditionFalse,
3215+
Reason: "TaskRunTimeout",
3216+
Message: "TaskRun test-taskrun-run-timeout-multiple-steps failed to finish within 10s",
3217+
},
3218+
expectedStepStates: []v1beta1.StepState{
3219+
{
3220+
ContainerState: corev1.ContainerState{
3221+
Terminated: &corev1.ContainerStateTerminated{
3222+
ExitCode: 0,
3223+
Reason: "Completed",
3224+
},
3225+
},
3226+
},
3227+
{
3228+
ContainerState: corev1.ContainerState{
3229+
Terminated: &corev1.ContainerStateTerminated{
3230+
ExitCode: 1,
3231+
Reason: "TaskRunTimeout",
3232+
},
3233+
},
3234+
},
3235+
{
3236+
ContainerState: corev1.ContainerState{
3237+
Terminated: &corev1.ContainerStateTerminated{
3238+
ExitCode: 1,
3239+
Reason: "TaskRunTimeout",
3240+
},
3241+
},
3242+
},
3243+
},
3244+
}, {
3245+
name: "step-status-update-multiple-steps-waiting-state",
3246+
taskRun: tb.TaskRun("test-taskrun-run-timeout-multiple-steps-waiting", tb.TaskRunNamespace("foo"), tb.TaskRunSpec(
3247+
tb.TaskRunTaskRef(taskMultipleSteps.Name),
3248+
tb.TaskRunTimeout(time.Duration(10*time.Second)),
3249+
), tb.TaskRunStatus(tb.StatusCondition(apis.Condition{
3250+
Type: apis.ConditionSucceeded,
3251+
Status: corev1.ConditionUnknown,
3252+
}), tb.StepState(
3253+
tb.SetStepStateWaiting(corev1.ContainerStateWaiting{Reason: "PodInitializing"}),
3254+
), tb.StepState(
3255+
tb.SetStepStateWaiting(corev1.ContainerStateWaiting{Reason: "PodInitializing"}),
3256+
), tb.StepState(
3257+
tb.SetStepStateWaiting(corev1.ContainerStateWaiting{Reason: "PodInitializing"}),
3258+
),
3259+
tb.PodName("foo-is-bar"))),
3260+
pod: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
3261+
Namespace: "foo",
3262+
Name: "foo-is-bar",
3263+
}},
3264+
reason: "TaskRunTimeout",
3265+
message: "TaskRun test-taskrun-run-timeout-multiple-steps-waiting failed to finish within 10s",
3266+
expectedStatus: apis.Condition{
3267+
Type: apis.ConditionSucceeded,
3268+
Status: corev1.ConditionFalse,
3269+
Reason: "TaskRunTimeout",
3270+
Message: "TaskRun test-taskrun-run-timeout-multiple-steps-waiting failed to finish within 10s",
3271+
},
3272+
expectedStepStates: []v1beta1.StepState{
3273+
{
3274+
ContainerState: corev1.ContainerState{
3275+
Terminated: &corev1.ContainerStateTerminated{
3276+
ExitCode: 1,
3277+
Reason: "TaskRunTimeout",
3278+
},
3279+
},
3280+
},
3281+
{
3282+
ContainerState: corev1.ContainerState{
3283+
Terminated: &corev1.ContainerStateTerminated{
3284+
ExitCode: 1,
3285+
Reason: "TaskRunTimeout",
3286+
},
3287+
},
3288+
},
3289+
{
3290+
ContainerState: corev1.ContainerState{
3291+
Terminated: &corev1.ContainerStateTerminated{
3292+
ExitCode: 1,
3293+
Reason: "TaskRunTimeout",
3294+
},
3295+
},
3296+
},
3297+
},
31233298
}}
31243299

31253300
for _, tc := range testCases {
@@ -3156,6 +3331,13 @@ func TestFailTaskRun(t *testing.T) {
31563331
if d := cmp.Diff(tc.taskRun.Status.GetCondition(apis.ConditionSucceeded), &tc.expectedStatus, ignoreLastTransitionTime); d != "" {
31573332
t.Fatalf(diff.PrintWantGot(d))
31583333
}
3334+
3335+
if tc.expectedStepStates != nil {
3336+
ignoreTerminatedFields := cmpopts.IgnoreFields(corev1.ContainerStateTerminated{}, "StartedAt", "FinishedAt")
3337+
if c := cmp.Diff(tc.expectedStepStates, tc.taskRun.Status.Steps, ignoreTerminatedFields); c != "" {
3338+
t.Errorf("test %s failed: %s", tc.name, diff.PrintWantGot(c))
3339+
}
3340+
}
31593341
})
31603342
}
31613343
}

‎test/cancel_test.go

+9
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626

2727
jsonpatch "gomodules.xyz/jsonpatch/v2"
2828

29+
"github.com/google/go-cmp/cmp"
2930
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
3031
corev1 "k8s.io/api/core/v1"
3132
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -167,6 +168,14 @@ func TestTaskRunPipelineRunCancel(t *testing.T) {
167168
if err != nil {
168169
t.Errorf("Error waiting for TaskRun %s to be finished: %v", name, err)
169170
}
171+
for _, step := range taskrunItem.Status.Steps {
172+
if step.Terminated == nil {
173+
t.Errorf("TaskRun %s step %s does not have a terminated state but should", name, step.Name)
174+
}
175+
if d := cmp.Diff(step.Terminated.Reason, string(v1beta1.TaskRunReasonCancelled)); d != "" {
176+
t.Fatalf("-got, +want: %v", d)
177+
}
178+
}
170179
}(taskrunItem.Name)
171180
}
172181
wg.Wait()

‎test/timeout_test.go

+17-2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"testing"
2525
"time"
2626

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

201202
t.Logf("Waiting for TaskRun %s in namespace %s to complete", "run-giraffe", namespace)
202-
if err := WaitForTaskRunState(c, "run-giraffe", FailedWithReason("TaskRunTimeout", "run-giraffe"), "TaskRunTimeout"); err != nil {
203+
if err := WaitForTaskRunState(c, taskRun.Name, FailedWithReason("TaskRunTimeout", taskRun.Name), "TaskRunTimeout"); err != nil {
203204
t.Errorf("Error waiting for TaskRun %s to finish: %s", "run-giraffe", err)
204205
}
206+
207+
tr, err := c.TaskRunClient.Get(taskRun.Name, metav1.GetOptions{})
208+
if err != nil {
209+
t.Errorf("Error retrieving TaskRun %s: %v", taskRun.Name, err)
210+
}
211+
212+
for _, step := range tr.Status.Steps {
213+
if step.Terminated == nil {
214+
t.Errorf("TaskRun %s step %s does not have a terminated state but should", taskRun.Name, step.Name)
215+
}
216+
if d := cmp.Diff(step.Terminated.Reason, string(v1beta1.TaskRunReasonTimedOut)); d != "" {
217+
t.Fatalf("-got, +want: %v", d)
218+
}
219+
}
205220
}
206221

207222
func TestPipelineTaskTimeout(t *testing.T) {

0 commit comments

Comments
 (0)