Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TEP-0059: Remove scope-when-expressions-to-task feature flag #4715

Merged
merged 1 commit into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 TestReconcileWithWhenExpressionsWithResultRefs(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