From 86a8006202e9a068f0c3b14655d3ff72e84d2b69 Mon Sep 17 00:00:00 2001 From: Timofey Durakov Date: Wed, 11 May 2016 16:00:43 +0300 Subject: [PATCH] force_live_migration remove redundant check During force_live_migration operation there are 2 checks of migration status, one on api layer and one on compute side, second one requires rpc request to be done and also to query the db. This check could be removed as additional one doesn't guarantee that live_migration is not already completed, also is very small window between api and compute side. Target compute host is now taken from migration object rather then instance.host field, which allows to guarantee that nova will not pause already migrated instance. Change-Id: I51585302e898251e26cbf311aeac38a0ec329001 --- doc/notification_samples/service-update.json | 2 +- nova/compute/api.py | 2 +- nova/compute/manager.py | 18 ++++---- nova/compute/rpcapi.py | 16 ++++--- nova/objects/service.py | 4 +- .../test_server_migrations.py | 1 + nova/tests/unit/compute/test_compute_api.py | 4 +- nova/tests/unit/compute/test_compute_mgr.py | 17 -------- nova/tests/unit/compute/test_rpcapi.py | 43 +++++++++++++++++-- 9 files changed, 67 insertions(+), 40 deletions(-) diff --git a/doc/notification_samples/service-update.json b/doc/notification_samples/service-update.json index 1501ff68fb4a..e6d51530fadf 100644 --- a/doc/notification_samples/service-update.json +++ b/doc/notification_samples/service-update.json @@ -13,7 +13,7 @@ "disabled_reason": null, "report_count": 1, "forced_down": false, - "version": 10 + "version": 11 } }, "event_type": "service.update", diff --git a/nova/compute/api.py b/nova/compute/api.py index b87f9e2d94c5..f14d43a332cd 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -3424,7 +3424,7 @@ class API(base.Base): context, instance, instance_actions.LIVE_MIGRATION_FORCE_COMPLETE) self.compute_rpcapi.live_migration_force_complete( - context, instance, migration.id) + context, instance, migration) @check_instance_lock @check_instance_cell diff --git a/nova/compute/manager.py b/nova/compute/manager.py index a78d0fe7d9c6..c1e525251579 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -499,7 +499,7 @@ class ComputeVirtAPI(virtapi.VirtAPI): class ComputeManager(manager.Manager): """Manages the running instances from creation to destruction.""" - target = messaging.Target(version='4.11') + target = messaging.Target(version='4.12') # How long to wait in seconds before re-issuing a shutdown # signal to an instance during power off. The overall @@ -5158,23 +5158,21 @@ class ComputeManager(manager.Manager): block_migration, migration, migrate_data) + # TODO(tdurakov): migration_id is used since 4.12 rpc api version + # remove migration_id parameter when the compute RPC version + # is bumped to 5.x. @wrap_exception() @wrap_instance_event @wrap_instance_fault - def live_migration_force_complete(self, context, instance, migration_id): + def live_migration_force_complete(self, context, instance, + migration_id=None): """Force live migration to complete. :param context: Security context :param instance: The instance that is being migrated - :param migration_id: ID of ongoing migration - + :param migration_id: ID of ongoing migration; is currently not used, + and isn't removed for backward compatibility """ - migration = objects.Migration.get_by_id(context, migration_id) - if migration.status != 'running': - raise exception.InvalidMigrationState(migration_id=migration_id, - instance_uuid=instance.uuid, - state=migration.status, - method='force complete') self._notify_about_instance_usage( context, instance, 'live.migration.force.complete.start') diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py index 7a1f8c8429e7..d20838ee2ccf 100644 --- a/nova/compute/rpcapi.py +++ b/nova/compute/rpcapi.py @@ -310,6 +310,8 @@ class ComputeAPI(object): ... Mitaka supports messaging version 4.11. So, any changes to existing methods in 4.x after that point should be done so that they can handle the version_cap being set to 4.11 + + * 4.12 - Remove migration_id from live_migration_force_complete ''' VERSION_ALIASES = { @@ -634,12 +636,16 @@ class ComputeAPI(object): dest=dest, block_migration=block_migration, migrate_data=migrate_data, **args) - def live_migration_force_complete(self, ctxt, instance, migration_id): - version = '4.9' - cctxt = self.client.prepare(server=_compute_host(None, instance), - version=version) + def live_migration_force_complete(self, ctxt, instance, migration): + version = '4.12' + kwargs = {} + if not self.client.can_send_version(version): + version = '4.9' + kwargs['migration_id'] = migration.id + cctxt = self.client.prepare(server=_compute_host( + migration.source_compute, instance), version=version) cctxt.cast(ctxt, 'live_migration_force_complete', instance=instance, - migration_id=migration_id) + **kwargs) def live_migration_abort(self, ctxt, instance, migration_id): version = '4.10' diff --git a/nova/objects/service.py b/nova/objects/service.py index d5f914ac6905..a674c0e2182d 100644 --- a/nova/objects/service.py +++ b/nova/objects/service.py @@ -29,7 +29,7 @@ LOG = logging.getLogger(__name__) # NOTE(danms): This is the global service version counter -SERVICE_VERSION = 10 +SERVICE_VERSION = 11 # NOTE(danms): This is our SERVICE_VERSION history. The idea is that any @@ -73,6 +73,8 @@ SERVICE_VERSION_HISTORY = ( {'compute_rpc': '4.11'}, # Version 10: Compute node conversion to Inventories {'compute_rpc': '4.11'}, + # Version 11: Removed migration_id from live_migration_force_complete + {'compute_rpc': '4.12'}, ) diff --git a/nova/tests/functional/api_sample_tests/test_server_migrations.py b/nova/tests/functional/api_sample_tests/test_server_migrations.py index 4a96d6acc9b2..f7da1d80dd16 100644 --- a/nova/tests/functional/api_sample_tests/test_server_migrations.py +++ b/nova/tests/functional/api_sample_tests/test_server_migrations.py @@ -47,6 +47,7 @@ class ServerMigrationsSampleJsonTest(test_servers.ServersSampleBase): migration = objects.Migration() migration.id = 1 migration.status = 'running' + migration.source_compute = self.compute.host get_by_id_and_instance.return_value = migration self._do_post('servers/%s/action' % self.uuid, 'live-migrate-server', {'hostname': self.compute.host}) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index cf14d0a58856..bcb1e2fb18d6 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -3631,11 +3631,11 @@ class _ComputeAPIUnitTestMixIn(object): with mock.patch.object( rpcapi, 'live_migration_force_complete') as lm_force_complete: self.compute_api.live_migrate_force_complete( - self.context, instance, migration.id) + self.context, instance, migration) lm_force_complete.assert_called_once_with(self.context, instance, - 0) + migration) action_start.assert_called_once_with( self.context, instance.uuid, 'live_migration_force_complete', want_result=False) diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index ab6278c6d13b..db20885d8ab8 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -4637,23 +4637,6 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): _do_test() - @mock.patch.object(compute_utils, 'add_instance_fault_from_exc') - def test_live_migration_pause_vm_invalid_migration_state( - self, add_instance_fault_from_exc): - - instance = objects.Instance(id=1234, uuid=str(uuid.uuid4())) - migration = objects.Migration() - migration.status = 'aborted' - migration.id = 0 - - @mock.patch.object(objects.Migration, 'get_by_id', - return_value=migration) - def _do_test(get_by_id): - self.assertRaises(exception.InvalidMigrationState, - self.compute.live_migration_force_complete, - self.context, instance, migration.id) - _do_test() - def test_post_live_migration_at_destination_success(self): @mock.patch.object(self.instance, 'save') diff --git a/nova/tests/unit/compute/test_rpcapi.py b/nova/tests/unit/compute/test_rpcapi.py index 485f22a8a03f..61379f36aab2 100644 --- a/nova/tests/unit/compute/test_rpcapi.py +++ b/nova/tests/unit/compute/test_rpcapi.py @@ -25,6 +25,7 @@ from nova import context from nova import exception from nova.objects import block_device as objects_block_dev from nova.objects import migrate_data as migrate_data_obj +from nova.objects import migration as migration_obj from nova import test from nova.tests.unit import fake_block_device from nova.tests.unit import fake_flavor @@ -305,9 +306,45 @@ class ComputeRpcAPITestCase(test.NoDBTestCase): migrate_data={}, version='4.8') def test_live_migration_force_complete(self): - self._test_compute_api('live_migration_force_complete', 'cast', - instance=self.fake_instance_obj, - migration_id='1', version='4.9') + migration = migration_obj.Migration() + migration.id = 1 + migration.source_compute = 'fake' + ctxt = context.RequestContext('fake_user', 'fake_project') + version = '4.12' + rpcapi = compute_rpcapi.ComputeAPI() + mock_client = mock.MagicMock() + rpcapi.client = mock_client + mock_client.can_send_version.return_value = True + mock_cctx = mock.MagicMock() + mock_client.prepare.return_value = mock_cctx + rpcapi.live_migration_force_complete(ctxt, self.fake_instance_obj, + migration) + mock_client.prepare.assert_called_with(server=migration.source_compute, + version=version) + mock_cctx.cast.assert_called_with(ctxt, + 'live_migration_force_complete', + instance=self.fake_instance_obj) + + def test_live_migration_force_complete_backward_compatibility(self): + migration = migration_obj.Migration() + migration.id = 1 + migration.source_compute = 'fake' + version = '4.9' + ctxt = context.RequestContext('fake_user', 'fake_project') + rpcapi = compute_rpcapi.ComputeAPI() + mock_client = mock.MagicMock() + rpcapi.client = mock_client + mock_client.can_send_version.return_value = False + mock_cctx = mock.MagicMock() + mock_client.prepare.return_value = mock_cctx + rpcapi.live_migration_force_complete(ctxt, self.fake_instance_obj, + migration) + mock_client.prepare.assert_called_with(server=migration.source_compute, + version=version) + mock_cctx.cast.assert_called_with(ctxt, + 'live_migration_force_complete', + instance=self.fake_instance_obj, + migration_id=migration.id) def test_live_migration_abort(self): self._test_compute_api('live_migration_abort', 'cast',