TEP-0090: Matrix - Implement isSuccessful

[TEP-0090: Matrix][tep-0090] proposed executing a `PipelineTask` in parallel `TaskRuns` and `Runs` with substitutions from combinations of `Parameters` in a `Matrix`. In this change, we also implement the `isSuccessful` member function of `ResolvedPipelineRunTask`. If the `ResolvedPipelineRunTask` is matrixed, it is successful only if all of its `TaskRuns` completed successfully. [tep-0090]: https://github.com/tektoncd/community/blob/main/teps/0090-matrix.md
tektoncd · Jun 15, 2022 · c3ca510 · c3ca510
1 parent d5579f3
commit c3ca510
Show file tree

Hide file tree

Showing 4 changed files with 147 additions and 3 deletions.
diff --git a/pkg/apis/pipeline/v1alpha1/run_types.go b/pkg/apis/pipeline/v1alpha1/run_types.go
@@ -201,6 +201,9 @@ func (r *Run) HasStarted() bool {
 
 // IsSuccessful returns true if the Run's status indicates that it is done.
 func (r *Run) IsSuccessful() bool {
+	if r == nil {
+		return false
+	}
 	return r.Status.GetCondition(apis.ConditionSucceeded).IsTrue()
 }
 

diff --git a/pkg/apis/pipeline/v1beta1/taskrun_types.go b/pkg/apis/pipeline/v1beta1/taskrun_types.go
@@ -414,6 +414,9 @@ func (tr *TaskRun) HasStarted() bool {
 
 // IsSuccessful returns true if the TaskRun's status indicates that it is done.
 func (tr *TaskRun) IsSuccessful() bool {
+	if tr == nil {
+		return false
+	}
 	return tr.Status.GetCondition(apis.ConditionSucceeded).IsTrue()
 }
 

diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
@@ -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.

diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go
@@ -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 {