Remove redundant args for clone_image method

The clone_image method takes both image_id and image_meta
as arguments.  No big deal, except the image id is included
in the image_meta; so there's really no reason to have both.

This patch removes the image_id argument and updates those
drivers that implement clone_image to extract the image ID
from the provided image_metadata.

Change-Id: Ibf3d3f7520f0e6d30081e98bdcbf1a07ebdb44ab

diff --git a/cinder/tests/test_gpfs.py b/cinder/tests/test_gpfs.py
index 470b2e00f7f57803e7d939e65f9c032855924ed3..8ca523107299c2c286497fdd07e43104dac3736f 100644 (file)
--- a/cinder/tests/test_gpfs.py
+++ b/cinder/tests/test_gpfs.py
@@ -1055,7 +1055,7 @@ class GPFSDriverTestCase(test.TestCase):
 
     @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._clone_image')
     def test_clone_image_pub(self, mock_exec):
-        self.driver.clone_image('', '', '', '')
+        self.driver.clone_image('', '', {'id': 1})
 
     @patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._is_gpfs_path')
     def test_is_cloneable_ok(self, mock_is_gpfs_path):

diff --git a/cinder/tests/test_huaweistorac.py b/cinder/tests/test_huaweistorac.py
index ba34c0595896d3a49b1946e034d79478b95543ba..23dfdbce7cc40689a43a012281817520f1cb8524 100644 (file)
--- a/cinder/tests/test_huaweistorac.py
+++ b/cinder/tests/test_huaweistorac.py
@@ -269,6 +269,7 @@ class StorACDriverTestCase(test.TestCase):
 
     def setUp(self):
         super(StorACDriverTestCase, self).setUp()
+
         self.fake_conf_file = tempfile.mktemp(suffix='.xml')
         self.addCleanup(os.remove, self.fake_conf_file)
 

diff --git a/cinder/tests/test_netapp_nfs.py b/cinder/tests/test_netapp_nfs.py
index 63ff706073786f0bc435c8676e2692337d673717..a3111c184c357cce40093802b42e2b720a4f6757 100644 (file)
--- a/cinder/tests/test_netapp_nfs.py
+++ b/cinder/tests/test_netapp_nfs.py
@@ -477,7 +477,7 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
         drv._post_clone_image(volume)
 
         mox.ReplayAll()
-        drv.clone_image(volume, ('image_location', None), 'image_id', {})
+        drv.clone_image(volume, ('image_location', None), {'id': 'image_id'})
         mox.VerifyAll()
 
     def get_img_info(self, format):
@@ -501,7 +501,9 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
 
         mox.ReplayAll()
         (prop, cloned) = drv. clone_image(
-            volume, ('nfs://127.0.0.1:/share/img-id', None), 'image_id', {})
+            volume,
+            ('nfs://127.0.0.1:/share/img-id', None),
+            {'id': 'image_id'})
         mox.VerifyAll()
         if not cloned and not prop['provider_location']:
             pass
@@ -537,7 +539,9 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
 
         mox.ReplayAll()
         drv. clone_image(
-            volume, ('nfs://127.0.0.1:/share/img-id', None), 'image_id', {})
+            volume,
+            ('nfs://127.0.0.1:/share/img-id', None),
+            {'id': 'image_id'})
         mox.VerifyAll()
 
     def test_clone_image_cloneableshare_notraw(self):
@@ -575,7 +579,7 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
 
         mox.ReplayAll()
         drv.clone_image(
-            volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id', {})
+            volume, ('nfs://127.0.0.1/share/img-id', None), {'id': 'image_id'})
         mox.VerifyAll()
 
     def test_clone_image_file_not_discovered(self):
@@ -615,7 +619,7 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
 
         mox.ReplayAll()
         vol_dict, result = drv. clone_image(
-            volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id', {})
+            volume, ('nfs://127.0.0.1/share/img-id', None), {'id': 'image_id'})
         mox.VerifyAll()
         self.assertFalse(result)
         self.assertFalse(vol_dict['bootable'])
@@ -663,7 +667,7 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
 
         mox.ReplayAll()
         vol_dict, result = drv. clone_image(
-            volume, ('nfs://127.0.0.1/share/img-id', None), 'image_id', {})
+            volume, ('nfs://127.0.0.1/share/img-id', None), {'id': 'image_id'})
         mox.VerifyAll()
         self.assertFalse(result)
         self.assertFalse(vol_dict['bootable'])
@@ -1108,8 +1112,8 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase):
         image_service.show.return_value = {'size': 1,
                                            'disk_format': 'raw'}
 
