integration: update TestMemberPromote test

Update TestMemberPromote to include both learner not-ready and learner
ready test cases.

Removed unit test TestPromoteMember, it requires underlying raft node to
be started and running. The member promote is covered by the integration
test.

clientv3/integration/cluster_test.go

@@ -19,6 +19,7 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 
 	"go.etcd.io/etcd/integration"
 	"go.etcd.io/etcd/pkg/testutil"
@@ -214,13 +215,19 @@ func TestMemberAddForLearner(t *testing.T) {
 	}
 }
 
-func TestMemberPromoteForNotReadyLearner(t *testing.T) {
+func TestMemberPromote(t *testing.T) {
 	defer testutil.AfterTest(t)
 
-	clus := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 1})
+	clus := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 3})
 	defer clus.Terminate(t)
-	// first client is talked to leader because cluster size is 1
-	capi := clus.Client(0)
+
+	// member promote request can be sent to any server in cluster,
+	// the request will be auto-forwarded to leader on server-side.
+	// This test explicitly includes the server-side forwarding by
+	// sending the request to follower.
+	leaderIdx := clus.WaitLeader(t)
+	followerIdx := (leaderIdx + 1) % 3
+	capi := clus.Client(followerIdx)
 
 	urls := []string{"http://127.0.0.1:1234"}
 	memberAddResp, err := capi.MemberAddAsLearner(context.Background(), urls)
@@ -243,14 +250,45 @@ func TestMemberPromoteForNotReadyLearner(t *testing.T) {
 		t.Fatalf("Added 1 learner node to cluster, got %d", numberOfLearners)
 	}
 
-	// since we do not start learner, learner must be not ready.
+	// learner is not started yet. Expect learner progress check to fail.
+	// As the result, member promote request will fail.
 	_, err = capi.MemberPromote(context.Background(), learnerID)
 	expectedErrKeywords := "can only promote a learner member which is in sync with leader"
 	if err == nil {
 		t.Fatalf("expecting promote not ready learner to fail, got no error")
 	}
 	if !strings.Contains(err.Error(), expectedErrKeywords) {
-		t.Errorf("expecting error to contain %s, got %s", expectedErrKeywords, err.Error())
+		t.Fatalf("expecting error to contain %s, got %s", expectedErrKeywords, err.Error())
+	}
+
+	// create and launch learner member based on the response of V3 Member Add API.
+	// (the response has information on peer urls of the existing members in cluster)
+	learnerMember := clus.MustNewMember(t, memberAddResp)
+	clus.Members = append(clus.Members, learnerMember)
+	if err := learnerMember.Launch(); err != nil {
+		t.Fatal(err)
+	}
+
+	// retry until promote succeed or timeout
+	timeout := time.After(5 * time.Second)
+	for {
+		select {
+		case <-time.After(500 * time.Millisecond):
+		case <-timeout:
+			t.Errorf("failed all attempts to promote learner member, last error: %v", err)
+			break
+		}
+
+		_, err = capi.MemberPromote(context.Background(), learnerID)
+		// successfully promoted learner
+		if err == nil {
+			break
+		}
+		// if member promote fails due to learner not ready, retry.
+		// otherwise fails the test.
+		if !strings.Contains(err.Error(), expectedErrKeywords) {
+			t.Fatalf("unexpected error when promoting learner member: %v", err)
+		}
 	}
 }
 

etcdserver/server_test.go

@@ -1340,54 +1340,6 @@ func TestRemoveMember(t *testing.T) {
 	}
 }
 
-// TestPromoteMember tests PromoteMember can propose and perform learner node promotion.
-func TestPromoteMember(t *testing.T) {
-	n := newNodeConfChangeCommitterRecorder()
-	n.readyc <- raft.Ready{
-		SoftState: &raft.SoftState{RaftState: raft.StateLeader},
-	}
-	cl := newTestCluster(nil)
-	st := v2store.New()
-	cl.SetStore(v2store.New())
-	cl.AddMember(&membership.Member{
-		ID: 1234,
-		RaftAttributes: membership.RaftAttributes{
-			IsLearner: true,
-		},
-	})
-	r := newRaftNode(raftNodeConfig{
-		lg:          zap.NewExample(),
-		Node:        n,
-		raftStorage: raft.NewMemoryStorage(),
-		storage:     mockstorage.NewStorageRecorder(""),
-		transport:   newNopTransporter(),
-	})
-	s := &EtcdServer{
-		lgMu:       new(sync.RWMutex),
-		lg:         zap.NewExample(),
-		r:          *r,
-		v2store:    st,
-		cluster:    cl,
-		reqIDGen:   idutil.NewGenerator(0, time.Time{}),
-		SyncTicker: &time.Ticker{},
-	}
-	s.start()
-	_, err := s.PromoteMember(context.TODO(), 1234)
-	gaction := n.Action()
-	s.Stop()
-
-	if err != nil {
-		t.Fatalf("PromoteMember error: %v", err)
-	}
-	wactions := []testutil.Action{{Name: "ProposeConfChange:ConfChangeAddNode"}, {Name: "ApplyConfChange:ConfChangeAddNode"}}
-	if !reflect.DeepEqual(gaction, wactions) {
-		t.Errorf("action = %v, want %v", gaction, wactions)
-	}
-	if cl.Member(1234).IsLearner {
-		t.Errorf("member with id 1234 is not promoted")
-	}
-}
-
 // TestUpdateMember tests RemoveMember can propose and perform node update.
 func TestUpdateMember(t *testing.T) {
 	n := newNodeConfChangeCommitterRecorder()

integration/cluster.go

@@ -1396,3 +1396,18 @@ func (p SortableProtoMemberSliceByPeerURLs) Less(i, j int) bool {
 	return p[i].PeerURLs[0] < p[j].PeerURLs[0]
 }
 func (p SortableProtoMemberSliceByPeerURLs) Swap(i, j int) { p[i], p[j] = p[j], p[i] }
+
+// MustNewMember creates a new member instance based on the response of V3 Member Add API.
+func (c *ClusterV3) MustNewMember(t testing.TB, resp *clientv3.MemberAddResponse) *member {
+	m := c.mustNewMember(t)
+	m.isLearner = resp.Member.IsLearner
+	m.NewCluster = false
+
+	m.InitialPeerURLsMap = types.URLsMap{}
+	for _, mm := range c.Members {
+		m.InitialPeerURLsMap[mm.Name] = mm.PeerURLs
+	}
+	m.InitialPeerURLsMap[m.Name] = types.MustNewURLs(resp.Member.PeerURLs)
+
+	return m
+}