TEP-0059: Skip Guarded Task Only

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' ]
```

teps/0059-skip-guarded-task-only.md

@@ -2,7 +2,7 @@
 status: proposed
 title: Skip Guarded Task Only
 creation-date: '2021-03-24'
-last-updated: '2021-03-24'
+last-updated: '2021-04-16'
 authors:
 - '@jerop'
 ---
@@ -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 -->
 
@@ -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
 
 <!--
@@ -117,7 +151,7 @@ When [`WhenExpressions`](https://github.com/tektoncd/pipeline/blob/main/docs/pip
 
 Take this example:
 
-```
+```yaml
 tasks:
   - name: previous-task
     taskRef:
@@ -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
              |
@@ -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
@@ -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`.
 
@@ -249,6 +287,263 @@ 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.
+
+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.
+
+## 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.
+
+### 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. 
+
+### 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. 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.
+
 ## References
 
 <!--