topology-gc: refactor unit tests

Remove a lot of boilerplate code by defining reusable functions. Also, test the Run() method instead of the functions callees of Run() as it is the top level functionality that was tested in practice (we don't have separate unit tests for the callee functions).
kubernetes-sigs · Aug 18, 2023 · 7cdeae1 · 7cdeae1
1 parent 4939d0f
commit 7cdeae1
Showing 1 changed file with 83 additions and 165 deletions.
diff --git a/pkg/nfd-topology-gc/nfd-nrt-gc_test.go b/pkg/nfd-topology-gc/nfd-nrt-gc_test.go
@@ -22,212 +22,130 @@ import (
 	"time"
 
 	"github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/apis/topology/v1alpha2"
+	topologyclientset "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/generated/clientset/versioned"
 	faketopologyv1alpha2 "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/generated/clientset/versioned/fake"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/informers"
+	k8sclientset "k8s.io/client-go/kubernetes"
 	fakek8sclientset "k8s.io/client-go/kubernetes/fake"
 
 	. "github.com/smartystreets/goconvey/convey"
 )
 
 func TestNRTGC(t *testing.T) {
 	Convey("When theres is old NRT ", t, func() {
-		k8sClient := fakek8sclientset.NewSimpleClientset()
+		gc := newMockGC(nil, []string{"node1"})
 
-		fakeClient := faketopologyv1alpha2.NewSimpleClientset(&v1alpha2.NodeResourceTopology{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "node1",
-			},
-		})
-		factory := informers.NewSharedInformerFactory(k8sClient, 5*time.Minute)
-
-		stopChan := make(chan struct{}, 1)
-
-		gc := &topologyGC{
-			factory:    factory,
-			topoClient: fakeClient,
-			stopChan:   stopChan,
-			gcPeriod:   10 * time.Minute,
-		}
-
-		err := gc.run()
-		So(err, ShouldBeNil)
+		errChan := make(chan error, 1)
+		go func() { errChan <- gc.Run() }()
 
-		nrts, err := fakeClient.TopologyV1alpha2().NodeResourceTopologies().List(context.TODO(), metav1.ListOptions{})
-		So(err, ShouldBeNil)
-		So(nrts.Items, ShouldHaveLength, 0)
+		So(waitForNRT(gc.topoClient), ShouldBeTrue)
 
 		gc.Stop()
+		So(<-errChan, ShouldBeNil)
 	})
 	Convey("When theres is one old NRT and one up to date", t, func() {
-		k8sClient := fakek8sclientset.NewSimpleClientset(&corev1.Node{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "node1",
-			},
-		})
-
-		fakeClient := faketopologyv1alpha2.NewSimpleClientset(&v1alpha2.NodeResourceTopology{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "node1",
-			},
-		},
-			&v1alpha2.NodeResourceTopology{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node2",
-				},
-			},
-		)
+		gc := newMockGC([]string{"node1"}, []string{"node1", "node2"})
 
-		stopChan := make(chan struct{}, 1)
+		errChan := make(chan error, 1)
+		go func() { errChan <- gc.Run() }()
 
-		factory := informers.NewSharedInformerFactory(k8sClient, 5*time.Minute)
+		So(waitForNRT(gc.topoClient, "node1"), ShouldBeTrue)
 
-		gc := &topologyGC{
-			factory:    factory,
-			topoClient: fakeClient,
-			stopChan:   stopChan,
-			gcPeriod:   10 * time.Minute,
-		}
+		gc.Stop()
+		So(<-errChan, ShouldBeNil)
+	})
+	Convey("Should react to delete event", t, func() {
+		gc := newMockGC([]string{"node1", "node2"}, []string{"node1", "node2"})
 
-		err := gc.run()
-		So(err, ShouldBeNil)
+		errChan := make(chan error, 1)
+		go func() { errChan <- gc.Run() }()
 
-		nrts, err := fakeClient.TopologyV1alpha2().NodeResourceTopologies().List(context.TODO(), metav1.ListOptions{})
+		err := gc.k8sClient.CoreV1().Nodes().Delete(context.TODO(), "node1", metav1.DeleteOptions{})
 		So(err, ShouldBeNil)
-		So(nrts.Items, ShouldHaveLength, 1)
-		So(nrts.Items[0].GetName(), ShouldEqual, "node1")
 
+		So(waitForNRT(gc.topoClient, "node2"), ShouldBeTrue)
 	})
