sql: increase tenant testing coverage

pkg/base/test_server_args.go

@@ -526,6 +526,19 @@ func TestIsForStuffThatShouldWorkWithSharedProcessModeButDoesntYet(
 //
 // It should link to a github issue with label C-investigation.
 func TestSkippedForExternalModeDueToPerformance(issueNumber int) DefaultTestTenantOptions {
+	return testSkippedForExternalProcessMode(issueNumber)
+}
+
+// TestDoesNotWorkWithExternalProcessMode disables selecting the external
+// process virtual cluster for tests that are not functional in that mode and
+// require further investigation. Any test using this function should reference
+// a GitHub issue tagged with "C-investigation" describing the underlying
+// problem.
+func TestDoesNotWorkWithExternalProcessMode(issueNumber int) DefaultTestTenantOptions {
+	return testSkippedForExternalProcessMode(issueNumber)
+}
+
+func testSkippedForExternalProcessMode(issueNumber int) DefaultTestTenantOptions {
 	return DefaultTestTenantOptions{
 		testBehavior:           ttSharedProcess,
 		allowAdditionalTenants: true,

pkg/sql/jobs_profiler_execution_details_test.go

@@ -115,7 +115,7 @@ func TestShowJobsWithExecutionDetails(t *testing.T) {
 	ctx, cancel := context.WithTimeout(ctx, time.Minute*2)
 	defer cancel()
 
-	params, _ := createTestServerParams()
+	params, _ := createTestServerParamsAllowTenants()
 	params.Knobs.JobsTestingKnobs = jobs.NewTestingKnobsWithShortIntervals()
 	defer jobs.ResetConstructors()()
 	s, sqlDB, _ := serverutils.StartServer(t, params)
@@ -159,7 +159,7 @@ func TestReadWriteProfilerExecutionDetails(t *testing.T) {
 	ctx, cancel := context.WithTimeout(ctx, time.Minute*2)
 	defer cancel()
 
-	params, _ := createTestServerParams()
+	params, _ := createTestServerParamsAllowTenants()
 	params.Knobs.JobsTestingKnobs = jobs.NewTestingKnobsWithShortIntervals()
 	defer jobs.ResetConstructors()()
 	s, sqlDB, _ := serverutils.StartServer(t, params)
@@ -224,8 +224,18 @@ func TestReadWriteProfilerExecutionDetails(t *testing.T) {
 		require.NoError(t, err)
 		continueCh <- struct{}{}
 		jobutils.WaitForJobToSucceed(t, runner, jobspb.JobID(importJobID))
-		require.True(t, strings.Contains(string(goroutines), fmt.Sprintf("labels: {\"foo\":\"bar\", \"job\":\"IMPORT id=%d\", \"n\":\"1\"}", importJobID)))
-		require.True(t, strings.Contains(string(goroutines), "github.com/cockroachdb/cockroach/pkg/sql_test.fakeExecResumer.Resume"))
+		expectedSubstrings := []string{
+			fmt.Sprintf("labels: {\"foo\":\"bar\", \"job\":\"IMPORT id=%d\", \"n\":\"1\"}", importJobID),
+			"github.com/cockroachdb/cockroach/pkg/sql_test.fakeExecResumer.Resume",
+		}
+		goroutinesStr := string(goroutines)
+		for _, substr := range expectedSubstrings {
+			if s.DeploymentMode().IsExternal() {
+				require.NotContains(t, goroutinesStr, substr)
+			} else {
+				require.Contains(t, goroutinesStr, substr)
+			}
+		}
 	})
 
 	t.Run("execution details for invalid job ID", func(t *testing.T) {
@@ -323,7 +333,7 @@ func TestListProfilerExecutionDetails(t *testing.T) {
 	ctx, cancel := context.WithTimeout(ctx, time.Minute*2)
 	defer cancel()
 
-	params, _ := createTestServerParams()
+	params, _ := createTestServerParamsAllowTenants()
 	params.Knobs.JobsTestingKnobs = jobs.NewTestingKnobsWithShortIntervals()
 	defer jobs.ResetConstructors()()
 	s, sqlDB, _ := serverutils.StartServer(t, params)
@@ -366,19 +376,30 @@ func TestListProfilerExecutionDetails(t *testing.T) {
 
 		runner.Exec(t, `SELECT crdb_internal.request_job_execution_details($1)`, importJobID)
 		files := listExecutionDetails(t, s, jobspb.JobID(importJobID))
-		require.Len(t, files, 3)
-		require.Regexp(t, "distsql\\..*\\.html", files[0])
-		require.Regexp(t, "goroutines\\..*\\.txt", files[1])
-		require.Regexp(t, "trace\\..*\\.zip", files[2])
+
+		patterns := []string{
+			"distsql\\..*\\.html",
+		}
+		if !s.DeploymentMode().IsExternal() {
+			patterns = append(patterns, "goroutines\\..*\\.txt")
+		}
+		patterns = append(patterns, "trace\\..*\\.zip")
+
+		require.Len(t, files, len(patterns))
+		for i, pattern := range patterns {
+			require.Regexp(t, pattern, files[i])
+		}
 
 		continueCh <- struct{}{}
 		jobutils.WaitForJobToPause(t, runner, jobspb.JobID(importJobID))
 
 		testutils.SucceedsSoon(t, func() error {
 			files = listExecutionDetails(t, s, jobspb.JobID(importJobID))
-			if len(files) != 5 {
-				return errors.Newf("expected 5 files, got %d: %v", len(files), files)
+			expectedCount := 5
+			if s.DeploymentMode().IsExternal() {
+				expectedCount--
 			}
+			require.Len(t, files, expectedCount)
 			return nil
 		})
 
@@ -393,21 +414,28 @@ func TestListProfilerExecutionDetails(t *testing.T) {
 		jobutils.WaitForJobToSucceed(t, runner, jobspb.JobID(importJobID))
 		testutils.SucceedsSoon(t, func() error {
 			files = listExecutionDetails(t, s, jobspb.JobID(importJobID))
-			if len(files) != 10 {
-				return errors.Newf("expected 10 files, got %d: %v", len(files), files)
+			expectedCount := 10
+			if s.DeploymentMode().IsExternal() {
+				expectedCount = 8
 			}
+			require.Len(t, files, expectedCount)
 			return nil
 		})
-		require.Regexp(t, "[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb", files[0])
-		require.Regexp(t, "[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb.txt", files[1])
-		require.Regexp(t, "[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb", files[2])
-		require.Regexp(t, "[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb.txt", files[3])
-		require.Regexp(t, "distsql\\..*\\.html", files[4])
-		require.Regexp(t, "distsql\\..*\\.html", files[5])
-		require.Regexp(t, "goroutines\\..*\\.txt", files[6])
-		require.Regexp(t, "goroutines\\..*\\.txt", files[7])
-		require.Regexp(t, "trace\\..*\\.zip", files[8])
-		require.Regexp(t, "trace\\..*\\.zip", files[9])
+		patterns = []string{
+			"[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb",
+			"[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb.txt",
+			"[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb",
+			"[0-9]/resumer-trace/.*~cockroach\\.sql\\.jobs\\.jobspb\\.TraceData\\.binpb.txt",
+			"distsql\\..*\\.html",
+			"distsql\\..*\\.html",
+		}
+		if !s.DeploymentMode().IsExternal() {
+			patterns = append(patterns, "goroutines\\..*\\.txt", "goroutines\\..*\\.txt")
+		}
+		patterns = append(patterns, "trace\\..*\\.zip", "trace\\..*\\.zip")
+		for i, pattern := range patterns {
+			require.Regexp(t, pattern, files[i])
+		}
 	})
 }
 

pkg/sql/mvcc_backfiller_test.go

@@ -555,7 +555,7 @@ func TestIndexBackfillMergeTxnRetry(t *testing.T) {
 		additionalRowsForMerge = 10
 	)
 
-	params, _ := createTestServerParams()
+	params, _ := createTestServerParamsAllowTenants()
 	params.Knobs = base.TestingKnobs{
 		SQLSchemaChanger: &sql.SchemaChangerTestingKnobs{
 			// Ensure that the temp index has work to do.
@@ -597,6 +597,7 @@ func TestIndexBackfillMergeTxnRetry(t *testing.T) {
 	var err error
 	scratch, err = s.ScratchRange()
 	require.NoError(t, err)
+	scratch = append(s.Codec().TenantPrefix(), scratch...)
 
 	if _, err := sqlDB.Exec(`
 SET use_declarative_schema_changer='off';

pkg/sql/txn_restart_test.go

@@ -25,13 +25,13 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
+	"github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilitiespb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
 	"github.com/cockroachdb/cockroach/pkg/sql/tests"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
-	"github.com/cockroachdb/cockroach/pkg/testutils/pgurlutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/util/caller"
@@ -213,7 +213,8 @@ func checkRestarts(t *testing.T, magicVals *filterVals) {
 //		s, sqlDB, _ := serverutils.StartServer(t, params)
 //		defer s.Stopper().Stop(context.Background())
 //		{
-//			pgURL, cleanup := sqlutils.PGUrl(t, s.AdvSQLAddr(), "TestTxnAutoRetry", url.User(security.RootUser)
+//			pgURL, cleanup := s.ApplicationLayer().PGUrl(t,
+//				serverutils.CertsDirPrefix("TestTxnAutoRetry"), serverutils.User(username.RootUser))
 //			defer cleanup()
 //			if err := aborter.Init(pgURL); err != nil {
 //				t.Fatal(err)
@@ -449,12 +450,13 @@ func TestTxnAutoRetry(t *testing.T) {
 
 	aborter := NewTxnAborter()
 	defer aborter.Close(t)
-	params, cmdFilters := createTestServerParams()
+	params, cmdFilters := createTestServerParamsAllowTenants()
 	params.Knobs.SQLExecutor = aborter.executorKnobs()
 	s, sqlDB, _ := serverutils.StartServer(t, params)
 	defer s.Stopper().Stop(context.Background())
 	{
-		pgURL, cleanup := pgurlutils.PGUrl(t, s.AdvSQLAddr(), "TestTxnAutoRetry", url.User(username.RootUser))
+		pgURL, cleanup := s.ApplicationLayer().PGUrl(t,
+			serverutils.CertsDirPrefix("TestTxnAutoRetry"), serverutils.User(username.RootUser))
 		defer cleanup()
 		if err := aborter.Init(pgURL); err != nil {
 			t.Fatal(err)
@@ -628,12 +630,13 @@ func TestAbortedTxnOnlyRetriedOnce(t *testing.T) {
 
 	aborter := NewTxnAborter()
 	defer aborter.Close(t)
-	params, _ := createTestServerParams()
+	params, _ := createTestServerParamsAllowTenants()
 	params.Knobs.SQLExecutor = aborter.executorKnobs()
 	s, sqlDB, _ := serverutils.StartServer(t, params)
 	defer s.Stopper().Stop(context.Background())
 	{
-		pgURL, cleanup := pgurlutils.PGUrl(t, s.AdvSQLAddr(), "TestAbortedTxnOnlyRetriedOnce", url.User(username.RootUser))
+		pgURL, cleanup := s.ApplicationLayer().PGUrl(t,
+			serverutils.CertsDirPrefix("TestAbortedTxnOnlyRetriedOnce"), serverutils.User(username.RootUser))
 		defer cleanup()
 		if err := aborter.Init(pgURL); err != nil {
 			t.Fatal(err)
@@ -784,12 +787,14 @@ func TestTxnUserRestart(t *testing.T) {
 			t.Run(fmt.Sprintf("err=%s,stgy=%d", tc.expectedErr, rs), func(t *testing.T) {
 				aborter := NewTxnAborter()
 				defer aborter.Close(t)
-				params, cmdFilters := createTestServerParams()
+				params, cmdFilters := createTestServerParamsAllowTenants()
 				params.Knobs.SQLExecutor = aborter.executorKnobs()
 				s, sqlDB, _ := serverutils.StartServer(t, params)
 				defer s.Stopper().Stop(context.Background())
 				{
-					pgURL, cleanup := pgurlutils.PGUrl(t, s.AdvSQLAddr(), "TestTxnUserRestart", url.User(username.RootUser))
+
+					pgURL, cleanup := s.ApplicationLayer().PGUrl(t,
+						serverutils.CertsDirPrefix("TestTxnUserRestart"), serverutils.User(username.RootUser))
 					defer cleanup()
 					if err := aborter.Init(pgURL); err != nil {
 						t.Fatal(err)
@@ -864,7 +869,7 @@ func TestCommitWaitState(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	params, _ := createTestServerParams()
+	params, _ := createTestServerParamsAllowTenants()
 	s, sqlDB, _ := serverutils.StartServer(t, params)
 	defer s.Stopper().Stop(context.Background())
 	if _, err := sqlDB.Exec(`
@@ -900,12 +905,14 @@ func TestErrorOnCommitFinalizesTxn(t *testing.T) {
 
 	aborter := NewTxnAborter()
 	defer aborter.Close(t)
-	params, _ := createTestServerParams()
+	params, _ := createTestServerParamsAllowTenants()
 	params.Knobs.SQLExecutor = aborter.executorKnobs()
 	s, sqlDB, _ := serverutils.StartServer(t, params)
 	defer s.Stopper().Stop(context.Background())
 	{
-		pgURL, cleanup := pgurlutils.PGUrl(t, s.AdvSQLAddr(), "TestErrorOnCommitFinalizesTxn", url.User(username.RootUser))
+
+		pgURL, cleanup := s.ApplicationLayer().PGUrl(t,
+			serverutils.CertsDirPrefix("TestErrorOnCommitFinalizesTxn"), serverutils.User(username.RootUser))
 		defer cleanup()
 		if err := aborter.Init(pgURL); err != nil {
 			t.Fatal(err)
@@ -987,12 +994,13 @@ func TestRollbackInRestartWait(t *testing.T) {
 
 	aborter := NewTxnAborter()
 	defer aborter.Close(t)
-	params, _ := createTestServerParams()
+	params, _ := createTestServerParamsAllowTenants()
 	params.Knobs.SQLExecutor = aborter.executorKnobs()
 	s, sqlDB, _ := serverutils.StartServer(t, params)
 	defer s.Stopper().Stop(context.Background())
 	{
-		pgURL, cleanup := pgurlutils.PGUrl(t, s.AdvSQLAddr(), "TestRollbackInRestartWait", url.User(username.RootUser))
+		pgURL, cleanup := s.ApplicationLayer().PGUrl(t,
+			serverutils.CertsDirPrefix("TestRollbackInRestartWait"), serverutils.User(username.RootUser))
 		defer cleanup()
 		if err := aborter.Init(pgURL); err != nil {
 			t.Fatal(err)
@@ -1048,7 +1056,7 @@ func TestUnexpectedStatementInRestartWait(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	params, _ := createTestServerParams()
+	params, _ := createTestServerParamsAllowTenants()
 	s, sqlDB, _ := serverutils.StartServer(t, params)
 	defer s.Stopper().Stop(context.Background())
 
@@ -1099,7 +1107,7 @@ func TestNonRetryableError(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	params, cmdFilters := createTestServerParams()
+	params, cmdFilters := createTestServerParamsAllowTenants()
 	s, sqlDB, _ := serverutils.StartServer(t, params)
 	defer s.Stopper().Stop(context.Background())
 
@@ -1191,7 +1199,7 @@ func TestReacquireLeaseOnRestart(t *testing.T) {
 		DisableMaxOffsetCheck: true,
 	}
 
-	params, _ := createTestServerParams()
+	params, _ := createTestServerParamsAllowTenants()
 	params.Knobs.Store = storeTestingKnobs
 	params.Knobs.KVClient = clientTestingKnobs
 	var sqlDB *gosql.DB
@@ -1241,7 +1249,7 @@ func TestFlushUncommitedDescriptorCacheOnRestart(t *testing.T) {
 		},
 	}
 
-	params, _ := createTestServerParams()
+	params, _ := createTestServerParamsAllowTenants()
 	params.Knobs.Store = testingKnobs
 	s, sqlDB, _ := serverutils.StartServer(t, params)
 	defer s.Stopper().Stop(context.Background())
@@ -1311,7 +1319,9 @@ func TestDistSQLRetryableError(t *testing.T) {
 	// targetKey is represents one of the rows in the table.
 	// +2 since the first two available ids are allocated to the database and
 	// public schema.
-	firstTableID := func() (id uint32) {
+	var firstTableID uint32
+	var codec keys.SQLCodec
+	func() {
 		tc := serverutils.StartCluster(t, 3, /* numNodes */
 			base.TestClusterArgs{
 				ReplicationMode: base.ReplicationManual,
@@ -1322,12 +1332,12 @@ func TestDistSQLRetryableError(t *testing.T) {
 		createTable(db)
 		row := db.QueryRow("SELECT 't'::REGCLASS::OID")
 		require.NotNil(t, row)
-		require.NoError(t, row.Scan(&id))
-		return id
+		require.NoError(t, row.Scan(&firstTableID))
+		codec = tc.Server(0).Codec()
 	}()
 	indexID := uint32(1)
 	valInTable := uint64(2)
-	indexKey := keys.SystemSQLCodec.IndexPrefix(firstTableID, indexID)
+	indexKey := codec.IndexPrefix(firstTableID, indexID)
 	targetKey := encoding.EncodeUvarintAscending(indexKey, valInTable)
 
 	restarted := true
@@ -1366,6 +1376,11 @@ func TestDistSQLRetryableError(t *testing.T) {
 		})
 	defer tc.Stopper().Stop(context.Background())
 
+	if tc.DefaultTenantDeploymentMode().IsExternal() {
+		tc.GrantTenantCapabilities(ctx, t, serverutils.TestTenantID(),
+			map[tenantcapabilitiespb.ID]string{tenantcapabilitiespb.CanAdminRelocateRange: "true"})
+	}
+
 	db := tc.ServerConn(0)
 	createTable(db)
 
@@ -1460,7 +1475,7 @@ func TestRollbackToSavepointFromUnusualStates(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	params, _ := createTestServerParams()
+	params, _ := createTestServerParamsAllowTenants()
 	s, sqlDB, _ := serverutils.StartServer(t, params)
 	defer s.Stopper().Stop(context.Background())
 
@@ -1523,7 +1538,7 @@ func TestTxnAutoRetriesDisabledAfterResultsHaveBeenSentToClient(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	params, _ := createTestServerParams()
+	params, _ := createTestServerParamsAllowTenants()
 	s, sqlDB, _ := serverutils.StartServer(t, params)
 	defer s.Stopper().Stop(context.Background())
 	defer sqlDB.Close()
@@ -1612,7 +1627,7 @@ func TestTxnAutoRetryReasonAvailable(t *testing.T) {
 	const numRetries = 3
 	retryCount := 0
 
-	params, cmdFilters := createTestServerParams()
+	params, cmdFilters := createTestServerParamsAllowTenants()
 	params.Knobs.SQLExecutor = &sql.ExecutorTestingKnobs{
 		BeforeRestart: func(ctx context.Context, reason error) {
 			retryCount++

pkg/sql/unsplit_range_test.go

@@ -12,6 +12,7 @@ import (
 	"errors"
 	"testing"
 
+	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/jobs"
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv"
@@ -269,7 +270,8 @@ func TestUnsplitRanges(t *testing.T) {
 
 	ctx := context.Background()
 	run := func(t *testing.T, tc testCase) {
-		params, _ := createTestServerParams()
+		params, _ := createTestServerParamsAllowTenants()
+		params.DefaultTestTenant = base.TestDoesNotWorkWithExternalProcessMode(142388)
 		params.Knobs.JobsTestingKnobs = jobs.NewTestingKnobsWithShortIntervals()
 		params.Knobs.GCJob = &sql.GCJobTestingKnobs{
 			SkipWaitingForMVCCGC: true,