Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove helmfile --args #1804

Closed
mumoshu opened this issue Apr 24, 2021 · 45 comments
Closed

Remove helmfile --args #1804

mumoshu opened this issue Apr 24, 2021 · 45 comments

Comments

@mumoshu
Copy link
Collaborator

mumoshu commented Apr 24, 2021

helmfile --args is a historical artifact that only worked in the very beginning of the helmfile project. At that time helmfile sync only ran a single helm command.

Nowadays a helmfile run involves many helm commands so --args doesn't make sense. Which custom args should be passed to helm template, helm template, helm diff, helm repo up, helm dep build, etc? Impossible to deduce.

Just drop it from helmfile, and alternatively provide more specific flags if necessary.

@vavdoshka
Copy link
Contributor

one thing which it might affect is an argoCD plugin https://github.com/travisghansen/argo-cd-helmfile/blob/master/src/argo-cd-helmfile.sh#L242

not sure though

@mumoshu
Copy link
Collaborator Author

mumoshu commented Apr 26, 2021

@vavdoshka Thanks for sharing!

And... Whoa! I've never expected it to work like that. A helmfile template command involves many commands depending on your configuration, not only the helm template to render the final result but also helm repo up, helm dep up, helm dep build, and even helm template used internally by chartify.

So, the usage of args there somehow passed the args to the final helm template only? That sounds like a miracle.

@travisghansen What was your primary motivation to use args? Can you remove the use of args if we added dedicated flags like --kube-version, --api-versions,, --is-upgrade, and --no-hooks to helmfile template?

I think the use of --args --skip-crds can be replaced with helmfile template --include-crds=false today.

@travisghansen
Copy link

I think it's nice to have an ability to pass through generic args to the helm template command (there was an issue about this specifically...I'll see if I can find it). For example just today I had to use this to pass --skip-tests down helm using the project linked above with argocd.

Likely unrelated, but I ran into issues using the chartify feature with pure templates. It seemed to puke on some of the arguments being passed in the argocd integration bit but don't recall what those were either.

@travisghansen
Copy link

#1060

@mumoshu
Copy link
Collaborator Author

mumoshu commented Apr 27, 2021

Likely unrelated, but I ran into issues using the chartify feature with pure templates. It seemed to puke on some of the arguments being passed in the argocd integration bit but don't recall what those were either.

Yes, it is very likely to break. We have no complete list mapping that knows "which helm command supports which args", neither in helmfile nor in chartify, which makes implementing your desired feature correctly very hard.

@mumoshu
Copy link
Collaborator Author

mumoshu commented Apr 27, 2021

For example just today I had to use this to pass --skip-tests down helm using the project linked above with argocd.

Can we just add it to helmfile-template as a dedicated flag?

@mumoshu
Copy link
Collaborator Author

mumoshu commented Apr 27, 2021

Well, let me be more clear. I know it is a very "nice to have" feature. What I'm saying is implementing it correctly seems too hard from my perspective.

If anyone can come up with a nice way to implement it in a way that works through the helmfile's code, including chartify, that would be great.

Otherwise, adding dedicated flags to helmfile-template should be the way to go. I can easily see that I'll be able to maintain it that way.

@travisghansen
Copy link

Yeah, I want to say it blew up on the --api-version business...I just never got around to reporting it.

A dedicated flag to template seems ideal honestly to keep pollution down. I'm only using it with template operation anyway.

Appreciate the dialog and willingness to work jointly!

@travisghansen
Copy link

BTW, I'm using that plugin fairly heavily with argocd at this point and it all works beautifully...managing many clusters using it jointly with appset etc to orchestrate at many layers...I'm even running a silly controller to push rancher clusters automatically down to argocd as clusters at that layer.

@mumoshu
Copy link
Collaborator Author

mumoshu commented Apr 27, 2021

@travisghansen Thanks for the constructive feedbacks! I very much appreciate it. I've still some idea to try on args so you can expect me to not delete args anytime soon, at least.

Also, I've created #1812 to discuss about Helmfiel with ArgoCD. If you could come up with any topics around that, I'd appreciate your comments.

