update finalizer #159
update finalizer #159
Conversation
Signed-off-by: huiwq1990 <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: huiwq1990 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| } | ||
|
|
||
| // Spoke cluster is deleting, we remove its related resources | ||
| if !managedCluster.DeletionTimestamp.IsZero() { | ||
| if err := c.removeManagedClusterResources(ctx, managedClusterName); err != nil { | ||
| return err | ||
| } | ||
| return c.removeManagedClusterFinalizer(ctx, managedCluster) | ||
| if controllerutil.ContainsFinalizer(managedCluster, managedClusterFinalizer) { | ||
| controllerutil.RemoveFinalizer(managedCluster, managedClusterFinalizer) |
There was a problem hiding this comment.
I hasn't take a look at the details of RemoveFinalizer, will it ensure the ordering after a deletion?
There was a problem hiding this comment.
Sorry, I don't understand the ordering meaing, did you say the order of finalizes? Why we need order? RemoveFinalizer will not change finalize order.
f = append(f[:i], f[i+1:]...)
There was a problem hiding this comment.
yes, the ordering of the finalizers. Some controllers may rely on the ordering of the finalizers, it is not normal but it happens.
There was a problem hiding this comment.
RemoveFinalizer will keep the order. By the way, could you specify which controller use this feature?
There was a problem hiding this comment.
not in registration, but an external consumer (controller) could do that.
|
This may need to run |
|
@huiwq1990: PR needs rebase. 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/test-infra repository. |
use controller-runtime to delete or update finalizer.