util: [rmtree] try best to catch errors for race condition

fixes: #2481

This approach is ugly, but just working.

ENOENT and ESTALE errors are catched in `chdir`, `listdir` calls to stomach the deletion by other process/thread

ENOTEMPTY is catched when calling `os.rmdir(path)` in `rmtree()` too. It happens when `path` is on an NFS like where ESTALE happens.
This commit is contained in:
Yu Ming Zhu 2020-10-02 12:01:18 +00:00 committed by Tomas Kopecek
parent fa56a5b25b
commit 52a63f732d
2 changed files with 67 additions and 18 deletions

View file

@ -428,29 +428,44 @@ def lazysetattr(object, name, func, args, kwargs=None, cache=False):
setattr(object, name, value)
def rmtree(path):
def rmtree(path, logger=None):
"""Delete a directory tree without crossing fs boundaries"""
# implemented to avoid forming long paths
# see: https://pagure.io/koji/issue/201
logger = logger or logging.getLogger('koji')
st = os.lstat(path)
if not stat.S_ISDIR(st.st_mode):
raise koji.GenericError("Not a directory: %s" % path)
dev = st.st_dev
cwd = os.getcwd()
root = os.path.abspath(path)
try:
try:
os.chdir(path)
_rmtree(dev)
except OSError as e:
if e.errno not in (errno.ENOENT, errno.ESTALE):
return
raise
_rmtree(dev, root)
finally:
os.chdir(cwd)
try:
os.rmdir(path)
except OSError as e:
if e.errno == errno.ENOTEMPTY:
logger.warning('%s path is not empty, but it may be a phantom error caused by some'
' race condition', path, exc_info=True)
elif e.errno != errno.ENOENT:
raise
def _rmtree(dev):
def _rmtree(dev, root):
dirstack = []
while True:
dirs = _stripcwd(dev)
# if no dirs, walk back up until we find some
while not dirs and dirstack:
try:
os.chdir('..')
dirs = dirstack.pop()
empty_dir = dirs.pop()
@ -459,6 +474,20 @@ def _rmtree(dev):
except OSError:
# we'll still fail at the top level
pass
except OSError as e:
if e.errno in (errno.ENOENT, errno.ESTALE):
# go back to root if chdir fails
dirstack = []
dirs = _stripcwd(dev)
try:
os.chdir(root)
except OSError as e:
# root has been deleted
if e.errno == errno.ENOENT:
return
raise
else:
raise
if not dirs:
# we are done
break
@ -466,13 +495,33 @@ def _rmtree(dev):
subdir = dirs[-1]
# note: we do not pop here because we need to remember to remove subdir later
dirstack.append(dirs)
try:
os.chdir(subdir)
except OSError as e:
# go back to root if subdir doesn't exist
if e.errno == errno.ENOENT:
dirstack = []
try:
os.chdir(root)
except OSError as e:
# root has been deleted
if e.errno == errno.ENOENT:
return
raise
else:
raise
def _stripcwd(dev):
"""Unlink all files in cwd and return list of subdirs"""
dirs = []
for fn in os.listdir('.'):
try:
fdirs = os.listdir('.')
except OSError as e:
# cwd has been removed by others, just return an empty list
if e.errno in (errno.ENOENT, errno.ESTALE):
return dirs
for fn in fdirs:
try:
st = os.lstat(fn)
except OSError as e:

View file

@ -1277,7 +1277,7 @@ class TestRmtree(unittest.TestCase):
self.assertEquals(koji.util.rmtree(path), None)
chdir.assert_called_with('cwd')
_rmtree.assert_called_once_with('dev')
_rmtree.assert_called_once_with('dev', path)
rmdir.assert_called_once_with(path)
@patch('koji.util._rmtree')
@ -1308,7 +1308,7 @@ class TestRmtree(unittest.TestCase):
dev = 'dev'
stripcwd.return_value = []
koji.util._rmtree(dev)
koji.util._rmtree(dev, 'any')
stripcwd.assert_called_once_with(dev)
rmdir.assert_not_called()
@ -1321,7 +1321,7 @@ class TestRmtree(unittest.TestCase):
dev = 'dev'
stripcwd.side_effect = (['a', 'b'], [], [])
koji.util._rmtree(dev)
koji.util._rmtree(dev, 'any')
stripcwd.assert_has_calls([call(dev), call(dev), call(dev)])
rmdir.assert_has_calls([call('b'), call('a')])
@ -1336,7 +1336,7 @@ class TestRmtree(unittest.TestCase):
rmdir.side_effect = OSError()
# don't fail on anything
koji.util._rmtree(dev)
koji.util._rmtree(dev, 'any')
stripcwd.assert_has_calls([call(dev), call(dev), call(dev)])
rmdir.assert_has_calls([call('b'), call('a')])