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

TEP-0090: Matrix - Implement isSuccessful #4980

Merged
merged 1 commit into from
Jun 15, 2022
Merged
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
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1alpha1/run_types.go
Original file line number Diff line number Diff line change
@@ -201,7 +201,7 @@ func (r *Run) HasStarted() bool {

// IsSuccessful returns true if the Run's status indicates that it is done.
func (r *Run) IsSuccessful() bool {
return r.Status.GetCondition(apis.ConditionSucceeded).IsTrue()
return r != nil && r.Status.GetCondition(apis.ConditionSucceeded).IsTrue()
}

// GetRunKey return the run's key for timeout handler map
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1beta1/taskrun_types.go
Original file line number Diff line number Diff line change
@@ -414,7 +414,7 @@ func (tr *TaskRun) HasStarted() bool {

// IsSuccessful returns true if the TaskRun's status indicates that it is done.
func (tr *TaskRun) IsSuccessful() bool {
return tr.Status.GetCondition(apis.ConditionSucceeded).IsTrue()
return tr != nil && tr.Status.GetCondition(apis.ConditionSucceeded).IsTrue()
}

// IsCancelled returns true if the TaskRun's spec status is set to Cancelled state
19 changes: 16 additions & 3 deletions pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
Original file line number Diff line number Diff line change
@@ -100,11 +100,24 @@ func (t ResolvedPipelineRunTask) IsMatrixed() bool {
}

// isSuccessful returns true only if the run has completed successfully
// If the PipelineTask has a Matrix, isSuccessful returns true if all runs have completed successfully
func (t ResolvedPipelineRunTask) isSuccessful() bool {
if t.IsCustomTask() {
return t.Run != nil && t.Run.IsSuccessful()
switch {
case t.IsCustomTask():
return t.Run.IsSuccessful()
case t.IsMatrixed():
if len(t.TaskRuns) == 0 {
return false
}
for _, taskRun := range t.TaskRuns {
if !taskRun.IsSuccessful() {
return false
}
}
return true
default:
return t.TaskRun.IsSuccessful()
}
return t.TaskRun != nil && t.TaskRun.IsSuccessful()
}

// isFailure returns true only if the run has failed and will not be retried.
125 changes: 125 additions & 0 deletions pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go
Original file line number Diff line number Diff line change
@@ -3159,6 +3159,131 @@ func TestIsSuccessful(t *testing.T) {
CustomTask: true,
},
want: false,
}, {
name: "matrixed taskruns not started",
rprt: ResolvedPipelineRunTask{
PipelineTask: matrixedPipelineTask,
},
want: false,
}, {
name: "matrixed taskruns running",
rprt: ResolvedPipelineRunTask{
PipelineTask: matrixedPipelineTask,
TaskRuns: []*v1beta1.TaskRun{makeStarted(trs[0]), makeStarted(trs[1])},
},
want: false,
}, {
name: "one matrixed taskrun running",
rprt: ResolvedPipelineRunTask{
PipelineTask: matrixedPipelineTask,
TaskRuns: []*v1beta1.TaskRun{makeStarted(trs[0]), makeSucceeded(trs[1])},
},
want: false,
}, {
name: "matrixed taskruns succeeded",
rprt: ResolvedPipelineRunTask{
PipelineTask: matrixedPipelineTask,
TaskRuns: []*v1beta1.TaskRun{makeSucceeded(trs[0]), makeSucceeded(trs[1])},
},
want: true,
}, {
name: "one matrixed taskrun succeeded",
rprt: ResolvedPipelineRunTask{
PipelineTask: matrixedPipelineTask,
TaskRuns: []*v1beta1.TaskRun{makeSucceeded(trs[0]), makeStarted(trs[1])},
},
want: false,
}, {
name: "matrixed taskruns failed",
rprt: ResolvedPipelineRunTask{
PipelineTask: matrixedPipelineTask,
TaskRuns: []*v1beta1.TaskRun{makeFailed(trs[0]), makeFailed(trs[1])},
},
want: false,
}, {
name: "one matrixed taskrun failed, one matrixed taskrun running",
rprt: ResolvedPipelineRunTask{
PipelineTask: matrixedPipelineTask,
TaskRuns: []*v1beta1.TaskRun{makeFailed(trs[0]), makeStarted(trs[1])},
},
want: false,
}, {
name: "matrixed taskruns failed: retries remaining",
rprt: ResolvedPipelineRunTask{
PipelineTask: withPipelineTaskRetries(*matrixedPipelineTask, 1),
TaskRuns: []*v1beta1.TaskRun{makeFailed(trs[0]), makeFailed(trs[1])},
},
want: false,
}, {
name: "matrixed taskruns failed: one taskrun with retries remaining",
rprt: ResolvedPipelineRunTask{
PipelineTask: withPipelineTaskRetries(*matrixedPipelineTask, 1),
TaskRuns: []*v1beta1.TaskRun{makeFailed(trs[0]), withRetries(makeFailed(trs[1]))},
},
want: false,
}, {
name: "matrixed taskruns failed: no retries remaining",
rprt: ResolvedPipelineRunTask{
PipelineTask: withPipelineTaskRetries(*matrixedPipelineTask, 1),
TaskRuns: []*v1beta1.TaskRun{withRetries(makeFailed(trs[0])), withRetries(makeFailed(trs[1]))},
},
want: false,
}, {
name: "matrixed taskruns cancelled",
rprt: ResolvedPipelineRunTask{
PipelineTask: matrixedPipelineTask,
TaskRuns: []*v1beta1.TaskRun{withCancelled(makeFailed(trs[0])), withCancelled(makeFailed(trs[1]))},
},
want: false,
}, {
name: "one matrixed taskrun cancelled, one matrixed taskrun running",
rprt: ResolvedPipelineRunTask{
PipelineTask: matrixedPipelineTask,
TaskRuns: []*v1beta1.TaskRun{withCancelled(makeFailed(trs[0])), makeStarted(trs[1])},
},
want: false,
}, {
name: "matrixed taskruns cancelled but not failed",
rprt: ResolvedPipelineRunTask{
PipelineTask: matrixedPipelineTask,
TaskRuns: []*v1beta1.TaskRun{withCancelled(newTaskRun(trs[0])), withCancelled(newTaskRun(trs[1]))},
},
want: false,
}, {
name: "one matrixed taskrun cancelled but not failed",
rprt: ResolvedPipelineRunTask{
PipelineTask: matrixedPipelineTask,
TaskRuns: []*v1beta1.TaskRun{withCancelled(newTaskRun(trs[0])), makeStarted(trs[1])},
},
want: false,
}, {
name: "matrixed taskruns cancelled: retries remaining",
rprt: ResolvedPipelineRunTask{
PipelineTask: withPipelineTaskRetries(*matrixedPipelineTask, 1),
TaskRuns: []*v1beta1.TaskRun{withCancelled(makeFailed(trs[0])), withCancelled(makeFailed(trs[1]))},
},
want: false,
}, {
name: "one matrixed taskrun cancelled: retries remaining, one matrixed taskrun running",
rprt: ResolvedPipelineRunTask{
PipelineTask: withPipelineTaskRetries(*matrixedPipelineTask, 1),
TaskRuns: []*v1beta1.TaskRun{withCancelled(makeFailed(trs[0])), makeStarted(trs[1])},
},
want: false,
}, {
name: "matrixed taskruns cancelled: no retries remaining",
rprt: ResolvedPipelineRunTask{
PipelineTask: withPipelineTaskRetries(*matrixedPipelineTask, 1),
TaskRuns: []*v1beta1.TaskRun{withCancelled(withRetries(makeFailed(trs[0]))), withCancelled(withRetries(makeFailed(trs[1])))},
},
want: false,
}, {
name: "one matrixed taskrun cancelled: no retries remaining, one matrixed taskrun running",
rprt: ResolvedPipelineRunTask{
PipelineTask: withPipelineTaskRetries(*matrixedPipelineTask, 1),
TaskRuns: []*v1beta1.TaskRun{withCancelled(withRetries(makeFailed(trs[0]))), makeStarted(trs[1])},
},
want: false,
}} {
t.Run(tc.name, func(t *testing.T) {
if got := tc.rprt.isSuccessful(); got != tc.want {