Merge "db: Move db.sqalchemy.migration to db.migration"

nova/db/migration.py

@@ -14,23 +14,127 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-"""Database setup and migration commands."""
+import os
 
-from nova.db.sqlalchemy import migration
+from migrate import exceptions as versioning_exceptions
+from migrate.versioning import api as versioning_api
+from migrate.versioning.repository import Repository
+from oslo_log import log as logging
+import sqlalchemy
 
-IMPL = migration
+from nova.db.sqlalchemy import api as db_session
+from nova import exception
+from nova.i18n import _
+
+INIT_VERSION = {}
+INIT_VERSION['main'] = 401
+INIT_VERSION['api'] = 66
+_REPOSITORY = {}
+
+LOG = logging.getLogger(__name__)
+
+
+def get_engine(database='main', context=None):
+    if database == 'main':
+        return db_session.get_engine(context=context)
+
+    if database == 'api':
+        return db_session.get_api_engine()
+
+
+def find_migrate_repo(database='main'):
+    """Get the path for the migrate repository."""
+    global _REPOSITORY
+    rel_path = os.path.join('sqlalchemy', 'migrate_repo')
+    if database == 'api':
+        rel_path = os.path.join('sqlalchemy', 'api_migrations', 'migrate_repo')
+    path = os.path.join(os.path.abspath(os.path.dirname(__file__)), rel_path)
+    assert os.path.exists(path)
+    if _REPOSITORY.get(database) is None:
+        _REPOSITORY[database] = Repository(path)
+    return _REPOSITORY[database]
 
 
 def db_sync(version=None, database='main', context=None):
     """Migrate the database to `version` or the most recent version."""
-    return IMPL.db_sync(version=version, database=database, context=context)
+    if version is not None:
+        try:
+            version = int(version)
+        except ValueError:
+            raise exception.NovaException(_("version should be an integer"))
+
+    current_version = db_version(database, context=context)
+    repository = find_migrate_repo(database)
+    engine = get_engine(database, context=context)
+    if version is None or version > current_version:
+        return versioning_api.upgrade(engine, repository, version)
+    else:
+        return versioning_api.downgrade(engine, repository, version)
 
 
 def db_version(database='main', context=None):
     """Display the current database version."""
-    return IMPL.db_version(database=database, context=context)
+    repository = find_migrate_repo(database)
+
+    # NOTE(mdbooth): This is a crude workaround for races in _db_version. The 2
+    # races we have seen in practise are:
+    # * versioning_api.db_version() fails because the migrate_version table
+    #   doesn't exist, but meta.tables subsequently contains tables because
+    #   another thread has already started creating the schema. This results in
+    #   the 'Essex' error.
+    # * db_version_control() fails with pymysql.error.InternalError(1050)
+    #   (Create table failed) because of a race in sqlalchemy-migrate's
+    #   ControlledSchema._create_table_version, which does:
+    #     if not table.exists(): table.create()
+    #   This means that it doesn't raise the advertised
+    #   DatabaseAlreadyControlledError, which we could have handled explicitly.
+    #
+    # I believe the correct fix should be:
+    # * Delete the Essex-handling code as unnecessary complexity which nobody
+    #   should still need.
+    # * Fix the races in sqlalchemy-migrate such that version_control() always
+    #   raises a well-defined error, and then handle that error here.
+    #
+    # Until we do that, though, we should be able to just try again if we
+    # failed for any reason. In both of the above races, trying again should
+    # succeed the second time round.
+    #
+    # For additional context, see:
+    # * https://bugzilla.redhat.com/show_bug.cgi?id=1652287
+    # * https://bugs.launchpad.net/nova/+bug/1804652
+    try:
+        return _db_version(repository, database, context)
+    except Exception:
+        return _db_version(repository, database, context)
+
+
+def _db_version(repository, database, context):
+    engine = get_engine(database, context=context)
+    try:
+        return versioning_api.db_version(engine, repository)
+    except versioning_exceptions.DatabaseNotControlledError as exc:
+        meta = sqlalchemy.MetaData()
+        meta.reflect(bind=engine)
+        tables = meta.tables
+        if len(tables) == 0:
+            db_version_control(
+                INIT_VERSION[database], database, context=context)
+            return versioning_api.db_version(engine, repository)
+        else:
+            LOG.exception(exc)
+            # Some pre-Essex DB's may not be version controlled.
+            # Require them to upgrade using Essex first.
+            raise exception.NovaException(
+                _("Upgrade DB using Essex release first."))
 
 
 def db_initial_version(database='main'):
     """The starting version for the database."""
