From e25dff5a2897f783df5f0b5d1fe8d1eeefeaab44 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 14 Sep 2017 10:04:48 -0700 Subject: [PATCH 1/3] Sort /v1/agent/servers output This PR sorts the output of the endpoint since its results are used as part of Consul checks to avoid the value changing unnecessarily. Fixes https://github.com/hashicorp/nomad/issues/3211 --- command/agent/agent_endpoint.go | 2 ++ command/agent/agent_endpoint_test.go | 24 +++++++----------------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/command/agent/agent_endpoint.go b/command/agent/agent_endpoint.go index 6427b93f2b24..92002ead8b04 100644 --- a/command/agent/agent_endpoint.go +++ b/command/agent/agent_endpoint.go @@ -3,6 +3,7 @@ package agent import ( "net" "net/http" + "sort" "strings" "github.com/hashicorp/nomad/nomad/structs" @@ -148,6 +149,7 @@ func (s *HTTPServer) listServers(resp http.ResponseWriter, req *http.Request) (i } peers := s.agent.client.GetServers() + sort.Strings(peers) return peers, nil } diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index 6e977d9a00f7..24890b1f35ad 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "reflect" "strings" "testing" @@ -175,10 +176,10 @@ func TestHTTP_AgentSetServers(t *testing.T) { respW = httptest.NewRecorder() // Make the request and check the result - expected := map[string]bool{ - "127.0.0.1:4647": true, - "127.0.0.2:4647": true, - "127.0.0.3:4647": true, + expected := []string{ + "127.0.0.1:4647", + "127.0.0.2:4647", + "127.0.0.3:4647", } out, err := s.Server.AgentServersRequest(respW, req) if err != nil { @@ -188,19 +189,8 @@ func TestHTTP_AgentSetServers(t *testing.T) { if n := len(servers); n != len(expected) { t.Fatalf("expected %d servers, got: %d: %v", len(expected), n, servers) } - received := make(map[string]bool, len(servers)) - for _, server := range servers { - received[server] = true - } - foundCount := 0 - for k, _ := range received { - _, found := expected[k] - if found { - foundCount++ - } - } - if foundCount != len(expected) { - t.Fatalf("bad servers result") + if !reflect.DeepEqual(servers, expected) { + t.Fatalf("got %v; want %v", servers, expected) } }) } From d0a9389a27367df08624ffbde9eab2139c398f20 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 14 Sep 2017 14:20:10 -0700 Subject: [PATCH 2/3] use assert --- command/agent/agent_endpoint_test.go | 41 ++++++++-------------------- 1 file changed, 12 insertions(+), 29 deletions(-) diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index 24890b1f35ad..99e3c3953db6 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -6,11 +6,10 @@ import ( "fmt" "net/http" "net/http/httptest" - "reflect" - "strings" "testing" "github.com/hashicorp/nomad/nomad/structs" + "github.com/stretchr/testify/assert" ) func TestHTTP_AgentSelf(t *testing.T) { @@ -134,45 +133,35 @@ func TestHTTP_AgentForceLeave(t *testing.T) { func TestHTTP_AgentSetServers(t *testing.T) { t.Parallel() + assert := assert.New(t) httpTest(t, nil, func(s *TestAgent) { // Establish a baseline number of servers req, err := http.NewRequest("GET", "/v1/agent/servers", nil) - if err != nil { - t.Fatalf("err: %s", err) - } + assert.Nil(err) respW := httptest.NewRecorder() // Create the request req, err = http.NewRequest("PUT", "/v1/agent/servers", nil) - if err != nil { - t.Fatalf("err: %s", err) - } + assert.Nil(err) // Send the request respW = httptest.NewRecorder() _, err = s.Server.AgentServersRequest(respW, req) - if err == nil || !strings.Contains(err.Error(), "missing server address") { - t.Fatalf("expected missing servers error, got: %#v", err) - } + assert.NotNil(err) + assert.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) - if err != nil { - t.Fatalf("err: %s", err) - } + assert.Nil(err) // Send the request respW = httptest.NewRecorder() _, err = s.Server.AgentServersRequest(respW, req) - if err != nil { - t.Fatalf("err: %s", err) - } + assert.Nil(err) // Retrieve the servers again req, err = http.NewRequest("GET", "/v1/agent/servers", nil) - if err != nil { - t.Fatalf("err: %s", err) - } + assert.Nil(err) respW = httptest.NewRecorder() // Make the request and check the result @@ -182,16 +171,10 @@ func TestHTTP_AgentSetServers(t *testing.T) { "127.0.0.3:4647", } out, err := s.Server.AgentServersRequest(respW, req) - if err != nil { - t.Fatalf("err: %s", err) - } + assert.Nil(err) servers := out.([]string) - if n := len(servers); n != len(expected) { - t.Fatalf("expected %d servers, got: %d: %v", len(expected), n, servers) - } - if !reflect.DeepEqual(servers, expected) { - t.Fatalf("got %v; want %v", servers, expected) - } + assert.Len(servers, len(expected)) + assert.Equal(expected, servers) }) } From 083aca50f6b606545407c7c755168d42a3c575ae Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 14 Sep 2017 14:21:41 -0700 Subject: [PATCH 3/3] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21e6947ee564..e253c5737afa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ IMPROVEMENTS: BUG FIXES: * core: *Fix restoration of stopped periodic jobs [GH-3201] + * api: Sort /v1/agent/servers output so that output of Consul checks does not + change [GH-3214] * api: Fix search handling of jobs with more than four hyphens and case were length could cause lookup error [GH-3203] * client: Fix lock contention that could cause a node to miss a heartbeat and