Lastly... shouldn't --api-version be within helmfile.yaml, rather than the command-line flags, if it's up to the chart being used which K8s API versions are supported?

@travisghansen
Copy link

Thank you for the great tool! I know it can get tiring maintaining such a project for various reasons.

api-versions shouldn't be in the helmfile.yaml because it's technically generated per-cluster. I'm using the same set of helmfiles deployed across many clusters while using 2 high-level required envs ENVIRONMENT_ID and CLUSTER_ID. A whole slew of values are derived from those 2 values driving automated ingress URLs, dns keys for external-dns etc. Technically if cluster versions (or api-versions) differ the produced output of the chart may differ slightly from cluster to cluster. Or said differently, if I upgrade a cluster from one version to another I may see updates in produced output.

Does that make sense why I keep it out of the helmfiles?

I can provide some concrete examples from my own charts if that's helpful..

@mumoshu
Copy link
Collaborator Author

mumoshu commented Apr 27, 2021

@travisghansen Thanks for confirming! It's now crystal clear that it shouldn't be in helmfile.yaml 👍

@travisghansen
Copy link

It’s not a bad feature to have by the way, there are use cases when the templating tool may not have access to live cluster info so having a failswitch for it is nice.

@GolubevV
Copy link

GolubevV commented May 11, 2021

Hello!
First of all, thank you @mumoshu for a wonderful tool and @travisghansen for a script integrating helmfile into ArgoCD.

I completely agree with the need to support the --args flag for helmfile, especially in scenarios where helmfile is used as a Helm chart compose tool to render/template manifests, e.g when it is invoked by ArgoCD, Spinnaker etc.

I was trying to use this possibility (via the integration script from @travisghansen ) to dynamically propagate the target cluster API capabilities, which are made available by ArgoCD as build environment - to the helm for correct render of templates.

However, it seems like it is still not working.
Helm template command expects to get the api versions passed as string array, e.g something like this:
helm template --api-versions=cert-manager.io/v1alpha3 --api-versions=cert-manager.io/v1alpha2.

However, helmfile seems to pass only the first of the specified flags. This problem was already mentioned in the following issue: #457

Here is example of this behaviour:

#snippet from Helm chart using capabilities
....
{{- if .Capabilities.APIVersions.Has "cert-manager.io/v1alpha3" -}}
apiVersion: cert-manager.io/v1alpha3
{{- else -}}
apiVersion: certmanager.k8s.io/v1alpha1
{{- end }}

# invoke  template  with `args` passing a single api-versions flag
helmfile -l chart=cert-issuer template --args "--api-versions=cert-manager.io/v1alpha3"
---
# Source: cert-issuer/templates/self_signed_issuer.yaml
apiVersion: cert-manager.io/v1alpha3
kind: ClusterIssuer
metadata:
  name: kfrfpr-issuer-ca
spec:
  ca:
    secretName: ca-secret

#invoke template with `args` passing several api-versions flags, where first is not the required one
helmfile -l chart=cert-issuer template --args "--api-versions=cert-manager.io/v1alpha2 --api-versions=cert-manager.io/v1alpha3"
---
# Source: cert-issuer/templates/self_signed_issuer.yaml
apiVersion: certmanager.k8s.io/v1alpha1
kind: ClusterIssuer
metadata:
  name: kfrfpr-issuer-ca
spec:
  ca:
    secretName: ca-secret

# running the later command in debug mode, you can see that only the first flag was passed downward to the helm executable
helmfile --debug -l chart=cert-issuer template --args "--api-versions=cert-manager.io/v1alpha2 --api-versions=cert-manager.io/v1alpha3"
....
Templating release=cert-issuer, chart=../cert-issuer
exec: helm template cert-issuer ../cert-issuer --version 1.2.1 --namespace cert-manager --values /var/folders/j8/4x2hx6_13sq78dh_2cqkz99m0000gq/T/values595035734 --api-versions=cert-manager.io/v1alpha2 --debug