-        drv._check_get_nfs_path_segs = mock.Mock(return_value=
-                                                 ('ip1', '/openstack'))
+        drv._check_get_nfs_path_segs =\
+            mock.Mock(return_value=('ip1', '/openstack'))
         drv._get_ip_verify_on_cluster = mock.Mock(return_value='ip1')
         drv._get_host_ip = mock.Mock(return_value='ip2')
         drv._get_export_path = mock.Mock(return_value='/exp_path')
@@ -1149,8 +1153,8 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase):
                                                    None)
         image_service.show.return_value = {'size': 1,
                                            'disk_format': 'qcow2'}
-        drv._check_get_nfs_path_segs = mock.Mock(return_value=
-                                                 ('ip1', '/openstack'))
+        drv._check_get_nfs_path_segs =\
+            mock.Mock(return_value=('ip1', '/openstack'))
 
         drv._get_ip_verify_on_cluster = mock.Mock(return_value='ip1')
         drv._get_host_ip = mock.Mock(return_value='ip2')

diff --git a/cinder/tests/test_rbd.py b/cinder/tests/test_rbd.py
index 7b6496160b37b1d21244878819419c82e9f27caa..544696008b611d8e6ebffa86ba01432ee06f26fc 100644 (file)
--- a/cinder/tests/test_rbd.py
+++ b/cinder/tests/test_rbd.py
@@ -1038,7 +1038,7 @@ class ManagedRBDTestCase(DriverTestCase):
     def test_create_vol_from_image_status_available(self):
         """Clone raw image then verify volume is in available state."""
 
-        def _mock_clone_image(volume, image_location, image_id, image_meta):
+        def _mock_clone_image(volume, image_location, image_meta):
             return {'provider_location': None}, True
 
         with mock.patch.object(self.volume.driver, 'clone_image') as \
@@ -1057,7 +1057,7 @@ class ManagedRBDTestCase(DriverTestCase):
     def test_create_vol_from_non_raw_image_status_available(self):
         """Clone non-raw image then verify volume is in available state."""
 
-        def _mock_clone_image(volume, image_location, image_id, image_meta):
+        def _mock_clone_image(volume, image_location, image_meta):
             return {'provider_location': None}, False
 
         with mock.patch.object(self.volume.driver, 'clone_image') as \
@@ -1093,12 +1093,11 @@ class ManagedRBDTestCase(DriverTestCase):
 
         with mock.patch.object(driver, '_is_cloneable', lambda *args: False):
             image_loc = (mock.Mock(), mock.Mock())
-            actual = driver.clone_image(mock.Mock(), image_loc,
-                                        mock.Mock(), {})
+            actual = driver.clone_image(mock.Mock(), image_loc, {})
             self.assertEqual(({}, False), actual)
 
         self.assertEqual(({}, False),
-                         driver.clone_image(object(), None, None, {}))
+                         driver.clone_image(object(), None, {}))
 
     def test_clone_success(self):
         expected = ({'provider_location': None}, True)
@@ -1115,8 +1114,8 @@ class ManagedRBDTestCase(DriverTestCase):
 
                     volume = {'name': 'vol1'}
                     actual = driver.clone_image(volume, image_loc,
-                                                'id.foo',
-                                                {'disk_format': 'raw'})
+                                                {'disk_format': 'raw',
+                                                 'id': 'id.foo'})
 
                     self.assertEqual(expected, actual)
                     mock_clone.assert_called_once_with(volume,

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index 5af42eef517d4e9d0fbaf32b413ceea7c094eb28..ce2b9144c213c07d9be580ceea3e4d78c3eaa2d0 100644 (file)
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -2163,7 +2163,7 @@ class VolumeTestCase(BaseVolumeTestCase):
                               size=None):
             pass
 
-        def fake_clone_image(volume_ref, image_location, image_id, image_meta):
+        def fake_clone_image(volume_ref, image_location, image_meta):
             return {'provider_location': None}, True
 
         dst_fd, dst_path = tempfile.mkstemp()

diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py
index 4c07865ad89e3bcd5370543408ea9a4bdc26ee36..215996b760ed112a57fc05ed2a281cdebde92b01 100755 (executable)
--- a/cinder/volume/driver.py
+++ b/cinder/volume/driver.py
@@ -521,7 +521,7 @@ class VolumeDriver(object):
                                                       {'path': host_device}))
         return {'conn': conn, 'device': device, 'connector': connector}
 
