diff --git a/client/client.go b/client/client.go index 7adefd1e8..771004aad 100644 --- a/client/client.go +++ b/client/client.go @@ -592,25 +592,37 @@ func (c *Client) GetServers() []string { // SetServers sets a new list of nomad servers to connect to. As long as one // server is resolvable no error is returned. func (c *Client) SetServers(in []string) error { - endpoints := make([]*servers.Server, 0, len(in)) + var mu sync.Mutex + var wg sync.WaitGroup var merr multierror.Error + + endpoints := make([]*servers.Server, 0, len(in)) + wg.Add(len(in)) + for _, s := range in { - addr, err := resolveServer(s) - if err != nil { - c.logger.Printf("[DEBUG] client: ignoring server %s due to resolution error: %v", s, err) - merr.Errors = append(merr.Errors, err) - continue - } + go func(srv string) { + defer wg.Done() + addr, err := resolveServer(srv) + if err != nil { + c.logger.Printf("[DEBUG] client: ignoring server %s due to resolution error: %v", srv, err) + merr.Errors = append(merr.Errors, err) + return + } - // Try to ping to check if it is a real server - if err := c.Ping(addr); err != nil { - merr.Errors = append(merr.Errors, fmt.Errorf("Server at address %s failed ping: %v", addr, err)) - continue - } + // Try to ping to check if it is a real server + if err := c.Ping(addr); err != nil { + merr.Errors = append(merr.Errors, fmt.Errorf("Server at address %s failed ping: %v", addr, err)) + return + } - endpoints = append(endpoints, &servers.Server{Addr: addr}) + mu.Lock() + endpoints = append(endpoints, &servers.Server{Addr: addr}) + mu.Unlock() + }(s) } + wg.Wait() + // Only return errors if no servers are valid if len(endpoints) == 0 { if len(merr.Errors) > 0 { diff --git a/client/client_test.go b/client/client_test.go index aab61d3ca..a612ba71b 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -6,7 +6,6 @@ import ( "log" "os" "path/filepath" - "strings" "testing" "time" @@ -955,16 +954,11 @@ func TestClient_ServerList(t *testing.T) { if err := client.SetServers([]string{"123.456.13123.123.13:80"}); err == nil { t.Fatalf("expected setting a bad server to return an error") } - if err := client.SetServers([]string{"123.456.13123.123.13:80", "127.0.0.1:1234", "127.0.0.1"}); err != nil { + if err := client.SetServers([]string{"123.456.13123.123.13:80", "127.0.0.1:1234", "127.0.0.1"}); err == nil { t.Fatalf("expected setting at least one good server to succeed but received: %v", err) } s := client.GetServers() - if len(s) != 2 { + if len(s) != 0 { t.Fatalf("expected 2 servers but received: %+q", s) } - for _, host := range s { - if !strings.HasPrefix(host, "127.0.0.1:") { - t.Errorf("expected both servers to be localhost and include port but found: %s", host) - } - } } diff --git a/client/servers/manager.go b/client/servers/manager.go index dbd9f0426..604b109a2 100644 --- a/client/servers/manager.go +++ b/client/servers/manager.go @@ -53,6 +53,9 @@ type Server struct { } func (s *Server) Copy() *Server { + s.Lock() + defer s.Unlock() + return &Server{ Addr: s.Addr, addr: s.addr, @@ -243,11 +246,9 @@ func (m *Manager) GetServers() Servers { // to contact each server. If a server successfully responds it is used, otherwise // it is rotated such that it will be the last attempted server. func (m *Manager) RebalanceServers() { - m.Lock() - defer m.Unlock() - // Shuffle servers so we have a chance of picking a new one. - m.servers.shuffle() + servers := m.GetServers() + servers.shuffle() // Iterate through the shuffled server list to find an assumed // healthy server. NOTE: Do not iterate on the list directly because @@ -256,7 +257,7 @@ func (m *Manager) RebalanceServers() { for i := 0; i < len(m.servers); i++ { // Always test the first server. Failed servers are cycled // while Serf detects the node has failed. - srv := m.servers[0] + srv := servers[0] err := m.connPoolPinger.Ping(srv.Addr) if err == nil { @@ -265,14 +266,18 @@ func (m *Manager) RebalanceServers() { } m.logger.Printf(`[DEBUG] manager: pinging server "%s" failed: %s`, srv, err) - m.servers.cycle() + servers.cycle() } if !foundHealthyServer { m.logger.Printf("[DEBUG] manager: No healthy servers during rebalance") + return } - return + // Save the servers + m.Lock() + m.servers = servers + m.Unlock() } // refreshServerRebalanceTimer is only called once m.rebalanceTimer expires. diff --git a/client/stats/host.go b/client/stats/host.go index 23826b10a..1da2b4641 100644 --- a/client/stats/host.go +++ b/client/stats/host.go @@ -1,6 +1,7 @@ package stats import ( + "fmt" "log" "math" "runtime" @@ -133,7 +134,7 @@ func (h *HostStatsCollector) collectLocked() error { // Getting the disk stats for the allocation directory usage, err := disk.Usage(h.allocDir) if err != nil { - return err + return fmt.Errorf("failed to find disk usage of alloc_dir %q: %v", h.allocDir, err) } hs.AllocDirStats = h.toDiskStats(usage, nil) diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index 46748bb93..85b8faced 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -6,12 +6,14 @@ import ( "fmt" "net/http" "net/http/httptest" + "net/url" "testing" "github.com/hashicorp/nomad/acl" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" - "github.com/stretchr/testify/assert" + "github.com/hashicorp/nomad/testutil" + "github.com/stretchr/testify/require" ) func TestHTTP_AgentSelf(t *testing.T) { @@ -44,7 +46,7 @@ func TestHTTP_AgentSelf(t *testing.T) { t.Fatalf("bad: %#v", self) } - // Assign a Vault token and assert it is redacted. + // Assign a Vault token and require it is redacted. s.Config.Vault.Token = "badc0deb-adc0-deba-dc0d-ebadc0debadc" respW = httptest.NewRecorder() obj, err = s.Server.AgentSelfRequest(respW, req) @@ -60,21 +62,21 @@ func TestHTTP_AgentSelf(t *testing.T) { func TestHTTP_AgentSelf_ACL(t *testing.T) { t.Parallel() - assert := assert.New(t) + require := require.New(t) httpACLTest(t, nil, func(s *TestAgent) { state := s.Agent.server.State() // Make the HTTP request req, err := http.NewRequest("GET", "/v1/agent/self", nil) - assert.Nil(err) + require.Nil(err) // Try request without a token and expect failure { respW := httptest.NewRecorder() _, err := s.Server.AgentSelfRequest(respW, req) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + require.NotNil(err) + require.Equal(err.Error(), structs.ErrPermissionDenied.Error()) } // Try request with an invalid token and expect failure @@ -83,8 +85,8 @@ func TestHTTP_AgentSelf_ACL(t *testing.T) { token := mock.CreatePolicyAndToken(t, state, 1005, "invalid", mock.NodePolicy(acl.PolicyWrite)) setToken(req, token) _, err := s.Server.AgentSelfRequest(respW, req) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + require.NotNil(err) + require.Equal(err.Error(), structs.ErrPermissionDenied.Error()) } // Try request with a valid token @@ -93,11 +95,11 @@ func TestHTTP_AgentSelf_ACL(t *testing.T) { token := mock.CreatePolicyAndToken(t, state, 1007, "valid", mock.AgentPolicy(acl.PolicyWrite)) setToken(req, token) obj, err := s.Server.AgentSelfRequest(respW, req) - assert.Nil(err) + require.Nil(err) self := obj.(agentSelf) - assert.NotNil(self.Config) - assert.NotNil(self.Stats) + require.NotNil(self.Config) + require.NotNil(self.Stats) } // Try request with a root token @@ -105,18 +107,17 @@ func TestHTTP_AgentSelf_ACL(t *testing.T) { respW := httptest.NewRecorder() setToken(req, s.RootToken) obj, err := s.Server.AgentSelfRequest(respW, req) - assert.Nil(err) + require.Nil(err) self := obj.(agentSelf) - assert.NotNil(self.Config) - assert.NotNil(self.Stats) + require.NotNil(self.Config) + require.NotNil(self.Stats) } }) } func TestHTTP_AgentJoin(t *testing.T) { - // TODO(alexdadgar) - // t.Parallel() + t.Parallel() httpTest(t, nil, func(s *TestAgent) { // Determine the join address member := s.Agent.Server().LocalMember() @@ -173,21 +174,21 @@ func TestHTTP_AgentMembers(t *testing.T) { func TestHTTP_AgentMembers_ACL(t *testing.T) { t.Parallel() - assert := assert.New(t) + require := require.New(t) httpACLTest(t, nil, func(s *TestAgent) { state := s.Agent.server.State() // Make the HTTP request req, err := http.NewRequest("GET", "/v1/agent/members", nil) - assert.Nil(err) + require.Nil(err) // Try request without a token and expect failure { respW := httptest.NewRecorder() _, err := s.Server.AgentMembersRequest(respW, req) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + require.NotNil(err) + require.Equal(err.Error(), structs.ErrPermissionDenied.Error()) } // Try request with an invalid token and expect failure @@ -196,8 +197,8 @@ func TestHTTP_AgentMembers_ACL(t *testing.T) { token := mock.CreatePolicyAndToken(t, state, 1005, "invalid", mock.AgentPolicy(acl.PolicyWrite)) setToken(req, token) _, err := s.Server.AgentMembersRequest(respW, req) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + require.NotNil(err) + require.Equal(err.Error(), structs.ErrPermissionDenied.Error()) } // Try request with a valid token @@ -206,10 +207,10 @@ func TestHTTP_AgentMembers_ACL(t *testing.T) { token := mock.CreatePolicyAndToken(t, state, 1007, "valid", mock.NodePolicy(acl.PolicyRead)) setToken(req, token) obj, err := s.Server.AgentMembersRequest(respW, req) - assert.Nil(err) + require.Nil(err) members := obj.(structs.ServerMembersResponse) - assert.Len(members.Members, 1) + require.Len(members.Members, 1) } // Try request with a root token @@ -217,10 +218,10 @@ func TestHTTP_AgentMembers_ACL(t *testing.T) { respW := httptest.NewRecorder() setToken(req, s.RootToken) obj, err := s.Server.AgentMembersRequest(respW, req) - assert.Nil(err) + require.Nil(err) members := obj.(structs.ServerMembersResponse) - assert.Len(members.Members, 1) + require.Len(members.Members, 1) } }) } @@ -245,21 +246,21 @@ func TestHTTP_AgentForceLeave(t *testing.T) { func TestHTTP_AgentForceLeave_ACL(t *testing.T) { t.Parallel() - assert := assert.New(t) + require := require.New(t) httpACLTest(t, nil, func(s *TestAgent) { state := s.Agent.server.State() // Make the HTTP request req, err := http.NewRequest("PUT", "/v1/agent/force-leave?node=foo", nil) - assert.Nil(err) + require.Nil(err) // Try request without a token and expect failure { respW := httptest.NewRecorder() _, err := s.Server.AgentForceLeaveRequest(respW, req) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + require.NotNil(err) + require.Equal(err.Error(), structs.ErrPermissionDenied.Error()) } // Try request with an invalid token and expect failure @@ -268,8 +269,8 @@ func TestHTTP_AgentForceLeave_ACL(t *testing.T) { token := mock.CreatePolicyAndToken(t, state, 1005, "invalid", mock.NodePolicy(acl.PolicyRead)) setToken(req, token) _, err := s.Server.AgentForceLeaveRequest(respW, req) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + require.NotNil(err) + require.Equal(err.Error(), structs.ErrPermissionDenied.Error()) } // Try request with a valid token @@ -278,8 +279,8 @@ func TestHTTP_AgentForceLeave_ACL(t *testing.T) { token := mock.CreatePolicyAndToken(t, state, 1007, "valid", mock.AgentPolicy(acl.PolicyWrite)) setToken(req, token) _, err := s.Server.AgentForceLeaveRequest(respW, req) - assert.Nil(err) - assert.Equal(http.StatusOK, respW.Code) + require.Nil(err) + require.Equal(http.StatusOK, respW.Code) } // Try request with a root token @@ -287,71 +288,70 @@ func TestHTTP_AgentForceLeave_ACL(t *testing.T) { respW := httptest.NewRecorder() setToken(req, s.RootToken) _, err := s.Server.AgentForceLeaveRequest(respW, req) - assert.Nil(err) - assert.Equal(http.StatusOK, respW.Code) + require.Nil(err) + require.Equal(http.StatusOK, respW.Code) } }) } func TestHTTP_AgentSetServers(t *testing.T) { t.Parallel() - assert := assert.New(t) + require := require.New(t) httpTest(t, nil, func(s *TestAgent) { // Create the request req, err := http.NewRequest("PUT", "/v1/agent/servers", nil) - assert.Nil(err) + require.Nil(err) // Send the request respW := httptest.NewRecorder() _, err = s.Server.AgentServersRequest(respW, req) - assert.NotNil(err) - assert.Contains(err.Error(), "missing server address") + require.NotNil(err) + require.Contains(err.Error(), "missing server address") // Create a valid request req, err = http.NewRequest("PUT", "/v1/agent/servers?address=127.0.0.1%3A4647&address=127.0.0.2%3A4647&address=127.0.0.3%3A4647", nil) - assert.Nil(err) + require.Nil(err) - // Send the request + // Send the request which should fail respW = httptest.NewRecorder() _, err = s.Server.AgentServersRequest(respW, req) - assert.Nil(err) + require.NotNil(err) // Retrieve the servers again req, err = http.NewRequest("GET", "/v1/agent/servers", nil) - assert.Nil(err) + require.Nil(err) respW = httptest.NewRecorder() // Make the request and check the result expected := []string{ - "127.0.0.1:4647", - "127.0.0.2:4647", - "127.0.0.3:4647", + s.GetConfig().AdvertiseAddrs.RPC, } out, err := s.Server.AgentServersRequest(respW, req) - assert.Nil(err) + require.Nil(err) servers := out.([]string) - assert.Len(servers, len(expected)) - assert.Equal(expected, servers) + require.Len(servers, len(expected)) + require.Equal(expected, servers) }) } func TestHTTP_AgentSetServers_ACL(t *testing.T) { t.Parallel() - assert := assert.New(t) + require := require.New(t) httpACLTest(t, nil, func(s *TestAgent) { state := s.Agent.server.State() // Make the HTTP request - req, err := http.NewRequest("PUT", "/v1/agent/servers?address=127.0.0.1%3A4647&address=127.0.0.2%3A4647&address=127.0.0.3%3A4647", nil) - assert.Nil(err) + path := fmt.Sprintf("/v1/agent/servers?address=%s", url.QueryEscape(s.GetConfig().AdvertiseAddrs.RPC)) + req, err := http.NewRequest("PUT", path, nil) + require.Nil(err) // Try request without a token and expect failure { respW := httptest.NewRecorder() _, err := s.Server.AgentServersRequest(respW, req) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + require.NotNil(err) + require.Equal(err.Error(), structs.ErrPermissionDenied.Error()) } // Try request with an invalid token and expect failure @@ -360,8 +360,8 @@ func TestHTTP_AgentSetServers_ACL(t *testing.T) { token := mock.CreatePolicyAndToken(t, state, 1005, "invalid", mock.NodePolicy(acl.PolicyRead)) setToken(req, token) _, err := s.Server.AgentServersRequest(respW, req) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + require.NotNil(err) + require.Equal(err.Error(), structs.ErrPermissionDenied.Error()) } // Try request with a valid token @@ -370,8 +370,8 @@ func TestHTTP_AgentSetServers_ACL(t *testing.T) { token := mock.CreatePolicyAndToken(t, state, 1007, "valid", mock.AgentPolicy(acl.PolicyWrite)) setToken(req, token) _, err := s.Server.AgentServersRequest(respW, req) - assert.Nil(err) - assert.Equal(http.StatusOK, respW.Code) + require.Nil(err) + require.Equal(http.StatusOK, respW.Code) } // Try request with a root token @@ -379,47 +379,33 @@ func TestHTTP_AgentSetServers_ACL(t *testing.T) { respW := httptest.NewRecorder() setToken(req, s.RootToken) _, err := s.Server.AgentServersRequest(respW, req) - assert.Nil(err) - assert.Equal(http.StatusOK, respW.Code) + require.Nil(err) + require.Equal(http.StatusOK, respW.Code) } }) } func TestHTTP_AgentListServers_ACL(t *testing.T) { t.Parallel() - assert := assert.New(t) + require := require.New(t) httpACLTest(t, nil, func(s *TestAgent) { state := s.Agent.server.State() - // Set some servers - { - req, err := http.NewRequest("PUT", "/v1/agent/servers?address=127.0.0.1%3A4647&address=127.0.0.2%3A4647&address=127.0.0.3%3A4647", nil) - assert.Nil(err) - - respW := httptest.NewRecorder() - setToken(req, s.RootToken) - _, err = s.Server.AgentServersRequest(respW, req) - assert.Nil(err) - assert.Equal(http.StatusOK, respW.Code) - } - // Create list request req, err := http.NewRequest("GET", "/v1/agent/servers", nil) - assert.Nil(err) + require.Nil(err) expected := []string{ - "127.0.0.1:4647", - "127.0.0.2:4647", - "127.0.0.3:4647", + s.GetConfig().AdvertiseAddrs.RPC, } // Try request without a token and expect failure { respW := httptest.NewRecorder() _, err := s.Server.AgentServersRequest(respW, req) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + require.NotNil(err) + require.Equal(err.Error(), structs.ErrPermissionDenied.Error()) } // Try request with an invalid token and expect failure @@ -428,20 +414,27 @@ func TestHTTP_AgentListServers_ACL(t *testing.T) { token := mock.CreatePolicyAndToken(t, state, 1005, "invalid", mock.NodePolicy(acl.PolicyRead)) setToken(req, token) _, err := s.Server.AgentServersRequest(respW, req) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + require.NotNil(err) + require.Equal(err.Error(), structs.ErrPermissionDenied.Error()) } + // Wait for client to have a server + testutil.WaitForResult(func() (bool, error) { + return len(s.client.GetServers()) != 0, fmt.Errorf("no servers") + }, func(err error) { + t.Fatal(err) + }) + // Try request with a valid token { respW := httptest.NewRecorder() token := mock.CreatePolicyAndToken(t, state, 1007, "valid", mock.AgentPolicy(acl.PolicyRead)) setToken(req, token) out, err := s.Server.AgentServersRequest(respW, req) - assert.Nil(err) + require.Nil(err) servers := out.([]string) - assert.Len(servers, len(expected)) - assert.Equal(expected, servers) + require.Len(servers, len(expected)) + require.Equal(expected, servers) } // Try request with a root token @@ -449,10 +442,10 @@ func TestHTTP_AgentListServers_ACL(t *testing.T) { respW := httptest.NewRecorder() setToken(req, s.RootToken) out, err := s.Server.AgentServersRequest(respW, req) - assert.Nil(err) + require.Nil(err) servers := out.([]string) - assert.Len(servers, len(expected)) - assert.Equal(expected, servers) + require.Len(servers, len(expected)) + require.Equal(expected, servers) } }) } @@ -472,19 +465,15 @@ func TestHTTP_AgentListKeys(t *testing.T) { respW := httptest.NewRecorder() out, err := s.Server.KeyringOperationRequest(respW, req) - if err != nil { - t.Fatalf("err: %s", err) - } + require.Nil(t, err) kresp := out.(structs.KeyringResponse) - if len(kresp.Keys) != 1 { - t.Fatalf("bad: %v", kresp) - } + require.Len(t, kresp.Keys, 1) }) } func TestHTTP_AgentListKeys_ACL(t *testing.T) { t.Parallel() - assert := assert.New(t) + require := require.New(t) key1 := "HS5lJ+XuTlYKWaeGYyG+/A==" @@ -497,14 +486,14 @@ func TestHTTP_AgentListKeys_ACL(t *testing.T) { // Make the HTTP request req, err := http.NewRequest("GET", "/v1/agent/keyring/list", nil) - assert.Nil(err) + require.Nil(err) // Try request without a token and expect failure { respW := httptest.NewRecorder() _, err := s.Server.KeyringOperationRequest(respW, req) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + require.NotNil(err) + require.Equal(err.Error(), structs.ErrPermissionDenied.Error()) } // Try request with an invalid token and expect failure @@ -513,8 +502,8 @@ func TestHTTP_AgentListKeys_ACL(t *testing.T) { token := mock.CreatePolicyAndToken(t, state, 1005, "invalid", mock.AgentPolicy(acl.PolicyRead)) setToken(req, token) _, err := s.Server.KeyringOperationRequest(respW, req) - assert.NotNil(err) - assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) + require.NotNil(err) + require.Equal(err.Error(), structs.ErrPermissionDenied.Error()) } // Try request with a valid token @@ -523,10 +512,10 @@ func TestHTTP_AgentListKeys_ACL(t *testing.T) { token := mock.CreatePolicyAndToken(t, state, 1007, "valid", mock.AgentPolicy(acl.PolicyWrite)) setToken(req, token) out, err := s.Server.KeyringOperationRequest(respW, req) - assert.Nil(err) + require.Nil(err) kresp := out.(structs.KeyringResponse) - assert.Len(kresp.Keys, 1) - assert.Contains(kresp.Keys, key1) + require.Len(kresp.Keys, 1) + require.Contains(kresp.Keys, key1) } // Try request with a root token @@ -534,17 +523,16 @@ func TestHTTP_AgentListKeys_ACL(t *testing.T) { respW := httptest.NewRecorder() setToken(req, s.RootToken) out, err := s.Server.KeyringOperationRequest(respW, req) - assert.Nil(err) + require.Nil(err) kresp := out.(structs.KeyringResponse) - assert.Len(kresp.Keys, 1) - assert.Contains(kresp.Keys, key1) + require.Len(kresp.Keys, 1) + require.Contains(kresp.Keys, key1) } }) } func TestHTTP_AgentInstallKey(t *testing.T) { - // TODO(alexdadgar) - // t.Parallel() + t.Parallel() key1 := "HS5lJ+XuTlYKWaeGYyG+/A==" key2 := "wH1Bn9hlJ0emgWB1JttVRA==" @@ -584,8 +572,7 @@ func TestHTTP_AgentInstallKey(t *testing.T) { } func TestHTTP_AgentRemoveKey(t *testing.T) { - // TODO(alexdadgar) - // t.Parallel() + t.Parallel() key1 := "HS5lJ+XuTlYKWaeGYyG+/A==" key2 := "wH1Bn9hlJ0emgWB1JttVRA==" @@ -635,87 +622,87 @@ func TestHTTP_AgentRemoveKey(t *testing.T) { func TestHTTP_AgentHealth_Ok(t *testing.T) { t.Parallel() - assert := assert.New(t) + require := require.New(t) // Enable ACLs to ensure they're not enforced httpACLTest(t, nil, func(s *TestAgent) { // No ?type= { req, err := http.NewRequest("GET", "/v1/agent/health", nil) - assert.Nil(err) + require.Nil(err) respW := httptest.NewRecorder() healthI, err := s.Server.HealthRequest(respW, req) - assert.Nil(err) - assert.Equal(http.StatusOK, respW.Code) - assert.NotNil(healthI) + require.Nil(err) + require.Equal(http.StatusOK, respW.Code) + require.NotNil(healthI) health := healthI.(*healthResponse) - assert.NotNil(health.Client) - assert.True(health.Client.Ok) - assert.Equal("ok", health.Client.Message) - assert.NotNil(health.Server) - assert.True(health.Server.Ok) - assert.Equal("ok", health.Server.Message) + require.NotNil(health.Client) + require.True(health.Client.Ok) + require.Equal("ok", health.Client.Message) + require.NotNil(health.Server) + require.True(health.Server.Ok) + require.Equal("ok", health.Server.Message) } // type=client { req, err := http.NewRequest("GET", "/v1/agent/health?type=client", nil) - assert.Nil(err) + require.Nil(err) respW := httptest.NewRecorder() healthI, err := s.Server.HealthRequest(respW, req) - assert.Nil(err) - assert.Equal(http.StatusOK, respW.Code) - assert.NotNil(healthI) + require.Nil(err) + require.Equal(http.StatusOK, respW.Code) + require.NotNil(healthI) health := healthI.(*healthResponse) - assert.NotNil(health.Client) - assert.True(health.Client.Ok) - assert.Equal("ok", health.Client.Message) - assert.Nil(health.Server) + require.NotNil(health.Client) + require.True(health.Client.Ok) + require.Equal("ok", health.Client.Message) + require.Nil(health.Server) } // type=server { req, err := http.NewRequest("GET", "/v1/agent/health?type=server", nil) - assert.Nil(err) + require.Nil(err) respW := httptest.NewRecorder() healthI, err := s.Server.HealthRequest(respW, req) - assert.Nil(err) - assert.Equal(http.StatusOK, respW.Code) - assert.NotNil(healthI) + require.Nil(err) + require.Equal(http.StatusOK, respW.Code) + require.NotNil(healthI) health := healthI.(*healthResponse) - assert.NotNil(health.Server) - assert.True(health.Server.Ok) - assert.Equal("ok", health.Server.Message) - assert.Nil(health.Client) + require.NotNil(health.Server) + require.True(health.Server.Ok) + require.Equal("ok", health.Server.Message) + require.Nil(health.Client) } // type=client&type=server { req, err := http.NewRequest("GET", "/v1/agent/health?type=client&type=server", nil) - assert.Nil(err) + require.Nil(err) respW := httptest.NewRecorder() healthI, err := s.Server.HealthRequest(respW, req) - assert.Nil(err) - assert.Equal(http.StatusOK, respW.Code) - assert.NotNil(healthI) + require.Nil(err) + require.Equal(http.StatusOK, respW.Code) + require.NotNil(healthI) health := healthI.(*healthResponse) - assert.NotNil(health.Client) - assert.True(health.Client.Ok) - assert.Equal("ok", health.Client.Message) - assert.NotNil(health.Server) - assert.True(health.Server.Ok) - assert.Equal("ok", health.Server.Message) + require.NotNil(health.Client) + require.True(health.Client.Ok) + require.Equal("ok", health.Client.Message) + require.NotNil(health.Server) + require.True(health.Server.Ok) + require.Equal("ok", health.Server.Message) } }) } func TestHTTP_AgentHealth_BadServer(t *testing.T) { t.Parallel() - assert := assert.New(t) + require := require.New(t) // Enable ACLs to ensure they're not enforced httpACLTest(t, nil, func(s *TestAgent) { @@ -726,39 +713,39 @@ func TestHTTP_AgentHealth_BadServer(t *testing.T) { // No ?type= means server is just skipped { req, err := http.NewRequest("GET", "/v1/agent/health", nil) - assert.Nil(err) + require.Nil(err) respW := httptest.NewRecorder() healthI, err := s.Server.HealthRequest(respW, req) - assert.Nil(err) - assert.Equal(http.StatusOK, respW.Code) - assert.NotNil(healthI) + require.Nil(err) + require.Equal(http.StatusOK, respW.Code) + require.NotNil(healthI) health := healthI.(*healthResponse) - assert.NotNil(health.Client) - assert.True(health.Client.Ok) - assert.Equal("ok", health.Client.Message) - assert.Nil(health.Server) + require.NotNil(health.Client) + require.True(health.Client.Ok) + require.Equal("ok", health.Client.Message) + require.Nil(health.Server) } // type=server means server is considered unhealthy { req, err := http.NewRequest("GET", "/v1/agent/health?type=server", nil) - assert.Nil(err) + require.Nil(err) respW := httptest.NewRecorder() _, err = s.Server.HealthRequest(respW, req) - assert.NotNil(err) + require.NotNil(err) httpErr, ok := err.(HTTPCodedError) - assert.True(ok) - assert.Equal(500, httpErr.Code()) - assert.Equal(`{"server":{"ok":false,"message":"server not enabled"}}`, err.Error()) + require.True(ok) + require.Equal(500, httpErr.Code()) + require.Equal(`{"server":{"ok":false,"message":"server not enabled"}}`, err.Error()) } }) } func TestHTTP_AgentHealth_BadClient(t *testing.T) { t.Parallel() - assert := assert.New(t) + require := require.New(t) // Enable ACLs to ensure they're not enforced httpACLTest(t, nil, func(s *TestAgent) { @@ -769,32 +756,32 @@ func TestHTTP_AgentHealth_BadClient(t *testing.T) { // No ?type= means client is just skipped { req, err := http.NewRequest("GET", "/v1/agent/health", nil) - assert.Nil(err) + require.Nil(err) respW := httptest.NewRecorder() healthI, err := s.Server.HealthRequest(respW, req) - assert.Nil(err) - assert.Equal(http.StatusOK, respW.Code) - assert.NotNil(healthI) + require.Nil(err) + require.Equal(http.StatusOK, respW.Code) + require.NotNil(healthI) health := healthI.(*healthResponse) - assert.NotNil(health.Server) - assert.True(health.Server.Ok) - assert.Equal("ok", health.Server.Message) - assert.Nil(health.Client) + require.NotNil(health.Server) + require.True(health.Server.Ok) + require.Equal("ok", health.Server.Message) + require.Nil(health.Client) } // type=client means client is considered unhealthy { req, err := http.NewRequest("GET", "/v1/agent/health?type=client", nil) - assert.Nil(err) + require.Nil(err) respW := httptest.NewRecorder() _, err = s.Server.HealthRequest(respW, req) - assert.NotNil(err) + require.NotNil(err) httpErr, ok := err.(HTTPCodedError) - assert.True(ok) - assert.Equal(500, httpErr.Code()) - assert.Equal(`{"client":{"ok":false,"message":"client not enabled"}}`, err.Error()) + require.True(ok) + require.Equal(500, httpErr.Code()) + require.Equal(`{"client":{"ok":false,"message":"client not enabled"}}`, err.Error()) } }) } diff --git a/command/agent/testagent.go b/command/agent/testagent.go index 539890004..617e3cced 100644 --- a/command/agent/testagent.go +++ b/command/agent/testagent.go @@ -5,6 +5,7 @@ import ( "io" "io/ioutil" "math/rand" + "net" "net/http" "net/http/httptest" "os" @@ -17,8 +18,11 @@ import ( metrics "github.com/armon/go-metrics" "github.com/hashicorp/consul/lib/freeport" + msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/client/fingerprint" + "github.com/hashicorp/nomad/helper/pool" + "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/nomad" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" @@ -115,7 +119,10 @@ func (a *TestAgent) Start() *TestAgent { a.Config.NomadConfig.DataDir = d } - for i := 10; i >= 0; i-- { + i := 10 + +RETRY: + for ; i >= 0; i-- { a.pickRandomPorts(a.Config) if a.Config.NodeName == "" { a.Config.NodeName = fmt.Sprintf("Node %d", a.Config.Ports.RPC) @@ -137,14 +144,14 @@ func (a *TestAgent) Start() *TestAgent { a.Agent = agent break } else if i == 0 { - fmt.Println(a.Name, "Error starting agent:", err) + a.T.Logf(a.Name, "Error starting agent:", err) runtime.Goexit() } else { if agent != nil { agent.Shutdown() } wait := time.Duration(rand.Int31n(2000)) * time.Millisecond - fmt.Println(a.Name, "retrying in", wait) + a.T.Logf("%s: retrying in %v", a.Name, wait) time.Sleep(wait) } @@ -153,20 +160,35 @@ func (a *TestAgent) Start() *TestAgent { // the data dir, such as in the Raft configuration. if a.DataDir != "" { if err := os.RemoveAll(a.DataDir); err != nil { - fmt.Println(a.Name, "Error resetting data dir:", err) + a.T.Logf("%s: Error resetting data dir: %v", a.Name, err) runtime.Goexit() } } } + failed := false if a.Config.NomadConfig.Bootstrap && a.Config.Server.Enabled { + addr := a.Config.AdvertiseAddrs.RPC testutil.WaitForResult(func() (bool, error) { + conn, err := net.DialTimeout("tcp", addr, 100*time.Millisecond) + if err != nil { + return false, err + } + defer conn.Close() + + // Write the Consul RPC byte to set the mode + if _, err := conn.Write([]byte{byte(pool.RpcNomad)}); err != nil { + return false, err + } + + codec := pool.NewClientCodec(conn) args := &structs.GenericRequest{} var leader string - err := a.RPC("Status.Leader", args, &leader) + err = msgpackrpc.CallWithCodec(codec, "Status.Leader", args, &leader) return leader != "", err }, func(err error) { - a.T.Fatalf("failed to find leader: %v", err) + a.T.Logf("failed to find leader: %v", err) + failed = true }) } else { testutil.WaitForResult(func() (bool, error) { @@ -175,9 +197,14 @@ func (a *TestAgent) Start() *TestAgent { _, err := a.Server.AgentSelfRequest(resp, req) return err == nil && resp.Code == 200, err }, func(err error) { - a.T.Fatalf("failed OK response: %v", err) + a.T.Logf("failed to find leader: %v", err) + failed = true }) } + if failed { + a.Agent.Shutdown() + goto RETRY + } // Check if ACLs enabled. Use special value of PolicyTTL 0s // to do a bypass of this step. This is so we can test bootstrap @@ -194,7 +221,7 @@ func (a *TestAgent) Start() *TestAgent { func (a *TestAgent) start() (*Agent, error) { if a.LogOutput == nil { - a.LogOutput = os.Stderr + a.LogOutput = testlog.NewWriter(a.T) } inm := metrics.NewInmemSink(10*time.Second, time.Minute) @@ -264,6 +291,15 @@ func (a *TestAgent) pickRandomPorts(c *Config) { c.Ports.RPC = ports[1] c.Ports.Serf = ports[2] + // Clear out the advertise addresses such that through retries we + // re-normalize the addresses correctly instead of using the values from the + // last port selection that had a port conflict. + if c.AdvertiseAddrs != nil { + c.AdvertiseAddrs.HTTP = "" + c.AdvertiseAddrs.RPC = "" + c.AdvertiseAddrs.Serf = "" + } + if err := c.normalizeAddrs(); err != nil { a.T.Fatalf("error normalizing config: %v", err) } diff --git a/command/client_config_test.go b/command/client_config_test.go index e00bb6e30..cb9275ca0 100644 --- a/command/client_config_test.go +++ b/command/client_config_test.go @@ -33,23 +33,16 @@ func TestClientConfigCommand_UpdateServers(t *testing.T) { } ui.ErrorWriter.Reset() - // Set the servers list + // Set the servers list with bad addresses code = cmd.Run([]string{"-address=" + url, "-update-servers", "127.0.0.42", "198.18.5.5"}) - if code != 0 { - t.Fatalf("expected exit 0, got: %d", code) + if code != 1 { + t.Fatalf("expected exit 1, got: %d", code) } - // Query the servers list - code = cmd.Run([]string{"-address=" + url, "-servers"}) + // Set the servers list with good addresses + code = cmd.Run([]string{"-address=" + url, "-update-servers", srv.Config.AdvertiseAddrs.RPC}) if code != 0 { - t.Fatalf("expect exit 0, got: %d", code) - } - out := ui.OutputWriter.String() - if !strings.Contains(out, "127.0.0.42") { - t.Fatalf("missing 127.0.0.42") - } - if !strings.Contains(out, "198.18.5.5") { - t.Fatalf("missing 198.18.5.5") + t.Fatalf("expected exit 0, got: %d", code) } }