test: Use shadow catalog in legacy upgrade tests

[jkosh44]

This commit updates the legacy upgrade tests to use the shadow catalog when possible instead of the stash catalog.

Works towards resolving #24218

Motivation

This PR adds a known-desirable feature.

Checklist

• [X] This PR has adequate test coverage / QA involvement has been duly considered.
• [X] This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
• [X] If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
• [X] If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
• [X] This PR includes the following user-facing behavior changes:
  • There are no user-facing behavior changes.

[jkosh44]

I'm very confused as to why this is passing and not failing. I would imagine that it would fail until we update the legacy upgrade tests to not skip versions during an upgrade. Any thoughts @MaterializeInc/testing?

[def-]

Indeed, strange. This sounds like it should fail:

Testing upgrade from Materialize v0.86.1 to current_source.

[jkosh44]

Indeed, strange. This sounds like it should fail:

Testing upgrade from Materialize v0.86.1 to current_source.

Ah ok, the check that we don't skip versions was only added in v0.87.0, so it will probably take a release or two for this to start failing.

[jkosh44]

@def- Ok, there's finally been enough releases for this to start failing with the expected error. We'll need to update the legacy upgrade tests so that it doesn't skip versions for it to stop failing.

[def-]

This is kind of awkward for the legacy upgrade tests. They are supposed to start on a specific version and check that the migrations inbetween work to the current version. But this would now mean that we have to upgrade to each intermediate version, which would make the test very slow at some point. I'm wondering if there is still much value in even keeping the test around then? Maybe we should check what it has that is missing in platform-checks and port those parts over. @nrainer-materialize What do you think?

[nrainer-materialize]

This is kind of awkward for the legacy upgrade tests. They are supposed to start on a specific version and check that the migrations inbetween work to the current version. But this would now mean that we have to upgrade to each intermediate version, which would make the test very slow at some point. I'm wondering if there is still much value in even keeping the test around then? Maybe we should check what it has that is missing in platform-checks and port those parts over. @nrainer-materialize What do you think?

Well, isn't this the discussion we already had in https://materializeinc.slack.com/archives/C01LKF361MZ/p1708091944440539? I agree that we should avoid testing this behavior twice since we already have UpgradeEntireMzFourVersions.

[jkosh44]

This is kind of awkward for the legacy upgrade tests. They are supposed to start on a specific version and check that the migrations inbetween work to the current version. But this would now mean that we have to upgrade to each intermediate version, which would make the test very slow at some point. I'm wondering if there is still much value in even keeping the test around then? Maybe we should check what it has that is missing in platform-checks and port those parts over. @nrainer-materialize What do you think?

Well, isn't this the discussion we already had in https://materializeinc.slack.com/archives/C01LKF361MZ/p1708091944440539? I agree that we should avoid testing this behavior twice since we already have UpgradeEntireMzFourVersions.

Do you have any thoughts on the timeline for this? We're going to want to remove the stash catalog fairly soon and this will be a blocker for that.

[def-]

Let me try to implement this and push it into this PR when I'm done. Edit: Seems to work: https://buildkite.com/materialize/tests/builds/77841#018e18e3-0b87-4911-a49c-1860815e14d8

[jkosh44]

Let me try to implement this and push it into this PR when I'm done. Edit: Seems to work: https://buildkite.com/materialize/tests/builds/77841#018e18e3-0b87-4911-a49c-1860815e14d8

Thanks so much! Are you on-board with merging this as is?

[def-]

Yes, good to go imo.