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

WhenExpressions are neglected when finally tasks are present #3196

Closed
VeereshAradhya opened this issue Sep 10, 2020 · 9 comments · Fixed by #3217
Closed

WhenExpressions are neglected when finally tasks are present #3196

VeereshAradhya opened this issue Sep 10, 2020 · 9 comments · Fixed by #3217
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@VeereshAradhya
Copy link

Expected Behavior

WhenExpressions should work when finally tasks are present

Actual Behavior

WhenExpressions will not work when finally tasks are present

Steps to Reproduce the Problem

  1. Create a Pipeline with tasks, when expressions and finally tasks
  2. Start the pipeline

Additional Info

  • Kubernetes version:

    Output of kubectl version:

    Client Version: version.Info{Major:"1", Minor:"10+", GitVersion:"v1.10.0+b3b92b2", GitCommit:"b3b92b2", GitTreeState:"clean", BuildDate:"2020-07-15T09:27:21Z", GoVersion:"go1.14.3", Compiler:"gc", Platform:"linux/amd64"}
    Server Version: version.Info{Major:"1", Minor:"18+", GitVersion:"v1.18.3+8b0a82f", GitCommit:"8b0a82f", GitTreeState:"clean", BuildDate:"2020-07-10T05:34:00Z", GoVersion:"go1.13.4", Compiler:"gc", Platform:"linux/amd64"}
    
  • Tekton Pipeline version:

    $ tkn version
    Client version: 0.12.0
    Pipeline version: v0.16.0
    Triggers version: unknown
    

Command Execution Logs:
Execution logs contain the output of both pipeline with finally task and pipeline without finally task

[varadhya@localhost workspace-testing]$ tkn t ls
NAME          DESCRIPTION          AGE
final-task    This is first task   8 minutes ago
first-task    This is first task   17 minutes ago
second-task   This is first task   17 minutes ago
[varadhya@localhost workspace-testing]$ 
[varadhya@localhost workspace-testing]$ tkn p ls
No Pipelines found
[varadhya@localhost workspace-testing]$ 
[varadhya@localhost workspace-testing]$ cat pipeline.yaml 
apiVersion: tekton.dev/v1beta1 
kind: Pipeline 
metadata:
  name: test-when-expression-pipeline
spec:
  params:
    - name: status
  tasks:
    - name: first-task
      taskRef:
        name: first-task
        kind: Task
    - name: second-task
      taskRef:
        name: second-task
        kind: Task
      when:
        - input: "$(params.status)"
          operator: in
          values: ["Success", "success"]
[varadhya@localhost workspace-testing]$ 
[varadhya@localhost workspace-testing]$ kubectl apply -f pipeline.yaml 
pipeline.tekton.dev/test-when-expression-pipeline created
[varadhya@localhost workspace-testing]$ 
[varadhya@localhost workspace-testing]$ tkn p start test-when-expression-pipeline 
? Value for param `status` of type `string`? Failure
PipelineRun started: test-when-expression-pipeline-run-pvlw9

