fix: current code was not deleting 'sriovnetwork.openshift.io/use-external-drainer' annotation, it was only setting empty annotation value

Signed-off-by: Ido Heyvi <iheyvi@nvidia.com>

Author: heyvister1

pkg/daemon/daemon.go

@@ -424,12 +424,16 @@ func (dn *NodeReconciler) apply(ctx context.Context, desiredNodeState *sriovnetw
 		return ctrl.Result{}, err
 	}
 
-	// remove external drainer nodestate annotation
-	err = utils.AnnotateObject(ctx, desiredNodeState,
-		consts.NodeStateExternalDrainerAnnotation, "", dn.client)
-	if err != nil {
-		if !errors.IsNotFound(err) {
-			reqLogger.Error(err, "failed to remove node external drainer annotation")
+	// remove external drainer nodestate annotation if exists
+	annotations := desiredNodeState.GetAnnotations()
+	if _, ok := annotations[consts.NodeStateExternalDrainerAnnotation]; ok {
+		reqLogger.Info("remove external drainer nodestate annotation", "annotation", consts.NodeStateExternalDrainerAnnotation)
+		original := desiredNodeState.DeepCopy()
+		delete(annotations, consts.NodeStateExternalDrainerAnnotation)
+		desiredNodeState.SetAnnotations(annotations)
+		// Patch only the annotations
+		if err := dn.client.Patch(ctx, desiredNodeState, client.MergeFrom(original)); err != nil {
+			reqLogger.Error(err, "failed to patch nodestate after removing external drainer annotation")
 			return ctrl.Result{}, err
 		}
 	}
@@ -563,7 +567,7 @@ func (dn *NodeReconciler) handleDrain(ctx context.Context, desiredNodeState *sri
 		err := utils.AnnotateObject(ctx, desiredNodeState,
 			consts.NodeStateExternalDrainerAnnotation, "true", dn.client)
 		if err != nil {
-			funcLog.Error(err, "failed to add node external drainer annotation")
+			funcLog.Error(err, "failed to add nodestate external drainer annotation")
 			return false, err
 		}
 	}

pkg/daemon/daemon_test.go

@@ -347,12 +347,18 @@ var _ = Describe("Daemon Controller", Ordered, func() {
 			err := k8sClient.Update(ctx, nodeState)
 			Expect(err).ToNot(HaveOccurred())
 
+			nodeState = &sriovnetworkv1.SriovNetworkNodeState{ObjectMeta: metav1.ObjectMeta{Name: nodeName, Namespace: testNamespace}}
 			eventuallySyncStatusEqual(nodeState, constants.SyncStatusSucceeded)
 
 			By("Simulate node policy removal")
-			nodeState.Spec.Interfaces = []sriovnetworkv1.Interface{}
-			err = k8sClient.Update(ctx, nodeState)
-			Expect(err).ToNot(HaveOccurred())
+			EventuallyWithOffset(1, func(g Gomega) {
+				err := k8sClient.Get(ctx, types.NamespacedName{Namespace: nodeState.Namespace, Name: nodeState.Name}, nodeState)
+				g.Expect(err).ToNot(HaveOccurred())
+
+				nodeState.Spec.Interfaces = []sriovnetworkv1.Interface{}
+				err = k8sClient.Update(ctx, nodeState)
+				g.Expect(err).ToNot(HaveOccurred())
+			}, waitTime, retryTime).Should(Succeed())
 
 			eventuallySyncStatusEqual(nodeState, constants.SyncStatusSucceeded)
 			assertLastStatusTransitionsContains(nodeState, 2, constants.SyncStatusInProgress)
@@ -361,7 +367,6 @@ var _ = Describe("Daemon Controller", Ordered, func() {
 		It("Should apply external drainer annotation when useExternalDrainer is true", func(ctx context.Context) {
 			DeferCleanup(func(x bool) { vars.UseExternalDrainer = x }, vars.UseExternalDrainer)
 			vars.UseExternalDrainer = true
-
 			discoverSriovReturn.Store(&[]sriovnetworkv1.InterfaceExt{
 				{
 					Name:           "eno1",
@@ -375,16 +380,47 @@ var _ = Describe("Daemon Controller", Ordered, func() {
 					LinkType:       "ETH",
 					Mac:            "aa:bb:cc:dd:ee:ff",
 					Mtu:            1500,
-					TotalVfs:       2,
-					NumVfs:         0,
+					TotalVfs:       3,
+					NumVfs:         3,
+					VFs: []sriovnetworkv1.VirtualFunction{
+						{
+							Name:       "eno1f0",
+							PciAddress: "0000:16:00.1",
+							VfID:       0,
+						},
+						{
+							Name:       "eno1f1",
+							PciAddress: "0000:16:00.2",
+							VfID:       1,
+						},
+						{
+							Name:       "eno1f2",
+							PciAddress: "0000:16:00.3",
+							VfID:       2,
+						},
+					},
 				},
 			})
 
-			By("waiting for state to be in progress")
-			eventuallySyncStatusEqual(nodeState, constants.SyncStatusInProgress)
-
-			err := k8sClient.Get(ctx, types.NamespacedName{Namespace: nodeState.Namespace, Name: nodeState.Name}, nodeState)
-			Expect(err).ToNot(HaveOccurred())
+			EventuallyWithOffset(1, func(g Gomega) {
+				err := k8sClient.Get(ctx, types.NamespacedName{Namespace: nodeState.Namespace, Name: nodeState.Name}, nodeState)
+				g.Expect(err).ToNot(HaveOccurred())
+
+				nodeState.Spec.Interfaces = []sriovnetworkv1.Interface{
+					{Name: "eno1",
+						PciAddress: "0000:16:00.0",
+						LinkType:   "eth",
+						NumVfs:     3,
+						VfGroups: []sriovnetworkv1.VfGroup{
+							{ResourceName: "test",
+								DeviceType: "netdevice",
+								PolicyName: "test-policy",
+								VfRange:    "0-2"},
+						}},
+				}
+				err = k8sClient.Update(ctx, nodeState)
+				g.Expect(err).ToNot(HaveOccurred())
+			}, waitTime, retryTime).Should(Succeed())
 
 			By("waiting to require drain")
 			EventuallyWithOffset(1, func(g Gomega) {