Unleash / helm-charts

Contains helm-charts for Unleash
Apache License 2.0
40 stars 54 forks source link

Unleash Edge should not set default resources #121

Closed mbarczyk closed 7 months ago

mbarczyk commented 7 months ago

Describe the bug

Currently Unleash Edge helm charts sets default cpu and memory resources limits in: https://github.com/Unleash/helm-charts/blob/4cd3a4a2afec1a3048ccbafc2a7c869c64393ed4/charts/unleash-edge/values.yaml#L47-L53

As a consequence we lost the ability to specify only subset of requests in our local values.yaml files, e.g:

  resources:
    limits:
      memory: 6G
      ephemeral-storage: "8Gi"
    requests:
      memory: 4G
      cpu: 1
      ephemeral-storage: "8Gi"

It is rendering to:

  resources:
    limits:
      cpu: 200m                                  # <- I don't want to set up cpu, I have no option to skip it
      memory: 64Mi
    requests:
      cpu: 1
      ephemeral-storage: 8Gi
      memory: 4G

resources.limits.cpu is inherited from charts values.yaml file and I have no option to skip any part of resources. In my particular scenario I don't want to set resources.limits.cpu.

This is a known helm behavior: https://github.com/helm/helm/issues/12488 where if the value is defined in the ancestor (helm chart in this case) I'm not able to omit a part of it.

Steps to reproduce the bug

  1. Create values.yaml file with following content (without resources.limits.cpu):
    resources:
    limits:
    memory: 64Mi
    requests:
    cpu: 1
    ephemeral-storage: 8Gi
    memory: 4G
  2. Render helm chart and see how resources are defined:
    helm template unleash/unleash-edge -s templates/deployment.yaml -f values.yaml | grep -A50 resources:                                                                                                                                                                                                     (use1-rdev/ep-unleash-edge) 
          resources:
            limits:
              cpu: 200m
              memory: 64Mi
            requests:
              cpu: 1
              ephemeral-storage: 8Gi
              memory: 4G

Expected behavior

The chart should not set default resources, similar to how this is done in Unleash: https://github.com/Unleash/helm-charts/blob/4cd3a4a2afec1a3048ccbafc2a7c869c64393ed4/charts/unleash/values.yaml#L193-L204

Logs, error output, etc.

No response

Screenshots

No response

Additional context

Checked on the latest helm chart version: 3.0.6

Unleash version

No response

Subscription type

Open source

Hosting type

Self-hosted

SDK information (language and version)

No response

chriswk commented 7 months ago

Hi @mbarczyk - Thanks for reporting this. I see you also filed #122 to fix this, so I'll help you getting that merged