-    def clone_image(self, volume, image_location, image_id, image_meta):
+    def clone_image(self, volume, image_location, image_meta):
         """Create a volume efficiently from an existing image.
 
         image_location is a string whose format depends on the

diff --git a/cinder/volume/drivers/ibm/gpfs.py b/cinder/volume/drivers/ibm/gpfs.py
index 9396612d14eaa28760ed0d8e5e7b86f056fa8f1b..08e101a00ab4e2572d134486011d1addf61191df 100644 (file)
--- a/cinder/volume/drivers/ibm/gpfs.py
+++ b/cinder/volume/drivers/ibm/gpfs.py
@@ -703,9 +703,9 @@ class GPFSDriver(driver.VolumeDriver):
         data['reserved_percentage'] = 0
         self._stats = data
 
-    def clone_image(self, volume, image_location, image_id, image_meta):
+    def clone_image(self, volume, image_location, image_meta):
         """Create a volume from the specified image."""
-        return self._clone_image(volume, image_location, image_id)
+        return self._clone_image(volume, image_location, image_meta['id'])
 
     def _is_cloneable(self, image_id):
         """Return true if the specified image can be cloned by GPFS."""

diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py
index 49107024331a7405762f056929babefcbadaaa14..0f21c7cf5ef9552d3730f8555ac89e6d93e46a18 100644 (file)
--- a/cinder/volume/drivers/lvm.py
+++ b/cinder/volume/drivers/lvm.py
@@ -310,7 +310,7 @@ class LVMVolumeDriver(driver.VolumeDriver):
         finally:
             self.delete_snapshot(temp_snapshot)
 
-    def clone_image(self, volume, image_location, image_id, image_meta):
+    def clone_image(self, volume, image_location, image_meta):
         return None, False
 
     def backup_volume(self, context, backup, backup_service):

diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_base.py b/cinder/volume/drivers/netapp/dataontap/nfs_base.py
index 42cbc9267a84f764532ec5630f71603de58e9133..deb5b065ce9b033af191333b228ea72e2f6a10c1 100644 (file)
--- a/cinder/volume/drivers/netapp/dataontap/nfs_base.py
+++ b/cinder/volume/drivers/netapp/dataontap/nfs_base.py
@@ -366,7 +366,7 @@ class NetAppNfsDriver(nfs.NfsDriver):
             LOG.warning(_LW('Exception during deleting %s'), ex.__str__())
             return False
 
-    def clone_image(self, volume, image_location, image_id, image_meta):
+    def clone_image(self, volume, image_location, image_meta):
         """Create a volume efficiently from an existing image.
 
         image_location is a string whose format depends on the
@@ -380,7 +380,7 @@ class NetAppNfsDriver(nfs.NfsDriver):
         Returns a dict of volume properties eg. provider_location,
         boolean indicating whether cloning occurred.
         """
-
+        image_id = image_meta['id']
         cloned = False
         post_clone = False
         try:

diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py
index 0c1a0749dd698ec29b8f574d0059e64eb6fb8ef1..ba7ae4ea1874f62aa35fd467b1479751095f9ef5 100644 (file)
--- a/cinder/volume/drivers/rbd.py
+++ b/cinder/volume/drivers/rbd.py
@@ -804,7 +804,7 @@ class RBDDriver(driver.VolumeDriver):
                       dict(loc=image_location, err=e))
             return False
 
-    def clone_image(self, volume, image_location, image_id, image_meta):
+    def clone_image(self, volume, image_location, image_meta):
         image_location = image_location[0] if image_location else None
         if image_location is None or not self._is_cloneable(
                 image_location, image_meta):

diff --git a/cinder/volume/drivers/scality.py b/cinder/volume/drivers/scality.py
index 82f2c1baca5ed58c5c7216f3a8b9c1c5bf466382..a16bb53db97af473e516f1c59f80f0f9cb166feb 100644 (file)
--- a/cinder/volume/drivers/scality.py
+++ b/cinder/volume/drivers/scality.py
@@ -256,7 +256,7 @@ class ScalityDriver(driver.VolumeDriver):
                                   image_meta,
                                   self.local_path(volume))
 
-    def clone_image(self, volume, image_location, image_id, image_meta):
+    def clone_image(self, volume, image_location, image_meta):
         """Create a volume efficiently from an existing image.
 
         image_location is a string whose format depends on the

diff --git a/cinder/volume/flows/manager/create_volume.py b/cinder/volume/flows/manager/create_volume.py
index 903dd314bba38c32bda6cd273d8729da3c4e573e..c089f15874ebfa41bfdd5a0124a7d6c7adb817bf 100644 (file)
--- a/cinder/volume/flows/manager/create_volume.py
+++ b/cinder/volume/flows/manager/create_volume.py
@@ -568,7 +568,7 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask):
         # dict containing provider_location for cloned volume
         # and clone status.
         model_update, cloned = self.driver.clone_image(
-            volume_ref, image_location, image_id, image_meta)
+            volume_ref, image_location, image_meta)
         if not cloned:
             # TODO(harlowja): what needs to be rolled back in the clone if this
             # volume create fails?? Likely this should be a subflow or broken