Improve finalizer handling

[sbueringer]

/kind bug

I think we have to improve our finalizer handling in CAPV.

The ideal behavior in Reconcile is the following:

• if deletionTimestamp is set
  • => do nothing
• if deletionTimestamp is unset
  • if finalizer is set
    • => do nothing
  • if finalizer is unset
    • => set finalizer and immediately requeue

I recently had a PR to fix this everywhere in core CAPI: kubernetes-sigs/cluster-api#8949

Reasoning:

• if we don't immediately requeue we potentially create resources in vSphere that would be orphaned if the same resource is deleted before we can set the finalizer in defer
• if we try to set the finalizer when the deletionTimestamp is already set we just get an error from the apiserver because this operation is not allowed

An easy example how this should look like can be found here: https://github.com/sbueringer/cluster-api/blob/cef1cf1d9312088a785c520f49970b1c609ab9ad/internal/controllers/cluster/cluster_controller.go#L145-L158

Note: it's important that the defer patch is done in a way that the finalizer is actually written

Environment:

• Cluster-api-provider-vsphere version:
• Kubernetes version: (use kubectl version):
• OS (e.g. from /etc/os-release):

[sbueringer]

cc @yastij @srm09 @randomvariable

@chrischdi @killianmuldoon Would be great if one of you can pick this up. I think we should implement this until the v1.8.0 release and also consider cherry-picking it back.

[sbueringer]

/kind release-blocking

[zhanggbj]

If this is still available, I would like to work on it :）
/assign

[randomvariable]

If you start work on it, use a
/lifecycle active to show you're actively working on it.

[zhanggbj]

Thanks @randomvariable for the kind reminder :)
/lifecycle active