From cacfd372acb4eb056c4391db3c988bfe91c957df Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Fri, 17 Nov 2017 13:17:16 -0500 Subject: [PATCH] Add regression test for rebuild with new image doubling allocations Commit 984dd8ad6add4523d93c7ce5a666a32233e02e34 makes a rebuild with a new image go through the scheduler again to validate the image against the instance.host (we rebuild to the same host that the instance already lives on). The problem is that change introduced a regression where the FilterScheduler is going to think it's doing a resize to the same host and double the allocations for the instance (and usage for the compute node provider) in Placement, which is wrong since the flavor is the same. This adds a regression test to show the bug. Change-Id: Ie0949b4e6101f0b29ec4542146d523a07a683991 Related-Bug: #1664931 --- nova/tests/functional/integrated_helpers.py | 3 +- nova/tests/functional/test_servers.py | 95 +++++++++++++++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/nova/tests/functional/integrated_helpers.py b/nova/tests/functional/integrated_helpers.py index 50fe6f234581..8e272d03e996 100644 --- a/nova/tests/functional/integrated_helpers.py +++ b/nova/tests/functional/integrated_helpers.py @@ -79,7 +79,8 @@ class _IntegratedTestBase(test.TestCase): nova.tests.unit.image.fake.stub_out_image_service(self) self.useFixture(cast_as_call.CastAsCall(self)) - self.useFixture(nova_fixtures.PlacementFixture()) + placement = self.useFixture(nova_fixtures.PlacementFixture()) + self.placement_api = placement.api self._setup_services() diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index 4d9c740a6d06..b066d0ce4033 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -1141,6 +1141,101 @@ class ServerRebuildTestCase(integrated_helpers._IntegratedTestBase, server = self.api.get_server(server['id']) self.assertEqual(rebuild_image_ref, server['image']['id']) + def test_rebuild_with_new_image(self): + """Rebuilds a server with a different image which will run it through + the scheduler to validate the image is still OK with the compute host + that the instance is running on. + + Validates that additional resources are not allocated against the + instance.host in Placement due to the rebuild on same host. + """ + admin_api = self.api_fixture.admin_api + admin_api.microversion = '2.53' + + def _get_provider_uuid_by_host(host): + resp = admin_api.api_get( + 'os-hypervisors?hypervisor_hostname_pattern=%s' % host).body + return resp['hypervisors'][0]['id'] + + def _get_provider_usages(provider_uuid): + return self.placement_api.get( + '/resource_providers/%s/usages' % provider_uuid).body['usages'] + + def _get_allocations_by_server_uuid(server_uuid): + return self.placement_api.get( + '/allocations/%s' % server_uuid).body['allocations'] + + def assertFlavorMatchesAllocation(flavor, allocation): + self.assertEqual(flavor['vcpus'], allocation['VCPU']) + self.assertEqual(flavor['ram'], allocation['MEMORY_MB']) + self.assertEqual(flavor['disk'], allocation['DISK_GB']) + + def assertFlavorsMatchAllocation(old_flavor, new_flavor, + allocation): + self.assertEqual(old_flavor['vcpus'] + new_flavor['vcpus'], + allocation['VCPU']) + self.assertEqual(old_flavor['ram'] + new_flavor['ram'], + allocation['MEMORY_MB']) + self.assertEqual(old_flavor['disk'] + new_flavor['disk'], + allocation['DISK_GB']) + + nodename = self.compute.manager._get_nodename(None) + rp_uuid = _get_provider_uuid_by_host(nodename) + # make sure we start with no usage on the compute node + rp_usages = _get_provider_usages(rp_uuid) + self.assertEqual({'VCPU': 0, 'MEMORY_MB': 0, 'DISK_GB': 0}, rp_usages) + + server_req_body = { + 'server': { + # We hard-code from a fake image since we can't get images + # via the compute /images proxy API with microversion > 2.35. + 'imageRef': '155d900f-4e14-4e4c-a73d-069cbf4541e6', + 'flavorRef': '1', # m1.tiny from DefaultFlavorsFixture, + 'name': 'test_rebuild_with_new_image', + # We don't care about networking for this test. This requires + # microversion >= 2.37. + 'networks': 'none' + } + } + server = self.api.post_server(server_req_body) + self._wait_for_state_change(self.api, server, 'ACTIVE') + + flavor = self.api.api_get('/flavors/1').body['flavor'] + + # There should be usage for the server on the compute node now. + rp_usages = _get_provider_usages(rp_uuid) + assertFlavorMatchesAllocation(flavor, rp_usages) + allocs = _get_allocations_by_server_uuid(server['id']) + self.assertIn(rp_uuid, allocs) + allocs = allocs[rp_uuid]['resources'] + assertFlavorMatchesAllocation(flavor, allocs) + + rebuild_image_ref = ( + nova.tests.unit.image.fake.AUTO_DISK_CONFIG_ENABLED_IMAGE_UUID) + # Now rebuild the server with a different image. + rebuild_req_body = { + 'rebuild': { + 'imageRef': rebuild_image_ref + } + } + self.api.api_post('/servers/%s/action' % server['id'], + rebuild_req_body) + self._wait_for_server_parameter( + self.api, server, {'OS-EXT-STS:task_state': None}) + + # The usage and allocations should not have changed. + rp_usages = _get_provider_usages(rp_uuid) + # FIXME(mriedem): This is a bug where the scheduler doubled up the + # allocations for the instance even though we're just rebuilding + # to the same host. Uncomment this once fixed. + # assertFlavorMatchesAllocation(flavor, rp_usages) + assertFlavorsMatchAllocation(flavor, flavor, rp_usages) + allocs = _get_allocations_by_server_uuid(server['id']) + self.assertIn(rp_uuid, allocs) + allocs = allocs[rp_uuid]['resources'] + # assertFlavorMatchesAllocation(flavor, allocs) + assertFlavorsMatchAllocation(flavor, flavor, allocs) + class ProviderUsageBaseTestCase(test.TestCase, integrated_helpers.InstanceHelperMixin):