Skip to content

Commit

Permalink
Remove _path_exists method.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mouadino committed Apr 25, 2013
1 parent f4f75ca commit 51218b2
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 229 deletions.
94 changes: 3 additions & 91 deletions cinder/tests/test_glusterfs.py
Expand Up @@ -81,39 +81,6 @@ def stub_out_not_replaying(self, obj, attr_name):
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
Expand All @@ -132,10 +99,8 @@ def test_mount_glusterfs_should_mount_correctly(self):
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)

Expand All @@ -152,10 +117,8 @@ def test_mount_glusterfs_should_suppress_already_mounted_error(self):
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(
Expand All @@ -175,10 +138,8 @@ def test_mount_glusterfs_should_reraise_already_mounted_error(self):
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',
Expand All @@ -202,9 +163,6 @@ def test_mount_glusterfs_should_create_mountpoint_if_not_yet(self):
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())
Expand All @@ -215,24 +173,6 @@ def test_mount_glusterfs_should_create_mountpoint_if_not_yet(self):

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
Expand Down Expand Up @@ -590,9 +530,6 @@ def test_delete_volume(self):
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)

Expand Down Expand Up @@ -640,28 +577,3 @@ def test_delete_should_not_delete_if_provider_location_not_provided(self):
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()
113 changes: 4 additions & 109 deletions cinder/tests/test_nfs.py
Expand Up @@ -96,39 +96,6 @@ def test_set_rw_permissions_for_all(self):

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
Expand Down Expand Up @@ -171,39 +138,6 @@ def stub_out_not_replaying(self, obj, attr_name):
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
Expand All @@ -222,10 +156,8 @@ def test_mount_nfs_should_mount_correctly(self):
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)

Expand All @@ -241,10 +173,8 @@ def test_mount_nfs_should_suppress_already_mounted_error(self):
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(
Expand All @@ -262,10 +192,8 @@ def test_mount_nfs_should_reraise_already_mounted_error(self):
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',
Expand All @@ -287,9 +215,6 @@ def test_mount_nfs_should_create_mountpoint_if_not_yet(self):
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())
Expand All @@ -305,10 +230,8 @@ def test_mount_nfs_should_not_create_mountpoint_if_already(self):
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()
Expand Down Expand Up @@ -657,9 +580,6 @@ def test_delete_volume(self):
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)

Expand Down Expand Up @@ -708,31 +628,6 @@ def test_delete_should_not_delete_if_provider_location_not_provided(self):

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
Expand Down
10 changes: 1 addition & 9 deletions cinder/volume/drivers/glusterfs.py
Expand Up @@ -113,13 +113,6 @@ def delete_volume(self, volume):

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):
Expand Down Expand Up @@ -244,8 +237,7 @@ def _get_available_capacity(self, glusterfs_share):

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,
Expand Down
21 changes: 1 addition & 20 deletions cinder/volume/drivers/nfs.py
Expand Up @@ -104,17 +104,6 @@ def local_path(self, volume):
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)."""
Expand Down Expand Up @@ -179,13 +168,6 @@ def delete_volume(self, volume):

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):
Expand Down Expand Up @@ -309,8 +291,7 @@ def _get_available_capacity(self, nfs_share):

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']
Expand Down

0 comments on commit 51218b2

Please sign in to comment.