db: Migrate to alembic

This is significantly easier than Cinder, Nova etc. as Heat hasn't had
any database migrations in multiple cycles, meaning we don't need to
worry about having to apply any older sqlalchemy-migrate migrations
before switching to alembic. Instead, we simply need to determine we're
upgrading a deployment that was previously using sqlalchemy-migrate,
upgrading a deployment that has already migrated to alembic, or
deploying a new deployment, adjusting accordingly.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Change-Id: I808af9cb21ba21808209b1daddac7426f4cad310

7 files changed, 160 insertions, 223 deletions

diff --git a/bin/heat-db-setup b/bin/heat-db-setup
index b0e39ac74..8df02aa08 100755
--- a/bin/heat-db-setup
+++ b/bin/heat-db-setup
@@ -289,7 +289,7 @@ rm $log_conf
 
 # Do a final sanity check on the database.
 
-echo "SELECT * FROM migrate_version;" | mysql -u heat --password=${MYSQL_HEAT_PW} heat > /dev/null
+echo "SELECT * FROM alembic_version;" | mysql -u heat --password=${MYSQL_HEAT_PW} heat > /dev/null
 if ! [ $? -eq 0 ]
 then
 	echo "Final sanity check failed." >&2
diff --git a/heat/cmd/manage.py b/heat/cmd/manage.py
index c853d9ac2..9f7b8e316 100644
--- a/heat/cmd/manage.py
+++ b/heat/cmd/manage.py
@@ -26,17 +26,17 @@ from heat.common.i18n import _
 from heat.common import messaging
 from heat.common import service_utils
 from heat.db.sqlalchemy import api as db_api
+from heat.db.sqlalchemy import migration as db_migration
 from heat.objects import service as service_objects
 from heat.rpc import client as rpc_client
 from heat import version
 
-
 CONF = cfg.CONF
 
 
 def do_db_version():
     """Print database's current migration level."""
-    print(db_api.db_version(db_api.get_engine()))
+    print(db_migration.db_version())
 
 
 def do_db_sync():
