Merge pull request #15580 from chaochn47/fix_flaking_auth_member_remove_test

fix flaking auth member remove test

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 (

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")
 	})
 }
 

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)
+}

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)
+}

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
+}

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) {}
+}

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(),