Skip to content

Commit 5a56ac4

Browse files
Zuulopenstack-gerrit
authored andcommitted
Merge "db: Move db.sqalchemy.migration to db.migration"
2 parents 35ddf1a + 4bbea58 commit 5a56ac4

File tree

6 files changed

+135
-177
lines changed

6 files changed

+135
-177
lines changed

nova/db/migration.py

Lines changed: 110 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,127 @@
1414
# License for the specific language governing permissions and limitations
1515
# under the License.
1616

17-
"""Database setup and migration commands."""
17+
import os
1818

19-
from nova.db.sqlalchemy import migration
19+
from migrate import exceptions as versioning_exceptions
20+
from migrate.versioning import api as versioning_api
21+
from migrate.versioning.repository import Repository
22+
from oslo_log import log as logging
23+
import sqlalchemy
2024

21-
IMPL = migration
25+
from nova.db.sqlalchemy import api as db_session
26+
from nova import exception
27+
from nova.i18n import _
28+
29+
INIT_VERSION = {}
30+
INIT_VERSION['main'] = 401
31+
INIT_VERSION['api'] = 66
32+
_REPOSITORY = {}
33+
34+
LOG = logging.getLogger(__name__)
35+
36+
37+
def get_engine(database='main', context=None):
38+
if database == 'main':
39+
return db_session.get_engine(context=context)
40+
41+
if database == 'api':
42+
return db_session.get_api_engine()
43+
44+
45+
def find_migrate_repo(database='main'):
46+
"""Get the path for the migrate repository."""
47+
global _REPOSITORY
48+
rel_path = os.path.join('sqlalchemy', 'migrate_repo')
49+
if database == 'api':
50+
rel_path = os.path.join('sqlalchemy', 'api_migrations', 'migrate_repo')
51+
path = os.path.join(os.path.abspath(os.path.dirname(__file__)), rel_path)
52+
assert os.path.exists(path)
53+
if _REPOSITORY.get(database) is None:
54+
_REPOSITORY[database] = Repository(path)
55+
return _REPOSITORY[database]
2256

2357

2458
def db_sync(version=None, database='main', context=None):
2559
"""Migrate the database to `version` or the most recent version."""
26-
return IMPL.db_sync(version=version, database=database, context=context)
60+
if version is not None:
61+
try:
62+
version = int(version)
63+
except ValueError:
64+
raise exception.NovaException(_("version should be an integer"))
65+
66+
current_version = db_version(database, context=context)
67+
repository = find_migrate_repo(database)
68+
engine = get_engine(database, context=context)
69+
if version is None or version > current_version:
70+
return versioning_api.upgrade(engine, repository, version)
71+
else:
72+
return versioning_api.downgrade(engine, repository, version)
2773

2874

2975
def db_version(database='main', context=None):
3076
"""Display the current database version."""
31-
return IMPL.db_version(database=database, context=context)
77+
repository = find_migrate_repo(database)
78+
79+
# NOTE(mdbooth): This is a crude workaround for races in _db_version. The 2
80+
# races we have seen in practise are:
81+
# * versioning_api.db_version() fails because the migrate_version table
82+
# doesn't exist, but meta.tables subsequently contains tables because
83+
# another thread has already started creating the schema. This results in
84+
# the 'Essex' error.
85+
# * db_version_control() fails with pymysql.error.InternalError(1050)
86+
# (Create table failed) because of a race in sqlalchemy-migrate's
87+
# ControlledSchema._create_table_version, which does:
88+
# if not table.exists(): table.create()
89+
# This means that it doesn't raise the advertised
90+
# DatabaseAlreadyControlledError, which we could have handled explicitly.
91+
#
92+
# I believe the correct fix should be:
93+
# * Delete the Essex-handling code as unnecessary complexity which nobody
94+
# should still need.
95+
# * Fix the races in sqlalchemy-migrate such that version_control() always
96+
# raises a well-defined error, and then handle that error here.
97+
#
98+
# Until we do that, though, we should be able to just try again if we
99+
# failed for any reason. In both of the above races, trying again should
100+
# succeed the second time round.
101+
#
102+
# For additional context, see:
103+
# * https://bugzilla.redhat.com/show_bug.cgi?id=1652287
104+
# * https://bugs.launchpad.net/nova/+bug/1804652
105+
try:
106+
return _db_version(repository, database, context)
107+
except Exception:
108+
return _db_version(repository, database, context)
109+
110+
111+
def _db_version(repository, database, context):
112+
engine = get_engine(database, context=context)
113+
try:
114+
return versioning_api.db_version(engine, repository)
115+
except versioning_exceptions.DatabaseNotControlledError as exc:
116+
meta = sqlalchemy.MetaData()
117+
meta.reflect(bind=engine)
118+
tables = meta.tables
119+
if len(tables) == 0:
120+
db_version_control(
121+
INIT_VERSION[database], database, context=context)
122+
return versioning_api.db_version(engine, repository)
123+
else:
124+
LOG.exception(exc)
125+
# Some pre-Essex DB's may not be version controlled.
126+
# Require them to upgrade using Essex first.
127+
raise exception.NovaException(
128+
_("Upgrade DB using Essex release first."))
32129

