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 · Aug 4, 2022 · e112776 · e112776
1 parent 9d6ba1b
commit e112776
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 6 deletions.
diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go
@@ -313,11 +313,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
@@ -333,7 +334,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{
@@ -400,9 +401,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{})
@@ -411,6 +411,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
@@ -14,6 +14,8 @@ 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/generic"
 	"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils"
 
 	fakemcclientset "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned/fake"
@@ -23,7 +25,7 @@ import (
 
 var FakeSupportedNicIDs corev1.ConfigMap = corev1.ConfigMap{
 	ObjectMeta: metav1.ObjectMeta{
-		Name:      sriovnetworkv1.SUPPORTED_NIC_ID_CONFIGMAP,
+		Name:      sriovnetworkv1.SupportedNicIDConfigmap,
 		Namespace: namespace,
 	},
 	Data: map[string]string{},
@@ -102,7 +104,7 @@ var _ = Describe("Config Daemon", func() {
 			utils.Baremetal,
 		)
 
-		sut.LoadedPlugins = map[string]VendorPlugin{GenericPlugin: &fakePlugin{}}
+		sut.enabledPlugins = map[string]plugin.VendorPlugin{generic.PluginName: &fakePlugin{}}
 	})
 
 	AfterEach(func() {
@@ -176,6 +178,48 @@ var _ = Describe("Config Daemon", func() {
 			}, "10s").Should(BeZero())
 
 		})
+
+		It("ignore non latest SriovNetworkNodeState generations", func() {
+			go func() {
+				Expect(sut.Run(stopCh, exitCh)).To(BeNil())
+			}()
+
+			_, 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))
+		})
 	})
 })
 
@@ -191,6 +235,13 @@ func createSriovNetworkNodeState(c snclientset.Interface, nodeState *sriovnetwor
 	return err
 }
 
+func updateSriovNetworkNodeState(c snclientset.Interface, nodeState *sriovnetworkv1.SriovNetworkNodeState) error {
+	_, err := c.SriovnetworkV1().
+		SriovNetworkNodeStates(namespace).
+		Update(context.Background(), nodeState, metav1.UpdateOptions{})
+	return err
+}
+
 type fakePlugin struct{}
 
 func (f *fakePlugin) Name() string {