From 33cab70e1c9584d8253a47e49f057381265f1817 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Mon, 5 Nov 2018 15:33:13 -0500 Subject: [PATCH] Add functional test to delete a server while in VERIFY_RESIZE This scenario came up while discussing what might be causing leaked resource allocations in Placement [1]. I fully expected the new test to fail because I couldn't see from the compute manager where the migration-based allocations would be cleaned up when deleting the server. It turns out that when deleting a VERIFY_RESIZE server, the API confirms the resize which drops the migration-based allocations on the source node before deleting the server on the target node. Since this is not obvious, a comment in the compute API _confirm_resize_on_deleting() method is added. [1] http://lists.openstack.org/pipermail/openstack-dev/2018-November/136295.html Change-Id: I4c12502c86c7ac27369d119e0f97768cf41695b5 --- nova/compute/api.py | 4 +++- nova/tests/functional/integrated_helpers.py | 23 +++++++++++++++++++++ nova/tests/functional/test_servers.py | 22 ++++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 99344370626e..519bf9fd7f38 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2134,7 +2134,9 @@ class API(base.Base): def _confirm_resize_on_deleting(self, context, instance): # If in the middle of a resize, use confirm_resize to - # ensure the original instance is cleaned up too + # ensure the original instance is cleaned up too along + # with its allocations (and migration-based allocations) + # in placement. migration = None for status in ('finished', 'confirming'): try: diff --git a/nova/tests/functional/integrated_helpers.py b/nova/tests/functional/integrated_helpers.py index 5d54a19abad8..a5f1d65e7548 100644 --- a/nova/tests/functional/integrated_helpers.py +++ b/nova/tests/functional/integrated_helpers.py @@ -40,6 +40,7 @@ from nova.tests.unit import cast_as_call from nova.tests.unit import fake_notifier import nova.tests.unit.image.fake from nova.tests.unit import policy_fixture +from nova import utils from nova.virt import fake @@ -714,9 +715,26 @@ class ProviderUsageBaseTestCase(test.TestCase, InstanceHelperMixin): def _delete_and_check_allocations(self, server): """Delete the instance and asserts that the allocations are cleaned + If the server was moved (resized or live migrated), also checks that + migration-based allocations are also cleaned up. + :param server: The API representation of the instance to be deleted """ + # First check to see if there is a related migration record so we can + # assert its allocations (if any) are not leaked. + with utils.temporary_mutation(self.admin_api, microversion='2.59'): + migrations = self.admin_api.api_get( + '/os-migrations?instance_uuid=%s' % + server['id']).body['migrations'] + if migrations: + # If there is more than one migration, they are sorted by + # created_at in descending order so we'll get the last one + # which is probably what we'd always want anyway. + migration_uuid = migrations[0]['uuid'] + else: + migration_uuid = None + self.api.delete_server(server['id']) self._wait_until_deleted(server) # NOTE(gibi): The resource allocation is deleted after the instance is @@ -737,6 +755,11 @@ class ProviderUsageBaseTestCase(test.TestCase, InstanceHelperMixin): allocations = self._get_allocations_by_server_uuid(server['id']) self.assertEqual(0, len(allocations)) + if migration_uuid: + # and no allocations for the delete migration + allocations = self._get_allocations_by_server_uuid(migration_uuid) + self.assertEqual(0, len(allocations)) + def _run_periodics(self): """Run the update_available_resource task on every compute manager diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index 3d9c25eeda16..851b78d81aff 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -2688,6 +2688,28 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase): self._delete_and_check_allocations(server) + def test_resize_delete_while_verify(self): + """Test scenario where the server is deleted while in the + VERIFY_RESIZE state and ensures the allocations are properly + cleaned up from the source and target compute node resource providers. + The _confirm_resize_on_deleting() method in the API is actually + responsible for making sure the migration-based allocations get + cleaned up by confirming the resize on the source host before deleting + the server from the target host. + """ + dest_hostname = 'host2' + source_hostname = self._other_hostname(dest_hostname) + source_rp_uuid = self._get_provider_uuid_by_host(source_hostname) + dest_rp_uuid = self._get_provider_uuid_by_host(dest_hostname) + + server = self._boot_and_check_allocations(self.flavor1, + source_hostname) + + self._resize_and_check_allocations(server, self.flavor1, self.flavor2, + source_rp_uuid, dest_rp_uuid) + + self._delete_and_check_allocations(server) + def _wait_for_notification_event_type(self, event_type, max_retries=50): retry_counter = 0 while True: