Skip to content

Commit

Permalink
eliminate short-circuiting in logic to add/remove finalizers
Browse files Browse the repository at this point in the history
  • Loading branch information
nojnhuh authored and k8s-infra-cherrypick-robot committed Nov 2, 2023
1 parent 8cb9b90 commit f47fd94
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
5 changes: 3 additions & 2 deletions controllers/azuremanagedcontrolplane_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,9 @@ func (amcpr *AzureManagedControlPlaneReconciler) reconcileNormal(ctx context.Con
log.Info("Reconciling AzureManagedControlPlane")

// Remove deprecated Cluster finalizer if it exists, if the AzureManagedControlPlane doesn't have our finalizer, add it.
if controllerutil.RemoveFinalizer(scope.ControlPlane, infrav1.ClusterFinalizer) ||
controllerutil.AddFinalizer(scope.ControlPlane, infrav1.ManagedClusterFinalizer) {
needsPatch := controllerutil.RemoveFinalizer(scope.ControlPlane, infrav1.ClusterFinalizer)
needsPatch = controllerutil.AddFinalizer(scope.ControlPlane, infrav1.ManagedClusterFinalizer) || needsPatch
if needsPatch {

Check warning on line 229 in controllers/azuremanagedcontrolplane_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/azuremanagedcontrolplane_controller.go#L227-L229

Added lines #L227 - L229 were not covered by tests
// Register the finalizer immediately to avoid orphaning Azure resources on delete
if err := scope.PatchObject(ctx); err != nil {
amcpr.Recorder.Eventf(scope.ControlPlane, corev1.EventTypeWarning, "AzureManagedControlPlane unavailable", "failed to patch resource: %s", err)
Expand Down
5 changes: 3 additions & 2 deletions controllers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -656,8 +656,9 @@ func EnsureClusterIdentity(ctx context.Context, c client.Client, object conditio
}

// Remove deprecated finalizer if it exists, Register the finalizer immediately to avoid orphaning Azure resources on delete.
if controllerutil.RemoveFinalizer(identity, deprecatedClusterIdentityFinalizer(finalizerPrefix, namespace, name)) ||
controllerutil.AddFinalizer(identity, clusterIdentityFinalizer(finalizerPrefix, namespace, name)) {
needsPatch := controllerutil.RemoveFinalizer(identity, deprecatedClusterIdentityFinalizer(finalizerPrefix, namespace, name))
needsPatch = controllerutil.AddFinalizer(identity, clusterIdentityFinalizer(finalizerPrefix, namespace, name)) || needsPatch
if needsPatch {

Check warning on line 661 in controllers/helpers.go

View check run for this annotation

Codecov / codecov/patch

controllers/helpers.go#L659-L661

Added lines #L659 - L661 were not covered by tests
// finalizers are added/removed then patch the object
identityHelper, err := patch.NewHelper(identity, c)
if err != nil {
Expand Down

0 comments on commit f47fd94

Please sign in to comment.