From 9e494b1be69d177f43fddbc7cd7b99baec0f72bd Mon Sep 17 00:00:00 2001 From: Eric Harney Date: Wed, 26 Aug 2015 15:11:59 -0400 Subject: [PATCH] Remove driver.set_execute() This method doesn't appear to really be needed, and mostly exists to help unit tests. Update a few tests to mock out the required execute calls, and remove the method to make the driver interface a bit simpler. Change-Id: I70d8a7e74761ce7bc1e51bf9f3c49eb0ce0986e0 --- cinder/tests/unit/test_glusterfs.py | 13 --- cinder/tests/unit/test_gpfs.py | 6 +- cinder/tests/unit/test_scality.py | 4 + cinder/tests/unit/test_srb.py | 81 ++++++++++--------- cinder/tests/unit/test_volume.py | 5 +- cinder/volume/driver.py | 5 +- cinder/volume/drivers/glusterfs.py | 5 -- cinder/volume/drivers/hitachi/hnas_nfs.py | 3 - cinder/volume/drivers/ibm/ibmnas.py | 4 +- .../drivers/netapp/dataontap/nfs_base.py | 3 - cinder/volume/drivers/nfs.py | 5 -- cinder/volume/drivers/srb.py | 5 -- 12 files changed, 56 insertions(+), 83 deletions(-) diff --git a/cinder/tests/unit/test_glusterfs.py b/cinder/tests/unit/test_glusterfs.py index 776acafa2..0f19d5096 100644 --- a/cinder/tests/unit/test_glusterfs.py +++ b/cinder/tests/unit/test_glusterfs.py @@ -119,19 +119,6 @@ class GlusterFsDriverTestCase(test.TestCase): if not caught: self.fail('Expected raised exception but nothing caught.') - def test_set_execute(self): - drv = self._driver - - rfsclient = os_brick.remotefs.remotefs.RemoteFsClient - - with mock.patch.object(rfsclient, 'set_execute') as mock_set_execute: - def my_execute(*a, **k): - pass - - drv.set_execute(my_execute) - - mock_set_execute.assert_called_once_with(my_execute) - def test_local_path(self): """local_path common use case.""" self.override_config("glusterfs_mount_point_base", diff --git a/cinder/tests/unit/test_gpfs.py b/cinder/tests/unit/test_gpfs.py index 2114f41df..23698e25e 100644 --- a/cinder/tests/unit/test_gpfs.py +++ b/cinder/tests/unit/test_gpfs.py @@ -65,7 +65,10 @@ class GPFSDriverTestCase(test.TestCase): self.driver = gpfs.GPFSDriver(configuration=conf.Configuration(None)) self.driver.gpfs_execute = self._execute_wrapper - self.driver.set_execute(self._execute_wrapper) + exec_patcher = mock.patch.object(self.driver, '_execute', + self._execute_wrapper) + exec_patcher.start() + self.addCleanup(exec_patcher.stop) self.driver._cluster_id = '123456' self.driver._gpfs_device = '/dev/gpfs' self.driver._storage_pool = 'system' @@ -1915,7 +1918,6 @@ class GPFSNFSDriverTestCase(test.TestCase): self.driver = gpfs.GPFSNFSDriver(configuration=conf. Configuration(None)) self.driver.gpfs_execute = self._execute_wrapper - self.driver.set_execute(self._execute_wrapper) self.context = context.get_admin_context() self.context.user_id = 'fake' self.context.project_id = 'fake' diff --git a/cinder/tests/unit/test_scality.py b/cinder/tests/unit/test_scality.py index d7cd462e7..c82cd9d65 100644 --- a/cinder/tests/unit/test_scality.py +++ b/cinder/tests/unit/test_scality.py @@ -82,6 +82,10 @@ class ScalityDriverTestCase(test.TestCase): self.assertRaises(exception.VolumeBackendAPIException, self.drv.check_for_setup_error) + exec_patcher = mock.patch.object(self.drv, '_execute', + mock.MagicMock()) + exec_patcher.start() + self.addCleanup(exec_patcher.stop) @mock.patch.object(driver.urllib.request, 'urlopen') def test_check_for_setup_error_with_urlerror(self, mock_urlopen): diff --git a/cinder/tests/unit/test_srb.py b/cinder/tests/unit/test_srb.py index dd82f7678..a5cb94ade 100644 --- a/cinder/tests/unit/test_srb.py +++ b/cinder/tests/unit/test_srb.py @@ -56,52 +56,51 @@ class SRBLvmTestCase(test_brick_lvm.BrickLvmTestCase): raise AssertionError('unexpected command called: %s' % cmd_string) def test_activate_vg(self): - executor = mock.MagicMock() - self.vg.set_execute(executor) - self.vg.activate_vg() - executor.assert_called_once_with('vgchange', '-ay', - self.configuration.volume_group_name, - root_helper=self.vg._root_helper, - run_as_root=True) + with mock.patch.object(self.vg, '_execute') as executor: + self.vg.activate_vg() + executor.assert_called_once_with( + 'vgchange', '-ay', + self.configuration.volume_group_name, + root_helper=self.vg._root_helper, + run_as_root=True) def test_deactivate_vg(self): - executor = mock.MagicMock() - self.vg.set_execute(executor) - self.vg.deactivate_vg() - executor.assert_called_once_with('vgchange', '-an', - self.configuration.volume_group_name, - root_helper=self.vg._root_helper, - run_as_root=True) + with mock.patch.object(self.vg, '_execute') as executor: + self.vg.deactivate_vg() + executor.assert_called_once_with( + 'vgchange', '-an', + self.configuration.volume_group_name, + root_helper=self.vg._root_helper, + run_as_root=True) def test_destroy_vg(self): - executor = mock.MagicMock() - self.vg.set_execute(executor) - self.vg.destroy_vg() - executor.assert_called_once_with('vgremove', '-f', - self.configuration.volume_group_name, - root_helper=self.vg._root_helper, - run_as_root=True) + with mock.patch.object(self.vg, '_execute') as executor: + self.vg.destroy_vg() + executor.assert_called_once_with( + 'vgremove', '-f', + self.configuration.volume_group_name, + root_helper=self.vg._root_helper, + run_as_root=True) def test_pv_resize(self): - executor = mock.MagicMock() - self.vg.set_execute(executor) - self.vg.pv_resize('fake-pv', '50G') - executor.assert_called_once_with('pvresize', - '--setphysicalvolumesize', - '50G', 'fake-pv', - root_helper=self.vg._root_helper, - run_as_root=True) + with mock.patch.object(self.vg, '_execute') as executor: + self.vg.pv_resize('fake-pv', '50G') + executor.assert_called_once_with( + 'pvresize', + '--setphysicalvolumesize', + '50G', 'fake-pv', + root_helper=self.vg._root_helper, + run_as_root=True) def test_extend_thin_pool_nothin(self): - executor =\ - mock.MagicMock(side_effect=Exception('Unexpected call to execute')) - self.vg.set_execute(executor) - thin_calc =\ - mock.MagicMock( - side_effect= - Exception('Unexpected call to _calculate_thin_pool_size')) - self.vg._calculate_thin_pool_size = thin_calc - self.vg.extend_thin_pool() + with mock.patch.object(self.vg, '_execute') as executor: + executor.side_effect = AssertionError + thin_calc =\ + mock.MagicMock( + side_effect= + Exception('Unexpected call to _calculate_thin_pool_size')) + self.vg._calculate_thin_pool_size = thin_calc + self.vg.extend_thin_pool() def test_extend_thin_pool_thin(self): self.stubs.Set(processutils, 'execute', self.fake_execute) @@ -737,7 +736,11 @@ class SRBDriverTestCase(test.TestCase): self._driver = srb.SRBDriver(configuration=self.configuration) # Stub processutils.execute for static methods self.stubs.Set(processutils, 'execute', self._fake_execute) - self._driver.set_execute(self._fake_execute) + exec_patcher = mock.patch.object(self._driver, + '_execute', + self._fake_execute) + exec_patcher.start() + self.addCleanup(exec_patcher.stop) self._configure_driver() def test_setup(self): diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index 256c27209..00246389e 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -5616,7 +5616,10 @@ class DriverTestCase(test.TestCase): def _fake_execute(_command, *_args, **_kwargs): """Fake _execute.""" return self.output, None - self.volume.driver.set_execute(_fake_execute) + exec_patcher = mock.patch.object(self.volume.driver, '_execute', + _fake_execute) + exec_patcher.start() + self.addCleanup(exec_patcher.stop) self.volume.driver.set_initialized() self.addCleanup(self._cleanup) diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index 7f6c8646e..86c6f700a 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -306,7 +306,7 @@ class BaseVD(object): self.configuration.append_config_values(iser_opts) utils.setup_tracing(self.configuration.safe_get('trace_flags')) - self.set_execute(execute) + self._execute = execute self._stats = {} self.pools = [] @@ -439,9 +439,6 @@ class BaseVD(object): raise exception.RemoveExportException(volume=snapshot.id, reason=ex) - def set_execute(self, execute): - self._execute = execute - def set_initialized(self): self._initialized = True diff --git a/cinder/volume/drivers/glusterfs.py b/cinder/volume/drivers/glusterfs.py index 49914a0a4..761ec48e6 100644 --- a/cinder/volume/drivers/glusterfs.py +++ b/cinder/volume/drivers/glusterfs.py @@ -76,11 +76,6 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver, driver.CloneableVD, 'glusterfs', root_helper, execute, glusterfs_mount_point_base=self.base) - def set_execute(self, execute): - super(GlusterfsDriver, self).set_execute(execute) - if self._remotefsclient: - self._remotefsclient.set_execute(execute) - def do_setup(self, context): """Any initialization the volume driver does while starting.""" super(GlusterfsDriver, self).do_setup(context) diff --git a/cinder/volume/drivers/hitachi/hnas_nfs.py b/cinder/volume/drivers/hitachi/hnas_nfs.py index 55daa86c4..727543379 100644 --- a/cinder/volume/drivers/hitachi/hnas_nfs.py +++ b/cinder/volume/drivers/hitachi/hnas_nfs.py @@ -221,9 +221,6 @@ class HDSNFSDriver(nfs.NfsDriver): return service - def set_execute(self, execute): - self._execute = execute - def extend_volume(self, volume, new_size): """Extend an existing volume. diff --git a/cinder/volume/drivers/ibm/ibmnas.py b/cinder/volume/drivers/ibm/ibmnas.py index 5b4e1c113..9f038e5c6 100644 --- a/cinder/volume/drivers/ibm/ibmnas.py +++ b/cinder/volume/drivers/ibm/ibmnas.py @@ -90,12 +90,10 @@ class IBMNAS_NFSDriver(nfs.NfsDriver, san.SanDriver): self.configuration.san_ssh_port = self.configuration.nas_ssh_port self.configuration.ibmnas_platform_type = \ self.configuration.ibmnas_platform_type.lower() + self._execute = execute LOG.info(_LI('Initialized driver for IBMNAS Platform: %s.'), self.configuration.ibmnas_platform_type) - def set_execute(self, execute): - self._execute = utils.execute - def do_setup(self, context): """Any initialization the volume driver does while starting.""" super(IBMNAS_NFSDriver, self).do_setup(context) diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_base.py b/cinder/volume/drivers/netapp/dataontap/nfs_base.py index 2b0e0c092..cb933063d 100644 --- a/cinder/volume/drivers/netapp/dataontap/nfs_base.py +++ b/cinder/volume/drivers/netapp/dataontap/nfs_base.py @@ -75,9 +75,6 @@ class NetAppNfsDriver(driver.ManageableVD, self.configuration.append_config_values(na_opts.netapp_img_cache_opts) self.configuration.append_config_values(na_opts.netapp_nfs_extra_opts) - def set_execute(self, execute): - self._execute = execute - def do_setup(self, context): super(NetAppNfsDriver, self).do_setup(context) self._context = context diff --git a/cinder/volume/drivers/nfs.py b/cinder/volume/drivers/nfs.py index 78c178434..f8d109fdd 100644 --- a/cinder/volume/drivers/nfs.py +++ b/cinder/volume/drivers/nfs.py @@ -112,11 +112,6 @@ class NfsDriver(driver.ExtendVD, remotefs.RemoteFSDriver): self._sparse_copy_volume_data = True - def set_execute(self, execute): - super(NfsDriver, self).set_execute(execute) - if self._remotefsclient: - self._remotefsclient.set_execute(execute) - def do_setup(self, context): """Any initialization the volume driver does while starting.""" super(NfsDriver, self).do_setup(context) diff --git a/cinder/volume/drivers/srb.py b/cinder/volume/drivers/srb.py index 8d1630768..cb79dd861 100644 --- a/cinder/volume/drivers/srb.py +++ b/cinder/volume/drivers/srb.py @@ -836,11 +836,6 @@ class SRBISCSIDriver(SRBDriver, driver.ISCSIDriver): self.configuration.safe_get('volume_backend_name') or 'SRB_iSCSI' self.protocol = 'iSCSI' - def set_execute(self, execute): - super(SRBISCSIDriver, self).set_execute(execute) - if self.target_driver is not None: - self.target_driver.set_execute(execute) - def ensure_export(self, context, volume): device_path = self._mapper_path(volume) -- 2.45.2