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

Results "interpolation" in embedded spec (or "results" propagation) #7086

Closed
vdemeester opened this issue Sep 1, 2023 · 6 comments
Closed
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug.

Comments

@vdemeester
Copy link
Member

Expected Behavior

A "somewhat" consistent behavior when using results and embedded spec. Naively, I would assume the following would work (but it does not).

cat <<'EOF' | kubectl create -f -
---
apiVersion: tekton.dev/v1
kind: Task
metadata:
  name: uid-task
spec:
  results:
    - name: uid
  steps:
    - name: uid
      image: alpine
      command: ["/bin/sh", "-c"]
      args:
        - echo "1001" | tee $(results.uid.path)
---
apiVersion: tekton.dev/v1
kind: PipelineRun
metadata:
  name: uid-pipeline-run
spec:
  pipelineSpec:
    tasks:
    - name: add-uid
      taskRef:
        name: uid-task
    - name: show-uid
      taskSpec:
        steps:
          - name: show-uid
            image: alpine
            command: ["/bin/sh", "-c"]
            args:
              - echo $(tasks.add-uid.results.uid)
EOF

Actual Behavior

The above example fails with /bin/sh: tasks.add-uid.results.uid: not found. My guess is that, we do have params and workspace propagation on embedded spec, but we do not have them for results.

Steps to Reproduce the Problem

  1. Run the command above
  2. See it fail

Additional Info

  • Kubernetes version: 1.25.0
  • Tekton Pipeline version: v0.50.x

As said above, this might be more a feature request than a bug as, I feel this is mainly because we do not "propagate" results to embedded spec. This does feel inconsistent as we do it for other fields (like params and workspaces).

cc @jerop @lbernick

@vdemeester vdemeester added the kind/bug Categorizes issue or PR as related to a bug. label Sep 1, 2023
@jerop
Copy link
Member

jerop commented Sep 6, 2023

I also think it's a great idea to propagate results as well -- we descoped this as future work in TEP-0107 which proposed propagating parameters -- https://github.com/tektoncd/community/blob/main/teps/0107-propagating-parameters.md#passing-results

@jerop jerop added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Sep 6, 2023
@chengjoey
Copy link
Member

chengjoey commented Sep 8, 2023

+1, Execute the above pipelinerun, the two tasks will be executed at the same time, the subsequent implementation of the passing-results function, dose need the user to explicitly specify runAfter, let the show-uid task execute after add-uid

chengjoey added a commit to chengjoey/pipeline that referenced this issue Sep 11, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue tektoncd#7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
chengjoey added a commit to chengjoey/pipeline that referenced this issue Sep 11, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue tektoncd#7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
@jerop
Copy link
Member

jerop commented Sep 11, 2023

@chengjoey I don't think the user should have to specify runAfter. Currently, we deduce that there's a dependency when a Result is used in Params or When Expressions without users having to specify runAfter. I'd prefer to maintain that behavior.

@chengjoey
Copy link
Member

@chengjoey I don't think the user should have to specify runAfter. Currently, we deduce that there's a dependency when a Result is used in Params or When Expressions without users having to specify runAfter. I'd prefer to maintain that behavior.

in pr, i added task steps sub dependency expression to result references, so that the user does not need to specify runAfter`

@jerop
Copy link
Member

jerop commented Sep 12, 2023

sounds good, thank you @chengjoey

chengjoey added a commit to chengjoey/pipeline that referenced this issue Sep 21, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue tektoncd#7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
chengjoey added a commit to chengjoey/pipeline that referenced this issue Oct 7, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue tektoncd#7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
chengjoey added a commit to chengjoey/pipeline that referenced this issue Oct 10, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue tektoncd#7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
chengjoey added a commit to chengjoey/pipeline that referenced this issue Oct 11, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue tektoncd#7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
chengjoey added a commit to chengjoey/pipeline that referenced this issue Oct 11, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue tektoncd#7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
chengjoey added a commit to chengjoey/pipeline that referenced this issue Oct 11, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue tektoncd#7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
chengjoey added a commit to chengjoey/pipeline that referenced this issue Oct 20, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue tektoncd#7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
chengjoey added a commit to chengjoey/pipeline that referenced this issue Oct 20, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue tektoncd#7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
chengjoey added a commit to chengjoey/pipeline that referenced this issue Oct 20, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue tektoncd#7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
chengjoey added a commit to chengjoey/pipeline that referenced this issue Oct 20, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue tektoncd#7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
chengjoey added a commit to chengjoey/pipeline that referenced this issue Oct 20, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue tektoncd#7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
chengjoey added a commit to chengjoey/pipeline that referenced this issue Oct 20, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue tektoncd#7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
chengjoey added a commit to chengjoey/pipeline that referenced this issue Oct 20, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue tektoncd#7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
tekton-robot pushed a commit that referenced this issue Oct 20, 2023
… reference

2. propagate results to embedded task spec
Part of work on issue #7086

Signed-off-by: chengjoey <zchengjoey@gmail.com>
@vdemeester
Copy link
Member Author

This is fixed by #7100.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

3 participants