Cluster system test for nodes/

It's failing however.

http/service.go

@@ -17,7 +17,6 @@ import (
 	"net/http/pprof"
 	"os"
 	"runtime"
-	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -628,18 +627,32 @@ func (s *Service) handleNodes(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	reachable, err := s.checkNodesReachable(nodes, t)
+	lAddr, err := s.store.LeaderAddr()
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
-	resp := make([]map[string]string, len(nodes))
+	apiAddrs, err := s.checkNodesAPIAddr(nodes, t)
-	for i, n := range nodes {
+	if err != nil {
-		resp[i] = make(map[string]string)
+		http.Error(w, err.Error(), http.StatusInternalServerError)
-		resp[i]["id"] = n.ID
+		return
-		resp[i]["addr"] = n.Addr
+	}
-		resp[i]["reachable"] = strconv.FormatBool(reachable[n.ID])
+
+	resp := make(map[string]struct {
+		APIAddr   string `json:"api_addr,omitempty"`
+		Addr      string `json:"addr,omitempty"`
+		Reachable bool   `json:"reachable,omitempty"`
+		Leader    bool   `json:"leader,omitempty"`
+	})
+
+	for _, n := range nodes {
+		nn := resp[n.ID]
+		nn.Addr = n.Addr
+		nn.Leader = nn.Addr == lAddr
+		nn.APIAddr = apiAddrs[n.ID]
+		nn.Reachable = apiAddrs[n.ID] != ""
+		resp[n.ID] = nn
 	}
 
 	pretty, _ := isPretty(r)
@@ -897,34 +910,34 @@ func (s *Service) LeaderAPIAddr() string {
 	return apiAddr
 }
 
-// checkNodesReachable returns a map of node ID to reachable status, reachable
+// checkNodesAPIAddr returns a map of node ID to API addresses, reachable
 // being defined as node responds to a simple request over the network.
-func (s *Service) checkNodesReachable(nodes []*store.Server, timeout time.Duration) (map[string]bool, error) {
+func (s *Service) checkNodesAPIAddr(nodes []*store.Server, timeout time.Duration) (map[string]string, error) {
 	var wg sync.WaitGroup
 	var mu sync.Mutex
-	reachable := make(map[string]bool)
+	apiAddrs := make(map[string]string)
 
-	// Assume reachable.
+	// Assume unreachable
 	for _, n := range nodes {
-		reachable[n.ID] = true
+		apiAddrs[n.ID] = ""
 	}
 
 	// Now confirm.
 	for _, n := range nodes {
-		go func() {
 		wg.Add(1)
+		go func() {
 			defer wg.Done()
-			_, err := s.cluster.GetNodeAPIAddr(n.Addr)
+			addr, err := s.cluster.GetNodeAPIAddr(n.Addr)
-			if err != nil {
+			if err == nil {
 				mu.Lock()
-				reachable[n.ID] = false
+				apiAddrs[n.ID] = addr
 				mu.Unlock()
 			}
 		}()
 	}
 	wg.Wait()
 
-	return reachable, nil
+	return apiAddrs, nil
 }
 
 // addBuildVersion adds the build version to the HTTP response.

http/service_test.go

@@ -586,6 +586,30 @@ func Test_FormRedirectHTTPS(t *testing.T) {
 	}
 }
 
+func Test_Nodes(t *testing.T) {
+	m := &MockStore{
+		leaderAddr: "foo:1234",
+	}
+	c := &mockClusterService{
+		apiAddr: "https://bar:5678",
+	}
+	s := New("127.0.0.1:0", m, c, nil)
+	if err := s.Start(); err != nil {
+		t.Fatalf("failed to start service")
+	}
+	defer s.Close()
+
+	client := &http.Client{}
+	host := fmt.Sprintf("http://%s", s.Addr().String())
+	resp, err := client.Get(host + "/nodes")
+	if err != nil {
+		t.Fatalf("failed to make nodes request")
+	}
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("failed to get expected StatusOK for nodes, got %d", resp.StatusCode)
+	}
+}
+
 func Test_TLSServce(t *testing.T) {
 	m := &MockStore{}
 	c := &mockClusterService{}
@@ -798,3 +822,12 @@ func mustParseDuration(d string) time.Duration {
 		return dur
 	}
 }
+
+func mustReadResponseBody(resp *http.Response) string {
+	response, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		panic("failed to ReadAll response body")
+	}
+	resp.Body.Close()
+	return string(response)
+}

system_test/cluster_test.go

@@ -1,6 +1,7 @@
 package system
 
 import (
+	"fmt"
 	"testing"
 	"time"
 )
@@ -156,6 +157,97 @@ func Test_MultiNodeCluster(t *testing.T) {
 	}
 }
 
+// Test_MultiNodeClusterNodes checks nodes/ endpoint under various situations.
+func Test_MultiNodeClusterNodes(t *testing.T) {
+	node1 := mustNewLeaderNode()
+	defer node1.Deprovision()
+
+	node2 := mustNewNode(false)
+	defer node2.Deprovision()
+	if err := node2.Join(node1); err != nil {
+		t.Fatalf("node failed to join leader: %s", err.Error())
+	}
+	_, err := node2.WaitForLeader()
+	if err != nil {
+		t.Fatalf("failed waiting for leader: %s", err.Error())
+	}
+
+	// Get the new leader, in case it changed.
+	c := Cluster{node1, node2}
+	leader, err := c.Leader()
+	if err != nil {
+		t.Fatalf("failed to find cluster leader: %s", err.Error())
+	}
+
+	node3 := mustNewNode(false)
+	defer node3.Deprovision()
+	if err := node3.Join(leader); err != nil {
+		t.Fatalf("node failed to join leader: %s", err.Error())
+	}
+	_, err = node3.WaitForLeader()
+	if err != nil {
+		t.Fatalf("failed waiting for leader: %s", err.Error())
+	}
+
+	// Get the new leader, in case it changed.
+	c = Cluster{node1, node2, node3}
+	leader, err = c.Leader()
+	if err != nil {
+		t.Fatalf("failed to find cluster leader: %s", err.Error())
+	}
+
+	// Get nodes/ status from a node
+	nodes, err := node1.Nodes()
+	if err != nil {
+		t.Fatalf("failed to get nodes status: %s", err.Error())
+	}
+	fmt.Println(nodes)
+	if len(nodes) != len(c) {
+		t.Fatalf("nodes/ output returned wrong number of nodes, got %d, exp %d", len(nodes), len(c))
+	}
+	ns, ok := nodes[leader.ID]
+	if !ok {
+		t.Fatalf("failed to find leader with ID %s in node status", leader.ID)
+	}
+	if !ns.Leader {
+		t.Fatalf("node is not leader")
+	}
+	if ns.Addr != leader.RaftAddr {
+		t.Fatalf("node has wrong Raft address for leader")
+	}
+	leaderAPIAddr := fmt.Sprintf("http://%s", leader.APIAddr)
+	if ns.APIAddr != leaderAPIAddr {
+		t.Fatalf("node has wrong API address for leader, got %s, exp %s", ns.APIAddr, leaderAPIAddr)
+	}
+	if !ns.Reachable {
+		t.Fatalf("node is not reachable")
+	}
+
+	// Get a follower and confirm nodes/ looks good.
+	followers, err := c.Followers()
+	if err != nil {
+		t.Fatalf("failed to get followers: %s", err.Error())
+	}
+	if len(followers) != 2 {
+		t.Fatalf("got incorrect number of followers: %d", len(followers))
+	}
+	f := followers[0]
+	ns = nodes[f.ID]
+	if ns.Addr != leader.RaftAddr {
+		t.Fatalf("node has wrong Raft address for follower")
+	}
+	if ns.APIAddr != fmt.Sprintf("http://%s", f.APIAddr) {
+		t.Fatalf("node has wrong API address for follower")
+	}
+	if ns.Leader {
+		t.Fatalf("node is not a follower")
+	}
+	if !ns.Reachable {
+		t.Fatalf("node is not reachable")
+	}
+
+}
+
 // Test_MultiNodeClusterNodeEncrypted tests formation of a 3-node cluster, and its operation.
 // This test enables inter-node encryption, but keeps the unencrypted HTTP API.
 func Test_MultiNodeClusterNodeEncrypted(t *testing.T) {

system_test/helpers.go

@@ -181,6 +181,37 @@ func (n *Node) JoinAsNonVoter(leader *Node) error {
 	return nil
 }
 
+type NodesStatus map[string]struct {
+	APIAddr   string `json:"api_addr,omitempty"`
+	Addr      string `json:"addr,omitempty"`
+	Reachable bool   `json:"reachable,omitempty"`
+	Leader    bool   `json:"leader,omitempty"`
+}
+
+// Nodes returns the sNodes endpoint output for node.
+func (n *Node) Nodes() (NodesStatus, error) {
+	v, _ := url.Parse("http://" + n.APIAddr + "/nodes")
+
+	resp, err := http.Get(v.String())
+	if err != nil {
+		return nil, err
+	}
+	if resp.StatusCode != 200 {
+		return nil, fmt.Errorf("nodes endpoint returned: %s", resp.Status)
+	}
+	defer resp.Body.Close()
+	body, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	var nstatus NodesStatus
+	if err = json.Unmarshal(body, &nstatus); err != nil {
+		return nil, err
+	}
+	return nstatus, nil
+}
+
 // Status returns the status and diagnostic output for node.
 func (n *Node) Status() (string, error) {
 	v, _ := url.Parse("http://" + n.APIAddr + "/status")
@@ -480,6 +511,9 @@ func mustNodeEncryptedOnDisk(dir string, enableSingle, httpEncrypt bool, mux *tc
 	}
 	node.APIAddr = node.Service.Addr().String()
 
+	// Finally, set API address in Cluster service
+	cluster.SetAPIAddr(node.APIAddr)
+
 	return node
 }
 

system_test/single_node_test.go

@@ -387,6 +387,37 @@ func Test_SingleNodeRestart(t *testing.T) {
 	}
 }
 
+func Test_SingleNodeNodes(t *testing.T) {
+	node := mustNewLeaderNode()
+	defer node.Deprovision()
+
+	// Access endpoints to ensure the code is covered.
+	nodes, err := node.Nodes()
+	if err != nil {
+		t.Fatalf("failed to access nodes endpoint: %s", err.Error())
+	}
+
+	if len(nodes) != 1 {
+		t.Fatalf("wrong number of nodes in response")
+	}
+	n, ok := nodes[node.ID]
+	if !ok {
+		t.Fatalf("node not found by ID in response")
+	}
+	if n.Addr != node.RaftAddr {
+		t.Fatalf("node has wrong Raft address")
+	}
+	if n.APIAddr != fmt.Sprintf("http://%s", node.APIAddr) {
+		t.Fatalf("node has wrong API address")
+	}
+	if !n.Leader {
+		t.Fatalf("node is not leader")
+	}
+	if !n.Reachable {
+		t.Fatalf("node is not reachable")
+	}
+}
+
 func Test_SingleNodeCoverage(t *testing.T) {
 	node := mustNewLeaderNode()
 	defer node.Deprovision()