From 51218b2b32edbc79e89195d58b7b7e330a6d1486 Mon Sep 17 00:00:00 2001 From: mouad benchchaoui Date: Thu, 25 Apr 2013 18:12:09 +0200 Subject: [PATCH] Remove _path_exists method. This method is buggy because it works only on host machine with english language, and apparently this method is also useless because we don't need to check if a path exist if we can use command option -p for "mkdir" command and option -f for "rm" command. bug: LP#1172777 Change-Id: I418b32772ca97b42e1a43275a7abec9f96688607 --- cinder/tests/test_glusterfs.py | 94 +----------------------- cinder/tests/test_nfs.py | 113 +---------------------------- cinder/volume/drivers/glusterfs.py | 10 +-- cinder/volume/drivers/nfs.py | 21 +----- 4 files changed, 9 insertions(+), 229 deletions(-) diff --git a/cinder/tests/test_glusterfs.py b/cinder/tests/test_glusterfs.py index 4739cafc6..a210d57cb 100644 --- a/cinder/tests/test_glusterfs.py +++ b/cinder/tests/test_glusterfs.py @@ -81,39 +81,6 @@ class GlusterFsDriverTestCase(test.TestCase): stub = mox_lib.MockObject(attr_to_replace) self.stubs.Set(obj, attr_name, stub) - def test_path_exists_should_return_true(self): - """_path_exists should return True if stat returns 0.""" - mox = self._mox - drv = self._driver - - mox.StubOutWithMock(drv, '_execute') - drv._execute('stat', self.TEST_FILE_NAME, run_as_root=True) - - mox.ReplayAll() - - self.assertTrue(drv._path_exists(self.TEST_FILE_NAME)) - - mox.VerifyAll() - - def test_path_exists_should_return_false(self): - """_path_exists should return True if stat doesn't return 0.""" - mox = self._mox - drv = self._driver - - mox.StubOutWithMock(drv, '_execute') - drv._execute( - 'stat', - self.TEST_FILE_NAME, run_as_root=True).\ - AndRaise(ProcessExecutionError( - stderr="stat: cannot stat `test.txt': No such file " - "or directory")) - - mox.ReplayAll() - - self.assertFalse(drv._path_exists(self.TEST_FILE_NAME)) - - mox.VerifyAll() - def test_local_path(self): """local_path common use case.""" glusterfs.FLAGS.glusterfs_mount_point_base = self.TEST_MNT_POINT_BASE @@ -132,10 +99,8 @@ class GlusterFsDriverTestCase(test.TestCase): mox = self._mox drv = self._driver - mox.StubOutWithMock(drv, '_path_exists') - drv._path_exists(self.TEST_MNT_POINT).AndReturn(True) - mox.StubOutWithMock(drv, '_execute') + drv._execute('mkdir', '-p', self.TEST_MNT_POINT) drv._execute('mount', '-t', 'glusterfs', self.TEST_EXPORT1, self.TEST_MNT_POINT, run_as_root=True) @@ -152,10 +117,8 @@ class GlusterFsDriverTestCase(test.TestCase): mox = self._mox drv = self._driver - mox.StubOutWithMock(drv, '_path_exists') - drv._path_exists(self.TEST_MNT_POINT).AndReturn(True) - mox.StubOutWithMock(drv, '_execute') + drv._execute('mkdir', '-p', self.TEST_MNT_POINT) drv._execute('mount', '-t', 'glusterfs', self.TEST_EXPORT1, self.TEST_MNT_POINT, run_as_root=True).\ AndRaise(ProcessExecutionError( @@ -175,10 +138,8 @@ class GlusterFsDriverTestCase(test.TestCase): mox = self._mox drv = self._driver - mox.StubOutWithMock(drv, '_path_exists') - drv._path_exists(self.TEST_MNT_POINT).AndReturn(True) - mox.StubOutWithMock(drv, '_execute') + drv._execute('mkdir', '-p', self.TEST_MNT_POINT) drv._execute( 'mount', '-t', @@ -202,9 +163,6 @@ class GlusterFsDriverTestCase(test.TestCase): mox = self._mox drv = self._driver - mox.StubOutWithMock(drv, '_path_exists') - drv._path_exists(self.TEST_MNT_POINT).AndReturn(False) - mox.StubOutWithMock(drv, '_execute') drv._execute('mkdir', '-p', self.TEST_MNT_POINT) drv._execute(*([IgnoreArg()] * 5), run_as_root=IgnoreArg()) @@ -215,24 +173,6 @@ class GlusterFsDriverTestCase(test.TestCase): mox.VerifyAll() - def test_mount_glusterfs_should_not_create_mountpoint_if_already(self): - """_mount_glusterfs should not create mountpoint if it already exists. - """ - mox = self._mox - drv = self._driver - - mox.StubOutWithMock(drv, '_path_exists') - drv._path_exists(self.TEST_MNT_POINT).AndReturn(True) - - mox.StubOutWithMock(drv, '_execute') - drv._execute(*([IgnoreArg()] * 5), run_as_root=IgnoreArg()) - - mox.ReplayAll() - - drv._mount_glusterfs(self.TEST_EXPORT1, self.TEST_MNT_POINT) - - mox.VerifyAll() - def test_get_hash_str(self): """_get_hash_str should calculation correct value.""" drv = self._driver @@ -590,9 +530,6 @@ class GlusterFsDriverTestCase(test.TestCase): mox.StubOutWithMock(drv, 'local_path') drv.local_path(volume).AndReturn(self.TEST_LOCAL_PATH) - mox.StubOutWithMock(drv, '_path_exists') - drv._path_exists(self.TEST_LOCAL_PATH).AndReturn(True) - mox.StubOutWithMock(drv, '_execute') drv._execute('rm', '-f', self.TEST_LOCAL_PATH, run_as_root=True) @@ -640,28 +577,3 @@ class GlusterFsDriverTestCase(test.TestCase): drv.delete_volume(volume) mox.VerifyAll() - - def test_delete_should_not_delete_if_there_is_no_file(self): - """delete_volume should not try to delete if file missed.""" - mox = self._mox - drv = self._driver - - self.stub_out_not_replaying(drv, '_ensure_share_mounted') - - volume = DumbVolume() - volume['name'] = 'volume-123' - volume['provider_location'] = self.TEST_EXPORT1 - - mox.StubOutWithMock(drv, 'local_path') - drv.local_path(volume).AndReturn(self.TEST_LOCAL_PATH) - - mox.StubOutWithMock(drv, '_path_exists') - drv._path_exists(self.TEST_LOCAL_PATH).AndReturn(False) - - mox.StubOutWithMock(drv, '_execute') - - mox.ReplayAll() - - drv.delete_volume(volume) - - mox.VerifyAll() diff --git a/cinder/tests/test_nfs.py b/cinder/tests/test_nfs.py index a79304531..242755491 100644 --- a/cinder/tests/test_nfs.py +++ b/cinder/tests/test_nfs.py @@ -96,39 +96,6 @@ class RemoteFsDriverTestCase(test.TestCase): mox.VerifyAll() - def test_path_exists_should_return_true(self): - """_path_exists should return True if stat returns 0.""" - mox = self._mox - drv = self._driver - - mox.StubOutWithMock(drv, '_execute') - drv._execute('stat', self.TEST_FILE_NAME, run_as_root=True) - - mox.ReplayAll() - - self.assertTrue(drv._path_exists(self.TEST_FILE_NAME)) - - mox.VerifyAll() - - def test_path_exists_should_return_false(self): - """_path_exists should return True if stat doesn't return 0.""" - mox = self._mox - drv = self._driver - - mox.StubOutWithMock(drv, '_execute') - drv._execute( - 'stat', - self.TEST_FILE_NAME, run_as_root=True).\ - AndRaise(ProcessExecutionError( - stderr="stat: cannot stat `test.txt': No such file " - "or directory")) - - mox.ReplayAll() - - self.assertFalse(drv._path_exists(self.TEST_FILE_NAME)) - - mox.VerifyAll() - def test_get_hash_str(self): """_get_hash_str should calculation correct value.""" drv = self._driver @@ -171,39 +138,6 @@ class NfsDriverTestCase(test.TestCase): stub = mox_lib.MockObject(attr_to_replace) self.stubs.Set(obj, attr_name, stub) - def test_path_exists_should_return_true(self): - """_path_exists should return True if stat returns 0.""" - mox = self._mox - drv = self._driver - - mox.StubOutWithMock(drv, '_execute') - drv._execute('stat', self.TEST_FILE_NAME, run_as_root=True) - - mox.ReplayAll() - - self.assertTrue(drv._path_exists(self.TEST_FILE_NAME)) - - mox.VerifyAll() - - def test_path_exists_should_return_false(self): - """_path_exists should return True if stat doesn't return 0.""" - mox = self._mox - drv = self._driver - - mox.StubOutWithMock(drv, '_execute') - drv._execute( - 'stat', - self.TEST_FILE_NAME, run_as_root=True).\ - AndRaise(ProcessExecutionError( - stderr="stat: cannot stat `test.txt': No such file " - "or directory")) - - mox.ReplayAll() - - self.assertFalse(drv._path_exists(self.TEST_FILE_NAME)) - - mox.VerifyAll() - def test_local_path(self): """local_path common use case.""" self.configuration.nfs_mount_point_base = self.TEST_MNT_POINT_BASE @@ -222,10 +156,8 @@ class NfsDriverTestCase(test.TestCase): mox = self._mox drv = self._driver - mox.StubOutWithMock(drv, '_path_exists') - drv._path_exists(self.TEST_MNT_POINT).AndReturn(True) - mox.StubOutWithMock(drv, '_execute') + drv._execute('mkdir', '-p', self.TEST_MNT_POINT) drv._execute('mount', '-t', 'nfs', self.TEST_NFS_EXPORT1, self.TEST_MNT_POINT, run_as_root=True) @@ -241,10 +173,8 @@ class NfsDriverTestCase(test.TestCase): mox = self._mox drv = self._driver - mox.StubOutWithMock(drv, '_path_exists') - drv._path_exists(self.TEST_MNT_POINT).AndReturn(True) - mox.StubOutWithMock(drv, '_execute') + drv._execute('mkdir', '-p', self.TEST_MNT_POINT) drv._execute('mount', '-t', 'nfs', self.TEST_NFS_EXPORT1, self.TEST_MNT_POINT, run_as_root=True).\ AndRaise(ProcessExecutionError( @@ -262,10 +192,8 @@ class NfsDriverTestCase(test.TestCase): mox = self._mox drv = self._driver - mox.StubOutWithMock(drv, '_path_exists') - drv._path_exists(self.TEST_MNT_POINT).AndReturn(True) - mox.StubOutWithMock(drv, '_execute') + drv._execute('mkdir', '-p', self.TEST_MNT_POINT) drv._execute( 'mount', '-t', @@ -287,9 +215,6 @@ class NfsDriverTestCase(test.TestCase): mox = self._mox drv = self._driver - mox.StubOutWithMock(drv, '_path_exists') - drv._path_exists(self.TEST_MNT_POINT).AndReturn(False) - mox.StubOutWithMock(drv, '_execute') drv._execute('mkdir', '-p', self.TEST_MNT_POINT) drv._execute(*([IgnoreArg()] * 5), run_as_root=IgnoreArg()) @@ -305,10 +230,8 @@ class NfsDriverTestCase(test.TestCase): mox = self._mox drv = self._driver - mox.StubOutWithMock(drv, '_path_exists') - drv._path_exists(self.TEST_MNT_POINT).AndReturn(True) - mox.StubOutWithMock(drv, '_execute') + drv._execute('mkdir', '-p', self.TEST_MNT_POINT) drv._execute(*([IgnoreArg()] * 5), run_as_root=IgnoreArg()) mox.ReplayAll() @@ -657,9 +580,6 @@ class NfsDriverTestCase(test.TestCase): mox.StubOutWithMock(drv, 'local_path') drv.local_path(volume).AndReturn(self.TEST_LOCAL_PATH) - mox.StubOutWithMock(drv, '_path_exists') - drv._path_exists(self.TEST_LOCAL_PATH).AndReturn(True) - mox.StubOutWithMock(drv, '_execute') drv._execute('rm', '-f', self.TEST_LOCAL_PATH, run_as_root=True) @@ -708,31 +628,6 @@ class NfsDriverTestCase(test.TestCase): mox.VerifyAll() - def test_delete_should_not_delete_if_there_is_no_file(self): - """delete_volume should not try to delete if file missed.""" - mox = self._mox - drv = self._driver - - self.stub_out_not_replaying(drv, '_ensure_share_mounted') - - volume = DumbVolume() - volume['name'] = 'volume-123' - volume['provider_location'] = self.TEST_NFS_EXPORT1 - - mox.StubOutWithMock(drv, 'local_path') - drv.local_path(volume).AndReturn(self.TEST_LOCAL_PATH) - - mox.StubOutWithMock(drv, '_path_exists') - drv._path_exists(self.TEST_LOCAL_PATH).AndReturn(False) - - mox.StubOutWithMock(drv, '_execute') - - mox.ReplayAll() - - drv.delete_volume(volume) - - mox.VerifyAll() - def test_get_volume_stats(self): """get_volume_stats must fill the correct values""" mox = self._mox diff --git a/cinder/volume/drivers/glusterfs.py b/cinder/volume/drivers/glusterfs.py index 17252bf07..3e287411f 100644 --- a/cinder/volume/drivers/glusterfs.py +++ b/cinder/volume/drivers/glusterfs.py @@ -113,13 +113,6 @@ class GlusterfsDriver(nfs.RemoteFsDriver): mounted_path = self.local_path(volume) - if not self._path_exists(mounted_path): - volume = volume['name'] - - LOG.warn(_('Trying to delete non-existing volume %(volume)s at ' - 'path %(mounted_path)s') % locals()) - return - self._execute('rm', '-f', mounted_path, run_as_root=True) def ensure_export(self, ctx, volume): @@ -244,8 +237,7 @@ class GlusterfsDriver(nfs.RemoteFsDriver): def _mount_glusterfs(self, glusterfs_share, mount_path, ensure=False): """Mount GlusterFS share to mount path.""" - if not self._path_exists(mount_path): - self._execute('mkdir', '-p', mount_path) + self._execute('mkdir', '-p', mount_path) try: self._execute('mount', '-t', 'glusterfs', glusterfs_share, diff --git a/cinder/volume/drivers/nfs.py b/cinder/volume/drivers/nfs.py index 67b9d6abd..4638e1268 100755 --- a/cinder/volume/drivers/nfs.py +++ b/cinder/volume/drivers/nfs.py @@ -104,17 +104,6 @@ class RemoteFsDriver(driver.VolumeDriver): return os.path.join(self._get_mount_point_for_share(nfs_share), volume['name']) - def _path_exists(self, path): - """Check for existence of given path.""" - try: - self._execute('stat', path, run_as_root=True) - return True - except exception.ProcessExecutionError as exc: - if 'No such file or directory' in exc.stderr: - return False - else: - raise - def _get_hash_str(self, base_str): """returns string that represents hash of base_str (in a hex format).""" @@ -179,13 +168,6 @@ class NfsDriver(RemoteFsDriver): mounted_path = self.local_path(volume) - if not self._path_exists(mounted_path): - volume = volume['name'] - - LOG.warn(_('Trying to delete non-existing volume %(volume)s at ' - 'path %(mounted_path)s') % locals()) - return - self._execute('rm', '-f', mounted_path, run_as_root=True) def ensure_export(self, ctx, volume): @@ -309,8 +291,7 @@ class NfsDriver(RemoteFsDriver): def _mount_nfs(self, nfs_share, mount_path, ensure=False): """Mount NFS share to mount path""" - if not self._path_exists(mount_path): - self._execute('mkdir', '-p', mount_path) + self._execute('mkdir', '-p', mount_path) # Construct the NFS mount command. nfs_cmd = ['mount', '-t', 'nfs'] -- 2.45.2