Extract Pre/Post Steps from PipelineResources 2 design into Tasks

[ghost]

Words

Pipeline Resources introduced the idea of Pre / Post Steps that could be injected into a Task to make it perform specific work before the main body of the Task runs. Examples: The Git PipelineResource type injects a git clone step before a Task's steps and the Pull Request resource type updates a pull request with modifications by injecting steps after a Task's steps.

This behaviour of injecting steps could be more broadly useful than in just pipeline resources and in fact might end up being much less opaque if made generally available in Tasks.

So this issue is about exploring a design for Pre/Post steps and "wrapping" one task with another. We've thrown around a few different terms for this (wrapping, yielding, "higher order tasks", task slots, inheriting, hooks, SuperTasks etc etc etc) and there are existing issues that walk this path as well. I'll link to those below. But the outcome of this issue should ideally be a design doc that gets some amount of community approval and gives us enough scope to expand the idea further if we decide its a route we want to continue down on.

Related issues:

• Add Hooks into Pipeline Lifecycle
• Abstract task and nested tasks

[pierretasci]

Just to play devil's advocate here, what do we get from injecting Pre/Post steps that you couldn't do just by editing the set of steps of the task definition? In practice, it will lead to the same outcome right?

[ghost]

In isolation a user with just a single task or very small set of tasks / pipelines probably won't benefit from this wrapping behaviour.

I think the benefits will manifest more when managing a portfolio of teams and their tasks:

1. You might have many projects that have their own build processes but each will ultimately be reusing the same git clone wrapper to get the source into the Task for build steps to operate on.
2. You want to reuse as much from the community as possible - git wrappers, pull request wrappers, etc etc
3. You want to provide wrappers that perform some kind of regulatory or security validations on inputs / outputs of tasks. You probably don't want to rely on your individual software teams to copy/paste those steps into their own tasks.
4. There are tricky configuration steps required to interact with certain pieces of your company's infrastructure and so you can provide a wrapper task that handles the boilerplate by injecting some steps.

These are some of the benefits that already manifest when using pipeline resources. The problem (or one of the problems...) with pipeline resources is how opaque they can seem. Wrapping tasks would fix that issue by making the wrappers legible like any other tekton resource.

[ghost]

I guess the technical benefit is that using a wrapper task will allow that wrapper to put stuff on the pod's disk for the other steps in the wrapped task to access. No need for PVCs or what have you to share data from the wrapper to the wrapped.

[pierretasci]

I think I understand the intention better now. It is less that a Task will specify it's own wrapper but rather in defining a pipeline, you can wrap a task with another task while still sharing the workspace. That way, a task could be shareable and independently, so could the wrapper, do I have that about right?

I wonder how this would work if multiple wrappers are needed? What is the order they are executed in?

I also wonder how the intention of the inputs and outputs of a task could be enforced. Say the task is running a build from a git repo which the wrapper task fetches and sets up. Doesn't that strictly tie the task itself to the "git pull" wrapper task? It wouldn't be able to properly function without it. In which case, you don't really gain anything by making it a wrapper. It might as well be an independent task that runs before the build task and emits the repo as an input to the build task.

[ghost]

That way, a task could be shareable and independently, so could the wrapper, do I have that about right?

Yep, nailed it.

I wonder how this would work if multiple wrappers are needed? What is the order they are executed in?

I've been thinking of them as being layered like an onion. So given TaskA is wrapped by Task1, Task2, Task3, I expect the step ordering to be as follows:

• Task1.PreSteps
• Task2.PreSteps
• Task3.PreSteps
• TaskA Steps
• Task3.PostSteps
• Task2.PostSteps
• Task1.PostSteps

Say the task is running a build from a git repo which the wrapper task fetches and sets up. Doesn't that strictly tie the task itself to the "git pull" wrapper task?

I don't think it would. Here's a snippet of how this might look:

kind: Task
metadata:
  name: build-from-source
spec:
  params:
    - name: directory
  steps:
    - name: build
      script: |
        cd $(params.directory)
        npm install .
        npm run build
---
kind: TaskWrapper
metadata:
  name: git-clone
spec:
  params:
    - name: repo
    - name: directory
  preSteps:
    - name: get-repo
      git clone $(params.repo) $(params.directory)
---
kind: TaskRun
spec:
  taskRef:
    name: build-from-source
  inputs:
    params:
      - name: directory
        value: /workspace/source
  wrappedBy:
    - taskRef:
        name: git-clone
      inputs:
        params:
          - name: repo
            value: https://github.com/tektoncd/pipeline.git
          - name: directory
            value: /workspace/source

