openstack
/
cinder
Code Issues Proposed changes

Add missing consistencygroup_id in volume 

consistencygroup_id is missing from the volume object during
volume creation after it is provided by the user. This problem
was introduced by the migrating CG code. This patch fixed this
issue. It also made consistencygroup object available in the
volume object during volume creation and made consistencygroup
object available in the cgsnapshot object when cgsnapshot is
created/deleted.

Change-Id: I243f9306df883831165eaed450c76cb4c831787c
Closes-Bug: #1661393
This commit is contained in:
xing-yang 2016-12-02 05:57:08 -05:00
parent 1c948c791c
commit c7db2b81d4
4 changed files with 73 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
cinder/objects/volume.py
View File

@ -340,8 +340,11 @@ class Volume(cleanable.CinderCleanableObject, base.CinderObject,
updates = self.cinder_obj_get_changes()
if updates:
if 'consistencygroup' in updates:
raise exception.ObjectActionError(
action='save', reason=_('consistencygroup changed'))
# NOTE(xyang): Allow this to pass if 'consistencygroup'is
# set to None. This is to support backward compatibility.
if updates.get('consistencygroup'):
raise exception.ObjectActionError(
action='save', reason=_('consistencygroup changed'))
if 'group' in updates:
raise exception.ObjectActionError(
action='save', reason=_('group changed'))

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

@ -706,13 +706,16 @@ class CreateVolumeFlowManagerTestCase(test.TestCase):
super(CreateVolumeFlowManagerTestCase, self).setUp()
self.ctxt = context.get_admin_context()
@mock.patch('cinder.volume.flows.manager.create_volume.'
'CreateVolumeFromSpecTask.'
'_cleanup_cg_in_volume')
@mock.patch('cinder.volume.flows.manager.create_volume.'
'CreateVolumeFromSpecTask.'
'_handle_bootable_volume_glance_meta')
@mock.patch('cinder.objects.Volume.get_by_id')
@mock.patch('cinder.objects.Snapshot.get_by_id')
def test_create_from_snapshot(self, snapshot_get_by_id, volume_get_by_id,
handle_bootable):
handle_bootable, cleanup_cg):
fake_db = mock.MagicMock()
fake_driver = mock.MagicMock()
fake_volume_manager = mock.MagicMock()
@ -730,25 +733,34 @@ class CreateVolumeFlowManagerTestCase(test.TestCase):
volume_obj, snapshot_obj)
handle_bootable.assert_called_once_with(self.ctxt, volume_obj,
snapshot_id=snapshot_obj.id)
cleanup_cg.assert_called_once_with(volume_obj)
@mock.patch('cinder.volume.flows.manager.create_volume.'
'CreateVolumeFromSpecTask.'
'_cleanup_cg_in_volume')
@mock.patch('cinder.objects.Snapshot.get_by_id')
def test_create_from_snapshot_update_failure(self, snapshot_get_by_id):
def test_create_from_snapshot_update_failure(self, snapshot_get_by_id,
mock_cleanup_cg):
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)
volume = fake_volume.fake_db_volume()
volume_obj = fake_volume.fake_volume_obj(self.ctxt)
snapshot_obj = fake_snapshot.fake_snapshot_obj(self.ctxt)
snapshot_get_by_id.return_value = snapshot_obj
fake_db.volume_get.side_effect = exception.CinderException
self.assertRaises(exception.MetadataUpdateFailure,
fake_manager._create_from_snapshot, self.ctxt,
volume, snapshot_obj.id)
volume_obj, snapshot_obj.id)
fake_driver.create_volume_from_snapshot.assert_called_once_with(
volume, snapshot_obj)
volume_obj, snapshot_obj)
mock_cleanup_cg.assert_called_once_with(volume_obj)
@mock.patch('cinder.volume.flows.manager.create_volume.'
'CreateVolumeFromSpecTask.'
'_cleanup_cg_in_volume')
@mock.patch('cinder.volume.flows.manager.create_volume.'
'CreateVolumeFromSpecTask.'
'_handle_bootable_volume_glance_meta')
@ -759,7 +771,8 @@ class CreateVolumeFlowManagerTestCase(test.TestCase):
mock_check_size,
mock_qemu_img,
mock_fetch_img,
mock_handle_bootable):
mock_handle_bootable,
mock_cleanup_cg):
fake_db = mock.MagicMock()
fake_driver = mock.MagicMock()
fake_volume_manager = mock.MagicMock()
@ -789,6 +802,7 @@ class CreateVolumeFlowManagerTestCase(test.TestCase):
mock_handle_bootable.assert_called_once_with(self.ctxt, volume,
image_id=image_id,
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):
@ -822,13 +836,17 @@ class CreateVolumeFlowManagerGlanceCinderBackendCase(test.TestCase):
super(CreateVolumeFlowManagerGlanceCinderBackendCase, self).setUp()
self.ctxt = context.get_admin_context()
@mock.patch('cinder.volume.flows.manager.create_volume.'
'CreateVolumeFromSpecTask.'
'_cleanup_cg_in_volume')
@mock.patch('cinder.image.image_utils.TemporaryImages.fetch')
@mock.patch('cinder.volume.flows.manager.create_volume.'
'CreateVolumeFromSpecTask.'
'_handle_bootable_volume_glance_meta')
@mock.patch('cinder.image.image_utils.qemu_img_info')
def test_create_from_image_volume(self, mock_qemu_info, handle_bootable,
mock_fetch_img, format='raw', owner=None,
mock_fetch_img, mock_cleanup_cg,
format='raw', owner=None,
location=True):
self.flags(allowed_direct_url_schemes=['cinder'])
mock_fetch_img.return_value = mock.MagicMock(
@ -875,6 +893,7 @@ class CreateVolumeFlowManagerGlanceCinderBackendCase(test.TestCase):
image_meta=image_meta)
else:
self.assertFalse(fake_driver.create_cloned_volume.called)
mock_cleanup_cg.assert_called_once_with(volume)
def test_create_from_image_volume_in_qcow2_format(self):
self.test_create_from_image_volume(format='qcow2')

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