-	Convey("Should react to delete event", t, func() {
-		k8sClient := fakek8sclientset.NewSimpleClientset(
-			&corev1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node1",
-				},
-			},
-			&corev1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node2",
-				},
-			},
-		)
+	Convey("periodic GC should remove obsolete NRT", t, func() {
+		gc := newMockGC([]string{"node1", "node2"}, []string{"node1", "node2"})
+		// Override period to run fast
+		gc.gcPeriod = 100 * time.Millisecond
 
-		fakeClient := faketopologyv1alpha2.NewSimpleClientset(
-			&v1alpha2.NodeResourceTopology{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node1",
-				},
-			},
-			&v1alpha2.NodeResourceTopology{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node2",
-				},
+		nrt := v1alpha2.NodeResourceTopology{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "not-existing",
 			},
-		)
-
-		stopChan := make(chan struct{}, 1)
-
-		factory := informers.NewSharedInformerFactory(k8sClient, 5*time.Minute)
-		gc := &topologyGC{
-			factory:    factory,
-			topoClient: fakeClient,
-			stopChan:   stopChan,
-			gcPeriod:   10 * time.Minute,
 		}
 
-		err := gc.run()
-		So(err, ShouldBeNil)
+		errChan := make(chan error, 1)
+		go func() { errChan <- gc.Run() }()
 
-		nrts, err := fakeClient.TopologyV1alpha2().NodeResourceTopologies().List(context.TODO(), metav1.ListOptions{})
+		_, err := gc.topoClient.TopologyV1alpha2().NodeResourceTopologies().Create(context.TODO(), &nrt, metav1.CreateOptions{})
 		So(err, ShouldBeNil)
 
-		So(nrts.Items, ShouldHaveLength, 2)
-
-		err = k8sClient.CoreV1().Nodes().Delete(context.TODO(), "node1", metav1.DeleteOptions{})
-		So(err, ShouldBeNil)
-		// simple sleep with retry loop to make sure indexer will pick up event and trigger deleteNode Function
-		deleted := false
-		for i := 0; i < 5; i++ {
-			nrts, err := fakeClient.TopologyV1alpha2().NodeResourceTopologies().List(context.TODO(), metav1.ListOptions{})
-			So(err, ShouldBeNil)
-
-			if len(nrts.Items) == 1 {
-				deleted = true
-				break
-			}
-			time.Sleep(time.Second)
-		}
-		So(deleted, ShouldBeTrue)
+		So(waitForNRT(gc.topoClient, "node1", "node2"), ShouldBeTrue)
 	})
-	Convey("periodic GC should remove obsolete NRT", t, func() {
-		k8sClient := fakek8sclientset.NewSimpleClientset(
-			&corev1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node1",
-				},
-			},
-			&corev1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node2",
-				},
-			},
-		)
+}
 
-		fakeClient := faketopologyv1alpha2.NewSimpleClientset(
-			&v1alpha2.NodeResourceTopology{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node1",
-				},
-			},
-			&v1alpha2.NodeResourceTopology{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node2",
-				},
-			},
-		)
+func newMockGC(nodes, nrts []string) *mockGC {
+	k8sClient := fakek8sclientset.NewSimpleClientset(createFakeNodes(nodes...)...)
+	return &mockGC{
+		topologyGC: topologyGC{
+			factory:    informers.NewSharedInformerFactory(k8sClient, 5*time.Minute),
+			topoClient: faketopologyv1alpha2.NewSimpleClientset(createFakeNRTs(nrts...)...),
+			stopChan:   make(chan struct{}, 1),
+			gcPeriod:   10 * time.Minute,
+		},
+		k8sClient: k8sClient,
+	}
+}
 
-		stopChan := make(chan struct{}, 1)
+func createFakeNodes(names ...string) []runtime.Object {
+	nodes := make([]runtime.Object, len(names))
+	for i, n := range names {
+		nodes[i] = &corev1.Node{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: n,
+			}}
+	}
+	return nodes
+}
 
-		factory := informers.NewSharedInformerFactory(k8sClient, 5*time.Minute)
-		gc := &topologyGC{
-			factory:    factory,
-			topoClient: fakeClient,
-			stopChan:   stopChan,
-			gcPeriod:   time.Second,
-		}
+func createFakeNRTs(names ...string) []runtime.Object {
+	nrts := make([]runtime.Object, len(names))
+	for i, n := range names {
+		nrts[i] = &v1alpha2.NodeResourceTopology{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: n,
+			}}
+	}
+	return nrts
+}
 
-		err := gc.run()
-		So(err, ShouldBeNil)
+type mockGC struct {
+	topologyGC
 
-		nrts, err := fakeClient.TopologyV1alpha2().NodeResourceTopologies().List(context.TODO(), metav1.ListOptions{})
-		So(err, ShouldBeNil)
+	k8sClient k8sclientset.Interface
+}
 
-		So(nrts.Items, ShouldHaveLength, 2)
+func waitForNRT(cli topologyclientset.Interface, names ...string) bool {
+	nameSet := sets.NewString(names...)
+	for i := 0; i < 2; i++ {
+		nrts, err := cli.TopologyV1alpha2().NodeResourceTopologies().List(context.TODO(), metav1.ListOptions{})
+		So(err, ShouldBeNil)
 
-		nrt := v1alpha2.NodeResourceTopology{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "not-existing",
-			},
+		nrtNames := sets.NewString()
+		for _, nrt := range nrts.Items {
+			nrtNames.Insert(nrt.Name)
 		}
 
-		go gc.periodicGC(time.Second)
-
-		_, err = fakeClient.TopologyV1alpha2().NodeResourceTopologies().Create(context.TODO(), &nrt, metav1.CreateOptions{})
-		So(err, ShouldBeNil)
-		// simple sleep with retry loop to make sure GC was triggered
-		deleted := false
-		for i := 0; i < 5; i++ {
-			nrts, err := fakeClient.TopologyV1alpha2().NodeResourceTopologies().List(context.TODO(), metav1.ListOptions{})
-			So(err, ShouldBeNil)
-
-			if len(nrts.Items) == 2 {
-				deleted = true
-				break
-			}
-			time.Sleep(2 * time.Second)
+		if nrtNames.Equal(nameSet) {
+			return true
 		}
-		So(deleted, ShouldBeTrue)
-	})
-
+		time.Sleep(1 * time.Second)
+	}
+	return false
 }