Skip to content

Commit

Permalink
TEP-0059: Remove scope-when-expressions-to-task feature flag
Browse files Browse the repository at this point in the history
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 #4461.

[tep-0007]: https:/tektoncd/community/blob/main/teps/0007-conditions-beta.md
[tep-0059]: https:/tektoncd/community/blob/main/teps/0059-skipping-strategies.md
[policy]: https:/tektoncd/pipeline/blob/main/api_compatibility_policy.md
  • Loading branch information
jerop committed Mar 29, 2022
1 parent 23a856d commit d66120c
Show file tree
Hide file tree
Showing 13 changed files with 34 additions and 272 deletions.
4 changes: 0 additions & 4 deletions config/config-feature-flags.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
1 change: 0 additions & 1 deletion docs/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,4 @@ being deprecated.
| [The `PipelineRun.Spec.ServiceAccountNames` field is deprecated and will be removed.](https:/tektoncd/pipeline/issues/2614) | [v0.15.0](https:/tektoncd/pipeline/releases/tag/v0.15.0) | Beta | May 15 2021 |
| [`Conditions` CRD is deprecated and will be removed. Use `when` expressions instead.](https:/tektoncd/community/blob/main/teps/0007-conditions-beta.md) | [v0.16.0](https:/tektoncd/pipeline/releases/tag/v0.16.0) | Alpha | Nov 02 2020 |
| [`PipelineRunCancelled` is deprecated and will be removed](https:/tektoncd/pipeline/issues/4611) | [v0.25.0](https:/tektoncd/pipeline/releases/tag/v0.25.0) | Beta | March 15 2022 |
| [The `scope-when-expressions-to-task` flag will be removed](https:/tektoncd/pipeline/issues/4461) | [v0.27.0](https:/tektoncd/pipeline/releases/tag/v0.27.0) | Beta | March 10 2022 |
| [`PipelineResources` are deprecated.](https:/tektoncd/community/blob/main/teps/0074-deprecate-pipelineresources.md) | [v0.30.0](https:/tektoncd/pipeline/releases/tag/v0.30.0) | Alpha | Dec 20 2021 |
4 changes: 0 additions & 4 deletions docs/install.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 2 additions & 21 deletions docs/pipelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:/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:/tektoncd/pipeline/blob/main/docs/pipelines.md#using-the-runafter-parameter))
or resource (e.g. [`Results`](https:/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`
Expand Down Expand Up @@ -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
Expand Down
6 changes: 0 additions & 6 deletions pkg/apis/config/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand All @@ -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"
)
Expand Down Expand Up @@ -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
}
Expand Down
7 changes: 0 additions & 7 deletions pkg/apis/config/feature_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
{
expectedConfig: &config.FeatureFlags{
RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars,
ScopeWhenExpressionsToTask: config.DefaultScopeWhenExpressionsToTask,
EnableAPIFields: "stable",
EmbeddedStatus: config.DefaultEmbeddedStatus,
},
Expand All @@ -49,7 +48,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
RequireGitSSHSecretKnownHosts: true,
EnableTektonOCIBundles: true,
EnableCustomTasks: true,
ScopeWhenExpressionsToTask: true,
EnableAPIFields: "alpha",
SendCloudEventsForRuns: true,
EmbeddedStatus: "both",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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,
}
Expand Down Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion pkg/apis/config/testdata/feature-flags-all-flags-set.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"

This file was deleted.

9 changes: 4 additions & 5 deletions pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
25 changes: 2 additions & 23 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4878,7 +4878,7 @@ func TestReconcileWithWhenExpressionsWithTaskResults(t *testing.T) {
}
}

func TestReconcileWithWhenExpressionsScopedToTask(t *testing.T) {
func TestReconcileWithWhenExpressions(t *testing.T) {
// (b)
// /
// (a) ———— (c) ———— (d)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -5083,7 +5072,7 @@ func TestReconcileWithWhenExpressionsScopedToTask(t *testing.T) {
}
}

func TestReconcileWithWhenExpressionsScopedToTaskWitResultRefs(t *testing.T) {
func TestReconcileWithWhenExpressionsScopedToTaskWithResultRefs(t *testing.T) {
names.TestingSeed()
ps := []*v1beta1.Pipeline{{
ObjectMeta: baseObjectMeta("test-pipeline", "foo"),
Expand Down Expand Up @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading

0 comments on commit d66120c

Please sign in to comment.