Update instance.availability_zone on revertResize

When resizing a server that was not created in an explicit
zone, the scheduler can pick a host in another zone and
conductor will update the instance.availability_zone value
for the new dest host zone.

The problem is when reverting the resize, the server goes
back to the original source host/zone but the
instance.availability_zone value in the database is not
updated which can lead to incorrect results when listing
servers and filtering by zone.

This fixes the bug by updating the instance.availability_zone
value in the API (where we have access to the aggregates
table in the API DB) before casting to nova-compute to
complete the revert. As noted in the comment within, this
is not fail-safe in case the revert fails before the
instance.host is updated in finish_revert_resize, but we
don't have a lot of great backportable options here that
don't involve "up-calls" from the compute to the API DB.

Change-Id: I8dc862b90d398b693b259abd3583616d07d8d206
Closes-Bug: #1819963
(cherry picked from commit 40f6672f53794b563f4c7e27ede7b59a1d63c14a)
(cherry picked from commit 26e59912838b145b627aa247c45b0b19393466c0)
(cherry picked from commit 1342cd75e9a091d28bec857f44f833ab5a7d1b96)
(cherry picked from commit 99cbbcfd61c3569b9c299c99e4a2eaf38d335d19)

3 files changed, 21 insertions, 9 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index f27b7ad355..5cb58d8416 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -3188,6 +3188,19 @@ class API(base.Base):
         self._check_quota_for_upsize(context, instance, instance.flavor,
                                      instance.old_flavor)
 
+        # The AZ for the server may have changed when it was migrated so while
+        # we are in the API and have access to the API DB, update the
+        # instance.availability_zone before casting off to the compute service.
+        # Note that we do this in the API to avoid an "up-call" from the
+        # compute service to the API DB. This is not great in case something
+        # fails during revert before the instance.host is updated to the
+        # original source host, but it is good enough for now. Long-term we
+        # could consider passing the AZ down to compute so it can set it when
+        # the instance.host value is set in finish_revert_resize.
+        instance.availability_zone = (
+            availability_zones.get_host_availability_zone(
+                context, migration.source_compute))
+
         instance.task_state = task_states.RESIZE_REVERTING
         instance.save(expected_task_state=[None])
 
diff --git a/nova/tests/functional/test_availability_zones.py b/nova/tests/functional/test_availability_zones.py
index f4a5337af7..afdd5bc2af 100644
--- a/nova/tests/functional/test_availability_zones.py
+++ b/nova/tests/functional/test_availability_zones.py
@@ -164,10 +164,4 @@ class TestAvailabilityZoneScheduling(
         # Revert the resize and the server should be back in the original AZ.
         self.api.post_server_action(server['id'], {'revertResize': None})
         server = self._wait_for_state_change(self.api, server, 'ACTIVE')
-        # FIXME(mriedem): This is bug 1819963 where the API will show the
-        # source host AZ but the instance.availability_zone value in the DB
-        # is still wrong because nothing updated it on revert. Remove the
-        # explicit API check and uncomment the _assert_instance_az call when
-        # the bug is fixed.
-        self.assertEqual(original_az, server['OS-EXT-AZ:availability_zone'])
-        # self._assert_instance_az(server, original_az)
+        self._assert_instance_az(server, original_az)
diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py
index 865297e951..835283a67a 100644
--- a/nova/tests/unit/compute/test_compute_api.py
+++ b/nova/tests/unit/compute/test_compute_api.py
@@ -1655,12 +1655,14 @@ class _ComputeAPIUnitTestMixIn(object):
     def test_confirm_resize_with_migration_ref(self):
         self._test_confirm_resize(mig_ref_passed=True)
 
+    @mock.patch('nova.availability_zones.get_host_availability_zone',
+                return_value='nova')
     @mock.patch('nova.objects.Quotas.check_deltas')
     @mock.patch('nova.objects.Migration.get_by_instance_and_status')
     @mock.patch('nova.context.RequestContext.elevated')
     @mock.patch('nova.objects.RequestSpec.get_by_instance_uuid')
     def _test_revert_resize(self, mock_get_reqspec, mock_elevated,
-                            mock_get_migration, mock_check):
+                            mock_get_migration, mock_check, mock_get_host_az):
         params = dict(vm_state=vm_states.RESIZED)
         fake_inst = self._create_instance_obj(params=params)
         fake_inst.old_flavor = fake_inst.flavor
@@ -1703,11 +1705,14 @@ class _ComputeAPIUnitTestMixIn(object):
     def test_revert_resize(self):
         self._test_revert_resize()
 
+    @mock.patch('nova.availability_zones.get_host_availability_zone',
+                return_value='nova')
     @mock.patch('nova.objects.Quotas.check_deltas')
     @mock.patch('nova.objects.Migration.get_by_instance_and_status')
     @mock.patch('nova.context.RequestContext.elevated')
     def test_revert_resize_concurrent_fail(self, mock_elevated,
-                                           mock_get_migration, mock_check):
+                                           mock_get_migration, mock_check,
+                                           mock_get_host_az):
         params = dict(vm_state=vm_states.RESIZED)
         fake_inst = self._create_instance_obj(params=params)
         fake_inst.old_flavor = fake_inst.flavor