Skip to content

mixedversion: remove special case for skipping current version #157852

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 18, 2025
Merged

mixedversion: remove special case for skipping current version #157852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions pkg/cmd/roachtest/roachtestutil/mixedversion/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ go_library(
importpath = "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil/mixedversion",
visibility = ["//visibility:public"],
deps = [
"//pkg/clusterversion",
"//pkg/cmd/roachprod/grafana",
"//pkg/cmd/roachtest/cluster",
"//pkg/cmd/roachtest/option",
Expand All @@ -39,7 +38,6 @@ go_library(
"//pkg/util/syncutil",
"//pkg/util/timeutil",
"@com_github_cockroachdb_errors//:errors",
"@com_github_cockroachdb_version//:version",
"@org_golang_x_exp//maps",
],
)
Expand All @@ -59,7 +57,6 @@ go_test(
embed = [":mixedversion"],
embedsrcs = ["testdata/test_releases.yaml"],
deps = [
"//pkg/clusterversion",
"//pkg/cmd/roachtest/option",
"//pkg/cmd/roachtest/registry",
"//pkg/cmd/roachtest/roachtestutil",
Expand Down
10 changes: 0 additions & 10 deletions pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ import (
"sync"
"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"
Expand All @@ -94,7 +93,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/testutils/release"
"github.com/cockroachdb/cockroach/pkg/util/randutil"
"github.com/cockroachdb/errors"
"github.com/cockroachdb/version"
)

const (
Expand Down Expand Up @@ -613,14 +611,6 @@ func (t *Test) supportsSkipUpgradeTo(pred, v *clusterupgrade.Version) bool {
if t.options.minimumSupportedVersion.Series() == pred.Series() {
return false
}
// Special case for the current release series. This is useful to keep the
// test correct when we bump the minimum supported version separately from
// the current version.
r := clusterversion.Latest.ReleaseSeries()
currentMajor := version.MajorVersion{Year: int(r.Major), Ordinal: int(r.Minor)}
Copy link
Member

@srosenberg srosenberg Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that Test_UpgradePaths didn't fail suggests it's weakly specified. I think we should strengthen it by asserting the "ordinal invariant", i.e., no skips from even.

if currentMajor.Equals(v.Version.Major()) {
return len(clusterversion.SupportedPreviousReleases()) > 1
}

series := v.Version.Major()
switch {
Expand Down
8 changes: 0 additions & 8 deletions pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"math/rand"
"testing"

"github.com/cockroachdb/cockroach/pkg/clusterversion"
"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"
Expand Down Expand Up @@ -468,13 +467,6 @@ func TestSupportsSkipUpgradeTo(t *testing.T) {
expect := func(verStr string, expected bool) {
t.Helper()
v := clusterupgrade.MustParseVersion(verStr)
r := clusterversion.Latest.ReleaseSeries()
currentMajor := version.MajorVersion{Year: int(r.Major), Ordinal: int(r.Minor)}
if currentMajor.Equals(v.Version.Major()) {
// We have to special case the current series, to allow for bumping the
// min supported version separately from the current version.
expected = len(clusterversion.SupportedPreviousReleases()) > 1
}
require.Equal(t, expected, mvt.supportsSkipUpgradeTo(prev, v))
}
for _, v := range []string{"v24.3.0", "v24.3.0-beta.1", "v25.2.1", "v25.2.0-rc.1"} {
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/roachtest/roachtestutil/mixedversion/testdata/planner/separate_process
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ Plan:
├── delete all-tenants override for the `version` key (9) [stage=system:tenant-setup;tenant:tenant-setup]
├── run startup hooks concurrently
│ ├── run "create tables" hookId="planner_test.go:141", after 3m0s delay (10) [stage=system:on-startup;tenant:on-startup]
│ └── run "initialize bank workload" hookId="mixedversion.go:884", after 100ms delay (11) [stage=system:on-startup;tenant:on-startup]
├── run "bank workload" hookId="mixedversion.go:892" (12) [stage=system:background;tenant:background]
│ └── run "initialize bank workload" hookId="mixedversion.go:874", after 100ms delay (11) [stage=system:on-startup;tenant:on-startup]
├── run "bank workload" hookId="mixedversion.go:882" (12) [stage=system:background;tenant:background]
├── upgrade cluster from "v22.2.3" to "v23.1.4"
│ ├── upgrade storage cluster
│ │ ├── prevent auto-upgrades on system tenant by setting `preserve_downgrade_option` (13) [stage=system:init;tenant:upgrading-system]
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/roachtest/roachtestutil/mixedversion/testdata/planner/shared_process
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ Plan:
├── delete all-tenants override for the `version` key (16) [stage=system:tenant-setup;tenant:tenant-setup]
├── run startup hooks concurrently
│ ├── run "create tables" hookId="planner_test.go:141", after 30s delay (17) [stage=system:on-startup;tenant:on-startup]
│ └── run "initialize bank workload" hookId="mixedversion.go:884", after 3m0s delay (18) [stage=system:on-startup;tenant:on-startup]
├── run "bank workload" hookId="mixedversion.go:892" (19) [stage=system:background;tenant:background]
│ └── run "initialize bank workload" hookId="mixedversion.go:874", after 3m0s delay (18) [stage=system:on-startup;tenant:on-startup]
├── run "bank workload" hookId="mixedversion.go:882" (19) [stage=system:background;tenant:background]
├── upgrade cluster from "v23.1.12" to "v23.2.0"
│ ├── prevent auto-upgrades on system tenant by setting `preserve_downgrade_option` (20) [stage=system:init;tenant:init]
│ ├── prevent auto-upgrades on mixed-version-tenant-cyvju tenant by setting `preserve_downgrade_option` (21) [stage=system:init;tenant:init]
Expand Down
6 changes: 3 additions & 3 deletions pkg/cmd/roachtest/roachtestutil/mixedversion/testdata/planner/step_stages
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ Plan:
├── install fixtures for version "v22.2.3" (1) [stage=system-setup]
├── start cluster at version "v22.2.3" (2) [stage=system-setup]
├── wait for all nodes (:1-4) to acknowledge cluster version '22.2' on system tenant (3) [stage=system-setup]
├── run "initialize bank workload" hookId="mixedversion.go:884" (4) [stage=on-startup]
├── run "initialize bank workload" hookId="mixedversion.go:874" (4) [stage=on-startup]
├── start background hooks concurrently
│ ├── run "bank workload" hookId="mixedversion.go:892", after 0s delay (5) [stage=background]
│ └── run "csv server" hookId="mixedversion.go:861", after 0s delay (6) [stage=background]
│ ├── run "bank workload" hookId="mixedversion.go:882", after 0s delay (5) [stage=background]
│ └── run "csv server" hookId="mixedversion.go:851", after 0s delay (6) [stage=background]
├── upgrade cluster from "v22.2.3" to "v23.1.4"
│ ├── prevent auto-upgrades on system tenant by setting `preserve_downgrade_option` (7) [stage=init]
│ ├── upgrade nodes :1-4 from "v22.2.3" to "v23.1.4"
Expand Down