From 5f515dda64de7047fbb2fe5c4076874d1f2aa175 Mon Sep 17 00:00:00 2001 From: Joshua Hesketh Date: Thu, 23 Apr 2015 11:43:41 +1000 Subject: [PATCH] Add support for forcing migrate_flavor_data In some cases it might be helpful to force the migration of flavor data. For example, when operating on a snapshot of a production database there may be instances in an operating state that would stop the migration from finishing. Blocking on this will make it impossible to finish the migrations (once the migrations require all the data has been migrated). Change-Id: I4f6c0679b144897f2bfc2fa06bc7c72b25a85fd3 --- nova/cmd/manage.py | 7 +++++-- nova/db/api.py | 6 ++++-- nova/db/sqlalchemy/api.py | 14 ++++++++------ nova/tests/unit/db/test_db_api.py | 29 ++++++++++++++++++++++------- 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index 433ddcbec826..eabb15901682 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -960,7 +960,10 @@ class DbCommands(object): @args('--max-number', metavar='', dest='max_number', help='Maximum number of instances to consider') - def migrate_flavor_data(self, max_number=None): + @args('--force', action='store_true', dest='force', + help='Force instances to migrate (even if they may be performing ' + 'another operation). Warning, this is potentially dangerous.') + def migrate_flavor_data(self, max_number=None, force=False): if max_number is not None: max_number = int(max_number) if max_number < 0: @@ -969,7 +972,7 @@ class DbCommands(object): admin_context = context.get_admin_context() flavor_cache = {} match, done = db.migrate_flavor_data(admin_context, max_number, - flavor_cache) + flavor_cache, force) print(_('%(total)i instances matched query, %(done)i completed') % {'total': match, 'done': done}) diff --git a/nova/db/api.py b/nova/db/api.py index 4aad16ce42e3..39e3fe5cd876 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -1952,17 +1952,19 @@ def archive_deleted_rows_for_table(context, tablename, max_rows=None): max_rows=max_rows) -def migrate_flavor_data(context, max_count, flavor_cache): +def migrate_flavor_data(context, max_count, flavor_cache, force=False): """Migrate instance flavor data from system_metadata to instance_extra. :param max_count: The maximum number of instances to consider in this run. :param flavor_cache: A dict to persist flavor information in across calls (just pass an empty dict here) + :param force: Boolean whether or not to force migration of instances that + are performing another operation. :returns: number of instances needing migration, number of instances migrated (both will always be less than max_count) """ - return IMPL.migrate_flavor_data(context, max_count, flavor_cache) + return IMPL.migrate_flavor_data(context, max_count, flavor_cache, force) #################### diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 9dd56ff54dc1..7424886906da 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -6015,7 +6015,7 @@ def _augment_flavors_to_migrate(instance, flavor_cache): @require_admin_context -def migrate_flavor_data(context, max_count, flavor_cache): +def migrate_flavor_data(context, max_count, flavor_cache, force=False): # NOTE(danms): This is only ever run in nova-manage, and we need to avoid # a circular import from nova import objects @@ -6042,10 +6042,11 @@ def migrate_flavor_data(context, max_count, flavor_cache): # middle of some other operation. This is just a guess and not # a lock. There is still a race here, although it's the same # race as the normal code, since we use expected_task_state below. - if instance.task_state is not None: - continue - if instance.vm_state in [vm_states.RESCUED, vm_states.RESIZED]: - continue + if not force: + if instance.task_state is not None: + continue + if instance.vm_state in [vm_states.RESCUED, vm_states.RESIZED]: + continue _augment_flavors_to_migrate(instance, flavor_cache) if instance.obj_what_changed(): @@ -6054,7 +6055,8 @@ def migrate_flavor_data(context, max_count, flavor_cache): {'instance_uuid': db_instance['uuid']}) LOG.debug( 'Created instance_extra for %s' % db_instance['uuid']) - instance.save(expected_task_state=[None]) + instance.save(expected_task_state=[instance.task_state], + expected_vm_state=[instance.vm_state]) count_hit += 1 return count_all, count_hit diff --git a/nova/tests/unit/db/test_db_api.py b/nova/tests/unit/db/test_db_api.py index 9e31e848d933..1a6f8963c783 100644 --- a/nova/tests/unit/db/test_db_api.py +++ b/nova/tests/unit/db/test_db_api.py @@ -7604,7 +7604,7 @@ class FlavorMigrationTestCase(test.TestCase): self.assertEqual(0, match) self.assertEqual(0, done) - def test_migrate_flavor_honors_states(self): + def _test_migrate_flavor_states(self, force=False): ctxt = context.get_admin_context() flavor = flavors.get_default_flavor() sysmeta = flavors.save_flavor_info({}, flavor) @@ -7632,13 +7632,28 @@ class FlavorMigrationTestCase(test.TestCase): } db.instance_create(ctxt, values) - match, done = db.migrate_flavor_data(ctxt, None, {}) - self.assertEqual(4, match) - self.assertEqual(1, done) + match, done = db.migrate_flavor_data(ctxt, None, {}, force) - match, done = db.migrate_flavor_data(ctxt, None, {}) - self.assertEqual(3, match) - self.assertEqual(0, done) + if not force: + self.assertEqual(4, match) + self.assertEqual(1, done) + + match, done = db.migrate_flavor_data(ctxt, None, {}, force) + self.assertEqual(3, match) + self.assertEqual(0, done) + else: + self.assertEqual(4, match) + self.assertEqual(4, done) + + match, done = db.migrate_flavor_data(ctxt, None, {}, force) + self.assertEqual(0, match) + self.assertEqual(0, done) + + def test_migrate_flavor_honors_states(self): + self._test_migrate_flavor_states(force=False) + + def test_migrate_flavor_force_states(self): + self._test_migrate_flavor_states(force=True) def test_migrate_flavor_gets_missing_extra_rows(self): ctxt = context.get_admin_context()