Helmfile: Proposal: `helmfile sync --prune` removes any charts not declared

[lswith]

This idea comes from the kubectl apply -f --prune where kubectl deletes any resources that aren't referenced.
Helmfile should have an option for the sync command that checks what helm charts are deployed, and then compares that to the helmfile. If there are charts that aren't referenced by the helmfile, they are deleted (possibly purged) from helm.

This would be extremely useful for keeping a specific state in the cluster.

[sstarcher]

I think that's a great idea, but with the implementation of helmfile.d where we have seperate charts and the possibility that users will have separate helmfiles for the same cluster that could also be dangerous.

I think the largest issue is how helmfile.d is implemented

[mumoshu]

Yeah, sounds like a great idea! How would you expect it to work when we run:

1. helmfile -f myhelmfile.yaml sync
2. helmfile -f myhelmfile.yaml -l foo=bar sync
3. helmfile -f myhelmfile.d/ sync
4. helmfile -f myhelmfile.d -l foo=bar sync

My initial thoughts:

For 1., remove all the releases stored in the cluster but not exists in the helmfile
For 2., remove releases w/ the label foo=bar, stored in the cluster, but not exists in the helmfile
For 3., delete all the releases stored in the cluster, but missing corresponding helmfile(s) in the directory. Then do 1. for each helmfile in the directory.
For 4., do 2. for each helmfile in the directory

Detailed specs would include:

• unnecessary releases are deleted by running helm delete
• renaming of helmfile(s) in a helmfile.d will result in recreation of all the releases stored in the helmfile(s)
• the name of helmfile.d and the name of helmfile are stored in the cluster as a custom resource, so that we can see which releases are deleted from helmfile.d and helmfile(s)

[mumoshu]

Or should we rethink how the helmfile.d feature works, by any chance?

[sstarcher]

I'm not following this statement

renaming of helmfile(s) in a helmfile.d will result in recreation of all the releases stored in the helmfile(s)

Why would renaming a helmfile in helmfile.d effect the release names?

And if this is added it should certainly be flag and not the default behavior.

[lswith]

I’d say, take the same flow that kubectl apply -f —prune takes.

Selectors wouldn’t work for this feature.

Any charts not referenced in helmfile.d would be deleted.

[sstarcher]

In that case it would likely make sense for this to be a seperate command like helmfile prune vs a flag on helmfile sync/charts

[lswith]

The only trouble with it being “prune” is that it implies that it won’t create new releases. This command should just “sync” the exact helm charts you have to a “specific” tiller.

[lswith]

While also cleaning up unused releases.

[sstarcher]

The issue with leveraging the current sync is you now have a conflict with the selectors.

[mumoshu]

I'm not following this statement

renaming of helmfile(s) in a helmfile.d will result in recreation of all the releases stored in the helmfile(s)

Why would renaming a helmfile in helmfile.d effect the release names?

I cannot be sure yet but believe that it is because you need something like a helm release stored in the k8s cluster, for each helmfile in a helmfile.d. Otherwise we can't take diff between the current state and the desired state(in a helmfile yaml).

helm uses the name of a release to identify the current state, compares it with the desired state in order to remove unnecessary k8s resources on helm upgrade(afaik). I thought that we'd need to do it for helmfile too. Then, my question was - how to "identify the current state" that corresponds to the desired state? My quick guess was to use the name of each helmfile contained in the helmfile.d, hence renaming results in recreation of all the releases in the helmfile.

[mumoshu]

@sstarcher I'm not completely opted to implement this into sync. But anyway, I tried to address the conflict with the proposed feature vs selector in the number 2 and 4 of my proposed design. To be short, -l foo=bar should restrict which releases are installed/upgraded/deleted.

Suppose the "current" stated stored somehow inside your cluster look like:

- releases
  - name: foo
     labels:
        kind: foo
  - name: bar
       kind: bar

Whereas your "desired" state in helmfile.yml looks like:

