From e456cc3f2cda3fa4ebb0f3ad280b17681fcf5f1c Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Wed, 6 Nov 2024 07:22:52 -0800 Subject: [PATCH] mixedversion: update skip upgrades logic This commit updates the logic that determines which versions support skipping the previous major release during upgrade. Epic: REL-1292 Release note: None --- .../roachtestutil/mixedversion/BUILD.bazel | 2 + .../mixedversion/mixedversion.go | 67 +++++++++++-------- .../mixedversion/mixedversion_test.go | 21 ++++++ 3 files changed, 63 insertions(+), 27 deletions(-) diff --git a/pkg/cmd/roachtest/roachtestutil/mixedversion/BUILD.bazel b/pkg/cmd/roachtest/roachtestutil/mixedversion/BUILD.bazel index 2546431d325d..d591d17bd376 100644 --- a/pkg/cmd/roachtest/roachtestutil/mixedversion/BUILD.bazel +++ b/pkg/cmd/roachtest/roachtestutil/mixedversion/BUILD.bazel @@ -14,6 +14,7 @@ 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", @@ -54,6 +55,7 @@ go_test( embed = [":mixedversion"], embedsrcs = ["testdata/test_releases.yaml"], deps = [ + "//pkg/clusterversion", "//pkg/cmd/roachtest/option", "//pkg/cmd/roachtest/registry", "//pkg/cmd/roachtest/roachtestutil", diff --git a/pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion.go b/pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion.go index 849bf842263a..aa4ab0a46382 100644 --- a/pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion.go +++ b/pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion.go @@ -76,6 +76,7 @@ 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" @@ -481,10 +482,32 @@ func WithTag(tag string) CustomOption { } } -// minSupportedSkipVersionUpgrade is the minimum version after which -// "skip version" upgrades are supported (i.e., upgrading two major -// releases in a single upgrade). -var minSupportedSkipVersionUpgrade = clusterupgrade.MustParseVersion("v24.1.0") +// supportsSkipUpgradeTo returns true if the given version supports skipping the +// previous major version during upgrade. For example, 24.3 supports upgrade +// directly from 24.1, but 25.1 only supports upgrade from 24.3. +func supportsSkipUpgradeTo(v *clusterupgrade.Version) bool { + major, minor := v.Version.Major(), v.Version.Minor() + + // 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. + if r := clusterversion.Latest.ReleaseSeries(); int(r.Major) == major && int(r.Minor) == minor { + return len(clusterversion.SupportedPreviousReleases()) > 1 + } + + switch { + case major < 24: + return false + case major == 24: + // v24.2 supported skip upgrades experimentally. + // v24.3 is the first version which officially supports the skip upgrade. + return minor == 2 || minor == 3 + default: + // The current plan for 2025+ is for .1 and .3 to be skippable innovation + // releases. + return minor == 2 || minor == 4 + } +} func defaultTestOptions() testOptions { return testOptions{ @@ -835,11 +858,9 @@ func (t *Test) choosePreviousReleases() ([]*clusterupgrade.Version, error) { return nil, nil } - // If skip-version upgrades are not enabled, the only possible - // predecessor is the immediate predecessor release. If the - // predecessor doesn't support skip versions, then its predecessor - // won't either. Don't attempt to find it. - if !skipVersions || !pred.AtLeast(minSupportedSkipVersionUpgrade) { + // If skip-version upgrades are not enabled or v does not support them, the + // only possible predecessor is the immediate predecessor release. + if !skipVersions || !supportsSkipUpgradeTo(v) { return []*clusterupgrade.Version{pred}, nil } @@ -848,26 +869,18 @@ func (t *Test) choosePreviousReleases() ([]*clusterupgrade.Version, error) { return nil, err } - if predPred.AtLeast(minSupportedSkipVersionUpgrade) { - // If the predecessor's predecessor supports skip-version - // upgrades and we haven't performed a skip-version upgrade yet, - // do it. This logic makes sure that, when skip-version upgrades - // are enabled, it happens when upgrading to the current - // release, which is the most important upgrade to be tested on - // any release branch. - if numSkips == 0 { - numSkips++ - return []*clusterupgrade.Version{predPred}, nil - } - - // If we already performed a skip-version upgrade on this test - // plan, we can choose to do another one or not. - return []*clusterupgrade.Version{pred, predPred}, nil + // If we haven't performed a skip-version upgrade yet, do it. This logic + // makes sure that, when skip-version upgrades are enabled, it happens + // when upgrading to the current release, which is the most important + // upgrade to be tested on any release branch. + if numSkips == 0 { + numSkips++ + return []*clusterupgrade.Version{predPred}, nil } - // If the predecessor is too old and does not support skip-version - // upgrades, it's the only possible predecessor. - return []*clusterupgrade.Version{pred}, nil + // If we already performed a skip-version upgrade on this test + // plan, we can choose to do another one or not. + return []*clusterupgrade.Version{pred, predPred}, nil } currentVersion := clusterupgrade.CurrentVersion() diff --git a/pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion_test.go b/pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion_test.go index 028de870e2fd..603bda0e31c7 100644 --- a/pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion_test.go +++ b/pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion_test.go @@ -11,6 +11,7 @@ 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" @@ -368,3 +369,23 @@ func withTestBuildVersion(v string) func() { clusterupgrade.TestBuildVersion = testBuildVersion return func() { clusterupgrade.TestBuildVersion = nil } } + +func TestSupportsSkipUpgradeTo(t *testing.T) { + expect := func(verStr string, expected bool) { + t.Helper() + v := clusterupgrade.MustParseVersion(verStr) + if r := clusterversion.Latest.ReleaseSeries(); int(r.Major) == v.Major() && int(r.Minor) == v.Minor() { + // 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, supportsSkipUpgradeTo(v)) + } + for _, v := range []string{"v24.3.0", "v24.3.0-beta.1", "v25.2.1", "v25.2.0-rc.1"} { + expect(v, true) + } + + for _, v := range []string{"v25.1.0", "v25.1.0-beta.1", "v25.3.1", "v25.3.0-rc.1"} { + expect(v, false) + } +}