diff --git a/docs/pipelines.md b/docs/pipelines.md index 188c3d5ff22..667654e5581 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -6,27 +6,47 @@ weight: 3 --> # Pipelines -- [Overview](#pipelines) -- [Configuring a `Pipeline`](#configuring-a-pipeline) +- [Pipelines](#pipelines) + - [Overview](#overview) + - [Configuring a `Pipeline`](#configuring-a-pipeline) - [Specifying `Resources`](#specifying-resources) - [Specifying `Workspaces`](#specifying-workspaces) - [Specifying `Parameters`](#specifying-parameters) - [Adding `Tasks` to the `Pipeline`](#adding-tasks-to-the-pipeline) - - [Using Tekton Bundles](#tekton-bundles) + - [Tekton Bundles](#tekton-bundles) - [Using the `from` parameter](#using-the-from-parameter) - [Using the `runAfter` parameter](#using-the-runafter-parameter) - [Using the `retries` parameter](#using-the-retries-parameter) - - [Guard `Task` execution using `When Expressions`](#guard-task-execution-using-whenexpressions) + - [Guard `Task` execution using `WhenExpressions`](#guard-task-execution-using-whenexpressions) - [Guard `Task` execution using `Conditions`](#guard-task-execution-using-conditions) - [Configuring the failure timeout](#configuring-the-failure-timeout) - [Using variable substitution](#using-variable-substitution) - [Using `Results`](#using-results) - - [Passing one Task's `Results` into the `Parameters` of another](#passing-one-tasks-results-into-the-parameters-of-another) + - [Passing one Task's `Results` into the `Parameters` or `WhenExpressions` of another](#passing-one-tasks-results-into-the-parameters-or-whenexpressions-of-another) - [Emitting `Results` from a `Pipeline`](#emitting-results-from-a-pipeline) - [Configuring the `Task` execution order](#configuring-the-task-execution-order) - [Adding a description](#adding-a-description) - [Adding `Finally` to the `Pipeline`](#adding-finally-to-the-pipeline) + - [Specifying `Workspaces` in Final Tasks](#specifying-workspaces-in-final-tasks) + - [Specifying `Parameters` in Final Tasks](#specifying-parameters-in-final-tasks) + - [Consuming `Task` execution results in `finally`](#consuming-task-execution-results-in-finally) + - [`PipelineRun` Status with `finally`](#pipelinerun-status-with-finally) + - [Using Execution `Status` of `pipelineTask`](#using-execution-status-of-pipelinetask) + - [Guard `Finally Task` execution using `WhenExpressions`](#guard-finally-task-execution-using-whenexpressions) + - [`WhenExpressions` using `Parameters` in `Finally Tasks`](#whenexpressions-using-parameters-in-finally-tasks) + - [`WhenExpressions` using `Results` in `Finally Tasks`](#whenexpressions-using-results-in-finally-tasks) + - [`WhenExpressions` using `Execution Status` of `PipelineTask` in `Finally Tasks`](#whenexpressions-using-execution-status-of-pipelinetask-in-finally-tasks) + - [Known Limitations](#known-limitations) + - [Specifying `Resources` in Final Tasks](#specifying-resources-in-final-tasks) + - [Cannot configure the Final Task execution order](#cannot-configure-the-final-task-execution-order) + - [Cannot specify execution `Conditions` in Final Tasks](#cannot-specify-execution-conditions-in-final-tasks) + - [Cannot configure `Pipeline` result with `finally`](#cannot-configure-pipeline-result-with-finally) - [Using Custom Tasks](#using-custom-tasks) + - [Specifying the target Custom Task](#specifying-the-target-custom-task) + - [Specifying parameters](#specifying-parameters-1) + - [Specifying workspaces](#specifying-workspaces-1) + - [Using `Results`](#using-results-1) + - [Limitations](#limitations) - [Code examples](#code-examples) ## Overview @@ -788,7 +808,7 @@ spec: value: "someURL" ``` -#### Consuming `Task` execution results in `finally` +### Consuming `Task` execution results in `finally` Final tasks can be configured to consume `Results` of `PipelineTask` from the `tasks` section: @@ -880,10 +900,119 @@ This kind of variable can have any one of the values from the following table: For an end-to-end example, see [`status` in a `PipelineRun`](../examples/v1beta1/pipelineruns/pipelinerun-task-execution-status.yaml). +### Guard `Finally Task` execution using `WhenExpressions` + +Similar to `Tasks`, `Finally Tasks` can be guarded using [`WhenExpressions`](#guard-task-execution-using-whenexpressions) +that operate on static inputs or variables. Like in `Tasks`, `WhenExpressions` in `Finally Tasks` can operate on +`Parameters` and `Results`. Unlike in `Tasks`, `WhenExpressions` in `Finally Tasks` can also operate on the [`Execution +Status`](#using-execution-status-of-pipelinetask) of `Tasks`. + +#### `WhenExpressions` using `Parameters` in `Finally Tasks` + +`WhenExpressions` in `Finally Tasks` can utilize `Parameters` as demonstrated using [`golang-build`](https://github.com/tektoncd/catalog/tree/master/task/golang-build/0.1) +and [`send-to-channel-slack`](https://github.com/tektoncd/catalog/tree/master/task/send-to-channel-slack/0.1) Catalog +`Tasks`: + +```yaml +apiVersion: tekton.dev/v1beta1 +kind: PipelineRun +metadata: + generateName: pipelinerun- +spec: + pipelineSpec: + params: + - name: enable-notifications + type: string + description: a boolean indicating whether the notifications should be sent + tasks: + - name: golang-build + taskRef: + name: golang-build + # […] + finally: + - name: notify-build-failure # executed only when build task fails and notifications are enabled + when: + - input: $(tasks.golang-build.status) + operator: in + values: ["Failed"] + - input: $(params.enable-notifications) + operator: in + values: ["true"] + taskRef: + name: send-to-slack-channel + # […] + params: + - name: enable-notifications + value: true +``` + +#### `WhenExpressions` using `Results` in `Finally Tasks` + +`WhenExpressions` in `Finally Tasks` can utilize `Results`, as demonstrated using [`boskos-acquire`](https://github.com/tektoncd/catalog/tree/master/task/boskos-acquire/0.1) +and [`boskos-release`](https://github.com/tektoncd/catalog/tree/master/task/boskos-release/0.1) Catalog `Tasks`: + +```yaml +apiVersion: tekton.dev/v1beta1 +kind: PipelineRun +metadata: + generateName: pipelinerun- +spec: + pipelineSpec: + tasks: + - name: boskos-acquire + taskRef: + name: boskos-acquire + - name: use-resource + # […] + finally: + - name: boskos-release # executed only when leased resource is phonetic-project + when: + - input: $(tasks.boskos-acquire.results.leased-resource) + operator: in + values: ["phonetic-project"] + taskRef: + name: boskos-release + # […] +``` + +If the `WhenExpressions` in a `Finally Task` use `Results` from a skipped or failed non-finally `Tasks`, then the +`Finally Task` would also be skipped and be included in the list of `Skipped Tasks` in the `Status`, [similarly to when using +`Results` in other parts of the `Finally Task`](https://github.com/tektoncd/pipeline/blob/master/docs/pipelines.md#consuming-task-execution-results-in-finally). + +#### `WhenExpressions` using `Execution Status` of `PipelineTask` in `Finally Tasks` + +`WhenExpressions` in `Finally Tasks` can utilize [`Execution Status` of `PipelineTasks`](#using-execution-status-of-pipelinetask), +as as demonstrated using [`golang-build`](https://github.com/tektoncd/catalog/tree/master/task/golang-build/0.1) and +[`send-to-channel-slack`](https://github.com/tektoncd/catalog/tree/master/task/send-to-channel-slack/0.1) Catalog `Tasks`: + +```yaml +apiVersion: tekton.dev/v1beta1 +kind: PipelineRun +metadata: + generateName: pipelinerun- +spec: + pipelineSpec: + tasks: + - name: golang-build + taskRef: + name: golang-build + # […] + finally: + - name: notify-build-failure # executed only when build task fails + when: + - input: $(tasks.golang-build.status) + operator: in + values: ["Failed"] + taskRef: + name: send-to-slack-channel + # […] +``` + +For an end-to-end example, see [PipelineRun with WhenExpressions](../examples/v1beta1/pipelineruns/pipelinerun-with-when-expressions.yaml). ### Known Limitations -### Specifying `Resources` in Final Tasks +#### Specifying `Resources` in Final Tasks Similar to `tasks`, you can use [PipelineResources](#specifying-resources) as inputs and outputs for final tasks in the Pipeline. The only difference here is, final tasks with an input resource can not have a `from` clause @@ -914,13 +1043,13 @@ spec: - tests ``` -### Cannot configure the Final Task execution order +#### Cannot configure the Final Task execution order It's not possible to configure or modify the execution order of the final tasks. Unlike `Tasks` in a `Pipeline`, all final tasks run simultaneously and start executing once all `PipelineTasks` under `tasks` have settled which means no `runAfter` can be specified in final tasks. -### Cannot specify execution `Conditions` in Final Tasks +#### Cannot specify execution `Conditions` in Final Tasks `Tasks` in a `Pipeline` can be configured to run only if some conditions are satisfied using `conditions`. But the final tasks are guaranteed to be executed after all `PipelineTasks` therefore no `conditions` can be specified in diff --git a/examples/v1beta1/pipelineruns/pipelinerun-with-when-expressions.yaml b/examples/v1beta1/pipelineruns/pipelinerun-with-when-expressions.yaml index 6a552d508c9..cb3161a557c 100644 --- a/examples/v1beta1/pipelineruns/pipelinerun-with-when-expressions.yaml +++ b/examples/v1beta1/pipelineruns/pipelinerun-with-when-expressions.yaml @@ -100,7 +100,47 @@ spec: image: ubuntu script: exit 1 finally: - - name: do-something-finally + - name: finally-task-should-be-skipped-1 # when expression using execution status, evaluates to false + when: + - input: "$(tasks.echo-file-exists.status)" + operator: in + values: ["Failure"] + taskSpec: + steps: + - name: echo + image: ubuntu + script: exit 1 + - name: finally-task-should-be-skipped-2 # when expression using task result, evaluates to false + when: + - input: "$(tasks.check-file.results.exists)" + operator: in + values: ["missing"] + taskSpec: + steps: + - name: echo + image: ubuntu + script: exit 1 + - name: finally-task-should-be-skipped-3 # when expression using parameter, evaluates to false + when: + - input: "$(params.path)" + operator: notin + values: ["README.md"] + taskSpec: + steps: + - name: echo + image: ubuntu + script: exit 1 + - name: finally-task-should-be-executed # when expression using execution status, param and results + when: + - input: "$(tasks.echo-file-exists.status)" + operator: in + values: ["Succeeded"] + - input: "$(tasks.check-file.results.exists)" + operator: in + values: ["yes"] + - input: "$(params.path)" + operator: in + values: ["README.md"] taskSpec: steps: - name: echo diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index 556966b12f5..33966e2bb84 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -69,7 +69,7 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(validatePipelineResults(ps.Results)) errs = errs.Also(validateTasksAndFinallySection(ps)) errs = errs.Also(validateFinalTasks(ps.Tasks, ps.Finally)) - errs = errs.Also(validateWhenExpressions(ps.Tasks)) + errs = errs.Also(validateWhenExpressions(ps.Tasks, ps.Finally)) return errs } @@ -328,22 +328,32 @@ func validateExecutionStatusVariablesInFinally(tasks []PipelineTask, finally []P ptNames := PipelineTaskList(tasks).Names() for idx, t := range finally { for _, param := range t.Params { - // retrieve a list of substitution expression from a param - if ps, ok := GetVarSubstitutionExpressionsForParam(param); ok { - // validate tasks.pipelineTask.status if this expression is not a result reference - if !LooksLikeContainsResultRefs(ps) { - for _, p := range ps { - // check if it contains context variable accessing execution status - $(tasks.taskname.status) - if containsExecutionStatusRef(p) { - // strip tasks. and .status from tasks.taskname.status to further verify task name - pt := strings.TrimSuffix(strings.TrimPrefix(p, "tasks."), ".status") - // report an error if the task name does not exist in the list of dag tasks - if !ptNames.Has(pt) { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("pipeline task %s is not defined in the pipeline", pt), - "value").ViaFieldKey("params", param.Name).ViaFieldIndex("finally", idx)) - } - } - } + if expressions, ok := GetVarSubstitutionExpressionsForParam(param); ok { + errs = errs.Also(validateExecutionStatusVariablesExpressions(expressions, ptNames, "value").ViaFieldKey( + "params", param.Name).ViaFieldIndex("finally", idx)) + } + } + for i, we := range t.WhenExpressions { + if expressions, ok := we.GetVarSubstitutionExpressions(); ok { + errs = errs.Also(validateExecutionStatusVariablesExpressions(expressions, ptNames, "").ViaFieldIndex( + "when", i).ViaFieldIndex("finally", idx)) + } + } + } + return errs +} + +func validateExecutionStatusVariablesExpressions(expressions []string, ptNames sets.String, fieldPath string) (errs *apis.FieldError) { + // validate tasks.pipelineTask.status if this expression is not a result reference + if !LooksLikeContainsResultRefs(expressions) { + for _, expression := range expressions { + // check if it contains context variable accessing execution status - $(tasks.taskname.status) + if containsExecutionStatusRef(expression) { + // strip tasks. and .status from tasks.taskname.status to further verify task name + pt := strings.TrimSuffix(strings.TrimPrefix(expression, "tasks."), ".status") + // report an error if the task name does not exist in the list of dag tasks + if !ptNames.Has(pt) { + errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("pipeline task %s is not defined in the pipeline", pt), fieldPath)) } } } @@ -428,9 +438,6 @@ func validateFinalTasks(tasks []PipelineTask, finalTasks []PipelineTask) *apis.F if len(f.Conditions) != 0 { return apis.ErrInvalidValue(fmt.Sprintf("no conditions allowed under spec.finally, final task %s has conditions specified", f.Name), "").ViaFieldIndex("finally", idx) } - if len(f.WhenExpressions) != 0 { - return apis.ErrInvalidValue(fmt.Sprintf("no when expressions allowed under spec.finally, final task %s has when expressions specified", f.Name), "").ViaFieldIndex("finally", idx) - } } ts := PipelineTaskList(tasks).Names() @@ -486,11 +493,14 @@ func validateTasksInputFrom(tasks []PipelineTask) (errs *apis.FieldError) { return errs } -func validateWhenExpressions(tasks []PipelineTask) (errs *apis.FieldError) { +func validateWhenExpressions(tasks []PipelineTask, finalTasks []PipelineTask) (errs *apis.FieldError) { for i, t := range tasks { errs = errs.Also(validateOneOfWhenExpressionsOrConditions(t).ViaFieldIndex("tasks", i)) errs = errs.Also(t.WhenExpressions.validate().ViaFieldIndex("tasks", i)) } + for i, t := range finalTasks { + errs = errs.Also(t.WhenExpressions.validate().ViaFieldIndex("finally", i)) + } return errs } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index a97d47c8c80..ec045553d01 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -271,10 +271,19 @@ func TestPipelineSpec_Validate_Failure(t *testing.T) { Values: []string{"foo"}, }}, }}, + Finally: []PipelineTask{{ + Name: "invalid-pipeline-task-finally", + TaskRef: &TaskRef{Name: "bar-task"}, + WhenExpressions: []WhenExpression{{ + Input: "foo", + Operator: selection.Exists, + Values: []string{"foo"}, + }}, + }}, }, expectedError: apis.FieldError{ Message: `invalid value: operator "exists" is not recognized. valid operators: in,notin`, - Paths: []string{"tasks[0].when[0]"}, + Paths: []string{"tasks[0].when[0]", "finally[0].when[0]"}, }, }, { name: "invalid pipeline with one pipeline task having when expression with invalid values (empty)", @@ -289,10 +298,19 @@ func TestPipelineSpec_Validate_Failure(t *testing.T) { Values: []string{}, }}, }}, + Finally: []PipelineTask{{ + Name: "invalid-pipeline-task-finally", + TaskRef: &TaskRef{Name: "foo-task"}, + WhenExpressions: []WhenExpression{{ + Input: "foo", + Operator: selection.In, + Values: []string{}, + }}, + }}, }, expectedError: apis.FieldError{ Message: `invalid value: expecting non-empty values field`, - Paths: []string{"tasks[0].when[0]"}, + Paths: []string{"tasks[0].when[0]", "finally[0].when[0]"}, }, }, { name: "invalid pipeline with one pipeline task having when expression with invalid operator (missing)", @@ -306,10 +324,18 @@ func TestPipelineSpec_Validate_Failure(t *testing.T) { Values: []string{"foo"}, }}, }}, + Finally: []PipelineTask{{ + Name: "invalid-pipeline-task-finally", + TaskRef: &TaskRef{Name: "foo-task"}, + WhenExpressions: []WhenExpression{{ + Input: "foo", + Values: []string{"foo"}, + }}, + }}, }, expectedError: apis.FieldError{ Message: `invalid value: operator "" is not recognized. valid operators: in,notin`, - Paths: []string{"tasks[0].when[0]"}, + Paths: []string{"tasks[0].when[0]", "finally[0].when[0]"}, }, }, { name: "invalid pipeline with one pipeline task having when expression with invalid values (missing)", @@ -323,10 +349,18 @@ func TestPipelineSpec_Validate_Failure(t *testing.T) { Operator: selection.In, }}, }}, + Finally: []PipelineTask{{ + Name: "invalid-pipeline-task-finally", + TaskRef: &TaskRef{Name: "foo-task"}, + WhenExpressions: []WhenExpression{{ + Input: "foo", + Operator: selection.In, + }}, + }}, }, expectedError: apis.FieldError{ Message: `invalid value: expecting non-empty values field`, - Paths: []string{"tasks[0].when[0]"}, + Paths: []string{"tasks[0].when[0]", "finally[0].when[0]"}, }, }, { name: "invalid pipeline with one pipeline task having when expression with misconfigured result reference", @@ -344,10 +378,19 @@ func TestPipelineSpec_Validate_Failure(t *testing.T) { Values: []string{"bar"}, }}, }}, + Finally: []PipelineTask{{ + Name: "invalid-pipeline-task-finally", + TaskRef: &TaskRef{Name: "foo-task"}, + WhenExpressions: []WhenExpression{{ + Input: "$(tasks.a-task.resultTypo.bResult)", + Operator: selection.In, + Values: []string{"bar"}, + }}, + }}, }, expectedError: apis.FieldError{ Message: `invalid value: expected all of the expressions [tasks.a-task.resultTypo.bResult] to be result expressions but only [] were`, - Paths: []string{"tasks[1].when[0]"}, + Paths: []string{"tasks[1].when[0]", "finally[0].when[0]"}, }, }, { name: "invalid pipeline with one pipeline task having blank when expression", @@ -361,10 +404,15 @@ func TestPipelineSpec_Validate_Failure(t *testing.T) { TaskRef: &TaskRef{Name: "foo-task"}, WhenExpressions: []WhenExpression{{}}, }}, + Finally: []PipelineTask{{ + Name: "invalid-pipeline-task-finally", + TaskRef: &TaskRef{Name: "foo-task"}, + WhenExpressions: []WhenExpression{{}}, + }}, }, expectedError: apis.FieldError{ Message: `missing field(s)`, - Paths: []string{"tasks[1].when[0]"}, + Paths: []string{"tasks[1].when[0]", "finally[0].when[0]"}, }, }, { name: "invalid pipeline with pipeline task having reference to resources which does not exist", @@ -2007,21 +2055,6 @@ func TestValidateFinalTasks_Failure(t *testing.T) { Message: `invalid value: invalid task result reference, final task param param1 has task result reference from a task which is not defined in the pipeline`, Paths: []string{"finally[0].params"}, }, - }, { - name: "invalid pipeline with final task specifying when expressions", - finalTasks: []PipelineTask{{ - Name: "final-task", - TaskRef: &TaskRef{Name: "final-task"}, - WhenExpressions: []WhenExpression{{ - Input: "foo", - Operator: selection.In, - Values: []string{"foo", "bar"}, - }}, - }}, - expectedError: apis.FieldError{ - Message: `invalid value: no when expressions allowed under spec.finally, final task final-task has when expressions specified`, - Paths: []string{"finally[0]"}, - }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -2249,10 +2282,18 @@ func TestPipelineTasksExecutionStatus(t *testing.T) { Params: []Param{{ Name: "notask-status", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(tasks.notask.status)"}, }}, + }, { + Name: "foo", + TaskRef: &TaskRef{Name: "foo-task"}, + WhenExpressions: WhenExpressions{{ + Input: "$(tasks.notask.status)", + Operator: selection.In, + Values: []string{"Success"}, + }}, }}, expectedError: apis.FieldError{ Message: `invalid value: pipeline task notask is not defined in the pipeline`, - Paths: []string{"finally[0].params[notask-status].value"}, + Paths: []string{"finally[0].params[notask-status].value", "finally[1].when[0]"}, }, }, { name: "invalid variable concatenated with extra string in finally accessing missing pipelineTask status", diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 5d8115cbbaf..798557465e5 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -4704,6 +4704,11 @@ func TestReconcileWithTaskResultsInFinalTasks(t *testing.T) { StringVal: "$(tasks.dag-task-1.results.aResult)", }}, }, + WhenExpressions: v1beta1.WhenExpressions{{ + Input: "$(tasks.dag-task-1.results.aResult)", + Operator: selection.In, + Values: []string{"aResultValue"}, + }}, }, { Name: "final-task-2", TaskRef: &v1beta1.TaskRef{Name: "final-task"}, @@ -4714,6 +4719,21 @@ func TestReconcileWithTaskResultsInFinalTasks(t *testing.T) { StringVal: "$(tasks.dag-task-2.results.aResult)", }}, }, + }, { + Name: "final-task-3", + TaskRef: &v1beta1.TaskRef{Name: "final-task"}, + Params: []v1beta1.Param{{ + Name: "finalParam", + Value: v1beta1.ArrayOrString{ + Type: "string", + StringVal: "$(tasks.dag-task-2.results.aResult)", + }}, + }, + WhenExpressions: v1beta1.WhenExpressions{{ + Input: "$(tasks.dag-task-1.results.aResult)", + Operator: selection.NotIn, + Values: []string{"aResultValue"}, + }}, }}, }, }} @@ -4855,10 +4875,12 @@ func TestReconcileWithTaskResultsInFinalTasks(t *testing.T) { } expectedSkippedTasks := []v1beta1.SkippedTask{{ Name: "final-task-2", + }, { + Name: "final-task-3", }} if d := cmp.Diff(expectedSkippedTasks, reconciledRun.Status.SkippedTasks); d != "" { - t.Fatalf("Expected to see only one final task (final-task-2) in the list of skipped tasks. Diff: %s", diff.PrintWantGot(d)) + t.Fatalf("Didn't get the expected the list of skipped tasks. Diff: %s", diff.PrintWantGot(d)) } } diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index e327924e532..409b6b1a525 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -93,6 +93,7 @@ func ApplyPipelineTaskContext(state PipelineRunState, replacements map[string]st if resolvedPipelineRunTask.PipelineTask != nil { pipelineTask := resolvedPipelineRunTask.PipelineTask.DeepCopy() pipelineTask.Params = replaceParamValues(pipelineTask.Params, replacements, nil) + pipelineTask.WhenExpressions = pipelineTask.WhenExpressions.ReplaceWhenExpressionsVariables(replacements) resolvedPipelineRunTask.PipelineTask = pipelineTask } } @@ -129,6 +130,7 @@ func ApplyReplacements(p *v1beta1.PipelineSpec, replacements map[string]string, for i := range p.Finally { p.Finally[i].Params = replaceParamValues(p.Finally[i].Params, replacements, arrayReplacements) + p.Finally[i].WhenExpressions = p.Finally[i].WhenExpressions.ReplaceWhenExpressionsVariables(replacements) } return p diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index 8db62221be4..0c44b050d8e 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -197,6 +197,11 @@ func TestApplyParameters(t *testing.T) { {Name: "final-task-first-param", Value: *v1beta1.NewArrayOrString("$(params.first-param)")}, {Name: "final-task-second-param", Value: *v1beta1.NewArrayOrString("$(params.second-param)")}, }, + WhenExpressions: v1beta1.WhenExpressions{{ + Input: "$(params.first-param)", + Operator: selection.In, + Values: []string{"$(params.second-param)"}, + }}, }}, }, params: []v1beta1.Param{{Name: "second-param", Value: *v1beta1.NewArrayOrString("second-value")}}, @@ -210,6 +215,11 @@ func TestApplyParameters(t *testing.T) { {Name: "final-task-first-param", Value: *v1beta1.NewArrayOrString("default-value")}, {Name: "final-task-second-param", Value: *v1beta1.NewArrayOrString("second-value")}, }, + WhenExpressions: v1beta1.WhenExpressions{{ + Input: "default-value", + Operator: selection.In, + Values: []string{"second-value"}, + }}, }}, }, }, { @@ -230,6 +240,11 @@ func TestApplyParameters(t *testing.T) { {Name: "final-task-first-param", Value: *v1beta1.NewArrayOrString("$(params.first-param)")}, {Name: "final-task-second-param", Value: *v1beta1.NewArrayOrString("$(params.second-param)")}, }, + WhenExpressions: v1beta1.WhenExpressions{{ + Input: "$(params.first-param)", + Operator: selection.In, + Values: []string{"$(params.second-param)"}, + }}, }}, }, params: []v1beta1.Param{{Name: "second-param", Value: *v1beta1.NewArrayOrString("second-value")}}, @@ -249,6 +264,11 @@ func TestApplyParameters(t *testing.T) { {Name: "final-task-first-param", Value: *v1beta1.NewArrayOrString("default-value")}, {Name: "final-task-second-param", Value: *v1beta1.NewArrayOrString("second-value")}, }, + WhenExpressions: v1beta1.WhenExpressions{{ + Input: "default-value", + Operator: selection.In, + Values: []string{"second-value"}, + }}, }}, }, }} { @@ -1062,6 +1082,11 @@ func TestApplyTaskRunContext(t *testing.T) { Name: "task3", Value: *v1beta1.NewArrayOrString("$(tasks.task3.status)"), }}, + WhenExpressions: v1beta1.WhenExpressions{{ + Input: "$(tasks.task1.status)", + Operator: selection.In, + Values: []string{"$(tasks.task3.status)"}, + }}, }, }} expectedState := PipelineRunState{{ @@ -1075,6 +1100,11 @@ func TestApplyTaskRunContext(t *testing.T) { Name: "task3", Value: *v1beta1.NewArrayOrString("none"), }}, + WhenExpressions: v1beta1.WhenExpressions{{ + Input: "succeeded", + Operator: selection.In, + Values: []string{"none"}, + }}, }, }} ApplyPipelineTaskContext(state, r) diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index 5164b3405ea..6577623184a 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -156,12 +156,20 @@ func (t *ResolvedPipelineRunTask) checkParentsDone(facts *PipelineRunFacts) bool } func (t *ResolvedPipelineRunTask) skip(facts *PipelineRunFacts) bool { - if facts.isFinalTask(t.PipelineTask.Name) || t.IsStarted() { + if t.IsStarted() { return false } - if t.conditionsSkip() || t.whenExpressionsSkip(facts) || t.parentTasksSkip(facts) || facts.IsStopping() { - return true + if facts.isFinalTask(t.PipelineTask.Name) { + if t.whenExpressionsSkip(facts) { + return true + } + } + + if facts.isDAGTask(t.PipelineTask.Name) { + if t.conditionsSkip() || t.whenExpressionsSkip(facts) || t.parentTasksSkip(facts) || facts.IsStopping() { + return true + } } return false @@ -193,7 +201,7 @@ func (t *ResolvedPipelineRunTask) conditionsSkip() bool { } func (t *ResolvedPipelineRunTask) whenExpressionsSkip(facts *PipelineRunFacts) bool { - if t.checkParentsDone(facts) { + if facts.isFinalTask(t.PipelineTask.Name) || t.checkParentsDone(facts) { if len(t.PipelineTask.WhenExpressions) > 0 { if !t.PipelineTask.WhenExpressions.HaveVariables() { if !t.PipelineTask.WhenExpressions.AllowsExecution() { diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 5baf3d35788..7ef9793ec92 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -673,11 +673,12 @@ func dagFromState(state PipelineRunState) (*dag.Graph, error) { func TestIsSkipped(t *testing.T) { for _, tc := range []struct { name string - state PipelineRunState + tasks PipelineRunState + finally PipelineRunState expected map[string]bool }{{ name: "tasks-condition-passed", - state: PipelineRunState{{ + tasks: PipelineRunState{{ PipelineTask: &pts[0], TaskRunName: "pipelinerun-conditionaltask", TaskRun: nil, @@ -691,7 +692,7 @@ func TestIsSkipped(t *testing.T) { }, }, { name: "tasks-condition-failed", - state: PipelineRunState{{ + tasks: PipelineRunState{{ PipelineTask: &pts[0], TaskRunName: "pipelinerun-conditionaltask", TaskRun: nil, @@ -705,7 +706,7 @@ func TestIsSkipped(t *testing.T) { }, }, { name: "tasks-multiple-conditions-passed-failed", - state: PipelineRunState{{ + tasks: PipelineRunState{{ PipelineTask: &pts[0], TaskRunName: "pipelinerun-conditionaltask", TaskRun: nil, @@ -727,13 +728,13 @@ func TestIsSkipped(t *testing.T) { }, }, { name: "tasks-condition-running", - state: conditionCheckStartedState, + tasks: conditionCheckStartedState, expected: map[string]bool{ "mytask6": false, }, }, { name: "tasks-parent-condition-passed", - state: PipelineRunState{{ + tasks: PipelineRunState{{ PipelineTask: &pts[5], TaskRunName: "pipelinerun-conditionaltask", TaskRun: nil, @@ -749,7 +750,7 @@ func TestIsSkipped(t *testing.T) { }, }, { name: "tasks-parent-condition-failed", - state: PipelineRunState{{ + tasks: PipelineRunState{{ PipelineTask: &pts[5], TaskRunName: "pipelinerun-conditionaltask", TaskRun: nil, @@ -765,7 +766,7 @@ func TestIsSkipped(t *testing.T) { }, }, { name: "tasks-parent-condition-running", - state: PipelineRunState{{ + tasks: PipelineRunState{{ PipelineTask: &pts[5], TaskRunName: "pipelinerun-conditionaltask", TaskRun: nil, @@ -785,25 +786,25 @@ func TestIsSkipped(t *testing.T) { }, }, { name: "tasks-failed", - state: oneFailedState, + tasks: oneFailedState, expected: map[string]bool{ "mytask1": false, }, }, { name: "tasks-passed", - state: oneFinishedState, + tasks: oneFinishedState, expected: map[string]bool{ "mytask1": false, }, }, { name: "tasks-cancelled", - state: taskCancelled, + tasks: taskCancelled, expected: map[string]bool{ "mytask5": false, }, }, { name: "tasks-parent-failed", - state: PipelineRunState{{ + tasks: PipelineRunState{{ PipelineTask: &pts[5], TaskRunName: "pipelinerun-mytask1", TaskRun: makeFailed(trs[0]), @@ -823,7 +824,7 @@ func TestIsSkipped(t *testing.T) { }, }, { name: "tasks-parent-cancelled", - state: PipelineRunState{{ + tasks: PipelineRunState{{ PipelineTask: &pts[5], TaskRunName: "pipelinerun-mytask1", TaskRun: withCancelled(makeFailed(trs[0])), @@ -843,7 +844,7 @@ func TestIsSkipped(t *testing.T) { }, }, { name: "tasks-grandparent-failed", - state: PipelineRunState{{ + tasks: PipelineRunState{{ PipelineTask: &pts[5], TaskRunName: "pipelinerun-mytask1", TaskRun: makeFailed(trs[0]), @@ -874,7 +875,7 @@ func TestIsSkipped(t *testing.T) { }, }, { name: "tasks-parents-failed-passed", - state: PipelineRunState{{ + tasks: PipelineRunState{{ PipelineTask: &pts[5], TaskRunName: "pipelinerun-mytask1", TaskRun: makeSucceeded(trs[0]), @@ -901,7 +902,7 @@ func TestIsSkipped(t *testing.T) { }, }, { name: "task-failed-pipeline-stopping", - state: PipelineRunState{{ + tasks: PipelineRunState{{ PipelineTask: &pts[0], TaskRunName: "pipelinerun-mytask1", TaskRun: makeFailed(trs[0]), @@ -928,7 +929,7 @@ func TestIsSkipped(t *testing.T) { }, }, { name: "tasks-when-expressions-passed", - state: PipelineRunState{{ + tasks: PipelineRunState{{ PipelineTask: &pts[9], TaskRunName: "pipelinerun-guardedtask", TaskRun: nil, @@ -936,12 +937,29 @@ func TestIsSkipped(t *testing.T) { TaskSpec: &task.Spec, }, }}, + finally: PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "mytask19", + TaskRef: &v1beta1.TaskRef{Name: "taskWithWhenExpressions"}, + WhenExpressions: []v1beta1.WhenExpression{{ + Input: "foo", + Operator: selection.In, + Values: []string{"foo", "bar"}, + }}, + }, + TaskRunName: "pipelinerun-guardedfinallytask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, expected: map[string]bool{ "mytask10": false, + "mytask19": false, }, }, { name: "tasks-when-expression-failed", - state: PipelineRunState{{ + tasks: PipelineRunState{{ PipelineTask: &pts[10], TaskRunName: "pipelinerun-guardedtask", TaskRun: nil, @@ -949,12 +967,29 @@ func TestIsSkipped(t *testing.T) { TaskSpec: &task.Spec, }, }}, + finally: PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "mytask19", + TaskRef: &v1beta1.TaskRef{Name: "taskWithWhenExpressions"}, + WhenExpressions: []v1beta1.WhenExpression{{ + Input: "foo", + Operator: selection.In, + Values: []string{"bar"}, + }}, + }, + TaskRunName: "pipelinerun-guardedfinallytask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, expected: map[string]bool{ "mytask11": true, + "mytask19": true, }, }, { name: "when-expression-task-but-without-parent-done", - state: PipelineRunState{{ + tasks: PipelineRunState{{ PipelineTask: &pts[0], TaskRun: nil, ResolvedTaskResources: &resources.ResolvedTaskResources{ @@ -972,32 +1007,38 @@ func TestIsSkipped(t *testing.T) { }, }, { name: "run-started", - state: oneRunStartedState, + tasks: oneRunStartedState, expected: map[string]bool{ "mytask13": false, }, }, { name: "run-cancelled", - state: runCancelled, + tasks: runCancelled, expected: map[string]bool{ "mytask13": false, }, }} { t.Run(tc.name, func(t *testing.T) { - d, err := dagFromState(tc.state) + d, err := dagFromState(tc.tasks) + if err != nil { + t.Fatalf("Could not get a dag from the TC state %#v: %v", tc.tasks, err) + } + f, err := dagFromState(tc.finally) if err != nil { - t.Fatalf("Could not get a dag from the TC state %#v: %v", tc.state, err) + t.Fatalf("Could not get a dag from the TC finally state %#v: %v", tc.finally, err) } - stateMap := tc.state.ToMap() + + state := append(tc.tasks, tc.finally...) facts := PipelineRunFacts{ - State: tc.state, + State: state, TasksGraph: d, - FinalTasksGraph: &dag.Graph{}, + FinalTasksGraph: f, } + stateMap := state.ToMap() for taskName, isSkipped := range tc.expected { rprt := stateMap[taskName] if rprt == nil { - t.Fatalf("Could not get task %s from the state: %v", taskName, tc.state) + t.Fatalf("Could not get task %s from the tasks %v and finally %v", taskName, tc.tasks, tc.finally) } if d := cmp.Diff(isSkipped, rprt.Skip(&facts)); d != "" { t.Errorf("Didn't get expected isSkipped %s", diff.PrintWantGot(d))