NET-667: force delete daemon node (#2645)

* force delete zombie node * return correct resp * fix zombie hosts processing * add nil check rather checking error * pr comments
gravitl · Oct 31, 2023 · 7a9dc34 · 7a9dc34
1 parent b72fef8
commit 7a9dc34
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 40 deletions.
diff --git a/controllers/hosts.go b/controllers/hosts.go
@@ -321,6 +321,23 @@ func deleteHostFromNetwork(w http.ResponseWriter, r *http.Request) {
 
 	node, err := logic.UpdateHostNetwork(currHost, network, false)
 	if err != nil {
+		if node == nil && forceDelete {
+			// force cleanup the node
+			node, err := logic.GetNodeByHostRef(hostid, network)
+			if err != nil {
+				slog.Error("couldn't get node for host", "hostid", hostid, "network", network, "error", err)
+				logic.ReturnErrorResponse(w, r, logic.FormatError(err, "badrequest"))
+				return
+			}
+			if err = logic.DeleteNodeByID(&node); err != nil {
+				slog.Error("failed to force delete daemon node",
+					"nodeid", node.ID.String(), "hostid", hostid, "network", network, "error", err)
+				logic.ReturnErrorResponse(w, r, logic.FormatError(fmt.Errorf("failed to force delete daemon node: "+err.Error()), "internal"))
+				return
+			}
+			logic.ReturnSuccessResponse(w, r, "force deleted daemon node successfully")
+			return
+		}
 		logger.Log(0, r.Header.Get("user"), "failed to remove host from network:", hostid, network, err.Error())
 		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "internal"))
 		return

diff --git a/controllers/node.go b/controllers/node.go
@@ -725,34 +725,6 @@ func deleteNode(w http.ResponseWriter, r *http.Request) {
 	}
 	forceDelete := r.URL.Query().Get("force") == "true"
 	fromNode := r.Header.Get("requestfrom") == "node"
-	if node.IsRelayed {
-		// cleanup node from relayednodes on relay node
-		relayNode, err := logic.GetNodeByID(node.RelayedBy)
-		if err == nil {
-			relayedNodes := []string{}
-			for _, relayedNodeID := range relayNode.RelayedNodes {
-				if relayedNodeID == node.ID.String() {
-					continue
-				}
-				relayedNodes = append(relayedNodes, relayedNodeID)
-			}
-			relayNode.RelayedNodes = relayedNodes
-			logic.UpsertNode(&relayNode)
-		}
-	}
-	if node.IsRelay {
-		// unset all the relayed nodes
-		logic.SetRelayedNodes(false, node.ID.String(), node.RelayedNodes)
-	}
-	if node.IsIngressGateway {
-		// delete ext clients belonging to ingress gatewa
-		go func(node models.Node) {
-			if err = logic.DeleteGatewayExtClients(node.ID.String(), node.Network); err != nil {
-				slog.Error("failed to delete extclients", "gatewayid", node.ID.String(), "network", node.Network, "error", err.Error())
-			}
-		}(node)
-
-	}
 
 	purge := forceDelete || fromNode
 	if err := logic.DeleteNode(&node, purge); err != nil {
@@ -764,6 +736,8 @@ func deleteNode(w http.ResponseWriter, r *http.Request) {
 	logger.Log(1, r.Header.Get("user"), "Deleted node", nodeid, "from network", params["network"])
 	go func() { // notify of peer change
 		if !fromNode {
+			node.PendingDelete = true
+			node.Action = models.NODE_DELETE
 			if err := mq.NodeUpdate(&node); err != nil {
 				slog.Error("error publishing node update to node", "node", node.ID, "error", err)
 			}

diff --git a/logic/hosts.go b/logic/hosts.go
@@ -392,7 +392,7 @@ func DissasociateNodeFromHost(n *models.Node, h *models.Host) error {
 			}
 		}
 	}()
-	if err := deleteNodeByID(n); err != nil {
+	if err := DeleteNodeByID(n); err != nil {
 		return err
 	}
 

diff --git a/logic/nodes.go b/logic/nodes.go
@@ -183,6 +183,35 @@ func UpdateNode(currentNode *models.Node, newNode *models.Node) error {
 func DeleteNode(node *models.Node, purge bool) error {
 	alreadyDeleted := node.PendingDelete || node.Action == models.NODE_DELETE
 	node.Action = models.NODE_DELETE
+
+	if !alreadyDeleted {
+		//delete ext clients if node is ingress gw
+		if node.IsIngressGateway {
+			if err := DeleteGatewayExtClients(node.ID.String(), node.Network); err != nil {
+				slog.Error("failed to delete ext clients", "nodeid", node.ID.String(), "error", err.Error())
+			}
+		}
+		if node.IsRelayed {
+			// cleanup node from relayednodes on relay node
+			relayNode, err := GetNodeByID(node.RelayedBy)
+			if err == nil {
+				relayedNodes := []string{}
+				for _, relayedNodeID := range relayNode.RelayedNodes {
+					if relayedNodeID == node.ID.String() {
+						continue
+					}
+					relayedNodes = append(relayedNodes, relayedNodeID)
+				}
+				relayNode.RelayedNodes = relayedNodes
+				UpsertNode(&relayNode)
+			}
+		}
+		if node.IsRelay {
+			// unset all the relayed nodes
+			SetRelayedNodes(false, node.ID.String(), node.RelayedNodes)
+		}
+	}
+
 	if !purge && !alreadyDeleted {
 		newnode := *node
 		newnode.PendingDelete = true
@@ -198,7 +227,7 @@ func DeleteNode(node *models.Node, purge bool) error {
 	host, err := GetHost(node.HostID.String())
 	if err != nil {
 		logger.Log(1, "no host found for node", node.ID.String(), "deleting..")
-		if delErr := deleteNodeByID(node); delErr != nil {
+		if delErr := DeleteNodeByID(node); delErr != nil {
 			logger.Log(0, "failed to delete node", node.ID.String(), delErr.Error())
 		}
 		return err
@@ -215,16 +244,25 @@ func DeleteNode(node *models.Node, purge bool) error {
 	return nil
 }
 
-// deleteNodeByID - deletes a node from database
-func deleteNodeByID(node *models.Node) error {
-	var err error
-	var key = node.ID.String()
-	//delete any ext clients as required
-	if node.IsIngressGateway {
-		if err := DeleteGatewayExtClients(node.ID.String(), node.Network); err != nil {
-			logger.Log(0, "failed to deleted ext clients", err.Error())
+// GetNodeByHostRef - gets the node by host id and network
+func GetNodeByHostRef(hostid, network string) (node models.Node, err error) {
+	nodes, err := GetNetworkNodes(network)
+	if err != nil {
+		return models.Node{}, err
+	}
+	for _, node := range nodes {
+		if node.HostID.String() == hostid && node.Network == network {
+			return node, nil
 		}
 	}
+	return models.Node{}, errors.New("node not found")
+}
+
+// DeleteNodeByID - deletes a node from database
+func DeleteNodeByID(node *models.Node) error {
+	var err error
+	var key = node.ID.String()
+
 	if err = database.DeleteRecord(database.NODES_TABLE_NAME, key); err != nil {
 		if !database.IsEmptyRecord(err) {
 			return err

diff --git a/logic/zombie.go b/logic/zombie.go
@@ -107,6 +107,7 @@ func ManageZombies(ctx context.Context, peerUpdate chan *models.Node) {
 							logger.Log(1, "error deleting zombie node", zombies[i].String(), err.Error())
 							continue
 						}
+						node.PendingDelete = true
 						node.Action = models.NODE_DELETE
 						peerUpdate <- &node
 						logger.Log(1, "deleting zombie node", node.ID.String())
@@ -120,14 +121,17 @@ func ManageZombies(ctx context.Context, peerUpdate chan *models.Node) {
 					host, err := GetHost(hostZombies[i].String())
 					if err != nil {
 						logger.Log(1, "error retrieving zombie host", err.Error())
-						logger.Log(1, "deleting ", host.ID.String(), " from zombie list")
-						zombies = append(zombies[:i], zombies[i+1:]...)
+						if host != nil {
+							logger.Log(1, "deleting ", host.ID.String(), " from zombie list")
+						}
+						hostZombies = append(hostZombies[:i], hostZombies[i+1:]...)
 						continue
 					}
 					if len(host.Nodes) == 0 {
 						if err := RemoveHost(host, true); err != nil {
 							logger.Log(0, "error deleting zombie host", host.ID.String(), err.Error())
 						}
+						hostZombies = append(hostZombies[:i], hostZombies[i+1:]...)
 					}
 				}
 			}
-Original file line number
+Diff line change
@@ Expand Up @@
     			}
     		}
     	}()
-    	if err := deleteNodeByID(n); err != nil {
+    	if err := DeleteNodeByID(n); err != nil {
     		return err
     	}
@@ Expand Down @@