use merge-patch on finalizer operations to resolve racing conflicts

charts/kubefed/charts/controllermanager/templates/aggregate_clusterroles.yaml

@@ -51,7 +51,7 @@ rules:
   - list
   - update
   - create
-  - update
+  - patch
   - delete
 - apiGroups:
   - multiclusterdns.kubefed.io
@@ -63,6 +63,7 @@ rules:
   - list
   - create
   - update
+  - patch
   - delete
 - apiGroups:
   - core.kubefed.io
@@ -74,6 +75,7 @@ rules:
   - list
   - create
   - update
+  - patch
   - delete
 - apiGroups:
   - types.kubefed.io
@@ -85,6 +87,7 @@ rules:
   - list
   - create
   - update
+  - patch
   - delete
 ---
 apiVersion: rbac.authorization.k8s.io/v1

charts/kubefed/charts/controllermanager/templates/clusterrole.yaml

@@ -17,6 +17,7 @@ rules:
   - watch
   - list
   - update
+  - patch
 - apiGroups:
   - multiclusterdns.kubefed.io
   resources:
@@ -27,6 +28,7 @@ rules:
   - list
   - create
   - update
+  - patch
   - delete
 - apiGroups:
   - core.kubefed.io
@@ -38,6 +40,7 @@ rules:
   - list
   - create
   - update
+  - patch
 - apiGroups:
   - types.kubefed.io
   resources:
@@ -47,6 +50,7 @@ rules:
   - watch
   - list
   - update
+  - patch
 - apiGroups:
   - ""
   resources:

charts/kubefed/charts/controllermanager/templates/roles.yaml

@@ -18,6 +18,7 @@ rules:
   - watch
   - list
   - update
+  - patch
 - apiGroups:
   - multiclusterdns.kubefed.io
   resources:
@@ -28,6 +29,7 @@ rules:
   - list
   - create
   - update
+  - patch
 - apiGroups:
   - core.kubefed.io
   resources:
@@ -38,6 +40,7 @@ rules:
   - list
   - create
   - update
+  - patch
   - delete
 - apiGroups:
   - types.kubefed.io
@@ -48,6 +51,7 @@ rules:
   - watch
   - list
   - update
+  - patch
 - apiGroups:
   - ""
   resources:
@@ -76,6 +80,7 @@ rules:
   - get
   - create
   - update
+  - patch
 - apiGroups:
   - ""
   resources:

pkg/client/generic/genericclient.go

@@ -34,6 +34,7 @@ type Client interface {
 	Delete(ctx context.Context, obj runtime.Object, namespace, name string, opts ...client.DeleteOption) error
 	List(ctx context.Context, obj runtime.Object, namespace string, opts ...client.ListOption) error
 	UpdateStatus(ctx context.Context, obj runtime.Object) error
+	Patch(ctx context.Context, obj runtime.Object, patch client.Patch, opts ...client.PatchOption) error
 }
 
 type genericClient struct {
@@ -89,3 +90,7 @@ func (c *genericClient) List(ctx context.Context, obj runtime.Object, namespace
 func (c *genericClient) UpdateStatus(ctx context.Context, obj runtime.Object) error {
 	return c.client.Status().Update(ctx, obj)
 }
+
+func (c *genericClient) Patch(ctx context.Context, obj runtime.Object, patch client.Patch, opts ...client.PatchOption) error {
+	return c.client.Patch(ctx, obj, patch, opts...)
+}

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)
 }