Fix some inconsistencies with segment logic and comments

agent/agent.go

@@ -58,7 +58,7 @@ type delegate interface {
 	GetLANCoordinate() (lib.CoordinateSet, error)
 	Leave() error
 	LANMembers() []serf.Member
-	LANSegmentMembers(name string) ([]serf.Member, error)
+	LANSegmentMembers(segment string) ([]serf.Member, error)
 	LocalMember() serf.Member
 	JoinLAN(addrs []string) (n int, err error)
 	RemoveFailedNode(node string) error
@@ -812,9 +812,9 @@ func (a *Agent) segmentConfig() ([]consul.NetworkSegment, error) {
 
 		segments = append(segments, consul.NetworkSegment{
 			Name:       segment.Name,
-			Bind:       segment.Bind,
+			Bind:       serfConf.MemberlistConfig.BindAddr,
 			Port:       segment.Port,
-			Advertise:  segment.Advertise,
+			Advertise:  serfConf.MemberlistConfig.AdvertiseAddr,
 			RPCAddr:    rpcAddr,
 			SerfConfig: serfConf,
 		})
@@ -2169,10 +2169,7 @@ func (a *Agent) loadMetadata(conf *Config) error {
 		a.state.metadata[key] = value
 	}
 
-	// The segment isn't reloadable so we only add it once.
-	if _, ok := a.state.metadata[structs.MetaSegmentKey]; !ok {
-		a.state.metadata[structs.MetaSegmentKey] = conf.Segment
-	}
+	a.state.metadata[structs.MetaSegmentKey] = conf.Segment
 
 	a.state.changeMade()
 

agent/agent_endpoint.go

@@ -166,9 +166,13 @@ func (s *HTTPServer) AgentMembers(resp http.ResponseWriter, req *http.Request) (
 	var members []serf.Member
 	if wan {
 		members = s.agent.WANMembers()
-	} else if segment == "" {
-		members = s.agent.LANMembers()
 	} else {
+		// If the segment is blank when querying a client, use the agent's
+		// segment instead of the empty string.
+		if !s.agent.config.Server && segment == "" {
+			segment = s.agent.config.Segment
+		}
+
 		var err error
 		members, err = s.agent.delegate.LANSegmentMembers(segment)
 		if err != nil {

agent/config.go

@@ -493,10 +493,11 @@ type Config struct {
 	// Address configurations
 	Addresses AddressConfig
 
-	// (Enterprise-only) NetworkSegment is the network segment for this client to join
+	// (Enterprise-only) NetworkSegment is the network segment for this client to join.
 	Segment string `mapstructure:"segment"`
 
-	// Segments
+	// (Enterprise-only) Segments is the list of network segments for this server to
+	// initialize.
 	Segments []NetworkSegment `mapstructure:"segments"`
 
 	// Tagged addresses. These are used to publish a set of addresses for
@@ -1480,6 +1481,10 @@ func ValidateSegments(conf *Config) error {
 
 	takenPorts := make(map[int]string, len(conf.Segments))
 	for _, segment := range conf.Segments {
+		if segment.Name == "" {
+			return fmt.Errorf("Segment name cannot be blank")
+		}
+
 		if len(segment.Name) > SegmentNameLimit {
 			return fmt.Errorf("Segment name %q exceeds maximum length of %d", segment.Name, SegmentNameLimit)
 		}

agent/config_test.go

@@ -1325,6 +1325,13 @@ func TestDecodeConfig_ValidateSegments(t *testing.T) {
 		t.Fatalf("bad: %v", err)
 	}
 
+	if err := ValidateSegments(&Config{
+		Segments: []NetworkSegment{{Name: ""}},
+		Server:   true,
+	}); !strings.Contains(err.Error(), "Segment name cannot be blank") {
+		t.Fatalf("bad: %v", err)
+	}
+
 	segmentNameTooLong := &Config{
 		Segments: []NetworkSegment{{Name: strings.Repeat("a", SegmentNameLimit+1)}},
 		Server:   true,

agent/consul/client.go

@@ -196,12 +196,12 @@ func (c *Client) LANMembers() []serf.Member {
 
 // LANSegmentMembers only returns our own segment's members, because clients
 // can't be in multiple segments.
-func (c *Client) LANSegmentMembers(name string) ([]serf.Member, error) {
-	if name == c.config.Segment {
+func (c *Client) LANSegmentMembers(segment string) ([]serf.Member, error) {
+	if segment == c.config.Segment {
 		return c.LANMembers(), nil
 	}
 
-	return nil, fmt.Errorf("segment %q not found", name)
+	return nil, fmt.Errorf("segment %q not found", segment)
 }
 
 // RemoveFailedNode is used to remove a failed node from the cluster

agent/consul/config.go

@@ -49,7 +49,8 @@ func init() {
 	}
 }
 
-// (Enterprise-only)
+// (Enterprise-only) NetworkSegment is the address and port configuration
+// for a network segment.
 type NetworkSegment struct {
 	Name       string
 	Bind       string

agent/consul/internal_endpoint.go

@@ -89,11 +89,6 @@ func (m *Internal) EventFire(args *structs.EventFireRequest,
 	// Fire the event on all LAN segments
 	segments := m.srv.LANSegments()
 	var errs error
-	err = m.srv.serfLAN.UserEvent(eventName, args.Payload, false)
-	if err != nil {
-		err = fmt.Errorf("error broadcasting event to default segment: %v", err)
-		errs = multierror.Append(errs, err)
-	}
 	for name, segment := range segments {
 		err := segment.UserEvent(eventName, args.Payload, false)
 		if err != nil {
@@ -157,7 +152,6 @@ func (m *Internal) executeKeyringOp(
 		m.executeKeyringOpMgr(mgr, args, reply, wan)
 	} else {
 		segments := m.srv.LANSegments()
-		m.executeKeyringOpMgr(m.srv.KeyManagerLAN(), args, reply, wan)
 		for _, segment := range segments {
 			mgr := segment.KeyManager()
 			m.executeKeyringOpMgr(mgr, args, reply, wan)

agent/consul/leader.go

@@ -63,9 +63,6 @@ func (s *Server) monitorLeadership() {
 func (s *Server) leaderLoop(stopCh chan struct{}) {
 	// Fire a user event indicating a new leader
 	payload := []byte(s.config.NodeName)
-	if err := s.serfLAN.UserEvent(newLeaderEvent, payload, false); err != nil {
-		s.logger.Printf("[WARN] consul: failed to broadcast new leader event on default segment: %v", err)
-	}
 	for name, segment := range s.LANSegments() {
 		if err := segment.UserEvent(newLeaderEvent, payload, false); err != nil {
 			s.logger.Printf("[WARN] consul: failed to broadcast new leader event on segment %q: %v", name, err)

agent/consul/segment_stub.go

@@ -17,8 +17,8 @@ var (
 )
 
 // LANSegmentMembers is used to return the members of the given LAN segment.
-func (s *Server) LANSegmentMembers(name string) ([]serf.Member, error) {
-	if name == "" {
+func (s *Server) LANSegmentMembers(segment string) ([]serf.Member, error) {
+	if segment == "" {
 		return s.LANMembers(), nil
 	}
 

agent/consul/server.go

@@ -913,7 +913,8 @@ func (s *Server) LANSegments() map[string]*serf.Serf {
 	s.segmentLock.RLock()
 	defer s.segmentLock.RUnlock()
 
-	segments := make(map[string]*serf.Serf, len(s.segmentLAN))
+	segments := make(map[string]*serf.Serf, len(s.segmentLAN)+1)
+	segments[""] = s.serfLAN
 	for name, segment := range s.segmentLAN {
 		segments[name] = segment
 	}

command/agent.go

@@ -400,7 +400,7 @@ func (cmd *AgentCommand) readConfig() *agent.Config {
 	}
 
 	if !cfg.Server && len(cfg.Segments) > 0 {
-		cmd.UI.Error("Cannot define segments on clients")
+		cmd.UI.Error("Segments can only be configured on servers")
 		return nil
 	}