From caed563e088adb6a381b6ff12fc190bf961ad2ea Mon Sep 17 00:00:00 2001 From: Chao Chen Date: Tue, 28 Mar 2023 10:58:53 -0700 Subject: [PATCH] fix flaking auth member remove test Signed-off-by: Chao Chen --- server/etcdserver/server.go | 5 +++-- tests/common/auth_test.go | 28 +++++++++++++++++++------- tests/common/e2e_test.go | 4 ++++ tests/common/integration_test.go | 4 ++++ tests/common/member_test.go | 11 ++++++++++ tests/common/unit_test.go | 4 ++++ tests/framework/integration/cluster.go | 7 +++++++ 7 files changed, 54 insertions(+), 9 deletions(-) diff --git a/server/etcdserver/server.go b/server/etcdserver/server.go index d3edf7cf8..8275bfae8 100644 --- a/server/etcdserver/server.go +++ b/server/etcdserver/server.go @@ -40,6 +40,9 @@ import ( "go.etcd.io/etcd/server/v3/etcdserver/apply" "go.etcd.io/etcd/server/v3/etcdserver/errors" + "go.etcd.io/raft/v3" + "go.etcd.io/raft/v3/raftpb" + pb "go.etcd.io/etcd/api/v3/etcdserverpb" "go.etcd.io/etcd/api/v3/membershippb" "go.etcd.io/etcd/api/v3/version" @@ -69,8 +72,6 @@ import ( "go.etcd.io/etcd/server/v3/storage/backend" "go.etcd.io/etcd/server/v3/storage/mvcc" "go.etcd.io/etcd/server/v3/storage/schema" - "go.etcd.io/raft/v3" - "go.etcd.io/raft/v3/raftpb" ) const ( diff --git a/tests/common/auth_test.go b/tests/common/auth_test.go index 6b54ca296..4722a1a47 100644 --- a/tests/common/auth_test.go +++ b/tests/common/auth_test.go @@ -590,25 +590,39 @@ func TestAuthMemberRemove(t *testing.T) { testRunner.BeforeTest(t) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - clusterSize := 2 + clusterSize := 3 clus := testRunner.NewCluster(ctx, t, config.WithClusterConfig(config.ClusterConfig{ClusterSize: clusterSize})) defer clus.Close() cc := testutils.MustClient(clus.Client()) testutils.ExecuteUntil(ctx, t, func() { + memberIDToEndpoints := getMemberIDToEndpoints(ctx, t, clus) require.NoErrorf(t, setupAuth(cc, []authRole{testRole}, []authUser{rootUser, testUser}), "failed to enable auth") rootAuthClient := testutils.MustClient(clus.Client(WithAuth(rootUserName, rootPassword))) - testUserAuthClient := testutils.MustClient(clus.Client(WithAuth(testUserName, testPassword))) memberId, clusterId := memberToRemove(ctx, t, rootAuthClient, clusterSize) - + delete(memberIDToEndpoints, memberId) + endpoints := make([]string, 0, len(memberIDToEndpoints)) + for _, ep := range memberIDToEndpoints { + endpoints = append(endpoints, ep) + } + testUserAuthClient := testutils.MustClient(clus.Client(WithAuth(testUserName, testPassword))) // ordinary user cannot remove a member _, err := testUserAuthClient.MemberRemove(ctx, memberId) require.ErrorContains(t, err, PermissionDenied) - // root can remove a member - removeResp, err := rootAuthClient.MemberRemove(ctx, memberId) - require.NoError(t, err, "MemberRemove failed") - require.Equal(t, removeResp.Header.ClusterId, clusterId) + // root can remove a member, building a client excluding removed member endpoint + rootAuthClient2 := testutils.MustClient(clus.Client(WithAuth(rootUserName, rootPassword), WithEndpoints(endpoints))) + resp, err := rootAuthClient2.MemberRemove(ctx, memberId) + require.NoError(t, err) + require.Equal(t, resp.Header.ClusterId, clusterId) + found := false + for _, member := range resp.Members { + if member.ID == memberId { + found = true + break + } + } + require.False(t, found, "expect removed member not found in member remove response") }) } diff --git a/tests/common/e2e_test.go b/tests/common/e2e_test.go index fd82064ae..11c4f94a3 100644 --- a/tests/common/e2e_test.go +++ b/tests/common/e2e_test.go @@ -77,3 +77,7 @@ func e2eClusterTestCases() []testCase { func WithAuth(userName, password string) config.ClientOption { return e2e.WithAuth(userName, password) } + +func WithEndpoints(endpoints []string) config.ClientOption { + return e2e.WithEndpoints(endpoints) +} diff --git a/tests/common/integration_test.go b/tests/common/integration_test.go index 9bd686b89..c4cabeeb1 100644 --- a/tests/common/integration_test.go +++ b/tests/common/integration_test.go @@ -55,3 +55,7 @@ func integrationClusterTestCases() []testCase { func WithAuth(userName, password string) config.ClientOption { return integration.WithAuth(userName, password) } + +func WithEndpoints(endpoints []string) config.ClientOption { + return integration.WithEndpoints(endpoints) +} diff --git a/tests/common/member_test.go b/tests/common/member_test.go index 46241ccdd..7bace8197 100644 --- a/tests/common/member_test.go +++ b/tests/common/member_test.go @@ -272,3 +272,14 @@ func memberToRemove(ctx context.Context, t *testing.T, client intf.Client, clust } return memberId, clusterId } + +func getMemberIDToEndpoints(ctx context.Context, t *testing.T, clus intf.Cluster) (memberIDToEndpoints map[uint64]string) { + memberIDToEndpoints = make(map[uint64]string, len(clus.Endpoints())) + for _, ep := range clus.Endpoints() { + cc := testutils.MustClient(clus.Client(WithEndpoints([]string{ep}))) + gresp, err := cc.Get(ctx, "health", config.GetOptions{}) + require.NoError(t, err) + memberIDToEndpoints[gresp.Header.MemberId] = ep + } + return memberIDToEndpoints +} diff --git a/tests/common/unit_test.go b/tests/common/unit_test.go index 3e6f9a9dc..4b172e7a3 100644 --- a/tests/common/unit_test.go +++ b/tests/common/unit_test.go @@ -36,3 +36,7 @@ func unitClusterTestCases() []testCase { func WithAuth(userName, password string) config.ClientOption { return func(any) {} } + +func WithEndpoints(endpoints []string) config.ClientOption { + return func(any) {} +} diff --git a/tests/framework/integration/cluster.go b/tests/framework/integration/cluster.go index 175582737..e63cc0ce9 100644 --- a/tests/framework/integration/cluster.go +++ b/tests/framework/integration/cluster.go @@ -1466,6 +1466,13 @@ func WithAuth(userName, password string) framecfg.ClientOption { } } +func WithEndpoints(endpoints []string) framecfg.ClientOption { + return func(c any) { + cfg := c.(*clientv3.Config) + cfg.Endpoints = endpoints + } +} + func (c *Cluster) newClientCfg() (*clientv3.Config, error) { cfg := &clientv3.Config{ Endpoints: c.Endpoints(),