diff --git a/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py b/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py index 2e1c5260fd2..b38efb1ee80 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py @@ -52,6 +52,7 @@ CINDER_EMC_CONFIG_DIR = '/etc/cinder/' class VMAXCommonData(object): # array info array = '000197800123' + uni_array = u'000197800123' array_herc = '000197900123' srp = 'SRP_1' srp2 = 'SRP_2' @@ -90,6 +91,7 @@ class VMAXCommonData(object): target_group_name = 'Grp_target' storagegroup_name_with_id = 'GrpId_group_name' rdf_managed_async_grp = "OS-%s-Asynchronous-rdf-sg" % rdf_group_name + volume_id = '2b06255d-f5f0-4520-a953-b029196add6a' # connector info wwpn1 = "123456789012345" @@ -157,6 +159,9 @@ class VMAXCommonData(object): provider_location3 = {'array': six.text_type(remote_array), 'device_id': device_id2} + provider_location4 = {'array': six.text_type(uni_array), + 'device_id': device_id} + legacy_provider_location = { 'classname': 'Symm_StorageVolume', 'keybindings': {'CreationClassName': u'Symm_StorageVolume', @@ -198,7 +203,16 @@ class VMAXCommonData(object): provider_location=six.text_type(provider_location2), host=fake_host) + test_volume_snap_manage = fake_volume.fake_volume_obj( + context=ctx, name='vol1', size=2, provider_auth=None, + display_name='vol1', + provider_location=six.text_type(provider_location), + volume_type=test_volume_type, host=fake_host, + replication_driver_data=six.text_type(provider_location4)) + + snapshot_display_id = 'my_snap' snapshot_id = '390eeb4d-0f56-4a02-ba14-167167967014' + managed_snap_id = 'OS-390eeb4d-0f56-4a02-ba14-167167967014' test_snapshot_snap_name = 'OS-' + snapshot_id[:6] + snapshot_id[-9:] snap_location = {'snap_name': test_snapshot_snap_name, @@ -223,6 +237,13 @@ class VMAXCommonData(object): provider_location=six.text_type(snap_location), host=fake_host, volume=test_volume) + test_snapshot_manage = fake_snapshot.fake_snapshot_obj( + context=ctx, id=snapshot_id, + name='my_snap', size=2, + provider_location=six.text_type(snap_location), + host=fake_host, volume=test_volume_snap_manage, + display_name='my_snap') + location_info = {'location_info': '000197800123#SRP_1#Diamond#DSS', 'storage_protocol': 'FC'} test_host = {'capabilities': location_info, @@ -1458,6 +1479,24 @@ class VMAXUtilsTest(test.TestCase): self.assertTrue(self.utils.does_vol_need_rdf_management_group( extra_specs)) + def test_modify_snapshot_prefix_manage(self): + snap_name = self.data.snapshot_id + expected_snap_name = self.data.managed_snap_id + + updated_name = self.utils.modify_snapshot_prefix( + snap_name, manage=True) + + self.assertEqual(expected_snap_name, updated_name) + + def test_modify_snapshot_prefix_unmanage(self): + snap_name = self.data.managed_snap_id + expected_snap_name = self.data.snapshot_id + + updated_name = self.utils.modify_snapshot_prefix( + snap_name, unmanage=True) + + self.assertEqual(expected_snap_name, updated_name) + class VMAXRestTest(test.TestCase): def setUp(self): @@ -2784,6 +2823,19 @@ class VMAXRestTest(test.TestCase): self.data.device_id, self.data.device_id2, self.data.extra_specs) + @mock.patch.object(rest.VMAXRest, 'modify_resource', + return_value=('200', 'JobComplete')) + def test_modify_volume_snap_rename(self, mock_modify): + array = self.data.array + source_id = self.data.device_id + old_snap_backend_name = self.data.snapshot_id + new_snap_backend_name = self.data.managed_snap_id + self.rest.modify_volume_snap( + array, source_id, source_id, old_snap_backend_name, + self.data.extra_specs, link=False, unlink=False, + rename=True, new_snap_name=new_snap_backend_name) + mock_modify.assert_called_once() + class VMAXProvisionTest(test.TestCase): def setUp(self): @@ -4732,6 +4784,87 @@ class VMAXCommonTest(test.TestCase): kwargs = self.common.get_attributes_from_cinder_config() self.assertIsNone(kwargs) + @mock.patch.object(rest.VMAXRest, + 'get_size_of_device_on_array', + return_value=2.0) + def test_manage_snapshot_get_size_success(self, mock_get_size): + size = self.common.manage_existing_snapshot_get_size( + self.data.test_snapshot) + self.assertEqual(2, size) + + @mock.patch.object(rest.VMAXRest, 'get_volume_snap', + return_value={'snap_name': 'snap_name'}) + def test_manage_snapshot_success(self, mock_snap): + snapshot = self.data.test_snapshot_manage + existing_ref = {u'source-name': u'test_snap'} + updates_response = self.common.manage_existing_snapshot( + snapshot, existing_ref) + + prov_loc = {'source_id': self.data.device_id, + 'snap_name': 'OS-%s' % existing_ref['source-name']} + + updates = { + 'display_name': self.data.test_snapshot_manage.display_name, + 'provider_location': six.text_type(prov_loc)} + + self.assertEqual(updates_response, updates) + + def test_manage_snapshot_fail_already_managed(self): + snapshot = self.data.test_snapshot_manage + existing_ref = {u'source-name': u'OS-test_snap'} + + self.assertRaises(exception.VolumeBackendAPIException, + self.common.manage_existing_snapshot, + snapshot, existing_ref) + + @mock.patch.object(utils.VMAXUtils, + 'is_volume_failed_over', + return_value=True) + def test_manage_snapshot_fail_vol_failed_over(self, mock_failed): + snapshot = self.data.test_snapshot_manage + existing_ref = {u'source-name': u'test_snap'} + self.assertRaises(exception.VolumeBackendAPIException, + self.common.manage_existing_snapshot, + snapshot, existing_ref) + + @mock.patch.object(rest.VMAXRest, 'get_volume_snap', return_value=False) + def test_manage_snapshot_fail_vol_not_snap_src(self, mock_snap): + snapshot = self.data.test_snapshot_manage + existing_ref = {u'source-name': u'test_snap'} + self.assertRaises(exception.VolumeBackendAPIException, + self.common.manage_existing_snapshot, + snapshot, existing_ref) + + @mock.patch.object(utils.VMAXUtils, 'modify_snapshot_prefix', + side_effect=exception.VolumeBackendAPIException) + def test_manage_snapshot_fail_add_prefix(self, mock_mod): + snapshot = self.data.test_snapshot_manage + existing_ref = {u'source-name': u'test_snap'} + self.assertRaises(exception.VolumeBackendAPIException, + self.common.manage_existing_snapshot, + snapshot, existing_ref) + + @mock.patch.object(common.VMAXCommon, '_sync_check') + @mock.patch.object(rest.VMAXRest, 'modify_volume_snap') + def test_unmanage_snapshot_success(self, mock_mod, mock_sync): + self.common.unmanage_snapshot(self.data.test_snapshot_manage) + mock_mod.assert_called_once() + + @mock.patch.object( + utils.VMAXUtils, 'is_volume_failed_over', return_value=True) + def test_unmanage_snapshot_fail_failover(self, mock_failed): + self.assertRaises(exception.VolumeBackendAPIException, + self.common.unmanage_snapshot, + self.data.test_snapshot_manage) + + @mock.patch.object(rest.VMAXRest, + 'modify_volume_snap', + side_effect=exception.VolumeBackendAPIException) + def test_unmanage_snapshot_fail_rename(self, mock_snap): + self.assertRaises(exception.VolumeBackendAPIException, + self.common.unmanage_snapshot, + self.data.test_snapshot_manage) + class VMAXFCTest(test.TestCase): def setUp(self): diff --git a/cinder/volume/drivers/dell_emc/vmax/common.py b/cinder/volume/drivers/dell_emc/vmax/common.py index 6776782fd34..6f5da6558f1 100644 --- a/cinder/volume/drivers/dell_emc/vmax/common.py +++ b/cinder/volume/drivers/dell_emc/vmax/common.py @@ -1968,9 +1968,9 @@ class VMAXCommon(object): # Check if there are any replication sessions associated # with the volume. - snapvx_tgt, snapvx_src, rdf = self.rest.is_vol_in_rep_session( + snapvx_tgt, __, rdf = self.rest.is_vol_in_rep_session( array, device_id) - if snapvx_tgt or snapvx_src or rdf: + if snapvx_tgt or rdf: msg = (_("Unable to import volume %(device_id)s to cinder. " "It is part of a replication session.") % {'device_id': device_id}) @@ -2017,6 +2017,7 @@ class VMAXCommon(object): """Export VMAX volume from Cinder. Leave the volume intact on the backend array. + :param volume: the volume object """ volume_name = volume.name @@ -2042,6 +2043,147 @@ class VMAXCommon(object): self.rest.rename_volume( extra_specs[utils.ARRAY], device_id, volume_id) + def manage_existing_snapshot(self, snapshot, existing_ref): + """Manage an existing VMAX Snapshot (import to Cinder). + + Renames the Snapshot to prefix it with OS- to indicate + it is managed by Cinder + + :param snapshot: the snapshot object + :param existing_ref: the snapshot name on the backend VMAX + :raises: VolumeBackendAPIException + :returns: model update + """ + volume = snapshot.volume + extra_specs = self._initial_setup(volume) + array = extra_specs[utils.ARRAY] + device_id = self._find_device_on_array(volume, extra_specs) + + try: + snap_name = existing_ref['source-name'] + except KeyError: + snap_name = existing_ref['source-id'] + + if snapshot.display_name: + snap_display_name = snapshot.display_name + else: + snap_display_name = snapshot.id + + if snap_name.startswith(utils.VOLUME_ELEMENT_NAME_PREFIX): + exception_message = ( + _("Unable to manage existing Snapshot. Snapshot " + "%(snapshot)s is already managed by Cinder.") % + {'snapshot': snap_name}) + raise exception.VolumeBackendAPIException( + data=exception_message) + + if self.utils.is_volume_failed_over(volume): + exception_message = ( + (_("Volume %(name)s is failed over from the source volume, " + "it is not possible to manage a snapshot of a failed over " + "volume.") % {'name': volume.id})) + LOG.exception(exception_message) + raise exception.VolumeBackendAPIException( + data=exception_message) + + if not self.rest.get_volume_snap(array, device_id, snap_name): + exception_message = ( + _("Snapshot %(snap_name)s is not associated with specified " + "volume %(device_id)s, it is not possible to manage a " + "snapshot that is not associated with the specified " + "volume.") + % {'device_id': device_id, 'snap_name': snap_name}) + LOG.exception(exception_message) + raise exception.VolumeBackendAPIException( + data=exception_message) + + snap_backend_name = self.utils.modify_snapshot_prefix( + snap_name, manage=True) + + try: + self.rest.modify_volume_snap( + array, device_id, device_id, snap_name, + extra_specs, rename=True, new_snap_name=snap_backend_name) + + except Exception as e: + exception_message = ( + _("There was an issue managing %(snap_name)s, it was not " + "possible to add the OS- prefix. Error Message: %(e)s.") + % {'snap_name': snap_name, 'e': six.text_type(e)}) + LOG.exception(exception_message) + raise exception.VolumeBackendAPIException(data=exception_message) + + prov_loc = {'source_id': device_id, 'snap_name': snap_backend_name} + + updates = {'display_name': snap_display_name, + 'provider_location': six.text_type(prov_loc)} + + LOG.info("Managing SnapVX Snapshot %(snap_name)s of source " + "volume %(device_id)s, OpenStack Snapshot display name: " + "%(snap_display_name)s", { + 'snap_name': snap_name, 'device_id': device_id, + 'snap_display_name': snap_display_name}) + + return updates + + def manage_existing_snapshot_get_size(self, snapshot): + """Return the size of the source volume for manage-existing-snapshot. + + :param snapshot: the snapshot object + :returns: size of the source volume in GB + """ + volume = snapshot.volume + extra_specs = self._initial_setup(volume) + device_id = self._find_device_on_array(volume, extra_specs) + return self.rest.get_size_of_device_on_array( + extra_specs[utils.ARRAY], device_id) + + def unmanage_snapshot(self, snapshot): + """Export VMAX Snapshot from Cinder. + + Leaves the snapshot intact on the backend VMAX + + :param snapshot: the snapshot object + :raises: VolumeBackendAPIException + """ + volume = snapshot.volume + extra_specs = self._initial_setup(volume) + array = extra_specs[utils.ARRAY] + device_id, snap_name = self._parse_snap_info(array, snapshot) + + if self.utils.is_volume_failed_over(volume): + exception_message = ( + _("It is not possible to unmanage a snapshot where the " + "source volume is failed-over, revert back to source " + "VMAX to unmanage snapshot %(snap_name)s") + % {'snap_name': snap_name}) + + LOG.exception(exception_message) + raise exception.VolumeBackendAPIException( + data=exception_message) + + new_snap_backend_name = self.utils.modify_snapshot_prefix( + snap_name, unmanage=True) + + try: + self.rest.modify_volume_snap( + array, device_id, device_id, snap_name, extra_specs, + rename=True, new_snap_name=new_snap_backend_name) + except Exception as e: + exception_message = ( + _("There was an issue unmanaging Snapshot, it " + "was not possible to remove the OS- prefix. Error " + "message is: %(e)s.") + % {'snap_name': snap_name, 'e': six.text_type(e)}) + LOG.exception(exception_message) + raise exception.VolumeBackendAPIException(data=exception_message) + + self._sync_check(array, device_id, volume.name, extra_specs) + + LOG.info("Snapshot %(snap_name)s is no longer managed in " + "OpenStack but still remains on VMAX source " + "%(array_id)s", {'snap_name': snap_name, 'array_id': array}) + def retype(self, volume, new_type, host): """Migrate volume to another host using retype. diff --git a/cinder/volume/drivers/dell_emc/vmax/fc.py b/cinder/volume/drivers/dell_emc/vmax/fc.py index ce94e581709..281839f8529 100644 --- a/cinder/volume/drivers/dell_emc/vmax/fc.py +++ b/cinder/volume/drivers/dell_emc/vmax/fc.py @@ -87,6 +87,8 @@ class VMAXFCDriver(san.SanDriver, driver.FibreChannelDriver): - Deprecate backend xml configuration - Support for async replication (vmax-replication-enhancements) - Support for SRDF/Metro (vmax-replication-enhancements) + - Support for manage/unmanage snapshots + (vmax-manage-unmanage-snapshot) """ VERSION = "3.1.0" @@ -480,6 +482,36 @@ class VMAXFCDriver(san.SanDriver, driver.FibreChannelDriver): """ return self.common.unmanage(volume) + def manage_existing_snapshot(self, snapshot, existing_ref): + """Manage an existing VMAX Snapshot (import to Cinder). + + Renames the Snapshot to prefix it with OS- to indicate + it is managed by Cinder. + + :param snapshot: the snapshot object + :param existing_ref: the snapshot name on the backend VMAX + :returns: model_update + """ + return self.common.manage_existing_snapshot(snapshot, existing_ref) + + def manage_existing_snapshot_get_size(self, snapshot, existing_ref): + """Return the size of the source volume for manage-existing-snapshot. + + :param snapshot: the snapshot object + :param existing_ref: the snapshot name on the backend VMAX + :returns: size of the source volume in GB + """ + return self.common.manage_existing_snapshot_get_size(snapshot) + + def unmanage_snapshot(self, snapshot): + """Export VMAX Snapshot from Cinder. + + Leaves the snapshot intact on the backend VMAX. + + :param snapshot: the snapshot object + """ + self.common.unmanage_snapshot(snapshot) + def retype(self, ctxt, volume, new_type, diff, host): """Migrate volume to another host using retype. diff --git a/cinder/volume/drivers/dell_emc/vmax/iscsi.py b/cinder/volume/drivers/dell_emc/vmax/iscsi.py index 60d830c63d2..08a84db2f52 100644 --- a/cinder/volume/drivers/dell_emc/vmax/iscsi.py +++ b/cinder/volume/drivers/dell_emc/vmax/iscsi.py @@ -92,6 +92,8 @@ class VMAXISCSIDriver(san.SanISCSIDriver): - Deprecate backend xml configuration - Support for async replication (vmax-replication-enhancements) - Support for SRDF/Metro (vmax-replication-enhancements) + - Support for manage/unmanage snapshots + (vmax-manage-unmanage-snapshot) """ VERSION = "3.1.0" @@ -404,6 +406,36 @@ class VMAXISCSIDriver(san.SanISCSIDriver): """ return self.common.unmanage(volume) + def manage_existing_snapshot(self, snapshot, existing_ref): + """Manage an existing VMAX Snapshot (import to Cinder). + + Renames the Snapshot to prefix it with OS- to indicate + it is managed by Cinder. + + :param snapshot: the snapshot object + :param existing_ref: the snapshot name on the backend VMAX + :returns: model_update + """ + return self.common.manage_existing_snapshot(snapshot, existing_ref) + + def manage_existing_snapshot_get_size(self, snapshot, existing_ref): + """Return the size of the source volume for manage-existing-snapshot. + + :param snapshot: the snapshot object + :param existing_ref: the snapshot name on the backend VMAX + :returns: size of the source volume in GB + """ + return self.common.manage_existing_snapshot_get_size(snapshot) + + def unmanage_snapshot(self, snapshot): + """Export VMAX Snapshot from Cinder. + + Leaves the snapshot intact on the backend VMAX. + + :param snapshot: the snapshot object + """ + self.common.unmanage_snapshot(snapshot) + def retype(self, ctxt, volume, new_type, diff, host): """Migrate volume to another host using retype. diff --git a/cinder/volume/drivers/dell_emc/vmax/rest.py b/cinder/volume/drivers/dell_emc/vmax/rest.py index a0a9e832ec4..5563a00f97a 100644 --- a/cinder/volume/drivers/dell_emc/vmax/rest.py +++ b/cinder/volume/drivers/dell_emc/vmax/rest.py @@ -1540,7 +1540,8 @@ class VMAXRest(object): job, extra_specs) def modify_volume_snap(self, array, source_id, target_id, snap_name, - extra_specs, link=False, unlink=False): + extra_specs, link=False, unlink=False, + rename=False, new_snap_name=None): """Link or unlink a snapVx to or from a target volume. :param array: the array serial number @@ -1550,20 +1551,32 @@ class VMAXRest(object): :param extra_specs: extra specifications :param link: Flag to indicate action = Link :param unlink: Flag to indicate action = Unlink + :param rename: Flag to indicate action = Rename + :param new_snap_name: Optional new snapshot name """ - action = '' + action = None if link: action = "Link" elif unlink: action = "Unlink" - if action: + elif rename: + action = "Rename" + + payload = {} + if action and link or unlink: payload = {"deviceNameListSource": [{"name": source_id}], - "deviceNameListTarget": [ - {"name": target_id}], + "deviceNameListTarget": [{"name": target_id}], "copy": 'true', "action": action, "star": 'false', "force": 'false', "exact": 'false', "remote": 'false', "symforce": 'false', "nocopy": 'false'} + + elif action and rename: + payload = {"deviceNameListSource": [{"name": source_id}], + "deviceNameListTarget": [{"name": source_id}], + "action": action, "newsnapshotname": new_snap_name} + + if action: status_code, job = self.modify_resource( array, REPLICATION, 'snapshot', payload, resource_name=snap_name, private='/private') diff --git a/cinder/volume/drivers/dell_emc/vmax/utils.py b/cinder/volume/drivers/dell_emc/vmax/utils.py index 1fb6fcf5075..83d9f14ab2e 100644 --- a/cinder/volume/drivers/dell_emc/vmax/utils.py +++ b/cinder/volume/drivers/dell_emc/vmax/utils.py @@ -224,6 +224,31 @@ class VMAXUtils(object): {'elementName': element_name}) return element_name + @staticmethod + def modify_snapshot_prefix(snapshot_name, manage=False, unmanage=False): + """Modify a Snapshot prefix on VMAX backend. + + Prepare a snapshot name for manage/unmanage snapshot process either + by adding or removing 'OS-' prefix. + + :param snapshot_name: the old snapshot backend display name + :param manage: (bool) if the operation is managing a snapshot + :param unmanage: (bool) if the operation is unmanaging a snapshot + :return: snapshot name ready for backend VMAX assignment + """ + new_snap_name = None + if manage: + new_snap_name = ("%(prefix)s%(snapshot_name)s" + % {'prefix': 'OS-', + 'snapshot_name': snapshot_name}) + + if unmanage: + snap_split = snapshot_name.split("-", 1) + if snap_split[0] == 'OS': + new_snap_name = snap_split[1] + + return new_snap_name + def generate_unique_trunc_host(self, host_name): """Create a unique short host name under 16 characters. diff --git a/releasenotes/notes/vmax-manage-unmanage-snapshot-3805c4ac64b8133a.yaml b/releasenotes/notes/vmax-manage-unmanage-snapshot-3805c4ac64b8133a.yaml new file mode 100644 index 00000000000..2bac4c5befe --- /dev/null +++ b/releasenotes/notes/vmax-manage-unmanage-snapshot-3805c4ac64b8133a.yaml @@ -0,0 +1,3 @@ +--- +features: + - Support for manage/ unmanage snapshots on VMAX cinder driver.