In order to track the PipelineRun progress run:
tkn pipelinerun logs test-when-expression-pipeline-run-pvlw9 -f -n veeresh-testing
[varadhya@localhost workspace-testing]$ 
[varadhya@localhost workspace-testing]$ tkn pr describe test-when-expression-pipeline-run-pvlw9 -o yaml
apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"tekton.dev/v1beta1","kind":"Pipeline","metadata":{"annotations":{},"name":"test-when-expression-pipeline","namespace":"veeresh-testing"},"spec":{"params":[{"name":"status"}],"tasks":[{"name":"first-task","taskRef":{"kind":"Task","name":"first-task"}},{"name":"second-task","taskRef":{"kind":"Task","name":"second-task"},"when":[{"input":"$(params.status)","operator":"in","values":["Success","success"]}]}]}}
  creationTimestamp: "2020-09-10T15:57:41Z"
  generateName: test-when-expression-pipeline-run-
  generation: 1
  labels:
    tekton.dev/pipeline: test-when-expression-pipeline
  managedFields:
  - apiVersion: tekton.dev/v1beta1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:generateName: {}
      f:spec:
        .: {}
        f:params: {}
        f:pipelineRef:
          .: {}
          f:name: {}
    manager: tkn
    operation: Update
    time: "2020-09-10T15:57:41Z"
  - apiVersion: tekton.dev/v1beta1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          .: {}
          f:kubectl.kubernetes.io/last-applied-configuration: {}
        f:labels:
          .: {}
          f:tekton.dev/pipeline: {}
      f:status:
        .: {}
        f:completionTime: {}
        f:conditions: {}
        f:pipelineSpec:
          .: {}
          f:params: {}
          f:tasks: {}
        f:skippedTasks: {}
        f:startTime: {}
        f:taskRuns:
          .: {}
          f:test-when-expression-pipeline-run-pvlw9-first-task-t4qk4:
            .: {}
            f:pipelineTaskName: {}
            f:status:
              .: {}
              f:completionTime: {}
              f:conditions: {}
              f:podName: {}
              f:startTime: {}
              f:steps: {}
              f:taskSpec:
                .: {}
                f:description: {}
                f:results: {}
                f:steps: {}
    manager: controller
    operation: Update
    time: "2020-09-10T15:57:53Z"
  name: test-when-expression-pipeline-run-pvlw9
  namespace: veeresh-testing
  resourceVersion: "354062"
  selfLink: /apis/tekton.dev/v1beta1/namespaces/veeresh-testing/pipelineruns/test-when-expression-pipeline-run-pvlw9
  uid: d94cb1a6-ac99-44f6-8389-eea2e5541d78
spec:
  params:
  - name: status
    value: Failure
  pipelineRef:
    name: test-when-expression-pipeline
  timeout: 1h0m0s
status:
  completionTime: "2020-09-10T15:57:53Z"
  conditions:
  - lastTransitionTime: "2020-09-10T15:57:53Z"
    message: 'Tasks Completed: 1 (Failed: 0, Cancelled 0), Skipped: 1'
    reason: Completed
    status: "True"
    type: Succeeded
  pipelineSpec:
    params:
    - name: status
      type: string
    tasks:
    - name: first-task
      taskRef:
        kind: Task
        name: first-task
    - name: second-task
      taskRef:
        kind: Task
        name: second-task
      when:
      - Input: $(params.status)
        Operator: in
        Values:
        - Success
        - success
  skippedTasks:
  - name: second-task
  startTime: "2020-09-10T15:57:41Z"
  taskRuns:
    test-when-expression-pipeline-run-pvlw9-first-task-t4qk4:
      pipelineTaskName: first-task
      status:
        completionTime: "2020-09-10T15:57:53Z"
        conditions:
        - lastTransitionTime: "2020-09-10T15:57:53Z"
          message: All Steps have completed executing
          reason: Succeeded
          status: "True"
          type: Succeeded
        podName: test-when-expression-pipeline-run-pvlw9-first-task-t4qk4--ngrw6
        startTime: "2020-09-10T15:57:41Z"
        steps:
        - container: step-shell-example
          imageID: docker.io/library/ubuntu@sha256:31dfb10d52ce76c5ca0aa19d10b3e6424b830729e32a89a7c6eee2cda2be67a5
          name: shell-example
          terminated:
            containerID: cri-o://5c0e1ab4325a6bfb5f3e31414e474a344e57cb1afd4f773e6b96a90fd124eb79
            exitCode: 0
            finishedAt: "2020-09-10T15:57:52Z"
            reason: Completed
            startedAt: "2020-09-10T15:57:52Z"
        taskSpec:
          description: This is first task
          results:
          - description: this stores the status
            name: status
          steps:
          - image: ubuntu
            name: shell-example
            resources: {}
            script: |
              #!/bin/bash
              echo "This is first task"
[varadhya@localhost workspace-testing]$ 
[varadhya@localhost workspace-testing]$ 
[varadhya@localhost workspace-testing]$ cat pipeline-finally.yaml 
apiVersion: tekton.dev/v1beta1 
kind: Pipeline 
metadata:
  name: test-when-expression-pipeline-finally
