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

HelmRelease valuesFiles defaulting to values.yaml

Open BeardyC opened this issue 2 years ago • 6 comments

When deploying a helmChart using the below HelmRelease and using helm-controller:v0.12.1

---
apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  name: redis-cluster
spec:
  chart:
    spec:
      chart: redis-cluster
      reconcileStrategy: ChartVersion
      sourceRef:
        apiVersion: source.toolkit.fluxcd.io/v1beta1
        kind: HelmRepository
        name: mdx-helm-snapshots
      version: 3.2.8
      valuesFile: values-dev-gcp.yaml
  install:
    crds: Skip
  interval: 1m0s
  upgrade:
    crds: Skip
  values: {}

I'm getting the following

NAME                            CHART           VERSION   SOURCE KIND      SOURCE NAME          READY   STATUS                                      AGE
dev1-e2-dev-mdx-redis-cluster   redis-cluster   3.2.8     HelmRepository   mdx-helm-snapshots   False   failed to locate values file: values.yaml   4d2h

The helmchart does not have a values.yaml and I don't intend to have one in to merge values on, is a base values.yaml needed even though no merging is needed?

I've tried using a relative path - ./values-dev-gcp.yaml - but no luck. Any recommendations or known issues related to this?

BeardyC avatar Dec 13 '21 18:12 BeardyC

The valuesFile (or valuesFiles) field is a last-mile modification feature, and still expects a valid chart as a starting point. Which means that it should contain a values.yaml file if it makes use of values, as otherwise it's not a valid chart (and helm lint will also warn about this and/or error with nil pointer evaluating interface {}).

hiddeco avatar Dec 13 '21 19:12 hiddeco

It's a valid chart without the need of a values.yaml as the full values are encapsulated in values-dev-gcp.yaml. We've deployed this on various clusters before using just helm. I can provide the helm chart if needed

BeardyC avatar Dec 14 '21 10:12 BeardyC

If you could share the chart that would be useful for me to check if I somehow made a mistake interpreting all possible chart structures, and made an assumption that is simply incorrect.

To provide some more clearance: it is not falling over the merge operation itself (which does not include a values.yaml as a base if not specified), but rather during the "overwrite", which assumes a chart must have values if they are being overwritten: https://github.com/fluxcd/source-controller/blob/main/internal/helm/chart/metadata.go#L77-L78

hiddeco avatar Dec 14 '21 10:12 hiddeco

Apologies for the wait, was on leave. Attached is the helm chart, let me know if you can reproduce the issue. redis-cluster-3.2.8.tgz.zip

BeardyC avatar Jan 17 '22 11:01 BeardyC

Have you had some time to have a look at the above @hiddeco?

It works if I add a values.yaml file to the chart and omit the valuesFiles: from the HelmRelease.

When I try to target a specific values file other than values.yaml it does not find the file values files merge merror: failed to merge chart values: no values file found at path - Unless specified, I thought there would be no merging of value files?

BeardyC avatar Jan 27 '22 10:01 BeardyC

Sorry about the slowness in my response. According to the helm lint CLI, the chart you provided is not valid:

$ helm lint
==> Linting .
[INFO] values.yaml: file does not exist
[ERROR] templates/: template: redis-cluster/templates/update-cluster.yaml:1:18: executing "redis-cluster/templates/update-cluster.yaml"
at <.Values.cluster.update.addNodes>: nil pointer evaluating interface {}.update

Error: 1 chart(s) linted, 1 chart(s) failed
$ helm template .
Error: template: redis-cluster/templates/update-cluster.yaml:1:18: executing "redis-cluster/templates/update-cluster.yaml" at <.Values.cluster.update.addNodes>: nil pointer evaluating interface {}.update

Use --debug flag to render out invalid YAML

I therefore deem our current behavior correct.

When I try to target a specific values file other than values.yaml it does not find the file values files merge merror: failed to merge chart values: no values file found at path - Unless specified, I thought there would be no merging of value files?

Can you share the HelmRelease which returned this? It sounds like a path configuration issue.

hiddeco avatar Feb 04 '22 13:02 hiddeco