From 1342cd75e9a091d28bec857f44f833ab5a7d1b96 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Wed, 13 Mar 2019 16:20:47 -0400 Subject: [PATCH] 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. Conflicts: nova/compute/api.py nova/tests/unit/compute/test_compute_api.py NOTE(mriedem): The conflict is due to not having change I34ffaf285718059b55f90e812b57f1e11d566c6f in Rocky. Change-Id: I8dc862b90d398b693b259abd3583616d07d8d206 Closes-Bug: #1819963 (cherry picked from commit 40f6672f53794b563f4c7e27ede7b59a1d63c14a) (cherry picked from commit 26e59912838b145b627aa247c45b0b19393466c0) --- nova/compute/api.py | 13 +++++++++++++ nova/tests/functional/test_availability_zones.py | 8 +------- nova/tests/unit/compute/test_compute_api.py | 9 +++++++-- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index e4388def8365..22dddaa38053 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -3336,6 +3336,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 bb69db027306..23a83c607f2c 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 3ef273db90ac..21ba2ccb8f1d 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -1831,12 +1831,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 @@ -1879,11 +1881,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