diff --git a/pkg/upgrade/upgradebase/testing_knobs.go b/pkg/upgrade/upgradebase/testing_knobs.go index 59d0215e2648..48bc4d55466a 100644 --- a/pkg/upgrade/upgradebase/testing_knobs.go +++ b/pkg/upgrade/upgradebase/testing_knobs.go @@ -27,9 +27,9 @@ const ( // are useful for testing. type TestingKnobs struct { - // ListBetweenOverride injects an override for `clusterversion.ListBetween() - // in order to run upgrades corresponding to versions which do not - // actually exist. + // ListBetweenOverride injects an override for clusterversion.ListBetween() in + // order to run upgrades corresponding to versions which do not actually + // exist. This function has to return versions in the range (from, to]. ListBetweenOverride func(from, to roachpb.Version) []roachpb.Version // RegistryOverride is used to inject upgrades for specific cluster versions. diff --git a/pkg/upgrade/upgrademanager/manager.go b/pkg/upgrade/upgrademanager/manager.go index c548e5a6c14b..9f0c202abf87 100644 --- a/pkg/upgrade/upgrademanager/manager.go +++ b/pkg/upgrade/upgrademanager/manager.go @@ -828,7 +828,14 @@ func (m *Manager) getRunningMigrationJob( func (m *Manager) listBetween(from roachpb.Version, to roachpb.Version) []roachpb.Version { if m.knobs.ListBetweenOverride != nil { - return m.knobs.ListBetweenOverride(from, to) + result := m.knobs.ListBetweenOverride(from, to) + // Sanity check result to catch invalid overrides. + for _, v := range result { + if v.LessEq(from) || to.Less(v) { + panic(fmt.Sprintf("ListBetweenOverride(%s, %s) returned invalid version %s", from, to, v)) + } + } + return result } return clusterversion.ListBetween(from, to) } diff --git a/pkg/upgrade/upgrademanager/manager_external_test.go b/pkg/upgrade/upgrademanager/manager_external_test.go index 6402b2809c67..3430037a0ca3 100644 --- a/pkg/upgrade/upgrademanager/manager_external_test.go +++ b/pkg/upgrade/upgrademanager/manager_external_test.go @@ -100,7 +100,7 @@ func TestAlreadyRunningJobsAreHandledProperly(t *testing.T) { }, UpgradeManager: &upgradebase.TestingKnobs{ ListBetweenOverride: func(from, to roachpb.Version) []roachpb.Version { - return []roachpb.Version{from, to} + return []roachpb.Version{to} }, RegistryOverride: func(v roachpb.Version) (upgradebase.Upgrade, bool) { if v != endCV.Version() { @@ -399,7 +399,7 @@ func TestMigrateUpdatesReplicaVersion(t *testing.T) { }, UpgradeManager: &upgradebase.TestingKnobs{ ListBetweenOverride: func(from, to roachpb.Version) []roachpb.Version { - return []roachpb.Version{from, to} + return []roachpb.Version{to} }, RegistryOverride: func(cv roachpb.Version) (upgradebase.Upgrade, bool) { if cv != endCV { @@ -504,7 +504,13 @@ func TestConcurrentMigrationAttempts(t *testing.T) { }, UpgradeManager: &upgradebase.TestingKnobs{ ListBetweenOverride: func(from, to roachpb.Version) []roachpb.Version { - return versions + var res []roachpb.Version + for _, v := range versions { + if from.Less(v) && v.LessEq(to) { + res = append(res, v) + } + } + return res }, RegistryOverride: func(cv roachpb.Version) (upgradebase.Upgrade, bool) { return upgrade.NewSystemUpgrade("test", cv, func( @@ -556,7 +562,7 @@ func TestConcurrentMigrationAttempts(t *testing.T) { for k, c := range migrationRunCounts { require.Equalf(t, 1, c, "version: %v", k) } - require.Len(t, migrationRunCounts, len(versions)) + require.Len(t, migrationRunCounts, len(versions)-1) } // TestPauseMigration ensures that upgrades can indeed be paused and that @@ -592,7 +598,13 @@ func TestPauseMigration(t *testing.T) { }, UpgradeManager: &upgradebase.TestingKnobs{ ListBetweenOverride: func(from, to roachpb.Version) []roachpb.Version { - return []roachpb.Version{from, to} + // We expect calls to ListBetween to be made for (0.0, startCV] and (startCV, endCV]. + if to != startCV.Version() { + if from != startCV.Version() || to != endCV.Version() { + panic(fmt.Sprintf("unexpected versions %v, %v\n", from, to)) + } + } + return []roachpb.Version{to} }, RegistryOverride: func(cv roachpb.Version) (upgradebase.Upgrade, bool) { if cv != endCV.Version() { diff --git a/pkg/upgrade/upgrades/schema_changes_external_test.go b/pkg/upgrade/upgrades/schema_changes_external_test.go index a364ca1824f7..3395e111fd0d 100644 --- a/pkg/upgrade/upgrades/schema_changes_external_test.go +++ b/pkg/upgrade/upgrades/schema_changes_external_test.go @@ -351,9 +351,7 @@ func testMigrationWithFailures( }, UpgradeManager: &upgradebase.TestingKnobs{ ListBetweenOverride: func(from, to roachpb.Version) []roachpb.Version { - return []roachpb.Version{ - endCV, - } + return []roachpb.Version{to} }, RegistryOverride: func(cv roachpb.Version) (upgradebase.Upgrade, bool) { if cv.Equal(endCV) { @@ -364,7 +362,7 @@ func testMigrationWithFailures( upgrade.RestoreActionNotRequired("test"), ), true } - panic("unexpected version") + return nil, false }}, }, },