PR#658: consolidate safe_rmtree, rmtree and shutil.rmtree

Merges #658
https://pagure.io/koji/pull-request/658

Fixes: #648
https://pagure.io/koji/issue/648
Consolidate rmtree usage
This commit is contained in:
Mike McLean 2017-10-30 12:09:08 -04:00
commit 423eac9e7f
6 changed files with 325 additions and 112 deletions

View file

@ -4970,7 +4970,7 @@ def recycle_build(old, data):
update.execute()
builddir = koji.pathinfo.build(data)
if os.path.exists(builddir):
shutil.rmtree(builddir)
koji.util.rmtree(builddir)
koji.plugin.run_callbacks('postBuildStateChange', attribute='state',
old=old['state'], new=data['state'], info=data)
@ -7119,19 +7119,10 @@ def _delete_build(binfo):
update.execute()
update = """UPDATE build SET state=%(st_deleted)i WHERE id=%(build_id)i"""
_dml(update, locals())
#now clear the build dirs
dirs_to_clear = []
# now clear the build dir
builddir = koji.pathinfo.build(binfo)
if os.path.exists(builddir):
dirs_to_clear.append(builddir)
for filedir in dirs_to_clear:
rv = os.system(r"find '%s' -xdev \! -type d -print0 |xargs -0 rm -f" % filedir)
if rv != 0:
raise koji.GenericError('file removal failed (code %r) for %s' % (rv, filedir))
#and clear out the emptied dirs
rv = os.system(r"find '%s' -xdev -depth -type d -print0 |xargs -0 rmdir" % filedir)
if rv != 0:
raise koji.GenericError('directory removal failed (code %r) for %s' % (rv, filedir))
koji.util.rmtree(builddir)
koji.plugin.run_callbacks('postBuildStateChange', attribute='state', old=binfo['state'], new=st_deleted, info=binfo)
def reset_build(build):
@ -7195,19 +7186,10 @@ def reset_build(build):
binfo['state'] = koji.BUILD_STATES['CANCELED']
update = """UPDATE build SET state=%(state)s, task_id=NULL, volume_id=0 WHERE id=%(id)s"""
_dml(update, binfo)
#now clear the build dirs
dirs_to_clear = []
# now clear the build dir
builddir = koji.pathinfo.build(binfo)
if os.path.exists(builddir):
dirs_to_clear.append(builddir)
for filedir in dirs_to_clear:
rv = os.system(r"find '%s' -xdev \! -type d -print0 |xargs -0 rm -f" % filedir)
if rv != 0:
raise koji.GenericError('file removal failed (code %r) for %s' % (rv, filedir))
#and clear out the emptied dirs
rv = os.system(r"find '%s' -xdev -depth -type d -print0 |xargs -0 rmdir" % filedir)
if rv != 0:
raise koji.GenericError('directory removal failed (code %r) for %s' % (rv, filedir))
koji.util.rmtree(builddir)
koji.plugin.run_callbacks('postBuildStateChange', attribute='state', old=binfo['state'], new=koji.BUILD_STATES['CANCELED'], info=binfo)
def cancel_build(build_id, cancel_task=True):

View file

@ -67,8 +67,6 @@ def umount_all(topdir):
def safe_rmtree(path, unmount=False, strict=True):
logger = logging.getLogger("koji.build")
#safe remove: with -xdev the find cmd will not cross filesystems
# (though it will cross bind mounts from the same filesystem)
if unmount:
umount_all(path)
if os.path.isfile(path) or os.path.islink(path):
@ -80,30 +78,22 @@ def safe_rmtree(path, unmount=False, strict=True):
raise
else:
logger.warn("Error removing: %s", exc_info=True)
return
return 1
return 0
if not os.path.exists(path):
logger.debug("No such path: %s" % path)
return
#first rm -f non-directories
return 0
logger.debug('Scrubbing files in %s' % path)
rv = os.system("find '%s' -xdev \\! -type d -print0 |xargs -0 rm -f" % path)
msg = 'file removal failed (code %r) for %s' % (rv, path)
if rv != 0:
logger.warn(msg)
try:
koji.util.rmtree(path)
except Exception:
logger.warn('file removal failed for %s' % path)
if strict:
raise koji.GenericError(msg)
else:
return rv
#them rmdir directories
#with -depth, we start at the bottom and work up
logger.debug('Scrubbing directories in %s' % path)
rv = os.system("find '%s' -xdev -depth -type d -print0 |xargs -0 rmdir" % path)
msg = 'dir removal failed (code %r) for %s' % (rv, path)
if rv != 0:
logger.warn(msg)
if strict:
raise koji.GenericError(msg)
return rv
raise
return 1
return 0
class ServerExit(Exception):
"""Raised to shutdown the server"""

View file

@ -8,6 +8,7 @@
import koji
from koji.context import context
from koji.plugin import callback
from koji.util import rmtree
import ConfigParser
import fnmatch
import os
@ -42,13 +43,13 @@ def maven_import(cbtype, *args, **kws):
tmpdir = os.path.join(koji.pathinfo.work(), 'rpm2maven', koji.buildLabel(buildinfo))
try:
if os.path.exists(tmpdir):
shutil.rmtree(tmpdir)
rmtree(tmpdir)
koji.ensuredir(tmpdir)
expand_rpm(filepath, tmpdir)
scan_and_import(buildinfo, rpminfo, tmpdir)
finally:
if os.path.exists(tmpdir):
shutil.rmtree(tmpdir)
rmtree(tmpdir)
def expand_rpm(filepath, tmpdir):
devnull = file('/dev/null', 'r+')

View file

@ -16,7 +16,7 @@ class TestRecycleBuild():
side_effect=self.getUpdate).start()
self._dml = mock.patch('kojihub._dml').start()
self.run_callbacks = mock.patch('koji.plugin.run_callbacks').start()
self.rmtree = mock.patch('shutil.rmtree').start()
self.rmtree = mock.patch('koji.util.rmtree').start()
self.exists = mock.patch('os.path.exists').start()
self.updates = []

View file

