diff options
author | Dmitry Tantsur <divius.inside@gmail.com> | 2018-01-19 12:59:44 +0100 |
---|---|---|
committer | Julia Kreger <juliaashleykreger@gmail.com> | 2018-01-25 16:51:42 +0000 |
commit | d8f10c01ae4105a88e8caf58e89041d4c81f1670 (patch) | |
tree | c455abdf966e82fdc1937e4135a50462190500b3 | |
parent | f5654bfd00bd11564203657062d4c53803dcf0c7 (diff) | |
download | ironic-d8f10c01ae4105a88e8caf58e89041d4c81f1670.tar.gz |
Allow data migrations to accept options
This allows migration to be tuned to a specific deployment. For example,
when we will migrate nodes to hardware types, an option will be used
to specify what to do with missing interfaces.
Change-Id: Ie5045b20b7420fc9b5d864bfb18258a4d8b93334
Related-Bug: #1690185
-rw-r--r-- | doc/source/cli/ironic-dbsync.rst | 7 | ||||
-rw-r--r-- | ironic/cmd/dbsync.py | 39 | ||||
-rw-r--r-- | ironic/tests/unit/cmd/test_dbsync.py | 78 |
3 files changed, 95 insertions, 29 deletions
diff --git a/doc/source/cli/ironic-dbsync.rst b/doc/source/cli/ironic-dbsync.rst index bba2633d8..e45f22166 100644 --- a/doc/source/cli/ironic-dbsync.rst +++ b/doc/source/cli/ironic-dbsync.rst @@ -112,6 +112,11 @@ online_data_migrations If not specified, all the objects will be migrated (in batches of 50 to avoid locking the database for long periods of time). +.. option:: --option <MIGRATION.KEY=VALUE> + + If a migration accepts additional parameters, they can be passed via this + argument. It can be specified several times. + This command will migrate objects in the database to their most recent versions. This command must be successfully run (return code 0) before upgrading to a future release. @@ -124,7 +129,7 @@ It returns: * 0 (success) after migrations are finished or there are no data to migrate -* 127 (error) if max-count is not a positive value +* 127 (error) if max-count is not a positive value or an option is invalid * 2 (error) if the database is not compatible with this release. This command needs to be run using the previous release of ironic, before upgrading and diff --git a/ironic/cmd/dbsync.py b/ironic/cmd/dbsync.py index 0450ffede..30b484df3 100644 --- a/ironic/cmd/dbsync.py +++ b/ironic/cmd/dbsync.py @@ -111,9 +111,10 @@ class DBCommand(object): def online_data_migrations(self): self._check_versions() - self._run_online_data_migrations(max_count=CONF.command.max_count) + self._run_online_data_migrations(max_count=CONF.command.max_count, + options=CONF.command.options) - def _run_migration_functions(self, context, max_count): + def _run_migration_functions(self, context, max_count, options): """Runs the migration functions. Runs the data migration functions in the ONLINE_MIGRATIONS list. @@ -124,6 +125,8 @@ class DBCommand(object): :param: context: an admin context :param: max_count: the maximum number of objects (rows) to migrate; a value >= 1. + :param: options: migration options - dict mapping migration name + to a dictionary of options for this migration. :raises: Exception from the migration function :returns: Boolean value indicating whether migrations are done. Returns False if max_count objects have been migrated (since at that @@ -135,10 +138,12 @@ class DBCommand(object): for migration_func_obj, migration_func_name in ONLINE_MIGRATIONS: migration_func = getattr(migration_func_obj, migration_func_name) + migration_opts = options.get(migration_func_name, {}) num_to_migrate = max_count - total_migrated try: total_to_do, num_migrated = migration_func(context, - num_to_migrate) + num_to_migrate, + **migration_opts) except Exception as e: print(_("Error while running %(migration)s: %(err)s.") % {'migration': migration_func.__name__, 'err': e}, @@ -165,7 +170,7 @@ class DBCommand(object): return True - def _run_online_data_migrations(self, max_count=None): + def _run_online_data_migrations(self, max_count=None, options=None): """Perform online data migrations for the release. Online data migrations are done by running all the data migration @@ -177,13 +182,27 @@ class DBCommand(object): :param: max_count: the maximum number of individual object migrations or modified rows, a value >= 1. If None, migrations are run in a loop in batches of 50, until completion. + :param: options: options to pass to migrations. List of values in the + form of <migration name>.<option>=<value> :raises: SystemExit. With exit code of: 0: when all migrations are complete. 1: when objects were migrated and the command needs to be re-run (because there might be more objects to be migrated) - 127: if max_count is < 1 + 127: if max_count is < 1 or any option is invalid :raises: Exception from a migration function """ + parsed_options = {} + if options: + for option in options: + try: + migration, key_value = option.split('.', 1) + key, value = key_value.split('=', 1) + except ValueError: + print(_("Malformed option %s") % option) + sys.exit(127) + else: + parsed_options.setdefault(migration, {})[key] = value + admin_context = context.get_admin_context() finished_migrating = False if max_count is None: @@ -192,7 +211,7 @@ class DBCommand(object): 'completed.') % max_count) while not finished_migrating: finished_migrating = self._run_migration_functions( - admin_context, max_count) + admin_context, max_count, parsed_options) print(_('Data migrations have completed.')) sys.exit(0) @@ -201,7 +220,8 @@ class DBCommand(object): sys.exit(127) finished_migrating = self._run_migration_functions(admin_context, - max_count) + max_count, + parsed_options) if finished_migrating: print(_('Data migrations have completed.')) sys.exit(0) @@ -269,6 +289,11 @@ def add_command_parsers(subparsers): '--max-count', metavar='<number>', dest='max_count', type=int, help=_("Maximum number of objects to migrate. If unspecified, all " "objects are migrated.")) + parser.add_argument( + '--option', metavar='<migration.opt=val>', action='append', + dest='options', default=[], + help=_("Options to pass to the migrations in the form of " + "<migration name>.<option>=<value>")) parser.set_defaults(func=command_object.online_data_migrations) diff --git a/ironic/tests/unit/cmd/test_dbsync.py b/ironic/tests/unit/cmd/test_dbsync.py index 0b0299017..406215c1a 100644 --- a/ironic/tests/unit/cmd/test_dbsync.py +++ b/ironic/tests/unit/cmd/test_dbsync.py @@ -57,7 +57,7 @@ class OnlineMigrationTestCase(db_base.DbTestCase): mock_func = mock.MagicMock(side_effect=((15, 15),), __name__='foo') with mock.patch.object(self.dbapi, 'foo', mock_func, create=True): self.assertTrue( - self.db_cmds._run_migration_functions(self.context, 50)) + 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) @@ -65,7 +65,7 @@ class OnlineMigrationTestCase(db_base.DbTestCase): # No migration functions to run mock_migrations.__iter__.return_value = () self.assertTrue( - self.db_cmds._run_migration_functions(self.context, 50)) + self.db_cmds._run_migration_functions(self.context, 50, {})) @mock.patch.object(dbsync, 'ONLINE_MIGRATIONS', autospec=True) def test__run_migration_functions_exception(self, mock_migrations): @@ -76,7 +76,7 @@ class OnlineMigrationTestCase(db_base.DbTestCase): with mock.patch.object(self.dbapi, 'foo', mock_func, create=True): self.assertRaises( TypeError, self.db_cmds._run_migration_functions, - self.context, 50) + self.context, 50, {}) mock_func.assert_called_once_with(self.context, 50) @mock.patch.object(dbsync, 'ONLINE_MIGRATIONS', autospec=True) @@ -89,10 +89,12 @@ class OnlineMigrationTestCase(db_base.DbTestCase): 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) + options = {'func1': {'key': 'value'}, + 'func2': {'x': 1, 'y': 2}} + self.assertTrue(self.db_cmds._run_migration_functions( + self.context, 50, options)) + mock_func1.assert_called_once_with(self.context, 50, key='value') + mock_func2.assert_called_once_with(self.context, 35, x=1, y=2) @mock.patch.object(dbsync, 'ONLINE_MIGRATIONS', autospec=True) def test__run_migration_functions_2_notdone(self, mock_migrations): @@ -104,8 +106,8 @@ class OnlineMigrationTestCase(db_base.DbTestCase): 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)) + 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) @@ -119,8 +121,8 @@ class OnlineMigrationTestCase(db_base.DbTestCase): 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)) + 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) @@ -134,8 +136,8 @@ class OnlineMigrationTestCase(db_base.DbTestCase): 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)) + 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) @@ -151,12 +153,12 @@ class OnlineMigrationTestCase(db_base.DbTestCase): 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)) + 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)) + 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) @@ -167,7 +169,41 @@ class OnlineMigrationTestCase(db_base.DbTestCase): exit = self.assertRaises(SystemExit, self.db_cmds._run_online_data_migrations) self.assertEqual(0, exit.code) - mock_functions.assert_called_once_with(self.db_cmds, mock.ANY, 50) + mock_functions.assert_called_once_with(self.db_cmds, mock.ANY, 50, {}) + + @mock.patch.object(dbsync.DBCommand, '_run_migration_functions', + autospec=True) + def test__run_online_data_migrations_with_options(self, mock_functions): + mock_functions.return_value = True + exit = self.assertRaises(SystemExit, + self.db_cmds._run_online_data_migrations, + options=["m1.key1=value1", "m1.key2=value2", + "m2.key3=value3"]) + self.assertEqual(0, exit.code) + mock_functions.assert_called_once_with(self.db_cmds, mock.ANY, 50, + {'m1': {'key1': 'value1', + 'key2': 'value2'}, + 'm2': {'key3': 'value3'}}) + + @mock.patch.object(dbsync.DBCommand, '_run_migration_functions', + autospec=True) + def test__run_online_data_migrations_invalid_option1(self, mock_functions): + mock_functions.return_value = True + exit = self.assertRaises(SystemExit, + self.db_cmds._run_online_data_migrations, + options=["m1key1=value1"]) + self.assertEqual(127, exit.code) + self.assertFalse(mock_functions.called) + + @mock.patch.object(dbsync.DBCommand, '_run_migration_functions', + autospec=True) + def test__run_online_data_migrations_invalid_option2(self, mock_functions): + mock_functions.return_value = True + exit = self.assertRaises(SystemExit, + self.db_cmds._run_online_data_migrations, + options=["m1.key1value1"]) + self.assertEqual(127, exit.code) + self.assertFalse(mock_functions.called) @mock.patch.object(dbsync.DBCommand, '_run_migration_functions', autospec=True) @@ -177,7 +213,7 @@ class OnlineMigrationTestCase(db_base.DbTestCase): self.db_cmds._run_online_data_migrations) self.assertEqual(0, exit.code) mock_functions.assert_has_calls( - (mock.call(self.db_cmds, mock.ANY, 50),) * 2) + (mock.call(self.db_cmds, mock.ANY, 50, {}),) * 2) @mock.patch.object(dbsync.DBCommand, '_run_migration_functions', autospec=True) @@ -187,7 +223,7 @@ class OnlineMigrationTestCase(db_base.DbTestCase): self.db_cmds._run_online_data_migrations, max_count=30) self.assertEqual(1, exit.code) - mock_functions.assert_called_once_with(self.db_cmds, mock.ANY, 30) + mock_functions.assert_called_once_with(self.db_cmds, mock.ANY, 30, {}) @mock.patch.object(dbsync.DBCommand, '_run_migration_functions', autospec=True) @@ -204,4 +240,4 @@ class OnlineMigrationTestCase(db_base.DbTestCase): def test__run_online_data_migrations_exception(self, mock_functions): mock_functions.side_effect = TypeError("yuck") self.assertRaises(TypeError, self.db_cmds._run_online_data_migrations) - mock_functions.assert_called_once_with(self.db_cmds, mock.ANY, 50) + mock_functions.assert_called_once_with(self.db_cmds, mock.ANY, 50, {}) |