Phew, ok, that's a lot of yaml for such a trivial example but here we can see the wrapper task (git-clone) is configured to checkout a repo to a location on disk. The build task is configured to process source code from a location on disk. Because Tekton automatically shares the /workspace directory between steps the code checkout is available to the build task. The build task has zero knowledge of git or where that /workspace/source directory came from.

For trivial examples like this it seems like overkill. But when you have many pipelines in an organization that can each share the git-clone task I think the utility increases a lot.

[startxfr]

Thanks @sbwsg for your example, I now completly understand the utility of preSteps ans postSteps steps and how it's completly related to Task wrapping, inheriting or whatever name we call it ;).

Is there any negative impact if we turn the TaskWrapper kind you suggest into a regular Task resource ? Why introducing this new kind and restrict these declaration to be wrapped ?

I copy an adaptation of my previous sample witch is close to @sbwsg example and comply with the description given by @pierretasci.

Example without pre and post steps

Primary and wrapped task example without pre or post steps

apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: primary-task
spec:
  params:
      - name: myParam
        type: string
        description: Default param from primary task
        default: my primary string from primary task
      - name: myPrimaryParam
        type: string
        description: Default param from primary task
        default: my string from primary task
      - name: myPrimary2Param
        type: string
        description: Default param from primary2 task
        default: my string from primary2 task
  steps:
    - name: "exec-primary-task"
      image: gcr.io/example-builders/build-example
      command: ["echo"]
      args: 
        - "primary task - ",
          "myParam : ",
          "$(inputs.params.myParam)",
          "myPrimaryParam : ",
          "$(inputs.params.myPrimaryParam)"
          "myPrimaryParam2 : ",
          "$(inputs.params.myPrimary2Param)"
---
apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: wrapped-task
spec:
  params:
      - name: myParam
        type: string
        description: Default param from wrapped task
        default: my param string from wrapped task
      - name: myWrappedParam
        type: string
        description: Default param from wrapped task
        default: my string from wrapped task
  steps: 
    - name: "call-primary-task"
      taskRef:
        name: primary-task
      inputs:
         params:
           - name: myParam
             value: "$(inputs.params.myParam)"
           - name: myPrimary2Param
             value: "$(inputs.params.myWrappedParam)"

Taskrun example without pre or post steps

kind: TaskRun
spec:
  taskRef:
    name: task-with-primary-run
  inputs:
    params:
    - name: myParam
      type: string
      description: Default param from taskrun task
      default: my param string from taskrun task
  steps:
    - name: primary-task
      taskRef:
        name: primary-task
      params:
        - name: myParam
          value: "$(params.myParam)"
    - name: wrapped-task
      taskRef:
        name: wrapped-task
      params:
        - name: myWrappedParam
          value: "$(params.myParam)"

Pipeline example without pre or post steps

apiVersion: tekton.dev/v1alpha1
kind: Pipeline
metadata:
  name: pipeline-with-wrapped-and-primary
spec:
  params:
    - name: myParam
      type: string
      description: Default param from pipeline task
      default: my param string from pipeline task
  tasks:
    - name: primary-task
      taskRef:
        name: primary-task
      params:
        - name: myParam
          value: "$(params.myParam)"
    - name: wrapped-task
      taskRef:
        name: wrapped-task
      params:
        - name: myWrappedParam
          value: "$(params.myParam)"

The coresponding sequence (taskRun or Pipeline) will be :

• primary-task
  • exec-primary-task
• wrapped-task
  • call-primary-task
    • exec-primary-task

Example with pre and post steps

Primary and wrapped task example with pre and post steps

apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: primary-task
spec:
  params:
      - name: myParam
        type: string
        description: Default param from primary task
        default: my primary string from primary task
      - name: myPrimaryParam
        type: string
        description: Default param from primary task
        default: my string from primary task
      - name: myPrimary2Param
        type: string
        description: Default param from primary2 task
        default: my string from primary2 task
  preSteps:
    - name: "...exec-primary-prestep-task..."
  steps:
    - name: "exec-primary-task"
      image: gcr.io/example-builders/build-example
      command: ["echo"]
      args: 
        - "primary task - ",
          "myParam : ",
          "$(inputs.params.myParam)",
          "myPrimaryParam : ",
          "$(inputs.params.myPrimaryParam)"
          "myPrimaryParam2 : ",
          "$(inputs.params.myPrimary2Param)"
  postSteps:
    - name: "...exec-primary-postep-task..."
