use merge-patch on finalizer operations to resolve racing conflicts

Signed-off-by: Bruce Ma <[email protected]>

Author: mars1024

pkg/controller/federatedtypeconfig/controller.go

@@ -23,14 +23,14 @@ import (
 
 	"github.com/pkg/errors"
 
-	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	pkgruntime "k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/runtime"
-	"k8s.io/apimachinery/pkg/util/sets"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/klog/v2"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	corev1b1 "sigs.k8s.io/kubefed/pkg/apis/core/v1beta1"
 	genericclient "sigs.k8s.io/kubefed/pkg/client/generic"
@@ -376,33 +376,23 @@ func (c *Controller) refreshSyncController(tc *corev1b1.FederatedTypeConfig) err
 }
 
 func (c *Controller) ensureFinalizer(tc *corev1b1.FederatedTypeConfig) (bool, error) {
-	accessor, err := meta.Accessor(tc)
-	if err != nil {
-		return false, err
-	}
-	finalizers := sets.NewString(accessor.GetFinalizers()...)
-	if finalizers.Has(finalizer) {
+	if controllerutil.ContainsFinalizer(tc, finalizer) {
 		return false, nil
 	}
-	finalizers.Insert(finalizer)
-	accessor.SetFinalizers(finalizers.List())
-	err = c.client.Update(context.TODO(), tc)
-	return true, err
+
+	patch := client.MergeFrom(tc.DeepCopy())
+	controllerutil.AddFinalizer(tc, finalizer)
+	return true, c.client.Patch(context.TODO(), tc, patch)
 }
 
 func (c *Controller) removeFinalizer(tc *corev1b1.FederatedTypeConfig) error {
-	accessor, err := meta.Accessor(tc)
-	if err != nil {
-		return err
-	}
-	finalizers := sets.NewString(accessor.GetFinalizers()...)
-	if !finalizers.Has(finalizer) {
+	if !controllerutil.ContainsFinalizer(tc, finalizer) {
 		return nil
 	}
-	finalizers.Delete(finalizer)
-	accessor.SetFinalizers(finalizers.List())
-	err = c.client.Update(context.TODO(), tc)
-	return err
+
+	patch := client.MergeFrom(tc.DeepCopy())
+	controllerutil.RemoveFinalizer(tc, finalizer)
+	return c.client.Patch(context.TODO(), tc, patch)
 }
 
 func (c *Controller) namespaceFTCExists() bool {

pkg/controller/sync/controller.go

@@ -40,14 +40,14 @@ import (
 	"k8s.io/klog/v2"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	"sigs.k8s.io/kubefed/pkg/apis/core/typeconfig"
 	fedv1b1 "sigs.k8s.io/kubefed/pkg/apis/core/v1beta1"
 	genericclient "sigs.k8s.io/kubefed/pkg/client/generic"
 	"sigs.k8s.io/kubefed/pkg/controller/sync/dispatch"
 	"sigs.k8s.io/kubefed/pkg/controller/sync/status"
 	"sigs.k8s.io/kubefed/pkg/controller/util"
-	finalizersutil "sigs.k8s.io/kubefed/pkg/controller/util/finalizers"
 	"sigs.k8s.io/kubefed/pkg/metrics"
 )
 
@@ -657,20 +657,24 @@ func (s *KubeFedSyncController) handleDeletionInClusters(gvk schema.GroupVersion
 
 func (s *KubeFedSyncController) ensureFinalizer(fedResource FederatedResource) error {
 	obj := fedResource.Object()
-	isUpdated, err := finalizersutil.AddFinalizers(obj, sets.NewString(FinalizerSyncController))
-	if err != nil || !isUpdated {
-		return err
+	if controllerutil.ContainsFinalizer(obj, FinalizerSyncController) {
+		return nil
 	}
+
+	patch := client.MergeFrom(obj.DeepCopy())
+	controllerutil.AddFinalizer(obj, FinalizerSyncController)
 	klog.V(2).Infof("Adding finalizer %s to %s %q", FinalizerSyncController, fedResource.FederatedKind(), fedResource.FederatedName())
-	return s.hostClusterClient.Update(context.TODO(), obj)
+	return s.hostClusterClient.Patch(context.TODO(), obj, patch)
 }
 
 func (s *KubeFedSyncController) removeFinalizer(fedResource FederatedResource) error {
 	obj := fedResource.Object()
-	isUpdated, err := finalizersutil.RemoveFinalizers(obj, sets.NewString(FinalizerSyncController))
-	if err != nil || !isUpdated {
-		return err
+	if !controllerutil.ContainsFinalizer(obj, FinalizerSyncController) {
+		return nil
 	}
+
+	patch := client.MergeFrom(obj.DeepCopy())
+	controllerutil.RemoveFinalizer(obj, FinalizerSyncController)
 	klog.V(2).Infof("Removing finalizer %s from %s %q", FinalizerSyncController, fedResource.FederatedKind(), fedResource.FederatedName())
-	return s.hostClusterClient.Update(context.TODO(), obj)
+	return s.hostClusterClient.Patch(context.TODO(), obj, patch)
 }