BTW, possibility to pass the args for templating can also become pretty useful if the Helm tool community will finally agree to re-introduce the --kube-version into Helm3 (there is a pending issue on that: helm/helm#9040).

As helm template command does not communicate with target cluster, currently it uses the embedded K8s Go library in order to identify the Kubernetes Version to use. As such, for Helm starting with version 3.5.X, the default target K8s version become 1.20 which broke a lot of Helm charts rendering with Ingress objects dynamically identifying the correct API version to use like this:

{{- if semverCompare "<1.14-0" .Capabilities.KubeVersion.GitVersion -}}
{{- print "extensions/v1beta1" -}}
{{- else if semverCompare "<1.19-0" .Capabilities.KubeVersion.GitVersion -}}
{{- print "networking.k8s.io/v1beta1" -}}
{{- else -}}
{{- print "networking.k8s.io/v1" -}}
{{- end -}}

Hopefully, the Helm3 will allow to explicitly specify target K8s version for template command and than we will need to somehow propagate it down from helmfile. The --args parameter is currently the only way that i know of to achieve it.

@travisghansen
Copy link

Interesting, are you saying that —args is filtering duplicate keys but generally allows passing multiple args/options? That support in my script was added via external PR so I didn’t spend much time testing it and didn’t notice that limitation.

@GolubevV
Copy link

GolubevV commented May 11, 2021

The problem is coming not from the script (i was author of that PR:) ) but from the helmfile internal implementation which seem to keep only one of multiple args while helm tool requires to pass multiple args in this way:
#457

As mentioned earlier and tested today, until this problem in helmfile is fixed (so that api-versions from command line are passed completely), the only workaround to dynamically pass the api-versions into helmfile logic is to use specially crafted snippet - in that case helmfile correctly propogates it down to helm:

# helmfile snippet to get KUBE_API_VERSIONS from ArgoCD  build env and pass down to helm
---
apiVersions:
{{- range $apiV := ( requiredEnv "KUBE_API_VERSIONS" | split "," ) }}
- {{ $apiV }}
{{- end }}


#helmfile debug output running snippet above and passing KUBE_API_VERSIONS as if it was ArgoCD runtime:

KUBE_API_VERSIONS="v1,apiregistration.k8s.io/v1,apiregistration.k8s.io/v1beta1,extensions/v1beta1,apps/v1,cert-manager.io/v1alpha3,..."  helmfile --debug -l chart=cert-issuer template

