openstack/cinder
Code Issues Proposed changes

Merge "Extract copy_image_to_volume into volume utils"

Browse Source
This commit is contained in:
Zuul 2018-12-25 04:26:41 +00:00 committed by Gerrit Code Review
commit 94202a2464
5 changed files with 88 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
cinder/tests/unit/test_volume_utils.py
View File

@ -38,6 +38,7 @@ from cinder.tests.unit import fake_constants as fake
from cinder.tests.unit import fake_group
from cinder.tests.unit import fake_snapshot
from cinder.tests.unit import fake_volume
from cinder.tests.unit.image import fake as fake_image
from cinder.tests.unit import utils as test_utils
from cinder import utils
from cinder.volume import throttling
@ -1142,3 +1143,25 @@ class VolumeUtilsTestCase(test.TestCase):
self.assertEqual(
expected,
volume_utils.get_volume_image_metadata(fake.IMAGE_ID, image_meta))
@ddt.data(True, False)
def test_copy_image_to_volume(self, is_encrypted):
ctxt = context.get_admin_context()
fake_driver = mock.MagicMock()
key = fake.ENCRYPTION_KEY_ID if is_encrypted else None
volume = fake_volume.fake_volume_obj(ctxt, encryption_key_id=key)
fake_image_service = fake_image.FakeImageService()
image_id = fake.IMAGE_ID
image_meta = {'id': image_id}
image_location = 'abc'
volume_utils.copy_image_to_volume(fake_driver, ctxt, volume,
image_meta, image_location,
fake_image_service)
if is_encrypted:
fake_driver.copy_image_to_encrypted_volume.assert_called_once_with(
ctxt, volume, fake_image_service, image_id)
else:
fake_driver.copy_image_to_volume.assert_called_once_with(
ctxt, volume, fake_image_service, image_id)

14
cinder/tests/unit/volume/drivers/test_rbd.py
View File

@ -42,7 +42,7 @@ from cinder.tests.unit import utils
from cinder.tests.unit.volume import test_driver
from cinder.volume import configuration as conf
import cinder.volume.drivers.rbd as driver
from cinder.volume.flows.manager import create_volume
from cinder.volume import utils as volume_utils
# This is used to collect raised exceptions so that tests may check what was
@ -2380,8 +2380,8 @@ class ManagedRBDTestCase(test_driver.BaseDriverTestCase):
mock_clone_image.side_effect = _mock_clone_image
with mock.patch.object(self.volume.driver, 'create_volume') as \
mock_create:
with mock.patch.object(create_volume.CreateVolumeFromSpecTask,
'_copy_image_to_volume') as mock_copy:
with mock.patch.object(volume_utils,
'copy_image_to_volume') as mock_copy:
self._create_volume_from_image('available', raw=True)
self.assertFalse(mock_copy.called)
@ -2414,8 +2414,8 @@ class ManagedRBDTestCase(test_driver.BaseDriverTestCase):
mock_clone_image.side_effect = _mock_clone_image
with mock.patch.object(self.volume.driver, 'create_volume') as \
mock_create:
with mock.patch.object(create_volume.CreateVolumeFromSpecTask,
'_copy_image_to_volume') as mock_copy:
with mock.patch.object(volume_utils,
'copy_image_to_volume') as mock_copy:
self._create_volume_from_image('available', raw=False)
self.assertTrue(mock_copy.called)
@ -2432,8 +2432,8 @@ class ManagedRBDTestCase(test_driver.BaseDriverTestCase):
mock_clone_image:
mock_clone_image.side_effect = exception.CinderException
with mock.patch.object(self.volume.driver, 'create_volume'):
with mock.patch.object(create_volume.CreateVolumeFromSpecTask,
'_copy_image_to_volume') as mock_copy:
with mock.patch.object(volume_utils,
'copy_image_to_volume') as mock_copy:
self._create_volume_from_image('error', raw=True,
clone_error=True)
self.assertFalse(mock_copy.called)

26
cinder/tests/unit/volume/flows/test_create_volume_flow.py
View File

@ -1177,32 +1177,6 @@ class CreateVolumeFlowManagerTestCase(test.TestCase):
image_meta=image_meta)
mock_cleanup_cg.assert_called_once_with(volume)
@ddt.data(True, False)
def test__copy_image_to_volume(self, is_encrypted):
fake_db = mock.MagicMock()
fake_driver = mock.MagicMock()
fake_volume_manager = mock.MagicMock()
fake_manager = create_volume_manager.CreateVolumeFromSpecTask(
fake_volume_manager, fake_db, fake_driver)
key = fakes.ENCRYPTION_KEY_ID if is_encrypted else None
volume = fake_volume.fake_volume_obj(
self.ctxt,
encryption_key_id=key)
fake_image_service = fake_image.FakeImageService()
image_id = fakes.IMAGE_ID
image_meta = {'id': image_id}
image_location = 'abc'
fake_manager._copy_image_to_volume(self.ctxt, volume, image_meta,
image_location, fake_image_service)
if is_encrypted:
fake_driver.copy_image_to_encrypted_volume.assert_called_once_with(
self.ctxt, volume, fake_image_service, image_id)
else:
fake_driver.copy_image_to_volume.assert_called_once_with(
self.ctxt, volume, fake_image_service, image_id)
@ddt.data({'driver_error': True},
{'driver_error': False})
@mock.patch('cinder.backup.api.API.get_available_backup_service_host')

60
cinder/volume/flows/manager/create_volume.py
View File

