helm-controller icon indicating copy to clipboard operation
helm-controller copied to clipboard

Helm unset value seems not working

Open mecseid opened this issue 4 years ago • 13 comments

I wanted to upgrade my Rook installation with unset CPU limitation, but after the upgrade, the limitation still exists with the default value (500m). Am I doing something wrong, or it's maybe a bug?

Helm Controller version: v0.11.1 Chart: https://github.com/rook/rook/tree/v1.6.9/cluster/charts/rook-ceph

HelmRelease:

apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  name: rook-ceph
spec:
  chart:
    spec:
      chart: rook-ceph
      version: v1.6.9
      sourceRef:
        kind: HelmRepository
        name: rook
  interval: 24h
  maxHistory: 3
  releaseName: rook-ceph
  storageNamespace: rook-ceph
  targetNamespace: rook-ceph
  values:
    csi:
      enableCephfsSnapshotter: false
      enableRBDSnapshotter: false
      kubeletDirPath: /opt/rke/var/lib/kubelet
      pluginTolerations:
        - key: node.netfone.hu/environment
          operator: Exists
    pspEnable: false
    resources:
      limits:
        cpu: null
        memory: 256Mi
      requests:
        memory: 128Mi

Helm values (got with helm CLI v3.2.4):

# helm -n rook-ceph get values rook-ceph
USER-SUPPLIED VALUES:
csi:
  enableCephfsSnapshotter: false
  enableRBDSnapshotter: false
  kubeletDirPath: /opt/rke/var/lib/kubelet
  pluginTolerations:
  - key: node.netfone.hu/environment
    operator: Exists
pspEnable: false
resources:
  limits:
    memory: 256Mi
  requests:
    memory: 128Mi

mecseid avatar Sep 01 '21 22:09 mecseid

Helm as a pretty rich history around this bug, and there is a possibility that the merging code we borrowed and/or drew inspiration from for our own merging operations wasn't patched at the time.

hiddeco avatar Sep 03 '21 11:09 hiddeco

Hey @mecseid , I have been unable to reproduce this on the latest version of flux.

flux: v0.20.1
helm-controller: v0.12.1

Can you try upgrading to flux?

somtochiama avatar Nov 02 '21 12:11 somtochiama

Hi @SomtochiAma . Sorry for late response, I didn't have so much time to upgrade flux, until now. :) After upgraded to v0.22.1 and helm-controller to v0.12.2, I don't see any difference in helm values.

To check maybe an upgrade helps it, I pushed my chart to next patch version (rook v1.6.10), but I got the same result for helm -n rook-ceph get values rook-ceph.

Maybe am I doing something wrong?

mecseid avatar Nov 11 '21 22:11 mecseid

Hey @mecseid, I have been able to reproduce this on my end and currently looking at a fix.

somtochiama avatar Nov 19 '21 11:11 somtochiama

@somtochiama was you able to look into this further?

paprickar avatar Aug 22 '22 22:08 paprickar

also waiting for a fix of this

applike-ss avatar Sep 01 '22 07:09 applike-ss

Still happening with flux version 0.38.3 and helm-controller v0.28.1. Managed to workaround this by using postRenderers with delete patches.

Xplouder avatar Jan 25 '23 15:01 Xplouder

Still happening with flux version 0.38.3 and helm-controller v0.28.1. Managed to workaround this by using postRenderers with delete patches.

@Xplouder can you give a short example of how you did this?

@somtochiama any news on a potential fix for this? I also got bitten by a value that didn't fall back to the Chart default when I removed my override from the values list in my HelmRelease definition. So maybe my issue is slightly different from that of the original poster 🤔

torbjornvatn avatar Apr 14 '23 09:04 torbjornvatn

@torbjornvatn sure, needed to do this to remove the annotations, the chart did not allow to disable them in my use case:

apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  name: matomo
spec:
  releaseName: matomo
  chart:
    spec:
      chart: matomo
      version: 0.2.18
      sourceRef:
        kind: HelmRepository
        name: bitnami
        namespace: flux-system
  interval: 5m0s
  postRenderers:
    - kustomize:
        patches:
          - target:
              name: matomo
              kind: Deployment
            patch: |-
              - op: remove
                path: /spec/template/metadata/annotations/prometheus.io~1port
              - op: remove
                path: /spec/template/metadata/annotations/prometheus.io~1scrape

...

Xplouder avatar Apr 14 '23 16:04 Xplouder

Also ran into this issue when trying to override an object like the security context:

I tried the following options:

securityContext: {}
securityContext: ~
securityContext: null

Workaround (I know that a postRenderer also works but I want to post that option here, too):

securityContext:
  fsGroup: 0
  runAsGroup: 0
  runAsNonRoot: false
  runAsUser: 0

Can we do something to get this solved? It's very hard to figure out that this a issue with the helm-controller.

sebastiangaiser avatar Nov 07 '23 16:11 sebastiangaiser

It's a bit sad to see this issue not being popular enough because the use case seams to be pretty common. @somtochiama @hiddeco can we do something to get this fixed after >2 years?

sebastiangaiser avatar Nov 14 '23 15:11 sebastiangaiser

I suspect this will be solved in the next minor release, as Helm v3.13.x included changes to properly reset values when they have been set to null.

hiddeco avatar Nov 30 '23 11:11 hiddeco