...
second-pass rendering result of "common.yaml.part.0":
...
16: 
17: apiVersions:
18: - v1
19: - apiregistration.k8s.io/v1
20: - authorization.k8s.io/v1beta1
21: - autoscaling/v1
22: - autoscaling/v2beta1
23: - autoscaling/v2beta2
24: - batch/v1
25: - batch/v1beta1
26: - certificates.k8s.io/v1
27: - certificates.k8s.io/v1beta1
28: - networking.k8s.io/v1
29: - networking.k8s.io/v1beta1
30: - apiregistration.k8s.io/v1beta1
31: - policy/v1beta1
32: - rbac.authorization.k8s.io/v1
33: - rbac.authorization.k8s.io/v1beta1
34: - storage.k8s.io/v1
35: - storage.k8s.io/v1beta1
36: - admissionregistration.k8s.io/v1
37: - admissionregistration.k8s.io/v1beta1
38: - apiextensions.k8s.io/v1
39: - apiextensions.k8s.io/v1beta1
40: - scheduling.k8s.io/v1
41: - extensions/v1beta1
42: - scheduling.k8s.io/v1beta1
43: - coordination.k8s.io/v1
44: - coordination.k8s.io/v1beta1
45: - node.k8s.io/v1beta1
46: - discovery.k8s.io/v1beta1
47: - catalog.cattle.io/v1
48: - crd.projectcalico.org/v1
49: - monitoring.coreos.com/v1
50: - acme.cert-manager.io/v1alpha3
51: - acme.cert-manager.io/v1alpha2
52: - apps/v1
53: - cert-manager.io/v1alpha3
54: - cert-manager.io/v1alpha2
55: - cluster.cattle.io/v3
56: - management.cattle.io/v3
57: - metrics.k8s.io/v1beta1
58: - events.k8s.io/v1
59: - events.k8s.io/v1beta1
60: - authentication.k8s.io/v1
61: - authentication.k8s.io/v1beta1
62: - authorization.k8s.io/v1
...
Templating release=cert-issuer, chart=<our-org-repo>/cert-issuer
exec: helm template cert-issuer <our-org-repo>/cert-issuer --version 1.2.1 --api-versions v1 --api-versions apiregistration.k8s.io/v1 --api-versions authorization.k8s.io/v1beta1 --api-versions autoscaling/v1 --api-versions autoscaling/v2beta1 --api-versions autoscaling/v2beta2 --api-versions batch/v1 --api-versions batch/v1beta1 --api-versions certificates.k8s.io/v1 --api-versions certificates.k8s.io/v1beta1 --api-versions networking.k8s.io/v1 --api-versions networking.k8s.io/v1beta1 --api-versions apiregistration.k8s.io/v1beta1 --api-versions policy/v1beta1 --api-versions rbac.authorization.k8s.io/v1 --api-versions rbac.authorization.k8s.io/v1beta1 --api-versions storage.k8s.io/v1 --api-versions storage.k8s.io/v1beta1 --api-versions admissionregistration.k8s.io/v1 --api-versions admissionregistration.k8s.io/v1beta1 --api-versions apiextensions.k8s.io/v1 --api-versions apiextensions.k8s.io/v1beta1 --api-versions scheduling.k8s.io/v1 --api-versions extensions/v1beta1 --api-versions scheduling.k8s.io/v1beta1 --api-versions coordination.k8s.io/v1 --api-versions coordination.k8s.io/v1beta1 --api-versions node.k8s.io/v1beta1 --api-versions discovery.k8s.io/v1beta1 --api-versions catalog.cattle.io/v1 --api-versions crd.projectcalico.org/v1 --api-versions monitoring.coreos.com/v1 --api-versions acme.cert-manager.io/v1alpha3 --api-versions acme.cert-manager.io/v1alpha2 --api-versions apps/v1 --api-versions cert-manager.io/v1alpha3 --api-versions cert-manager.io/v1alpha2 --api-versions cluster.cattle.io/v3 --api-versions management.cattle.io/v3 --api-versions metrics.k8s.io/v1beta1 --api-versions events.k8s.io/v1 --api-versions events.k8s.io/v1beta1 --api-versions authentication.k8s.io/v1 --api-versions authentication.k8s.io/v1beta1 --api-versions authorization.k8s.io/v1 --namespace cert-manager --values /var/folders/j8/4x2hx6_13sq78dh_2cqkz99m0000gq/T/values473072862 --debug
helm:hxKQF> install.go:172: [debug] Original chart version: "1.2.1"
helm:hxKQF> install.go:189: [debug] CHART PATH: /Users/vgolubev/Library/Caches/helm/repository/cert-issuer-1.2.1.tgz
helm:hxKQF> ---
# Source: cert-issuer/templates/self_signed_issuer.yaml
apiVersion: cert-manager.io/v1alpha3
kind: ClusterIssuer
metadata:
  name: kfrfpr-issuer-ca
spec:
  ca:
    secretName: ca-secret

Currently, i have to add that snippet as common behaviour to all helmfiles even though using the metnioned script. Onec issue 457 is fixed and if --args behaviour is preserved, the argocd-helmfile script will automatically inject those params and there will be no longer need to copy-paste that snippet when api-versions dynamic templating is required.

@travisghansen
Copy link

Awesome thanks for the contribution and the snippet!

I don’t think the intent is to remove the functionality of —args entirely but rather remove it as a global option from helmfile and only support it for subcommands that make sense (or rather have subcommand specific implementations).

@mumoshu
Copy link
Collaborator Author

mumoshu commented May 11, 2021

@travisghansen Thanks for the comment! Yeah, something like that. As long as args makes clear sense in a sub-command, I'm fine to preserve it.

@GolubevV Your usecase makes sense a lot. But shouldn't we just enhance Helmfile to support your use-cases natively? Do you need to set apiVersions via envvars dynamically on helmfile run, and is that all you need?

What if helmfile.yaml's apiVersion automatically populated from KUBE_API_VERSIONS, without writing the template snippet you've shared?

@travisghansen
Copy link