@@ -44,7 +44,7 @@ def do_db_sync():
 
     Creating first if necessary.
     """
-    db_api.db_sync(db_api.get_engine(), CONF.command.version)
+    db_migration.db_sync(CONF.command.version)
 
 
 class ServiceManageCommand(object):
diff --git a/heat/db/sqlalchemy/api.py b/heat/db/sqlalchemy/api.py
index df627d43c..1556d2d76 100644
--- a/heat/db/sqlalchemy/api.py
+++ b/heat/db/sqlalchemy/api.py
@@ -39,7 +39,6 @@ from heat.common import crypt
 from heat.common import exception
 from heat.common.i18n import _
 from heat.db.sqlalchemy import filters as db_filters
-from heat.db.sqlalchemy import migration
 from heat.db.sqlalchemy import models
 from heat.db.sqlalchemy import utils as db_utils
 from heat.engine import environment as heat_environment
@@ -1622,19 +1621,6 @@ def sync_point_update_input_data(context, entity_id,
     return rows_updated
 
 
-def db_sync(engine, version=None):
-    """Migrate the database to `version` or the most recent version."""
-    if version is not None and int(version) < db_version(engine):
-        raise exception.Error(_("Cannot migrate to lower schema version."))
-
-    return migration.db_sync(engine, version=version)
-
-
-def db_version(engine):
-    """Display the current database version."""
-    return migration.db_version(engine)
-
-
 def _crypt_action(encrypt):
     if encrypt:
         return _('encrypt')
diff --git a/heat/db/sqlalchemy/migration.py b/heat/db/sqlalchemy/migration.py
index 7f030d90a..67ac4c3b4 100644
--- a/heat/db/sqlalchemy/migration.py
+++ b/heat/db/sqlalchemy/migration.py
@@ -1,4 +1,3 @@
-#
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
 #    not use this file except in compliance with the License. You may obtain
 #    a copy of the License at
@@ -13,26 +12,106 @@
 
 import os
 
-from oslo_db.sqlalchemy import migration as oslo_migration
+from alembic import command as alembic_api
+from alembic import config as alembic_config
+from alembic import migration as alembic_migration
+from oslo_log import log as logging
+import sqlalchemy as sa
+
+from heat.db.sqlalchemy import api as db_api
+
+LOG = logging.getLogger(__name__)
+
+ALEMBIC_INIT_VERSION = 'c6214ca60943'
+
+
+def _migrate_legacy_database(engine, connection, config):
+    """Check if database is a legacy sqlalchemy-migrate-managed database.
+
+    If it is, migrate it by "stamping" the initial alembic schema.
+    """
+    # If the database doesn't have the sqlalchemy-migrate legacy migration
+    # table, we don't have anything to do
+    if not sa.inspect(engine).has_table('migrate_version'):
+        return
+
+    # Likewise, if we've already migrated to alembic, we don't have anything to
+    # do
+    context = alembic_migration.MigrationContext.configure(connection)
+    if context.get_current_revision():
+        return
+
+    # We have legacy migrations but no alembic migration. Stamp (dummy apply)
+    # the initial alembic migration.
+
+    LOG.info(
+        'The database is still under sqlalchemy-migrate control; '
+        'fake applying the initial alembic migration'
+    )
+    alembic_api.stamp(config, ALEMBIC_INIT_VERSION)
+
+
+def _find_alembic_conf():
+    """Get the project's alembic configuration
+
+    :returns: An instance of ``alembic.config.Config``
+    """
+    path = os.path.join(
+        os.path.abspath(os.path.dirname(__file__)),
+        'alembic.ini',
+    )
+    config = alembic_config.Config(os.path.abspath(path))
+    # we don't want to use the logger configuration from the file, which is
+    # only really intended for the CLI
+    # https://stackoverflow.com/a/42691781/613428
+    config.attributes['configure_logger'] = False
+    return config
+
+
+def _upgrade_alembic(engine, config, version):
+    # re-use the connection rather than creating a new one
+    with engine.begin() as connection:
+        config.attributes['connection'] = connection
+        _migrate_legacy_database(engine, connection, config)
+        alembic_api.upgrade(config, version or 'head')
 
 
-INIT_VERSION = 72
+def db_sync(version=None, engine=None):
+    """Migrate the database to `version` or the most recent version."""
+    # if the user requested a specific version, check if it's an integer: if
+    # so, we're almost certainly in sqlalchemy-migrate land and won't support
+    # that
+    if version is not None and version.isdigit():
+        raise ValueError(
+            'You requested an sqlalchemy-migrate database version; this is '
+            'no longer supported'
+        )
 
+    if engine is None:
+        engine = db_api.get_engine()
 
-def db_sync(engine, version=None):
-    path = os.path.join(os.path.abspath(os.path.dirname(__file__)),
-                        'migrate_repo')
-    return oslo_migration.db_sync(engine, path, version,
-                                  init_version=INIT_VERSION)
+    config = _find_alembic_conf()
 
+    # discard the URL encoded in alembic.ini in favour of the URL configured
+    # for the engine by the database fixtures, casting from
+    # 'sqlalchemy.engine.url.URL' to str in the process. This returns a
+    # RFC-1738 quoted URL, which means that a password like "foo@" will be
+    # turned into "foo%40". This in turns causes a problem for
+    # set_main_option() because that uses ConfigParser.set, which (by design)
+    # uses *python* interpolation to write the string out ... where "%" is the
+    # special python interpolation character! Avoid this mismatch by quoting
+    # all %'s for the set below.
+    engine_url = str(engine.url).replace('%', '%%')
+    config.set_main_option('sqlalchemy.url', str(engine_url))
 
-def db_version(engine):
-    path = os.path.join(os.path.abspath(os.path.dirname(__file__)),
-                        'migrate_repo')
-    return oslo_migration.db_version(engine, path, INIT_VERSION)
+    LOG.info('Applying migration(s)')
+    _upgrade_alembic(engine, config, version)
+    LOG.info('Migration(s) applied')
 
 
-def db_version_control(engine, version=None):
-    path = os.path.join(os.path.abspath(os.path.dirname(__file__)),
-                        'migrate_repo')
-    return oslo_migration.db_version_control(engine, path, version)
+def db_version():
+    """Get database version."""
+    engine = db_api.get_engine()
+    with engine.connect() as connection:
+        m_context = alembic_migration.MigrationContext.configure(connection)
+        return m_context.get_current_revision()
diff --git a/heat/db/sqlalchemy/migrations/env.py b/heat/db/sqlalchemy/migrations/env.py
index 138d28374..84413f313 100644
--- a/heat/db/sqlalchemy/migrations/env.py
+++ b/heat/db/sqlalchemy/migrations/env.py
@@ -24,7 +24,7 @@ config = context.config
 
 # Interpret the config file for Python logging.
 # This line sets up loggers basically.
-if config.config_file_name is not None:
+if config.attributes.get('configure_logger', True):
     fileConfig(config.config_file_name)
 
 # this is the MetaData object for the various models in the database
@@ -58,16 +58,30 @@ def run_migrations_online() -> None:
 
     In this scenario we need to create an Engine and associate a connection
     with the context.
+
+    This is modified from the default based on the below, since we want to
+    share an engine when unit testing so in-memory database testing actually
+    works.
+
+    https://alembic.sqlalchemy.org/en/latest/cookbook.html#connection-sharing
     """
