From 0b5e51bd354bbee1fd5e62f338f0247bf865c2da Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Thu, 17 Aug 2023 17:58:16 +0300 Subject: [PATCH] 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). --- pkg/nfd-topology-gc/nfd-nrt-gc_test.go | 248 +++++++++---------------- 1 file changed, 83 insertions(+), 165 deletions(-) diff --git a/pkg/nfd-topology-gc/nfd-nrt-gc_test.go b/pkg/nfd-topology-gc/nfd-nrt-gc_test.go index cc8eef0e53..746be8694f 100644 --- a/pkg/nfd-topology-gc/nfd-nrt-gc_test.go +++ b/pkg/nfd-topology-gc/nfd-nrt-gc_test.go @@ -22,10 +22,14 @@ 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" @@ -33,201 +37,115 @@ import ( 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.startNodeInformer() - 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.startNodeInformer() - 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.startNodeInformer() - 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.startNodeInformer() - 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 }