releases:
- name: foo
  labels:
    kind: foo

helmfile -f helmfile.yml sync or helmfile -f helmfile.yml -l kind=bar should delete the released named bar only. On the other hand, helmfile -f helmfile.yml -l kind=foo should do nothing. This is an example of the number 2 of my proposed design above.

[lswith]

I’m actually happy for you guys to hash out this feature. The only use case is that I’d like CD for my helm configurations and using a helmfile is easily the best tool for doing that. Adding the ability to sync a desired state to the cluster will give me that

[lswith]

Deletes, updates and installs all in one

[mumoshu]

Yeah! I believe that I understand it, because I have the same use-case as yours. So I'm trying, but I still need your help to verify that my proposed solution work 😄

[sstarcher]

I see, what I imagined the design would be would, not to only remove what was in the helmfile previously, but to remove any resource that was not in the helmfile. In the second situation you would not need to store anything in k8s as you would just remove everything that was not currently in the file.

If you are storing things in k8s you also run into race conditions and require locking. I believe that might be getting more complicated then we really want. Unless we want to develop a server side helmfile component.

[sstarcher]

Yep, I 100% see value in this we just need to find a clean way to implement this.

[mumoshu]

@sstarcher Thanks for the comment! I'm still trying to figure out either.

you would just remove everything that was not currently in the file.

Everything in the cluster? Or perhaps "everything in the namespace"?

For the former, I'm wondering if it implies that we can have at most one helmfile per cluster. Otherwise helmfile sync --delete against two separate helmfiles would try to delete releases from each other?

[osterman]

I think it would be sweet in principal to have the helmfile sync --delete feature, but for all the aforementioned arguments struggle to see how it would work sanely with selectors or without shared state. It feels very dangerous. One possibility is to make --delete mutually exclusive with --selectors; that is, you cannot use it with selectors.

We have dozens of services in our helmfiles. It's become more of a "linux distribution" of cloud native apps for kubernetes. We use helmfile --selector chart=grafana sync just the way you would do something like apt-get install grafana on a classic linux distro.

Other ideas: (not well thought out)

• introduce a new verb called upsert which works like the current sync command but does (upgrades and installs). Then repurpose sync to work the way others have suggested.
• treat --delete like a --selector, but for charts that should be deleted.
  helmfile --delete install=false sync
• combination of the above two suggestions...
  helmfile --upsert install=true --delete install=false sync

[manics]

One of the things I really like about Helmfile is that all state is in helmfile.yaml, so it's easy to have multiple users maintaining a cluster, and it's also easy to switch between using Helmfile and Helm.

For example

• I find Terraform very nice to use as an individual, but to support a team you need to synchronise the tfstate file between all members. This adds another level of infrastructure and is one of the reasons we're not using it.
• Being able to use Helm directly is useful when rolling out Helmfile to an existing cluster, especially during an evaluation period where people are unsure of adding yet another tool to the deployment chain.

So, my vote would be to do everything using helmfile.yaml/helmfile.d, and to avoid storing additional state, whether locally or in the cluster.

[sstarcher]

ya, the additional state concerns me and I don't believe it's necissary. Yes, currently helmfile does not merge multiple files in helmfile.d together, but it could easily be implemented to keep track of the releases that are managed by helmfile.d and to accumulate them as it goes through. Once it processes all of the files it could delete any releases that are not in any of the files in helmfile.d.

This is a fairly trivial feature to implement and would cover the case to assure everything in helm is also in helmfile.

[mumoshu]

@sstarcher Interesting! You mean not to merge helmfile.yaml files into a single yaml file, but rather to load releases in context of each helmfile.yaml, and then merge the list of resulting releases, right?

[sstarcher]

ya, essentially we loop over all of the helmfiles. The function that does that could return the list of releases in the helmfile. And the overarching loop could accumulate that list. Once all helmfiles were processed you would have a full list of everything that is being managed.

At that point you could purge.

