From aa7b9656d29595e78ddfff1fb3341908343cf39c Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Mar 29 2023 20:28:26 +0000 Subject: [PATCH 1/3] additional logging in rmtree --- diff --git a/koji/util.py b/koji/util.py index 504e223..335b6b1 100644 --- a/koji/util.py +++ b/koji/util.py @@ -457,13 +457,15 @@ def rmtree(path, logger=None): if e.errno in (errno.ENOENT, errno.ESTALE): # likely racing with another rmtree # if the dir doesn't exist, we're done + logger.warning("Directory to remove has disappeared: %s" % path) return raise try: - _rmtree(dev) - except _RetryRmtree: + _rmtree(dev, logger) + except _RetryRmtree as e: # reset and retry os.chdir(cwd) + logger.warning("Retrying rmtree due to %s" % e) continue break finally: @@ -477,7 +479,7 @@ def rmtree(path, logger=None): raise -def _rmtree(dev): +def _rmtree(dev, logger): """Remove all contents of CWD""" # This implementation avoids forming long paths and recursion. Otherwise # we will have errors with very deep directory trees. @@ -507,11 +509,12 @@ def _rmtree(dev): empty_dir = dirs.pop() try: os.rmdir(empty_dir) - except OSError: + except OSError as e: # If this happens, either something else is writing to the dir, # or there is a bug in our code. # For now, we ignore this and proceed, but we'll still fail at # the top level rmdir + logger.error("Unable to remove directory %s: %s" % (empty_dir, e)) pass if not dirs: @@ -529,6 +532,7 @@ def _rmtree(dev): # we'll ignore this and continue # since subdir doesn't exist, we'll pop it off and forget about it dirs.pop() + logger.warning("Subdir disappeared during rmtree %s: %s" % (subdir, e)) continue # with dirstack unchanged raise dirstack.append(dirs) From 6ba7b26e1e25c9164f5fb8f1a5645696135091cd Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Mar 29 2023 20:37:03 +0000 Subject: [PATCH 2/3] more rmtree logging --- diff --git a/koji/util.py b/koji/util.py index 335b6b1..f3e5dce 100644 --- a/koji/util.py +++ b/koji/util.py @@ -490,7 +490,7 @@ def _rmtree(dev, logger): # As we descend into the tree, we append a new entry to dirstack # When we ascend back up after removal, we pop them off while True: - dirs = _stripcwd(dev) + dirs = _stripcwd(dev, logger) # if cwd has no subdirs, walk back up until we find some while not dirs and dirstack: @@ -538,14 +538,15 @@ def _rmtree(dev, logger): dirstack.append(dirs) -def _stripcwd(dev): +def _stripcwd(dev, logger): """Unlink all files in cwd and return list of subdirs""" dirs = [] 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): + # cwd could have been removed by others, just return an empty list + logger.warning("Unable to read directory: %s" % e) return dirs raise for fn in fdirs: From fc184c301f97b5d04dec6f38fff649edca581878 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Mar 29 2023 20:58:30 +0000 Subject: [PATCH 3/3] adjust unit tests for new args --- diff --git a/tests/test_lib/test_utils.py b/tests/test_lib/test_utils.py index 797fa44..f0d576d 100644 --- a/tests/test_lib/test_utils.py +++ b/tests/test_lib/test_utils.py @@ -1270,10 +1270,11 @@ class TestRmtree(unittest.TestCase): isdir.return_value = True getcwd.return_value = 'cwd' path = '/mnt/folder' + logger = mock.MagicMock() - self.assertEqual(koji.util.rmtree(path), None) + self.assertEqual(koji.util.rmtree(path, logger), None) chdir.assert_called_with('cwd') - _rmtree.assert_called_once_with('dev') + _rmtree.assert_called_once_with('dev', logger) rmdir.assert_called_once_with(path) @patch('koji.util._rmtree') @@ -1303,10 +1304,11 @@ class TestRmtree(unittest.TestCase): def test_rmtree_internal_empty(self, stripcwd, rmdir, chdir): dev = 'dev' stripcwd.return_value = [] + logger = mock.MagicMock() - koji.util._rmtree(dev) + koji.util._rmtree(dev, logger) - stripcwd.assert_called_once_with(dev) + stripcwd.assert_called_once_with(dev, logger) rmdir.assert_not_called() chdir.assert_not_called() @@ -1316,10 +1318,11 @@ class TestRmtree(unittest.TestCase): def test_rmtree_internal_dirs(self, stripcwd, rmdir, chdir): dev = 'dev' stripcwd.side_effect = (['a', 'b'], [], []) + logger = mock.MagicMock() - koji.util._rmtree(dev) + koji.util._rmtree(dev, logger) - stripcwd.assert_has_calls([call(dev), call(dev), call(dev)]) + stripcwd.assert_has_calls([call(dev, logger), call(dev, logger), call(dev, logger)]) rmdir.assert_has_calls([call('b'), call('a')]) chdir.assert_has_calls([call('b'), call('..'), call('a'), call('..')]) @@ -1330,11 +1333,12 @@ class TestRmtree(unittest.TestCase): dev = 'dev' stripcwd.side_effect = (['a', 'b'], [], []) rmdir.side_effect = OSError() + logger = mock.MagicMock() # don't fail on anything - koji.util._rmtree(dev) + koji.util._rmtree(dev, logger) - stripcwd.assert_has_calls([call(dev), call(dev), call(dev)]) + stripcwd.assert_has_calls([call(dev, logger), call(dev, logger), call(dev, logger)]) rmdir.assert_has_calls([call('b'), call('a')]) chdir.assert_has_calls([call('b'), call('..'), call('a'), call('..')]) @@ -1346,8 +1350,9 @@ class TestRmtree(unittest.TestCase): # simple empty directory dev = 'dev' listdir.return_value = [] + logger = mock.MagicMock() - koji.util._stripcwd(dev) + koji.util._stripcwd(dev, logger) listdir.assert_called_once_with('.') unlink.assert_not_called() @@ -1367,8 +1372,9 @@ class TestRmtree(unittest.TestCase): st.st_mode = 'mode' lstat.return_value = st isdir.side_effect = [True, False] + logger = mock.MagicMock() - koji.util._stripcwd(dev) + koji.util._stripcwd(dev, logger) listdir.assert_called_once_with('.') unlink.assert_called_once_with('b') @@ -1391,8 +1397,9 @@ class TestRmtree(unittest.TestCase): st2.st_mode = 'mode' lstat.side_effect = [st1, st2] isdir.side_effect = [True, False] + logger = mock.MagicMock() - koji.util._stripcwd(dev) + koji.util._stripcwd(dev, logger) listdir.assert_called_once_with('.') unlink.assert_not_called() @@ -1413,8 +1420,9 @@ class TestRmtree(unittest.TestCase): lstat.return_value = st isdir.side_effect = [True, False] unlink.side_effect = OSError() + logger = mock.MagicMock() - koji.util._stripcwd(dev) + koji.util._stripcwd(dev, logger) listdir.assert_called_once_with('.') unlink.assert_called_once_with('b') @@ -1430,8 +1438,9 @@ class TestRmtree(unittest.TestCase): dev = 'dev' listdir.return_value = ['will-not-exist.txt'] lstat.side_effect = OSError(errno.ENOENT, 'No such file or directory') + logger = mock.MagicMock() - koji.util._stripcwd(dev) + koji.util._stripcwd(dev, logger) listdir.assert_called_once_with('.') lstat.assert_called_once_with('will-not-exist.txt')