Do not clone non-raw images in rbd backend

RBD backend only supports booting from images in raw format. A volume
that was cloned from an image in any other format is not bootable. The
RBD driver will consider non-raw images to be uncloneable to trigger
automatic conversion to raw format.

Includes conversion of the corresponding unit test to use mock (instead
of mox) and expanded comments and error messages based on change #58893
by Edward Hope-Morley.

Change-Id: I5725d2f7576bc1b3e9b874ba944ad17d33a6e2cb
Closes-Bug: #1246219
Closes-Bug: #1247998
This commit is contained in:
Dmitry Borodaenko 2013-11-27 14:33:00 -08:00 committed by Dmitry Borodaenko
parent 1622b284c6
commit e066158b52
11 changed files with 126 additions and 79 deletions

View File

@ -291,7 +291,8 @@ class GPFSDriverTestCase(test.TestCase):
CONF.gpfs_images_share_mode = 'copy_on_write'
self.driver.clone_image(volume,
None,
self.image_id)
self.image_id,
{})
self.assertTrue(os.path.exists(volumepath))
self.volume.delete_volume(self.context, volume['id'])
@ -312,7 +313,8 @@ class GPFSDriverTestCase(test.TestCase):
CONF.gpfs_images_share_mode = 'copy'
self.driver.clone_image(volume,
None,
self.image_id)
self.image_id,
{})
self.assertTrue(os.path.exists(volumepath))
self.volume.delete_volume(self.context, volume['id'])

View File

