From f57d553d2398a8e6e97603e1ddd29604019480ec Mon Sep 17 00:00:00 2001 From: Andrea Frittoli <andrea.frittoli@uk.ibm.com> Date: Sun, 7 Jun 2020 11:38:22 +0100 Subject: [PATCH] Set pipeline status when all tasks complete We used to set the pipeline status to failed as soon as the first task in the pipeline failed or was cancelled. As soon as the first task in the pipeline fails or is cancelled, we stop scheduling new tasks, as we did before, but we will report status Unknown until all Tasks are complete. This allows to: - the completion time at the same time that the status is set and avoid inconsistencies - wait until all tasks are complete before we cleanup the pipeline artifact storage, affinity assistant and record metrics - report the correct number of failed / cancelled tasks, as there may be more than one. Other tasks that were already running when the first failure happened may fail too - prepare the pipeline controller more complex workflows, where the controller may continue working scheduling after failures --- docs/events.md | 2 + pkg/reconciler/pipelinerun/pipelinerun.go | 60 ++-- .../resources/pipelinerunresolution.go | 119 +++++--- .../resources/pipelinerunresolution_test.go | 286 ++++++++++++------ 4 files changed, 305 insertions(+), 162 deletions(-) diff --git a/docs/events.md b/docs/events.md index add381754cc..a0d9302d272 100644 --- a/docs/events.md +++ b/docs/events.md @@ -16,6 +16,7 @@ No events are emitted for `Conditions` today (https://github.com/tektoncd/pipeli ## TaskRuns `TaskRun` events are generated for the following `Reasons`: + - `Started`: this is triggered the first time the `TaskRun` is picked by the reconciler from its work queue, so it only happens if web-hook validation was successful. Note that this event does not imply that a step started executing, @@ -32,6 +33,7 @@ No events are emitted for `Conditions` today (https://github.com/tektoncd/pipeli ## PipelineRuns `PipelineRun` events are generated for the following `Reasons`: + - `Succeeded`: this is triggered once all `Tasks` reachable via the DAG are executed successfully. - `Failed`: this is triggered if the `PipelineRun` is completed, but not diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 8d0998fa6aa..cbd9f237da9 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -425,9 +425,48 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun) err } } + as, err := artifacts.InitializeArtifactStorage(c.Images, pr, pipelineSpec, c.KubeClientSet, c.Logger) + if err != nil { + c.Logger.Infof("PipelineRun failed to initialize artifact storage %s", pr.Name) + return err + } + + // When the pipeline run is stopping, we don't schedule any new task and only + // wait for all running tasks to complete and report their status + if !pipelineState.IsStopping() { + err = c.runNextSchedulableTask(ctx, pr, d, pipelineState, as) + if err != nil { + return err + } + } + + before := pr.Status.GetCondition(apis.ConditionSucceeded) + after := resources.GetPipelineConditionStatus(pr, pipelineState, c.Logger, d) + switch after.Status { + case corev1.ConditionTrue: + pr.Status.MarkSucceeded(after.Reason, after.Message) + case corev1.ConditionFalse: + pr.Status.MarkFailed(after.Reason, after.Message) + case corev1.ConditionUnknown: + pr.Status.MarkRunning(after.Reason, after.Message) + } + // Read the condition the way it was set by the Mark* helpers + after = pr.Status.GetCondition(apis.ConditionSucceeded) + events.Emit(c.Recorder, before, after, pr) + + pr.Status.TaskRuns = getTaskRunsStatus(pr, pipelineState) + c.Logger.Infof("PipelineRun %s status is being set to %s", pr.Name, after) + return nil +} + +// runNextSchedulableTask gets the next schedulable Tasks from the dag based on the current +// pipeline run state, and starts them +func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1beta1.PipelineRun, d *dag.Graph, pipelineState resources.PipelineRunState, as artifacts.ArtifactStorageInterface) error { + candidateTasks, err := dag.GetSchedulable(d, pipelineState.SuccessfulPipelineTaskNames()...) if err != nil { c.Logger.Errorf("Error getting potential next tasks for valid pipelinerun %s: %v", pr.Name, err) + return nil } nextRprts := pipelineState.GetNextTasks(candidateTasks) @@ -439,13 +478,6 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun) err } resources.ApplyTaskResults(nextRprts, resolvedResultRefs) - var as artifacts.ArtifactStorageInterface - - if as, err = artifacts.InitializeArtifactStorage(c.Images, pr, pipelineSpec, c.KubeClientSet, c.Logger); err != nil { - c.Logger.Infof("PipelineRun failed to initialize artifact storage %s", pr.Name) - return err - } - for _, rprt := range nextRprts { if rprt == nil { continue @@ -466,20 +498,6 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun) err } } } - before := pr.Status.GetCondition(apis.ConditionSucceeded) - after := resources.GetPipelineConditionStatus(pr, pipelineState, c.Logger, d) - switch after.Status { - case corev1.ConditionTrue: - pr.Status.MarkSucceeded(after.Reason, after.Message) - case corev1.ConditionFalse: - pr.Status.MarkFailed(after.Reason, after.Message) - case corev1.ConditionUnknown: - pr.Status.MarkRunning(after.Reason, after.Message) - } - events.Emit(c.Recorder, before, after, pr) - - pr.Status.TaskRuns = getTaskRunsStatus(pr, pipelineState) - c.Logger.Infof("PipelineRun %s status is being set to %s", pr.Name, after) return nil } diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index 20b0609b597..a2e7d048822 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -62,6 +62,10 @@ const ( // ReasonConditionCheckFailed indicates that the reason for the failure status is that the // condition check associated to the pipeline task evaluated to false ReasonConditionCheckFailed = "ConditionCheckFailed" + + // ReasonStopping indicates that no new Tasks will be scheduled by the controller, and the + // pipeline will stop once all running tasks complete their work + ReasonStopping = "PipelineRunStopping" ) // TaskNotFoundError indicates that the resolution failed because a referenced Task couldn't be retrieved @@ -181,6 +185,20 @@ func (state PipelineRunState) IsBeforeFirstTaskRun() bool { return true } +// IsStopping returns true if the PipelineRun won't be scheduling any new Task because +// at least one task already failed or was cancelled +func (state PipelineRunState) IsStopping() bool { + for _, t := range state { + if t.IsCancelled() { + return true + } + if t.IsFailure() { + return true + } + } + return false +} + // GetNextTasks will return the next ResolvedPipelineRunTasks to execute, which are the ones in the // list of candidateTasks which aren't yet indicated in state to be running. func (state PipelineRunState) GetNextTasks(candidateTasks map[string]struct{}) []*ResolvedPipelineRunTask { @@ -410,7 +428,7 @@ func GetTaskRunName(taskRunsStatus map[string]*v1beta1.PipelineRunTaskRunStatus, func GetPipelineConditionStatus(pr *v1beta1.PipelineRun, state PipelineRunState, logger *zap.SugaredLogger, dag *dag.Graph) *apis.Condition { // We have 4 different states here: // 1. Timed out -> Failed - // 2. Any one TaskRun has failed - >Failed. This should change with #1020 and #1023 + // 2. All tasks are done and at least one has failed or has been cancelled -> Failed // 3. All tasks are done or are skipped (i.e. condition check failed).-> Success // 4. A Task or Condition is running right now or there are things left to run -> Running if pr.IsTimedOut() { @@ -422,67 +440,78 @@ func GetPipelineConditionStatus(pr *v1beta1.PipelineRun, state PipelineRunState, } } - // A single failed task mean we fail the pipeline - for _, rprt := range state { - if rprt.IsCancelled() { - logger.Infof("TaskRun %s is cancelled, so PipelineRun %s is cancelled", rprt.TaskRunName, pr.Name) - return &apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionFalse, - Reason: ReasonCancelled, - Message: fmt.Sprintf("TaskRun %s has cancelled", rprt.TaskRun.Name), - } - } - - if rprt.IsFailure() { //IsDone ensures we have crossed the retry limit - logger.Infof("TaskRun %s has failed, so PipelineRun %s has failed, retries done: %b", rprt.TaskRunName, pr.Name, len(rprt.TaskRun.Status.RetriesStatus)) - return &apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionFalse, - Reason: ReasonFailed, - Message: fmt.Sprintf("TaskRun %s has failed", rprt.TaskRun.Name), - } - } - } - allTasks := []string{} - successOrSkipTasks := []string{} + withStatusTasks := []string{} skipTasks := int(0) + failedTasks := int(0) + cancelledTasks := int(0) + reason := ReasonSucceeded // Check to see if all tasks are success or skipped + // + // The completion reason is also calculated here, but it will only be used + // if all tasks are completed. + // + // The pipeline run completion reason is set from the taskrun completion reason + // according to the following logic: + // + // - All successful: ReasonSucceeded + // - Some successful, some skipped: ReasonCompleted + // - Some cancelled, none failed: ReasonCancelled + // - At least one failed: ReasonFailed for _, rprt := range state { allTasks = append(allTasks, rprt.PipelineTask.Name) - if rprt.IsSuccessful() { - successOrSkipTasks = append(successOrSkipTasks, rprt.PipelineTask.Name) - } - if isSkipped(rprt, state.ToMap(), dag) { + switch { + case rprt.IsSuccessful(): + withStatusTasks = append(withStatusTasks, rprt.PipelineTask.Name) + case isSkipped(rprt, state.ToMap(), dag): skipTasks++ - successOrSkipTasks = append(successOrSkipTasks, rprt.PipelineTask.Name) + withStatusTasks = append(withStatusTasks, rprt.PipelineTask.Name) + // At least one is skipped and no failure yet, mark as completed + if reason == ReasonSucceeded { + reason = ReasonCompleted + } + case rprt.IsCancelled(): + cancelledTasks++ + withStatusTasks = append(withStatusTasks, rprt.PipelineTask.Name) + if reason != ReasonFailed { + reason = ReasonCancelled + } + case rprt.IsFailure(): + withStatusTasks = append(withStatusTasks, rprt.PipelineTask.Name) + failedTasks++ + reason = ReasonFailed } } - if reflect.DeepEqual(allTasks, successOrSkipTasks) { - logger.Infof("All TaskRuns have finished for PipelineRun %s so it has finished", pr.Name) - reason := ReasonSucceeded - if skipTasks != 0 { - reason = ReasonCompleted + if reflect.DeepEqual(allTasks, withStatusTasks) { + status := corev1.ConditionTrue + if failedTasks > 0 || cancelledTasks > 0 { + status = corev1.ConditionFalse } - + logger.Infof("All TaskRuns have finished for PipelineRun %s so it has finished", pr.Name) return &apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - Reason: reason, - Message: fmt.Sprintf("Tasks Completed: %d, Skipped: %d", len(successOrSkipTasks)-skipTasks, skipTasks), + Type: apis.ConditionSucceeded, + Status: status, + Reason: reason, + Message: fmt.Sprintf("Tasks Completed: %d (Failed: %d, Cancelled %d), Skipped: %d", + len(allTasks)-skipTasks, failedTasks, cancelledTasks, skipTasks), } } - // Hasn't timed out; no taskrun failed yet; and not all tasks have finished.... + // Hasn't timed out; not all tasks have finished.... // Must keep running then.... + if failedTasks > 0 || cancelledTasks > 0 { + reason = ReasonStopping + } else { + reason = ReasonRunning + } return &apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionUnknown, - Reason: ReasonRunning, - Message: fmt.Sprintf("Tasks Completed: %d, Incomplete: %d, Skipped: %d", len(successOrSkipTasks)-skipTasks, len(allTasks)-len(successOrSkipTasks), skipTasks), + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Reason: reason, + Message: fmt.Sprintf("Tasks Completed: %d (Failed: %d, Cancelled %d), Incomplete: %d, Skipped: %d", + len(withStatusTasks)-skipTasks, failedTasks, cancelledTasks, len(allTasks)-len(withStatusTasks), skipTasks), } } diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 294e3b285f5..34264e315ce 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -298,7 +298,7 @@ var failedTaskConditionCheckState = TaskConditionCheckState{{ var conditionCheckSuccessNoTaskStartedState = PipelineRunState{{ PipelineTask: &pts[5], - TaskRunName: "pipeleinerun-conditionaltask", + TaskRunName: "pipelinerun-conditionaltask", TaskRun: nil, ResolvedTaskResources: &resources.ResolvedTaskResources{ TaskSpec: &task.Spec, @@ -308,7 +308,7 @@ var conditionCheckSuccessNoTaskStartedState = PipelineRunState{{ var conditionCheckStartedState = PipelineRunState{{ PipelineTask: &pts[5], - TaskRunName: "pipeleinerun-conditionaltask", + TaskRunName: "pipelinerun-conditionaltask", TaskRun: nil, ResolvedTaskResources: &resources.ResolvedTaskResources{ TaskSpec: &task.Spec, @@ -322,7 +322,7 @@ var conditionCheckStartedState = PipelineRunState{{ var conditionCheckFailedWithNoOtherTasksState = PipelineRunState{{ PipelineTask: &pts[5], - TaskRunName: "pipeleinerun-conditionaltask", + TaskRunName: "pipelinerun-conditionaltask", TaskRun: nil, ResolvedTaskResources: &resources.ResolvedTaskResources{ TaskSpec: &task.Spec, @@ -332,7 +332,7 @@ var conditionCheckFailedWithNoOtherTasksState = PipelineRunState{{ var conditionCheckFailedWithOthersPassedState = PipelineRunState{{ PipelineTask: &pts[5], - TaskRunName: "pipeleinerun-conditionaltask", + TaskRunName: "pipelinerun-conditionaltask", TaskRun: nil, ResolvedTaskResources: &resources.ResolvedTaskResources{ TaskSpec: &task.Spec, @@ -351,7 +351,7 @@ var conditionCheckFailedWithOthersPassedState = PipelineRunState{{ var conditionCheckFailedWithOthersFailedState = PipelineRunState{{ PipelineTask: &pts[5], - TaskRunName: "pipeleinerun-conditionaltask", + TaskRunName: "pipelinerun-conditionaltask", TaskRun: nil, ResolvedTaskResources: &resources.ResolvedTaskResources{ TaskSpec: &task.Spec, @@ -974,6 +974,15 @@ func TestSuccessfulPipelineTaskNames(t *testing.T) { } } +func getExpectedMessage(status corev1.ConditionStatus, successful, incomplete, skipped, failed, cancelled int) string { + if status == corev1.ConditionFalse || status == corev1.ConditionTrue { + return fmt.Sprintf("Tasks Completed: %d (Failed: %d, Cancelled %d), Skipped: %d", + successful+failed+cancelled, failed, cancelled, skipped) + } + return fmt.Sprintf("Tasks Completed: %d (Failed: %d, Cancelled %d), Incomplete: %d, Skipped: %d", + successful+failed+cancelled, failed, cancelled, incomplete, skipped) +} + func TestGetPipelineConditionStatus(t *testing.T) { var taskRetriedState = PipelineRunState{{ @@ -985,98 +994,176 @@ func TestGetPipelineConditionStatus(t *testing.T) { }, }} - tcs := []struct { - name string - state []*ResolvedPipelineRunTask - expectedStatus corev1.ConditionStatus - }{{ - name: "no-tasks-started", - state: noneStartedState, - expectedStatus: corev1.ConditionUnknown, - }, { - name: "one-task-started", - state: oneStartedState, - expectedStatus: corev1.ConditionUnknown, - }, { - name: "one-task-finished", - state: oneFinishedState, - expectedStatus: corev1.ConditionUnknown, - }, { - name: "one-task-failed", - state: oneFailedState, - expectedStatus: corev1.ConditionFalse, - }, { - name: "all-finished", - state: allFinishedState, - expectedStatus: corev1.ConditionTrue, - }, { - name: "one-retry-needed", - state: taskRetriedState, - expectedStatus: corev1.ConditionUnknown, - }, { - name: "condition-success-no-task started", - state: conditionCheckSuccessNoTaskStartedState, - expectedStatus: corev1.ConditionUnknown, - }, { - name: "condition-check-in-progress", - state: conditionCheckStartedState, - expectedStatus: corev1.ConditionUnknown, - }, { - name: "condition-failed-no-other-tasks", // 1 task pipeline with a condition that fails - state: conditionCheckFailedWithNoOtherTasksState, - expectedStatus: corev1.ConditionTrue, - }, { - name: "condition-failed-another-task-succeeded", // 1 task skipped due to condition, but others pass - state: conditionCheckFailedWithOthersPassedState, - expectedStatus: corev1.ConditionTrue, - }, { - name: "condition-failed-another-task-failed", // 1 task skipped due to condition, but others failed - state: conditionCheckFailedWithOthersFailedState, - expectedStatus: corev1.ConditionFalse, - }, { - name: "no-tasks-started", - state: noneStartedState, - expectedStatus: corev1.ConditionUnknown, - }, { - name: "one-task-started", - state: oneStartedState, - expectedStatus: corev1.ConditionUnknown, - }, { - name: "one-task-finished", - state: oneFinishedState, - expectedStatus: corev1.ConditionUnknown, - }, { - name: "one-task-failed", - state: oneFailedState, - expectedStatus: corev1.ConditionFalse, - }, { - name: "all-finished", - state: allFinishedState, - expectedStatus: corev1.ConditionTrue, - }, { - name: "one-retry-needed", - state: taskRetriedState, - expectedStatus: corev1.ConditionUnknown, - }, { - name: "task skipped due to condition failure in parent", - state: taskWithParentSkippedState, - expectedStatus: corev1.ConditionTrue, - }, { - name: "task with multiple parent tasks -> one of which is skipped", - state: taskWithMultipleParentsSkippedState, - expectedStatus: corev1.ConditionTrue, + var taskCancelledFailed = PipelineRunState{{ + PipelineTask: &pts[4], + TaskRunName: "pipelinerun-mytask1", + TaskRun: withCancelled(makeFailed(trs[0])), + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }} + + // 6 Tasks, 4 that run in parallel in the beginning + // Of the 4, 1 passed, 1 cancelled, 2 failed + // 1 runAfter the passed one, currently running + // 1 runAfter the failed one, which is marked as incomplete + var taskMultipleFailuresSkip = PipelineRunState{{ + TaskRunName: "task0taskrun", + PipelineTask: &pts[5], + TaskRun: makeSucceeded(trs[0]), + ResolvedConditionChecks: successTaskConditionCheckState, + }, { + TaskRunName: "runningTaskRun", + PipelineTask: &pts[6], + TaskRun: &trs[1], + }, { + TaskRunName: "failedTaskRun", + PipelineTask: &pts[0], + TaskRun: makeFailed(trs[0]), }, { - name: "task with grand parent task skipped", - state: taskWithGrandParentSkippedState, - expectedStatus: corev1.ConditionTrue, + TaskRunName: "skippedTaskRun", + PipelineTask: &pts[7], }, { - name: "task with grand parents; one parent failed", - state: taskWithGrandParentsOneFailedState, - expectedStatus: corev1.ConditionFalse, + TaskRunName: "anothertaskrun", + PipelineTask: &pts[1], + TaskRun: makeFailed(trs[1]), }, { - name: "task with grand parents; one not run yet", - state: taskWithGrandParentsOneNotRunState, - expectedStatus: corev1.ConditionUnknown, + TaskRunName: "taskrun", + PipelineTask: &pts[4], + TaskRun: withCancelled(makeFailed(trs[0])), + }} + + tcs := []struct { + name string + state []*ResolvedPipelineRunTask + expectedStatus corev1.ConditionStatus + expectedReason string + expectedSucceeded int // Successful + expectedIncomplete int + expectedSkipped int + expectedFailed int + expectedCancelled int + }{{ + name: "no-tasks-started", + state: noneStartedState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: ReasonRunning, + expectedIncomplete: 2, + }, { + name: "one-task-started", + state: oneStartedState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: ReasonRunning, + expectedIncomplete: 2, + }, { + name: "one-task-finished", + state: oneFinishedState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: ReasonRunning, + expectedSucceeded: 1, + expectedIncomplete: 1, + }, { + name: "one-task-failed", + state: oneFailedState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: ReasonStopping, + expectedIncomplete: 1, + expectedFailed: 1, + }, { + name: "all-finished", + state: allFinishedState, + expectedStatus: corev1.ConditionTrue, + expectedReason: ReasonSucceeded, + expectedSucceeded: 2, + }, { + name: "one-retry-needed", + state: taskRetriedState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: ReasonRunning, + expectedIncomplete: 1, + }, { + name: "condition-success-no-task started", + state: conditionCheckSuccessNoTaskStartedState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: ReasonRunning, + expectedIncomplete: 1, + }, { + name: "condition-check-in-progress", + state: conditionCheckStartedState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: ReasonRunning, + expectedIncomplete: 1, + }, { + name: "condition-failed-no-other-tasks", // 1 task pipeline with a condition that fails + state: conditionCheckFailedWithNoOtherTasksState, + expectedStatus: corev1.ConditionTrue, + expectedReason: ReasonCompleted, + expectedSkipped: 1, + expectedIncomplete: 1, + }, { + name: "condition-failed-another-task-succeeded", // 1 task skipped due to condition, but others pass + state: conditionCheckFailedWithOthersPassedState, + expectedStatus: corev1.ConditionTrue, + expectedReason: ReasonCompleted, + expectedSucceeded: 1, + expectedSkipped: 1, + }, { + name: "condition-failed-another-task-failed", // 1 task skipped due to condition, but others failed + state: conditionCheckFailedWithOthersFailedState, + expectedStatus: corev1.ConditionFalse, + expectedReason: ReasonFailed, + expectedFailed: 1, + expectedSkipped: 1, + }, { + name: "task skipped due to condition failure in parent", + state: taskWithParentSkippedState, + expectedStatus: corev1.ConditionTrue, + expectedReason: ReasonCompleted, + expectedSkipped: 2, + }, { + name: "task with multiple parent tasks -> one of which is skipped", + state: taskWithMultipleParentsSkippedState, + expectedStatus: corev1.ConditionTrue, + expectedReason: ReasonCompleted, + expectedSkipped: 2, + expectedSucceeded: 1, + }, { + name: "task with grand parent task skipped", + state: taskWithGrandParentSkippedState, + expectedStatus: corev1.ConditionTrue, + expectedReason: ReasonCompleted, + expectedSkipped: 3, + expectedSucceeded: 1, + }, { + name: "task with grand parents; one parent failed", + state: taskWithGrandParentsOneFailedState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: ReasonStopping, + expectedSucceeded: 1, + expectedIncomplete: 2, + expectedFailed: 1, + }, { + name: "task with grand parents; one not run yet", + state: taskWithGrandParentsOneNotRunState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: ReasonRunning, + expectedSucceeded: 1, + expectedIncomplete: 3, + }, { + name: "task that was cancelled", + state: taskCancelledFailed, + expectedReason: ReasonCancelled, + expectedStatus: corev1.ConditionFalse, + expectedCancelled: 1, + }, { + name: "task with multiple failures, one cancelled", + state: taskMultipleFailuresSkip, + expectedReason: ReasonStopping, + expectedStatus: corev1.ConditionUnknown, + expectedSucceeded: 1, + expectedFailed: 2, + expectedIncomplete: 2, + expectedCancelled: 1, }} for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { @@ -1086,8 +1173,15 @@ func TestGetPipelineConditionStatus(t *testing.T) { t.Fatalf("Unexpected error while buildig DAG for state %v: %v", tc.state, err) } c := GetPipelineConditionStatus(pr, tc.state, zap.NewNop().Sugar(), dag) - if c.Status != tc.expectedStatus { - t.Fatalf("Expected to get status %s but got %s for state %v", tc.expectedStatus, c.Status, tc.state) + wantCondition := &apis.Condition{ + Type: apis.ConditionSucceeded, + Status: tc.expectedStatus, + Reason: tc.expectedReason, + Message: getExpectedMessage(tc.expectedStatus, tc.expectedSucceeded, + tc.expectedIncomplete, tc.expectedSkipped, tc.expectedFailed, tc.expectedCancelled), + } + if d := cmp.Diff(wantCondition, c); d != "" { + t.Fatalf("Mismatch in condition %s", diff.PrintWantGot(d)) } }) }