From 950a9d221269edee486e33971f0eb27d9e780747 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Thu, 23 Feb 2017 13:08:40 -0800 Subject: [PATCH] Move raft_protocol out of autopilot config --- command/agent/config.go | 12 ++++++------ command/agent/config_test.go | 18 +++++++++++++----- consul/config.go | 5 +++-- consul/leader.go | 5 +++-- consul/util.go | 5 +++++ 5 files changed, 30 insertions(+), 15 deletions(-) diff --git a/command/agent/config.go b/command/agent/config.go index 1f77931469..2a78dcd367 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -264,9 +264,6 @@ type Telemetry struct { // Autopilot is used to configure helpful features for operating Consul servers. type Autopilot struct { - // RaftProtocolVersion sets the Raft protocol version to use on this server. - RaftProtocolVersion int `mapstructure:"raft_protocol"` - // DeadServerCleanup enables the automatic cleanup of dead servers when new ones // are added to the peer list. Defaults to true. DeadServerCleanup *bool `mapstructure:"dead_server_cleanup"` @@ -405,6 +402,9 @@ type Config struct { // Protocol is the Consul protocol version to use. Protocol int `mapstructure:"protocol"` + // RaftProtocol sets the Raft protocol version to use on this server. + RaftProtocol int `mapstructure:"raft_protocol"` + // EnableDebug is used to enable various debugging features EnableDebug bool `mapstructure:"enable_debug"` @@ -1299,6 +1299,9 @@ func MergeConfig(a, b *Config) *Config { if b.Protocol > 0 { result.Protocol = b.Protocol } + if b.RaftProtocol != 0 { + result.RaftProtocol = b.RaftProtocol + } if b.NodeID != "" { result.NodeID = b.NodeID } @@ -1347,9 +1350,6 @@ func MergeConfig(a, b *Config) *Config { if b.SkipLeaveOnInt != nil { result.SkipLeaveOnInt = b.SkipLeaveOnInt } - if b.Autopilot.RaftProtocolVersion != 0 { - result.Autopilot.RaftProtocolVersion = b.Autopilot.RaftProtocolVersion - } if b.Autopilot.DeadServerCleanup != nil { result.Autopilot.DeadServerCleanup = b.Autopilot.DeadServerCleanup } diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 1510c3fdd6..cbd00216ac 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -285,6 +285,17 @@ func TestDecodeConfig(t *testing.T) { t.Fatalf("bad: %#v", config) } + // raft protocol + input = `{"raft_protocol": 3}` + config, err = DecodeConfig(bytes.NewReader([]byte(input))) + if err != nil { + t.Fatalf("err: %s", err) + } + + if config.RaftProtocol != 3 { + t.Fatalf("bad: %#v", config) + } + // Node metadata fields input = `{"node_meta": {"thing1": "1", "thing2": "2"}}` config, err = DecodeConfig(bytes.NewReader([]byte(input))) @@ -1099,14 +1110,11 @@ func TestDecodeConfig_Performance(t *testing.T) { } func TestDecodeConfig_Autopilot(t *testing.T) { - input := `{"autopilot": { "raft_protocol": 3, "dead_server_cleanup": true }}` + input := `{"autopilot": { "dead_server_cleanup": true }}` config, err := DecodeConfig(bytes.NewReader([]byte(input))) if err != nil { t.Fatalf("err: %s", err) } - if config.Autopilot.RaftProtocolVersion != 3 { - t.Fatalf("bad: raft_protocol isn't set: %#v", config) - } if config.Autopilot.DeadServerCleanup == nil || !*config.Autopilot.DeadServerCleanup { t.Fatalf("bad: dead_server_cleanup isn't set: %#v", config) } @@ -1633,8 +1641,8 @@ func TestMergeConfig(t *testing.T) { Server: true, LeaveOnTerm: Bool(true), SkipLeaveOnInt: Bool(true), + RaftProtocol: 3, Autopilot: Autopilot{ - RaftProtocolVersion: 3, DeadServerCleanup: Bool(true), }, EnableDebug: true, diff --git a/consul/config.go b/consul/config.go index 01d80711e8..7cfc09470d 100644 --- a/consul/config.go +++ b/consul/config.go @@ -366,8 +366,9 @@ func DefaultConfig() *Config { conf.SerfLANConfig.MemberlistConfig.BindPort = DefaultLANSerfPort conf.SerfWANConfig.MemberlistConfig.BindPort = DefaultWANSerfPort - // Enable interoperability with unversioned Raft library, and don't - // start using new ID-based features yet. + // TODO: default to 3 in Consul 0.9 + // Use a transitional version of the raft protocol to interoperate with + // versions 1 and 3 conf.RaftConfig.ProtocolVersion = 2 conf.ScaleRaft(DefaultRaftMultiplier) diff --git a/consul/leader.go b/consul/leader.go index 8197a5e0fb..93fad4361c 100644 --- a/consul/leader.go +++ b/consul/leader.go @@ -620,16 +620,17 @@ func (s *Server) removeConsulServer(m serf.Member, port int) error { for _, server := range configFuture.Configuration().Servers { // If we understand the new add/remove APIs and the server was added by ID, use the new remove API if minRaftProtocol >= 2 && server.ID == raft.ServerID(parts.ID) { - s.logger.Printf("[INFO] consul: removing server via new api, %q %q", server.ID, server.Address) + s.logger.Printf("[INFO] consul: removing server by ID: %q", server.ID) future := s.raft.RemoveServer(raft.ServerID(parts.ID), 0, 0) if err := future.Error(); err != nil { s.logger.Printf("[ERR] consul: failed to remove raft peer '%v': %v", - addr, err) + server.ID, err) return err } break } else if server.Address == raft.ServerAddress(addr) { // If not, use the old remove API + s.logger.Printf("[INFO] consul: removing server by address: %q", server.Address) future := s.raft.RemovePeer(raft.ServerAddress(addr)) if err := future.Error(); err != nil { s.logger.Printf("[ERR] consul: failed to remove raft peer '%v': %v", diff --git a/consul/util.go b/consul/util.go index 0f5f33b210..f0d56e4533 100644 --- a/consul/util.go +++ b/consul/util.go @@ -112,6 +112,11 @@ func ServerMinRaftProtocol(members []serf.Member) (int, error) { minVersion = raftVsn } } + + if minVersion == -1 { + return minVersion, fmt.Errorf("No servers found") + } + return minVersion, nil }