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.


(cherry picked from commit 89b1fecce116bc44f558e76cbb5dc43497ea67cc)

Change-Id: Ibb5158f453abd9717e6d2ab501295351ca9d0dcf
Closes-Bug: #1379581
Conflicts:
	nova/tests/unit/compute/test_compute_mgr.py

3 files changed, 143 insertions, 94 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index faa6602db1..06f0dda062 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -5410,24 +5410,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 bf08f1b33b..2639a758e3 100644
--- a/nova/tests/unit/compute/test_compute.py
+++ b/nova/tests/unit/compute/test_compute.py
@@ -5834,84 +5834,6 @@ class ComputeTestCase(BaseTestCase):
             terminate_connection.assert_called_once_with(
                     c, 'fake-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 33b06709a3..7a9fa89e89 100644
--- a/nova/tests/unit/compute/test_compute_mgr.py
+++ b/nova/tests/unit/compute/test_compute_mgr.py
@@ -4336,3 +4336,122 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase):
         self.assertEqual(0, compute._live_migration_semaphore.balance)
         self.assertIsInstance(compute._live_migration_semaphore,
                               compute_utils.UnlimitedSemaphore)
+
+    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()