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

Directly access Pipeline Params from taskSpec #1879

Closed
pritidesai opened this issue Jan 15, 2020 · 13 comments
Closed

Directly access Pipeline Params from taskSpec #1879

pritidesai opened this issue Jan 15, 2020 · 13 comments
Labels
kind/question Issues or PRs that are questions around the project or a particular feature lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@pritidesai
Copy link
Member

With the PR #1554, we added support to specify taskSpec along with pipelineSpec so that we can embed task and pipeline specifications instead of referencing them. But, there is one caveat that it leads to when it comes to accessing pipeline params since its modeled based on the way task and pipeline are referenced. Ideally, we would like to directly access pipeline parameters directly instead of defining a param under taskSpec and then use that param. For example, task param MESSAGE is bound to pipeline param MESSAGE before referring to it inside the task script as $(inputs.params.MESSAGE):

apiVersion: tekton.dev/v1alpha1
kind: PipelineRun
metadata:
  name: pipelinerun-with-taskspec-to-echo-message
spec:
  pipelineSpec:
    params:
      - name: MESSAGE
        description: "Message, default is Hello World!"
        type: string
        default: "Hello World!"
    tasks:
      - name: echo-message
        taskSpec:
          inputs:
            params:
              - name: MESSAGE
                type: string
                default: "Hello World!"
          steps:
            - name: echo
              image: ubuntu
              script: |
                #!/usr/bin/env bash
                echo "$(inputs.params.MESSAGE)"
        params:
          - name: MESSAGE
            value: $(params.MESSAGE)
  params:
    - name: MESSAGE
      value: "Good Morning!"

Can we simplify this approach and instead of declaring task parameters, allow task authors to directly use pipeline params?

apiVersion: tekton.dev/v1alpha1
kind: PipelineRun
metadata:
  name: pipelinerun-with-taskspec-to-echo-message
spec:
  pipelineSpec:
    params:
      - name: MESSAGE
        description: "Message, default is Hello World!"
        type: string
        default: "Hello World!"
    tasks:
      - name: echo-message
        taskSpec:
          steps:
            - name: echo
              image: ubuntu
              script: |
                #!/usr/bin/env bash
                echo "$(params.MESSAGE)"
  params:
    - name: MESSAGE
      value: "Good Morning!"
@dibyom
Copy link
Member

dibyom commented Jan 16, 2020

Interesting idea...possibly related to #1185 ?
My $0.02 - while I definitely like the idea of being more consistent param usage, IMO we should do it across the board instead of just in one place. Otherwise something like this can become confusing -- say you start off with task specs embedded in your pipeline. Later you want to extract it out into its own task. Now you'd have to change all instance of params to input.params. The same thing applies if you want to take an existing task and embed it in your pipeline (say for small modifications/debugging/whatever)

@dibyom dibyom added the kind/question Issues or PRs that are questions around the project or a particular feature label Jan 16, 2020
@bobcatfish
Copy link
Collaborator

I agree with @dibyom - @pritidesai are you able to share more details around the use case you need this for?

imo embedding inside of Runs should only happen when either:

  1. we're talking about getting up and running quickly (e.g. getting a new user to run a "hello world" and see it work right away
  2. the Run is completely generated by a tool on top of it and the user isn't exposed to it at all

Embedding the specs reduces reuse, and like @dibyom is saying, the transition to wanting to extract the specs could be a bit rough

@pritidesai
Copy link
Member Author

Hey @bobcatfish and @dibyom, this was emerged as part of the PR discussion #1554

#1554 (comment)

@afrittoli
Copy link
Member

IMO if I decide to embed it's because it's something that I don't plan on reusing, or perhaps like @bobcatfish said something small or completely generated; in either case I feel that being forced to use parameters in tasks makes the yaml more verbose, harder to read and ultimately it gives up part of the advantage of embedding.

@pritidesai
Copy link
Member Author

yup agree with @bobcatfish, @dibyom, and @afrittoli, YAML becomes more verbose and harder to follow as the number of parameters grows and will be ideal to support for auto generated YAML but I guess generating a simplified YAML with different usage of params might create more confusion 😕

@bobcatfish
Copy link
Collaborator

I guess if it's being embedded anyway we might as well let ppl use the pipeline params :(

@tekton-robot
Copy link
Collaborator

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot
Copy link
Collaborator

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Aug 13, 2020
@tekton-robot
Copy link
Collaborator

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@bobcatfish
Copy link
Collaborator

The request here makes sense so I'm inclined to reopen, however it seems like no one has needed it since this was originally requested?

@VannTen
Copy link

VannTen commented Nov 18, 2022

For the record, I recently hit this. I was assuming to be able of doing this (Use a pipeline params directly inside an embedded taskSpec).

VannTen added a commit to VannTen/meteor-operator that referenced this issue Nov 18, 2022
We can't directly use the Pipeline parameters
See tektoncd/pipeline#1879
VannTen added a commit to VannTen/meteor-operator that referenced this issue Nov 18, 2022
We can't directly use the Pipeline parameters
See tektoncd/pipeline#1879
@dibyom
Copy link
Member

dibyom commented Nov 18, 2022

@VannTen - we have support for propagated parameters now which should enable this: https:/tektoncd/pipeline/blob/main/docs/taskruns.md#propagated-parameters

VannTen added a commit to VannTen/meteor-operator that referenced this issue Nov 21, 2022
We can't directly use the Pipeline parameters
See tektoncd/pipeline#1879
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/question Issues or PRs that are questions around the project or a particular feature lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

6 participants