roboll / helmfile

Deploy Kubernetes Helm Charts
MIT License
4.04k stars 565 forks source link

helmfile values override via CLI not working / undocumented #1348

Open Arabus opened 4 years ago

Arabus commented 4 years ago

When trying to enable/disable releases via the environment clause in helmfile I came about the issue that overriding those values via CLI seems to be impossible. The apparent merge behaviour seems to be helmfile.yaml values section <- helmfile cli --state-values-file|--state-values-set <- helmfile.yaml environment values section

I'd expect otherwise i.e., the CLI overriding everything but even if this is not intended I'd rather like a documentation snippet stating the merge behaviour of those sections/options

Testcases

This one works when overriding on the CLI (albeit I need to provide the boolean via a pseudofile)

helmfile --debug --selector name=two --state-values-file <(echo -e "second:\n  enabled: true") template --skip-deps
processing file "helmfile.yaml" in directory "."
envvals_loader: loaded /dev/fd/14:map[second:map[enabled:true]]
first-pass rendering starting for "helmfile.yaml.part.0": inherited=<nil>, overrode=&{default map[second:map[enabled:true]] map[]}
first-pass uses: &{default map[second:map[enabled:true]] map[]}
first-pass rendering output of "helmfile.yaml.part.0":
 0: repositories:
 1: - name: stable
 2:   url: https://kubernetes-charts.storage.googleapis.com
 3:
 4: releases:
 5:   - name: one
 6:     namespace: first
 7:     chart: stable/wordpress
 8:   - name: two
 9:     namespace: second
10:     chart: stable/wordpress
11:     condition: second.enabled
12:
13: values:
14:   - second:
15:       enabled: false
16:

first-pass produced: &{default map[second:map[enabled:true]] map[second:map[enabled:false]]}
first-pass rendering result of "helmfile.yaml.part.0": {default map[second:map[enabled:true]] map[second:map[enabled:false]]}
vals:
map[second:map[enabled:true]]
defaultVals:[map[second:map[enabled:false]]]
second-pass rendering result of "helmfile.yaml.part.0":
 0: repositories:
 1: - name: stable
 2:   url: https://kubernetes-charts.storage.googleapis.com
 3:
 4: releases:
 5:   - name: one
 6:     namespace: first
 7:     chart: stable/wordpress
 8:   - name: two
 9:     namespace: second
10:     chart: stable/wordpress
11:     condition: second.enabled
12:
13: values:
14:   - second:
15:       enabled: false
16:

merged environment: &{default map[second:map[enabled:true]] map[second:map[enabled:false]]}
Fetching stable/wordpress
Fetching stable/wordpress
exec: helm fetch stable/wordpress --untar --untardir /var/folders/bw/yl6vm0m92tz2wlz779qgzpt80000gn/T/535662376/one/latest/stable/wordpress
...

This one doesn't, in fact the second release stays disabled

helmfile --debug --selector name=two --state-values-file <(echo -e "second:\n  enabled: true") template --skip-deps
processing file "helmfile.yaml" in directory "."
envvals_loader: loaded /dev/fd/14:map[second:map[enabled:true]]
first-pass rendering starting for "helmfile.yaml.part.0": inherited=<nil>, overrode=&{default map[second:map[enabled:true]] map[]}
first-pass uses: &{default map[second:map[enabled:true]] map[]}
first-pass rendering output of "helmfile.yaml.part.0":
 0: repositories:
 1: - name: stable
 2:   url: https://kubernetes-charts.storage.googleapis.com
 3:
 4: environments:
 5:   default:
 6:     values:
 7:       - second:
 8:           enabled: false
 9:
10: releases:
11:   - name: one
12:     namespace: first
13:     chart: stable/wordpress
14:   - name: two
15:     namespace: second
16:     chart: stable/wordpress
17:     condition: second.enabled
18:

first-pass produced: &{default map[second:map[enabled:false]] map[]}
first-pass rendering result of "helmfile.yaml.part.0": {default map[second:map[enabled:true]] map[]}
vals:
map[second:map[enabled:true]]
defaultVals:[]
second-pass rendering result of "helmfile.yaml.part.0":
 0: repositories:
 1: - name: stable
 2:   url: https://kubernetes-charts.storage.googleapis.com
 3:
 4: environments:
 5:   default:
 6:     values:
 7:       - second:
 8:           enabled: false
 9:
10: releases:
11:   - name: one
12:     namespace: first
13:     chart: stable/wordpress
14:   - name: two
15:     namespace: second
16:     chart: stable/wordpress
17:     condition: second.enabled
18:

merged environment: &{default map[second:map[enabled:false]] map[]}
Fetching stable/wordpress
Fetching stable/wordpress
exec: helm fetch stable/wordpress --untar --untardir /var/folders/bw/yl6vm0m92tz2wlz779qgzpt80000gn/T/967262944/one/latest/stable/wordpress
exec: helm fetch stable/wordpress --untar --untardir /var/folders/bw/yl6vm0m92tz2wlz779qgzpt80000gn/T/967262944/two/latest/stable/wordpress
exec: helm fetch stable/wordpress --untar --untardir /var/folders/bw/yl6vm0m92tz2wlz779qgzpt80000gn/T/967262944/one/latest/stable/wordpress:
exec: helm fetch stable/wordpress --untar --untardir /var/folders/bw/yl6vm0m92tz2wlz779qgzpt80000gn/T/967262944/two/latest/stable/wordpress:
0 release(s) matching name=two found in helmfile.yaml

err: no releases found that matches specified selector(name=two) and environment(default), in any helmfile

Relevant Metadata

Helmfile version

$ helmfile version
helmfile version v0.120.0

Helm version & Plugins

$ helm version
version.BuildInfo{Version:"v3.2.4", GitCommit:"0ad800ef43d3b826f31a5ad8dfbb4fe05d143688", GitTreeState:"dirty", GoVersion:"go1.14.3"}

$ helm plugin list
NAME            VERSION DESCRIPTION
diff            3.1.1   Preview helm upgrade changes as a diff
helm-git        0.7.0   Get non-packaged Charts directly from Git.
inject          0.1.8   inject plugin for Helm
kubeval         0.13.0  "Validate Helm charts against the Kubernetes schemas"
x               0.8.1   Turn Kubernetes manifests and Kustomization into Helm Release

go version

$ go version
go version go1.14.4 darwin/amd64

mac os version

$ sw_vers
ProductName:    Mac OS X
ProductVersion: 10.14.6
mumoshu commented 4 years ago

@Arabus Hey! Thanks for reporting.

I admit this is confusing, but this is working as intended.

--state-values-set and --state-values-file is basically a way to "prepend" a set of state values to be used by "rendering" helmfile state values. It works like that because the target use-case of it is to support people trying to "feed" helmfile.yaml template to dynamically produce state values from the state values passed via command-line flags.

Internally, your second example translates to something like below:

values:
  second:
    enabled: true

---

environments:
  default:
    values:
      - second:
          enabled: false

releases:
- name: one
  namespace: first
  chart: stable/wordpress
- name: two
  namespace: second
  chart: stable/wordpress
  condition: second.enabled

