diff --git a/ironic/cmd/dbsync.py b/ironic/cmd/dbsync.py index 6c34bee219..0450ffede0 100644 --- a/ironic/cmd/dbsync.py +++ b/ironic/cmd/dbsync.py @@ -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, diff --git a/ironic/tests/unit/cmd/test_dbsync.py b/ironic/tests/unit/cmd/test_dbsync.py index d8a459b25b..0b02990175 100644 --- a/ironic/tests/unit/cmd/test_dbsync.py +++ b/ironic/tests/unit/cmd/test_dbsync.py @@ -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)