From 257319fb188e50aebad6fd6c2635a297e48b7987 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafael=20Fern=C3=A1ndez=20L=C3=B3pez?=
 <ereslibre@ereslibre.es>
Date: Wed, 19 Feb 2020 17:39:01 +0100
Subject: [PATCH] etcdserver: fix quorum calculation when promoting a learner
 member

When promoting a learner member we should not count already a voting
member, but take only into account the number of existing voting
members and their current status (started, unstarted) when taking the
decision whether a learner member can be promoted.

Before this change, it was impossible to grow from a quorum N to a N+1
through promoting a learning member.

Fixes: #11633
---
 etcdserver/api/membership/cluster.go      |  4 +-
 etcdserver/api/membership/cluster_test.go | 87 +++++++++++++++++++++++
 2 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/etcdserver/api/membership/cluster.go b/etcdserver/api/membership/cluster.go
index b1a011b50..d1cf220dd 100644
--- a/etcdserver/api/membership/cluster.go
+++ b/etcdserver/api/membership/cluster.go
@@ -657,8 +657,8 @@ func (c *RaftCluster) IsReadyToRemoveVotingMember(id uint64) bool {
 }
 
 func (c *RaftCluster) IsReadyToPromoteMember(id uint64) bool {
-	nmembers := 1
-	nstarted := 0
+	nmembers := 1 // We count the learner to be promoted for the future quorum
+	nstarted := 1 // and we also count it as started.
 
 	for _, member := range c.VotingMembers() {
 		if member.IsStarted() {
diff --git a/etcdserver/api/membership/cluster_test.go b/etcdserver/api/membership/cluster_test.go
index 55b72ee08..70da655e1 100644
--- a/etcdserver/api/membership/cluster_test.go
+++ b/etcdserver/api/membership/cluster_test.go
@@ -859,3 +859,90 @@ func TestIsReadyToRemoveVotingMember(t *testing.T) {
 		}
 	}
 }
+
+func TestIsReadyToPromoteMember(t *testing.T) {
+	tests := []struct {
+		members   []*Member
+		promoteID uint64
+		want      bool
+	}{
+		{
+			// 1/1 members ready, should succeed (quorum = 1, new quorum = 2)
+			[]*Member{
+				newTestMember(1, nil, "1", nil),
+				newTestMemberAsLearner(2, nil, "2", nil),
+			},
+			2,
+			true,
+		},
+		{
+			// 0/1 members ready, should fail (quorum = 1)
+			[]*Member{
+				newTestMember(1, nil, "", nil),
+				newTestMemberAsLearner(2, nil, "2", nil),
+			},
+			2,
+			false,
+		},
+		{
+			// 2/2 members ready, should succeed (quorum = 2)
+			[]*Member{
+				newTestMember(1, nil, "1", nil),
+				newTestMember(2, nil, "2", nil),
+				newTestMemberAsLearner(3, nil, "3", nil),
+			},
+			3,
+			true,
+		},
+		{
+			// 1/2 members ready, should succeed (quorum = 2)
+			[]*Member{
+				newTestMember(1, nil, "1", nil),
+				newTestMember(2, nil, "", nil),
+				newTestMemberAsLearner(3, nil, "3", nil),
+			},
+			3,
+			true,
+		},
+		{
+			// 1/3 members ready, should fail (quorum = 2)
+			[]*Member{
+				newTestMember(1, nil, "1", nil),
+				newTestMember(2, nil, "", nil),
+				newTestMember(3, nil, "", nil),
+				newTestMemberAsLearner(4, nil, "4", nil),
+			},
+			4,
+			false,
+		},
+		{
+			// 2/3 members ready, should succeed (quorum = 2, new quorum = 3)
+			[]*Member{
+				newTestMember(1, nil, "1", nil),
+				newTestMember(2, nil, "2", nil),
+				newTestMember(3, nil, "", nil),
+				newTestMemberAsLearner(4, nil, "4", nil),
+			},
+			4,
+			true,
+		},
+		{
+			// 2/4 members ready, should succeed (quorum = 3)
+			[]*Member{
+				newTestMember(1, nil, "1", nil),
+				newTestMember(2, nil, "2", nil),
+				newTestMember(3, nil, "", nil),
+				newTestMember(4, nil, "", nil),
+				newTestMemberAsLearner(5, nil, "5", nil),
+			},
+			5,
+			true,
+		},
+	}
+	for i, tt := range tests {
+		c := newTestCluster(tt.members)
+		if got := c.IsReadyToPromoteMember(tt.promoteID); got != tt.want {
+			t.Errorf("%d: isReadyToPromoteMember returned %t, want %t", i, got, tt.want)
+		}
+	}
+}