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

diff --git a/cinder/tests/unit/test_solidfire.py b/cinder/tests/unit/test_solidfire.py
index 3752d5e896ce8f222d30f7b949bad61c68dc5112..a7bfc1312bdf2771a7250efd7b27afe667fc395a 100644 (file)
--- 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 620d5f9773505803fdd2a463253c3182443416e1..4629ff37547a0bc9427b9bb279187d383022ad22 100644 (file)
--- 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
-
-        params = {'volumeID': int(sf_vol['volumeID'])}
-        params['attributes'] = attributes
-        data = self._issue_api_request('ModifyVolume', params)
+        # 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)
         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 "