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

Allow for TaskReference Interpolation within Pipeline #1367

Closed
vtereso opened this issue Sep 30, 2019 · 14 comments
Closed

Allow for TaskReference Interpolation within Pipeline #1367

vtereso opened this issue Sep 30, 2019 · 14 comments
Labels
area/api Indicates an issue or PR that deals with the API. kind/feature Categorizes issue or PR as related to a new feature.

Comments

@vtereso
Copy link

vtereso commented Sep 30, 2019

Expected Behavior

Without PipelineResources (google doc) Pipeline duplication happens as a consequence because there is no way to interchange the injection steps. In order to alleviate this shortcoming, taskRef injection will enable arbitrary Task to be similarly injected into the Pipeline.

Actual Behavior

Pipelines do not currently support this.

@vdemeester
Copy link
Member

/kind feature
/area api

@tekton-robot tekton-robot added kind/feature Categorizes issue or PR as related to a new feature. area/api Indicates an issue or PR that deals with the API. labels Oct 1, 2019
@bobcatfish
Copy link
Collaborator

taskRef injection will enable arbitrary Task to be similarly injected into the Pipeline.

@vtereso can you elaborate a bit more on what taskRef injection would look like, maybe with an example?

@vtereso
Copy link
Author

vtereso commented Oct 1, 2019

apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: echo1
spec:
  steps:
  - name: comment
    image: docker.io/bash
    args: ["-c", 'echo 1']
---
apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: echo2
spec:
  steps:
  - name: comment
    image: docker.io/bash
    args: ["-c", 'echo 2']
---
apiVersion: tekton.dev/v1alpha1
kind: Pipeline
metadata:
  name: echo-pipeline
spec:
  params:
  - name: taskRef
    type: string
  tasks:
  - name: echo
    taskRef: 
      name: $(params.taskRef)
---
apiVersion: tekton.dev/v1alpha1
kind: PipelineRun
metadata:
  name: echo-runner
spec:
  params:
  - name: taskRef
    value: echo1
  pipelineRef:
    name: echo-pipeline

@vtereso
Copy link
Author

vtereso commented Oct 1, 2019

With this change, we would likely have strong enforcement where the taskRef must be a valid Kubernetes name after interpolation (the task must exist). Alternatively, an empty taskRef could be ignored when creating the TaskRun. In either situation, what this allows for in injection anywhere within the definition, where PipelineResources only allow for prepending/appending steps. However, one thing to note here is the parameters would need to lineup between interchanged Task should they not have defaults/take parameters.

@bobcatfish
Copy link
Collaborator

Ah interesting, so the name of the Task could be interpolated! I think I could get behind that but I'm not quite sure about how well it addresses the use case you described:

Without PipelineResources (google doc) Pipeline duplication happens as a consequence because there is no way to interchange the injection steps

The way that PipelineResources work right now, the logic being used to fetch your data is pretty fixed - if you use a git PipelineResource, you always get the same containers injected. I guess that storage is a bit different, b/c you can use GCS or GCS Build (or soon VolumeResource). But anyway I'm not sure you need the Task to be dynamic to get parity with how PipelineResources work today, it seems to me like having a git fetching Task and using that in the Pipeline gives you the same amount of potential duplication as using a git PipelineResource in a Pipeline?

@vtereso
Copy link
Author

vtereso commented Oct 3, 2019

This creates a similar contract in terms of functionality, but rather than being at the Task level, it happens at the Pipeline level. For example, a Task can be interpolated to support both mercurial and git repository types, where the rest of the Pipeline would be identical. Without interpolated Tasks, this would create two distinct Pipeline. The combinatorics of not being able to interchange these Tasks is what creates duplication. The comparison to PipelineResources stems from their future plans (as I understand them) to support a sort of interface/inheritance model where an input something like a VCSResource that would have concrete handlers for git and the like as aforementioned.

@vtereso
Copy link
Author

vtereso commented Oct 3, 2019

With the standard Clone->Build->Push Pipeline, the build Task will be different per language. Instead of having to maintain many different Pipeline, interpolating each language build Task consolidates this. This compliments Pipelines whether they use PipelineResources or not.

@ghost
Copy link

ghost commented Oct 3, 2019

We might want a slightly more complicated example in order to interrogate this idea a bit more. Here's a mockup of what a git/hg/svn checkout pipeline might look like:

kind: Task
apiVersion: tekton.dev/v1alpha1
metadata:
  name: git-clone
