openstack
/
ironic
Code Issues Proposed changes

Do not access dbapi attributes on dbsync import 

This may cause troubles as some config options may not be
registered yet on the import of dbsync module. Accessing
dbapi attributes, which is lazy-loaded, will force the
object to initialize, and it may fail at such an early
stage.

Change-Id: I3b37498325a191f7c42b3b856b1b2cf326d57492
This commit is contained in:
Vladyslav Drok 2017-12-01 15:30:38 -08:00
parent 638943dbf8
commit 351677d3b8
2 changed files with 58 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
ironic/cmd/dbsync.py
View File

@ -57,10 +57,14 @@ dbapi = db_api.get_instance()
# migrated (at the beginning of this call) and the number
# of migrated objects.
# """
# NOTE(vdrok): Do not access objects' attributes, instead only provide object
# and attribute name tuples, so that not to trigger the load of the whole
# object, in case it is lazy loaded. The attribute will be accessed when needed
# by doing getattr on the object
ONLINE_MIGRATIONS = (
# Added in Pike, modified in Queens
# TODO(rloo): remove in Rocky
dbapi.backfill_version_column,
(dbapi, 'backfill_version_column'),
)
@ -129,7 +133,8 @@ class DBCommand(object):
"""
total_migrated = 0
for migration_func in ONLINE_MIGRATIONS:
for migration_func_obj, migration_func_name in ONLINE_MIGRATIONS:
migration_func = getattr(migration_func_obj, migration_func_name)
num_to_migrate = max_count - total_migrated
try:
total_to_do, num_migrated = migration_func(context,

79
ironic/tests/unit/cmd/test_dbsync.py
View File

@ -53,11 +53,12 @@ class OnlineMigrationTestCase(db_base.DbTestCase):
@mock.patch.object(dbsync, 'ONLINE_MIGRATIONS', autospec=True)
def test__run_migration_functions(self, mock_migrations):
mock_migrations.__iter__.return_value = ((self.dbapi, 'foo'),)
mock_func = mock.MagicMock(side_effect=((15, 15),), __name__='foo')
mock_migrations.__iter__.return_value = (mock_func,)
self.assertTrue(
self.db_cmds._run_migration_functions(self.context, 50))
mock_func.assert_called_once_with(self.context, 50)
with mock.patch.object(self.dbapi, 'foo', mock_func, create=True):
self.assertTrue(
self.db_cmds._run_migration_functions(self.context, 50))
mock_func.assert_called_once_with(self.context, 50)
@mock.patch.object(dbsync, 'ONLINE_MIGRATIONS', autospec=True)
def test__run_migration_functions_none(self, mock_migrations):
@ -68,74 +69,96 @@ class OnlineMigrationTestCase(db_base.DbTestCase):
@mock.patch.object(dbsync, 'ONLINE_MIGRATIONS', autospec=True)
def test__run_migration_functions_exception(self, mock_migrations):
mock_migrations.__iter__.return_value = ((self.dbapi, 'foo'),)
# Migration function raises exception
mock_func = mock.MagicMock(side_effect=TypeError("bar"),
__name__='foo')
mock_migrations.__iter__.return_value = (mock_func,)
self.assertRaises(TypeError, self.db_cmds._run_migration_functions,
self.context, 50)
with mock.patch.object(self.dbapi, 'foo', mock_func, create=True):
self.assertRaises(
TypeError, self.db_cmds._run_migration_functions,
self.context, 50)
mock_func.assert_called_once_with(self.context, 50)
@mock.patch.object(dbsync, 'ONLINE_MIGRATIONS', autospec=True)
def test__run_migration_functions_2(self, mock_migrations):
# 2 migration functions, migration completed
mock_migrations.__iter__.return_value = ((self.dbapi, 'func1'),
(self.dbapi, 'func2'))
mock_func1 = mock.MagicMock(side_effect=((15, 15),), __name__='func1')
mock_func2 = mock.MagicMock(side_effect=((20, 20),), __name__='func2')
mock_migrations.__iter__.return_value = (mock_func1, mock_func2)
self.assertTrue(
self.db_cmds._run_migration_functions(self.context, 50))
with mock.patch.object(self.dbapi, 'func1', mock_func1, create=True):
with mock.patch.object(self.dbapi, 'func2', mock_func2,
create=True):
self.assertTrue(
self.db_cmds._run_migration_functions(self.context, 50))
mock_func1.assert_called_once_with(self.context, 50)
mock_func2.assert_called_once_with(self.context, 35)
@mock.patch.object(dbsync, 'ONLINE_MIGRATIONS', autospec=True)
def test__run_migration_functions_2_notdone(self, mock_migrations):
# 2 migration functions; only first function was run but not completed
mock_migrations.__iter__.return_value = ((self.dbapi, 'func1'),
(self.dbapi, 'func2'))
mock_func1 = mock.MagicMock(side_effect=((15, 10),), __name__='func1')
mock_func2 = mock.MagicMock(side_effect=((20, 0),), __name__='func2')
mock_migrations.__iter__.return_value = (mock_func1, mock_func2)
self.assertFalse(
self.db_cmds._run_migration_functions(self.context, 10))
with mock.patch.object(self.dbapi, 'func1', mock_func1, create=True):
with mock.patch.object(self.dbapi, 'func2', mock_func2,
create=True):
self.assertFalse(
self.db_cmds._run_migration_functions(self.context, 10))
mock_func1.assert_called_once_with(self.context, 10)
self.assertFalse(mock_func2.called)
@mock.patch.object(dbsync, 'ONLINE_MIGRATIONS', autospec=True)
def test__run_migration_functions_2_onedone(self, mock_migrations):
# 2 migration functions; only first function was run and completed
mock_migrations.__iter__.return_value = ((self.dbapi, 'func1'),
(self.dbapi, 'func2'))
mock_func1 = mock.MagicMock(side_effect=((10, 10),), __name__='func1')
mock_func2 = mock.MagicMock(side_effect=((20, 0),), __name__='func2')
mock_migrations.__iter__.return_value = (mock_func1, mock_func2)
self.assertFalse(
self.db_cmds._run_migration_functions(self.context, 10))
with mock.patch.object(self.dbapi, 'func1', mock_func1, create=True):
with mock.patch.object(self.dbapi, 'func2', mock_func2,
create=True):
self.assertFalse(
self.db_cmds._run_migration_functions(self.context, 10))
mock_func1.assert_called_once_with(self.context, 10)
self.assertFalse(mock_func2.called)
@mock.patch.object(dbsync, 'ONLINE_MIGRATIONS', autospec=True)
def test__run_migration_functions_2_done(self, mock_migrations):
# 2 migration functions; migrations completed
mock_migrations.__iter__.return_value = ((self.dbapi, 'func1'),
(self.dbapi, 'func2'))
mock_func1 = mock.MagicMock(side_effect=((10, 10),), __name__='func1')
mock_func2 = mock.MagicMock(side_effect=((0, 0),), __name__='func2')
mock_migrations.__iter__.return_value = (mock_func1, mock_func2)
self.assertTrue(
self.db_cmds._run_migration_functions(self.context, 15))
with mock.patch.object(self.dbapi, 'func1', mock_func1, create=True):
with mock.patch.object(self.dbapi, 'func2', mock_func2,
create=True):
self.assertTrue(
self.db_cmds._run_migration_functions(self.context, 15))
mock_func1.assert_called_once_with(self.context, 15)
mock_func2.assert_called_once_with(self.context, 5)
@mock.patch.object(dbsync, 'ONLINE_MIGRATIONS', autospec=True)
def test__run_migration_functions_two_calls_done(self, mock_migrations):
# 2 migration functions; migrations completed after calling twice
mock_migrations.__iter__.return_value = ((self.dbapi, 'func1'),
(self.dbapi, 'func2'))
mock_func1 = mock.MagicMock(side_effect=((10, 10), (0, 0)),
__name__='func1')
mock_func2 = mock.MagicMock(side_effect=((0, 0), (0, 0)),
__name__='func2')
mock_migrations.__iter__.return_value = (mock_func1, mock_func2)
self.assertFalse(
self.db_cmds._run_migration_functions(self.context, 10))
mock_func1.assert_called_once_with(self.context, 10)
self.assertFalse(mock_func2.called)
self.assertTrue(
self.db_cmds._run_migration_functions(self.context, 10))
mock_func1.assert_has_calls((mock.call(self.context, 10),) * 2)
mock_func2.assert_called_once_with(self.context, 10)
with mock.patch.object(self.dbapi, 'func1', mock_func1, create=True):
with mock.patch.object(self.dbapi, 'func2', mock_func2,
create=True):
self.assertFalse(
self.db_cmds._run_migration_functions(self.context, 10))
mock_func1.assert_called_once_with(self.context, 10)
self.assertFalse(mock_func2.called)
self.assertTrue(
self.db_cmds._run_migration_functions(self.context, 10))
mock_func1.assert_has_calls((mock.call(self.context, 10),) * 2)
mock_func2.assert_called_once_with(self.context, 10)
@mock.patch.object(dbsync.DBCommand, '_run_migration_functions',
autospec=True)