From 6a5dd09fe6fa20c4528d97a09b764a516cc512ad Mon Sep 17 00:00:00 2001 From: Clark Date: Wed, 7 Sep 2022 01:44:33 +0800 Subject: [PATCH] tests: Migrate member remove tests to common framework Signed-off-by: Clark --- tests/common/member_test.go | 137 +++++++++++++++++++++++++++++--- tests/e2e/ctl_v3_member_test.go | 31 +------- tests/framework/integration.go | 4 + tests/framework/interface.go | 1 + 4 files changed, 130 insertions(+), 43 deletions(-) diff --git a/tests/common/member_test.go b/tests/common/member_test.go index b1f84491c..581cce689 100644 --- a/tests/common/member_test.go +++ b/tests/common/member_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/require" clientv3 "go.etcd.io/etcd/client/v3" "go.etcd.io/etcd/server/v3/etcdserver" + "go.etcd.io/etcd/tests/v3/framework" "go.etcd.io/etcd/tests/v3/framework/testutils" ) @@ -83,25 +84,18 @@ func TestMemberAdd(t *testing.T) { name: "StrictReconfigCheck/WaitForQuorum", strictReconfigCheck: true, waitForQuorum: true, - expectError: false, }, { name: "StrictReconfigCheck/NoWaitForQuorum", strictReconfigCheck: true, - waitForQuorum: false, expectError: true, }, { - name: "DisableStrictReconfigCheck/WaitForQuorum", - strictReconfigCheck: false, - waitForQuorum: true, - expectError: false, + name: "DisableStrictReconfigCheck/WaitForQuorum", + waitForQuorum: true, }, { - name: "DisableStrictReconfigCheck/NoWaitForQuorum", - strictReconfigCheck: false, - waitForQuorum: false, - expectError: false, + name: "DisableStrictReconfigCheck/NoWaitForQuorum", }, } @@ -112,9 +106,7 @@ func TestMemberAdd(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() c := clusterTc.config - if !quorumTc.strictReconfigCheck { - c.DisableStrictReconfigCheck = true - } + c.DisableStrictReconfigCheck = !quorumTc.strictReconfigCheck clus := testRunner.NewCluster(ctx, t, c) defer clus.Close() cc := clus.Client() @@ -152,3 +144,122 @@ func TestMemberAdd(t *testing.T) { } } } + +func TestMemberRemove(t *testing.T) { + testRunner.BeforeTest(t) + + tcs := []struct { + name string + strictReconfigCheck bool + waitForQuorum bool + expectSingleNodeError bool + expectClusterError bool + }{ + { + name: "StrictReconfigCheck/WaitForQuorum", + strictReconfigCheck: true, + waitForQuorum: true, + expectSingleNodeError: true, + }, + { + name: "StrictReconfigCheck/NoWaitForQuorum", + strictReconfigCheck: true, + expectSingleNodeError: true, + expectClusterError: true, + }, + { + name: "DisableStrictReconfigCheck/WaitForQuorum", + waitForQuorum: true, + }, + { + name: "DisableStrictReconfigCheck/NoWaitForQuorum", + }, + } + + for _, quorumTc := range tcs { + for _, clusterTc := range clusterTestCases { + if !quorumTc.strictReconfigCheck && clusterTc.config.ClusterSize == 1 { + // skip these test cases + // when strictReconfigCheck is disabled, calling MemberRemove will cause the single node to panic + continue + } + t.Run(quorumTc.name+"/"+clusterTc.name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + c := clusterTc.config + c.DisableStrictReconfigCheck = !quorumTc.strictReconfigCheck + clus := testRunner.NewCluster(ctx, t, c) + defer clus.Close() + // client connects to a specific member which won't be removed from cluster + cc := clus.Members()[0].Client() + + testutils.ExecuteUntil(ctx, t, func() { + if quorumTc.waitForQuorum { + time.Sleep(etcdserver.HealthInterval) + } + + memberId, clusterId := memberToRemove(ctx, t, cc, c.ClusterSize) + removeResp, err := cc.MemberRemove(ctx, memberId) + + if c.ClusterSize == 1 && quorumTc.expectSingleNodeError { + require.ErrorContains(t, err, "etcdserver: re-configuration failed due to not enough started members") + return + } + + if c.ClusterSize > 1 && quorumTc.expectClusterError { + require.ErrorContains(t, err, "etcdserver: unhealthy cluster") + return + } + + require.NoError(t, err, "MemberRemove failed") + t.Logf("removeResp.Members:%v", removeResp.Members) + if removeResp.Header.ClusterId != clusterId { + t.Fatalf("MemberRemove failed, expected ClusterId: %d, got: %d", clusterId, removeResp.Header.ClusterId) + } + if len(removeResp.Members) != c.ClusterSize-1 { + t.Fatalf("MemberRemove failed, expected length of members: %d, got: %d", c.ClusterSize-1, len(removeResp.Members)) + } + for _, m := range removeResp.Members { + if m.ID == memberId { + t.Fatalf("MemberRemove failed, member(id=%d) is still in cluster", memberId) + } + } + }) + }) + } + } +} + +// memberToRemove chooses a member to remove. +// If clusterSize == 1, return the only member. +// Otherwise, return a member that client has not connected to. +// It ensures that `MemberRemove` function does not return an "etcdserver: server stopped" error. +func memberToRemove(ctx context.Context, t *testing.T, client framework.Client, clusterSize int) (memberId uint64, clusterId uint64) { + listResp, err := client.MemberList(ctx) + if err != nil { + t.Fatal(err) + } + + clusterId = listResp.Header.ClusterId + if clusterSize == 1 { + memberId = listResp.Members[0].ID + } else { + // get status of the specific member that client has connected to + statusResp, err := client.Status(ctx) + if err != nil { + t.Fatal(err) + } + + // choose a member that client has not connected to + for _, m := range listResp.Members { + if m.ID != statusResp[0].Header.MemberId { + memberId = m.ID + break + } + } + if memberId == 0 { + t.Fatalf("memberToRemove failed. listResp:%v, statusResp:%v", listResp, statusResp) + } + } + return memberId, clusterId +} diff --git a/tests/e2e/ctl_v3_member_test.go b/tests/e2e/ctl_v3_member_test.go index 5679be600..480946c5c 100644 --- a/tests/e2e/ctl_v3_member_test.go +++ b/tests/e2e/ctl_v3_member_test.go @@ -28,29 +28,7 @@ import ( func TestCtlV3MemberList(t *testing.T) { testCtl(t, memberListTest) } func TestCtlV3MemberListWithHex(t *testing.T) { testCtl(t, memberListWithHexTest) } -func TestCtlV3MemberRemove(t *testing.T) { - testCtl(t, memberRemoveTest, withQuorum(), withDisableStrictReconfig()) -} -func TestCtlV3MemberRemoveNoTLS(t *testing.T) { - testCtl(t, memberRemoveTest, withQuorum(), withDisableStrictReconfig(), withCfg(*e2e.NewConfigNoTLS())) -} -func TestCtlV3MemberRemoveClientTLS(t *testing.T) { - testCtl(t, memberRemoveTest, withQuorum(), withDisableStrictReconfig(), withCfg(*e2e.NewConfigClientTLS())) -} -func TestCtlV3MemberRemoveClientAutoTLS(t *testing.T) { - testCtl(t, memberRemoveTest, withQuorum(), withDisableStrictReconfig(), withCfg( - // default ClusterSize is 1 - e2e.EtcdProcessClusterConfig{ - ClusterSize: 3, - IsClientAutoTLS: true, - ClientTLS: e2e.ClientTLS, - InitialToken: "new", - })) -} -func TestCtlV3MemberRemovePeerTLS(t *testing.T) { - testCtl(t, memberRemoveTest, withQuorum(), withDisableStrictReconfig(), withCfg(*e2e.NewConfigPeerTLS())) -} -func TestCtlV3MemberUpdate(t *testing.T) { testCtl(t, memberUpdateTest) } +func TestCtlV3MemberUpdate(t *testing.T) { testCtl(t, memberUpdateTest) } func TestCtlV3MemberUpdateNoTLS(t *testing.T) { testCtl(t, memberUpdateTest, withCfg(*e2e.NewConfigNoTLS())) } @@ -154,13 +132,6 @@ func memberListWithHexTest(cx ctlCtx) { } } -func memberRemoveTest(cx ctlCtx) { - ep, memIDToRemove, clusterID := cx.memberToRemove() - if err := ctlV3MemberRemove(cx, ep, memIDToRemove, clusterID); err != nil { - cx.t.Fatal(err) - } -} - func ctlV3MemberRemove(cx ctlCtx, ep, memberID, clusterID string) error { cmdArgs := append(cx.prefixArgs([]string{ep}), "member", "remove", memberID) return e2e.SpawnWithExpectWithEnv(cmdArgs, cx.envMap, fmt.Sprintf("%s removed from cluster %s", memberID, clusterID)) diff --git a/tests/framework/integration.go b/tests/framework/integration.go index ef02591a2..92602ac63 100644 --- a/tests/framework/integration.go +++ b/tests/framework/integration.go @@ -337,3 +337,7 @@ func (c integrationClient) MemberAdd(ctx context.Context, _ string, peerAddrs [] func (c integrationClient) MemberAddAsLearner(ctx context.Context, _ string, peerAddrs []string) (*clientv3.MemberAddResponse, error) { return c.Client.MemberAddAsLearner(ctx, peerAddrs) } + +func (c integrationClient) MemberRemove(ctx context.Context, id uint64) (*clientv3.MemberRemoveResponse, error) { + return c.Client.MemberRemove(ctx, id) +} diff --git a/tests/framework/interface.go b/tests/framework/interface.go index 9c5338b90..f3ff33de0 100644 --- a/tests/framework/interface.go +++ b/tests/framework/interface.go @@ -74,6 +74,7 @@ type Client interface { MemberList(context context.Context) (*clientv3.MemberListResponse, error) MemberAdd(context context.Context, name string, peerAddrs []string) (*clientv3.MemberAddResponse, error) MemberAddAsLearner(context context.Context, name string, peerAddrs []string) (*clientv3.MemberAddResponse, error) + MemberRemove(ctx context.Context, id uint64) (*clientv3.MemberRemoveResponse, error) Watch(ctx context.Context, key string, opts config.WatchOptions) clientv3.WatchChan }