diff --git a/pkg/ccl/changefeedccl/helpers_tenant_shim_test.go b/pkg/ccl/changefeedccl/helpers_tenant_shim_test.go index 328375feaa5e..3f018be6af76 100644 --- a/pkg/ccl/changefeedccl/helpers_tenant_shim_test.go +++ b/pkg/ccl/changefeedccl/helpers_tenant_shim_test.go @@ -69,9 +69,6 @@ func (t *testServerShim) RangeFeedFactory() interface{} { panic(unsuppor func (t *testServerShim) Clock() *hlc.Clock { panic(unsupportedShimMethod) } func (t *testServerShim) DistSenderI() interface{} { panic(unsupportedShimMethod) } func (t *testServerShim) MigrationServer() interface{} { panic(unsupportedShimMethod) } -func (t *testServerShim) SpanConfigAccessor() interface{} { panic(unsupportedShimMethod) } -func (t *testServerShim) SpanConfigSQLTranslator() interface{} { panic(unsupportedShimMethod) } -func (t *testServerShim) SpanConfigSQLWatcher() interface{} { panic(unsupportedShimMethod) } func (t *testServerShim) SQLServer() interface{} { panic(unsupportedShimMethod) } func (t *testServerShim) SQLLivenessProvider() interface{} { panic(unsupportedShimMethod) } func (t *testServerShim) StartupMigrationsManager() interface{} { panic(unsupportedShimMethod) } diff --git a/pkg/ccl/sqlproxyccl/proxy_handler_test.go b/pkg/ccl/sqlproxyccl/proxy_handler_test.go index 357c03dc5195..e980e16d6ae7 100644 --- a/pkg/ccl/sqlproxyccl/proxy_handler_test.go +++ b/pkg/ccl/sqlproxyccl/proxy_handler_test.go @@ -207,7 +207,7 @@ func TestProxyAgainstSecureCRDB(t *testing.T) { defer te.Close() sql, db, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: false}) - sql.(*server.TestServer).PGServer().TestingSetTrustClientProvidedRemoteAddr(true) + sql.(*server.TestServer).PGServer().(*pgwire.Server).TestingSetTrustClientProvidedRemoteAddr(true) defer sql.Stopper().Stop(ctx) sqlDB := sqlutils.MakeSQLRunner(db) @@ -323,7 +323,7 @@ func TestProxyTLSClose(t *testing.T) { defer te.Close() sql, db, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: false}) - sql.(*server.TestServer).PGServer().TestingSetTrustClientProvidedRemoteAddr(true) + sql.(*server.TestServer).PGServer().(*pgwire.Server).TestingSetTrustClientProvidedRemoteAddr(true) defer sql.Stopper().Stop(ctx) sqlDB := sqlutils.MakeSQLRunner(db) @@ -368,7 +368,7 @@ func TestProxyModifyRequestParams(t *testing.T) { defer te.Close() sql, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: false}) - sql.(*server.TestServer).PGServer().TestingSetTrustClientProvidedRemoteAddr(true) + sql.(*server.TestServer).PGServer().(*pgwire.Server).TestingSetTrustClientProvidedRemoteAddr(true) defer sql.Stopper().Stop(ctx) // Create some user with password authn. @@ -421,7 +421,7 @@ func TestInsecureProxy(t *testing.T) { defer te.Close() sql, db, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: false}) - sql.(*server.TestServer).PGServer().TestingSetTrustClientProvidedRemoteAddr(true) + sql.(*server.TestServer).PGServer().(*pgwire.Server).TestingSetTrustClientProvidedRemoteAddr(true) defer sql.Stopper().Stop(ctx) sqlDB := sqlutils.MakeSQLRunner(db) @@ -527,7 +527,7 @@ func TestDenylistUpdate(t *testing.T) { require.NoError(t, err) sql, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: false}) - sql.(*server.TestServer).PGServer().TestingSetTrustClientProvidedRemoteAddr(true) + sql.(*server.TestServer).PGServer().(*pgwire.Server).TestingSetTrustClientProvidedRemoteAddr(true) defer sql.Stopper().Stop(ctx) // Create some user with password authn. @@ -596,7 +596,7 @@ func TestDirectoryConnect(t *testing.T) { defer te.Close() srv, _, _ := serverutils.StartServer(t, base.TestServerArgs{Insecure: true}) - srv.(*server.TestServer).PGServer().TestingSetTrustClientProvidedRemoteAddr(true) + srv.(*server.TestServer).PGServer().(*pgwire.Server).TestingSetTrustClientProvidedRemoteAddr(true) defer srv.Stopper().Stop(ctx) // Create tenant 28. diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go index 7e5bd04d5e56..afae650039c1 100644 --- a/pkg/server/testserver.go +++ b/pkg/server/testserver.go @@ -47,7 +47,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" - "github.com/cockroachdb/cockroach/pkg/sql/pgwire" "github.com/cockroachdb/cockroach/pkg/sql/physicalplan" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" @@ -315,6 +314,8 @@ type TestServer struct { } } +var _ serverutils.TestServerInterface = &TestServer{} + // Node returns the Node as an interface{}. func (ts *TestServer) Node() interface{} { return ts.node @@ -421,6 +422,16 @@ func (ts *TestServer) HeartbeatNodeLiveness() error { return err } +// SQLInstanceID is part of TestServerInterface. +func (ts *TestServer) SQLInstanceID() base.SQLInstanceID { + return ts.sqlServer.sqlIDContainer.SQLInstanceID() +} + +// StatusServer is part of TestServerInterface. +func (ts *TestServer) StatusServer() interface{} { + return ts.status +} + // RPCContext returns the rpc context used by the TestServer. func (ts *TestServer) RPCContext() *rpc.Context { if ts != nil { @@ -445,8 +456,9 @@ func (ts *TestServer) DB() *kv.DB { return nil } -// PGServer returns the pgwire.Server used by the TestServer. -func (ts *TestServer) PGServer() *pgwire.Server { +// PGServer exposes the pgwire.Server instance used by the TestServer as an +// interface{}. +func (ts *TestServer) PGServer() interface{} { if ts != nil { return ts.sqlServer.pgServer } @@ -499,51 +511,93 @@ type TestTenant struct { httpAddr string } -// SQLAddr is part of the TestTenantInterface interface. +var _ serverutils.TestTenantInterface = &TestTenant{} + +// SQLAddr is part of TestTenantInterface interface. func (t *TestTenant) SQLAddr() string { return t.sqlAddr } -// HTTPAddr is part of the TestTenantInterface interface. +// HTTPAddr is part of TestTenantInterface interface. func (t *TestTenant) HTTPAddr() string { return t.httpAddr } -// PGServer is part of the TestTenantInterface interface. +// PGServer is part of TestTenantInterface. func (t *TestTenant) PGServer() interface{} { return t.pgServer } -// DiagnosticsReporter is part of the TestTenantInterface interface. +// DiagnosticsReporter is part of TestTenantInterface. func (t *TestTenant) DiagnosticsReporter() interface{} { return t.diagnosticsReporter } -// StatusServer is part of the TestTenantInterface interface. +// StatusServer is part of TestTenantInterface. func (t *TestTenant) StatusServer() interface{} { return t.execCfg.SQLStatusServer } -// DistSQLServer is part of the TestTenantInterface interface. +// DistSQLServer is part of TestTenantInterface. func (t *TestTenant) DistSQLServer() interface{} { return t.SQLServer.distSQLServer } -// RPCContext is part of the TestTenantInterface interface +// RPCContext is part of TestTenantInterface. func (t *TestTenant) RPCContext() *rpc.Context { return t.execCfg.RPCContext } -// JobRegistry is part of the TestTenantInterface interface. +// JobRegistry is part of TestTenantInterface. func (t *TestTenant) JobRegistry() interface{} { return t.SQLServer.jobRegistry } -// TestingKnobs is part of the TestTenantInterface interface. +// ExecutorConfig is part of TestTenantInterface. +func (t *TestTenant) ExecutorConfig() interface{} { + return *t.SQLServer.execCfg +} + +// RangeFeedFactory is part of TestTenantInterface. +func (t *TestTenant) RangeFeedFactory() interface{} { + return t.SQLServer.execCfg.RangeFeedFactory +} + +// ClusterSettings is part of TestTenantInterface. +func (t *TestTenant) ClusterSettings() *cluster.Settings { + return t.Cfg.Settings +} + +// Stopper is part of TestTenantInterface. +func (t *TestTenant) Stopper() *stop.Stopper { + return t.stopper +} + +// Clock is part of TestTenantInterface. +func (t *TestTenant) Clock() *hlc.Clock { + return t.SQLServer.execCfg.Clock +} + +// TestingKnobs is part TestTenantInterface. func (t *TestTenant) TestingKnobs() *base.TestingKnobs { return &t.Cfg.TestingKnobs } +// SpanConfigKVAccessor is part TestTenantInterface. +func (t *TestTenant) SpanConfigKVAccessor() interface{} { + return t.SQLServer.tenantConnect +} + +// SpanConfigSQLTranslator is part TestTenantInterface. +func (t *TestTenant) SpanConfigSQLTranslator() interface{} { + return t.SQLServer.spanconfigMgr.SQLTranslator +} + +// SpanConfigSQLWatcher is part TestTenantInterface. +func (t *TestTenant) SpanConfigSQLWatcher() interface{} { + return t.SQLServer.spanconfigMgr.SQLTranslator +} + // StartTenant starts a SQL tenant communicating with this TestServer. func (ts *TestServer) StartTenant( ctx context.Context, params base.TestTenantArgs, @@ -984,8 +1038,8 @@ func (ts *TestServer) MigrationServer() interface{} { return ts.migrationServer } -// SpanConfigAccessor is part of TestServerInterface. -func (ts *TestServer) SpanConfigAccessor() interface{} { +// SpanConfigKVAccessor is part of TestServerInterface. +func (ts *TestServer) SpanConfigKVAccessor() interface{} { return ts.Server.node.spanConfigAccessor } @@ -1011,7 +1065,7 @@ func (ts *TestServer) SpanConfigSQLWatcher() interface{} { // SQLServer is part of TestServerInterface. func (ts *TestServer) SQLServer() interface{} { - return ts.PGServer().SQLServer + return ts.sqlServer.pgServer.SQLServer } // DistSQLServer is part of TestServerInterface. diff --git a/pkg/spanconfig/spanconfigmanager/manager_test.go b/pkg/spanconfig/spanconfigmanager/manager_test.go index af51454b86f0..1b7525121c5d 100644 --- a/pkg/spanconfig/spanconfigmanager/manager_test.go +++ b/pkg/spanconfig/spanconfigmanager/manager_test.go @@ -73,7 +73,7 @@ func TestManagerConcurrentJobCreation(t *testing.T) { ts.InternalExecutor().(*sql.InternalExecutor), ts.Stopper(), ts.ClusterSettings(), - ts.SpanConfigAccessor().(spanconfig.KVAccessor), + ts.SpanConfigKVAccessor().(spanconfig.KVAccessor), ts.SpanConfigSQLWatcher().(spanconfig.SQLWatcher), ts.SpanConfigSQLTranslator().(spanconfig.SQLTranslator), &spanconfig.TestingKnobs{ @@ -162,7 +162,7 @@ func TestManagerStartsJobIfFailed(t *testing.T) { ts.InternalExecutor().(*sql.InternalExecutor), ts.Stopper(), ts.ClusterSettings(), - ts.SpanConfigAccessor().(spanconfig.KVAccessor), + ts.SpanConfigKVAccessor().(spanconfig.KVAccessor), ts.SpanConfigSQLWatcher().(spanconfig.SQLWatcher), ts.SpanConfigSQLTranslator().(spanconfig.SQLTranslator), &spanconfig.TestingKnobs{ @@ -238,7 +238,7 @@ func TestManagerCheckJobConditions(t *testing.T) { ts.InternalExecutor().(*sql.InternalExecutor), ts.Stopper(), ts.ClusterSettings(), - ts.SpanConfigAccessor().(spanconfig.KVAccessor), + ts.SpanConfigKVAccessor().(spanconfig.KVAccessor), ts.SpanConfigSQLWatcher().(spanconfig.SQLWatcher), ts.SpanConfigSQLTranslator().(spanconfig.SQLTranslator), &spanconfig.TestingKnobs{ diff --git a/pkg/sql/pgwire/auth_test.go b/pkg/sql/pgwire/auth_test.go index cfd25842251d..66a9c0823981 100644 --- a/pkg/sql/pgwire/auth_test.go +++ b/pkg/sql/pgwire/auth_test.go @@ -206,7 +206,7 @@ func hbaRunTest(t *testing.T, insecure bool) { // We can't use the cluster settings to do this, because // cluster settings propagate asynchronously. testServer := s.(*server.TestServer) - pgServer := s.(*server.TestServer).PGServer() + pgServer := s.(*server.TestServer).PGServer().(*pgwire.Server) pgServer.TestingEnableConnLogging() pgServer.TestingEnableAuthLogging() @@ -577,7 +577,7 @@ func TestClientAddrOverride(t *testing.T) { // We can't use the cluster settings to do this, because // cluster settings for booleans propagate asynchronously. testServer := s.(*server.TestServer) - pgServer := testServer.PGServer() + pgServer := testServer.PGServer().(*pgwire.Server) pgServer.TestingEnableAuthLogging() testCases := []struct { diff --git a/pkg/sql/pgwire/pgwire_test.go b/pkg/sql/pgwire/pgwire_test.go index f39db8dd97b3..5454006995e5 100644 --- a/pkg/sql/pgwire/pgwire_test.go +++ b/pkg/sql/pgwire/pgwire_test.go @@ -129,7 +129,7 @@ func TestPGWireDrainClient(t *testing.T) { } } - if !s.(*server.TestServer).PGServer().IsDraining() { + if !s.(*server.TestServer).PGServer().(*pgwire.Server).IsDraining() { t.Fatal("server should be draining, but is not") } } @@ -162,7 +162,7 @@ func TestPGWireDrainOngoingTxns(t *testing.T) { } defer db.Close() - pgServer := s.(*server.TestServer).PGServer() + pgServer := s.(*server.TestServer).PGServer().(*pgwire.Server) // Make sure that the server reports correctly the case in which a // connection did not respond to cancellation in time. diff --git a/pkg/testutils/serverutils/test_server_shim.go b/pkg/testutils/serverutils/test_server_shim.go index 9de4a23a259b..249132e8df13 100644 --- a/pkg/testutils/serverutils/test_server_shim.go +++ b/pkg/testutils/serverutils/test_server_shim.go @@ -28,10 +28,8 @@ import ( "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb" "github.com/cockroachdb/cockroach/pkg/roachpb" - "github.com/cockroachdb/cockroach/pkg/rpc" "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/server/status" - "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/storage" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" "github.com/cockroachdb/cockroach/pkg/util/hlc" @@ -45,10 +43,15 @@ import ( // TestServerInterface defines test server functionality that tests need; it is // implemented by server.TestServer. type TestServerInterface interface { - Stopper() *stop.Stopper - Start(context.Context) error + // TestTenantInterface embeds SQL-only APIs that tests need to interact with + // the host tenant. + // + // TODO(irfansharif): Audit the remaining symbols in TestServerInterface to + // see if they're better suited to TestTenantInterface. + TestTenantInterface + // Node returns the server.Node as an interface{}. Node() interface{} @@ -65,23 +68,13 @@ type TestServerInterface interface { // ServingSQLAddr returns the server's advertised SQL address. ServingSQLAddr() string - // HTTPAddr returns the server's http address. - HTTPAddr() string - // RPCAddr returns the server's RPC address. // Note: use ServingRPCAddr() instead unless specific reason not to. RPCAddr() string - // SQLAddr returns the server's SQL address. - // Note: use ServingSQLAddr() instead unless specific reason not to. - SQLAddr() string - // DB returns a *client.DB instance for talking to this KV server. DB() *kv.DB - // RPCContext returns the rpc context used by the test server. - RPCContext() *rpc.Context - // LeaseManager() returns the *sql.LeaseManager as an interface{}. LeaseManager() interface{} @@ -89,10 +82,6 @@ type TestServerInterface interface { // also implements sqlutil.InternalExecutor if the test cannot depend on sql). InternalExecutor() interface{} - // ExecutorConfig returns a copy of the server's ExecutorConfig. - // The real return type is sql.ExecutorConfig. - ExecutorConfig() interface{} - // TracerI returns a *tracing.Tracer as an interface{}. TracerI() interface{} @@ -100,13 +89,6 @@ type TestServerInterface interface { // The real return type is *gossip.Gossip. GossipI() interface{} - // RangeFeedFactory returns the range feed factory used by the TestServer. - // The real return type is *rangefeed.Factory. - RangeFeedFactory() interface{} - - // Clock returns the clock used by the TestServer. - Clock() *hlc.Clock - // DistSenderI returns the DistSender used by the TestServer. // The real return type is *kv.DistSender. DistSenderI() interface{} @@ -114,30 +96,12 @@ type TestServerInterface interface { // MigrationServer returns the internal *migrationServer as in interface{} MigrationServer() interface{} - // SpanConfigAccessor returns the underlying spanconfig.KVAccessor as an - // interface{}. - SpanConfigAccessor() interface{} - - // SpanConfigSQLTranslator returns the underlying spanconfig.SQLTranslator as - // an interface{}. - SpanConfigSQLTranslator() interface{} - - // SpanConfigSQLWatcher returns the underlying spanconfig.SQLWatcher as an - // interface{}. - SpanConfigSQLWatcher() interface{} - // SQLServer returns the *sql.Server as an interface{}. SQLServer() interface{} - // DistSQLServer returns the *distsql.ServerImpl as an interface{}. - DistSQLServer() interface{} - - // SQLLivenessStorage returns the sqlliveness.Provider as an interface{}. + // SQLLivenessProvider returns the sqlliveness.Provider as an interface{}. SQLLivenessProvider() interface{} - // JobRegistry returns the *jobs.Registry as an interface{}. - JobRegistry() interface{} - // StartupMigrationsManager returns the *startupmigrations.Manager as an interface{}. StartupMigrationsManager() interface{} @@ -197,10 +161,6 @@ type TestServerInterface interface { // The return value is of type *kvserver.Stores. GetStores() interface{} - // ClusterSettings returns the ClusterSettings shared by all components of - // this server. - ClusterSettings() *cluster.Settings - // Decommission idempotently sets the decommissioning flag for specified nodes. Decommission(ctx context.Context, targetStatus livenesspb.MembershipStatus, nodeIDs []roachpb.NodeID) error @@ -228,11 +188,6 @@ type TestServerInterface interface { // updates that are available. UpdateChecker() interface{} - // DiagnosticsReporter returns the server's *diagnostics.Reporter as an - // interface{}. The DiagnosticsReporter periodically phones home to report - // diagnostics and usage. - DiagnosticsReporter() interface{} - // StartTenant spawns off tenant process connecting to this TestServer. StartTenant(ctx context.Context, params base.TestTenantArgs) (TestTenantInterface, error) @@ -251,10 +206,6 @@ type TestServerInterface interface { // CollectionFactory returns a *descs.CollectionFactory. CollectionFactory() interface{} - - // TestingKnobs returns the TestingKnobs in use by the test - // server. - TestingKnobs() *base.TestingKnobs } // TestServerFactory encompasses the actual implementation of the shim diff --git a/pkg/testutils/serverutils/test_tenant_shim.go b/pkg/testutils/serverutils/test_tenant_shim.go index 86b28a6b3a95..37dd1ddaece6 100644 --- a/pkg/testutils/serverutils/test_tenant_shim.go +++ b/pkg/testutils/serverutils/test_tenant_shim.go @@ -18,10 +18,15 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/rpc" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/util/hlc" + "github.com/cockroachdb/cockroach/pkg/util/stop" ) // TestTenantInterface defines SQL-only tenant functionality that tests need; it -// is implemented by server.TestTenant. +// is implemented by server.Test{Tenant,Server}. Tests written against this +// interface are effectively agnostic to the type of tenant (host or secondary) +// they're dealing with. type TestTenantInterface interface { // SQLInstanceID is the ephemeral ID assigned to a running instance of the // SQLServer. Each tenant can have zero or more running SQLServer instances. @@ -45,20 +50,52 @@ type TestTenantInterface interface { // interface{}. StatusServer() interface{} - // DistSQLServer returns the *distsql.ServerImpl as an - // interface{}. + // DistSQLServer returns the *distsql.ServerImpl as an interface{}. DistSQLServer() interface{} // JobRegistry returns the *jobs.Registry as an interface{}. JobRegistry() interface{} - // TestingKnobs returns the TestingKnobs in use by the test - // tenant. - TestingKnobs() *base.TestingKnobs - - // RPCContext returns the *rpc.Context + // RPCContext returns the *rpc.Context used by the test tenant. RPCContext() *rpc.Context // AnnotateCtx annotates a context. AnnotateCtx(context.Context) context.Context + + // ExecutorConfig returns a copy of the tenant's ExecutorConfig. + // The real return type is sql.ExecutorConfig. + ExecutorConfig() interface{} + + // RangeFeedFactory returns the range feed factory used by the tenant. + // The real return type is *rangefeed.Factory. + RangeFeedFactory() interface{} + + // ClusterSettings returns the ClusterSettings shared by all components of + // this tenant. + ClusterSettings() *cluster.Settings + + // Stopper returns the stopper used by the tenant. + Stopper() *stop.Stopper + + // Clock returns the clock used by the tenant. + Clock() *hlc.Clock + + // SpanConfigKVAccessor returns the underlying spanconfig.KVAccessor as an + // interface{}. + SpanConfigKVAccessor() interface{} + + // SpanConfigSQLTranslator returns the underlying spanconfig.SQLTranslator as + // an interface{}. + SpanConfigSQLTranslator() interface{} + + // SpanConfigSQLWatcher returns the underlying spanconfig.SQLWatcher as an + // interface{}. + SpanConfigSQLWatcher() interface{} + + // TestingKnobs returns the TestingKnobs in use by the test server. + TestingKnobs() *base.TestingKnobs + + // TODO(irfansharif): We'd benefit from an API to construct a *gosql.DB, or + // better yet, a *sqlutils.SQLRunner. We use it all the time, constructing + // it by hand each time. }