From 24953a95e5989f44acaf488c253230e476c2fa59 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Sun, 19 Nov 2017 17:25:56 -0500 Subject: [PATCH] Get original image_id from volume for volume-backed instance rebuild A volume-backed instance will not have the instance.image_ref attribute set, to indicate to the API user that it is a volume-backed instance. Commit 984dd8ad6add4523d93c7ce5a666a32233e02e34 missed this subtle difference with how instance.image_ref is used, which means a rebuild of any volume-backed instance will now run through the scheduler, even if the image_href passed to rebuild is the same image ID as for the root volume. This fixes that case in rebuild by getting the image metadata off the root volume for a volume-backed instance and compares that to the image_href passed to rebuild. Change-Id: I48cda813b9effa37f6c3e0cd2e8a22bb78c79d72 Closes-Bug: #1732947 (cherry picked from commit 54407afef3b446e34c44ba5392ccbf121e266de2) (cherry picked from commit 4e36c4bd0eab2be619991ae00454906e171720b6) (cherry picked from commit 10bc4fdbc568670dfd8bec109e8b854ba8ad7949) --- nova/compute/api.py | 24 ++++++++++++++++++- .../regressions/test_bug_1732947.py | 12 ++++------ nova/tests/unit/compute/test_compute.py | 15 ++++++++---- nova/tests/unit/compute/test_compute_cells.py | 9 ++++--- 4 files changed, 44 insertions(+), 16 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 2d5464403960..6eb269d3033f 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2742,7 +2742,6 @@ class API(base.Base): def rebuild(self, context, instance, image_href, admin_password, files_to_inject=None, **kwargs): """Rebuild the given instance with the provided attributes.""" - orig_image_ref = instance.image_ref or '' files_to_inject = files_to_inject or [] metadata = kwargs.get('metadata', {}) preserve_ephemeral = kwargs.get('preserve_ephemeral', False) @@ -2755,6 +2754,29 @@ class API(base.Base): bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( context, instance.uuid) root_bdm = compute_utils.get_root_bdm(context, instance, bdms) + + # Check to see if the image is changing and we have a volume-backed + # server. + is_volume_backed = compute_utils.is_volume_backed_instance( + context, instance, bdms) + if is_volume_backed: + # For boot from volume, instance.image_ref is empty, so we need to + # query the image from the volume. + if root_bdm is None: + # This shouldn't happen and is an error, we need to fail. This + # is not the users fault, it's an internal error. Without a + # root BDM we have no way of knowing the backing volume (or + # image in that volume) for this instance. + raise exception.NovaException( + _('Unable to find root block device mapping for ' + 'volume-backed instance.')) + + volume = self.volume_api.get(context, root_bdm.volume_id) + volume_image_metadata = volume.get('volume_image_metadata', {}) + orig_image_ref = volume_image_metadata.get('image_id') + else: + orig_image_ref = instance.image_ref + self._checks_for_create_and_rebuild(context, image_id, image, flavor, metadata, files_to_inject, root_bdm) diff --git a/nova/tests/functional/regressions/test_bug_1732947.py b/nova/tests/functional/regressions/test_bug_1732947.py index f9be2ec6b285..a23b349d0a7d 100644 --- a/nova/tests/functional/regressions/test_bug_1732947.py +++ b/nova/tests/functional/regressions/test_bug_1732947.py @@ -83,10 +83,8 @@ class RebuildVolumeBackedSameImage(integrated_helpers._IntegratedTestBase, 'imageRef': '155d900f-4e14-4e4c-a73d-069cbf4541e6' } } - # Since we're using the CastAsCall fixture, the NoValidHost error - # should actually come back to the API and result in a 500 error. - # Normally the user would get a 202 response because nova-api RPC casts - # to nova-conductor which RPC calls the scheduler which raises the - # NoValidHost. - self.api.api_post('/servers/%s/action' % server['id'], - rebuild_req_body, check_response_status=[500]) + server = self.api.api_post('/servers/%s/action' % server['id'], + rebuild_req_body).body['server'] + # FIXME(mriedem): Once bug 1482040 is fixed, the server image ref + # should still be blank for a volume-backed server after the rebuild. + self.assertNotEqual('', server['image']) diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 7d13b7688aa2..4f44c488667c 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -8135,14 +8135,19 @@ class ComputeAPITestCase(BaseTestCase): "new password") def test_rebuild_no_image(self): + """Tests that rebuild fails if no root BDM is found for an instance + without an image_ref (volume-backed instance). + """ instance = self._create_fake_instance_obj(params={'image_ref': ''}) - instance_uuid = instance.uuid self.stub_out('nova.tests.unit.image.fake._FakeImageService.show', self.fake_show) - self.compute_api.rebuild(self.context, instance, '', 'new_password') - - instance = db.instance_get_by_uuid(self.context, instance_uuid) - self.assertEqual(instance['task_state'], task_states.REBUILDING) + # The API request schema validates that a UUID is passed for the + # imageRef parameter so we need to provide an image. + ex = self.assertRaises(exception.NovaException, + self.compute_api.rebuild, self.context, + instance, self.fake_image['id'], 'new_password') + self.assertIn('Unable to find root block device mapping for ' + 'volume-backed instance', six.text_type(ex)) def test_rebuild_with_deleted_image(self): # If we're given a deleted image by glance, we should not be able to diff --git a/nova/tests/unit/compute/test_compute_cells.py b/nova/tests/unit/compute/test_compute_cells.py index 8b5074397800..3260a800f0f3 100644 --- a/nova/tests/unit/compute/test_compute_cells.py +++ b/nova/tests/unit/compute/test_compute_cells.py @@ -511,13 +511,16 @@ class CellsConductorAPIRPCRedirect(test.NoDBTestCase): orig_system_metadata = {} instance = fake_instance.fake_instance_obj(self.context, vm_state=vm_states.ACTIVE, cell_name='fake-cell', - launched_at=timeutils.utcnow(), + launched_at=timeutils.utcnow(), image_ref=uuids.image_id, system_metadata=orig_system_metadata, expected_attrs=['system_metadata']) get_flavor.return_value = '' - image_href = '' + # The API request schema validates that a UUID is passed for the + # imageRef parameter so we need to provide an image. + image_href = uuids.image_id image = {"min_ram": 10, "min_disk": 1, - "properties": {'architecture': 'x86_64'}} + "properties": {'architecture': 'x86_64'}, + "id": uuids.image_id} admin_pass = '' files_to_inject = [] bdms = objects.BlockDeviceMappingList()