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 c7aed3d139)
(cherry picked from commit 064daf9aeb)

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

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

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',