Merge pull request #44139 from nikhiljindal/delFinalizerDup

Automatic merge from submit-queue (batch tested with PRs 44583, 44139, 44753)

Federation: Removing duplicate finalizer manipulation logic in federation controllers

Ref #40989

Using apimachinery's meta accessor to manipulate objects instead of duplicating that logic in each controller.
Just a cleanup, no behavior change.

cc @kubernetes/sig-federation-pr-reviews

federation/pkg/federation-controller/configmap/configmap_controller.go

@@ -175,9 +175,7 @@ func NewConfigMapController(client federationclientset.Interface) *ConfigMapCont
 		})
 
 	configmapcontroller.deletionHelper = deletionhelper.NewDeletionHelper(
-		configmapcontroller.hasFinalizerFunc,
+		configmapcontroller.updateConfigMap,
-		configmapcontroller.removeFinalizerFunc,
-		configmapcontroller.addFinalizerFunc,
 		// objNameFunc
 		func(obj pkgruntime.Object) string {
 			configmap := obj.(*apiv1.ConfigMap)
@@ -192,50 +190,11 @@ func NewConfigMapController(client federationclientset.Interface) *ConfigMapCont
 	return configmapcontroller
 }
 
-// hasFinalizerFunc returns true if the given object has the given finalizer in its ObjectMeta.
+// Sends the given updated object to apiserver.
-func (configmapcontroller *ConfigMapController) hasFinalizerFunc(obj pkgruntime.Object, finalizer string) bool {
+// Assumes that the given object is a configmap.
+func (configmapcontroller *ConfigMapController) updateConfigMap(obj pkgruntime.Object) (pkgruntime.Object, error) {
 	configmap := obj.(*apiv1.ConfigMap)
-	for i := range configmap.ObjectMeta.Finalizers {
+	return configmapcontroller.federatedApiClient.Core().ConfigMaps(configmap.Namespace).Update(configmap)
-		if string(configmap.ObjectMeta.Finalizers[i]) == finalizer {
-			return true
-		}
-	}
-	return false
-}
-
-// removeFinalizerFunc removes the given finalizers from the given objects ObjectMeta. Assumes that the given object is a configmap.
-func (configmapcontroller *ConfigMapController) removeFinalizerFunc(obj pkgruntime.Object, finalizers []string) (pkgruntime.Object, error) {
-	configmap := obj.(*apiv1.ConfigMap)
-	newFinalizers := []string{}
-	hasFinalizer := false
-	for i := range configmap.ObjectMeta.Finalizers {
-		if !deletionhelper.ContainsString(finalizers, configmap.ObjectMeta.Finalizers[i]) {
-			newFinalizers = append(newFinalizers, configmap.ObjectMeta.Finalizers[i])
-		} else {
-			hasFinalizer = true
-		}
-	}
-	if !hasFinalizer {
-		// Nothing to do.
-		return obj, nil
-	}
-	configmap.ObjectMeta.Finalizers = newFinalizers
-	configmap, err := configmapcontroller.federatedApiClient.Core().ConfigMaps(configmap.Namespace).Update(configmap)
-	if err != nil {
-		return nil, fmt.Errorf("failed to remove finalizers %v from configmap %s: %v", finalizers, configmap.Name, err)
-	}
-	return configmap, nil
-}
-
-// addFinalizerFunc adds the given finalizer to the given objects ObjectMeta. Assumes that the given object is a configmap.
-func (configmapcontroller *ConfigMapController) addFinalizerFunc(obj pkgruntime.Object, finalizers []string) (pkgruntime.Object, error) {
-	configmap := obj.(*apiv1.ConfigMap)
-	configmap.ObjectMeta.Finalizers = append(configmap.ObjectMeta.Finalizers, finalizers...)
-	configmap, err := configmapcontroller.federatedApiClient.Core().ConfigMaps(configmap.Namespace).Update(configmap)
-	if err != nil {
-		return nil, fmt.Errorf("failed to add finalizers %v to configmap %s: %v", finalizers, configmap.Name, err)
-	}
-	return configmap, nil
 }
 
 func (configmapcontroller *ConfigMapController) Run(stopChan <-chan struct{}) {

federation/pkg/federation-controller/configmap/configmap_controller_test.go

@@ -103,8 +103,8 @@ func TestConfigMapController(t *testing.T) {
 	configmapWatch.Add(configmap1)
 	// There should be 2 updates to add both the finalizers.
 	updatedConfigMap := GetConfigMapFromChan(configmapUpdateChan)
-	assert.True(t, configmapController.hasFinalizerFunc(updatedConfigMap, deletionhelper.FinalizerDeleteFromUnderlyingClusters))
+	AssertHasFinalizer(t, updatedConfigMap, deletionhelper.FinalizerDeleteFromUnderlyingClusters)
-	assert.True(t, configmapController.hasFinalizerFunc(updatedConfigMap, metav1.FinalizerOrphanDependents))
+	AssertHasFinalizer(t, updatedConfigMap, metav1.FinalizerOrphanDependents)
 
 	// Verify that the configmap is created in underlying cluster1.
 	createdConfigMap := GetConfigMapFromChan(cluster1CreateChan)

federation/pkg/federation-controller/daemonset/daemonset_controller.go

@@ -193,9 +193,7 @@ func NewDaemonSetController(client federationclientset.Interface) *DaemonSetCont
 		})
 
 	daemonsetcontroller.deletionHelper = deletionhelper.NewDeletionHelper(
-		daemonsetcontroller.hasFinalizerFunc,
+		daemonsetcontroller.updateDaemonSet,
-		daemonsetcontroller.removeFinalizerFunc,
-		daemonsetcontroller.addFinalizerFunc,
 		// objNameFunc
 		func(obj pkgruntime.Object) string {
 			daemonset := obj.(*extensionsv1.DaemonSet)
@@ -210,52 +208,11 @@ func NewDaemonSetController(client federationclientset.Interface) *DaemonSetCont
 	return daemonsetcontroller
 }
 
-// Returns true if the given object has the given finalizer in its ObjectMeta.
+// Sends the given updated object to apiserver.
-func (daemonsetcontroller *DaemonSetController) hasFinalizerFunc(obj pkgruntime.Object, finalizer string) bool {
-	daemonset := obj.(*extensionsv1.DaemonSet)
-	for i := range daemonset.ObjectMeta.Finalizers {
-		if string(daemonset.ObjectMeta.Finalizers[i]) == finalizer {
-			return true
-		}
-	}
-	return false
-}
-
-// Removes the finalizers from the given objects ObjectMeta.
 // Assumes that the given object is a daemonset.
-func (daemonsetcontroller *DaemonSetController) removeFinalizerFunc(obj pkgruntime.Object, finalizers []string) (pkgruntime.Object, error) {
+func (daemonsetcontroller *DaemonSetController) updateDaemonSet(obj pkgruntime.Object) (pkgruntime.Object, error) {
 	daemonset := obj.(*extensionsv1.DaemonSet)
-	newFinalizers := []string{}
+	return daemonsetcontroller.federatedApiClient.Extensions().DaemonSets(daemonset.Namespace).Update(daemonset)
-	hasFinalizer := false
-	for i := range daemonset.ObjectMeta.Finalizers {
-		if !deletionhelper.ContainsString(finalizers, daemonset.ObjectMeta.Finalizers[i]) {
-			newFinalizers = append(newFinalizers, daemonset.ObjectMeta.Finalizers[i])
-		} else {
-			hasFinalizer = true
-		}
-	}
-	if !hasFinalizer {
-		// Nothing to do.
-		return obj, nil
-	}
-	daemonset.ObjectMeta.Finalizers = newFinalizers
-	daemonset, err := daemonsetcontroller.federatedApiClient.Extensions().DaemonSets(daemonset.Namespace).Update(daemonset)
-	if err != nil {
-		return nil, fmt.Errorf("failed to remove finalizers %v from daemonset %s: %v", finalizers, daemonset.Name, err)
-	}
-	return daemonset, nil
-}
-
-// Adds the given finalizers to the given objects ObjectMeta.
-// Assumes that the given object is a daemonset.
-func (daemonsetcontroller *DaemonSetController) addFinalizerFunc(obj pkgruntime.Object, finalizers []string) (pkgruntime.Object, error) {
-	daemonset := obj.(*extensionsv1.DaemonSet)
-	daemonset.ObjectMeta.Finalizers = append(daemonset.ObjectMeta.Finalizers, finalizers...)
-	daemonset, err := daemonsetcontroller.federatedApiClient.Extensions().DaemonSets(daemonset.Namespace).Update(daemonset)
-	if err != nil {
-		return nil, fmt.Errorf("failed to add finalizers %v to daemonset %s: %v", finalizers, daemonset.Name, err)
-	}
-	return daemonset, nil
 }
 
 func (daemonsetcontroller *DaemonSetController) Run(stopChan <-chan struct{}) {

federation/pkg/federation-controller/daemonset/daemonset_controller_test.go

@@ -104,8 +104,8 @@ func TestDaemonSetController(t *testing.T) {
 
 	// There should be an update to add both the finalizers.
 	updatedDaemonSet := GetDaemonSetFromChan(daemonsetUpdateChan)
-	assert.True(t, daemonsetController.hasFinalizerFunc(updatedDaemonSet, deletionhelper.FinalizerDeleteFromUnderlyingClusters))
+	AssertHasFinalizer(t, updatedDaemonSet, deletionhelper.FinalizerDeleteFromUnderlyingClusters)
-	assert.True(t, daemonsetController.hasFinalizerFunc(updatedDaemonSet, metav1.FinalizerOrphanDependents))
+	AssertHasFinalizer(t, updatedDaemonSet, metav1.FinalizerOrphanDependents)
 	daemonset1 = *updatedDaemonSet
 
 	createdDaemonSet := GetDaemonSetFromChan(cluster1CreateChan)

federation/pkg/federation-controller/deployment/deploymentcontroller.go

@@ -207,9 +207,7 @@ func NewDeploymentController(federationClient fedclientset.Interface) *Deploymen
 		})
 
 	fdc.deletionHelper = deletionhelper.NewDeletionHelper(
-		fdc.hasFinalizerFunc,
+		fdc.updateDeployment,
-		fdc.removeFinalizerFunc,
-		fdc.addFinalizerFunc,
 		// objNameFunc
 		func(obj runtime.Object) string {
 			deployment := obj.(*extensionsv1.Deployment)
@@ -224,52 +222,11 @@ func NewDeploymentController(federationClient fedclientset.Interface) *Deploymen
 	return fdc
 }
 
-// Returns true if the given object has the given finalizer in its ObjectMeta.
+// Sends the given updated object to apiserver.
-func (fdc *DeploymentController) hasFinalizerFunc(obj runtime.Object, finalizer string) bool {
-	deployment := obj.(*extensionsv1.Deployment)
-	for i := range deployment.ObjectMeta.Finalizers {
-		if string(deployment.ObjectMeta.Finalizers[i]) == finalizer {
-			return true
-		}
-	}
-	return false
-}
-
-// Removes the finalizers from the given objects ObjectMeta.
 // Assumes that the given object is a deployment.
-func (fdc *DeploymentController) removeFinalizerFunc(obj runtime.Object, finalizers []string) (runtime.Object, error) {
+func (fdc *DeploymentController) updateDeployment(obj runtime.Object) (runtime.Object, error) {
 	deployment := obj.(*extensionsv1.Deployment)
-	newFinalizers := []string{}
+	return fdc.fedClient.Extensions().Deployments(deployment.Namespace).Update(deployment)
-	hasFinalizer := false
-	for i := range deployment.ObjectMeta.Finalizers {
-		if !deletionhelper.ContainsString(finalizers, deployment.ObjectMeta.Finalizers[i]) {
-			newFinalizers = append(newFinalizers, deployment.ObjectMeta.Finalizers[i])
-		} else {
-			hasFinalizer = true
-		}
-	}
-	if !hasFinalizer {
-		// Nothing to do.
-		return obj, nil
-	}
-	deployment.ObjectMeta.Finalizers = newFinalizers
-	deployment, err := fdc.fedClient.Extensions().Deployments(deployment.Namespace).Update(deployment)
-	if err != nil {
-		return nil, fmt.Errorf("failed to remove finalizers %v from deployment %s: %v", finalizers, deployment.Name, err)
-	}
-	return deployment, nil
-}
-
-// Adds the given finalizers to the given objects ObjectMeta.
-// Assumes that the given object is a deployment.
-func (fdc *DeploymentController) addFinalizerFunc(obj runtime.Object, finalizers []string) (runtime.Object, error) {
-	deployment := obj.(*extensionsv1.Deployment)
-	deployment.ObjectMeta.Finalizers = append(deployment.ObjectMeta.Finalizers, finalizers...)
-	deployment, err := fdc.fedClient.Extensions().Deployments(deployment.Namespace).Update(deployment)
-	if err != nil {
-		return nil, fmt.Errorf("failed to add finalizers %v to deployment %s: %v", finalizers, deployment.Name, err)
-	}
-	return deployment, nil
 }
 
 func (fdc *DeploymentController) Run(workers int, stopCh <-chan struct{}) {

federation/pkg/federation-controller/ingress/BUILD

@@ -47,6 +47,7 @@ go_test(
         "//federation/client/clientset_generated/federation_clientset/fake:go_default_library",
         "//federation/pkg/federation-controller/util:go_default_library",
         "//federation/pkg/federation-controller/util/deletionhelper:go_default_library",
+        "//federation/pkg/federation-controller/util/finalizers:go_default_library",
         "//federation/pkg/federation-controller/util/test:go_default_library",
         "//pkg/api/v1:go_default_library",
         "//pkg/apis/extensions/v1beta1:go_default_library",

federation/pkg/federation-controller/ingress/ingress_controller.go

@@ -290,9 +290,7 @@ func NewIngressController(client federationclientset.Interface) *IngressControll
 		})
 
 	ic.deletionHelper = deletionhelper.NewDeletionHelper(
-		ic.hasFinalizerFunc,
+		ic.updateIngress,
-		ic.removeFinalizerFunc,
-		ic.addFinalizerFunc,
 		// objNameFunc
 		func(obj pkgruntime.Object) string {
 			ingress := obj.(*extensionsv1beta1.Ingress)
@@ -306,52 +304,11 @@ func NewIngressController(client federationclientset.Interface) *IngressControll
 	return ic
 }
 
-// Returns true if the given object has the given finalizer in its ObjectMeta.
+// Sends the given updated object to apiserver.
-func (ic *IngressController) hasFinalizerFunc(obj pkgruntime.Object, finalizer string) bool {
+// Assumes that the given object is an ingress.
+func (ic *IngressController) updateIngress(obj pkgruntime.Object) (pkgruntime.Object, error) {
 	ingress := obj.(*extensionsv1beta1.Ingress)
-	for i := range ingress.ObjectMeta.Finalizers {
+	return ic.federatedApiClient.Extensions().Ingresses(ingress.Namespace).Update(ingress)
-		if string(ingress.ObjectMeta.Finalizers[i]) == finalizer {
-			return true
-		}
-	}
-	return false
-}
-
-// Removes the finalizers from the given objects ObjectMeta.
-// Assumes that the given object is a ingress.
-func (ic *IngressController) removeFinalizerFunc(obj pkgruntime.Object, finalizers []string) (pkgruntime.Object, error) {
-	ingress := obj.(*extensionsv1beta1.Ingress)
-	newFinalizers := []string{}
-	hasFinalizer := false
-	for i := range ingress.ObjectMeta.Finalizers {
-		if !deletionhelper.ContainsString(finalizers, ingress.ObjectMeta.Finalizers[i]) {
-			newFinalizers = append(newFinalizers, ingress.ObjectMeta.Finalizers[i])
-		} else {
-			hasFinalizer = true
-		}
-	}
-	if !hasFinalizer {
-		// Nothing to do.
-		return obj, nil
-	}
-	ingress.ObjectMeta.Finalizers = newFinalizers
-	ingress, err := ic.federatedApiClient.Extensions().Ingresses(ingress.Namespace).Update(ingress)
-	if err != nil {
-		return nil, fmt.Errorf("failed to remove finalizers %v from ingress %s: %v", finalizers, ingress.Name, err)
-	}
-	return ingress, nil
-}
-
-// Adds the given finalizers to the given objects ObjectMeta.
-// Assumes that the given object is a ingress.
-func (ic *IngressController) addFinalizerFunc(obj pkgruntime.Object, finalizers []string) (pkgruntime.Object, error) {
-	ingress := obj.(*extensionsv1beta1.Ingress)
-	ingress.ObjectMeta.Finalizers = append(ingress.ObjectMeta.Finalizers, finalizers...)
-	ingress, err := ic.federatedApiClient.Extensions().Ingresses(ingress.Namespace).Update(ingress)
-	if err != nil {
-		return nil, fmt.Errorf("failed to add finalizers %v to ingress %s: %v", finalizers, ingress.Name, err)
-	}
-	return ingress, nil
 }
 
 func (ic *IngressController) Run(stopChan <-chan struct{}) {

federation/pkg/federation-controller/ingress/ingress_controller_test.go

@@ -32,6 +32,7 @@ import (
 	fakefedclientset "k8s.io/kubernetes/federation/client/clientset_generated/federation_clientset/fake"
 	"k8s.io/kubernetes/federation/pkg/federation-controller/util"
 	"k8s.io/kubernetes/federation/pkg/federation-controller/util/deletionhelper"
+	finalizersutil "k8s.io/kubernetes/federation/pkg/federation-controller/util/finalizers"
 	. "k8s.io/kubernetes/federation/pkg/federation-controller/util/test"
 	apiv1 "k8s.io/kubernetes/pkg/api/v1"
 	extensionsv1beta1 "k8s.io/kubernetes/pkg/apis/extensions/v1beta1"
@@ -160,8 +161,8 @@ func TestIngressController(t *testing.T) {
 	t.Log("Checking that appropriate finalizers are added")
 	// There should be an update to add both the finalizers.
 	updatedIngress := GetIngressFromChan(t, fedIngressUpdateChan)
-	assert.True(t, ingressController.hasFinalizerFunc(updatedIngress, deletionhelper.FinalizerDeleteFromUnderlyingClusters))
+	AssertHasFinalizer(t, updatedIngress, deletionhelper.FinalizerDeleteFromUnderlyingClusters)
-	assert.True(t, ingressController.hasFinalizerFunc(updatedIngress, metav1.FinalizerOrphanDependents), fmt.Sprintf("ingress does not have the orphan finalizer: %v", updatedIngress))
+	AssertHasFinalizer(t, updatedIngress, metav1.FinalizerOrphanDependents)
 	fedIngress = *updatedIngress
 
 	t.Log("Checking that Ingress was correctly created in cluster 1")
@@ -333,8 +334,15 @@ func WaitForFinalizersInFederationStore(ingressController *IngressController, st
 			return false, err
 		}
 		ingress := obj.(*extensionsv1beta1.Ingress)
-		if ingressController.hasFinalizerFunc(ingress, metav1.FinalizerOrphanDependents) &&
+		hasOrphanFinalizer, err := finalizersutil.HasFinalizer(ingress, metav1.FinalizerOrphanDependents)
-			ingressController.hasFinalizerFunc(ingress, deletionhelper.FinalizerDeleteFromUnderlyingClusters) {
+		if err != nil {
+			return false, err
+		}
+		hasDeleteFinalizer, err := finalizersutil.HasFinalizer(ingress, deletionhelper.FinalizerDeleteFromUnderlyingClusters)
+		if err != nil {
+			return false, err
+		}
+		if hasOrphanFinalizer && hasDeleteFinalizer {
 			return true, nil
 		}
 		return false, nil

federation/pkg/federation-controller/namespace/BUILD

@@ -52,6 +52,7 @@ go_test(
         "//pkg/client/clientset_generated/clientset:go_default_library",
         "//pkg/client/clientset_generated/clientset/fake:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
+        "//vendor/github.com/stretchr/testify/require:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",

federation/pkg/federation-controller/namespace/namespace_controller.go

@@ -179,9 +179,7 @@ func NewNamespaceController(client federationclientset.Interface, dynamicClientP
 		})
 
 	nc.deletionHelper = deletionhelper.NewDeletionHelper(
-		nc.hasFinalizerFunc,
+		nc.updateNamespace,
-		nc.removeFinalizerFunc,
-		nc.addFinalizerFunc,
 		// objNameFunc
 		func(obj runtime.Object) string {
 			namespace := obj.(*apiv1.Namespace)
@@ -200,52 +198,11 @@ func NewNamespaceController(client federationclientset.Interface, dynamicClientP
 	return nc
 }
 
-// Returns true if the given object has the given finalizer in its ObjectMeta.
+// Sends the given update object to apiserver.
-func (nc *NamespaceController) hasFinalizerFunc(obj runtime.Object, finalizer string) bool {
-	namespace := obj.(*apiv1.Namespace)
-	for i := range namespace.ObjectMeta.Finalizers {
-		if string(namespace.ObjectMeta.Finalizers[i]) == finalizer {
-			return true
-		}
-	}
-	return false
-}
-
-// Removes the finalizers from the given objects ObjectMeta.
 // Assumes that the given object is a namespace.
-func (nc *NamespaceController) removeFinalizerFunc(obj runtime.Object, finalizers []string) (runtime.Object, error) {
+func (nc *NamespaceController) updateNamespace(obj runtime.Object) (runtime.Object, error) {
 	namespace := obj.(*apiv1.Namespace)
-	newFinalizers := []string{}
+	return nc.federatedApiClient.Core().Namespaces().Update(namespace)
-	hasFinalizer := false
-	for i := range namespace.ObjectMeta.Finalizers {
-		if !deletionhelper.ContainsString(finalizers, namespace.ObjectMeta.Finalizers[i]) {
-			newFinalizers = append(newFinalizers, namespace.ObjectMeta.Finalizers[i])
-		} else {
-			hasFinalizer = true
-		}
-	}
-	if !hasFinalizer {
-		// Nothing to do.
-		return obj, nil
-	}
-	namespace.ObjectMeta.Finalizers = newFinalizers
-	namespace, err := nc.federatedApiClient.Core().Namespaces().Update(namespace)
-	if err != nil {
-		return nil, fmt.Errorf("failed to remove finalizers %v from namespace %s: %v", finalizers, namespace.Name, err)
-	}
-	return namespace, nil
-}
-
-// Adds the given finalizers to the given objects ObjectMeta.
-// Assumes that the given object is a namespace.
-func (nc *NamespaceController) addFinalizerFunc(obj runtime.Object, finalizers []string) (runtime.Object, error) {
-	namespace := obj.(*apiv1.Namespace)
-	namespace.ObjectMeta.Finalizers = append(namespace.ObjectMeta.Finalizers, finalizers...)
-	namespace, err := nc.federatedApiClient.Core().Namespaces().Finalize(namespace)
-	if err != nil {
-		return nil, fmt.Errorf("failed to add finalizers %v to namespace %s: %v", finalizers, namespace.Name, err)
-	}
-	return namespace, nil
 }
 
 // Returns true if the given object has the given finalizer in its NamespaceSpec.

federation/pkg/federation-controller/namespace/namespace_controller_test.go

@@ -37,6 +37,7 @@ import (
 	fakekubeclientset "k8s.io/kubernetes/pkg/client/clientset_generated/clientset/fake"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -61,7 +62,7 @@ func TestNamespaceController(t *testing.T) {
 	RegisterFakeList(clusters, &fakeClient.Fake, &federationapi.ClusterList{Items: []federationapi.Cluster{*cluster1}})
 	RegisterFakeList(namespaces, &fakeClient.Fake, &apiv1.NamespaceList{Items: []apiv1.Namespace{}})
 	namespaceWatch := RegisterFakeWatch(namespaces, &fakeClient.Fake)
-	namespaceCreateChan := RegisterFakeCopyOnCreate(namespaces, &fakeClient.Fake, namespaceWatch)
+	namespaceUpdateChan := RegisterFakeCopyOnUpdate(namespaces, &fakeClient.Fake, namespaceWatch)
 	clusterWatch := RegisterFakeWatch(clusters, &fakeClient.Fake)
 
 	cluster1Client := &fakekubeclientset.Clientset{}
@@ -99,15 +100,14 @@ func TestNamespaceController(t *testing.T) {
 	// Test add federated namespace.
 	namespaceWatch.Add(&ns1)
 	// Verify that the DeleteFromUnderlyingClusters finalizer is added to the namespace.
-	// Note: finalize invokes the create action in Fake client.
+	updatedNamespace := GetNamespaceFromChan(namespaceUpdateChan)
-	// TODO: Seems like a bug. Should invoke update. Fix it.
+	require.NotNil(t, updatedNamespace)
-	updatedNamespace := GetNamespaceFromChan(namespaceCreateChan)
+	AssertHasFinalizer(t, updatedNamespace, deletionhelper.FinalizerDeleteFromUnderlyingClusters)
-	assert.True(t, namespaceController.hasFinalizerFunc(updatedNamespace, deletionhelper.FinalizerDeleteFromUnderlyingClusters))
 	ns1 = *updatedNamespace
 
 	// Verify that the namespace is created in underlying cluster1.
 	createdNamespace := GetNamespaceFromChan(cluster1CreateChan)
-	assert.NotNil(t, createdNamespace)
+	require.NotNil(t, createdNamespace)
 	assert.Equal(t, ns1.Name, createdNamespace.Name)
 
 	// Wait for the namespace to appear in the informer store
@@ -126,7 +126,7 @@ func TestNamespaceController(t *testing.T) {
 	// Test add cluster
 	clusterWatch.Add(cluster2)
 	createdNamespace2 := GetNamespaceFromChan(cluster2CreateChan)
-	assert.NotNil(t, createdNamespace2)
+	require.NotNil(t, createdNamespace2)
 	assert.Equal(t, ns1.Name, createdNamespace2.Name)
 	assert.Contains(t, createdNamespace2.Annotations, "A")
 

federation/pkg/federation-controller/replicaset/replicasetcontroller.go

@@ -215,9 +215,7 @@ func NewReplicaSetController(federationClient fedclientset.Interface) *ReplicaSe
 		})
 
 	frsc.deletionHelper = deletionhelper.NewDeletionHelper(
-		frsc.hasFinalizerFunc,
+		frsc.updateReplicaSet,
-		frsc.removeFinalizerFunc,
-		frsc.addFinalizerFunc,
 		// objNameFunc
 		func(obj runtime.Object) string {
 			replicaset := obj.(*extensionsv1.ReplicaSet)
@@ -232,52 +230,11 @@ func NewReplicaSetController(federationClient fedclientset.Interface) *ReplicaSe
 	return frsc
 }
 
-// Returns true if the given object has the given finalizer in its ObjectMeta.
+// Sends the given updated object to apiserver.
-func (frsc *ReplicaSetController) hasFinalizerFunc(obj runtime.Object, finalizer string) bool {
-	replicaset := obj.(*extensionsv1.ReplicaSet)
-	for i := range replicaset.ObjectMeta.Finalizers {
-		if string(replicaset.ObjectMeta.Finalizers[i]) == finalizer {
-			return true
-		}
-	}
-	return false
-}
-
-// Removes the finalizers from the given objects ObjectMeta.
 // Assumes that the given object is a replicaset.
-func (frsc *ReplicaSetController) removeFinalizerFunc(obj runtime.Object, finalizers []string) (runtime.Object, error) {
+func (frsc *ReplicaSetController) updateReplicaSet(obj runtime.Object) (runtime.Object, error) {
 	replicaset := obj.(*extensionsv1.ReplicaSet)
-	newFinalizers := []string{}
+	return frsc.fedClient.Extensions().ReplicaSets(replicaset.Namespace).Update(replicaset)
-	hasFinalizer := false
-	for i := range replicaset.ObjectMeta.Finalizers {
-		if !deletionhelper.ContainsString(finalizers, replicaset.ObjectMeta.Finalizers[i]) {
-			newFinalizers = append(newFinalizers, replicaset.ObjectMeta.Finalizers[i])
-		} else {
-			hasFinalizer = true
-		}
-	}
-	if !hasFinalizer {
-		// Nothing to do.
-		return obj, nil
-	}
-	replicaset.ObjectMeta.Finalizers = newFinalizers
-	replicaset, err := frsc.fedClient.Extensions().ReplicaSets(replicaset.Namespace).Update(replicaset)
-	if err != nil {
-		return nil, fmt.Errorf("failed to remove finalizers %v from replicaset %s: %v", finalizers, replicaset.Name, err)
-	}
-	return replicaset, nil
-}
-
-// Adds the given finalizers to the given objects ObjectMeta.
-// Assumes that the given object is a replicaset.
-func (frsc *ReplicaSetController) addFinalizerFunc(obj runtime.Object, finalizers []string) (runtime.Object, error) {
-	replicaset := obj.(*extensionsv1.ReplicaSet)
-	replicaset.ObjectMeta.Finalizers = append(replicaset.ObjectMeta.Finalizers, finalizers...)
-	replicaset, err := frsc.fedClient.Extensions().ReplicaSets(replicaset.Namespace).Update(replicaset)
-	if err != nil {
-		return nil, fmt.Errorf("failed to add finalizers %v to replicaset %s: %v", finalizers, replicaset.Name, err)
-	}
-	return replicaset, nil
 }
 
 func (frsc *ReplicaSetController) Run(workers int, stopCh <-chan struct{}) {

federation/pkg/federation-controller/service/servicecontroller.go

@@ -295,9 +295,7 @@ func New(federationClient fedclientset.Interface, dns dnsprovider.Interface,
 		})
 
 	s.deletionHelper = deletionhelper.NewDeletionHelper(
-		s.hasFinalizerFunc,
+		s.updateService,
-		s.removeFinalizerFunc,
-		s.addFinalizerFunc,
 		// objNameFunc
 		func(obj pkgruntime.Object) string {
 			service := obj.(*v1.Service)
@@ -316,52 +314,11 @@ func New(federationClient fedclientset.Interface, dns dnsprovider.Interface,
 	return s
 }
 
-// Returns true if the given object has the given finalizer in its ObjectMeta.
+// Sends the given updated object to apiserver.
-func (s *ServiceController) hasFinalizerFunc(obj pkgruntime.Object, finalizer string) bool {
-	service := obj.(*v1.Service)
-	for i := range service.ObjectMeta.Finalizers {
-		if string(service.ObjectMeta.Finalizers[i]) == finalizer {
-			return true
-		}
-	}
-	return false
-}
-
-// Removes the finalizers from the given objects ObjectMeta.
 // Assumes that the given object is a service.
-func (s *ServiceController) removeFinalizerFunc(obj pkgruntime.Object, finalizers []string) (pkgruntime.Object, error) {
+func (s *ServiceController) updateService(obj pkgruntime.Object) (pkgruntime.Object, error) {
 	service := obj.(*v1.Service)
-	newFinalizers := []string{}
+	return s.federationClient.Core().Services(service.Namespace).Update(service)
-	hasFinalizer := false
-	for i := range service.ObjectMeta.Finalizers {
-		if !deletionhelper.ContainsString(finalizers, service.ObjectMeta.Finalizers[i]) {
-			newFinalizers = append(newFinalizers, service.ObjectMeta.Finalizers[i])
-		} else {
-			hasFinalizer = true
-		}
-	}
-	if !hasFinalizer {
-		// Nothing to do.
-		return obj, nil
-	}
-	service.ObjectMeta.Finalizers = newFinalizers
-	service, err := s.federationClient.Core().Services(service.Namespace).Update(service)
-	if err != nil {
-		return nil, fmt.Errorf("failed to remove finalizers %v from service %s: %v", finalizers, service.Name, err)
-	}
-	return service, nil
-}
-
-// Adds the given finalizers to the given objects ObjectMeta.
-// Assumes that the given object is a service.
-func (s *ServiceController) addFinalizerFunc(obj pkgruntime.Object, finalizers []string) (pkgruntime.Object, error) {
-	service := obj.(*v1.Service)
-	service.ObjectMeta.Finalizers = append(service.ObjectMeta.Finalizers, finalizers...)
-	service, err := s.federationClient.Core().Services(service.Namespace).Update(service)
-	if err != nil {
-		return nil, fmt.Errorf("failed to add finalizers %v to service %s: %v", finalizers, service.Name, err)
-	}
-	return service, nil
 }
 
 // obj could be an *api.Service, or a DeletionFinalStateUnknown marker item.

federation/pkg/federation-controller/sync/controller.go

@@ -183,9 +183,7 @@ func newFederationSyncController(client federationclientset.Interface, adapter f
 		})
 
 	s.deletionHelper = deletionhelper.NewDeletionHelper(
-		s.hasFinalizerFunc,
+		s.updateObject,
-		s.removeFinalizerFunc,
-		s.addFinalizerFunc,
 		// objNameFunc
 		func(obj pkgruntime.Object) string {
 			return adapter.ObjectMeta(obj).Name
@@ -207,50 +205,9 @@ func (s *FederationSyncController) minimizeLatency() {
 	s.updateTimeout = 5 * time.Second
 }
 
-// hasFinalizerFunc returns true if the given object has the given finalizer in its ObjectMeta.
+// Sends the given updated object to apiserver.
-func (s *FederationSyncController) hasFinalizerFunc(obj pkgruntime.Object, finalizer string) bool {
+func (s *FederationSyncController) updateObject(obj pkgruntime.Object) (pkgruntime.Object, error) {
-	meta := s.adapter.ObjectMeta(obj)
+	return s.adapter.FedUpdate(obj)
-	for i := range meta.Finalizers {
-		if string(meta.Finalizers[i]) == finalizer {
-			return true
-		}
-	}
-	return false
-}
-
-// Removes the finalizer from the given objects ObjectMeta.
-func (s *FederationSyncController) removeFinalizerFunc(obj pkgruntime.Object, finalizers []string) (pkgruntime.Object, error) {
-	meta := s.adapter.ObjectMeta(obj)
-	newFinalizers := []string{}
-	hasFinalizer := false
-	for i := range meta.Finalizers {
-		if !deletionhelper.ContainsString(finalizers, meta.Finalizers[i]) {
-			newFinalizers = append(newFinalizers, meta.Finalizers[i])
-		} else {
-			hasFinalizer = true
-		}
-	}
-	if !hasFinalizer {
-		// Nothing to do.
-		return obj, nil
-	}
-	meta.Finalizers = newFinalizers
-	secret, err := s.adapter.FedUpdate(obj)
-	if err != nil {
-		return nil, fmt.Errorf("failed to remove finalizers %v from %s %s: %v", finalizers, s.adapter.Kind(), meta.Name, err)
-	}
-	return secret, nil
-}
-
-// Adds the given finalizers to the given objects ObjectMeta.
-func (s *FederationSyncController) addFinalizerFunc(obj pkgruntime.Object, finalizers []string) (pkgruntime.Object, error) {
-	meta := s.adapter.ObjectMeta(obj)
-	meta.Finalizers = append(meta.Finalizers, finalizers...)
-	secret, err := s.adapter.FedUpdate(obj)
-	if err != nil {
-		return nil, fmt.Errorf("failed to add finalizers %v to %s %s: %v", finalizers, s.adapter.Kind(), meta.Name, err)
-	}
-	return secret, nil
 }
 
 func (s *FederationSyncController) Run(stopChan <-chan struct{}) {

federation/pkg/federation-controller/sync/secret_controller_test.go

@@ -102,8 +102,8 @@ func TestSecretController(t *testing.T) {
 	secretWatch.Add(&secret1)
 	// There should be an update to add both the finalizers.
 	updatedSecret := GetSecretFromChan(secretUpdateChan)
-	assert.True(t, secretController.hasFinalizerFunc(updatedSecret, deletionhelper.FinalizerDeleteFromUnderlyingClusters))
+	AssertHasFinalizer(t, updatedSecret, deletionhelper.FinalizerDeleteFromUnderlyingClusters)
-	assert.True(t, secretController.hasFinalizerFunc(updatedSecret, metav1.FinalizerOrphanDependents))
+	AssertHasFinalizer(t, updatedSecret, metav1.FinalizerOrphanDependents)
 	secret1 = *updatedSecret
 
 	// Verify that the secret is created in underlying cluster1.

federation/pkg/federation-controller/util/BUILD

@@ -88,6 +88,7 @@ filegroup(
         ":package-srcs",
         "//federation/pkg/federation-controller/util/deletionhelper:all-srcs",
         "//federation/pkg/federation-controller/util/eventsink:all-srcs",
+        "//federation/pkg/federation-controller/util/finalizers:all-srcs",
         "//federation/pkg/federation-controller/util/planner:all-srcs",
         "//federation/pkg/federation-controller/util/podanalyzer:all-srcs",
         "//federation/pkg/federation-controller/util/test:all-srcs",

federation/pkg/federation-controller/util/deletionhelper/BUILD

@@ -9,17 +9,16 @@ load(
 
 go_library(
     name = "go_default_library",
-    srcs = [
+    srcs = ["deletion_helper.go"],
-        "deletion_helper.go",
-        "util.go",
-    ],
     tags = ["automanaged"],
     deps = [
         "//federation/pkg/federation-controller/util:go_default_library",
+        "//federation/pkg/federation-controller/util/finalizers:go_default_library",
         "//pkg/api:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//vendor/k8s.io/client-go/tools/record:go_default_library",
     ],
 )

federation/pkg/federation-controller/util/deletionhelper/deletion_helper.go

@@ -26,8 +26,10 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/kubernetes/federation/pkg/federation-controller/util"
+	finalizersutil "k8s.io/kubernetes/federation/pkg/federation-controller/util/finalizers"
 	"k8s.io/kubernetes/pkg/api"
 
 	"github.com/golang/glog"
@@ -44,15 +46,11 @@ const (
 	FinalizerDeleteFromUnderlyingClusters string = "federation.kubernetes.io/delete-from-underlying-clusters"
 )
 
-type HasFinalizerFunc func(runtime.Object, string) bool
+type UpdateObjFunc func(runtime.Object) (runtime.Object, error)
-type RemoveFinalizerFunc func(runtime.Object, []string) (runtime.Object, error)
-type AddFinalizerFunc func(runtime.Object, []string) (runtime.Object, error)
 type ObjNameFunc func(runtime.Object) string
 
 type DeletionHelper struct {
-	hasFinalizerFunc    HasFinalizerFunc
+	updateObjFunc UpdateObjFunc
-	removeFinalizerFunc RemoveFinalizerFunc
-	addFinalizerFunc    AddFinalizerFunc
 	objNameFunc   ObjNameFunc
 	updateTimeout time.Duration
 	eventRecorder record.EventRecorder
@@ -61,15 +59,11 @@ type DeletionHelper struct {
 }
 
 func NewDeletionHelper(
-	hasFinalizerFunc HasFinalizerFunc, removeFinalizerFunc RemoveFinalizerFunc,
+	updateObjFunc UpdateObjFunc, objNameFunc ObjNameFunc,
-	addFinalizerFunc AddFinalizerFunc, objNameFunc ObjNameFunc,
 	updateTimeout time.Duration, eventRecorder record.EventRecorder,
-	informer util.FederatedInformer,
+	informer util.FederatedInformer, updater util.FederatedUpdater) *DeletionHelper {
-	updater util.FederatedUpdater) *DeletionHelper {
 	return &DeletionHelper{
-		hasFinalizerFunc:    hasFinalizerFunc,
+		updateObjFunc: updateObjFunc,
-		removeFinalizerFunc: removeFinalizerFunc,
-		addFinalizerFunc:    addFinalizerFunc,
 		objNameFunc:   objNameFunc,
 		updateTimeout: updateTimeout,
 		eventRecorder: eventRecorder,
@@ -89,16 +83,24 @@ func NewDeletionHelper(
 // This method should be called before creating objects in underlying clusters.
 func (dh *DeletionHelper) EnsureFinalizers(obj runtime.Object) (
 	runtime.Object, error) {
-	finalizers := []string{}
+	finalizers := sets.String{}
-	if !dh.hasFinalizerFunc(obj, FinalizerDeleteFromUnderlyingClusters) {
+	hasFinalizer, err := finalizersutil.HasFinalizer(obj, FinalizerDeleteFromUnderlyingClusters)
-		finalizers = append(finalizers, FinalizerDeleteFromUnderlyingClusters)
+	if err != nil {
+		return obj, err
 	}
-	if !dh.hasFinalizerFunc(obj, metav1.FinalizerOrphanDependents) {
+	if !hasFinalizer {
-		finalizers = append(finalizers, metav1.FinalizerOrphanDependents)
+		finalizers.Insert(FinalizerDeleteFromUnderlyingClusters)
 	}
-	if len(finalizers) != 0 {
+	hasFinalizer, err = finalizersutil.HasFinalizer(obj, metav1.FinalizerOrphanDependents)
-		glog.V(2).Infof("Adding finalizers %v to %s", finalizers, dh.objNameFunc(obj))
+	if err != nil {
-		return dh.addFinalizerFunc(obj, finalizers)
+		return obj, err
+	}
+	if !hasFinalizer {
+		finalizers.Insert(metav1.FinalizerOrphanDependents)
+	}
+	if finalizers.Len() != 0 {
+		glog.V(2).Infof("Adding finalizers %v to %s", finalizers.List(), dh.objNameFunc(obj))
+		return dh.addFinalizers(obj, finalizers)
 	}
 	return obj, nil
 }
@@ -113,18 +115,25 @@ func (dh *DeletionHelper) HandleObjectInUnderlyingClusters(obj runtime.Object) (
 	runtime.Object, error) {
 	objName := dh.objNameFunc(obj)
 	glog.V(2).Infof("Handling deletion of federated dependents for object: %s", objName)
-	if !dh.hasFinalizerFunc(obj, FinalizerDeleteFromUnderlyingClusters) {
+	hasFinalizer, err := finalizersutil.HasFinalizer(obj, FinalizerDeleteFromUnderlyingClusters)
+	if err != nil {
+		return obj, err
+	}
+	if !hasFinalizer {
 		glog.V(2).Infof("obj does not have %s finalizer. Nothing to do", FinalizerDeleteFromUnderlyingClusters)
 		return obj, nil
 	}
-	hasOrphanFinalizer := dh.hasFinalizerFunc(obj, metav1.FinalizerOrphanDependents)
+	hasOrphanFinalizer, err := finalizersutil.HasFinalizer(obj, metav1.FinalizerOrphanDependents)
+	if err != nil {
+		return obj, err
+	}
 	if hasOrphanFinalizer {
 		glog.V(2).Infof("Found finalizer orphan. Nothing to do, just remove the finalizer")
 		// If the obj has FinalizerOrphan finalizer, then we need to orphan the
 		// corresponding objects in underlying clusters.
 		// Just remove both the finalizers in that case.
-		finalizers := []string{FinalizerDeleteFromUnderlyingClusters, metav1.FinalizerOrphanDependents}
+		finalizers := sets.NewString(FinalizerDeleteFromUnderlyingClusters, metav1.FinalizerOrphanDependents)
-		return dh.removeFinalizerFunc(obj, finalizers)
+		return dh.removeFinalizers(obj, finalizers)
 	}
 
 	glog.V(2).Infof("Deleting obj %s from underlying clusters", objName)
@@ -180,5 +189,33 @@ func (dh *DeletionHelper) HandleObjectInUnderlyingClusters(obj runtime.Object) (
 	}
 
 	// All done. Just remove the finalizer.
-	return dh.removeFinalizerFunc(obj, []string{FinalizerDeleteFromUnderlyingClusters})
+	return dh.removeFinalizers(obj, sets.NewString(FinalizerDeleteFromUnderlyingClusters))
+}
+
+// Adds the given finalizers to the given objects ObjectMeta.
+func (dh *DeletionHelper) addFinalizers(obj runtime.Object, finalizers sets.String) (runtime.Object, error) {
+	isUpdated, err := finalizersutil.AddFinalizers(obj, finalizers)
+	if err != nil || !isUpdated {
+		return obj, err
+	}
+	// Send the update to apiserver.
+	updatedObj, err := dh.updateObjFunc(obj)
+	if err != nil {
+		return nil, fmt.Errorf("failed to add finalizers %v to object %s: %v", finalizers, dh.objNameFunc(obj), err)
+	}
+	return updatedObj, nil
+}
+
+// Removes the given finalizers from the given objects ObjectMeta.
+func (dh *DeletionHelper) removeFinalizers(obj runtime.Object, finalizers sets.String) (runtime.Object, error) {
+	isUpdated, err := finalizersutil.RemoveFinalizers(obj, finalizers)
+	if err != nil || !isUpdated {
+		return obj, err
+	}
+	// Send the update to apiserver.
+	updatedObj, err := dh.updateObjFunc(obj)
+	if err != nil {
+		return nil, fmt.Errorf("failed to remove finalizers %v from object %s: %v", finalizers, dh.objNameFunc(obj), err)
+	}
+	return updatedObj, nil
 }

federation/pkg/federation-controller/util/deletionhelper/util.go

@@ -1,28 +0,0 @@
-/*
-Copyright 2017 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package deletionhelper
-
-// ContainsString returns true if the given string slice contains the given string.
-// Returns false otherwise.
-func ContainsString(arr []string, s string) bool {
-	for i := range arr {
-		if arr[i] == s {
-			return true
-		}
-	}
-	return false
-}

federation/pkg/federation-controller/util/finalizers/BUILD

@@ -0,0 +1,47 @@
+package(default_visibility = ["//visibility:public"])
+
+licenses(["notice"])
+
+load(
+    "@io_bazel_rules_go//go:def.bzl",
+    "go_library",
+    "go_test",
+)
+
+go_library(
+    name = "go_default_library",
+    srcs = ["finalizers.go"],
+    tags = ["automanaged"],
+    deps = [
+        "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
+    ],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = ["finalizers_test.go"],
+    library = ":go_default_library",
+    tags = ["automanaged"],
+    deps = [
+        "//pkg/api/v1:go_default_library",
+        "//vendor/github.com/stretchr/testify/assert:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
+    ],
+)

federation/pkg/federation-controller/util/finalizers/finalizers.go

@@ -0,0 +1,66 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Helper functions for manipulating finalizers.
+package finalizers
+
+import (
+	meta "k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
+)
+
+// HasFinalizer returns true if the given object has the given finalizer in its ObjectMeta.
+func HasFinalizer(obj runtime.Object, finalizer string) (bool, error) {
+	accessor, err := meta.Accessor(obj)
+	if err != nil {
+		return false, err
+	}
+	finalizers := sets.NewString(accessor.GetFinalizers()...)
+	return finalizers.Has(finalizer), nil
+}
+
+// AddFinalizers adds the given finalizers to the given objects ObjectMeta.
+// Returns true if the object was updated.
+func AddFinalizers(obj runtime.Object, newFinalizers sets.String) (bool, error) {
+	accessor, err := meta.Accessor(obj)
+	if err != nil {
+		return false, err
+	}
+	oldFinalizers := sets.NewString(accessor.GetFinalizers()...)
+	if oldFinalizers.IsSuperset(newFinalizers) {
+		return false, nil
+	}
+	allFinalizers := oldFinalizers.Union(newFinalizers)
+	accessor.SetFinalizers(allFinalizers.List())
+	return true, nil
+}
+
+// RemoveFinalizers removes the given finalizers from the given objects ObjectMeta.
+// Returns true if the object was updated.
+func RemoveFinalizers(obj runtime.Object, finalizers sets.String) (bool, error) {
+	accessor, err := meta.Accessor(obj)
+	if err != nil {
+		return false, err
+	}
+	oldFinalizers := sets.NewString(accessor.GetFinalizers()...)
+	if oldFinalizers.Intersection(finalizers).Len() == 0 {
+		return false, nil
+	}
+	newFinalizers := oldFinalizers.Difference(finalizers)
+	accessor.SetFinalizers(newFinalizers.List())
+	return true, nil
+}

federation/pkg/federation-controller/util/finalizers/finalizers_test.go

@@ -0,0 +1,171 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package finalizers
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	meta "k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/kubernetes/pkg/api/v1"
+)
+
+func newObj(finalizers []string) runtime.Object {
+	pod := v1.Pod{}
+	pod.ObjectMeta.Finalizers = finalizers
+	return &pod
+}
+
+func TestHasFinalizer(t *testing.T) {
+	testCases := []struct {
+		obj       runtime.Object
+		finalizer string
+		result    bool
+	}{
+		{
+			newObj([]string{}),
+			"",
+			false,
+		},
+		{
+			newObj([]string{}),
+			"someFinalizer",
+			false,
+		},
+		{
+			newObj([]string{"someFinalizer"}),
+			"",
+			false,
+		},
+		{
+			newObj([]string{"someFinalizer"}),
+			"anotherFinalizer",
+			false,
+		},
+		{
+			newObj([]string{"someFinalizer"}),
+			"someFinalizer",
+			true,
+		},
+		{
+			newObj([]string{"anotherFinalizer", "someFinalizer"}),
+			"someFinalizer",
+			true,
+		},
+	}
+	for index, test := range testCases {
+		hasFinalizer, _ := HasFinalizer(test.obj, test.finalizer)
+		assert.Equal(t, hasFinalizer, test.result, fmt.Sprintf("Test case %d failed. Expected: %v, actual: %v", index, test.result, hasFinalizer))
+	}
+}
+
+func TestAddFinalizers(t *testing.T) {
+	testCases := []struct {
+		obj           runtime.Object
+		finalizers    sets.String
+		isUpdated     bool
+		newFinalizers []string
+	}{
+		{
+			newObj([]string{}),
+			sets.NewString(),
+			false,
+			[]string{},
+		},
+		{
+			newObj([]string{}),
+			sets.NewString("someFinalizer"),
+			true,
+			[]string{"someFinalizer"},
+		},
+		{
+			newObj([]string{"someFinalizer"}),
+			sets.NewString(),
+			false,
+			[]string{"someFinalizer"},
+		},
+		{
+			newObj([]string{"someFinalizer"}),
+			sets.NewString("anotherFinalizer"),
+			true,
+			[]string{"anotherFinalizer", "someFinalizer"},
+		},
+		{
+			newObj([]string{"someFinalizer"}),
+			sets.NewString("someFinalizer"),
+			false,
+			[]string{"someFinalizer"},
+		},
+	}
+	for index, test := range testCases {
+		isUpdated, _ := AddFinalizers(test.obj, test.finalizers)
+		assert.Equal(t, isUpdated, test.isUpdated, fmt.Sprintf("Test case %d failed. Expected isUpdated: %v, actual: %v", index, test.isUpdated, isUpdated))
+		accessor, _ := meta.Accessor(test.obj)
+		newFinalizers := accessor.GetFinalizers()
+		assert.Equal(t, test.newFinalizers, newFinalizers, fmt.Sprintf("Test case %d failed. Expected finalizers: %v, actual: %v", index, test.newFinalizers, newFinalizers))
+	}
+}
+
+func TestRemoveFinalizers(t *testing.T) {
+	testCases := []struct {
+		obj           runtime.Object
+		finalizers    sets.String
+		isUpdated     bool
+		newFinalizers []string
+	}{
+		{
+			newObj([]string{}),
+			sets.NewString(),
+			false,
+			[]string{},
+		},
+		{
+			newObj([]string{}),
+			sets.NewString("someFinalizer"),
+			false,
+			[]string{},
+		},
+		{
+			newObj([]string{"someFinalizer"}),
+			sets.NewString(),
+			false,
+			[]string{"someFinalizer"},
+		},
+		{
+			newObj([]string{"someFinalizer"}),
+			sets.NewString("anotherFinalizer"),
+			false,
+			[]string{"someFinalizer"},
+		},
+		{
+			newObj([]string{"someFinalizer", "anotherFinalizer"}),
+			sets.NewString("someFinalizer"),
+			true,
+			[]string{"anotherFinalizer"},
+		},
+	}
+	for index, test := range testCases {
+		isUpdated, _ := RemoveFinalizers(test.obj, test.finalizers)
+		assert.Equal(t, isUpdated, test.isUpdated, fmt.Sprintf("Test case %d failed. Expected isUpdated: %v, actual: %v", index, test.isUpdated, isUpdated))
+		accessor, _ := meta.Accessor(test.obj)
+		newFinalizers := accessor.GetFinalizers()
+		assert.Equal(t, test.newFinalizers, newFinalizers, fmt.Sprintf("Test case %d failed. Expected finalizers: %v, actual: %v", index, test.newFinalizers, newFinalizers))
+	}
+}

federation/pkg/federation-controller/util/test/BUILD

@@ -14,9 +14,12 @@ go_library(
     deps = [
         "//federation/apis/federation/v1beta1:go_default_library",
         "//federation/pkg/federation-controller/util:go_default_library",
+        "//federation/pkg/federation-controller/util/finalizers:go_default_library",
         "//pkg/api:go_default_library",
         "//pkg/api/v1:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",
+        "//vendor/github.com/stretchr/testify/assert:go_default_library",
+        "//vendor/github.com/stretchr/testify/require:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",

federation/pkg/federation-controller/util/test/test_helper.go

@@ -22,6 +22,7 @@ import (
 	"reflect"
 	"runtime/pprof"
 	"sync"
+	"testing"
 	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -31,10 +32,13 @@ import (
 	core "k8s.io/client-go/testing"
 	federationapi "k8s.io/kubernetes/federation/apis/federation/v1beta1"
 	"k8s.io/kubernetes/federation/pkg/federation-controller/util"
+	finalizersutil "k8s.io/kubernetes/federation/pkg/federation-controller/util/finalizers"
 	"k8s.io/kubernetes/pkg/api"
 	apiv1 "k8s.io/kubernetes/pkg/api/v1"
 
 	"github.com/golang/glog"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -349,3 +353,9 @@ func MetaAndSpecCheckingFunction(expected runtime.Object) CheckingFunction {
 		return fmt.Errorf("Object different expected=%#v received=%#v", expected, obj)
 	}
 }
+
+func AssertHasFinalizer(t *testing.T, obj runtime.Object, finalizer string) {
+	hasFinalizer, err := finalizersutil.HasFinalizer(obj, finalizer)
+	require.Nil(t, err)
+	assert.True(t, hasFinalizer)
+}