spec:
  params:
    - name: status
  tasks:
    - name: first-task
      taskRef:
        name: first-task
        kind: Task
    - name: second-task
      taskRef:
        name: second-task
        kind: Task
      when:
        - input: "$(params.status)"
          operator: in
          values: ["Success", "success"]
  finally:
    - name: finally-task
      taskRef:
        name: final-task
        kind: Task
[varadhya@localhost workspace-testing]$ 
[varadhya@localhost workspace-testing]$ kubectl apply -f pipeline-finally.yaml 
pipeline.tekton.dev/test-when-expression-pipeline-finally created
[varadhya@localhost workspace-testing]$ 
[varadhya@localhost workspace-testing]$ tkn p start test-when-expression-pipeline-finally
? Value for param `status` of type `string`? Failure
PipelineRun started: test-when-expression-pipeline-finally-run-vgvsp

In order to track the PipelineRun progress run:
tkn pipelinerun logs test-when-expression-pipeline-finally-run-vgvsp -f -n veeresh-testing
[varadhya@localhost workspace-testing]$ 
[varadhya@localhost workspace-testing]$ tkn pr ls
NAME                                              STARTED          DURATION     STATUS
test-when-expression-pipeline-finally-run-vgvsp   40 seconds ago   29 seconds   Succeeded
test-when-expression-pipeline-run-pvlw9           4 minutes ago    12 seconds   Succeeded(Completed)
[varadhya@localhost workspace-testing]$ 
[varadhya@localhost workspace-testing]$ tkn pr describe test-when-expression-pipeline-finally-run-vgvsp -o yaml
apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"tekton.dev/v1beta1","kind":"Pipeline","metadata":{"annotations":{},"name":"test-when-expression-pipeline-finally","namespace":"veeresh-testing"},"spec":{"finally":[{"name":"finally-task","taskRef":{"kind":"Task","name":"final-task"}}],"params":[{"name":"status"}],"tasks":[{"name":"first-task","taskRef":{"kind":"Task","name":"first-task"}},{"name":"second-task","taskRef":{"kind":"Task","name":"second-task"},"when":[{"input":"$(params.status)","operator":"in","values":["Success","success"]}]}]}}
  creationTimestamp: "2020-09-10T16:01:02Z"
  generateName: test-when-expression-pipeline-finally-run-
  generation: 1
  labels:
    tekton.dev/pipeline: test-when-expression-pipeline-finally
  managedFields:
  - apiVersion: tekton.dev/v1beta1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:generateName: {}
      f:spec:
        .: {}
        f:params: {}
        f:pipelineRef:
          .: {}
          f:name: {}
    manager: tkn
    operation: Update
    time: "2020-09-10T16:01:02Z"
  - apiVersion: tekton.dev/v1beta1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          .: {}
          f:kubectl.kubernetes.io/last-applied-configuration: {}
        f:labels:
          .: {}
          f:tekton.dev/pipeline: {}
      f:status:
        .: {}
        f:completionTime: {}
        f:conditions: {}
        f:pipelineSpec:
          .: {}
          f:finally: {}
          f:params: {}
          f:tasks: {}
        f:startTime: {}
        f:taskRuns:
          .: {}
          f:test-when-expression-pipeline-finally-run-vgvsp-finally-t-6dwhq:
            .: {}
            f:pipelineTaskName: {}
            f:status:
              .: {}
              f:completionTime: {}
              f:conditions: {}
              f:podName: {}
              f:startTime: {}
              f:steps: {}
              f:taskSpec:
                .: {}
                f:description: {}
                f:results: {}
                f:steps: {}
          f:test-when-expression-pipeline-finally-run-vgvsp-first-tas-qbnrg:
            .: {}
            f:pipelineTaskName: {}
            f:status:
              .: {}
              f:completionTime: {}
              f:conditions: {}
              f:podName: {}
              f:startTime: {}
              f:steps: {}
              f:taskSpec:
                .: {}
                f:description: {}
                f:results: {}
                f:steps: {}
          f:test-when-expression-pipeline-finally-run-vgvsp-second-ta-8k64m:
            .: {}
            f:pipelineTaskName: {}
            f:status:
              .: {}
              f:completionTime: {}
              f:conditions: {}
              f:podName: {}
              f:startTime: {}
              f:steps: {}
              f:taskSpec:
                .: {}
                f:description: {}
                f:results: {}
                f:steps: {}
    manager: controller
    operation: Update
    time: "2020-09-10T16:01:31Z"
  name: test-when-expression-pipeline-finally-run-vgvsp
  namespace: veeresh-testing
  resourceVersion: "354942"
  selfLink: /apis/tekton.dev/v1beta1/namespaces/veeresh-testing/pipelineruns/test-when-expression-pipeline-finally-run-vgvsp
  uid: 5720518b-6dbc-46a4-80ff-6f262a9f6c7c
