Merge pull request #2724 from hashicorp/b-2701-fix-server-check

Fix path used by Nomad Server HTTP Check

.travis.yml

@@ -15,7 +15,7 @@ branches:
 
 before_install:
     - sudo apt-get update
-    - sudo apt-get install -y docker-engine liblxc1 lxc-dev lxc
+    - sudo apt-get install -y liblxc1 lxc-dev lxc
     - sudo apt-get install -y qemu
     - ./scripts/install_rkt.sh
     - ./scripts/install_consul.sh

command/agent/agent.go

@@ -366,7 +366,8 @@ func (a *Agent) setupServer() error {
 			PortLabel: a.config.AdvertiseAddrs.HTTP,
 			Tags:      []string{consul.ServiceTagHTTP},
 		}
-		if check := a.agentHTTPCheck(); check != nil {
+		const isServer = true
+		if check := a.agentHTTPCheck(isServer); check != nil {
 			httpServ.Checks = []*structs.ServiceCheck{check}
 		}
 		rpcServ := &structs.Service{
@@ -469,7 +470,8 @@ func (a *Agent) setupClient() error {
 			PortLabel: a.config.AdvertiseAddrs.HTTP,
 			Tags:      []string{consul.ServiceTagHTTP},
 		}
-		if check := a.agentHTTPCheck(); check != nil {
+		const isServer = false
+		if check := a.agentHTTPCheck(isServer); check != nil {
 			httpServ.Checks = []*structs.ServiceCheck{check}
 		}
 		if err := a.consulService.RegisterAgent(consulRoleClient, []*structs.Service{httpServ}); err != nil {
@@ -482,7 +484,7 @@ func (a *Agent) setupClient() error {
 
 // agentHTTPCheck returns a health check for the agent's HTTP API if possible.
 // If no HTTP health check can be supported nil is returned.
-func (a *Agent) agentHTTPCheck() *structs.ServiceCheck {
+func (a *Agent) agentHTTPCheck(server bool) *structs.ServiceCheck {
 	// Resolve the http check address
 	httpCheckAddr := a.config.normalizedAddrs.HTTP
 	if *a.config.Consul.ChecksUseAdvertise {
@@ -497,6 +499,11 @@ func (a *Agent) agentHTTPCheck() *structs.ServiceCheck {
 		Timeout:   clientHttpCheckTimeout,
 		PortLabel: httpCheckAddr,
 	}
+	// Switch to endpoint that doesn't require a leader for servers
+	if server {
+		check.Name = "Nomad Server HTTP Check"
+		check.Path = "/v1/status/peers"
+	}
 	if !a.config.TLSConfig.EnableHTTP {
 		// No HTTPS, return a plain http check
 		return &check

command/agent/agent_test.go

@@ -385,7 +385,7 @@ func TestAgent_HTTPCheck(t *testing.T) {
 
 	t.Run("Plain HTTP Check", func(t *testing.T) {
 		a := agent()
-		check := a.agentHTTPCheck()
+		check := a.agentHTTPCheck(false)
 		if check == nil {
 			t.Fatalf("expected non-nil check")
 		}
@@ -406,7 +406,7 @@ func TestAgent_HTTPCheck(t *testing.T) {
 	t.Run("Plain HTTP + ChecksUseAdvertise", func(t *testing.T) {
 		a := agent()
 		a.config.Consul.ChecksUseAdvertise = helper.BoolToPtr(true)
-		check := a.agentHTTPCheck()
+		check := a.agentHTTPCheck(false)
 		if check == nil {
 			t.Fatalf("expected non-nil check")
 		}
@@ -420,7 +420,7 @@ func TestAgent_HTTPCheck(t *testing.T) {
 		a.consulSupportsTLSSkipVerify = true
 		a.config.TLSConfig.EnableHTTP = true
 
-		check := a.agentHTTPCheck()
+		check := a.agentHTTPCheck(false)
 		if check == nil {
 			t.Fatalf("expected non-nil check")
 		}
@@ -437,7 +437,7 @@ func TestAgent_HTTPCheck(t *testing.T) {
 		a.consulSupportsTLSSkipVerify = false
 		a.config.TLSConfig.EnableHTTP = true
 
-		if check := a.agentHTTPCheck(); check != nil {
+		if check := a.agentHTTPCheck(false); check != nil {
 			t.Fatalf("expected nil check not: %#v", check)
 		}
 	})
@@ -448,7 +448,7 @@ func TestAgent_HTTPCheck(t *testing.T) {
 		a.config.TLSConfig.EnableHTTP = true
 		a.config.TLSConfig.VerifyHTTPSClient = true
 
-		if check := a.agentHTTPCheck(); check != nil {
+		if check := a.agentHTTPCheck(false); check != nil {
 			t.Fatalf("expected nil check not: %#v", check)
 		}
 	})
@@ -557,3 +557,39 @@ func TestAgent_ConsulSupportsTLSSkipVerify(t *testing.T) {
             "wan_join_port": "8302"
         }}`)
 }
+
+// TestAgent_HTTPCheckPath asserts clients and servers use different endpoints
+// for healthchecks.
+func TestAgent_HTTPCheckPath(t *testing.T) {
+	// Agent.agentHTTPCheck only needs a config and logger
+	a := &Agent{
+		config: DevConfig(),
+		logger: log.New(ioutil.Discard, "", 0),
+	}
+	if err := a.config.normalizeAddrs(); err != nil {
+		t.Fatalf("error normalizing config: %v", err)
+	}
+	if testing.Verbose() {
+		a.logger = log.New(os.Stderr, "", log.LstdFlags)
+	}
+
+	// Assert server check uses /v1/status/peers
+	isServer := true
+	check := a.agentHTTPCheck(isServer)
+	if expected := "Nomad Server HTTP Check"; check.Name != expected {
+		t.Errorf("expected server check name to be %q but found %q", expected, check.Name)
+	}
+	if expected := "/v1/status/peers"; check.Path != expected {
+		t.Errorf("expected server check path to be %q but found %q", expected, check.Path)
+	}
+
+	// Assert client check uses /v1/agent/servers
+	isServer = false
+	check = a.agentHTTPCheck(isServer)
+	if expected := "Nomad Client HTTP Check"; check.Name != expected {
+		t.Errorf("expected client check name to be %q but found %q", expected, check.Name)
+	}
+	if expected := "/v1/agent/servers"; check.Path != expected {
+		t.Errorf("expected client check path to be %q but found %q", expected, check.Path)
+	}
+}