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

Author: Zuul

nova/compute/manager.py

@@ -8229,8 +8229,9 @@ def _do_live_migration(self, context, dest, instance, block_migration,
         # 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)
 
@@ -8479,6 +8480,42 @@ def _post_live_migration_remove_source_vol_connections(
                                   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,
@@ -8490,7 +8527,7 @@ def _post_live_migration(self, ctxt, instance, dest,
         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

nova/tests/functional/regressions/test_bug_1628606.py

@@ -57,6 +57,5 @@ def test_post_live_migration(self, mock_migration):
         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'])

nova/tests/unit/compute/test_compute_mgr.py

@@ -9741,6 +9741,27 @@ def test_post_live_migration_new_allocations(self):
                                                 self.instance,
                                                 migration)
 
+    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)
+            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()