diff --git a/nomad/fsm_test.go b/nomad/fsm_test.go index 4cbc5720c..8e800300c 100644 --- a/nomad/fsm_test.go +++ b/nomad/fsm_test.go @@ -244,22 +244,22 @@ func TestFSM_UpsertNode_Canonicalize_Ineligible(t *testing.T) { func TestFSM_UpsertNode_NodePool(t *testing.T) { ci.Parallel(t) - fsm := testFSM(t) - - // Populate state with a test node pool. - existingPool := mock.NodePool() - err := fsm.State().UpsertNodePools(structs.MsgTypeTestSetup, 1000, []*structs.NodePool{existingPool}) - must.NoError(t, err) + existingPoolName := "dev" + nodeWithPoolID := uuid.Generate() + nodeWithoutPoolID := uuid.Generate() testCases := []struct { name string - previousPool string + nodeID string pool string + expectedPool string validateFn func(*testing.T, *structs.Node, *structs.NodePool) }{ { - name: "new node pool", - pool: "new", + name: "register new node in new node pool", + nodeID: "", + pool: "new", + expectedPool: "new", validateFn: func(t *testing.T, node *structs.Node, pool *structs.NodePool) { // Verify node pool was created in the same transaction as the // node registration. @@ -267,38 +267,115 @@ func TestFSM_UpsertNode_NodePool(t *testing.T) { }, }, { - name: "existing node pool", - pool: existingPool.Name, + name: "register new node in existing node pool", + nodeID: "", + pool: existingPoolName, + expectedPool: existingPoolName, validateFn: func(t *testing.T, node *structs.Node, pool *structs.NodePool) { // Verify node pool was not modified. must.NotEq(t, pool.CreateIndex, node.ModifyIndex) }, }, { - name: "built-in node pool", - pool: structs.NodePoolDefault, + name: "register new node in built-in node pool", + nodeID: "", + pool: structs.NodePoolDefault, + expectedPool: structs.NodePoolDefault, validateFn: func(t *testing.T, node *structs.Node, pool *structs.NodePool) { // Verify node pool was not modified. must.Eq(t, 1, pool.ModifyIndex) }, }, { - name: "move node to another node pool", - previousPool: structs.NodePoolDefault, + name: "register new node with empty node pool in default", + nodeID: "", + pool: "", + expectedPool: structs.NodePoolDefault, + }, + { + name: "move existing node to new node pool", + nodeID: nodeWithPoolID, pool: "new", + expectedPool: "new", + validateFn: func(t *testing.T, node *structs.Node, pool *structs.NodePool) { + // Verify node pool was created in the same transaction as the + // node was updated. + must.Eq(t, pool.CreateIndex, node.ModifyIndex) + }, + }, + { + name: "move existing node to existing node pool", + nodeID: nodeWithPoolID, + pool: existingPoolName, + expectedPool: existingPoolName, + }, + { + name: "move existing node to built-in node pool", + nodeID: nodeWithPoolID, + pool: structs.NodePoolDefault, + expectedPool: structs.NodePoolDefault, + }, + { + name: "move existing node with empty node pool to default", + nodeID: nodeWithPoolID, + pool: "", + expectedPool: structs.NodePoolDefault, + }, + { + name: "update node without pool to new node pool", + nodeID: nodeWithoutPoolID, + pool: "new", + expectedPool: "new", + }, + { + name: "update node without pool to existing node pool", + nodeID: nodeWithoutPoolID, + pool: existingPoolName, + expectedPool: existingPoolName, + }, + { + name: "update node without pool with empty string to default", + nodeID: nodeWithoutPoolID, + pool: "", + expectedPool: structs.NodePoolDefault, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - node := mock.Node() + fsm := testFSM(t) - // Pre-create node if test cases specifies if belonged to a - // previous node pool. - if tc.previousPool != "" { - node.NodePool = tc.previousPool - err := fsm.State().UpsertNode(structs.MsgTypeTestSetup, 1001, node) - must.NoError(t, err) + // Populate state with a test node pool. + existingPool := mock.NodePool() + existingPool.Name = existingPoolName + err := fsm.State().UpsertNodePools(structs.MsgTypeTestSetup, 1000, []*structs.NodePool{existingPool}) + must.NoError(t, err) + + // Populate state with pre-existing node assigned to the + // pre-existing node pool. + nodeWithPool := mock.Node() + nodeWithPool.ID = nodeWithPoolID + nodeWithPool.NodePool = existingPool.Name + + err = fsm.State().UpsertNode(structs.MsgTypeTestSetup, 1001, nodeWithPool) + must.NoError(t, err) + + // Populate state with pre-existing node with empty node pool to + // simulate an upgrade path. + nodeWithoutPool := mock.Node() + nodeWithoutPool.ID = nodeWithoutPoolID + err = fsm.State().UpsertNode(structs.MsgTypeTestSetup, 1002, nodeWithoutPool) + must.NoError(t, err) + + // Upsert test node. + var node *structs.Node + switch tc.nodeID { + case nodeWithPoolID: + node = nodeWithPool.Copy() + case nodeWithoutPoolID: + node = nodeWithoutPool.Copy() + default: + node = mock.Node() } // Set the node pool and apply node register log. @@ -319,12 +396,12 @@ func TestFSM_UpsertNode_NodePool(t *testing.T) { must.NotNil(t, got) // Verify node pool exists. - pool, err := s.NodePoolByName(nil, tc.pool) + pool, err := s.NodePoolByName(nil, tc.expectedPool) must.NoError(t, err) must.NotNil(t, pool) // Verify node was assigned to node pool. - must.Eq(t, pool.Name, got.NodePool) + must.Eq(t, tc.expectedPool, got.NodePool) if tc.validateFn != nil { tc.validateFn(t, got, pool) diff --git a/nomad/node_pool_endpoint_test.go b/nomad/node_pool_endpoint_test.go index 6965f8838..c15519e50 100644 --- a/nomad/node_pool_endpoint_test.go +++ b/nomad/node_pool_endpoint_test.go @@ -440,7 +440,7 @@ func TestNodePoolEndpoint_GetNodePool(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - // Make node pool list request. + // Make node pool fetch request. req := &structs.NodePoolSpecificRequest{ QueryOptions: structs.QueryOptions{ Region: "global", @@ -536,7 +536,7 @@ func TestNodePoolEndpoint_GetNodePool_ACL(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - // Make node pool list request. + // Make node pool fetch request. req := &structs.NodePoolSpecificRequest{ QueryOptions: structs.QueryOptions{ Region: "global", @@ -581,6 +581,7 @@ func TestNodePoolEndpoint_GetNodePool_BlockingQuery(t *testing.T) { }) // Update first node pool to trigger watcher. + pool1 = pool1.Copy() pool1.Meta["updated"] = "true" time.AfterFunc(300*time.Millisecond, func() { s.fsm.State().UpsertNodePools(structs.MsgTypeTestSetup, 1002, []*structs.NodePool{pool1}) diff --git a/nomad/state/state_store_test.go b/nomad/state/state_store_test.go index f302ba7c6..61ff10089 100644 --- a/nomad/state/state_store_test.go +++ b/nomad/state/state_store_test.go @@ -1303,12 +1303,6 @@ func TestStateStore_UpsertNode_NodePool(t *testing.T) { expectedPool: "new", expectedNewPool: true, }, - { - name: "register new node with empty pool in default", - nodeID: "", - poolName: "", - expectedPool: structs.NodePoolDefault, - }, { name: "update existing node in existing pool", nodeID: nodeWithPoolID, @@ -1322,12 +1316,6 @@ func TestStateStore_UpsertNode_NodePool(t *testing.T) { expectedPool: "new", expectedNewPool: true, }, - { - name: "update existing node with empty pool in default", - nodeID: nodeWithPoolID, - poolName: "", - expectedPool: devPoolName, - }, { name: "update legacy node with pool", nodeID: nodeWithoutPoolID, @@ -1341,12 +1329,6 @@ func TestStateStore_UpsertNode_NodePool(t *testing.T) { expectedPool: "new", expectedNewPool: true, }, - { - name: "update legacy node with empty pool places it in default", - nodeID: nodeWithoutPoolID, - poolName: "", - expectedPool: structs.NodePoolDefault, - }, } for _, tc := range testCases { @@ -1378,17 +1360,20 @@ func TestStateStore_UpsertNode_NodePool(t *testing.T) { var node *structs.Node switch tc.nodeID { case nodeWithPoolID: - node = nodeWithPool + node = nodeWithPool.Copy() case nodeWithoutPoolID: - node = nodeWithoutPool + node = nodeWithoutPool.Copy() default: node = mock.Node() } + node.NodePool = tc.poolName err = state.UpsertNode(structs.MsgTypeTestSetup, 1003, node) must.NoError(t, err) // Verify that node is part of the expected pool. - must.Eq(t, tc.expectedPool, node.NodePool) + got, err := state.NodeByID(nil, node.ID) + must.NoError(t, err) + must.Eq(t, tc.expectedPool, got.NodePool) // Fech pool. pool, err := state.NodePoolByName(nil, tc.expectedPool)