---
apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: wrapped-task
spec:
  params:
      - name: myParam
        type: string
        description: Default param from wrapped task
        default: my param string from wrapped task
      - name: myWrappedParam
        type: string
        description: Default param from wrapped task
        default: my string from wrapped task
  preSteps:
      - name: "...exec-wrapped-prestep-task..."
  steps: 
      - name: "call-primary-task"
      taskRef:
        name: primary-task
      inputs:
         params:
           - name: myParam
             value: "$(inputs.params.myParam)"
           - name: myPrimary2Param
             value: "$(inputs.params.myWrappedParam)"
  postSteps:
      - name: "...exec-wrapped-poststep-task..."

Taskrun example with pre and post steps

kind: TaskRun
spec:
  taskRef:
    name: task-with-primary-run
  inputs:
    params:
    - name: myParam
      type: string
      description: Default param from taskrun task
      default: my param string from taskrun task
  steps:
    - name: primary-task
      taskRef:
        name: primary-task
      params:
        - name: myParam
          value: "$(params.myParam)"
    - name: wrapped-task
      taskRef:
        name: wrapped-task
      params:
        - name: myWrappedParam
          value: "$(params.myParam)"

Pipeline example with pre and post steps

apiVersion: tekton.dev/v1alpha1
kind: Pipeline
metadata:
  name: pipeline-with-wrapped-and-primary
spec:
  params:
    - name: myParam
      type: string
      description: Default param from pipeline task
      default: my param string from pipeline task
  tasks:
    - name: primary-task
      taskRef:
        name: primary-task
      params:
        - name: myParam
          value: "$(params.myParam)"
    - name: wrapped-task
      taskRef:
        name: wrapped-task
      params:
        - name: myWrappedParam
          value: "$(params.myParam)"

The coresponding sequence (taskRun or Pipeline) will be :

• exec-primary-prestep-task
• exec-wrapped-prestep-task
• primary-task
  • exec-primary-task
• wrapped-task
  • call-primary-task
    • exec-primary-task
• exec-wrapped-poststep-task
• exec-primary-poststep-task

[ghost]

Is there any negative impact if we turn the TaskWrapper kind you suggest into a regular Task resource ? Why introducing this new kind and restrict these declaration to be wrapped ?

I like this idea - allow Task to wrap another Task. No new types needed. I will give it some thought and see if I can think of any problems with it.

[afrittoli]

Nice!
A few questions: how do pre/post step affect Task:

• status: do we track that a Task is in "running pre/post steps" status?
• lifecycle: overall passed/failed status, what happens in case of of failure during pre/post step
• metrics (e.g. task duration)

[ghost]

status: do we track that a Task is in "running pre/post steps" status?

I imagined them as being all merged into the step list of the wrapped task. Similarly to how pipeline resources will inject steps into a task now. So the status of a failed pre-step is treated the same as a failed step. We could add a field to steps which says something like, "I was injected by FooTask" to help diagnose errors.

lifecycle: overall passed/failed status, what happens in case of of failure during pre/post step

As above, I had been thinking of these as simply injected to the wrapped task. The failure in a pre or post step would manifest as failure of the wrapped task.

metrics (e.g. task duration)

This is interesting and not something I'd thought about. We'd likely want to be able to find out when the pre or post steps are causing long delays in execution. I'll give this some more thought, thanks!

[pierretasci]

I could imagine the shared workspaces feature could make task wrappers redundant. Instead of wrapping tasks and worrying about order, and failures, et al, a well-defined task that modifies a workspace which is also attached to a downstream task has the same effect and doesn't fundamentally change the abstraction. Would probably keep things much simpler. What do you think @sbwsg?

[startxfr]

your right @pierretasci , workspaces could help pass informations and states from one task to another and share the work done by them. I also agree that adding wrapping capacity to task could lead to much complex, spaggetti, task or pipeline if not properly used.

On the other hands, I think, it could also add more flexibility when we build and organize our tasks library and design our pipelines with multiples common tasks. For exemple, wrapping could help us building shared task at the company level (abstract), and use them in task defined at the project or service level (instanciable).
Small or simple project will not require it, but more complex or intricated project could be simplified by using this kind of feature.

@sbwsg explain things better than me. I hope he will enlight you with his answer like he have done with his explanation and example on pre and post steps.

[pierretasci]

I really appreciate the desire for shareable "work" that can be distributed. At the same time, there already is an abstraction for sharing a series of repeatable steps and that is a task. I think the compelling case for pre/post steps was directly modifying the task's workspace but the workspace sharing accomplishes the same thing.

Maybe it would help to go through an example? Say I want to make a single git clone task for my entire company to share and I want to use that as input to a compile task.

