roboll/helmfile

Possible to set selector in parent helmfile?

2ZZ opened this issue ยท 22 comments

2ZZ commented

I have a helmfile that pulls in another helmfile
cluster1-helmfile.yaml

---
context: cluster1
helmfiles:
 - apps-helmfile.yaml

Inside the second helmfile there are selectors
apps-helmfile.yaml

releases:
- name: app1
  chart: repo/app1
  version: '*'
  labels:
    enabled: app1
- name: app2
  chart: repo/app2
  version: '*'
  labels:
    enabled: app2

Currently I deploy it using

helmfile --selector enabled=app1 -f cluster-helmfile.yaml sync

Instead is it possible to set the enabled label in the parent helmfile? Similar to #168 (comment)

@2ZZ Yeah I see that there are use-cases like this!

What if we enhance helmfiles to accept more parameters per item, so that you could specify any supported options along with the name of sub-helmfile, like this:

helmfiles:
 - source: apps-helmfile.yaml
    selectors:
    - enabled=app1
2ZZ commented

that would be ideal for my use case ๐Ÿ‘

The implementation for this would rely on the same mechanism for #168. The difference is that we add an another path to specify default selectors that is used while running sub-helmfiles.

2ZZ commented

Hi @mumoshu , I just discovered the context setting in the parent helmfile is not inherited in the included helmfiles, is that a bug or intentional?

@2ZZ That's intentional! I basically want each helmfile.yaml to be self-contained. That is, there should be no global state among parent and sub helmfiles.

I would really appreciate this feature too cause we are compositing helmfiles but we sometimes do not want to install everything in the sub-helmfile.
I like the proposal with selector below the sub-helmfile path.
In fact there could be a section for each of the global options of the helmfile cli such as environment, kube-context, log-level, selector, namespace, interactive, quiet

@2ZZ That's intentional! I basically want each helmfile.yaml to be self-contained. That is, there should be no global state among parent and sub helmfiles.

@sgandon Can we start by reverting my previous comment above, and just pass the context(label selector from the command-line) down to all the sub-helmfiles?

The rationale is that I want everyone to be able to split helmfile.yaml without tears when it gets huge enough.

The label selector applied to the parent helmfile only defeats this use-case. Because then, once you start splitting your helmfiles, you are unable to rely on label selector to selectively deploy releases anymore.

@2ZZ Would you mind if I revert my comment and just make it pass label selector down to sub-helmfiles?

@mumoshu I am sorry I do not get what you are suggesting. But I think my use case is pretty similar to
@2ZZ .
Basically we have a 'team A' creating a set of charts and they are all gathered in a single helmfile.
And other teams want to use only a subset of this helmfile.
So the idea was to allow for other teams to pick a set of chart of 'teams A' using business labels selectors inside their own helmfiles.
In fact I believe if you allow inheritance like passing down commandline values to sub-helmfile, this creates more confusion and make is more complicated to understand what happens.
And when compositing helmfiles we really need a way to select what to deploy or not for sub-helmfile.

in my opinion the commandline should only impact the current helmfile for simplicity. That does not mean we can find a way to allow passing argument to sub-helmfile, but this should be done explicitly and not automatically.

I am not sure what you mean with env values if you are talking about environment variable then nope cause I want it to be specified in the parent helmfile, and if you mean helmfile environment values then why not, I don't see a real usage on the environment values right now for our use case.
I thought selectors where a good match as they seems to have been made for choosing what to deploy or not.
I also though including helmfile in an helmfile is just a way of launching multiple helmfile sub commands this is why I thought it would be great to propose helmfile cli global arguments in the parent helmfile definition something like that.

helmfiles:
 - ../common/all-common-svc-helmfile.yaml
    selectors:
    - enabled=app1
    - name=my-specific-release
    namespace: foo
    interactive: false
    kube-context: eks-dev

I believe this would match the desire of keeping the helmfile independent from each other in the sense that there is nothing more you can do using en composite helmfile that using multiple command lines.

In the end I think you should not leak selectors to subhelmfile cause this would mean you know all the labels of all the sub-helmfiles that are composed together.
To me the --environment is something that should be global but selector should only apply to the initially executed helmfile.
I was going to start the dev of this feature but I guess we should all agree on the selector behaviour.

the label of this issue is design finalized but since @mumoshu does not seem to agree we may want to remove it.

One possible way around this issue is to split the inital helmfile with multiple releases and labels to multiple helmfiles each for different set of releases. And this what we do right now, but then labels and selectors are no more helpfull and that is a shame cause you can cope with higher order of complexity using labels selector that you would with multiple helmfiles.

I am wondering if we could not provide both alternatives, that is have an inheritance of the selectors (either implicitly 'yucks', or explicitly) and also provide a way to specify them manually in the yaml.

  • so implicit which I don't like cause it is magical.
  • explicit with the possibility to specify those in the yaml like this
helmfiles:
 - apps-helmfile.yaml:
    selectors: inherited
 - apps1/*/helmfile.yaml:
    selectors:
    - enabled=app1
 - apps2/*/helmfile.yaml

This way you can have 3 different workflow, no selectors, inherited or manually set.
In fact I just realized while coding this that the implicit inheritance is already implemented and that is what @2ZZ is using. This is a pain cause it reduces flexibility and make things more "magical".

Ah, seems like I have been forgotten about that helmfile does passes selector down to sub-helmfiles! Thanks for reminding me about that.

For the meantime, I would like to just add selectors:. An empty selectors: can be used to prevent selectors from being inherited.

...and that is to keep backward-compatibility. We can definitely revisit before helmfile reaching v1.0.

So, in short term, your example would turn into something like:

helmfiles:
 - apps-helmfile.yaml:
 - apps1/*/helmfile.yaml:
    selectors:
    - enabled=app1
 - apps2/*/helmfile.yaml
    selectors: {}

Another idea is, in addition to selectors:, introduce something like EXPERIMENTAL=true helmfile ... so that helmfile changes its behavior to "no selector inheritance by default".

I like all those ideas, I am currently developing it and hope to provide a PR soon.

@mumoshu in your latest example one case is missing or I did not get requirements right.
I have implemented the EXPERIMENTAL environment variable to not force inheritance but in the case we want inheritance what would you suggest in the yaml ?
I did suggest

helmfiles:
 - apps1/*/helmfile.yaml:
    selectors: inherited

but may there is another more yaml/elegant way to specify this ?