diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index b43e5509c2..414bc97ae1 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -1108,7 +1108,7 @@ def cleanup(self, remove_files=False, unload_env=True): if remove_files: self.logger.debug('removing stage directory') - shutil.rmtree(self._stagedir) + os_ext.rmtree(self._stagedir) if unload_env: self.logger.debug("unloading test's environment") diff --git a/reframe/core/runtime.py b/reframe/core/runtime.py index 449125d0ab..01e4171b0a 100644 --- a/reframe/core/runtime.py +++ b/reframe/core/runtime.py @@ -5,7 +5,6 @@ import os import functools import re -import shutil import socket from datetime import datetime @@ -107,7 +106,7 @@ def __init__(self, prefix=None, stagedir=None, def _makedir(self, *dirs, wipeout=False): ret = os.path.join(*dirs) if wipeout: - shutil.rmtree(ret, True) + os_ext.rmtree(ret, ignore_errors=True) os.makedirs(ret, exist_ok=True) return ret diff --git a/reframe/utility/os_ext.py b/reframe/utility/os_ext.py index befb2ec5ae..846d0d3a4b 100644 --- a/reframe/utility/os_ext.py +++ b/reframe/utility/os_ext.py @@ -2,6 +2,7 @@ # OS and shell utility functions # +import errno import getpass import grp import os @@ -172,6 +173,39 @@ def ignore(dir, contents): os.symlink(f, link_name) +def rmtree(*args, max_retries=3, **kwargs): + """Persistent version of ``shutil.rmtree()``. + + If ``shutil.rmtree()`` with ``ENOTEMPTY``, retry up to ``max_retries`` + times to delete the directory. + + This version of ``rmtree()`` is mostly provided to work around a race + condition between when ``sacct`` reports a job as completed and when the + Slurm epilog runs. See https://github.com/eth-cscs/reframe/issues/291 for + more information. + + ``args`` and ``kwargs`` are passed through to ``shutil.rmtree()``. + + If ``onerror`` is specified in ``kwargs`` and is not :class:`None`, this + function is completely equivalent to ``shutil.rmtree()``. + """ + if 'onerror' in kwargs and kwargs['onerror'] is not None: + shutil.rmtree(*args, **kwargs) + return + + for i in range(max_retries): + try: + shutil.rmtree(*args, **kwargs) + return + except OSError as e: + if i == max_retries: + raise + elif e.errno == errno.ENOTEMPTY: + pass + else: + raise + + def inpath(entry, pathvar): """Check if entry is in pathvar. pathvar is a string of the form `entry1:entry2:entry3`.""" diff --git a/unittests/test_cli.py b/unittests/test_cli.py index e07a938070..750c4aeed6 100644 --- a/unittests/test_cli.py +++ b/unittests/test_cli.py @@ -2,7 +2,6 @@ import itertools import os import re -import shutil import sys import tempfile import unittest @@ -93,8 +92,8 @@ def setUp(self): self.perflogdir = '.rfm-perflogs' def tearDown(self): - shutil.rmtree(self.prefix) - shutil.rmtree(self.perflogdir, ignore_errors=True) + os_ext.rmtree(self.prefix) + os_ext.rmtree(self.perflogdir, ignore_errors=True) os_ext.force_remove_file(self.logfile) def _run_reframe(self): diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index 7fc1647449..5024795d62 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -1,9 +1,9 @@ import os -import shutil import tempfile import unittest import reframe.core.runtime as rt +import reframe.utility.os_ext as os_ext import reframe.utility.sanity as sn import unittests.fixtures as fixtures from reframe.core.exceptions import (BuildError, PipelineError, ReframeError, @@ -37,8 +37,8 @@ def setUp(self): rt.runtime().resources.prefix = tempfile.mkdtemp(dir='unittests') def tearDown(self): - shutil.rmtree(rt.runtime().resources.prefix, ignore_errors=True) - shutil.rmtree('.rfm_testing', ignore_errors=True) + os_ext.rmtree(rt.runtime().resources.prefix) + os_ext.rmtree('.rfm_testing', ignore_errors=True) def replace_prefix(self, filename, new_prefix): basename = os.path.basename(filename) @@ -477,7 +477,7 @@ def tearDown(self): self.output_file.close() os.remove(self.perf_file.name) os.remove(self.output_file.name) - shutil.rmtree(self.resourcesdir) + os_ext.rmtree(self.resourcesdir) def write_performance_output(self, fp=None, **kwargs): if not fp: diff --git a/unittests/test_policies.py b/unittests/test_policies.py index 4f598c0d31..49d41b9ac7 100644 --- a/unittests/test_policies.py +++ b/unittests/test_policies.py @@ -1,11 +1,11 @@ import os -import shutil import tempfile import unittest import reframe.core.runtime as rt import reframe.frontend.executors as executors import reframe.frontend.executors.policies as policies +import reframe.utility.os_ext as os_ext from reframe.core.exceptions import JobNotStartedError from reframe.frontend.loader import RegressionCheckLoader from unittests.resources.checks.hellocheck import HelloTest @@ -27,7 +27,7 @@ def setUp(self): rt.runtime().resources.prefix = tempfile.mkdtemp(dir='unittests') def tearDown(self): - shutil.rmtree(rt.runtime().resources.prefix, ignore_errors=True) + os_ext.rmtree(rt.runtime().resources.prefix) def _num_failures_stage(self, stage): stats = self.runner.stats diff --git a/unittests/test_schedulers.py b/unittests/test_schedulers.py index 113016b301..8354698389 100644 --- a/unittests/test_schedulers.py +++ b/unittests/test_schedulers.py @@ -1,7 +1,6 @@ import abc import os import re -import shutil import tempfile import time import unittest @@ -36,7 +35,7 @@ def setUp(self): self.parallel_cmd = 'hostname' def tearDown(self): - shutil.rmtree(self.workdir) + os_ext.rmtree(self.workdir) @property def commands(self): @@ -547,7 +546,7 @@ def setUp(self): self.testjob._num_tasks = 0 def tearDown(self): - shutil.rmtree(self.workdir) + os_ext.rmtree(self.workdir) def test_valid_constraint(self, expected_num_tasks=8): self.testjob._sched_reservation = 'Foo' diff --git a/unittests/test_utility.py b/unittests/test_utility.py index 2f3496b734..94b7ca9ce3 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -73,6 +73,26 @@ def test_copytree_src_parent_of_dst(self): shutil.rmtree(dst_path) + def _test_rmtree(self, *args, **kwargs): + testdir = tempfile.mkdtemp() + with open(os.path.join(testdir, 'foo.txt'), 'w') as fp: + fp.write('hello\n') + + os_ext.rmtree(testdir, *args, **kwargs) + self.assertFalse(os.path.exists(testdir)) + + def test_rmtree(self): + self._test_rmtree() + + def test_rmtree_onerror(self): + self._test_rmtree(onerror=lambda *args: None) + + def test_rmtree_error(self): + # Try to remove an inexistent directory + testdir = tempfile.mkdtemp() + os.rmdir(testdir) + self.assertRaises(OSError, os_ext.rmtree, testdir) + def test_inpath(self): self.assertTrue(os_ext.inpath('/foo/bin', '/bin:/foo/bin:/usr/bin')) self.assertFalse(os_ext.inpath('/foo/bin', '/bin:/usr/local/bin'))