Skip to content

Commit

Permalink
Merge pull request #1 from chitrangpatel/TEP-0103-Skipping-Reason
Browse files Browse the repository at this point in the history
Added SkippingReason field in the `SkippedTasks` field of `PipelineRunStatus`.
  • Loading branch information
chitrangpatel committed Apr 26, 2022
2 parents 18fa62e + a0cb92f commit af71639
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 70 deletions.
10 changes: 9 additions & 1 deletion pkg/apis/pipeline/v1beta1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 24 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipelinerun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 skiped.
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
Expand Down
8 changes: 7 additions & 1 deletion pkg/apis/pipeline/v1beta1/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -1883,14 +1883,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",
Expand Down
49 changes: 28 additions & 21 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down Expand Up @@ -2278,7 +2280,7 @@ status:
hasNilCompletionTime bool
isFailed bool
trInStatusCount int
skippedTasks []string
skippedTasks []v1beta1.SkippedTask
}{
{
name: "stopped PipelineRun",
Expand All @@ -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,
Expand Down Expand Up @@ -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}},
},
}

Expand Down Expand Up @@ -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))
}

Expand Down Expand Up @@ -3987,7 +3984,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",
Expand Down Expand Up @@ -4167,30 +4165,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))
Expand Down Expand Up @@ -4313,7 +4315,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",
Expand Down Expand Up @@ -6632,18 +6635,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 != "" {
Expand Down
64 changes: 21 additions & 43 deletions pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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,
}

Expand Down
6 changes: 4 additions & 2 deletions pkg/reconciler/pipelinerun/resources/pipelinerunstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1890,7 +1890,8 @@ func TestPipelineRunFacts_GetSkippedTasks(t *testing.T) {
dagTasks: []v1beta1.PipelineTask{pts[0]},
finallyTasks: []v1beta1.PipelineTask{pts[14]},
expectedSkippedTasks: []v1beta1.SkippedTask{{
Name: pts[14].Name,
Name: pts[14].Name,
Reason: v1beta1.MissingResultsSkip,
}},
}, {
name: "when-expressions-skip-finally",
Expand All @@ -1899,7 +1900,8 @@ func TestPipelineRunFacts_GetSkippedTasks(t *testing.T) {
}},
finallyTasks: []v1beta1.PipelineTask{pts[10]},
expectedSkippedTasks: []v1beta1.SkippedTask{{
Name: pts[10].Name,
Name: pts[10].Name,
Reason: v1beta1.WhenExpressionsSkip,
WhenExpressions: []v1beta1.WhenExpression{{
Input: "foo",
Operator: "notin",
Expand Down

0 comments on commit af71639

Please sign in to comment.