diff --git a/cinder/objects/volume.py b/cinder/objects/volume.py index c8b4e2f379c..ea12de04fca 100644 --- a/cinder/objects/volume.py +++ b/cinder/objects/volume.py @@ -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')) diff --git a/cinder/tests/unit/volume/flows/test_create_volume_flow.py b/cinder/tests/unit/volume/flows/test_create_volume_flow.py index 515ba8f42ce..01eced1e1a1 100644 --- a/cinder/tests/unit/volume/flows/test_create_volume_flow.py +++ b/cinder/tests/unit/volume/flows/test_create_volume_flow.py @@ -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') diff --git a/cinder/volume/flows/manager/create_volume.py b/cinder/volume/flows/manager/create_volume.py index 8e87e4c8335..d57ebda9818 100644 --- a/cinder/volume/flows/manager/create_volume.py +++ b/cinder/volume/flows/manager/create_volume.py @@ -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. diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index ad75c177c94..257e9c23a3e 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -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))