From eb12cb32e75a9c331e1a1a5702a91e9316a37e97 Mon Sep 17 00:00:00 2001 From: Adrian Chiris Date: Tue, 30 Aug 2022 14:16:18 +0300 Subject: [PATCH 1/2] Change Trackers Update method to return explicit true Current check for len has no real value. return true upon success. Signed-off-by: Adrian Chiris --- pkg/controllers/namespace.go | 2 +- pkg/controllers/net-attach-def.go | 2 +- pkg/controllers/networkpolicy.go | 2 +- pkg/controllers/pod.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/controllers/namespace.go b/pkg/controllers/namespace.go index e4919af..bf0dd6c 100644 --- a/pkg/controllers/namespace.go +++ b/pkg/controllers/namespace.go @@ -198,7 +198,7 @@ func (nct *NamespaceChangeTracker) Update(previous, current *v1.Namespace) bool if reflect.DeepEqual(change.previous, change.current) { delete(nct.items, ns.Name) } - return len(nct.items) >= 0 + return true } // NamespaceMap maps Namespace name to NamespaceInfo diff --git a/pkg/controllers/net-attach-def.go b/pkg/controllers/net-attach-def.go index 20e57e2..f89c4b0 100644 --- a/pkg/controllers/net-attach-def.go +++ b/pkg/controllers/net-attach-def.go @@ -296,7 +296,7 @@ func (ndt *NetDefChangeTracker) Update(previous, current *netdefv1.NetworkAttach delete(ndt.items, namespacedName) } - return len(ndt.items) >= 0 + return true } // NewNetDefChangeTracker creates a new instance of NetDefChangeTracker diff --git a/pkg/controllers/networkpolicy.go b/pkg/controllers/networkpolicy.go index 0bf9151..2d0395a 100644 --- a/pkg/controllers/networkpolicy.go +++ b/pkg/controllers/networkpolicy.go @@ -275,7 +275,7 @@ func (pct *PolicyChangeTracker) Update(previous, current *multiv1beta1.MultiNetw delete(pct.items, namespacedName) } - return len(pct.items) >= 0 + return true } // NewPolicyChangeTracker creates a new instance of PolicyChangeTracker diff --git a/pkg/controllers/pod.go b/pkg/controllers/pod.go index e3a7a32..f080720 100644 --- a/pkg/controllers/pod.go +++ b/pkg/controllers/pod.go @@ -369,7 +369,7 @@ func (pct *PodChangeTracker) Update(previous, current *v1.Pod) bool { if reflect.DeepEqual(change.previous, change.current) { delete(pct.items, namespacedName) } - return len(pct.items) >= 0 + return true } // PodMap maps Pod namespaced name to PodInfo From b5aa360768470a432bbb426a59c36707879dbc22 Mon Sep 17 00:00:00 2001 From: Adrian Chiris Date: Tue, 30 Aug 2022 14:23:45 +0300 Subject: [PATCH 2/2] rework controllers tests This commit reworks controllers unit tests - split to its own test package and trigger public methods only - modify test logic to spin up informers using fake clientset - add additional tests Signed-off-by: Adrian Chiris --- pkg/controllers/controller_suite_test.go | 4 +- pkg/controllers/namespace_test.go | 218 +++++++++------ pkg/controllers/net-attach-def_test.go | 214 +++++++++------ pkg/controllers/networkpolicy_test.go | 211 ++++++++------ pkg/controllers/pod_test.go | 332 ++++++++++++++--------- 5 files changed, 612 insertions(+), 367 deletions(-) diff --git a/pkg/controllers/controller_suite_test.go b/pkg/controllers/controller_suite_test.go index eb3fcbf..1ca6b28 100644 --- a/pkg/controllers/controller_suite_test.go +++ b/pkg/controllers/controller_suite_test.go @@ -1,4 +1,4 @@ -package controllers +package controllers_test import ( "flag" @@ -17,7 +17,7 @@ func TestControllers(t *testing.T) { var _ = BeforeSuite(func() { fs := flag.NewFlagSet("test-flag-set", flag.PanicOnError) klog.InitFlags(fs) - Expect(fs.Set("v", "8")).ToNot(HaveOccurred()) + Expect(fs.Set("v", "11")).ToNot(HaveOccurred()) }) var _ = AfterSuite(func() { diff --git a/pkg/controllers/namespace_test.go b/pkg/controllers/namespace_test.go index f8a183e..f19e40f 100644 --- a/pkg/controllers/namespace_test.go +++ b/pkg/controllers/namespace_test.go @@ -1,16 +1,20 @@ -package controllers +package controllers_test import ( + "context" + "sync" "time" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/cache" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + + "github.com/Mellanox/multi-networkpolicy-tc/pkg/controllers" ) type FakeNamespaceConfigStub struct { @@ -36,16 +40,7 @@ func (f *FakeNamespaceConfigStub) OnNamespaceSynced() { f.CounterSynced++ } -func NewFakeNamespaceConfig(stub *FakeNamespaceConfigStub) *NamespaceConfig { - configSync := 15 * time.Minute - fakeClient := fake.NewSimpleClientset() - informerFactory := informers.NewSharedInformerFactoryWithOptions(fakeClient, configSync) - nsConfig := NewNamespaceConfig(informerFactory.Core().V1().Namespaces(), configSync) - nsConfig.RegisterEventHandler(stub) - return nsConfig -} - -func NewNamespace(name string, labels map[string]string) *v1.Namespace { +func newTestNamespace(name string, labels map[string]string) *v1.Namespace { return &v1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -55,100 +50,155 @@ func NewNamespace(name string, labels map[string]string) *v1.Namespace { } var _ = Describe("namespace config", func() { + configSync := 15 * time.Minute + var wg sync.WaitGroup + var stopCtx context.Context + var stopFunc context.CancelFunc + var fakeClient *fake.Clientset + var informerFactory informers.SharedInformerFactory + var stub *FakeNamespaceConfigStub + var nsConfig *controllers.NamespaceConfig + + BeforeEach(func() { + wg = sync.WaitGroup{} + stopCtx, stopFunc = context.WithCancel(context.Background()) + fakeClient = fake.NewSimpleClientset() + informerFactory = informers.NewSharedInformerFactory(fakeClient, configSync) + nsInformer := informerFactory.Core().V1().Namespaces() + nsConfig = controllers.NewNamespaceConfig(nsInformer, configSync) + stub = &FakeNamespaceConfigStub{} + + nsConfig.RegisterEventHandler(stub) + informerFactory.Start(stopCtx.Done()) + + wg.Add(1) + go func() { + nsConfig.Run(stopCtx.Done()) + wg.Done() + }() + + cacheSyncCtx, cfn := context.WithTimeout(context.Background(), 1*time.Second) + defer cfn() + Expect(cache.WaitForCacheSync(cacheSyncCtx.Done(), nsInformer.Informer().HasSynced)).To(BeTrue()) + }) + + AfterEach(func() { + stopFunc() + wg.Wait() + }) + + It("check sync handler", func() { + Eventually(&stub.CounterSynced).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterAdd).Should(HaveValue(Equal(0))) + Eventually(&stub.CounterUpdate).Should(HaveValue(Equal(0))) + Eventually(&stub.CounterDelete).Should(HaveValue(Equal(0))) + }) + It("check add handler", func() { - stub := &FakeNamespaceConfigStub{} - nsConfig := NewFakeNamespaceConfig(stub) - nsConfig.handleAddNamespace(NewNamespace("test", nil)) - Expect(stub.CounterAdd).To(Equal(1)) - Expect(stub.CounterUpdate).To(Equal(0)) - Expect(stub.CounterDelete).To(Equal(0)) - Expect(stub.CounterSynced).To(Equal(0)) + _, err := fakeClient.CoreV1().Namespaces().Create( + context.Background(), newTestNamespace("test", nil), metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + Eventually(&stub.CounterAdd).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterUpdate).Should(HaveValue(Equal(0))) + Eventually(&stub.CounterDelete).Should(HaveValue(Equal(0))) + }) It("check update handler", func() { - stub := &FakeNamespaceConfigStub{} - nsConfig := NewFakeNamespaceConfig(stub) - nsConfig.handleUpdateNamespace(NewNamespace("test1", nil), NewNamespace("test2", nil)) - Expect(stub.CounterAdd).To(Equal(0)) - Expect(stub.CounterUpdate).To(Equal(1)) - Expect(stub.CounterDelete).To(Equal(0)) - Expect(stub.CounterSynced).To(Equal(0)) + ns, err := fakeClient.CoreV1().Namespaces().Create( + context.Background(), newTestNamespace("test", nil), metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + ns.Labels = map[string]string{"my": "label"} + _, err = fakeClient.CoreV1().Namespaces().Update( + context.Background(), ns, metav1.UpdateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + Eventually(&stub.CounterAdd).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterUpdate).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterDelete).Should(HaveValue(Equal(0))) }) It("check delete handler", func() { - stub := &FakeNamespaceConfigStub{} - nsConfig := NewFakeNamespaceConfig(stub) - nsConfig.handleDeleteNamespace(NewNamespace("test1", nil)) - Expect(stub.CounterAdd).To(Equal(0)) - Expect(stub.CounterUpdate).To(Equal(0)) - Expect(stub.CounterDelete).To(Equal(1)) - Expect(stub.CounterSynced).To(Equal(0)) + ns, err := fakeClient.CoreV1().Namespaces().Create( + context.Background(), newTestNamespace("test", nil), metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + err = fakeClient.CoreV1().Namespaces().Delete(context.Background(), ns.Name, metav1.DeleteOptions{}) + Expect(err).ToNot(HaveOccurred()) + + Eventually(&stub.CounterAdd).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterUpdate).Should(HaveValue(Equal(0))) + Eventually(&stub.CounterDelete).Should(HaveValue(Equal(1))) }) }) -var _ = Describe("namespace controller", func() { - It("Initialize and verify empty", func() { - nsChanges := NewNamespaceChangeTracker() - nsMap := make(NamespaceMap) +var _ = Describe("namespace change tracker", func() { + var nsMap controllers.NamespaceMap + var nsChanges *controllers.NamespaceChangeTracker + var ns1, ns2 *v1.Namespace + + checkNsMapWithNS := func(ns *v1.Namespace) { + nsTest, ok := nsMap[ns.Name] + ExpectWithOffset(1, ok).To(BeTrue()) + ExpectWithOffset(1, ok).To(BeTrue()) + ExpectWithOffset(1, nsTest.Name).To(Equal(ns.Name)) + ExpectWithOffset(1, nsTest.Labels).To(BeEquivalentTo(ns.Labels)) + } + + BeforeEach(func() { + nsChanges = controllers.NewNamespaceChangeTracker() + nsMap = make(controllers.NamespaceMap) + ns1 = newTestNamespace("test1", map[string]string{"labelName1": "labelValue1"}) + ns2 = newTestNamespace("test2", map[string]string{"labelName2": "labelValue2"}) + }) + + It("empty update", func() { nsMap.Update(nsChanges) - Expect(len(nsMap)).To(Equal(0)) + Expect(nsMap).To(BeEmpty()) }) - It("Add ns and verify", func() { - nsChanges := NewNamespaceChangeTracker() - Expect(nsChanges.Update(nil, NewNamespace("test1", map[string]string{"labelName1": "labelValue1"}))).To(BeTrue()) + It("invalid Update case both nil - NamespaceChangeTracker", func() { + Expect(nsChanges.Update(nil, nil)).To(BeFalse()) + }) + + It("invalid Update case - NamespaceMap", func() { + nsMap.Update(nil) + Expect(nsMap).To(BeEmpty()) + }) + + It("add ns and verify", func() { + Expect(nsChanges.Update(nil, ns1)).To(BeTrue()) - nsMap := make(NamespaceMap) nsMap.Update(nsChanges) - Expect(len(nsMap)).To(Equal(1)) - nsTest1, ok := nsMap["test1"] - Expect(ok).To(BeTrue()) - Expect(nsTest1.Name).To(Equal("test1")) - Expect(len(nsTest1.Labels)).To(Equal(1)) - - labelTest, ok := nsTest1.Labels["labelName1"] - Expect(ok).To(BeTrue()) - Expect(labelTest).To(Equal("labelValue1")) + Expect(nsMap).To(HaveLen(1)) + checkNsMapWithNS(ns1) }) - It("Add ns then del ns and verify", func() { - nsChanges := NewNamespaceChangeTracker() - Expect(nsChanges.Update(nil, NewNamespace("test1", map[string]string{"labelName1": "labelValue1"}))).To(BeTrue()) - Expect(nsChanges.Update(nil, NewNamespace("test2", map[string]string{"labelName2": "labelValue2"}))).To(BeTrue()) - Expect(nsChanges.Update(NewNamespace("test2", map[string]string{"labelName2": "labelValue2"}), nil)).To(BeTrue()) + It("add ns then del ns and verify", func() { + Expect(nsChanges.Update(nil, ns1)).To(BeTrue()) + Expect(nsChanges.Update(nil, ns2)).To(BeTrue()) + Expect(nsChanges.Update(ns2, nil)).To(BeTrue()) - nsMap := make(NamespaceMap) nsMap.Update(nsChanges) - Expect(len(nsMap)).To(Equal(1)) - nsTest1, ok := nsMap["test1"] - Expect(ok).To(BeTrue()) - Expect(nsTest1.Name).To(Equal("test1")) - Expect(len(nsTest1.Labels)).To(Equal(1)) - - labelTest, ok := nsTest1.Labels["labelName1"] - Expect(ok).To(BeTrue()) - Expect(labelTest).To(Equal("labelValue1")) + Expect(nsMap).To(HaveLen(1)) + checkNsMapWithNS(ns1) }) - It("invalid Update case", func() { - nsChanges := NewNamespaceChangeTracker() - Expect(nsChanges.Update(nil, nil)).To(BeFalse()) + It("add ns then update ns and verify", func() { + updatedNs1 := newTestNamespace("test1", map[string]string{"otherLabelName": "otherLabelValue"}) + Expect(nsChanges.Update(nil, ns1)).To(BeTrue()) + Expect(nsChanges.Update(ns1, updatedNs1)).To(BeTrue()) + + nsMap.Update(nsChanges) + Expect(nsMap).To(HaveLen(1)) + checkNsMapWithNS(updatedNs1) }) - It("Add ns then update ns and verify", func() { - nsChanges := NewNamespaceChangeTracker() - Expect(nsChanges.Update(nil, NewNamespace("test1", map[string]string{"labelName1": "labelValue1"}))).To(BeTrue()) - Expect(nsChanges.Update(nil, NewNamespace("test1", map[string]string{"labelName2": "labelValue2"}))).To(BeTrue()) - nsMap := make(NamespaceMap) + It("add same ns as current and previous", func() { + Expect(nsChanges.Update(ns1, ns1)).To(BeTrue()) nsMap.Update(nsChanges) - Expect(len(nsMap)).To(Equal(1)) - nsTest1, ok := nsMap["test1"] - Expect(ok).To(BeTrue()) - Expect(nsTest1.Name).To(Equal("test1")) - Expect(len(nsTest1.Labels)).To(Equal(1)) - - labelTest, ok := nsTest1.Labels["labelName2"] - Expect(ok).To(BeTrue()) - Expect(labelTest).To(Equal("labelValue2")) + Expect(nsMap).To(BeEmpty()) }) }) diff --git a/pkg/controllers/net-attach-def_test.go b/pkg/controllers/net-attach-def_test.go index 505b480..f0b109f 100644 --- a/pkg/controllers/net-attach-def_test.go +++ b/pkg/controllers/net-attach-def_test.go @@ -1,18 +1,23 @@ -package controllers +package controllers_test import ( + "context" "fmt" + "sync" "time" netdefv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" netdeffake "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/client/clientset/versioned/fake" netdefinformerv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/client/informers/externalversions" + "k8s.io/client-go/tools/cache" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" types "k8s.io/apimachinery/pkg/types" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + + "github.com/Mellanox/multi-networkpolicy-tc/pkg/controllers" ) type FakeNetDefConfigStub struct { @@ -38,15 +43,6 @@ func (f *FakeNetDefConfigStub) OnNetDefSynced() { f.CounterSynced++ } -func NewFakeNetDefConfig(stub *FakeNetDefConfigStub) *NetDefConfig { - configSync := 15 * time.Minute - fakeClient := netdeffake.NewSimpleClientset() - netdefInformarFactory := netdefinformerv1.NewSharedInformerFactoryWithOptions(fakeClient, configSync) - netdefConfig := NewNetDefConfig(netdefInformarFactory.K8sCniCncfIo().V1().NetworkAttachmentDefinitions(), configSync) - netdefConfig.RegisterEventHandler(stub) - return netdefConfig -} - func NewNetDef(namespace, name, cniConfig string) *netdefv1.NetworkAttachmentDefinition { return &netdefv1.NetworkAttachmentDefinition{ ObjectMeta: metav1.ObjectMeta{ @@ -81,98 +77,160 @@ func NewCNIConfigList(cniName, cniType string) string { } var _ = Describe("net-attach-def config", func() { + configSync := 15 * time.Minute + var wg sync.WaitGroup + var stopCtx context.Context + var stopFunc context.CancelFunc + var fakeClient *netdeffake.Clientset + var informerFactory netdefinformerv1.SharedInformerFactory + var stub *FakeNetDefConfigStub + var netDefConfig *controllers.NetDefConfig + var nd1 *netdefv1.NetworkAttachmentDefinition + + BeforeEach(func() { + wg = sync.WaitGroup{} + stopCtx, stopFunc = context.WithCancel(context.Background()) + fakeClient = netdeffake.NewSimpleClientset() + informerFactory = netdefinformerv1.NewSharedInformerFactory(fakeClient, configSync) + netDefInformer := informerFactory.K8sCniCncfIo().V1().NetworkAttachmentDefinitions() + netDefConfig = controllers.NewNetDefConfig(netDefInformer, configSync) + stub = &FakeNetDefConfigStub{} + nd1 = NewNetDef("testns1", "test1", NewCNIConfig("cniConfig1", "testType1")) + + netDefConfig.RegisterEventHandler(stub) + informerFactory.Start(stopCtx.Done()) + + wg.Add(1) + go func() { + netDefConfig.Run(stopCtx.Done()) + wg.Done() + }() + + cacheSyncCtx, cfn := context.WithTimeout(context.Background(), 1*time.Second) + defer cfn() + Expect(cache.WaitForCacheSync(cacheSyncCtx.Done(), netDefInformer.Informer().HasSynced)).To(BeTrue()) + }) + + AfterEach(func() { + stopFunc() + wg.Wait() + }) + + It("check sync handler", func() { + Eventually(&stub.CounterSynced).Should(HaveValue(Equal(1))) + }) + It("check add handler", func() { - stub := &FakeNetDefConfigStub{} - ndConfig := NewFakeNetDefConfig(stub) - ndConfig.handleAddNetDef(NewNetDef("testns1", "test1", NewCNIConfig("cniConfig1", "testType1"))) - Expect(stub.CounterAdd).To(Equal(1)) - Expect(stub.CounterUpdate).To(Equal(0)) - Expect(stub.CounterDelete).To(Equal(0)) - Expect(stub.CounterSynced).To(Equal(0)) + _, err := fakeClient.K8sCniCncfIoV1().NetworkAttachmentDefinitions(nd1.Namespace). + Create(context.Background(), nd1, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + Eventually(&stub.CounterAdd).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterUpdate).Should(HaveValue(Equal(0))) + Eventually(&stub.CounterDelete).Should(HaveValue(Equal(0))) }) It("check update handler", func() { - stub := &FakeNetDefConfigStub{} - ndConfig := NewFakeNetDefConfig(stub) - ndConfig.handleUpdateNetDef( - NewNetDef("testns1", "test1", NewCNIConfig("cniConfig1", "testType1")), - NewNetDef("testns1", "test1", NewCNIConfig("cniConfig2", "testType2"))) - Expect(stub.CounterAdd).To(Equal(0)) - Expect(stub.CounterUpdate).To(Equal(1)) - Expect(stub.CounterDelete).To(Equal(0)) - Expect(stub.CounterSynced).To(Equal(0)) + updatedNd, err := fakeClient.K8sCniCncfIoV1().NetworkAttachmentDefinitions(nd1.Namespace). + Create(context.Background(), nd1, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + updatedNd.Spec.Config = NewCNIConfig("cniConfig2", "testType2") + _, err = fakeClient.K8sCniCncfIoV1().NetworkAttachmentDefinitions(updatedNd.Namespace). + Update(context.Background(), updatedNd, metav1.UpdateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + Eventually(&stub.CounterAdd).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterUpdate).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterDelete).Should(HaveValue(Equal(0))) }) It("check delete handler", func() { - stub := &FakeNetDefConfigStub{} - ndConfig := NewFakeNetDefConfig(stub) - ndConfig.handleDeleteNetDef(NewNetDef("testns", "test", NewCNIConfig("cniConfig1", "testType1"))) - Expect(stub.CounterAdd).To(Equal(0)) - Expect(stub.CounterUpdate).To(Equal(0)) - Expect(stub.CounterDelete).To(Equal(1)) - Expect(stub.CounterSynced).To(Equal(0)) + _, err := fakeClient.K8sCniCncfIoV1().NetworkAttachmentDefinitions(nd1.Namespace). + Create(context.Background(), nd1, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + err = fakeClient.K8sCniCncfIoV1().NetworkAttachmentDefinitions(nd1.Namespace). + Delete(context.Background(), nd1.Name, metav1.DeleteOptions{}) + Expect(err).ToNot(HaveOccurred()) + + Eventually(&stub.CounterAdd).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterUpdate).Should(HaveValue(Equal(0))) + Eventually(&stub.CounterDelete).Should(HaveValue(Equal(1))) }) }) -var _ = Describe("net-attach-def controller", func() { - It("Initialize and verify empty", func() { - netDefChanges := NewNetDefChangeTracker() - ndMap := make(NetDefMap) - ndMap.Update(netDefChanges) - Expect(len(ndMap)).To(Equal(0)) +var _ = Describe("net-attach-def change tracker", func() { + var ndChanges *controllers.NetDefChangeTracker + var ndMap controllers.NetDefMap + var nd1, nd2 *netdefv1.NetworkAttachmentDefinition + + nsName := func(nd *netdefv1.NetworkAttachmentDefinition) types.NamespacedName { + return types.NamespacedName{Namespace: nd.Namespace, Name: nd.Name} + } + + checkNetDefMapWithNetDef := func(nd *netdefv1.NetworkAttachmentDefinition, expectedPluginType string) { + ndTest, ok := ndMap[nsName(nd)] + ExpectWithOffset(1, ok).To(BeTrue()) + ExpectWithOffset(1, ndTest.Name()).To(Equal(nd.Name)) + ExpectWithOffset(1, ndTest.PluginType).To(Equal(expectedPluginType)) + ExpectWithOffset(1, ndTest.Netdef).To(BeEquivalentTo(nd)) + } + + BeforeEach(func() { + ndMap = make(controllers.NetDefMap) + ndChanges = controllers.NewNetDefChangeTracker() + nd1 = NewNetDef("testns1", "test1", NewCNIConfig("cniConfig1", "testType1")) + nd2 = NewNetDef("testns2", "test2", NewCNIConfigList("cniConfig2", "testType2")) }) - It("Add netdef and verify", func() { - ndChanges := NewNetDefChangeTracker() - ndChanges.Update(nil, NewNetDef("testns1", "test1", NewCNIConfig("cniConfig1", "testType1"))) - ndChanges.Update(nil, NewNetDef("testns2", "test2", NewCNIConfigList("cniConfig2", "testType2"))) + It("invalid Update case both nil - NetDefChangeTracker", func() { + Expect(ndChanges.Update(nil, nil)).To(BeFalse()) + }) + + It("invalid Update case - NetDefMap", func() { + ndMap.Update(nil) + Expect(ndMap).To(BeEmpty()) + }) - ndMap := make(NetDefMap) + It("empty update - NetDefMap", func() { ndMap.Update(ndChanges) - Expect(len(ndMap)).To(Equal(2)) - ndTest1, ok := ndMap[types.NamespacedName{Namespace: "testns1", Name: "test1"}] - Expect(ok).To(BeTrue()) - Expect(ndTest1.Name()).To(Equal("test1")) - Expect(ndTest1.PluginType).To(Equal("testType1")) - - ndTest2, ok := ndMap[types.NamespacedName{Namespace: "testns2", Name: "test2"}] - Expect(ok).To(BeTrue()) - Expect(ndTest2.Name()).To(Equal("test2")) - Expect(ndTest2.PluginType).To(Equal("testType2")) + Expect(ndMap).To(BeEmpty()) }) - It("Add netdef then del it and verify", func() { - ndChanges := NewNetDefChangeTracker() - ndChanges.Update(nil, NewNetDef("testns1", "test1", NewCNIConfigList("cniConfig1", "testType1"))) - ndChanges.Update(nil, NewNetDef("testns1", "test2", NewCNIConfig("cniConfig2", "testType2"))) - ndChanges.Update(NewNetDef("testns1", "test2", NewCNIConfig("cniConfig2", "testType2")), nil) + It("Add netdef and verify", func() { + Expect(ndChanges.Update(nil, nd1)).To(BeTrue()) + Expect(ndChanges.Update(nil, nd2)).To(BeTrue()) - ndMap := make(NetDefMap) ndMap.Update(ndChanges) - Expect(len(ndMap)).To(Equal(1)) - ndTest1, ok := ndMap[types.NamespacedName{Namespace: "testns1", Name: "test1"}] - Expect(ok).To(BeTrue()) - Expect(ndTest1.Name()).To(Equal("test1")) - Expect(ndTest1.PluginType).To(Equal("testType1")) + Expect(ndMap).To(HaveLen(2)) + checkNetDefMapWithNetDef(nd1, "testType1") + checkNetDefMapWithNetDef(nd2, "testType2") }) - It("invalid Update case", func() { - ndChanges := NewNetDefChangeTracker() - Expect(ndChanges.Update(nil, nil)).To(BeFalse()) + It("Add netdef then del it and verify", func() { + Expect(ndChanges.Update(nil, nd1)).To(BeTrue()) + Expect(ndChanges.Update(nil, nd2)).To(BeTrue()) + Expect(ndChanges.Update(nd2, nil)).To(BeTrue()) + + ndMap.Update(ndChanges) + Expect(ndMap).To(HaveLen(1)) + checkNetDefMapWithNetDef(nd1, "testType1") }) It("Add netdef then update it and verify", func() { - ndChanges := NewNetDefChangeTracker() - ndChanges.Update(nil, NewNetDef("testns1", "test1", NewCNIConfig("cniConfig1", "testType1"))) - ndChanges.Update(NewNetDef("testns1", "test1", NewCNIConfig("cniConfig1", "testType1")), - NewNetDef("testns1", "test1", NewCNIConfigList("cniConfig2", "testType2"))) + Expect(ndChanges.Update(nil, nd1)).To(BeTrue()) + updatedNd1 := NewNetDef(nd1.Namespace, nd1.Name, NewCNIConfigList("cniConfig2", "testType2")) + Expect(ndChanges.Update(nd1, updatedNd1)).To(BeTrue()) + + ndMap.Update(ndChanges) + Expect(ndMap).To(HaveLen(1)) + checkNetDefMapWithNetDef(updatedNd1, "testType2") + }) - ndMap := make(NetDefMap) + It("add same netdef as current and previous", func() { + Expect(ndChanges.Update(nd1, nd1)).To(BeTrue()) ndMap.Update(ndChanges) - Expect(len(ndMap)).To(Equal(1)) - ndTest1, ok := ndMap[types.NamespacedName{Namespace: "testns1", Name: "test1"}] - Expect(ok).To(BeTrue()) - Expect(ndTest1.Name()).To(Equal("test1")) - Expect(ndTest1.PluginType).To(Equal("testType2")) + Expect(ndMap).To(BeEmpty()) }) }) diff --git a/pkg/controllers/networkpolicy_test.go b/pkg/controllers/networkpolicy_test.go index eb5dc5d..d6eb8f3 100644 --- a/pkg/controllers/networkpolicy_test.go +++ b/pkg/controllers/networkpolicy_test.go @@ -1,17 +1,22 @@ -package controllers +package controllers_test import ( + "context" + "sync" "time" multiv1beta1 "github.com/k8snetworkplumbingwg/multi-networkpolicy/pkg/apis/k8s.cni.cncf.io/v1beta1" multifake "github.com/k8snetworkplumbingwg/multi-networkpolicy/pkg/client/clientset/versioned/fake" multiinformerv1beta1 "github.com/k8snetworkplumbingwg/multi-networkpolicy/pkg/client/informers/externalversions" + "k8s.io/client-go/tools/cache" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + + "github.com/Mellanox/multi-networkpolicy-tc/pkg/controllers" ) type FakeNetworkPolicyConfigStub struct { @@ -37,15 +42,6 @@ func (f *FakeNetworkPolicyConfigStub) OnPolicySynced() { f.CounterSynced++ } -func NewFakeNetworkPolicyConfig(stub *FakeNetworkPolicyConfigStub) *NetworkPolicyConfig { - configSync := 15 * time.Minute - fakeClient := multifake.NewSimpleClientset() - informerFactory := multiinformerv1beta1.NewSharedInformerFactoryWithOptions(fakeClient, configSync) - policyConfig := NewNetworkPolicyConfig(informerFactory.K8sCniCncfIo().V1beta1().MultiNetworkPolicies(), configSync) - policyConfig.RegisterEventHandler(stub) - return policyConfig -} - func NewNetworkPolicy(namespace, name string) *multiv1beta1.MultiNetworkPolicy { return &multiv1beta1.MultiNetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ @@ -56,101 +52,158 @@ func NewNetworkPolicy(namespace, name string) *multiv1beta1.MultiNetworkPolicy { } var _ = Describe("networkpolicy config", func() { + configSync := 15 * time.Minute + var wg sync.WaitGroup + var stopCtx context.Context + var stopFunc context.CancelFunc + var fakeClient *multifake.Clientset + var informerFactory multiinformerv1beta1.SharedInformerFactory + var stub *FakeNetworkPolicyConfigStub + var netPolConfig *controllers.NetworkPolicyConfig + var mnp *multiv1beta1.MultiNetworkPolicy + + BeforeEach(func() { + wg = sync.WaitGroup{} + stopCtx, stopFunc = context.WithCancel(context.Background()) + fakeClient = multifake.NewSimpleClientset() + informerFactory = multiinformerv1beta1.NewSharedInformerFactory(fakeClient, configSync) + multiNetInformer := informerFactory.K8sCniCncfIo().V1beta1().MultiNetworkPolicies() + netPolConfig = controllers.NewNetworkPolicyConfig(multiNetInformer, configSync) + stub = &FakeNetworkPolicyConfigStub{} + mnp = NewNetworkPolicy("testns1", "test1") + + netPolConfig.RegisterEventHandler(stub) + informerFactory.Start(stopCtx.Done()) + + wg.Add(1) + go func() { + netPolConfig.Run(stopCtx.Done()) + wg.Done() + }() + + cacheSyncCtx, cfn := context.WithTimeout(context.Background(), 1*time.Second) + defer cfn() + Expect(cache.WaitForCacheSync(cacheSyncCtx.Done(), multiNetInformer.Informer().HasSynced)).To(BeTrue()) + }) + + AfterEach(func() { + stopFunc() + wg.Wait() + }) + + It("check sync handler", func() { + Eventually(&stub.CounterSynced).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterAdd).Should(HaveValue(Equal(0))) + Eventually(&stub.CounterUpdate).Should(HaveValue(Equal(0))) + Eventually(&stub.CounterDelete).Should(HaveValue(Equal(0))) + }) + It("check add handler", func() { - stub := &FakeNetworkPolicyConfigStub{} - networkPolicyConfig := NewFakeNetworkPolicyConfig(stub) - networkPolicyConfig.handleAddPolicy(NewNetworkPolicy("testns1", "test1")) - Expect(stub.CounterAdd).To(Equal(1)) - Expect(stub.CounterUpdate).To(Equal(0)) - Expect(stub.CounterDelete).To(Equal(0)) - Expect(stub.CounterSynced).To(Equal(0)) + _, err := fakeClient.K8sCniCncfIoV1beta1().MultiNetworkPolicies(mnp.Namespace).Create( + context.Background(), mnp, metav1.CreateOptions{}) + + Expect(err).ToNot(HaveOccurred()) + Eventually(&stub.CounterAdd).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterUpdate).Should(HaveValue(Equal(0))) + Eventually(&stub.CounterDelete).Should(HaveValue(Equal(0))) }) It("check update handler", func() { - stub := &FakeNetworkPolicyConfigStub{} - networkPolicyConfig := NewFakeNetworkPolicyConfig(stub) - networkPolicyConfig.handleUpdatePolicy( - NewNetworkPolicy("testns1", "test1"), - NewNetworkPolicy("testns2", "test1")) - Expect(stub.CounterAdd).To(Equal(0)) - Expect(stub.CounterUpdate).To(Equal(1)) - Expect(stub.CounterDelete).To(Equal(0)) - Expect(stub.CounterSynced).To(Equal(0)) + p, err := fakeClient.K8sCniCncfIoV1beta1().MultiNetworkPolicies(mnp.Namespace).Create( + context.Background(), mnp, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + p.Labels = map[string]string{"my": "label"} + _, err = fakeClient.K8sCniCncfIoV1beta1().MultiNetworkPolicies(mnp.Namespace).Update( + context.Background(), p, metav1.UpdateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + Eventually(&stub.CounterAdd).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterUpdate).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterDelete).Should(HaveValue(Equal(0))) }) It("check delete handler", func() { - stub := &FakeNetworkPolicyConfigStub{} - networkPolicyConfig := NewFakeNetworkPolicyConfig(stub) - networkPolicyConfig.handleDeletePolicy(NewNetworkPolicy("testns1", "test1")) - Expect(stub.CounterAdd).To(Equal(0)) - Expect(stub.CounterUpdate).To(Equal(0)) - Expect(stub.CounterDelete).To(Equal(1)) - Expect(stub.CounterSynced).To(Equal(0)) + p, err := fakeClient.K8sCniCncfIoV1beta1().MultiNetworkPolicies(mnp.Namespace).Create( + context.Background(), mnp, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + err = fakeClient.K8sCniCncfIoV1beta1().MultiNetworkPolicies(mnp.Namespace).Delete( + context.Background(), p.Name, metav1.DeleteOptions{}) + Expect(err).ToNot(HaveOccurred()) + + Eventually(&stub.CounterAdd).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterUpdate).Should(HaveValue(Equal(0))) + Eventually(&stub.CounterDelete).Should(HaveValue(Equal(1))) }) }) var _ = Describe("networkpolicy controller", func() { - It("Initialize and verify empty", func() { - policyChanges := NewPolicyChangeTracker() - policyMap := make(PolicyMap) + var policyChanges *controllers.PolicyChangeTracker + var policyMap controllers.PolicyMap + var policy1, policy2 *multiv1beta1.MultiNetworkPolicy + + BeforeEach(func() { + policyChanges = controllers.NewPolicyChangeTracker() + policyMap = make(controllers.PolicyMap) + policy1 = NewNetworkPolicy("testns1", "test1") + policy2 = NewNetworkPolicy("testns2", "test2") + }) + + nsName := func(np *multiv1beta1.MultiNetworkPolicy) types.NamespacedName { + return types.NamespacedName{Namespace: np.Namespace, Name: np.Name} + } + + checkPolicyMapWithPolicy := func(policy *multiv1beta1.MultiNetworkPolicy) { + policyTest, ok := policyMap[nsName(policy)] + ExpectWithOffset(1, ok).To(BeTrue()) + ExpectWithOffset(1, policyTest.Name()).To(Equal(policy.Name)) + ExpectWithOffset(1, policyTest.Namespace()).To(Equal(policy.Namespace)) + ExpectWithOffset(1, policyTest.Policy).To(BeEquivalentTo(policy)) + } + + It("invalid Update case both nil - NetDefChangeTracker", func() { + Expect(policyChanges.Update(nil, nil)).To(BeFalse()) + }) + + It("invalid Update case - NetDefMap", func() { + policyMap.Update(nil) + Expect(policyMap).To(BeEmpty()) + }) + + It("empty update - NetDefMap", func() { policyMap.Update(policyChanges) - Expect(len(policyMap)).To(Equal(0)) + Expect(policyMap).To(BeEmpty()) }) It("Add policy and verify", func() { - policyChanges := NewPolicyChangeTracker() - policyChanges.Update(nil, NewNetworkPolicy("testns1", "test1")) - policyChanges.Update(nil, NewNetworkPolicy("testns2", "test2")) + Expect(policyChanges.Update(nil, policy1)).To(BeTrue()) + Expect(policyChanges.Update(nil, policy2)).To(BeTrue()) - policyMap := make(PolicyMap) policyMap.Update(policyChanges) - Expect(len(policyMap)).To(Equal(2)) - - policyTest1, ok := policyMap[types.NamespacedName{Namespace: "testns1", Name: "test1"}] - Expect(ok).To(BeTrue()) - Expect(policyTest1.Name()).To(Equal("test1")) - Expect(policyTest1.Namespace()).To(Equal("testns1")) - policyTest2, ok := policyMap[types.NamespacedName{Namespace: "testns2", Name: "test2"}] - Expect(ok).To(BeTrue()) - Expect(policyTest2.Name()).To(Equal("test2")) - Expect(policyTest2.Namespace()).To(Equal("testns2")) + Expect(policyMap).To(HaveLen(2)) + checkPolicyMapWithPolicy(policy1) + checkPolicyMapWithPolicy(policy2) }) It("Add policy then delete it and verify", func() { - policyChanges := NewPolicyChangeTracker() - policyChanges.Update(nil, NewNetworkPolicy("testns1", "test1")) - policyChanges.Update(nil, NewNetworkPolicy("testns2", "test2")) - policyChanges.Update(NewNetworkPolicy("testns1", "test1"), nil) + Expect(policyChanges.Update(nil, policy1)).To(BeTrue()) + Expect(policyChanges.Update(nil, policy2)).To(BeTrue()) + Expect(policyChanges.Update(policy1, nil)).To(BeTrue()) - policyMap := make(PolicyMap) policyMap.Update(policyChanges) - Expect(len(policyMap)).To(Equal(1)) - - policyTest2, ok := policyMap[types.NamespacedName{Namespace: "testns2", Name: "test2"}] - Expect(ok).To(BeTrue()) - Expect(policyTest2.Name()).To(Equal("test2")) - Expect(policyTest2.Namespace()).To(Equal("testns2")) - }) - - It("invalid Update case", func() { - policyChanges := NewPolicyChangeTracker() - Expect(policyChanges.Update(nil, nil)).To(BeFalse()) + Expect(policyMap).To(HaveLen(1)) + checkPolicyMapWithPolicy(policy2) }) It("Add policy then update it and verify", func() { - policyChanges := NewPolicyChangeTracker() - policyChanges.Update(nil, NewNetworkPolicy("testns1", "test1")) - policyChanges.Update( - NewNetworkPolicy("testns1", "test1"), - NewNetworkPolicy("testns1", "test1")) + Expect(policyChanges.Update(nil, policy1)).To(BeTrue()) + updatedPolicy := NewNetworkPolicy("testns1", "test1") + updatedPolicy.Spec.PolicyTypes = []multiv1beta1.MultiPolicyType{multiv1beta1.PolicyTypeEgress} + Expect(policyChanges.Update(policy1, updatedPolicy)).To(BeTrue()) - policyMap := make(PolicyMap) policyMap.Update(policyChanges) - Expect(len(policyMap)).To(Equal(1)) - - policyTest1, ok := policyMap[types.NamespacedName{Namespace: "testns1", Name: "test1"}] - Expect(ok).To(BeTrue()) - Expect(policyTest1.Name()).To(Equal("test1")) - Expect(policyTest1.Namespace()).To(Equal("testns1")) + Expect(policyMap).To(HaveLen(1)) + checkPolicyMapWithPolicy(updatedPolicy) }) }) diff --git a/pkg/controllers/pod_test.go b/pkg/controllers/pod_test.go index ebe8d87..0521119 100644 --- a/pkg/controllers/pod_test.go +++ b/pkg/controllers/pod_test.go @@ -1,7 +1,9 @@ -package controllers +package controllers_test import ( + "context" "fmt" + "sync" "time" netdefv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" @@ -10,9 +12,12 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/cache" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + + "github.com/Mellanox/multi-networkpolicy-tc/pkg/controllers" ) type FakePodConfigStub struct { @@ -38,23 +43,14 @@ func (f *FakePodConfigStub) OnPodSynced() { f.CounterSynced++ } -func NewFakePodConfig(stub *FakePodConfigStub) *PodConfig { - configSync := 15 * time.Minute - fakeClient := fake.NewSimpleClientset() - informerFactory := informers.NewSharedInformerFactoryWithOptions(fakeClient, configSync) - podConfig := NewPodConfig(informerFactory.Core().V1().Pods(), configSync) - podConfig.RegisterEventHandler(stub) - return podConfig -} - -func NewFakePodWithNetAnnotation(namespace, name, annot, status string) *v1.Pod { +func NewFakePodWithNetAnnotation(namespace, name, networks, status string) *v1.Pod { return &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: namespace, Name: name, UID: "testUID", Annotations: map[string]string{ - "k8s.v1.cni.cncf.io/networks": annot, + "k8s.v1.cni.cncf.io/networks": networks, netdefv1.NetworkStatusAnnot: status, }, }, @@ -72,7 +68,8 @@ func NewFakePodWithNetAnnotation(namespace, name, annot, status string) *v1.Pod func NewFakeNetworkStatus(netns, netname string) string { baseStr := ` - [{ + [ + { "name": "", "interface": "eth0", "ips": [ @@ -96,7 +93,23 @@ func NewFakeNetworkStatus(netns, netname string) string { "pci-address": "0000:03:00.2" } } - }] + },{ + "name": "some-other-network", + "interface": "net2", + "ips": [ + "20.1.1.101" + ], + "mac": "42:90:65:12:3e:bf", + "dns": {}, + "device-info": { + "type": "pci", + "version": "1.0.0", + "pci": { + "pci-address": "0000:03:00.3" + } + } + } +] ` return fmt.Sprintf(baseStr, netns, netname) } @@ -119,139 +132,210 @@ func NewFakePod(namespace, name string) *v1.Pod { } } -func NewFakePodChangeTracker(ndt *NetDefChangeTracker) *PodChangeTracker { - return &PodChangeTracker{ - items: make(map[types.NamespacedName]*podChange), - netdefChanges: ndt, - networkPlugins: []string{"accelerated-bridge"}, - } -} - var _ = Describe("pod config", func() { - It("check add handler", func() { - stub := &FakePodConfigStub{} - nsConfig := NewFakePodConfig(stub) - nsConfig.handleAddPod(NewFakePod("testns1", "pod")) - Expect(stub.CounterAdd).To(Equal(1)) - Expect(stub.CounterUpdate).To(Equal(0)) - Expect(stub.CounterDelete).To(Equal(0)) - Expect(stub.CounterSynced).To(Equal(0)) - }) - - It("check update handler", func() { - stub := &FakePodConfigStub{} - nsConfig := NewFakePodConfig(stub) - nsConfig.handleUpdatePod(NewFakePod("testns1", "pod"), NewFakePod("testns2", "pod")) - Expect(stub.CounterAdd).To(Equal(0)) - Expect(stub.CounterUpdate).To(Equal(1)) - Expect(stub.CounterDelete).To(Equal(0)) - Expect(stub.CounterSynced).To(Equal(0)) + configSync := 15 * time.Minute + var wg sync.WaitGroup + var stopCtx context.Context + var stopFunc context.CancelFunc + var fakeClient *fake.Clientset + var informerFactory informers.SharedInformerFactory + var stub *FakePodConfigStub + var podConfig *controllers.PodConfig + var testPod1 *v1.Pod + + BeforeEach(func() { + wg = sync.WaitGroup{} + stopCtx, stopFunc = context.WithCancel(context.Background()) + fakeClient = fake.NewSimpleClientset() + informerFactory = informers.NewSharedInformerFactory(fakeClient, configSync) + podInformer := informerFactory.Core().V1().Pods() + podConfig = controllers.NewPodConfig(podInformer, configSync) + stub = &FakePodConfigStub{} + testPod1 = NewFakePod("testns1", "pod1") + + podConfig.RegisterEventHandler(stub) + informerFactory.Start(stopCtx.Done()) + + wg.Add(1) + go func() { + podConfig.Run(stopCtx.Done()) + wg.Done() + }() + + cacheSyncCtx, cfn := context.WithTimeout(context.Background(), 1*time.Second) + defer cfn() + Expect(cache.WaitForCacheSync(cacheSyncCtx.Done(), podInformer.Informer().HasSynced)).To(BeTrue()) }) - It("check update handler", func() { - stub := &FakePodConfigStub{} - nsConfig := NewFakePodConfig(stub) - nsConfig.handleDeletePod(NewFakePod("testns1", "pod")) - Expect(stub.CounterAdd).To(Equal(0)) - Expect(stub.CounterUpdate).To(Equal(0)) - Expect(stub.CounterDelete).To(Equal(1)) - Expect(stub.CounterSynced).To(Equal(0)) + AfterEach(func() { + stopFunc() + wg.Wait() }) -}) -var _ = Describe("pod controller", func() { - It("Initialize and verify empty", func() { - ndChanges := NewNetDefChangeTracker() - podChanges := NewFakePodChangeTracker(ndChanges) - podMap := make(PodMap) - podMap.Update(podChanges) - Expect(len(podMap)).To(Equal(0)) + It("check sync handler", func() { + Eventually(&stub.CounterSynced).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterAdd).Should(HaveValue(Equal(0))) + Eventually(&stub.CounterUpdate).Should(HaveValue(Equal(0))) + Eventually(&stub.CounterDelete).Should(HaveValue(Equal(0))) }) - It("Add pod and verify", func() { - ndChanges := NewNetDefChangeTracker() - podChanges := NewFakePodChangeTracker(ndChanges) - - Expect(podChanges.Update(nil, NewFakePod("testns1", "testpod1"))).To(BeTrue()) - - podMap := make(PodMap) - podMap.Update(podChanges) - Expect(len(podMap)).To(Equal(1)) + It("check add handler", func() { + _, err := fakeClient.CoreV1().Pods(testPod1.Namespace).Create( + context.Background(), testPod1, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) - pod1, ok := podMap[types.NamespacedName{Namespace: "testns1", Name: "testpod1"}] - Expect(ok).To(BeTrue()) - Expect(pod1.Name).To(Equal("testpod1")) - Expect(pod1.Namespace).To(Equal("testns1")) + Eventually(&stub.CounterAdd).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterUpdate).Should(HaveValue(Equal(0))) + Eventually(&stub.CounterDelete).Should(HaveValue(Equal(0))) }) - It("Add ns then del ns and verify", func() { - ndChanges := NewNetDefChangeTracker() - podChanges := NewFakePodChangeTracker(ndChanges) + It("check update handler", func() { + p, err := fakeClient.CoreV1().Pods(testPod1.Namespace).Create( + context.Background(), testPod1, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + p.Labels = map[string]string{"my": "label"} + _, err = fakeClient.CoreV1().Pods(p.Namespace).Update( + context.Background(), p, metav1.UpdateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + Eventually(&stub.CounterAdd).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterUpdate).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterDelete).Should(HaveValue(Equal(0))) + }) - Expect(podChanges.Update(nil, NewFakePod("testns1", "testpod1"))).To(BeTrue()) - Expect(podChanges.Update(NewFakePod("testns1", "testpod1"), nil)).To(BeTrue()) - Expect(podChanges.Update(nil, NewFakePod("testns2", "testpod2"))).To(BeTrue()) + It("check delete handler", func() { + p, err := fakeClient.CoreV1().Pods(testPod1.Namespace).Create( + context.Background(), testPod1, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) - podMap := make(PodMap) - podMap.Update(podChanges) - Expect(len(podMap)).To(Equal(1)) + err = fakeClient.CoreV1().Pods(p.Namespace).Delete( + context.Background(), p.Name, metav1.DeleteOptions{}) + Expect(err).ToNot(HaveOccurred()) - pod1, ok := podMap[types.NamespacedName{Namespace: "testns2", Name: "testpod2"}] - Expect(ok).To(BeTrue()) - Expect(pod1.Name).To(Equal("testpod2")) - Expect(pod1.Namespace).To(Equal("testns2")) - }) - - It("invalid Update case", func() { - ndChanges := NewNetDefChangeTracker() - podChanges := NewFakePodChangeTracker(ndChanges) - Expect(podChanges.Update(nil, nil)).To(BeFalse()) + Eventually(&stub.CounterAdd).Should(HaveValue(Equal(1))) + Eventually(&stub.CounterUpdate).Should(HaveValue(Equal(0))) + Eventually(&stub.CounterDelete).Should(HaveValue(Equal(1))) }) +}) - It("Add pod with net-attach annotation and status", func() { - ndChanges := NewNetDefChangeTracker() - podChanges := NewFakePodChangeTracker(ndChanges) +var _ = Describe("pod change tracker", func() { + var ndChanges *controllers.NetDefChangeTracker + var podChanges *controllers.PodChangeTracker + var podMap controllers.PodMap - Expect(ndChanges.Update(nil, NewNetDef("testns1", "net-attach1", NewCNIConfig("testCNI", "accelerated-bridge")))).To(BeTrue()) + nsName := func(p *v1.Pod) types.NamespacedName { + return types.NamespacedName{Namespace: p.Namespace, Name: p.Name} + } - Expect(podChanges.Update(nil, NewFakePodWithNetAnnotation("testns1", "testpod1", "net-attach1", NewFakeNetworkStatus("testns1", "net-attach1")))).To(BeTrue()) - podMap := make(PodMap) - podMap.Update(podChanges) - Expect(len(podMap)).To(Equal(1)) + checkPodInfo := func(p *v1.Pod, numOfInterfaces int) { + testPodInfo, ok := podMap[nsName(p)] + ExpectWithOffset(1, ok).To(BeTrue()) + ExpectWithOffset(1, testPodInfo.Name).To(Equal(p.Name)) + ExpectWithOffset(1, testPodInfo.Namespace).To(Equal(p.Namespace)) + ExpectWithOffset(1, testPodInfo.Interfaces).To(HaveLen(numOfInterfaces)) + } - pod1, ok := podMap[types.NamespacedName{Namespace: "testns1", Name: "testpod1"}] - Expect(ok).To(BeTrue()) - Expect(pod1.Name).To(Equal("testpod1")) - Expect(pod1.Namespace).To(Equal("testns1")) - Expect(len(pod1.Interfaces)).To(Equal(1)) + BeforeEach(func() { + ndChanges = controllers.NewNetDefChangeTracker() + podChanges = controllers.NewPodChangeTracker([]string{"accelerated-bridge"}, ndChanges) + podMap = make(controllers.PodMap) }) - It("Add pod with net-attach annotation no status", func() { - ndChanges := NewNetDefChangeTracker() - podChanges := NewFakePodChangeTracker(ndChanges) + Context("basic cases", func() { + It("invalid Update case both nil - NetDefChangeTracker", func() { + Expect(podChanges.Update(nil, nil)).To(BeFalse()) + }) - Expect(ndChanges.Update(nil, NewNetDef("testns1", "net-attach1", NewCNIConfig("testCNI", "accelerated-bridge")))).To(BeTrue()) - Expect(podChanges.Update(nil, NewFakePod("testns1", "testpod1"))).To(BeTrue()) - podMap := make(PodMap) - podMap.Update(podChanges) - Expect(len(podMap)).To(Equal(1)) + It("invalid Update case - NetDefMap", func() { + podMap.Update(nil) + Expect(podMap).To(BeEmpty()) + }) - pod1, ok := podMap[types.NamespacedName{Namespace: "testns1", Name: "testpod1"}] - Expect(ok).To(BeTrue()) - Expect(pod1.Name).To(Equal("testpod1")) - Expect(pod1.Namespace).To(Equal("testns1")) - Expect(len(pod1.Interfaces)).To(Equal(0)) - - Expect(podChanges.Update(NewFakePod("testns1", "testpod1"), NewFakePodWithNetAnnotation("testns1", "testpod1", "net-attach1", NewFakeNetworkStatus("testns1", "net-attach1")))).To(BeTrue()) - - podMap.Update(podChanges) - Expect(len(podMap)).To(Equal(1)) + It("empty update - NetDefMap", func() { + podMap.Update(podChanges) + Expect(podMap).To(BeEmpty()) + }) + }) - pod2, ok := podMap[types.NamespacedName{Namespace: "testns1", Name: "testpod1"}] - Expect(ok).To(BeTrue()) - Expect(pod2.Name).To(Equal("testpod1")) - Expect(pod2.Namespace).To(Equal("testns1")) - Expect(len(pod2.Interfaces)).To(Equal(1)) + Context("basic pods - no secondary network and status", func() { + var pod1, pod2 *v1.Pod + + BeforeEach(func() { + pod1 = NewFakePod("testns1", "testpod1") + pod2 = NewFakePod("testns2", "testpod2") + }) + + It("Add pod and verify", func() { + Expect(podChanges.Update(nil, pod1)).To(BeTrue()) + podMap.Update(podChanges) + Expect(podMap).To(HaveLen(1)) + checkPodInfo(pod1, 0) + }) + + It("Add ns then del ns and verify", func() { + Expect(podChanges.Update(nil, pod1)).To(BeTrue()) + Expect(podChanges.Update(nil, pod2)).To(BeTrue()) + Expect(podChanges.Update(pod1, nil)).To(BeTrue()) + + podMap.Update(podChanges) + Expect(podMap).To(HaveLen(1)) + checkPodInfo(pod2, 0) + }) + + It("Add ns then update ns and verify", func() { + podWithLables := NewFakePod("testns1", "testpod1") + podWithLables.Labels = map[string]string{"Some": "Label"} + + Expect(podChanges.Update(nil, pod1)).To(BeTrue()) + Expect(podChanges.Update(pod1, podWithLables)).To(BeTrue()) + + podMap.Update(podChanges) + Expect(podMap).To(HaveLen(1)) + checkPodInfo(podWithLables, 0) + }) }) + Context("pods with networks", func() { + BeforeEach(func() { + Expect(ndChanges.Update( + nil, NewNetDef("testns1", "net-attach1", NewCNIConfig( + "testCNI", "accelerated-bridge")))).To(BeTrue()) + + }) + + It("Add pod with net-attach annotation and status", func() { + podWithNeworkAndStatus := NewFakePodWithNetAnnotation("testns1", "testpod1", + "net-attach1", NewFakeNetworkStatus("testns1", "net-attach1")) + Expect(podChanges.Update(nil, podWithNeworkAndStatus)).To(BeTrue()) + podMap.Update(podChanges) + Expect(podMap).To(HaveLen(1)) + + checkPodInfo(podWithNeworkAndStatus, 1) + + // Check interface + pInfo := podMap[nsName(podWithNeworkAndStatus)] + Expect(pInfo.Interfaces[0].DeviceID).To(Equal("0000:03:00.2")) + Expect(pInfo.Interfaces[0].InterfaceType).To(Equal("accelerated-bridge")) + Expect(pInfo.Interfaces[0].InterfaceName).To(Equal("net1")) + Expect(pInfo.Interfaces[0].IPs).To(BeEquivalentTo([]string{"10.1.1.101"})) + Expect(pInfo.Interfaces[0].NetattachName).To(Equal("testns1/net-attach1")) + }) + + It("Add pod with net-attach annotation no status", func() { + podWitoutNeworkStatus := NewFakePodWithNetAnnotation("testns1", "testpod1", "net-attach1", "") + Expect(podChanges.Update(nil, podWitoutNeworkStatus)).To(BeTrue()) + podMap.Update(podChanges) + Expect(podMap).To(HaveLen(1)) + checkPodInfo(podWitoutNeworkStatus, 0) + + podWithNeworkAndStatus := NewFakePodWithNetAnnotation("testns1", "testpod1", + "net-attach1", NewFakeNetworkStatus("testns1", "net-attach1")) + Expect(podChanges.Update(podWitoutNeworkStatus, podWithNeworkAndStatus)).To(BeTrue()) + + podMap.Update(podChanges) + Expect(podMap).To(HaveLen(1)) + checkPodInfo(podWithNeworkAndStatus, 1) + }) + }) })