Pipeline: Conditionals: Configure what happens when a task is skipped due to condition failure

[dibyom]

Expected Behavior

Users can decide to either run or skip tasks that are dependent on the task that was skipped due to a condition check failure.

Actual Behavior

Tasks that are dependent on a skipped task are always skipped

Steps to Reproduce the Problem

1. Create a pipeline like similar to this:

apiVersion: tekton.dev/v1alpha1
kind: Pipeline
metadata:
  name: two-tasks-one-condition
spec:
  tasks:
  - name: conditional-task
    taskRef:
      name: task1
    conditions:
      - conditionRef: always-fail
  - name: run-if-1-or-1-skipped
    taskRef:
      name: task2
    runAfter:
      - conditional-task

1. Run the pipeline through a PipelineRun
2. task2 is not executed

Additional Info

1. Might be worth enumerating the use cases for this feature before implementing this.
2. Based on discussion from #27: https://docs.google.com/a/google.com/document/d/1iTb0rDDlgQcPbiYIGJ-atkaML8aCyPCyhfucCPJmNLs/edit?disco=AAAADD1TnxU

[dibyom]

ref: https://github.com/tektoncd/pipeline/issues/1137

[bobcatfish]

One thought after looking at #1137 :

• If a Task is getting a resource from a Task that has a failing condition, I'm not sure it makes any sense to run that dependent Task: e.g. if a Task is building an image as an output, and another task wants to get the built image (digest) from that Task, it wouldn't make sense to run it anyway
• runAfter is fuzzier - if it's just a matter of ordering, it feels like the user could potentially choose whether the tasks "after" should be skipped or not

[dibyom]

From @afrittoli in #1264:

In the example pipeline I used runAfter but I believe the behaviour would be the same if the dependency was introduced using from on a resource.

[afrittoli]

If a Task is getting a resource from a Task that has a failing condition, I'm not sure it makes any sense to run that dependent Task: e.g. if a Task is building an image as an output, and another task wants to get the built image (digest) from that Task, it wouldn't make sense to run it anyway

I'm not sure we have a good solution for this case specifically. I've been trying to build a pipeline that builds a docker image only if needed - which I encoded in a condition. The build task has the condition attached and the next Task deploys a cloud function that uses that image plus some code on top.
As long as I don't rely on the image digest, and I use a tag e.g. "latest", the second task can run
even if the first one did not.

[vdemeester]

/kind feature

[dibyom]

The proposal in #1684 has a solution to this using the runOn syntax. So, if a task's condition failed its state is "skip":

# 1. change execution graph i.e. onSuccess do something onFailure do something else: 
- name: task1
  conditions:
    conditionRef: "condition-that-sometime-fails"
  taskRef: { name: "my-task" }

- name: runIfTask1Fails 
  runAfter: task1
  runOn: ["failure"]

- name: runIfTask1Succeeds
  runAfter: task1
  runOn: ["success"]

- name: runIfTask1IsSkipped
  runAfter: task1
  runOn: ["skip"]

[afrittoli]

https://github.com/tektoncd/pipeline/pull/2094#discussion_r392661699

[afrittoli]

What happens though in this use case? Is runIfTask2IsSkipped executed when task1 is skipped because its condition fails?

- name: task1
  conditions:
    conditionRef: "condition-that-sometime-fails"
  taskRef: { name: "my-task" }

- name: runIfTask1IsSkipped
  runAfter: task1
  runOn: ["skip"]

- name: task2
  runAfter: task1
  conditions:
    conditionRef: "condition-that-sometime-fails"
  taskRef: { name: "my-task2" }

- name: runIfTask2IsSkipped
  runAfter: task2
  runOn: ["skip"]

What I want to say is that a runOn: ["skip"] would cover more cases than an else logic, so we still would need a way to specify that we only want the else in some cases.

[tekton-robot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

[tekton-robot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.

/lifecycle stale

Send feedback to tektoncd/plumbing.

[tekton-robot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

[tekton-robot]

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[bobcatfish]

/reopen
/remove-lifecycle rotten

/assign jerop

@jerop is actually taking care of this via https://github.com/tektoncd/community/blob/master/teps/0007-conditions-beta.md#skipping-1

[tekton-robot]

@bobcatfish: Reopened this issue.

In response to this:

/reopen
/remove-lifecycle rotten

/assign jerop

@jerop is actually taking care of this via https://github.com/tektoncd/community/blob/master/teps/0007-conditions-beta.md#skipping-1

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[tekton-robot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

[jerop]

open pr: https://github.com/tektoncd/pipeline/pull/3176

/remove-lifecycle rotten

[Bl4d3s]

@jerop I really could use the feature you implemented in the PR. is there any ETA when this will be merged?
If I can help with anything to speed things up, I can always take a look.

My use case:

1. Unit and Integration-Tests are executed
2. Optional additional tasks if systemtests or similar need to be executed (guarded with when expression)
3. Task collecting all test results from workspace (has to be after 2. obviously, but is not dependent on it)

[jerop]

@Bl4d3s we paused that work for a bit as we discussed some design choices and alternatives in https://github.com/tektoncd/pipeline/issues/3345 -- rebasing the pr now ~and planning to get it in before the v0.18 release~ (revisited syntax in https://github.com/tektoncd/community/pull/258)

[Bl4d3s]

@Bl4d3s we paused that work for a bit as we discussed some design choices and alternatives in #3345 -- rebasing the pr now and planning to get it in before the v0.18 release

I see the PR is already approved, but needs a rebase. Any hope to get it in with the next release?

[jerop]

I see the PR is already approved, but needs a rebase. Any hope to get it in with the next release?

@Bl4d3s we revisited the skipping syntax in this TEP to clarify that the skipping option is related to WhenExpressions -- thank you for your patience as we work to get it right and get it in soon