[mumoshu]

And we calculate a releases list per tiller namespace?

[sstarcher]

ahh namespaced tillers do make that more difficult. That would certainly have to go into the calculation for the rollup.

[mumoshu]

Got it, thanks for the suggestion and the confirmation! That should indeed work.

Mind if I altered the flag to --prune after kubectl apply --prune --all?
To me who is a little familiar with kubectl apply --prune, --delete and --purge sounded more like deleting/purging all the releases, rather than ones that are disappeared from helmfiles, which conflicts with the meaning of sync.

[sstarcher]

Sounds great

[mumoshu]

So for consistency with kubectl apply --prune [-l foo=bar] [--all], helmfile apply --prune --all should remove all releases that resides in all the targeted tiller namespaces. On the other hand, helmfile apply --prune -l foo=bar should remove all releases whose labels contain foo=bar, that resides in the all targeted tiller namespaces.

What we can implement as of today is the former, helmfile apply --prune --all only. That's because we have no way to label releases.

[mumoshu]

helmfile apply --prune -l K=V would benefit from https://github.com/helm/helm/issues/4639. It isn't a requisite feature-wise, but good to have to make the command complete faster.

[sstarcher]

So are we to assume that tiller only has access to the namespace it's in? Tiller can easily control all namespaces, just the one it's in, or x specific namespaces depending on RBAC.

Also I don't understand helmfile apply --prune -l foo=bar. So you are saying it won't do what the normal --prune does, but it will look at the helmfile and find the labels of foo=bar and remove those? That seems more like a helmfile delete -l foo=bar. prune is for syncing helmfile and kubernetes using a label in this context is very different and seems highly confusing.

[mumoshu]

So are we to assume that tiller only has access to the namespace it's in?

Nope. I assume tiller has access to all the releases that are allowed by the serviceaccount associated to the tiller instance.

Assume we have 2 releases mybackend and myfrontend managed by the tiller at tiller-system:

$ kubectl --namespace tiller-system get configmap
NAME                                         DATA      AGE
mybackend.v1                              1         4d
myfrontend.v1                              1         4d

And also assume mybackend.v1 is labeled tier: backend and myfrontend.v1 is labeled tier: frontend.

Let's say mybackend.v1 has corresponding k8s resources in the ns backend, as it was installed by helm install --tiller-namespace tiller-system --namespace backend charts/backend --name mybackend. And myfrontend's resources in frontend as similar as the backend.

If we removed both releases from helmfile.yaml, helmfile sync --prune (or helmfile sync --prune --all if we keep consistency with kubectl) would remove both. On the other hand, helmfile sync --prune -l tier=backend would remove only mybackend. This allows you to prune unneeded releases even when you use selector all over the places.

Suppose you have every release labeled appropriately, helmfile sync --prune --all is equivalent to helmfile sync --prune -l tier=backend + helmfile sync --prune -l tier=frontend.

[sstarcher]

• tiller namespace example now makes sense as you are relying on a listing of the tiller resources and comparing that to helmfile.
• For the labeling how will you know what the label is if it does not exist in helmfile? Or are you refering to a different label that is not the helmfile label?

[mumoshu]

For the labeling how will you know what the label is if it does not exist in helmfile?

There should be no way to know the labels in that case.

So, helmfile sync --prune -l foo=bar against the above example results in noop, as the helmfile.yaml contains no releases matching foo=bar and the tiller-system namespace doesn't contain any releases matching foo=bar.

Does this sound ok to you? I'm afraid of missing something!

[sstarcher]

Sounds good

[Morriz]

Hi @mumoshu, is there any activity here? Pruning non-matching releases would be amazing

[Morriz]

@mumoshu Pretty please? Or can you point me in another direction to achieve removal of charts not found in the releases?

[mumoshu]

@Morriz This is not going to be implemented. Instead, set releases[].installed to false and run either helmfile sync or helmfile apply to remove the releases in the helmfile.yaml but with installed: false.

