From d45b70ba028a8542bcba37ff72f80d9afc560e67 Mon Sep 17 00:00:00 2001
From: "vitess-bot[bot]" <108069721+vitess-bot[bot]@users.noreply.github.com>
Date: Tue, 18 Jun 2024 11:29:50 +0530
Subject: [PATCH] [release-18.0] Fix flaky tests that use vtcombo (#16178)
 (#16211)

Signed-off-by: Manan Gupta <manan@planetscale.com>
Co-authored-by: Manan Gupta <35839558+GuptaManan100@users.noreply.github.com>
Co-authored-by: Manan Gupta <manan@planetscale.com>
---
 go/cmd/vttestserver/cli/main_test.go | 18 +++++++++---------
 go/vt/vttest/environment.go          | 23 +++++++++++++++++++++--
 2 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/go/cmd/vttestserver/cli/main_test.go b/go/cmd/vttestserver/cli/main_test.go
index 39dc8e4ea78..613655fea47 100644
--- a/go/cmd/vttestserver/cli/main_test.go
+++ b/go/cmd/vttestserver/cli/main_test.go
@@ -59,7 +59,7 @@ func TestRunsVschemaMigrations(t *testing.T) {
 	cluster, err := startCluster()
 	defer cluster.TearDown()
 
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assertColumnVindex(t, cluster, columnVindex{keyspace: "test_keyspace", table: "test_table", vindex: "my_vdx", vindexType: "hash", column: "id"})
 	assertColumnVindex(t, cluster, columnVindex{keyspace: "app_customer", table: "customers", vindex: "hash", vindexType: "hash", column: "id"})
 
@@ -76,7 +76,7 @@ func TestPersistentMode(t *testing.T) {
 	dir := t.TempDir()
 
 	cluster, err := startPersistentCluster(dir)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	// Add a new "ad-hoc" vindex via vtgate once the cluster is up, to later make sure it is persisted across teardowns
 	err = addColumnVindex(cluster, "test_keyspace", "alter vschema on persistence_test add vindex my_vdx(id)")
@@ -115,7 +115,7 @@ func TestPersistentMode(t *testing.T) {
 		cluster.PersistentMode = false // Cleanup the tmpdir as we're done
 		cluster.TearDown()
 	}()
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	// rerun our sanity checks to make sure vschema is persisted correctly
 	assertColumnVindex(t, cluster, columnVindex{keyspace: "test_keyspace", table: "test_table", vindex: "my_vdx", vindexType: "hash", column: "id"})
@@ -136,7 +136,7 @@ func TestForeignKeysAndDDLModes(t *testing.T) {
 	defer resetConfig(conf)
 
 	cluster, err := startCluster("--foreign_key_mode=allow", "--enable_online_ddl=true", "--enable_direct_ddl=true")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	defer cluster.TearDown()
 
 	err = execOnCluster(cluster, "test_keyspace", func(conn *mysql.Conn) error {
@@ -162,7 +162,7 @@ func TestForeignKeysAndDDLModes(t *testing.T) {
 
 	cluster.TearDown()
 	cluster, err = startCluster("--foreign_key_mode=disallow", "--enable_online_ddl=false", "--enable_direct_ddl=false")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	defer cluster.TearDown()
 
 	err = execOnCluster(cluster, "test_keyspace", func(conn *mysql.Conn) error {
@@ -190,7 +190,7 @@ func TestCanGetKeyspaces(t *testing.T) {
 	defer resetConfig(conf)
 
 	cluster, err := startCluster()
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	defer cluster.TearDown()
 
 	assertGetKeyspaces(t, cluster)
@@ -215,7 +215,7 @@ func TestExternalTopoServerConsul(t *testing.T) {
 
 	cluster, err := startCluster("--external_topo_implementation=consul",
 		fmt.Sprintf("--external_topo_global_server_address=%s", serverAddr), "--external_topo_global_root=consul_test/global")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	defer cluster.TearDown()
 
 	assertGetKeyspaces(t, cluster)
@@ -251,7 +251,7 @@ func TestMtlsAuth(t *testing.T) {
 		fmt.Sprintf("--vtctld_grpc_cert=%s", clientCert),
 		fmt.Sprintf("--vtctld_grpc_ca=%s", caCert),
 		fmt.Sprintf("--grpc_auth_mtls_allowed_substrings=%s", "CN=ClientApp"))
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	defer func() {
 		cluster.PersistentMode = false // Cleanup the tmpdir as we're done
 		cluster.TearDown()
@@ -295,7 +295,7 @@ func TestMtlsAuthUnauthorizedFails(t *testing.T) {
 		fmt.Sprintf("--grpc_auth_mtls_allowed_substrings=%s", "CN=ClientApp"))
 	defer cluster.TearDown()
 
-	assert.Error(t, err)
+	require.Error(t, err)
 	assert.Contains(t, err.Error(), "code = Unauthenticated desc = client certificate not authorized")
 }
 
diff --git a/go/vt/vttest/environment.go b/go/vt/vttest/environment.go
index dac9d718827..8f356dccf37 100644
--- a/go/vt/vttest/environment.go
+++ b/go/vt/vttest/environment.go
@@ -19,8 +19,10 @@ package vttest
 import (
 	"fmt"
 	"math/rand"
+	"net"
 	"os"
 	"path"
+	"strconv"
 	"strings"
 
 	"vitess.io/vitess/go/vt/proto/vttest"
@@ -238,9 +240,26 @@ func tmpdir(dataroot string) (dir string, err error) {
 	return
 }
 
+// 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 {
-	v := rand.Int31n(20000)
-	return int(v + 10000)
+	for {
+		port := int(rand.Int31n(20000) + 10000)
+		portInUse := false
+		for i := 0; i < 6; i++ {
+			ln, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(port+i)))
+			if err != nil {
+				portInUse = true
+				break
+			}
+			ln.Close()
+		}
+		if portInUse {
+			continue
+		}
+		return port
+	}
 }
 
 // NewLocalTestEnv returns an instance of the default test environment used