Skip to content

Commit

Permalink
Set TaskRun Completion time in case of Failure or Success 🏁
Browse files Browse the repository at this point in the history
Before this change, the completion time of a TaskRun is only set in
case of a timeout, which is not quite right.

Signed-off-by: Vincent Demeester <[email protected]>
  • Loading branch information
vdemeester authored and tekton-robot committed Apr 8, 2019
1 parent 308f3ea commit 395bc32
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 1 deletion.
4 changes: 4 additions & 0 deletions pkg/reconciler/v1alpha1/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,8 @@ func updateStatusFromPod(taskRun *v1alpha1.TaskRun, pod *corev1.Pod) {
Status: corev1.ConditionFalse,
Message: msg,
})
// update tr completed time
taskRun.Status.CompletionTime = &metav1.Time{Time: time.Now()}
case corev1.PodPending:
msg := getWaitingMessage(pod)
taskRun.Status.SetCondition(&duckv1alpha1.Condition{
Expand All @@ -374,6 +376,8 @@ func updateStatusFromPod(taskRun *v1alpha1.TaskRun, pod *corev1.Pod) {
Type: duckv1alpha1.ConditionSucceeded,
Status: corev1.ConditionTrue,
})
// update tr completed time
taskRun.Status.CompletionTime = &metav1.Time{Time: time.Now()}
}
}

Expand Down
17 changes: 16 additions & 1 deletion pkg/reconciler/v1alpha1/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -994,6 +994,13 @@ func TestUpdateStatusFromPod(t *testing.T) {
Status: corev1.ConditionUnknown,
Reason: "Building",
}
// ignoreFields := cmpopts.IgnoreFields(v1alpha1.TaskRunStatus{}, "CompletionTime")
compareCompletionTime := cmp.Comparer(func(x, y *metav1.Time) bool {
if x == nil {
return y == nil
}
return y != nil
})
for _, c := range []struct {
desc string
podStatus corev1.PodStatus
Expand Down Expand Up @@ -1062,6 +1069,8 @@ func TestUpdateStatusFromPod(t *testing.T) {
want: v1alpha1.TaskRunStatus{
Conditions: []duckv1alpha1.Condition{conditionTrue},
Steps: []v1alpha1.StepState{},
// We don't actually care about the time, just that it's not nil
CompletionTime: &metav1.Time{Time: time.Now()},
},
}, {
desc: "running",
Expand Down Expand Up @@ -1099,6 +1108,8 @@ func TestUpdateStatusFromPod(t *testing.T) {
ExitCode: 123,
}},
}},
// We don't actually care about the time, just that it's not nil
CompletionTime: &metav1.Time{Time: time.Now()},
},
}, {
desc: "failure-message",
Expand All @@ -1113,6 +1124,8 @@ func TestUpdateStatusFromPod(t *testing.T) {
Message: "boom",
}},
Steps: []v1alpha1.StepState{},
// We don't actually care about the time, just that it's not nil
CompletionTime: &metav1.Time{Time: time.Now()},
},
}, {
desc: "failure-unspecified",
Expand All @@ -1124,6 +1137,8 @@ func TestUpdateStatusFromPod(t *testing.T) {
Message: "build failed for unspecified reasons.",
}},
Steps: []v1alpha1.StepState{},
// We don't actually care about the time, just that it's not nil
CompletionTime: &metav1.Time{Time: time.Now()},
},
}, {
desc: "pending-waiting-message",
Expand Down Expand Up @@ -1220,7 +1235,7 @@ func TestUpdateStatusFromPod(t *testing.T) {
c.want.PodName = "pod"
c.want.StartTime = &now

if d := cmp.Diff(tr.Status, c.want, ignoreVolatileTime); d != "" {
if d := cmp.Diff(tr.Status, c.want, ignoreVolatileTime, compareCompletionTime); d != "" {
t.Errorf("Diff:\n%s", d)
}
})
Expand Down

0 comments on commit 395bc32

Please sign in to comment.