From b5a183de293c88ae38ec38cf2c1847228273bef4 Mon Sep 17 00:00:00 2001 From: Jerop Date: Fri, 24 Jun 2022 09:12:09 -0400 Subject: [PATCH] TEP-0090: Matrix - Implement `isSuccessful` for `Runs` [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 implement the `isSuccessful` member function of `ResolvedPipelineRunTask` for matrixed `Runs`. If the `ResolvedPipelineRunTask` is matrixed, it is successful only if all of its `Runs` completed successfully. [tep-0090]: https://github.com/tektoncd/community/blob/main/teps/0090-matrix.md --- .../resources/pipelinerunresolution.go | 11 ++ .../resources/pipelinerunresolution_test.go | 143 ++++++++++++++++++ 2 files changed, 154 insertions(+) diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index 1a6c852707d..35bdf6b1a03 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -66,6 +66,7 @@ type ResolvedPipelineTask struct { CustomTask bool RunName string Run *v1alpha1.Run + Runs []*v1alpha1.Run PipelineTask *v1beta1.PipelineTask ResolvedTaskResources *resources.ResolvedTaskResources } @@ -108,6 +109,16 @@ func (t ResolvedPipelineTask) IsMatrixed() bool { // If the PipelineTask has a Matrix, isSuccessful returns true if all runs have completed successfully func (t ResolvedPipelineTask) isSuccessful() bool { switch { + case t.IsCustomTask() && t.IsMatrixed(): + if len(t.Runs) == 0 { + return false + } + for _, run := range t.Runs { + if !run.IsSuccessful() { + return false + } + } + return true case t.IsCustomTask(): return t.Run.IsSuccessful() case t.IsMatrixed(): diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 7b565a3ee5a..57d70c40aac 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -3316,6 +3316,13 @@ func TestIsSuccessful(t *testing.T) { PipelineTask: matrixedPipelineTask, }, want: false, + }, { + name: "matrixed runs not started", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: matrixedPipelineTask, + }, + want: false, }, { name: "matrixed taskruns running", rpt: ResolvedPipelineTask{ @@ -3323,6 +3330,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{makeStarted(trs[0]), makeStarted(trs[1])}, }, want: false, + }, { + name: "matrixed runs running", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: matrixedPipelineTask, + Runs: []*v1alpha1.Run{makeRunStarted(runs[0]), makeRunStarted(runs[1])}, + }, + want: false, }, { name: "one matrixed taskrun running", rpt: ResolvedPipelineTask{ @@ -3330,6 +3345,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{makeStarted(trs[0]), makeSucceeded(trs[1])}, }, want: false, + }, { + name: "one matrixed run running", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: matrixedPipelineTask, + Runs: []*v1alpha1.Run{makeRunStarted(runs[0]), makeRunSucceeded(runs[1])}, + }, + want: false, }, { name: "matrixed taskruns succeeded", rpt: ResolvedPipelineTask{ @@ -3337,6 +3360,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{makeSucceeded(trs[0]), makeSucceeded(trs[1])}, }, want: true, + }, { + name: "matrixed runs succeeded", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: matrixedPipelineTask, + Runs: []*v1alpha1.Run{makeRunSucceeded(runs[0]), makeRunSucceeded(runs[1])}, + }, + want: true, }, { name: "one matrixed taskrun succeeded", rpt: ResolvedPipelineTask{ @@ -3344,6 +3375,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{makeSucceeded(trs[0]), makeStarted(trs[1])}, }, want: false, + }, { + name: "one matrixed run succeeded", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: matrixedPipelineTask, + Runs: []*v1alpha1.Run{makeRunSucceeded(runs[0]), makeRunStarted(runs[1])}, + }, + want: false, }, { name: "matrixed taskruns failed", rpt: ResolvedPipelineTask{ @@ -3351,6 +3390,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{makeFailed(trs[0]), makeFailed(trs[1])}, }, want: false, + }, { + name: "matrixed runs failed", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: matrixedPipelineTask, + Runs: []*v1alpha1.Run{makeRunFailed(runs[0]), makeRunFailed(runs[1])}, + }, + want: false, }, { name: "one matrixed taskrun failed, one matrixed taskrun running", rpt: ResolvedPipelineTask{ @@ -3358,6 +3405,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{makeFailed(trs[0]), makeStarted(trs[1])}, }, want: false, + }, { + name: "one matrixed run failed, one matrixed run running", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: matrixedPipelineTask, + Runs: []*v1alpha1.Run{makeRunFailed(runs[0]), makeRunStarted(runs[1])}, + }, + want: false, }, { name: "matrixed taskruns failed: retries remaining", rpt: ResolvedPipelineTask{ @@ -3365,6 +3420,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{makeFailed(trs[0]), makeFailed(trs[1])}, }, want: false, + }, { + name: "matrixed runs failed: retries remaining", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: withPipelineTaskRetries(*matrixedPipelineTask, 1), + Runs: []*v1alpha1.Run{makeRunFailed(runs[0]), makeRunFailed(runs[1])}, + }, + want: false, }, { name: "matrixed taskruns failed: one taskrun with retries remaining", rpt: ResolvedPipelineTask{ @@ -3372,6 +3435,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{makeFailed(trs[0]), withRetries(makeFailed(trs[1]))}, }, want: false, + }, { + name: "matrixed runs failed: one run with retries remaining", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: withPipelineTaskRetries(*matrixedPipelineTask, 1), + Runs: []*v1alpha1.Run{makeRunFailed(runs[0]), withRunRetries(makeRunFailed(runs[1]))}, + }, + want: false, }, { name: "matrixed taskruns failed: no retries remaining", rpt: ResolvedPipelineTask{ @@ -3379,6 +3450,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{withRetries(makeFailed(trs[0])), withRetries(makeFailed(trs[1]))}, }, want: false, + }, { + name: "matrixed runs failed: no retries remaining", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: withPipelineTaskRetries(*matrixedPipelineTask, 1), + Runs: []*v1alpha1.Run{withRunRetries(makeRunFailed(runs[0])), withRunRetries(makeRunFailed(runs[1]))}, + }, + want: false, }, { name: "matrixed taskruns cancelled", rpt: ResolvedPipelineTask{ @@ -3386,6 +3465,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{withCancelled(makeFailed(trs[0])), withCancelled(makeFailed(trs[1]))}, }, want: false, + }, { + name: "matrixed runs cancelled", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: matrixedPipelineTask, + Runs: []*v1alpha1.Run{withRunCancelled(makeRunFailed(runs[0])), withRunCancelled(makeRunFailed(runs[1]))}, + }, + want: false, }, { name: "one matrixed taskrun cancelled, one matrixed taskrun running", rpt: ResolvedPipelineTask{ @@ -3393,6 +3480,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{withCancelled(makeFailed(trs[0])), makeStarted(trs[1])}, }, want: false, + }, { + name: "one matrixed run cancelled, one matrixed run running", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: matrixedPipelineTask, + Runs: []*v1alpha1.Run{withRunCancelled(makeRunFailed(runs[0])), makeRunStarted(runs[1])}, + }, + want: false, }, { name: "matrixed taskruns cancelled but not failed", rpt: ResolvedPipelineTask{ @@ -3400,6 +3495,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{withCancelled(newTaskRun(trs[0])), withCancelled(newTaskRun(trs[1]))}, }, want: false, + }, { + name: "matrixed runs cancelled but not failed", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: matrixedPipelineTask, + Runs: []*v1alpha1.Run{withRunCancelled(newRun(runs[0])), withRunCancelled(newRun(runs[1]))}, + }, + want: false, }, { name: "one matrixed taskrun cancelled but not failed", rpt: ResolvedPipelineTask{ @@ -3407,6 +3510,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{withCancelled(newTaskRun(trs[0])), makeStarted(trs[1])}, }, want: false, + }, { + name: "one matrixed run cancelled but not failed", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: matrixedPipelineTask, + Runs: []*v1alpha1.Run{withRunCancelled(newRun(runs[0])), makeRunStarted(runs[1])}, + }, + want: false, }, { name: "matrixed taskruns cancelled: retries remaining", rpt: ResolvedPipelineTask{ @@ -3414,6 +3525,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{withCancelled(makeFailed(trs[0])), withCancelled(makeFailed(trs[1]))}, }, want: false, + }, { + name: "matrixed runs cancelled: retries remaining", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: withPipelineTaskRetries(*matrixedPipelineTask, 1), + Runs: []*v1alpha1.Run{withRunCancelled(makeRunFailed(runs[0])), withRunCancelled(makeRunFailed(runs[1]))}, + }, + want: false, }, { name: "one matrixed taskrun cancelled: retries remaining, one matrixed taskrun running", rpt: ResolvedPipelineTask{ @@ -3421,6 +3540,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{withCancelled(makeFailed(trs[0])), makeStarted(trs[1])}, }, want: false, + }, { + name: "one matrixed run cancelled: retries remaining, one matrixed run running", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: withPipelineTaskRetries(*matrixedPipelineTask, 1), + Runs: []*v1alpha1.Run{withRunCancelled(makeRunFailed(runs[0])), makeRunStarted(runs[1])}, + }, + want: false, }, { name: "matrixed taskruns cancelled: no retries remaining", rpt: ResolvedPipelineTask{ @@ -3428,6 +3555,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{withCancelled(withRetries(makeFailed(trs[0]))), withCancelled(withRetries(makeFailed(trs[1])))}, }, want: false, + }, { + name: "matrixed runs cancelled: no retries remaining", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: withPipelineTaskRetries(*matrixedPipelineTask, 1), + Runs: []*v1alpha1.Run{withRunCancelled(withRunRetries(makeRunFailed(runs[0]))), withRunCancelled(withRunRetries(makeRunFailed(runs[1])))}, + }, + want: false, }, { name: "one matrixed taskrun cancelled: no retries remaining, one matrixed taskrun running", rpt: ResolvedPipelineTask{ @@ -3435,6 +3570,14 @@ func TestIsSuccessful(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{withCancelled(withRetries(makeFailed(trs[0]))), makeStarted(trs[1])}, }, want: false, + }, { + name: "one matrixed run cancelled: no retries remaining, one matrixed run running", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: withPipelineTaskRetries(*matrixedPipelineTask, 1), + Runs: []*v1alpha1.Run{withRunCancelled(withRunRetries(makeRunFailed(runs[0]))), makeRunStarted(runs[1])}, + }, + want: false, }} { t.Run(tc.name, func(t *testing.T) { if got := tc.rpt.isSuccessful(); got != tc.want {