task results not templated if task order is reversed

[gerrnot]

Expected Behavior

In a kind: "Pipeline", where you define the tasks (list), the order should not matter because the order in tekton is normally defined via the runAfter attribute or in case of a resource dependencies via a syntax like $(tasks.<task-name>.results.<result-name>).

Actual Behavior

However, it can be demonstrated, in the latest release of tekton pipelines (v0.51.0) that the ordering in the task list matters.
That is, tasks might behave in very awkward ways, depending on how they treat receiving an actual result string vs an untemplated string like $(tasks.<task-name>.results.<result-name>).

Steps to Reproduce the Problem

Unpack the attached tar.gz (contains only yaml files)

1. k apply -f task-exe.yaml
2. k apply -f pipeline-order-wrong.yaml
3. k create -f pipelinerun.yaml
4. The resulting pipelinerun will fail, reproducing the error.

Additional Info

• Kubernetes version:

  Output of kubectl version:

Server Version: version.Info{Major:"1", Minor:"26", GitVersion:"v1.26.8", GitCommit:"395f0a2fdc940aeb9ab88849e8fa4321decbf6e1", GitTreeState:"clean", BuildDate:"2023-08-24T00:43:07Z", GoVersion:"go1.20.7", Compiler:"gc", Platform:"linux/amd64"}

• Tekton Pipeline version:

  Output of tkn version or kubectl get pods -n tekton-pipelines -l app=tekton-pipelines-controller -o=jsonpath='{.items[0].metadata.labels.version}'

v0.51.0

cd-pipeline-dev-reduced.tar.gz

[adelmoradian]

I also bumped into this issue after upgrading to v0.51.0

Here is an example that showcases this issue:

This following example runs task3 after task1 and task2 however the output is

$(tasks.task1.results.foo)
$(tasks.task2.results.bar)

apiVersion: tekton.dev/v1
kind: PipelineRun
metadata:
  generateName: pr-echo-
spec:
  pipelineSpec:
    tasks:
      - name: task1
        taskSpec:
          results:
            - name: foo
          steps:
            - name: step1
              env:
                - name: RESULT_PATH
                  value: $(results.foo.path)
              image: ubuntu
              script: |
                #!/usr/bin/env bash
                echo "foo-value" > "$RESULT_PATH"

      - name: task3
        runAfter:
          - task1
          - task2
        params:
          - name: foo-value
            value: $(tasks.task1.results.foo)
          - name: bar-value
            value: $(tasks.task2.results.bar)
        taskSpec:
          steps:
            - name: step1
              env:
                - name: FOO_VALUE
                  value: $(params.foo-value)
                - name: BAR_VALUE
                  value: $(params.bar-value)
              image: ubuntu
              script: |
                #!/usr/bin/env bash
                echo "$FOO_VALUE"
                echo "$BAR_VALUE"

      - name: task2
        taskSpec:
          results:
            - name: bar
          steps:
            - name: step1
              env:
                - name: RESULT_PATH
                  value: $(results.bar.path)
              image: ubuntu
              script: |
                #!/usr/bin/env bash
                echo "bar-value" > "$RESULT_PATH"

If i put task3 after task2 however, the output is correct

foo-value

bar-value

[Tomcli]

This issue also happened in the latest LTS version 0.50.1 but it was working fine in the previous LTS version 0.47.4. I also have an example to reproduce this issue at #7155.

[afrittoli]

Thanks @Tomcli @adelmoradian and @gerrnot for reporting this. I verified that the issue was introduced in v0.50.0 - in v0.49.0 it still worked fine. I will now try and track down the exact source.

[afrittoli]

I believe this was introduced in #6792.

What happens in the logic is today:

• the pipeline run controller builds the status of the pipeline run in resolvePipelineState
• the function loops over all pipeline tasks and incrementally builds a state, using ResolvePipelineTask
• ResolvePipelineTask relies on the state to resolve result references. The state it's passed though is partial and it contains only tasks processed in the loop that far. As long as tasks are defined in the order of execution this ~works

This need to be changed so that a state without result resolution is built first, and result references are resolved then.
Unfortunately it's not possible to simply revert the original PR, as that would remove a feature, so the fix to this regression will be an additive.