From 43d0f96c42327f0e6cc9eca0a1a9fee1ef4df506 Mon Sep 17 00:00:00 2001
From: Alex Dadgar <alex.dadgar@gmail.com>
Date: Wed, 19 Sep 2018 17:41:36 -0700
Subject: [PATCH] do not bootstrap with non voters

---
 agent/consul/server_serf.go   | 15 ++++++++--
 agent/consul/server_test.go   | 56 +++++++++++++++++++++++++++++++++++
 agent/metadata/server.go      |  5 ++++
 agent/metadata/server_test.go | 10 +++++++
 4 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/agent/consul/server_serf.go b/agent/consul/server_serf.go
index e0781d912f..8d021e8714 100644
--- a/agent/consul/server_serf.go
+++ b/agent/consul/server_serf.go
@@ -245,6 +245,7 @@ func (s *Server) maybeBootstrap() {
 	// Scan for all the known servers.
 	members := s.serfLAN.Members()
 	var servers []metadata.Server
+	voters := 0
 	for _, member := range members {
 		valid, p := metadata.IsConsulServer(member)
 		if !valid {
@@ -262,11 +263,14 @@ func (s *Server) maybeBootstrap() {
 			s.logger.Printf("[ERR] consul: Member %v has bootstrap mode. Expect disabled.", member)
 			return
 		}
+		if !p.NonVoter {
+			voters++
+		}
 		servers = append(servers, *p)
 	}
 
 	// Skip if we haven't met the minimum expect count.
-	if len(servers) < s.config.BootstrapExpect {
+	if voters < s.config.BootstrapExpect {
 		return
 	}
 
@@ -322,9 +326,14 @@ func (s *Server) maybeBootstrap() {
 		} else {
 			id = raft.ServerID(addr)
 		}
+		suffrage := raft.Voter
+		if server.NonVoter {
+			suffrage = raft.Nonvoter
+		}
 		peer := raft.Server{
-			ID:      id,
-			Address: raft.ServerAddress(addr),
+			ID:       id,
+			Address:  raft.ServerAddress(addr),
+			Suffrage: suffrage,
 		}
 		configuration.Servers = append(configuration.Servers, peer)
 	}
diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go
index bbd3ef60d9..31dbf32ac0 100644
--- a/agent/consul/server_test.go
+++ b/agent/consul/server_test.go
@@ -137,6 +137,15 @@ func testServerDCExpect(t *testing.T, dc string, expect int) (string, *Server) {
 	})
 }
 
+func testServerDCExpectNonVoter(t *testing.T, dc string, expect int) (string, *Server) {
+	return testServerWithConfig(t, func(c *Config) {
+		c.Datacenter = dc
+		c.Bootstrap = false
+		c.BootstrapExpect = expect
+		c.NonVoter = true
+	})
+}
+
 func testServerWithConfig(t *testing.T, cb func(*Config)) (string, *Server) {
 	dir, config := testServerConfig(t)
 	if cb != nil {
@@ -579,6 +588,53 @@ func TestServer_Expect(t *testing.T) {
 	}
 }
 
+func TestServer_Expect_NonVoters(t *testing.T) {
+	t.Parallel()
+	dir1, s1 := testServerDCExpectNonVoter(t, "dc1", 2)
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+
+	dir2, s2 := testServerDCExpectNonVoter(t, "dc1", 2)
+	defer os.RemoveAll(dir2)
+	defer s2.Shutdown()
+
+	dir3, s3 := testServerDCExpect(t, "dc1", 2)
+	defer os.RemoveAll(dir3)
+	defer s3.Shutdown()
+
+	dir4, s4 := testServerDCExpect(t, "dc1", 2)
+	defer os.RemoveAll(dir4)
+	defer s4.Shutdown()
+
+	// Join the first three servers.
+	joinLAN(t, s2, s1)
+	joinLAN(t, s3, s1)
+
+	// Should have no peers yet since the bootstrap didn't occur.
+	retry.Run(t, func(r *retry.R) {
+		r.Check(wantPeers(s1, 0))
+		r.Check(wantPeers(s2, 0))
+		r.Check(wantPeers(s3, 0))
+	})
+
+	// Join the fourth node.
+	joinLAN(t, s4, s1)
+
+	// Now we have three servers so we should bootstrap.
+	retry.Run(t, func(r *retry.R) {
+		r.Check(wantPeers(s1, 4))
+		r.Check(wantPeers(s2, 4))
+		r.Check(wantPeers(s3, 4))
+		r.Check(wantPeers(s4, 4))
+	})
+
+	// Make sure a leader is elected
+	testrpc.WaitForLeader(t, s1.RPC, "dc1")
+	retry.Run(t, func(r *retry.R) {
+		r.Check(wantRaft([]*Server{s1, s2, s3, s4}))
+	})
+}
+
 func TestServer_BadExpect(t *testing.T) {
 	t.Parallel()
 	// this one is in expect=3 mode
diff --git a/agent/metadata/server.go b/agent/metadata/server.go
index 957de88f05..d83b7c1a80 100644
--- a/agent/metadata/server.go
+++ b/agent/metadata/server.go
@@ -38,6 +38,7 @@ type Server struct {
 	RaftVersion  int
 	Addr         net.Addr
 	Status       serf.MemberStatus
+	NonVoter     bool
 
 	// If true, use TLS when connecting to this server
 	UseTLS bool
@@ -139,6 +140,9 @@ func IsConsulServer(m serf.Member) (bool, *Server) {
 		}
 	}
 
+	// Check if the server is a non voter
+	_, nonVoter := m.Tags["nonvoter"]
+
 	addr := &net.TCPAddr{IP: m.Addr, Port: port}
 
 	parts := &Server{
@@ -158,6 +162,7 @@ func IsConsulServer(m serf.Member) (bool, *Server) {
 		RaftVersion:  raftVsn,
 		Status:       m.Status,
 		UseTLS:       useTLS,
+		NonVoter:     nonVoter,
 	}
 	return true, parts
 }
diff --git a/agent/metadata/server_test.go b/agent/metadata/server_test.go
index bd67115242..1b3f777630 100644
--- a/agent/metadata/server_test.go
+++ b/agent/metadata/server_test.go
@@ -65,6 +65,7 @@ func TestIsConsulServer(t *testing.T) {
 			"expect":        "3",
 			"raft_vsn":      "3",
 			"use_tls":       "1",
+			"nonvoter":      "1",
 		},
 		Status: serf.StatusLeft,
 	}
@@ -99,6 +100,9 @@ func TestIsConsulServer(t *testing.T) {
 	if !parts.UseTLS {
 		t.Fatalf("bad: %v", parts.UseTLS)
 	}
+	if !parts.NonVoter {
+		t.Fatalf("unexpected voter")
+	}
 	m.Tags["bootstrap"] = "1"
 	m.Tags["disabled"] = "1"
 	ok, parts = metadata.IsConsulServer(m)
@@ -125,6 +129,12 @@ func TestIsConsulServer(t *testing.T) {
 		t.Fatalf("unexpected bootstrap")
 	}
 
+	delete(m.Tags, "nonvoter")
+	ok, parts = metadata.IsConsulServer(m)
+	if !ok || parts.NonVoter {
+		t.Fatalf("unexpected nonvoter")
+	}
+
 	delete(m.Tags, "role")
 	ok, parts = metadata.IsConsulServer(m)
 	if ok {