@ -12,7 +12,6 @@
import traceback
from oslo_concurrency import processutils
from oslo_config import cfg
from oslo_log import log as logging
from oslo_utils import excutils
@ -492,60 +491,6 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask):
context, volume, source_volid=srcvol_ref.id)
return model_update
def _copy_image_to_volume(self, context, volume,
image_meta, image_location, image_service):
image_id = image_meta['id']
"""Downloads Glance image to the specified volume."""
LOG.debug("Attempting download of %(image_id)s (%(image_location)s)"
" to volume %(volume_id)s.",
{'image_id': image_id, 'volume_id': volume.id,
'image_location': image_location})
try:
image_encryption_key = image_meta.get('cinder_encryption_key_id')
if volume.encryption_key_id and image_encryption_key:
# If the image provided an encryption key, we have
# already cloned it to the volume's key in
# _get_encryption_key_id, so we can do a direct copy.
self.driver.copy_image_to_volume(
context, volume, image_service, image_id)
elif volume.encryption_key_id:
# Creating an encrypted volume from a normal, unencrypted,
# image.
self.driver.copy_image_to_encrypted_volume(
context, volume, image_service, image_id)
else:
self.driver.copy_image_to_volume(
context, volume, image_service, image_id)
except processutils.ProcessExecutionError as ex:
LOG.exception("Failed to copy image %(image_id)s to volume: "
"%(volume_id)s",
{'volume_id': volume.id, 'image_id': image_id})
raise exception.ImageCopyFailure(reason=ex.stderr)
except exception.ImageUnacceptable as ex:
LOG.exception("Failed to copy image to volume: %(volume_id)s",
{'volume_id': volume.id})
raise exception.ImageUnacceptable(ex)
except exception.ImageTooBig as ex:
with excutils.save_and_reraise_exception():
LOG.exception("Failed to copy image %(image_id)s to volume: "
"%(volume_id)s",
{'volume_id': volume.id, 'image_id': image_id})
except Exception as ex:
LOG.exception("Failed to copy image %(image_id)s to "
"volume: %(volume_id)s",
{'volume_id': volume.id, 'image_id': image_id})
if not isinstance(ex, exception.ImageCopyFailure):
raise exception.ImageCopyFailure(reason=ex)
else:
raise
LOG.debug("Downloaded image %(image_id)s (%(image_location)s)"
" to volume %(volume_id)s successfully.",
{'image_id': image_id, 'volume_id': volume.id,
'image_location': image_location})
def _capture_volume_image_metadata(self, context, volume_id,
image_id, image_meta):
volume_metadata = volume_utils.get_volume_image_metadata(
@ -634,8 +579,9 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask):
{'volume_id': volume.id,
'updates': model_update})
try:
self._copy_image_to_volume(context, volume, image_meta,
image_location, image_service)
volume_utils.copy_image_to_volume(self.driver, context, volume,
image_meta, image_location,
image_service)
except exception.ImageTooBig:
with excutils.save_and_reraise_exception():
LOG.exception("Failed to copy image to volume "

55
cinder/volume/utils.py
View File

@ -34,6 +34,7 @@ from keystoneauth1 import loading as ks_loading
from oslo_concurrency import processutils
from oslo_config import cfg
from oslo_log import log as logging
from oslo_utils import excutils
from oslo_utils import strutils
from oslo_utils import timeutils
from oslo_utils import units
@ -1118,3 +1119,57 @@ def get_volume_image_metadata(image_id, image_meta):
volume_metadata = dict(property_metadata)
volume_metadata.update(base_metadata)
return volume_metadata
def copy_image_to_volume(driver, context, volume, image_meta, image_location,
image_service):
"""Downloads Glance image to the specified volume."""
image_id = image_meta['id']
LOG.debug("Attempting download of %(image_id)s (%(image_location)s)"
" to volume %(volume_id)s.",
{'image_id': image_id, 'volume_id': volume.id,
'image_location': image_location})
try:
image_encryption_key = image_meta.get('cinder_encryption_key_id')
if volume.encryption_key_id and image_encryption_key:
# If the image provided an encryption key, we have
# already cloned it to the volume's key in
# _get_encryption_key_id, so we can do a direct copy.
driver.copy_image_to_volume(
context, volume, image_service, image_id)
elif volume.encryption_key_id:
# Creating an encrypted volume from a normal, unencrypted,
# image.
driver.copy_image_to_encrypted_volume(
context, volume, image_service, image_id)
else:
driver.copy_image_to_volume(
context, volume, image_service, image_id)
except processutils.ProcessExecutionError as ex:
LOG.exception("Failed to copy image %(image_id)s to volume: "
"%(volume_id)s",
{'volume_id': volume.id, 'image_id': image_id})
raise exception.ImageCopyFailure(reason=ex.stderr)
except exception.ImageUnacceptable as ex:
LOG.exception("Failed to copy image to volume: %(volume_id)s",
{'volume_id': volume.id})
raise exception.ImageUnacceptable(ex)
except exception.ImageTooBig as ex:
with excutils.save_and_reraise_exception():
LOG.exception("Failed to copy image %(image_id)s to volume: "
"%(volume_id)s",
{'volume_id': volume.id, 'image_id': image_id})
except Exception as ex:
LOG.exception("Failed to copy image %(image_id)s to "
"volume: %(volume_id)s",
{'volume_id': volume.id, 'image_id': image_id})
if not isinstance(ex, exception.ImageCopyFailure):
raise exception.ImageCopyFailure(reason=ex)
else:
raise
LOG.debug("Downloaded image %(image_id)s (%(image_location)s)"
" to volume %(volume_id)s successfully.",
{'image_id': image_id, 'volume_id': volume.id,
'image_location': image_location})