Revert "Merge pull request #3983 from pierresouchay/node_renaming"

This reverts commit 98ead2a, reversing changes made to f8dcff8.
hashicorp · Jul 12, 2018 · 776b4ce · 776b4ce
1 parent 81b35e3
commit 776b4ce
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 154 deletions.
diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go
@@ -350,21 +350,6 @@ func (s *Store) EnsureNode(idx uint64, node *structs.Node) error {
 	return nil
 }
 
-func (s *Store) ensureNoNodeWithSimilarNameTxn(tx *memdb.Txn, node *structs.Node) error {
-	// Retrieve all of the nodes
-	enodes, err := tx.Get("nodes", "id")
-	if err != nil {
-		return fmt.Errorf("Cannot lookup all nodes: %s", err)
-	}
-	for nodeIt := enodes.Next(); nodeIt != nil; nodeIt = enodes.Next() {
-		enode := nodeIt.(*structs.Node)
-		if strings.EqualFold(node.Node, enode.Node) && node.ID != enode.ID {
-			return fmt.Errorf("Node name %s is reserved by node %s with name %s", node.Node, enode.ID, enode.Node)
-		}
-	}
-	return nil
-}
-
 // ensureNodeTxn is the inner function called to actually create a node
 // registration or modify an existing one in the state store. It allows
 // passing in a memdb transaction so it may be part of a larger txn.
@@ -380,23 +365,8 @@ func (s *Store) ensureNodeTxn(tx *memdb.Txn, idx uint64, node *structs.Node) err
 		if existing != nil {
 			n = existing.(*structs.Node)
 			if n.Node != node.Node {
-				// Lets first get all nodes and check whether name do match
-				dupNameError := s.ensureNoNodeWithSimilarNameTxn(tx, node)
-				if dupNameError != nil {
-					return fmt.Errorf("Error while renaming Node ID: %q: %s", node.ID, dupNameError)
-				}
-				// We are actually renaming a node, remove its reference first
-				err := s.deleteNodeTxn(tx, idx, n.Node)
-				if err != nil {
-					return fmt.Errorf("Error while renaming Node ID: %q from %s to %s",
-						node.ID, n.Node, node.Node)
-				}
-			}
-
-		} else {
-			dupNameError := s.ensureNoNodeWithSimilarNameTxn(tx, node)
-			if dupNameError != nil {
-				return fmt.Errorf("Error while adding Node ID: %q: %s", node.ID, dupNameError)
+				return fmt.Errorf("node ID %q for node %q aliases existing node %q",
+					node.ID, node.Node, n.Node)
 			}
 		}
 	}

diff --git a/agent/consul/state/catalog_test.go b/agent/consul/state/catalog_test.go
@@ -480,131 +480,15 @@ func TestStateStore_EnsureNode(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	// Renaming a node should work if ID is the same
+	// Now try to add another node with the same ID
 	in = &structs.Node{
-		Node:    "node1-renamed",
+		Node:    "nope",
 		ID:      types.NodeID("cda916bc-a357-4a19-b886-59419fcee50c"),
-		Address: "1.1.1.2",
-	}
-	if err := s.EnsureNode(6, in); err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	// Retrieve the node
-	idx, out, err = s.GetNode("node1")
-	if out != nil {
-		t.Fatalf("Node should not exist anymore: %q", out)
-	}
-
-	idx, out, err = s.GetNode("node1-renamed")
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	// Node and indexes were updated
-	if out.CreateIndex != 1 || out.ModifyIndex != 6 || out.Address != "1.1.1.2" || out.Node != "node1-renamed" {
-		t.Fatalf("bad: %#v", out)
-	}
-	if idx != 6 {
-		t.Fatalf("bad index: %d", idx)
-	}
-
-	newNodeID := types.NodeID("d0347693-65cc-4d9f-a6e0-5025b2e6513f")
-
-	// Adding another node with same name should fail
-	in = &structs.Node{
-		Node:    "node1-renamed",
-		ID:      newNodeID,
-		Address: "1.1.1.7",
-	}
-	if err := s.EnsureNode(8, in); err == nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	// Adding another node with same name but different case should fail
-	in = &structs.Node{
-		Node:    "Node1-RENAMED",
-		ID:      newNodeID,
-		Address: "1.1.1.7",
-	}
-	if err := s.EnsureNode(8, in); err == nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	// Lets add another valid node now
-	in = &structs.Node{
-		Node:    "Node1bis",
-		ID:      newNodeID,
-		Address: "1.1.1.7",
-	}
-	if err := s.EnsureNode(9, in); err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	// Retrieve the node
-	idx, out, err = s.GetNode("Node1bis")
-	if out == nil {
-		t.Fatalf("Node should exist, but was null")
-	}
-
-	// Renaming should fail
-	in = &structs.Node{
-		Node:    "Node1bis",
-		ID:      newNodeID,
-		Address: "1.1.1.7",
-	}
-	if err := s.EnsureNode(9, in); err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	idx, out, err = s.GetNode("Node1bis")
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	// Node and indexes were updated
-	if out.ID != newNodeID || out.CreateIndex != 9 || out.ModifyIndex != 9 || out.Address != "1.1.1.7" || out.Node != "Node1bis" {
-		t.Fatalf("bad: %#v", out)
-	}
-	if idx != 9 {
-		t.Fatalf("bad index: %d", idx)
-	}
-
-	// Renaming to same value as first node should fail as well
-	// Adding another node with same name but different case should fail
-	in = &structs.Node{
-		Node:    "node1-renamed",
-		ID:      newNodeID,
-		Address: "1.1.1.7",
-	}
-	if err := s.EnsureNode(10, in); err == nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	// It should fail also with different case
-	in = &structs.Node{
-		Node:    "Node1-Renamed",
-		ID:      newNodeID,
-		Address: "1.1.1.7",
-	}
-	if err := s.EnsureNode(10, in); err == nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	// But should work if names are different
-	in = &structs.Node{
-		Node:    "Node1-Renamed2",
-		ID:      newNodeID,
-		Address: "1.1.1.7",
-	}
-	if err := s.EnsureNode(11, in); err != nil {
-		t.Fatalf("err: %s", err)
+		Address: "1.2.3.4",
 	}
-
-	// Node renamed should be Ok
-	idx, out, err = s.GetNode("Node1-Renamed2")
-	if err != nil {
-		t.Fatalf("err: %s", err)
+	err = s.EnsureNode(5, in)
+	if err == nil || !strings.Contains(err.Error(), "aliases existing node") {
+		t.Fatalf("err: %v", err)
 	}
 }