Merge cockroachdb#108773

108773: roachtest: add ability to perform multiple upgrades in mixedversion r=smg260 a=renatolabs

This commit adds the ability for `mixedversion` tests to perform
multiple upgrades in certain tests runs. It has been observed that
certain types of errors only manifest when features are enabled in
older releases, and this makes it possible for us to get some coverage
for these scenarios.

By default, `mixedversion` tests will perform a random number of
upgrades (up to a maximum of 3). Test authors are able to override
this default by providing a minimum, maximum, or exact number of
upgrades test runs should go through.

Epic: CRDB-19321

Release note: None

Co-authored-by: Renato Costa <[email protected]>

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

@@ -20,7 +20,9 @@ import (
 	"sync/atomic"
 
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
+	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil/clusterupgrade"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/logger"
+	"github.com/cockroachdb/cockroach/pkg/util/version"
 )
 
 func (h *Helper) RandomNode(prng *rand.Rand, nodes option.NodeListOption) int {
@@ -119,6 +121,34 @@ func (h *Helper) ExpectDeaths(n int) {
 	h.runner.monitor.ExpectDeaths(n)
 }
 
+// LowestBinaryVersion returns a parsed `version.Version` object
+// corresponding to the lowest binary version used in the current
+// upgrade. The {Major, Minor} information in the version returned
+// provides a lower bound on the cluster version active when this
+// function is called. Test authors can use this information to
+// determine whether a certain feature is available.
+func (h *Helper) LowestBinaryVersion() *version.Version {
+	tc := h.Context()
+
+	var lowestVersion string
+	if tc.FromVersion == clusterupgrade.MainVersion {
+		lowestVersion = tc.ToVersion
+	} else if tc.ToVersion == clusterupgrade.MainVersion {
+		lowestVersion = tc.FromVersion
+	} else {
+		fromVersion := version.MustParse("v" + tc.FromVersion)
+		toVersion := version.MustParse("v" + tc.ToVersion)
+
+		if fromVersion.Compare(toVersion) < 0 {
+			lowestVersion = tc.FromVersion
+		} else {
+			lowestVersion = tc.ToVersion
+		}
+	}
+
+	return version.MustParse("v" + lowestVersion)
+}
+
 // loggerFor creates a logger instance to be used by background
 // functions (created by calling `Background` on the helper
 // instance). It is similar to the logger instances created for

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

@@ -107,6 +107,12 @@ const (
 	// finalized.
 	runWhileMigratingProbability = 0.5
 
+	// rollbackIntermediateUpgradesProbability is the probability that
+	// an "intermediate" upgrade (i.e., an upgrade to a version older
+	// than the one being tested) will also go through a rollback during
+	// a test run.
+	rollbackIntermediateUpgradesProbability = 0.3
+
 	// numNodesInFixtures is the number of nodes expected to exist in a
 	// cluster that can use the test fixtures in
 	// `pkg/cmd/roachtest/fixtures`.
@@ -132,6 +138,8 @@ var (
 		// detect bugs, especially in migrations.
 		useFixturesProbability: 0.7,
 		upgradeTimeout:         clusterupgrade.DefaultUpgradeTimeout,
+		minUpgrades:            1,
+		maxUpgrades:            3,
 	}
 )
 
@@ -230,6 +238,8 @@ type (
 	testOptions struct {
 		useFixturesProbability float64
 		upgradeTimeout         time.Duration
+		minUpgrades            int
+		maxUpgrades            int
 	}
 
 	customOption func(*testOptions)
@@ -265,7 +275,7 @@ type (
 		_buildVersion *version.Version
 		// test-only field, allows us to have deterministic tests even as
 		// the predecessor data changes.
-		predecessorFunc func(*rand.Rand, *version.Version) (string, error)
+		predecessorFunc func(*rand.Rand, *version.Version, int) ([]string, error)
 	}
 
 	shouldStop chan struct{}
@@ -301,6 +311,31 @@ func UpgradeTimeout(timeout time.Duration) customOption {
 	}
 }
 
+// MinUpgrades allows callers to set a minimum number of upgrades each
+// test run should exercise.
+func MinUpgrades(n int) customOption {
+	return func(opts *testOptions) {
+		opts.minUpgrades = n
+	}
+}
+
+// MaxUpgrades allows callers to set a maximum number of upgrades to
+// be performed during a test run.
+func MaxUpgrades(n int) customOption {
+	return func(opts *testOptions) {
+		opts.maxUpgrades = n
+	}
+}
+
+// NumUpgrades allows callers to specify the exact number of upgrades
+// every test run should perform.
+func NumUpgrades(n int) customOption {
+	return func(opts *testOptions) {
+		opts.minUpgrades = n
+		opts.maxUpgrades = n
+	}
+}
+
 // NewTest creates a Test struct that users can use to create and run
 // a mixed-version roachtest.
 func NewTest(
@@ -336,7 +371,7 @@ func NewTest(
 		prng:            prng,
 		seed:            seed,
 		hooks:           &testHooks{prng: prng, crdbNodes: crdbNodes},
-		predecessorFunc: release.RandomPredecessor,
+		predecessorFunc: release.RandomPredecessorHistory,
 	}
 
 	assertValidTest(test, t.Fatal)
@@ -486,19 +521,19 @@ func (t *Test) run(plan *TestPlan) error {
 }
 
 func (t *Test) plan() (*TestPlan, error) {
-	previousRelease, err := t.predecessorFunc(t.prng, t.buildVersion())
+	previousReleases, err := t.predecessorFunc(t.prng, t.buildVersion(), t.numUpgrades())
 	if err != nil {
 		return nil, err
 	}
 
 	planner := testPlanner{
-		initialVersion: previousRelease,
-		options:        t.options,
-		rt:             t.rt,
-		crdbNodes:      t.crdbNodes,
-		hooks:          t.hooks,
-		prng:           t.prng,
-		bgChans:        t.bgChans,
+		versions:  append(previousReleases, clusterupgrade.MainVersion),
+		options:   t.options,
+		rt:        t.rt,
+		crdbNodes: t.crdbNodes,
+		hooks:     t.hooks,
+		prng:      t.prng,
+		bgChans:   t.bgChans,
 	}
 
 	return planner.Plan(), nil
@@ -519,6 +554,15 @@ func (t *Test) runCommandFunc(nodes option.NodeListOption, cmd string) userFunc
 	}
 }
 
+// numUpgrades returns the number of upgrades that will be performed
+// in this test run. Returns a number in the [minUpgrades, maxUpgrades]
+// range.
+func (t *Test) numUpgrades() int {
+	return t.prng.Intn(
+		t.options.maxUpgrades-t.options.minUpgrades+1,
+	) + t.options.minUpgrades
+}
+
 // installFixturesStep is the step that copies the fixtures from
 // `pkg/cmd/roachtest/fixtures` for a specific version into the nodes'
 // store dir.
@@ -532,7 +576,7 @@ func (s installFixturesStep) ID() int                { return s.id }
 func (s installFixturesStep) Background() shouldStop { return nil }
 
 func (s installFixturesStep) Description() string {
-	return fmt.Sprintf("installing fixtures for version %q", s.version)
+	return fmt.Sprintf("install fixtures for version %q", s.version)
 }
 
 func (s installFixturesStep) Run(
@@ -554,7 +598,7 @@ func (s startStep) ID() int                { return s.id }
 func (s startStep) Background() shouldStop { return nil }
 
 func (s startStep) Description() string {
-	return fmt.Sprintf("starting cluster at version %q", s.version)
+	return fmt.Sprintf("start cluster at version %q", s.version)
 }
 
 // Run uploads the binary associated with the given version and starts
@@ -612,7 +656,7 @@ func (s preserveDowngradeOptionStep) ID() int                { return s.id }
 func (s preserveDowngradeOptionStep) Background() shouldStop { return nil }
 
 func (s preserveDowngradeOptionStep) Description() string {
-	return "preventing auto-upgrades by setting `preserve_downgrade_option`"
+	return "prevent auto-upgrades by setting `preserve_downgrade_option`"
 }
 
 func (s preserveDowngradeOptionStep) Run(
@@ -879,4 +923,12 @@ func assertValidTest(test *Test, fatalFunc func(...interface{})) {
 		)
 		fatalFunc(errors.Wrap(err, "mixedversion.NewTest"))
 	}
+
+	if test.options.minUpgrades > test.options.maxUpgrades {
+		err := fmt.Errorf(
+			"invalid test options: maxUpgrades (%d) must be greater than minUpgrades (%d)",
+			test.options.maxUpgrades, test.options.minUpgrades,
+		)
+		fatalFunc(errors.Wrap(err, "mixedversion.NewTest"))
+	}
 }

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

@@ -30,9 +30,10 @@ func Test_assertValidTest(t *testing.T) {
 		}
 	}
 
