Allow setting resources for kubernetes on a per-step basis

[smuth4]

This add a simple implementation of requests/limits for individual steps. There is no validation of what the resource actually is beyond checking that it can successfully be converted to a Quantity, so it can be used for things other than just memory/CPU.

close #1809

[woodpecker-bot]

Deployment of preview was successful: https://woodpecker-ci-woodpecker-pr-1767.surge.sh

[codecov-commenter]

Codecov Report

Patch coverage has no change and project coverage change: -0.08 ⚠️

Comparison is base (1417763) 39.01% compared to head (83d3354) 38.94%.

❗ Current head 83d3354 differs from pull request most recent head 926205d. Consider uploading reports for the commit 926205d to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1767      +/-   ##
==========================================
- Coverage   39.01%   38.94%   -0.08%     
==========================================
  Files         166      166              
  Lines       10343    10349       +6     
==========================================
- Hits         4035     4030       -5     
- Misses       6045     6055      +10     
- Partials      263      264       +1     

Impacted Files	Coverage Δ
pipeline/backend/kubernetes/pod.go	0.00% <0.00%> (ø)
pipeline/frontend/yaml/compiler/convert.go	0.00% <0.00%> (ø)
pipeline/frontend/yaml/container.go	77.77% <ø> (ø)

... and 10 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[anbraten]

In general a really nice, clean and thoughtful PR. Thanks for that.

[pat-s]

Ah great! I was just playing around with this a few days ago without success...

Would it be hard to add a global resource definition? Which is overwritten by definitions in individual steps?

[smuth4]

Ah great! I was just playing around with this a few days ago without success...

Would it be hard to add a global resource definition? Which is overwritten by definitions in individual steps?

It certainly sounds like a good idea, how does a top-level backend_options sound @anbraten? It might not be a good idea to try and over-generalize the solution with some sort of top>step merge (since you might want to handle things like arrays differently depending on context), but the structure could at least be the same.

---
backend_options:
  kubernetes:
    resources:
      request:
        cpu: 1000m
        memory: 1024Mi
pipeline:
  my-step:
    image: golang
    backend_options:
      kubernetes:
        resources:
          request:
            cpu: 2000m # Requests 2000m CPU
                       # Requests 1024 Mi memory

[6543]

I would not add a top level definition feature as you can do that already with: https://woodpecker-ci.org/docs/next/usage/advanced-yaml-syntax#map-merges-and-overwrites

if it's an agent config that do overwrite step config ... if step.limit>agent.limit that's of course another request and valid

[pat-s]

I would not add a top level definition feature as you can do that already with: woodpecker-ci.org/docs/next/usage/advanced-yaml-syntax#map-merges-and-overwrites

Are you sure this is possible? To my knowledge this is not parsed to pod.go and the object created there. This is the nr.1 feature I am still waiting for (would take any workaround that works!). AFAICS the resources are currently hardcoded to 1 CPU, 2 GB mem.

[smuth4]

I would not add a top level definition feature as you can do that already with: https://woodpecker-ci.org/docs/next/usage/advanced-yaml-syntax#map-merges-and-overwrites

That's what I get for working with tools that scaffold YAML for me all day, I completely blanked on the native features. 😞

if it's an agent config that do overwrite step config ... if step.limit>agent.limit that's of course another request and valid

I thought about adding this, but kubernetes already has native tools for setting default/limit ranges (although only on a per-namespace basis): https://kubernetes.io/docs/concepts/policy/limit-range/

I would not add a top level definition feature as you can do that already with: woodpecker-ci.org/docs/next/usage/advanced-yaml-syntax#map-merges-and-overwrites

Are you sure this is possible? To my knowledge this is not parsed to pod.go and the object created there. This is the nr.1 feature I am still waiting for (would take any workaround that works!). AFAICS the resources are currently hardcoded to 1 CPU, 2 GB mem.

I tested it and it should work, e.g.

variables:
  - &resources
    requests:
      memory: "4Gi"
      cpu: "250m"
    limits:
      memory: "8Gi"
      cpu: "1000m"
pipeline:
  build:
    image: golang
    backend_options:
      kubernetes:
        # Gets 4Gi & 250m requests, 2000m limits
        resources:
          <<: *resources
          limits:
            cpu: "2000m"

[pat-s]

I tested it and it should work, e.g.

But does it work without explicit mapping? I.e. usually I would like to write it as follows

variables:
  - &resources
    requests:
      memory: "4Gi"
      cpu: "250m"
    limits:
      memory: "8Gi"
      cpu: "1000m"
pipeline:
  build:
    image: golang

and this should result in the step honoring the top-level resources unless they are explicitly set/overwritten in the respective step.

Imagine a pipeline with many steps: it should be possible that each step follows a global resource definition without a redundant resources mapping within each step.

@6543 Given that other top-level arguments like platform exist, I think it would be "cleaner" to have resources being an "official" top level setting rather than forcing users to invoke it via map-merges?

[zc-devs]

@smuth4, great work and thank you for this desired functionality!

As I understand, this configuration is per step or per pipeline. Could we add per agent configuration (like I've done here 8eb240b)?

[smuth4]

@smuth4, great work and thank you for this desired functionality!

As I understand, this configuration is per step or per pipeline. Could we add per agent configuration (like I've done here 8eb240b)?

I understand the desire for the feature, but can you explain the use case which requires it to be part of the woodpecker code? Specifically, I'm trying to avoid re-implementing LimitRanges, which are very powerful and native to kubernetes, but do require application at the namespace level.

[zc-devs]

@smuth4, yeah, you're right. Cool feature.

[6543]

@6543 Given that other top-level arguments like platform exist ...

well it does exist but I consider this an legacy and we do move to labels ... witch can filter backend, platform, ...

so there might come the time where it will be deprecated in favour of a simpler and more generic solution

but that's something for the #567 to talk about :D

outdated & addressed