Refactor controller tests

- Move Manager creation and initialization to each controller test - Move creation of k8s objs that are not common for all controllers to each controller test - Use ordered containers to only setup manager once per controller test - remove the usage of context.TODO() from tests Signed-off-by: adrianc <[email protected]>
adrianchiris · Feb 1, 2024 · 14a2c1b · 14a2c1b
1 parent a3a5ba9
commit 14a2c1b
Show file tree

Hide file tree

Showing 5 changed files with 287 additions and 154 deletions.
diff --git a/controllers/sriovibnetwork_controller_test.go b/controllers/sriovibnetwork_controller_test.go
@@ -1,10 +1,11 @@
 package controllers
 
 import (
-	goctx "context"
+	"context"
 	"fmt"
 	"io"
 	"strings"
+	"sync"
 	"time"
 
 	netattdefv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
@@ -21,7 +22,39 @@ import (
 	"github.com/k8snetworkplumbingwg/sriov-network-operator/test/util"
 )
 
-var _ = Describe("SriovIBNetwork Controller", func() {
+var _ = Describe("SriovIBNetwork Controller", Ordered, func() {
+	var cancel context.CancelFunc
+	var ctx context.Context
+
+	BeforeAll(func() {
+		By("Setup controller manager")
+		k8sManager, err := setupK8sManagerForTest()
+		Expect(err).ToNot(HaveOccurred())
+
+		err = (&SriovIBNetworkReconciler{
+			Client: k8sManager.GetClient(),
+			Scheme: k8sManager.GetScheme(),
+		}).SetupWithManager(k8sManager)
+		Expect(err).ToNot(HaveOccurred())
+
+		ctx, cancel = context.WithCancel(context.Background())
+
+		wg := sync.WaitGroup{}
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			defer GinkgoRecover()
+			By("Start controller manager")
+			err := k8sManager.Start(ctx)
+			Expect(err).ToNot(HaveOccurred())
+		}()
+
+		DeferCleanup(func() {
+			By("Shutdown controller manager")
+			cancel()
+			wg.Wait()
+		})
+	})
 
 	Context("with SriovIBNetwork", func() {
 		specs := map[string]sriovnetworkv1.SriovIBNetworkSpec{
@@ -52,7 +85,7 @@ var _ = Describe("SriovIBNetwork Controller", func() {
 
 				By("Create the SriovIBNetwork Custom Resource")
 				// get global framework variables
-				err = k8sClient.Create(goctx.TODO(), &cr)
+				err = k8sClient.Create(ctx, &cr)
 				Expect(err).NotTo(HaveOccurred())
 				ns := testNamespace
 				if cr.Spec.NetworkNamespace != "" {
@@ -68,9 +101,9 @@ var _ = Describe("SriovIBNetwork Controller", func() {
 
 				By("Delete the SriovIBNetwork Custom Resource")
 				found := &sriovnetworkv1.SriovIBNetwork{}
-				err = k8sClient.Get(goctx.TODO(), types.NamespacedName{Namespace: cr.GetNamespace(), Name: cr.GetName()}, found)
+				err = k8sClient.Get(ctx, types.NamespacedName{Namespace: cr.GetNamespace(), Name: cr.GetName()}, found)
 				Expect(err).NotTo(HaveOccurred())
-				err = k8sClient.Delete(goctx.TODO(), found, []dynclient.DeleteOption{}...)
+				err = k8sClient.Delete(ctx, found, []dynclient.DeleteOption{}...)
 				Expect(err).NotTo(HaveOccurred())
 
 				netAttDef = &netattdefv1.NetworkAttachmentDefinition{}
@@ -98,25 +131,25 @@ var _ = Describe("SriovIBNetwork Controller", func() {
 		DescribeTable("should be possible to update net-att-def",
 			func(old, new sriovnetworkv1.SriovIBNetwork) {
 				old.Name = new.GetName()
-				err := k8sClient.Create(goctx.TODO(), &old)
+				err := k8sClient.Create(ctx, &old)
 				Expect(err).NotTo(HaveOccurred())
 				defer func() {
 					// Cleanup the test resource
-					Expect(k8sClient.Delete(goctx.TODO(), &old)).To(Succeed())
+					Expect(k8sClient.Delete(ctx, &old)).To(Succeed())
 				}()
 				found := &sriovnetworkv1.SriovIBNetwork{}
 				expect := generateExpectedIBNetConfig(&new)
 
 				retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 					// Retrieve the latest version of SriovIBNetwork before attempting update
 					// RetryOnConflict uses exponential backoff to avoid exhausting the apiserver
-					getErr := k8sClient.Get(goctx.TODO(), types.NamespacedName{Namespace: old.GetNamespace(), Name: old.GetName()}, found)
+					getErr := k8sClient.Get(ctx, types.NamespacedName{Namespace: old.GetNamespace(), Name: old.GetName()}, found)
 					if getErr != nil {
 						io.WriteString(GinkgoWriter, fmt.Sprintf("Failed to get latest version of SriovIBNetwork: %v", getErr))
 					}
 					found.Spec = new.Spec
 					found.Annotations = new.Annotations
-					updateErr := k8sClient.Update(goctx.TODO(), found)
+					updateErr := k8sClient.Update(ctx, found)
 					if getErr != nil {
 						io.WriteString(GinkgoWriter, fmt.Sprintf("Failed to update latest version of SriovIBNetwork: %v", getErr))
 					}
@@ -164,7 +197,7 @@ var _ = Describe("SriovIBNetwork Controller", func() {
 			var err error
 			expect := generateExpectedIBNetConfig(&cr)
 
-			err = k8sClient.Create(goctx.TODO(), &cr)
+			err = k8sClient.Create(ctx, &cr)
 			Expect(err).NotTo(HaveOccurred())
 			ns := testNamespace
 			if cr.Spec.NetworkNamespace != "" {
@@ -174,7 +207,7 @@ var _ = Describe("SriovIBNetwork Controller", func() {
 			err = util.WaitForNamespacedObject(netAttDef, k8sClient, ns, cr.GetName(), util.RetryInterval, util.Timeout)
 			Expect(err).NotTo(HaveOccurred())
 
-			err = k8sClient.Delete(goctx.TODO(), netAttDef)
+			err = k8sClient.Delete(ctx, netAttDef)
 			Expect(err).NotTo(HaveOccurred())
 			time.Sleep(3 * time.Second)
 			err = util.WaitForNamespacedObject(netAttDef, k8sClient, ns, cr.GetName(), util.RetryInterval, util.Timeout)
@@ -184,9 +217,9 @@ var _ = Describe("SriovIBNetwork Controller", func() {
 			Expect(strings.TrimSpace(netAttDef.Spec.Config)).To(Equal(expect))
 
 			found := &sriovnetworkv1.SriovIBNetwork{}
-			err = k8sClient.Get(goctx.TODO(), types.NamespacedName{Namespace: cr.GetNamespace(), Name: cr.GetName()}, found)
+			err = k8sClient.Get(ctx, types.NamespacedName{Namespace: cr.GetNamespace(), Name: cr.GetName()}, found)
 			Expect(err).NotTo(HaveOccurred())
-			err = k8sClient.Delete(goctx.TODO(), found, []dynclient.DeleteOption{}...)
+			err = k8sClient.Delete(ctx, found, []dynclient.DeleteOption{}...)
 			Expect(err).NotTo(HaveOccurred())
 		})
 	})
@@ -207,11 +240,11 @@ var _ = Describe("SriovIBNetwork Controller", func() {
 			var err error
 			expect := generateExpectedIBNetConfig(&cr)
 
-			err = k8sClient.Create(goctx.TODO(), &cr)
+			err = k8sClient.Create(ctx, &cr)
 			Expect(err).NotTo(HaveOccurred())
 
 			DeferCleanup(func() {
-				err = k8sClient.Delete(goctx.TODO(), &cr)
+				err = k8sClient.Delete(ctx, &cr)
 				Expect(err).NotTo(HaveOccurred())
 			})
 
@@ -227,10 +260,10 @@ var _ = Describe("SriovIBNetwork Controller", func() {
 			nsObj := &corev1.Namespace{
 				ObjectMeta: metav1.ObjectMeta{Name: "ib-ns-xxx"},
 			}
-			err = k8sClient.Create(goctx.TODO(), nsObj)
+			err = k8sClient.Create(ctx, nsObj)
 			Expect(err).NotTo(HaveOccurred())
 			DeferCleanup(func() {
-				err = k8sClient.Delete(goctx.TODO(), nsObj)
+				err = k8sClient.Delete(ctx, nsObj)
 				Expect(err).NotTo(HaveOccurred())
 			})
 

diff --git a/controllers/sriovnetwork_controller_test.go b/controllers/sriovnetwork_controller_test.go
@@ -1,10 +1,11 @@
 package controllers
 
 import (
-	goctx "context"
+	"context"
 	"fmt"
 	"io"
 	"strings"
+	"sync"
 	"time"
 
 	netattdefv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
@@ -27,7 +28,39 @@ const (
 	emptyCurls = "{}"
 )
 
-var _ = Describe("SriovNetwork Controller", func() {
+var _ = Describe("SriovNetwork Controller", Ordered, func() {
+	var cancel context.CancelFunc
+	var ctx context.Context
+
+	BeforeAll(func() {
+		By("Setup controller manager")
+		k8sManager, err := setupK8sManagerForTest()
+		Expect(err).ToNot(HaveOccurred())
+
+		err = (&SriovNetworkReconciler{
+			Client: k8sManager.GetClient(),
+			Scheme: k8sManager.GetScheme(),
+		}).SetupWithManager(k8sManager)
+		Expect(err).ToNot(HaveOccurred())
+
+		ctx, cancel = context.WithCancel(context.Background())
+
+		wg := sync.WaitGroup{}
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			defer GinkgoRecover()
+			By("Start controller manager")
+			err := k8sManager.Start(ctx)
+			Expect(err).ToNot(HaveOccurred())
+		}()
+
+		DeferCleanup(func() {
+			By("Shutdown controller manager")
+			cancel()
+			wg.Wait()
+		})
+	})
 
 	Context("with SriovNetwork", func() {
 		specs := map[string]sriovnetworkv1.SriovNetworkSpec{
@@ -72,7 +105,7 @@ var _ = Describe("SriovNetwork Controller", func() {
 
 				By("Create the SriovNetwork Custom Resource")
 				// get global framework variables
-				err = k8sClient.Create(goctx.TODO(), &cr)
+				err = k8sClient.Create(ctx, &cr)
 				Expect(err).NotTo(HaveOccurred())
 				ns := testNamespace
 				if cr.Spec.NetworkNamespace != "" {
@@ -88,9 +121,9 @@ var _ = Describe("SriovNetwork Controller", func() {
 
 				By("Delete the SriovNetwork Custom Resource")
 				found := &sriovnetworkv1.SriovNetwork{}
-				err = k8sClient.Get(goctx.TODO(), types.NamespacedName{Namespace: cr.GetNamespace(), Name: cr.GetName()}, found)
+				err = k8sClient.Get(ctx, types.NamespacedName{Namespace: cr.GetNamespace(), Name: cr.GetName()}, found)
 				Expect(err).NotTo(HaveOccurred())
-				err = k8sClient.Delete(goctx.TODO(), found, []dynclient.DeleteOption{}...)
+				err = k8sClient.Delete(ctx, found, []dynclient.DeleteOption{}...)
 				Expect(err).NotTo(HaveOccurred())
 
 				netAttDef = &netattdefv1.NetworkAttachmentDefinition{}
@@ -131,10 +164,10 @@ var _ = Describe("SriovNetwork Controller", func() {
 		DescribeTable("should be possible to update net-att-def",
 			func(old, new sriovnetworkv1.SriovNetwork) {
 				old.Name = new.GetName()
-				err := k8sClient.Create(goctx.TODO(), &old)
+				err := k8sClient.Create(ctx, &old)
 				defer func() {
 					// Cleanup the test resource
-					Expect(k8sClient.Delete(goctx.TODO(), &old)).To(Succeed())
+					Expect(k8sClient.Delete(ctx, &old)).To(Succeed())
 				}()
 				Expect(err).NotTo(HaveOccurred())
 				found := &sriovnetworkv1.SriovNetwork{}
@@ -143,13 +176,13 @@ var _ = Describe("SriovNetwork Controller", func() {
 				retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 					// Retrieve the latest version of SriovNetwork before attempting update
 					// RetryOnConflict uses exponential backoff to avoid exhausting the apiserver
-					getErr := k8sClient.Get(goctx.TODO(), types.NamespacedName{Namespace: old.GetNamespace(), Name: old.GetName()}, found)
+					getErr := k8sClient.Get(ctx, types.NamespacedName{Namespace: old.GetNamespace(), Name: old.GetName()}, found)
 					if getErr != nil {
 						io.WriteString(GinkgoWriter, fmt.Sprintf("Failed to get latest version of SriovNetwork: %v", getErr))
 					}
 					found.Spec = new.Spec
 					found.Annotations = new.Annotations
-					updateErr := k8sClient.Update(goctx.TODO(), found)
+					updateErr := k8sClient.Update(ctx, found)
 					if getErr != nil {
 						io.WriteString(GinkgoWriter, fmt.Sprintf("Failed to update latest version of SriovNetwork: %v", getErr))
 					}
@@ -200,7 +233,7 @@ var _ = Describe("SriovNetwork Controller", func() {
 				var err error
 				expect := generateExpectedNetConfig(&cr)
 
-				err = k8sClient.Create(goctx.TODO(), &cr)
+				err = k8sClient.Create(ctx, &cr)
 				Expect(err).NotTo(HaveOccurred())
 				ns := testNamespace
 				if cr.Spec.NetworkNamespace != "" {
@@ -210,7 +243,7 @@ var _ = Describe("SriovNetwork Controller", func() {
 				err = util.WaitForNamespacedObject(netAttDef, k8sClient, ns, cr.GetName(), util.RetryInterval, util.Timeout)
 				Expect(err).NotTo(HaveOccurred())
 
-				err = k8sClient.Delete(goctx.TODO(), netAttDef)
+				err = k8sClient.Delete(ctx, netAttDef)
 				Expect(err).NotTo(HaveOccurred())
 				time.Sleep(3 * time.Second)
 				err = util.WaitForNamespacedObject(netAttDef, k8sClient, ns, cr.GetName(), util.RetryInterval, util.Timeout)
@@ -220,9 +253,9 @@ var _ = Describe("SriovNetwork Controller", func() {
 				Expect(strings.TrimSpace(netAttDef.Spec.Config)).To(Equal(expect))
 
 				found := &sriovnetworkv1.SriovNetwork{}
-				err = k8sClient.Get(goctx.TODO(), types.NamespacedName{Namespace: cr.GetNamespace(), Name: cr.GetName()}, found)
+				err = k8sClient.Get(ctx, types.NamespacedName{Namespace: cr.GetNamespace(), Name: cr.GetName()}, found)
 				Expect(err).NotTo(HaveOccurred())
-				err = k8sClient.Delete(goctx.TODO(), found, []dynclient.DeleteOption{}...)
+				err = k8sClient.Delete(ctx, found, []dynclient.DeleteOption{}...)
 				Expect(err).NotTo(HaveOccurred())
 			})
 		})
@@ -244,11 +277,11 @@ var _ = Describe("SriovNetwork Controller", func() {
 				var err error
 				expect := generateExpectedNetConfig(&cr)
 
-				err = k8sClient.Create(goctx.TODO(), &cr)
+				err = k8sClient.Create(ctx, &cr)
 				Expect(err).NotTo(HaveOccurred())
 
 				DeferCleanup(func() {
-					err = k8sClient.Delete(goctx.TODO(), &cr)
+					err = k8sClient.Delete(ctx, &cr)
 					Expect(err).NotTo(HaveOccurred())
 				})
 
@@ -264,10 +297,10 @@ var _ = Describe("SriovNetwork Controller", func() {
 				nsObj := &corev1.Namespace{
 					ObjectMeta: metav1.ObjectMeta{Name: "ns-xxx"},
 				}
-				err = k8sClient.Create(goctx.TODO(), nsObj)
+				err = k8sClient.Create(ctx, nsObj)
 				Expect(err).NotTo(HaveOccurred())
 				DeferCleanup(func() {
-					err = k8sClient.Delete(goctx.TODO(), nsObj)
+					err = k8sClient.Delete(ctx, nsObj)
 					Expect(err).NotTo(HaveOccurred())
 				})