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 984dd8ad6a
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
This commit is contained in:
parent
a4eebd5de7
commit
54407afef3
|
@ -2845,7 +2845,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)
|
||||
|
@ -2877,6 +2876,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)
|
||||
|
||||
|
|
|
@ -81,10 +81,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'])
|
||||
|
|
|
@ -37,6 +37,7 @@ from oslo_utils import fixture as utils_fixture
|
|||
from oslo_utils import timeutils
|
||||
from oslo_utils import units
|
||||
from oslo_utils import uuidutils
|
||||
import six
|
||||
import testtools
|
||||
from testtools import matchers as testtools_matchers
|
||||
|
||||
|
@ -8602,14 +8603,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
|
||||
|
|
|
@ -703,13 +703,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()
|
||||
|
|
Loading…
Reference in New Issue