@ -1,7 +1,7 @@
from __future__ import absolute_import
import mock
import unittest
from mock import call
from mock import call, patch
import os
import optparse
@ -488,6 +488,195 @@ class MavenUtilTestCase(unittest.TestCase):
config.readfp(conf_file)
return config
class TestRmtree(unittest.TestCase):
@patch('koji.util._rmtree')
@patch('os.rmdir')
@patch('os.chdir')
@patch('os.getcwd')
@patch('stat.S_ISDIR')
@patch('os.lstat')
def test_rmtree_file(self, lstat, isdir, getcwd, chdir, rmdir, _rmtree):
""" Tests that the koji.util.rmtree function raises error when the
path parameter is not a directory.
"""
stat = mock.MagicMock()
stat.st_dev = 'dev'
lstat.return_value = stat
isdir.return_value = False
getcwd.return_value = 'cwd'
with self.assertRaises(koji.GenericError):
koji.util.rmtree('/mnt/folder/some_file')
_rmtree.assert_not_called()
rmdir.assert_not_called()
@patch('koji.util._rmtree')
@patch('os.rmdir')
@patch('os.chdir')
@patch('os.getcwd')
@patch('stat.S_ISDIR')
@patch('os.lstat')
def test_rmtree_directory(self, lstat, isdir, getcwd, chdir, rmdir, _rmtree):
""" Tests that the koji.util.rmtree function returns nothing when the path is a directory.
"""
stat = mock.MagicMock()
stat.st_dev = 'dev'
lstat.return_value = stat
isdir.return_value = True
getcwd.return_value = 'cwd'
path = '/mnt/folder'
self.assertEquals(koji.util.rmtree(path), None)
chdir.assert_called_with('cwd')
_rmtree.assert_called_once_with('dev')
rmdir.assert_called_once_with(path)
@patch('koji.util._rmtree')
@patch('os.rmdir')
@patch('os.chdir')
@patch('os.getcwd')
@patch('stat.S_ISDIR')
@patch('os.lstat')
def test_rmtree_directory_scrub_failure(self, lstat, isdir, getcwd, chdir, rmdir, _rmtree):
""" Tests that the koji.util.rmtree function returns a GeneralException
when the scrub of the files in the directory fails.
"""
stat = mock.MagicMock()
stat.st_dev = 'dev'
lstat.return_value = stat
isdir.return_value = True
getcwd.return_value = 'cwd'
path = '/mnt/folder'
_rmtree.side_effect = OSError('xyz')
with self.assertRaises(OSError):
koji.util.rmtree(path)
@patch('os.chdir')
@patch('os.rmdir')
@patch('koji.util._stripcwd')
def test_rmtree_internal_empty(self, stripcwd, rmdir, chdir):
dev = 'dev'
stripcwd.return_value = []
koji.util._rmtree(dev)
stripcwd.assert_called_once_with(dev)
rmdir.assert_not_called()
chdir.assert_not_called()
@patch('os.chdir')
@patch('os.rmdir')
@patch('koji.util._stripcwd')
def test_rmtree_internal_dirs(self, stripcwd, rmdir, chdir):
dev = 'dev'
stripcwd.side_effect = (['a', 'b'], [], [])
koji.util._rmtree(dev)
stripcwd.assert_has_calls([call(dev), call(dev), call(dev)])
rmdir.assert_has_calls([call('b'), call('a')])
chdir.assert_has_calls([call('b'), call('..'), call('a'), call('..')])
@patch('os.chdir')
@patch('os.rmdir')
@patch('koji.util._stripcwd')
def test_rmtree_internal_fail(self, stripcwd, rmdir, chdir):
dev = 'dev'
stripcwd.side_effect = (['a', 'b'], [], [])
rmdir.side_effect = OSError()
# don't fail on anything
koji.util._rmtree(dev)
stripcwd.assert_has_calls([call(dev), call(dev), call(dev)])
rmdir.assert_has_calls([call('b'), call('a')])
chdir.assert_has_calls([call('b'), call('..'), call('a'), call('..')])
@patch('os.listdir')
@patch('os.lstat')
@patch('stat.S_ISDIR')
@patch('os.unlink')
def test_stripcwd_empty(dev, unlink, isdir, lstat, listdir):
# simple empty directory
dev = 'dev'
listdir.return_value = []
koji.util._stripcwd(dev)
listdir.assert_called_once_with('.')
unlink.assert_not_called()
isdir.assert_not_called()
lstat.assert_not_called()
@patch('os.listdir')
@patch('os.lstat')
@patch('stat.S_ISDIR')
@patch('os.unlink')
def test_stripcwd_all(dev, unlink, isdir, lstat, listdir):
# test valid file + dir
dev = 'dev'
listdir.return_value = ['a', 'b']
st = mock.MagicMock()
st.st_dev = dev
st.st_mode = 'mode'
lstat.return_value = st
isdir.side_effect = [True, False]
koji.util._stripcwd(dev)
listdir.assert_called_once_with('.')
unlink.assert_called_once_with('b')
isdir.assert_has_calls([call('mode'), call('mode')])
lstat.assert_has_calls([call('a'), call('b')])
@patch('os.listdir')
@patch('os.lstat')
@patch('stat.S_ISDIR')
@patch('os.unlink')
def test_stripcwd_diffdev(dev, unlink, isdir, lstat, listdir):
# ignore files on different devices
dev = 'dev'
listdir.return_value = ['a', 'b']
st1 = mock.MagicMock()
st1.st_dev = dev
st1.st_mode = 'mode'
st2 = mock.MagicMock()
st2.st_dev = 'other_dev'
st2.st_mode = 'mode'
lstat.side_effect = [st1, st2]
isdir.side_effect = [True, False]
koji.util._stripcwd(dev)
listdir.assert_called_once_with('.')
unlink.assert_not_called()
isdir.assert_called_once_with('mode')
lstat.assert_has_calls([call('a'), call('b')])
@patch('os.listdir')
@patch('os.lstat')
@patch('stat.S_ISDIR')
@patch('os.unlink')
def test_stripcwd_fails(dev, unlink, isdir, lstat, listdir):
# ignore all unlink errors
dev = 'dev'
listdir.return_value = ['a', 'b']
st = mock.MagicMock()
st.st_dev = dev
st.st_mode = 'mode'
lstat.return_value = st
isdir.side_effect = [True, False]
unlink.side_effect = OSError()
koji.util._stripcwd(dev)
listdir.assert_called_once_with('.')
unlink.assert_called_once_with('b')
isdir.assert_has_calls([call('mode'), call('mode')])
lstat.assert_has_calls([call('a'), call('b')])
if __name__ == '__main__':
unittest.main()

