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

Set deletion propagation for helm uninstall #698

Merged
merged 2 commits into from
Jun 30, 2023

Conversation

hoffimar
Copy link
Contributor

@hoffimar hoffimar commented Jun 9, 2023

In helm 3.12 a feature was added (commit) to support other deletion modes than background during a helm uninstall. This PR attempts to pass the new property DeletionPropagation on the HelmRelease to the runner.

  • Add an optional field DeletionPropagation to the HelmRelease api.
  • Set the DeletionPropagation when running the uninstall.

I'm not so sure about the needed default handling for this optional field. To not disturb existing installations, I made sure a value is returned in case there is no default yet persisted on the HelmRelease.

@hoffimar hoffimar force-pushed the deletionPropagation branch 2 times, most recently from d7c0350 to af466e5 Compare June 9, 2023 15:47
@hiddeco hiddeco force-pushed the deletionPropagation branch from 3ac9be0 to 4d39735 Compare June 26, 2023 11:05
@hiddeco hiddeco added enhancement New feature or request area/helm Helm related issues and pull requests labels Jun 26, 2023
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please document this new field in docs/spec/v2beta1/helmreleases.md?

@hoffimar hoffimar force-pushed the deletionPropagation branch from 1f66cc2 to 4de98bc Compare June 26, 2023 14:56
@hoffimar
Copy link
Contributor Author

I did not introduce a new type for the option like was done for CRDsPolicy, as I found other examples where it wasn't done and this string is just passed to helm. Not sure whether there is some guideline though.

@hiddeco hiddeco force-pushed the deletionPropagation branch from 4de98bc to b8cf651 Compare June 30, 2023 13:17
@hiddeco hiddeco added the area/api API related issues and pull requests label Jun 30, 2023
hoffimar-sap and others added 2 commits June 30, 2023 19:30
Signed-off-by: Martin Hoffmann <[email protected]>
Signed-off-by: Martin Hoffmann <[email protected]>
Signed-off-by: Martin Hoffmann <[email protected]>
@hiddeco hiddeco force-pushed the deletionPropagation branch from b8cf651 to d656cb3 Compare June 30, 2023 17:30
Copy link
Member

@aryan9600 aryan9600 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much! 🥇

@hiddeco hiddeco merged commit 9c2090d into fluxcd:main Jun 30, 2023
hiddeco added a commit that referenced this pull request Jul 11, 2023
Manual backport of the work done in #698, to keep things aligned.

Signed-off-by: Hidde Beydals <[email protected]>
hiddeco added a commit that referenced this pull request Jul 14, 2023
Manual backport of the work done in #698, to keep things aligned.

Signed-off-by: Hidde Beydals <[email protected]>
hiddeco added a commit that referenced this pull request Oct 31, 2023
Manual backport of the work done in #698, to keep things aligned.

Signed-off-by: Hidde Beydals <[email protected]>
hiddeco added a commit that referenced this pull request Nov 21, 2023
Manual backport of the work done in #698, to keep things aligned.

Signed-off-by: Hidde Beydals <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api API related issues and pull requests area/helm Helm related issues and pull requests enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants