Fix flaky s3tables tests: allocate all ports atomically

The test port allocation had a TOCTOU race where GetFreePort() would
open a listener, grab the port number, then immediately close it.
When called repeatedly, the OS could recycle a just-released port,
causing two services (e.g. Filer and S3) to be assigned the same port.

Replace per-call GetFreePort() with batch AllocatePorts() that holds
all listeners open until every port is obtained, matching the pattern
already used in test/volume_server/framework/cluster.go.
This commit is contained in:
Chris Lu
2026-04-02 11:58:00 -07:00
parent e93f4e3f39
commit e7fc243ee1
4 changed files with 52 additions and 32 deletions

View File

@@ -115,10 +115,13 @@ func NewTestEnvironment(t *testing.T) *TestEnvironment {
bindIP := testutil.FindBindIP() bindIP := testutil.FindBindIP()
masterPort, masterGrpcPort := testutil.MustFreePortPair(t, "Master") // Allocate all ports in a single batch to prevent the OS from recycling
volumePort, volumeGrpcPort := testutil.MustFreePortPair(t, "Volume") // a released port, which can cause two services to get the same port.
filerPort, filerGrpcPort := testutil.MustFreePortPair(t, "Filer") ports := testutil.MustAllocatePorts(t, 8)
s3Port, s3GrpcPort := testutil.MustFreePortPair(t, "S3") masterPort, masterGrpcPort := ports[0], ports[1]
volumePort, volumeGrpcPort := ports[2], ports[3]
filerPort, filerGrpcPort := ports[4], ports[5]
s3Port, s3GrpcPort := ports[6], ports[7]
return &TestEnvironment{ return &TestEnvironment{
seaweedDir: seaweedDir, seaweedDir: seaweedDir,

View File

@@ -89,11 +89,14 @@ func NewTestEnvironment(t *testing.T) *TestEnvironment {
bindIP := testutil.FindBindIP() bindIP := testutil.FindBindIP()
masterPort, masterGrpcPort := testutil.MustFreePortPair(t, "Master") // Allocate all ports in a single batch to prevent the OS from recycling
volumePort, volumeGrpcPort := testutil.MustFreePortPair(t, "Volume") // a released port, which can cause two services to get the same port.
filerPort, filerGrpcPort := testutil.MustFreePortPair(t, "Filer") ports := testutil.MustAllocatePorts(t, 10)
s3Port, s3GrpcPort := testutil.MustFreePortPair(t, "S3") masterPort, masterGrpcPort := ports[0], ports[1]
polarisPort, polarisAdminPort := testutil.MustFreePortPair(t, "Polaris") volumePort, volumeGrpcPort := ports[2], ports[3]
filerPort, filerGrpcPort := ports[4], ports[5]
s3Port, s3GrpcPort := ports[6], ports[7]
polarisPort, polarisAdminPort := ports[8], ports[9]
return &TestEnvironment{ return &TestEnvironment{
seaweedDir: seaweedDir, seaweedDir: seaweedDir,

View File

@@ -93,10 +93,13 @@ func NewTestEnvironment(t *testing.T) *TestEnvironment {
bindIP := testutil.FindBindIP() bindIP := testutil.FindBindIP()
masterPort, masterGrpcPort := testutil.MustFreePortPair(t, "Master") // Allocate all ports in a single batch to prevent the OS from recycling
volumePort, volumeGrpcPort := testutil.MustFreePortPair(t, "Volume") // a released port, which can cause two services to get the same port.
filerPort, filerGrpcPort := testutil.MustFreePortPair(t, "Filer") ports := testutil.MustAllocatePorts(t, 8)
s3Port, s3GrpcPort := testutil.MustFreePortPair(t, "S3") // Changed to use testutil.MustFreePortPair masterPort, masterGrpcPort := ports[0], ports[1]
volumePort, volumeGrpcPort := ports[2], ports[3]
filerPort, filerGrpcPort := ports[4], ports[5]
s3Port, s3GrpcPort := ports[6], ports[7]
return &TestEnvironment{ return &TestEnvironment{
seaweedDir: seaweedDir, seaweedDir: seaweedDir,

View File

@@ -15,33 +15,44 @@ func HasDocker() bool {
return cmd.Run() == nil return cmd.Run() == nil
} }
// MustFreePortPair is a convenience wrapper for tests that only need a single pair.
// Prefer MustAllocatePorts when allocating multiple pairs to guarantee uniqueness.
func MustFreePortPair(t *testing.T, name string) (int, int) { func MustFreePortPair(t *testing.T, name string) (int, int) {
httpPort, grpcPort, err := findAvailablePortPair() ports := MustAllocatePorts(t, 2)
if err != nil { return ports[0], ports[1]
t.Fatalf("Failed to get free port pair for %s: %v", name, err)
}
return httpPort, grpcPort
} }
func findAvailablePortPair() (int, int, error) { // MustAllocatePorts allocates count unique free ports atomically.
httpPort, err := GetFreePort() // All listeners are held open until every port is obtained, preventing
// the OS from recycling a port between successive allocations.
func MustAllocatePorts(t *testing.T, count int) []int {
t.Helper()
ports, err := AllocatePorts(count)
if err != nil { if err != nil {
return 0, 0, err t.Fatalf("Failed to allocate %d free ports: %v", count, err)
} }
grpcPort, err := GetFreePort() return ports
if err != nil {
return 0, 0, err
}
return httpPort, grpcPort, nil
} }
func GetFreePort() (int, error) { // AllocatePorts allocates count unique free ports atomically.
listener, err := net.Listen("tcp", "0.0.0.0:0") func AllocatePorts(count int) ([]int, error) {
if err != nil { listeners := make([]net.Listener, 0, count)
return 0, err ports := make([]int, 0, count)
for i := 0; i < count; i++ {
l, err := net.Listen("tcp", "0.0.0.0:0")
if err != nil {
for _, ll := range listeners {
_ = ll.Close()
}
return nil, err
}
listeners = append(listeners, l)
ports = append(ports, l.Addr().(*net.TCPAddr).Port)
} }
defer listener.Close() for _, l := range listeners {
return listener.Addr().(*net.TCPAddr).Port, nil _ = l.Close()
}
return ports, nil
} }
func WaitForService(url string, timeout time.Duration) bool { func WaitForService(url string, timeout time.Duration) bool {