-    connectable = engine_from_config(
-        config.get_section(config.config_ini_section, {}),
-        prefix="sqlalchemy.",
-        poolclass=pool.NullPool,
-    )
+    connectable = config.attributes.get('connection', None)
+
+    if connectable is None:
+        # only create Engine if we don't have a Connection from the outside
+        connectable = engine_from_config(
+            config.get_section(config.config_ini_section, {}),
+            prefix="sqlalchemy.",
+            poolclass=pool.NullPool,
+        )
+
+    # when connectable is already a Connection object, calling connect() gives
+    # us a *branched connection*
 
     with connectable.connect() as connection:
         context.configure(
-            connection=connection, target_metadata=target_metadata
+            connection=connection,
+            target_metadata=target_metadata,
         )
 
         with context.begin_transaction():
diff --git a/heat/tests/db/test_migrations.py b/heat/tests/db/test_migrations.py
index 0dbf85bf3..31485c1b0 100644
--- a/heat/tests/db/test_migrations.py
+++ b/heat/tests/db/test_migrations.py
@@ -1,4 +1,3 @@
-#
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
 #    not use this file except in compliance with the License. You may obtain
 #    a copy of the License at
@@ -11,31 +10,18 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-"""
-Tests for database migrations. This test case reads the configuration
-file test_migrations.conf for database connection settings
-to use in the tests. For each connection found in the config file,
-the test case runs a series of test cases to ensure that migrations work
-properly both upgrading and downgrading, and that no data loss occurs
-if possible.
-"""
+"""Tests for database migrations."""
 
 import fixtures
-import os
-
-from migrate.versioning import repository
 from oslo_db.sqlalchemy import enginefacade
 from oslo_db.sqlalchemy import test_fixtures
 from oslo_db.sqlalchemy import test_migrations
-from oslo_db.sqlalchemy import utils
 from oslotest import base as test_base
 import sqlalchemy
 import testtools
 
-from heat.db.sqlalchemy import migrate_repo
 from heat.db.sqlalchemy import migration
 from heat.db.sqlalchemy import models
-from heat.tests import common
 
 
 class DBNotAllowed(Exception):
@@ -80,168 +66,14 @@ class TestBannedDBSchemaOperations(testtools.TestCase):
             self.assertRaises(DBNotAllowed, table.alter)
 
 
