From 1a27edcc5ff7e728cd64e691f853e3e581473e0d Mon Sep 17 00:00:00 2001
From: Tobias Schottdorf <tobias.schottdorf@gmail.com>
Date: Fri, 29 May 2020 13:34:05 +0200
Subject: [PATCH] server: make cli-ready startTenant method()

The goal is to turn `testSQLServerArgs` into something that can be
re-used to introduce a cli command for starting a SQL tenant. This
commit takes a step in that direction by lifting some testing-specific
inputs up.

Release note: None
---
 pkg/server/server.go     |  2 +-
 pkg/server/server_sql.go |  4 ++--
 pkg/server/testserver.go | 50 +++++++++++++++++++++-------------------
 3 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/pkg/server/server.go b/pkg/server/server.go
index 65c211031a2d..e5a75931ae2a 100644
--- a/pkg/server/server.go
+++ b/pkg/server/server.go
@@ -553,7 +553,7 @@ func NewServer(cfg Config, stopper *stop.Stopper) (*Server, error) {
 		registry:                 registry,
 		sessionRegistry:          sessionRegistry,
 		circularInternalExecutor: internalExecutor,
-		jobRegistry:              jobRegistry,
+		circularJobRegistry:      jobRegistry,
 		jobAdoptionStopFile:      jobAdoptionStopFile,
 		protectedtsProvider:      protectedtsProvider,
 	})
diff --git a/pkg/server/server_sql.go b/pkg/server/server_sql.go
index 48591c2ea073..22ca2dc4b9ea 100644
--- a/pkg/server/server_sql.go
+++ b/pkg/server/server_sql.go
@@ -158,7 +158,7 @@ type sqlServerArgs struct {
 	// The protected timestamps KV subsystem depends on this, so we pass a
 	// pointer to an empty struct in this configuration, which newSQLServer
 	// fills.
-	jobRegistry         *jobs.Registry
+	circularJobRegistry *jobs.Registry
 	jobAdoptionStopFile string
 
 	// The executorConfig uses the provider.
@@ -176,7 +176,7 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*sqlServer, error) {
 	}
 	blobspb.RegisterBlobServer(cfg.grpcServer, blobService)
 
-	jobRegistry := cfg.jobRegistry
+	jobRegistry := cfg.circularJobRegistry
 
 	{
 		regLiveness := cfg.nodeLiveness
diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go
index c17619123960..092543707183 100644
--- a/pkg/server/testserver.go
+++ b/pkg/server/testserver.go
@@ -438,15 +438,10 @@ func (d dummyProtectedTSProvider) Protect(context.Context, *kv.Txn, *ptpb.Record
 //  this is tracked in https://github.com/cockroachdb/cockroach/issues/47892.
 const fakeNodeID = roachpb.NodeID(1)
 
-func testSQLServerArgs(
-	stopper *stop.Stopper, kvClusterName string, tenID roachpb.TenantID,
+func makeSQLServerArgs(
+	stopper *stop.Stopper, kvClusterName string, baseCfg BaseConfig, sqlCfg SQLConfig,
 ) sqlServerArgs {
-
-	st := cluster.MakeTestingClusterSettings()
-
-	sqlCfg := makeTestSQLConfig(st, tenID)
-
-	baseCfg := makeTestBaseConfig(st)
+	st := baseCfg.Settings
 	baseCfg.AmbientCtx.AddLogTag("sql", nil)
 	// TODO(tbg): this is needed so that the RPC heartbeats between the testcluster
 	// and this tenant work.
@@ -582,14 +577,14 @@ func testSQLServerArgs(
 			grpcServer:   dummyRPCServer,
 			recorder:     dummyRecorder,
 			isMeta1Leaseholder: func(timestamp hlc.Timestamp) (bool, error) {
-				return false, errors.New("fake isMeta1Leaseholder")
+				return false, errors.New("isMeta1Leaseholder is not available to tenants")
 			},
 			nodeIDContainer: idContainer,
 			externalStorage: func(ctx context.Context, dest roachpb.ExternalStorage) (cloud.ExternalStorage, error) {
-				return nil, errors.New("fake external storage")
+				return nil, errors.New("external storage is not available to tenants")
 			},
 			externalStorageFromURI: func(ctx context.Context, uri string) (cloud.ExternalStorage, error) {
-				return nil, errors.New("fake external uri storage")
+				return nil, errors.New("external uri storage is not available to tenants")
 			},
 		},
 		SQLConfig:                &sqlCfg,
@@ -601,7 +596,7 @@ func testSQLServerArgs(
 		registry:                 registry,
 		sessionRegistry:          sql.NewSessionRegistry(),
 		circularInternalExecutor: circularInternalExecutor,
-		jobRegistry:              &jobs.Registry{},
+		circularJobRegistry:      &jobs.Registry{},
 		protectedtsProvider:      protectedTSProvider,
 	}
 }
@@ -616,25 +611,32 @@ func (ts *TestServer) StartTenant(params base.TestTenantArgs) (pgAddr string, _
 		return "", err
 	}
 
-	return startTenant(ts.Stopper(), ts.Cfg.ClusterName, ts.RPCAddr(), params.TenantID, params.AllowSettingClusterSettings)
+	st := cluster.MakeTestingClusterSettings()
+	sqlCfg := makeTestSQLConfig(st, params.TenantID)
+	baseCfg := makeTestBaseConfig(st)
+	if params.AllowSettingClusterSettings {
+		baseCfg.TestingKnobs.TenantTestingKnobs = &sql.TenantTestingKnobs{
+			ClusterSettingsUpdater: st.MakeUpdater(),
+		}
+	}
+	return startTenant(
+		ts.Stopper(),
+		ts.Cfg.ClusterName,
+		ts.RPCAddr(),
+		baseCfg,
+		sqlCfg,
+	)
 }
 
 func startTenant(
 	stopper *stop.Stopper,
-	kvClusterName string,
+	kvClusterName string, // NB: gone after https://github.com/cockroachdb/cockroach/issues/42519
 	tsRPCAddr string,
-	tenID roachpb.TenantID,
-	allowSetClusterSetting bool,
+	baseCfg BaseConfig,
+	sqlCfg SQLConfig,
 ) (pgAddr string, _ error) {
-	args := testSQLServerArgs(stopper, kvClusterName, tenID)
-	// TODO(tbg): clean this up.
-	if allowSetClusterSetting {
-		args.TestingKnobs.TenantTestingKnobs = &sql.TenantTestingKnobs{
-			ClusterSettingsUpdater: args.Settings.MakeUpdater(),
-		}
-	}
+	args := makeSQLServerArgs(stopper, kvClusterName, baseCfg, sqlCfg)
 	ctx := context.Background()
-
 	s, err := newSQLServer(ctx, args)
 	if err != nil {
 		return "", err