-    return IMPL.db_initial_version(database=database)
+    return INIT_VERSION[database]
+
+
+def db_version_control(version=None, database='main', context=None):
+    repository = find_migrate_repo(database)
+    engine = get_engine(database, context=context)
+    versioning_api.version_control(engine, repository, version)
+    return version

nova/db/sqlalchemy/migration.py

@@ -1,140 +0,0 @@
-# Copyright 2010 United States Government as represented by the
-# Administrator of the National Aeronautics and Space Administration.
-# All Rights Reserved.
-#
-#    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
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-import os
-
-from migrate import exceptions as versioning_exceptions
-from migrate.versioning import api as versioning_api
-from migrate.versioning.repository import Repository
-from oslo_log import log as logging
-import sqlalchemy
-
-from nova.db.sqlalchemy import api as db_session
-from nova import exception
-from nova.i18n import _
-
-INIT_VERSION = {}
-INIT_VERSION['main'] = 401
-INIT_VERSION['api'] = 66
-_REPOSITORY = {}
-
-LOG = logging.getLogger(__name__)
-
-
-def get_engine(database='main', context=None):
-    if database == 'main':
-        return db_session.get_engine(context=context)
-
-    if database == 'api':
-        return db_session.get_api_engine()
-
-
-def find_migrate_repo(database='main'):
-    """Get the path for the migrate repository."""
-    global _REPOSITORY
-    rel_path = 'migrate_repo'
-    if database == 'api':
-        rel_path = os.path.join('api_migrations', 'migrate_repo')
-    path = os.path.join(os.path.abspath(os.path.dirname(__file__)), rel_path)
-    assert os.path.exists(path)
-    if _REPOSITORY.get(database) is None:
-        _REPOSITORY[database] = Repository(path)
-    return _REPOSITORY[database]
-
-
-def db_sync(version=None, database='main', context=None):
-    """Migrate the database to `version` or the most recent version."""
-    if version is not None:
-        try:
-            version = int(version)
-        except ValueError:
-            raise exception.NovaException(_("version should be an integer"))
-
-    current_version = db_version(database, context=context)
-    repository = find_migrate_repo(database)
-    engine = get_engine(database, context=context)
-    if version is None or version > current_version:
-        return versioning_api.upgrade(engine, repository, version)
-    else:
-        return versioning_api.downgrade(engine, repository, version)
-
-
-def db_version(database='main', context=None):
-    """Display the current database version."""
-    repository = find_migrate_repo(database)
-
-    # NOTE(mdbooth): This is a crude workaround for races in _db_version. The 2
-    # races we have seen in practise are:
-    # * versioning_api.db_version() fails because the migrate_version table
-    #   doesn't exist, but meta.tables subsequently contains tables because
-    #   another thread has already started creating the schema. This results in
-    #   the 'Essex' error.
-    # * db_version_control() fails with pymysql.error.InternalError(1050)
-    #   (Create table failed) because of a race in sqlalchemy-migrate's
-    #   ControlledSchema._create_table_version, which does:
-    #     if not table.exists(): table.create()
-    #   This means that it doesn't raise the advertised
-    #   DatabaseAlreadyControlledError, which we could have handled explicitly.
-    #
-    # I believe the correct fix should be:
-    # * Delete the Essex-handling code as unnecessary complexity which nobody
-    #   should still need.
-    # * Fix the races in sqlalchemy-migrate such that version_control() always
-    #   raises a well-defined error, and then handle that error here.
-    #
-    # Until we do that, though, we should be able to just try again if we
-    # failed for any reason. In both of the above races, trying again should
-    # succeed the second time round.
-    #
-    # For additional context, see:
-    # * https://bugzilla.redhat.com/show_bug.cgi?id=1652287
-    # * https://bugs.launchpad.net/nova/+bug/1804652
-    try:
-        return _db_version(repository, database, context)
-    except Exception:
-        return _db_version(repository, database, context)
-
-
-def _db_version(repository, database, context):
-    engine = get_engine(database, context=context)
-    try:
-        return versioning_api.db_version(engine, repository)
-    except versioning_exceptions.DatabaseNotControlledError as exc:
-        meta = sqlalchemy.MetaData()
-        meta.reflect(bind=engine)
-        tables = meta.tables
-        if len(tables) == 0:
-            db_version_control(
-                INIT_VERSION[database], database, context=context)
-            return versioning_api.db_version(engine, repository)
-        else:
-            LOG.exception(exc)
-            # Some pre-Essex DB's may not be version controlled.
-            # Require them to upgrade using Essex first.
-            raise exception.NovaException(
-                _("Upgrade DB using Essex release first."))
-
-
-def db_initial_version(database='main'):
-    """The starting version for the database."""
-    return INIT_VERSION[database]
-
-
-def db_version_control(version=None, database='main', context=None):
-    repository = find_migrate_repo(database)
-    engine = get_engine(database, context=context)
-    versioning_api.version_control(engine, repository, version)
-    return version