View file

@ -1,7 +1,7 @@
from __future__ import absolute_import
import random
import shutil
from os import path, makedirs
from shutil import rmtree
from tempfile import gettempdir
from unittest import TestCase
from mock import patch, MagicMock, Mock, call
@ -74,7 +74,7 @@ class TasksTestCase(TestCase):
temp_dir_root = get_temp_dir_root()
if path.isdir(temp_dir_root):
rmtree(get_temp_dir_root())
shutil.rmtree(get_temp_dir_root())
def test_scan_mounts_results(self):
""" Tests the scan_mounts function with a mocked /proc/mounts file. A list containing mount points
@ -128,66 +128,6 @@ class TasksTestCase(TestCase):
except koji.GenericError as e:
self.assertEquals(e.args[0], 'Unmounting incomplete: [\'/dev/shm\', \'/dev/mqueue\']')
@patch('os.path.isfile', return_value=True)
@patch('os.remove')
def test_safe_rmtree_file(self, mock_remove, mock_isfile):
""" Tests that the safe_rmtree function returns nothing when the path parameter is a file.
"""
self.assertEquals(safe_rmtree('/mnt/folder/some_file', False, True), None)
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=True)
@patch('os.remove')
def test_safe_rmtree_link(self, mock_remove, mock_islink, mock_isfile):
""" Tests that the safe_rmtree function returns nothing when the path parameter is a link.
"""
self.assertEquals(safe_rmtree('/mnt/folder/some_link', False, True), None)
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=False)
@patch('os.path.exists', return_value=False)
def test_safe_rmtree_does_not_exist(self, mock_exists, mock_islink, mock_isfile):
""" Tests that the safe_rmtree function returns nothing if the path does not exist.
"""
self.assertEquals(safe_rmtree('/mnt/folder/some_file', False, True), None)
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=False)
@patch('os.path.exists', return_value=True)
@patch('os.system', side_effect=[0, 0])
def test_safe_rmtree_directory(self, mock_os_system, mock_exists, mock_islink, mock_isfile):
""" Tests that the safe_rmtree function returns nothing when the path is a directory.
"""
self.assertEquals(safe_rmtree('/mnt/folder', False, True), 0)
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=False)
@patch('os.path.exists', return_value=True)
@patch('os.system', side_effect=[1, 0])
def test_safe_rmtree_directory_scrub_file_failure(self, mock_os_system, mock_exists, mock_islink, mock_isfile):
""" Tests that the safe_rmtree function returns a GeneralException when the path parameter is a directory
and the scrub of the files in the directory fails.
"""
try:
safe_rmtree('/mnt/folder', False, True)
raise Exception('A GenericError was not raised during the test')
except koji.GenericError as e:
self.assertEquals(e.args[0], 'file removal failed (code 1) for /mnt/folder')
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=False)
@patch('os.path.exists', return_value=True)
@patch('os.system', side_effect=[0, 1])
def test_safe_rmtree_directory_scrub_directory_failure(self, mock_os_system, mock_exists, mock_islink, mock_isfile):
""" Tests that the safe_rmtree function returns a GeneralException when the path parameter is a directory
and the scrub of the directories in the directory fails.
"""
try:
safe_rmtree('/mnt/folder', False, True)
raise Exception('A GenericError was not raised during the test')
except koji.GenericError as e:
self.assertEquals(e.args[0], 'dir removal failed (code 1) for /mnt/folder')
def test_BaseTaskHandler_handler_not_set(self):
""" Tests that an exception is thrown when the handler function is not overwritten by the child class.
"""
@ -230,7 +170,7 @@ class TasksTestCase(TestCase):
obj = TestTask(123, 'some_method', ['random_arg'], None, None, temp_path)
obj.createWorkdir()
self.assertEquals(path.isdir(temp_path), True)
rmtree(get_temp_dir_root())
shutil.rmtree(get_temp_dir_root())
def test_BaseTaskHandler_removeWorkdir(self):
""" Tests that the removeWOrkdir function deletes a folder based on the path given to the
@ -759,3 +699,114 @@ class TasksTestCase(TestCase):
# getTaskResult should be called in 2nd round only for task 3, as 4
# will be skipped as 'canfail'
obj.session.getTaskResult.assert_has_calls([call(3)])
class TestSafeRmtree(TestCase):
@patch('os.path.exists', return_value=True)
@patch('os.path.isfile', return_value=True)
@patch('os.path.islink', return_value=False)
@patch('os.remove')
@patch('koji.util.rmtree')
def test_safe_rmtree_file(self, rmtree, remove, islink, isfile, exists):
""" Tests that the koji.util.rmtree function returns nothing when the path parameter is a file.
"""
path = '/mnt/folder/some_file'
self.assertEquals(safe_rmtree(path, False, True), 0)
isfile.assert_called_once_with(path)
islink.assert_not_called()
exists.assert_not_called()
remove.assert_called_once_with(path)
rmtree.assert_not_called()
@patch('os.path.exists', return_value=True)
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=True)
@patch('os.remove')
@patch('koji.util.rmtree')
def test_rmtree_link(self, rmtree, remove, islink, isfile, exists):
""" Tests that the koji.util.rmtree function returns nothing when the path parameter is a link.
"""
path = '/mnt/folder/some_link'
self.assertEquals(safe_rmtree(path, False, True), 0)
isfile.assert_called_once_with(path)
islink.assert_called_once_with(path)
exists.assert_not_called()
remove.assert_called_once_with(path)
rmtree.assert_not_called()
@patch('os.path.exists', return_value=False)
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=False)
@patch('os.remove')
@patch('koji.util.rmtree')
def test_rmtree_does_not_exist(self, rmtree, remove, islink, isfile, exists):
""" Tests that the koji.util.rmtree function returns nothing if the path does not exist.
"""
path = '/mnt/folder/some_file'
self.assertEquals(safe_rmtree(path, False, True), 0)
isfile.assert_called_once_with(path)
islink.assert_called_once_with(path)
exists.assert_called_once_with(path)
remove.assert_not_called()
rmtree.assert_not_called()
@patch('os.path.exists', return_value=True)
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=False)
@patch('os.remove')
@patch('koji.util.rmtree')
def test_rmtree_directory(self, rmtree, remove, islink, isfile, exists):
""" Tests that the koji.util.rmtree function returns nothing when the path is a directory.
"""
path = '/mnt/folder'
self.assertEquals(safe_rmtree(path, False, True), 0)
isfile.assert_called_once_with(path)
islink.assert_called_once_with(path)
exists.assert_called_once_with(path)
remove.assert_not_called()
rmtree.assert_called_once_with(path)
@patch('os.path.exists', return_value=True)
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=False)
@patch('os.remove')
@patch('koji.util.rmtree')
def test_rmtree_directory_scrub_file_failure(self, rmtree, remove, islink, isfile, exists):
""" Tests that the koji.util.rmtree function returns a GeneralException when the path parameter is a directory
and the scrub of the files in the directory fails.
"""
rmtree.side_effect = koji.GenericError('xyz')
path = '/mnt/folder'
try:
safe_rmtree(path, False, 1)
raise Exception('A GenericError was not raised during the test')
except koji.GenericError as e:
self.assertEquals(e.args[0], 'xyz')
isfile.assert_called_once_with(path)
islink.assert_called_once_with(path)
exists.assert_called_once_with(path)
remove.assert_not_called()
rmtree.assert_called_once_with(path)
@patch('os.path.exists', return_value=True)
@patch('os.path.isfile', return_value=False)
@patch('os.path.islink', return_value=False)
@patch('os.remove')
@patch('koji.util.rmtree')
def test_safe_rmtree_directory_scrub_directory_failure(self, rmtree, remove, islink, isfile, exists):
""" Tests that the koji.util.rmtree function returns a GeneralException when the path parameter is a directory
and the scrub of the directories in the directory fails.
"""
rmtree.side_effect = OSError('xyz')
path = '/mnt/folder'
try:
safe_rmtree(path, False, True)
raise Exception('An OSError was not raised during the test')
except OSError as e:
self.assertEquals(e.args[0], 'xyz')
isfile.assert_called_once_with(path)
islink.assert_called_once_with(path)
exists.assert_called_once_with(path)
remove.assert_not_called()
rmtree.assert_called_once_with(path)