From cf40a001dac4d2f63165b6e4bbd14acb1d09ed54 Mon Sep 17 00:00:00 2001 From: Helen Walsh Date: Thu, 16 Nov 2017 17:11:22 +0000 Subject: [PATCH] VMAX driver - revert a volume to last snapshot VMAX Support for reverting a volume to last snapshot. Change-Id: I1a4b5c000d1b2fc50ed89f9ad874e9e6c62f0290 Implements: blueprint vmax-revert-volume-to-snapshot --- .../volume/drivers/dell_emc/vmax/test_vmax.py | 118 +++++++++++++++++- cinder/volume/drivers/dell_emc/vmax/common.py | 45 +++++++ cinder/volume/drivers/dell_emc/vmax/fc.py | 10 ++ cinder/volume/drivers/dell_emc/vmax/iscsi.py | 10 ++ .../volume/drivers/dell_emc/vmax/provision.py | 89 ++++++++++++- cinder/volume/drivers/dell_emc/vmax/rest.py | 34 +++-- ...t-volume-to-snapshot-b4a837d84a8b2a85.yaml | 4 + 7 files changed, 297 insertions(+), 13 deletions(-) create mode 100644 releasenotes/notes/vmax-revert-volume-to-snapshot-b4a837d84a8b2a85.yaml 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 1262fa2267e..653336d2ff9 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 @@ -35,6 +35,7 @@ from cinder import test 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 import utils as test_utils from cinder.volume.drivers.dell_emc.vmax import common from cinder.volume.drivers.dell_emc.vmax import fc from cinder.volume.drivers.dell_emc.vmax import iscsi @@ -2419,6 +2420,10 @@ class VMAXRestTest(test.TestCase): "star": 'false', "force": 'false', "exact": 'false', "remote": 'false', "symforce": 'false', "nocopy": 'false'} + payload_restore = {"deviceNameListSource": [{"name": source_id}], + "deviceNameListTarget": [{"name": source_id}], + "action": "Restore", + "star": 'false', "force": 'false'} with mock.patch.object( self.rest, 'modify_resource', return_value=( 202, self.data.job_list[0])) as mock_modify: @@ -2438,6 +2443,15 @@ class VMAXRestTest(test.TestCase): self.rest.modify_resource.assert_called_once_with( array, 'replication', 'snapshot', payload, resource_name=snap_name, private='/private') + # restore + mock_modify.reset_mock() + payload["action"] = "Restore" + self.rest.modify_volume_snap( + array, source_id, "", snap_name, + extra_specs, unlink=False, restore=True) + self.rest.modify_resource.assert_called_once_with( + array, 'replication', 'snapshot', payload_restore, + resource_name=snap_name, private='/private') # none selected mock_modify.reset_mock() self.rest.modify_volume_snap( @@ -2457,6 +2471,20 @@ class VMAXRestTest(test.TestCase): array, 'replication', 'snapshot', snap_name, payload=payload, private='/private') + def test_delete_volume_snap_restore(self): + array = self.data.array + snap_name = (self.data.volume_snap_vx + ['snapshotSrcs'][0]['snapshotName']) + source_device_id = self.data.device_id + payload = {"deviceNameListSource": [{"name": source_device_id}], + "restore": True} + with mock.patch.object(self.rest, 'delete_resource'): + self.rest.delete_volume_snap( + array, snap_name, source_device_id, restored=True) + self.rest.delete_resource.assert_called_once_with( + array, 'replication', 'snapshot', snap_name, + payload=payload, private='/private') + def test_get_volume_snap_info(self): array = self.data.array source_device_id = self.data.device_id @@ -2808,6 +2836,8 @@ class VMAXRestTest(test.TestCase): is_next_gen2 = self.rest.is_next_gen_array(self.data.array_herc) self.assertTrue(is_next_gen2) + @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', + new=test_utils.ZeroIntervalLoopingCall) @mock.patch.object(rest.VMAXRest, 'are_vols_rdf_paired', side_effect=[('', '', 'syncinprog'), ('', '', 'consistent'), @@ -2849,7 +2879,6 @@ class VMAXProvisionTest(test.TestCase): configuration = FakeConfiguration(self.fake_xml, config_group) rest.VMAXRest._establish_rest_session = mock.Mock( return_value=FakeRequestsSession()) - provision.UNLINK_INTERVAL = 0 driver = iscsi.VMAXISCSIDriver(configuration=configuration) self.driver = driver self.common = self.driver.common @@ -2949,6 +2978,8 @@ class VMAXProvisionTest(test.TestCase): array, source_device_id, target_device_id, snap_name, extra_specs, unlink=True)) + @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', + new=test_utils.ZeroIntervalLoopingCall) def test_unlink_volume(self): with mock.patch.object(self.rest, 'modify_volume_snap') as mock_mod: self.provision._unlink_volume( @@ -2959,6 +2990,8 @@ class VMAXProvisionTest(test.TestCase): self.data.snap_location['snap_name'], self.data.extra_specs, unlink=True) + @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', + new=test_utils.ZeroIntervalLoopingCall) def test_unlink_volume_exception(self): with mock.patch.object( self.rest, 'modify_volume_snap', side_effect=[ @@ -2977,7 +3010,70 @@ class VMAXProvisionTest(test.TestCase): self.provision.delete_volume_snap( array, snap_name, source_device_id) self.provision.rest.delete_volume_snap.assert_called_once_with( - array, snap_name, source_device_id) + array, snap_name, source_device_id, False) + + def test_delete_volume_snap_restore(self): + array = self.data.array + source_device_id = self.data.device_id + snap_name = self.data.snap_location['snap_name'] + restored = True + with mock.patch.object(self.provision.rest, 'delete_volume_snap'): + self.provision.delete_volume_snap( + array, snap_name, source_device_id, restored) + self.provision.rest.delete_volume_snap.assert_called_once_with( + array, snap_name, source_device_id, True) + + @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', + new=test_utils.ZeroIntervalLoopingCall) + def test_restore_complete(self): + array = self.data.array + source_device_id = self.data.device_id + snap_name = self.data.snap_location['snap_name'] + extra_specs = self.data.extra_specs + with mock.patch.object( + self.provision, '_is_restore_complete', + return_value=True): + isrestored = self.provision.is_restore_complete( + array, source_device_id, snap_name, extra_specs) + self.assertTrue(isrestored) + with mock.patch.object( + self.provision, '_is_restore_complete', + side_effect=exception.CinderException): + self.assertRaises(exception.VolumeBackendAPIException, + self.provision.is_restore_complete, + array, source_device_id, snap_name, extra_specs) + + def test_is_restore_complete(self): + array = self.data.array + source_device_id = self.data.device_id + snap_name = self.data.snap_location['snap_name'] + snap_details = { + 'linkedDevices': + [{'targetDevice': source_device_id, 'state': "Restored"}]} + with mock.patch.object(self.provision.rest, + 'get_volume_snap', return_value=snap_details): + isrestored = self.provision._is_restore_complete( + array, source_device_id, snap_name) + self.assertTrue(isrestored) + snap_details['linkedDevices'][0]['state'] = "Restoring" + with mock.patch.object(self.provision.rest, + 'get_volume_snap', return_value=snap_details): + isrestored = self.provision._is_restore_complete( + array, source_device_id, snap_name) + self.assertFalse(isrestored) + + def test_revert_volume_snapshot(self): + array = self.data.array + source_device_id = self.data.device_id + snap_name = self.data.snap_location['snap_name'] + extra_specs = self.data.extra_specs + with mock.patch.object( + self.provision.rest, 'modify_volume_snap', return_value=None): + self.provision.revert_volume_snapshot( + array, source_device_id, snap_name, extra_specs) + self.provision.rest.modify_volume_snap.assert_called_once_with( + array, source_device_id, "", snap_name, + extra_specs, restore=True) def test_extend_volume(self): array = self.data.array @@ -4866,6 +4962,24 @@ class VMAXCommonTest(test.TestCase): self.common.unmanage_snapshot, self.data.test_snapshot_manage) + @mock.patch.object(provision.VMAXProvision, 'is_restore_complete', + return_value=True) + @mock.patch.object(common.VMAXCommon, '_sync_check') + @mock.patch.object(provision.VMAXProvision, + 'revert_volume_snapshot') + def test_revert_to_snapshot(self, mock_revert, mock_sync, mock_complete): + volume = self.data.test_volume + snapshot = self.data.test_snapshot + array = self.data.array + device_id = self.data.device_id + snap_name = self.data.snap_location['snap_name'] + extra_specs = deepcopy(self.data.extra_specs_intervals_set) + extra_specs['storagetype:portgroupname'] = ( + self.data.port_group_name_f) + self.common.revert_to_snapshot(volume, snapshot) + mock_revert.assert_called_once_with( + array, device_id, snap_name, extra_specs) + 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 9b0c81e72e4..f263934df59 100644 --- a/cinder/volume/drivers/dell_emc/vmax/common.py +++ b/cinder/volume/drivers/dell_emc/vmax/common.py @@ -4079,3 +4079,48 @@ class VMAXCommon(object): if slo is not None: kwargs.update({'ServiceLevel': slo, 'Workload': workload}) return kwargs + + def revert_to_snapshot(self, volume, snapshot): + """Revert volume to snapshot. + + :param volume: the volume object + :param snapshot: the snapshot object + """ + extra_specs = self._initial_setup(volume) + array = extra_specs[utils.ARRAY] + sourcedevice_id, snap_name = self._parse_snap_info( + array, snapshot) + if not sourcedevice_id or not snap_name: + LOG.error("No snapshot found on the array") + exception_message = (_( + "Failed to revert the volume to the snapshot")) + raise exception.VolumeDriverException(data=exception_message) + self._sync_check(array, sourcedevice_id, volume.name, extra_specs) + try: + LOG.info("Reverting device: %(deviceid)s " + "to snapshot: %(snapname)s.", + {'deviceid': sourcedevice_id, 'snapname': snap_name}) + self.provision.revert_volume_snapshot( + array, sourcedevice_id, snap_name, extra_specs) + # Once the restore is done, we need to check if it is complete + restore_complete = self.provision.is_restore_complete( + array, sourcedevice_id, snap_name, extra_specs) + if not restore_complete: + LOG.debug("Restore couldn't complete in the specified " + "time interval. The terminate restore may fail") + LOG.debug("Terminating restore session") + # This may throw an exception if restore_complete is False + self.provision.delete_volume_snap( + array, snap_name, sourcedevice_id, restored=True) + # Revert volume to snapshot is successful if termination was + # successful - possible even if restore_complete was False + # when we checked last. + LOG.debug("Restored session was terminated") + LOG.info("Reverted the volume to snapshot successfully") + except Exception as e: + exception_message = (_( + "Failed to revert the volume to the snapshot" + "Exception received was %(e)s") % {'e': six.text_type(e)}) + LOG.error(exception_message) + raise exception.VolumeBackendAPIException( + data=exception_message) diff --git a/cinder/volume/drivers/dell_emc/vmax/fc.py b/cinder/volume/drivers/dell_emc/vmax/fc.py index 281839f8529..2813864b69d 100644 --- a/cinder/volume/drivers/dell_emc/vmax/fc.py +++ b/cinder/volume/drivers/dell_emc/vmax/fc.py @@ -89,6 +89,7 @@ class VMAXFCDriver(san.SanDriver, driver.FibreChannelDriver): - Support for SRDF/Metro (vmax-replication-enhancements) - Support for manage/unmanage snapshots (vmax-manage-unmanage-snapshot) + - Support for revert to volume snapshot """ VERSION = "3.1.0" @@ -638,3 +639,12 @@ class VMAXFCDriver(san.SanDriver, driver.FibreChannelDriver): """ return self.common.failover_replication( context, group, volumes, secondary_backend_id) + + def revert_to_snapshot(self, context, volume, snapshot): + """Revert volume to snapshot + + :param context: the context + :param volume: the cinder volume object + :param snapshot: the cinder snapshot object + """ + self.common.revert_to_snapshot(volume, snapshot) diff --git a/cinder/volume/drivers/dell_emc/vmax/iscsi.py b/cinder/volume/drivers/dell_emc/vmax/iscsi.py index 08a84db2f52..a9c1921c8fc 100644 --- a/cinder/volume/drivers/dell_emc/vmax/iscsi.py +++ b/cinder/volume/drivers/dell_emc/vmax/iscsi.py @@ -94,6 +94,7 @@ class VMAXISCSIDriver(san.SanISCSIDriver): - Support for SRDF/Metro (vmax-replication-enhancements) - Support for manage/unmanage snapshots (vmax-manage-unmanage-snapshot) + - Support for revert to volume snapshot """ VERSION = "3.1.0" @@ -562,3 +563,12 @@ class VMAXISCSIDriver(san.SanISCSIDriver): """ return self.common.failover_replication( context, group, volumes, secondary_backend_id) + + def revert_to_snapshot(self, context, volume, snapshot): + """Revert volume to snapshot + + :param context: the context + :param volume: the cinder volume object + :param snapshot: the cinder snapshot object + """ + self.common.revert_to_snapshot(volume, snapshot) diff --git a/cinder/volume/drivers/dell_emc/vmax/provision.py b/cinder/volume/drivers/dell_emc/vmax/provision.py index 587ad96e0ac..739a9d70880 100644 --- a/cinder/volume/drivers/dell_emc/vmax/provision.py +++ b/cinder/volume/drivers/dell_emc/vmax/provision.py @@ -214,16 +214,85 @@ class VMAXProvision(object): rc = timer.start(interval=UNLINK_INTERVAL).wait() return rc - def delete_volume_snap(self, array, snap_name, source_device_id): + def delete_volume_snap(self, array, snap_name, + source_device_id, restored=False): """Delete a snapVx snapshot of a volume. :param array: the array serial number :param snap_name: the snapshot name :param source_device_id: the source device id + :param restored: Flag to indicate if restored session is being deleted """ LOG.debug("Delete SnapVx: %(snap_name)s for volume %(vol)s.", {'vol': source_device_id, 'snap_name': snap_name}) - self.rest.delete_volume_snap(array, snap_name, source_device_id) + self.rest.delete_volume_snap( + array, snap_name, source_device_id, restored) + + def is_restore_complete(self, array, source_device_id, + snap_name, extra_specs): + """Check and wait for a restore to complete + + :param array: the array serial number + :param source_device_id: source device id + :param snap_name: snapshot name + :param extra_specs: extra specification + :returns: bool + """ + + def _wait_for_restore(): + """Called at an interval until the restore is finished. + + :raises: loopingcall.LoopingCallDone + :raises: VolumeBackendAPIException + """ + retries = kwargs['retries'] + try: + kwargs['retries'] = retries + 1 + if not kwargs['wait_for_restore_called']: + if self._is_restore_complete( + array, source_device_id, snap_name): + kwargs['wait_for_restore_called'] = True + except Exception: + exception_message = (_("Issue encountered waiting for " + "restore.")) + LOG.exception(exception_message) + raise exception.VolumeBackendAPIException( + data=exception_message) + + if kwargs['wait_for_restore_called']: + raise loopingcall.LoopingCallDone() + if kwargs['retries'] > int(extra_specs[utils.RETRIES]): + LOG.error("_wait_for_restore failed after %(retries)d " + "tries.", {'retries': retries}) + raise loopingcall.LoopingCallDone( + retvalue=int(extra_specs[utils.RETRIES])) + + kwargs = {'retries': 0, + 'wait_for_restore_called': False} + timer = loopingcall.FixedIntervalLoopingCall(_wait_for_restore) + rc = timer.start(interval=int(extra_specs[utils.INTERVAL])).wait() + return rc + + def _is_restore_complete(self, array, source_device_id, snap_name): + """Helper function to check if restore is complete. + + :param array: the array serial number + :param source_device_id: source device id + :param snap_name: the snapshot name + :returns: restored -- bool + """ + restored = False + snap_details = self.rest.get_volume_snap( + array, source_device_id, snap_name) + if snap_details: + linked_devices = snap_details.get("linkedDevices", []) + for linked_device in linked_devices: + if ('targetDevice' in linked_device and + source_device_id == linked_device['targetDevice']): + if ('state' in linked_device and + linked_device['state'] == "Restored"): + restored = True + return restored def delete_temp_volume_snap(self, array, snap_name, source_device_id): """Delete the temporary snapshot created for clone operations. @@ -739,3 +808,19 @@ class VMAXProvision(object): {'sg': storagegroup_name}) self.rest.delete_storagegroup_rdf( array, storagegroup_name, rdf_group_num) + + def revert_volume_snapshot(self, array, source_device_id, + snap_name, extra_specs): + """Revert a volume snapshot + + :param array: the array serial number + :param source_device_id: device id of the source + :param snap_name: snapvx snapshot name + :param extra_specs: the extra specifications + """ + start_time = time.time() + self.rest.modify_volume_snap( + array, source_device_id, "", snap_name, extra_specs, restore=True) + LOG.debug("Restore volume snapshot took: %(delta)s H:MM:SS.", + {'delta': self.utils.get_time_delta(start_time, + time.time())}) diff --git a/cinder/volume/drivers/dell_emc/vmax/rest.py b/cinder/volume/drivers/dell_emc/vmax/rest.py index 5563a00f97a..994f944f79d 100644 --- a/cinder/volume/drivers/dell_emc/vmax/rest.py +++ b/cinder/volume/drivers/dell_emc/vmax/rest.py @@ -1541,8 +1541,8 @@ class VMAXRest(object): def modify_volume_snap(self, array, source_id, target_id, snap_name, extra_specs, link=False, unlink=False, - rename=False, new_snap_name=None): - """Link or unlink a snapVx to or from a target volume. + rename=False, new_snap_name=None, restore=False): + """Modify a snapvx snapshot :param array: the array serial number :param source_id: the source device id @@ -1553,17 +1553,28 @@ class VMAXRest(object): :param unlink: Flag to indicate action = Unlink :param rename: Flag to indicate action = Rename :param new_snap_name: Optional new snapshot name + :param restore: Flag to indicate action = Restore """ action = None + operation = '' if link: action = "Link" elif unlink: action = "Unlink" elif rename: action = "Rename" + elif restore: + action = "Restore" payload = {} - if action and link or unlink: + if action == "Restore": + operation = 'Restore snapVx snapshot' + payload = {"deviceNameListSource": [{"name": source_id}], + "deviceNameListTarget": [{"name": source_id}], + "action": action, + "star": 'false', "force": 'false'} + elif action in ('Link', 'Unlink'): + operation = 'Modify snapVx relationship to target' payload = {"deviceNameListSource": [{"name": source_id}], "deviceNameListTarget": [{"name": target_id}], "copy": 'true', "action": action, @@ -1571,7 +1582,8 @@ class VMAXRest(object): "exact": 'false', "remote": 'false', "symforce": 'false', "nocopy": 'false'} - elif action and rename: + elif action == "Rename": + operation = 'Rename snapVx snapshot' payload = {"deviceNameListSource": [{"name": source_id}], "deviceNameListTarget": [{"name": source_id}], "action": action, "newsnapshotname": new_snap_name} @@ -1580,18 +1592,22 @@ class VMAXRest(object): status_code, job = self.modify_resource( array, REPLICATION, 'snapshot', payload, resource_name=snap_name, private='/private') + self.wait_for_job(operation, status_code, job, extra_specs) - self.wait_for_job('Modify snapVx relationship to target', - status_code, job, extra_specs) - - def delete_volume_snap(self, array, snap_name, source_device_id): + def delete_volume_snap(self, array, snap_name, + source_device_id, restored=False): """Delete the snapshot of a volume. :param array: the array serial number :param snap_name: the name of the snapshot :param source_device_id: the source device id + :param restored: Flag to indicate terminate restore session """ - payload = {"deviceNameListSource": [{"name": source_device_id}]} + if restored: + payload = {"deviceNameListSource": [{"name": source_device_id}], + "restore": True} + else: + payload = {"deviceNameListSource": [{"name": source_device_id}]} return self.delete_resource( array, REPLICATION, 'snapshot', snap_name, payload=payload, private='/private') diff --git a/releasenotes/notes/vmax-revert-volume-to-snapshot-b4a837d84a8b2a85.yaml b/releasenotes/notes/vmax-revert-volume-to-snapshot-b4a837d84a8b2a85.yaml new file mode 100644 index 00000000000..e0b525e9a5c --- /dev/null +++ b/releasenotes/notes/vmax-revert-volume-to-snapshot-b4a837d84a8b2a85.yaml @@ -0,0 +1,4 @@ +--- +features: + - Support for reverting a volume to a previous snapshot in VMAX cinder + driver.