From c9afc16d96751ef22bdbc7657ca503ec0698b87a Mon Sep 17 00:00:00 2001
From: Sean Chittenden <sean@chittenden.org>
Date: Mon, 28 Mar 2016 13:38:58 -0700
Subject: [PATCH] Return error from PingConsulServer

In order to report why a Ping failed, change the signature of PingConsulServers to include an error message.
---
 consul/client_test.go                                 | 5 +++--
 consul/pool.go                                        | 8 ++++----
 consul/server_manager/server_manager.go               | 6 ++++--
 consul/server_manager/server_manager_internal_test.go | 4 ++--
 consul/server_manager/server_manager_test.go          | 4 ++--
 5 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/consul/client_test.go b/consul/client_test.go
index f8c5d37071..102ea53737 100644
--- a/consul/client_test.go
+++ b/consul/client_test.go
@@ -287,8 +287,9 @@ func TestClient_RPC_ConsulServerPing(t *testing.T) {
 	for range servers {
 		time.Sleep(1 * time.Second)
 		s := c.serverMgr.FindServer()
-		if !c.connPool.PingConsulServer(s) {
-			t.Errorf("Unable to ping server %v", s.String())
+		ok, err := c.connPool.PingConsulServer(s)
+		if !ok {
+			t.Errorf("Unable to ping server %v: %s", s.String(), err)
 		}
 		pingCount += 1
 	}
diff --git a/consul/pool.go b/consul/pool.go
index 4d718d82e8..cdfd2129df 100644
--- a/consul/pool.go
+++ b/consul/pool.go
@@ -408,11 +408,11 @@ func (p *ConnPool) RPC(dc string, addr net.Addr, version int, method string, arg
 
 // PingConsulServer sends a Status.Ping message to the specified server and
 // returns true if healthy, false if an error occurred
-func (p *ConnPool) PingConsulServer(s *server_details.ServerDetails) bool {
+func (p *ConnPool) PingConsulServer(s *server_details.ServerDetails) (bool, error) {
 	// Get a usable client
 	conn, sc, err := p.getClient(s.Datacenter, s.Addr, s.Version)
 	if err != nil {
-		return false
+		return false, err
 	}
 
 	// Make the RPC call
@@ -421,13 +421,13 @@ func (p *ConnPool) PingConsulServer(s *server_details.ServerDetails) bool {
 	if err != nil {
 		sc.Close()
 		p.releaseConn(conn)
-		return false
+		return false, err
 	}
 
 	// Done with the connection
 	conn.returnClient(sc)
 	p.releaseConn(conn)
-	return true
+	return true, nil
 }
 
 // Reap is used to close conns open over maxTime
diff --git a/consul/server_manager/server_manager.go b/consul/server_manager/server_manager.go
index a241a68f3a..21ff8c76ba 100644
--- a/consul/server_manager/server_manager.go
+++ b/consul/server_manager/server_manager.go
@@ -56,7 +56,7 @@ type ConsulClusterInfo interface {
 // ConnPoolTester is an interface wrapping client.ConnPool to prevent a
 // cyclic import dependency
 type ConnPoolPinger interface {
-	PingConsulServer(server *server_details.ServerDetails) bool
+	PingConsulServer(server *server_details.ServerDetails) (bool, error)
 }
 
 // serverConfig is the thread-safe configuration struct used to maintain the
@@ -306,11 +306,13 @@ FAILED_SERVER_DURING_REBALANCE:
 		selectedServer := sc.servers[0]
 
 		// sm.logger.Printf("[INFO] server manager: Preemptively testing server %s before rebalance", selectedServer.String())
-		ok := sm.connPoolPinger.PingConsulServer(selectedServer)
+		ok, err := sm.connPoolPinger.PingConsulServer(selectedServer)
 		if ok {
 			foundHealthyServer = true
 			break
 		}
+		sm.logger.Printf("[DEBUG] server manager: pinging server %s failed: %s", selectedServer.String(), err)
+
 		sc.cycleServer()
 	}
 
diff --git a/consul/server_manager/server_manager_internal_test.go b/consul/server_manager/server_manager_internal_test.go
index 11ab550191..6da74431be 100644
--- a/consul/server_manager/server_manager_internal_test.go
+++ b/consul/server_manager/server_manager_internal_test.go
@@ -28,8 +28,8 @@ func GetBufferedLogger() *log.Logger {
 type fauxConnPool struct {
 }
 
-func (s *fauxConnPool) PingConsulServer(server *server_details.ServerDetails) bool {
-	return true
+func (s *fauxConnPool) PingConsulServer(server *server_details.ServerDetails) (bool, error) {
+	return true, nil
 }
 
 type fauxSerf struct {
diff --git a/consul/server_manager/server_manager_test.go b/consul/server_manager/server_manager_test.go
index 65360ff2f5..fcfd38bafc 100644
--- a/consul/server_manager/server_manager_test.go
+++ b/consul/server_manager/server_manager_test.go
@@ -29,8 +29,8 @@ func GetBufferedLogger() *log.Logger {
 type fauxConnPool struct {
 }
 
-func (s *fauxConnPool) PingConsulServer(server *server_details.ServerDetails) bool {
-	return true
+func (s *fauxConnPool) PingConsulServer(server *server_details.ServerDetails) (bool, error) {
+	return true, nil
 }
 
 type fauxSerf struct {