From ab9f165efd861bf04e04c32902eabea385339764 Mon Sep 17 00:00:00 2001
From: Herko Lategan <herko@cockroachlabs.com>
Date: Thu, 15 Jun 2023 21:24:19 +0100
Subject: [PATCH] roachtest: multi-tenant distributed sql fix

This change fixes the `multitenant_distsql` test by passing
`COCKROACH_MIN_RANGE_MAX_BYTES` as part of the env vars when starting tenants as
well.

The code around port handling on local clusters have also been updated to avoid
collisions. There is a mechanism in place to offset the ports, but it still
clashed with local ports from other nodes created before the tenants, so a
simple naive shift has been put in place.

Fixes: #100260
Epic: None
---
 pkg/cmd/roachtest/tests/multitenant_distsql.go | 11 +++++++----
 pkg/cmd/roachtest/tests/multitenant_utils.go   | 15 +++++++++++++--
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/pkg/cmd/roachtest/tests/multitenant_distsql.go b/pkg/cmd/roachtest/tests/multitenant_distsql.go
index 6805ee496a01..5568fdfb05d4 100644
--- a/pkg/cmd/roachtest/tests/multitenant_distsql.go
+++ b/pkg/cmd/roachtest/tests/multitenant_distsql.go
@@ -37,7 +37,6 @@ func registerMultiTenantDistSQL(r registry.Registry) {
 			b := bundle
 			to := timeout
 			r.Add(registry.TestSpec{
-				Skip:    "the test is skipped until #100260 is resolved",
 				Name:    fmt.Sprintf("multitenant/distsql/instances=%d/bundle=%s/timeout=%d", numInstances, b, to),
 				Owner:   registry.OwnerSQLQueries,
 				Cluster: r.MakeClusterSpec(4),
@@ -64,6 +63,7 @@ func runMultiTenantDistSQL(
 	// 1 byte to bypass the guardrails.
 	settings := install.MakeClusterSettings(install.SecureOption(true))
 	settings.Env = append(settings.Env, "COCKROACH_MIN_RANGE_MAX_BYTES=1")
+	tenantEnvOpt := createTenantEnvVar(settings.Env[len(settings.Env)-1])
 	c.Start(ctx, t.L(), option.DefaultStartOpts(), settings, c.Node(1))
 	c.Start(ctx, t.L(), option.DefaultStartOpts(), settings, c.Node(2))
 	c.Start(ctx, t.L(), option.DefaultStartOpts(), settings, c.Node(3))
@@ -72,17 +72,20 @@ func runMultiTenantDistSQL(
 		tenantID           = 11
 		tenantBaseHTTPPort = 8081
 		tenantBaseSQLPort  = 26259
+		// localPortOffset is used to avoid port conflicts with nodes on a local
+		// cluster.
+		localPortOffset = 1000
 	)
 
 	tenantHTTPPort := func(offset int) int {
 		if c.IsLocal() || numInstances > c.Spec().NodeCount {
-			return tenantBaseHTTPPort + offset
+			return tenantBaseHTTPPort + localPortOffset + offset
 		}
 		return tenantBaseHTTPPort
 	}
 	tenantSQLPort := func(offset int) int {
 		if c.IsLocal() || numInstances > c.Spec().NodeCount {
-			return tenantBaseSQLPort + offset
+			return tenantBaseSQLPort + localPortOffset + offset
 		}
 		return tenantBaseSQLPort
 	}
@@ -93,7 +96,7 @@ func runMultiTenantDistSQL(
 
 	instances := make([]*tenantNode, 0, numInstances)
 	instance1 := createTenantNode(ctx, t, c, c.Node(1), tenantID, 2 /* node */, tenantHTTPPort(0), tenantSQLPort(0),
-		createTenantCertNodes(c.All()))
+		createTenantCertNodes(c.All()), tenantEnvOpt)
 	instances = append(instances, instance1)
 	defer instance1.stop(ctx, t, c)
 	instance1.start(ctx, t, c, "./cockroach")
diff --git a/pkg/cmd/roachtest/tests/multitenant_utils.go b/pkg/cmd/roachtest/tests/multitenant_utils.go
index d2a6611b62c4..6fa1a5b2e96c 100644
--- a/pkg/cmd/roachtest/tests/multitenant_utils.go
+++ b/pkg/cmd/roachtest/tests/multitenant_utils.go
@@ -44,6 +44,7 @@ type tenantNode struct {
 	httpPort, sqlPort int
 	kvAddrs           []string
 	pgURL             string
+	envVars           []string
 	// Same as pgURL but with relative ssl parameters.
 	relativeSecureURL string
 
@@ -55,6 +56,9 @@ type tenantNode struct {
 type createTenantOptions struct {
 	// Set this to expand the scope of the nodes added to the tenant certs.
 	certNodes option.NodeListOption
+
+	// Set this to add additional environment variables to the tenant.
+	envVars []string
 }
 type createTenantOpt func(*createTenantOptions)
 
@@ -62,6 +66,10 @@ func createTenantCertNodes(nodes option.NodeListOption) createTenantOpt {
 	return func(c *createTenantOptions) { c.certNodes = nodes }
 }
 
+func createTenantEnvVar(envVar string) createTenantOpt {
+	return func(c *createTenantOptions) { c.envVars = append(c.envVars, envVar) }
+}
+
 func createTenantNodeInternal(
 	ctx context.Context,
 	t test.Test,
@@ -88,6 +96,7 @@ func createTenantNodeInternal(
 		kvAddrs:    kvAddrs,
 		node:       node,
 		sqlPort:    sqlPort,
+		envVars:    append(config.DefaultEnvVars(), createOptions.envVars...),
 	}
 	if certs {
 		tn.createTenantCert(ctx, t, c, createOptions.certNodes)
@@ -181,7 +190,7 @@ func (tn *tenantNode) start(ctx context.Context, t test.Test, c cluster.Cluster,
 	require.NoError(t, err)
 	tn.errCh = startTenantServer(
 		ctx, c, c.Node(tn.node), internalIPs[0], binary, tn.kvAddrs, tn.tenantID,
-		tn.httpPort, tn.sqlPort,
+		tn.httpPort, tn.sqlPort, tn.envVars,
 		extraArgs...,
 	)
 
@@ -244,6 +253,7 @@ func startTenantServer(
 	tenantID int,
 	httpPort int,
 	sqlPort int,
+	envVars []string,
 	extraFlags ...string,
 ) chan error {
 	args := []string{
@@ -259,7 +269,7 @@ func startTenantServer(
 	errCh := make(chan error, 1)
 	go func() {
 		errCh <- c.RunE(tenantCtx, node,
-			append(append(append([]string{}, config.DefaultEnvVars()...), binary, "mt", "start-sql"), args...)...,
+			append(append(append([]string{}, envVars...), binary, "mt", "start-sql"), args...)...,
 		)
 		close(errCh)
 	}()
@@ -278,6 +288,7 @@ func newTenantInstance(
 		node:       node,
 		httpPort:   http,
 		sqlPort:    sql,
+		envVars:    tn.envVars,
 	}
 	tenantCertsDir, err := os.MkdirTemp("", "tenant-certs")
 	if err != nil {