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_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/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/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/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/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/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) +}