pipeline icon indicating copy to clipboard operation
pipeline copied to clipboard

Task results can't be used in resolver parameters of other tasks

Open fische opened this issue 1 year ago • 5 comments

Expected Behavior

I am currently playing with the resolver concept and trying to understand everything that is possible with it. I was particularly interested in being able to re-use a task result in the resolver parameters, so a task outcome could decide the next task to run. I know you can do this statically but I was more interested in a dynamic approach.

Let's say in some pipeline, task-a outputs a target result and task-b has the following reference:

      taskRef:
        resolver: woodhouse-git
        params:
        - name: target
          value: $(tasks.task-a.results.target)

I would expect $(tasks.task-a.results.target) to be substituted in the ResolutionRequest for task-b.

Actual Behavior

$(tasks.task-a.results.target) is not substituted in the ResolutionRequest.

Steps to Reproduce the Problem

I just applied the following:

kind: PipelineRun
apiVersion: tekton.dev/v1
metadata:
  generateName: some-pipeline-
spec:
  pipelineSpec:
    params:
    - name: commit
    tasks:
    - name: task-a
      taskSpec:
        results:
        - name: target
          description: target for next task
        steps:
        - image: alpine:3.15
          script: |
            echo "//corp/woodhouse/tasks:task-a" > $(results.target.path)
    - name: task-b
      taskRef:
        resolver: woodhouse-git
        params:
        - name: repo
          value: [email protected]:tektoncd/pipeline.git
        - name: commitish
          value: $(params.commit)
        - name: target
          value: $(tasks.task-a.results.target)
  params:
  - name: commit
    value: "master"

This created the following ResolutionRequest:

apiVersion: resolution.tekton.dev/v1beta1
kind: ResolutionRequest
metadata:
  creationTimestamp: "2024-02-26T13:54:48Z"
  generation: 1
  labels:
    resolution.tekton.dev/type: woodhouse-git
  name: woodhouse-git-ff164e182c04759de8ea8acdd2ba272b
  namespace: woodhouse-tekton
  ownerReferences:
  - apiVersion: tekton.dev/v1
    blockOwnerDeletion: true
    controller: true
    kind: PipelineRun
    name: some-pipeline-qsqc7
    uid: 4fa2e350-a62e-40d2-be2e-ba5fe67b20c9
  resourceVersion: "1239944"
  uid: e5456bb6-e738-4147-a917-ce55e8fb8ff7
spec:
  params:
  - name: repo
    value: [email protected]:tektoncd/pipeline.git
  - name: commitish
    value: master
  - name: target
    value: $(tasks.task-a.results.target)

Notice the target value: $(tasks.task-a.results.target), while I was expecting it to be //corp/woodhouse/tasks:task-a as outputted by task-a.

I have also just realised that the ResolutionRequests for pipeline's tasks get created right when the PipelineRun is created (which is most likely related to this issue) and are owned by that PipelineRun. I don't understand this approach as it would have been so much simpler (and worked so much nicer with the rest), if the PipelineRun instead created the TaskRuns, and then those would create the ResolutionRequests. This would've meant that you would not need the additional logic when reconciling pipelines to check if there's any task to resolve and start their resolution, and instead you would be able to start the pipeline ASAP. I guess the only "tradeoff" with this is that you won't see a resolution failure immediately.

Additional Info

  • Kubernetes version:

    Output of kubectl version:

Client Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.17", GitCommit:"22a9682c8fe855c321be75c5faacde343f909b04", GitTreeState:"clean", BuildDate:"2023-08-23T23:44:35Z", GoVersion:"go1.20.7", Compiler:"gc", Platform:"linux/amd64"}
Kustomize Version: v4.5.4
Server Version: version.Info{Major:"1", Minor:"26", GitVersion:"v1.26.11", GitCommit:"3cd242c51317aed8858119529ccab22079f523b1", GitTreeState:"clean", BuildDate:"2023-11-15T16:50:12Z", GoVersion:"go1.20.11", 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.50.0

fische avatar Feb 26 '24 14:02 fische

@fische thanks for the issue. I think this is more of a feature request than a bug (as, it is not listed anywhere that it is possible to use variable interpolation in the resolver "spec", see here). That said, I agree 100% that it should be possible.

I have also just realised that the ResolutionRequests for pipeline's tasks get created right when the PipelineRun is created (which is most likely related to this issue) and are owned by that PipelineRun.

I don't think it is the case, but if it is, it is probably an issue we need to fix. In your example, it might happen because the dependency between task-a and task-b is implicit (aka not using runAfter), but I am not 100% sure. But if it happens that way, it is a bug in my opinion (so the issue is both a feature request and an issue 😅 ).

I don't understand this approach as it would have been so much simpler (and worked so much nicer with the rest), if the PipelineRun instead created the TaskRuns, and then those would create the ResolutionRequests. This would've meant that you would not need the additional logic when reconciling pipelines to check if there's any task to resolve and start their resolution, and instead you would be able to start the pipeline ASAP. I guess the only "tradeoff" with this is that you won't see a resolution failure immediately.

So we would create the TaskRun with the resolver and let the TaskRun reconciler do its job. I am not sure why it's not working like this, @abayer @afrittoli do you know anything about it ?

vdemeester avatar Feb 26 '24 15:02 vdemeester

@fische thanks for the issue. I think this is more of a feature request than a bug (as, it is not listed anywhere that it is possible to use variable interpolation in the resolver "spec", see here). That said, I agree 100% that it should be possible.

The reason I raised this as a bug is because variable substitution is possible in resolver parameters with the pipeline parameters (ie $(params.target)). This is not documented anywhere either, so I just assumed the documentation was out of date and the fact task results are not supported was a bug. But I get your point, I can turn this into a kind/feature if needed?

fische avatar Feb 27 '24 11:02 fische

@fische we can mark it as both maybe 🙃 /kind feature

vdemeester avatar Feb 27 '24 12:02 vdemeester

cc @chitrangpatel

vdemeester avatar Mar 19 '24 12:03 vdemeester

Hi,

Thanks for the issue. If I'm not wrong, param substitution of pipelinerun params into resolution request for a pipelineref does not work either.So it is a bit broken in that sense.

I think marking it as a feature for allowing variable replacements in remote resolution request makes sense to me. We should investigate it fully and add this capability. My worry is that if we ever want to change the behavior of remote resolution to an upfront remote resolution approach then the param substitutions can only happen as the dag expands and so upfront remote resolution will not be possible in that scenario. But those details can be questioned and hashed out when we discuss this feature in more detail.

chitrangpatel avatar Mar 19 '24 13:03 chitrangpatel