@ -28,6 +28,7 @@ from cinder.i18n import _, _LE, _LI, _LW
from cinder.image import glance
from cinder.image import image_utils
from cinder import objects
from cinder.objects import consistencygroup
from cinder import utils
from cinder.volume.flows import common
from cinder.volume import utils as volume_utils
@ -447,6 +448,7 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask):
snapshot = objects.Snapshot.get_by_id(context, snapshot_id)
model_update = self.driver.create_volume_from_snapshot(volume,
snapshot)
self._cleanup_cg_in_volume(volume)
# NOTE(harlowja): Subtasks would be useful here since after this
# point the volume has already been created and further failures
# will not destroy the volume (although they could in the future).
@ -488,6 +490,7 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask):
# that use the source volume are underway.
srcvol_ref = objects.Volume.get_by_id(context, source_volid)
model_update = self.driver.create_cloned_volume(volume, srcvol_ref)
self._cleanup_cg_in_volume(volume)
# NOTE(harlowja): Subtasks would be useful here since after this
# point the volume has already been created and further failures
# will not destroy the volume (although they could in the future).
@ -507,6 +510,7 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask):
srcvol_ref = objects.Volume.get_by_id(context, source_replicaid)
model_update = self.driver.create_replica_test_volume(volume,
srcvol_ref)
self._cleanup_cg_in_volume(volume)
# NOTE(harlowja): Subtasks would be useful here since after this
# point the volume has already been created and further failures
# will not destroy the volume (although they could in the future).
@ -639,7 +643,9 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask):
return None, False
try:
return self.driver.create_cloned_volume(volume, image_volume), True
ret = self.driver.create_cloned_volume(volume, image_volume)
self._cleanup_cg_in_volume(volume)
return ret, True
except (NotImplementedError, exception.CinderException):
LOG.exception(_LE('Failed to clone image volume %(id)s.'),
{'id': image_volume['id']})
@ -654,6 +660,7 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask):
# clone image 'path' resumable and revertable in the correct
# manner.
model_update = self.driver.create_volume(volume) or {}
self._cleanup_cg_in_volume(volume)
model_update['status'] = 'downloading'
try:
volume.update(model_update)
@ -827,7 +834,9 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask):
return model_update
def _create_raw_volume(self, volume, **kwargs):
return self.driver.create_volume(volume)
ret = self.driver.create_volume(volume)
self._cleanup_cg_in_volume(volume)
return ret
def execute(self, context, volume, volume_spec):
volume_spec = dict(volume_spec)
@ -842,6 +851,15 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask):
"Volume driver %s not initialized"), driver_name)
raise exception.DriverNotInitialized()
# NOTE(xyang): Populate consistencygroup_id and consistencygroup
# fields before passing to the driver. This is to support backward
# compatibility of consistencygroup.
if volume.group_id:
volume.consistencygroup_id = volume.group_id
cg = consistencygroup.ConsistencyGroup()
cg.from_group(volume.group)
volume.consistencygroup = cg
create_type = volume_spec.pop('type', None)
LOG.info(_LI("Volume %(volume_id)s: being created as %(create_type)s "
"with specification: %(volume_spec)s"),
@ -880,6 +898,17 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask):
{'volume_id': volume_id, 'model': model_update})
raise
def _cleanup_cg_in_volume(self, volume):
# NOTE(xyang): Cannot have both group_id and consistencygroup_id.
# consistencygroup_id needs to be removed to avoid DB reference
# error because there isn't an entry in the consistencygroups table.
if (('group_id' in volume and volume.group_id) and
('consistencygroup_id' in volume and
volume.consistencygroup_id)):
volume.consistencygroup_id = None
if 'consistencygroup' in volume:
volume.consistencygroup = None
class CreateVolumeOnFinishTask(NotifyVolumeActionTask):
"""On successful volume creation this will perform final volume actions.

16
cinder/volume/manager.py
View File

@ -2831,7 +2831,7 @@ class VolumeManager(manager.CleanableManager,
group, volumes)
cgsnapshot, sorted_snapshots = (
self._convert_group_snapshot_to_cgsnapshot(
group_snapshot, sorted_snapshots))
group_snapshot, sorted_snapshots, context))
source_cg, sorted_source_vols = (
self._convert_group_to_cg(source_group,
sorted_source_vols))
@ -3294,7 +3294,7 @@ class VolumeManager(manager.CleanableManager,
for vol in volumes:
vol.consistencygroup_id = vol.group_id
return group, volumes
return cg, volumes
def _remove_consistencygroup_id_from_volumes(self, volumes):
if not volumes:
@ -3302,7 +3302,8 @@ class VolumeManager(manager.CleanableManager,
for vol in volumes:
vol.consistencygroup_id = None
def _convert_group_snapshot_to_cgsnapshot(self, group_snapshot, snapshots):
def _convert_group_snapshot_to_cgsnapshot(self, group_snapshot, snapshots,
ctxt):
if not group_snapshot:
return None, None
cgsnap = cgsnapshot.CGSnapshot()
@ -3310,6 +3311,11 @@ class VolumeManager(manager.CleanableManager,
for snap in snapshots:
snap.cgsnapshot_id = snap.group_snapshot_id
# Populate consistencygroup object
grp = objects.Group.get_by_id(ctxt, group_snapshot.group_id)
cg, __ = self._convert_group_to_cg(grp, [])
cgsnap.consistencygroup = cg
return cgsnap, snapshots
def _remove_cgsnapshot_id_from_snapshots(self, snapshots):
@ -3791,7 +3797,7 @@ class VolumeManager(manager.CleanableManager,
else:
cgsnapshot, snapshots = (
self._convert_group_snapshot_to_cgsnapshot(
group_snapshot, snapshots))
group_snapshot, snapshots, context))
model_update, snapshots_model_update = (
self.driver.create_cgsnapshot(context, cgsnapshot,
snapshots))
@ -4055,7 +4061,7 @@ class VolumeManager(manager.CleanableManager,
else:
cgsnapshot, snapshots = (
self._convert_group_snapshot_to_cgsnapshot(
group_snapshot, snapshots))
group_snapshot, snapshots, context))
model_update, snapshots_model_update = (
self.driver.delete_cgsnapshot(context, cgsnapshot,
snapshots))