Fix reimage with snapshot backed image
When we create a snapshot of a server that is volume-backed, glance creates a zero sized metadata entry of the image which is backed by the volume snapshot. When we try to reimage a volume with that given image, we need to fetch the volume snapshot details from the image metadata. Now to reimage, we need to create a new volume from snapshot and copy the data from our snapshot-volume to our original volume for which we use the generic revert to snapshot mechanism as it performs the same steps. Closes-Bug: #2062539 Change-Id: Ic4bf44c320ad53b514178ecd4d5f57f037169bfe
This commit is contained in:
Rajat Dhasmana
parent
c8ceeeb150
commit
ae22195df7
@ -26,6 +26,7 @@ from cinder.api.schemas import volume_actions as volume_action
|
||||
from cinder.api import validation
|
||||
from cinder import exception
|
||||
from cinder.i18n import _
|
||||
from cinder.image import glance
|
||||
from cinder.policies import volume_actions as policy
|
||||
from cinder import volume
|
||||
from cinder.volume import volume_utils
|
||||
@ -327,6 +328,42 @@ class VolumeActionsController(wsgi.Controller):
|
||||
|
||||
self.volume_api.update(context, volume, update_dict)
|
||||
|
||||
def _get_image_snapshot_and_check_size(self, context, image_uuid,
|
||||
volume_size):
|
||||
image_snapshot = None
|
||||
if image_uuid:
|
||||
image_service = glance.get_default_image_service()
|
||||
image_meta = image_service.show(context, image_uuid)
|
||||
if image_meta is not None:
|
||||
bdms = image_meta.get('properties', {}).get(
|
||||
'block_device_mapping', [])
|
||||
if bdms:
|
||||
boot_bdm = [bdm for bdm in bdms if (
|
||||
bdm.get('source_type') == 'snapshot' and
|
||||
bdm.get('boot_index') == 0)]
|
||||
if boot_bdm:
|
||||
try:
|
||||
# validate size
|
||||
image_snap_size = boot_bdm[0].get('volume_size')
|
||||
if image_snap_size > volume_size:
|
||||
msg = (_(
|
||||
"Volume size must be greater than the "
|
||||
"image size. (Image: %(img_size)s, "
|
||||
"Volume: %(vol_size)s).") % {
|
||||
'img_size': image_snap_size,
|
||||
'vol_size': volume_size})
|
||||
raise webob.exc.HTTPBadRequest(explanation=msg)
|
||||
image_snapshot = self.volume_api.get_snapshot(
|
||||
context, boot_bdm[0].get('snapshot_id'))
|
||||
except exception.NotFound:
|
||||
explanation = _(
|
||||
'Nova specific image is found, but boot '
|
||||
'volume snapshot id:%s not found.'
|
||||
) % boot_bdm[0].get('snapshot_id')
|
||||
raise webob.exc.HTTPNotFound(
|
||||
explanation=explanation)
|
||||
return image_snapshot
|
||||
|
||||
@wsgi.Controller.api_version(mv.SUPPORT_REIMAGE_VOLUME)
|
||||
@wsgi.response(HTTPStatus.ACCEPTED)
|
||||
@wsgi.action('os-reimage')
|
||||
@ -341,9 +378,11 @@ class VolumeActionsController(wsgi.Controller):
|
||||
reimage_reserved = strutils.bool_from_string(reimage_reserved,
|
||||
strict=True)
|
||||
image_id = params['image_id']
|
||||
image_snap = self._get_image_snapshot_and_check_size(
|
||||
context, image_id, volume.size)
|
||||
try:
|
||||
self.volume_api.reimage(context, volume, image_id,
|
||||
reimage_reserved)
|
||||
reimage_reserved, image_snap)
|
||||
except exception.InvalidVolume as error:
|
||||
raise webob.exc.HTTPBadRequest(explanation=error.msg)
|
||||
|
||||
|
||||
@ -1592,8 +1592,11 @@ class VolumeImageActionsTest(test.TestCase):
|
||||
return req
|
||||
|
||||
@ddt.data(None, False, True)
|
||||
@mock.patch.object(volume_actions.VolumeActionsController,
|
||||
'_get_image_snapshot_and_check_size')
|
||||
@mock.patch.object(volume_api.API, "reimage")
|
||||
def test_volume_reimage(self, reimage_reserved, mock_image):
|
||||
def test_volume_reimage(
|
||||
self, reimage_reserved, mock_image, mock_get_img_snap):
|
||||
vol = utils.create_volume(self.context)
|
||||
body = {"os-reimage": {"image_id": fake.IMAGE_ID}}
|
||||
if reimage_reserved is not None:
|
||||
@ -1619,7 +1622,9 @@ class VolumeImageActionsTest(test.TestCase):
|
||||
self.assertRaises(exception.VersionNotFoundForAPIMethod,
|
||||
self.controller._reimage, req, vol.id, body=body)
|
||||
|
||||
def test_reimage_volume_invalid_status(self):
|
||||
@mock.patch.object(volume_actions.VolumeActionsController,
|
||||
'_get_image_snapshot_and_check_size')
|
||||
def test_reimage_volume_invalid_status(self, mock_get_img_snap):
|
||||
def fake_reimage_volume(*args, **kwargs):
|
||||
msg = "Volume status must be available."
|
||||
raise exception.InvalidVolume(reason=msg)
|
||||
@ -1633,8 +1638,11 @@ class VolumeImageActionsTest(test.TestCase):
|
||||
self.controller._reimage, req,
|
||||
vol.id, body=body)
|
||||
|
||||
@mock.patch.object(volume_actions.VolumeActionsController,
|
||||
'_get_image_snapshot_and_check_size')
|
||||
@mock.patch('cinder.context.RequestContext.authorize')
|
||||
def test_reimage_volume_attach_more_than_one_server(self, mock_authorize):
|
||||
def test_reimage_volume_attach_more_than_one_server(self, mock_authorize,
|
||||
mock_get_img_snap):
|
||||
vol = utils.create_volume(self.context)
|
||||
va_objs = [objects.VolumeAttachment(context=self.context, id=i)
|
||||
for i in [fake.OBJECT_ID, fake.OBJECT2_ID, fake.OBJECT3_ID]]
|
||||
@ -1646,3 +1654,77 @@ class VolumeImageActionsTest(test.TestCase):
|
||||
req = self._build_reimage_req(body, vol)
|
||||
self.assertRaises(webob.exc.HTTPConflict,
|
||||
self.controller._reimage, req, vol.id, body=body)
|
||||
|
||||
@mock.patch.object(volume_api.API, 'get_snapshot')
|
||||
@mock.patch.object(volume_api.API, 'get', fake_volume_get_obj)
|
||||
@mock.patch.object(glance, 'get_default_image_service')
|
||||
@mock.patch.object(volume_api.API, "reimage")
|
||||
def test_volume_reimage_image_snapshot(
|
||||
self, mock_image, mock_image_service, mock_get_snap):
|
||||
vol = utils.create_volume(self.context)
|
||||
image_meta = {
|
||||
'properties': {
|
||||
'block_device_mapping': [
|
||||
{
|
||||
'source_type': 'snapshot',
|
||||
'boot_index': 0,
|
||||
'volume_size': 1,
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
mock_image_service.return_value = mock.MagicMock()
|
||||
mock_image_service.return_value.show.return_value = image_meta
|
||||
body = {"os-reimage": {"image_id": fake.IMAGE_ID}}
|
||||
req = self._build_reimage_req(body, vol.id)
|
||||
self.controller._reimage(req, vol.id, body=body)
|
||||
|
||||
@mock.patch.object(volume_api.API, 'get', fake_volume_get_obj)
|
||||
@mock.patch.object(glance, 'get_default_image_service')
|
||||
@mock.patch.object(volume_api.API, "reimage")
|
||||
def test_volume_reimage_image_snapshot_size_mismatch(
|
||||
self, mock_image, mock_image_service):
|
||||
vol = utils.create_volume(self.context)
|
||||
image_meta = {
|
||||
'properties': {
|
||||
'block_device_mapping': [
|
||||
{
|
||||
'source_type': 'snapshot',
|
||||
'boot_index': 0,
|
||||
'volume_size': 2,
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
mock_image_service.return_value = mock.MagicMock()
|
||||
mock_image_service.return_value.show.return_value = image_meta
|
||||
body = {"os-reimage": {"image_id": fake.IMAGE_ID}}
|
||||
req = self._build_reimage_req(body, vol.id)
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller._reimage, req, vol.id, body=body)
|
||||
|
||||
@mock.patch.object(volume_api.API, 'get_snapshot')
|
||||
@mock.patch.object(volume_api.API, 'get', fake_volume_get_obj)
|
||||
@mock.patch.object(glance, 'get_default_image_service')
|
||||
@mock.patch.object(volume_api.API, "reimage")
|
||||
def test_volume_reimage_image_snapshot_snap_not_found(
|
||||
self, mock_image, mock_image_service, mock_get_snap):
|
||||
vol = utils.create_volume(self.context)
|
||||
image_meta = {
|
||||
'properties': {
|
||||
'block_device_mapping': [
|
||||
{
|
||||
'source_type': 'snapshot',
|
||||
'boot_index': 0,
|
||||
'volume_size': 1,
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
mock_image_service.return_value = mock.MagicMock()
|
||||
mock_image_service.return_value.show.return_value = image_meta
|
||||
mock_get_snap.side_effect = exception.NotFound
|
||||
body = {"os-reimage": {"image_id": fake.IMAGE_ID}}
|
||||
req = self._build_reimage_req(body, vol.id)
|
||||
self.assertRaises(webob.exc.HTTPNotFound,
|
||||
self.controller._reimage, req, vol.id, body=body)
|
||||
|
||||
@ -677,11 +677,16 @@ class VolumeRPCAPITestCase(test.RPCAPITestCase):
|
||||
group=self.fake_group,
|
||||
version='3.14')
|
||||
|
||||
def test_reimage(self):
|
||||
@ddt.data('3.18', '3.20')
|
||||
def test_reimage(self, version):
|
||||
if version == '3.18':
|
||||
self.can_send_version_mock.side_effect = (
|
||||
True, True, False, False)
|
||||
self._test_rpc_api('reimage', rpc_method='cast',
|
||||
server=self.fake_volume_obj.host,
|
||||
volume=self.fake_volume_obj,
|
||||
image_meta={'id': fake.IMAGE_ID,
|
||||
'container_format': 'fake_type',
|
||||
'disk_format': 'fake_format'},
|
||||
version='3.18')
|
||||
image_snap='fake_snap',
|
||||
version=version)
|
||||
|
||||
@ -50,6 +50,26 @@ class VolumeReimageTestCase(base.BaseVolumeTestCase):
|
||||
disable_sparse=True)
|
||||
self.assertEqual(volume.status, 'available')
|
||||
|
||||
def test_volume_reimage_image_snapshot(self):
|
||||
volume = tests_utils.create_volume(self.context, status='downloading',
|
||||
previous_status='available')
|
||||
self.assertEqual(volume.status, 'downloading')
|
||||
self.assertEqual(volume.previous_status, 'available')
|
||||
self.volume.create_volume(self.context, volume)
|
||||
|
||||
with mock.patch.object(self.volume.driver, 'copy_image_to_volume'
|
||||
) as mock_cp_img, \
|
||||
mock.patch.object(self.volume, '_revert_to_snapshot_generic'
|
||||
) as generic_revert:
|
||||
fake_snap = mock.MagicMock(
|
||||
id='08f850d7-8b43-4656-a71c-647c864a3599')
|
||||
self.volume.reimage(
|
||||
self.context, volume, self.image_meta, image_snap=fake_snap)
|
||||
mock_cp_img.assert_not_called()
|
||||
generic_revert.assert_called_once_with(
|
||||
self.context, volume, fake_snap)
|
||||
self.assertEqual(volume.status, 'available')
|
||||
|
||||
def test_volume_reimage_raise_exception(self):
|
||||
volume = tests_utils.create_volume(self.context)
|
||||
self.volume.create_volume(self.context, volume)
|
||||
@ -107,7 +127,7 @@ class VolumeReimageTestCase(base.BaseVolumeTestCase):
|
||||
self.volume_api.reimage(self.context, volume, self.image_meta['id'])
|
||||
mock_check.assert_called_once_with(self.image_meta, volume.size)
|
||||
mock_reimage.assert_called_once_with(self.context, volume,
|
||||
self.image_meta)
|
||||
self.image_meta, image_snap=None)
|
||||
|
||||
@mock.patch('cinder.volume.volume_utils.check_image_metadata')
|
||||
@mock.patch('cinder.volume.rpcapi.VolumeAPI.reimage')
|
||||
@ -139,7 +159,7 @@ class VolumeReimageTestCase(base.BaseVolumeTestCase):
|
||||
reimage_reserved=True)
|
||||
mock_check.assert_called_once_with(self.image_meta, volume.size)
|
||||
mock_reimage.assert_called_once_with(self.context, volume,
|
||||
self.image_meta)
|
||||
self.image_meta, image_snap=None)
|
||||
|
||||
def test_volume_reimage_api_with_invaild_status(self):
|
||||
volume = tests_utils.create_volume(self.context)
|
||||
@ -167,3 +187,16 @@ class VolumeReimageTestCase(base.BaseVolumeTestCase):
|
||||
self.assertIn("status must be "
|
||||
"available or error or reserved",
|
||||
str(ex))
|
||||
|
||||
@mock.patch('cinder.volume.volume_utils.check_image_metadata')
|
||||
@mock.patch('cinder.volume.rpcapi.VolumeAPI.reimage')
|
||||
def test_volume_reimage_api_image_snapshot(
|
||||
self, mock_reimage, mock_check):
|
||||
volume = tests_utils.create_volume(self.context)
|
||||
self.volume_api.reimage(
|
||||
self.context, volume, self.image_meta['id'],
|
||||
image_snap='fake_snap')
|
||||
mock_check.assert_called_once_with(self.image_meta, volume['size'])
|
||||
mock_reimage.assert_called_once_with(self.context, volume,
|
||||
self.image_meta,
|
||||
image_snap='fake_snap')
|
||||
|
||||
@ -2689,7 +2689,8 @@ class API(base.Base):
|
||||
volume_utils.notify_about_volume_usage(ctxt, volume, "detach.end")
|
||||
return volume.volume_attachment
|
||||
|
||||
def reimage(self, context, volume, image_id, reimage_reserved=False):
|
||||
def reimage(self, context, volume, image_id, reimage_reserved=False,
|
||||
image_snap=None):
|
||||
if volume.status in ['reserved']:
|
||||
context.authorize(vol_action_policy.REIMAGE_RESERVED_POLICY,
|
||||
target_obj=volume)
|
||||
@ -2717,6 +2718,10 @@ class API(base.Base):
|
||||
raise exception.InvalidVolume(reason=msg)
|
||||
image_meta = self.image_service.show(context, image_id)
|
||||
try:
|
||||
# If the source of the image is a volume snapshot
|
||||
# (image_snap is not None), we will get image 'size' as 0 and
|
||||
# 'virtual_size' as None but at least we will verify the image
|
||||
# 'status' and 'min_disk' properties.
|
||||
volume_utils.check_image_metadata(image_meta, volume['size'])
|
||||
# Currently we only raise InvalidInput and ImageUnacceptable
|
||||
# exceptions in the check_image_metadata call but having Exception
|
||||
@ -2725,16 +2730,18 @@ class API(base.Base):
|
||||
# Also this helps makes adding new exceptions easier in the future.
|
||||
except Exception:
|
||||
with excutils.save_and_reraise_exception():
|
||||
LOG.exception("Failed to reimage volume %(volume_id)s with "
|
||||
"image %(image_id)s",
|
||||
{'volume_id': volume.id, 'image_id': image_id})
|
||||
LOG.exception("Failed to reimage volume %(volume_id)s "
|
||||
"with image %(image_id)s",
|
||||
{'volume_id': volume.id,
|
||||
'image_id': image_id})
|
||||
volume.conditional_update(
|
||||
{'status': volume.model.previous_status,
|
||||
'previous_status': None},
|
||||
{'status': 'downloading'})
|
||||
self.volume_rpcapi.reimage(context,
|
||||
volume,
|
||||
image_meta)
|
||||
image_meta,
|
||||
image_snap=image_snap)
|
||||
|
||||
|
||||
class HostAPI(base.Base):
|
||||
|
||||
@ -5350,10 +5350,20 @@ class VolumeManager(manager.CleanableManager,
|
||||
self.db.volume_glance_metadata_bulk_create(context, volume.id,
|
||||
volume_meta)
|
||||
|
||||
def reimage(self, context, volume, image_meta):
|
||||
def reimage(self, context, volume, image_meta, image_snap=None):
|
||||
"""Reimage a volume with specific image."""
|
||||
image_id = None
|
||||
try:
|
||||
if image_snap:
|
||||
# We are not calling the driver method here since the snapshot
|
||||
# could belong to a different volume.
|
||||
# Even if the snapshot belongs to a different volume, we are
|
||||
# doing generic revert where we create a volume out of the
|
||||
# snapshot and do a copy so we are safe here.
|
||||
# Size checks are already done on the API layer so we don't
|
||||
# need to worry about the image fitting into the volume.
|
||||
self._revert_to_snapshot_generic(context, volume, image_snap)
|
||||
else:
|
||||
image_id = image_meta['id']
|
||||
image_service, _ = glance.get_remote_image_service(
|
||||
context, image_meta['id'])
|
||||
|
||||
@ -139,9 +139,10 @@ class VolumeAPI(rpc.RPCAPI):
|
||||
3.17 - Make get_backup_device a cast (async)
|
||||
3.18 - Add reimage method
|
||||
3.19 - Add extend_volume_completion method
|
||||
3.20 - Add image_snap parameter to reimage method
|
||||
"""
|
||||
|
||||
RPC_API_VERSION = '3.19'
|
||||
RPC_API_VERSION = '3.20'
|
||||
RPC_DEFAULT_VERSION = '3.0'
|
||||
TOPIC = constants.VOLUME_TOPIC
|
||||
BINARY = constants.VOLUME_BINARY
|
||||
@ -544,6 +545,11 @@ class VolumeAPI(rpc.RPCAPI):
|
||||
group=group)
|
||||
|
||||
@rpc.assert_min_rpc_version('3.18')
|
||||
def reimage(self, ctxt, volume, image_meta):
|
||||
cctxt = self._get_cctxt(volume.service_topic_queue, version='3.18')
|
||||
def reimage(self, ctxt, volume, image_meta, image_snap=None):
|
||||
cctxt = self._get_cctxt(
|
||||
volume.service_topic_queue, version=('3.20', '3.18'))
|
||||
if cctxt.can_send_version('3.20'):
|
||||
cctxt.cast(ctxt, 'reimage', volume=volume, image_meta=image_meta,
|
||||
image_snap=image_snap)
|
||||
else:
|
||||
cctxt.cast(ctxt, 'reimage', volume=volume, image_meta=image_meta)
|
||||
|
||||
@ -0,0 +1,5 @@
|
||||
---
|
||||
fixes:
|
||||
- |
|
||||
`Bug #2062539 <https://bugs.launchpad.net/cinder/+bug/2062539>`_: Fixed
|
||||
reimage operation when the image is backed by a volume snapshot.
|
||||
Loading…
x
Reference in New Issue
Block a user