Merge "Add lock decorator to SolidFire clone_image method"

cinder/tests/unit/test_solidfire.py

@@ -24,6 +24,7 @@ from cinder import context
 from cinder import exception
 from cinder.objects import fields
 from cinder import test
+from cinder.tests.unit.image import fake as fake_image
 from cinder.volume import configuration as conf
 from cinder.volume.drivers import solidfire
 from cinder.volume import qos_specs
@@ -79,7 +80,7 @@ class SolidFireVolumeTestCase(test.TestCase):
                                                                 325355),
                                 'is_public': True,
                                 'owner': 'testprjid'}
-        self.fake_image_service = 'null'
+        self.fake_image_service = fake_image.FakeImageService()
 
     def fake_init_cluster_pairs(*args, **kwargs):
         return None
@@ -960,7 +961,19 @@ class SolidFireVolumeTestCase(test.TestCase):
                                          'fake'))
 
     @mock.patch.object(solidfire.SolidFireDriver, '_create_template_account')
-    def test_clone_image_authorization(self, _mock_create_template_account):
+    @mock.patch.object(solidfire.SolidFireDriver, '_create_image_volume')
+    def test_clone_image_authorization(self,
+                                       _mock_create_image_volume,
+                                       _mock_create_template_account):
+        fake_sf_vref = {
+            'status': 'active', 'volumeID': 1,
+            'attributes': {
+                'image_info':
+                    {'image_updated_at': '2014-12-17T00:16:23+00:00',
+                     'image_id': '155d900f-4e14-4e4c-a73d-069cbf4541e6',
+                     'image_name': 'fake-image',
+                     'image_created_at': '2014-12-17T00:16:23+00:00'}}}
+        _mock_create_image_volume.return_value = fake_sf_vref
         _mock_create_template_account.return_value = 1
 
         self.configuration.sf_allow_template_caching = True
@@ -968,14 +981,24 @@ class SolidFireVolumeTestCase(test.TestCase):
 
         # Make sure if it's NOT public and we're NOT the owner it
         # doesn't try and cache
-        _fake_image_meta = {'id': '17c550bb-a411-44c0-9aaf-0d96dd47f501',
+        timestamp = datetime.datetime(2011, 1, 1, 1, 2, 3)
-                            'updated_at': datetime.datetime(2013, 9,
+        _fake_image_meta = {
-                                                            28, 15,
+            'id': '155d900f-4e14-4e4c-a73d-069cbf4541e6',
-                                                            27, 36,
+            'name': 'fakeimage123456',
-                                                            325355),
+            'created_at': timestamp,
-                            'properties': {'virtual_size': 1},
+            'updated_at': timestamp,
-                            'is_public': False,
+            'deleted_at': None,
-                            'owner': 'wrong-owner'}
+            'deleted': False,
+            'status': 'active',
+            'visibility': 'private',
+            'protected': False,
+            'container_format': 'raw',
+            'disk_format': 'raw',
+            'owner': 'wrong-owner',
+            'properties': {'kernel_id': 'nokernel',
+                           'ramdisk_id': 'nokernel',
+                           'architecture': 'x86_64'}}
+
         with mock.patch.object(sfv, '_do_clone_volume',
                                return_value=('fe', 'fi', 'fo')):
             self.assertEqual((None, False),
@@ -983,32 +1006,39 @@ class SolidFireVolumeTestCase(test.TestCase):
                                              self.mock_volume,
                                              'fake',
                                              _fake_image_meta,
-                                             'fake'))
+                                             self.fake_image_service))
 
             # And is_public False, but the correct owner does work
             _fake_image_meta['owner'] = 'testprjid'
-            self.assertEqual(('fo', True), sfv.clone_image(self.ctxt,
+            self.assertEqual(
+                ('fo', True),
+                sfv.clone_image(
+                    self.ctxt,
                     self.mock_volume,
                     'fake',
                     _fake_image_meta,
-                                                           'fake'))
+                    self.fake_image_service))
 
             # And is_public True, even if not the correct owner
             _fake_image_meta['is_public'] = True
             _fake_image_meta['owner'] = 'wrong-owner'
-            self.assertEqual(('fo', True), sfv.clone_image(self.ctxt,
+            self.assertEqual(
+                ('fo', True),
+                sfv.clone_image(self.ctxt,
                                 self.mock_volume,
                                 'fake',
                                 _fake_image_meta,
-                                                           'fake'))
+                                self.fake_image_service))
             # And using the new V2 visibility tag
             _fake_image_meta['visibility'] = 'public'
             _fake_image_meta['owner'] = 'wrong-owner'