travisghansen commented May 12, 2021

If I understand your suggestion about the env var that would actually work quite nicely yes. May want to make that an argument to the template sub command to allow folks to opt-in/out.

EDIT: or do something like helm secrets does and automatically alter behavior based on detection of running in argocd - https://github.com/jkroepke/helm-secrets#argocd-support

@GolubevV
Copy link

GolubevV commented May 13, 2021

I agree that env var can help but question is - what should be the name of that var?
Should it be aligned by default with the name used by ArgoCD e.g KUBE_API_VERSIONS ?
Or probably make the name of that env var configurable?

In that regard, I like the idea of @travisghansen to implement runtime detection to be able to automatically propagate/map some ArgoCD build variables down to helmfile internal variables, like:

  • ARGOCD_APP_NAMESPACE
  • KUBE_VERSION
  • KUBE_API_VERSIONS

It can be one of those improvements for better integration that we want to track in #1812

BTW, In defence of the --args parameter, i continue to find more and more use-cases when helmfile is used as templating engine. Recently, it was very useful for propagating the parameter --skip-testsdown to helm to exclude Helm chart tests manifests from output. But maybe there is already native support for such behaviour in helmfile?

@mumoshu
Copy link
Collaborator Author

mumoshu commented May 14, 2021

@GolubevV Hey!

Or probably make the name of that env var configurable?

Probably.

ARGOCD_APP_NAMESPACE
KUBE_VERSION
KUBE_API_VERSIONS

All these seem to make sense to me.
But I'd perhaps prefer it being an opt-in feature. What if helmfile template --argocd resulted in reading those envvars appropriately?

Also, would you mind writing a more detailed feature request with how you'd exactly like those envvars to be mapped to helmfile settings, and link it from #1812 for reference?

That way, designing/implementing the feature would be more approachable and feasible. Currently, everyone writes many ideas about ArgoCD integration in various places, which is starting to kill me

BTW, In defence of the --args parameter, i continue to find more and more use-cases when helmfile is used as templating engine. Recently, it was very useful for propagating the parameter --skip-testsdown to helm to exclude Helm chart tests manifests from output. But maybe there is already native support for such behaviour in helmfile?

As I've been repeatedly saying to everyone, I'd like everyone to submit feature requests to add necessary and concrete flags to helmfile-template. That said, writing a feature request to add --skip-tests to helmfile-template does make sense.

I'd remove --args only after I'm confident we have all the necessary flags added.

I consider the use of --args is just a work-around, not even a necessary evil.

@mumoshu
Copy link
Collaborator Author

mumoshu commented May 18, 2021

Another voice on args #1853 (comment)

@yujunz
Copy link
Contributor

yujunz commented Jun 1, 2021

As I've been repeatedly saying to everyone, I'd like everyone to submit feature requests to add necessary and concrete flags to helmfile-template.

It won't be easy to have an exhaustive list of necessary and concrete flags since the upstream helm will evolve. Currently I rely on --args to pass --kube-version and --include-crds and etc and it works fine on CLI.

There is bug in state file directive though.

helmDefaults:
  args:
    - "--set k=v"

will produce

ARGS:
  0: helm (4 bytes)
  1: template (8 bytes)
  2: mysql (5 bytes)
  3: bitnami/mysql (13 bytes)
  4: --namespace (11 bytes)
  5: mysql (5 bytes)
  6: --debug (7 bytes)
  7: set (3 bytes)
  8: k=v (3 bytes)

It seems stripping -- from the string and there is no way to apply args to specific command only.

My 2 cents:

  • +1 for removing the buggy args from state file
  • -1 for removing --args from CLI unless there is an alternative solution for passing arbitrary arguments to helm as is.

@mumoshu
Copy link
Collaborator Author

mumoshu commented Jun 1, 2021

@yujunz Hey! Thanks for the feedback. But I'm feeling like I'm repeating the same call again and again.

alternative solution for passing arbitrary arguments to helm as is.

This turns out to be impossible to me. If anyone has some time to speculate it and able to come up with a concrete idea on how it should look like, please help me.

The hardest part if that we really don't have a concrete spec around how current args SHOULD work and even the understanding of how it's currently working, which makes it very hard to decide where it should go.