nova/tests/functional/db/api/test_migrations.py

@@ -40,7 +40,6 @@ import testtools
 from nova.db import migration
 from nova.db.sqlalchemy.api_migrations import migrate_repo
 from nova.db.sqlalchemy import api_models
-from nova.db.sqlalchemy import migration as sa_migration
 from nova import test
 from nova.tests import fixtures as nova_fixtures
 
@@ -53,9 +52,8 @@ class NovaAPIModelsSync(test_migrations.ModelsMigrationsSync):
         self.engine = enginefacade.writer.get_engine()
 
     def db_sync(self, engine):
-        with mock.patch.object(sa_migration, 'get_engine',
+        with mock.patch.object(migration, 'get_engine', return_value=engine):
-                               return_value=engine):
+            migration.db_sync(database='api')
-            sa_migration.db_sync(database='api')
 
     @property
     def migrate_engine(self):
@@ -160,7 +158,7 @@ class NovaAPIMigrationsWalk(test_migrations.WalkVersionsMixin):
 
     @property
     def migration_api(self):
-        return sa_migration.versioning_api
+        return migration.versioning_api
 
     @property
     def migrate_engine(self):

nova/tests/unit/cmd/test_manage.py

@@ -30,7 +30,7 @@ from nova.cmd import manage
 from nova import conf
 from nova import context
 from nova.db import api as db
-from nova.db.sqlalchemy import migration as sqla_migration
+from nova.db import migration
 from nova import exception
 from nova import objects
 from nova.scheduler.client import report
@@ -618,18 +618,17 @@ Cell %s: 456
         self.assertEqual(4, ret)
         self.assertIn('Unable to get cell list', self.output.getvalue())
 
-    @mock.patch.object(sqla_migration, 'db_version', return_value=2)
+    @mock.patch.object(migration, 'db_version', return_value=2)
-    def test_version(self, sqla_migrate):
+    def test_version(self, mock_db_version):
         self.commands.version()
-        sqla_migrate.assert_called_once_with(context=None, database='main')
+        mock_db_version.assert_called_once_with()
 
