Merge pull request #127776 from cockroachdb/blathers/backport-release…

…-23.2-127615

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

@@ -82,6 +82,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
+	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil/clusterupgrade"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
@@ -636,7 +637,7 @@ func (t *Test) Workload(
 func (t *Test) Run() {
 	plan, err := t.plan()
 	if err != nil {
-		t.rt.Fatal(fmt.Errorf("error creating test plan: %w", err))
+		t.rt.Fatal(err)
 	}
 
 	t.logger.Printf("mixed-version test:\n%s", plan.PrettyPrint())
@@ -652,7 +653,18 @@ func (t *Test) run(plan *TestPlan) error {
 	).run()
 }
 
-func (t *Test) plan() (*TestPlan, error) {
+func (t *Test) plan() (plan *TestPlan, retErr error) {
+	defer func() {
+		if retErr != nil {
+			// Planning failures are always internal framework errors, so
+			// they should be sent to TestEng.
+			retErr = registry.ErrorWithOwner(
+				registry.OwnerTestEng,
+				fmt.Errorf("error creating test plan: %w", retErr),
+			)
+		}
+	}()
+
 	previousReleases, err := t.choosePreviousReleases()
 	if err != nil {
 		return nil, err

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

@@ -16,10 +16,12 @@ import (
 	"testing"
 
 	"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"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/vm"
 	"github.com/cockroachdb/cockroach/pkg/testutils/release"
 	"github.com/cockroachdb/cockroach/pkg/util/version"
+	"github.com/cockroachdb/errors"
 	"github.com/stretchr/testify/require"
 )
 
@@ -192,6 +194,23 @@ func Test_choosePreviousReleases(t *testing.T) {
 	}
 }
 
+func TestTest_plan(t *testing.T) {
+	// Assert that planning failures are owned by test-eng. At the time
+	// of writing, planning can only return an error if we fail to find
+	// a required predecessor of a certain release.
+	mvt := newTest(NumUpgrades(100))
+	_, err := mvt.plan()
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "no known predecessor for")
+
+	var errWithOwnership registry.ErrorWithOwnership
+	require.True(t,
+		errors.As(err, &errWithOwnership),
+		"error %q (%T) is not ErrorWithOwnership", err.Error(), err,
+	)
+	require.Equal(t, registry.OwnerTestEng, errWithOwnership.Owner)
+}
+
 // withTestBuildVersion overwrites the `TestBuildVersion` variable in
 // the `clusterupgrade` package, allowing tests to set a fixed
 // "current version". Returns a function that resets that variable.

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

@@ -375,7 +375,12 @@ func boolP(b bool) *bool {
 func testPredecessorFunc(
 	rng *rand.Rand, v *clusterupgrade.Version,
 ) (*clusterupgrade.Version, error) {
-	return testPredecessorMapping[release.VersionSeries(&v.Version)], nil
+	pred, ok := testPredecessorMapping[release.VersionSeries(&v.Version)]
+	if !ok {
+		return nil, fmt.Errorf("no known predecessor for %q", v)
+	}
+
+	return pred, nil
 }
 
 // createDataDrivenMixedVersionTest creates a `*Test` instance based