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/apis/pipeline/v1beta1/pipelinerun_types.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go index e8a102b9ca4..2b259e4f44f 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go @@ -231,6 +231,9 @@ const ( PipelineRunReasonCancelled PipelineRunReason = "Cancelled" // PipelineRunReasonTimedOut is the reason set when the PipelineRun has timed out PipelineRunReasonTimedOut PipelineRunReason = "PipelineRunTimeout" + // ReasonStopping indicates that no new Tasks will be scheduled by the controller, and the + // pipeline will stop once all running tasks complete their work + PipelineRunReasonStopping PipelineRunReason = "PipelineRunStopping" ) func (t PipelineRunReason) String() string { diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 6b5f44e06b4..352d759abf3 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -426,9 +426,51 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun) err } } + as, err := artifacts.InitializeArtifactStorage(c.Images, pr, pipelineSpec, c.KubeClientSet, logger) + if err != nil { + 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, 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(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 { + + logger := logging.FromContext(ctx) + recorder := controller.GetEventRecorder(ctx) + candidateTasks, err := dag.GetSchedulable(d, pipelineState.SuccessfulPipelineTaskNames()...) if err != nil { logger.Errorf("Error getting potential next tasks for valid pipelinerun %s: %v", pr.Name, err) + return nil } nextRprts := pipelineState.GetNextTasks(candidateTasks) @@ -440,13 +482,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, logger); err != nil { - logger.Infof("PipelineRun failed to initialize artifact storage %s", pr.Name) - return err - } - for _, rprt := range nextRprts { if rprt == nil { continue @@ -467,20 +502,6 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun) err } } } - before := pr.Status.GetCondition(apis.ConditionSucceeded) - after := resources.GetPipelineConditionStatus(pr, pipelineState, 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(recorder, before, after, pr) - - pr.Status.TaskRuns = getTaskRunsStatus(pr, pipelineState) - 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 77c046f81dc..4a2b143f50b 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -127,6 +127,20 @@ func (t ResolvedPipelineRunTask) IsCancelled() bool { return c.IsFalse() && c.Reason == v1beta1.TaskRunReasonCancelled.String() } +// IsStarted returns true only if the PipelineRunTask itself has a TaskRun associated +func (t ResolvedPipelineRunTask) IsStarted() bool { + if t.TaskRun == nil { + return false + } + + c := t.TaskRun.Status.GetCondition(apis.ConditionSucceeded) + if c == nil { + return false + } + + return true +} + // ToMap returns a map that maps pipeline task name to the resolved pipeline run task func (state PipelineRunState) ToMap() map[string]*ResolvedPipelineRunTask { m := make(map[string]*ResolvedPipelineRunTask) @@ -160,6 +174,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 { @@ -389,9 +417,9 @@ 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 + // 4. A Task or Condition is running right now or there are things left to run -> Running if pr.IsTimedOut() { return &apis.Condition{ Type: apis.ConditionSucceeded, @@ -401,72 +429,91 @@ 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: v1beta1.PipelineRunReasonCancelled.String(), - 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: v1beta1.PipelineRunReasonFailed.String(), - 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 := v1beta1.PipelineRunReasonSuccessful.String() + stateAsMap := state.ToMap() + isStopping := state.IsStopping() // 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.IsStarted() && isStopping: + // If the pipeline is in stopping mode, all tasks that are not running + // already will be skipped. Otherwise these tasks end up in the + // incomplete count. + skipTasks++ + withStatusTasks = append(withStatusTasks, rprt.PipelineTask.Name) + case rprt.IsSuccessful(): + withStatusTasks = append(withStatusTasks, rprt.PipelineTask.Name) + case isSkipped(rprt, stateAsMap, 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 == v1beta1.PipelineRunReasonSuccessful.String() { + reason = v1beta1.PipelineRunReasonCompleted.String() + } + case rprt.IsCancelled(): + cancelledTasks++ + withStatusTasks = append(withStatusTasks, rprt.PipelineTask.Name) + if reason != v1beta1.PipelineRunReasonFailed.String() { + reason = v1beta1.PipelineRunReasonCancelled.String() + } + case rprt.IsFailure(): + withStatusTasks = append(withStatusTasks, rprt.PipelineTask.Name) + failedTasks++ + reason = v1beta1.PipelineRunReasonFailed.String() } } - if reflect.DeepEqual(allTasks, successOrSkipTasks) { - logger.Infof("All TaskRuns have finished for PipelineRun %s so it has finished", pr.Name) - reason := v1beta1.PipelineRunReasonSuccessful.String() - if skipTasks != 0 { - reason = v1beta1.PipelineRunReasonCompleted.String() + 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 = v1beta1.PipelineRunReasonStopping.String() + } else { + reason = v1beta1.PipelineRunReasonRunning.String() + } return &apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionUnknown, - Reason: v1beta1.PipelineRunReasonRunning.String(), - 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), } } // isSkipped returns true if a Task in a TaskRun will not be run either because -// its Condition Checks failed or because one of the parent tasks's conditions failed +// its Condition Checks failed or because one of the parent tasks' conditions failed // Note that this means isSkipped returns false if a conditionCheck is in progress func isSkipped(rprt *ResolvedPipelineRunTask, stateMap map[string]*ResolvedPipelineRunTask, d *dag.Graph) bool { // Taskrun not skipped if it already exists diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 294e3b285f5..686b9dd2437 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, @@ -938,6 +938,244 @@ func TestIsDone(t *testing.T) { } } +func TestIsSkipped(t *testing.T) { + + tcs := []struct { + name string + taskName string + state PipelineRunState + expected bool + }{{ + name: "tasks-condition-passed", + taskName: "mytask1", + state: PipelineRunState{{ + PipelineTask: &pts[0], + TaskRunName: "pipelinerun-conditionaltask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + ResolvedConditionChecks: successTaskConditionCheckState, + }}, + expected: false, + }, { + name: "tasks-condition-failed", + taskName: "mytask1", + state: PipelineRunState{{ + PipelineTask: &pts[0], + TaskRunName: "pipelinerun-conditionaltask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + ResolvedConditionChecks: failedTaskConditionCheckState, + }}, + expected: true, + }, { + name: "tasks-multiple-conditions-passed-failed", + taskName: "mytask1", + state: PipelineRunState{{ + PipelineTask: &pts[0], + TaskRunName: "pipelinerun-conditionaltask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + ResolvedConditionChecks: TaskConditionCheckState{{ + ConditionCheckName: "myconditionCheck", + Condition: &condition, + ConditionCheck: v1beta1.NewConditionCheck(makeFailed(conditionChecks[0])), + }, { + ConditionCheckName: "myconditionCheck", + Condition: &condition, + ConditionCheck: v1beta1.NewConditionCheck(makeSucceeded(conditionChecks[0])), + }}, + }}, + expected: true, + }, { + name: "tasks-condition-running", + taskName: "mytask6", + state: conditionCheckStartedState, + expected: false, + }, { + name: "tasks-parent-condition-passed", + taskName: "mytask7", + state: PipelineRunState{{ + PipelineTask: &pts[5], + TaskRunName: "pipelinerun-conditionaltask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + ResolvedConditionChecks: successTaskConditionCheckState, + }, { + PipelineTask: &pts[6], + }}, + expected: false, + }, { + name: "tasks-parent-condition-failed", + taskName: "mytask7", + state: PipelineRunState{{ + PipelineTask: &pts[5], + TaskRunName: "pipelinerun-conditionaltask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + ResolvedConditionChecks: failedTaskConditionCheckState, + }, { + PipelineTask: &pts[6], + }}, + expected: true, + }, { + name: "tasks-parent-condition-running", + taskName: "mytask7", + state: PipelineRunState{{ + PipelineTask: &pts[5], + TaskRunName: "pipelinerun-conditionaltask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + ResolvedConditionChecks: TaskConditionCheckState{{ + ConditionCheckName: "myconditionCheck", + Condition: &condition, + ConditionCheck: v1beta1.NewConditionCheck(makeStarted(conditionChecks[0])), + }}, + }, { + PipelineTask: &pts[6], + }}, + expected: false, + }, { + name: "tasks-failed", + taskName: "mytask1", + state: oneFailedState, + expected: false, + }, { + name: "tasks-passed", + taskName: "mytask1", + state: oneFinishedState, + expected: false, + }, { + name: "tasks-cancelled", + taskName: "mytask5", + state: taskCancelled, + expected: false, + }, { + name: "tasks-parent-failed", + taskName: "mytask7", + state: PipelineRunState{{ + PipelineTask: &pts[5], + TaskRunName: "pipelinerun-mytask1", + TaskRun: makeFailed(trs[0]), + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + PipelineTask: &pts[6], // mytask7 runAfter mytask6 + TaskRunName: "pipelinerun-mytask2", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, + expected: false, + }, { + name: "tasks-parent-cancelled", + taskName: "mytask7", + state: PipelineRunState{{ + PipelineTask: &pts[5], + TaskRunName: "pipelinerun-mytask1", + TaskRun: withCancelled(makeFailed(trs[0])), + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + PipelineTask: &pts[6], // mytask7 runAfter mytask6 + TaskRunName: "pipelinerun-mytask2", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, + expected: false, + }, { + name: "tasks-grandparent-failed", + taskName: "mytask10", + state: PipelineRunState{{ + PipelineTask: &pts[5], + TaskRunName: "pipelinerun-mytask1", + TaskRun: makeFailed(trs[0]), + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + PipelineTask: &pts[6], // mytask7 runAfter mytask6 + TaskRunName: "pipelinerun-mytask2", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + PipelineTask: &v1beta1.PipelineTask{ + Name: "mytask10", + TaskRef: &v1beta1.TaskRef{Name: "task"}, + RunAfter: []string{"mytask7"}, + }, // mytask10 runAfter mytask7 runAfter mytask6 + TaskRunName: "pipelinerun-mytask3", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, + expected: false, + }, { + name: "tasks-parents-failed-passed", + taskName: "mytask8", + state: PipelineRunState{{ + PipelineTask: &pts[5], + TaskRunName: "pipelinerun-mytask1", + TaskRun: makeSucceeded(trs[0]), + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + PipelineTask: &pts[0], + TaskRunName: "pipelinerun-mytask2", + TaskRun: makeFailed(trs[0]), + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + PipelineTask: &pts[7], // mytask8 runAfter mytask1, mytask6 + TaskRunName: "pipelinerun-mytask3", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, + expected: false, + }} + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + dag, err := DagFromState(tc.state) + if err != nil { + t.Fatalf("Could not get a dag from the TC state %#v: %v", tc.state, err) + } + stateMap := tc.state.ToMap() + rprt := stateMap[tc.taskName] + if rprt == nil { + t.Fatalf("Could not get task %s from the state: %v", tc.taskName, tc.state) + } + isSkipped := isSkipped(rprt, stateMap, dag) + if d := cmp.Diff(isSkipped, tc.expected); d != "" { + t.Errorf("Didn't get expected isSkipped %s", diff.PrintWantGot(d)) + } + }) + } +} + func TestSuccessfulPipelineTaskNames(t *testing.T) { tcs := []struct { name string @@ -974,6 +1212,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 +1232,201 @@ 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, + 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 taskMultipleFailuresSkipRunning = PipelineRunState{{ + TaskRunName: "task0taskrun", + PipelineTask: &pts[5], + TaskRun: makeSucceeded(trs[0]), + ResolvedConditionChecks: successTaskConditionCheckState, + }, { + TaskRunName: "runningTaskRun", // this is running + PipelineTask: &pts[6], + TaskRun: makeStarted(trs[1]), + }, { + TaskRunName: "failedTaskRun", // this failed + PipelineTask: &pts[0], + TaskRun: makeFailed(trs[0]), }, { - name: "task skipped due to condition failure in parent", - state: taskWithParentSkippedState, - expectedStatus: corev1.ConditionTrue, + TaskRunName: "skippedTaskRun", // skipped because parent failed + PipelineTask: &pts[7], // mytask8 runAfter mytask1 (failed), mytask6 (succeeded) + TaskRun: nil, }, { - name: "task with multiple parent tasks -> one of which is skipped", - state: taskWithMultipleParentsSkippedState, - expectedStatus: corev1.ConditionTrue, + TaskRunName: "anothertaskrun", // this failed + PipelineTask: &pts[1], + TaskRun: makeFailed(trs[1]), }, { - name: "task with grand parent task skipped", - state: taskWithGrandParentSkippedState, - expectedStatus: corev1.ConditionTrue, + TaskRunName: "taskrun", // this was cancelled + PipelineTask: &pts[4], + TaskRun: withCancelled(makeFailed(trs[0])), + }} + + var taskNotRunningWithSuccesfulParentsOneFailed = PipelineRunState{{ + TaskRunName: "task0taskrun", + PipelineTask: &pts[5], + TaskRun: makeSucceeded(trs[0]), + ResolvedConditionChecks: successTaskConditionCheckState, }, { - name: "task with grand parents; one parent failed", - state: taskWithGrandParentsOneFailedState, - expectedStatus: corev1.ConditionFalse, + TaskRunName: "notRunningTaskRun", // runAfter pts[5], not started yet + PipelineTask: &pts[6], + TaskRun: nil, }, { - name: "task with grand parents; one not run yet", - state: taskWithGrandParentsOneNotRunState, - expectedStatus: corev1.ConditionUnknown, + TaskRunName: "failedTaskRun", // this failed + PipelineTask: &pts[0], + TaskRun: makeFailed(trs[0]), + }} + + tcs := []struct { + name string + state []*ResolvedPipelineRunTask + expectedStatus corev1.ConditionStatus + expectedReason string + expectedSucceeded int + expectedIncomplete int + expectedSkipped int + expectedFailed int + expectedCancelled int + }{{ + name: "no-tasks-started", + state: noneStartedState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: v1beta1.PipelineRunReasonRunning.String(), + expectedIncomplete: 2, + }, { + name: "one-task-started", + state: oneStartedState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: v1beta1.PipelineRunReasonRunning.String(), + expectedIncomplete: 2, + }, { + name: "one-task-finished", + state: oneFinishedState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: v1beta1.PipelineRunReasonRunning.String(), + expectedSucceeded: 1, + expectedIncomplete: 1, + }, { + name: "one-task-failed", + state: oneFailedState, + expectedStatus: corev1.ConditionFalse, + expectedReason: v1beta1.PipelineRunReasonFailed.String(), + expectedFailed: 1, + expectedSkipped: 1, + }, { + name: "all-finished", + state: allFinishedState, + expectedStatus: corev1.ConditionTrue, + expectedReason: v1beta1.PipelineRunReasonSuccessful.String(), + expectedSucceeded: 2, + }, { + name: "one-retry-needed", + state: taskRetriedState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: v1beta1.PipelineRunReasonRunning.String(), + expectedIncomplete: 1, + }, { + name: "condition-success-no-task started", + state: conditionCheckSuccessNoTaskStartedState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: v1beta1.PipelineRunReasonRunning.String(), + expectedIncomplete: 1, + }, { + name: "condition-check-in-progress", + state: conditionCheckStartedState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: v1beta1.PipelineRunReasonRunning.String(), + expectedIncomplete: 1, + }, { + name: "condition-failed-no-other-tasks", // 1 task pipeline with a condition that fails + state: conditionCheckFailedWithNoOtherTasksState, + expectedStatus: corev1.ConditionTrue, + expectedReason: v1beta1.PipelineRunReasonCompleted.String(), + 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: v1beta1.PipelineRunReasonCompleted.String(), + 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: v1beta1.PipelineRunReasonFailed.String(), + expectedFailed: 1, + expectedSkipped: 1, + }, { + name: "task skipped due to condition failure in parent", + state: taskWithParentSkippedState, + expectedStatus: corev1.ConditionTrue, + expectedReason: v1beta1.PipelineRunReasonCompleted.String(), + expectedSkipped: 2, + }, { + name: "task with multiple parent tasks -> one of which is skipped", + state: taskWithMultipleParentsSkippedState, + expectedStatus: corev1.ConditionTrue, + expectedReason: v1beta1.PipelineRunReasonCompleted.String(), + expectedSkipped: 2, + expectedSucceeded: 1, + }, { + name: "task with grand parent task skipped", + state: taskWithGrandParentSkippedState, + expectedStatus: corev1.ConditionTrue, + expectedReason: v1beta1.PipelineRunReasonCompleted.String(), + expectedSkipped: 3, + expectedSucceeded: 1, + }, { + name: "task with grand parents; one parent failed", + state: taskWithGrandParentsOneFailedState, + expectedStatus: corev1.ConditionFalse, + expectedReason: v1beta1.PipelineRunReasonFailed.String(), + expectedSucceeded: 1, + expectedSkipped: 2, + expectedFailed: 1, + }, { + name: "task with grand parents; one not run yet", + state: taskWithGrandParentsOneNotRunState, + expectedStatus: corev1.ConditionUnknown, + expectedReason: v1beta1.PipelineRunReasonRunning.String(), + expectedSucceeded: 1, + expectedIncomplete: 3, + }, { + name: "task that was cancelled", + state: taskCancelledFailed, + expectedReason: v1beta1.PipelineRunReasonCancelled.String(), + expectedStatus: corev1.ConditionFalse, + expectedCancelled: 1, + }, { + name: "task with multiple failures; one cancelled", + state: taskMultipleFailuresSkipRunning, + expectedReason: v1beta1.PipelineRunReasonStopping.String(), + expectedStatus: corev1.ConditionUnknown, + expectedSucceeded: 1, + expectedFailed: 2, + expectedIncomplete: 1, + expectedCancelled: 1, + expectedSkipped: 1, + }, { + name: "task not started with passed parent; one failed", + state: taskNotRunningWithSuccesfulParentsOneFailed, + expectedReason: v1beta1.PipelineRunReasonFailed.String(), + expectedStatus: corev1.ConditionFalse, + expectedSucceeded: 1, + expectedFailed: 1, + expectedSkipped: 1, }} for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { @@ -1086,8 +1436,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)) } }) }