Skip to content

Commit

Permalink
Fix bug which should delete node from store when there is node not fo…
Browse files Browse the repository at this point in the history
…und error

When node is deleted, the phase node.ObjectMeta.DeletionTimestamp being not zero is intermittent,
and it is more possible the node is not found, we should delete the node from store.
  • Loading branch information
zhengxiexie committed Dec 31, 2024
1 parent c729a64 commit defd8df
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 4 deletions.
11 changes: 8 additions & 3 deletions pkg/controllers/node/node_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ 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
}
return common.ResultNormal, nil
} else {
log.Error(err, "unable to fetch node", "req", req.NamespacedName)
}
Expand All @@ -53,7 +58,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)
Expand All @@ -62,9 +67,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.
Expand Down
12 changes: 11 additions & 1 deletion pkg/controllers/node/node_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,28 @@ func TestNodeReconciler_Reconcile(t *testing.T) {
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" {
patchesSyncNodeStore := gomonkey.ApplyFunc((*node.NodeService).SyncNodeStore,
func(n *node.NodeService, nodeName string, deleted bool) error {
return nil
})
defer patchesSyncNodeStore.Reset()
}

result, err := reconciler.Reconcile(ctx, req)

if tt.expectedErr {
Expand Down

0 comments on commit defd8df

Please sign in to comment.