diff --git a/pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion.go b/pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion.go index 660aa2ab308d..aec86c1f9f47 100644 --- a/pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion.go +++ b/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 diff --git a/pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion_test.go b/pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion_test.go index dea82bb6e07f..cbb3459446d9 100644 --- a/pkg/cmd/roachtest/roachtestutil/mixedversion/mixedversion_test.go +++ b/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. diff --git a/pkg/cmd/roachtest/roachtestutil/mixedversion/planner_test.go b/pkg/cmd/roachtest/roachtestutil/mixedversion/planner_test.go index 1b54d1084f1d..914416be1d0b 100644 --- a/pkg/cmd/roachtest/roachtestutil/mixedversion/planner_test.go +++ b/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