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

[FR] Specifying compute resources on Pipeline Tasks #5110

Closed
lbernick opened this issue Jul 8, 2022 · 16 comments
Closed

[FR] Specifying compute resources on Pipeline Tasks #5110

lbernick opened this issue Jul 8, 2022 · 16 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

@lbernick
Copy link
Member

lbernick commented Jul 8, 2022

Feature request

Example:

kind: Pipeline
spec:
  tasks:
  - name: build
    taskRef:
      name: build-go
    resources:
      limits:
         cpu: 100m
         memory: 128Mi
      requests:
         cpu: 100m
         memory: 128Mi

Use case

Spin-off from #4080. In OpenDevStack, the user is responsible for configuring Pipeline Tasks, and the platform automatically injects some additional starting/finally tasks and creates PipelineRuns. From @michaelsauter "We have been thinking to simply create PipelineRuns instead of Pipelines from our config file, in order to move into the "runtime", but the downside of that is that e.g. grouping of runs in the UI is lost (we're using the OpenShift console)."

Design principles thoughts

Compute resources are definitely a "runtime" concept instead of an "authoring time" concept, but there's some precedent for including runtime concepts in PipelineTasks: e.g. timeout.

Implementation thoughts

One way to implement this feature would be to support parameterization of resource requirements in Tasks (the original request in #4080). This would allow a Task's resource requirements to be defined in a Pipeline. For example:

kind: Task
metadata:
  name: mytask
spec:
  params:
  - name: cpu-request
  - name: memory-request
     steps:
     - name: step-1
        resources:
          requests:
            cpu: $(params.cpu-request)
            memory: $(params.memory-request)
kind: Pipeline
spec:
  params:
  - name: cpu-request
     value: 100m
  - name: memory-request
     value: 200Mi
  tasks:
  - name: mytask
    taskRef:
      name: mytask
    params:
    - name: cpu-request
       value: $(params.cpu-request)
    - name: memory-request
       value: $(params.memory-request)

I don't think this is a good idea long term, because I think we should move away from a model where compute resources are specified on Tasks (they should be specified in TaskRuns and PipelineRuns instead, and Pipelines if we choose to implement this FR). However, parameterizing Task compute resource requirements could address the use case in the short term.

A different way to implement this feature would be to just add these fields to our API, as in the first example above. I personally feel this would be a better solution. If we go this route, it's probably a good idea to wait until #4470 is implemented and we have feedback on it.

@lbernick lbernick added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 8, 2022
@michaelsauter
Copy link

Thanks for opening this!

We were discussing this again internally, and we came out of that discussion one step closer to "let's switch to PipelineRuns". There are other things that can be done with PipelineRuns that can't be done with Pipelines, such as inline task specs. So we're wondering if we would just fight against a design choice if we continue to go with the pipeline approach.

On top, viewing the config file in the Git repo as the "pipeline definition" feels "right". We only really use the Pipeline CRD to have a grouping mechanism in the UI. Since most people probably navigate straight from the CI build status to the PipelineRun, it may not be so bad if we loose the grouping in the UI. Plus, one can still filter runs by label, which is more cumbersome, but still OK.

All in all, it looks like this is becoming less critical for us, but #4235 (for pipeline runs) is still very high on our "wish list" :)

@lbernick
Copy link
Member Author

Good to know! Just FYI-- I am wondering if our upcoming pipelines in pipelines feature will help with your use case? Your users could define Pipelines, and they could be used as sub-Pipelines within Pipelines you create.

@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 9, 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 Nov 8, 2022
@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.

Repository owner moved this from Todo to Done in Tekton Community Roadmap Dec 8, 2022
@BlueCog
Copy link

BlueCog commented Mar 23, 2023

Hey @lbernick your wish (parameterization of resource requirements in Tasks) seems still inpossible. Did you our your team have any solution and/or workaround for this in the meantime?

@lbernick
Copy link
Member Author

Compute resource requirements can't be parameterized, but they can be specified in TaskRuns at the task level or the step/sidecar level. They can also be specified in pipelinerun.spec.taskrunspecs.

@BlueCog
Copy link

BlueCog commented Mar 23, 2023

Compute resource requirements can't be parameterized, but they can be specified in TaskRuns at the task level or the step/sidecar level. They can also be specified in pipelinerun.spec.taskrunspecs.

Thanks for the quick reply! Yeah we doing that now. But i'd like to parameterize so that we could be more flexible. A trigger from Repo X needs a bigger resource (for a specific task) then a trigger from repo Y for example.

@lbernick
Copy link
Member Author

I'd like to parameterize so that we could be more flexible. A trigger from Repo X needs a bigger resource (for a specific task) then a trigger from repo Y for example.

Can you try this?

apiVersion: triggers.tekton.dev/v1beta1
kind: TriggerTemplate
metadata:
  name: foo
spec:
  params:
  - name: cpu
  resourcetemplates:
  - apiVersion: tekton.dev/v1beta1
    kind: TaskRun
    spec:
      computeResources:
        requests:
           cpu: $(tt.params.cpu)

A TriggerTemplate's resourceTemplates can be arbitrary strings and should get substituted with the values of the trigger template params (unlike a task.spec.step.computeresources, which disallows $(params.foo) syntax). (I was able to create this triggertemplate but haven't tried running it yet)

