Making tests less flaky (#172)

projectcapsule · Dec 20, 2020 · 82bbd23 · 82bbd23
1 parent 03eb6e6
commit 82bbd23
Show file tree

Hide file tree

Showing 23 changed files with 303 additions and 265 deletions.
diff --git a/e2e/additional_role_bindings_test.go b/e2e/additional_role_bindings_test.go
@@ -30,7 +30,7 @@ import (
 	"github.com/clastix/capsule/api/v1alpha1"
 )
 
-var _ = Describe("creating a Namespace with additional Role Binding", func() {
+var _ = Describe("creating a Namespace with an additional Role Binding", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "additional-role-binding",
@@ -54,6 +54,7 @@ var _ = Describe("creating a Namespace with additional Role Binding", func() {
 			},
 		},
 	}
+
 	JustBeforeEach(func() {
 		EventuallyCreation(func() error {
 			tnt.ResourceVersion = ""
@@ -63,7 +64,8 @@ var _ = Describe("creating a Namespace with additional Role Binding", func() {
 	JustAfterEach(func() {
 		Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed())
 	})
-	It("should be assigned on each Namespace", func() {
+
+	It("should be assigned to each Namespace", func() {
 		for _, ns := range []string{"rb-1", "rb-2", "rb-3"} {
 			ns := NewNamespace(ns)
 			NamespaceCreation(ns, tnt, defaultTimeoutInterval).Should(Succeed())

diff --git a/e2e/allowed_external_ips_test.go b/e2e/allowed_external_ips_test.go
@@ -30,7 +30,7 @@ import (
 	"github.com/clastix/capsule/api/v1alpha1"
 )
 
-var _ = Describe("enforcing an allowed set of Service External IPs", func() {
+var _ = Describe("enforcing an allowed set of Service external IPs", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "allowed-external-ip",
@@ -48,14 +48,17 @@ var _ = Describe("enforcing an allowed set of Service External IPs", func() {
 			},
 		},
 	}
+
 	JustBeforeEach(func() {
 		EventuallyCreation(func() error {
-			return k8sClient.Create(context.TODO(), tnt.DeepCopy())
+			tnt.ResourceVersion = ""
+			return k8sClient.Create(context.TODO(), tnt)
 		}).Should(Succeed())
 	})
 	JustAfterEach(func() {
-		Expect(k8sClient.Delete(context.TODO(), tnt.DeepCopy())).Should(Succeed())
+		Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed())
 	})
+
 	It("should fail creating an evil service", func() {
 		ns := NewNamespace("evil-service")
 		NamespaceCreation(ns, tnt, defaultTimeoutInterval).Should(Succeed())
@@ -88,6 +91,7 @@ var _ = Describe("enforcing an allowed set of Service External IPs", func() {
 			return err
 		}).ShouldNot(Succeed())
 	})
+
 	It("should allow the first CIDR block", func() {
 		ns := NewNamespace("allowed-service-cidr")
 		NamespaceCreation(ns, tnt, defaultTimeoutInterval).Should(Succeed())

diff --git a/e2e/container_registry_test.go b/e2e/container_registry_test.go
@@ -33,7 +33,7 @@ import (
 var _ = Describe("enforcing a Container Registry", func() {
 	tnt := &v1alpha1.Tenant{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "additional-role-binding",
+			Name: "container-registry",
 		},
 		Spec: v1alpha1.TenantSpec{
 			Owner: v1alpha1.OwnerSpec{
@@ -44,51 +44,36 @@ var _ = Describe("enforcing a Container Registry", func() {
 				Allowed:      []string{"docker.io", "docker.tld"},
 				AllowedRegex: `quay\.\w+`,
 			},
-			NamespacesMetadata: v1alpha1.AdditionalMetadata{},
-			ServicesMetadata:   v1alpha1.AdditionalMetadata{},
-			StorageClasses:     &v1alpha1.StorageClassesSpec{},
-			IngressClasses:     &v1alpha1.IngressClassesSpec{},
-			LimitRanges:        []corev1.LimitRangeSpec{},
-			NodeSelector:       map[string]string{},
-			ResourceQuota:      []corev1.ResourceQuotaSpec{},
 		},
 	}
+
 	JustBeforeEach(func() {
 		EventuallyCreation(func() error {
-			return k8sClient.Create(context.TODO(), tnt.DeepCopy())
+			tnt.ResourceVersion = ""
+			return k8sClient.Create(context.TODO(), tnt)
 		}).Should(Succeed())
 	})
 	JustAfterEach(func() {
-		Expect(k8sClient.Delete(context.TODO(), tnt.DeepCopy())).Should(Succeed())
+		Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed())
 	})
+
 	It("should add labels to Namespace", func() {
 		ns := NewNamespace("registry-labels")
 		NamespaceCreation(ns, tnt, defaultTimeoutInterval).Should(Succeed())
-
-		Eventually(func() bool {
-			if err := k8sClient.Get(context.Background(), types.NamespacedName{Name: ns.Name}, ns); err != nil {
-				return false
+		Eventually(func() (ok bool) {
+			Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: ns.Name}, ns)).Should(Succeed())
+			ok, _ = HaveKeyWithValue("capsule.clastix.io/allowed-registries", "docker.io,docker.tld").Match(ns.Annotations)
+			if !ok {
+				return
 			}
-
-			for a, expected := range map[string]string{
-				"capsule.clastix.io/allowed-registries":        "docker.io,docker.tld",
-				"capsule.clastix.io/allowed-registries-regexp": `quay\.\w+`,
-			} {
-				var v string
-				var ok bool
-
-				v, ok = ns.Annotations[a]
-				if !ok {
-					return false
-				}
-				if ok = v == expected; !ok {
-					return false
-				}
+			ok, _ = HaveKeyWithValue("capsule.clastix.io/allowed-registries-regexp", `quay\.\w+`).Match(ns.Annotations)
+			if !ok {
+				return
 			}
-
 			return true
 		}, defaultTimeoutInterval, defaultPollInterval).Should(BeTrue())
 	})
+
 	It("should deny running a gcr.io container", func() {
 		ns := NewNamespace("registry-deny")
 		NamespaceCreation(ns, tnt, defaultTimeoutInterval).Should(Succeed())
@@ -110,7 +95,8 @@ var _ = Describe("enforcing a Container Registry", func() {
 		_, err := cs.CoreV1().Pods(ns.Name).Create(context.Background(), pod, metav1.CreateOptions{})
 		Expect(err).ShouldNot(Succeed())
 	})
-	It("should allow using an item in the list", func() {
+
+	It("should allow using an exact match", func() {
 		ns := NewNamespace("registry-list")
 		NamespaceCreation(ns, tnt, defaultTimeoutInterval).Should(Succeed())
 
@@ -134,7 +120,8 @@ var _ = Describe("enforcing a Container Registry", func() {
 			return err
 		}).Should(Succeed())
 	})
-	It("should allow using a registry from regex", func() {
+
+	It("should allow using a regex match", func() {
 		ns := NewNamespace("registry-regex")
 		NamespaceCreation(ns, tnt, defaultTimeoutInterval).Should(Succeed())
 

diff --git a/e2e/custom_capsule_group_test.go b/e2e/custom_capsule_group_test.go
@@ -40,6 +40,7 @@ var _ = Describe("creating a Namespace as Tenant owner with custom --capsule-gro
 			},
 		},
 	}
+
 	JustBeforeEach(func() {
 		EventuallyCreation(func() error {
 			tnt.ResourceVersion = ""
@@ -49,19 +50,20 @@ var _ = Describe("creating a Namespace as Tenant owner with custom --capsule-gro
 	JustAfterEach(func() {
 		Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed())
 	})
-	It("should fail", func() {
+
+	It("should fail using a User non matching the capsule-user-group flag", func() {
 		args := append(defaulManagerPodArgs, []string{"--capsule-user-group=test"}...)
 		ModifyCapsuleManagerPodArgs(args)
 		CapsuleClusterGroupParam(podRecreationTimeoutInterval).Should(BeIdenticalTo("test"))
 		ns := NewNamespace("cg-namespace-fail")
 		NamespaceCreation(ns, tnt, podRecreationTimeoutInterval).ShouldNot(Succeed())
 	})
+
 	It("should succeed and be available in Tenant namespaces list", func() {
 		ModifyCapsuleManagerPodArgs(defaulManagerPodArgs)
 		CapsuleClusterGroupParam(podRecreationTimeoutInterval).Should(BeIdenticalTo("capsule.clastix.io"))
 		ns := NewNamespace("cg-namespace")
 		NamespaceCreation(ns, tnt, podRecreationTimeoutInterval).Should(Succeed())
 		TenantNamespaceList(tnt, podRecreationTimeoutInterval).Should(ContainElement(ns.GetName()))
-
 	})
 })
diff --git a/e2e/force_tenant_prefix_test.go b/e2e/force_tenant_prefix_test.go
@@ -51,6 +51,7 @@ var _ = Describe("creating a Namespace with --force-tenant-name flag", func() {
 			},
 		},
 	}
+
 	JustBeforeEach(func() {
 		EventuallyCreation(func() error {
 			t1.ResourceVersion = ""
@@ -67,15 +68,18 @@ var _ = Describe("creating a Namespace with --force-tenant-name flag", func() {
 		Expect(k8sClient.Delete(context.TODO(), t2)).Should(Succeed())
 		ModifyCapsuleManagerPodArgs(defaulManagerPodArgs)
 	})
-	It("should fail with missing prefix", func() {
+
+	It("should fail when non using prefix", func() {
 		ns := NewNamespace("awesome")
 		NamespaceCreation(ns, t1, defaultTimeoutInterval).ShouldNot(Succeed())
 	})
+
 	It("should succeed using prefix", func() {
 		ns := NewNamespace("awesome-namespace")
 		NamespaceCreation(ns, t1, defaultTimeoutInterval).Should(Succeed())
 	})
-	It("should be assigned according closest match", func() {
+
+	It("should succeed and assigned according to closest match", func() {
 		ns1 := NewNamespace("awesome-tenant")
 		ns2 := NewNamespace("awesome-tenant-namespace")
 

diff --git a/e2e/ingress_class_test.go b/e2e/ingress_class_test.go
@@ -2,10 +2,13 @@
 
 /*
 Copyright 2020 Clastix Labs.
+
 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.
@@ -47,6 +50,7 @@ var _ = Describe("when Tenant handles Ingress classes", func() {
 			},
 		},
 	}
+
 	JustBeforeEach(func() {
 		EventuallyCreation(func() error {
 			tnt.ResourceVersion = ""
@@ -56,14 +60,15 @@ var _ = Describe("when Tenant handles Ingress classes", func() {
 	JustAfterEach(func() {
 		Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed())
 	})
-	It("should block non allowed Ingress class", func() {
+
+	It("should block a non allowed class", func() {
 		ns := NewNamespace("ingress-class-disallowed")
 		cs := ownerClient(tnt)
 
 		NamespaceCreation(ns, tnt, defaultTimeoutInterval).Should(Succeed())
 		TenantNamespaceList(tnt, podRecreationTimeoutInterval).Should(ContainElement(ns.GetName()))
 
-		By("non-specifying the class", func() {
+		By("non-specifying at all", func() {
 			Eventually(func() (err error) {
 				i := &extensionsv1beta1.Ingress{
 					ObjectMeta: metav1.ObjectMeta{
@@ -80,7 +85,7 @@ var _ = Describe("when Tenant handles Ingress classes", func() {
 				return
 			}, defaultTimeoutInterval, defaultPollInterval).ShouldNot(Succeed())
 		})
-		By("using a forbidden class as Annotation", func() {
+		By("defining as deprecated annotation", func() {
 			Eventually(func() (err error) {
 				i := &extensionsv1beta1.Ingress{
 					ObjectMeta: metav1.ObjectMeta{
@@ -100,7 +105,7 @@ var _ = Describe("when Tenant handles Ingress classes", func() {
 				return
 			}, defaultTimeoutInterval, defaultPollInterval).ShouldNot(Succeed())
 		})
-		By("specifying a forbidden class", func() {
+		By("using the ingressClassName", func() {
 			Eventually(func() (err error) {
 				i := &extensionsv1beta1.Ingress{
 					ObjectMeta: metav1.ObjectMeta{
@@ -119,7 +124,8 @@ var _ = Describe("when Tenant handles Ingress classes", func() {
 			}, defaultTimeoutInterval, defaultPollInterval).ShouldNot(Succeed())
 		})
 	})
-	It("should allow enabled Ingress class using the deprecated Annotation", func() {
+
+	It("should allow enabled class using the deprecated annotation", func() {
 		ns := NewNamespace("ingress-class-allowed-annotation")
 		cs := ownerClient(tnt)
 
@@ -147,7 +153,8 @@ var _ = Describe("when Tenant handles Ingress classes", func() {
 			}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
 		}
 	})
-	It("should allow enabled Ingress class using the IngressClassName field", func() {
+
+	It("should allow enabled class using the ingressClassName field", func() {
 		ns := NewNamespace("ingress-class-allowed-annotation")
 		cs := ownerClient(tnt)
 
@@ -178,7 +185,8 @@ var _ = Describe("when Tenant handles Ingress classes", func() {
 			}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
 		}
 	})
-	It("should allow enabled Ingress class regexp using the deprecated Annotation", func() {
+
+	It("should allow enabled Ingress by regex using the deprecated annotation", func() {
 		ns := NewNamespace("ingress-class-allowed-annotation")
 		cs := ownerClient(tnt)
 		ingressClass := "oil-ingress"
@@ -205,7 +213,8 @@ var _ = Describe("when Tenant handles Ingress classes", func() {
 			return
 		}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
 	})
-	It("should allow enabled Ingress class regexp using the IngressClassName field", func() {
+
+	It("should allow enabled Ingress by regex using the ingressClassName field", func() {
 		ns := NewNamespace("ingress-class-allowed-annotation")
 		cs := ownerClient(tnt)
 		ingressClass := "oil-haproxy"

diff --git a/e2e/missing_tenant_test.go b/e2e/missing_tenant_test.go
@@ -28,7 +28,7 @@ import (
 	"github.com/clastix/capsule/api/v1alpha1"
 )
 
-var _ = Describe("Namespace creation with no Tenant assigned", func() {
+var _ = Describe("creating a Namespace creation with no Tenant assigned", func() {
 	It("should fail", func() {
 		tnt := &v1alpha1.Tenant{
 			Spec: v1alpha1.TenantSpec{

diff --git a/e2e/namespace_capsule_label_test.go b/e2e/namespace_capsule_label_test.go
@@ -42,14 +42,17 @@ var _ = Describe("creating several Namespaces for a Tenant", func() {
 			},
 		},
 	}
+
 	JustBeforeEach(func() {
 		EventuallyCreation(func() error {
-			return k8sClient.Create(context.TODO(), tnt.DeepCopy())
+			tnt.ResourceVersion = ""
+			return k8sClient.Create(context.TODO(), tnt)
 		}).Should(Succeed())
 	})
 	JustAfterEach(func() {
 		Expect(k8sClient.Delete(context.TODO(), tnt)).Should(Succeed())
 	})
+
 	It("should contains the default Capsule label", func() {
 		namespaces := []*v1.Namespace{
 			NewNamespace("first-capsule-ns"),
@@ -58,10 +61,11 @@ var _ = Describe("creating several Namespaces for a Tenant", func() {
 		}
 		for _, ns := range namespaces {
 			NamespaceCreation(ns, tnt, defaultTimeoutInterval).Should(Succeed())
-			TenantNamespaceList(tnt, defaultTimeoutInterval).Should(ContainElement(ns.GetName()))
-
-			Expect(k8sClient.Get(context.TODO(), types.NamespacedName{Name: ns.GetName()}, ns)).Should(Succeed())
-			Expect(ns.Labels).Should(HaveKeyWithValue("capsule.clastix.io/tenant", tnt.Name))
+			Eventually(func() (ok bool) {
+				Expect(k8sClient.Get(context.TODO(), types.NamespacedName{Name: ns.GetName()}, ns)).Should(Succeed())
+				ok, _ = HaveKeyWithValue("capsule.clastix.io/tenant", tnt.Name).Match(ns.Labels)
+				return
+			}, defaultTimeoutInterval, defaultPollInterval).Should(BeTrue())
 		}
 	})
 })