From a6191a0600fdaba836fc0e994b7bf6a8924c303b Mon Sep 17 00:00:00 2001 From: Priti Desai Date: Tue, 13 Oct 2020 23:16:42 -0700 Subject: [PATCH] access execution status of pipelineTask Introducing a variable which can be used to access the execution status of any pipelineTask in a pipeline. Use $(tasks..status) as param value which contains the status, one of, Succeeded, Failed, or None. --- docs/pipelines.md | 34 ++++++ docs/variables.md | 1 + .../pipelinerun-task-execution-status.yaml | 44 +++++++ .../pipeline/v1beta1/pipeline_validation.go | 35 ++++++ .../v1beta1/pipeline_validation_test.go | 74 ++++++++++++ pkg/reconciler/pipelinerun/pipelinerun.go | 7 +- .../pipelinerun/pipelinerun_test.go | 97 +++++++++++++++ pkg/reconciler/pipelinerun/resources/apply.go | 10 ++ .../pipelinerun/resources/apply_test.go | 38 ++++++ .../pipelinerun/resources/pipelinerunstate.go | 36 ++++++ .../resources/pipelinerunstate_test.go | 114 ++++++++++++++++++ pkg/substitution/substitution.go | 16 +++ pkg/substitution/substitution_test.go | 33 +++++ test/pipelinefinally_test.go | 73 ++++++++++- 14 files changed, 606 insertions(+), 6 deletions(-) create mode 100644 examples/v1beta1/pipelineruns/pipelinerun-task-execution-status.yaml diff --git a/docs/pipelines.md b/docs/pipelines.md index 9db48dd7679..db7530d21ed 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -806,6 +806,40 @@ Overall, `PipelineRun` state transitioning is explained below for respective sce Please refer to the [table](pipelineruns.md#monitoring-execution-status) under Monitoring Execution Status to learn about what kind of events are triggered based on the `Pipelinerun` status. + +### Using Execution `Status` of `pipelineTask` + +Finally Task can utilize execution status of any of the `pipelineTasks` under `tasks` section using param: + +```yaml + finally: + - name: finaltask + params: + - name: task1Status + value: "$(tasks.task1.status)" + taskSpec: + params: + - name: task1Status + steps: + - image: ubuntu + name: print-task-status + script: | + if [ $(params.task1Status) == "Failed" ] + then + echo "Task1 has failed, continue processing the failure" + fi +``` + +This kind of variable can have any one of the values from the following table: + +| Status | Description | +| ------- | -----------| +| Succeeded | `taskRun` for the `pipelineTask` completed successfully | +| Failed | `taskRun` for the `pipelineTask` completed with a failure or cancelled by the user | +| None | the `pipelineTask` has been skipped or no execution information available for the `pipelineTask` | + +For an end-to-end example, see [`status` in a `PipelineRun`](../examples/v1beta1/pipelineruns/pipelinerun-task-execution-status.yaml). + ### Known Limitations ### Specifying `Resources` in Final Tasks diff --git a/docs/variables.md b/docs/variables.md index 61df9d6cc9a..84651369d7e 100644 --- a/docs/variables.md +++ b/docs/variables.md @@ -23,6 +23,7 @@ For instructions on using variable substitutions see the relevant section of [th | `context.pipelineRun.namespace` | The namespace of the `PipelineRun` that this `Pipeline` is running in. | | `context.pipelineRun.uid` | The uid of the `PipelineRun` that this `Pipeline` is running in. | | `context.pipeline.name` | The name of this `Pipeline` . | +| `tasks..status` | The execution status of the specified `pipelineTask`, only available in `finally` tasks. | ## Variables available in a `Task` diff --git a/examples/v1beta1/pipelineruns/pipelinerun-task-execution-status.yaml b/examples/v1beta1/pipelineruns/pipelinerun-task-execution-status.yaml new file mode 100644 index 00000000000..4f57b4abf65 --- /dev/null +++ b/examples/v1beta1/pipelineruns/pipelinerun-task-execution-status.yaml @@ -0,0 +1,44 @@ +kind: PipelineRun +apiVersion: tekton.dev/v1beta1 +metadata: + generateName: pr-execution-status- +spec: + serviceAccountName: 'default' + pipelineSpec: + tasks: + - name: task1 # successful task + taskSpec: + steps: + - image: ubuntu + name: hello + script: | + echo "Hello World!" + - name: task2 # skipped task + when: + - input: "true" + operator: "notin" + values: ["true"] + taskSpec: + steps: + - image: ubuntu + name: success + script: | + exit 0 + finally: + - name: task3 + params: + - name: task1Status + value: "$(tasks.task1.status)" + - name: task2Status + value: "$(tasks.task2.status)" + taskSpec: + params: + - name: task1Status + - name: task2Status + steps: + - image: alpine + name: print-failed-task-status + script: | + if [[ $(params.task1Status) != "Succeeded" || $(params.task2Status) != "None" ]]; then + exit 1; + fi diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index 8a6351fcda3..fc1864f0d63 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -62,6 +62,7 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(validatePipelineParameterVariables(ps.Tasks, ps.Params).ViaField("tasks")) errs = errs.Also(validatePipelineParameterVariables(ps.Finally, ps.Params).ViaField("finally")) errs = errs.Also(validatePipelineContextVariables(ps.Tasks)) + errs = errs.Also(validateExecutionStatusVariables(ps.Tasks, ps.Finally)) // Validate the pipeline's workspaces. errs = errs.Also(validatePipelineWorkspaces(ps.Workspaces, ps.Tasks, ps.Finally)) // Validate the pipeline's results @@ -290,6 +291,40 @@ func validatePipelineContextVariables(tasks []PipelineTask) *apis.FieldError { return errs.Also(validatePipelineContextVariablesInParamValues(paramValues, "context\\.pipeline", pipelineContextNames)) } +func validateExecutionStatusVariables(tasks []PipelineTask, finallyTasks []PipelineTask) (errs *apis.FieldError) { + // creating a list of pipelineTask names to validate tasks..status + pipelineRunTasksContextNames := sets.String{} + for idx, t := range tasks { + for _, param := range t.Params { + // validate dag pipeline tasks not accessing execution status of other pipeline task + if ps, ok := GetVarSubstitutionExpressionsForParam(param); ok { + for _, p := range ps { + if strings.HasPrefix(p, "tasks.") && strings.HasSuffix(p, ".status") { + errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("pipeline tasks can not refer to execution status of any other pipeline task"), + "value").ViaFieldKey("params", param.Name).ViaFieldIndex("tasks", idx)) + } + } + } + } + pipelineRunTasksContextNames.Insert(t.Name) + } + + // validate finally tasks accessing execution status of a dag task specified in the pipeline + var paramValues []string + for _, t := range finallyTasks { + for _, param := range t.Params { + paramValues = append(paramValues, param.Value.StringVal) + paramValues = append(paramValues, param.Value.ArrayVal...) + } + } + for _, paramValue := range paramValues { + if strings.HasPrefix(stripVarSubExpression(paramValue), "tasks.") && strings.HasSuffix(stripVarSubExpression(paramValue), ".status") { + errs = errs.Also(substitution.ValidateVariablePS(paramValue, "tasks", "status", pipelineRunTasksContextNames).ViaField("value")) + } + } + return errs +} + func validatePipelineContextVariablesInParamValues(paramValues []string, prefix string, contextNames sets.String) (errs *apis.FieldError) { for _, paramValue := range paramValues { errs = errs.Also(substitution.ValidateVariableP(paramValue, prefix, contextNames).ViaField("value")) diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index d2fab8280a9..2498256710d 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -2116,6 +2116,80 @@ func TestContextInvalid(t *testing.T) { } } +func TestPipelineTasksExecutionStatus(t *testing.T) { + tests := []struct { + name string + tasks []PipelineTask + finalTasks []PipelineTask + expectedError apis.FieldError + }{{ + name: "valid string variable in finally accessing pipelineTask status", + tasks: []PipelineTask{{ + Name: "foo", + }}, + finalTasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: []Param{{ + Name: "foo-status", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(tasks.foo.status)"}, + }}, + }}, + }, { + name: "invalid string variable in dag task accessing pipelineTask status", + tasks: []PipelineTask{{ + Name: "foo", + TaskRef: &TaskRef{Name: "foo-task"}, + Params: []Param{{ + Name: "bar-status", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(tasks.bar.status)"}, + }}, + }}, + expectedError: apis.FieldError{ + Message: `invalid value: pipeline tasks can not refer to execution status of any other pipeline task`, + Paths: []string{"tasks[0].params[bar-status].value"}, + }, + }, { + name: "invalid array variable in dag task accessing pipelineTask status", + tasks: []PipelineTask{{ + Name: "foo", + TaskRef: &TaskRef{Name: "foo-task"}, + Params: []Param{{ + Name: "bar-status", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.bar.status)"}}, + }}, + }}, + expectedError: apis.FieldError{ + Message: `invalid value: pipeline tasks can not refer to execution status of any other pipeline task`, + Paths: []string{"tasks[0].params[bar-status].value"}, + }, + }, { + name: "invalid string variable in finally accessing missing pipelineTask status", + finalTasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: []Param{{ + Name: "notask-status", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(tasks.notask.status)"}, + }}, + }}, + expectedError: *apis.ErrGeneric(`non-existent variable in "$(tasks.notask.status)"`, "value"), + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateExecutionStatusVariables(tt.tasks, tt.finalTasks) + if len(tt.expectedError.Error()) == 0 { + if err != nil { + t.Errorf("Pipeline.validateExecutionStatusVariables() returned error for valid pipeline variable accessing execution status: %s: %v", tt.name, err) + } + } else { + if err == nil { + t.Errorf("Pipeline.validateExecutionStatusVariables() did not return error for invalid pipeline parameters accessing execution status: %s, %s", tt.name, tt.tasks[0].Params) + } + if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" { + t.Errorf("PipelineSpec.Validate() errors diff %s", diff.PrintWantGot(d)) + } + } + }) + } +} + func getTaskSpec() TaskSpec { return TaskSpec{ Steps: []Step{{ diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 0f3b0584253..ab85aef6f88 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -589,7 +589,12 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1beta1.Pip pipelineRunFacts.ResetSkippedCache() // GetFinalTasks only returns tasks when a DAG is complete - nextRprts = append(nextRprts, pipelineRunFacts.GetFinalTasks()...) + fnextRprts := pipelineRunFacts.GetFinalTasks() + if len(fnextRprts) != 0 { + // apply the runtime context just before creating taskRuns for final tasks in queue + resources.ApplyPipelineTaskContext(fnextRprts, pipelineRunFacts.GetPipelineTaskStatus(ctx)) + nextRprts = append(nextRprts, fnextRprts...) + } for _, rprt := range nextRprts { if rprt == nil || rprt.Skip(pipelineRunFacts) { diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 779213dfc25..73c813e9ddd 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -4549,6 +4549,103 @@ func TestReconcilePipeline_TaskSpecMetadata(t *testing.T) { } } +func TestReconciler_ReconcileKind_PipelineTaskContext(t *testing.T) { + names.TestingSeed() + + pipelineName := "p-pipelinetask-status" + pipelineRunName := "pr-pipelinetask-status" + + ps := []*v1beta1.Pipeline{tb.Pipeline(pipelineName, tb.PipelineNamespace("foo"), tb.PipelineSpec( + tb.PipelineTask("task1", "mytask"), + tb.FinalPipelineTask("finaltask", "finaltask", + tb.PipelineTaskParam("pipelineRun-tasks-task1", "$(tasks.task1.status)"), + ), + ))} + + prs := []*v1beta1.PipelineRun{tb.PipelineRun(pipelineRunName, tb.PipelineRunNamespace("foo"), + tb.PipelineRunSpec(pipelineName, tb.PipelineRunServiceAccountName("test-sa")), + )} + + ts := []*v1beta1.Task{ + tb.Task("mytask", tb.TaskNamespace("foo")), + tb.Task("finaltask", tb.TaskNamespace("foo"), + tb.TaskSpec( + tb.TaskParam("pipelineRun-tasks-task1", v1beta1.ParamTypeString), + ), + ), + } + + trs := []*v1beta1.TaskRun{ + tb.TaskRun(pipelineRunName+"-task1-xxyyy", + tb.TaskRunNamespace("foo"), + tb.TaskRunOwnerReference("PipelineRun", pipelineRunName, + tb.OwnerReferenceAPIVersion("tekton.dev/v1beta1"), + tb.Controller, tb.BlockOwnerDeletion, + ), + tb.TaskRunLabel("tekton.dev/pipeline", pipelineName), + tb.TaskRunLabel("tekton.dev/pipelineRun", pipelineRunName), + tb.TaskRunLabel("tekton.dev/pipelineTask", "task1"), + tb.TaskRunSpec( + tb.TaskRunTaskRef("mytask"), + tb.TaskRunServiceAccountName("test-sa"), + ), + tb.TaskRunStatus( + tb.StatusCondition( + apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionFalse, + Reason: v1beta1.TaskRunReasonFailed.String(), + }, + ), + ), + ), + } + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + TaskRuns: trs, + } + prt := NewPipelineRunTest(d, t) + defer prt.Cancel() + + _, clients := prt.reconcileRun("foo", pipelineRunName, []string{}, false) + + expectedTaskRunName := pipelineRunName + "-finaltask-9l9zj" + expectedTaskRun := tb.TaskRun(expectedTaskRunName, + tb.TaskRunNamespace("foo"), + tb.TaskRunOwnerReference("PipelineRun", pipelineRunName, + tb.OwnerReferenceAPIVersion("tekton.dev/v1beta1"), + tb.Controller, tb.BlockOwnerDeletion, + ), + tb.TaskRunLabel("tekton.dev/pipeline", pipelineName), + tb.TaskRunLabel("tekton.dev/pipelineRun", pipelineRunName), + tb.TaskRunLabel("tekton.dev/pipelineTask", "finaltask"), + tb.TaskRunSpec( + tb.TaskRunTaskRef("finaltask"), + tb.TaskRunServiceAccountName("test-sa"), + tb.TaskRunParam("pipelineRun-tasks-task1", "Failed"), + ), + ) + // Check that the expected TaskRun was created + actual, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(prt.TestAssets.Ctx, metav1.ListOptions{ + LabelSelector: "tekton.dev/pipelineTask=finaltask,tekton.dev/pipelineRun=" + pipelineRunName, + Limit: 1, + }) + + if err != nil { + t.Fatalf("Failure to list TaskRun's %s", err) + } + if len(actual.Items) != 1 { + t.Fatalf("Expected 1 TaskRuns got %d", len(actual.Items)) + } + actualTaskRun := actual.Items[0] + if d := cmp.Diff(&actualTaskRun, expectedTaskRun, ignoreResourceVersion); d != "" { + t.Errorf("expected to see TaskRun %v created. Diff %s", expectedTaskRunName, diff.PrintWantGot(d)) + } +} + // NewPipelineRunTest returns PipelineRunTest with a new PipelineRun controller created with specified state through data // This PipelineRunTest can be reused for multiple PipelineRuns by calling reconcileRun for each pipelineRun func NewPipelineRunTest(data test.Data, t *testing.T) *PipelineRunTest { diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index 3e026f0a986..002afc4a34f 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -84,6 +84,16 @@ func ApplyTaskResults(targets PipelineRunState, resolvedResultRefs ResolvedResul } } +func ApplyPipelineTaskContext(state PipelineRunState, replacements map[string]string) { + for _, resolvedPipelineRunTask := range state { + if resolvedPipelineRunTask.PipelineTask != nil { + pipelineTask := resolvedPipelineRunTask.PipelineTask.DeepCopy() + pipelineTask.Params = replaceParamValues(pipelineTask.Params, replacements, nil) + resolvedPipelineRunTask.PipelineTask = pipelineTask + } + } +} + func ApplyWorkspaces(p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) *v1beta1.PipelineSpec { p = p.DeepCopy() replacements := map[string]string{} diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index c27cae71fce..8a5f4400ba8 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -610,3 +610,41 @@ func TestApplyWorkspaces(t *testing.T) { }) } } + +func TestApplyTaskRunContext(t *testing.T) { + r := map[string]string{ + "tasks.task1.status": "succeeded", + "tasks.task3.status": "none", + } + state := PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "task4", + TaskRef: &v1beta1.TaskRef{Name: "task"}, + Params: []v1beta1.Param{{ + Name: "task1", + Value: *v1beta1.NewArrayOrString("$(tasks.task1.status)"), + }, { + Name: "task3", + Value: *v1beta1.NewArrayOrString("$(tasks.task3.status)"), + }}, + }, + }} + expectedState := PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "task4", + TaskRef: &v1beta1.TaskRef{Name: "task"}, + Params: []v1beta1.Param{{ + Name: "task1", + Value: *v1beta1.NewArrayOrString("succeeded"), + }, { + Name: "task3", + Value: *v1beta1.NewArrayOrString("none"), + }}, + }, + }} + ApplyPipelineTaskContext(state, r) + if d := cmp.Diff(expectedState, state); d != "" { + t.Fatalf("AppluTaskRunContext() %s", diff.PrintWantGot(d)) + } + +} diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go index 10382dc1cbc..4de5f1e1d9d 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go @@ -17,6 +17,7 @@ limitations under the License. package resources import ( + "context" "fmt" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" @@ -28,6 +29,15 @@ import ( "knative.dev/pkg/apis" ) +const ( + // ReasonUnknown indicates that the execution status of a pipelineTask is unknown + PipelineTaskStateNone = "None" + // PipelineTaskStatusPrefix is a prefix of the param representing execution state of pipelineTask + PipelineTaskStatusPrefix = "tasks." + // PipelineTaskStatusSuffix is a suffix of the param representing execution state of pipelineTask + PipelineTaskStatusSuffix = ".status" +) + // PipelineRunState is a slice of ResolvedPipelineRunTasks the represents the current execution // state of the PipelineRun. type PipelineRunState []*ResolvedPipelineRunTask @@ -370,6 +380,32 @@ func (facts *PipelineRunFacts) GetSkippedTasks() []v1beta1.SkippedTask { return skipped } +// GetPipelineTaskStatus returns the status of a PipelineTask depending on its taskRun +func (facts *PipelineRunFacts) GetPipelineTaskStatus(ctx context.Context) map[string]string { + // construct a map of tasks..status and its state + tStatus := make(map[string]string) + for _, t := range facts.State { + if facts.isDAGTask(t.PipelineTask.Name) { + var s string + switch { + case t.IsSuccessful(): + s = v1beta1.TaskRunReasonSuccessful.String() + case t.IsFailure(): + s = v1beta1.TaskRunReasonFailed.String() + case t.IsCancelled(): + s = v1beta1.TaskRunReasonFailed.String() + case t.TaskRun != nil && t.TaskRun.HasTimedOut(ctx): + s = v1beta1.TaskRunReasonFailed.String() + default: + // None includes skipped as well + s = PipelineTaskStateNone + } + tStatus[PipelineTaskStatusPrefix+t.PipelineTask.Name+PipelineTaskStatusSuffix] = s + } + } + return tStatus +} + // successfulOrSkippedTasks returns a list of the names of all of the PipelineTasks in state // which have successfully completed or skipped func (facts *PipelineRunFacts) successfulOrSkippedDAGTasks() []string { diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go index 06fc000cb86..880f523b02d 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go @@ -17,6 +17,7 @@ limitations under the License. package resources import ( + "context" "fmt" "testing" "time" @@ -1318,3 +1319,116 @@ func TestAdjustStartTime(t *testing.T) { }) } } + +func TestPipelineRunFacts_GetPipelineTaskStatus(t *testing.T) { + tcs := []struct { + name string + state PipelineRunState + dagTasks []v1beta1.PipelineTask + expectedStatus map[string]string + }{{ + name: "no-tasks-started", + state: noneStartedState, + dagTasks: []v1beta1.PipelineTask{pts[0], pts[1]}, + expectedStatus: map[string]string{ + PipelineTaskStatusPrefix + pts[0].Name + PipelineTaskStatusSuffix: PipelineTaskStateNone, + PipelineTaskStatusPrefix + pts[1].Name + PipelineTaskStatusSuffix: PipelineTaskStateNone, + }, + }, { + name: "one-task-started", + state: oneStartedState, + dagTasks: []v1beta1.PipelineTask{pts[0], pts[1]}, + expectedStatus: map[string]string{ + PipelineTaskStatusPrefix + pts[0].Name + PipelineTaskStatusSuffix: PipelineTaskStateNone, + PipelineTaskStatusPrefix + pts[1].Name + PipelineTaskStatusSuffix: PipelineTaskStateNone, + }, + }, { + name: "one-task-finished", + state: oneFinishedState, + dagTasks: []v1beta1.PipelineTask{pts[0], pts[1]}, + expectedStatus: map[string]string{ + PipelineTaskStatusPrefix + pts[0].Name + PipelineTaskStatusSuffix: v1beta1.TaskRunReasonSuccessful.String(), + PipelineTaskStatusPrefix + pts[1].Name + PipelineTaskStatusSuffix: PipelineTaskStateNone, + }, + }, { + name: "one-task-failed", + state: oneFailedState, + dagTasks: []v1beta1.PipelineTask{pts[0], pts[1]}, + expectedStatus: map[string]string{ + PipelineTaskStatusPrefix + pts[0].Name + PipelineTaskStatusSuffix: v1beta1.TaskRunReasonFailed.String(), + PipelineTaskStatusPrefix + pts[1].Name + PipelineTaskStatusSuffix: PipelineTaskStateNone, + }, + }, { + name: "all-finished", + state: allFinishedState, + dagTasks: []v1beta1.PipelineTask{pts[0], pts[1]}, + expectedStatus: map[string]string{ + PipelineTaskStatusPrefix + pts[0].Name + PipelineTaskStatusSuffix: v1beta1.TaskRunReasonSuccessful.String(), + PipelineTaskStatusPrefix + pts[1].Name + PipelineTaskStatusSuffix: v1beta1.TaskRunReasonSuccessful.String(), + }, + }, { + name: "task-with-when-expressions-passed", + state: PipelineRunState{{ + PipelineTask: &pts[9], + TaskRunName: "pr-guard-succeeded-task-not-started", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, + dagTasks: []v1beta1.PipelineTask{pts[9]}, + expectedStatus: map[string]string{ + PipelineTaskStatusPrefix + pts[9].Name + PipelineTaskStatusSuffix: PipelineTaskStateNone, + }, + }, { + name: "tasks-when-expression-failed-and-task-skipped", + state: PipelineRunState{{ + PipelineTask: &pts[10], + TaskRunName: "pr-guardedtask-skipped", + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, + dagTasks: []v1beta1.PipelineTask{pts[10]}, + expectedStatus: map[string]string{ + PipelineTaskStatusPrefix + pts[10].Name + PipelineTaskStatusSuffix: PipelineTaskStateNone, + }, + }, { + name: "when-expression-task-with-parent-started", + state: PipelineRunState{{ + PipelineTask: &pts[0], + TaskRun: makeStarted(trs[0]), + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + PipelineTask: &pts[11], + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, + dagTasks: []v1beta1.PipelineTask{pts[0], pts[11]}, + expectedStatus: map[string]string{ + PipelineTaskStatusPrefix + pts[0].Name + PipelineTaskStatusSuffix: PipelineTaskStateNone, + PipelineTaskStatusPrefix + pts[11].Name + PipelineTaskStatusSuffix: PipelineTaskStateNone, + }, + }} + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + d, err := dag.Build(v1beta1.PipelineTaskList(tc.dagTasks), v1beta1.PipelineTaskList(tc.dagTasks).Deps()) + if err != nil { + t.Fatalf("Unexpected error while buildig graph for DAG tasks %v: %v", tc.dagTasks, err) + } + facts := PipelineRunFacts{ + State: tc.state, + TasksGraph: d, + FinalTasksGraph: &dag.Graph{}, + } + s := facts.GetPipelineTaskStatus(context.Background()) + if d := cmp.Diff(tc.expectedStatus, s); d != "" { + t.Fatalf("Test failed: %s Mismatch in pipelineTask execution state %s", tc.name, diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/substitution/substitution.go b/pkg/substitution/substitution.go index feddb442a49..d63e01d9136 100644 --- a/pkg/substitution/substitution.go +++ b/pkg/substitution/substitution.go @@ -60,6 +60,22 @@ func ValidateVariableP(value, prefix string, vars sets.String) *apis.FieldError return nil } +func ValidateVariablePS(value, prefix string, suffix string, vars sets.String) *apis.FieldError { + if vs, present := extractVariablesFromString(value, prefix); present { + for _, v := range vs { + v = strings.TrimSuffix(v, suffix) + if !vars.Has(v) { + return &apis.FieldError{ + Message: fmt.Sprintf("non-existent variable in %q", value), + // Empty path is required to make the `ViaField`, … work + Paths: []string{""}, + } + } + } + } + return nil +} + // Verifies that variables matching the relevant string expressions do not reference any of the names present in vars. func ValidateVariableProhibited(name, value, prefix, locationName, path string, vars sets.String) *apis.FieldError { if vs, present := extractVariablesFromString(value, prefix); present { diff --git a/pkg/substitution/substitution_test.go b/pkg/substitution/substitution_test.go index d3cd4627548..801de32222c 100644 --- a/pkg/substitution/substitution_test.go +++ b/pkg/substitution/substitution_test.go @@ -116,6 +116,39 @@ func TestValidateVariables(t *testing.T) { } } +func TestValidateVariablePS(t *testing.T) { + type args struct { + paramValue string + vars sets.String + } + for _, tc := range []struct { + name string + paramValue string + vars sets.String + expectedError *apis.FieldError + }{{ + name: "valid pipeline task in variable", + paramValue: "--flag=$(tasks.task1.status)", + vars: sets.NewString("task1"), + expectedError: nil, + }, { + name: "undefined pipeline task", + paramValue: "--flag=$(tasks.task1.status)", + vars: sets.NewString("foo"), + expectedError: &apis.FieldError{ + Message: `non-existent variable in "--flag=$(tasks.task1.status)"`, + Paths: []string{""}, + }, + }} { + t.Run(tc.name, func(t *testing.T) { + got := substitution.ValidateVariablePS(tc.paramValue, "tasks", "status", tc.vars) + if d := cmp.Diff(got, tc.expectedError, cmp.AllowUnexported(apis.FieldError{})); d != "" { + t.Errorf("ValidateVariablePS() error did not match expected error for %s: %s", tc.name, diff.PrintWantGot(d)) + } + }) + } +} + func TestApplyReplacements(t *testing.T) { type args struct { input string diff --git a/test/pipelinefinally_test.go b/test/pipelinefinally_test.go index 40f0274b8b6..7cd2549d479 100644 --- a/test/pipelinefinally_test.go +++ b/test/pipelinefinally_test.go @@ -60,6 +60,11 @@ func TestPipelineLevelFinally_OneDAGTaskFailed_Failure(t *testing.T) { t.Fatalf("Failed to create final Task: %s", err) } + finalTaskWithStatus := getTaskVerifyingStatus(t, namespace) + if _, err := c.TaskClient.Create(ctx, finalTaskWithStatus, metav1.CreateOptions{}); err != nil { + t.Fatalf("Failed to create final Task checking executing status: %s", err) + } + pipeline := getPipeline(t, namespace, map[string]string{ @@ -72,6 +77,28 @@ func TestPipelineLevelFinally_OneDAGTaskFailed_Failure(t *testing.T) { }, map[string]string{ "finaltask1": finalTask.Name, + "finaltask2": finalTaskWithStatus.Name, + }, + map[string][]v1beta1.Param{ + "finaltask2": {{ + Name: "dagtask1-status", + Value: v1beta1.ArrayOrString{ + Type: "string", + StringVal: "$(tasks.dagtask1.status)", + }, + }, { + Name: "dagtask2-status", + Value: v1beta1.ArrayOrString{ + Type: "string", + StringVal: "$(tasks.dagtask2.status)", + }, + }, { + Name: "dagtask3-status", + Value: v1beta1.ArrayOrString{ + Type: "string", + StringVal: "$(tasks.dagtask3.status)", + }, + }}, }, ) if _, err := c.PipelineClient.Create(ctx, pipeline, metav1.CreateOptions{}); err != nil { @@ -115,6 +142,27 @@ func TestPipelineLevelFinally_OneDAGTaskFailed_Failure(t *testing.T) { t.Errorf("Error waiting for TaskRun to succeed: %v", err) } finalTaskStartTime = taskrunItem.Status.StartTime + case n == "finaltask2": + if err := WaitForTaskRunState(ctx, c, taskrunItem.Name, TaskRunSucceed(taskrunItem.Name), "TaskRunSuccess"); err != nil { + t.Errorf("Error waiting for TaskRun to succeed: %v", err) + } + for _, p := range taskrunItem.Spec.Params { + switch param := p.Name; param { + case "dagtask1-status": + if p.Value.StringVal != v1beta1.TaskRunReasonFailed.String() { + t.Errorf("Task param \"%s\" is set to \"%s\", expected it to resolve to \"%s\"", param, p.Value.StringVal, v1beta1.TaskRunReasonFailed.String()) + } + case "dagtask2-status": + if p.Value.StringVal != v1beta1.TaskRunReasonSuccessful.String() { + t.Errorf("Task param \"%s\" is set to \"%s\", expected it to resolve to \"%s\"", param, p.Value.StringVal, v1beta1.TaskRunReasonSuccessful.String()) + } + + case "dagtask3-status": + if p.Value.StringVal != resources.PipelineTaskStateNone { + t.Errorf("Task param \"%s\" is set to \"%s\", expected it to resolve to \"%s\"", param, p.Value.StringVal, resources.PipelineTaskStateNone) + } + } + } default: t.Fatalf("TaskRuns were not found for both final and dag tasks") } @@ -152,6 +200,7 @@ func TestPipelineLevelFinally_OneFinalTaskFailed_Failure(t *testing.T) { map[string]string{ "finaltask1": finalTask.Name, }, + map[string][]v1beta1.Param{}, ) if _, err := c.PipelineClient.Create(ctx, pipeline, metav1.CreateOptions{}); err != nil { t.Fatalf("Failed to create Pipeline: %s", err) @@ -215,7 +264,7 @@ func isSkipped(t *testing.T, taskRunName string, conds duckv1beta1.Conditions) b return false } -func getTaskDef(n, namespace, script string) *v1beta1.Task { +func getTaskDef(n, namespace, script string, params []v1beta1.ParamSpec) *v1beta1.Task { return &v1beta1.Task{ ObjectMeta: metav1.ObjectMeta{Name: n, Namespace: namespace}, Spec: v1beta1.TaskSpec{ @@ -223,20 +272,33 @@ func getTaskDef(n, namespace, script string) *v1beta1.Task { Container: corev1.Container{Image: "alpine"}, Script: script, }}, + Params: params, }, } } func getSuccessTask(t *testing.T, namespace string) *v1beta1.Task { - return getTaskDef(helpers.ObjectNameForTest(t), namespace, "exit 0") + return getTaskDef(helpers.ObjectNameForTest(t), namespace, "exit 0", []v1beta1.ParamSpec{}) } func getFailTask(t *testing.T, namespace string) *v1beta1.Task { - return getTaskDef(helpers.ObjectNameForTest(t), namespace, "exit 1") + return getTaskDef(helpers.ObjectNameForTest(t), namespace, "exit 1", []v1beta1.ParamSpec{}) } func getDelaySuccessTask(t *testing.T, namespace string) *v1beta1.Task { - return getTaskDef(helpers.ObjectNameForTest(t), namespace, "sleep 5; exit 0") + return getTaskDef(helpers.ObjectNameForTest(t), namespace, "sleep 5; exit 0", []v1beta1.ParamSpec{}) +} + +func getTaskVerifyingStatus(t *testing.T, namespace string) *v1beta1.Task { + params := []v1beta1.ParamSpec{{ + Name: "dagtask1-status", + }, { + Name: "dagtask2-status", + }, { + Name: "dagtask3-status", + }} + script := "exit 0" + return getTaskDef(helpers.ObjectNameForTest(t), namespace, script, params) } func getCondition(t *testing.T, namespace string) *v1alpha1.Condition { @@ -251,7 +313,7 @@ func getCondition(t *testing.T, namespace string) *v1alpha1.Condition { } } -func getPipeline(t *testing.T, namespace string, ts map[string]string, c map[string]string, f map[string]string) *v1beta1.Pipeline { +func getPipeline(t *testing.T, namespace string, ts map[string]string, c map[string]string, f map[string]string, p map[string][]v1beta1.Param) *v1beta1.Pipeline { var pt []v1beta1.PipelineTask var fpt []v1beta1.PipelineTask for k, v := range ts { @@ -270,6 +332,7 @@ func getPipeline(t *testing.T, namespace string, ts map[string]string, c map[str fpt = append(fpt, v1beta1.PipelineTask{ Name: k, TaskRef: &v1beta1.TaskRef{Name: v}, + Params: p[k], }) } pipeline := &v1beta1.Pipeline{