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 Task sidecars to be specified in Pipeline/PipelineRun #4235

Closed
michaelsauter opened this issue Sep 16, 2021 · 24 comments
Closed

Allow Task sidecars to be specified in Pipeline/PipelineRun #4235

michaelsauter opened this issue Sep 16, 2021 · 24 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@michaelsauter
Copy link

michaelsauter commented Sep 16, 2021

Feature request

At the moment, sidecars are defined as part of a Task. I don't see a way to add a sidecar to a task from the Pipeline/PipelineRun definition. However, I think this would be really helpful.

Use case

Our use case is this: we (an enterprise platform) want to provide a set of Tekton tasks (https://github.com/opendevstack/ods-pipeline) in the cluster for our users to consume. The goal is to provide a streamlined way to build & deploy applications. One challenge is to hit a good balance between a common standard across the platform and enough flexibility to not be a pain for consumers. If sidecars could be specified from a pipeline definition, it would help with this balance.

As an example, we have a task that builds Go applications. This task also runs linters, checks format and executes the test suite. Sometimes, those tests will want to use some dependencies (e.g. a database or external service) to test proper integration. If the task definition is the only place to specify sidecars, this doesn't really work as the task authors (the platform) cannot know which sidecars are needed. The ideal place to specify which sidecar to use (image, environment variables, resource constraints, ...) is the pipeline(run), which is authored by the consumers, not the task.

Within the current design, the only option seems to be to create "sidecar variants" for our tasks, and allow to exchange the image via parameter (as implemented in opendevstack/ods-pipeline#210). However that has serious limitations as e.g. env variables cannot be changed, resource constraints cannot be set and one is limited to one sidecar only.

Additional Information

@michaelsauter michaelsauter added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 16, 2021
@michaelsauter
Copy link
Author

ping

Any comment on this? Or is this the wrong place to suggest such a feature? 😄

@michaelsauter
Copy link
Author

@vdemeester @bobcatfish maybe you can comment on this as you discussed on the linked issue #2973? I would love to get some idea on whether this is something you would consider contributions for, or whether this is impossible, or .... anything really :)

@michaelsauter
Copy link
Author

Alright, one more attempt :) Any comment on this? Let me know if you need more info, or if I should bring this up in another place.

@vdemeester
Copy link
Member

@michaelsauter sorry 😓
So, as of today, your use case could be achieve by an early "in parallel" Task that provision the dependency you need, and a finally task that assure to bring it down, no matter the results, am I right ? Although, you might also want the option to be able to "parametrize" the task you run (in order to bring up and down a different dependency, depending on the paramater, right ?)

@afrittoli
Copy link
Member

@michaelsauter sorry 😓 So, as of today, your use case could be achieve by an early "in parallel" Task that provision the dependency you need, and a finally task that assure to bring it down, no matter the results, am I right ? Although, you might also want the option to be able to "parametrize" the task you run (in order to bring up and down a different dependency, depending on the paramater, right ?)

As per @michaelsauter comment:

Note this is different from Pipeline level sidecar #2973, as that discusses sidecars that run alongside a pipeline. My request here does not change how sidecars work (their lifecycle), "just" where they are specified.

I think this is a different use case. Sidecars would only live for the duration of a Task, but they would be specified at Pipeline level, to move the decision about which sidecars are needed from Task authoring time to Pipeline authoring time.

In my mind this would look like:

spec:
  tasks:
    - name: first-task
      taskRef:
        name: a-task-without-sidecars
      sidecars:
        - image: busybox
          name: hello-sidecar
          script: |
            echo 'Hello from sidecar!'

@vdemeester
Copy link
Member

oh I see, thanks @afrittoli .. I didn't read it as such, but now I think I understand 😅

@michaelsauter
Copy link
Author

@vdemeester @afrittoli Thanks for picking this up!

And yes, the example you are giving @afrittoli is exactly what I have in mind with this feature idea.

@michaelsauter
Copy link
Author

We just hit this use case yesterday again. Any update on whether this is feasible and desired?

I'd be happy to take a stab at this but would need some pointers where to start :)

@nikhil-thomas
Copy link
Member

@michaelsauter May be a custom mutating webhook is what this case needs. For example, in tektoncd/operator we have a webhook which injects some environment variables into TaskRun Pods.

In short, a quick way to improve this situation might be to deploy the custom webhook beside the TektonPipelines installation on your cluster 🧑‍💻. To consider supporting such a webhook in the operator or in pipelines we will have to define the problem in a wider scope (eg: enduser specific TaskRun extensions 😇 )

@michaelsauter
Copy link
Author

@nikhil-thomas Thanks for this hint! This is indeed something we could explore.

However I believe our use case should not require a mutating webhook, nor are we asking for full-blown enduser specific TaskRun extensions. We simply want to allow specifying sidecars when authoring pipelines instead of only when authoring tasks. As stated before I think this is a crucial feature to make tasks reusable.

Since there seems to be no quick solution to this, we are planning to sidestep the issue altogether for now. We plan to get rid of cluster tasks which are centrally provided, and instead provide the tasks everyone should be using as a Helm chart via Git, to be installed in their own namespace. The Helm chart allows templating, which allows users to specify the sidecars they want to use while still largely authoring the tasks centrally for them. This solution also "fixes" the other issue linked above that pipeline authors cannot modify resource constraints (now they can via Helm templating).

Maybe this "workaround" helps someone else as well ...

I would still appreciate a "proper" solution within Tekton though!

@nikhil-thomas
Copy link
Member

cc @siamaksade

@tekton-robot
Copy link
Collaborator

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 tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 23, 2022
@tekton-robot
Copy link
Collaborator

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.

@tekton-robot tekton-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 24, 2022
@dibyom dibyom removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jun 22, 2022
@lbernick lbernick changed the title Allow sidecars to be specified in Pipeline/PipelineRun Allow Task sidecars to be specified in Pipeline/PipelineRun Jul 8, 2022
@tekton-robot
Copy link
Collaborator

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 tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 6, 2022
@michaelsauter
Copy link
Author

Any chance this can move forward as a feature request? The workaround isn't great.

@vdemeester
Copy link
Member

/remove-lifecycle stale

@tekton-robot tekton-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 10, 2022
@michaelsauter
Copy link
Author

With the availability of remote task resolution (via Git or ArtifactHub), this issue will become more pressing for us in the future.

So far our workaround is that each user defines the task in the Helm chart and can therefore add the sidecar at "Helm chart authoring time". We would like to make things easier for our users so that they do not need to maintain the task definitions in a Helm chart anymore, and can simply reference tasks from a central Git repo / Artefact Hub. However, then they loose the ability to define the sidecars as that feature is not available at "pipeline run authoring time".

Is this feature something on your radar?

@lbernick
Copy link
Member

I don't know of any plans to implement this feature at the moment, but it sounds like it would add a lot of value for your team. Would you consider proposing a TEP for this?

@michaelsauter
Copy link
Author

@lbernick Sure: tektoncd/community#877. Let me know how I can help to move this forward.

@lbernick
Copy link
Member

Thanks Michael! We will see if anyone is able to review the proposal at our regular monday working group. Other great ways to get eyes on the proposal are to post on the #tep channel on the tekton slack or highlight your proposal on the tekton-dev or tekton-users mailing list. This doc has a bit more detail on the TEP process.

@tekton-robot
Copy link
Collaborator

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 tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 9, 2023
@tekton-robot
Copy link
Collaborator

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.

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

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

@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 with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
Status: Done
Development

No branches or pull requests

7 participants