Add --args option to sync command

[mxey]

The option exists for some of the other commands, and I'd like to pass options to helm upgrade like --wait and --reset-values

[mumoshu]

@mxey Thanks for the PR!

Code LGTM, but would you mind elaborating a bit more on your expected use-case?

For me --reset-values doesn't seem necessary in helmfile, because helmfile literally passes all the values defined in helmfile.yaml to tiller, which would always override what are persisted in the previous revision of the release. Ref helm/helm#1569

[mxey]

TBH I am not sure if I need --reset-values. Does --set remove all other values that I might have previously set, even if I do not overwrite them with new values, but have just removed them from my Helmfile entirely? I wanted to make sure there are no old values still around.

--wait is obvious, I assume. I want my deployment pipeline to only be finished when everything is up and running.

[willejs]

@mxey I feel like --wait should be against each chart in the yaml?

[mxey]

@mxey I feel like --wait should be against each chart in the yaml?

@willejs I don't understand what you mean. The arguments are set on the struct that executes all the Helm commands, so it applies to every chart.

[mumoshu]

@willejs Probably you think that one would want to decide whether --wait should be added or not per release?

Somethine like:

releases:
- name: mywebapp
  chart: mycharts/mywebapp
  args: "--wait"
- name: fluentd
  chart: mycharts/fluentd-daemonset

[mumoshu]

@mxey Thanks for the response!

I wanted to make sure there are no old values still around.

Ah! It sounds like a completely reasonable use-case now.

Oh, but then, don't we need to FORCE --reset-values for all releases, rather than passing it via an optional flag?
The original goal of helmfile as a "declarative spec for a set of helm releases" - it should converge any targeted helm release to the desired state regardless of what is persisted in the previous release.

[mumoshu]

@mxey Thanks for 👍 on my question!
Regarding that I've opened a dedicated issue #63.
I'm still merging this on purpose of making helmfile sync consistent with other existing helmfile sub-commands.

[sstarcher]

I think we would be better calling out specific values instead of allowing generic arguments. If we want support for --wait I would recommend it at the individual release level.

releases:
- name: mywebapp
  chart: mycharts/mywebapp
  wait: true
- name: fluentd
  chart: mycharts/fluentd-daemonset

[mumoshu]

@sstarcher Thx for the feedback!

Honestly saying I'm still considering if it is good to do so.

Basically helmfile.yaml for me is a more or less a declarative spec of helm releases, rather than configuration of specific operation executed by helmfile. That being said, --wait seems like rather an operational concern to me.

For example, helmfile sync --args "--wait" and helmfile sync --wait or even with the newly introduced labels/selector feature helmfile --selector type=oneshot-job sync --args "--wait" and helmfile --selector type=oneshot-job sync --wait makes sense to me, but making it configurable in helmfile.yaml does not.

WDYT?

[sstarcher]

My thoughts are

• As an operator it makes sense to me to always want to be notified when certain charts fail.
• As an operator I don't want to have to know which charts I should wait for on a case by case basis and would want it allowable in the helmfile itself
• As an operator I would want all available flags documented instead of a generic --args as some underlying flags may not be compatible with helmfile also passing flags

[mumoshu]

@sstarcher Thx for the confirmation!

As an operator it makes sense to me to always want to be notified when certain charts fail.
As an operator I would want all available flags documented instead of a generic --args as some underlying flags may not be compatible with helmfile also passing flags

Adding --wait would be good in this respect.

As an operator I don't want to have to know which charts I should wait for on a case by case basis and would want it allowable in the helmfile itself

Definitely an interesting use-case! Would you mind providing me a brief example?
Perhaps there should be a better semantics than wait for helmfile.yaml, something more declarative in regard to the release, rather than specific helmfile operations.

[sstarcher]

Having a --wait at a global level makes sense as currently we have no way of knowing without digging if what we just applied is working. A --wait would go a long way toward that.

As for not applying --wait to everything and wanting to select it or not. I have in the past hard charts like Kafka that can take 10 mins to start and waiting for it some times does not make sense.

Another example of when not to wait is related to charts that have a job that fails and later succeeds. In this case --wait will fail, but the chart succeeds. I do agree that this is more or less a fault of the chart and may not be something we specifically want to work around.

[mumoshu]

@sstarcher Thx for the detailed answer!
Opened #64 for the --wait flag and #65 for wait: in helmfile.yaml. I agree to add the former at this point. Would you mind submitting a PR for that?

[mumoshu]

@mxey Thank you very much for your suggestion and contribution 👍