travisghansen / argo-cd-helmfile

Integration between argo-cd and helmfile
MIT License
213 stars 55 forks source link

separate helmfile cache for each app by default #21

Closed tpatrascu closed 2 years ago

tpatrascu commented 2 years ago

Hello,

This PR is related to the helmfile cache cleanup option. If we have an argo-cd instance with multiple apps running helmfile cache cleanup, one of them might wipe out the cache for another, and to prevent this we should make it safe by default. I have also added a better helm cache configuration option, as by default helm and helmfile both use XDG_CACHE_HOME and maybe people don't want their helm cache path to be automatically changed.

Does this look OK to you? Or maybe it should be applied only when the HELMFILE_CACHE_CLEANUP option is activated, but it would make the code more complicated and it shouldn't affect very much and can be overridden.

Thanks!

travisghansen commented 2 years ago

Is XDG_CACHE_HOME not based on HOME? Where are the cache files for helmfile? I would like to take a look and see what is being cached etc to see how this is being managed.

tpatrascu commented 2 years ago

XDG_CACHE_HOME can be set to whatever directory, doesn't need to be in HOME, of course, it is standard so could be used by other apps, but is the only way to control helmfile cache location. From what I know the only things being cached there by helmfile are remote helmfiles and remote values:

https://helmfile.readthedocs.io/en/latest/#loading-remote-environment-values-files

https://github.com/helmfile/helmfile/blob/main/examples/remote/helmfile.yaml

It will clone the whole git repo to which you are pointing and use the helmfiles or values from that path in the cache. When the helmfile cache cleanup command is run, it will just delete everything from that cache directory. Otherwise the cache will never be updated.

Clearing the cache is useful if you point to a branch, mostly for dev environments.

travisghansen commented 2 years ago

Understood it can be set to whatever you want, but we already set a per-app HOME directory so each app should be pretty isolated anyhow..

tpatrascu commented 2 years ago

Right, I didn't notice that, I only saw HELM_DATA_HOME being set. So I guess everything should be ok as is.

Thanks!

travisghansen commented 2 years ago

Well, as long as those cache dirs will actually read HOME yes. If you run into issues just open it back up and we'll get a solution!