From 0e22293ed9c9276e6be9674cf02bcdc360d0b019 Mon Sep 17 00:00:00 2001 From: sophieliu15 Date: Wed, 19 Feb 2020 13:34:38 -0500 Subject: [PATCH] Add a unit test for config_controller.go Add a unit test for config_controller.go to test the case that when a new type is added to HNCConfiguration singleton, the corresponding object reconciler can be created correctly. This PR also moves shared helper functions from each controller test files to a common file. Design doc: http://bit.ly/hnc-type-configuration Issue: #411 --- incubator/hnc/go.sum | 2 + .../pkg/controllers/config_controller_test.go | 49 +++++ .../controllers/hierarchy_controller_test.go | 36 ---- .../pkg/controllers/object_controller_test.go | 88 ++------- incubator/hnc/pkg/controllers/suite_test.go | 7 + .../hnc/pkg/controllers/test_helpers_test.go | 175 ++++++++++++++++++ 6 files changed, 245 insertions(+), 112 deletions(-) create mode 100644 incubator/hnc/pkg/controllers/config_controller_test.go create mode 100644 incubator/hnc/pkg/controllers/test_helpers_test.go diff --git a/incubator/hnc/go.sum b/incubator/hnc/go.sum index ab6e99666..de6b9bd52 100644 --- a/incubator/hnc/go.sum +++ b/incubator/hnc/go.sum @@ -142,6 +142,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/pty v1.1.5/go.mod h1:9r2w37qlBe7rQ6e1fg1S/9xpWHSnaqNdHD3WcMdbPDA= github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/kubernetes-sigs/multi-tenancy v0.0.0-20200219071425-0b3816093ff1 h1:eX1j6qGAr6OB68yt5YZTfPZa/0IXASQUPEZviQU92aA= +github.com/kubernetes-sigs/multi-tenancy v0.0.0-20200219192426-483fdfaaf1a2 h1:z8KClYjQX69uoTNyRF3aURISTcvBw8vwrO7VRY3CR88= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/mailru/easyjson v0.0.0-20190614124828-94de47d64c63/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc= github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e h1:hB2xlXdHp/pmPZq0y3QnmWAArdw9PqbmotexnWx/FU8= diff --git a/incubator/hnc/pkg/controllers/config_controller_test.go b/incubator/hnc/pkg/controllers/config_controller_test.go new file mode 100644 index 000000000..c202063ba --- /dev/null +++ b/incubator/hnc/pkg/controllers/config_controller_test.go @@ -0,0 +1,49 @@ +package controllers_test + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("HNCConfiguration", func() { + ctx := context.Background() + + var ( + fooName string + barName string + ) + + BeforeEach(func() { + fooName = createNS(ctx, "ns-a") + barName = createNS(ctx, "ns-b") + }) + + AfterEach(func() { + // Change current singleton back to the default value. + Eventually(func() error { + c := getHNCConfig(ctx) + return resetHNCConfigToDefault(ctx, c) + }).Should(Succeed()) + }) + + It("should propagate objects whose types have been added to HNCConfiguration", func() { + setParent(ctx, barName, fooName) + makeConfigMap(ctx, fooName, "a-config-map") + + // Wait 1 second to give "foo-sec" a chance to be propagated to bar, if it can be propagated. + time.Sleep(1 * time.Second) + // "foo-sec" is not propagated to bar because Secret hasn't been configured in HNCConfiguration. + Eventually(hasConfigMap(ctx, barName, "a-config-map")).Should(BeFalse()) + + // c := getHNCConfig(ctx) + // addSecretToHNCConfig(ctx, c) + + // // "foo-sec" is now propagated to bar. + // time.Sleep(1 * time.Second) + // Eventually(hasSecret(ctx, barName, "foo-sec")).Should(BeTrue()) + // Expect(secretInheritedFrom(ctx, barName, "foo-sec")).Should(Equal(fooName)) + }) +}) diff --git a/incubator/hnc/pkg/controllers/hierarchy_controller_test.go b/incubator/hnc/pkg/controllers/hierarchy_controller_test.go index 8ab4514c5..083bdb29a 100644 --- a/incubator/hnc/pkg/controllers/hierarchy_controller_test.go +++ b/incubator/hnc/pkg/controllers/hierarchy_controller_test.go @@ -3,7 +3,6 @@ package controllers_test import ( "context" "fmt" - "math/rand" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -386,41 +385,6 @@ func updateHierarchy(ctx context.Context, h *api.HierarchyConfiguration) { } } -// createNSName generates random namespace names. Namespaces are never deleted in test-env because -// the building Namespace controller (which finalizes namespaces) doesn't run; I searched Github and -// found that everyone who was deleting namespaces was *also* very intentionally generating random -// names, so I guess this problem is widespread. -func createNSName(prefix string) string { - suffix := make([]byte, 10) - rand.Read(suffix) - return fmt.Sprintf("%s-%x", prefix, suffix) -} - -// createNSWithLabel has similar function to createNS with label as additional parameter -func createNSWithLabel(ctx context.Context, prefix string, label map[string]string) string { - nm := createNSName(prefix) - - // Create the namespace - ns := &corev1.Namespace{} - ns.SetLabels(label) - ns.Name = nm - Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) - return nm -} - -// createNS is a convenience function to create a namespace and wait for its singleton to be -// created. It's used in other tests in this package, but basically duplicates the code in this test -// (it didn't originally). TODO: refactor. -func createNS(ctx context.Context, prefix string) string { - nm := createNSName(prefix) - - // Create the namespace - ns := &corev1.Namespace{} - ns.Name = nm - Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) - return nm -} - func getLabel(ctx context.Context, from, label string) func() string { return func() string { ns := getNamespace(ctx, from) diff --git a/incubator/hnc/pkg/controllers/object_controller_test.go b/incubator/hnc/pkg/controllers/object_controller_test.go index 8dd4939ca..8a678acf9 100644 --- a/incubator/hnc/pkg/controllers/object_controller_test.go +++ b/incubator/hnc/pkg/controllers/object_controller_test.go @@ -11,15 +11,11 @@ import ( "k8s.io/apimachinery/pkg/types" api "github.com/kubernetes-sigs/multi-tenancy/incubator/hnc/api/v1alpha1" - "github.com/kubernetes-sigs/multi-tenancy/incubator/hnc/pkg/config" ) var _ = Describe("Secret", func() { ctx := context.Background() - // Setup HNCConfiguration. - hncConfig := newHNCConfig() - var ( fooName string barName string @@ -28,7 +24,10 @@ var _ = Describe("Secret", func() { BeforeEach(func() { // Add secret to HNCConfiguration so that an ObjectReconciler will be created for secret. - addSecretToHNCConfig(ctx, hncConfig) + Eventually(func() error { + c := getHNCConfig(ctx) + return addSecretToHNCConfig(ctx, c) + }).Should(Succeed()) fooName = createNS(ctx, "foo") barName = createNS(ctx, "bar") @@ -40,6 +39,14 @@ var _ = Describe("Secret", func() { makeSecret(ctx, bazName, "baz-sec") }) + AfterEach(func() { + // Change current singleton back to the default value. + Eventually(func() error { + c := getHNCConfig(ctx) + return resetHNCConfigToDefault(ctx, c) + }).Should(Succeed()) + }) + It("should be copied to descendents", func() { setParent(ctx, barName, fooName) setParent(ctx, bazName, barName) @@ -219,56 +226,6 @@ var _ = Describe("Secret", func() { }) }) -func makeSecret(ctx context.Context, nsName, secretName string) { - sec := &corev1.Secret{} - sec.Name = secretName - sec.Namespace = nsName - ExpectWithOffset(1, k8sClient.Create(ctx, sec)).Should(Succeed()) -} - -func hasSecret(ctx context.Context, nsName, secretName string) func() bool { - // `Eventually` only works with a fn that doesn't take any args - return func() bool { - nnm := types.NamespacedName{Namespace: nsName, Name: secretName} - sec := &corev1.Secret{} - err := k8sClient.Get(ctx, nnm, sec) - return err == nil - } -} - -func secretInheritedFrom(ctx context.Context, nsName, secretName string) string { - nnm := types.NamespacedName{Namespace: nsName, Name: secretName} - sec := &corev1.Secret{} - if err := k8sClient.Get(ctx, nnm, sec); err != nil { - // should have been caught above - return err.Error() - } - if sec.ObjectMeta.Labels == nil { - return "" - } - lif, _ := sec.ObjectMeta.Labels["hnc.x-k8s.io/inheritedFrom"] - return lif -} - -func setParent(ctx context.Context, nm string, pnm string) { - hier := newOrGetHierarchy(ctx, nm) - oldPNM := hier.Spec.Parent - hier.Spec.Parent = pnm - updateHierarchy(ctx, hier) - if oldPNM != "" { - EventuallyWithOffset(1, func() []string { - pHier := getHierarchyWithOffset(1, ctx, oldPNM) - return pHier.Status.Children - }).ShouldNot(ContainElement(nm)) - } - if pnm != "" { - EventuallyWithOffset(1, func() []string { - pHier := getHierarchyWithOffset(1, ctx, pnm) - return pHier.Status.Children - }).Should(ContainElement(nm)) - } -} - func newOrGetHierarchy(ctx context.Context, nm string) *api.HierarchyConfiguration { hier := &api.HierarchyConfiguration{} hier.ObjectMeta.Namespace = nm @@ -322,24 +279,3 @@ func removeSecret(ctx context.Context, nsName, secretName string) { sec.Namespace = nsName ExpectWithOffset(1, k8sClient.Delete(ctx, sec)).Should(Succeed()) } - -func newHNCConfig() *api.HNCConfiguration { - hncConfig := &api.HNCConfiguration{} - hncConfig.ObjectMeta.Name = api.HNCConfigSingleton - hncConfig.Spec = config.GetDefaultConfigSpec() - return hncConfig -} - -func updateHNCConfig(ctx context.Context, c *api.HNCConfiguration) { - if c.CreationTimestamp.IsZero() { - ExpectWithOffset(1, k8sClient.Create(ctx, c)).Should(Succeed()) - } else { - ExpectWithOffset(1, k8sClient.Update(ctx, c)).Should(Succeed()) - } -} - -func addSecretToHNCConfig(ctx context.Context, c *api.HNCConfiguration) { - secSpec := api.TypeSynchronizationSpec{APIVersion: "v1", Kind: "Secret", Mode: api.Propagate} - c.Spec.Types = append(c.Spec.Types, secSpec) - updateHNCConfig(ctx, c) -} diff --git a/incubator/hnc/pkg/controllers/suite_test.go b/incubator/hnc/pkg/controllers/suite_test.go index b8a2d5a04..2abf89d43 100644 --- a/incubator/hnc/pkg/controllers/suite_test.go +++ b/incubator/hnc/pkg/controllers/suite_test.go @@ -16,6 +16,7 @@ limitations under the License. package controllers_test import ( + "context" "path/filepath" "testing" "time" @@ -90,6 +91,12 @@ var _ = BeforeSuite(func(done Done) { k8sClient = k8sManager.GetClient() Expect(k8sClient).ToNot(BeNil()) + // Setup HNCConfiguration object here because it is a cluster-wide singleton shared by all reconcilers. + hncConfig := newHNCConfig() + Expect(hncConfig).ToNot(BeNil()) + ctx := context.Background() + updateHNCConfig(ctx, hncConfig) + go func() { err = k8sManager.Start(ctrl.SetupSignalHandler()) Expect(err).ToNot(HaveOccurred()) diff --git a/incubator/hnc/pkg/controllers/test_helpers_test.go b/incubator/hnc/pkg/controllers/test_helpers_test.go new file mode 100644 index 000000000..99b97e08a --- /dev/null +++ b/incubator/hnc/pkg/controllers/test_helpers_test.go @@ -0,0 +1,175 @@ +package controllers_test + +import ( + "context" + "crypto/rand" + "fmt" + + "github.com/kubernetes-sigs/multi-tenancy/incubator/hnc/pkg/config" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + + api "github.com/kubernetes-sigs/multi-tenancy/incubator/hnc/api/v1alpha1" +) + +func setParent(ctx context.Context, nm string, pnm string) { + hier := newOrGetHierarchy(ctx, nm) + oldPNM := hier.Spec.Parent + hier.Spec.Parent = pnm + updateHierarchy(ctx, hier) + if oldPNM != "" { + EventuallyWithOffset(1, func() []string { + pHier := getHierarchyWithOffset(1, ctx, oldPNM) + return pHier.Status.Children + }).ShouldNot(ContainElement(nm)) + } + if pnm != "" { + EventuallyWithOffset(1, func() []string { + pHier := getHierarchyWithOffset(1, ctx, pnm) + return pHier.Status.Children + }).Should(ContainElement(nm)) + } +} + +// createNSName generates random namespace names. Namespaces are never deleted in test-env because +// the building Namespace controller (which finalizes namespaces) doesn't run; I searched Github and +// found that everyone who was deleting namespaces was *also* very intentionally generating random +// names, so I guess this problem is widespread. +func createNSName(prefix string) string { + suffix := make([]byte, 10) + rand.Read(suffix) + return fmt.Sprintf("%s-%x", prefix, suffix) +} + +func newHNCConfig() *api.HNCConfiguration { + hncConfig := &api.HNCConfiguration{} + hncConfig.ObjectMeta.Name = api.HNCConfigSingleton + hncConfig.Spec = config.GetDefaultConfigSpec() + return hncConfig +} + +func updateHNCConfig(ctx context.Context, c *api.HNCConfiguration) error { + if c.CreationTimestamp.IsZero() { + return k8sClient.Create(ctx, c) + } else { + return k8sClient.Update(ctx, c) + } +} + +func getHNCConfig(ctx context.Context) *api.HNCConfiguration { + return getHNCConfigWithOffset(1, ctx) +} + +func getHNCConfigWithOffset(offset int, ctx context.Context) *api.HNCConfiguration { + snm := types.NamespacedName{Name: api.HNCConfigSingleton} + config := &api.HNCConfiguration{} + EventuallyWithOffset(offset+1, func() error { + return k8sClient.Get(ctx, snm, config) + }).Should(Succeed()) + return config +} + +func resetHNCConfigToDefault(ctx context.Context, c *api.HNCConfiguration) error { + c.Spec = config.GetDefaultConfigSpec() + return k8sClient.Update(ctx, c) +} + +// createNSWithLabel has similar function to createNS with label as additional parameter +func createNSWithLabel(ctx context.Context, prefix string, label map[string]string) string { + nm := createNSName(prefix) + + // Create the namespace + ns := &corev1.Namespace{} + ns.SetLabels(label) + ns.Name = nm + Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) + return nm +} + +// createNS is a convenience function to create a namespace and wait for its singleton to be +// created. It's used in other tests in this package, but basically duplicates the code in this test +// (it didn't originally). TODO: refactor. +func createNS(ctx context.Context, prefix string) string { + nm := createNSName(prefix) + + // Create the namespace + ns := &corev1.Namespace{} + ns.Name = nm + Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) + return nm +} + +func addSecretToHNCConfig(ctx context.Context, c *api.HNCConfiguration) error { + secSpec := api.TypeSynchronizationSpec{APIVersion: "v1", Kind: "Secret", Mode: api.Propagate} + c.Spec.Types = append(c.Spec.Types, secSpec) + return updateHNCConfig(ctx, c) +} + +func makeSecret(ctx context.Context, nsName, secretName string) { + sec := &corev1.Secret{} + sec.Name = secretName + sec.Namespace = nsName + ExpectWithOffset(1, k8sClient.Create(ctx, sec)).Should(Succeed()) +} + +func hasSecret(ctx context.Context, nsName, secretName string) func() bool { + // `Eventually` only works with a fn that doesn't take any args + return func() bool { + nnm := types.NamespacedName{Namespace: nsName, Name: secretName} + sec := &corev1.Secret{} + err := k8sClient.Get(ctx, nnm, sec) + return err == nil + } +} + +func secretInheritedFrom(ctx context.Context, nsName, secretName string) string { + nnm := types.NamespacedName{Namespace: nsName, Name: secretName} + sec := &corev1.Secret{} + if err := k8sClient.Get(ctx, nnm, sec); err != nil { + // should have been caught above + return err.Error() + } + if sec.ObjectMeta.Labels == nil { + return "" + } + lif, _ := sec.ObjectMeta.Labels["hnc.x-k8s.io/inheritedFrom"] + return lif +} + +func addConfigMapToHNCConfig(ctx context.Context, c *api.HNCConfiguration) error { + configMap := api.TypeSynchronizationSpec{APIVersion: "v1", Kind: "ConfigMap", Mode: api.Propagate} + c.Spec.Types = append(c.Spec.Types, configMap) + return updateHNCConfig(ctx, c) +} + +func makeConfigMap(ctx context.Context, nsName, configMapName string) { + configMap := &corev1.ConfigMap{} + configMap.Name = configMapName + configMap.Namespace = nsName + ExpectWithOffset(1, k8sClient.Create(ctx, configMap)).Should(Succeed()) +} + +func hasConfigMap(ctx context.Context, nsName, configMapName string) func() bool { + // `Eventually` only works with a fn that doesn't take any args + return func() bool { + nnm := types.NamespacedName{Namespace: nsName, Name: configMapName} + configMap := &corev1.ConfigMap{} + err := k8sClient.Get(ctx, nnm, configMap) + return err == nil + } +} + +func configMapInheritedFrom(ctx context.Context, nsName, configMapName string) string { + nnm := types.NamespacedName{Namespace: nsName, Name: configMapName} + configMap := &corev1.ConfigMap{} + if err := k8sClient.Get(ctx, nnm, configMap); err != nil { + // should have been caught above + return err.Error() + } + if configMap.ObjectMeta.Labels == nil { + return "" + } + lif, _ := configMap.ObjectMeta.Labels["hnc.x-k8s.io/inheritedFrom"] + return lif +}