From 6bf609b96d22b0e98c4f40e1827db4088f4ab3bd Mon Sep 17 00:00:00 2001 From: Jingyi Hu Date: Sun, 5 May 2019 21:02:00 -0700 Subject: [PATCH] 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 | 50 ++++++++++++++++++++++++---- etcdserver/server_test.go | 48 -------------------------- integration/cluster.go | 15 +++++++++ 3 files changed, 59 insertions(+), 54 deletions(-) diff --git a/clientv3/integration/cluster_test.go b/clientv3/integration/cluster_test.go index 5046c71c3..9c02b7163 100644 --- a/clientv3/integration/cluster_test.go +++ b/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) + } } } diff --git a/etcdserver/server_test.go b/etcdserver/server_test.go index 5122a8ade..36550ba4c 100644 --- a/etcdserver/server_test.go +++ b/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() diff --git a/integration/cluster.go b/integration/cluster.go index fb6967a28..73b32cae1 100644 --- a/integration/cluster.go +++ b/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 +}