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

Missing command - dependency update #26

Closed
xeor opened this issue Jan 9, 2018 · 16 comments
Closed

Missing command - dependency update #26

xeor opened this issue Jan 9, 2018 · 16 comments

Comments

@xeor
Copy link

xeor commented Jan 9, 2018

I've started using helmfile, and after a couple of days of use, I am missing a way to do helm dependency update. Should it be a part of helmfile? And included in sync

@mumoshu
Copy link
Collaborator

mumoshu commented Feb 27, 2018

@xeor Sounds reasonable!

But out of curiosity, would you mind sharing me how do you separate uses between helm and helmfile when you are able to use the helm's umbrella chart pattern like you do?

Can't you just transform your helmfile into a single umbrella helm chart so that perhaps you can just use helm status against it?

@xeor
Copy link
Author

xeor commented Feb 28, 2018

This was only for my personal charts.. The umbrella is not that big, but I got multiple "umbrellacharts".
I don't really like to put all the variables inside the helmfile. The helmfile for me is more or less a collection of charts. If it's an upstream chart, and it got just a couple of values to change. I might put the values in the helmfile. Otherwise, I like not to..

@mumoshu
Copy link
Collaborator

mumoshu commented Mar 18, 2018

@xeor Thanks for the clarification!

I've iterated on your explanation and my current/possible use-cases of helmfile.
And now I believe your use-case is ABSOLUTELY valid!

Would it be good for you if helmfile sync just runs helm dependency update before updating the releases? Would there be any bad side-effect of doing so?

@mumoshu
Copy link
Collaborator

mumoshu commented Mar 18, 2018

cc/ @sstarcher Would you also be happy with this change?

@mumoshu
Copy link
Collaborator

mumoshu commented Mar 18, 2018

@xeor FWIW, I also believe a small enhancement to helmfile like #28(which would also support #49) would allow helmfile to support what we can't easily do with umbrella charts. Appreciate your comments on those two issues, too!

@sstarcher
Copy link
Contributor

sstarcher commented Mar 18, 2018

I could see this being useful for local file paths. It should detect it's going against a local filepath and run dep update.

@mumoshu
Copy link
Collaborator

mumoshu commented Mar 26, 2018

@sstarcher Thx for the response!

How about just improving helmfile sync to also run helm dep update when the release's chart: references a local chart?

I'm curious about the usefulness of only running dep update or even repo update(already implemented as helmfile repos but I have never got to use that myself) through helmfile.

@cmeury
Copy link
Contributor

cmeury commented Apr 2, 2018

@xeor Would you mind giving this a test run?

@fsilberstein
Copy link
Contributor

This merged is working but it's bringing a new issue. Previously, when you wanted to wait for the readiness of your pods, you just had to pass args flag with "--wait".
Now this args flags are also passed to the update dependencies command.I can do a PR to fix it if needed.

@mumoshu
Copy link
Collaborator

mumoshu commented Apr 4, 2018

@fsilberstein Thanks for the good catch! Ah, so we seem to set helm extra args too early?. A PR would be much appreciated anyway.

@fsilberstein
Copy link
Contributor

@mumoshu Exactly, we apply args too early, I just inverted the lines of code, that's all. My PR is ready for your review :) Thank you

@mumoshu
Copy link
Collaborator

mumoshu commented Apr 4, 2018

Closing this as resolved via #85. Thanks for the PR @fsilberstein!

@mumoshu mumoshu closed this as completed Apr 4, 2018
@mxey
Copy link
Contributor

mxey commented Apr 16, 2018

I just run into this accidentally, when my helmfile sync failed. I see two problems with this feature:

  • helm dependency update fails if the chart has no requirements.yaml
  • Doing this implicitly as part of the sync command breaks the declarative nature of Helmfile and umbrella charts for me. I want to control the requirements.lock and decide when I update the charts I use. I think this is bad surprise from what people expect from the “sync” command because it should just apply the local state to the cluster, not change the local state. If you run Helmfile in CI, like I do, the changed requirements.lock files won't even be saved.

@cmeury
Copy link
Contributor

cmeury commented Apr 16, 2018

If you only want to upgrade --install, you could you just run the charts command? I understand sync as some sort of "all in one" step.

@mxey
Copy link
Contributor

mxey commented Apr 16, 2018

Yeah, I actually just realized charts is probably better for me. Although I still think it would be sensible to have separate commands, like sync and update.

@mumoshu
Copy link
Collaborator

mumoshu commented Jan 24, 2019

From the discussion made in #415, I'm considering to add helmfile dependency update(#450), as well as changing helmfile to run dependency buidl rather than dependency update in helmfile sync, diff, apply and template.

Does that make sense to you?

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

No branches or pull requests

6 participants