diff --git a/federation/pkg/federation-controller/configmap/configmap_controller.go b/federation/pkg/federation-controller/configmap/configmap_controller.go index a7281af6809..ca0c0955a18 100644 --- a/federation/pkg/federation-controller/configmap/configmap_controller.go +++ b/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.removeFinalizerFunc, - configmapcontroller.addFinalizerFunc, + configmapcontroller.updateConfigMap, // 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. -func (configmapcontroller *ConfigMapController) hasFinalizerFunc(obj pkgruntime.Object, finalizer string) bool { +// Sends the given updated object to apiserver. +// 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 { - 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 + return configmapcontroller.federatedApiClient.Core().ConfigMaps(configmap.Namespace).Update(configmap) } func (configmapcontroller *ConfigMapController) Run(stopChan <-chan struct{}) { diff --git a/federation/pkg/federation-controller/configmap/configmap_controller_test.go b/federation/pkg/federation-controller/configmap/configmap_controller_test.go index 97a5c4c9d51..fdb6642ad81 100644 --- a/federation/pkg/federation-controller/configmap/configmap_controller_test.go +++ b/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)) - assert.True(t, configmapController.hasFinalizerFunc(updatedConfigMap, metav1.FinalizerOrphanDependents)) + AssertHasFinalizer(t, updatedConfigMap, deletionhelper.FinalizerDeleteFromUnderlyingClusters) + AssertHasFinalizer(t, updatedConfigMap, metav1.FinalizerOrphanDependents) // Verify that the configmap is created in underlying cluster1. createdConfigMap := GetConfigMapFromChan(cluster1CreateChan) diff --git a/federation/pkg/federation-controller/daemonset/daemonset_controller.go b/federation/pkg/federation-controller/daemonset/daemonset_controller.go index fef4b1618b7..2ee33334887 100644 --- a/federation/pkg/federation-controller/daemonset/daemonset_controller.go +++ b/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.removeFinalizerFunc, - daemonsetcontroller.addFinalizerFunc, + daemonsetcontroller.updateDaemonSet, // 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. -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. +// Sends the given updated object to apiserver. // 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{} - 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 + return daemonsetcontroller.federatedApiClient.Extensions().DaemonSets(daemonset.Namespace).Update(daemonset) } func (daemonsetcontroller *DaemonSetController) Run(stopChan <-chan struct{}) { diff --git a/federation/pkg/federation-controller/daemonset/daemonset_controller_test.go b/federation/pkg/federation-controller/daemonset/daemonset_controller_test.go index 15e1d4c8820..52b9f9f39bc 100644 --- a/federation/pkg/federation-controller/daemonset/daemonset_controller_test.go +++ b/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)) - assert.True(t, daemonsetController.hasFinalizerFunc(updatedDaemonSet, metav1.FinalizerOrphanDependents)) + AssertHasFinalizer(t, updatedDaemonSet, deletionhelper.FinalizerDeleteFromUnderlyingClusters) + AssertHasFinalizer(t, updatedDaemonSet, metav1.FinalizerOrphanDependents) daemonset1 = *updatedDaemonSet createdDaemonSet := GetDaemonSetFromChan(cluster1CreateChan) diff --git a/federation/pkg/federation-controller/deployment/deploymentcontroller.go b/federation/pkg/federation-controller/deployment/deploymentcontroller.go index 524815400c6..4b2328428cf 100644 --- a/federation/pkg/federation-controller/deployment/deploymentcontroller.go +++ b/federation/pkg/federation-controller/deployment/deploymentcontroller.go @@ -207,9 +207,7 @@ func NewDeploymentController(federationClient fedclientset.Interface) *Deploymen }) fdc.deletionHelper = deletionhelper.NewDeletionHelper( - fdc.hasFinalizerFunc, - fdc.removeFinalizerFunc, - fdc.addFinalizerFunc, + fdc.updateDeployment, // 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. -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. +// Sends the given updated object to apiserver. // 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{} - 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 + return fdc.fedClient.Extensions().Deployments(deployment.Namespace).Update(deployment) } func (fdc *DeploymentController) Run(workers int, stopCh <-chan struct{}) { diff --git a/federation/pkg/federation-controller/ingress/BUILD b/federation/pkg/federation-controller/ingress/BUILD index dec8abc80dd..ade9b9717ce 100644 --- a/federation/pkg/federation-controller/ingress/BUILD +++ b/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", diff --git a/federation/pkg/federation-controller/ingress/ingress_controller.go b/federation/pkg/federation-controller/ingress/ingress_controller.go index d5e29b52ded..ad9ff847636 100644 --- a/federation/pkg/federation-controller/ingress/ingress_controller.go +++ b/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.removeFinalizerFunc, - ic.addFinalizerFunc, + ic.updateIngress, // 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. -func (ic *IngressController) hasFinalizerFunc(obj pkgruntime.Object, finalizer string) bool { +// Sends the given updated object to apiserver. +// 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 { - 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 + return ic.federatedApiClient.Extensions().Ingresses(ingress.Namespace).Update(ingress) } func (ic *IngressController) Run(stopChan <-chan struct{}) { diff --git a/federation/pkg/federation-controller/ingress/ingress_controller_test.go b/federation/pkg/federation-controller/ingress/ingress_controller_test.go index 7a93930a85e..c1067b00fe4 100644 --- a/federation/pkg/federation-controller/ingress/ingress_controller_test.go +++ b/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)) - assert.True(t, ingressController.hasFinalizerFunc(updatedIngress, metav1.FinalizerOrphanDependents), fmt.Sprintf("ingress does not have the orphan finalizer: %v", updatedIngress)) + AssertHasFinalizer(t, updatedIngress, deletionhelper.FinalizerDeleteFromUnderlyingClusters) + 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) && - ingressController.hasFinalizerFunc(ingress, deletionhelper.FinalizerDeleteFromUnderlyingClusters) { + hasOrphanFinalizer, err := finalizersutil.HasFinalizer(ingress, metav1.FinalizerOrphanDependents) + 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 diff --git a/federation/pkg/federation-controller/namespace/BUILD b/federation/pkg/federation-controller/namespace/BUILD index 73a98cfbc62..82628434b40 100644 --- a/federation/pkg/federation-controller/namespace/BUILD +++ b/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", diff --git a/federation/pkg/federation-controller/namespace/namespace_controller.go b/federation/pkg/federation-controller/namespace/namespace_controller.go index 6de0e23a95f..b33e4ad1dba 100644 --- a/federation/pkg/federation-controller/namespace/namespace_controller.go +++ b/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.removeFinalizerFunc, - nc.addFinalizerFunc, + nc.updateNamespace, // 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. -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. +// Sends the given update object to apiserver. // 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{} - 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 + return nc.federatedApiClient.Core().Namespaces().Update(namespace) } // Returns true if the given object has the given finalizer in its NamespaceSpec. diff --git a/federation/pkg/federation-controller/namespace/namespace_controller_test.go b/federation/pkg/federation-controller/namespace/namespace_controller_test.go index 654a9efd388..11506ceb722 100644 --- a/federation/pkg/federation-controller/namespace/namespace_controller_test.go +++ b/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. - // TODO: Seems like a bug. Should invoke update. Fix it. - updatedNamespace := GetNamespaceFromChan(namespaceCreateChan) - assert.True(t, namespaceController.hasFinalizerFunc(updatedNamespace, deletionhelper.FinalizerDeleteFromUnderlyingClusters)) + updatedNamespace := GetNamespaceFromChan(namespaceUpdateChan) + require.NotNil(t, updatedNamespace) + AssertHasFinalizer(t, 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") diff --git a/federation/pkg/federation-controller/replicaset/replicasetcontroller.go b/federation/pkg/federation-controller/replicaset/replicasetcontroller.go index 3213040dc3a..2d2b5242170 100644 --- a/federation/pkg/federation-controller/replicaset/replicasetcontroller.go +++ b/federation/pkg/federation-controller/replicaset/replicasetcontroller.go @@ -215,9 +215,7 @@ func NewReplicaSetController(federationClient fedclientset.Interface) *ReplicaSe }) frsc.deletionHelper = deletionhelper.NewDeletionHelper( - frsc.hasFinalizerFunc, - frsc.removeFinalizerFunc, - frsc.addFinalizerFunc, + frsc.updateReplicaSet, // 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. -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. +// Sends the given updated object to apiserver. // 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{} - 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 + return frsc.fedClient.Extensions().ReplicaSets(replicaset.Namespace).Update(replicaset) } func (frsc *ReplicaSetController) Run(workers int, stopCh <-chan struct{}) { diff --git a/federation/pkg/federation-controller/service/servicecontroller.go b/federation/pkg/federation-controller/service/servicecontroller.go index 9b6cecc0d63..d17e2e0a402 100644 --- a/federation/pkg/federation-controller/service/servicecontroller.go +++ b/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.removeFinalizerFunc, - s.addFinalizerFunc, + s.updateService, // 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. -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. +// Sends the given updated object to apiserver. // 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{} - 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 + return s.federationClient.Core().Services(service.Namespace).Update(service) } // obj could be an *api.Service, or a DeletionFinalStateUnknown marker item. diff --git a/federation/pkg/federation-controller/sync/controller.go b/federation/pkg/federation-controller/sync/controller.go index 5180b7911eb..529521fd787 100644 --- a/federation/pkg/federation-controller/sync/controller.go +++ b/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.removeFinalizerFunc, - s.addFinalizerFunc, + s.updateObject, // 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. -func (s *FederationSyncController) hasFinalizerFunc(obj pkgruntime.Object, finalizer string) bool { - meta := s.adapter.ObjectMeta(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 +// Sends the given updated object to apiserver. +func (s *FederationSyncController) updateObject(obj pkgruntime.Object) (pkgruntime.Object, error) { + return s.adapter.FedUpdate(obj) } func (s *FederationSyncController) Run(stopChan <-chan struct{}) { diff --git a/federation/pkg/federation-controller/sync/secret_controller_test.go b/federation/pkg/federation-controller/sync/secret_controller_test.go index 2f55cbce2fe..7499070cc26 100644 --- a/federation/pkg/federation-controller/sync/secret_controller_test.go +++ b/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)) - assert.True(t, secretController.hasFinalizerFunc(updatedSecret, metav1.FinalizerOrphanDependents)) + AssertHasFinalizer(t, updatedSecret, deletionhelper.FinalizerDeleteFromUnderlyingClusters) + AssertHasFinalizer(t, updatedSecret, metav1.FinalizerOrphanDependents) secret1 = *updatedSecret // Verify that the secret is created in underlying cluster1. diff --git a/federation/pkg/federation-controller/util/BUILD b/federation/pkg/federation-controller/util/BUILD index b78689d6cbc..64c28474476 100644 --- a/federation/pkg/federation-controller/util/BUILD +++ b/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", diff --git a/federation/pkg/federation-controller/util/deletionhelper/BUILD b/federation/pkg/federation-controller/util/deletionhelper/BUILD index 6c4a2293197..be5628c873e 100644 --- a/federation/pkg/federation-controller/util/deletionhelper/BUILD +++ b/federation/pkg/federation-controller/util/deletionhelper/BUILD @@ -9,17 +9,16 @@ load( go_library( name = "go_default_library", - srcs = [ - "deletion_helper.go", - "util.go", - ], + srcs = ["deletion_helper.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", ], ) diff --git a/federation/pkg/federation-controller/util/deletionhelper/deletion_helper.go b/federation/pkg/federation-controller/util/deletionhelper/deletion_helper.go index 440f540f1a6..9efeeb573d4 100644 --- a/federation/pkg/federation-controller/util/deletionhelper/deletion_helper.go +++ b/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,37 +46,29 @@ const ( FinalizerDeleteFromUnderlyingClusters string = "federation.kubernetes.io/delete-from-underlying-clusters" ) -type HasFinalizerFunc func(runtime.Object, string) bool -type RemoveFinalizerFunc func(runtime.Object, []string) (runtime.Object, error) -type AddFinalizerFunc func(runtime.Object, []string) (runtime.Object, error) +type UpdateObjFunc func(runtime.Object) (runtime.Object, error) type ObjNameFunc func(runtime.Object) string type DeletionHelper struct { - hasFinalizerFunc HasFinalizerFunc - removeFinalizerFunc RemoveFinalizerFunc - addFinalizerFunc AddFinalizerFunc - objNameFunc ObjNameFunc - updateTimeout time.Duration - eventRecorder record.EventRecorder - informer util.FederatedInformer - updater util.FederatedUpdater + updateObjFunc UpdateObjFunc + objNameFunc ObjNameFunc + updateTimeout time.Duration + eventRecorder record.EventRecorder + informer util.FederatedInformer + updater util.FederatedUpdater } func NewDeletionHelper( - hasFinalizerFunc HasFinalizerFunc, removeFinalizerFunc RemoveFinalizerFunc, - addFinalizerFunc AddFinalizerFunc, objNameFunc ObjNameFunc, + updateObjFunc UpdateObjFunc, objNameFunc ObjNameFunc, updateTimeout time.Duration, eventRecorder record.EventRecorder, - informer util.FederatedInformer, - updater util.FederatedUpdater) *DeletionHelper { + informer util.FederatedInformer, updater util.FederatedUpdater) *DeletionHelper { return &DeletionHelper{ - hasFinalizerFunc: hasFinalizerFunc, - removeFinalizerFunc: removeFinalizerFunc, - addFinalizerFunc: addFinalizerFunc, - objNameFunc: objNameFunc, - updateTimeout: updateTimeout, - eventRecorder: eventRecorder, - informer: informer, - updater: updater, + updateObjFunc: updateObjFunc, + objNameFunc: objNameFunc, + updateTimeout: updateTimeout, + eventRecorder: eventRecorder, + informer: informer, + updater: updater, } } @@ -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{} - if !dh.hasFinalizerFunc(obj, FinalizerDeleteFromUnderlyingClusters) { - finalizers = append(finalizers, FinalizerDeleteFromUnderlyingClusters) + finalizers := sets.String{} + hasFinalizer, err := finalizersutil.HasFinalizer(obj, FinalizerDeleteFromUnderlyingClusters) + if err != nil { + return obj, err } - if !dh.hasFinalizerFunc(obj, metav1.FinalizerOrphanDependents) { - finalizers = append(finalizers, metav1.FinalizerOrphanDependents) + if !hasFinalizer { + finalizers.Insert(FinalizerDeleteFromUnderlyingClusters) } - if len(finalizers) != 0 { - glog.V(2).Infof("Adding finalizers %v to %s", finalizers, dh.objNameFunc(obj)) - return dh.addFinalizerFunc(obj, finalizers) + hasFinalizer, err = finalizersutil.HasFinalizer(obj, metav1.FinalizerOrphanDependents) + if err != nil { + 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} - return dh.removeFinalizerFunc(obj, finalizers) + finalizers := sets.NewString(FinalizerDeleteFromUnderlyingClusters, metav1.FinalizerOrphanDependents) + 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 } diff --git a/federation/pkg/federation-controller/util/deletionhelper/util.go b/federation/pkg/federation-controller/util/deletionhelper/util.go deleted file mode 100644 index e4b6f5f189f..00000000000 --- a/federation/pkg/federation-controller/util/deletionhelper/util.go +++ /dev/null @@ -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 -} diff --git a/federation/pkg/federation-controller/util/finalizers/BUILD b/federation/pkg/federation-controller/util/finalizers/BUILD new file mode 100644 index 00000000000..9a70cc1ab2e --- /dev/null +++ b/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", + ], +) diff --git a/federation/pkg/federation-controller/util/finalizers/finalizers.go b/federation/pkg/federation-controller/util/finalizers/finalizers.go new file mode 100644 index 00000000000..c1644887fc6 --- /dev/null +++ b/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 +} diff --git a/federation/pkg/federation-controller/util/finalizers/finalizers_test.go b/federation/pkg/federation-controller/util/finalizers/finalizers_test.go new file mode 100644 index 00000000000..8140c44507c --- /dev/null +++ b/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)) + } +} diff --git a/federation/pkg/federation-controller/util/test/BUILD b/federation/pkg/federation-controller/util/test/BUILD index 919699045d2..2e6e5468566 100644 --- a/federation/pkg/federation-controller/util/test/BUILD +++ b/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", diff --git a/federation/pkg/federation-controller/util/test/test_helper.go b/federation/pkg/federation-controller/util/test/test_helper.go index 4a9020725e5..aa5c38984eb 100644 --- a/federation/pkg/federation-controller/util/test/test_helper.go +++ b/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) +}