Merge #107866

107866: testserver: remove direct casts to `*server.TestServer` r=yuzefovich,stevendanna a=knz

First 3 commits from #107868.

In a later stage we would like the result of `StartServer` to return different concrete types depending on parameters. Before we can do that however, we need to remove direct casts of that return value to `*server.TestServer`.


Incidentally, this patch improves the APIs as follows:

- the health probe for secondary tenant servers now properly
  includes the draining state of the RPC interface (previously,
  only the SQL draining state was included).

- `ApplicationLayerInterface` now includes `Readiness()` and
  `DefaultZoneConfig()`.

- `StorageLayerInterface` now includes
  `ScratchRangeWithExpirationLease()`, `GetRangeLease()`, `TsDB()`,
  `Locality()` and `DefaultSystemZoneConfig()`.

Epic: CRDB-18499

Co-authored-by: Raphael 'kena' Poss <[email protected]>

.github/CODEOWNERS

@@ -490,6 +490,7 @@
 /pkg/roachpb/.gitattributes  @cockroachdb/dev-inf
 #!/pkg/roachpb/BUILD.bazel     @cockroachdb/kv-prs-noreview
 /pkg/roachpb/data*           @cockroachdb/kv-prs
+/pkg/roachpb/leaseinfo*      @cockroachdb/kv-prs
 /pkg/roachpb/index*          @cockroachdb/cluster-observability
 /pkg/roachpb/internal*       @cockroachdb/kv-prs
 /pkg/roachpb/io-formats*     @cockroachdb/disaster-recovery

pkg/BUILD.bazel

