From 89b1fecce116bc44f558e76cbb5dc43497ea67cc Mon Sep 17 00:00:00 2001 From: Pawel Koniszewski Date: Mon, 1 Feb 2016 11:56:59 +0100 Subject: [PATCH] Update instance host in post live migration even when exception occurs Currently when, e.g., port binding fails on destination host nova loses track of running VM. Operator needs to change record in DB manually in order to recover VM in nova and then perform operations on destination host to repair such VM. Because VM is on destination host already it should be updated regardless of post live migration at destination result. Change-Id: Ibb5158f453abd9717e6d2ab501295351ca9d0dcf Closes-Bug: #1379581 --- nova/compute/manager.py | 40 ++++--- nova/tests/unit/compute/test_compute.py | 78 ------------- nova/tests/unit/compute/test_compute_mgr.py | 117 ++++++++++++++++++++ 3 files changed, 141 insertions(+), 94 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 0d5e41355d..c0fce9503b 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -5465,24 +5465,32 @@ class ComputeManager(manager.Manager): block_device_info = self._get_instance_block_device_info(context, instance) - self.driver.post_live_migration_at_destination(context, instance, - network_info, - block_migration, block_device_info) - # Restore instance state - current_power_state = self._get_power_state(context, instance) - node_name = None - prev_host = instance.host try: - compute_node = self._get_compute_info(context, self.host) - node_name = compute_node.hypervisor_hostname - except exception.ComputeHostNotFound: - LOG.exception(_LE('Failed to get compute_info for %s'), self.host) + self.driver.post_live_migration_at_destination( + context, instance, network_info, block_migration, + block_device_info) + except Exception: + with excutils.save_and_reraise_exception(): + instance.vm_state = vm_states.ERROR + LOG.error(_LE('Unexpected error during post live migration at ' + 'destination host.'), instance=instance) finally: - instance.host = self.host - instance.power_state = current_power_state - instance.task_state = None - instance.node = node_name - instance.save(expected_task_state=task_states.MIGRATING) + # Restore instance state and update host + current_power_state = self._get_power_state(context, instance) + node_name = None + prev_host = instance.host + try: + compute_node = self._get_compute_info(context, self.host) + node_name = compute_node.hypervisor_hostname + except exception.ComputeHostNotFound: + LOG.exception(_LE('Failed to get compute_info for %s'), + self.host) + finally: + instance.host = self.host + instance.power_state = current_power_state + instance.task_state = None + instance.node = node_name + instance.save(expected_task_state=task_states.MIGRATING) # NOTE(tr3buchet): tear down networks on source host self.network_api.setup_networks_on_host(context, instance, diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 19c746d1ce..1004213913 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -5840,84 +5840,6 @@ class ComputeTestCase(BaseTestCase): terminate_connection.assert_called_once_with( c, uuids.volume_id, 'fake-connector') - def _begin_post_live_migration_at_destination(self): - self.mox.StubOutWithMock(self.compute.network_api, - 'setup_networks_on_host') - self.mox.StubOutWithMock(self.compute.network_api, - 'migrate_instance_finish') - self.mox.StubOutWithMock(self.compute, '_get_power_state') - self.mox.StubOutWithMock(self.compute, '_get_compute_info') - - params = {'task_state': task_states.MIGRATING, - 'power_state': power_state.PAUSED, } - self.instance = self._create_fake_instance_obj(params) - - self.admin_ctxt = context.get_admin_context() - - self.compute.network_api.setup_networks_on_host(self.admin_ctxt, - self.instance, - self.compute.host) - migration = {'source_compute': self.instance['host'], - 'dest_compute': self.compute.host, } - self.compute.network_api.migrate_instance_finish( - self.admin_ctxt, self.instance, migration) - fake_net_info = [] - fake_block_dev_info = {'foo': 'bar'} - self.compute.driver.post_live_migration_at_destination(self.admin_ctxt, - self.instance, - fake_net_info, - False, - fake_block_dev_info) - self.compute._get_power_state(self.admin_ctxt, - self.instance).AndReturn(10001) - - def _finish_post_live_migration_at_destination(self): - self.compute.network_api.setup_networks_on_host(self.admin_ctxt, - mox.IgnoreArg(), mox.IgnoreArg(), teardown=True) - self.compute.network_api.setup_networks_on_host(self.admin_ctxt, - mox.IgnoreArg(), self.compute.host) - - fake_notifier.NOTIFICATIONS = [] - self.mox.ReplayAll() - - self.compute.post_live_migration_at_destination(self.admin_ctxt, - self.instance, False) - - self.assertEqual(len(fake_notifier.NOTIFICATIONS), 2) - msg = fake_notifier.NOTIFICATIONS[0] - self.assertEqual(msg.event_type, - 'compute.instance.live_migration.post.dest.start') - msg = fake_notifier.NOTIFICATIONS[1] - self.assertEqual(msg.event_type, - 'compute.instance.live_migration.post.dest.end') - - return objects.Instance.get_by_uuid(self.admin_ctxt, - self.instance['uuid']) - - def test_post_live_migration_at_destination_with_compute_info(self): - """The instance's node property should be updated correctly.""" - self._begin_post_live_migration_at_destination() - hypervisor_hostname = 'fake_hypervisor_hostname' - fake_compute_info = objects.ComputeNode( - hypervisor_hostname=hypervisor_hostname) - self.compute._get_compute_info(mox.IgnoreArg(), - mox.IgnoreArg()).AndReturn( - fake_compute_info) - updated = self._finish_post_live_migration_at_destination() - self.assertEqual(updated['node'], hypervisor_hostname) - - def test_post_live_migration_at_destination_without_compute_info(self): - """The instance's node property should be set to None if we fail to - get compute_info. - """ - self._begin_post_live_migration_at_destination() - self.compute._get_compute_info( - mox.IgnoreArg(), - mox.IgnoreArg()).AndRaise( - exception.ComputeHostNotFound(host='fake-host')) - updated = self._finish_post_live_migration_at_destination() - self.assertIsNone(updated['node']) - @mock.patch('nova.objects.BlockDeviceMappingList.get_by_instance_uuid') def test_rollback_live_migration(self, mock_bdms): c = context.get_admin_context() diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 68cf96ffc9..daf9352a90 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -4402,4 +4402,121 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): self.compute.live_migration_force_complete, self.context, instance, migration.id) + def test_post_live_migration_at_destination_success(self): + + @mock.patch.object(self.instance, 'save') + @mock.patch.object(self.compute.network_api, 'get_instance_nw_info', + return_value='test_network') + @mock.patch.object(self.compute.network_api, 'setup_networks_on_host') + @mock.patch.object(self.compute.network_api, 'migrate_instance_finish') + @mock.patch.object(self.compute, '_notify_about_instance_usage') + @mock.patch.object(self.compute, '_get_instance_block_device_info') + @mock.patch.object(self.compute, '_get_power_state', return_value=1) + @mock.patch.object(self.compute, '_get_compute_info') + @mock.patch.object(self.compute.driver, + 'post_live_migration_at_destination') + def _do_test(post_live_migration_at_destination, _get_compute_info, + _get_power_state, _get_instance_block_device_info, + _notify_about_instance_usage, migrate_instance_finish, + setup_networks_on_host, get_instance_nw_info, save): + + cn = mock.Mock(spec_set=['hypervisor_hostname']) + cn.hypervisor_hostname = 'test_host' + _get_compute_info.return_value = cn + cn_old = self.instance.host + instance_old = self.instance + + self.compute.post_live_migration_at_destination( + self.context, self.instance, False) + + setup_networks_calls = [ + mock.call(self.context, self.instance, self.compute.host), + mock.call(self.context, self.instance, cn_old, teardown=True), + mock.call(self.context, self.instance, self.compute.host) + ] + setup_networks_on_host.assert_has_calls(setup_networks_calls) + + notify_usage_calls = [ + mock.call(self.context, instance_old, + "live_migration.post.dest.start", + network_info='test_network'), + mock.call(self.context, self.instance, + "live_migration.post.dest.end", + network_info='test_network') + ] + _notify_about_instance_usage.assert_has_calls(notify_usage_calls) + + migrate_instance_finish.assert_called_once_with( + self.context, self.instance, + {'source_compute': cn_old, + 'dest_compute': self.compute.host}) + _get_instance_block_device_info.assert_called_once_with( + self.context, self.instance + ) + get_instance_nw_info.assert_called_once_with(self.context, + self.instance) + _get_power_state.assert_called_once_with(self.context, + self.instance) + _get_compute_info.assert_called_once_with(self.context, + self.compute.host) + + self.assertEqual(self.compute.host, self.instance.host) + self.assertEqual('test_host', self.instance.node) + self.assertEqual(1, self.instance.power_state) + self.assertIsNone(self.instance.task_state) + save.assert_called_once_with( + expected_task_state=task_states.MIGRATING) + + _do_test() + + def test_post_live_migration_at_destination_compute_not_found(self): + + @mock.patch.object(self.instance, 'save') + @mock.patch.object(self.compute, 'network_api') + @mock.patch.object(self.compute, '_notify_about_instance_usage') + @mock.patch.object(self.compute, '_get_instance_block_device_info') + @mock.patch.object(self.compute, '_get_power_state', return_value=1) + @mock.patch.object(self.compute, '_get_compute_info', + side_effect=exception.ComputeHostNotFound( + host='fake')) + @mock.patch.object(self.compute.driver, + 'post_live_migration_at_destination') + def _do_test(post_live_migration_at_destination, _get_compute_info, + _get_power_state, _get_instance_block_device_info, + _notify_about_instance_usage, network_api, save): + cn = mock.Mock(spec_set=['hypervisor_hostname']) + cn.hypervisor_hostname = 'test_host' + _get_compute_info.return_value = cn + + self.compute.post_live_migration_at_destination( + self.context, self.instance, False) + self.assertIsNone(self.instance.node) + + _do_test() + + def test_post_live_migration_at_destination_unexpected_exception(self): + + @mock.patch.object(compute_utils, 'add_instance_fault_from_exc') + @mock.patch.object(self.instance, 'save') + @mock.patch.object(self.compute, 'network_api') + @mock.patch.object(self.compute, '_notify_about_instance_usage') + @mock.patch.object(self.compute, '_get_instance_block_device_info') + @mock.patch.object(self.compute, '_get_power_state', return_value=1) + @mock.patch.object(self.compute, '_get_compute_info') + @mock.patch.object(self.compute.driver, + 'post_live_migration_at_destination', + side_effect=exception.NovaException) + def _do_test(post_live_migration_at_destination, _get_compute_info, + _get_power_state, _get_instance_block_device_info, + _notify_about_instance_usage, network_api, save, + add_instance_fault_from_exc): + cn = mock.Mock(spec_set=['hypervisor_hostname']) + cn.hypervisor_hostname = 'test_host' + _get_compute_info.return_value = cn + + self.assertRaises(exception.NovaException, + self.compute.post_live_migration_at_destination, + self.context, self.instance, False) + self.assertEqual(vm_states.ERROR, self.instance.vm_state) + _do_test()