Merge pull request #980 from vmware-tanzu/topic/zhengxie/main/fix_node

Fix bug which should delete node from store when node is not found

pkg/controllers/node/node_controller.go

@@ -45,6 +45,10 @@ func (r *NodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 		if errors.IsNotFound(err) {
 			log.Info("node not found", "req", req.NamespacedName)
 			deleted = true
+			if err := r.Service.SyncNodeStore(req.NamespacedName.Name, deleted); err != nil {
+				log.Error(err, "failed to sync node store", "req", req.NamespacedName)
+				return common.ResultNormal, err
+			}
 		} else {
 			log.Error(err, "unable to fetch node", "req", req.NamespacedName)
 		}
@@ -53,7 +57,7 @@ func (r *NodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 	if common.NodeIsMaster(node) {
 		// For WCP supervisor cluster, the master node isn't a transport node.
 		log.Info("skipping handling master node", "node", req.NamespacedName)
-		return ctrl.Result{}, nil
+		return common.ResultNormal, nil
 	}
 	if !node.ObjectMeta.DeletionTimestamp.IsZero() {
 		log.Info("node is being deleted", "node", req.NamespacedName)
@@ -62,9 +66,9 @@ func (r *NodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 
 	if err := r.Service.SyncNodeStore(node.Name, deleted); err != nil {
 		log.Error(err, "failed to sync node store", "req", req.NamespacedName)
-		return ctrl.Result{}, err
+		return common.ResultNormal, err
 	}
-	return ctrl.Result{}, nil
+	return common.ResultNormal, nil
 }
 
 // SetupWithManager sets up the controller with the Manager.

pkg/controllers/node/node_controller_test.go

@@ -2,6 +2,7 @@ package node
 
 import (
 	"context"
+	"errors"
 	"testing"
 
 	"github.com/agiledragon/gomonkey/v2"
@@ -16,6 +17,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/event"
 
 	"github.com/vmware-tanzu/nsx-operator/pkg/config"
+	"github.com/vmware-tanzu/nsx-operator/pkg/controllers/common"
 	"github.com/vmware-tanzu/nsx-operator/pkg/metrics"
 	servicecommon "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common"
 	"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/node"
@@ -59,7 +61,7 @@ func TestNodeReconciler_Reconcile(t *testing.T) {
 		{
 			name:           "Node not found",
 			existingNode:   nil,
-			expectedResult: ctrl.Result{},
+			expectedResult: common.ResultNormal,
 			expectedErr:    false,
 		},
 		{
@@ -72,7 +74,7 @@ func TestNodeReconciler_Reconcile(t *testing.T) {
 					},
 				},
 			},
-			expectedResult: ctrl.Result{},
+			expectedResult: common.ResultNormal,
 			expectedErr:    false,
 		},
 		{
@@ -82,26 +84,50 @@ func TestNodeReconciler_Reconcile(t *testing.T) {
 					Name: "worker-node",
 				},
 			},
-			expectedResult: ctrl.Result{},
+			expectedResult: common.ResultNormal,
 			expectedErr:    false,
 		},
+		{
+			name:           "Sync node error",
+			existingNode:   nil,
+			expectedResult: common.ResultNormal,
+			expectedErr:    true,
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ctx := context.Background()
+			nodeName := "test-node"
 
 			if tt.existingNode != nil {
+				nodeName = tt.existingNode.Name
 				err := reconciler.Client.Create(ctx, tt.existingNode)
 				assert.NoError(t, err)
 			}
 
 			req := ctrl.Request{
 				NamespacedName: types.NamespacedName{
-					Name: "test-node",
+					Name: nodeName,
 				},
 			}
 
+			if tt.name != "Master Node" && tt.name != "Sync node error" {
+				patchesSyncNodeStore := gomonkey.ApplyFunc((*node.NodeService).SyncNodeStore,
+					func(n *node.NodeService, nodeName string, deleted bool) error {
+						return nil
+					})
+				defer patchesSyncNodeStore.Reset()
+			}
+
+			if tt.name == "Sync node error" {
+				patchesSyncNodeStore := gomonkey.ApplyFunc((*node.NodeService).SyncNodeStore,
+					func(n *node.NodeService, nodeName string, deleted bool) error {
+						return errors.New("Sync node error")
+					})
+				defer patchesSyncNodeStore.Reset()
+			}
+
 			result, err := reconciler.Reconcile(ctx, req)
 
 			if tt.expectedErr {