From baedb8c6c1642c0551675e311a1b3f869cecfc57 Mon Sep 17 00:00:00 2001 From: John Griffith Date: Thu, 18 Dec 2014 09:26:12 -0700 Subject: [PATCH] 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 --- cinder/tests/test_gpfs.py | 2 +- cinder/tests/test_huaweistorac.py | 1 + cinder/tests/test_netapp_nfs.py | 24 +++++++++++-------- cinder/tests/test_rbd.py | 13 +++++----- cinder/tests/test_volume.py | 2 +- cinder/volume/driver.py | 2 +- cinder/volume/drivers/ibm/gpfs.py | 4 ++-- cinder/volume/drivers/lvm.py | 2 +- .../drivers/netapp/dataontap/nfs_base.py | 4 ++-- cinder/volume/drivers/rbd.py | 2 +- cinder/volume/drivers/scality.py | 2 +- cinder/volume/flows/manager/create_volume.py | 2 +- 12 files changed, 32 insertions(+), 28 deletions(-) diff --git a/cinder/tests/test_gpfs.py b/cinder/tests/test_gpfs.py index 470b2e00f..8ca523107 100644 --- 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 ba34c0595..23dfdbce7 100644 --- 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 63ff70607..a3111c184 100644 --- 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 7b6496160..544696008 100644 --- 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 5af42eef5..ce2b9144c 100644 --- 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 4c07865ad..215996b76 100755 --- 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 9396612d1..08e101a00 100644 --- 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 491070243..0f21c7cf5 100644 --- 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 42cbc9267..deb5b065c 100644 --- 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 0c1a0749d..ba7ae4ea1 100644 --- 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 82f2c1bac..a16bb53db 100644 --- 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 903dd314b..c089f1587 100644 --- 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 -- 2.45.2