Test non latest NodeState generation are ingored

In case of two (or more) SriovNetworkNodeState objects land on the config daemon while it is already working on something, only the latest is considered. Removed the nodeStateSyncHandler's key parameter as it is used only for logging and log the right generation value. Add a specific unit test. Signed-off-by: Andrea Panattoni <[email protected]>
k8snetworkplumbingwg · Sep 5, 2022 · 8d79dee · 8d79dee
1 parent 7342e7f
commit 8d79dee
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 29 deletions.
diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go
@@ -314,11 +314,12 @@ func (dn *Daemon) enqueueNodeState(obj interface{}) {
 func (dn *Daemon) processNextWorkItem() bool {
 	glog.V(2).Infof("worker queue size: %d", dn.workqueue.Len())
 	obj, shutdown := dn.workqueue.Get()
-	glog.V(2).Infof("get item: %d", obj.(int64))
 	if shutdown {
 		return false
 	}
 
+	glog.V(2).Infof("get item: %d", obj.(int64))
+
 	// We wrap this block in a func so we can defer c.workqueue.Done.
 	err := func(obj interface{}) error {
 		// We call Done here so the workqueue knows we have finished
@@ -334,7 +335,7 @@ func (dn *Daemon) processNextWorkItem() bool {
 			return nil
 		}
 
-		err := dn.nodeStateSyncHandler(key)
+		err := dn.nodeStateSyncHandler()
 		if err != nil {
 			// Ereport error message, and put the item back to work queue for retry.
 			dn.refreshCh <- Message{
@@ -401,9 +402,8 @@ func (dn *Daemon) operatorConfigChangeHandler(old, new interface{}) {
 	}
 }
 
-func (dn *Daemon) nodeStateSyncHandler(generation int64) error {
+func (dn *Daemon) nodeStateSyncHandler() error {
 	var err error
-	glog.V(0).Infof("nodeStateSyncHandler(): new generation is %d", generation)
 	// Get the latest NodeState
 	var latestState *sriovnetworkv1.SriovNetworkNodeState
 	latestState, err = dn.client.SriovnetworkV1().SriovNetworkNodeStates(namespace).Get(context.Background(), dn.name, metav1.GetOptions{})
@@ -412,6 +412,8 @@ func (dn *Daemon) nodeStateSyncHandler(generation int64) error {
 		return err
 	}
 	latest := latestState.GetGeneration()
+	glog.V(0).Infof("nodeStateSyncHandler(): new generation is %d", latest)
+
 	if dn.nodeState.GetGeneration() == latest {
 		glog.V(0).Infof("nodeStateSyncHandler(): Interface not changed")
 		if latestState.Status.LastSyncError != "" ||

diff --git a/pkg/daemon/daemon_test.go b/pkg/daemon/daemon_test.go
@@ -16,12 +16,15 @@ import (
 
 	snclientset "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/client/clientset/versioned"
 	fakesnclientset "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/client/clientset/versioned/fake"
+	plugin "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/plugins"
+	"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/plugins/fake"
+	"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/plugins/generic"
 	"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils"
 )
 
 var FakeSupportedNicIDs corev1.ConfigMap = corev1.ConfigMap{
 	ObjectMeta: metav1.ObjectMeta{
-		Name:      sriovnetworkv1.SUPPORTED_NIC_ID_CONFIGMAP,
+		Name:      sriovnetworkv1.SupportedNicIDConfigmap,
 		Namespace: namespace,
 	},
 	Data: map[string]string{},
@@ -100,7 +103,11 @@ var _ = Describe("Config Daemon", func() {
 			utils.Baremetal,
 		)
 
-		sut.LoadedPlugins = map[string]VendorPlugin{GenericPlugin: &fakePlugin{}}
+		sut.enabledPlugins = map[string]plugin.VendorPlugin{generic.PluginName: &fake.FakePlugin{}}
+
+		go func() {
+			sut.Run(stopCh, exitCh)
+		}()
 	})
 
 	AfterEach(func() {
@@ -115,9 +122,6 @@ var _ = Describe("Config Daemon", func() {
 	Context("Should", func() {
 
 		It("restart sriov-device-plugin pod", func() {
-			go func() {
-				Expect(sut.Run(stopCh, exitCh)).To(BeNil())
-			}()
 
 			_, err := sut.kubeClient.CoreV1().Nodes().
 				Create(context.Background(), &corev1.Node{
@@ -174,6 +178,45 @@ var _ = Describe("Config Daemon", func() {
 			}, "10s").Should(BeZero())
 
 		})
+
+		It("ignore non latest SriovNetworkNodeState generations", func() {
+
+			_, err := sut.kubeClient.CoreV1().Nodes().Create(context.Background(), &corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-node",
+				},
+			}, metav1.CreateOptions{})
+			Expect(err).To(BeNil())
+
+			nodeState1 := &sriovnetworkv1.SriovNetworkNodeState{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "test-node",
+					Generation: 123,
+				},
+			}
+			Expect(
+				createSriovNetworkNodeState(sut.client, nodeState1)).
+				To(BeNil())
+
+			nodeState2 := &sriovnetworkv1.SriovNetworkNodeState{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "test-node",
+					Generation: 777,
+				},
+			}
+			Expect(
+				updateSriovNetworkNodeState(sut.client, nodeState2)).
+				To(BeNil())
+
+			var msg Message
+			Eventually(refreshCh, "10s").Should(Receive(&msg))
+			Expect(msg.syncStatus).To(Equal("InProgress"))
+
+			Eventually(refreshCh, "10s").Should(Receive(&msg))
+			Expect(msg.syncStatus).To(Equal("Succeeded"))
+
+			Expect(sut.nodeState.GetGeneration()).To(BeNumerically("==", 777))
+		})
 	})
 })
 
@@ -189,24 +232,9 @@ func createSriovNetworkNodeState(c snclientset.Interface, nodeState *sriovnetwor
 	return err
 }
 
-type fakePlugin struct{}
-
-func (f *fakePlugin) Name() string {
-	return "fake_plugin"
-}
-
-func (f *fakePlugin) Spec() string {
-	return "1.0"
-}
-
-func (f *fakePlugin) OnNodeStateAdd(state *sriovnetworkv1.SriovNetworkNodeState) (bool, bool, error) {
-	return false, false, nil
-}
-
-func (f *fakePlugin) OnNodeStateChange(old, new *sriovnetworkv1.SriovNetworkNodeState) (bool, bool, error) {
-	return false, false, nil
-}
-
-func (f *fakePlugin) Apply() error {
-	return nil
+func updateSriovNetworkNodeState(c snclientset.Interface, nodeState *sriovnetworkv1.SriovNetworkNodeState) error {
+	_, err := c.SriovnetworkV1().
+		SriovNetworkNodeStates(namespace).
+		Update(context.Background(), nodeState, metav1.UpdateOptions{})
+	return err
 }
diff --git a/pkg/plugins/fake/fake_plugin.go b/pkg/plugins/fake/fake_plugin.go
@@ -0,0 +1,24 @@
+package fake
+
+import (
+	sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
+)
+
+// This plugin is used in Daemon unit tests
+type FakePlugin struct{}
+
+func (f *FakePlugin) Name() string {
+	return "fake_plugin"
+}
+
+func (f *FakePlugin) Spec() string {
+	return "1.0"
+}
+
+func (f *FakePlugin) OnNodeStateChange(new *sriovnetworkv1.SriovNetworkNodeState) (bool, bool, error) {
+	return false, false, nil
+}
+
+func (f *FakePlugin) Apply() error {
+	return nil
+}