Skip to content

Commit

Permalink
TEP-0059: Skip Guarded Task Only
Browse files Browse the repository at this point in the history 
In #388, we added the problem
statement for [TEP-0059: Skip Guarded Task Only](https://github.com/tektoncd/community/blob/main/teps/0059-skip-guarded-task-only.md)
that addresses skipping strategies to give users the flexibility to skip
a single guarded `Task` only and unblock execution of its dependent
`Tasks`.

In this change, we add the proposal and discuss alternatives for solving
that problem.

Today, we support specifying a list of `WhenExpressions` through
the `when` field as such:

```yaml
when:
  - input: 'foo'
    operator: in
    values: [ 'bar' ]
```

We propose changing the `when` field from a list to a dictionary and
adding `scope` and `expressions` fields under the `when` field.
- The `scope` field would be used to specify whether the
`WhenExpressions` guard the `Task` only or the whole `Branch`.
- The `expressions` field would be used to specify the list of
`WhenExpressions`, each of which has `input`, `operator` and `values`
fields.

```yaml
when:
  scope: Task / Branch
  expressions:
    - input: 'foo'
      operator: in
      values: [ 'bar' ]
```
  • Loading branch information
@jerop
jerop committed Apr 22, 2021
1 parent 7292697 commit ecab855
Show file tree
Hide file tree
Showing 2 changed files with 306 additions and 7 deletions.
311 changes: 305 additions & 6 deletions teps/0059-skip-guarded-task-only.md
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
---
status: proposed
status: implementable
title: Skip Guarded Task Only
creation-date: '2021-03-24'
last-updated: '2021-03-24'
last-updated: '2021-04-22'
authors:
- '@jerop'
---
Expand Down Expand Up @@ -71,6 +71,18 @@ tags, and then generate with `hack/update-toc.sh`.
- [Non-Goals](#non-goals)
- [Use Cases](#use-cases)
- [Requirements](#requirements)
- [Proposal](#proposal)
- [Test Plan](#test-plan)
- [Design Evaluation](#design-evaluation)
- [Reusability](#reusability)
- [Simplicity](#simplicity)
- [Flexibility](#flexibility)
- [Upgrade & Migration Strategy](#upgrade--migration-strategy)
- [Alternatives](#alternatives)
- [Skipping Policies](#skipping-policies)
- [Execution Policies](#execution-policies)
- [Boolean Flag](#boolean-flag)
- [Special runAfter](#special-runafter)
- [References](#references)
<!-- /toc -->

Expand All @@ -95,6 +107,28 @@ updates.

This TEP addresses skipping strategies to give users the flexibility to skip a single guarded `Task` only and unblock execution of its dependent `Tasks`.

Today, we support specifying a list of `WhenExpressions` through the `when` field as such:

```yaml
when:
- input: 'foo'
operator: in
values: [ 'bar' ]
```
We propose changing the `when` field from a list to a dictionary and adding `scope` and `expressions` fields under the `when` field.
- The `scope` field would be used to specify whether the `WhenExpressions` guard the `Task` only or the whole `Branch`.
- The `expressions` field would be used to specify the list of `WhenExpressions`, each of which has `input`, `operator` and `values` fields.

```yaml
when:
scope: Task / Branch
expressions:
- input: 'foo'
operator: in
values: [ 'bar' ]
```

## Motivation

<!--
Expand All @@ -117,7 +151,7 @@ When [`WhenExpressions`](https://github.com/tektoncd/pipeline/blob/main/docs/pip

Take this example:

```
```yaml
tasks:
- name: previous-task
taskRef:
Expand Down Expand Up @@ -148,7 +182,7 @@ The visualization/workflow of the `Pipeline` graph would be:
v
next-task # skipped
```
This TEP aims to support `WhenExpressions` that are specified within `PipelineTasks` to guard the `PipelineTask` only (not its dependent `PipelineTasks`). Thus, visualization/workflow of the `Pipeline` graph would be possible:
This TEP aims to support `WhenExpressions` that are specified within `PipelineTasks` to guard the `PipelineTask` only (not its dependent `PipelineTasks`). Thus, this visualization/workflow of the `Pipeline` graph would be possible:
```
previous-task # executed
|
Expand Down Expand Up @@ -197,7 +231,7 @@ Cluster Admin? etc...) and experience (what workflows or actions are enhanced
if this problem is solved?).
-->

A user needs to design a `Pipeline` with a _manual approval_ `Task` that is executed when merging a pull request only. The execution of the _manual approval_ `Task` is guarded using `WhenExpressions`. To reuse the same `Pipeline` when merging and not merging, the user needs the subsequent `Tasks` to execute regardless of whether the guarded _manual approval_ `Task` is skipped or executed.
A user needs to design a `Pipeline` with a _manual approval_ `Task` that is executed when merging a pull request only. The execution of the _manual approval_ `Task` is guarded using `WhenExpressions`. To reuse the same `Pipeline` when merging and not merging, the user needs the dependent `Tasks` to execute when the guarded _manual approval_ `Task` is skipped.

```
lint unit-tests
Expand All @@ -215,7 +249,11 @@ A user needs to design a `Pipeline` with a _manual approval_ `Task` that is exec
deploy-image
```
Today, if `manual-approval` is skipped then `build-image` and `deploy-image` would be skipped as well while `lint` and `report-linter-output` would execute. In this TEP, we'll provide the flexibility to execute `build-image` and `deploy-image` when `manual-approval` is skipped. This would allow the user to reuse the `Pipeline` in both scenarios.
If the `WhenExpressions` in `manual-approval` evaluate to `True`, then `manual-approval` is executed and:
- if `manual-approval` succeeds, then `build-image` and `deploy-image` are executed
- if `manual-approval` fails, then `build-image` and `deploy-image` are not executed because the `Pipeline` fails
Today, if the `WhenExpressions` in `manual-approval` evaluate to `False`, then `manual-approval`, `build-image` and `deploy-image` are all skipped. In this TEP, we'll provide the flexibility to execute `build-image` and `deploy-image` when `manual-approval` is skipped. This would allow the user to reuse the `Pipeline` in both scenarios (merging and not merging).
Building on the above use case, the user adds `slack-msg` which sends a notification to slack that it was manually approved with the name of the approver that is passed as a `Result` from `manual-approval` to `slack-msg`.
Expand Down Expand Up @@ -249,6 +287,267 @@ Users should be able to specify that a guarded `Task` only should be skipped whe
- *ordering-dependent* `Tasks`, based on `runAfter`, should execute as expected
- *resource-dependent* `Tasks`, based on resources such as `Results`, should be attempted but might be skipped if they can't resolve missing resources
## Proposal
<!--
This is where we get down to the specifics of what the proposal actually is.
This should have enough detail that reviewers can understand exactly what
you're proposing, but should not include things like API designs or
implementation. The "Design Details" section below is for the real
nitty-gritty.
-->
Today, we support specifying a list of `WhenExpressions` through the `when` field as such:
```yaml
when:
- input: 'foo'
operator: in
values: [ 'bar' ]
```

To provide the flexibility to skip a guarded `Task` when its `WhenExpressions` evaluate to `False` while unblocking the execution of its dependent `Tasks`, we propose changing the `when` field from a list to a dictionary and adding `scope` and `expressions` fields under the `when` field.
- The `scope` field would be used to specify whether the `WhenExpressions` guard the `Task` only or the whole `Branch` (the `Task` and its dependencies). To unblock execution of subsequent `Tasks`, users would set `scope` to `Task`.
- The `expressions` field would be used to specify the list of `WhenExpressions`, each of which has `input`, `operator` and `values` fields, as it is currently.


```yaml
when:
scope: Task
expressions:
- input: 'foo'
operator: in
values: [ 'bar' ]
---
when:
scope: Branch
expressions:
- input: 'foo'
operator: notin
values: [ 'bar' ]
```
To enable users to smoothly transition from the old syntax to the new syntax, we will initially support both sytanxes. Then we will deprecate the old syntax and support the new syntax only as we go towards v1 API.
To support both syntaxes under `when`, we'll detect whether it's a list or dictionary in `UnmarshalJSON` function that implements the `json.Unmarshaller` interface, using the first character. This is how similar scenarios have been handled elsewhere, including:
- Tekton does the same thing in `Parameters` to detect whether the type of the value is a `String` or `Array` ([code](https://github.com/tektoncd/pipeline/blob/879ba4f65732808a0614d76d5993af0bac736009/pkg/apis/pipeline/v1beta1/param_types.go#L98-L118))
- Kubernetes does the same thing in `IntOrString` to detect whether the type is `Int` or `String` ([code](https://github.com/kubernetes/apimachinery/blob/8daf28983e6ecf28bd8271925ee135c1179ad13a/pkg/util/intstr/intstr.go#L80-L99))

A `Pipeline` designed to solve the [use case](#use-cases) described above would be specified as such:
```yaml
tasks:
...
- name: manual-approval
runAfter:
- integration-tests
when:
scope: Task
expressions:
- input: $(params.git-action)
operator: in
values:
- merge
taskRef:
- name: manual-approval
- name: slack-msg
params:
- name: approver
value: $(tasks.manual-approval.results.approver)
taskRef:
- name: slack-msg
- name: build-image
runAfter:
- manual-approval
taskRef:
- name: build-image
- name: deploy-image
runAfter:
- build-image
taskRef:
- name: deploy-image
```

If the `WhenExpressions` in `manual-approval` evaluate to `False`, then `manual-approval` would be skipped and:
- `build-image` and `deploy-image` would be executed
- `slack-msg` would be skipped due to missing resource from `manual-approval`


## Test Plan

<!--
**Note:** *Not required until targeted at a release.*

Consider the following in developing a test plan for this enhancement:
- Will there be e2e and integration tests, in addition to unit tests?
- How will it be tested in isolation vs with other components?

No need to outline all of the test cases, just the general strategy. Anything
that would count as tricky in the implementation and anything particularly
challenging to test should be called out.

All code is expected to have adequate tests (eventually with coverage
expectations).
-->

`WhenExpressions` already have tests for validation and functionality. We will add unit tests and integration tests for validation and functionality of the new syntax for full coverage.

## Design Evaluation
<!--
How does this proposal affect the reusability, simplicity, flexibility
and conformance of Tekton, as described in [design principles](https://github.com/tektoncd/community/blob/master/design-principles.md)
-->
### Reusability
By unblocking the execution of dependent `Tasks` when a guarded `Task` is skipped, we enable execution to continue when the guarded `Task` is either successful or skipped, making the `Pipeline` more reusable for more scenarios or use cases

### Simplicity

By scoping the skipping strategy to `WhenExpressions` only, we provide the flexibility safely with a minimal change. We also limit the interleaving of `Pipeline` graph execution paths and maintain the simplicity of the workflows.

### Flexibility

This proposal gives users more control to specify what happens when each guarded `Task` is skipped. Moreover, changing the structure under `when` from a `list` to a `dictionary` makes it extensible because we can add more fields in the future if needed.

## Upgrade & Migration Strategy

<!--
Use this section to detail wether this feature needs an upgrade or
migration strategy. This is especially useful when we modify a
behavior or add a feature that may replace and deprecate a current one.
-->

To enable users to smoothly transition from the old syntax to the new syntax, we will initially support both sytanxes. Then we will deprecate the old syntax and support the new syntax only as we go towards v1 API.


## Alternatives

<!--
What other approaches did you consider and why did you rule them out? These do
not need to be as detailed as the proposal, but should include enough
information to express the idea and why it was not acceptable.
-->

### Skipping Policies

Add a field - `whenSkipped` - that can be set to `runBranch` to unblock or `skipBranch` to block the execution of `Tasks` that are dependent on the guarded `Task`.

```go
type SkippingPolicy string
const (
RunBranch SkippingPolicy = "runBranch"
SkipBranch SkippingPolicy = "skipBranch"
)
```

```yaml
tasks:
- name: task
when:
- input: foo
operator: in
values: [ bar ]
whenSkipped: runBranch / skipBranch
taskRef:
- name: task
```
Another option would be a field - `whenScope` - than can be set to `Task` to unblock or `Branch` to block the execution of `Tasks` that are dependent on the guarded `Task`.
```go
type WhenScope string
const (
Task WhenScope = "task"
Branch WhenScope = "branch"
)
```

```yaml
tasks:
- name: task
when:
- input: foo
operator: in
values: [ bar ]
whenScope: task / branch
taskRef:
- name: task
```

However, it won't be clear that the skipping policies are related to `WhenExpressions` specifically and can be confusing to reason about when they are specified separately.

### Execution Policies

Add a field - `executionPolicies` - that takes a list of execution policies for the skipping and failure strategies for given `Task`.
This would align well with [TEP-0050: Ignore Task Failures](https://github.com/tektoncd/community/blob/main/teps/0050-ignore-task-failures.md) and is easily extensible.

```go
type ExecutionPolicy string
const (
IgnoreFailure ExecutionPolicy = "ignoreFailure"
ContinueAfterSkip ExecutionPolicy = "continueAfterSkip"
...
)
```

```yaml
tasks:
- name: task
when:
- input: foo
operator: in
values: [ bar ]
executionPolicies:
- ignoreFailure
- continueAfterSkip
taskRef:
- name: task
```

However, it won't be clear that the skipping policies are related to `WhenExpressions` specifically and can be confusing to reason about when they are specified separately.

### Boolean Flag

Add a field - `continueAfterSkip` - that can be set to `true` to unblock or `false` to block the execution of `Tasks` that are dependent on the guarded `Task`.

```yaml
tasks:
- name: task
when:
- input: foo
operator: in
values: [ bar ]
continueAfterSkip: true / false
taskRef:
- name: task
```

However, it won't be clear that the boolean flag is related to `WhenExpressions` specifically and can be confusing to reason about when they are specified separately. In addition, booleans [limit future extensions](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md).

### Special runAfter
Provide a special kind of `runAfter` - `runAfterWhenSkipped` - that users can use instead of `runAfter` to allow for the ordering-dependent `Task` to execute even when the `Task` has been skipped. Related ideas discussed in [tektoncd/pipeline#2653](https://github.com/tektoncd/pipeline/issues/2635) as `runAfterUnconditionally` and [tektoncd/pipeline#1684](https://github.com/tektoncd/pipeline/issues/1684) as `runOn`.

```yaml
tasks:
- name: task1
when:
- input: foo
operator: in
values: [ bar ]
taskRef:
- name: task1
- name: task2
runAfterWhenSkipped:
- task1
taskRef:
- name: task2
```

However, it won't be clear that the skipping policies are related to `WhenExpressions` specifically and can be confusing to reason about when they are specified separately.

## References

<!--
Expand Down
2 changes: 1 addition & 1 deletion teps/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,5 +182,5 @@ This is the complete list of Tekton teps:
|[TEP-0053](0053-nested-triggers.md) | Nested Triggers | proposed | 2021-02-24 |
|[TEP-0056](0056-pipelines-in-pipelines.md) | Pipelines in Pipelines | proposed | 2021-03-08 |
|[TEP-0058](0058-graceful-pipeline-run-termination.md) | Graceful Pipeline Run Termination | proposed | 2021-03-18 |
|[TEP-0059](0059-skip-guarded-task-only.md) | Skip Guarded Task Only | proposed | 2021-03-24 |
|[TEP-0059](0059-skip-guarded-task-only.md) | Skip Guarded Task Only | implementable | 2021-04-22 |
|[TEP-0061](0061-allow-custom-task-to-be-embedded-in-pipeline.md) | Allow custom task to be embedded in pipeline | proposed | 2021-03-27 |

0 comments on commit ecab855

Please sign in to comment.