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

Dependent Task after Task with when condition are skipped #7680

Open
Allda opened this issue Feb 16, 2024 · 10 comments
Open

Dependent Task after Task with when condition are skipped #7680

Allda opened this issue Feb 16, 2024 · 10 comments
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@Allda
Copy link

Allda commented Feb 16, 2024

Expected Behavior

Hello, I am building a pipeline with a conditional series of tasks that use the same when condition, after that series of tasks there is one or more other tasks that don't have the condition.

Simplified task schema looks like this

A -> B -> C

where A and B use the condition given by the pipeline argument. At the same time, B uses the output of A as an input parameter.

I expect the C is triggered regardless of A or B but as shown bellow this is no the case and if A and B are skipped the C is also skipped.

Actual Behavior

Based on the pipeline argument A and B are skipped as expected, but C is also skipped which is unexpected. Skipped reasons are the following:
A - When Expressions evaluated to false
B - Results were missing < -- This task uses when and should be also skipped the same way as task A
C - Parent Tasks were skipped

Steps to Reproduce the Problem

  1. Create the following task and pipeline
apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: basic-task
spec:
  params:
    - default: Hello
      name: appName
      type: string
  results:
    - name: my-result
      type: string
  steps:
    - image: registry.redhat.io/ubi7/ubi-minimal
      name: ''
      resources: {}
      script: |

        echo $(inputs.params.appName)

        echo $(inputs.params.appName) > $(results.my-result.path)
apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
  name: when
spec:
  params:
    - default: foo
      name: param1
      type: string
  tasks:
    - name: basic-task
      params:
        - name: appName
          value: Hello
      taskRef:
        kind: Task
        name: basic-task
      when:
        - input: $(params.param1)
          operator: in
          values:
            - bar
    - name: basic-task-2
      params:
        - name: appName
          value: $(tasks.basic-task.results.my-result)
      runAfter:
        - basic-task
      taskRef:
        kind: Task
        name: basic-task
      when:
        - input: $(params.param1)
          operator: in
          values:
            - bar
    - name: basic-task-3
      params:
        - name: appName
          value: Hello
      runAfter:
        - basic-task-2
      taskRef:
        kind: Task
        name: basic-task

  1. Execute pipeline with default params

Additional Info

Client Version: 4.14.6
Kustomize Version: v5.0.1
Server Version: 4.13.31
Kubernetes Version: v1.26.13+77e61a2

  • Tekton Pipeline version:

Client version: 0.33.0
Chains version: v0.16.1
Pipeline version: v0.47.6
Triggers version: v0.24.2
Operator version: v0.67.3

@Allda Allda added the kind/bug Categorizes issue or PR as related to a bug. label Feb 16, 2024
@l-qing
Copy link
Contributor

l-qing commented Feb 17, 2024

I once encountered a similar problem, and I suspect it is related to the status "Results were missing" in Phase B.
After all, after skipping Phase A, Phase B still made a judgment on whether to execute.

@l-qing
Copy link
Contributor

l-qing commented Feb 17, 2024

I'm not sure if this behavior is by design or a bug.

Relevant code:

