diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index 159768f71d3..50f438a82a6 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -108,7 +108,8 @@ func (t ResolvedPipelineRunTask) isSuccessful() bool { } // isFailure returns true only if the run has failed and will not be retried. -// If the PipelineTask has a Matrix, isFailure returns true if any run has failed and will not be retried. +// If the PipelineTask has a Matrix, isFailure returns true if any run has failed (no remaining retries) +// and all other runs are done. func (t ResolvedPipelineRunTask) isFailure() bool { if t.isCancelled() { return true @@ -130,14 +131,14 @@ func (t ResolvedPipelineRunTask) isFailure() bool { if len(t.TaskRuns) == 0 { return false } + isDone = true + atLeastOneFailed := false for _, taskRun := range t.TaskRuns { - c = taskRun.Status.GetCondition(apis.ConditionSucceeded) - isDone = taskRun.IsDone() - if isDone && c.IsFalse() && !t.hasRemainingRetries() { - return true - } + isDone = isDone && taskRun.IsDone() + taskRunFailed := taskRun.Status.GetCondition(apis.ConditionSucceeded).IsFalse() && !t.hasRemainingRetries() + atLeastOneFailed = atLeastOneFailed || taskRunFailed } - return false + return atLeastOneFailed && isDone default: if t.TaskRun == nil { return false @@ -180,6 +181,7 @@ func (t ResolvedPipelineRunTask) hasRemainingRetries() bool { } // isCancelled returns true only if the run is cancelled +// If the PipelineTask has a Matrix, isCancelled returns true if any run is cancelled and all other runs are done. func (t ResolvedPipelineRunTask) isCancelled() bool { switch { case t.IsCustomTask(): @@ -192,14 +194,15 @@ func (t ResolvedPipelineRunTask) isCancelled() bool { if len(t.TaskRuns) == 0 { return false } - // is cancelled when any TaskRun is cancelled to fail fast + isDone := true + atLeastOneCancelled := false for _, taskRun := range t.TaskRuns { + isDone = isDone && taskRun.IsDone() c := taskRun.Status.GetCondition(apis.ConditionSucceeded) - if c != nil && c.IsFalse() && c.Reason == v1beta1.TaskRunReasonCancelled.String() { - return true - } + taskRunCancelled := c.IsFalse() && c.Reason == v1beta1.TaskRunReasonCancelled.String() + atLeastOneCancelled = atLeastOneCancelled || taskRunCancelled } - return false + return atLeastOneCancelled && isDone default: if t.TaskRun == nil { return false diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index e4481a0f238..daff9f0d886 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -1120,12 +1120,12 @@ func TestIsFailure(t *testing.T) { }, want: true, }, { - name: "one matrixed taskruns failed", + name: "one matrixed taskrun failed, one matrixed taskrun running", rprt: ResolvedPipelineRunTask{ PipelineTask: matrixedPipelineTask, TaskRuns: []*v1beta1.TaskRun{makeFailed(trs[0]), makeStarted(trs[1])}, }, - want: true, + want: false, }, { name: "matrixed taskruns failed: retries remaining", rprt: ResolvedPipelineRunTask{ @@ -1155,12 +1155,12 @@ func TestIsFailure(t *testing.T) { }, want: true, }, { - name: "one matrixed taskrun cancelled", + name: "one matrixed taskrun cancelled, one matrixed taskrun running", rprt: ResolvedPipelineRunTask{ PipelineTask: matrixedPipelineTask, TaskRuns: []*v1beta1.TaskRun{withCancelled(makeFailed(trs[0])), makeStarted(trs[1])}, }, - want: true, + want: false, }, { name: "matrixed taskruns cancelled but not failed", rprt: ResolvedPipelineRunTask{ @@ -1183,12 +1183,12 @@ func TestIsFailure(t *testing.T) { }, want: true, }, { - name: "one matrixed taskrun cancelled: retries remaining", + 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: true, + want: false, }, { name: "matrixed taskruns cancelled: no retries remaining", rprt: ResolvedPipelineRunTask{ @@ -1197,12 +1197,12 @@ func TestIsFailure(t *testing.T) { }, want: true, }, { - name: "one matrixed taskrun cancelled: no retries remaining", + 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: true, + want: false, }} { t.Run(tc.name, func(t *testing.T) { if got := tc.rprt.isFailure(); got != tc.want {