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

Should the compiler generate Pipeline or PipelineRun #59

Closed
3 of 4 tasks
ckadner opened this issue Mar 24, 2020 · 21 comments
Closed
3 of 4 tasks

Should the compiler generate Pipeline or PipelineRun #59

ckadner opened this issue Mar 24, 2020 · 21 comments

Comments

@ckadner
Copy link
Member

ckadner commented Mar 24, 2020

This is a general discussion, not a work item.

Conceptually a Pipeline is a static specification orchestrating how a set of Tasks should be executed. A PipelineRun is an instantiation of a pipeline. A pipeline is a re-usable artifact that can be "run" repeatedly, a pipeline-run is one-and-done.

Currently the KFP-Tekton compiler generates a YAML with (multiple) Tasks(s) and one Pipeline document. There are however certain features supported by the KFP DSL which are not easily transferred into an equivalent feature in Tekton Pipelines, but instead require the concept of a PipelineRun, like mounting PVC volume to a pipeline workspace or setting a timeout at the pipeline level.

Linked issues

@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the labels:

Label Probability
question 0.84

Please mark this comment with 👍 or 👎 to give our bot feedback!
Links: app homepage, dashboard and code for this bot.

@Tomcli
Copy link
Member

Tomcli commented Mar 24, 2020

For comparison, every workflow in Argo is like a PipelineRun. Therefore, when we upload a pipeline in kfp, it's treated as a template and stored in object storage instead of applying the template to Kubernetes.

@ckadner
Copy link
Member Author

ckadner commented Mar 24, 2020

For comparison, every workflow in Argo is like a PipelineRun. Therefore, when we upload a pipeline in kfp, it's treated as a template and stored in object storage instead of applying the template to Kubernetes.

True. But for Kubeflow Pipelines with Argo there is the KFP UI which allows users to interact with pipelines and provide parameters and see logs, etc. For Tekton we have to rely on kubectl and tkn. If we generate PipelineRuns then a user cannot provide parameters at run-time, rather parameters, volume mounts, PVC are locked in at compilation time.

@ckadner
Copy link
Member Author

ckadner commented Mar 24, 2020

@Tomcli
Copy link
Member

Tomcli commented Mar 24, 2020

tektoncd/cli#786

@ckadner
Copy link
Member Author

ckadner commented Mar 24, 2020

we could also introduce a --generate-pipelinerun flag for the dsl-compile-tekton CLI

@Tomcli
Copy link
Member

Tomcli commented Mar 24, 2020

I will add a flag for development purpose and we should push the cli team to support workspaces.

@fenglixa
Copy link
Member

Great, I think we should support generate-pipelinerun feature, it's convenient for user to run the whole workflow directly, but not just leave several templates there, also, pipelinerun has several fields which could be defined in there, including pipeline timeout.

@Tomcli
Copy link
Member

Tomcli commented Mar 25, 2020

generating pipelinerun might also be useful for adding pod-template such as node selector and affinity features https://github.com/tektoncd/pipeline/blob/master/docs/pipelineruns.md#pod-template

@fenglixa
Copy link
Member

Personally, I think we should generate tekton pipelinerun by default to align with argo workflow but not pipeline.
We may need to list pros and cons for each optionals and adopt one of them, convenient for further API and UI integration

@animeshsingh
Copy link
Collaborator

Given more preference towards pipelinerun, lets go with that

@vincent-pli
Copy link
Contributor

Pipelinerun with Pipeline definition embed:

apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  name: pipelinerun-with-taskspec-to-echo-good-morning
spec:
  pipelineSpec:
    tasks:
      - name: echo-good-morning
        taskSpec:
          steps:
            - name: echo
              image: ubuntu
              script: |
                #!/usr/bin/env bash
                echo "Good Morning!"

@ckadner
Copy link
Member Author

ckadner commented Mar 26, 2020

PipelineRun with Pipeline definition embed

I think @Tomcli 's approach (PR #62) with generating an optional PipelineRun in addition to the existing Pipeline with Tasks will be the best compromise and allows to keep existing code intact. At least for the time being.

@afrittoli
Copy link
Contributor

PipelineRun with Pipeline definition embed

I think @Tomcli 's approach (PR #62) with generating an optional PipelineRun in addition to the existing Pipeline with Tasks will be the best compromise and allows to keep existing code intact. At least for the time being.

Eventually you could have a flag that let you decide whether to embed the pipeline in the pipelinerun or not.

@afrittoli
Copy link
Contributor

Whether you want to use Pipeline or PipelineRun depends on what kind of experience you want to give to end-users.

Tekton separates static definitions from runtimes and it provides a component called “triggers” that allow you to create runtime objects with several presets. Today “triggers” only respond to HTTPs requests, but I can imagine a future were it will be possible to run “triggers” in different ways.
It seems to be that that separation between static definition and runtime is not so clear in the python sdk, so after reading all the discussion it feels like you may have to go with generating PipelineRuns. If you still need to support changing some of the parameters at run time, you could support that using kustomize overlay.

@ckadner
Copy link
Member Author

ckadner commented Mar 27, 2020

/kind discussion

@ckadner
Copy link
Member Author

ckadner commented Mar 27, 2020

/remove-kind question

@ckadner
Copy link
Member Author

ckadner commented Jun 5, 2020

Related issues:

@Udiknedormin
Copy link
Contributor

Shouldn't this be closed, considering issue #143 has been closed with PR #166?

@Tomcli
Copy link
Member

Tomcli commented Jun 26, 2020

Thanks @Udiknedormin
/close

@k8s-ci-robot
Copy link

@Tomcli: Closing this issue.

In response to this:

Thanks @Udiknedormin
/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants