fix(test/s3/policy): allocate fresh admin port per subtest (#9332)

* fix(test/s3/policy): allocate fresh admin port per subtest

startMiniCluster ran weed mini in-process and explicitly assigned
master/volume/filer/s3 ports allocated by MustAllocatePorts, but it
left -admin.port and -admin.port.grpc unset, so each subtest reused
the hardcoded defaults 23646 / 33646.

The package's subtests run sequentially within the same go test
process. The previous subtest's admin goroutine is still bound to
23646 by the time the next subtest spins up its own mini, so the
new admin can never bind, mini.go's waitForAdminServerReady hits
its 240-attempt cap, and glog.Fatalf kills the test binary. This
has been the dominant cause of "admin server did not become ready"
flakes across recent IAM PRs.

Allocate two extra ports for admin and pass them through. The other
subprocess-based tests (s3tables/*) are not affected because each
launches weed mini in a fresh OS process.

* fix(mini): make admin readiness wait context-aware

waitForAdminServerReady polled for 240 attempts × 500ms regardless of
whether the surrounding mini context was cancelled. When mini is run
in-process from a test harness (test/s3/policy/...) and the test calls
its cancel func, the leftover wait keeps spinning for the full two
minutes and then glog.Fatalf's, terminating the entire test binary —
including any sibling subtest that has since started its own mini.

Thread the existing miniClientsCtx through the wait so a Stop / cancel
returns context.Canceled immediately. The caller (startMiniAdminWithWorker)
treats a context-cancelled outcome as a graceful shutdown signal and
logs+returns instead of fataling.
This commit is contained in:
Chris Lu
2026-05-05 11:24:43 -07:00
committed by GitHub
parent 95560076e6
commit 6141222ab0
2 changed files with 33 additions and 5 deletions
+10 -1
View File
@@ -704,11 +704,18 @@ func uniqueName(prefix string) string {
// --- Test setup helpers ---
func startMiniCluster(t *testing.T) (*TestCluster, error) {
ports := testutil.MustAllocatePorts(t, 8)
// Allocate two extra ports for admin HTTP + gRPC. Without explicit
// values mini falls back to 23646/33646, which collide across the
// sequential subtests in this package: each subtest spins up a fresh
// mini in a goroutine, but the previous mini's admin goroutine is
// still binding 23646 — so the next test's admin can never become
// ready and mini.go fatals on its readiness check.
ports := testutil.MustAllocatePorts(t, 10)
masterPort, masterGrpcPort := ports[0], ports[1]
volumePort, volumeGrpcPort := ports[2], ports[3]
filerPort, filerGrpcPort := ports[4], ports[5]
s3Port, s3GrpcPort := ports[6], ports[7]
adminPort, adminGrpcPort := ports[8], ports[9]
// Manually-managed temp dir (not t.TempDir()) so we control removal order:
// the dir is removed inside Stop() AFTER the mini goroutine has fully
@@ -779,6 +786,8 @@ enabled = true
"-filer.port.grpc=" + strconv.Itoa(filerGrpcPort),
"-s3.port=" + strconv.Itoa(s3Port),
"-s3.port.grpc=" + strconv.Itoa(s3GrpcPort),
"-admin.port=" + strconv.Itoa(adminPort),
"-admin.port.grpc=" + strconv.Itoa(adminGrpcPort),
"-webdav.port=0",
"-admin.ui=false",
"-master.volumeSizeLimitMB=32",
+23 -4
View File
@@ -1253,7 +1253,15 @@ func startMiniAdminWithWorker(allServicesReady chan struct{}) {
// Wait for admin server's HTTP port to be ready before launching worker
adminAddr := fmt.Sprintf("http://%s:%d", bindIp, *miniAdminOptions.port)
glog.V(1).Infof("Waiting for admin server to be ready at %s...", adminAddr)
if err := waitForAdminServerReady(adminAddr); err != nil {
if err := waitForAdminServerReady(ctx, adminAddr); err != nil {
// If the parent context was cancelled (e.g. a previous in-process
// mini run is being torn down), bail out gracefully instead of
// fataling — the test harness uses `cmd.Run` directly so a Fatalf
// would terminate the entire test binary.
if ctx.Err() != nil {
glog.Warningf("Admin server readiness wait aborted: %v", err)
return
}
glog.Fatalf("Admin server readiness check failed: %v", err)
}
@@ -1272,8 +1280,12 @@ func startMiniAdminWithWorker(allServicesReady chan struct{}) {
waitForWorkerReady(workerGrpcAddr)
}
// waitForAdminServerReady pings the admin server HTTP endpoint to check if it's ready
func waitForAdminServerReady(adminAddr string) error {
// waitForAdminServerReady pings the admin server HTTP endpoint to check if it's ready.
// Returns ctx.Err() (typically context.Canceled) if the parent context is
// cancelled mid-poll so a torn-down mini doesn't keep spinning for the full
// timeout — relevant when tests embed mini in-process and reuse the same
// goroutine pool across subtests.
func waitForAdminServerReady(ctx context.Context, adminAddr string) error {
healthAddr := getHealthCheckAddr(fmt.Sprintf("%s/health", adminAddr))
// 240 * 500ms = 120 seconds max wait. The previous 30-second ceiling was
// too tight on busy CI runners where master + filer + volume + admin all
@@ -1287,6 +1299,9 @@ func waitForAdminServerReady(adminAddr string) error {
}
for attempt < maxAttempts {
if err := ctx.Err(); err != nil {
return err
}
resp, err := client.Get(healthAddr)
if err == nil {
resp.Body.Close()
@@ -1294,7 +1309,11 @@ func waitForAdminServerReady(adminAddr string) error {
return nil
}
attempt++
time.Sleep(500 * time.Millisecond)
select {
case <-ctx.Done():
return ctx.Err()
case <-time.After(500 * time.Millisecond):
}
}
return fmt.Errorf("admin server did not become ready at %s after %d attempts", adminAddr, maxAttempts)