After that you can safely and freely remove the releases from the helmfile.yaml.

[mumoshu]

The above is much better in terms of reliability as we only need the information provided by the helmfile.yaml itself to make it work.

Something like --prune requires additional external state and convention to be handled, which makes helmfle more complex.

[Morriz]

Ah, I see. Too bad. We generate state dynamically based on a list of teams, and once these are removed, their team helm chart is removed as well. Might have to come up with a maintenance task that removes charts based on a diff ;)

On 15 Apr 2020, at 12:46, KUOKA Yusuke [e-mail] wrote:

@Morriz https://github.com/Morriz This is not going to be implemented. Instead, set releases[].installed to false and run either helmfile sync or helmfile apply to remove the releases in the helmfile.yaml but with installed: false.

After that you can safely and freely remove the releases from the helmfile.yaml.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/roboll/helmfile/issues/194#issuecomment-613964672, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHF6KNDHG4GJY7S43WJFZLRMWF7LANCNFSM4FKXX56Q.

[mumoshu]

@Morriz That makes sense. Is the list of teams fully dynamic? I mean, would there be a "maser" list of teams? If so I think you could merge the mater list and the "current" list to produce all the possible releases[] where only releases for the "current" teams would have installed: true and others have installed: false.

[mumoshu]

@Morriz Also I'm not really sure what you would need.

What if helmfile apply --prune listed all the releases in all the namespaces and automatically removed releases that doesn't exist in the specified helmfile? Does it work for you?

Wouldn't it accidentally remove releases managed by other helmfile.yaml(s) or installed manually?

[mumoshu]

// I'm biased. I even avoid using kubectl apply --prune because it always has some potential of unintended deletions.

[Morriz]

We automate everything in a predictable fashion and need this. So yes, something like helmfile apply --prune would be awesome.

[Morriz]

@Morriz Also I'm not really sure what you would need.

What if helmfile apply --prune listed all the releases in all the namespaces and automatically removed releases that doesn't exist in the specified helmfile? Does it work for you?

Wouldn't it accidentally remove releases managed by other helmfile.yaml(s) or installed manually?

Why do you say

in the specified helmfile

?
When you could just use the default loading mechanism when not specified?

[mumoshu]

@Morriz Let me change the question as I now know that everything is managed by helmfile in your env.

So - If I implemented it in a naive way, helmfile apply --prune would prune releases for each sub-helmfile run.

Let's say you had two helmfile.yaml files, one for releases in namespace1 and another for releases in namespace1 and namespace2:

helmfiles:
- helmfile1.yaml
- helmfile2.yaml

helmfile apply --prune would firstly upgrade releases in helmfile1.yaml and then remove unnecessary releases. But the "unnecessary releases" for helmfile1 may include previously installed by helmfile2.yaml. Similar things happen for helmfile2.yaml as well.

Does this mean I need to somehow aggregate all the releases and namespaces across all the sub-helmfiles, and prune unnecessary releases only at the end of the whole helmfile apply --prune run?

[Morriz]

I understand that helmfile has not kept state as to what chart came from which helmfile, right? Or does it add an annotation? Then it has all the bookkeeping to match what was removed. Just query all the charts for where they came from, and then prune what not exists in the source.

But that might be a future thing. For now it would be nice to get what you just mentioned in your last comment ;)

[mumoshu]

It seems much difficult than I thought initially.

How should helmfile diff tell things being pruned? Currently helmfile diff iterates over each sub-helmfile and show diff for each of them, not aggregating them.

If helmfile apply --prune needs to diff based on all the releases/namespaces in all the sub-helmfiles, we need to redesign the whole helmfile diff behaviour.