With pre/post

kind: Task
metadata:
  name: build-from-source
spec:
  params:
    - name: directory
  steps:
    - name: build
      script: |
        cd $(params.directory)
        npm install .
        npm run build
---
kind: TaskWrapper
metadata:
  name: git-clone
spec:
  params:
    - name: repo
    - name: directory
  preSteps:
    - name: get-repo
      git clone $(params.repo) $(params.directory)
---
kind: TaskRun
spec:
  taskRef:
    name: build-from-source
  inputs:
    params:
      - name: directory
        value: /workspace/source
  wrappedBy:
    - taskRef:
        name: git-clone
      inputs:
        params:
          - name: repo
            value: https://github.com/tektoncd/pipeline.git
          - name: directory
            value: /workspace/source

With Workspaces

kind: Task
metadata:
  name: build-from-source
spec:
  workspaces:
    - name: repo
       description: Folder containing the repository to build
       mountPath: /workspace/build-repo
  steps:
    - name: build
      script: |
        cd /workspace/build-repo
        npm install .
        npm run build
---
kind: Task
metadata:
  name: fetch-repo
spec:
  params:
    - name: repo
  workspaces:
    - name: repo
       description: Folder to place the repo into
       mountPath: /workspace/build-repo
  steps:
    - name: fetch
      script: |
        git clone ${params.repo} /workspace/build-repo
---
kind: Pipeline
spec:
  workspaces:
    - name: build-dir
  tasks:
    - name: fetch
       taskRef:
         name: fetch-repo
       resources:
         inputs: 
           - name: repo
              value: github.com/foo/bar 
       workspaces: 
         - name: repo
            workspace: build-dir
    - name: build-from-source
      taskRef:
        name: build-from-source
      workspaces: 
         - name: repo
            workspace: build-dir 

These two options look very similar. I would make the case though that the latter is just as shareable
by any client with a clearly defined contract (give me a workspace, and the git url) and it can be inserted anywhere it is needed in a pipeline. It also has the subtle benefit of being able to share the output (if I wanted to pull the repo once and have multiple tasks do work on it in parallel).

Don't get me wrong, I am not trying to derail this discussion 😄I am just thinking of ways to accomplish the same thing for less.

[startxfr]

thanks @pierretasci and your solution is the best actually and will still remain the best choice for a long time (unless workspaces disapear ;).

Take a look at the s2i wrapped sample task I've done on #1796

It's a task that wrap a tekton-catalog task (s2i). But more than wrapping, this task also aimed to change the input parameters and enforce using some global shared configuration (repo,logs,tlsverify)

apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: my-corp-s2i
spec:
      - name: IMAGE_NAME
        type: string
        description: The name of the s2i builder image.
        default: nodejs
      - name: IMAGE_TAG
        type: string
        description: The s2i builder image tag to use
        default: latest
     - name: PATH_CONTEXT
       description: The location of the path to run s2i from.
       default: .
  steps: 
      - name: "my-corp-s2i"
        taskRef:
          name: s2i <-- from tkn catalog
        inputs:
           params:
             - name: BUILDER_IMAGE
               value: "quay.io/startx/$(inputs.params.IMAGE_NAME):$(inputs.params.IMAGE_TAG)"
             - name: PATH_CONTEXT
               value: "$(inputs.params.PATH_CONTEXT)"
             - name: TLSVERIFY
               value: "false"
             - name: LOGLEVEL
               value: "2"
---
apiVersion: tekton.dev/v1alpha1
kind: Pipeline
metadata:
  name: my-nodejs-pipeline
spec:
  tasks:
    - name: build-nodejs
      taskRef:
        name: my-corp-s2i
      params:
        - name: IMAGE_NAME
          value: "nodejs"
    - name: build-php
      taskRef:
        name: my-corp-s2i
      params:
        - name: IMAGE_NAME
          value: "php"

With the wrapping feature, we can have a "corporate" task (my-corp-s2i) shared by multiples tasks in multiples pipelines with no code duplication or redundant variation of the same task.

But as I've previously said, I'm a tekton noobs (but enought old to still use the noobs term ;) and using it on my playtime. I've probably missed an way to face this challenge with the actual availables tools, but if there is one, I'll be more than happy to read it.

[ghost]

I really appreciate the desire for shareable "work" that can be distributed. At the same time, there already is an abstraction for sharing a series of repeatable steps and that is a task. I think the compelling case for pre/post steps was directly modifying the task's workspace but the workspace sharing accomplishes the same thing.

Thought a bit about this over the weekend. I think you're right that everything which can be accomplished via pre/post could also be accomplished through shared workspaces.