-    @mock.patch.object(sqla_migration, 'db_sync')
+    @mock.patch.object(migration, 'db_sync')
-    def test_sync(self, sqla_sync):
+    def test_sync(self, mock_db_sync):
         self.commands.sync(version=4, local_cell=True)
-        sqla_sync.assert_called_once_with(context=None,
+        mock_db_sync.assert_called_once_with(4)
-                                          version=4, database='main')
 
-    @mock.patch('nova.db.migration.db_sync')
+    @mock.patch.object(migration, 'db_sync')
     @mock.patch.object(objects.CellMapping, 'get_by_uuid', return_value='map')
     def test_sync_cell0(self, mock_get_by_uuid, mock_db_sync):
         ctxt = context.get_admin_context()
@@ -810,17 +809,15 @@ class ApiDbCommandsTestCase(test.NoDBTestCase):
         self.useFixture(fixtures.MonkeyPatch('sys.stdout', self.output))
         self.commands = manage.ApiDbCommands()
 
-    @mock.patch.object(sqla_migration, 'db_version', return_value=2)
+    @mock.patch.object(migration, 'db_version', return_value=2)
-    def test_version(self, sqla_migrate):
+    def test_version(self, mock_db_version):
         self.commands.version()
-        sqla_migrate.assert_called_once_with(context=None,
+        mock_db_version.assert_called_once_with(database='api')
-                                             database='api')
 
-    @mock.patch.object(sqla_migration, 'db_sync')
+    @mock.patch.object(migration, 'db_sync')
-    def test_sync(self, sqla_sync):
+    def test_sync(self, mock_db_sync):
         self.commands.sync(version=4)
-        sqla_sync.assert_called_once_with(context=None,
+        mock_db_sync.assert_called_once_with(4, database='api')
-                                          version=4, database='api')
 
 
 @ddt.ddt

nova/tests/unit/db/test_migration.py

@@ -17,8 +17,8 @@ from migrate.versioning import api as versioning_api
 import mock
 import sqlalchemy
 
+from nova.db import migration
 from nova.db.sqlalchemy import api as db_api
-from nova.db.sqlalchemy import migration
 from nova import test
 
 

nova/tests/unit/db/test_migrations.py

@@ -48,7 +48,6 @@ import testtools
 
 from nova.db import migration
 from nova.db.sqlalchemy import migrate_repo
-from nova.db.sqlalchemy import migration as sa_migration
 from nova.db.sqlalchemy import models
 from nova import test
 from nova.tests import fixtures as nova_fixtures
@@ -78,7 +77,7 @@ class NovaMigrationsCheckers(test_migrations.ModelsMigrationsSync,
 
     @property
     def migration_api(self):
-        return sa_migration.versioning_api
+        return migration.versioning_api
 
     @property
     def migrate_engine(self):
@@ -136,9 +135,8 @@ class NovaMigrationsCheckers(test_migrations.ModelsMigrationsSync,
 
     # Implementations for ModelsMigrationsSync
     def db_sync(self, engine):
-        with mock.patch.object(sa_migration, 'get_engine',
+        with mock.patch.object(migration, 'get_engine', return_value=engine):
-                               return_value=engine):
+            migration.db_sync()
-            sa_migration.db_sync()
 
     def get_engine(self, context=None):
         return self.migrate_engine
@@ -239,9 +237,10 @@ class TestNovaMigrationsMySQL(NovaMigrationsCheckers,
     FIXTURE = test_fixtures.MySQLOpportunisticFixture
 
     def test_innodb_tables(self):
-        with mock.patch.object(sa_migration, 'get_engine',
+        with mock.patch.object(
-                               return_value=self.migrate_engine):
+            migration, 'get_engine', return_value=self.migrate_engine,
-            sa_migration.db_sync()
+        ):
+            migration.db_sync()
 
         total = self.migrate_engine.execute(
             "SELECT count(*) "