spec:
  params:
  - name: status
    value: Failure
  pipelineRef:
    name: test-when-expression-pipeline-finally
  timeout: 1h0m0s
status:
  completionTime: "2020-09-10T16:01:31Z"
  conditions:
  - lastTransitionTime: "2020-09-10T16:01:31Z"
    message: 'Tasks Completed: 3 (Failed: 0, Cancelled 0), Skipped: 0'
    reason: Succeeded
    status: "True"
    type: Succeeded
  pipelineSpec:
    finally:
    - name: finally-task
      taskRef:
        kind: Task
        name: final-task
    params:
    - name: status
      type: string
    tasks:
    - name: first-task
      taskRef:
        kind: Task
        name: first-task
    - name: second-task
      taskRef:
        kind: Task
        name: second-task
      when:
      - Input: $(params.status)
        Operator: in
        Values:
        - Success
        - success
  startTime: "2020-09-10T16:01:02Z"
  taskRuns:
    test-when-expression-pipeline-finally-run-vgvsp-finally-t-6dwhq:
      pipelineTaskName: finally-task
      status:
        completionTime: "2020-09-10T16:01:31Z"
        conditions:
        - lastTransitionTime: "2020-09-10T16:01:31Z"
          message: All Steps have completed executing
          reason: Succeeded
          status: "True"
          type: Succeeded
        podName: test-when-expression-pipeline-finally-run-vgvsp-finally-t-d6blv
        startTime: "2020-09-10T16:01:17Z"
        steps:
        - container: step-shell-example
          imageID: docker.io/library/ubuntu@sha256:31dfb10d52ce76c5ca0aa19d10b3e6424b830729e32a89a7c6eee2cda2be67a5
          name: shell-example
          terminated:
            containerID: cri-o://04027c0ed7ebed6e144e31224d66cf1b6de8ab0840b70ca0fcc1a8660896efa0
            exitCode: 0
            finishedAt: "2020-09-10T16:01:30Z"
            reason: Completed
            startedAt: "2020-09-10T16:01:30Z"
        taskSpec:
          description: This is first task
          results:
          - description: this stores the status
            name: status
          steps:
          - image: ubuntu
            name: shell-example
            resources: {}
            script: |
              #!/bin/bash
              echo "This is final task"
    test-when-expression-pipeline-finally-run-vgvsp-first-tas-qbnrg:
      pipelineTaskName: first-task
      status:
        completionTime: "2020-09-10T16:01:17Z"
        conditions:
        - lastTransitionTime: "2020-09-10T16:01:17Z"
          message: All Steps have completed executing
          reason: Succeeded
          status: "True"
          type: Succeeded
        podName: test-when-expression-pipeline-finally-run-vgvsp-first-tas-4xqgm
        startTime: "2020-09-10T16:01:02Z"
        steps:
        - container: step-shell-example
          imageID: docker.io/library/ubuntu@sha256:31dfb10d52ce76c5ca0aa19d10b3e6424b830729e32a89a7c6eee2cda2be67a5
          name: shell-example
          terminated:
            containerID: cri-o://15cb134c06d14d88ef7ad647a035fcefb0275f6114aee76e7887ee00dc735a90
            exitCode: 0
            finishedAt: "2020-09-10T16:01:17Z"
            reason: Completed
            startedAt: "2020-09-10T16:01:17Z"
        taskSpec:
          description: This is first task
          results:
          - description: this stores the status
            name: status
          steps:
          - image: ubuntu
            name: shell-example
            resources: {}
            script: |
              #!/bin/bash
              echo "This is first task"
    test-when-expression-pipeline-finally-run-vgvsp-second-ta-8k64m:
      pipelineTaskName: second-task
      status:
        completionTime: "2020-09-10T16:01:17Z"
        conditions:
        - lastTransitionTime: "2020-09-10T16:01:17Z"
          message: All Steps have completed executing
          reason: Succeeded
          status: "True"
          type: Succeeded
        podName: test-when-expression-pipeline-finally-run-vgvsp-second-ta-52kbv
        startTime: "2020-09-10T16:01:02Z"
        steps:
        - container: step-shell-example
          imageID: docker.io/library/ubuntu@sha256:31dfb10d52ce76c5ca0aa19d10b3e6424b830729e32a89a7c6eee2cda2be67a5
          name: shell-example
          terminated:
            containerID: cri-o://c03527d787b2aa70c8bdbcbd873aea70631619e5313882b3f41d262941a7a56c
            exitCode: 0
            finishedAt: "2020-09-10T16:01:17Z"
            reason: Completed
            startedAt: "2020-09-10T16:01:17Z"
        taskSpec:
          description: This is first task
          results:
          - description: this stores the status
            name: status
          steps:
          - image: ubuntu
            name: shell-example
            resources: {}
            script: |
              #!/bin/bash
              echo "This is second task"