func (t *ResolvedPipelineTask) skip(facts *PipelineRunFacts) TaskSkipStatus {
var skippingReason v1.SkippingReason
switch {
case facts.isFinalTask(t.PipelineTask.Name) || t.isScheduled():
skippingReason = v1.None
case facts.IsStopping():
skippingReason = v1.StoppingSkip
case facts.IsGracefullyCancelled():
skippingReason = v1.GracefullyCancelledSkip
case facts.IsGracefullyStopped():
skippingReason = v1.GracefullyStoppedSkip
case t.skipBecauseParentTaskWasSkipped(facts):
skippingReason = v1.ParentTasksSkip

// skipBecauseParentTaskWasSkipped loops through the parent tasks and checks if the parent task skipped:
//
// 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
func (t *ResolvedPipelineTask) skipBecauseParentTaskWasSkipped(facts *PipelineRunFacts) bool {
stateMap := facts.State.ToMap()
node := facts.TasksGraph.Nodes[t.PipelineTask.Name]
for _, p := range node.Prev {
parentTask := stateMap[p.Key]
if parentSkipStatus := parentTask.Skip(facts); parentSkipStatus.IsSkipped {
// 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 == v1.WhenExpressionsSkip {
continue
}
return true
}
}
return false
}

// SkippingReason explains why a PipelineTask was skipped.
type SkippingReason string
const (
// WhenExpressionsSkip means the task was skipped due to at least one of its when expressions evaluating to false
WhenExpressionsSkip SkippingReason = "When Expressions evaluated to false"
// ParentTasksSkip means the task was skipped because its parent was skipped
ParentTasksSkip SkippingReason = "Parent Tasks were skipped"
// StoppingSkip means the task was skipped because the pipeline run is stopping
StoppingSkip SkippingReason = "PipelineRun was stopping"
// GracefullyCancelledSkip means the task was skipped because the pipeline run has been gracefully cancelled
GracefullyCancelledSkip SkippingReason = "PipelineRun was gracefully cancelled"
// GracefullyStoppedSkip means the task was skipped because the pipeline run has been gracefully stopped
GracefullyStoppedSkip SkippingReason = "PipelineRun was gracefully stopped"
// MissingResultsSkip means the task was skipped because it's missing necessary results
MissingResultsSkip SkippingReason = "Results were missing"

@AlanGreene
Copy link
Member

I think this is the relevant section of the docs on when expressions:
https://tekton.dev/docs/pipelines/pipelines/#guarding-a-task-only

When when expressions evaluate to False, the Task will be skipped and:

  • The ordering-dependent Tasks will be executed
  • The resource-dependent Tasks (and their dependencies) will be skipped because of missing Results from the skipped parent Task.

Specifically the last point.

Also from the example just after that:

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

As far as I understand this matches the behaviour you're describing so is working as designed.

@Allda
Copy link
Author

Allda commented Feb 19, 2024

I have spent last several days reading the documentation about how the conditional Tekton task works so I understand why this is happening, but on the other hand if a task depends on the previous one (B depends on A) and B is is not executed due to when condition should this be also blocking and skipping a rest of the pipeline? The missing results from A used in B as params are not needed because the B is skipped.

Would you happen to have any suggestions on how to workaround this? The usecase for this is when a pipeline contains a series of task that depends on each other and are controlled by input parameter. In such a case with a current behavior, the pipeline always skips the rest of the pipeline even though there are no missing data for dependencies.

Allda added a commit to redhat-openshift-ecosystem/operator-pipelines that referenced this issue Feb 19, 2024
Due to a Tekton limitation we can't execute a merge task if any previous
tasks have been skipped. This usually happens when not bundle is added
and ci.yaml file is updated. Until tekton enables this workflow I am
temporarily move the merge task into finally section.

The issue has been create in Tekton upstream to fix this issue
tektoncd/pipeline#7680

JIRA: ISV-4476

Signed-off-by: Ales Raszka <[email protected]>
Allda added a commit to redhat-openshift-ecosystem/operator-pipelines that referenced this issue Feb 19, 2024
Due to a Tekton limitation we can't execute a merge task if any previous
tasks have been skipped. This usually happens when not bundle is added
and ci.yaml file is updated. Until tekton enables this workflow I am
temporarily move the merge task into finally section.

The issue has been create in Tekton upstream to fix this issue
tektoncd/pipeline#7680

JIRA: ISV-4476

Signed-off-by: Ales Raszka <[email protected]>
Allda added a commit to redhat-openshift-ecosystem/operator-pipelines that referenced this issue Feb 19, 2024
Due to a Tekton limitation we can't execute a merge task if any previous
tasks have been skipped. This usually happens when not bundle is added
and ci.yaml file is updated. Until tekton enables this workflow I am
temporarily move the merge task into finally section.

The issue has been create in Tekton upstream to fix this issue
tektoncd/pipeline#7680

JIRA: ISV-4476

Signed-off-by: Ales Raszka <[email protected]>
Allda added a commit to redhat-openshift-ecosystem/operator-pipelines that referenced this issue Feb 20, 2024
Due to a Tekton limitation we can't execute a merge task if any previous
tasks have been skipped. This usually happens when not bundle is added
and ci.yaml file is updated. Until tekton enables this workflow I am
temporarily move the merge task into finally section.

The issue has been create in Tekton upstream to fix this issue
tektoncd/pipeline#7680

JIRA: ISV-4476

Signed-off-by: Ales Raszka <[email protected]>
@Allda
Copy link
Author

Allda commented Feb 28, 2024

@AlanGreene @l-qing Do you know if this issue can be workaround somehow until we get any official conclusion of this issue?

@Allda
Copy link
Author

Allda commented Feb 28, 2024

I checked the code briefly and I think moving skipBecauseWhenExpressionsEvaluatedToFalse higher in the switch hierarchy and giving it higher priority would resolve this issue. Is there any reason why it is below skipBecauseResultReferencesAreMissing and skipBecauseParentTaskWasSkipped ?

switch {
case facts.isFinalTask(t.PipelineTask.Name) || t.isScheduled():
skippingReason = v1.None
case facts.IsStopping():
skippingReason = v1.StoppingSkip
case facts.IsGracefullyCancelled():
skippingReason = v1.GracefullyCancelledSkip
case facts.IsGracefullyStopped():
skippingReason = v1.GracefullyStoppedSkip
case t.skipBecauseParentTaskWasSkipped(facts):
skippingReason = v1.ParentTasksSkip
case t.skipBecauseResultReferencesAreMissing(facts):
skippingReason = v1.MissingResultsSkip
case t.skipBecauseWhenExpressionsEvaluatedToFalse(facts):
skippingReason = v1.WhenExpressionsSkip
case t.skipBecausePipelineRunPipelineTimeoutReached(facts):
skippingReason = v1.PipelineTimedOutSkip
case t.skipBecausePipelineRunTasksTimeoutReached(facts):
skippingReason = v1.TasksTimedOutSkip
case t.skipBecauseEmptyArrayInMatrixParams():
skippingReason = v1.EmptyArrayInMatrixParams
default:
skippingReason = v1.None
}

@willejs-ec
Copy link

@Allda did you get anywhere with this? I think we are facing the same issue.

@Allda
Copy link
Author

Allda commented Jun 27, 2024

@willejs-ec Unfortunately I haven't. Due to this limitation, I wasn't able to use any when condition for tasks that have a common when condition and depend on each other. The only workaround I found was to put the condition inside of the task into the actual task code. This solution however makes the pipeline flow harder to read and forces a pipeline to create additional pods that are not doing any useful work and take resources and execution time.

@jameshwc
Copy link

jameshwc commented Sep 5, 2024

We're facing the same issue.

@tommyb82
Copy link

We are facing same issue, BTW I think this duplicates stale (but still open) issue that I just commented on before seeing this one: #7029

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

6 participants