clusterversion: move ReleaseSeries functionality to roacphb.Version

This change moves the implementation of
`clusterversion.Key.ReleaseSeries()` to `roachpb`. We now use a
hardcoded map of sucessor versions.

Epic: none
Release note: None

pkg/clusterversion/cockroach_versions.go

@@ -10,10 +10,7 @@
 
 package clusterversion
 
-import (
-	"github.com/cockroachdb/cockroach/pkg/build"
-	"github.com/cockroachdb/cockroach/pkg/roachpb"
-)
+import "github.com/cockroachdb/cockroach/pkg/roachpb"
 
 // Key is a unique identifier for a version of CockroachDB.
 type Key int
@@ -425,27 +422,18 @@ func (k Key) IsFinal() bool {
 	return k.Version().IsFinal()
 }
 
-// ReleaseSeries returns the final version for the release series the Key
-// belongs to. Specifically:
-//   - if the key corresponds to a final version (e.g. 23.2), the result is the
-//     same version; e.g. V23_2.ReleaseSeries() is v23.2.
-//   - if the key corresponds to a transitional upgrade version (e.g. v23.2-8),
-//     the result is the next final version (e.g. v24.1).
-//
-// Note that the result does not have any DevOffset applied.
-func (k Key) ReleaseSeries() roachpb.Version {
-	// Find the first key >= k that is a final version.
-	for k := k; k < numKeys; k++ {
-		if k.IsFinal() {
-			return removeDevOffset(k.Version())
-		}
-	}
-	// k is a dev version in the latest release series.
-	major, minor := build.BranchReleaseSeries()
-	return roachpb.Version{
-		Major: int32(major),
-		Minor: int32(minor),
-	}
+// ReleaseSeries returns the release series the Key. Specifically:
+//   - if the key corresponds to a final version (e.g. V23_2), the result has the
+//     same major/minor;
+//   - if the key corresponds to a transitional upgrade version (e.g.
+//     V23_2SomeFeature with version 23.2-x), the result is the next series
+//     (e.g. 24.1).
+//
+// The key must be in the range [MinSupported, Latest].
+func (k Key) ReleaseSeries() roachpb.ReleaseSeries {
+	// Note: TestReleaseSeries ensures that this works for all valid Keys.
+	s, _ := removeDevOffset(k.Version()).ReleaseSeries()
+	return s
 }
 
 func (k Key) String() string {

pkg/clusterversion/cockroach_versions_test.go

@@ -160,13 +160,23 @@ func TestClusterVersionPrettyPrint(t *testing.T) {
 }
 
 func TestReleaseSeries(t *testing.T) {
-	require.Equal(t, fmt.Sprintf("v%s", Latest.ReleaseSeries()), build.BinaryVersionPrefix())
-	if Latest.IsFinal() {
-		require.True(t, Latest.Version() == Latest.ReleaseSeries())
-	} else {
-		require.True(t, removeDevOffset(Latest.Version()).Less(Latest.ReleaseSeries()))
+	// Verify that the ReleaseSeries call works on all keys.
+	for k := Latest; k > 0; k-- {
+		if k.Version().Major > 0 {
+			require.NotEqual(t, roachpb.ReleaseSeries{}, k.ReleaseSeries())
+		} else {
+			require.Equal(t, roachpb.ReleaseSeries{}, k.ReleaseSeries())
+		}
+	}
+
+	// Verify the ReleaseSeries results down to MinSupported.
+	expected := Latest.ReleaseSeries()
+	require.Equal(t, fmt.Sprintf("v%s", expected), build.BinaryVersionPrefix())
+	for k := Latest; k >= MinSupported; k-- {
+		if k.IsFinal() {
+			v := removeDevOffset(k.Version())
+			expected = roachpb.ReleaseSeries{Major: v.Major, Minor: v.Minor}
+		}
+		require.Equalf(t, expected, k.ReleaseSeries(), "version: %s", k)
 	}
-	require.Equal(t, PreviousRelease.ReleaseSeries(), removeDevOffset(PreviousRelease.Version()))
-	require.Equal(t, (PreviousRelease - 1).ReleaseSeries(), removeDevOffset(PreviousRelease.Version()))
-	require.Equal(t, MinSupported.ReleaseSeries(), removeDevOffset(MinSupported.Version()))
 }

pkg/cmd/sql-bootstrap-data/main.go

@@ -55,7 +55,7 @@ func main() {
 }
 
 // writeDataFile creates a file in pkg/sql/catalog/bootstrap/data.
-func writeDataFile(version roachpb.Version, filenameSuffix, data string) {
+func writeDataFile(version roachpb.ReleaseSeries, filenameSuffix, data string) {
 	filename := filepath.Join(
 		"pkg", "sql", "catalog", "bootstrap", "data",
 		fmt.Sprintf("%d_%d_%s", version.Major, version.Minor, filenameSuffix),

pkg/roachpb/version.go

@@ -130,3 +130,56 @@ func MustParseVersion(s string) Version {
 	}
 	return v
 }
+
+// ReleaseSeries is just the major.minor part of a Version.
+type ReleaseSeries struct {
+	Major int32
+	Minor int32
+}
+
+func (s ReleaseSeries) String() string {
+	return fmt.Sprintf("%d.%d", s.Major, s.Minor)
+}
+
+// Successor returns the next release series, if known. This is only guaranteed
+// to work for versions from the minimum supported series up to the previous
+// series.
+func (s ReleaseSeries) Successor() (_ ReleaseSeries, ok bool) {
+	res, ok := successorSeries[s]
+	return res, ok
+}
+
+// successorSeries stores the successor for each series. We are only concerned
+// with versions within our compatibility window, but there is no harm in
+// populating more if they are known.
+//
+// When this map is updated, the expected result in TestReleaseSeriesSuccessor
+// needs to be updated. Also note that clusterversion tests ensure that this map
+// contains all necessary versions.
+var successorSeries = map[ReleaseSeries]ReleaseSeries{
+	{20, 1}: {20, 2},
+	{20, 2}: {21, 1},
+	{21, 1}: {21, 2},
+	{21, 2}: {22, 1},
+	{22, 1}: {22, 2},
+	{22, 2}: {23, 1},
+	{23, 1}: {23, 2},
+	{23, 2}: {24, 1},
+}
+
+// ReleaseSeries obtains the release series for the given version. Specifically:
+//   - if the version is final (Internal=0), the ReleaseSeries has the same major/minor.
+//   - if the version is a transitional version during upgrade (e.g. v23.1-8),
+//     the result is the next final version (e.g. v23.1).
+//
+// For non-final versions (which indicate an update to the next series), this
+// requires knowledge of the next series; unknown non-final versions will return
+// ok=false.
+func (v Version) ReleaseSeries() (_ ReleaseSeries, ok bool) {
+	base := ReleaseSeries{v.Major, v.Minor}
+	if v.IsFinal() {
+		return base, true
+	}
+	res, ok := base.Successor()
+	return res, ok
+}

pkg/roachpb/version_test.go

@@ -11,9 +11,11 @@
 package roachpb
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/kr/pretty"
+	"github.com/stretchr/testify/require"
 )
 
 func TestVersionCmp(t *testing.T) {
@@ -60,3 +62,45 @@ func TestVersionCmp(t *testing.T) {
 		})
 	}
 }
+
+func TestReleaseSeriesSuccessor(t *testing.T) {
+	r := ReleaseSeries{20, 1}
+	var seq []string
+	for ok := true; ok; r, ok = r.Successor() {
+		seq = append(seq, r.String())
+	}
+	expected := "20.1, 20.2, 21.1, 21.2, 22.1, 22.2, 23.1, 23.2, 24.1"
+	require.Equal(t, expected, strings.Join(seq, ", "))
+}
+
+func TestReleaseSeries(t *testing.T) {
+	testCases := []struct {
+		v Version
+		s ReleaseSeries
+	}{
+		{
+			v: Version{Major: 22, Minor: 2, Internal: 0},
+			s: ReleaseSeries{Major: 22, Minor: 2},
+		},
+		{
+			v: Version{Major: 22, Minor: 2, Internal: 8},
+			s: ReleaseSeries{Major: 23, Minor: 1},
+		},
+		{
+			v: Version{Major: 23, Minor: 1, Internal: 0},
+			s: ReleaseSeries{Major: 23, Minor: 1},
+		},
+		{
+			v: Version{Major: 23, Minor: 1, Internal: 2},
+			s: ReleaseSeries{Major: 23, Minor: 2},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run("", func(t *testing.T) {
+			res, ok := tc.v.ReleaseSeries()
+			require.True(t, ok)
+			require.Equal(t, tc.s, res)
+		})
+	}
+}