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

Prevent installing if previous CRDs are pending to be removed #553

Conversation

davidcassany
Copy link
Contributor

@davidcassany davidcassany commented Oct 24, 2023

This PR checks there are no previous CRDs having a deletion time stamp set. If so it assumes these are leftovers of previous installations and refuses to install the crds chart.

For the time being it only checks on machineinventories, however it could be expanded if we consider that necessary. For now I'd keep the check to the minimum.

When trying to install crds chart if the machine inventory is already there with a deletionTimestamp set produces an error like the following:

$ helm upgrade --install -n cattle-elemental-system --create-namespace \
    elemental-operator-crds \
    oci://registry.opensuse.org/isv/rancher/elemental/pr/rancher/elemental-operator/pr-553/charts/rancher/elemental-operator-crds-chart                                                                                                                                                                                                                            
Release "elemental-operator-crds" does not exist. Installing it now.                                                                                                                                                                               
Pulled: registry.opensuse.org/isv/rancher/elemental/pr/rancher/elemental-operator/pr-553/charts/rancher/elemental-operator-crds-chart:1.4.0                                                                                                        
Digest: sha256:263aa9a6d54990923ea92aa3c6f8eb1bad4550545d45981cd043087a964d9c52                                                                                                                                                                    
Error: execution error at (elemental-operator-crds/templates/validate-no-pending-deletions.yaml:4:9): CRDs from previous installations are pending to be removed (deletionTimestamp is set). Fully deleting them before (re-)installing is required

In the particular case of #515 to fully clear the offending resource it can be done by removing the finalizers of any MachineInventory resource:

$ kubectl patch machineinventory <machineInventoryName> -n fleet-default --type merge \
    -p '{"metadata": {"finalizers": []}}'

Fixes #515

@davidcassany davidcassany requested a review from a team as a code owner October 24, 2023 10:27
@davidcassany davidcassany marked this pull request as draft October 24, 2023 10:27
Signed-off-by: David Cassany <dcassany@suse.com>
@davidcassany davidcassany force-pushed the prevent_installation_if_pending_deletions_are_found branch from 3f019fb to f3a4b85 Compare October 24, 2023 11:24
@davidcassany davidcassany marked this pull request as ready for review October 24, 2023 11:25
@davidcassany davidcassany requested a review from fgiudici October 24, 2023 11:28
@codecov
Copy link

codecov bot commented Oct 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1f53971) 53.65% compared to head (f3a4b85) 53.65%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #553   +/-   ##
=======================================
  Coverage   53.65%   53.65%           
=======================================
  Files          39       39           
  Lines        5690     5690           
=======================================
  Hits         3053     3053           
  Misses       2368     2368           
  Partials      269      269           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@davidcassany
Copy link
Contributor Author

@fgiudici I think this is a more reasonable solution to #515. I did not want to provide many details or messages as guessing the reason for which a previous uninstall left stuff behind might not be limited to #515 use case only. I am fine as long we prevent the installation and provide a message to review if any already existing CRD is pending to be deleted. How to proceed then could be documented somewhere else if we see fit.

Copy link
Member

@fgiudici fgiudici left a comment

Choose a reason for hiding this comment

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

Thanks David, this will catch the issue indeed.
I like not having some "auto" clean-up here, as the user will be in a position where resources (and so potentially Elemental clusters) are running and will be cleaned up as soon as the operator chart is installed: one should be aware of what is doing at this point and to have to erase manually the finalizers looks to me really requires the user to double check and be sure of what is doing. Like it, nice patch 👍🏼

@davidcassany davidcassany merged commit 5ff509b into rancher:main Oct 24, 2023
@davidcassany davidcassany deleted the prevent_installation_if_pending_deletions_are_found branch June 26, 2024 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

e2e: cannot delete remaining Elemental cluster after uninstallation of operator
2 participants