Skip to content

Commit

Permalink
upgrademanager: simplify TestMigrationFailure
Browse files Browse the repository at this point in the history
This also removes `TODOTestTenantDisabled`.

(I have verified the test works, albeit still flaky due to cockroachdb#106648, by
temporarily removing the skip.)

Release note: None
  • Loading branch information
knz committed Jul 17, 2023
1 parent e0d4e62 commit a2127b2
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 13 deletions.
1 change: 1 addition & 0 deletions pkg/upgrade/upgrademanager/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ go_test(
args = ["-test.timeout=295s"],
deps = [
"//pkg/base",
"//pkg/ccl",
"//pkg/ccl/kvccl/kvtenantccl",
"//pkg/clusterversion",
"//pkg/jobs",
Expand Down
2 changes: 2 additions & 0 deletions pkg/upgrade/upgrademanager/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"os"
"testing"

"github.com/cockroachdb/cockroach/pkg/ccl"
"github.com/cockroachdb/cockroach/pkg/security/securityassets"
"github.com/cockroachdb/cockroach/pkg/security/securitytest"
"github.com/cockroachdb/cockroach/pkg/server"
Expand All @@ -25,6 +26,7 @@ func TestMain(m *testing.M) {
securityassets.SetLoader(securitytest.EmbeddedAssets)
serverutils.InitTestServerFactory(server.TestServerFactory)
serverutils.InitTestClusterFactory(testcluster.TestClusterFactory)
defer ccl.TestingEnableEnterprise()()
os.Exit(m.Run())
}

Expand Down
22 changes: 9 additions & 13 deletions pkg/upgrade/upgrademanager/manager_external_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -689,24 +689,20 @@ func TestMigrationFailure(t *testing.T) {
fenceVersion := upgrade.FenceVersionFor(ctx, clusterversion.ClusterVersion{Version: failVersion}).Version
t.Logf("test will fail at version: %s", failVersion.String())

// Create a storage cluster for the tenant
testCluster := serverutils.StartNewTestCluster(t, 1, base.TestClusterArgs{
ServerArgs: base.TestServerArgs{
DefaultTestTenant: base.TODOTestTenantDisabled,
Knobs: base.TestingKnobs{
SQLEvalContext: &eval.TestingKnobs{
TenantLogicalVersionKeyOverride: startVersionKey,
},
// Create a storage cluster for the tenant.
s, goDB, _ := serverutils.StartServer(t, base.TestServerArgs{
DefaultTestTenant: base.TestControlsTenantsExplicitly,
Knobs: base.TestingKnobs{
SQLEvalContext: &eval.TestingKnobs{
TenantLogicalVersionKeyOverride: startVersionKey,
},
},
})
defer testCluster.Stopper().Stop(ctx)
defer s.Stopper().Stop(ctx)

// Set the version override so that the tenant is able to upgrade. If this is
// not set, the tenant treats the storage cluster as if it had the oldest
// supported binary version.
s := testCluster.Server(0)
goDB := serverutils.OpenDBConn(t, s.ServingSQLAddr(), "system", false, s.Stopper())
_, err := goDB.Exec(`ALTER TENANT ALL SET CLUSTER SETTING version = $1`, endVersion.String())
require.NoError(t, err)

Expand All @@ -722,7 +718,7 @@ func TestMigrationFailure(t *testing.T) {
false,
)
require.NoError(t, clusterversion.Initialize(ctx, startVersion, &tenantSettings.SV))
tenant, db := serverutils.StartTenant(t, testCluster.Server(0), base.TestTenantArgs{
tenant, db := serverutils.StartTenant(t, s, base.TestTenantArgs{
TenantID: roachpb.MustMakeTenantID(10),
Settings: tenantSettings,
TestingKnobs: base.TestingKnobs{
Expand Down Expand Up @@ -773,7 +769,7 @@ func TestMigrationFailure(t *testing.T) {
checkActiveVersion(t, startVersion)
checkSettingVersion(t, startVersion)

// Try to finalize
// Try to finalize.
_, err = db.Exec(`SET CLUSTER SETTING version = $1`, endVersion.String())
require.Error(t, err)
checkActiveVersion(t, fenceVersion)
Expand Down

0 comments on commit a2127b2

Please sign in to comment.