Skip to content

migrator: Check previous instance version before making schema changes

Warren Gifford requested to merge ef/migrator-version-checks into main

Created by: efritz

This PR adds a check to the migrator up and sg migration up commands that will ensure that the upgrade is legal. Previously we would only check this in the frontend after a successful schema migration, which can lock the instance in a state that requires a downgrade (or nasty manual intervention). Fixes #37794 (closed).

Relevant changes (mostly broken up by commit):

  • Migrate logic from cmd/frontend/backend/versions to internal/version/upgradestore so that it's a proper store interface
  • Expose the underlying database handle of the migration Store interface, which we need to use to construct the new upgradestore in the migrator
  • Ensure the migration Runner caches stores as they're created (duplicate migration issues will happen if we double-request the same store, which we are about to do)
  • Add a check to the up migration subcommand that hits the new upgradestore

TODO:

  • Ensure that the migrator doesn't crash on empty database, which I just realized is a possible condition only introduced by this change 7bf8fa5a37ccad136f4904ca03d63435165750b0

Test plan

Existing unit tests still remain; validated error condition by running sg migration up locally with a dummy value in the versions table.

Merge request reports

Loading