From fbc91183ffa32e881ca5cf15758f5f602401acc0 Mon Sep 17 00:00:00 2001 From: Sivasathurappan Radhakrishnan Date: Wed, 23 Nov 2016 00:17:47 +0000 Subject: [PATCH] Fix host validity check for live-migration When live migrating instance to invalid host, live migration fails with host not found and sets instance task state to migrating. This change handles host validity in API layer before changing instance task_state to 'MIGRATING' and raise proper exception on invalid host. Change-Id: I7c5e80298b9adf1bd53cc6c464a3744b5397b7e8 Related-Bug: #1643623 Closes-Bug: #1785031 (cherry picked from commit c7aed3d139909913387e4ac2e771f19c9502c5b1) (cherry picked from commit 064daf9aeb39a7f977de4989ad096821e7830ff6) --- nova/compute/api.py | 6 ++++- nova/tests/unit/compute/test_compute_api.py | 27 ++++++++++++++++--- nova/tests/unit/compute/test_compute_cells.py | 4 ++- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 2401f0e63a5e..2d660ca7cb6d 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -3950,6 +3950,11 @@ class API(base.Base): LOG.debug("Going to try to live migrate instance to %s", host_name or "another host", instance=instance) + if host_name: + # Validate the specified host before changing the instance task + # state. + nodes = objects.ComputeNodeList.get_all_by_host(context, host_name) + instance.task_state = task_states.MIGRATING instance.save(expected_task_state=[None]) @@ -3965,7 +3970,6 @@ class API(base.Base): # NOTE(sbauza): Force is a boolean by the new related API version if force is False and host_name: - nodes = objects.ComputeNodeList.get_all_by_host(context, host_name) # NOTE(sbauza): Unset the host to make sure we call the scheduler host_name = None # FIXME(sbauza): Since only Ironic driver uses more than one diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 10003de6ffc7..5c15e35a9da3 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -2315,21 +2315,25 @@ class _ComputeAPIUnitTestMixIn(object): self.compute_api.get_instance_diagnostics, self.context, instance) - def test_live_migrate_active_vm_state(self): + @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host') + def test_live_migrate_active_vm_state(self, mock_nodelist): instance = self._create_instance_obj() self._live_migrate_instance(instance) - def test_live_migrate_paused_vm_state(self): + @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host') + def test_live_migrate_paused_vm_state(self, mock_nodelist): paused_state = dict(vm_state=vm_states.PAUSED) instance = self._create_instance_obj(params=paused_state) self._live_migrate_instance(instance) + @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host') @mock.patch.object(compute_utils, 'add_instance_fault_from_exc') @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') @mock.patch.object(objects.InstanceAction, 'action_start') @mock.patch.object(objects.Instance, 'save') def test_live_migrate_messaging_timeout(self, _save, _action, get_spec, - add_instance_fault_from_exc): + add_instance_fault_from_exc, + mock_nodelist): instance = self._create_instance_obj() if self.cell_type == 'api': api = self.compute_api.cells_rpcapi @@ -2348,6 +2352,23 @@ class _ComputeAPIUnitTestMixIn(object): instance, mock.ANY) + @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') + @mock.patch.object(objects.InstanceAction, 'action_start') + @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host', + side_effect=exception.ComputeHostNotFound( + host='fake_host')) + def test_live_migrate_computehost_notfound(self, mock_nodelist, + mock_action, + mock_get_spec): + instance = self._create_instance_obj() + self.assertRaises(exception.ComputeHostNotFound, + self.compute_api.live_migrate, + self.context, instance, + host_name='fake_host', + block_migration='auto', + disk_over_commit=False) + self.assertIsNone(instance.task_state) + @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') @mock.patch.object(objects.Instance, 'save') @mock.patch.object(objects.InstanceAction, 'action_start') diff --git a/nova/tests/unit/compute/test_compute_cells.py b/nova/tests/unit/compute/test_compute_cells.py index a5dc833c52fe..d28a35386d38 100644 --- a/nova/tests/unit/compute/test_compute_cells.py +++ b/nova/tests/unit/compute/test_compute_cells.py @@ -538,10 +538,12 @@ class CellsConductorAPIRPCRedirect(test.NoDBTestCase): self.compute_api.resize(self.context, instance) self.assertTrue(self.cells_rpcapi.resize_instance.called) + @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host') @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') @mock.patch.object(compute_api.API, '_record_action_start') @mock.patch.object(objects.Instance, 'save') - def test_live_migrate_instance(self, instance_save, _record, _get_spec): + def test_live_migrate_instance(self, instance_save, _record, _get_spec, + mock_nodelist): orig_system_metadata = {} instance = fake_instance.fake_instance_obj(self.context, vm_state=vm_states.ACTIVE, cell_name='fake-cell',