Skip to content

Commit

Permalink
roachtest: normalize versions in multitenant-upgrade roachtest
Browse files Browse the repository at this point in the history
If the multitenant-upgrade roachtest uses both binaries built
with `crdb_test` and ones that weren't, it may be using versions
that are technically the same but fail to confirm that because
version in test binaries are incremented by 1M.

This code change fixes the issue by normalizing versions before
comparing them.

Closes cockroachdb#95648

Epic: none
Release note: None
  • Loading branch information
healthy-pod committed Jan 30, 2023
1 parent 69dd453 commit 452c5ee
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 13 deletions.
6 changes: 4 additions & 2 deletions pkg/clusterversion/cockroach_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,9 @@ func (k Key) String() string {
// previously referenced a < 22.1 version until that check/gate can be removed.
const TODOPreV22_1 = V22_1

// Offset every version +1M major versions into the future if this is a dev branch.
const DevOffset = 1000000

// rawVersionsSingleton lists all historical versions here in chronological
// order, with comments describing what backwards-incompatible features were
// introduced.
Expand Down Expand Up @@ -750,7 +753,6 @@ var versionsSingleton = func() keyedVersions {
// which conceptually is actually back down to 2 -- then back to to 1000003,
// then on to 1000004, etc.
skipFirst := allowUpgradeToDev
const devOffset = 1000000
first := true
for i := range rawVersionsSingleton {
// VPrimordial versions are not offset; they don't matter for the logic
Expand All @@ -763,7 +765,7 @@ var versionsSingleton = func() keyedVersions {
first = false
continue
}
rawVersionsSingleton[i].Major += devOffset
rawVersionsSingleton[i].Major += DevOffset
}
}
return rawVersionsSingleton
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/roachtest/tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ go_library(
"//pkg/cloud",
"//pkg/cloud/amazon",
"//pkg/cloud/gcp",
"//pkg/clusterversion",
"//pkg/cmd/cmpconn",
"//pkg/cmd/roachtest/cluster",
"//pkg/cmd/roachtest/clusterstats",
Expand Down
55 changes: 44 additions & 11 deletions pkg/cmd/roachtest/tests/multitenant_upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ import (
"runtime"
"time"

"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster"
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil/clusterupgrade"
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/roachprod/install"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/util/version"
Expand Down Expand Up @@ -107,7 +109,8 @@ func runMultiTenantUpgrade(ctx context.Context, t test.Test, c cluster.Cluster,

verifySQL(t, tenant11.pgURL,
mkStmt("SHOW CLUSTER SETTING version").
withResults([][]string{{initialVersion}}),
withResults([][]string{{initialVersion}}).
withCustomResultsEqualFn(requireEqualVersionsIgnoreDevOffset),
)

t.Status("preserving downgrade option on system tenant")
Expand Down Expand Up @@ -149,7 +152,8 @@ func runMultiTenantUpgrade(ctx context.Context, t test.Test, c cluster.Cluster,
mkStmt(`SELECT * FROM foo LIMIT 1`).
withResults([][]string{{"1", "bar"}}),
mkStmt("SHOW CLUSTER SETTING version").
withResults([][]string{{initialVersion}}),
withResults([][]string{{initialVersion}}).
withCustomResultsEqualFn(requireEqualVersionsIgnoreDevOffset),
)

t.Status("creating a new tenant 13")
Expand All @@ -171,7 +175,8 @@ func runMultiTenantUpgrade(ctx context.Context, t test.Test, c cluster.Cluster,
mkStmt(`SELECT * FROM foo LIMIT 1`).
withResults([][]string{{"1", "bar"}}),
mkStmt("SHOW CLUSTER SETTING version").
withResults([][]string{{initialVersion}}),
withResults([][]string{{initialVersion}}).
withCustomResultsEqualFn(requireEqualVersionsIgnoreDevOffset),
)

t.Status("stopping the tenant 11 server ahead of upgrading")
Expand All @@ -186,7 +191,9 @@ func runMultiTenantUpgrade(ctx context.Context, t test.Test, c cluster.Cluster,
mkStmt(`SELECT * FROM foo LIMIT 1`).
withResults([][]string{{"1", "bar"}}),
mkStmt("SHOW CLUSTER SETTING version").
withResults([][]string{{initialVersion}}))
withResults([][]string{{initialVersion}}).
withCustomResultsEqualFn(requireEqualVersionsIgnoreDevOffset),
)
}

t.Status("attempting to upgrade tenant 11 before storage cluster is finalized and expecting a failure")
Expand All @@ -206,7 +213,8 @@ func runMultiTenantUpgrade(ctx context.Context, t test.Test, c cluster.Cluster,
mkStmt(`SELECT * FROM foo LIMIT 1`).
withResults([][]string{{"1", "bar"}}),
mkStmt("SHOW CLUSTER SETTING version").
withResults([][]string{{initialVersion}}),
withResults([][]string{{initialVersion}}).
withCustomResultsEqualFn(requireEqualVersionsIgnoreDevOffset),
mkStmt("SET CLUSTER SETTING version = crdb_internal.node_executable_version()"),
mkStmt("SELECT version = crdb_internal.node_executable_version() FROM [SHOW CLUSTER SETTING version]").
withResults([][]string{{"true"}}),
Expand All @@ -223,7 +231,9 @@ func runMultiTenantUpgrade(ctx context.Context, t test.Test, c cluster.Cluster,
mkStmt(`SELECT * FROM foo LIMIT 1`).
withResults([][]string{{"1", "bar"}}),
mkStmt("SHOW CLUSTER SETTING version").
withResults([][]string{{initialVersion}}))
withResults([][]string{{initialVersion}}).
withCustomResultsEqualFn(requireEqualVersionsIgnoreDevOffset),
)

// Upgrade the tenant created in the mixed version state to the final version.
t.Status("migrating tenant 12 to the current version")
Expand Down Expand Up @@ -295,19 +305,42 @@ func runMultiTenantUpgrade(ctx context.Context, t test.Test, c cluster.Cluster,
withResults([][]string{{"true"}}))
}

// EqualFn is implemented by both `require.Equal` and `requireEqualVersionsIgnoreDevOffset`.
type EqualFn = func(t require.TestingT, expected interface{}, actual interface{}, msgAndArgs ...interface{})

func requireEqualVersionsIgnoreDevOffset(
t require.TestingT, expected interface{}, actual interface{}, msgAndArgs ...interface{},
) {
normalizeVersion := func(v roachpb.Version) roachpb.Version {
if (v.Major - clusterversion.DevOffset) >= 0 {
v.Major -= clusterversion.DevOffset
}
return v
}
normalizedExpectedVersion := normalizeVersion(roachpb.MustParseVersion(expected.([][]string)[0][0]))
normalizedActualVersion := normalizeVersion(roachpb.MustParseVersion(actual.([][]string)[0][0]))
require.Equal(t, normalizedExpectedVersion, normalizedActualVersion, msgAndArgs...)
}

type sqlVerificationStmt struct {
stmt string
args []interface{}
optionalResults [][]string
stmt string
args []interface{}
optionalResults [][]string
optionalResultsEqualFn EqualFn
}

func (s sqlVerificationStmt) withResults(res [][]string) sqlVerificationStmt {
s.optionalResults = res
return s
}

func (s sqlVerificationStmt) withCustomResultsEqualFn(fn EqualFn) sqlVerificationStmt {
s.optionalResultsEqualFn = fn
return s
}

func mkStmt(stmt string, args ...interface{}) sqlVerificationStmt {
return sqlVerificationStmt{stmt: stmt, args: args}
return sqlVerificationStmt{stmt: stmt, args: args, optionalResultsEqualFn: require.Equal}
}

func openDBAndMakeSQLRunner(t test.Test, url string) (*sqlutils.SQLRunner, func()) {
Expand All @@ -328,7 +361,7 @@ func verifySQL(t test.Test, url string, stmts ...sqlVerificationStmt) {
tdb.Exec(t, stmt.stmt, stmt.args...)
} else {
res := tdb.QueryStr(t, stmt.stmt, stmt.args...)
require.Equal(t, stmt.optionalResults, res)
stmt.optionalResultsEqualFn(t, stmt.optionalResults, res)
}
}
}
Expand Down

0 comments on commit 452c5ee

Please sign in to comment.