openstack/ironic
Code Issues Proposed changes

Merge "Allow data migrations to accept options"

Browse Source
This commit is contained in:
Zuul 2018-01-25 22:50:18 +00:00 committed by Gerrit Code Review
commit 3258662dc4
3 changed files with 95 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
doc/source/cli/ironic-dbsync.rst
View File

@ -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

39
ironic/cmd/dbsync.py
View File

@ -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)

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

@ -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, {})