[BUG] Reconcile() fails to delete the corresponding pvc if missing deletionTimestamp of Cassandra pod

[srteam2020]

Describe the bug
We find that the reconcile() in cassandradatecenter_controller.go may miss the deletionTimestamp of a "to-be-deleted" cassandra pod and fail to delete the corresponding pvc.
In the current implementation, the controller finds out all deleted pods by calling AllDeletedPods which decides whether a pod is deleted by checking its deletionTimestamp. However, if the delete event for such a "to-be-deleted" pod comes earlier and removes the Cassandra pod from the local indexer, then AllDeletedPods will never see such a deletionTimestamp for the pod (because the pod has disappeared from the local indexer). In that way, the pvc belonging to this pod will never be deleted by the controller.

Consider a concrete example here:

1. Create a cassandradatecenter cdc with node=2 and deletePVCs=true (now we have two pods: ca-0 and ca-1, and two pvc for the two pods)
2. Scale cdc down by changing node to 1 (now ca-1 is going to be deleted as well as its pvc)
3. Now the apiserver will notify the informer that ca-1 will be deleted with a deletionTimestamp. The local indexer will be updated by the new status of ca-1 (with deletionTimestamp)
4. reconcile() will also be notified by the informer, and is going to check the pod status in indexer
5. Now the deletion of ca-1 is done by the kubelet, and the informer is notified by the apiserver again that ca-1 is deleted. All the information about ca-1 is deleted from the indexer now.
6. reconcile() checks the indexer and finds there is only ca-0. Since no deletionTimestamp is found, the pvc is not going to be deleted.

The previous example happens when the deletion finishes faster than the reconciler. Although it is not deterministic, we observed this many times when running the above workloads.

This may cause wasted storage space. More importantly, if later scale up the cdc, a new ca-1 will not be started successfully because of the old data in the pvc for the previous ca-1.

To Reproduce
Run the scale-down workload as mentioned above. If the reconciler runs slightly slower than the deletion, then you will find the pvc remains after ca-1 gets deleted.

Expected behavior
We can register a finalizer for each Cassandra pod just like what the controller did for the CassandraDataCenter in addFinalizer. In that way, the ca-1 will not be removed from indexer until we finish deleting the pvc and removing the finalizer.

Environment

• OS Linux
• Kubernetes version v1.18.9
• kubectl version v1.20.1
• Go version 1.13.9
• Cassandra version 3

Additional context
We are willing to issue a PR to add a finalizer for Cassandra pods.

[smiklosovic]

@srteam2020 thank you for this elaborative description, if only everybody wrote it like you :)

We will be more than happy to accept your PR which fixes this issue and I will gladly merge it aftewards.

warm regards

[srteam2020]

@smiklosovic Thanks for confirming the issue.
Pushed one PR to fix it. The build failure from CI seems to be unrelated to the patch.

[smiklosovic]

@srteam2020 thanks a lot, I ll take it from here.

[smiklosovic]

@srteam2020 I commented on your PR.

[smiklosovic]

merged / released thanks