vttest: fix race from randomPort() (#18899)

Signed-off-by: Tim Vaillancourt <[email protected]>

Author: timvaillancourt

go/vt/vttest/environment.go

@@ -21,15 +21,25 @@ import (
 	"net"
 	"os"
 	"path"
+	"slices"
 	"strconv"
 	"strings"
+	"sync"
 
 	"vitess.io/vitess/go/vt/proto/vttest"
 
 	// we use gRPC everywhere, so import the vtgate client.
 	_ "vitess.io/vitess/go/vt/vtgate/grpcvtgateconn"
 )
 
+var (
+	// randomPortsMu provides synchronization for randomPorts().
+	randomPortsMu sync.Mutex
+
+	// usedRandomPorts stores ports that have been used by remotePort().
+	usedRandomPorts []int
+)
+
 // Environment is the interface that customizes the global settings for
 // the test cluster. Usually the same environment settings are shared by
 // all the LocalCluster instances in a given test suite, with each instance
@@ -235,21 +245,31 @@ func tmpdir(dataroot string) (dir string, err error) {
 // randomPort gets a random port that is available for a TCP connection.
 // After we generate a random port, we try to establish tcp connections on it and the next 5 values.
 // If any of them fail, then we try a different port.
-func randomPort() int {
+func randomPort() (port int) {
+	randomPortsMu.Lock()
+	defer randomPortsMu.Unlock()
 	for {
-		port := int(rand.Int32N(20000) + 10000)
+		portBase := int(rand.Int32N(20000) + 10000)
 		portInUse := false
+		portRange := make([]int, 0, 6)
 		for i := 0; i < 6; i++ {
-			ln, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(port+i)))
+			port = portBase + i
+			if slices.Contains(usedRandomPorts, port) {
+				portInUse = true
+				break
+			}
+			ln, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(port)))
 			if err != nil {
 				portInUse = true
 				break
 			}
+			portRange = append(portRange, port)
 			ln.Close()
 		}
 		if portInUse {
 			continue
 		}
+		usedRandomPorts = append(usedRandomPorts, portRange...)
 		return port
 	}
 }

go/vt/vttest/environment_test.go

@@ -44,3 +44,15 @@ func TestVtcomboArguments(t *testing.T) {
 		assert.ElementsMatch(t, expectedServiceList, serviceMapList, "--service-map list does not contain expected vtcombo services")
 	})
 }
+
+func TestVtcomboRandomPort(t *testing.T) {
+	require.Empty(t, usedRandomPorts)
+	port := randomPort()
+	// 10000-30000 is the range the rand call in randomPorts() can return
+	require.GreaterOrEqual(t, port, 10000)
+	require.LessOrEqual(t, port, 30000)
+	require.Len(t, usedRandomPorts, 6)
+	require.Contains(t, usedRandomPorts, port)
+	require.NotEqual(t, port, randomPort())
+	require.Len(t, usedRandomPorts, 12)
+}