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

Block reinstall if crds are still pending to be deleted #784

Conversation

davidcassany
Copy link
Contributor

@davidcassany davidcassany commented Jun 26, 2024

With #775 reinstalling elemental-operator with helm results in a broken installation. This change is to prevent reinstalling CRDs if there are still instances pending to be deleted.

This will prevent from having silently broken installations. This is the same issue as in #515

@davidcassany davidcassany requested a review from a team as a code owner June 26, 2024 14:17
Copy link
Contributor

@kkaempf kkaempf left a comment

Choose a reason for hiding this comment

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

🤞🏻

@davidcassany davidcassany force-pushed the prevent_reinstalling_with_pending_deletions branch from 85949e0 to ed4ad39 Compare June 26, 2024 14:40
Signed-off-by: David Cassany <dcassany@suse.com>
@davidcassany davidcassany force-pushed the prevent_reinstalling_with_pending_deletions branch from ed4ad39 to 9465ba7 Compare June 26, 2024 14:41
{{- if $inventoryCRD -}}
{{- if $inventoryCRD.metadata.deletionTimestamp -}}
{{- required "CRDs from previous installations are pending to be removed (deletionTimestamp is set). Fully deleting them before (re-)installing is required" "" -}}
{{- $crds := list
Copy link
Contributor

Choose a reason for hiding this comment

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

Super nice!

@davidcassany davidcassany force-pushed the prevent_reinstalling_with_pending_deletions branch 2 times, most recently from 6d72d34 to e31b17d Compare June 26, 2024 15:08
Signed-off-by: David Cassany <dcassany@suse.com>
@davidcassany davidcassany force-pushed the prevent_reinstalling_with_pending_deletions branch from e31b17d to d375589 Compare June 26, 2024 15:19
@davidcassany davidcassany enabled auto-merge (squash) June 26, 2024 15:29
@davidcassany
Copy link
Contributor Author

For full reference other approaches were considered at a time by having some pre-delete hook to run a cleanup logic #536 This could be an option to try to automate cleanup of an uninstalling operation, feels a bit hacky and error prone, I would not consider it for now, referencing it here in case we need it. In such case the current PR would still be needed as a sanity check.

@davidcassany davidcassany merged commit f92a2de into rancher:main Jun 26, 2024
22 checks passed
@davidcassany davidcassany deleted the prevent_reinstalling_with_pending_deletions branch June 26, 2024 15:52
anmazzotti pushed a commit to anmazzotti/elemental-operator that referenced this pull request Jun 26, 2024
* Block reinstall if crds are still pending to be deleted

Signed-off-by: David Cassany <dcassany@suse.com>
anmazzotti added a commit that referenced this pull request Jun 27, 2024
* Add managedosversion finalizer (#775)

* Implement ManagedOSVersion controller and finalizer

Signed-off-by: Andrea Mazzotti <andrea.mazzotti@suse.com>

* Remove unused ManagedOSVersion Status

Signed-off-by: Andrea Mazzotti <andrea.mazzotti@suse.com>

* Block reinstall if crds are still pending to be deleted (#784)

Signed-off-by: David Cassany <dcassany@suse.com>

---------

Signed-off-by: Andrea Mazzotti <andrea.mazzotti@suse.com>
Signed-off-by: David Cassany <dcassany@suse.com>
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 this pull request may close these issues.

3 participants