[varadhya@localhost workspace-testing]$ 
@VeereshAradhya VeereshAradhya added the kind/bug Categorizes issue or PR as related to a bug. label Sep 10, 2020
@VeereshAradhya VeereshAradhya changed the title WhenExpressions will not work when finally tasks are present WhenExpressions are neglected when finally tasks are present Sep 10, 2020
jerop added a commit to jerop/pipeline that referenced this issue Sep 10, 2020
Tasks with when expressions using variables that evaluated to false were
mistakenly were mistakenly executed because we missed an additional check
to validate that the task should be skipped after variable replacement.

Fixes tektoncd#3196
@bobcatfish bobcatfish added this to the Pipelines 0.16.1 Bug-fix milestone Sep 10, 2020
@bobcatfish
Copy link
Collaborator

Quick update: @jerop is working on these and we'll make a 0.16.1 release (https://github.com/tektoncd/pipeline/milestone/34) once these are fixed. Thanks for the quick report @VeereshAradhya !!

@pritidesai
Copy link
Member

pritidesai commented Sep 11, 2020

The issue is when expression param is not getting resolved and remains as is when a list of candidates are calculated.

// candidateTasks is initialized to DAG root nodes to start pipeline execution
// candidateTasks is derived based on successfully finished tasks and/or skipped tasks
candidateTasks, err := dag.GetSchedulable(d, pipelineState.SuccessfulOrSkippedDAGTasks(d)...)
if err != nil {
logger.Errorf("Error getting potential next tasks for valid pipelinerun %s: %v", pr.Name, err)
return controller.NewPermanentError(err)
}
// nextRprts holds a list of pipeline tasks which should be executed next
nextRprts = pipelineState.GetNextTasks(candidateTasks)

