diff --git a/pkg/ccl/serverccl/BUILD.bazel b/pkg/ccl/serverccl/BUILD.bazel index a119293f3d47..6d73ae7e6ca7 100644 --- a/pkg/ccl/serverccl/BUILD.bazel +++ b/pkg/ccl/serverccl/BUILD.bazel @@ -90,7 +90,6 @@ go_test( "//pkg/util/metric", "//pkg/util/protoutil", "//pkg/util/randutil", - "//pkg/util/stop", "//pkg/util/timeutil", "@com_github_cockroachdb_datadriven//:datadriven", "@com_github_cockroachdb_errors//:errors", diff --git a/pkg/ccl/serverccl/server_startup_guardrails_test.go b/pkg/ccl/serverccl/server_startup_guardrails_test.go index e306a0a33850..4f84446fe516 100644 --- a/pkg/ccl/serverccl/server_startup_guardrails_test.go +++ b/pkg/ccl/serverccl/server_startup_guardrails_test.go @@ -22,7 +22,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/util/leaktest" - "github.com/cockroachdb/cockroach/pkg/util/stop" + "github.com/cockroachdb/cockroach/pkg/util/log" ) // TestServerStartupGuardrails ensures that a SQL server will fail to start if @@ -74,69 +74,59 @@ func TestServerStartupGuardrails(t *testing.T) { } for i, test := range tests { - storageSettings := cluster.MakeTestingClusterSettingsWithVersions( - test.storageBinaryVersion, - test.storageBinaryMinSupportedVersion, - false, /* initializeVersion */ - ) + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + defer log.Scope(t).Close(t) - s, _, _ := serverutils.StartServer(t, base.TestServerArgs{ - // Disable the default test tenant, since we create one explicitly - // below. - DefaultTestTenant: base.TestTenantDisabled, - Settings: storageSettings, - Knobs: base.TestingKnobs{ - Server: &server.TestingKnobs{ - BinaryVersionOverride: test.storageBinaryVersion, - BootstrapVersionKeyOverride: clusterversion.V22_2, - DisableAutomaticVersionUpgrade: make(chan struct{}), - }, - SQLEvalContext: &eval.TestingKnobs{ - TenantLogicalVersionKeyOverride: test.TenantLogicalVersionKey, - }, - }, - }) + storageSettings := cluster.MakeTestingClusterSettingsWithVersions( + test.storageBinaryVersion, + test.storageBinaryMinSupportedVersion, + false, /* initializeVersion */ + ) - tenantSettings := cluster.MakeTestingClusterSettingsWithVersions( - test.tenantBinaryVersion, - test.tenantBinaryMinSupportedVersion, - true, /* initializeVersion */ - ) - - // The tenant will be created with an active version equal to the version - // corresponding to TenantLogicalVersionKey. Tenant creation is expected - // to succeed for all test cases but server creation is expected to succeed - // only if tenantBinaryVersion is at least equal to the version corresponding - // to TenantLogicalVersionKey. - stopper := stop.NewStopper() - tenantServer, err := s.StartTenant(context.Background(), - base.TestTenantArgs{ - Settings: tenantSettings, - TenantID: serverutils.TestTenantID(), - Stopper: stopper, - TestingKnobs: base.TestingKnobs{ + s, _, _ := serverutils.StartServer(t, base.TestServerArgs{ + // Disable the default test tenant, since we create one explicitly + // below. + DefaultTestTenant: base.TestTenantDisabled, + Settings: storageSettings, + Knobs: base.TestingKnobs{ Server: &server.TestingKnobs{ - BinaryVersionOverride: test.tenantBinaryVersion, + BinaryVersionOverride: test.storageBinaryVersion, + BootstrapVersionKeyOverride: clusterversion.V22_2, DisableAutomaticVersionUpgrade: make(chan struct{}), }, + SQLEvalContext: &eval.TestingKnobs{ + TenantLogicalVersionKeyOverride: test.TenantLogicalVersionKey, + }, }, }) + defer s.Stopper().Stop(context.Background()) - if !testutils.IsError(err, test.expErrMatch) { - t.Fatalf("test %d: got error %s, wanted error matching '%s'", i, err, test.expErrMatch) - } + tenantSettings := cluster.MakeTestingClusterSettingsWithVersions( + test.tenantBinaryVersion, + test.tenantBinaryMinSupportedVersion, + true, /* initializeVersion */ + ) - // Only attempt to stop the tenant if it was started successfully. - if err == nil { - tenantServer.Stopper().Stop(context.Background()) - } else { - // Test - stop the failed SQL server using a custom stopper - // NOTE: This custom stopper should not be required, but is because - // currently, if a SQL server fails to start it will not be cleaned - // up immediately without invoking the custom stopper. This could - // be a problem, and is tracked with #98868. - stopper.Stop(context.Background()) - } - s.Stopper().Stop(context.Background()) + // The tenant will be created with an active version equal to the version + // corresponding to TenantLogicalVersionKey. Tenant creation is expected + // to succeed for all test cases but server creation is expected to succeed + // only if tenantBinaryVersion is at least equal to the version corresponding + // to TenantLogicalVersionKey. + _, err := s.StartTenant(context.Background(), + base.TestTenantArgs{ + Settings: tenantSettings, + TenantID: serverutils.TestTenantID(), + TestingKnobs: base.TestingKnobs{ + Server: &server.TestingKnobs{ + BinaryVersionOverride: test.tenantBinaryVersion, + DisableAutomaticVersionUpgrade: make(chan struct{}), + }, + }, + }) + + if !testutils.IsError(err, test.expErrMatch) { + t.Fatalf("test %d: got error %s, wanted error matching '%s'", i, err, test.expErrMatch) + } + }) } }