Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added SkippingReason field in the SkippedTasks field of PipelineRunStatus. #1

Merged
merged 2 commits into from
Apr 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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