openstack/cinder
Code Issues Proposed changes

Merge "Do not clone non-raw images in rbd backend"

Browse Source
This commit is contained in:
Jenkins 2013-12-18 00:39:23 +00:00 committed by Gerrit Code Review
commit 25eff9e3a9
11 changed files with 126 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
cinder/tests/test_gpfs.py
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'])

12
cinder/tests/test_netapp_nfs.py
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'])

154
cinder/tests/test_rbd.py
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
@ -687,26 +700,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,
@ -716,58 +734,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()

2
cinder/tests/test_volume.py
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()

7
cinder/volume/driver.py
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
"""

2
cinder/volume/drivers/gpfs.py
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):

2
cinder/volume/drivers/lvm.py
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):

2
cinder/volume/drivers/netapp/nfs.py
View File

@ -365,7 +365,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

14
cinder/volume/drivers/rbd.py
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)

2
cinder/volume/drivers/scality.py
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

2
cinder/volume/flows/create_volume/__init__.py
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