Merge "[compute] always set instance.host in post_livemigration" into stable/trainstable/train

3 files changed, 68 insertions, 9 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 0e013a3cc5..9185d5fd66 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -7356,8 +7356,9 @@ class ComputeManager(manager.Manager):
         # host attachment. We fetch BDMs before that to retain connection_info
         # and attachment_id relating to the source host for post migration
         # cleanup.
-        post_live_migration = functools.partial(self._post_live_migration,
-                                                source_bdms=source_bdms)
+        post_live_migration = functools.partial(
+            self._post_live_migration_update_host, source_bdms=source_bdms
+            )
         rollback_live_migration = functools.partial(
             self._rollback_live_migration, source_bdms=source_bdms)
 
@@ -7592,6 +7593,42 @@ class ComputeManager(manager.Manager):
                                   bdm.attachment_id, self.host,
                                   six.text_type(e), instance=instance)
 
+    # TODO(sean-k-mooney): add typing
+    def _post_live_migration_update_host(
+        self, ctxt, instance, dest, block_migration=False,
+        migrate_data=None, source_bdms=None
+    ):
+        try:
+            self._post_live_migration(
+                ctxt, instance, dest, block_migration, migrate_data,
+                source_bdms)
+        except Exception:
+            # Restore the instance object
+            node_name = None
+            try:
+                # get node name of compute, where instance will be
+                # running after migration, that is destination host
+                compute_node = self._get_compute_info(ctxt, dest)
+                node_name = compute_node.hypervisor_hostname
+            except exception.ComputeHostNotFound:
+                LOG.exception('Failed to get compute_info for %s', dest)
+
+            # we can never rollback from post live migration and we can only
+            # get here if the instance is running on the dest so we ensure
+            # the instance.host is set correctly and reraise the original
+            # exception unmodified.
+            if instance.host != dest:
+                # apply saves the new fields while drop actually removes the
+                # migration context from the instance, so migration persists.
+                instance.apply_migration_context()
+                instance.drop_migration_context()
+                instance.host = dest
+                instance.task_state = None
+                instance.node = node_name
+                instance.progress = 0
+                instance.save()
+            raise
+
     @wrap_exception()
     @wrap_instance_fault
     def _post_live_migration(self, ctxt, instance, dest,
@@ -7603,7 +7640,7 @@ class ComputeManager(manager.Manager):
         and mainly updating database record.
 
         :param ctxt: security context
-        :param instance: instance dict
+        :param instance: instance object
         :param dest: destination host
         :param block_migration: if true, prepare for block migration
         :param migrate_data: if not None, it is a dict which has data
diff --git a/nova/tests/functional/regressions/test_bug_1628606.py b/nova/tests/functional/regressions/test_bug_1628606.py
index ba51d2e071..b6f7a034b9 100644
--- a/nova/tests/functional/regressions/test_bug_1628606.py
+++ b/nova/tests/functional/regressions/test_bug_1628606.py
@@ -67,6 +67,5 @@ class PostLiveMigrationFail(
         server = self._live_migrate(
             server, migration_expected_state='error',
             server_expected_state='ERROR')
-        # FIXME(amit): this should point to the dest as after migration
-        # but does not because of bug 1628606
-        self.assertEqual(self.src.host, server['OS-EXT-SRV-ATTR:host'])
+
+        self.assertEqual(self.dest.host, server['OS-EXT-SRV-ATTR:host'])
diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py
index c22aa01ffd..ab690c69ed 100644
--- a/nova/tests/unit/compute/test_compute_mgr.py
+++ b/nova/tests/unit/compute/test_compute_mgr.py
@@ -9363,9 +9363,32 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
                                                 self.instance,
                                                 migration)
 
-    def test_post_live_migration_cinder_v3_api(self):
-        # Because live migration has succeeded, _post_live_migration
-        # should call attachment_delete with the original/old attachment_id
+    def test_post_live_migration_update_host(self):
+        @mock.patch.object(self.compute, '_get_compute_info')
+        def _test_post_live_migration(_get_compute_info):
+            dest_host = 'dest'
+            cn = objects.ComputeNode(hypervisor_hostname=dest_host)
+            _get_compute_info.return_value = cn
+            instance = fake_instance.fake_instance_obj(self.context,
+                                                        node='src',
+                                                        uuid=uuids.instance)
+            instance.migration_context = None
+            with mock.patch.object(self.compute, "_post_live_migration"
+                    ) as plm, mock.patch.object(instance, "save") as save:
+                error = ValueError("some failure")
+                plm.side_effect = error
+                self.assertRaises(
+                    ValueError, self.compute._post_live_migration_update_host,
+                    self.context, instance, dest_host)
+                save.assert_called_once()
+                self.assertEqual(instance.host, dest_host)
+
+        _test_post_live_migration()
+
+    def test_post_live_migration_cinder_pre_344_api(self):
+        # Because live migration has
+        # succeeded,_post_live_migration_remove_source_vol_connections()
+        # should call terminate_connection() with the volume UUID.
         dest_host = 'test_dest_host'
         instance = fake_instance.fake_instance_obj(self.context,
                                                    node='dest',