From 84da9bd71ea0c8fcad9db5be8f75c41fe4e9d526 Mon Sep 17 00:00:00 2001 From: Jerop Date: Tue, 8 Jun 2021 15:52:00 -0400 Subject: [PATCH] Skipping Strategies This change implements skipping strategies to give users the flexibility to skip a single guarded Task only and unblock execution of its dependent Tasks. Today, WhenExpressions are specified within Tasks but they guard the Task and its dependent Tasks. To provide flexible skipping strategies, we want to change the scope of WhenExpressions from guarding a Task and its dependent Tasks to guarding the Task only. If a user wants to guard a Task and its dependent Tasks, they can: - cascade the WhenExpressions to the dependent Tasks - compose the Task and its dependent Tasks as a sub-Pipeline that's guarded and executed together using Pipelines in Pipelines (but this is still an experimental feature) Changing the scope of WhenExpressions to guard the Task only is backwards-incompatible, so to make the transition smooth: - we'll provide a feature flag, scope-when-expressions-to-task, which: - will default to false to guard a Task and its dependent Tasks - can be set to true to guard a Task only - after migration, we'll change the global default for the feature flag to true to guard a Task only by default - eventually, we'll remove the feature flag and guard a Task only going forward Implements [TEP-0059: Skipping Strategies](https://github.com/tektoncd/community/blob/main/teps/0059-skipping-strategies.md) Closes https://github.com/tektoncd/pipeline/issues/2127 --- config/config-feature-flags.yaml | 3 + docs/deprecations.md | 1 + docs/pipelines.md | 16 ++ pkg/apis/config/feature_flags.go | 6 + pkg/apis/config/feature_flags_test.go | 5 + .../testdata/feature-flags-all-flags-set.yaml | 1 + pkg/reconciler/pipelinerun/pipelinerun.go | 1 + .../pipelinerun/pipelinerun_test.go | 150 +++++++++++++ .../resources/pipelinerunresolution.go | 6 +- .../resources/pipelinerunresolution_test.go | 206 +++++++++++++++++- .../pipelinerun/resources/pipelinerunstate.go | 3 +- 11 files changed, 389 insertions(+), 9 deletions(-) diff --git a/config/config-feature-flags.yaml b/config/config-feature-flags.yaml index 9ab11ef6e4b..be4f98e7a7e 100644 --- a/config/config-feature-flags.yaml +++ b/config/config-feature-flags.yaml @@ -82,3 +82,6 @@ data: # Setting this flag will determine which gated features are enabled. # Acceptable values are "stable" or "alpha". enable-api-fields: "stable" + # Setting this flag to "true" scopes WhenExpressions to guard a Task only + # instead of a Task and its dependent Tasks. + scope-when-expressions-to-task: "false" diff --git a/docs/deprecations.md b/docs/deprecations.md index deff9b68e0d..4ce1644ce4d 100644 --- a/docs/deprecations.md +++ b/docs/deprecations.md @@ -25,3 +25,4 @@ being deprecated. | [`Conditions` CRD is deprecated and will be removed. Use `WhenExpressions` instead.](https://github.com/tektoncd/community/blob/main/teps/0007-conditions-beta.md) | [v0.16.0](https://github.com/tektoncd/pipeline/releases/tag/v0.16.0) | Alpha | Nov 02 2020 | | [The `disable-home-env-overwrite` flag will be removed](https://github.com/tektoncd/pipeline/issues/2013) | [v0.24.0](https://github.com/tektoncd/pipeline/releases/tag/v0.24.0) | Beta | February 10 2022 | | [The `disable-working-dir-overwrite` flag will be removed](https://github.com/tektoncd/pipeline/issues/1836) | [v0.24.0](https://github.com/tektoncd/pipeline/releases/tag/v0.24.0) | Beta | February 10 2022 | +| [The `scope-when-expressions-to-task` flag will be removed](https://github.com/tektoncd/pipeline/issues/1836) | [v0.26.0](https://github.com/tektoncd/pipeline/releases/tag/v0.26.0) | Beta | February 10 2022 | diff --git a/docs/pipelines.md b/docs/pipelines.md index 20c4531badd..6e5c07ac281 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -465,6 +465,22 @@ There are a lot of scenarios where `WhenExpressions` can be really useful. Some - Checking if the name of a CI job matches - Checking if an optional Workspace has been provided +#### Guarding a `Task` and its dependent `Tasks` + +When `WhenExpressions` evaluate to `False`, the `Task` and its branch (of dependent `Tasks`) will be skipped by +default while the rest of the `Pipeline` will execute. The global default scope of `WhenExpressions` is set to `Branch`; +`scope-when-expressions-to-task` field in [`config/config-feature-flags.yaml`](./../config/config-feature-flags.yaml) +defaults to `False`. + +**Note:** Scoping `WhenExpressions` to a `Task` and its dependent `Tasks` is deprecated. To guard a `Task` and its +dependent `Tasks`, cascade `WhenExpressions` to the specific dependent `Tasks` to be guarded as well. + +#### Guarding a `Task` only + +To guard a `Task` only and unblock execution of its dependent `Tasks`, set the global default scope of `WhenExpressions` +to `Task` using the `scope-when-expressions-to-task` field in [`config/config-feature-flags.yaml`](./../config/config-feature-flags.yaml) +by changing it to `True`. + ### Guard `Task` execution using `Conditions` **Note:** `Conditions` are [deprecated](./deprecations.md), use [`WhenExpressions`](#guard-task-execution-using-whenexpressions) instead. diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go index 5073ea5d169..639c37148b5 100644 --- a/pkg/apis/config/feature_flags.go +++ b/pkg/apis/config/feature_flags.go @@ -37,6 +37,7 @@ const ( enableTektonOCIBundles = "enable-tekton-oci-bundles" enableCustomTasks = "enable-custom-tasks" enableAPIFields = "enable-api-fields" + scopeWhenExpressionsToTask = "scope-when-expressions-to-task" DefaultDisableHomeEnvOverwrite = true DefaultDisableWorkingDirOverwrite = true DefaultDisableAffinityAssistant = false @@ -45,6 +46,7 @@ const ( DefaultRequireGitSSHSecretKnownHosts = false DefaultEnableTektonOciBundles = false DefaultEnableCustomTasks = false + DefaultScopeWhenExpressionsToTask = false DefaultEnableAPIFields = StableAPIFields ) @@ -59,6 +61,7 @@ type FeatureFlags struct { RequireGitSSHSecretKnownHosts bool EnableTektonOCIBundles bool EnableCustomTasks bool + ScopeWhenExpressionsToTask bool EnableAPIFields string } @@ -105,6 +108,9 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) { if err := setFeature(requireGitSSHSecretKnownHostsKey, DefaultRequireGitSSHSecretKnownHosts, &tc.RequireGitSSHSecretKnownHosts); err != nil { return nil, err } + if err := setFeature(scopeWhenExpressionsToTask, DefaultScopeWhenExpressionsToTask, &tc.ScopeWhenExpressionsToTask); err != nil { + return nil, err + } if err := setEnabledAPIFields(cfgMap, DefaultEnableAPIFields, &tc.EnableAPIFields); err != nil { return nil, err } diff --git a/pkg/apis/config/feature_flags_test.go b/pkg/apis/config/feature_flags_test.go index 752069c6436..52528d093c1 100644 --- a/pkg/apis/config/feature_flags_test.go +++ b/pkg/apis/config/feature_flags_test.go @@ -38,6 +38,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { DisableHomeEnvOverwrite: false, DisableWorkingDirOverwrite: false, RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars, + ScopeWhenExpressionsToTask: config.DefaultScopeWhenExpressionsToTask, EnableAPIFields: "stable", }, fileName: config.GetFeatureFlagsConfigName(), @@ -51,6 +52,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { RequireGitSSHSecretKnownHosts: true, EnableTektonOCIBundles: true, EnableCustomTasks: true, + ScopeWhenExpressionsToTask: true, EnableAPIFields: "alpha", }, fileName: "feature-flags-all-flags-set", @@ -66,6 +68,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { DisableHomeEnvOverwrite: true, DisableWorkingDirOverwrite: true, RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars, + ScopeWhenExpressionsToTask: config.DefaultScopeWhenExpressionsToTask, }, fileName: "feature-flags-enable-api-fields-overrides-bundles-and-custom-tasks", }, @@ -78,6 +81,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { DisableHomeEnvOverwrite: true, DisableWorkingDirOverwrite: true, RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars, + ScopeWhenExpressionsToTask: config.DefaultScopeWhenExpressionsToTask, }, fileName: "feature-flags-bundles-and-custom-tasks", }, @@ -98,6 +102,7 @@ func TestNewFeatureFlagsFromEmptyConfigMap(t *testing.T) { DisableHomeEnvOverwrite: true, DisableWorkingDirOverwrite: true, RunningInEnvWithInjectedSidecars: true, + ScopeWhenExpressionsToTask: config.DefaultScopeWhenExpressionsToTask, EnableAPIFields: "stable", } verifyConfigFileWithExpectedFeatureFlagsConfig(t, FeatureFlagsConfigEmptyName, expectedConfig) diff --git a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml index e56766124ce..6e5e4958e51 100644 --- a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml +++ b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml @@ -25,4 +25,5 @@ data: require-git-ssh-secret-known-hosts: "true" enable-tekton-oci-bundles: "true" enable-custom-tasks: "true" + scope-when-expressions-to-task: "true" enable-api-fields: "alpha" diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 3e125f332ba..c0b7e993830 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -497,6 +497,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get SpecStatus: pr.Spec.Status, TasksGraph: d, FinalTasksGraph: dfinally, + ScopeWhenExpressionsToTask: config.FromContextOrDefaults(ctx).FeatureFlags.ScopeWhenExpressionsToTask, } for _, rprt := range pipelineRunFacts.State { diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 9c9a1758cf9..e9061867940 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -3487,6 +3487,156 @@ func TestReconcileWithWhenExpressionsWithTaskResults(t *testing.T) { } } +func TestReconcileWithWhenExpressionsScopedToTask(t *testing.T) { + names.TestingSeed() + ps := []*v1beta1.Pipeline{tb.Pipeline("test-pipeline", tb.PipelineNamespace("foo"), tb.PipelineSpec( + tb.PipelineTask("a-task", "a-task"), + tb.PipelineTask("b-task", "b-task", + tb.PipelineTaskWhenExpression("$(tasks.a-task.results.aResult)", selection.In, []string{"aResultValue"}), + tb.PipelineTaskWhenExpression("aResultValue", selection.In, []string{"$(tasks.a-task.results.aResult)"}), + ), + tb.PipelineTask("c-task", "c-task", + tb.PipelineTaskWhenExpression("$(tasks.a-task.results.aResult)", selection.In, []string{"missing"}), + ), + tb.PipelineTask("d-task", "d-task", tb.RunAfter("c-task")), + ))} + prs := []*v1beta1.PipelineRun{tb.PipelineRun("test-pipeline-run-different-service-accs", tb.PipelineRunNamespace("foo"), + tb.PipelineRunSpec("test-pipeline", + tb.PipelineRunServiceAccountName("test-sa-0"), + ), + )} + ts := []*v1beta1.Task{ + tb.Task("a-task", tb.TaskNamespace("foo")), + tb.Task("b-task", tb.TaskNamespace("foo")), + tb.Task("c-task", tb.TaskNamespace("foo")), + tb.Task("d-task", tb.TaskNamespace("foo")), + } + trs := []*v1beta1.TaskRun{ + tb.TaskRun("test-pipeline-run-different-service-accs-a-task-xxyyy", + tb.TaskRunNamespace("foo"), + tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run-different-service-accs", + tb.OwnerReferenceAPIVersion("tekton.dev/v1beta1"), + tb.Controller, tb.BlockOwnerDeletion, + ), + tb.TaskRunLabel("tekton.dev/pipeline", "test-pipeline"), + tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-different-service-accs"), + tb.TaskRunLabel("tekton.dev/pipelineTask", "a-task"), + tb.TaskRunSpec( + tb.TaskRunTaskRef("hello-world"), + tb.TaskRunServiceAccountName("test-sa"), + ), + tb.TaskRunStatus( + tb.StatusCondition( + apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }, + ), + tb.TaskRunResult("aResult", "aResultValue"), + ), + ), + } + + cms := []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{ + "scope-when-expressions-to-task": "true", + }, + }, + } + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + TaskRuns: trs, + ConfigMaps: cms, + } + prt := newPipelineRunTest(d, t) + defer prt.Cancel() + + wantEvents := []string{ + "Normal Started", + "Normal Running Tasks Completed: 1 \\(Failed: 0, Cancelled 0\\), Incomplete: 2, Skipped: 1", + } + pipelineRun, clients := prt.reconcileRun("foo", "test-pipeline-run-different-service-accs", wantEvents, false) + + expectedTaskRunName := "test-pipeline-run-different-service-accs-b-task-9l9zj" + expectedTaskRun := tb.TaskRun(expectedTaskRunName, + tb.TaskRunNamespace("foo"), + tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run-different-service-accs", + tb.OwnerReferenceAPIVersion("tekton.dev/v1beta1"), + tb.Controller, tb.BlockOwnerDeletion, + ), + tb.TaskRunLabel("tekton.dev/pipeline", "test-pipeline"), + tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-different-service-accs"), + tb.TaskRunLabel("tekton.dev/pipelineTask", "b-task"), + tb.TaskRunSpec( + tb.TaskRunTaskRef("b-task"), + tb.TaskRunServiceAccountName("test-sa-0"), + ), + ) + // Check that the expected TaskRun was created + actual, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(prt.TestAssets.Ctx, metav1.ListOptions{ + LabelSelector: "tekton.dev/pipelineTask=b-task,tekton.dev/pipelineRun=test-pipeline-run-different-service-accs", + 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)) + } + + actualWhenExpressionsInTaskRun := pipelineRun.Status.PipelineRunStatusFields.TaskRuns[expectedTaskRunName].WhenExpressions + expectedWhenExpressionsInTaskRun := []v1beta1.WhenExpression{{ + Input: "aResultValue", + Operator: "in", + Values: []string{"aResultValue"}, + }, { + Input: "aResultValue", + Operator: "in", + Values: []string{"aResultValue"}, + }} + if d := cmp.Diff(expectedWhenExpressionsInTaskRun, actualWhenExpressionsInTaskRun); d != "" { + t.Errorf("expected to see When Expressions %v created. Diff %s", expectedTaskRunName, diff.PrintWantGot(d)) + } + + actualSkippedTasks := pipelineRun.Status.SkippedTasks + expectedSkippedTasks := []v1beta1.SkippedTask{{ + Name: "c-task", + WhenExpressions: v1beta1.WhenExpressions{{ + Input: "aResultValue", + Operator: "in", + Values: []string{"missing"}, + }}, + }} + if d := cmp.Diff(actualSkippedTasks, expectedSkippedTasks); d != "" { + t.Errorf("expected to find Skipped Tasks %v. Diff %s", expectedSkippedTasks, diff.PrintWantGot(d)) + } + + skippedTasks := []string{"c-task", "d-task"} + for _, skippedTask := range skippedTasks { + labelSelector := fmt.Sprintf("tekton.dev/pipelineTask=%s,tekton.dev/pipelineRun=test-pipeline-run-different-service-accs", skippedTask) + actualSkippedTask, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(prt.TestAssets.Ctx, metav1.ListOptions{ + LabelSelector: labelSelector, + Limit: 1, + }) + if err != nil { + t.Fatalf("Failure to list TaskRun's %s", err) + } + if len(actualSkippedTask.Items) != 0 { + t.Fatalf("Expected 0 TaskRuns got %d", len(actualSkippedTask.Items)) + } + } +} + // TestReconcileWithAffinityAssistantStatefulSet tests that given a pipelineRun with workspaces, // an Affinity Assistant StatefulSet is created for each PVC workspace and // that the Affinity Assistant names is propagated to TaskRuns. diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index a4759fad948..2aa3f6515d2 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -227,7 +227,11 @@ func (t *ResolvedPipelineRunTask) parentTasksSkip(facts *PipelineRunFacts) bool stateMap := facts.State.ToMap() node := facts.TasksGraph.Nodes[t.PipelineTask.Name] for _, p := range node.Prev { - if stateMap[p.Task.HashKey()].Skip(facts) { + parentTask := stateMap[p.Task.HashKey()] + if parentTask.Skip(facts) { + if facts.ScopeWhenExpressionsToTask && parentTask.PipelineTask.WhenExpressions != nil { + continue + } return true } } diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 7433e45454f..c8453c7734c 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -673,9 +673,10 @@ func dagFromState(state PipelineRunState) (*dag.Graph, error) { func TestIsSkipped(t *testing.T) { for _, tc := range []struct { - name string - state PipelineRunState - expected map[string]bool + name string + state PipelineRunState + scopeWhenExpressionsToTask bool + expected map[string]bool }{{ name: "tasks-condition-passed", state: PipelineRunState{{ @@ -983,6 +984,196 @@ func TestIsSkipped(t *testing.T) { expected: map[string]bool{ "mytask13": false, }, + }, { + name: "tasks-with-when-expression-scoped-to-branch", + state: PipelineRunState{{ + // skipped because when expressions evaluate to false + PipelineTask: &pts[10], + TaskRunName: "pipelinerun-guardedtask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + // skipped because parent was skipped and when expressions are scoped to branch + PipelineTask: &v1beta1.PipelineTask{ + Name: "mytask18", + TaskRef: &v1beta1.TaskRef{Name: "task"}, + RunAfter: []string{"mytask11"}, + }, + TaskRunName: "pipelinerun-ordering-dependent-task-1", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, + scopeWhenExpressionsToTask: false, + expected: map[string]bool{ + "mytask11": true, + "mytask18": true, + }, + }, { + name: "tasks-when-expressions-scoped-to-task", + state: PipelineRunState{{ + // skipped because when expressions evaluate to false + PipelineTask: &pts[10], + TaskRunName: "pipelinerun-guardedtask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + // not skipped regardless of its parent task being skipped because when expressions are scoped to task + PipelineTask: &v1beta1.PipelineTask{ + Name: "mytask18", + TaskRef: &v1beta1.TaskRef{Name: "task"}, + RunAfter: []string{"mytask11"}, + }, + TaskRunName: "pipelinerun-ordering-dependent-task-1", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, + scopeWhenExpressionsToTask: true, + expected: map[string]bool{ + "mytask11": true, + "mytask18": false, + }, + }, { + name: "tasks-when-expressions-scoped-to-branch-skip-multiple-dependent-tasks", + state: PipelineRunState{{ + // skipped because when expressions evaluate to false + PipelineTask: &pts[10], + TaskRunName: "pipelinerun-guardedtask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + // skipped because parent was skipped and when expressions are scoped to branch + PipelineTask: &v1beta1.PipelineTask{ + Name: "mytask18", + TaskRef: &v1beta1.TaskRef{Name: "task"}, + RunAfter: []string{"mytask11"}, + }, + TaskRunName: "pipelinerun-ordering-dependent-task-1", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + // skipped because parent was skipped and when expressions are scoped to branch + PipelineTask: &v1beta1.PipelineTask{ + Name: "mytask19", + TaskRef: &v1beta1.TaskRef{Name: "task"}, + RunAfter: []string{"mytask18"}, + }, + TaskRunName: "pipelinerun-ordering-dependent-task-2", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, + scopeWhenExpressionsToTask: false, + expected: map[string]bool{ + "mytask11": true, + "mytask18": true, + "mytask19": true, + }, + }, { + name: "tasks-when-expressions-scoped-to-task-run-multiple-dependent-tasks", + state: PipelineRunState{{ + // skipped because when expressions evaluate to false + PipelineTask: &pts[10], + TaskRunName: "pipelinerun-guardedtask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + // not skipped regardless of its parent task being skipped because when expressions are scoped to task + PipelineTask: &v1beta1.PipelineTask{ + Name: "mytask18", + TaskRef: &v1beta1.TaskRef{Name: "task"}, + RunAfter: []string{"mytask11"}, + }, + TaskRunName: "pipelinerun-ordering-dependent-task-1", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + // not skipped regardless of its parent task being skipped because when expressions are scoped to task + PipelineTask: &v1beta1.PipelineTask{ + Name: "mytask19", + TaskRef: &v1beta1.TaskRef{Name: "task"}, + RunAfter: []string{"mytask18"}, + }, + TaskRunName: "pipelinerun-ordering-dependent-task-2", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, + scopeWhenExpressionsToTask: true, + expected: map[string]bool{ + "mytask11": true, + "mytask18": false, + "mytask19": false, + }, + }, { + name: "tasks-parent-condition-failed-parent-when-expressions-passed-scoped-to-task", + state: PipelineRunState{{ + // skipped because conditions fail + PipelineTask: &pts[5], + TaskRunName: "pipelinerun-conditionaltask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + ResolvedConditionChecks: failedTaskConditionCheckState, + }, { + // skipped because when expressions evaluate to false + PipelineTask: &pts[10], + TaskRunName: "pipelinerun-guardedtask", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + // skipped because of parent task guarded using conditions is skipped, regardless of another parent task + // being guarded with when expressions that are scoped to task + PipelineTask: &v1beta1.PipelineTask{ + Name: "mytask18", + TaskRef: &v1beta1.TaskRef{Name: "task"}, + RunAfter: []string{"mytask6", "mytask11"}, + }, + TaskRunName: "pipelinerun-ordering-dependent-task-1", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }, { + // not skipped regardless of its parent task being skipped because when expressions are scoped to task + PipelineTask: &v1beta1.PipelineTask{ + Name: "mytask19", + TaskRef: &v1beta1.TaskRef{Name: "task"}, + RunAfter: []string{"mytask11"}, + }, + TaskRunName: "pipelinerun-ordering-dependent-task-2", + TaskRun: nil, + ResolvedTaskResources: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + }}, + scopeWhenExpressionsToTask: true, + expected: map[string]bool{ + "mytask6": true, + "mytask11": true, + "mytask18": true, + "mytask19": false, + }, }} { t.Run(tc.name, func(t *testing.T) { d, err := dagFromState(tc.state) @@ -991,9 +1182,10 @@ func TestIsSkipped(t *testing.T) { } stateMap := tc.state.ToMap() facts := PipelineRunFacts{ - State: tc.state, - TasksGraph: d, - FinalTasksGraph: &dag.Graph{}, + State: tc.state, + TasksGraph: d, + FinalTasksGraph: &dag.Graph{}, + ScopeWhenExpressionsToTask: tc.scopeWhenExpressionsToTask, } for taskName, isSkipped := range tc.expected { rprt := stateMap[taskName] @@ -1001,7 +1193,7 @@ func TestIsSkipped(t *testing.T) { t.Fatalf("Could not get task %s from the state: %v", taskName, tc.state) } if d := cmp.Diff(isSkipped, rprt.Skip(&facts)); d != "" { - t.Errorf("Didn't get expected isSkipped %s", diff.PrintWantGot(d)) + t.Errorf("Didn't get expected isSkipped from task %s: %s", taskName, diff.PrintWantGot(d)) } } }) diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go index 1feea87443e..528f019ef3c 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go @@ -56,7 +56,8 @@ type PipelineRunFacts struct { // needed, via the `Skip` method in pipelinerunresolution.go // The skip data is sensitive to changes in the state. The ResetSkippedCache method // can be used to clean the cache and force re-computation when needed. - SkipCache map[string]bool + SkipCache map[string]bool + ScopeWhenExpressionsToTask bool } // pipelineRunStatusCount holds the count of successful, failed, cancelled, skipped, and incomplete tasks