From 6b13b90b7e596f7c2ca2bd9a62660723d20ec6ef Mon Sep 17 00:00:00 2001 From: davemay99 Date: Tue, 5 Oct 2021 21:51:02 -0400 Subject: [PATCH 01/24] Rename argNodes to generic utility function --- command/operator_debug.go | 9 +++++---- command/operator_debug_test.go | 23 +++++++++++++++++------ 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/command/operator_debug.go b/command/operator_debug.go index 41fdbe74471..77a5dcc8cc3 100644 --- a/command/operator_debug.go +++ b/command/operator_debug.go @@ -329,7 +329,7 @@ func (c *OperatorDebugCommand) Run(args []string) int { nodeLookupFailCount := 0 nodeCaptureCount := 0 - for _, id := range argNodes(nodeIDs) { + for _, id := range stringToSlice(nodeIDs) { if id == "all" { // Capture from all nodes using empty prefix filter id = "" @@ -390,7 +390,7 @@ func (c *OperatorDebugCommand) Run(args []string) int { c.serverIDs = append(c.serverIDs, member.Name) } } else { - c.serverIDs = append(c.serverIDs, argNodes(serverIDs)...) + c.serverIDs = append(c.serverIDs, stringToSlice(serverIDs)...) } serversFound := 0 @@ -1055,8 +1055,9 @@ func TarCZF(archive string, src, target string) error { }) } -// argNodes splits node ids from the command line by "," -func argNodes(input string) []string { +// stringToSlice splits CSV string into slice, trims whitespace, and prunes +// empty values +func stringToSlice(input string) []string { ns := strings.Split(input, ",") var out []string for _, n := range ns { diff --git a/command/operator_debug_test.go b/command/operator_debug_test.go index a5aa6f88dcc..0c5b5809fc0 100644 --- a/command/operator_debug_test.go +++ b/command/operator_debug_test.go @@ -432,15 +432,26 @@ func TestDebug_Fail_Pprof(t *testing.T) { require.Contains(t, ui.OutputWriter.String(), "Created debug archive") // Archive should be generated anyway } -func TestDebug_Utils(t *testing.T) { +func TestDebug_StringToSlice(t *testing.T) { t.Parallel() - xs := argNodes("foo, bar") - require.Equal(t, []string{"foo", "bar"}, xs) + cases := []struct { + input string + expected []string + }{ + {input: ",,", expected: []string(nil)}, + {input: "", expected: []string(nil)}, + {input: "foo, bar", expected: []string{"foo", "bar"}}, + } + for _, tc := range cases { + out := stringToSlice(tc.input) + require.Equal(t, tc.expected, out) + require.Equal(t, true, helper.CompareSliceSetString(tc.expected, out)) + } +} - xs = argNodes("") - require.Len(t, xs, 0) - require.Empty(t, xs) +func TestDebug_External(t *testing.T) { + t.Parallel() // address calculation honors CONSUL_HTTP_SSL // ssl: true - Correct alignment From 3c47f922550744c6a2c0c65424023f89a3f6e8b8 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Tue, 5 Oct 2021 22:04:52 -0400 Subject: [PATCH 02/24] Add region and prefix matching for server members --- command/operator_debug.go | 51 +++++++++++++++++++++++++++++++++------ helper/funcs.go | 10 ++++++++ 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/command/operator_debug.go b/command/operator_debug.go index 77a5dcc8cc3..bab6f449a31 100644 --- a/command/operator_debug.go +++ b/command/operator_debug.go @@ -382,15 +382,15 @@ func (c *OperatorDebugCommand) Run(args []string) int { c.Ui.Error(fmt.Sprintf("Failed to retrieve server list; err: %v", err)) return 1 } + + // Write complete list of server members to file c.writeJSON("version", "members.json", members, err) - // We always write the error to the file, but don't range if no members found - if serverIDs == "all" && members != nil { - // Special case to capture from all servers - for _, member := range members.Members { - c.serverIDs = append(c.serverIDs, member.Name) - } - } else { - c.serverIDs = append(c.serverIDs, stringToSlice(serverIDs)...) + + // Filter for servers matching criteria + c.serverIDs, err = parseMembers(members, serverIDs, c.region) + if err != nil { + c.Ui.Error(fmt.Sprintf("Failed to parse server list; err: %v", err)) + return 1 } serversFound := 0 @@ -1055,6 +1055,41 @@ func TarCZF(archive string, src, target string) error { }) } +// parseMembers returns a slice of server member names matching the search criteria +func parseMembers(serverMembers *api.ServerMembers, serverIDs string, region string) (membersFound []string, err error) { + prefixes := stringToSlice(serverIDs) + + if serverMembers.Members == nil { + return nil, fmt.Errorf("Failed to parse server members, members==nil") + } + + for _, member := range serverMembers.Members { + // If region is provided it must match exactly + if region != "" && member.Tags["region"] != region { + continue + } + + // Always include "all" + if serverIDs == "all" { + membersFound = append(membersFound, member.Name) + continue + } + + // Special case passthrough as literally "leader" + if serverIDs == "leader" { + membersFound = append(membersFound, "leader") + continue + } + + // Include member if name matches any prefix + if helper.SliceStringContainsPrefix(prefixes, member.Name) { + membersFound = append(membersFound, member.Name) + } + } + + return membersFound, nil +} + // stringToSlice splits CSV string into slice, trims whitespace, and prunes // empty values func stringToSlice(input string) []string { diff --git a/helper/funcs.go b/helper/funcs.go index 690ee3f7c24..63ca376b7de 100644 --- a/helper/funcs.go +++ b/helper/funcs.go @@ -197,6 +197,16 @@ func SliceStringContains(list []string, item string) bool { return false } +// SliceStringContainsPrefix returns true if any string in list matches prefix +func SliceStringContainsPrefix(list []string, prefix string) bool { + for _, s := range list { + if strings.HasPrefix(s, prefix) { + return true + } + } + return false +} + func SliceSetDisjoint(first, second []string) (bool, []string) { contained := make(map[string]struct{}, len(first)) for _, k := range first { From 8897ed6bdf95ca597c033d8b778f8fdaf21d875f Mon Sep 17 00:00:00 2001 From: davemay99 Date: Tue, 5 Oct 2021 22:05:46 -0400 Subject: [PATCH 03/24] Include region and namespace in CLI output --- command/operator_debug.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/command/operator_debug.go b/command/operator_debug.go index bab6f449a31..11b22c7347f 100644 --- a/command/operator_debug.go +++ b/command/operator_debug.go @@ -412,6 +412,8 @@ func (c *OperatorDebugCommand) Run(args []string) int { // Display general info about the capture c.Ui.Output("Starting debugger...") c.Ui.Output("") + c.Ui.Output(fmt.Sprintf(" Region: %s", c.region)) + c.Ui.Output(fmt.Sprintf(" Namespace: %s", c.namespace)) c.Ui.Output(fmt.Sprintf(" Servers: (%d/%d) %v", serverCaptureCount, serversFound, c.serverIDs)) c.Ui.Output(fmt.Sprintf(" Clients: (%d/%d) %v", nodeCaptureCount, nodesFound, c.nodeIDs)) if nodeCaptureCount > 0 && nodeCaptureCount == c.maxNodes { From 25ed8ffef5ee02b0e78154682b59f16261edcf1a Mon Sep 17 00:00:00 2001 From: davemay99 Date: Tue, 5 Oct 2021 22:28:18 -0400 Subject: [PATCH 04/24] Add region awareness to WaitForClient helper --- command/operator_debug_test.go | 12 ++++++++---- nomad/client_agent_endpoint_test.go | 2 +- testutil/wait.go | 7 +++++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/command/operator_debug_test.go b/command/operator_debug_test.go index 0c5b5809fc0..ca2a94d3369 100644 --- a/command/operator_debug_test.go +++ b/command/operator_debug_test.go @@ -78,7 +78,8 @@ func TestDebug_NodeClass(t *testing.T) { // Wait for client1 to connect client1NodeID := client1.Agent.Client().NodeID() - testutil.WaitForClient(t, srv.Agent.RPC, client1NodeID) + client1Region := client1.Agent.Client().Region() + testutil.WaitForClient(t, srv.Agent.RPC, client1NodeID, client1Region) t.Logf("[TEST] Client1 ready, id: %s", client1NodeID) // Setup client 2 (nodeclass = clientb) @@ -96,7 +97,8 @@ func TestDebug_NodeClass(t *testing.T) { // Wait for client2 to connect client2NodeID := client2.Agent.Client().NodeID() - testutil.WaitForClient(t, srv.Agent.RPC, client2NodeID) + client2Region := client2.Agent.Client().Region() + testutil.WaitForClient(t, srv.Agent.RPC, client2NodeID, client2Region) t.Logf("[TEST] Client2 ready, id: %s", client2NodeID) // Setup client 3 (nodeclass = clienta) @@ -112,7 +114,8 @@ func TestDebug_NodeClass(t *testing.T) { // Wait for client3 to connect client3NodeID := client3.Agent.Client().NodeID() - testutil.WaitForClient(t, srv.Agent.RPC, client3NodeID) + client3Region := client3.Agent.Client().Region() + testutil.WaitForClient(t, srv.Agent.RPC, client3NodeID, client3Region) t.Logf("[TEST] Client3 ready, id: %s", client3NodeID) // Setup test cases @@ -174,7 +177,8 @@ func TestDebug_ClientToServer(t *testing.T) { // Wait for client 1 to connect client1NodeID := client1.Agent.Client().NodeID() - testutil.WaitForClient(t, srv.Agent.RPC, client1NodeID) + client1Region := client1.Agent.Client().Region() + testutil.WaitForClient(t, srv.Agent.RPC, client1NodeID, client1Region) t.Logf("[TEST] Client1 ready, id: %s", client1NodeID) // Get API addresses diff --git a/nomad/client_agent_endpoint_test.go b/nomad/client_agent_endpoint_test.go index 2f737ebffb7..e3c11ee8370 100644 --- a/nomad/client_agent_endpoint_test.go +++ b/nomad/client_agent_endpoint_test.go @@ -548,7 +548,7 @@ func TestAgentProfile_RemoteClient(t *testing.T) { }) defer cleanupC() - testutil.WaitForClient(t, s2.RPC, c.NodeID()) + testutil.WaitForClient(t, s2.RPC, c.NodeID(), c.Region()) testutil.WaitForResult(func() (bool, error) { nodes := s2.connectedNodes() return len(nodes) == 1, nil diff --git a/testutil/wait.go b/testutil/wait.go index 19215d0ced0..a4eb7d345a4 100644 --- a/testutil/wait.go +++ b/testutil/wait.go @@ -112,12 +112,15 @@ func WaitForLeader(t testing.TB, rpc rpcFn) { } // WaitForClient blocks until the client can be found -func WaitForClient(t testing.TB, rpc rpcFn, nodeID string) { +func WaitForClient(t testing.TB, rpc rpcFn, nodeID string, region string) { t.Helper() + if region == "" { + region = "global" + } WaitForResult(func() (bool, error) { req := structs.NodeSpecificRequest{ NodeID: nodeID, - QueryOptions: structs.QueryOptions{Region: "global"}, + QueryOptions: structs.QueryOptions{Region: region}, } var out structs.SingleNodeResponse From 2db750d19960a1b18eb7fbd2c26702404146c40b Mon Sep 17 00:00:00 2001 From: davemay99 Date: Tue, 5 Oct 2021 22:31:22 -0400 Subject: [PATCH 05/24] Align variable names with underlying type --- command/operator_debug_test.go | 46 ++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/command/operator_debug_test.go b/command/operator_debug_test.go index ca2a94d3369..823e26addb8 100644 --- a/command/operator_debug_test.go +++ b/command/operator_debug_test.go @@ -73,14 +73,15 @@ func TestDebug_NodeClass(t *testing.T) { } // Start client 1 - client1 := agent.NewTestAgent(t, "client1", agentConfFunc1) - defer client1.Shutdown() + agent1 := agent.NewTestAgent(t, "client1", agentConfFunc1) + defer agent1.Shutdown() // Wait for client1 to connect - client1NodeID := client1.Agent.Client().NodeID() - client1Region := client1.Agent.Client().Region() + client1 := agent1.Agent.Client() + client1NodeID := client1.NodeID() + client1Region := client1.Region() testutil.WaitForClient(t, srv.Agent.RPC, client1NodeID, client1Region) - t.Logf("[TEST] Client1 ready, id: %s", client1NodeID) + t.Logf("[TEST] Client1 ready, id: %s, region: %s", client1NodeID, client1Region) // Setup client 2 (nodeclass = clientb) agentConfFunc2 := func(c *agent.Config) { @@ -92,14 +93,15 @@ func TestDebug_NodeClass(t *testing.T) { } // Start client 2 - client2 := agent.NewTestAgent(t, "client2", agentConfFunc2) - defer client2.Shutdown() + agent2 := agent.NewTestAgent(t, "client2", agentConfFunc2) + defer agent2.Shutdown() // Wait for client2 to connect - client2NodeID := client2.Agent.Client().NodeID() - client2Region := client2.Agent.Client().Region() + client2 := agent2.Agent.Client() + client2NodeID := client2.NodeID() + client2Region := client2.Region() testutil.WaitForClient(t, srv.Agent.RPC, client2NodeID, client2Region) - t.Logf("[TEST] Client2 ready, id: %s", client2NodeID) + t.Logf("[TEST] Client2 ready, id: %s, region: %s", client2NodeID, client2Region) // Setup client 3 (nodeclass = clienta) agentConfFunc3 := func(c *agent.Config) { @@ -109,14 +111,15 @@ func TestDebug_NodeClass(t *testing.T) { } // Start client 3 - client3 := agent.NewTestAgent(t, "client3", agentConfFunc3) - defer client3.Shutdown() + agent3 := agent.NewTestAgent(t, "client3", agentConfFunc3) + defer agent3.Shutdown() // Wait for client3 to connect - client3NodeID := client3.Agent.Client().NodeID() - client3Region := client3.Agent.Client().Region() + client3 := agent3.Agent.Client() + client3NodeID := client3.NodeID() + client3Region := client3.Region() testutil.WaitForClient(t, srv.Agent.RPC, client3NodeID, client3Region) - t.Logf("[TEST] Client3 ready, id: %s", client3NodeID) + t.Logf("[TEST] Client3 ready, id: %s, region: %s", client3NodeID, client3Region) // Setup test cases cases := testCases{ @@ -172,18 +175,19 @@ func TestDebug_ClientToServer(t *testing.T) { } // Start client 1 - client1 := agent.NewTestAgent(t, "client1", agentConfFunc1) - defer client1.Shutdown() + agent1 := agent.NewTestAgent(t, "client1", agentConfFunc1) + defer agent1.Shutdown() // Wait for client 1 to connect - client1NodeID := client1.Agent.Client().NodeID() - client1Region := client1.Agent.Client().Region() + client1 := agent1.Agent.Client() + client1NodeID := client1.NodeID() + client1Region := client1.Region() testutil.WaitForClient(t, srv.Agent.RPC, client1NodeID, client1Region) - t.Logf("[TEST] Client1 ready, id: %s", client1NodeID) + t.Logf("[TEST] Client1 ready, id: %s, region: %s", client1NodeID, client1Region) // Get API addresses addrServer := srv.HTTPAddr() - addrClient1 := client1.HTTPAddr() + addrClient1 := agent1.HTTPAddr() t.Logf("[TEST] testAgent api address: %s", url) t.Logf("[TEST] Server api address: %s", addrServer) From 4c8fa58ea8529f9dd9b8ed0dc623f34e94b7b096 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Tue, 5 Oct 2021 22:37:37 -0400 Subject: [PATCH 06/24] Add test for region --- command/operator_debug_test.go | 82 ++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/command/operator_debug_test.go b/command/operator_debug_test.go index 823e26addb8..d0b95cf1502 100644 --- a/command/operator_debug_test.go +++ b/command/operator_debug_test.go @@ -218,6 +218,88 @@ func TestDebug_ClientToServer(t *testing.T) { runTestCases(t, cases) } +func TestDebug_ClientToServer_Region(t *testing.T) { + agentConfFunc := func(c *agent.Config) { + c.Region = "testregion" + } + + // Start test server and API client + srv, _, url := testServer(t, false, agentConfFunc) + defer srv.Shutdown() + + // Wait for leadership to establish + testutil.WaitForLeader(t, srv.Agent.RPC) + + // Retrieve server RPC address to join client + srvRPCAddr := srv.GetConfig().AdvertiseAddrs.RPC + t.Logf("[TEST] Leader started, srv.GetConfig().AdvertiseAddrs.RPC: %s", srvRPCAddr) + + // Setup client 1 (nodeclass = clienta) + agentConfFunc1 := func(c *agent.Config) { + c.Region = "testregion" + c.Server.Enabled = false + c.Client.NodeClass = "clienta" + c.Client.Enabled = true + c.Client.Servers = []string{srvRPCAddr} + } + + // Start client 1 + agent1 := agent.NewTestAgent(t, "client1", agentConfFunc1) + defer agent1.Shutdown() + + // Wait for client 1 to connect + client1NodeID := agent1.Agent.Client().NodeID() + client1Region := agent1.Agent.Client().Region() + testutil.WaitForClient(t, srv.Agent.RPC, client1NodeID, client1Region) + t.Logf("[TEST] Client1 ready, id: %s, region: %s", client1NodeID, client1Region) + + // Get API addresses + addrServer := srv.HTTPAddr() + addrClient1 := agent1.HTTPAddr() + + t.Logf("[TEST] testAgent api address: %s", url) + t.Logf("[TEST] Server api address: %s", addrServer) + t.Logf("[TEST] Client1 api address: %s", addrClient1) + + // Setup test cases + var cases = testCases{ + // Good + { + name: "testAgent api server", + args: []string{"-address", url, "-region", "testregion", "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, + expectedCode: 0, + expectedOutputs: []string{ + "Region: testregion\n", + "Servers: (1/1)", + "Clients: (1/1)", + "Created debug archive", + }, + }, + { + name: "server address", + args: []string{"-address", addrServer, "-region", "testregion", "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, + expectedCode: 0, + expectedOutputs: []string{"Created debug archive"}, + }, + { + name: "client1 address - verify no SIGSEGV panic", + args: []string{"-address", addrClient1, "-region", "testregion", "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, + expectedCode: 0, + expectedOutputs: []string{"Created debug archive"}, + }, + + // Bad + { + name: "invalid region - all servers, all clients", + args: []string{"-address", url, "-region", "never", "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, + expectedCode: 1, + expectedError: "500 (No path to region)", + }, + } + + runTestCases(t, cases) +} + func TestDebug_SingleServer(t *testing.T) { srv, _, url := testServer(t, false, nil) defer srv.Shutdown() From 33132cf9599073aa6dc64a093027cc2657c1fe81 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Tue, 5 Oct 2021 23:06:55 -0400 Subject: [PATCH 07/24] Add namespaces and regions to cluster meta info --- command/operator_debug.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/command/operator_debug.go b/command/operator_debug.go index 11b22c7347f..d48144fd71a 100644 --- a/command/operator_debug.go +++ b/command/operator_debug.go @@ -470,6 +470,13 @@ func (c *OperatorDebugCommand) collect(client *api.Client) error { self, err := client.Agent().Self() c.writeJSON(dir, "agent-self.json", self, err) + var qo *api.QueryOptions + namespaces, _, err := client.Namespaces().List(qo) + c.writeJSON(dir, "namespaces.json", namespaces, err) + + regions, err := client.Regions().List() + c.writeJSON(dir, "regions.json", regions, err) + // Fetch data directly from consul and vault. Ignore errors var consul, vault string From c43b6972b649d3bcd353f9ac4e6ba37413677a68 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Tue, 5 Oct 2021 23:26:37 -0400 Subject: [PATCH 08/24] Add changelog --- .changelog/11269.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/11269.txt diff --git a/.changelog/11269.txt b/.changelog/11269.txt new file mode 100644 index 00000000000..59cbd29b465 --- /dev/null +++ b/.changelog/11269.txt @@ -0,0 +1,3 @@ +```release-note:improvement +cli: Improve debug namespace and region support +``` From d45cfa059022a56f1643772d11bd0955824afbe8 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Wed, 6 Oct 2021 20:02:07 -0400 Subject: [PATCH 09/24] Refactor WaitForClient helper function --- command/operator_debug_test.go | 39 ++++------------------------- nomad/client_agent_endpoint_test.go | 2 +- testutil/wait.go | 9 ++++--- 3 files changed, 12 insertions(+), 38 deletions(-) diff --git a/command/operator_debug_test.go b/command/operator_debug_test.go index d0b95cf1502..a9abf331e88 100644 --- a/command/operator_debug_test.go +++ b/command/operator_debug_test.go @@ -75,13 +75,7 @@ func TestDebug_NodeClass(t *testing.T) { // Start client 1 agent1 := agent.NewTestAgent(t, "client1", agentConfFunc1) defer agent1.Shutdown() - - // Wait for client1 to connect - client1 := agent1.Agent.Client() - client1NodeID := client1.NodeID() - client1Region := client1.Region() - testutil.WaitForClient(t, srv.Agent.RPC, client1NodeID, client1Region) - t.Logf("[TEST] Client1 ready, id: %s, region: %s", client1NodeID, client1Region) + testutil.WaitForClient(t, srv.Agent.RPC, agent1.Agent.Client()) // Setup client 2 (nodeclass = clientb) agentConfFunc2 := func(c *agent.Config) { @@ -95,13 +89,7 @@ func TestDebug_NodeClass(t *testing.T) { // Start client 2 agent2 := agent.NewTestAgent(t, "client2", agentConfFunc2) defer agent2.Shutdown() - - // Wait for client2 to connect - client2 := agent2.Agent.Client() - client2NodeID := client2.NodeID() - client2Region := client2.Region() - testutil.WaitForClient(t, srv.Agent.RPC, client2NodeID, client2Region) - t.Logf("[TEST] Client2 ready, id: %s, region: %s", client2NodeID, client2Region) + testutil.WaitForClient(t, srv.Agent.RPC, agent2.Agent.Client()) // Setup client 3 (nodeclass = clienta) agentConfFunc3 := func(c *agent.Config) { @@ -113,13 +101,7 @@ func TestDebug_NodeClass(t *testing.T) { // Start client 3 agent3 := agent.NewTestAgent(t, "client3", agentConfFunc3) defer agent3.Shutdown() - - // Wait for client3 to connect - client3 := agent3.Agent.Client() - client3NodeID := client3.NodeID() - client3Region := client3.Region() - testutil.WaitForClient(t, srv.Agent.RPC, client3NodeID, client3Region) - t.Logf("[TEST] Client3 ready, id: %s, region: %s", client3NodeID, client3Region) + testutil.WaitForClient(t, srv.Agent.RPC, agent3.Agent.Client()) // Setup test cases cases := testCases{ @@ -177,13 +159,7 @@ func TestDebug_ClientToServer(t *testing.T) { // Start client 1 agent1 := agent.NewTestAgent(t, "client1", agentConfFunc1) defer agent1.Shutdown() - - // Wait for client 1 to connect - client1 := agent1.Agent.Client() - client1NodeID := client1.NodeID() - client1Region := client1.Region() - testutil.WaitForClient(t, srv.Agent.RPC, client1NodeID, client1Region) - t.Logf("[TEST] Client1 ready, id: %s, region: %s", client1NodeID, client1Region) + testutil.WaitForClient(t, srv.Agent.RPC, agent1.Agent.Client()) // Get API addresses addrServer := srv.HTTPAddr() @@ -246,12 +222,7 @@ func TestDebug_ClientToServer_Region(t *testing.T) { // Start client 1 agent1 := agent.NewTestAgent(t, "client1", agentConfFunc1) defer agent1.Shutdown() - - // Wait for client 1 to connect - client1NodeID := agent1.Agent.Client().NodeID() - client1Region := agent1.Agent.Client().Region() - testutil.WaitForClient(t, srv.Agent.RPC, client1NodeID, client1Region) - t.Logf("[TEST] Client1 ready, id: %s, region: %s", client1NodeID, client1Region) + testutil.WaitForClient(t, srv.Agent.RPC, agent1.Agent.Client()) // Get API addresses addrServer := srv.HTTPAddr() diff --git a/nomad/client_agent_endpoint_test.go b/nomad/client_agent_endpoint_test.go index e3c11ee8370..70a01ec3b0c 100644 --- a/nomad/client_agent_endpoint_test.go +++ b/nomad/client_agent_endpoint_test.go @@ -548,7 +548,7 @@ func TestAgentProfile_RemoteClient(t *testing.T) { }) defer cleanupC() - testutil.WaitForClient(t, s2.RPC, c.NodeID(), c.Region()) + testutil.WaitForClient(t, s2.RPC, c) testutil.WaitForResult(func() (bool, error) { nodes := s2.connectedNodes() return len(nodes) == 1, nil diff --git a/testutil/wait.go b/testutil/wait.go index a4eb7d345a4..5f1792bc126 100644 --- a/testutil/wait.go +++ b/testutil/wait.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/hashicorp/nomad/client" "github.com/hashicorp/nomad/nomad/structs" "github.com/kr/pretty" "github.com/stretchr/testify/require" @@ -112,15 +113,15 @@ func WaitForLeader(t testing.TB, rpc rpcFn) { } // WaitForClient blocks until the client can be found -func WaitForClient(t testing.TB, rpc rpcFn, nodeID string, region string) { +func WaitForClient(t testing.TB, rpc rpcFn, client *client.Client) { t.Helper() if region == "" { region = "global" } WaitForResult(func() (bool, error) { req := structs.NodeSpecificRequest{ - NodeID: nodeID, - QueryOptions: structs.QueryOptions{Region: region}, + NodeID: client.NodeID(), + QueryOptions: structs.QueryOptions{Region: client.Region()}, } var out structs.SingleNodeResponse @@ -135,6 +136,8 @@ func WaitForClient(t testing.TB, rpc rpcFn, nodeID string, region string) { }, func(err error) { t.Fatalf("failed to find node: %v", err) }) + + t.Logf("[TEST] Client %s ready, id: %s, region: %s", client.GetConfig().Node.Name, client.NodeID(), client.Region()) } // WaitForVotingMembers blocks until autopilot promotes all server peers From 74867b17d81b06f3b58bdc781d40cb68eab45f36 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Wed, 6 Oct 2021 20:16:43 -0400 Subject: [PATCH 10/24] Simplify test agent configuration functions --- command/operator_debug_test.go | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/command/operator_debug_test.go b/command/operator_debug_test.go index a9abf331e88..e566b19a868 100644 --- a/command/operator_debug_test.go +++ b/command/operator_debug_test.go @@ -66,9 +66,7 @@ func TestDebug_NodeClass(t *testing.T) { // Setup client 1 (nodeclass = clienta) agentConfFunc1 := func(c *agent.Config) { c.Region = "global" - c.Server.Enabled = false c.Client.NodeClass = "clienta" - c.Client.Enabled = true c.Client.Servers = []string{srvRPCAddr} } @@ -80,9 +78,7 @@ func TestDebug_NodeClass(t *testing.T) { // Setup client 2 (nodeclass = clientb) agentConfFunc2 := func(c *agent.Config) { c.Region = "global" - c.Server.Enabled = false c.Client.NodeClass = "clientb" - c.Client.Enabled = true c.Client.Servers = []string{srvRPCAddr} } @@ -93,7 +89,6 @@ func TestDebug_NodeClass(t *testing.T) { // Setup client 3 (nodeclass = clienta) agentConfFunc3 := func(c *agent.Config) { - c.Server.Enabled = false c.Client.NodeClass = "clienta" c.Client.Servers = []string{srvRPCAddr} } @@ -149,10 +144,7 @@ func TestDebug_ClientToServer(t *testing.T) { // Setup client 1 (nodeclass = clienta) agentConfFunc1 := func(c *agent.Config) { - c.Region = "global" - c.Server.Enabled = false c.Client.NodeClass = "clienta" - c.Client.Enabled = true c.Client.Servers = []string{srvRPCAddr} } @@ -195,12 +187,11 @@ func TestDebug_ClientToServer(t *testing.T) { } func TestDebug_ClientToServer_Region(t *testing.T) { - agentConfFunc := func(c *agent.Config) { + // Start test server and API client + srv, _, url := testServer(t, false, func(c *agent.Config) { c.Region = "testregion" - } + }) - // Start test server and API client - srv, _, url := testServer(t, false, agentConfFunc) defer srv.Shutdown() // Wait for leadership to establish @@ -213,9 +204,7 @@ func TestDebug_ClientToServer_Region(t *testing.T) { // Setup client 1 (nodeclass = clienta) agentConfFunc1 := func(c *agent.Config) { c.Region = "testregion" - c.Server.Enabled = false c.Client.NodeClass = "clienta" - c.Client.Enabled = true c.Client.Servers = []string{srvRPCAddr} } From 8274faf7f1071f8ac1699a90275a3fdc9ae72bc5 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Wed, 6 Oct 2021 21:05:58 -0400 Subject: [PATCH 11/24] Tighten StringToSlice test coverage --- command/operator_debug_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/command/operator_debug_test.go b/command/operator_debug_test.go index e566b19a868..f7d62159c2e 100644 --- a/command/operator_debug_test.go +++ b/command/operator_debug_test.go @@ -492,11 +492,12 @@ func TestDebug_StringToSlice(t *testing.T) { {input: ",,", expected: []string(nil)}, {input: "", expected: []string(nil)}, {input: "foo, bar", expected: []string{"foo", "bar"}}, + {input: " foo, bar ", expected: []string{"foo", "bar"}}, + {input: "foo,,bar", expected: []string{"foo", "bar"}}, } for _, tc := range cases { out := stringToSlice(tc.input) require.Equal(t, tc.expected, out) - require.Equal(t, true, helper.CompareSliceSetString(tc.expected, out)) } } From 23bd22ee183989dd007412538747db9c76c1c2f7 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Wed, 6 Oct 2021 21:17:26 -0400 Subject: [PATCH 12/24] Clarify test names --- command/operator_debug_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/command/operator_debug_test.go b/command/operator_debug_test.go index f7d62159c2e..110c0b1290f 100644 --- a/command/operator_debug_test.go +++ b/command/operator_debug_test.go @@ -225,7 +225,7 @@ func TestDebug_ClientToServer_Region(t *testing.T) { var cases = testCases{ // Good { - name: "testAgent api server", + name: "region - testAgent api server", args: []string{"-address", url, "-region", "testregion", "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, expectedCode: 0, expectedOutputs: []string{ @@ -236,13 +236,13 @@ func TestDebug_ClientToServer_Region(t *testing.T) { }, }, { - name: "server address", + name: "region - erver address", args: []string{"-address", addrServer, "-region", "testregion", "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, expectedCode: 0, expectedOutputs: []string{"Created debug archive"}, }, { - name: "client1 address - verify no SIGSEGV panic", + name: "region - client1 address - verify no SIGSEGV panic", args: []string{"-address", addrClient1, "-region", "testregion", "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, expectedCode: 0, expectedOutputs: []string{"Created debug archive"}, From 38bb29b3e7e1eb1ec214ff5303e59161abf30a91 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Wed, 6 Oct 2021 22:15:49 -0400 Subject: [PATCH 13/24] Rename server filter function for clarity --- command/operator_debug.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/command/operator_debug.go b/command/operator_debug.go index d48144fd71a..70eacd7a8b1 100644 --- a/command/operator_debug.go +++ b/command/operator_debug.go @@ -387,7 +387,7 @@ func (c *OperatorDebugCommand) Run(args []string) int { c.writeJSON("version", "members.json", members, err) // Filter for servers matching criteria - c.serverIDs, err = parseMembers(members, serverIDs, c.region) + c.serverIDs, err = filterServerMembers(members, serverIDs, c.region) if err != nil { c.Ui.Error(fmt.Sprintf("Failed to parse server list; err: %v", err)) return 1 @@ -1064,10 +1064,8 @@ func TarCZF(archive string, src, target string) error { }) } -// parseMembers returns a slice of server member names matching the search criteria -func parseMembers(serverMembers *api.ServerMembers, serverIDs string, region string) (membersFound []string, err error) { - prefixes := stringToSlice(serverIDs) - +// filterServerMembers returns a slice of server member names matching the search criteria +func filterServerMembers(serverMembers *api.ServerMembers, serverIDs string, region string) (membersFound []string, err error) { if serverMembers.Members == nil { return nil, fmt.Errorf("Failed to parse server members, members==nil") } From a99ab0eb9d0de7f3182a626bcf85d2cb18b35b81 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Wed, 6 Oct 2021 22:16:14 -0400 Subject: [PATCH 14/24] Move leader check outside loop to prevent duplicates --- command/operator_debug.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/command/operator_debug.go b/command/operator_debug.go index 70eacd7a8b1..06f5c1bb4e7 100644 --- a/command/operator_debug.go +++ b/command/operator_debug.go @@ -1070,6 +1070,16 @@ func filterServerMembers(serverMembers *api.ServerMembers, serverIDs string, reg return nil, fmt.Errorf("Failed to parse server members, members==nil") } + prefixes := stringToSlice(serverIDs) + + // "leader" is a special case which Nomad handles in the API. If "leader" + // appears in serverIDs, add it to membersFound and remove it from the list + // so that it isn't processed by the range loop + if helper.SliceStringContains(prefixes, "leader") { + membersFound = append(membersFound, "leader") + helper.RemoveEqualFold(&prefixes, "leader") + } + for _, member := range serverMembers.Members { // If region is provided it must match exactly if region != "" && member.Tags["region"] != region { @@ -1082,12 +1092,6 @@ func filterServerMembers(serverMembers *api.ServerMembers, serverIDs string, reg continue } - // Special case passthrough as literally "leader" - if serverIDs == "leader" { - membersFound = append(membersFound, "leader") - continue - } - // Include member if name matches any prefix if helper.SliceStringContainsPrefix(prefixes, member.Name) { membersFound = append(membersFound, member.Name) From 1db43151305571d0d54b968c18a5d43d79e71370 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Wed, 6 Oct 2021 22:16:37 -0400 Subject: [PATCH 15/24] Adjust comment for clarity --- command/operator_debug.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/operator_debug.go b/command/operator_debug.go index 06f5c1bb4e7..a2570581bb3 100644 --- a/command/operator_debug.go +++ b/command/operator_debug.go @@ -1101,8 +1101,8 @@ func filterServerMembers(serverMembers *api.ServerMembers, serverIDs string, reg return membersFound, nil } -// stringToSlice splits CSV string into slice, trims whitespace, and prunes -// empty values +// stringToSlice splits comma-separated input string into slice, trims +// whitespace, and prunes empty values func stringToSlice(input string) []string { ns := strings.Split(input, ",") var out []string From 1ee480ee49f75b8cff574ec465aaf459b496063d Mon Sep 17 00:00:00 2001 From: davemay99 Date: Mon, 11 Oct 2021 22:10:58 -0400 Subject: [PATCH 16/24] Fix region regression --- testutil/wait.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/testutil/wait.go b/testutil/wait.go index 5f1792bc126..11d05d129e3 100644 --- a/testutil/wait.go +++ b/testutil/wait.go @@ -115,13 +115,15 @@ func WaitForLeader(t testing.TB, rpc rpcFn) { // WaitForClient blocks until the client can be found func WaitForClient(t testing.TB, rpc rpcFn, client *client.Client) { t.Helper() + + region := client.Region() if region == "" { region = "global" } WaitForResult(func() (bool, error) { req := structs.NodeSpecificRequest{ NodeID: client.NodeID(), - QueryOptions: structs.QueryOptions{Region: client.Region()}, + QueryOptions: structs.QueryOptions{Region: region}, } var out structs.SingleNodeResponse @@ -137,7 +139,7 @@ func WaitForClient(t testing.TB, rpc rpcFn, client *client.Client) { t.Fatalf("failed to find node: %v", err) }) - t.Logf("[TEST] Client %s ready, id: %s, region: %s", client.GetConfig().Node.Name, client.NodeID(), client.Region()) + t.Logf("[TEST] Client %s ready, id: %s, region: %s", client.GetConfig().Node.Name, client.NodeID(), region) } // WaitForVotingMembers blocks until autopilot promotes all server peers From c9b03934217ce10ba6437b3a5c01e60b86a8408a Mon Sep 17 00:00:00 2001 From: davemay99 Date: Mon, 11 Oct 2021 22:27:34 -0400 Subject: [PATCH 17/24] Refactor test client agent generation --- command/operator_debug_test.go | 100 +++++++++++---------------------- command/util_test.go | 20 +++++++ 2 files changed, 54 insertions(+), 66 deletions(-) diff --git a/command/operator_debug_test.go b/command/operator_debug_test.go index 110c0b1290f..f037026bfb5 100644 --- a/command/operator_debug_test.go +++ b/command/operator_debug_test.go @@ -50,6 +50,19 @@ func runTestCases(t *testing.T, cases testCases) { }) } } +func newClientAgentConfigFunc(region string, nodeClass string, srvRPCAddr string) func(*agent.Config) { + if region == "" { + region = "global" + } + + return func(c *agent.Config) { + c.Region = region + c.Client.NodeClass = nodeClass + c.Client.Servers = []string{srvRPCAddr} + c.Client.Enabled = true + c.Server.Enabled = false + } +} func TestDebug_NodeClass(t *testing.T) { // Start test server and API client @@ -63,64 +76,34 @@ func TestDebug_NodeClass(t *testing.T) { srvRPCAddr := srv.GetConfig().AdvertiseAddrs.RPC t.Logf("[TEST] Leader started, srv.GetConfig().AdvertiseAddrs.RPC: %s", srvRPCAddr) - // Setup client 1 (nodeclass = clienta) - agentConfFunc1 := func(c *agent.Config) { - c.Region = "global" - c.Client.NodeClass = "clienta" - c.Client.Servers = []string{srvRPCAddr} - } - - // Start client 1 - agent1 := agent.NewTestAgent(t, "client1", agentConfFunc1) - defer agent1.Shutdown() - testutil.WaitForClient(t, srv.Agent.RPC, agent1.Agent.Client()) - - // Setup client 2 (nodeclass = clientb) - agentConfFunc2 := func(c *agent.Config) { - c.Region = "global" - c.Client.NodeClass = "clientb" - c.Client.Servers = []string{srvRPCAddr} - } - - // Start client 2 - agent2 := agent.NewTestAgent(t, "client2", agentConfFunc2) - defer agent2.Shutdown() - testutil.WaitForClient(t, srv.Agent.RPC, agent2.Agent.Client()) - - // Setup client 3 (nodeclass = clienta) - agentConfFunc3 := func(c *agent.Config) { - c.Client.NodeClass = "clienta" - c.Client.Servers = []string{srvRPCAddr} - } - - // Start client 3 - agent3 := agent.NewTestAgent(t, "client3", agentConfFunc3) - defer agent3.Shutdown() - testutil.WaitForClient(t, srv.Agent.RPC, agent3.Agent.Client()) + // Start test clients + testClient(t, "client1", newClientAgentConfigFunc("global", "classA", srvRPCAddr)) + testClient(t, "client2", newClientAgentConfigFunc("global", "classB", srvRPCAddr)) + testClient(t, "client3", newClientAgentConfigFunc("global", "classA", srvRPCAddr)) // Setup test cases cases := testCases{ { - name: "address=api, node-class=clienta, max-nodes=2", - args: []string{"-address", url, "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all", "-node-class", "clienta", "-max-nodes", "2"}, + name: "address=api, node-class=classA, max-nodes=2", + args: []string{"-address", url, "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all", "-node-class", "classA", "-max-nodes", "2"}, expectedCode: 0, expectedOutputs: []string{ "Servers: (1/1)", "Clients: (2/3)", "Max node count reached (2)", - "Node Class: clienta", + "Node Class: classA", "Created debug archive", }, expectedError: "", }, { - name: "address=api, node-class=clientb, max-nodes=2", - args: []string{"-address", url, "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all", "-node-class", "clientb", "-max-nodes", "2"}, + name: "address=api, node-class=classB, max-nodes=2", + args: []string{"-address", url, "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all", "-node-class", "classB", "-max-nodes", "2"}, expectedCode: 0, expectedOutputs: []string{ "Servers: (1/1)", "Clients: (1/3)", - "Node Class: clientb", + "Node Class: classB", "Created debug archive", }, expectedError: "", @@ -142,16 +125,8 @@ func TestDebug_ClientToServer(t *testing.T) { srvRPCAddr := srv.GetConfig().AdvertiseAddrs.RPC t.Logf("[TEST] Leader started, srv.GetConfig().AdvertiseAddrs.RPC: %s", srvRPCAddr) - // Setup client 1 (nodeclass = clienta) - agentConfFunc1 := func(c *agent.Config) { - c.Client.NodeClass = "clienta" - c.Client.Servers = []string{srvRPCAddr} - } - - // Start client 1 - agent1 := agent.NewTestAgent(t, "client1", agentConfFunc1) - defer agent1.Shutdown() - testutil.WaitForClient(t, srv.Agent.RPC, agent1.Agent.Client()) + // Start client + agent1, _, _ := testClient(t, "client1", newClientAgentConfigFunc("", "", srvRPCAddr)) // Get API addresses addrServer := srv.HTTPAddr() @@ -187,9 +162,11 @@ func TestDebug_ClientToServer(t *testing.T) { } func TestDebug_ClientToServer_Region(t *testing.T) { + region := "testregion" + // Start test server and API client srv, _, url := testServer(t, false, func(c *agent.Config) { - c.Region = "testregion" + c.Region = region }) defer srv.Shutdown() @@ -201,17 +178,8 @@ func TestDebug_ClientToServer_Region(t *testing.T) { srvRPCAddr := srv.GetConfig().AdvertiseAddrs.RPC t.Logf("[TEST] Leader started, srv.GetConfig().AdvertiseAddrs.RPC: %s", srvRPCAddr) - // Setup client 1 (nodeclass = clienta) - agentConfFunc1 := func(c *agent.Config) { - c.Region = "testregion" - c.Client.NodeClass = "clienta" - c.Client.Servers = []string{srvRPCAddr} - } - - // Start client 1 - agent1 := agent.NewTestAgent(t, "client1", agentConfFunc1) - defer agent1.Shutdown() - testutil.WaitForClient(t, srv.Agent.RPC, agent1.Agent.Client()) + // Start client + agent1, _, _ := testClient(t, "client1", newClientAgentConfigFunc(region, "", srvRPCAddr)) // Get API addresses addrServer := srv.HTTPAddr() @@ -226,7 +194,7 @@ func TestDebug_ClientToServer_Region(t *testing.T) { // Good { name: "region - testAgent api server", - args: []string{"-address", url, "-region", "testregion", "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, + args: []string{"-address", url, "-region", region, "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, expectedCode: 0, expectedOutputs: []string{ "Region: testregion\n", @@ -236,14 +204,14 @@ func TestDebug_ClientToServer_Region(t *testing.T) { }, }, { - name: "region - erver address", - args: []string{"-address", addrServer, "-region", "testregion", "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, + name: "region - server address", + args: []string{"-address", addrServer, "-region", region, "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, expectedCode: 0, expectedOutputs: []string{"Created debug archive"}, }, { name: "region - client1 address - verify no SIGSEGV panic", - args: []string{"-address", addrClient1, "-region", "testregion", "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, + args: []string{"-address", addrClient1, "-region", region, "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, expectedCode: 0, expectedOutputs: []string{"Created debug archive"}, }, diff --git a/command/util_test.go b/command/util_test.go index e1d96161448..d4e068343af 100644 --- a/command/util_test.go +++ b/command/util_test.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/command/agent" "github.com/hashicorp/nomad/helper" + "github.com/hashicorp/nomad/testutil" ) func testServer(t *testing.T, runClient bool, cb func(*agent.Config)) (*agent.TestAgent, *api.Client, string) { @@ -23,6 +24,25 @@ func testServer(t *testing.T, runClient bool, cb func(*agent.Config)) (*agent.Te return a, c, a.HTTPAddr() } +// testClient starts a new test client, blocks until it joins, and performs +// cleanup after the test is complete. +func testClient(t *testing.T, name string, cb func(*agent.Config)) (*agent.TestAgent, *api.Client, string) { + t.Logf("[TEST] Starting client agent %s", name) + // a := agent.NewTestAgent(t, name, cb) + a := agent.NewTestAgent(t, name, func(config *agent.Config) { + if cb != nil { + cb(config) + } + }) + t.Cleanup(func() { a.Shutdown() }) + + c := a.Client() + t.Logf("[TEST] Waiting for client %s to join server(s) %s", name, a.GetConfig().Client.Servers) + testutil.WaitForClient(t, a.Agent.RPC, a.Agent.Client()) + + return a, c, a.HTTPAddr() +} + func testJob(jobID string) *api.Job { task := api.NewTask("task1", "mock_driver"). SetConfig("kill_after", "1s"). From 6d3c8ecae70e94fb912047c7d991ee25bd537a05 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Mon, 11 Oct 2021 22:28:52 -0400 Subject: [PATCH 18/24] testServer already handles agent shutdown --- command/operator_debug_test.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/command/operator_debug_test.go b/command/operator_debug_test.go index f037026bfb5..6cab4cc4934 100644 --- a/command/operator_debug_test.go +++ b/command/operator_debug_test.go @@ -67,7 +67,6 @@ func newClientAgentConfigFunc(region string, nodeClass string, srvRPCAddr string func TestDebug_NodeClass(t *testing.T) { // Start test server and API client srv, _, url := testServer(t, false, nil) - defer srv.Shutdown() // Wait for leadership to establish testutil.WaitForLeader(t, srv.Agent.RPC) @@ -116,7 +115,6 @@ func TestDebug_NodeClass(t *testing.T) { func TestDebug_ClientToServer(t *testing.T) { // Start test server and API client srv, _, url := testServer(t, false, nil) - defer srv.Shutdown() // Wait for leadership to establish testutil.WaitForLeader(t, srv.Agent.RPC) @@ -169,8 +167,6 @@ func TestDebug_ClientToServer_Region(t *testing.T) { c.Region = region }) - defer srv.Shutdown() - // Wait for leadership to establish testutil.WaitForLeader(t, srv.Agent.RPC) @@ -230,7 +226,6 @@ func TestDebug_ClientToServer_Region(t *testing.T) { func TestDebug_SingleServer(t *testing.T) { srv, _, url := testServer(t, false, nil) - defer srv.Shutdown() testutil.WaitForLeader(t, srv.Agent.RPC) var cases = testCases{ @@ -263,7 +258,6 @@ func TestDebug_SingleServer(t *testing.T) { func TestDebug_Failures(t *testing.T) { srv, _, url := testServer(t, false, nil) - defer srv.Shutdown() testutil.WaitForLeader(t, srv.Agent.RPC) var cases = testCases{ @@ -311,7 +305,6 @@ func TestDebug_Failures(t *testing.T) { func TestDebug_Bad_CSIPlugin_Names(t *testing.T) { // Start test server and API client srv, _, url := testServer(t, false, nil) - defer srv.Shutdown() // Wait for leadership to establish testutil.WaitForLeader(t, srv.Agent.RPC) @@ -352,7 +345,6 @@ func TestDebug_Bad_CSIPlugin_Names(t *testing.T) { func TestDebug_CapturedFiles(t *testing.T) { srv, _, url := testServer(t, false, nil) - defer srv.Shutdown() testutil.WaitForLeader(t, srv.Agent.RPC) ui := cli.NewMockUi() @@ -431,7 +423,6 @@ func TestDebug_Fail_Pprof(t *testing.T) { // Start test server and API client srv, _, url := testServer(t, false, agentConfFunc) - defer srv.Shutdown() // Wait for leadership to establish testutil.WaitForLeader(t, srv.Agent.RPC) @@ -440,7 +431,7 @@ func TestDebug_Fail_Pprof(t *testing.T) { ui := cli.NewMockUi() cmd := &OperatorDebugCommand{Meta: Meta{Ui: ui}} - // Debug on client - node class = "clienta" + // Debug on server with endpoints disabled code := cmd.Run([]string{"-address", url, "-duration", "250ms", "-interval", "250ms", "-server-id", "all"}) assert.Equal(t, 0, code) // Pprof failure isn't fatal From 668e3bdf9bf5602d9582bbcd32d39216f12287f4 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Mon, 11 Oct 2021 22:38:38 -0400 Subject: [PATCH 19/24] Use region var for expected outputs --- command/operator_debug_test.go | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/command/operator_debug_test.go b/command/operator_debug_test.go index 6cab4cc4934..92b5fdb6076 100644 --- a/command/operator_debug_test.go +++ b/command/operator_debug_test.go @@ -193,23 +193,33 @@ func TestDebug_ClientToServer_Region(t *testing.T) { args: []string{"-address", url, "-region", region, "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, expectedCode: 0, expectedOutputs: []string{ - "Region: testregion\n", + "Region: " + region + "\n", "Servers: (1/1)", "Clients: (1/1)", "Created debug archive", }, }, { - name: "region - server address", - args: []string{"-address", addrServer, "-region", region, "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, - expectedCode: 0, - expectedOutputs: []string{"Created debug archive"}, + name: "region - server address", + args: []string{"-address", addrServer, "-region", region, "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, + expectedCode: 0, + expectedOutputs: []string{ + "Region: " + region + "\n", + "Servers: (1/1)", + "Clients: (1/1)", + "Created debug archive", + }, }, { - name: "region - client1 address - verify no SIGSEGV panic", - args: []string{"-address", addrClient1, "-region", region, "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, - expectedCode: 0, - expectedOutputs: []string{"Created debug archive"}, + name: "region - client1 address - verify no SIGSEGV panic", + args: []string{"-address", addrClient1, "-region", region, "-duration", "250ms", "-interval", "250ms", "-server-id", "all", "-node-id", "all"}, + expectedCode: 0, + expectedOutputs: []string{ + "Region: " + region + "\n", + "Servers: (1/1)", + "Clients: (1/1)", + "Created debug archive", + }, }, // Bad From 48ff7161f2ac47106d55eef2f40798397b9f3d35 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Mon, 11 Oct 2021 22:48:36 -0400 Subject: [PATCH 20/24] Add test for SliceStringContainsPrefix --- helper/funcs_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/helper/funcs_test.go b/helper/funcs_test.go index d6ec74b91ee..40e3f6f7615 100644 --- a/helper/funcs_test.go +++ b/helper/funcs_test.go @@ -34,6 +34,16 @@ func TestSliceStringContains(t *testing.T) { require.False(t, SliceStringContains(list, "d")) } +func TestSliceStringContainsPrefix(t *testing.T) { + list := []string{"abc", "def", "abcdef", "definitely", "most definitely"} + require.True(t, SliceStringContainsPrefix(list, "a")) + require.False(t, SliceStringContainsPrefix(list, "b")) + require.False(t, SliceStringContainsPrefix(list, "c")) + require.True(t, SliceStringContainsPrefix(list, "d")) + require.True(t, SliceStringContainsPrefix(list, "mos")) + require.True(t, SliceStringContainsPrefix(list, "def")) +} + func TestCompareTimePtrs(t *testing.T) { t.Run("nil", func(t *testing.T) { a := (*time.Duration)(nil) From 4c9f6557bf499034fe3130eef516d2ffc29dab44 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Mon, 11 Oct 2021 23:39:23 -0400 Subject: [PATCH 21/24] Fix logic/tests for slice prefix helper functions --- command/operator_debug.go | 4 ++-- helper/funcs.go | 14 ++++++++++++-- helper/funcs_test.go | 25 ++++++++++++++++++------- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/command/operator_debug.go b/command/operator_debug.go index a2570581bb3..97385bf3ad0 100644 --- a/command/operator_debug.go +++ b/command/operator_debug.go @@ -1092,8 +1092,8 @@ func filterServerMembers(serverMembers *api.ServerMembers, serverIDs string, reg continue } - // Include member if name matches any prefix - if helper.SliceStringContainsPrefix(prefixes, member.Name) { + // Include member if name matches any prefix from serverIDs + if helper.StringHasPrefixInSlice(member.Name, prefixes) { membersFound = append(membersFound, member.Name) } } diff --git a/helper/funcs.go b/helper/funcs.go index 63ca376b7de..f5526681927 100644 --- a/helper/funcs.go +++ b/helper/funcs.go @@ -197,8 +197,8 @@ func SliceStringContains(list []string, item string) bool { return false } -// SliceStringContainsPrefix returns true if any string in list matches prefix -func SliceStringContainsPrefix(list []string, prefix string) bool { +// SliceStringHasPrefix returns true if any string in list starts with prefix +func SliceStringHasPrefix(list []string, prefix string) bool { for _, s := range list { if strings.HasPrefix(s, prefix) { return true @@ -207,6 +207,16 @@ func SliceStringContainsPrefix(list []string, prefix string) bool { return false } +// StringHasPrefixInSlice returns true if string starts with any prefix in list +func StringHasPrefixInSlice(s string, prefixes []string) bool { + for _, prefix := range prefixes { + if strings.HasPrefix(s, prefix) { + return true + } + } + return false +} + func SliceSetDisjoint(first, second []string) (bool, []string) { contained := make(map[string]struct{}, len(first)) for _, k := range first { diff --git a/helper/funcs_test.go b/helper/funcs_test.go index 40e3f6f7615..7845d3d4c32 100644 --- a/helper/funcs_test.go +++ b/helper/funcs_test.go @@ -34,14 +34,25 @@ func TestSliceStringContains(t *testing.T) { require.False(t, SliceStringContains(list, "d")) } -func TestSliceStringContainsPrefix(t *testing.T) { +func TestSliceStringHasPrefix(t *testing.T) { list := []string{"abc", "def", "abcdef", "definitely", "most definitely"} - require.True(t, SliceStringContainsPrefix(list, "a")) - require.False(t, SliceStringContainsPrefix(list, "b")) - require.False(t, SliceStringContainsPrefix(list, "c")) - require.True(t, SliceStringContainsPrefix(list, "d")) - require.True(t, SliceStringContainsPrefix(list, "mos")) - require.True(t, SliceStringContainsPrefix(list, "def")) + require.True(t, SliceStringHasPrefix(list, "a")) + require.False(t, SliceStringHasPrefix(list, "b")) + require.False(t, SliceStringHasPrefix(list, "c")) + require.True(t, SliceStringHasPrefix(list, "d")) + require.True(t, SliceStringHasPrefix(list, "mos")) + require.True(t, SliceStringHasPrefix(list, "def")) +} + +func TestStringHasPrefixInSlice(t *testing.T) { + prefixes := []string{"a", "b", "c", "definitely", "most definitely"} + require.True(t, StringHasPrefixInSlice("alpha", prefixes)) + require.True(t, StringHasPrefixInSlice("bravo", prefixes)) + require.True(t, StringHasPrefixInSlice("charlie", prefixes)) + require.False(t, StringHasPrefixInSlice("delta", prefixes)) + require.True(t, StringHasPrefixInSlice("definitely", prefixes)) + require.False(t, StringHasPrefixInSlice("most", prefixes)) + require.True(t, StringHasPrefixInSlice("most definitely", prefixes)) } func TestCompareTimePtrs(t *testing.T) { From 037e801799335bf2268dbce9713a9a747632ceed Mon Sep 17 00:00:00 2001 From: davemay99 Date: Tue, 12 Oct 2021 01:31:40 -0400 Subject: [PATCH 22/24] revert testutil.WaitForClient addition --- command/util_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/command/util_test.go b/command/util_test.go index d4e068343af..59f69465dcd 100644 --- a/command/util_test.go +++ b/command/util_test.go @@ -6,7 +6,7 @@ import ( "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/command/agent" "github.com/hashicorp/nomad/helper" - "github.com/hashicorp/nomad/testutil" + // "github.com/hashicorp/nomad/testutil" ) func testServer(t *testing.T, runClient bool, cb func(*agent.Config)) (*agent.TestAgent, *api.Client, string) { @@ -28,7 +28,6 @@ func testServer(t *testing.T, runClient bool, cb func(*agent.Config)) (*agent.Te // cleanup after the test is complete. func testClient(t *testing.T, name string, cb func(*agent.Config)) (*agent.TestAgent, *api.Client, string) { t.Logf("[TEST] Starting client agent %s", name) - // a := agent.NewTestAgent(t, name, cb) a := agent.NewTestAgent(t, name, func(config *agent.Config) { if cb != nil { cb(config) @@ -37,8 +36,8 @@ func testClient(t *testing.T, name string, cb func(*agent.Config)) (*agent.TestA t.Cleanup(func() { a.Shutdown() }) c := a.Client() - t.Logf("[TEST] Waiting for client %s to join server(s) %s", name, a.GetConfig().Client.Servers) - testutil.WaitForClient(t, a.Agent.RPC, a.Agent.Client()) + // t.Logf("[TEST] Waiting for client %s to join server(s) %s", name, a.GetConfig().Client.Servers) + // testutil.WaitForClient(t, a.Agent.RPC, a.Agent.Client()) return a, c, a.HTTPAddr() } From 37e5e226ee0294d4a3ec0021e926e0e313557864 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Tue, 12 Oct 2021 02:22:09 -0400 Subject: [PATCH 23/24] eliminate import cycle caused by nomad/client --- command/util_test.go | 6 +++--- nomad/client_agent_endpoint_test.go | 2 +- testutil/wait.go | 9 ++++----- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/command/util_test.go b/command/util_test.go index 59f69465dcd..d1fd96a452a 100644 --- a/command/util_test.go +++ b/command/util_test.go @@ -6,7 +6,7 @@ import ( "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/command/agent" "github.com/hashicorp/nomad/helper" - // "github.com/hashicorp/nomad/testutil" + "github.com/hashicorp/nomad/testutil" ) func testServer(t *testing.T, runClient bool, cb func(*agent.Config)) (*agent.TestAgent, *api.Client, string) { @@ -36,8 +36,8 @@ func testClient(t *testing.T, name string, cb func(*agent.Config)) (*agent.TestA t.Cleanup(func() { a.Shutdown() }) c := a.Client() - // t.Logf("[TEST] Waiting for client %s to join server(s) %s", name, a.GetConfig().Client.Servers) - // testutil.WaitForClient(t, a.Agent.RPC, a.Agent.Client()) + t.Logf("[TEST] Waiting for client %s to join server(s) %s", name, a.GetConfig().Client.Servers) + testutil.WaitForClient(t, a.Agent.RPC, a.Agent.Client().NodeID(), a.Agent.Client().Region()) return a, c, a.HTTPAddr() } diff --git a/nomad/client_agent_endpoint_test.go b/nomad/client_agent_endpoint_test.go index 70a01ec3b0c..e3c11ee8370 100644 --- a/nomad/client_agent_endpoint_test.go +++ b/nomad/client_agent_endpoint_test.go @@ -548,7 +548,7 @@ func TestAgentProfile_RemoteClient(t *testing.T) { }) defer cleanupC() - testutil.WaitForClient(t, s2.RPC, c) + testutil.WaitForClient(t, s2.RPC, c.NodeID(), c.Region()) testutil.WaitForResult(func() (bool, error) { nodes := s2.connectedNodes() return len(nodes) == 1, nil diff --git a/testutil/wait.go b/testutil/wait.go index 11d05d129e3..fee5b6b0592 100644 --- a/testutil/wait.go +++ b/testutil/wait.go @@ -6,7 +6,6 @@ import ( "testing" "time" - "github.com/hashicorp/nomad/client" "github.com/hashicorp/nomad/nomad/structs" "github.com/kr/pretty" "github.com/stretchr/testify/require" @@ -113,16 +112,16 @@ func WaitForLeader(t testing.TB, rpc rpcFn) { } // WaitForClient blocks until the client can be found -func WaitForClient(t testing.TB, rpc rpcFn, client *client.Client) { +func WaitForClient(t testing.TB, rpc rpcFn, nodeID string, region string) { + t.Helper() - region := client.Region() if region == "" { region = "global" } WaitForResult(func() (bool, error) { req := structs.NodeSpecificRequest{ - NodeID: client.NodeID(), + NodeID: nodeID, QueryOptions: structs.QueryOptions{Region: region}, } var out structs.SingleNodeResponse @@ -139,7 +138,7 @@ func WaitForClient(t testing.TB, rpc rpcFn, client *client.Client) { t.Fatalf("failed to find node: %v", err) }) - t.Logf("[TEST] Client %s ready, id: %s, region: %s", client.GetConfig().Node.Name, client.NodeID(), region) + t.Logf("[TEST] Client for test %s ready, id: %s, region: %s", t.Name(), nodeID, region) } // WaitForVotingMembers blocks until autopilot promotes all server peers From 4059695ca78d3b508a9464257e3dcf00c011d098 Mon Sep 17 00:00:00 2001 From: davemay99 Date: Tue, 12 Oct 2021 16:24:59 -0400 Subject: [PATCH 24/24] Clarify slice HasPrefix tests --- helper/funcs_test.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/helper/funcs_test.go b/helper/funcs_test.go index 7845d3d4c32..265e3f479f9 100644 --- a/helper/funcs_test.go +++ b/helper/funcs_test.go @@ -35,24 +35,31 @@ func TestSliceStringContains(t *testing.T) { } func TestSliceStringHasPrefix(t *testing.T) { - list := []string{"abc", "def", "abcdef", "definitely", "most definitely"} + list := []string{"alpha", "bravo", "charlie", "definitely", "most definitely"} + // At least one string in the slice above starts with the following test prefix strings require.True(t, SliceStringHasPrefix(list, "a")) - require.False(t, SliceStringHasPrefix(list, "b")) - require.False(t, SliceStringHasPrefix(list, "c")) + require.True(t, SliceStringHasPrefix(list, "b")) + require.True(t, SliceStringHasPrefix(list, "c")) require.True(t, SliceStringHasPrefix(list, "d")) require.True(t, SliceStringHasPrefix(list, "mos")) require.True(t, SliceStringHasPrefix(list, "def")) + require.False(t, SliceStringHasPrefix(list, "delta")) + } func TestStringHasPrefixInSlice(t *testing.T) { prefixes := []string{"a", "b", "c", "definitely", "most definitely"} + // The following strings all start with at least one prefix in the slice above require.True(t, StringHasPrefixInSlice("alpha", prefixes)) require.True(t, StringHasPrefixInSlice("bravo", prefixes)) require.True(t, StringHasPrefixInSlice("charlie", prefixes)) - require.False(t, StringHasPrefixInSlice("delta", prefixes)) require.True(t, StringHasPrefixInSlice("definitely", prefixes)) - require.False(t, StringHasPrefixInSlice("most", prefixes)) require.True(t, StringHasPrefixInSlice("most definitely", prefixes)) + + require.False(t, StringHasPrefixInSlice("mos", prefixes)) + require.False(t, StringHasPrefixInSlice("def", prefixes)) + require.False(t, StringHasPrefixInSlice("delta", prefixes)) + } func TestCompareTimePtrs(t *testing.T) {