This is what pipelineState contains and Skip function returns false since when expression still has variables:

  WhenExpressions: (v1beta1.WhenExpressions) (len=1 cap=1) {
    (v1beta1.WhenExpression) {
     Input: (string) (len=17) "$(params.MESSAGE)",
     Operator: (selection.Operator) (len=5) "notin",
     Values: ([]string) (len=1 cap=1) {
      (string) (len=12) "Hi"
     }    

// Check if the when expressions are false, based on the input's relationship to the values
if len(t.PipelineTask.WhenExpressions) > 0 {
if !t.PipelineTask.WhenExpressions.HaveVariables() {
if !t.PipelineTask.WhenExpressions.AllowsExecution() {
return true

Sometimes replacements does not have any entry for that param:

tasks[i].WhenExpressions = tasks[i].WhenExpressions.ReplaceWhenExpressionsVariables(replacements)

(map[string]string) (len=4) {
 (string) (len=24) "context.pipelineRun.name": (string) (len=41) "pipelinerun-with-taskspec-to-echo-message",
 (string) (len=21) "context.pipeline.name": (string) (len=41) "pipelinerun-with-taskspec-to-echo-message",
 (string) (len=29) "context.pipelineRun.namespace": (string) (len=7) "default",
 (string) (len=23) "context.pipelineRun.uid": (string) (len=36) "a6d3d26e-db0d-472b-9e6a-e7420bac975b"
}

and at times it does:

(map[string]string) (len=1) {
 (string) (len=14) "params.MESSAGE": (string) (len=12) "Hi"
}

@pritidesai
Copy link
Member

when expression with constants works as expected in presence of finally tasks.

@pritidesai
Copy link
Member

when expression with task results (with PR #3197) behaves odd with finally tasks. PipelineRun gets stuck and finally tasks are waiting for their turn to be scheduled 😞 I havent got chance to debug this but here is a sample pipelinerun and its output.

apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  name: when-expression-9
spec:
  pipelineSpec:
    tasks:
      - name: echo-message-1
        taskSpec:
          results:
            - name: message
          steps:
            - name: echo
              image: ubuntu
              script: |
                #!/usr/bin/env bash
                echo -n "HI" | tee $(results.message.path)
      - name: echo-message-2
        when:
          - input: $(tasks.echo-message-1.results.message)
            operator: notin
            values: ["HI"]
        taskSpec:
          steps:
            - name: echo
              image: ubuntu
              script: |
                #!/usr/bin/env bash
                echo "Hi!"
    finally:
      - name: echo-message-3
        taskSpec:
          steps:
            - name: echo
              image: ubuntu
              script: |
                #!/usr/bin/env bash
                echo "Hi!"
---

Output of kubectl get pr -o json:
https://gist.github.com/pritidesai/dd783e34781224552feeaf8d8c84d25f

@bobcatfish
Copy link
Collaborator

Thanks @pritidesai !

@jerop and i were able to make some progress towards a fix (wip in master...bobcatfish:when_finally if you're curious) - don't want to rush it so ETA for getting this in will be next week, hoping to have a release with a fix Monday!

@pritidesai
Copy link
Member

Thanks @bobcatfish, was just talking to @jerop, yup no rush, lets fix it right.

Both the issues mentioned above (when expression params and task results not getting resolved in presence of finally task) are connected to the when expression resolution :

func (wes WhenExpressions) ReplaceWhenExpressionsVariables(replacements map[string]string) WhenExpressions {
var replaced []WhenExpression
for _, we := range wes {
replaced = append(replaced, we.applyReplacements(replacements))
}
return replaced

I was asking @jerop if we can modify this function to:

  1. Pointer receiver instead of value receiver
  2. Modify in place instead of creating a new copy of when expression.
  3. Return &WhenExpression instead of WhenExpression:

return WhenExpression{Input: replacedInput, Operator: we.Operator, Values: replacedValues}

If we fix this logic, we might not need skip flag. Also, instead of introducing skip in pipelineRunState which is created with every reconcile cycle, we could rely on pr.Status.SkippedTasks which is consistent across the entire pipeline run execution.

Examples:

when-expression-params-with-finally.yaml
when-expression-task-results-with-finally.yaml

Changes on fixing first example very similar to what you have 😜

@pritidesai
Copy link
Member

Mix of var replaced []WhenExpression with append 🤔 may be causing weird memory allocation ...

@pritidesai
Copy link
Member

pritidesai commented Sep 12, 2020

@bobcatfish @jerop

PR #3217 partially fixes this issue and partially issue #3203, will try to add tests for the use cases its fixing.

@pritidesai
Copy link
Member

I have updated PR #3217 to fix both issues (#3203 and #3217). Haven't got chance to add more integration tests, any help would be appreciated with adding tests 🙏

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

Successfully merging a pull request may close this issue.

3 participants