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

Refactor K8ssandraCluster deletion cleanup operation #992

Open
adejanovski opened this issue Jun 2, 2023 · 1 comment · May be fixed by #1022
Open

Refactor K8ssandraCluster deletion cleanup operation #992

adejanovski opened this issue Jun 2, 2023 · 1 comment · May be fixed by #1022
Assignees
Labels
needs-dod product-backlog Issues in the state 'product-backlog' refactoring

Comments

@adejanovski
Copy link
Contributor

adejanovski commented Jun 2, 2023

In controllers/k8ssandra/cleanup.go we delete all the objects that are tied to a deleted K8ssandraCluster object.
The deleteDeployments(), deleteServices() and deleteConfigMaps() could benefit from the following improvements:

  • Combine them into a generic function/method if possible
  • Move a few function arguments to the K8ssandraClusterReconciler object in order to reduce their number in the calls
  • Evaluate the possibility of returning a more meaningful object than a boolean, which could carry more information (such as an error or a ReconcileResult)
  • make use of DeleteAllOf, using the labels in the LabelSelector, instead of looping over objects to delete
  • Write unit tests for it

┆Issue is synchronized with this Jira Story by Unito
┆Issue Number: K8OP-90

@adejanovski adejanovski mentioned this issue Jun 2, 2023
5 tasks
@Miles-Garnsey Miles-Garnsey moved this to Ready in K8ssandra Jun 14, 2023
@adejanovski adejanovski added the ready Issues in the state 'ready' label Jun 14, 2023
@olim7t
Copy link
Contributor

olim7t commented Jul 14, 2023

Note: this is closely related to #961.

As I'm digging into the code I'm encountering more questions:

  • there are two main methods: checkDeletion which handles the deletion of the whole cluster, and checkDcDeletion which handles a single DC (if it's been removed from the cluster spec). However they don't share any code, which feels like a wasted opportunity for reuse.
  • case in point: checkDeletion removes Stargate, Reaper, deployments, services and configMaps. checkDcDeletion only removes Stargate and Reaper. Decommissioning a DC does indeed leave at least one configMap behind (tested on a very simple cluster, there's probably more in complex scenarios).
  • checkDcDeletion looks up the components by exact name. checkDeletion deletes every cluster sub-component in the current namespace. This works now but is something we'll need to keep in mind if we factor the code (two DCs can exist in the same namespace, so we can't reuse the shotgun approach for decommission).
  • the Stargate/Reaper deployments and services already use a controller reference for garbage collection. It looks like deleteDeployments and deleteServices were only added for Medusa. Why can't we use controller references there too?

In fact, I think we could generalize the use of controller references. As long as every remote object we create is owned by the CassandraDatacenter (directly or transitively), we shouldn't need to do anything beyond deleting the DC.
I'm going to experiment with that.

@olim7t olim7t linked a pull request Jul 14, 2023 that will close this issue
5 tasks
@adejanovski adejanovski moved this from Ready to Ready For Review in K8ssandra Jul 17, 2023
@adejanovski adejanovski added ready-for-review Issues in the state 'ready-for-review' and removed ready Issues in the state 'ready' labels Jul 17, 2023
@adejanovski adejanovski moved this from Ready For Review to In Progress in K8ssandra Jul 17, 2023
@adejanovski adejanovski added in-progress Issues in the state 'in-progress' and removed ready-for-review Issues in the state 'ready-for-review' labels Jul 17, 2023
@adejanovski adejanovski moved this from In Progress to Ready in K8ssandra Sep 4, 2023
@adejanovski adejanovski added ready Issues in the state 'ready' and removed in-progress Issues in the state 'in-progress' labels Sep 4, 2023
@adejanovski adejanovski moved this from Ready to Product Backlog in K8ssandra Aug 22, 2024
@adejanovski adejanovski added product-backlog Issues in the state 'product-backlog' and removed ready Issues in the state 'ready' labels Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-dod product-backlog Issues in the state 'product-backlog' refactoring
Projects
No open projects
Status: Product Backlog
Development

Successfully merging a pull request may close this issue.

2 participants