Merge pull request #4702 from hashicorp/b-non-voter-boostrap

Do not bootstrap with non voters

nomad/serf.go

@@ -118,6 +118,7 @@ func (s *Server) maybeBootstrap() {
 	// Scan for all the known servers
 	members := s.serf.Members()
 	var servers []serverParts
+	voters := 0
 	for _, member := range members {
 		valid, p := isNomadServer(member)
 		if !valid {
@@ -134,11 +135,14 @@ func (s *Server) maybeBootstrap() {
 			s.logger.Error("peer has bootstrap mode. Expect disabled", "member", member)
 			return
 		}
+		if !p.NonVoter {
+			voters++
+		}
 		servers = append(servers, *p)
 	}
 
 	// Skip if we haven't met the minimum expect count
-	if len(servers) < int(atomic.LoadInt32(&s.config.BootstrapExpect)) {
+	if voters < int(atomic.LoadInt32(&s.config.BootstrapExpect)) {
 		return
 	}
 
@@ -200,9 +204,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)
 	}

nomad/serf_test.go

@@ -7,6 +7,7 @@ import (
 	"path"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/nomad/testutil"
 	"github.com/hashicorp/serf/serf"
@@ -298,6 +299,114 @@ func TestNomad_BootstrapExpect(t *testing.T) {
 	}
 }
 
+func TestNomad_BootstrapExpect_NonVoter(t *testing.T) {
+	t.Parallel()
+	dir := tmpDir(t)
+	defer os.RemoveAll(dir)
+
+	s1 := TestServer(t, func(c *Config) {
+		c.BootstrapExpect = 2
+		c.DevMode = false
+		c.DevDisableBootstrap = true
+		c.DataDir = path.Join(dir, "node1")
+		c.NonVoter = true
+	})
+	defer s1.Shutdown()
+	s2 := TestServer(t, func(c *Config) {
+		c.BootstrapExpect = 2
+		c.DevMode = false
+		c.DevDisableBootstrap = true
+		c.DataDir = path.Join(dir, "node2")
+		c.NonVoter = true
+	})
+	defer s2.Shutdown()
+	s3 := TestServer(t, func(c *Config) {
+		c.BootstrapExpect = 2
+		c.DevMode = false
+		c.DevDisableBootstrap = true
+		c.DataDir = path.Join(dir, "node3")
+	})
+	defer s3.Shutdown()
+	TestJoin(t, s1, s2, s3)
+
+	// Assert that we do not bootstrap
+	testutil.AssertUntil(testutil.Timeout(time.Second), func() (bool, error) {
+		_, p := s1.getLeader()
+		if p != nil {
+			return false, fmt.Errorf("leader %v", p)
+		}
+
+		return true, nil
+	}, func(err error) {
+		t.Fatalf("should not have leader: %v", err)
+	})
+
+	// Add the fourth server that is a voter
+	s4 := TestServer(t, func(c *Config) {
+		c.BootstrapExpect = 2
+		c.DevMode = false
+		c.DevDisableBootstrap = true
+		c.DataDir = path.Join(dir, "node4")
+	})
+	defer s4.Shutdown()
+	TestJoin(t, s1, s2, s3, s4)
+
+	testutil.WaitForResult(func() (bool, error) {
+		// Retry the join to decrease flakiness
+		TestJoin(t, s1, s2, s3, s4)
+		peers, err := s1.numPeers()
+		if err != nil {
+			return false, err
+		}
+		if peers != 4 {
+			return false, fmt.Errorf("bad: %#v", peers)
+		}
+		peers, err = s2.numPeers()
+		if err != nil {
+			return false, err
+		}
+		if peers != 4 {
+			return false, fmt.Errorf("bad: %#v", peers)
+		}
+		peers, err = s3.numPeers()
+		if err != nil {
+			return false, err
+		}
+		if peers != 4 {
+			return false, fmt.Errorf("bad: %#v", peers)
+		}
+		peers, err = s4.numPeers()
+		if err != nil {
+			return false, err
+		}
+		if peers != 4 {
+			return false, fmt.Errorf("bad: %#v", peers)
+		}
+
+		if len(s1.localPeers) != 4 {
+			return false, fmt.Errorf("bad: %#v", s1.localPeers)
+		}
+		if len(s2.localPeers) != 4 {
+			return false, fmt.Errorf("bad: %#v", s2.localPeers)
+		}
+		if len(s3.localPeers) != 4 {
+			return false, fmt.Errorf("bad: %#v", s3.localPeers)
+		}
+		if len(s4.localPeers) != 4 {
+			return false, fmt.Errorf("bad: %#v", s3.localPeers)
+		}
+
+		_, p := s1.getLeader()
+		if p == nil {
+			return false, fmt.Errorf("no leader")
+		}
+		return true, nil
+	}, func(err error) {
+		t.Fatalf("err: %v", err)
+	})
+
+}
+
 func TestNomad_BadExpect(t *testing.T) {
 	t.Parallel()
 	s1 := TestServer(t, func(c *Config) {

nomad/util.go

@@ -38,6 +38,7 @@ type serverParts struct {
 	Addr         net.Addr
 	RPCAddr      net.Addr
 	Status       serf.MemberStatus
+	NonVoter     bool
 }
 
 func (s *serverParts) String() string {
@@ -117,6 +118,9 @@ func isNomadServer(m serf.Member) (bool, *serverParts) {
 		}
 	}
 
+	// Check if the server is a non voter
+	_, nonVoter := m.Tags["nonvoter"]
+
 	addr := &net.TCPAddr{IP: m.Addr, Port: port}
 	rpcAddr := &net.TCPAddr{IP: rpcIP, Port: port}
 	parts := &serverParts{
@@ -134,6 +138,7 @@ func isNomadServer(m serf.Member) (bool, *serverParts) {
 		Build:        *buildVersion,
 		RaftVersion:  raftVsn,
 		Status:       m.Status,
+		NonVoter:     nonVoter,
 	}
 	return true, parts
 }

nomad/util_test.go

@@ -25,6 +25,7 @@ func TestIsNomadServer(t *testing.T) {
 			"vsn":      "1",
 			"raft_vsn": "2",
 			"build":    "0.7.0+ent",
+			"nonvoter": "1",
 		},
 	}
 	valid, parts := isNomadServer(m)
@@ -55,6 +56,9 @@ func TestIsNomadServer(t *testing.T) {
 	} else if seg[0] != 0 && seg[1] != 7 && seg[2] != 0 {
 		t.Fatalf("bad: %v", parts.Build)
 	}
+	if !parts.NonVoter {
+		t.Fatalf("should be nonvoter")
+	}
 
 	m.Tags["bootstrap"] = "1"
 	valid, parts = isNomadServer(m)
@@ -74,6 +78,12 @@ func TestIsNomadServer(t *testing.T) {
 	if !valid || parts.Expect != 3 {
 		t.Fatalf("bad: %v", parts.Expect)
 	}
+
+	delete(m.Tags, "nonvoter")
+	valid, parts = isNomadServer(m)
+	if !valid || parts.NonVoter {
+		t.Fatalf("should be a voter")
+	}
 }
 
 func TestServersMeetMinimumVersion(t *testing.T) {