Before evaluating second.enabled, Helmfile merges {"second": {"enabled": true}} and {"second": {"enabled": false}} and produces `{"second":{"enabled":false}}" as the latter wins.

A workaround that you can use today would be to explicitly program it like:

environments:
  default:
    values:
      - second:
          enabled: {{ get "second.enabled" .Values false }}

so that your second example internally translates to:

# Again, this is coming from `--state-values-file`
values:
  second:
    enabled: true

---

environments:
  default:
    values:
      - second:
          enabled: {{ get "second.enabled" .Values false }}

I believe this defeats your (possible) original motivation to use --state-values-set to avoid templates just for defaulting. But unfortunately this is how it works today.

I'm not satisfied with this solution. So my suggestion is to add Helmfile any way to "append" or "override"(which term do you prefer, btw?) state values via command-line.

Maybe we'd better deprecate --state-values-(set|file) as confusing, rename it to --prepend-values, and later add --append-values for what you might have wanted?

Any input is much appreciated. Thanks anyway for trying Helmfile!

mumoshu commented 4 years ago

To be extra clear, internally Helmfile produces the below for your first example:

# The original `values` defined in your helmfile.yaml
values:
- second:
  enabled: false

---

# From your --state-values-file flag
values:
  second:
    enabled: true

---

releases:
- name: one
  namespace: first
  chart: stable/wordpress
- name: two
  namespace: second
  chart: stable/wordpress
  condition: second.enabled

Please note that --state-values-file is inserted between your original values and releases in this case, which is why it works.

jwilkicki commented 3 years ago

Hi!

I'm having a similar problem and the examples you have here haven't really cleared them up for me. The {{ get "second.enabled" .Values false}} that you list actually crashes for me.

In my case, I want to have a helmfile that has a small set of releases that are enabled by default and then a larger set of optional releases that are disabled by default but can be turned on by passing information when helmfile apply/sync are called.

Unfortunately, it looks like conditions can only be satisfied by passing a --state-values-file (I can't get --state-values-set to work at all) and if you don't supply one at all the entire command crashes with the error:

panic: interface conversion: interface {} is nil, not map[string]interface {}

goroutine 1 [running]:
github.com/roboll/helmfile/pkg/state.markExcludedReleases(0xc000365b00, 0x1, 0x1, 0xc0007007a0, 0x1, 0x1, 0x0, 0xc0001bf4d0, 0x0, 0x0, ...)
        /home/circleci/workspace/helmfile/pkg/state/state.go:1832 +0xb5a

However, I can't seem to find a combination of passing values either external to a helm file or internal to it that let something like

releases:
   - name: foo
     chart: foo
     version: 1.0.0
     condition: foo.enabled 

pick up a value for foo.enabled unless I set it through --state-values-file. That is, the code just dies instantly if it can't find a value for the condition and there's no way to give it a default value for foo.enabled.

I think either the prepend or append switches you propose are still pretty confusing. In most systems I've encountered, the order of precedence is anything set on the command-line wins, then anything in a configuration file, then anything specified in the code itself.

I also tried to use selectors to pick which things I want to install, which seems to work better but you have specify the selectors for everything you want or you get nothing. I can't have selectors that are turned off by default and then re-enable them on the command line.

There seems to be a lot of flexibility in what helmfile allows but the order of precedence is pretty hard for me to understand.. I might suggest looking at how something like ansible is doing this as an example of a system that seems more predicable.

unacceptable commented 3 years ago

I ran into this as well while working on a CD pipeline. As a workaround for now I am doing the following for a microservice deployment:

# in helmfile.yaml
values:
- image:
    tag: '{{ requiredEnv "IMAGE_TAG" }}'
IMAGE_TAG=v1.2.3 helmfile -f helmfile.yaml diff

I'd be interested in the proper way to use --state-values-set though. I assumed that this would be like the following:

helmfile -f helmfile.yaml --state-values-set .image.tag=v1.2.3 diff # this did not work for me.
spazm commented 3 years ago

I'm not satisfied with this solution. So my suggestion is to add Helmfile any way to "append" or "override"(which term do you prefer, btw?) state values via command-line.

I prefer override to append. I expect CLI values to have highest priority.

In general I expect CLIs to support an option priority like: explicit CLI values > ENVIRONMENT > user config > specific config > defaults.

Maybe we'd better deprecate --state-values-(set|file) as confusing, rename it to --prepend-values, and later add --append-values for what you might have wanted?

I agree with deprecation and also with OP's request that the usage and interaction and merge behavior be documented. --state-values-set does not behave similarly to helm's --set and this is surprising and unexpected.

Use Case: I want to temporarily disable a helmfile app temporarily. In my case, while making some changes to the underlying PV/PVC that need to be atomic relative to the app.