One possible solution would be that you avoid using sub-helmfiles, and include all the sub-helmfiles(if you had) into the parent helmfile with {{ tpl (readFile "subhelmfile.yaml" . }}.

Is that possible for you? @Morriz

[Morriz]

We have many helmfile.d/* files without subhelmfiles.

This becomes overcomplicated now. I am only talking about aggregating releases being deployed, and remove any charts in any namespace that were previously deployed with helmfile, and are not in the current releases any more.

Can you add chart labels like this:

• helmfile.date: {datetime}
• helmfile.release: {releaseName}

So that they can be pruned safely later?
That may be done with backwards compatibility I believe, but will force reloading, so would need to become optional (addHelmfileLabels: true, with default false could work with documentation)

What do you think?

[mumoshu]

@Morriz Thanks for your confirmation and suggestion.

Yes it seems technically possible. Helm doesn't actually provide any way to label releases. But we can shell out to kubectl to label those release secrets externally:

$ k get secret | grep helm
sh.helm.release.v1.podinfo-1586757488.v1   helm.sh/release.v1                    1      3d18h
sh.helm.release.v1.raw.v1                  helm.sh/release.v1                    1      34m
sh.helm.release.v1.raw.v2                  helm.sh/release.v1                    1      2m14s

[mumoshu]

A few nits. helmfile.release won't be needed as you can extract it out of the release secret.

The value passed to helmfile.date must be provided externally to Helmfile OR Helmfile needs to report the exact helmfile.date generated. Otherwise you'll be enable to determine releases with which hemlfile.date needs to be pruned.

[mumoshu]

@Morriz A simpler way would be enhancing Helmfile to report the summary of the apply in a specified file, for your use:

$ helmfile apply --summary-output summary.yaml
....

$ cat summary.yaml
releases:
- namespace: myns1
  name: myapp1
  installed: true
  upgraded: false
  deployed: true
- namespace: myns2
  name: myapp2
  installed: true
  upgraded: false
  deployed: true
- namespace:
  name: myapp3
  installed: false
  upgraded: false
  deleted: true
  deployed: false

installed, upgraded and deleted should be set to true when and only when the respective operation is done by Helmfile in this helmfile apply run.

deployed should be true when the release should exist in the K8s cluster after the run.

So you'd use this to produce a list of deployed releases, so that you can remove all the releases other than deployed ones.

WDYT?

[Morriz]

Almost there. We need the full list of all charts installed, also alien charts (not installed by helmfile), so that we can iterate over those. And then another flag like alien: true would allow us to NOT prune it. We could prune if all the flags are false.

[Morriz]

My last comment was based on the assumption that your list would still not show releases not present in helmfile anymore but previously under helmfile control. But if your approach would also include those with a flag registered: false then we don’t need the aliens flag (to exclude from pruning).

[mumoshu]

your list would still not show releases not present in helmfile AND previously under helmfile control

True. It would be your responsibility to query the K8s cluster (for helm releases, by running kubectl get secret or helm list for every namespace, either way you prefer) in a way you like, take diff between the query result and the summary.yaml, to finally compute the alien releases to be pruned.

This way, Helmfile will stay stateless, the implementation cost is minimum. But given the sumary.yaml, impossible is now possible.

[Morriz]

I see, and for now that makes sense maybe. But imo this solution would be best housed somewhere close to this ecosystem (as a plugin?). So that others may benefit. It now feels hacky, and I think it is best when fitted in helmfile itself. Don't you agree?

I could of course create a hacky script that does what you suggest, but it feels more elegant to keep it under control of this repo.

[mumoshu]

@Morriz I FULLY agree that this would be a good addition/nice fit to Helmfile. But what I'm feeling not good about is that it sounds like I'm being forced to implement the full solution in oneshot, myself :)

That's why I'm proposing to provide the interim/foundational solution first, which makes me feel a bit more easy to get it done in my spare time, while it makes impossible possible for helmfile users.

If anyone could contribute the full solution based on our discussion, I'd be more than happy to review/merge it.

[Morriz]

I understand. I wish I was a Go programmer. Maybe I should start my Go journey here and see what I can do. Any suggestions as to which files I need to look at?