Someone uses it in a way, another uses it in another way. Everyone brings in various ideas. I try to summarize, balance all and try to come up with the best idea to move forward. And that turned out to be just removing args while adding concrete flags/config keys for necessary helm flags.

@yujunz
Copy link
Contributor

yujunz commented Jun 1, 2021

It is pretty common to pass args as is in tool chains. For example bazel run will pass whatever followed by a bare -- to the target binary. See example in https://stackoverflow.com/questions/54175366/bazel-run-passing-main-arguments

It doesn't have to be called --args if that is confusing some users. We just need something to distinguish between options/flags for helmfile and helm commands. -- is a common separator for such purpose.

After all, this will not conflict with concrete flags/config which can be parsed by helmfile but complement them for flexibility and extensibility.

Does it make sense?

@mumoshu
Copy link
Collaborator Author

mumoshu commented Jun 1, 2021

@yujunz Sorry but no. I raed bazel run to pass the args to the your main application that bazel built. bazel runs a single application to pass args to. That makes sense. Helmfile commands and helmfile commands don't map 1:1.

For example helmfile diff runs helm repo add, helm template, helm fetch, helm dep build, helm dep up, and finally helm diff, depending on the scenario and the configuration. "Perhaps" helmfile --args $ARGS diff is somehow passing args to helm diff only today? But how could I pass to it helm template only so that I can inject some args for use by chartify? That's the problem I have with args. I know it accidentally works as in a way someone expects today, but not for another. It's fundamentally flawed. The fact that it works sometimes doesn't negate the fact that it is fundamentally flawed.

@yujunz
Copy link
Contributor

yujunz commented Jun 2, 2021

For example helmfile diff runs helm repo add, helm template, helm fetch, helm dep build, helm dep up, and finally helm diff

TIL.

I was expecting --args to apply to the main helm command only (normally the last one). IMO that would cover most cases and seems an acceptable trade-off of implementation complexity.

For arguments specific to each steps, either implement it as what you have suggested

I'd like everyone to submit feature requests to add necessary and concrete flags to helmfile-template.

Or with specific flags ,e.g. --diff-args, --repo-add-args, which might be overkilled IMO.

What do you think? @mumoshu

@mumoshu
Copy link
Collaborator Author

mumoshu commented Jun 2, 2021

I was expecting --args to apply to the main helm command only (normally the last one). IMO that would cover most cases and seems an acceptable trade-off of implementation complexity.

@yujunz Thanks. Yeah, maybe. I understand that it might cover most cases.

Or with specific flags ,e.g. --diff-args, --repo-add-args, which might be overkilled IMO.

Yeah, this might work too. The only concern for me is the possibility that people start asking various questions for various combinations of yaml settings and --foo-args combinations that I have to answer, which is a maintenance burden :)

Maybe --args will remain as-is. I have no right to remove anything that people are still using, anyway. Hopefully, I can just tolerate it.

@dudicoco
Copy link
Contributor

dudicoco commented Jun 3, 2021

@mumoshu if --args is to be removed all of the helm possible arguments should be ported into helmfile, otherwise it breaks current functionality.

I agree that --args should probably stay as it is.

We for example are using the --dryrun and --debug args

@mumoshu
Copy link
Collaborator Author

mumoshu commented Jun 3, 2021

Thanks, everyone!

@mumoshu mumoshu closed this as completed Jun 3, 2021
@travisghansen
Copy link

@mumoshu I think it might still be good to address array style parameters and what should be done if anything about that.

@mumoshu
Copy link
Collaborator Author

mumoshu commented Jun 4, 2021

@travisghansen Hey! Sry but what was "array style parameters` thing? Where did we discuss about that before

@travisghansen
Copy link

I don’t recall which issue it landed on but its the problem where specifying the same arg name multiple times results in only the last variant of it being passed down to helm (ie: —api-versions).

@mumoshu
Copy link
Collaborator Author

mumoshu commented Jun 4, 2021

@travisghansen Thanks! Sry but I can't recall either. Would you mind creating a dedicated issue for it, with a short concrete example of how you would use it, so that I can try to implement and test it. It's currently too vague for me to work on.

