From bb72558ff21b026afd8935d7d697f6b6f52edfc3 Mon Sep 17 00:00:00 2001 From: Jerop Date: Sun, 26 Jun 2022 20:30:14 -0400 Subject: [PATCH] TEP-0090: Matrix - Implement `isFailure` 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 `isFailure` and other member functions of `ResolvedPipelineRunTask` that `isFailure` uses: `isCancelled` and `hasRemainingRetries`. `isFailure` evaluates to `true` only when there is a failure and there are no running `Runs` in the `rprt`. [tep-0090]: https://github.com/tektoncd/community/blob/main/teps/0090-matrix.md --- .../resources/pipelinerunresolution.go | 37 +++++ .../resources/pipelinerunresolution_test.go | 135 ++++++++++++++++++ 2 files changed, 172 insertions(+) diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index cc8c70460e3..1ba33c2c16d 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -150,6 +150,18 @@ func (t ResolvedPipelineTask) isFailure() bool { var isDone bool switch { + case t.IsCustomTask() && t.IsMatrixed(): + if len(t.Runs) == 0 { + return false + } + isDone = true + atLeastOneFailed := false + for _, run := range t.Runs { + isDone = isDone && run.IsDone() + runFailed := run.Status.GetCondition(apis.ConditionSucceeded).IsFalse() && !t.hasRemainingRetries() + atLeastOneFailed = atLeastOneFailed || runFailed + } + return atLeastOneFailed && isDone case t.IsCustomTask(): if t.Run == nil { return false @@ -183,6 +195,18 @@ func (t ResolvedPipelineTask) isFailure() bool { func (t ResolvedPipelineTask) hasRemainingRetries() bool { var retriesDone int switch { + case t.IsCustomTask() && t.IsMatrixed(): + if len(t.Runs) == 0 { + return true + } + // has remaining retries when any Run has a remaining retry + for _, run := range t.Runs { + retriesDone = len(run.Status.RetriesStatus) + if retriesDone < t.PipelineTask.Retries { + return true + } + } + return false case t.IsCustomTask(): if t.Run == nil { return true @@ -213,6 +237,19 @@ func (t ResolvedPipelineTask) hasRemainingRetries() bool { // If the PipelineTask has a Matrix, isCancelled returns true if any run is cancelled and all other runs are done. func (t ResolvedPipelineTask) isCancelled() bool { switch { + case t.IsCustomTask() && t.IsMatrixed(): + if len(t.Runs) == 0 { + return false + } + isDone := true + atLeastOneCancelled := false + for _, run := range t.Runs { + isDone = isDone && run.IsDone() + c := run.Status.GetCondition(apis.ConditionSucceeded) + runCancelled := c.IsFalse() && c.Reason == v1alpha1.RunReasonCancelled + atLeastOneCancelled = atLeastOneCancelled || runCancelled + } + return atLeastOneCancelled && isDone case t.IsCustomTask(): if t.Run == nil { return false diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index d46d84d4925..e4719187d45 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -1135,6 +1135,13 @@ func TestIsFailure(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{ @@ -1142,6 +1149,14 @@ func TestIsFailure(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{ @@ -1149,6 +1164,14 @@ func TestIsFailure(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{ @@ -1163,6 +1186,14 @@ func TestIsFailure(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{ @@ -1170,6 +1201,14 @@ func TestIsFailure(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{makeFailed(trs[0]), makeFailed(trs[1])}, }, want: true, + }, { + name: "matrixed runs failed", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: matrixedPipelineTask, + Runs: []*v1alpha1.Run{makeRunFailed(runs[0]), makeRunFailed(runs[1])}, + }, + want: true, }, { name: "one matrixed taskrun failed, one matrixed taskrun running", rpt: ResolvedPipelineTask{ @@ -1177,6 +1216,14 @@ func TestIsFailure(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{ @@ -1184,6 +1231,14 @@ func TestIsFailure(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{ @@ -1191,6 +1246,14 @@ func TestIsFailure(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{ @@ -1198,6 +1261,14 @@ func TestIsFailure(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{withRetries(makeFailed(trs[0])), withRetries(makeFailed(trs[1]))}, }, want: true, + }, { + 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: true, }, { name: "matrixed taskruns cancelled", rpt: ResolvedPipelineTask{ @@ -1205,6 +1276,14 @@ func TestIsFailure(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{withCancelled(makeFailed(trs[0])), withCancelled(makeFailed(trs[1]))}, }, want: true, + }, { + name: "matrixed runs cancelled", + rpt: ResolvedPipelineTask{ + CustomTask: true, + PipelineTask: matrixedPipelineTask, + Runs: []*v1alpha1.Run{withRunCancelled(makeRunFailed(runs[0])), withRunCancelled(makeRunFailed(runs[1]))}, + }, + want: true, }, { name: "one matrixed taskrun cancelled, one matrixed taskrun running", rpt: ResolvedPipelineTask{ @@ -1212,6 +1291,14 @@ func TestIsFailure(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{ @@ -1219,6 +1306,14 @@ func TestIsFailure(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{ @@ -1226,6 +1321,14 @@ func TestIsFailure(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{ @@ -1233,6 +1336,14 @@ func TestIsFailure(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{withCancelled(makeFailed(trs[0])), withCancelled(makeFailed(trs[1]))}, }, want: true, + }, { + 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: true, }, { name: "one matrixed taskrun cancelled: retries remaining, one matrixed taskrun running", rpt: ResolvedPipelineTask{ @@ -1240,6 +1351,14 @@ func TestIsFailure(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{ @@ -1247,6 +1366,14 @@ func TestIsFailure(t *testing.T) { TaskRuns: []*v1beta1.TaskRun{withCancelled(withRetries(makeFailed(trs[0]))), withCancelled(withRetries(makeFailed(trs[1])))}, }, want: true, + }, { + 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: true, }, { name: "one matrixed taskrun cancelled: no retries remaining, one matrixed taskrun running", rpt: ResolvedPipelineTask{ @@ -1254,6 +1381,14 @@ func TestIsFailure(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.isFailure(); got != tc.want {