Update state store test for changing node ID

This commit is contained in:
Kyle Havlovitz 2019-03-13 16:23:05 -07:00
parent 106fde20b3
commit 3aec844fd2
3 changed files with 87 additions and 34 deletions

View file

@ -1332,10 +1332,6 @@ AFTER_CHECK:
Status: api.HealthPassing, Status: api.HealthPassing,
Output: structs.SerfCheckAliveOutput, Output: structs.SerfCheckAliveOutput,
}, },
// If there's existing information about the node, do not
// clobber it.
//SkipNodeUpdate: true,
} }
if node != nil { if node != nil {
req.TaggedAddresses = node.TaggedAddresses req.TaggedAddresses = node.TaggedAddresses

View file

@ -375,16 +375,16 @@ func (s *Store) ensureNoNodeWithSimilarNameTxn(tx *memdb.Txn, node *structs.Node
if err != nil { if err != nil {
return fmt.Errorf("Cannot get status of node %s: %s", enode.Node, err) return fmt.Errorf("Cannot get status of node %s: %s", enode.Node, err)
} }
if enodeCheck == nil {
return fmt.Errorf("Cannot rename node %s: Serf health check not found for existing node", enode.Node) var nodeHealthy bool
if enodeCheck != nil {
enodeSerfCheck, ok := enodeCheck.(*structs.HealthCheck)
if ok {
nodeHealthy = enodeSerfCheck.Status != api.HealthCritical
}
} }
enodeSerfCheck, ok := enodeCheck.(*structs.HealthCheck) if !(enode.ID == "" && allowClashWithoutID) && nodeHealthy {
if !ok {
return fmt.Errorf("Existing node %q's Serf health check has type %T", enode.Node, enodeSerfCheck)
}
if !((enode.ID == "" || enodeSerfCheck.Status == api.HealthCritical) && allowClashWithoutID) {
return fmt.Errorf("Node name %s is reserved by node %s with name %s", node.Node, enode.ID, enode.Node) return fmt.Errorf("Node name %s is reserved by node %s with name %s", node.Node, enode.ID, enode.Node)
} }
} }

View file

@ -84,6 +84,11 @@ func TestStateStore_ensureNoNodeWithSimilarNameTxn(t *testing.T) {
Address: "1.2.3.4", Address: "1.2.3.4",
TaggedAddresses: map[string]string{"hello": "world"}, TaggedAddresses: map[string]string{"hello": "world"},
NodeMeta: map[string]string{"somekey": "somevalue"}, NodeMeta: map[string]string{"somekey": "somevalue"},
Check: &structs.HealthCheck{
Node: "node1",
CheckID: structs.SerfCheckID,
Status: api.HealthPassing,
},
} }
if err := s.EnsureRegistration(1, req); err != nil { if err := s.EnsureRegistration(1, req); err != nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
@ -92,6 +97,11 @@ func TestStateStore_ensureNoNodeWithSimilarNameTxn(t *testing.T) {
ID: types.NodeID(""), ID: types.NodeID(""),
Node: "node2", Node: "node2",
Address: "10.0.0.1", Address: "10.0.0.1",
Check: &structs.HealthCheck{
Node: "node2",
CheckID: structs.SerfCheckID,
Status: api.HealthPassing,
},
} }
if err := s.EnsureRegistration(2, req); err != nil { if err := s.EnsureRegistration(2, req); err != nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
@ -119,6 +129,23 @@ func TestStateStore_ensureNoNodeWithSimilarNameTxn(t *testing.T) {
t.Fatalf("Should not clash with another similar node name without ID, err:=%q", err) t.Fatalf("Should not clash with another similar node name without ID, err:=%q", err)
} }
// Set node1's Serf health to failing and replace it.
newNode := &structs.Node{
ID: makeRandomNodeID(t),
Node: "node1",
Address: "2.3.4.5",
}
if err := s.ensureNoNodeWithSimilarNameTxn(tx, newNode, false); err == nil {
t.Fatalf("Should return an error since the previous node is still healthy")
}
s.ensureCheckTxn(tx, 5, &structs.HealthCheck{
Node: "node1",
CheckID: structs.SerfCheckID,
Status: api.HealthCritical,
})
if err := s.ensureNoNodeWithSimilarNameTxn(tx, newNode, false); err != nil {
t.Fatal(err)
}
} }
func TestStateStore_EnsureRegistration(t *testing.T) { func TestStateStore_EnsureRegistration(t *testing.T) {
@ -599,6 +626,13 @@ func TestNodeRenamingNodes(t *testing.T) {
if err := s.EnsureNode(1, in1); err != nil { if err := s.EnsureNode(1, in1); err != nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }
if err := s.EnsureCheck(2, &structs.HealthCheck{
Node: "node1",
CheckID: structs.SerfCheckID,
Status: api.HealthPassing,
}); err != nil {
t.Fatalf("err: %s", err)
}
// Node2 with ID // Node2 with ID
in2 := &structs.Node{ in2 := &structs.Node{
@ -607,7 +641,14 @@ func TestNodeRenamingNodes(t *testing.T) {
Address: "1.1.1.2", Address: "1.1.1.2",
} }
if err := s.EnsureNode(2, in2); err != nil { if err := s.EnsureNode(3, in2); err != nil {
t.Fatalf("err: %s", err)
}
if err := s.EnsureCheck(4, &structs.HealthCheck{
Node: "node2",
CheckID: structs.SerfCheckID,
Status: api.HealthPassing,
}); err != nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }
@ -617,7 +658,14 @@ func TestNodeRenamingNodes(t *testing.T) {
Address: "1.1.1.3", Address: "1.1.1.3",
} }
if err := s.EnsureNode(3, in3); err != nil { if err := s.EnsureNode(5, in3); err != nil {
t.Fatalf("err: %s", err)
}
if err := s.EnsureCheck(6, &structs.HealthCheck{
Node: "node3",
CheckID: structs.SerfCheckID,
Status: api.HealthPassing,
}); err != nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }
@ -635,7 +683,7 @@ func TestNodeRenamingNodes(t *testing.T) {
Node: "node1", Node: "node1",
Address: "1.1.1.2", Address: "1.1.1.2",
} }
if err := s.EnsureNode(4, in2Modify); err == nil { if err := s.EnsureNode(7, in2Modify); err == nil {
t.Fatalf("Renaming node2 into node1 should fail") t.Fatalf("Renaming node2 into node1 should fail")
} }
@ -645,7 +693,7 @@ func TestNodeRenamingNodes(t *testing.T) {
Node: "NoDe1", Node: "NoDe1",
Address: "1.1.1.2", Address: "1.1.1.2",
} }
if err := s.EnsureNode(5, in2Modify); err == nil { if err := s.EnsureNode(8, in2Modify); err == nil {
t.Fatalf("Renaming node2 into node1 should fail") t.Fatalf("Renaming node2 into node1 should fail")
} }
@ -655,7 +703,7 @@ func TestNodeRenamingNodes(t *testing.T) {
Node: "NoDe3", Node: "NoDe3",
Address: "1.1.1.2", Address: "1.1.1.2",
} }
if err := s.EnsureNode(6, in2Modify); err == nil { if err := s.EnsureNode(9, in2Modify); err == nil {
t.Fatalf("Renaming node2 into node1 should fail") t.Fatalf("Renaming node2 into node1 should fail")
} }
@ -665,7 +713,7 @@ func TestNodeRenamingNodes(t *testing.T) {
Node: "node2bis", Node: "node2bis",
Address: "1.1.1.2", Address: "1.1.1.2",
} }
if err := s.EnsureNode(6, in2Modify); err != nil { if err := s.EnsureNode(10, in2Modify); err != nil {
t.Fatalf("Renaming node2 into node1 should fail") t.Fatalf("Renaming node2 into node1 should fail")
} }
@ -825,13 +873,22 @@ func TestStateStore_EnsureNode(t *testing.T) {
newNodeID := types.NodeID("d0347693-65cc-4d9f-a6e0-5025b2e6513f") newNodeID := types.NodeID("d0347693-65cc-4d9f-a6e0-5025b2e6513f")
// Set a Serf check on the new node to inform whether to allow changing ID
if err := s.EnsureCheck(8, &structs.HealthCheck{
Node: "node1-renamed",
CheckID: structs.SerfCheckID,
Status: api.HealthPassing,
}); err != nil {
t.Fatalf("err: %s", err)
}
// Adding another node with same name should fail // Adding another node with same name should fail
in = &structs.Node{ in = &structs.Node{
Node: "node1-renamed", Node: "node1-renamed",
ID: newNodeID, ID: newNodeID,
Address: "1.1.1.7", Address: "1.1.1.7",
} }
if err := s.EnsureNode(8, in); err == nil { if err := s.EnsureNode(9, in); err == nil {
t.Fatalf("There should be an error since node1-renamed already exists") t.Fatalf("There should be an error since node1-renamed already exists")
} }
@ -841,7 +898,7 @@ func TestStateStore_EnsureNode(t *testing.T) {
ID: newNodeID, ID: newNodeID,
Address: "1.1.1.7", Address: "1.1.1.7",
} }
if err := s.EnsureNode(8, in); err == nil { if err := s.EnsureNode(9, in); err == nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }
@ -851,7 +908,7 @@ func TestStateStore_EnsureNode(t *testing.T) {
ID: newNodeID, ID: newNodeID,
Address: "1.1.1.7", Address: "1.1.1.7",
} }
if err := s.EnsureNode(9, in); err != nil { if err := s.EnsureNode(10, in); err != nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }
@ -867,7 +924,7 @@ func TestStateStore_EnsureNode(t *testing.T) {
ID: newNodeID, ID: newNodeID,
Address: "1.1.1.7", Address: "1.1.1.7",
} }
if err := s.EnsureNode(9, in); err != nil { if err := s.EnsureNode(10, in); err != nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }
@ -877,10 +934,10 @@ func TestStateStore_EnsureNode(t *testing.T) {
} }
// Node and indexes were updated // Node and indexes were updated
if out.ID != newNodeID || out.CreateIndex != 9 || out.ModifyIndex != 9 || out.Address != "1.1.1.7" || out.Node != "Node1bis" { if out.ID != newNodeID || out.CreateIndex != 10 || out.ModifyIndex != 10 || out.Address != "1.1.1.7" || out.Node != "Node1bis" {
t.Fatalf("bad: %#v", out) t.Fatalf("bad: %#v", out)
} }
if idx != 9 { if idx != 10 {
t.Fatalf("bad index: %d", idx) t.Fatalf("bad index: %d", idx)
} }
@ -891,7 +948,7 @@ func TestStateStore_EnsureNode(t *testing.T) {
ID: newNodeID, ID: newNodeID,
Address: "1.1.1.7", Address: "1.1.1.7",
} }
if err := s.EnsureNode(10, in); err == nil { if err := s.EnsureNode(11, in); err == nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }
@ -901,7 +958,7 @@ func TestStateStore_EnsureNode(t *testing.T) {
ID: newNodeID, ID: newNodeID,
Address: "1.1.1.7", Address: "1.1.1.7",
} }
if err := s.EnsureNode(10, in); err == nil { if err := s.EnsureNode(11, in); err == nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }
@ -911,7 +968,7 @@ func TestStateStore_EnsureNode(t *testing.T) {
ID: newNodeID, ID: newNodeID,
Address: "1.1.1.7", Address: "1.1.1.7",
} }
if err := s.EnsureNode(11, in); err != nil { if err := s.EnsureNode(12, in); err != nil {
t.Fatalf("err: %s", err) t.Fatalf("err: %s", err)
} }
idx, out, err = s.GetNode("Node1-Renamed2") idx, out, err = s.GetNode("Node1-Renamed2")
@ -920,10 +977,10 @@ func TestStateStore_EnsureNode(t *testing.T) {
} }
// Node and indexes were updated // Node and indexes were updated
if out.ID != newNodeID || out.CreateIndex != 9 || out.ModifyIndex != 11 || out.Address != "1.1.1.7" || out.Node != "Node1-Renamed2" { if out.ID != newNodeID || out.CreateIndex != 10 || out.ModifyIndex != 12 || out.Address != "1.1.1.7" || out.Node != "Node1-Renamed2" {
t.Fatalf("bad: %#v", out) t.Fatalf("bad: %#v", out)
} }
if idx != 11 { if idx != 12 {
t.Fatalf("bad index: %d", idx) t.Fatalf("bad index: %d", idx)
} }
@ -931,27 +988,27 @@ func TestStateStore_EnsureNode(t *testing.T) {
// See https://github.com/hashicorp/consul/pull/3983 for context // See https://github.com/hashicorp/consul/pull/3983 for context
// Deprecated behavior is following // Deprecated behavior is following
deprecatedEnsureNodeWithoutIDCanRegister(t, s, "new-node-without-id", 12) deprecatedEnsureNodeWithoutIDCanRegister(t, s, "new-node-without-id", 13)
// Deprecated, but should work as well // Deprecated, but should work as well
deprecatedEnsureNodeWithoutIDCanRegister(t, s, "new-node-without-id", 13) deprecatedEnsureNodeWithoutIDCanRegister(t, s, "new-node-without-id", 14)
// All of this is deprecated as well, should be removed // All of this is deprecated as well, should be removed
in = &structs.Node{ in = &structs.Node{
Node: "Node1-Renamed2", Node: "Node1-Renamed2",
Address: "1.1.1.66", Address: "1.1.1.66",
} }
if err := s.EnsureNode(14, in); err != nil { if err := s.EnsureNode(15, in); err != nil {
t.Fatalf("[DEPRECATED] it should work, err:= %q", err) t.Fatalf("[DEPRECATED] it should work, err:= %q", err)
} }
idx, out, err = s.GetNode("Node1-Renamed2") idx, out, err = s.GetNode("Node1-Renamed2")
if err != nil { if err != nil {
t.Fatalf("[DEPRECATED] err: %s", err) t.Fatalf("[DEPRECATED] err: %s", err)
} }
if out.CreateIndex != 9 { if out.CreateIndex != 10 {
t.Fatalf("[DEPRECATED] We expected to modify node previously added, but add index = %d for node %q", out.CreateIndex, out) t.Fatalf("[DEPRECATED] We expected to modify node previously added, but add index = %d for node %q", out.CreateIndex, out)
} }
if out.Address != "1.1.1.66" || out.ModifyIndex != 14 { if out.Address != "1.1.1.66" || out.ModifyIndex != 15 {
t.Fatalf("[DEPRECATED] Node with newNodeID should have been updated, but was: %d with content := %q", out.CreateIndex, out) t.Fatalf("[DEPRECATED] Node with newNodeID should have been updated, but was: %d with content := %q", out.CreateIndex, out)
} }
} }