@BlueCog
Copy link

BlueCog commented Mar 27, 2023

I'll try to create an environment to test this!

@EoinMan
Copy link

EoinMan commented Aug 24, 2023

Sorry to be a pain

I know this is closed but I am looking at this now

triggertemplate that has a PipelineRun

I have spec.podTemplate for toleration's and affinity

and spec.taskRunSpecs.computeResources with requests and limits so any pod from this triggerTemplate is within limits but its not working

I was just wondering do you have examples of this or a pointer to go to ? docs seem a bit sparse

Thank you @lbernick

@lbernick
Copy link
Member Author

Thanks @EoinMan for testing this out! Can you please provide your trigger template + pipelineRun spec and more details about what is not working? Existing docs for compute resources are here and docs for trigger templates are here. If there's info you'd like to see in docs but isn't there, it would be super helpful if you could open an issue in pipelines or triggers with the information you'd like to see.

@EoinMan
Copy link

EoinMan commented Aug 28, 2023

@lbernick

ty for getting back to me

I might have read things wrong on second look

apiVersion: triggers.tekton.dev/v1beta1
kind: TriggerTemplate
metadata:
  name: triggertemplate-test-pipeline
spec:
  params:
  - name: version
    description: C version "tag"
  resourcetemplates:
  - apiVersion: tekton.dev/v1beta1
    kind: PipelineRun
    metadata:
      generateName: -pipeline-run-$(tt.params.version)-
    spec:
      podTemplate:
        tolerations:
        - key: worker-type
          value: keyValue
        affinity:
          nodeAffinity:
            preferredDuringSchedulingIgnoredDuringExecution:
            - preference:
                matchExpressions:
                - key: worker-type
                  operator: In
                  values:
                  - keyValue
              weight: 100
      taskRunSpecs:
        computeResources:
          requests:
            cpu: 2
            memory: 2048Mi
          limits:
            cpu: 2
            memory: 2048Mi

@lbernick
Copy link
Member Author

Thanks @EoinMan, could you please provide a bit more detail on what's not working? Looking at this now I don't think this is a valid PipelineRun (it doesn't have a pipelineSpec or pipelineRef but I'm not sure if you omitted those for brevity). It looks like taskRunSpecs are also not specified correctly here; they also need a pipeline task name (see docs). It seems like there's some work we could do in Triggers to make it more clear when a PipelineRun cannot be created due to invalid syntax, but I need a bit more detail to know if there's anything that needs updating in Pipelines.

@ChrisPaul33
Copy link

Hi @lbernick 😄 , any updates on this issue?
Why is it possible to parametrize the name or the image of a task but not the resources?
Is there a workaround maybe? If I'll try to use PIpelineRuns resources, how will I be able to override the resources (when using a task ref and not specifying it inside the PipelineRun yaml file)?

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

6 participants