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,
|
def rebuild(self, context, instance, image_href, admin_password,
|
||||||
files_to_inject=None, **kwargs):
|
files_to_inject=None, **kwargs):
|
||||||
"""Rebuild the given instance with the provided attributes."""
|
"""Rebuild the given instance with the provided attributes."""
|
||||||
orig_image_ref = instance.image_ref or ''
|
|
||||||
files_to_inject = files_to_inject or []
|
files_to_inject = files_to_inject or []
|
||||||
metadata = kwargs.get('metadata', {})
|
metadata = kwargs.get('metadata', {})
|
||||||
preserve_ephemeral = kwargs.get('preserve_ephemeral', False)
|
preserve_ephemeral = kwargs.get('preserve_ephemeral', False)
|
||||||
|
@ -2877,6 +2876,29 @@ class API(base.Base):
|
||||||
bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
|
bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
|
||||||
context, instance.uuid)
|
context, instance.uuid)
|
||||||
root_bdm = compute_utils.get_root_bdm(context, instance, bdms)
|
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,
|
self._checks_for_create_and_rebuild(context, image_id, image,
|
||||||
flavor, metadata, files_to_inject, root_bdm)
|
flavor, metadata, files_to_inject, root_bdm)
|
||||||
|
|
||||||
|
|
|
@ -81,10 +81,8 @@ class RebuildVolumeBackedSameImage(integrated_helpers._IntegratedTestBase,
|
||||||
'imageRef': '155d900f-4e14-4e4c-a73d-069cbf4541e6'
|
'imageRef': '155d900f-4e14-4e4c-a73d-069cbf4541e6'
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
# Since we're using the CastAsCall fixture, the NoValidHost error
|
server = self.api.api_post('/servers/%s/action' % server['id'],
|
||||||
# should actually come back to the API and result in a 500 error.
|
rebuild_req_body).body['server']
|
||||||
# Normally the user would get a 202 response because nova-api RPC casts
|
# FIXME(mriedem): Once bug 1482040 is fixed, the server image ref
|
||||||
# to nova-conductor which RPC calls the scheduler which raises the
|
# should still be blank for a volume-backed server after the rebuild.
|
||||||
# NoValidHost.
|
self.assertNotEqual('', server['image'])
|
||||||
self.api.api_post('/servers/%s/action' % server['id'],
|
|
||||||
rebuild_req_body, check_response_status=[500])
|
|
||||||
|
|
|
@ -37,6 +37,7 @@ from oslo_utils import fixture as utils_fixture
|
||||||
from oslo_utils import timeutils
|
from oslo_utils import timeutils
|
||||||
from oslo_utils import units
|
from oslo_utils import units
|
||||||
from oslo_utils import uuidutils
|
from oslo_utils import uuidutils
|
||||||
|
import six
|
||||||
import testtools
|
import testtools
|
||||||
from testtools import matchers as testtools_matchers
|
from testtools import matchers as testtools_matchers
|
||||||
|
|
||||||
|
@ -8602,14 +8603,19 @@ class ComputeAPITestCase(BaseTestCase):
|
||||||
"new password")
|
"new password")
|
||||||
|
|
||||||
def test_rebuild_no_image(self):
|
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 = self._create_fake_instance_obj(params={'image_ref': ''})
|
||||||
instance_uuid = instance.uuid
|
|
||||||
self.stub_out('nova.tests.unit.image.fake._FakeImageService.show',
|
self.stub_out('nova.tests.unit.image.fake._FakeImageService.show',
|
||||||
self.fake_show)
|
self.fake_show)
|
||||||
self.compute_api.rebuild(self.context, instance, '', 'new_password')
|
# The API request schema validates that a UUID is passed for the
|
||||||
|
# imageRef parameter so we need to provide an image.
|
||||||
instance = db.instance_get_by_uuid(self.context, instance_uuid)
|
ex = self.assertRaises(exception.NovaException,
|
||||||
self.assertEqual(instance['task_state'], task_states.REBUILDING)
|
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):
|
def test_rebuild_with_deleted_image(self):
|
||||||
# If we're given a deleted image by glance, we should not be able to
|
# 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 = {}
|
orig_system_metadata = {}
|
||||||
instance = fake_instance.fake_instance_obj(self.context,
|
instance = fake_instance.fake_instance_obj(self.context,
|
||||||
vm_state=vm_states.ACTIVE, cell_name='fake-cell',
|
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,
|
system_metadata=orig_system_metadata,
|
||||||
expected_attrs=['system_metadata'])
|
expected_attrs=['system_metadata'])
|
||||||
get_flavor.return_value = ''
|
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,
|
image = {"min_ram": 10, "min_disk": 1,
|
||||||
"properties": {'architecture': 'x86_64'}}
|
"properties": {'architecture': 'x86_64'},
|
||||||
|
"id": uuids.image_id}
|
||||||
admin_pass = ''
|
admin_pass = ''
|
||||||
files_to_inject = []
|
files_to_inject = []
|
||||||
bdms = objects.BlockDeviceMappingList()
|
bdms = objects.BlockDeviceMappingList()
|
||||||
|
|
Loading…
Reference in New Issue