From eb4d9b640ab2aeeaa731c4c1846df97e733558ee Mon Sep 17 00:00:00 2001
From: Tobias Schottdorf <tobias.schottdorf@gmail.com>
Date: Fri, 19 Jul 2019 17:13:08 +0200
Subject: [PATCH] etcdserver: fix createConfChangeEnts

It created a sequence of conf changes that could intermittently cause an
empty set of voters, which Raft asserts against as of #10889.

This fixes TestCtlV2BackupSnapshot and TestCtlV2BackupV3Snapshot, see:
https://github.com/etcd-io/etcd/issues/10700#issuecomment-512358126
---
 etcdserver/raft.go      | 41 ++++++++++++++++++++++++++---------------
 etcdserver/raft_test.go |  9 ++++-----
 2 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/etcdserver/raft.go b/etcdserver/raft.go
index 75845d7720c..5498630a595 100644
--- a/etcdserver/raft.go
+++ b/etcdserver/raft.go
@@ -686,27 +686,18 @@ func getIDs(lg *zap.Logger, snap *raftpb.Snapshot, ents []raftpb.Entry) []uint64
 // If `self` is not inside the given ids, it creates a Raft entry to add a
 // default member with the given `self`.
 func createConfigChangeEnts(lg *zap.Logger, ids []uint64, self uint64, term, index uint64) []raftpb.Entry {
-	ents := make([]raftpb.Entry, 0)
-	next := index + 1
 	found := false
 	for _, id := range ids {
 		if id == self {
 			found = true
-			continue
-		}
-		cc := &raftpb.ConfChange{
-			Type:   raftpb.ConfChangeRemoveNode,
-			NodeID: id,
 		}
-		e := raftpb.Entry{
-			Type:  raftpb.EntryConfChange,
-			Data:  pbutil.MustMarshal(cc),
-			Term:  term,
-			Index: next,
-		}
-		ents = append(ents, e)
-		next++
 	}
+
+	var ents []raftpb.Entry
+	next := index + 1
+
+	// NB: always add self first, then remove other nodes. Raft will panic if the
+	// set of voters ever becomes empty.
 	if !found {
 		m := membership.Member{
 			ID:             types.ID(self),
@@ -732,6 +723,26 @@ func createConfigChangeEnts(lg *zap.Logger, ids []uint64, self uint64, term, ind
 			Index: next,
 		}
 		ents = append(ents, e)
+		next++
 	}
+
+	for _, id := range ids {
+		if id == self {
+			continue
+		}
+		cc := &raftpb.ConfChange{
+			Type:   raftpb.ConfChangeRemoveNode,
+			NodeID: id,
+		}
+		e := raftpb.Entry{
+			Type:  raftpb.EntryConfChange,
+			Data:  pbutil.MustMarshal(cc),
+			Term:  term,
+			Index: next,
+		}
+		ents = append(ents, e)
+		next++
+	}
+
 	return ents
 }
diff --git a/etcdserver/raft_test.go b/etcdserver/raft_test.go
index 9d731c688e1..fa5830ef4bd 100644
--- a/etcdserver/raft_test.go
+++ b/etcdserver/raft_test.go
@@ -27,7 +27,6 @@ import (
 	"go.etcd.io/etcd/pkg/types"
 	"go.etcd.io/etcd/raft"
 	"go.etcd.io/etcd/raft/raftpb"
-
 	"go.uber.org/zap"
 )
 
@@ -97,7 +96,7 @@ func TestCreateConfigChangeEnts(t *testing.T) {
 			1,
 			1, 1,
 
-			[]raftpb.Entry{},
+			nil,
 		},
 		{
 			[]uint64{1, 2},
@@ -138,9 +137,9 @@ func TestCreateConfigChangeEnts(t *testing.T) {
 			2, 2,
 
 			[]raftpb.Entry{
-				{Term: 2, Index: 3, Type: raftpb.EntryConfChange, Data: pbutil.MustMarshal(removecc2)},
-				{Term: 2, Index: 4, Type: raftpb.EntryConfChange, Data: pbutil.MustMarshal(removecc3)},
-				{Term: 2, Index: 5, Type: raftpb.EntryConfChange, Data: pbutil.MustMarshal(addcc1)},
+				{Term: 2, Index: 3, Type: raftpb.EntryConfChange, Data: pbutil.MustMarshal(addcc1)},
+				{Term: 2, Index: 4, Type: raftpb.EntryConfChange, Data: pbutil.MustMarshal(removecc2)},
+				{Term: 2, Index: 5, Type: raftpb.EntryConfChange, Data: pbutil.MustMarshal(removecc3)},
 			},
 		},
 	}