-class HeatMigrationsCheckers(test_migrations.WalkVersionsMixin,
-                             common.FakeLogMixin):
-    """Test sqlalchemy-migrate migrations."""
-
-    snake_walk = False
-    downgrade = False
-
-    @property
-    def INIT_VERSION(self):
-        return migration.INIT_VERSION
-
-    @property
-    def REPOSITORY(self):
-        migrate_file = migrate_repo.__file__
-        return repository.Repository(
-            os.path.abspath(os.path.dirname(migrate_file))
-        )
-
-    @property
-    def migration_api(self):
-        temp = __import__('oslo_db.sqlalchemy.migration', globals(),
-                          locals(), ['versioning_api'], 0)
-        return temp.versioning_api
-
-    @property
-    def migrate_engine(self):
-        return self.engine
-
-    def migrate_up(self, version, with_data=False):
-        """Check that migrations don't cause downtime.
-
-        Schema migrations can be done online, allowing for rolling upgrades.
-        """
-        # NOTE(xek): This is a list of migrations where we allow dropping
-        # things. The rules for adding exceptions are very very specific.
-        # Chances are you don't meet the critera.
-        # Reviewers: DO NOT ALLOW THINGS TO BE ADDED HERE
-        exceptions = [
-            64,  # drop constraint
-            86,  # drop watch_rule/watch_data tables
-        ]
-        # Reviewers: DO NOT ALLOW THINGS TO BE ADDED HERE
-
-        # NOTE(xek): We start requiring things be additive in
-        # liberty, so ignore all migrations before that point.
-        LIBERTY_START = 63
-
-        if version >= LIBERTY_START and version not in exceptions:
-            banned = ['Table', 'Column']
-        else:
-            banned = None
-        with BannedDBSchemaOperations(banned):
-            super(HeatMigrationsCheckers, self).migrate_up(version, with_data)
-
-    def test_walk_versions(self):
-        self.walk_versions(self.snake_walk, self.downgrade)
-
-    def assertColumnExists(self, engine, table, column):
-        t = utils.get_table(engine, table)
-        self.assertIn(column, t.c)
-
-    def assertColumnType(self, engine, table, column, sqltype):
-        t = utils.get_table(engine, table)
-        col = getattr(t.c, column)
-        self.assertIsInstance(col.type, sqltype)
-
-    def assertColumnNotExists(self, engine, table, column):
-        t = utils.get_table(engine, table)
-        self.assertNotIn(column, t.c)
-
-    def assertColumnIsNullable(self, engine, table, column):
-        t = utils.get_table(engine, table)
-        col = getattr(t.c, column)
-        self.assertTrue(col.nullable)
-
-    def assertColumnIsNotNullable(self, engine, table, column_name):
-        table = utils.get_table(engine, table)
-        column = getattr(table.c, column_name)
-        self.assertFalse(column.nullable)
-
-    def assertIndexExists(self, engine, table, index):
-        t = utils.get_table(engine, table)
-        index_names = [idx.name for idx in t.indexes]
-        self.assertIn(index, index_names)
+class HeatModelsMigrationsSync(test_migrations.ModelsMigrationsSync):
 
-    def assertIndexMembers(self, engine, table, index, members):
-        self.assertIndexExists(engine, table, index)
-
-        t = utils.get_table(engine, table)
-        index_columns = []
-        for idx in t.indexes:
-            if idx.name == index:
-                for ix in idx.columns:
-                    index_columns.append(ix.name)
-                break
-
-        self.assertEqual(sorted(members), sorted(index_columns))
-
-    def _check_073(self, engine, data):
-        # check if column still exists and is not nullable.
-        self.assertColumnIsNotNullable(engine, 'resource_data', 'resource_id')
-        # Ensure that only one foreign key exists and is created as expected.
-        inspector = sqlalchemy.engine.reflection.Inspector.from_engine(engine)
-        resource_data_fkeys = inspector.get_foreign_keys('resource_data')
-        self.assertEqual(1, len(resource_data_fkeys))
-        fk = resource_data_fkeys[0]
-        self.assertEqual('fk_resource_id', fk['name'])
-        self.assertEqual(['resource_id'], fk['constrained_columns'])
-        self.assertEqual('resource', fk['referred_table'])
-        self.assertEqual(['id'], fk['referred_columns'])
-
-    def _check_079(self, engine, data):
-        self.assertColumnExists(engine, 'resource',
-                                'rsrc_prop_data_id')
-        self.assertColumnExists(engine, 'event',
-                                'rsrc_prop_data_id')
-        column_list = [('id', False),
-                       ('data', True),
-                       ('encrypted', True),
-                       ('updated_at', True),
-                       ('created_at', True)]
-
-        for column in column_list:
-            self.assertColumnExists(engine,
-                                    'resource_properties_data', column[0])
-            if not column[1]:
-                self.assertColumnIsNotNullable(engine,
-                                               'resource_properties_data',
-                                               column[0])
-            else:
-                self.assertColumnIsNullable(engine,
-                                            'resource_properties_data',
-                                            column[0])
-
-    def _check_080(self, engine, data):
-        self.assertColumnExists(engine, 'resource',
-                                'attr_data_id')
-
-
-class DbTestCase(test_fixtures.OpportunisticDBTestMixin,
-                 test_base.BaseTestCase):
     def setUp(self):
