Refactor consul.serverParts into server_details.ServerDetails

This may be short-lived, but it also seems like this is going to lead us down a path where ServerDetails is going to evolve into a more powerful package that will encapsulate more behavior behind a coherent API.

consul/client.go

@@ -11,6 +11,7 @@ import (
 	"sync/atomic"
 	"time"
 
+	"github.com/hashicorp/consul/consul/server_details"
 	"github.com/hashicorp/consul/consul/structs"
 	"github.com/hashicorp/serf/coordinate"
 	"github.com/hashicorp/serf/serf"
@@ -263,7 +264,7 @@ func (c *Client) lanEventHandler() {
 // nodeJoin is used to handle join events on the serf cluster
 func (c *Client) nodeJoin(me serf.MemberEvent) {
 	for _, m := range me.Members {
-		ok, parts := isConsulServer(m)
+		ok, parts := server_details.IsConsulServer(m)
 		if !ok {
 			continue
 		}
@@ -285,7 +286,7 @@ func (c *Client) nodeJoin(me serf.MemberEvent) {
 // nodeFail is used to handle fail events on the serf cluster
 func (c *Client) nodeFail(me serf.MemberEvent) {
 	for _, m := range me.Members {
-		ok, parts := isConsulServer(m)
+		ok, parts := server_details.IsConsulServer(m)
 		if !ok {
 			continue
 		}

consul/leader.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/armon/go-metrics"
+	"github.com/hashicorp/consul/consul/server_details"
 	"github.com/hashicorp/consul/consul/structs"
 	"github.com/hashicorp/raft"
 	"github.com/hashicorp/serf/serf"
@@ -349,7 +350,7 @@ func (s *Server) shouldHandleMember(member serf.Member) bool {
 	if valid, dc := isConsulNode(member); valid && dc == s.config.Datacenter {
 		return true
 	}
-	if valid, parts := isConsulServer(member); valid && parts.Datacenter == s.config.Datacenter {
+	if valid, parts := server_details.IsConsulServer(member); valid && parts.Datacenter == s.config.Datacenter {
 		return true
 	}
 	return false
@@ -360,7 +361,7 @@ func (s *Server) shouldHandleMember(member serf.Member) bool {
 func (s *Server) handleAliveMember(member serf.Member) error {
 	// Register consul service if a server
 	var service *structs.NodeService
-	if valid, parts := isConsulServer(member); valid {
+	if valid, parts := server_details.IsConsulServer(member); valid {
 		service = &structs.NodeService{
 			ID:      ConsulServiceID,
 			Service: ConsulServiceName,
@@ -496,7 +497,7 @@ func (s *Server) handleDeregisterMember(reason string, member serf.Member) error
 	}
 
 	// Remove from Raft peers if this was a server
-	if valid, parts := isConsulServer(member); valid {
+	if valid, parts := server_details.IsConsulServer(member); valid {
 		if err := s.removeConsulServer(member, parts.Port); err != nil {
 			return err
 		}
@@ -523,7 +524,7 @@ func (s *Server) handleDeregisterMember(reason string, member serf.Member) error
 }
 
 // joinConsulServer is used to try to join another consul server
-func (s *Server) joinConsulServer(m serf.Member, parts *serverParts) error {
+func (s *Server) joinConsulServer(m serf.Member, parts *server_details.ServerDetails) error {
 	// Do not join ourself
 	if m.Name == s.config.NodeName {
 		return nil
@@ -533,7 +534,7 @@ func (s *Server) joinConsulServer(m serf.Member, parts *serverParts) error {
 	if parts.Bootstrap {
 		members := s.serfLAN.Members()
 		for _, member := range members {
-			valid, p := isConsulServer(member)
+			valid, p := server_details.IsConsulServer(member)
 			if valid && member.Name != m.Name && p.Bootstrap {
 				s.logger.Printf("[ERR] consul: '%v' and '%v' are both in bootstrap mode. Only one node should be in bootstrap mode, not adding Raft peer.", m.Name, member.Name)
 				return nil

consul/merge.go

@@ -3,6 +3,7 @@ package consul
 import (
 	"fmt"
 
+	"github.com/hashicorp/consul/consul/server_details"
 	"github.com/hashicorp/serf/serf"
 )
 
@@ -24,7 +25,7 @@ func (md *lanMergeDelegate) NotifyMerge(members []*serf.Member) error {
 			continue
 		}
 
-		ok, parts := isConsulServer(*m)
+		ok, parts := server_details.IsConsulServer(*m)
 		if ok && parts.Datacenter != md.dc {
 			return fmt.Errorf("Member '%s' part of wrong datacenter '%s'",
 				m.Name, parts.Datacenter)
@@ -41,7 +42,7 @@ type wanMergeDelegate struct {
 
 func (md *wanMergeDelegate) NotifyMerge(members []*serf.Member) error {
 	for _, m := range members {
-		ok, _ := isConsulServer(*m)
+		ok, _ := server_details.IsConsulServer(*m)
 		if !ok {
 			return fmt.Errorf("Member '%s' is not a server", m.Name)
 		}

consul/serf.go

@@ -4,6 +4,7 @@ import (
 	"net"
 	"strings"
 
+	"github.com/hashicorp/consul/consul/server_details"
 	"github.com/hashicorp/serf/serf"
 )
 
@@ -140,7 +141,7 @@ func (s *Server) localEvent(event serf.UserEvent) {
 // lanNodeJoin is used to handle join events on the LAN pool.
 func (s *Server) lanNodeJoin(me serf.MemberEvent) {
 	for _, m := range me.Members {
-		ok, parts := isConsulServer(m)
+		ok, parts := server_details.IsConsulServer(m)
 		if !ok {
 			continue
 		}
@@ -163,7 +164,7 @@ func (s *Server) lanNodeJoin(me serf.MemberEvent) {
 // wanNodeJoin is used to handle join events on the WAN pool.
 func (s *Server) wanNodeJoin(me serf.MemberEvent) {
 	for _, m := range me.Members {
-		ok, parts := isConsulServer(m)
+		ok, parts := server_details.IsConsulServer(m)
 		if !ok {
 			s.logger.Printf("[WARN] consul: non-server in WAN pool: %s", m.Name)
 			continue
@@ -209,7 +210,7 @@ func (s *Server) maybeBootstrap() {
 	members := s.serfLAN.Members()
 	addrs := make([]string, 0)
 	for _, member := range members {
-		valid, p := isConsulServer(member)
+		valid, p := server_details.IsConsulServer(member)
 		if !valid {
 			continue
 		}
@@ -247,7 +248,7 @@ func (s *Server) maybeBootstrap() {
 // lanNodeFailed is used to handle fail events on the LAN pool.
 func (s *Server) lanNodeFailed(me serf.MemberEvent) {
 	for _, m := range me.Members {
-		ok, parts := isConsulServer(m)
+		ok, parts := server_details.IsConsulServer(m)
 		if !ok {
 			continue
 		}
@@ -262,7 +263,7 @@ func (s *Server) lanNodeFailed(me serf.MemberEvent) {
 // wanNodeFailed is used to handle fail events on the WAN pool.
 func (s *Server) wanNodeFailed(me serf.MemberEvent) {
 	for _, m := range me.Members {
-		ok, parts := isConsulServer(m)
+		ok, parts := server_details.IsConsulServer(m)
 		if !ok {
 			continue
 		}

consul/server.go

@@ -15,6 +15,7 @@ import (
 	"time"
 
 	"github.com/hashicorp/consul/acl"
+	"github.com/hashicorp/consul/consul/server_details"
 	"github.com/hashicorp/consul/consul/state"
 	"github.com/hashicorp/consul/tlsutil"
 	"github.com/hashicorp/raft"
@@ -97,7 +98,7 @@ type Server struct {
 
 	// localConsuls is used to track the known consuls
 	// in the local datacenter. Used to do leader forwarding.
-	localConsuls map[string]*serverParts
+	localConsuls map[string]*server_details.ServerDetails
 	localLock    sync.RWMutex
 
 	// Logger uses the provided LogOutput
@@ -119,7 +120,7 @@ type Server struct {
 
 	// remoteConsuls is used to track the known consuls in
 	// remote datacenters. Used to do DC forwarding.
-	remoteConsuls map[string][]*serverParts
+	remoteConsuls map[string][]*server_details.ServerDetails
 	remoteLock    sync.RWMutex
 
 	// rpcListener is used to listen for incoming connections
@@ -216,10 +217,10 @@ func NewServer(config *Config) (*Server, error) {
 		connPool:      NewPool(config.LogOutput, serverRPCCache, serverMaxStreams, tlsWrap),
 		eventChLAN:    make(chan serf.Event, 256),
 		eventChWAN:    make(chan serf.Event, 256),
-		localConsuls:  make(map[string]*serverParts),
+		localConsuls:  make(map[string]*server_details.ServerDetails),
 		logger:        logger,
 		reconcileCh:   make(chan serf.Member, 32),
-		remoteConsuls: make(map[string][]*serverParts),
+		remoteConsuls: make(map[string][]*server_details.ServerDetails),
 		rpcServer:     rpc.NewServer(),
 		rpcTLS:        incomingTLS,
 		tombstoneGC:   gc,

consul/server_details/server_details.go

@@ -0,0 +1,81 @@
+package server_details
+
+import (
+	"fmt"
+	"net"
+	"strconv"
+
+	"github.com/hashicorp/serf/serf"
+)
+
+// ServerDetails is used to return details of a consul server
+type ServerDetails struct {
+	Name       string
+	Datacenter string
+	Port       int
+	Bootstrap  bool
+	Expect     int
+	Version    int
+	Addr       net.Addr
+
+	// Disabled is a uint64 in order to support atomic integer
+	// operations.  Zero means enabled, non-zero is the number of times
+	// this server has failed without being marked healthy.
+	Disabled uint64
+}
+
+func (s *ServerDetails) String() string {
+	return fmt.Sprintf("%s (Addr: %s) (DC: %s)", s.Name, s.Addr, s.Datacenter)
+}
+
+// IsConsulServer returns true if a serf member is a consul server. Returns a
+// bool and a pointer to the ServerDetails.
+func IsConsulServer(m serf.Member) (bool, *ServerDetails) {
+	if m.Tags["role"] != "consul" {
+		return false, nil
+	}
+
+	datacenter := m.Tags["dc"]
+	_, bootstrap := m.Tags["bootstrap"]
+	var disabled uint64 = 0
+	_, disabledStr := m.Tags["disabled"]
+	if disabledStr {
+		disabled = 1
+	}
+
+	expect := 0
+	expect_str, ok := m.Tags["expect"]
+	var err error
+	if ok {
+		expect, err = strconv.Atoi(expect_str)
+		if err != nil {
+			return false, nil
+		}
+	}
+
+	port_str := m.Tags["port"]
+	port, err := strconv.Atoi(port_str)
+	if err != nil {
+		return false, nil
+	}
+
+	vsn_str := m.Tags["vsn"]
+	vsn, err := strconv.Atoi(vsn_str)
+	if err != nil {
+		return false, nil
+	}
+
+	addr := &net.TCPAddr{IP: m.Addr, Port: port}
+
+	parts := &ServerDetails{
+		Name:       m.Name,
+		Datacenter: datacenter,
+		Port:       port,
+		Bootstrap:  bootstrap,
+		Expect:     expect,
+		Addr:       addr,
+		Version:    vsn,
+		Disabled:   disabled,
+	}
+	return true, parts
+}

consul/server_details/server_details_test.go

@@ -0,0 +1,63 @@
+package server_details_test
+
+import (
+	"net"
+	"testing"
+
+	"github.com/hashicorp/consul/consul/server_details"
+	"github.com/hashicorp/serf/serf"
+)
+
+func TestIsConsulServer(t *testing.T) {
+	m := serf.Member{
+		Name: "foo",
+		Addr: net.IP([]byte{127, 0, 0, 1}),
+		Tags: map[string]string{
+			"role": "consul",
+			"dc":   "east-aws",
+			"port": "10000",
+			"vsn":  "1",
+		},
+	}
+	valid, parts := server_details.IsConsulServer(m)
+	if !valid || parts.Datacenter != "east-aws" || parts.Port != 10000 {
+		t.Fatalf("bad: %v %v", valid, parts)
+	}
+	if parts.Name != "foo" {
+		t.Fatalf("bad: %v", parts)
+	}
+	if parts.Bootstrap {
+		t.Fatalf("unexpected bootstrap")
+	}
+	if parts.Disabled > 0 {
+		t.Fatalf("unexpected disabled")
+	}
+	if parts.Expect != 0 {
+		t.Fatalf("bad: %v", parts.Expect)
+	}
+	m.Tags["bootstrap"] = "1"
+	m.Tags["disabled"] = "1"
+	valid, parts = server_details.IsConsulServer(m)
+	if !valid {
+		t.Fatalf("expected a valid consul server")
+	}
+	if !parts.Bootstrap {
+		t.Fatalf("expected bootstrap")
+	}
+	if parts.Disabled == 0 {
+		t.Fatalf("expected disabled")
+	}
+	if parts.Addr.String() != "127.0.0.1:10000" {
+		t.Fatalf("bad addr: %v", parts.Addr)
+	}
+	if parts.Version != 1 {
+		t.Fatalf("bad: %v", parts)
+	}
+	m.Tags["expect"] = "3"
+	delete(m.Tags, "bootstrap")
+	delete(m.Tags, "disabled")
+	valid, parts = server_details.IsConsulServer(m)
+	if !valid || parts.Expect != 3 {
+		t.Fatalf("bad: %v", parts.Expect)
+	}
+}

consul/util.go

@@ -23,26 +23,6 @@ import (
  */
 var privateBlocks []*net.IPNet
 
-// serverParts is used to return the parts of a server role
-type serverParts struct {
-	Name       string
-	Datacenter string
-	Port       int
-	Bootstrap  bool
-	Expect     int
-	Version    int
-	Addr       net.Addr
-
-	// Disabled is a uint64 in order to support atomic integer
-	// operations.  Zero means enabled, non-zero is the number of times
-	// this server has failed without being marked healthy.
-	Disabled uint64
-}
-
-func (s *serverParts) String() string {
-	return fmt.Sprintf("%s (Addr: %s) (DC: %s)", s.Name, s.Addr, s.Datacenter)
-}
-
 func init() {
 	// Add each private block
 	privateBlocks = make([]*net.IPNet, 6)
@@ -121,58 +101,6 @@ func CanServersUnderstandProtocol(members []serf.Member, version uint8) (bool, e
 	return (numServers > 0) && (numWhoGrok == numServers), nil
 }
 
-// Returns true if a serf member is a consul server. Returns a bool and a
-// pointer to the serverParts.
-func isConsulServer(m serf.Member) (bool, *serverParts) {
-	if m.Tags["role"] != "consul" {
-		return false, nil
-	}
-
-	datacenter := m.Tags["dc"]
-	_, bootstrap := m.Tags["bootstrap"]
-	var disabled uint64 = 0
-	_, disabledStr := m.Tags["disabled"]
-	if disabledStr {
-		disabled = 1
-	}
-
-	expect := 0
-	expect_str, ok := m.Tags["expect"]
-	var err error
-	if ok {
-		expect, err = strconv.Atoi(expect_str)
-		if err != nil {
-			return false, nil
-		}
-	}
-
-	port_str := m.Tags["port"]
-	port, err := strconv.Atoi(port_str)
-	if err != nil {
-		return false, nil
-	}
-
-	vsn_str := m.Tags["vsn"]
-	vsn, err := strconv.Atoi(vsn_str)
-	if err != nil {
-		return false, nil
-	}
-
-	addr := &net.TCPAddr{IP: m.Addr, Port: port}
-
-	parts := &serverParts{
-		Name:       m.Name,
-		Datacenter: datacenter,
-		Port:       port,
-		Bootstrap:  bootstrap,
-		Expect:     expect,
-		Addr:       addr,
-		Version:    vsn,
-		Disabled:   disabled,
-	}
-	return true, parts
-}
-
 // Returns if a member is a consul node. Returns a bool,
 // and the datacenter.
 func isConsulNode(m serf.Member) (bool, string) {

consul/util_test.go

@@ -196,60 +196,6 @@ func TestUtil_CanServersUnderstandProtocol(t *testing.T) {
 	}
 }
 
-func TestIsConsulServer(t *testing.T) {
-	m := serf.Member{
-		Name: "foo",
-		Addr: net.IP([]byte{127, 0, 0, 1}),
-		Tags: map[string]string{
-			"role": "consul",
-			"dc":   "east-aws",
-			"port": "10000",
-			"vsn":  "1",
-		},
-	}
-	valid, parts := isConsulServer(m)
-	if !valid || parts.Datacenter != "east-aws" || parts.Port != 10000 {
-		t.Fatalf("bad: %v %v", valid, parts)
-	}
-	if parts.Name != "foo" {
-		t.Fatalf("bad: %v", parts)
-	}
-	if parts.Bootstrap {
-		t.Fatalf("unexpected bootstrap")
-	}
-	if parts.Disabled > 0 {
-		t.Fatalf("unexpected disabled")
-	}
-	if parts.Expect != 0 {
-		t.Fatalf("bad: %v", parts.Expect)
-	}
-	m.Tags["bootstrap"] = "1"
-	m.Tags["disabled"] = "1"
-	valid, parts = isConsulServer(m)
-	if !valid {
-		t.Fatalf("expected a valid consul server")
-	}
-	if !parts.Bootstrap {
-		t.Fatalf("expected bootstrap")
-	}
-	if parts.Disabled == 0 {
-		t.Fatalf("expected disabled")
-	}
-	if parts.Addr.String() != "127.0.0.1:10000" {
-		t.Fatalf("bad addr: %v", parts.Addr)
-	}
-	if parts.Version != 1 {
-		t.Fatalf("bad: %v", parts)
-	}
-	m.Tags["expect"] = "3"
-	delete(m.Tags, "bootstrap")
-	delete(m.Tags, "disabled")
-	valid, parts = isConsulServer(m)
-	if !valid || parts.Expect != 3 {
-		t.Fatalf("bad: %v", parts.Expect)
-	}
-}
-
 func TestIsConsulNode(t *testing.T) {
 	m := serf.Member{
 		Tags: map[string]string{