From 97fe2fd804a53b98d719fa9232db84b5e44f8e5b Mon Sep 17 00:00:00 2001 From: Jerop Date: Tue, 29 Mar 2022 06:43:53 -0400 Subject: [PATCH] TEP-0059: Remove `scope-when-expressions-to-task` feature flag In [TEP-0007: Conditions Beta][tep-0007], we introduced `when` expressions to guard execution of `Tasks` in `Pipelines`. To align with `Conditions`, we set scope of `when` expressions to the guarded `Task` and its dependent `Tasks`. In [TEP-0059: Skipping Strategies][tep-0059], we proposed changing the scope of `when` expressions to the guarded `Task` only. This was implemented in #4085. We provided a feature flag, `scope-when-expressions-to-task`, to support migration. It defaulted to `false` for 9 months per our [Beta API compatibility policy][policy], meaning that we continued to guard the `Task` and its dependent `Tasks`. Then in #4580, we flipped the flag to `true` to guard the `Task` only by default. In this change, we remove the `scope-when-expressions-to-task` flag and complete the migration. Closes https://github.com/tektoncd/pipeline/issues/4461. [tep-0007]: https://github.com/tektoncd/community/blob/main/teps/0007-conditions-beta.md [tep-0059]: https://github.com/tektoncd/community/blob/main/teps/0059-skipping-strategies.md [policy]: https://github.com/tektoncd/pipeline/blob/main/api_compatibility_policy.md --- config/config-feature-flags.yaml | 4 - docs/deprecations.md | 1 - docs/install.md | 4 - docs/pipelines.md | 23 +-- pkg/apis/config/feature_flags.go | 6 - pkg/apis/config/feature_flags_test.go | 7 - .../testdata/feature-flags-all-flags-set.yaml | 1 - ...nvalid-scope-when-expressions-to-task.yaml | 21 -- pkg/reconciler/pipelinerun/pipelinerun.go | 9 +- .../pipelinerun/pipelinerun_test.go | 25 +-- .../resources/pipelinerunresolution.go | 6 +- .../resources/pipelinerunresolution_test.go | 194 ++---------------- .../pipelinerun/resources/pipelinerunstate.go | 5 +- 13 files changed, 34 insertions(+), 272 deletions(-) delete mode 100644 pkg/apis/config/testdata/feature-flags-invalid-scope-when-expressions-to-task.yaml diff --git a/config/config-feature-flags.yaml b/config/config-feature-flags.yaml index da4bff2d9a7..578a66eb0fa 100644 --- a/config/config-feature-flags.yaml +++ b/config/config-feature-flags.yaml @@ -70,10 +70,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 "false" scopes when expressions to guard a Task and - # its dependent Tasks. This flag defaults to "true"; when expressions guard - # the Task only. See TEP-0059 and Pipeline documentation for more details. - scope-when-expressions-to-task: "true" # Setting this flag to "true" enables CloudEvents for Runs, as long as a # CloudEvents sink is configured in the config-defaults config map send-cloudevents-for-runs: "false" diff --git a/docs/deprecations.md b/docs/deprecations.md index 4e46f0b1783..fc04f59d097 100644 --- a/docs/deprecations.md +++ b/docs/deprecations.md @@ -23,5 +23,4 @@ being deprecated. | [The `PipelineRun.Spec.ServiceAccountNames` field is deprecated and will be removed.](https://github.com/tektoncd/pipeline/issues/2614) | [v0.15.0](https://github.com/tektoncd/pipeline/releases/tag/v0.15.0) | Beta | May 15 2021 | | [`Conditions` CRD is deprecated and will be removed. Use `when` expressions 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 | | [`PipelineRunCancelled` is deprecated and will be removed](https://github.com/tektoncd/pipeline/issues/4611) | [v0.25.0](https://github.com/tektoncd/pipeline/releases/tag/v0.25.0) | Beta | March 15 2022 | -| [The `scope-when-expressions-to-task` flag will be removed](https://github.com/tektoncd/pipeline/issues/4461) | [v0.27.0](https://github.com/tektoncd/pipeline/releases/tag/v0.27.0) | Beta | March 10 2022 | | [`PipelineResources` are deprecated.](https://github.com/tektoncd/community/blob/main/teps/0074-deprecate-pipelineresources.md) | [v0.30.0](https://github.com/tektoncd/pipeline/releases/tag/v0.30.0) | Alpha | Dec 20 2021 | diff --git a/docs/install.md b/docs/install.md index e3276036d59..4d0da9a1084 100644 --- a/docs/install.md +++ b/docs/install.md @@ -386,10 +386,6 @@ use of custom tasks in pipelines. most stable features to be used. Set it to "alpha" to allow [alpha features](#alpha-features) to be used. -- `scope-when-expressions-to-task`: set this flag to "true" to scope `when` expressions to guard a `Task` only. Set it - to "false" to guard a `Task` and its dependent `Tasks`. It defaults to "true". For more information, see [guarding - `Task` execution using `when` expressions](pipelines.md#guard-task-execution-using-whenexpressions). - - `embedded-status`: set this flag to "full" to enable full embedding of `TaskRun` and `Run` statuses in the `PipelineRun` status. Set it to "minimal" to populate the `ChildReferences` field in the `PipelineRun` status with name, kind, and API version information for each `TaskRun` and `Run` in the `PipelineRun` instead. Set it to "both" to diff --git a/docs/pipelines.md b/docs/pipelines.md index 03f5d66ad5e..f59a52574e6 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -578,25 +578,6 @@ There are a lot of scenarios where `when` expressions can be really useful. Some #### Guarding a `Task` and its dependent `Tasks` -> :warning: **Scoping `when` expressions to a `Task` and its dependent `Tasks` is deprecated.** -> -> Consider migrating to scoping `when` expressions to the guarded `Task` only instead. -> Read more in the [documentation](#guarding-a-task-only) and [TEP-0059: Skipping Strategies][tep-0059]. -> -[tep-0059]: https://github.com/tektoncd/community/blob/main/teps/0059-skipping-strategies.md - -To guard a `Task` and its dependent `Tasks`, set the global default scope of `when` expressions to `Task` using the -`scope-when-expressions-to-task` field in [`config/config-feature-flags.yaml`](install.md#customizing-the-pipelines-controller-behavior) -by changing it to "false". - -When `when` expressions evaluate to `False`, and `scope-when-expressions-to-task` is set to "false", the `Task` and -its dependent `Tasks` will be skipped while the rest of the `Pipeline` will execute. Dependencies between `Tasks` can -be either ordering ([`runAfter`](https://github.com/tektoncd/pipeline/blob/main/docs/pipelines.md#using-the-runafter-parameter)) -or resource (e.g. [`Results`](https://github.com/tektoncd/pipeline/blob/main/docs/pipelines.md#using-results)) -dependencies, as further described in [configuring execution order](#configuring-the-task-execution-order). The global -default scope of `when` expressions is set to a `Task` only; `scope-when-expressions-to-task` field in -[`config/config-feature-flags.yaml`](install.md#customizing-the-pipelines-controller-behavior) defaults to "true". - To guard a `Task` and its dependent Tasks: - cascade the `when` expressions to the specific dependent `Tasks` to be guarded as well - compose the `Task` and its dependent `Tasks` as a unit to be guarded and executed together using `Pipelines` in `Pipelines` @@ -798,8 +779,8 @@ tasks: name: slack-msg ``` -With `when` expressions scoped to `Task`, if `manual-approval` is skipped, execution of its dependent `Tasks` -(`slack-msg`, `build-image` and `deploy-image`) would be unblocked regardless: +If `manual-approval` is skipped, execution of its dependent `Tasks` (`slack-msg`, `build-image` and `deploy-image`) +would be unblocked regardless: - `build-image` and `deploy-image` should be executed successfully - `slack-msg` will be skipped because it is missing the `approver` `Result` from `manual-approval` - dependents of `slack-msg` would have been skipped too if it had any of them diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go index 463eacfaebf..17ef479ae61 100644 --- a/pkg/apis/config/feature_flags.go +++ b/pkg/apis/config/feature_flags.go @@ -51,8 +51,6 @@ const ( DefaultEnableTektonOciBundles = false // DefaultEnableCustomTasks is the default value for "enable-custom-tasks". DefaultEnableCustomTasks = false - // DefaultScopeWhenExpressionsToTask is the default value for "scope-when-expressions-to-task". - DefaultScopeWhenExpressionsToTask = true // DefaultEnableAPIFields is the default value for "enable-api-fields". DefaultEnableAPIFields = StableAPIFields // DefaultSendCloudEventsForRuns is the default value for "send-cloudevents-for-runs". @@ -67,7 +65,6 @@ const ( enableTektonOCIBundles = "enable-tekton-oci-bundles" enableCustomTasks = "enable-custom-tasks" enableAPIFields = "enable-api-fields" - scopeWhenExpressionsToTask = "scope-when-expressions-to-task" sendCloudEventsForRuns = "send-cloudevents-for-runs" embeddedStatus = "embedded-status" ) @@ -124,9 +121,6 @@ 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 dccf4138596..97b2cd4e865 100644 --- a/pkg/apis/config/feature_flags_test.go +++ b/pkg/apis/config/feature_flags_test.go @@ -36,7 +36,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { { expectedConfig: &config.FeatureFlags{ RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars, - ScopeWhenExpressionsToTask: config.DefaultScopeWhenExpressionsToTask, EnableAPIFields: "stable", EmbeddedStatus: config.DefaultEmbeddedStatus, }, @@ -49,7 +48,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { RequireGitSSHSecretKnownHosts: true, EnableTektonOCIBundles: true, EnableCustomTasks: true, - ScopeWhenExpressionsToTask: true, EnableAPIFields: "alpha", SendCloudEventsForRuns: true, EmbeddedStatus: "both", @@ -65,7 +63,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableCustomTasks: true, RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars, - ScopeWhenExpressionsToTask: config.DefaultScopeWhenExpressionsToTask, EmbeddedStatus: config.DefaultEmbeddedStatus, }, fileName: "feature-flags-enable-api-fields-overrides-bundles-and-custom-tasks", @@ -77,7 +74,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableCustomTasks: true, RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars, - ScopeWhenExpressionsToTask: config.DefaultScopeWhenExpressionsToTask, EmbeddedStatus: config.DefaultEmbeddedStatus, }, fileName: "feature-flags-bundles-and-custom-tasks", @@ -97,7 +93,6 @@ func TestNewFeatureFlagsFromEmptyConfigMap(t *testing.T) { FeatureFlagsConfigEmptyName := "feature-flags-empty" expectedConfig := &config.FeatureFlags{ RunningInEnvWithInjectedSidecars: true, - ScopeWhenExpressionsToTask: config.DefaultScopeWhenExpressionsToTask, EnableAPIFields: "stable", EmbeddedStatus: config.DefaultEmbeddedStatus, } @@ -144,8 +139,6 @@ func TestNewFeatureFlagsConfigMapErrors(t *testing.T) { fileName: "feature-flags-invalid-enable-api-fields", }, { fileName: "feature-flags-invalid-embedded-status", - }, { - fileName: "feature-flags-invalid-scope-when-expressions-to-task", }} { t.Run(tc.fileName, func(t *testing.T) { cm := test.ConfigMapFromTestFile(t, tc.fileName) 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 f80f3be96c2..d08fac96460 100644 --- a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml +++ b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml @@ -23,7 +23,6 @@ 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" send-cloudevents-for-runs: "true" embedded-status: "both" diff --git a/pkg/apis/config/testdata/feature-flags-invalid-scope-when-expressions-to-task.yaml b/pkg/apis/config/testdata/feature-flags-invalid-scope-when-expressions-to-task.yaml deleted file mode 100644 index 54569ae7cba..00000000000 --- a/pkg/apis/config/testdata/feature-flags-invalid-scope-when-expressions-to-task.yaml +++ /dev/null @@ -1,21 +0,0 @@ -# Copyright 2021 The Tekton Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# https://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -apiVersion: v1 -kind: ConfigMap -metadata: - name: feature-flags - namespace: tekton-pipelines -data: - scope-when-expressions-to-task: "im-not-a-boolean" diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index c1fc966020e..65ad778f7f8 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -462,11 +462,10 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get // Build PipelineRunFacts with a list of resolved pipeline tasks, // dag tasks graph and final tasks graph pipelineRunFacts := &resources.PipelineRunFacts{ - State: pipelineRunState, - SpecStatus: pr.Spec.Status, - TasksGraph: d, - FinalTasksGraph: dfinally, - ScopeWhenExpressionsToTask: config.FromContextOrDefaults(ctx).FeatureFlags.ScopeWhenExpressionsToTask, + State: pipelineRunState, + SpecStatus: pr.Spec.Status, + TasksGraph: d, + FinalTasksGraph: dfinally, } for _, rprt := range pipelineRunFacts.State { diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 5ce478550c8..9a92f06a677 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -4878,7 +4878,7 @@ func TestReconcileWithWhenExpressionsWithTaskResults(t *testing.T) { } } -func TestReconcileWithWhenExpressionsScopedToTask(t *testing.T) { +func TestReconcileWithWhenExpressions(t *testing.T) { // (b) // / // (a) ———— (c) ———— (d) @@ -4977,21 +4977,10 @@ func TestReconcileWithWhenExpressionsScopedToTask(t *testing.T) { {ObjectMeta: baseObjectMeta("f-task", "foo")}, } - // set the scope of when expressions to task -- execution of dependent tasks is unblocked - 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, - ConfigMaps: cms, } prt := newPipelineRunTest(d, t) defer prt.Cancel() @@ -5083,7 +5072,7 @@ func TestReconcileWithWhenExpressionsScopedToTask(t *testing.T) { } } -func TestReconcileWithWhenExpressionsScopedToTaskWitResultRefs(t *testing.T) { +func TestReconcileWithWhenExpressionsWithResultRefs(t *testing.T) { names.TestingSeed() ps := []*v1beta1.Pipeline{{ ObjectMeta: baseObjectMeta("test-pipeline", "foo"), @@ -5160,22 +5149,12 @@ func TestReconcileWithWhenExpressionsScopedToTaskWitResultRefs(t *testing.T) { }, }, }} - // set the scope of when expressions to task -- execution of dependent tasks is unblocked - 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() diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index 414341e20ad..0b08e103db5 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -300,7 +300,7 @@ func (t *ResolvedPipelineRunTask) skipBecauseWhenExpressionsEvaluatedToFalse(fac } // skipBecauseParentTaskWasSkipped loops through the parent tasks and checks if the parent task skipped: -// if yes, is it because of when expressions and are when expressions? +// if yes, is it because of when expressions? // if yes, it ignores this parent skip and continue evaluating other parent tasks // if no, it returns true to skip the current task because this parent task was skipped // if no, it continues checking the other parent tasks @@ -310,9 +310,9 @@ func (t *ResolvedPipelineRunTask) skipBecauseParentTaskWasSkipped(facts *Pipelin for _, p := range node.Prev { parentTask := stateMap[p.Task.HashKey()] if parentSkipStatus := parentTask.Skip(facts); parentSkipStatus.IsSkipped { - // if the `when` expressions are scoped to task and the parent task was skipped due to its `when` expressions, + // 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 && facts.ScopeWhenExpressionsToTask { + if parentSkipStatus.SkippingReason == WhenExpressionsSkip { continue } return true diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 20146f9d750..bf0de9b04b7 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -693,10 +693,9 @@ func dagFromState(state PipelineRunState) (*dag.Graph, error) { func TestIsSkipped(t *testing.T) { for _, tc := range []struct { - name string - state PipelineRunState - scopeWhenExpressionsToTask bool - expected map[string]bool + name string + state PipelineRunState + expected map[string]bool }{{ name: "tasks-condition-passed", state: PipelineRunState{{ @@ -1005,35 +1004,7 @@ func TestIsSkipped(t *testing.T) { "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", + name: "tasks-when-expressions", state: PipelineRunState{{ // skipped because when expressions evaluate to false PipelineTask: &pts[10], @@ -1055,54 +1026,12 @@ func TestIsSkipped(t *testing.T) { 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", + name: "tasks-when-expressions-run-multiple-dependent-tasks", state: PipelineRunState{{ // skipped because when expressions evaluate to false PipelineTask: &pts[10], @@ -1136,14 +1065,13 @@ func TestIsSkipped(t *testing.T) { TaskSpec: &task.Spec, }, }}, - scopeWhenExpressionsToTask: true, expected: map[string]bool{ "mytask11": true, "mytask18": false, "mytask19": false, }, }, { - name: "tasks-when-expressions-scoped-to-task-run-multiple-ordering-and-resource-dependent-tasks", + name: "tasks-when-expressions-run-multiple-ordering-and-resource-dependent-tasks", state: PipelineRunState{{ // skipped because when expressions evaluate to false PipelineTask: &pts[10], @@ -1234,7 +1162,6 @@ func TestIsSkipped(t *testing.T) { TaskSpec: &task.Spec, }, }}, - scopeWhenExpressionsToTask: true, expected: map[string]bool{ "mytask11": true, "mytask18": false, @@ -1245,7 +1172,7 @@ func TestIsSkipped(t *testing.T) { "mytask23": true, }, }, { - name: "tasks-parent-condition-failed-parent-when-expressions-passed-scoped-to-task", + name: "tasks-parent-condition-failed-parent-when-expressions-passed", state: PipelineRunState{{ // skipped because conditions fail PipelineTask: &pts[5], @@ -1265,7 +1192,7 @@ func TestIsSkipped(t *testing.T) { }, }, { // 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 + // being guarded with when expressions PipelineTask: &v1beta1.PipelineTask{ Name: "mytask18", TaskRef: &v1beta1.TaskRef{Name: "task"}, @@ -1277,7 +1204,7 @@ func TestIsSkipped(t *testing.T) { TaskSpec: &task.Spec, }, }, { - // not skipped regardless of its parent task being skipped because when expressions are scoped to task + // not skipped regardless of its parent task being skipped because when expressions PipelineTask: &v1beta1.PipelineTask{ Name: "mytask19", TaskRef: &v1beta1.TaskRef{Name: "task"}, @@ -1289,7 +1216,6 @@ func TestIsSkipped(t *testing.T) { TaskSpec: &task.Spec, }, }}, - scopeWhenExpressionsToTask: true, expected: map[string]bool{ "mytask6": true, "mytask11": true, @@ -1304,10 +1230,9 @@ func TestIsSkipped(t *testing.T) { } stateMap := tc.state.ToMap() facts := PipelineRunFacts{ - State: tc.state, - TasksGraph: d, - FinalTasksGraph: &dag.Graph{}, - ScopeWhenExpressionsToTask: tc.scopeWhenExpressionsToTask, + State: tc.state, + TasksGraph: d, + FinalTasksGraph: &dag.Graph{}, } for taskName, isSkipped := range tc.expected { rprt := stateMap[taskName] @@ -1492,10 +1417,9 @@ func TestIsFailure(t *testing.T) { func TestSkipBecauseParentTaskWasSkipped(t *testing.T) { for _, tc := range []struct { - name string - state PipelineRunState - scopeWhenExpressionsToTask bool - expected map[string]bool + name string + state PipelineRunState + expected map[string]bool }{{ name: "tasks-parent-condition-passed", state: PipelineRunState{{ @@ -1575,36 +1499,7 @@ func TestSkipBecauseParentTaskWasSkipped(t *testing.T) { "mytask12": false, }, }, { - name: "tasks-with-when-expression-scoped-to-branch", - state: PipelineRunState{{ - // parent task is skipped because when expressions evaluate to false - PipelineTask: &pts[10], - TaskRunName: "pipelinerun-guardedtask", - TaskRun: nil, - ResolvedTaskResources: &resources.ResolvedTaskResources{ - TaskSpec: &task.Spec, - }, - }, { - // child task is skipped because parent was skipped due to its when expressions evaluating to false when - // they are scoped to task and its dependent tasks - 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": false, - "mytask18": true, - }, - }, { - name: "tasks-when-expressions-scoped-to-task", + name: "tasks-when-expressions", state: PipelineRunState{{ // parent task is skipped because when expressions evaluate to false, not because of its parent tasks PipelineTask: &pts[10], @@ -1627,56 +1522,12 @@ func TestSkipBecauseParentTaskWasSkipped(t *testing.T) { TaskSpec: &task.Spec, }, }}, - scopeWhenExpressionsToTask: true, expected: map[string]bool{ "mytask11": false, "mytask18": false, }, }, { - name: "tasks-when-expressions-scoped-to-branch-skip-multiple-dependent-tasks", - state: PipelineRunState{{ - // parent task is skipped because when expressions evaluate to false, not because of its parent tasks - PipelineTask: &pts[10], - TaskRunName: "pipelinerun-guardedtask", - TaskRun: nil, - ResolvedTaskResources: &resources.ResolvedTaskResources{ - TaskSpec: &task.Spec, - }, - }, { - // child task is skipped because parent was skipped due to its when expressions evaluating to false when - // they are scoped to task and its dependent tasks - 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, - }, - }, { - // child task is skipped because parent was skipped due to its when expressions evaluating to false when - // they are scoped to task and its dependent tasks - 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": false, - "mytask18": true, - "mytask19": true, - }, - }, { - name: "tasks-when-expressions-scoped-to-task-run-multiple-dependent-tasks", + name: "tasks-when-expressions-run-multiple-dependent-tasks", state: PipelineRunState{{ // parent task is skipped because when expressions evaluate to false, not because of its parent tasks PipelineTask: &pts[10], @@ -1712,14 +1563,13 @@ func TestSkipBecauseParentTaskWasSkipped(t *testing.T) { TaskSpec: &task.Spec, }, }}, - scopeWhenExpressionsToTask: true, expected: map[string]bool{ "mytask11": false, "mytask18": false, "mytask19": false, }, }, { - name: "tasks-parent-condition-failed-parent-when-expressions-passed-scoped-to-task", + name: "tasks-parent-condition-failed-parent-when-expressions-passed", state: PipelineRunState{{ // parent task is skipped because conditions fail, not because of its parent tasks PipelineTask: &pts[5], @@ -1764,7 +1614,6 @@ func TestSkipBecauseParentTaskWasSkipped(t *testing.T) { TaskSpec: &task.Spec, }, }}, - scopeWhenExpressionsToTask: true, expected: map[string]bool{ "mytask6": false, "mytask11": false, @@ -1779,10 +1628,9 @@ func TestSkipBecauseParentTaskWasSkipped(t *testing.T) { } stateMap := tc.state.ToMap() facts := PipelineRunFacts{ - State: tc.state, - TasksGraph: d, - FinalTasksGraph: &dag.Graph{}, - ScopeWhenExpressionsToTask: tc.scopeWhenExpressionsToTask, + State: tc.state, + TasksGraph: d, + FinalTasksGraph: &dag.Graph{}, } for taskName, isSkipped := range tc.expected { rprt := stateMap[taskName] diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go index 914fcd9a535..8fd2592e286 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go @@ -47,7 +47,7 @@ type PipelineRunState []*ResolvedPipelineRunTask // PipelineRunFacts holds the state of all the components that make up the Pipeline graph that are used to track the // PipelineRun state without passing all these components separately. It helps simplify our implementation for getting // and scheduling the next tasks. It is a collection of list of ResolvedPipelineTask, graph of DAG tasks, graph of -// finally tasks, cache of skipped tasks, and the scope of when expressions. +// finally tasks, cache of skipped tasks. type PipelineRunFacts struct { State PipelineRunState SpecStatus v1beta1.PipelineRunSpecStatus @@ -62,8 +62,7 @@ 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]TaskSkipStatus - ScopeWhenExpressionsToTask bool + SkipCache map[string]TaskSkipStatus } // pipelineRunStatusCount holds the count of successful, failed, cancelled, skipped, and incomplete tasks