a NonVoter node should never be able to transition to a Candidate state (#483)

* only voters can transition to a `Candidate` state

* clarify log message and remove not used func `inConfig`

* add tests to make sure leader transition happen as expected when the `HeartbeatTimeout` is reached

* fix race

Author: Dhia Ayachi

configuration.go

@@ -173,17 +173,6 @@ func hasVote(configuration Configuration, id ServerID) bool {
 	return false
 }
 
-// hasVote returns true if the server identified by 'id' is a Voter in the
-// provided Configuration.
-func inConfig(configuration Configuration, id ServerID) bool {
-	for _, server := range configuration.Servers {
-		if server.ID == id {
-			return true
-		}
-	}
-	return false
-}
-
 // checkConfiguration tests a cluster membership configuration for common
 // errors.
 func checkConfiguration(configuration Configuration) error {

raft.go

@@ -214,15 +214,13 @@ func (r *Raft) runFollower() {
 				}
 			} else {
 				metrics.IncrCounter([]string{"raft", "transition", "heartbeat_timeout"}, 1)
-				if inConfig(r.configurations.latest, r.localID) {
+				if hasVote(r.configurations.latest, r.localID) {
 					r.logger.Warn("heartbeat timeout reached, starting election", "last-leader", lastLeader)
 					r.setState(Candidate)
 					return
-				} else {
-					if !didWarn {
-						r.logger.Warn("heartbeat timeout reached, not part of stable configuration, not triggering a leader election")
-						didWarn = true
-					}
+				} else if !didWarn {
+					r.logger.Warn("heartbeat timeout reached, not part of a stable configuration or a non-voter, not triggering a leader election")
+					didWarn = true
 				}
 			}
 

raft_test.go

@@ -2345,3 +2345,47 @@ func TestRaft_InstallSnapshot_InvalidPeers(t *testing.T) {
 	require.Error(t, resp.Error)
 	require.Contains(t, resp.Error.Error(), "failed to decode peers")
 }
+
+func TestRaft_runFollower_State_Transition(t *testing.T) {
+	type fields struct {
+		conf     *Config
+		servers  []Server
+		serverID ServerID
+	}
+	tests := []struct {
+		name          string
+		fields        fields
+		expectedState RaftState
+	}{
+		{"NonVoter", fields{conf: DefaultConfig(), servers: []Server{{Nonvoter, "first", ""}}, serverID: "first"}, Follower},
+		{"Voter", fields{conf: DefaultConfig(), servers: []Server{{Voter, "first", ""}}, serverID: "first"}, Candidate},
+		{"Not in Config", fields{conf: DefaultConfig(), servers: []Server{{Voter, "second", ""}}, serverID: "first"}, Follower},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// set timeout to tests specific
+			tt.fields.conf.LocalID = tt.fields.serverID
+			tt.fields.conf.HeartbeatTimeout = 50 * time.Millisecond
+			tt.fields.conf.ElectionTimeout = 50 * time.Millisecond
+			tt.fields.conf.LeaderLeaseTimeout = 50 * time.Millisecond
+			tt.fields.conf.CommitTimeout = 5 * time.Millisecond
+			tt.fields.conf.SnapshotThreshold = 100
+			tt.fields.conf.TrailingLogs = 10
+			tt.fields.conf.skipStartup = true
+
+			// Create a raft instance and set the latest configuration
+			env1 := MakeRaft(t, tt.fields.conf, false)
+			env1.raft.setLatestConfiguration(Configuration{Servers: tt.fields.servers}, 1)
+			env1.raft.setState(Follower)
+
+			// run the follower loop exclusively
+			go env1.raft.runFollower()
+
+			// wait enough time to have HeartbeatTimeout
+			time.Sleep(tt.fields.conf.HeartbeatTimeout * 3)
+
+			// Check the follower loop set the right state
+			require.Equal(t, tt.expectedState, env1.raft.getState())
+		})
+	}
+}