From 8d40aceb4862dd7be89962fc034ba9b479524bd1 Mon Sep 17 00:00:00 2001
From: Chris Lu <chris.lu@gmail.com>
Date: Sun, 21 Dec 2025 20:12:55 -0800
Subject: [PATCH] refactor: prevent HTTP/gRPC port collisions and improve error
 handling
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Add upfront reservation of all calculated gRPC ports before allocating HTTP ports
  to prevent collisions where an HTTP port allocation could use a port that will
  later be needed for a gRPC port calculation.

  Example scenario that is now prevented:
  - Master HTTP reallocated from 9333 to 9334 (original in use)
  - Filer HTTP search finds 19334 available and assigns it
  - Master gRPC calculated as 9334 + GrpcPortOffset = 19334 → collision!

  Now: reserved gRPC ports are tracked upfront and HTTP port search skips them.

- Improve admin server gRPC port fallback error handling:
  - Change from silent V(1) verbose log to Warningf to make the error visible
  - Update comment to clarify this indicates a problem in the port initialization sequence
  - Add explanation that the fallback calculation may cause bind failure

- Update ensureAllPortsAvailableOnIP comment to clarify it avoids reserved ports
---
 weed/command/mini.go | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/weed/command/mini.go b/weed/command/mini.go
index cf31aeb0c..875008369 100644
--- a/weed/command/mini.go
+++ b/weed/command/mini.go
@@ -411,12 +411,29 @@ func ensureAllPortsAvailableOnIP(bindIp string) {
 		{miniAdminOptions.port, "Admin", miniAdminOptions.grpcPort},
 	}
 
+	// First, reserve all gRPC ports that will be calculated to prevent HTTP port allocation from using them
+	// This prevents collisions like: HTTP port moves to X, then gRPC port is calculated as Y where Y == X
+	reservedPorts := make(map[int]bool)
+	for _, config := range portConfigs {
+		if config.grpcPtr != nil && *config.grpcPtr == 0 {
+			// This gRPC port will be calculated as httpPort + GrpcPortOffset
+			calculatedGrpcPort := *config.port + GrpcPortOffset
+			reservedPorts[calculatedGrpcPort] = true
+		}
+	}
+
 	// Check all HTTP ports sequentially to avoid race conditions
 	// Each port check and allocation must complete before the next one starts
 	// to prevent multiple goroutines from claiming the same available port
+	// Also avoid allocating ports that are reserved for gRPC calculation
 	for _, config := range portConfigs {
-		// Check main port on the specific IP
+		original := *config.port
 		ensurePortAvailableOnIP(config.port, config.name, bindIp)
+		// If port was changed, update the reserved gRPC ports mapping
+		if *config.port != original && config.grpcPtr != nil && *config.grpcPtr == 0 {
+			delete(reservedPorts, original+GrpcPortOffset)
+			reservedPorts[*config.port+GrpcPortOffset] = true
+		}
 	}
 
 	// Initialize all gRPC ports before services start
@@ -851,11 +868,11 @@ func startMiniAdminWithWorker(allServicesReady chan struct{}) {
 	// Set admin options
 	*miniAdminOptions.master = masterAddr
 
-	// Note: gRPC port should already be initialized by ensureAllPortsAvailableOnIP
-	// only set it here if it's still 0 (shouldn't happen in normal flow)
+	// gRPC port should have been initialized by ensureAllPortsAvailableOnIP in runMini
+	// If it's still 0, that indicates a problem with the port initialization sequence
 	if *miniAdminOptions.grpcPort == 0 {
+		glog.Warningf("Admin gRPC port was not initialized before startAdminServer, using fallback calculation (may cause bind failure)")
 		*miniAdminOptions.grpcPort = *miniAdminOptions.port + GrpcPortOffset
-		glog.V(1).Infof("Admin gRPC port was 0, calculated to %d", *miniAdminOptions.grpcPort)
 	}
 
 	// Create data directory if specified