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

Implement helm.sh/resource-deletion-policy annotation to override deletion cascade per resource #12917

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Danil-Grigorev
Copy link

@Danil-Grigorev Danil-Grigorev commented Mar 29, 2024

What this PR does / why we need it:
Helm uninstall command allows to provider an optional --cascade flag to specify a deletion policy to use for all resources. In some cases this policy has to be foreground, to ensure the ownership resources are deleted in order, and the deletion is finished until the next hook phase is executed.

To achieve this, a new annotation is provided, helm.sh/resource-deletion-policy. When it is set on the resource, the policy from the value is applied on delete request, regardless of the --cascade flag value.

Special notes for your reviewer:

If applicable:

  • this PR contains documentation
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility
    (backwards compatibility is ensured as it is just adding an annotation. Previous releases would ignore it)

@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 29, 2024
@robertsirc
Copy link
Contributor

Not to be difficult but shouldn't this have some testing?

@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 2, 2024
…etion cascade per resource

Signed-off-by: Danil Grigorev <danil.grigorev@suse.com>
@Danil-Grigorev
Copy link
Author

Hi @robertsirc yeah, tests sound good. Just added some, can this be ok-to-test now? It is passing for me locally.

@robertsirc
Copy link
Contributor

Hey @Danil-Grigorev this is a great question if you could join us for the weekly developer meeting on Thursday and ask one of their maintainers. I was just helping read through these and as i looked at all the code you added I wanted to question if it made sense for us to write up some test as we could ensure we are getting the functionality you added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants