Skipping Reason included in the SkippedTasks field of `PipelineRunS…

…tatus`.

Today, users only know that a PipelineTask was skipped, but they don't know for which exact reason.
This can be confusing when debugging Pipelines. In this PR, we add the reason for skipping to the
SkippedTasks field in PipelineRunStatus to improve usability and debuggability.

This commit adds a field `Reason` of type [`SkippingReason`][reasons] (a string alias) to the [`SkippedTasks`][skipped-tasks] field in `PipelineRunStatus`.

docs/pipelineruns.md

@@ -665,6 +665,8 @@ False|PipelineRunTimeout|Yes|The `PipelineRun` timed out.
 
 When a `PipelineRun` changes status, [events](events.md#pipelineruns) are triggered accordingly.
 
+When a `PipelineRun` has `Tasks` that were `skipped`, the `reason` for skipping the task will be listed in the `Skipped Tasks` section of the `status` of the `PipelineRun`.
+
 When a `PipelineRun` has `Tasks` with [`when` expressions](pipelines.md#guard-task-execution-using-when-expressions):
 - If the `when` expressions evaluate to `true`, the `Task` is executed then the `TaskRun` and its resolved `when` expressions will be listed in the `Task Runs` section of the `status` of the `PipelineRun`.
 - If the `when` expressions evaluate to `false`, the `Task` is skipped then its name and its resolved `when` expressions will be listed in the `Skipped Tasks` section of the `status` of the `PipelineRun`.
@@ -678,6 +680,7 @@ Conditions:
   Type:                  Succeeded
 Skipped Tasks:
   Name:       skip-this-task
+  Reason:     WhenExpressionsSkip
   When Expressions:
     Input:     foo
     Operator:  in

pkg/apis/pipeline/v1beta1/pipelinerun_types.go

@@ -484,12 +484,36 @@ type PipelineRunStatusFields struct {
 type SkippedTask struct {
 	// Name is the Pipeline Task name
 	Name string `json:"name"`
+	// Reason is the cause of the PipelineTask being skipped.
+	Reason SkippingReason `json:"reason"`
 	// WhenExpressions is the list of checks guarding the execution of the PipelineTask
 	// +optional
 	// +listType=atomic
 	WhenExpressions []WhenExpression `json:"whenExpressions,omitempty"`
 }
 
+// SkippingReason explains why a PipelineTask was skipped.
+type SkippingReason string
+
+const (
+	// WhenExpressionsSkip means the task was skipped due to at least one of its when expressions evaluating to false
+	WhenExpressionsSkip SkippingReason = "WhenExpressionsSkip"
+	// ConditionsSkip means the task was skipped due to at least one of its conditions failing
+	ConditionsSkip SkippingReason = "ConditionsSkip"
+	// ParentTasksSkip means the task was skipped because its parent was skipped
+	ParentTasksSkip SkippingReason = "ParentTasksSkip"
+	// IsStoppingSkip means the task was skipped because the pipeline run is stopping
+	IsStoppingSkip SkippingReason = "IsStoppingSkip"
+	// IsGracefullyCancelledSkip means the task was skipped because the pipeline run has been gracefully cancelled
+	IsGracefullyCancelledSkip SkippingReason = "IsGracefullyCancelledSkip"
+	// IsGracefullyStoppedSkip means the task was skipped because the pipeline run has been gracefully stopped
+	IsGracefullyStoppedSkip SkippingReason = "IsGracefullyStoppedSkip"
+	// MissingResultsSkip means the task was skipped because it's missing necessary results
+	MissingResultsSkip SkippingReason = "MissingResultsSkip"
+	// None means the task was not skipped
+	None SkippingReason = "None"
+)
+
 // PipelineRunResult used to describe the results of a pipeline
 type PipelineRunResult struct {
 	// Name is the result's name as declared by the Pipeline

pkg/apis/pipeline/v1beta1/swagger.json

@@ -1889,14 +1889,20 @@
       "description": "SkippedTask is used to describe the Tasks that were skipped due to their When Expressions evaluating to False. This is a struct because we are looking into including more details about the When Expressions that caused this Task to be skipped.",
       "type": "object",
       "required": [
-        "name"
+        "name",
+        "reason"
       ],
       "properties": {
         "name": {
           "description": "Name is the Pipeline Task name",
           "type": "string",
           "default": ""
         },
+        "reason": {
+          "description": "Reason is the cause of the PipelineTask being skipped.",
+          "type": "string",
+          "default": ""
+        },
         "whenExpressions": {
           "description": "WhenExpressions is the list of checks guarding the execution of the PipelineTask",
           "type": "array",

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -1836,9 +1836,11 @@ func runTestReconcileOnCancelledRunFinallyPipelineRun(t *testing.T, embeddedStat
 	verifyTaskRunStatusesCount(t, embeddedStatus, reconciledRun.Status, 0)
 
 	expectedSkippedTasks := []v1beta1.SkippedTask{{
-		Name: "hello-world-1",
+		Name:   "hello-world-1",
+		Reason: v1beta1.IsGracefullyCancelledSkip,
 	}, {
-		Name: "hello-world-2",
+		Name:   "hello-world-2",
+		Reason: v1beta1.IsGracefullyCancelledSkip,
 	}}
 
 	if d := cmp.Diff(expectedSkippedTasks, reconciledRun.Status.SkippedTasks); d != "" {
@@ -2278,7 +2280,7 @@ status:
 		hasNilCompletionTime bool
 		isFailed             bool
 		trInStatusCount      int
-		skippedTasks         []string
+		skippedTasks         []v1beta1.SkippedTask
 	}{
 		{
 			name:                 "stopped PipelineRun",
@@ -2289,7 +2291,7 @@ status:
 			hasNilCompletionTime: false,
 			isFailed:             true,
 			trInStatusCount:      0,
-			skippedTasks:         []string{"hello-world-1"},
+			skippedTasks:         []v1beta1.SkippedTask{{Name: "hello-world-1", Reason: v1beta1.IsGracefullyStoppedSkip}},
 		}, {
 			name:     "with running task",
 			pipeline: simpleHelloWorldPipeline,
@@ -2333,7 +2335,7 @@ status:
 			hasNilCompletionTime: false,
 			isFailed:             true,
 			trInStatusCount:      1,
-			skippedTasks:         []string{"hello-world-2"},
+			skippedTasks:         []v1beta1.SkippedTask{{Name: "hello-world-2", Reason: v1beta1.IsGracefullyStoppedSkip}},
 		},
 	}
 
@@ -2376,12 +2378,7 @@ status:
 				t.Fatalf("Expected %d TaskRuns in status but got %d", tc.trInStatusCount, len(reconciledRun.Status.TaskRuns))
 			}
 
-			var expectedSkipped []v1beta1.SkippedTask
-			for _, st := range tc.skippedTasks {
-				expectedSkipped = append(expectedSkipped, v1beta1.SkippedTask{Name: st})
-			}
-
-			if d := cmp.Diff(expectedSkipped, reconciledRun.Status.SkippedTasks); d != "" {
+			if d := cmp.Diff(tc.skippedTasks, reconciledRun.Status.SkippedTasks); d != "" {
 				t.Fatalf("Didn't get the expected list of skipped tasks. Diff: %s", diff.PrintWantGot(d))
 			}
 
@@ -3985,7 +3982,8 @@ spec:
 
 	actualSkippedTasks := pipelineRun.Status.SkippedTasks
 	expectedSkippedTasks := []v1beta1.SkippedTask{{
-		Name: "c-task",
+		Name:   "c-task",
+		Reason: v1beta1.WhenExpressionsSkip,
 		WhenExpressions: v1beta1.WhenExpressions{{
 			Input:    "aResultValue",
 			Operator: "in",
@@ -4165,30 +4163,34 @@ spec:
 	actualSkippedTasks := pipelineRun.Status.SkippedTasks
 	expectedSkippedTasks := []v1beta1.SkippedTask{{
 		// its when expressions evaluate to false
-		Name: "a-task",
+		Name:   "a-task",
+		Reason: v1beta1.WhenExpressionsSkip,
 		WhenExpressions: v1beta1.WhenExpressions{{
 			Input:    "foo",
 			Operator: "in",
 			Values:   []string{"bar"},
 		}},
 	}, {
 		// its when expressions evaluate to false
-		Name: "c-task",
+		Name:   "c-task",
+		Reason: v1beta1.WhenExpressionsSkip,
 		WhenExpressions: v1beta1.WhenExpressions{{
 			Input:    "foo",
 			Operator: "in",
 			Values:   []string{"bar"},
 		}},
 	}, {
 		// was attempted, but has missing results references
-		Name: "e-task",
+		Name:   "e-task",
+		Reason: v1beta1.MissingResultsSkip,
 		WhenExpressions: v1beta1.WhenExpressions{{
 			Input:    "$(tasks.a-task.results.aResult)",
 			Operator: "in",
 			Values:   []string{"aResultValue"},
 		}},
 	}, {
-		Name: "f-task",
+		Name:   "f-task",
+		Reason: v1beta1.ParentTasksSkip,
 	}}
 	if d := cmp.Diff(expectedSkippedTasks, actualSkippedTasks); d != "" {
 		t.Errorf("expected to find Skipped Tasks %v. Diff %s", expectedSkippedTasks, diff.PrintWantGot(d))
@@ -4311,7 +4313,8 @@ status:
 	actualSkippedTasks := pipelineRun.Status.SkippedTasks
 	expectedSkippedTasks := []v1beta1.SkippedTask{{
 		// its when expressions evaluate to false
-		Name: "b-task",
+		Name:   "b-task",
+		Reason: v1beta1.WhenExpressionsSkip,
 		WhenExpressions: v1beta1.WhenExpressions{{
 			Input:    "aResultValue",
 			Operator: "in",
@@ -6630,18 +6633,22 @@ spec:
 		t.Errorf("expected to see TaskRun %v created. Diff %s", expectedTaskRunName, diff.PrintWantGot(d))
 	}
 	expectedSkippedTasks := []v1beta1.SkippedTask{{
-		Name: "final-task-2",
+		Name:   "final-task-2",
+		Reason: v1beta1.MissingResultsSkip,
 	}, {
-		Name: "final-task-3",
+		Name:   "final-task-3",
+		Reason: v1beta1.WhenExpressionsSkip,
 		WhenExpressions: v1beta1.WhenExpressions{{
 			Input:    "aResultValue",
 			Operator: "notin",
 			Values:   []string{"aResultValue"},
 		}},
 	}, {
-		Name: "final-task-5",
+		Name:   "final-task-5",
+		Reason: v1beta1.MissingResultsSkip,
 	}, {
-		Name: "final-task-6",
+		Name:   "final-task-6",
+		Reason: v1beta1.MissingResultsSkip,
 	}}
 
 	if d := cmp.Diff(expectedSkippedTasks, reconciledRun.Status.SkippedTasks); d != "" {

pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go

@@ -39,32 +39,10 @@ const (
 	ReasonConditionCheckFailed = "ConditionCheckFailed"
 )
 
-// SkippingReason explains why a task was skipped
-type SkippingReason string
-
-const (
-	// WhenExpressionsSkip means the task was skipped due to at least one of its when expressions evaluating to false
-	WhenExpressionsSkip SkippingReason = "WhenExpressionsSkip"
-	// ConditionsSkip means the task was skipped due to at least one of its conditions failing
-	ConditionsSkip SkippingReason = "ConditionsSkip"
-	// ParentTasksSkip means the task was skipped because its parent was skipped
-	ParentTasksSkip SkippingReason = "ParentTasksSkip"
-	// IsStoppingSkip means the task was skipped because the pipeline run is stopping
-	IsStoppingSkip SkippingReason = "IsStoppingSkip"
-	// IsGracefullyCancelledSkip means the task was skipped because the pipeline run has been gracefully cancelled
-	IsGracefullyCancelledSkip SkippingReason = "IsGracefullyCancelledSkip"
-	// IsGracefullyStoppedSkip means the task was skipped because the pipeline run has been gracefully stopped
-	IsGracefullyStoppedSkip SkippingReason = "IsGracefullyStoppedSkip"
-	// MissingResultsSkip means the task was skipped because it's missing necessary results
-	MissingResultsSkip SkippingReason = "MissingResultsSkip"
-	// None means the task was not skipped
-	None SkippingReason = "None"
-)
-
 // TaskSkipStatus stores whether a task was skipped and why
 type TaskSkipStatus struct {
 	IsSkipped      bool
-	SkippingReason SkippingReason
+	SkippingReason v1beta1.SkippingReason
 }
 
 // TaskNotFoundError indicates that the resolution failed because a referenced Task couldn't be retrieved
@@ -231,31 +209,31 @@ func (t *ResolvedPipelineRunTask) checkParentsDone(facts *PipelineRunFacts) bool
 }
 
 func (t *ResolvedPipelineRunTask) skip(facts *PipelineRunFacts) TaskSkipStatus {
-	var skippingReason SkippingReason
+	var skippingReason v1beta1.SkippingReason
 
 	switch {
 	case facts.isFinalTask(t.PipelineTask.Name) || t.IsStarted():
-		skippingReason = None
+		skippingReason = v1beta1.None
 	case facts.IsStopping():
-		skippingReason = IsStoppingSkip
+		skippingReason = v1beta1.IsStoppingSkip
 	case facts.IsGracefullyCancelled():
-		skippingReason = IsGracefullyCancelledSkip
+		skippingReason = v1beta1.IsGracefullyCancelledSkip
 	case facts.IsGracefullyStopped():
-		skippingReason = IsGracefullyStoppedSkip
+		skippingReason = v1beta1.IsGracefullyStoppedSkip
 	case t.skipBecauseParentTaskWasSkipped(facts):
-		skippingReason = ParentTasksSkip
+		skippingReason = v1beta1.ParentTasksSkip
 	case t.skipBecauseConditionsFailed():
-		skippingReason = ConditionsSkip
+		skippingReason = v1beta1.ConditionsSkip
 	case t.skipBecauseResultReferencesAreMissing(facts):
-		skippingReason = MissingResultsSkip
+		skippingReason = v1beta1.MissingResultsSkip
 	case t.skipBecauseWhenExpressionsEvaluatedToFalse(facts):
-		skippingReason = WhenExpressionsSkip
+		skippingReason = v1beta1.WhenExpressionsSkip
 	default:
-		skippingReason = None
+		skippingReason = v1beta1.None
 	}
 
 	return TaskSkipStatus{
-		IsSkipped:      skippingReason != None,
+		IsSkipped:      skippingReason != v1beta1.None,
 		SkippingReason: skippingReason,
 	}
 }
@@ -312,7 +290,7 @@ func (t *ResolvedPipelineRunTask) skipBecauseParentTaskWasSkipped(facts *Pipelin
 		if parentSkipStatus := parentTask.Skip(facts); parentSkipStatus.IsSkipped {
 			// if the parent task was skipped due to its `when` expressions,
 			// then we should ignore that and continue evaluating if we should skip because of other parent tasks
-			if parentSkipStatus.SkippingReason == WhenExpressionsSkip {
+			if parentSkipStatus.SkippingReason == v1beta1.WhenExpressionsSkip {
 				continue
 			}
 			return true
@@ -327,7 +305,7 @@ func (t *ResolvedPipelineRunTask) skipBecauseResultReferencesAreMissing(facts *P
 	if t.checkParentsDone(facts) && t.hasResultReferences() {
 		resolvedResultRefs, pt, err := ResolveResultRefs(facts.State, PipelineRunState{t})
 		rprt := facts.State.ToMap()[pt]
-		if err != nil && (t.IsFinalTask(facts) || rprt.Skip(facts).SkippingReason == WhenExpressionsSkip) {
+		if err != nil && (t.IsFinalTask(facts) || rprt.Skip(facts).SkippingReason == v1beta1.WhenExpressionsSkip) {
 			return true
 		}
 		ApplyTaskResults(PipelineRunState{t}, resolvedResultRefs)
@@ -343,26 +321,26 @@ func (t *ResolvedPipelineRunTask) IsFinalTask(facts *PipelineRunFacts) bool {
 
 // IsFinallySkipped returns true if a finally task is not executed and skipped due to task result validation failure
 func (t *ResolvedPipelineRunTask) IsFinallySkipped(facts *PipelineRunFacts) TaskSkipStatus {
-	var skippingReason SkippingReason
+	var skippingReason v1beta1.SkippingReason
 
 	switch {
 	case t.IsStarted():
-		skippingReason = None
+		skippingReason = v1beta1.None
 	case facts.checkDAGTasksDone() && facts.isFinalTask(t.PipelineTask.Name):
 		switch {
 		case t.skipBecauseResultReferencesAreMissing(facts):
-			skippingReason = MissingResultsSkip
+			skippingReason = v1beta1.MissingResultsSkip
 		case t.skipBecauseWhenExpressionsEvaluatedToFalse(facts):
-			skippingReason = WhenExpressionsSkip
+			skippingReason = v1beta1.WhenExpressionsSkip
 		default:
-			skippingReason = None
+			skippingReason = v1beta1.None
 		}
 	default:
-		skippingReason = None
+		skippingReason = v1beta1.None
 	}
 
 	return TaskSkipStatus{
-		IsSkipped:      skippingReason != None,
+		IsSkipped:      skippingReason != v1beta1.None,
 		SkippingReason: skippingReason,
 	}
 

pkg/reconciler/pipelinerun/resources/pipelinerunstate.go

@@ -556,17 +556,19 @@ func (facts *PipelineRunFacts) GetSkippedTasks() []v1beta1.SkippedTask {
 		if rprt.Skip(facts).IsSkipped {
 			skippedTask := v1beta1.SkippedTask{
 				Name:            rprt.PipelineTask.Name,
+				Reason:          rprt.Skip(facts).SkippingReason,
 				WhenExpressions: rprt.PipelineTask.WhenExpressions,
 			}
 			skipped = append(skipped, skippedTask)
 		}
 		if rprt.IsFinallySkipped(facts).IsSkipped {
 			skippedTask := v1beta1.SkippedTask{
-				Name: rprt.PipelineTask.Name,
+				Name:   rprt.PipelineTask.Name,
+				Reason: rprt.IsFinallySkipped(facts).SkippingReason,
 			}
 			// include the when expressions only when the finally task was skipped because
 			// its when expressions evaluated to false (not because results variables were missing)
-			if rprt.IsFinallySkipped(facts).SkippingReason == WhenExpressionsSkip {
+			if rprt.IsFinallySkipped(facts).SkippingReason == v1beta1.WhenExpressionsSkip {
 				skippedTask.WhenExpressions = rprt.PipelineTask.WhenExpressions
 			}
 			skipped = append(skipped, skippedTask)