+	// Validating that number of nodes matches what is encoded in the
+	// fixtures if using them.
 	notEnoughNodes := option.NodeListOption{1, 2, 3}
 	tooManyNodes := option.NodeListOption{1, 2, 3, 5, 6}
-
 	for _, crdbNodes := range []option.NodeListOption{notEnoughNodes, tooManyNodes} {
 		mvt := newTest()
 		mvt.crdbNodes = crdbNodes
@@ -47,4 +48,15 @@ func Test_assertValidTest(t *testing.T) {
 		assertValidTest(mvt, fatalFunc())
 		require.NoError(t, fatalErr)
 	}
+
+	// Validating number of upgrades specified by the test.
+	mvt := newTest(MinUpgrades(10))
+	assertValidTest(mvt, fatalFunc())
+	require.Error(t, fatalErr)
+	require.Contains(t, fatalErr.Error(), "mixedversion.NewTest: invalid test options: maxUpgrades (3) must be greater than minUpgrades (10)")
+
+	mvt = newTest(MaxUpgrades(0))
+	assertValidTest(mvt, fatalFunc())
+	require.Error(t, fatalErr)
+	require.Contains(t, fatalErr.Error(), "mixedversion.NewTest: invalid test options: maxUpgrades (0) must be greater than minUpgrades (1)")
 }