Skip to content

Commit d88aca2

Browse files
committed
mixedversion: remove special case for skipping current version
In the past, we used to bump CRDB's minSupportedVersion _before_ bumping CRDB's current version. Consider the old version bump process: 1. Bump minimum supported version (e.g. 25.2→25.3) 2. Bump current version (e.g. 25.4→26.1) 3. Bump minimum supported version again (e.g. 25.3→25.4) After the first step, we would be in a state where the mixed version framework would see v25.3 as a skippable version (.1 and .3 ordinal versions are skippable), but since the minimum supported version is 25.3, 25.2→25.4 is (temporarily) not a legal upgrade. This required a special case to disallow skip upgrades in the case of this edge case. However, we now bump the current version _first_. This means we no longer need to protect against the above and we actually hit a different bug. Since we now bump the current version first, the special case will always view the current version as having multiple supported predecessors and force a skip upgrade even if not applicable.
1 parent ae0b3d7 commit d88aca2

File tree

6 files changed

+7
-28
lines changed

6 files changed

+7
-28
lines changed

pkg/cmd/roachtest/roachtestutil/mixedversion/BUILD.bazel

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ go_library(
1414
importpath = "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil/mixedversion",
1515
visibility = ["//visibility:public"],
1616
deps = [
17-
"//pkg/clusterversion",
1817
"//pkg/cmd/roachprod/grafana",
1918
"//pkg/cmd/roachtest/cluster",
2019
"//pkg/cmd/roachtest/option",
@@ -39,7 +38,6 @@ go_library(
3938
"//pkg/util/syncutil",
4039
"//pkg/util/timeutil",
4140
"@com_github_cockroachdb_errors//:errors",
42-
"@com_github_cockroachdb_version//:version",
4341
"@org_golang_x_exp//maps",
4442
],
4543
)
@@ -59,7 +57,6 @@ go_test(
5957
embed = [":mixedversion"],
6058
embedsrcs = ["testdata/test_releases.yaml"],
6159
deps = [
62-
"//pkg/clusterversion",
6360
"//pkg/cmd/roachtest/option",
6461
"//pkg/cmd/roachtest/registry",
6562
"//pkg/cmd/roachtest/roachtestutil",

pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ import (
7979
"sync"
8080
"time"
8181

82-
"github.com/cockroachdb/cockroach/pkg/clusterversion"
8382
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster"
8483
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
8584
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
@@ -94,7 +93,6 @@ import (
9493
"github.com/cockroachdb/cockroach/pkg/testutils/release"
9594
"github.com/cockroachdb/cockroach/pkg/util/randutil"
9695
"github.com/cockroachdb/errors"
97-
"github.com/cockroachdb/version"
9896
)
9997

10098
const (
@@ -613,14 +611,6 @@ func (t *Test) supportsSkipUpgradeTo(pred, v *clusterupgrade.Version) bool {
613611
if t.options.minimumSupportedVersion.Series() == pred.Series() {
614612
return false
615613
}
616-
// Special case for the current release series. This is useful to keep the
617-
// test correct when we bump the minimum supported version separately from
618-
// the current version.
619-
r := clusterversion.Latest.ReleaseSeries()
620-
currentMajor := version.MajorVersion{Year: int(r.Major), Ordinal: int(r.Minor)}
621-
if currentMajor.Equals(v.Version.Major()) {
622-
return len(clusterversion.SupportedPreviousReleases()) > 1
623-
}
624614

625615
series := v.Version.Major()
626616
switch {

pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion_test.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"math/rand"
1212
"testing"
1313

14-
"github.com/cockroachdb/cockroach/pkg/clusterversion"
1514
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
1615
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
1716
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil/clusterupgrade"
@@ -468,13 +467,6 @@ func TestSupportsSkipUpgradeTo(t *testing.T) {
468467
expect := func(verStr string, expected bool) {
469468
t.Helper()
470469
v := clusterupgrade.MustParseVersion(verStr)
471-
r := clusterversion.Latest.ReleaseSeries()
472-
currentMajor := version.MajorVersion{Year: int(r.Major), Ordinal: int(r.Minor)}
473-
if currentMajor.Equals(v.Version.Major()) {
474-
// We have to special case the current series, to allow for bumping the
475-
// min supported version separately from the current version.
476-
expected = len(clusterversion.SupportedPreviousReleases()) > 1
477-
}
478470
require.Equal(t, expected, mvt.supportsSkipUpgradeTo(prev, v))
479471
}
480472
for _, v := range []string{"v24.3.0", "v24.3.0-beta.1", "v25.2.1", "v25.2.0-rc.1"} {

pkg/cmd/roachtest/roachtestutil/mixedversion/testdata/planner/separate_process

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ Plan:
4444
├── delete all-tenants override for the `version` key (9) [stage=system:tenant-setup;tenant:tenant-setup]
4545
├── run startup hooks concurrently
4646
│ ├── run "create tables" hookId="planner_test.go:141", after 3m0s delay (10) [stage=system:on-startup;tenant:on-startup]
47-
│ └── run "initialize bank workload" hookId="mixedversion.go:884", after 100ms delay (11) [stage=system:on-startup;tenant:on-startup]
48-
├── run "bank workload" hookId="mixedversion.go:892" (12) [stage=system:background;tenant:background]
47+
│ └── run "initialize bank workload" hookId="mixedversion.go:874", after 100ms delay (11) [stage=system:on-startup;tenant:on-startup]
48+
├── run "bank workload" hookId="mixedversion.go:882" (12) [stage=system:background;tenant:background]
4949
├── upgrade cluster from "v22.2.3" to "v23.1.4"
5050
│ ├── upgrade storage cluster
5151
│ │ ├── prevent auto-upgrades on system tenant by setting `preserve_downgrade_option` (13) [stage=system:init;tenant:upgrading-system]

pkg/cmd/roachtest/roachtestutil/mixedversion/testdata/planner/shared_process

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ Plan:
5353
├── delete all-tenants override for the `version` key (16) [stage=system:tenant-setup;tenant:tenant-setup]
5454
├── run startup hooks concurrently
5555
│ ├── run "create tables" hookId="planner_test.go:141", after 30s delay (17) [stage=system:on-startup;tenant:on-startup]
56-
│ └── run "initialize bank workload" hookId="mixedversion.go:884", after 3m0s delay (18) [stage=system:on-startup;tenant:on-startup]
57-
├── run "bank workload" hookId="mixedversion.go:892" (19) [stage=system:background;tenant:background]
56+
│ └── run "initialize bank workload" hookId="mixedversion.go:874", after 3m0s delay (18) [stage=system:on-startup;tenant:on-startup]
57+
├── run "bank workload" hookId="mixedversion.go:882" (19) [stage=system:background;tenant:background]
5858
├── upgrade cluster from "v23.1.12" to "v23.2.0"
5959
│ ├── prevent auto-upgrades on system tenant by setting `preserve_downgrade_option` (20) [stage=system:init;tenant:init]
6060
│ ├── prevent auto-upgrades on mixed-version-tenant-cyvju tenant by setting `preserve_downgrade_option` (21) [stage=system:init;tenant:init]

pkg/cmd/roachtest/roachtestutil/mixedversion/testdata/planner/step_stages

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,10 @@ Plan:
3535
├── install fixtures for version "v22.2.3" (1) [stage=system-setup]
3636
├── start cluster at version "v22.2.3" (2) [stage=system-setup]
3737
├── wait for all nodes (:1-4) to acknowledge cluster version '22.2' on system tenant (3) [stage=system-setup]
38-
├── run "initialize bank workload" hookId="mixedversion.go:884" (4) [stage=on-startup]
38+
├── run "initialize bank workload" hookId="mixedversion.go:874" (4) [stage=on-startup]
3939
├── start background hooks concurrently
40-
│ ├── run "bank workload" hookId="mixedversion.go:892", after 0s delay (5) [stage=background]
41-
│ └── run "csv server" hookId="mixedversion.go:861", after 0s delay (6) [stage=background]
40+
│ ├── run "bank workload" hookId="mixedversion.go:882", after 0s delay (5) [stage=background]
41+
│ └── run "csv server" hookId="mixedversion.go:851", after 0s delay (6) [stage=background]
4242
├── upgrade cluster from "v22.2.3" to "v23.1.4"
4343
│ ├── prevent auto-upgrades on system tenant by setting `preserve_downgrade_option` (7) [stage=init]
4444
│ ├── upgrade nodes :1-4 from "v22.2.3" to "v23.1.4"

0 commit comments

Comments
 (0)