So to me the question then becomes: do we always want to push users to design their tasks and pipelines around sharing pvcs through workspaces?

There are trivial cases where I think the answer is no. Examples like: very simple tasks for illustrative or tutorial purposes - showing how to use git to fetch a repo so you can do some work on it would also require an intro to workspaces. We can safely ignore these because it's not an insurmountable hurdle for learners.

But I think there might be cases where the workspaces/pvc combo aren't great for real-world problems. An example of this might be the existing cluster resource type. All it really does is write the credentials needed to use kubectl to talk to a cluster - it effectively just writes out ~/.kube. This is a trivial action to perform but very useful to encapsulate creds initialization. Do we want to push users to mount a workspace to transfer these creds from the "cluster-init" task to the "kubectl-apply" task? Seems both like a considerable delay to introduce (pvcs take time to move around between pods) as well as an extra cognitive burden to put on the user.

I'm still somewhat torn on this. I think there are usability / compositional benefits to the pre/post step idea that go beyond pure implementation. I might move ahead with a design doc regardless and gauge what kind of reaction it gets in the community. WDYT @pierretasci and @startxfr ?

[pierretasci]

To extend your example of the cluster resource, I would imagine a "cluster-init" task would be the atomic, shareable unit. To me, the scope of the work that a cluster-init task performs is well defined and the output is too. Requiring any task that wants to use it to explicitly mount this pvc makes the task definition declarative. "I need a kubecfg file as input".

That said, you make a good point about pvc mount delay especially if a task takes 10 input workspaces. Pre/post tasks introduce a delay as well since the "workspace" will need to be offloaded somewhere that the pre-step can then load onto the workspace of the task.

I think proposing a design to flush out all of the approaches and discuss the benefits is worthwhile.

[ghost]

After a bit more thought I'm going to hold off on designing this right now. As we move to getting ready for beta I think I need to focus on tasks and documentation to help users fill the gap that pipeline resources are leaving by remaining alpha. Workspaces and task results should give us enough to paper over that gap in the short term 🤞

[bioball]

Per our Slack convo from earlier: another possible use-case for this is to be able to re-use a task with different sidecar definitions.

In my use-case, I have two task definitions that look pretty much the same, but the only difference is that one of the tasks uses sidecars, and another one does not. Here's a pretty contrived example:

---
apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: test-with-gradle-and-redis
spec:
  sidecars:
    - name: redis
      image: redis
      command: ['./start.sh']
  steps:
    - image: adoptopenjdk
      script: ./gradlew check
    - image: adoptopenjdk
      script: ./gradlew package
---
apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: test-with-gradle
spec:
  steps:
    - image: adoptopenjdk
      script: ./gradlew check
    - image: adoptopenjdk
      script: ./gradlew package

[ghost]

Another use-case came up on Slack today. A user is operating their Tasks in a cluster without the option to use storage (iow no PVCs). They've previously been using the Storage Resource as an output to upload results of a Task to a bucket. With PipelineResources remaining alpha they're now a bit stuck - wanting to migrate away from resources but unable to shuttle data between Tasks because Workspaces require PVCs to do so.

A GCS Task with PostRun steps would help here - they could "wrap" their Task with the GCS Task and this would work exactly the same as the Storage Resource does today.

[bobcatfish]

I think that task specialization and the currently post appealing options (all part of #1673) might actually provide this so I'm going to reopen this and assign it to myself.

/reopen
/assign
/remove-lifecycle rotten

[tekton-robot]

@bobcatfish: Reopened this issue.

In response to this:

I think that task specialization and the currently post appealing options (all part of #1673) might actually provide this so I'm going to reopen this and assign it to myself.

/reopen
/assign
/remove-lifecycle rotten

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.

[tekton-robot]

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.

[vdemeester]

/remove-lifecycle stale

[bobcatfish]

@jerop is planning to look into this

/assign jerop

[bobcatfish]

TEP that might go in this direction: tektoncd/community#316

[tekton-robot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
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 with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

[tekton-robot]

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

/lifecycle rotten

Send feedback to tektoncd/plumbing.

[ghost]

Yeah, I think this issue has been superseded by TEP-0044 at this point. https://github.com/tektoncd/community/blob/main/teps/0044-decouple-task-composition-from-scheduling.md

/close

[tekton-robot]

@sbwsg: Closing this issue.

In response to this:

Yeah, I think this issue has been superseded by TEP-0044 at this point. https://github.com/tektoncd/community/blob/main/teps/0044-decouple-task-composition-from-scheduling.md

/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.