@travisghansen
Copy link

@mumoshu start here I guess: #1060 (comment)

@james-mchugh
Copy link

First off, thank you for the awesome tool!

Sorry to comment on an old issue, but what is the current expected behavior of --args? I'm currently running 0.139.9 on Mac, and I noticed that if I run the following command:

helmfile template --args="--skip-tests"

It passes the --skip-tests flag to helm dependency build, resulting in the following error:

in ./helmfile.yaml: in .helmfiles[1]: in helmfile.yaml: [building dependencies of local chart: command "/usr/local/bin/helm" exited with non-zero status:

PATH:
  /usr/local/bin/helm

ARGS:
  0: helm (4 bytes)
  1: dependency (10 bytes)
  2: build (5 bytes)
  3: ../helm-charts/my-chart (37 bytes)
  4: --skip-tests (12 bytes)

ERROR:
  exit status 1

EXIT STATUS
  1

STDERR:
  Error: unknown flag: --skip-tests

COMBINED OUTPUT:
  Error: unknown flag: --skip-tests]

But if I pass in the --selector option, the command runs as expected and passes the --skip-tests flag to helm template.

I haven't tested this on 0.140.0 yet, but has anyone else seen similar behavior? Is this expected?

@mumoshu
Copy link
Collaborator Author

mumoshu commented Feb 15, 2022

We have no well-designed way to specify which helm commands receive which args from helmfile's --args, so it may work. And that's why I don't recommend using --args and why I tend to ask people to submit specific feature requests for your use-cases that you tried to address with --args.

@AssafKatz3
Copy link

@mumoshu I think that a better option is to transform args to go-template, similar to hooks, with two parameters:

  • command
  • arguments

It will allow any arguments while keeping backward compalibity.

@mumoshu
Copy link
Collaborator Author

mumoshu commented Feb 16, 2022

@AssafKatz3 Thanks. How would it look like in helmfile.yaml? Perhaps something like the below?

releases:
- name: myapp
  chart: ./mychart
  commands:
  - name: ["diff", "upgrade"]
    args:
    - # some helm diff upgrade flags...
  - name: ["template"]
    args:
    - --post-render=....

Assuming the above isn't too far away from what you imagine, I'm wondering if how would declare customization on commands unrelated to releases, like helm repo add and helm dep build, etc.

@dudicoco
Copy link
Contributor

@mumoshu I think that a better option is to transform args to go-template, similar to hooks, with two parameters:

  • command
  • arguments

It will allow any arguments while keeping backward complicity.

That wouldn't work for our use case, we are passing --args dryrun on PR checks and we don't pass it when helmfile runs after merging the PR to the base branch.

@AssafKatz3
Copy link

Hi @mumoshu ,
We need options for all command helm repo add since helmfile doesn't support all its options. But I think that what you sugggest is good enough for first phase, the options for other command tend to be globally and therefore can set by enviornment variables.
In long run, such command should be under same key as relevant parameters, for helm repo add this means under specific repository and for per release for helm dep build.

@AssafKatz3
Copy link

Hi @dudicoco,
It will support your use case - you will be able to use something like:
{{ if env "CHECK_PR" }}--args dryrun{{end}}
And set CHECK_PR enviornment variable only for PR checks.

@dudicoco
Copy link
Contributor

Hi @dudicoco, It will support your use case - you will be able to use something like: {{ if env "CHECK_PR" }}--args dryrun{{end}} And set CHECK_PR enviornment variable only for PR checks.

Thanks @AssafKatz3.
It's still problematic since we are also running helmfile locally with and without the dry-run flag and setting an env var every time would be more of a hassle.

Personally I don't see a problem with just keeping --args rather than keeping track of every single flag helm uses and supporting it directly within helmfile.

terragrunt does the same thing by letting you pass the same command + args you would pass to terraform normally.

@AssafKatz3
Copy link

Hi @dudicoco,
You can use not and check if some enviornment variables isn't existed.

@talonx
Copy link

talonx commented Mar 25, 2022

I was looking for a way to run a custom post-renderer script in my helmfiles and came across #1530 and then this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants