Consider executing "reconcileDelete" only if finalizer is set

[sbueringer]

Today our controllers are implemted like this:

• if object has no deletionTimestamp
  • => Ensure finalizer is set
• else
  • => execute "reconcileDelete" (remove finalizer at the end of reconcileDelete)

In later reconciles we continue to run "reconcileDelete" even if the finalizer is not set anymore.

I wonder if this should be changed to only execute reconcileDelete as long as the finalizer is on the object.

Pro: always running reconcileDelete:

• reconcileDelete code will always make sure everything is deleted even if the finalizer is not set (but this also means that the object could be removed from etcd in the middle of reconcileDelete)

Pro: running reconcileDelete only when finalizer is set:

• reconcileDelete will only be executed until the finalizer has been removed
  • Example: Machine deletion will stop executing reconcileDelete as soon as it completed successfully once (so we stop continuously trying to drain Pods, waiting for volume detach , ...)

[k8s-ci-robot]

This issue is currently awaiting triage.

If CAPI contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[sbueringer]

@fabriziopandini @vincepri @enxebre @chrischdi Do you have any opinions on this?

[vincepri]

What's the underlying problem we're trying to solve? The assumption that the reconcileDelete always gets executed is generally fine, although I can see the point of only running actions when the finalizer is present. That said, in terms of re-entrance and ongoing correctness leaning towards more extra reconciliation is probably preferred to rely on the finalizer being present

[fabriziopandini]

I agree with Vince.
For the machine use case, we should probably stop doing actions like drain when the infra machine is deleted (before the removing the finalizer).