33130

34131
def db_initial_version(database='main'):
35132
"""The starting version for the database."""
36-
return IMPL.db_initial_version(database=database)
133+
return INIT_VERSION[database]
134+
135+
136+
def db_version_control(version=None, database='main', context=None):
137+
repository = find_migrate_repo(database)
138+
engine = get_engine(database, context=context)
139+
versioning_api.version_control(engine, repository, version)
140+
return version

nova/db/sqlalchemy/migration.py

Lines changed: 0 additions & 140 deletions
This file was deleted.

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
from nova.db import migration
4141
from nova.db.sqlalchemy.api_migrations import migrate_repo
4242
from nova.db.sqlalchemy import api_models
43-
from nova.db.sqlalchemy import migration as sa_migration
4443
from nova import test
4544
from nova.tests import fixtures as nova_fixtures
4645

@@ -53,9 +52,8 @@ def setUp(self):
5352
self.engine = enginefacade.writer.get_engine()
5453

5554
def db_sync(self, engine):
56-
with mock.patch.object(sa_migration, 'get_engine',
57-
return_value=engine):
58-
sa_migration.db_sync(database='api')
55+
with mock.patch.object(migration, 'get_engine', return_value=engine):
56+
migration.db_sync(database='api')
5957

6058
@property
6159
def migrate_engine(self):
@@ -160,7 +158,7 @@ def REPOSITORY(self):
160158

161159
@property
162160
def migration_api(self):
163-
return sa_migration.versioning_api
161+
return migration.versioning_api
164162

165163
@property
166164
def migrate_engine(self):

nova/tests/unit/cmd/test_manage.py

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
from nova import conf
3131
from nova import context
3232
from nova.db import api as db
33-
from nova.db.sqlalchemy import migration as sqla_migration
33+
from nova.db import migration
3434
from nova import exception
3535
from nova import objects
3636
from nova.scheduler.client import report
@@ -618,18 +618,17 @@ def test_purge_all_cells_no_api_config(self, mock_get_cells):
618618
self.assertEqual(4, ret)
619619
self.assertIn('Unable to get cell list', self.output.getvalue())
620620

621-
@mock.patch.object(sqla_migration, 'db_version', return_value=2)
622-
def test_version(self, sqla_migrate):
621+
@mock.patch.object(migration, 'db_version', return_value=2)
622+
def test_version(self, mock_db_version):
623623
self.commands.version()
624-
sqla_migrate.assert_called_once_with(context=None, database='main')
624+
mock_db_version.assert_called_once_with()
625625

626-
@mock.patch.object(sqla_migration, 'db_sync')
627-
def test_sync(self, sqla_sync):
626+
@mock.patch.object(migration, 'db_sync')
627+
def test_sync(self, mock_db_sync):
628628
self.commands.sync(version=4, local_cell=True)
629-
sqla_sync.assert_called_once_with(context=None,
630-
version=4, database='main')
629+
mock_db_sync.assert_called_once_with(4)
631630

632-
@mock.patch('nova.db.migration.db_sync')
631+
@mock.patch.object(migration, 'db_sync')
633632
@mock.patch.object(objects.CellMapping, 'get_by_uuid', return_value='map')
634633
def test_sync_cell0(self, mock_get_by_uuid, mock_db_sync):
635634
ctxt = context.get_admin_context()
@@ -810,17 +809,15 @@ def setUp(self):
810809
self.useFixture(fixtures.MonkeyPatch('sys.stdout', self.output))
811810
self.commands = manage.ApiDbCommands()
812811

813-
@mock.patch.object(sqla_migration, 'db_version', return_value=2)
814-
def test_version(self, sqla_migrate):
812+
@mock.patch.object(migration, 'db_version', return_value=2)
813+
def test_version(self, mock_db_version):
815814
self.commands.version()
816-
sqla_migrate.assert_called_once_with(context=None,
817-
database='api')
815+
mock_db_version.assert_called_once_with(database='api')
818816

819-
@mock.patch.object(sqla_migration, 'db_sync')
820-
def test_sync(self, sqla_sync):
817+
@mock.patch.object(migration, 'db_sync')
818+
def test_sync(self, mock_db_sync):
821819
self.commands.sync(version=4)
822-
sqla_sync.assert_called_once_with(context=None,
823-
version=4, database='api')
820+
mock_db_sync.assert_called_once_with(4, database='api')
824821

825822

826823
@ddt.ddt

nova/tests/unit/db/test_sqlalchemy_migration.py renamed to nova/tests/unit/db/test_migration.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
import mock
1818
import sqlalchemy
1919

20+
from nova.db import migration
2021
from nova.db.sqlalchemy import api as db_api
21-
from nova.db.sqlalchemy import migration
2222
from nova import test
2323

2424

0 commit comments

Comments
 (0)