From 6864201def963ce20e10c0ceddfd4ee776269730 Mon Sep 17 00:00:00 2001 From: John Griffith Date: Fri, 29 May 2015 23:41:25 +0000 Subject: [PATCH] Use SolidFire snapshots for Cinder snapshots This patch just replaces the use of clones on the SolidFire backend for SolidFire Snapshot objects. This isn't necessarily an issue, but it is important necessary to do things like consistency group snaps. Note, that we have to keep compatability for things like create_from_snapshot where the system may have clones that were used for snapshots prior to updating the driver. While we're here, we also removed the logging calls in the test which shouldn't really be there, as well as the overly chatty debug messages like "entering/leaving". More cleanup work can be done to logs but should be in their own patch. Change-Id: Ia67b2b9e3c0bf3b0b1f991e958f8eac13830f9cc --- cinder/tests/unit/test_solidfire.py | 37 ++++---- cinder/volume/drivers/solidfire.py | 141 +++++++++++++++++----------- 2 files changed, 102 insertions(+), 76 deletions(-) diff --git a/cinder/tests/unit/test_solidfire.py b/cinder/tests/unit/test_solidfire.py index 3752d5e896c..a7bfc1312bd 100644 --- a/cinder/tests/unit/test_solidfire.py +++ b/cinder/tests/unit/test_solidfire.py @@ -18,7 +18,6 @@ import datetime import mock from mox3 import mox -from oslo_log import log as logging from oslo_utils import timeutils from oslo_utils import units @@ -30,8 +29,6 @@ from cinder.volume.drivers import solidfire from cinder.volume import qos_specs from cinder.volume import volume_types -LOG = logging.getLogger(__name__) - def create_configuration(): configuration = mox.MockObject(conf.Configuration) @@ -101,7 +98,6 @@ class SolidFireVolumeTestCase(test.TestCase): def fake_issue_api_request(obj, method, params, version='1.0'): if method is 'GetClusterCapacity' and version == '1.0': - LOG.info('Called Fake GetClusterCapacity...') data = {'result': {'clusterCapacity': {'maxProvisionedSpace': 107374182400, 'usedSpace': 1073741824, @@ -111,7 +107,6 @@ class SolidFireVolumeTestCase(test.TestCase): return data elif method is 'GetClusterInfo' and version == '1.0': - LOG.info('Called Fake GetClusterInfo...') results = {'result': {'clusterInfo': {'name': 'fake-cluster', 'mvip': '1.1.1.1', @@ -122,11 +117,9 @@ class SolidFireVolumeTestCase(test.TestCase): return results elif method is 'AddAccount' and version == '1.0': - LOG.info('Called Fake AddAccount...') return {'result': {'accountID': 25}, 'id': 1} elif method is 'GetAccountByName' and version == '1.0': - LOG.info('Called Fake GetAccountByName...') results = {'result': {'account': {'accountID': 25, 'username': params['username'], @@ -139,15 +132,15 @@ class SolidFireVolumeTestCase(test.TestCase): return results elif method is 'CreateVolume' and version == '1.0': - LOG.info('Called Fake CreateVolume...') return {'result': {'volumeID': 5}, 'id': 1} + elif method is 'CreateSnapshot' and version == '6.0': + return {'result': {'snapshotID': 5}, 'id': 1} + elif method is 'DeleteVolume' and version == '1.0': - LOG.info('Called Fake DeleteVolume...') return {'result': {}, 'id': 1} elif method is 'ModifyVolume' and version == '5.0': - LOG.info('Called Fake ModifyVolume...') return {'result': {}, 'id': 1} elif method is 'CloneVolume': @@ -158,7 +151,6 @@ class SolidFireVolumeTestCase(test.TestCase): elif method is 'ListVolumesForAccount' and version == '1.0': test_name = 'OS-VOLID-a720b3c0-d1f0-11e1-9b23-0800200c9a66' - LOG.info('Called Fake ListVolumesForAccount...') result = {'result': { 'volumes': [{'volumeID': 5, 'name': test_name, @@ -189,7 +181,8 @@ class SolidFireVolumeTestCase(test.TestCase): 'iqn': test_name}]}} return result else: - LOG.error('Crap, unimplemented API call in Fake:%s' % method) + # Crap, unimplemented API call in Fake + return None def fake_issue_api_request_fails(obj, method, params, version='1.0', @@ -265,13 +258,7 @@ class SolidFireVolumeTestCase(test.TestCase): '4096 4096') self.configuration.sf_emulate_512 = True - def test_create_snapshot(self): - self.stubs.Set(solidfire.SolidFireDriver, - '_issue_api_request', - self.fake_issue_api_request) - self.stubs.Set(solidfire.SolidFireDriver, - '_get_model_info', - self.fake_get_model_info) + def test_create_delete_snapshot(self): testvol = {'project_id': 'testprjid', 'name': 'testvol', 'size': 1, @@ -290,6 +277,11 @@ class SolidFireVolumeTestCase(test.TestCase): sfv = solidfire.SolidFireDriver(configuration=self.configuration) sfv.create_volume(testvol) sfv.create_snapshot(testsnap) + with mock.patch.object(solidfire.SolidFireDriver, + '_get_sf_snapshots', + return_value=[{'snapshotID': '1', + 'name': 'testvol'}]): + sfv.delete_snapshot(testsnap) def test_create_clone(self): self.stubs.Set(solidfire.SolidFireDriver, @@ -312,8 +304,11 @@ class SolidFireVolumeTestCase(test.TestCase): 'volume_type_id': None, 'created_at': timeutils.utcnow()} - sfv = solidfire.SolidFireDriver(configuration=self.configuration) - sfv.create_cloned_volume(testvol_b, testvol) + with mock.patch.object(solidfire.SolidFireDriver, + '_get_sf_snapshots', + return_value=[]): + sfv = solidfire.SolidFireDriver(configuration=self.configuration) + sfv.create_cloned_volume(testvol_b, testvol) def test_initialize_connector_with_blocksizes(self): connector = {'initiator': 'iqn.2012-07.org.fake:01'} diff --git a/cinder/volume/drivers/solidfire.py b/cinder/volume/drivers/solidfire.py index 620d5f97735..4629ff37547 100644 --- a/cinder/volume/drivers/solidfire.py +++ b/cinder/volume/drivers/solidfire.py @@ -343,7 +343,9 @@ class SolidFireDriver(san.SanISCSIDriver): return model_update - def _do_clone_volume(self, src_uuid, src_project_id, v_ref): + def _do_clone_volume(self, src_uuid, + src_project_id, + v_ref): """Create a clone of an existing volume. Currently snapshots are the same as clones on the SF cluster. @@ -353,17 +355,11 @@ class SolidFireDriver(san.SanISCSIDriver): implemented in the pre-release version of the SolidFire Cluster. """ + attributes = {} qos = {} sfaccount = self._get_sfaccount(src_project_id) - params = {'accountID': sfaccount['accountID']} - - sf_vol = self._get_sf_volume(src_uuid, params) - - if sf_vol is None: - raise exception.VolumeNotFound(volume_id=src_uuid) - if src_project_id != v_ref['project_id']: sfaccount = self._create_sfaccount(v_ref['project_id']) @@ -372,12 +368,30 @@ class SolidFireDriver(san.SanISCSIDriver): else: new_size = v_ref['volume_size'] - params = {'volumeID': int(sf_vol['volumeID']), - 'name': 'UUID-%s' % v_ref['id'], + params = {'name': 'UUID-%s' % v_ref['id'], 'newSize': int(new_size * units.Gi), 'newAccountID': sfaccount['accountID']} - data = self._issue_api_request('CloneVolume', params) + # NOTE(jdg): First check the SF snapshots + # if we don't find a snap by the given name, just move on to check + # volumes. This may be a running system that was updated from + # before we did snapshots, so need to check both + snap_name = 'UUID-%s' % src_uuid + snaps = self._get_sf_snapshots() + snap = next((s for s in snaps if s["name"] == snap_name), None) + is_clone = False + if snap: + params['snapshotID'] = int(snap['snapshotID']) + params['volumeID'] = int(snap['volumeID']) + else: + sf_vol = self._get_sf_volume( + src_uuid, {'accountID': sfaccount['accountID']}) + if sf_vol is None: + raise exception.VolumeNotFound(volume_id=src_uuid) + params['volumeID'] = int(sf_vol['volumeID']) + is_clone = True + + data = self._issue_api_request('CloneVolume', params, version='6.0') if (('result' not in data) or ('volumeID' not in data['result'])): msg = _("API response: %s") % data raise exception.SolidFireAPIException(msg) @@ -415,19 +429,20 @@ class SolidFireDriver(san.SanISCSIDriver): raise exception.SolidFireAPIException(mesg) # Increment the usage count, just for data collection - cloned_count = sf_vol['attributes'].get('cloned_count', 0) - cloned_count += 1 - attributes = sf_vol['attributes'] - attributes['cloned_count'] = cloned_count + # We're only doing this for clones, not create_from snaps + if is_clone: + cloned_count = sf_vol['attributes'].get('cloned_count', 0) + cloned_count += 1 + attributes = sf_vol['attributes'] + attributes['cloned_count'] = cloned_count - params = {'volumeID': int(sf_vol['volumeID'])} - params['attributes'] = attributes - data = self._issue_api_request('ModifyVolume', params) + params = {'volumeID': int(sf_vol['volumeID'])} + params['attributes'] = attributes + data = self._issue_api_request('ModifyVolume', params) return (data, sfaccount, model_update) def _do_volume_create(self, project_id, params): sfaccount = self._create_sfaccount(project_id) - params['accountID'] = sfaccount['accountID'] data = self._issue_api_request('CreateVolume', params) @@ -438,6 +453,13 @@ class SolidFireDriver(san.SanISCSIDriver): sf_volume_id = data['result']['volumeID'] return self._get_model_info(sfaccount, sf_volume_id) + def _do_snapshot_create(self, params): + data = self._issue_api_request('CreateSnapshot', params, version='6.0') + if (('result' not in data) or ('snapshotID' not in data['result'])): + msg = _("Failed snapshot create: %s") % data + raise exception.SolidFireAPIException(msg) + return data['result']['snapshotID'] + def _set_qos_presets(self, volume): qos = {} valid_presets = self.sf_qos_dict.keys() @@ -480,9 +502,6 @@ class SolidFireDriver(san.SanISCSIDriver): return qos def _get_sf_volume(self, uuid, params): - # TODO(jdg): Going to fix this shortly to not iterate - # but instead use the cinder UUID and our internal - # mapping to get this more efficiently data = self._issue_api_request('ListVolumesForAccount', params) if 'result' not in data: msg = _("Failed to get SolidFire Volume: %s") % data @@ -518,6 +537,16 @@ class SolidFireDriver(san.SanISCSIDriver): return sf_volref + def _get_sf_snapshots(self, sf_volid=None): + params = {} + if sf_volid: + params = {'volumeID': sf_volid} + data = self._issue_api_request('ListSnapshots', params, version='6.0') + if 'result' not in data: + msg = _("Failed to get SolidFire Snapshot: %s") % data + raise exception.SolidFireAPIException(msg) + return data['result']['snapshots'] + def _create_image_volume(self, context, image_meta, image_service, image_id): @@ -743,9 +772,6 @@ class SolidFireDriver(san.SanISCSIDriver): volumeID is what's guaranteed unique. """ - - LOG.debug("Enter SolidFire delete_volume...") - sfaccount = self._get_sfaccount(volume['project_id']) if sfaccount is None: LOG.error(_LE("Account for Volume ID %s was not found on " @@ -756,7 +782,6 @@ class SolidFireDriver(san.SanISCSIDriver): return params = {'accountID': sfaccount['accountID']} - sf_vol = self._get_sf_volume(volume['id'], params) if sf_vol is not None: @@ -771,11 +796,8 @@ class SolidFireDriver(san.SanISCSIDriver): "the SolidFire Cluster while attempting " "delete_volume operation!"), volume['id']) - LOG.debug("Leaving SolidFire delete_volume") - def ensure_export(self, context, volume): """Verify the iscsi export info.""" - LOG.debug("Executing SolidFire ensure_export...") try: return self._do_export(volume) except exception.SolidFireAPIException: @@ -783,29 +805,50 @@ class SolidFireDriver(san.SanISCSIDriver): def create_export(self, context, volume): """Setup the iscsi export info.""" - LOG.debug("Executing SolidFire create_export...") return self._do_export(volume) def delete_snapshot(self, snapshot): """Delete the specified snapshot from the SolidFire cluster.""" - self.delete_volume(snapshot) + sf_snap_name = 'UUID-%s' % snapshot['id'] + sfaccount = self._get_sfaccount(snapshot['project_id']) + params = {'accountID': sfaccount['accountID'], + 'name': sf_snap_name} + params = {'accountID': sfaccount['accountID']} + + # Get the parent volume of the snapshot + sf_vol = self._get_sf_volume(snapshot['volume_id'], params) + sf_snaps = self._get_sf_snapshots(sf_vol['volumeID']) + snap = next((s for s in sf_snaps if s["name"] == sf_snap_name), None) + if snap: + params = {'snapshotID': snap['snapshotID']} + data = self._issue_api_request('DeleteSnapshot', + params, + version='6.0') + if 'result' not in data: + msg = (_("Failed to delete SolidFire Snapshot: %s") % + data) + raise exception.SolidFireAPIException(msg) + else: + # Make sure it's not "old style" using clones as snaps + LOG.debug("Snapshot not found, checking old style clones.") + self.delete_volume(snapshot) def create_snapshot(self, snapshot): - """Create a snapshot of a volume on the SolidFire cluster. + sfaccount = self._get_sfaccount(snapshot['project_id']) + if sfaccount is None: + LOG.error(_LE("Account for Volume ID %s was not found on " + "the SolidFire Cluster while attempting " + "create_snapshot operation!"), snapshot['volume_id']) - Note that for SolidFire Clusters currently there is no snapshot - implementation. Due to the way SF does cloning there's no performance - hit or extra space used. The only thing that's lacking from this is - the ability to restore snaps. + params = {'accountID': sfaccount['accountID']} + sf_vol = self._get_sf_volume(snapshot['volume_id'], params) - After GA a true snapshot implementation will be available with - restore at which time we'll rework this appropriately. + if sf_vol is None: + raise exception.VolumeNotFound(volume_id=snapshot['volume_id']) + params = {'volumeID': sf_vol['volumeID'], + 'name': 'UUID-%s' % snapshot['id']} - """ - (_data, _sfaccount, _model) = self._do_clone_volume( - snapshot['volume_id'], - snapshot['project_id'], - snapshot) + self._do_snapshot_create(params) def create_volume_from_snapshot(self, volume, snapshot): """Create a volume from the specified snapshot.""" @@ -834,8 +877,6 @@ class SolidFireDriver(san.SanISCSIDriver): def extend_volume(self, volume, new_size): """Extend an existing volume.""" - LOG.debug("Entering SolidFire extend_volume...") - sfaccount = self._get_sfaccount(volume['project_id']) params = {'accountID': sfaccount['accountID']} @@ -857,13 +898,8 @@ class SolidFireDriver(san.SanISCSIDriver): if 'result' not in data: raise exception.SolidFireAPIDataException(data=data) - LOG.debug("Leaving SolidFire extend_volume") - def _update_cluster_status(self): """Retrieve status info for the Cluster.""" - - LOG.debug("Updating cluster status info") - params = {} # NOTE(jdg): The SF api provides an UNBELIEVABLE amount @@ -901,7 +937,6 @@ class SolidFireDriver(san.SanISCSIDriver): instance_uuid, host_name, mountpoint): - LOG.debug("Entering SolidFire attach_volume...") sfaccount = self._get_sfaccount(volume['project_id']) params = {'accountID': sfaccount['accountID']} @@ -926,8 +961,6 @@ class SolidFireDriver(san.SanISCSIDriver): raise exception.SolidFireAPIDataException(data=data) def detach_volume(self, context, volume, attachment=None): - - LOG.debug("Entering SolidFire attach_volume...") sfaccount = self._get_sfaccount(volume['project_id']) params = {'accountID': sfaccount['accountID']} @@ -980,7 +1013,6 @@ class SolidFireDriver(san.SanISCSIDriver): volume['project_id'] = new_project volume['user_id'] = new_user model_update = self._do_export(volume) - LOG.debug("Leaving SolidFire transfer volume") return model_update def retype(self, ctxt, volume, new_type, diff, host): @@ -1101,7 +1133,6 @@ class SolidFireDriver(san.SanISCSIDriver): def unmanage(self, volume): """Mark SolidFire Volume as unmanaged (export from Cinder).""" - LOG.debug("Enter SolidFire unmanage...") sfaccount = self._get_sfaccount(volume['project_id']) if sfaccount is None: LOG.error(_LE("Account for Volume ID %s was not found on "