tektoncd / pipeline

A cloud-native Pipeline resource.
https://tekton.dev
Apache License 2.0
8.52k stars 1.78k forks source link

[FR] Specifying compute resources on Pipeline Tasks #5110

Closed lbernick closed 1 year ago

lbernick commented 2 years ago

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.

michaelsauter commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

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 commented 1 year ago

@tekton-robot: Closing this issue.

In response to [this](https://github.com/tektoncd/pipeline/issues/5110#issuecomment-1342848556): >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](https://github.com/tektoncd/plumbing). Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
BlueCog commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

EoinMan commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 4 months ago

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)?