Merge pull request #3214 from hashicorp/f-agent-servers

Sort /v1/agent/servers output

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

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
 }
 

command/agent/agent_endpoint_test.go

@@ -6,10 +6,10 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
-	"strings"
 	"testing"
 
 	"github.com/hashicorp/nomad/nomad/structs"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestHTTP_AgentSelf(t *testing.T) {
@@ -133,75 +133,48 @@ 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
-		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 {
-			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)
-		}
-		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")
-		}
+		assert.Len(servers, len(expected))
+		assert.Equal(expected, servers)
 	})
 }