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

Configmaps and Secrets annotated by default hooks cannot be uninstalled #44

Open
GRomR1 opened this issue Apr 2, 2023 · 7 comments · May be fixed by #45
Open

Configmaps and Secrets annotated by default hooks cannot be uninstalled #44

GRomR1 opened this issue Apr 2, 2023 · 7 comments · May be fixed by #45
Assignees

Comments

@GRomR1
Copy link
Contributor

GRomR1 commented Apr 2, 2023

Now helm hooks are added by default to all resources like ConfigMap and Secret. It makes a problem with manage them in helm releases. ConfigMap with hooks cannot be tracked and removed while uninstall a release.
The helm creators know about that. They write about it here.
The solution could be helm.sh/hook-delete-policy but it is not possible to apply for regular resources to store data (ConfigMap, Secrets).

I think this default behavior should be changed. Should be a possibility to add hooks manually where it really needs. And should stop add hooks in all configmaps and secrets by default.

GRomR1 added a commit to GRomR1/nxs-universal-chart that referenced this issue Apr 2, 2023
@GRomR1 GRomR1 linked a pull request Apr 2, 2023 that will close this issue
@randreev1321
Copy link
Collaborator

Hi, @GRomR1.

Thank you for pointing out this problem. Yes, it is a known problem, and it seems to us to be the lesser of the evils.

If your hooks, for example with migration, need variables from secrets or configmaps, you can get a whole range of problems:

  • the first time you deploying with a hook, it uses secrets/configmaps that haven't been deployed yet
  • when you update your secrets/configmaps, the hooks will use the old data
  • when you add annotations to secrets/configmaps already created, for example, when you start using a hook, you get errors saying that the objects have already been created

That's why we use this approach.

@randreev1321 randreev1321 self-assigned this Apr 5, 2023
@evgkrsk
Copy link
Contributor

evgkrsk commented Apr 5, 2023

Maybe it worth to use alternative approach: dont mark configmap/secrets as hook by default, but add optional flag to generate second, "hooked" version of object? Migration jobs will able to use "hooked" configmap/secrets AND chart uninstalling will remove non-hooked ones.

@randreev1321
Copy link
Collaborator

Hi, @evgkrsk.

Thanks for the idea. We've been discussing this too, and it seems there are no better options at the moment.

@GRomR1
Copy link
Contributor Author

GRomR1 commented Apr 8, 2023

Agree with @evgkrsk.
This would be an option to user. The variant where exists "hooked" configmap/secrets sounds good! It should be removed after migration job finished.

To me the best option is don't setup this hook annotations because the hook job is an option. Someone like us prefer to run migrations in CI and don't use migration job. And this annotations was clutter up our cluster in review-stands.

@randreev1321
Copy link
Collaborator

Okay, let's try this approach.

@GRomR1
Copy link
Contributor Author

GRomR1 commented May 2, 2023

@randreev1321 push an additional commit 7860171 that will merge hookAnnotations with genericAnnotations

approve plz

@reddare
Copy link

reddare commented Sep 23, 2024

Any updates?

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

Successfully merging a pull request may close this issue.

4 participants