spec:
  inputs:
    params:
      - name: repoURL
        type: string
        description: The repo to clone
      - name: outputDirectory
        type: string
        description: The directory to clone into, relative to the volume mount
        default: "git-clone"
      - name: volumeName
        type: string
        description: The name of the volume to mount for the clone to populate
  steps:
    - name: "clone-repo"
      image: "alpine/git"
      args: ["clone", "$(inputs.params.repoURL)", "/ws/$(inputs.params.outputDirectory)"]
      volumeMounts:
        - name: "$(inputs.params.volumeName)"
          mountPath: "/ws"
---
kind: Task
apiVersion: tekton.dev/v1alpha1
metadata:
  name: cat-file
spec:
  inputs:
    params:
      - name: file
        type: string
        description: The file path to cat, relative to the mounted volume
      - name: volumeName
        type: string
        description: The name of the volume to mount that contains the file to cat
  steps:
    - image: "alpine:3.10"
      command: ["sh"]
      args: ["-c", "echo catting file... && cat /ws/$(inputs.params.file)"]
      volumeMounts:
        - name: "$(inputs.params.volumeName)"
          mountPath: "/ws"
---
kind: Pipeline
apiVersion: tekton.dev/v1alpha1
metadata:
  name: clone-and-cat
spec:
  params:
    - name: cloneTask
      type: string
      description: The task name to use for cloning the repo
    - name: dir
      type: string
      description: Directory to clone in to
      default: /some/where/or/other
    - name: repo
      type: string
      description: Repository to clone the thing from
    - name: catFilePath
      type: string
      description: File to cat after clone
    - name: volumeName
      type: string
      description: The volume to share between tasks
  tasks:
    - name: clone-time-yeah
      taskRef:
        name: "$(params.cloneTask)"
      params:
        - name: outputDirectory
          value: "$(params.dir)"
        - name: repoURL
          value: "$(params.repo)"
        - name: volumeName
          value: "$(params.volumeName)"
    - name: cat-time-yeah
      runAfter:
       - clone-time-yeah
      taskRef:
        name: cat-file
      params:
        - name: file
          value: "$(params.catFilePath)"
        - name: volumeName
          value: "$(params.volumeName)"

And the PipelineRun to execute it:

kind: PersistentVolumeClaim
apiVersion: v1
metadata:
  name: scratch
spec:
  accessModes:
    - ReadWriteOnce
  resources:
    requests:
      storage: 8Gi
---
apiVersion: tekton.dev/v1alpha1
kind: PipelineRun
metadata:
  generateName: clone-and-cat-
spec:
  params:
  - name: cloneTask
    value: "git-clone"
  - name: volumeName
    value: "ws"
  - name: repo
    value: "https://github.com/tektoncd/pipeline.git"
  - name: dir
    value: "src/"
  - name: catFilePath
    value: "src/README.md"
  pipelineRef:
    name: clone-and-cat
  podTemplate:
    volumes:
    - name: ws
      persistentVolumeClaim:
        claimName: scratch

This feels like it's again a question of early vs late validation. With a parameterised taskRef it's essentially impossible to perform any kind of early check that the task exists, is shaped how we expect, that the user has typed the parameter names with the right spelling, yada yada yada

@ghost
Copy link

ghost commented Oct 3, 2019

Oh I should've included another task in the pipeline and a volume mount to demonstrate sharing between tasks.

Done.

@ghost
Copy link

ghost commented Oct 4, 2019

I've updated the git clone example above to a mostly-working example (spent a couple hours getting it working locally minus the taskRef interpolation to make sure that the volume stuff was right).

The example clones a repo and cats the README file. It took me a long time to get everything lined up right with the volume configs.

Also, fun note, you can't name a volume "workspace" right now because we silently mount a workspace volume in the controller. Spent too long figuring that out.

@bobcatfish
Copy link
Collaborator

Without interpolated Tasks, this would create two distinct Pipeline. The combinatorics of not being able to interchange these Tasks is what creates duplication. The comparison to PipelineResources stems from their future plans (as I understand them) to support a sort of interface/inheritance model where an input something like a VCSResource that would have concrete handlers for git and the like as aforementioned.

Ah okay, I see what you mean. I do think that once we have better PipelineResources - and #1285 that we'll be able to meet your use case nicely tho. Maybe we can revisit this issue once #1285 is done or at least POC?

@bobcatfish
Copy link
Collaborator

Now we have workspaces which allow you to separate out how you get the files from what you do to the files.

I'm wondering if we can close this issue, reopen if it is requested again?

@vdemeester
Copy link
Member

/close

@tekton-robot
Copy link
Collaborator

@vdemeester: Closing this issue.

In response to this:

/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
Labels
area/api Indicates an issue or PR that deals with the API. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants