Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
64703: roachtest: fix multitenant-upgrade r=erikgrinaker a=tbg

The test failed for two reasons:

- it was hard-coding 20.2 in one place where it shouldn't have
- it implicitly assumes that PredecessorVersion(21.2) is a 21.1 release
  that has cockroachdb#60730, but that will only be true in a month or so (when we
  have 21.1.1). Without cockroachdb#60730, tenant 11 in the test will have no
  cluster version persisted and so things don't work well. Work around
  this by setting the cluster version explicitly. This isn't a
  production concern since we are not going to upgrade tenants without
  a cluster version into 21.2; they will either have been created by a
  21.1.1+ host cluster (and thus have a cluster version) or been
  migrated from a 20.2 deployment into a 21.1.1+ deployment, which also
  entails giving them a cluster version.

Fixes cockroachdb#64615.

Release note: None


Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>
  • Loading branch information
craig[bot] and tbg committed May 5, 2021
2 parents 6e4f431 + 8ce1da0 commit 3b8ec3a
Showing 1 changed file with 29 additions and 6 deletions.
35 changes: 29 additions & 6 deletions pkg/cmd/roachtest/multitenant_upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,13 @@ func runMultiTenantUpgrade(ctx context.Context, t *test, c *cluster, v version.V
withResults([][]string{{"1", "bar"}}))

t.Status("preserving downgrade option on host server")
runner.Exec(t, `SET CLUSTER SETTING cluster.preserve_downgrade_option = '20.2'`)
{
s := runner.QueryStr(t, `SHOW CLUSTER SETTING version`)
runner.Exec(
t,
`SET CLUSTER SETTING cluster.preserve_downgrade_option = $1`, s[0][0],
)
}

t.Status("upgrading host server")
c.Stop(ctx, kvNodes)
Expand Down Expand Up @@ -230,12 +236,29 @@ func runMultiTenantUpgrade(ctx context.Context, t *test, c *cluster, v version.V
t.Status("starting the tenant 11 server with the current binary")
tenant11.start(ctx, t, c, currentBinary)

if v, err := version.Parse("v" + predecessor); err != nil {
t.Fatal(err)
} else if !v.AtLeast(version.MustParse("v21.1.1")) {
// If we take this branch, the current branch is 21.2 but we haven't released 21.1.1 yet.
// The multi-tenant upgrade PR #60730 will only be contained in 21.1.1. That PR is responsible
// for giving tenants the initial cluster version, which is thus absent here. We still want to
// run this test until #60730 is backported, so fill the cluster version in. Without this, the
// tenant would report a cluster version of 20.2 which is going to trip up the test. We won't
// actually upgrade tenants in production in scenarios in which this branch is active (the host
// cluster will be at the latest patch release before attempting an upgrade, which is going to
// be at least 21.1.1).
// This can be deleted once the PredecessorVersion(21.2) is >= 21.1.1.
verifySQL(t, tenant11.pgURL, mkStmt(`SET CLUSTER SETTING version = $1`, initialVersion))
}

t.Status("verify tenant 11 server works with the new binary")
verifySQL(t, tenant11.pgURL,
mkStmt(`SELECT * FROM foo LIMIT 1`).
withResults([][]string{{"1", "bar"}}),
mkStmt("SHOW CLUSTER SETTING version").
withResults([][]string{{initialVersion}}))
{
verifySQL(t, tenant11.pgURL,
mkStmt(`SELECT * FROM foo LIMIT 1`).
withResults([][]string{{"1", "bar"}}),
mkStmt("SHOW CLUSTER SETTING version").
withResults([][]string{{initialVersion}}))
}

// Note that this is exercising a path we likely want to eliminate in the
// future where the tenant is upgraded before the KV nodes.
Expand Down

0 comments on commit 3b8ec3a

Please sign in to comment.