@@ -1529,6 +1529,7 @@ GO_TARGETS = [
     "//pkg/server/debug/replay:replay",
     "//pkg/server/debug:debug",
     "//pkg/server/debug:debug_test",
+    "//pkg/server/decommissioning:decommissioning",
     "//pkg/server/diagnostics/diagnosticspb:diagnosticspb",
     "//pkg/server/diagnostics:diagnostics",
     "//pkg/server/diagnostics:diagnostics_test",

pkg/bench/BUILD.bazel

@@ -15,7 +15,6 @@ go_library(
         "//pkg/ccl",
         "//pkg/multitenant/tenantcapabilities",
         "//pkg/roachpb",
-        "//pkg/server",
         "//pkg/testutils",
         "//pkg/testutils/serverutils",
         "//pkg/testutils/skip",

pkg/bench/foreachdb.go

@@ -26,7 +26,6 @@ import (
 	_ "github.com/cockroachdb/cockroach/pkg/ccl"
 	"github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
-	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
@@ -71,7 +70,7 @@ func benchmarkSharedProcessTenantCockroach(b *testing.B, f BenchmarkFn) {
 
 	// Create our own test tenant with a known name.
 	tenantName := "benchtenant"
-	_, tenantDB, err := s.(*server.TestServer).StartSharedProcessTenant(ctx,
+	_, tenantDB, err := s.TenantController().StartSharedProcessTenant(ctx,
 		base.TestSharedProcessTenantArgs{
 			TenantName:  roachpb.TenantName(tenantName),
 			UseDatabase: "bench",

pkg/ccl/backupccl/backup_tenant_test.go

@@ -19,7 +19,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/jobs"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
-	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	_ "github.com/cockroachdb/cockroach/pkg/sql/importer"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
@@ -87,7 +86,7 @@ func TestBackupTenantImportingTable(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if err := tc.Server(0).(*server.TestServer).WaitForTenantReadiness(ctx, tenantID); err != nil {
+	if err := tc.Server(0).TenantController().WaitForTenantReadiness(ctx, tenantID); err != nil {
 		t.Fatal(err)
 	}
 

pkg/ccl/backupccl/backup_test.go

@@ -69,7 +69,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
-	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/spanconfig"
 	"github.com/cockroachdb/cockroach/pkg/sql"
@@ -7150,7 +7149,7 @@ func TestBackupRestoreTenant(t *testing.T) {
 		)
 
 		tenantID := roachpb.MustMakeTenantID(10)
-		if err := restoreTC.Server(0).(*server.TestServer).WaitForTenantReadiness(ctx, tenantID); err != nil {
+		if err := restoreTC.Server(0).TenantController().WaitForTenantReadiness(ctx, tenantID); err != nil {
 			t.Fatal(err)
 		}
 
@@ -7242,7 +7241,7 @@ func TestBackupRestoreTenant(t *testing.T) {
 		)
 
 		tenantID := roachpb.MustMakeTenantID(10)
-		if err := restoreTC.Server(0).(*server.TestServer).WaitForTenantReadiness(ctx, tenantID); err != nil {
+		if err := restoreTC.Server(0).TenantController().WaitForTenantReadiness(ctx, tenantID); err != nil {
 			t.Fatal(err)
 		}
 
@@ -7316,7 +7315,7 @@ func TestBackupRestoreTenant(t *testing.T) {
 		)
 
 		tenantID := roachpb.MustMakeTenantID(10)
-		if err := restoreTC.Server(0).(*server.TestServer).WaitForTenantReadiness(ctx, tenantID); err != nil {
+		if err := restoreTC.Server(0).TenantController().WaitForTenantReadiness(ctx, tenantID); err != nil {
 			t.Fatal(err)
 		}
 
@@ -7334,7 +7333,7 @@ func TestBackupRestoreTenant(t *testing.T) {
 		restoreTenant10.CheckQueryResults(t, `SHOW CLUSTER SETTING tenant_cost_model.write_payload_cost_per_mebibyte`, [][]string{{"456"}})
 
 		tenantID = roachpb.MustMakeTenantID(11)
-		if err := restoreTC.Server(0).(*server.TestServer).WaitForTenantReadiness(ctx, tenantID); err != nil {
+		if err := restoreTC.Server(0).TenantController().WaitForTenantReadiness(ctx, tenantID); err != nil {
 			t.Fatal(err)
 		}
 
@@ -7355,7 +7354,7 @@ func TestBackupRestoreTenant(t *testing.T) {
 		restoreDB.Exec(t, `RESTORE TENANT 11 FROM 'nodelocal://1/clusterwide' WITH virtual_cluster = '20', virtual_cluster_name = 'tenant-20'`)
 
 		tenantID = roachpb.MustMakeTenantID(20)
-		if err := restoreTC.Server(0).(*server.TestServer).WaitForTenantReadiness(ctx, tenantID); err != nil {
+		if err := restoreTC.Server(0).TenantController().WaitForTenantReadiness(ctx, tenantID); err != nil {
 			t.Fatal(err)
 		}
 
@@ -7392,7 +7391,7 @@ func TestBackupRestoreTenant(t *testing.T) {
 		restoreDB.Exec(t, `RESTORE TENANT 10 FROM 'nodelocal://1/t10' AS OF SYSTEM TIME `+ts1)
 
 		tenantID := roachpb.MustMakeTenantID(10)
-		if err := restoreTC.Server(0).(*server.TestServer).WaitForTenantReadiness(ctx, tenantID); err != nil {
+		if err := restoreTC.Server(0).TenantController().WaitForTenantReadiness(ctx, tenantID); err != nil {
 			t.Fatal(err)
 		}
 
@@ -7418,7 +7417,7 @@ func TestBackupRestoreTenant(t *testing.T) {
 		restoreDB.Exec(t, `RESTORE TENANT 20 FROM 'nodelocal://1/t20'`)
 
 		tenantID := roachpb.MustMakeTenantID(20)
-		if err := restoreTC.Server(0).(*server.TestServer).WaitForTenantReadiness(ctx, tenantID); err != nil {
+		if err := restoreTC.Server(0).TenantController().WaitForTenantReadiness(ctx, tenantID); err != nil {
 			t.Fatal(err)
 		}
 

pkg/ccl/changefeedccl/alter_changefeed_test.go

@@ -26,7 +26,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
-	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/desctestutils"
@@ -50,7 +49,9 @@ func TestAlterChangefeedAddTargetPrivileges(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{
+	ctx := context.Background()
+
+	s, db, _ := serverutils.StartServer(t, base.TestServerArgs{
 		DefaultTestTenant: base.TODOTestTenantDisabled,
 		Knobs: base.TestingKnobs{
 			JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(),
@@ -66,8 +67,6 @@ func TestAlterChangefeedAddTargetPrivileges(t *testing.T) {
 			},
 		},
 	})
-	ctx := context.Background()
-	s := srv.(*server.TestServer)
 	defer s.Stopper().Stop(ctx)
 
 	rootDB := sqlutils.MakeSQLRunner(db)

pkg/ccl/changefeedccl/changefeed_test.go

@@ -1744,7 +1744,7 @@ func TestChangefeedLaggingSpanCheckpointing(t *testing.T) {
 	defer stopServer()
 	sqlDB := sqlutils.MakeSQLRunner(db)
 
-	knobs := s.(*server.TestServer).Cfg.TestingKnobs.
+	knobs := s.TestingKnobs().
 		DistSQL.(*execinfra.TestingKnobs).
 		Changefeed.(*TestingKnobs)
 
@@ -2695,7 +2695,8 @@ func TestChangefeedCreateAuthorizationWithChangefeedPriv(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{
+	ctx := context.Background()
+	s, db, _ := serverutils.StartServer(t, base.TestServerArgs{
 		DefaultTestTenant: base.TODOTestTenantDisabled,
 		Knobs: base.TestingKnobs{
 			JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(),
@@ -2711,8 +2712,6 @@ func TestChangefeedCreateAuthorizationWithChangefeedPriv(t *testing.T) {
 			},
 		},
 	})
-	ctx := context.Background()
-	s := srv.(*server.TestServer)
 	defer s.Stopper().Stop(ctx)
 
 	rootDB := sqlutils.MakeSQLRunner(db)

pkg/ccl/changefeedccl/helpers_test.go

@@ -796,7 +796,7 @@ func makeSystemServerWithOptions(
 			TestServer: TestServer{
 				DB:           systemDB,
 				Server:       systemServer,
-				TestingKnobs: systemServer.(*server.TestServer).Cfg.TestingKnobs,
+				TestingKnobs: *systemServer.SystemLayer().TestingKnobs(),
 				Codec:        keys.SystemSQLCodec,
 			},
 			SystemServer: systemServer,
@@ -822,7 +822,7 @@ func makeTenantServerWithOptions(
 			TestServer: TestServer{
 				DB:           tenantDB,
 				Server:       tenantServer,
-				TestingKnobs: tenantServer.(*server.TestTenant).Cfg.TestingKnobs,
+				TestingKnobs: *tenantServer.TestingKnobs(),
 				Codec:        keys.MakeSQLCodec(tenantID),
 			},
 			SystemDB:     systemDB,

pkg/ccl/changefeedccl/scheduled_changefeed_test.go

@@ -28,7 +28,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/scheduledjobs"
 	"github.com/cockroachdb/cockroach/pkg/scheduledjobs/schedulebase"
 	"github.com/cockroachdb/cockroach/pkg/security/username"
-	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
 	"github.com/cockroachdb/cockroach/pkg/sql/parser"
@@ -290,7 +289,8 @@ func TestCreateChangefeedScheduleChecksPermissionsDuringDryRun(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{
+	ctx := context.Background()
+	s, db, _ := serverutils.StartServer(t, base.TestServerArgs{
 		DefaultTestTenant: base.TODOTestTenantDisabled,
 		Knobs: base.TestingKnobs{
 			JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(),
@@ -306,8 +306,6 @@ func TestCreateChangefeedScheduleChecksPermissionsDuringDryRun(t *testing.T) {
 			},
 		},
 	})
-	ctx := context.Background()
-	s := srv.(*server.TestServer)
 	defer s.Stopper().Stop(ctx)
 	rootDB := sqlutils.MakeSQLRunner(db)
 	rootDB.Exec(t, `SET CLUSTER SETTING kv.rangefeed.enabled = true`)

pkg/ccl/kvccl/kvfollowerreadsccl/followerreads_test.go

@@ -803,7 +803,7 @@ func TestFollowerReadsWithStaleDescriptor(t *testing.T) {
 	// Make a note of the follower reads metric on n3. We'll check that it was
 	// incremented.
 	var followerReadsCountBefore int64
-	err := tc.Servers[2].Stores().VisitStores(func(s *kvserver.Store) error {
+	err := tc.Servers[2].GetStores().(*kvserver.Stores).VisitStores(func(s *kvserver.Store) error {
 		followerReadsCountBefore = s.Metrics().FollowerReadsCount.Count()
 		return nil
 	})
@@ -820,7 +820,7 @@ func TestFollowerReadsWithStaleDescriptor(t *testing.T) {
 
 	// Check that the follower read metric was incremented.
 	var followerReadsCountAfter int64
-	err = tc.Servers[2].Stores().VisitStores(func(s *kvserver.Store) error {
+	err = tc.Servers[2].GetStores().(*kvserver.Stores).VisitStores(func(s *kvserver.Store) error {
 		followerReadsCountAfter = s.Metrics().FollowerReadsCount.Count()
 		return nil
 	})
@@ -1025,7 +1025,7 @@ func TestSecondaryTenantFollowerReadsRouting(t *testing.T) {
 		getFollowerReadCounts := func() [numNodes]int64 {
 			var counts [numNodes]int64
 			for i := range tc.Servers {
-				err := tc.Servers[i].Stores().VisitStores(func(s *kvserver.Store) error {
+				err := tc.Servers[i].GetStores().(*kvserver.Stores).VisitStores(func(s *kvserver.Store) error {
 					counts[i] = s.Metrics().FollowerReadsCount.Count()
 					return nil
 				})

pkg/ccl/multiregionccl/BUILD.bazel

@@ -66,6 +66,7 @@ go_test(
         "//pkg/server",
         "//pkg/server/serverpb",
         "//pkg/settings/cluster",
+        "//pkg/spanconfig",
         "//pkg/sql",
         "//pkg/sql/catalog",
         "//pkg/sql/catalog/catpb",

pkg/ccl/multiregionccl/cold_start_latency_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/rpc"
 	"github.com/cockroachdb/cockroach/pkg/server"
+	"github.com/cockroachdb/cockroach/pkg/spanconfig"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils/regionlatency"
@@ -288,7 +289,7 @@ SELECT checkpoint > extract(epoch from after)
 	// Wait for the configs to be applied.
 	testutils.SucceedsWithin(t, func() error {
 		for _, server := range tc.Servers {
-			reporter := server.Server.SpanConfigReporter()
+			reporter := server.SpanConfigReporter().(spanconfig.Reporter)
 			report, err := reporter.SpanConfigConformance(ctx, []roachpb.Span{
 				{Key: keys.TableDataMin, EndKey: keys.TenantTableDataMax},
 			})

pkg/ccl/multiregionccl/region_util_test.go

@@ -41,7 +41,7 @@ func TestGetLocalityRegionEnumPhysicalRepresentation(t *testing.T) {
 	tDB := sqlutils.MakeSQLRunner(sqlDB)
 	tDB.Exec(t, `CREATE DATABASE foo PRIMARY REGION "us-east1" REGIONS "us-east1", "us-east2", "us-east3"`)
 
-	s0 := tc.ServerTyped(0)
+	s0 := tc.Server(0)
 	idb := s0.InternalDB().(descs.DB)
 	dbID := descpb.ID(sqlutils.QueryDatabaseID(t, sqlDB, "foo"))
 

pkg/ccl/partitionccl/zone_test.go

@@ -17,7 +17,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/config/zonepb"
 	"github.com/cockroachdb/cockroach/pkg/keys"
-	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
@@ -55,9 +54,9 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
 			PARTITION p1 VALUES IN (DEFAULT)
 		)`)
 
-	yamlDefault := fmt.Sprintf("gc: {ttlseconds: %d}", s.(*server.TestServer).Cfg.DefaultZoneConfig.GC.TTLSeconds)
+	yamlDefault := fmt.Sprintf("gc: {ttlseconds: %d}", s.DefaultZoneConfig().GC.TTLSeconds)
 	yamlOverride := "gc: {ttlseconds: 42}"
-	zoneOverride := s.(*server.TestServer).Cfg.DefaultZoneConfig
+	zoneOverride := s.DefaultZoneConfig()
 	zoneOverride.GC = &zonepb.GCPolicy{TTLSeconds: 42}
 	partialZoneOverride := *zonepb.NewZoneConfig()
 	partialZoneOverride.GC = &zonepb.GCPolicy{TTLSeconds: 42}
@@ -67,7 +66,7 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
 
 	defaultRow := sqlutils.ZoneRow{
 		ID:     keys.RootNamespaceID,
-		Config: s.(*server.TestServer).Cfg.DefaultZoneConfig,
+		Config: s.DefaultZoneConfig(),
 	}
 	defaultOverrideRow := sqlutils.ZoneRow{
 		ID:     keys.RootNamespaceID,
@@ -403,11 +402,11 @@ CREATE TABLE t.test (k INT PRIMARY KEY, v INT);`); err != nil {
 
 	defaultRow := sqlutils.ZoneRow{
 		ID:     keys.RootNamespaceID,
-		Config: s.(*server.TestServer).Cfg.DefaultZoneConfig,
+		Config: s.DefaultZoneConfig(),
 	}
 
 	tableID := sqlutils.QueryTableID(t, sqlDB, "t", "public", "test")
-	zoneOverride := s.(*server.TestServer).Cfg.DefaultZoneConfig
+	zoneOverride := s.DefaultZoneConfig()
 	zoneOverride.GC = &zonepb.GCPolicy{TTLSeconds: 42}
 
 	overrideRow := sqlutils.ZoneRow{

pkg/ccl/serverccl/admin_test.go

@@ -16,7 +16,6 @@ import (
 	"testing"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
-	"github.com/cockroachdb/cockroach/pkg/server"
 	"github.com/cockroachdb/cockroach/pkg/server/serverpb"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
@@ -177,7 +176,7 @@ func TestListTenants(t *testing.T) {
 	})
 	defer s.Stopper().Stop(ctx)
 
-	_, _, err := s.(*server.TestServer).StartSharedProcessTenant(ctx,
+	_, _, err := s.TenantController().StartSharedProcessTenant(ctx,
 		base.TestSharedProcessTenantArgs{
 			TenantName: "test",
 		})