@ -481,7 +481,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase):
drv._post_clone_image(volume)
mox.ReplayAll()
drv. clone_image(volume, ('image_location', None), 'image_id')
drv.clone_image(volume, ('image_location', None), 'image_id', {})
mox.VerifyAll()
def get_img_info(self, format):
@ -505,7 +505,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase):
mox.ReplayAll()
(prop, cloned) = drv. clone_image(
volume, ('nfs://127.0.0.1:/share/img-id', None), 'image_id')
volume, ('nfs://127.0.0.1:/share/img-id', None), 'image_id', {})
mox.VerifyAll()
if not cloned and not prop['provider_location']:
pass
@ -541,7 +541,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase):
mox.ReplayAll()
drv. clone_image(
volume, ('nfs://127.0.0.1:/share/img-id', None), 'image_id')
volume, ('nfs://127.0.0.1:/share/img-id', None), 'image_id', {})
mox.VerifyAll()
def test_clone_image_cloneableshare_notraw(self):
@ -578,7 +578,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase):
mox.ReplayAll()
drv. clone_image(
volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id')
volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id', {})
mox.VerifyAll()
def test_clone_image_file_not_discovered(self):
@ -617,7 +617,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase):
mox.ReplayAll()
vol_dict, result = drv. clone_image(
volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id')
volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id', {})
mox.VerifyAll()
self.assertFalse(result)
self.assertFalse(vol_dict['bootable'])
@ -664,7 +664,7 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase):
mox.ReplayAll()
vol_dict, result = drv. clone_image(
volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id')
volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id', {})
mox.VerifyAll()
self.assertFalse(result)
self.assertFalse(vol_dict['bootable'])

View File

@ -35,6 +35,7 @@ from cinder.tests.test_volume import DriverTestCase
from cinder import units
from cinder.volume import configuration as conf
import cinder.volume.drivers.rbd as driver
from cinder.volume.flows import create_volume
LOG = logging.getLogger(__name__)
@ -310,7 +311,8 @@ class RBDTestCase(test.TestCase):
self.assertRaises(exception.ImageUnacceptable,
self.driver._parse_location,
loc)
self.assertFalse(self.driver._is_cloneable(loc))
self.assertFalse(
self.driver._is_cloneable(loc, {'disk_format': 'raw'}))
def test_cloneable(self):
self.stubs.Set(self.driver, '_get_fsid', lambda: 'abc')
@ -327,12 +329,14 @@ class RBDTestCase(test.TestCase):
self.mox.ReplayAll()
self.assertTrue(self.driver._is_cloneable(location))
self.assertTrue(
self.driver._is_cloneable(location, {'disk_format': 'raw'}))
def test_uncloneable_different_fsid(self):
self.stubs.Set(self.driver, '_get_fsid', lambda: 'abc')
location = 'rbd://def/pool/image/snap'
self.assertFalse(self.driver._is_cloneable(location))
self.assertFalse(
self.driver._is_cloneable(location, {'disk_format': 'raw'}))
def test_uncloneable_unreadable(self):
self.stubs.Set(self.driver, '_get_fsid', lambda: 'abc')
@ -347,7 +351,16 @@ class RBDTestCase(test.TestCase):
self.mox.ReplayAll()
self.assertFalse(self.driver._is_cloneable(location))
self.assertFalse(
self.driver._is_cloneable(location, {'disk_format': 'raw'}))
def test_uncloneable_bad_format(self):
self.stubs.Set(self.driver, '_get_fsid', lambda: 'abc')
location = 'rbd://abc/pool/image/snap'
formats = ['qcow2', 'vmdk', 'vdi']
for f in formats:
self.assertFalse(
self.driver._is_cloneable(location, {'disk_format': f}))
def _copy_image(self):
@contextlib.contextmanager
@ -567,26 +580,31 @@ class ManagedRBDTestCase(DriverTestCase):
super(ManagedRBDTestCase, self).setUp()
fake_image.stub_out_image_service(self.stubs)
self.volume.driver.set_initialized()
self.called = []
def _clone_volume_from_image(self, expected_status,
clone_works=True):
def _create_volume_from_image(self, expected_status, raw=False,
clone_error=False):
"""Try to clone a volume from an image, and check the status
afterwards.
NOTE: if clone_error is True we force the image type to raw otherwise
clone_image is not called
"""
def fake_clone_image(volume, image_location, image_id):
return {'provider_location': None}, True
def mock_clone_image(volume, image_location, image_id, image_meta):
self.called.append('clone_image')
if clone_error:
raise exception.CinderException()
else:
return {'provider_location': None}, True
def fake_clone_error(volume, image_location, image_id):
raise exception.CinderException()
self.stubs.Set(self.volume.driver, '_is_cloneable', lambda x: True)
if clone_works:
self.stubs.Set(self.volume.driver, 'clone_image', fake_clone_image)
# See tests.image.fake for image types.
if raw:
image_id = '155d900f-4e14-4e4c-a73d-069cbf4541e6'
else:
self.stubs.Set(self.volume.driver, 'clone_image', fake_clone_error)
image_id = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77'
image_id = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77'
volume_id = 1
# creating volume testdata
db.volume_create(self.context,
{'id': volume_id,
@ -596,58 +614,72 @@ class ManagedRBDTestCase(DriverTestCase):
'status': 'creating',
'instance_uuid': None,
'host': 'dummy'})
try:
if clone_works:
self.volume.create_volume(self.context,
volume_id,
image_id=image_id)
else:
self.assertRaises(exception.CinderException,
self.volume.create_volume,
self.context,
volume_id,
image_id=image_id)
volume = db.volume_get(self.context, volume_id)
self.assertEqual(volume['status'], expected_status)
finally:
# cleanup
db.volume_destroy(self.context, volume_id)
mpo = mock.patch.object
with mpo(self.volume.driver, 'create_volume') as mock_create_volume:
with mpo(self.volume.driver, 'clone_image', mock_clone_image):
with mpo(create_volume.CreateVolumeFromSpecTask,
'_copy_image_to_volume') as mock_copy_image_to_volume:
try:
if not clone_error:
self.volume.create_volume(self.context,
volume_id,
image_id=image_id)
else:
self.assertRaises(exception.CinderException,
self.volume.create_volume,
self.context,
volume_id,
image_id=image_id)
volume = db.volume_get(self.context, volume_id)
self.assertEqual(volume['status'], expected_status)
finally:
# cleanup
db.volume_destroy(self.context, volume_id)
self.assertEqual(self.called, ['clone_image'])
mock_create_volume.assert_called()
mock_copy_image_to_volume.assert_called()
def test_create_vol_from_image_status_available(self):
"""Verify that before cloning, an image is in the available state."""
self._clone_volume_from_image('available', True)
"""Clone raw image then verify volume is in available state."""
self._create_volume_from_image('available', raw=True)
def test_create_vol_from_non_raw_image_status_available(self):
"""Clone non-raw image then verify volume is in available state."""
self._create_volume_from_image('available', raw=False)
def test_create_vol_from_image_status_error(self):
"""Verify that before cloning, an image is in the available state."""
self._clone_volume_from_image('error', False)
"""Fail to clone raw image then verify volume is in error state."""
self._create_volume_from_image('error', raw=True, clone_error=True)
def test_clone_image(self):
# Test Failure Case(s)
expected = ({}, False)
def test_clone_failure(self):
driver = self.volume.driver
self.stubs.Set(self.volume.driver, '_is_cloneable', lambda x: False)
image_loc = (object(), object())
actual = self.volume.driver.clone_image(object(), image_loc, object())
self.assertEqual(expected, actual)
with mock.patch.object(driver, '_is_cloneable', lambda *args: False):
image_loc = (mock.Mock(), mock.Mock())
actual = driver.clone_image(mock.Mock(), image_loc,
mock.Mock(), {})
self.assertEqual(({}, False), actual)
self.stubs.Set(self.volume.driver, '_is_cloneable', lambda x: True)
self.assertEqual(expected,
self.volume.driver.clone_image(object(), None, None))
# Test Success Case(s)
expected = ({'provider_location': None}, True)
self.stubs.Set(self.volume.driver, '_parse_location',
lambda x: ('a', 'b', 'c', 'd'))
self.stubs.Set(self.volume.driver, '_clone', lambda *args: None)
self.stubs.Set(self.volume.driver, '_resize', lambda *args: None)
actual = self.volume.driver.clone_image(object(), image_loc, object())
self.assertEqual(expected, actual)
self.assertEqual(({}, False),
driver.clone_image(object(), None, None, {}))
def test_clone_success(self):
self.stubs.Set(self.volume.driver, '_is_cloneable', lambda x: True)
self.stubs.Set(self.volume.driver, 'clone_image', lambda a, b, c: True)
image_id = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77'
self.assertTrue(self.volume.driver.clone_image({}, image_id, image_id))
expected = ({'provider_location': None}, True)
driver = self.volume.driver
mpo = mock.patch.object
with mpo(driver, '_is_cloneable', lambda *args: True):
with mpo(driver, '_parse_location', lambda x: (1, 2, 3, 4)):
with mpo(driver, '_clone') as mock_clone:
with mpo(driver, '_resize') as mock_resize:
image_loc = (mock.Mock(), mock.Mock())
actual = driver.clone_image(mock.Mock(),
image_loc,
mock.Mock(),
{'disk_format': 'raw'})
self.assertEqual(expected, actual)
mock_clone.assert_called()
mock_resize.assert_called()

View File

@ -1486,7 +1486,7 @@ class VolumeTestCase(BaseVolumeTestCase):
def fake_fetch_to_raw(ctx, image_service, image_id, path, size=None):
pass
def fake_clone_image(volume_ref, image_location, image_id):
def fake_clone_image(volume_ref, image_location, image_id, image_meta):
return {'provider_location': None}, True
dst_fd, dst_path = tempfile.mkstemp()

View File

@ -400,7 +400,7 @@ class VolumeDriver(object):
connector.disconnect_volume(attach_info['conn']['data'],
attach_info['device'])
def clone_image(self, volume, image_location, image_id):
def clone_image(self, volume, image_location, image_id, image_meta):
"""Create a volume efficiently from an existing image.
image_location is a string whose format depends on the
@ -411,6 +411,11 @@ class VolumeDriver(object):
It can be used by the driver to introspect internal
stores or registry to do an efficient image clone.
image_meta is a dictionary that includes 'disk_format' (e.g.
raw, qcow2) and other image attributes that allow drivers to
decide whether they can clone the image without first requiring
conversion.
Returns a dict of volume properties eg. provider_location,
boolean indicating whether cloning occurred
"""

View File

@ -463,7 +463,7 @@ class GPFSDriver(driver.VolumeDriver):
return '100M'
return '%sG' % size_in_g
def clone_image(self, volume, image_location, image_id):
def clone_image(self, volume, image_location, image_id, image_meta):
return self._clone_image(volume, image_location, image_id)
def _is_cloneable(self, image_id):

View File

@ -317,7 +317,7 @@ class LVMVolumeDriver(driver.VolumeDriver):
finally:
self.delete_snapshot(temp_snapshot)
def clone_image(self, volume, image_location, image_id):
def clone_image(self, volume, image_location, image_id, image_meta):
return None, False
def backup_volume(self, context, backup, backup_service):

View File

@ -374,7 +374,7 @@ class NetAppNFSDriver(nfs.NfsDriver):
LOG.warning(_('Exception during deleting %s'), ex.__str__())
return False
def clone_image(self, volume, image_location, image_id):
def clone_image(self, volume, image_location, image_id, image_meta):
"""Create a volume efficiently from an existing image.
image_location is a string whose format depends on the

View File

@ -717,7 +717,7 @@ class RBDDriver(driver.VolumeDriver):
with RADOSClient(self) as client:
return client.cluster.get_fsid()
def _is_cloneable(self, image_location):
def _is_cloneable(self, image_location, image_meta):
try:
fsid, pool, image, snapshot = self._parse_location(image_location)
except exception.ImageUnacceptable as e:
@ -729,6 +729,13 @@ class RBDDriver(driver.VolumeDriver):
LOG.debug(reason)
return False
if image_meta['disk_format'] != 'raw':
reason = _("rbd image clone requires image format to be "
"'raw' but image {0} is '{1}'").format(
image_location, image_meta['disk_format'])
LOG.debug(reason)
return False
# check that we can read the image
try:
with RBDVolumeProxy(self, image,
@ -741,9 +748,10 @@ class RBDDriver(driver.VolumeDriver):
dict(loc=image_location, err=e))
return False
def clone_image(self, volume, image_location, image_id):
def clone_image(self, volume, image_location, image_id, image_meta):
image_location = image_location[0] if image_location else None
if image_location is None or not self._is_cloneable(image_location):
if image_location is None or not self._is_cloneable(
image_location, image_meta):
return ({}, False)
prefix, pool, image, snapshot = self._parse_location(image_location)
self._clone(volume, pool, image, snapshot)

View File

@ -250,7 +250,7 @@ class ScalityDriver(driver.VolumeDriver):
image_meta,
self.local_path(volume))
def clone_image(self, volume, image_location, image_id):
def clone_image(self, volume, image_location, image_id, image_meta):
"""Create a volume efficiently from an existing image.
image_location is a string whose format depends on the

View File

@ -1364,7 +1364,7 @@ class CreateVolumeFromSpecTask(base.CinderTask):
# dict containing provider_location for cloned volume
# and clone status.
model_update, cloned = self.driver.clone_image(
volume_ref, image_location, image_id)
volume_ref, image_location, image_id, image_meta)
if not cloned:
# TODO(harlowja): what needs to be rolled back in the clone if this
# volume create fails?? Likely this should be a subflow or broken