-            self.assertEqual(('fo', True), sfv.clone_image(self.ctxt,
+            self.assertEqual(
+                ('fo', True),
+                sfv.clone_image(self.ctxt,
                                 self.mock_volume,
                                 'fake',
                                 _fake_image_meta,
-                                                           'fake'))
+                                self.fake_image_service))
 
     def test_create_template_no_account(self):
         sfv = solidfire.SolidFireDriver(configuration=self.configuration)

cinder/volume/drivers/solidfire.py

@@ -1,6 +1,6 @@
 # All Rights Reserved.
 # Copyright 2013 SolidFire Inc
-#
+
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
 #    not use this file except in compliance with the License. You may obtain
 #    a copy of the License at
@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import inspect
 import json
 import math
 import random
@@ -37,6 +38,7 @@ from cinder.i18n import _, _LE, _LW
 from cinder.image import image_utils
 from cinder import interface
 from cinder.objects import fields
+from cinder import utils
 from cinder.volume.drivers.san import san
 from cinder.volume import qos_specs
 from cinder.volume.targets import iscsi as iscsi_driver
@@ -151,9 +153,11 @@ class SolidFireDriver(san.SanISCSIDriver):
         2.0.4 - Implement volume replication
         2.0.5 - Try and deal with the stupid retry/clear issues from objects
                 and tflow
+        2.0.6 - Add a lock decorator around the clone_image method
     """
 
-    VERSION = '2.0.2'
+    VERSION = '2.0.6'
+    driver_prefix = 'solidfire'
 
     sf_qos_dict = {'slow': {'minIOPS': 100,
                             'maxIOPS': 200,
@@ -746,6 +750,24 @@ class SolidFireDriver(san.SanISCSIDriver):
         return self._issue_api_request(
             'ListSnapshots', params, version='6.0')['result']['snapshots']
 
+    def locked_image_id_operation(f, external=False):
+        def lvo_inner1(inst, *args, **kwargs):
+            lock_tag = inst.driver_prefix
+            call_args = inspect.getcallargs(f, inst, *args, **kwargs)
+
+            if call_args.get('image_meta'):
+                image_id = call_args['image_meta']['id']
+            else:
+                err_msg = _('The decorated method must accept image_meta.')
+                raise exception.VolumeBackendAPIException(data=err_msg)
+
+            @utils.synchronized('%s-%s' % (lock_tag, image_id),
+                                external=external)
+            def lvo_inner2():
+                return f(inst, *args, **kwargs)
+            return lvo_inner2()
+        return lvo_inner1
+
     def _create_image_volume(self, context,
                              image_meta, image_service,
                              image_id):
@@ -839,24 +861,22 @@ class SolidFireDriver(san.SanISCSIDriver):
 
         params = {'accountID': self.template_account_id}
         sf_vol = self._get_sf_volume(image_meta['id'], params)
-        if sf_vol is None:
+        if not sf_vol:
+            self._create_image_volume(context,
+                                      image_meta,
+                                      image_service,
+                                      image_meta['id'])
             return
 
-        # Check updated_at field, delete copy and update if needed
+        if sf_vol['attributes']['image_info']['image_updated_at'] != (
-        if sf_vol['attributes']['image_info']['image_updated_at'] == (
                 image_meta['updated_at'].isoformat()):
-            return
-        else:
-            # Bummer, it's been updated, delete it
             params = {'accountID': self.template_account_id}
             params['volumeID'] = sf_vol['volumeID']
             self._issue_api_request('DeleteVolume', params)
-            if not self._create_image_volume(context,
+            self._create_image_volume(context,
                                       image_meta,
                                       image_service,
-                                             image_meta['id']):
+                                      image_meta['id'])
-                msg = _("Failed to create SolidFire Image-Volume")
-                raise exception.SolidFireAPIException(msg)
 
     def _get_sfaccounts_for_tenant(self, cinder_project_id):
         accounts = self._issue_api_request(
@@ -1082,6 +1102,7 @@ class SolidFireDriver(san.SanISCSIDriver):
         for vag in sorted_targets[:limit]:
             self._remove_vag(vag['volumeAccessGroupID'])
 
+    @locked_image_id_operation
     def clone_image(self, context,
                     volume, image_location,
                     image_meta, image_service):
@@ -1116,21 +1137,9 @@ class SolidFireDriver(san.SanISCSIDriver):
         except exception.SolidFireAPIException:
             return None, False
 
-        try:
-            (data, sfaccount, model) = self._do_clone_volume(image_meta['id'],
-                                                             volume)
-        except exception.VolumeNotFound:
-            if self._create_image_volume(context,
-                                         image_meta,
-                                         image_service,
-                                         image_meta['id']) is None:
-                # We failed, dump out
-                return None, False
-
         # Ok, should be good to go now, try it again
         (data, sfaccount, model) = self._do_clone_volume(image_meta['id'],
                                                          volume)
-
         return model, True
 
     def _retrieve_qos_setting(self, volume):