-        super(DbTestCase, self).setUp()
+        super().setUp()
 
         self.engine = enginefacade.writer.get_engine()
         self.sessionmaker = enginefacade.writer.get_sessionmaker()
 
-
-class TestHeatMigrationsMySQL(DbTestCase, HeatMigrationsCheckers):
-    FIXTURE = test_fixtures.MySQLOpportunisticFixture
-
-
-class TestHeatMigrationsPostgreSQL(DbTestCase, HeatMigrationsCheckers):
-    FIXTURE = test_fixtures.PostgresqlOpportunisticFixture
-
-
-class TestHeatMigrationsSQLite(DbTestCase, HeatMigrationsCheckers):
-    pass
-
-
-class ModelsMigrationSyncMixin(object):
-
     def get_metadata(self):
         return models.BASE.metadata
 
@@ -252,24 +84,28 @@ class ModelsMigrationSyncMixin(object):
         migration.db_sync(engine=engine)
 
     def include_object(self, object_, name, type_, reflected, compare_to):
-        if name in ['migrate_version'] and type_ == 'table':
-            return False
         return True
 
 
-class ModelsMigrationsSyncMysql(DbTestCase,
-                                ModelsMigrationSyncMixin,
-                                test_migrations.ModelsMigrationsSync):
+class ModelsMigrationsSyncMysql(
+    HeatModelsMigrationsSync,
+    test_fixtures.OpportunisticDBTestMixin,
+    test_base.BaseTestCase,
+):
     FIXTURE = test_fixtures.MySQLOpportunisticFixture
 
 
-class ModelsMigrationsSyncPostgres(DbTestCase,
-                                   ModelsMigrationSyncMixin,
-                                   test_migrations.ModelsMigrationsSync):
+class ModelsMigrationsSyncPostgres(
+    HeatModelsMigrationsSync,
+    test_fixtures.OpportunisticDBTestMixin,
+    test_base.BaseTestCase,
+):
     FIXTURE = test_fixtures.PostgresqlOpportunisticFixture
 
 
-class ModelsMigrationsSyncSQLite(DbTestCase,
-                                 ModelsMigrationSyncMixin,
-                                 test_migrations.ModelsMigrationsSync):
+class ModelsMigrationsSyncSQLite(
+    HeatModelsMigrationsSync,
+    test_fixtures.OpportunisticDBTestMixin,
+    test_base.BaseTestCase,
+):
     pass
diff --git a/releasenotes/notes/switch-to-alembic-7af6f8e71e4bf56b.yaml b/releasenotes/notes/switch-to-alembic-7af6f8e71e4bf56b.yaml
new file mode 100644
index 000000000..883f74db3
--- /dev/null
+++ b/releasenotes/notes/switch-to-alembic-7af6f8e71e4bf56b.yaml
@@ -0,0 +1,22 @@
+---
+upgrade:
+  - |
+    The database migration engine has changed from `sqlalchemy-migrate`__ to
+    `alembic`__. For most deployments, this should have minimal to no impact
+    and the switch should be mostly transparent. The main user-facing impact is
+    the change in schema versioning. While sqlalchemy-migrate used a linear,
+    integer-based versioning scheme, which required placeholder migrations to
+    allow for potential migration backports, alembic uses a distributed version
+    control-like schema where a migration's ancestor is encoded in the file and
+    branches are possible. The alembic migration files therefore use a
+    arbitrary UUID-like naming scheme and the ``heat-manage db_sync`` command
+    now expects such an version when manually specifying the version that should
+    be applied. For example::
+
+        $ heat-manage db_sync c6214ca60943
+
+    Attempting to specify an sqlalchemy-migrate-based version will result in an
+    error.
+
+    .. __: https://sqlalchemy-migrate.readthedocs.io/en/latest/
+    .. __: https://alembic.sqlalchemy.org/en/latest/