fluxcd / flux

Successor: https://github.com/fluxcd/flux2
https://fluxcd.io
Apache License 2.0
6.9k stars 1.08k forks source link

Helm ValuesFiles not updated during reconciliation #3556

Closed jonerer closed 3 years ago

jonerer commented 3 years ago

Describe the bug

Thanks for a great piece of software! I'm having a blast with it.

I have a HelmRelease where I load the values from normal helm value files. According to https://fluxcd.io/docs/components/helm/helmreleases/ the way to do that is to put them in the Spec, like so:

apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  name: my_helm_release
  namespace: agile
spec:
  interval: 5m
  chart:
    spec:
      chart: cluster/charts/my_helm_chart
      version: "0.1.2"
      sourceRef:
        kind: GitRepository
        name: flux-system
        namespace: flux-system
      interval: 1m
      valuesFiles:
        - cluster/charts/my_helm_chart/values.yaml
        - cluster/charts/my_helm_chart/values_prod_tag.yaml

This works perfectly file, loading in the values.yaml file and adding values_prod_tag.yaml on top. But it doesn't update the helm release on reconciliation if either of the values files are changed. Reconciliation will only pick up the changes if I bump the "version" number on the chart.

The reason why I'm using normal helm values.yaml-files instead of putting all of my values under a values-block outside the spec is twofold:

  1. I want to be able to easily update the values_prod_tag.yaml-file in my CI pipeline, to specify what image tag to use; updating inside a k8s resource file is trickier
  2. I want to be able to use the standard helm CLI tools for debugging/development. In this case, I could do helm template my_helm_release . -f values_prod_tag.yaml to see the output and validate or so. Having my values inside a k8s resource file (like a ConfigMap or sth) would be harder

Steps to reproduce

Install a helm chart from a git repo, like so:

apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  name: my_helm_release
  namespace: agile
spec:
  interval: 5m
  chart:
    spec:
      chart: cluster/charts/my_helm_chart
      version: "0.1.2"
      sourceRef:
        kind: GitRepository
        name: flux-system
        namespace: flux-system
      interval: 1m
      valuesFiles:
        - cluster/charts/my_helm_chart/values.yaml
        - cluster/charts/my_helm_chart/values_prod_tag.yaml

Commit changes to "values_prod_tag.yaml"

See that the installed helm chart isn't updated (by checking helm list and seeing the revision number not be bumped)

Expected behavior

Updating a values file referenced by a HelmRelease should re-render the helm chart and update the installed helm release upon reconciliation.

Alternatively, if there is some other more "Flux"-y way I would be very interested to learn!

Kubernetes version / Distro / Cloud provider

Rancher rke 1.20.4

Flux version

Flux version 0.17.2

Git provider

Github

Container Registry provider

No response

Additional context

No response

Maintenance Acknowledgement

Code of Conduct

boeboe commented 3 years ago

+1

It looks like reconciliation only takes helm version chart changes into account and ignores input value changes.

kingdonb commented 3 years ago

Thanks for reporting this issue. It looks like you're using Flux v2, which is hosted out of a different repo.

Please find https://github.com/fluxcd/flux2 or https://github.com/fluxcd/helm-controller where these reports can be handled by the appropriate person. That being said, I'm not 100% sure what you've specified here is going to be considered as a bug.

The valuesFiles spec is for manipulating the values based on files contained in the chart itself. Flux considers that a Helm chart is versioned and that the versions are immutable. You may have already heard this before, not to belabor the point, when you use valuesFiles it alters something inside of the structure of the chart. It isn't considered a separate input.

Please note the difference between (values from sources) and (values inside the chart).

You want your values file from the chart to be considered as an outside input. For that, use valuesFrom with chartFileRef:

https://fluxcd.io/docs/migration/helm-operator-migration/#chart-file-references

Please let me know if this helps!

kingdonb commented 3 years ago

I think this is an issue with documentation, as we should really have one doc which you can reference that tells you all the important tricks of Helm Controller, but I'm afraid I wasn't able to explain this issue without referring to more than one doc.

If the combination of valuesFrom and chartFileRef do not solve the issue for you, I'll be happy to revisit this!

kingdonb commented 3 years ago

I am sorry to further confuse the issue, I am afraid I am very mixed up, (and it is still a documentation issue as I think I've suggested a Helm Operator feature that will not work at all in Helm Controller.)

The root issue is that the Chart itself is immutable at each version and this does not mesh well with the semantics of a GitRepository. You only get a new HelmChart Custom Resource once for each time the Chart.yaml gets an update to the version field. The general sense of what I wrote is correct – if you want these input values to be considered as input to the chart, you need them to be specified as either values or valuesFrom

The content of valuesFiles is considered as part of the content of the chart and it will not update more frequently than your chart's Chart.yaml updates its version field.

jonerer commented 3 years ago

Thanks for the feedback @kingdonb ! I understand what you mean.

It seems like what the Helm Operator did was more in line with my expectations. It does indeed make sense that the chart itself is locked with the version number (we typically don't really bump it internally when we make a change, since we're used to a monorepo where "latest" is always the only version. But we could sort that out I guess).

But the values we input to the chart is, as you say, not part of the chart itself. Which makes the placement of valuesFiles inside the chart spec a bit odd. If it were somehow possible to input typical helm values files under like with valuesFrom: chartFileRef, that would be amazing (not just references to ConfigMaps or Secrets). I will file an issue about this under the https://github.com/fluxcd/flux2 repo then :)