From 6be1e07fec6bcc369c504c07ad4e55cf823f87a5 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Tue, 30 Aug 2016 11:30:56 -0700 Subject: [PATCH] Makes the Raft configuration API easier to consume. --- api/operator.go | 42 +++++++++++++++-------- api/operator_test.go | 6 ++-- command/agent/operator_endpoint_test.go | 6 ++-- command/operator.go | 11 ++----- consul/operator_endpoint.go | 44 ++++++++++++++----------- consul/operator_endpoint_test.go | 36 ++++++++++++++------ consul/structs/operator.go | 43 ++++++++++++++++-------- 7 files changed, 116 insertions(+), 72 deletions(-) diff --git a/api/operator.go b/api/operator.go index 8d74da364..b39a015be 100644 --- a/api/operator.go +++ b/api/operator.go @@ -14,23 +14,37 @@ func (c *Client) Operator() *Operator { return &Operator{c} } +// RaftServer has information about a server in the Raft configuration. +type RaftServer struct { + // ID is the unique ID for the server. These are currently the same + // as the address, but they will be changed to a real GUID in a future + // release of Consul. + ID raft.ServerID + + // Node is the node name of the server, as known by Consul, or this + // will be set to "(unknown)" otherwise. + Node string + + // Address is the IP:port of the server, used for Raft communications. + Address raft.ServerAddress + + // Leader is true if this server is the current cluster leader. + Leader bool + + // Voter is true if this server has a vote in the cluster. This might + // be false if the server is staging and still coming online, or if + // it's a non-voting server, which will be added in a future release of + // Consul. + Voter bool +} + // RaftConfigration is returned when querying for the current Raft configuration. -// This has the low-level Raft structure, as well as some supplemental -// information from Consul. type RaftConfiguration struct { - // Configuration is the low-level Raft configuration structure. - Configuration raft.Configuration + // Servers has the list of servers in the Raft configuration. + Servers []*RaftServer - // NodeMap maps IDs in the Raft configuration to node names known by - // Consul. It's possible that not all configuration entries may have - // an entry here if the node isn't known to Consul. Given how this is - // generated, this may also contain entries that aren't present in the - // Raft configuration. - NodeMap map[raft.ServerID]string - - // Leader is the ID of the current Raft leader. This may be blank if - // there isn't one. - Leader raft.ServerID + // Index has the Raft index of this configuration. + Index uint64 } // RaftGetConfiguration is used to query the current Raft peer set. diff --git a/api/operator_test.go b/api/operator_test.go index a0d8af69e..f9d242b81 100644 --- a/api/operator_test.go +++ b/api/operator_test.go @@ -15,9 +15,9 @@ func TestOperator_RaftGetConfiguration(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - if len(out.Configuration.Servers) != 1 || - len(out.NodeMap) != 1 || - len(out.Leader) == 0 { + if len(out.Servers) != 1 || + !out.Servers[0].Leader || + !out.Servers[0].Voter { t.Fatalf("bad: %v", out) } } diff --git a/command/agent/operator_endpoint_test.go b/command/agent/operator_endpoint_test.go index 8e3ebe720..bc9b51ad4 100644 --- a/command/agent/operator_endpoint_test.go +++ b/command/agent/operator_endpoint_test.go @@ -30,9 +30,9 @@ func TestOperator_OperatorRaftConfiguration(t *testing.T) { if !ok { t.Fatalf("unexpected: %T", obj) } - if len(out.Configuration.Servers) != 1 || - len(out.NodeMap) != 1 || - len(out.Leader) == 0 { + if len(out.Servers) != 1 || + !out.Servers[0].Leader || + !out.Servers[0].Voter { t.Fatalf("bad: %v", out) } }) diff --git a/command/operator.go b/command/operator.go index d1a6d8d3b..7d6973924 100644 --- a/command/operator.go +++ b/command/operator.go @@ -140,18 +140,13 @@ func (c *OperatorCommand) raft(args []string) error { // Format it as a nice table. result := []string{"Node|ID|Address|State|Voter"} - for _, s := range reply.Configuration.Servers { - node := "(unknown)" - if mappedNode, ok := reply.NodeMap[s.ID]; ok { - node = mappedNode - } + for _, s := range reply.Servers { state := "follower" - if s.ID == reply.Leader { + if s.Leader { state = "leader" } - voter := s.Suffrage == raft.Voter result = append(result, fmt.Sprintf("%s|%s|%s|%s|%v", - node, s.ID, s.Address, state, voter)) + s.Node, s.ID, s.Address, state, s.Voter)) } c.Ui.Output(columnize.SimpleFormat(result)) } else if removePeer { diff --git a/consul/operator_endpoint.go b/consul/operator_endpoint.go index dac7ff748..2add169a2 100644 --- a/consul/operator_endpoint.go +++ b/consul/operator_endpoint.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/consul/consul/agent" "github.com/hashicorp/consul/consul/structs" "github.com/hashicorp/raft" + "github.com/hashicorp/serf/serf" ) // Operator endpoint is used to perform low-level operator tasks for Consul. @@ -35,33 +36,36 @@ func (op *Operator) RaftGetConfiguration(args *structs.DCSpecificRequest, reply if err := future.Error(); err != nil { return err } - reply.Configuration = future.Configuration() - leader := op.srv.raft.Leader() - // Index the configuration so we can easily look up IDs by address. - idMap := make(map[raft.ServerAddress]raft.ServerID) - for _, s := range reply.Configuration.Servers { - idMap[s.Address] = s.ID - } - - // Fill out the node map and leader. - reply.NodeMap = make(map[raft.ServerID]string) - members := op.srv.serfLAN.Members() - for _, member := range members { + // Index the Consul information about the servers. + serverMap := make(map[raft.ServerAddress]*serf.Member) + for _, member := range op.srv.serfLAN.Members() { valid, parts := agent.IsConsulServer(member) if !valid { continue } - // TODO (slackpad) We need to add a Raft API to get the leader by - // ID so we don't have to do this mapping. addr := (&net.TCPAddr{IP: member.Addr, Port: parts.Port}).String() - if id, ok := idMap[raft.ServerAddress(addr)]; ok { - reply.NodeMap[id] = member.Name - if leader == raft.ServerAddress(addr) { - reply.Leader = id - } + serverMap[raft.ServerAddress(addr)] = &member + } + + // Fill out the reply. + leader := op.srv.raft.Leader() + reply.Index = future.Index() + for _, server := range future.Configuration().Servers { + node := "(unknown)" + if member, ok := serverMap[server.Address]; ok { + node = member.Name } + + entry := &structs.RaftServer{ + ID: server.ID, + Node: node, + Address: server.Address, + Leader: server.Address == leader, + Voter: server.Suffrage == raft.Voter, + } + reply.Servers = append(reply.Servers, entry) } return nil } @@ -118,6 +122,6 @@ REMOVE: return err } - op.srv.logger.Printf("[WARN] consul.operator: Removed Raft peer %q by", args.Address) + op.srv.logger.Printf("[WARN] consul.operator: Removed Raft peer %q", args.Address) return nil } diff --git a/consul/operator_endpoint_test.go b/consul/operator_endpoint_test.go index c48ff8381..6fcc1bc7d 100644 --- a/consul/operator_endpoint_test.go +++ b/consul/operator_endpoint_test.go @@ -34,13 +34,21 @@ func TestOperator_RaftGetConfiguration(t *testing.T) { if err := future.Error(); err != nil { t.Fatalf("err: %v", err) } - + if len(future.Configuration().Servers) != 1 { + t.Fatalf("bad: %v", future.Configuration().Servers) + } + me := future.Configuration().Servers[0] expected := structs.RaftConfigurationResponse{ - Configuration: future.Configuration(), - NodeMap: map[raft.ServerID]string{ - raft.ServerID(s1.config.RPCAddr.String()): s1.config.NodeName, + Servers: []*structs.RaftServer{ + &structs.RaftServer{ + ID: me.ID, + Node: s1.config.NodeName, + Address: me.Address, + Leader: true, + Voter: true, + }, }, - Leader: raft.ServerID(s1.config.RPCAddr.String()), + Index: future.Index(), } if !reflect.DeepEqual(reply, expected) { t.Fatalf("bad: %v", reply) @@ -102,13 +110,21 @@ func TestOperator_RaftGetConfiguration_ACLDeny(t *testing.T) { if err := future.Error(); err != nil { t.Fatalf("err: %v", err) } - + if len(future.Configuration().Servers) != 1 { + t.Fatalf("bad: %v", future.Configuration().Servers) + } + me := future.Configuration().Servers[0] expected := structs.RaftConfigurationResponse{ - Configuration: future.Configuration(), - NodeMap: map[raft.ServerID]string{ - raft.ServerID(s1.config.RPCAddr.String()): s1.config.NodeName, + Servers: []*structs.RaftServer{ + &structs.RaftServer{ + ID: me.ID, + Node: s1.config.NodeName, + Address: me.Address, + Leader: true, + Voter: true, + }, }, - Leader: raft.ServerID(s1.config.RPCAddr.String()), + Index: future.Index(), } if !reflect.DeepEqual(reply, expected) { t.Fatalf("bad: %v", reply) diff --git a/consul/structs/operator.go b/consul/structs/operator.go index 83372d131..d564400bf 100644 --- a/consul/structs/operator.go +++ b/consul/structs/operator.go @@ -4,23 +4,38 @@ import ( "github.com/hashicorp/raft" ) +// RaftServer has information about a server in the Raft configuration. +type RaftServer struct { + // ID is the unique ID for the server. These are currently the same + // as the address, but they will be changed to a real GUID in a future + // release of Consul. + ID raft.ServerID + + // Node is the node name of the server, as known by Consul, or this + // will be set to "(unknown)" otherwise. + Node string + + // Address is the IP:port of the server, used for Raft communications. + Address raft.ServerAddress + + // Leader is true if this server is the current cluster leader. + Leader bool + + // Voter is true if this server has a vote in the cluster. This might + // be false if the server is staging and still coming online, or if + // it's a non-voting server, which will be added in a future release of + // Consul. + Voter bool +} + // RaftConfigrationResponse is returned when querying for the current Raft -// configuration. This has the low-level Raft structure, as well as some -// supplemental information from Consul. +// configuration. type RaftConfigurationResponse struct { - // Configuration is the low-level Raft configuration structure. - Configuration raft.Configuration + // Servers has the list of servers in the Raft configuration. + Servers []*RaftServer - // NodeMap maps IDs in the Raft configuration to node names known by - // Consul. It's possible that not all configuration entries may have - // an entry here if the node isn't known to Consul. Given how this is - // generated, this may also contain entries that aren't present in the - // Raft configuration. - NodeMap map[raft.ServerID]string - - // Leader is the ID of the current Raft leader. This may be blank if - // there isn't one. - Leader raft.ServerID + // Index has the Raft index of this configuration. + Index uint64 } // RaftPeerByAddressRequest is used by the Operator endpoint to apply a Raft