From 10e25db4befe1173af516a053ac01f0f7b6dac56 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Thu, 16 Aug 2012 16:51:04 -0700 Subject: [PATCH] Makes sure tests don't leave lockfiles around Our new lockfile code works fine if the locks already exist, but it is annoying to pollute the source tree with lockfiles in the tests. This modifies the lockfile code slightly to use a tempdir if lock_path is none. Note that a tempdir will not work in production but it is fine for testing. We set the lock_path to None in the testing flags. Also removes the explicit cleanup in run_tests.sh. The new solution works with tox and will work even if we add new locks. Fixes bug 1035426 Change-Id: I613926e4288442e79df50ea5f5bd8f9eafdfe7e7 --- nova/tests/fake_flags.py | 1 + nova/tests/test_misc.py | 70 +++++++++++++++++++++------------------- nova/utils.py | 21 ++++++++---- run_tests.sh | 2 -- 4 files changed, 52 insertions(+), 42 deletions(-) diff --git a/nova/tests/fake_flags.py b/nova/tests/fake_flags.py index aada0d7e3ca9..f6d9496b14e9 100644 --- a/nova/tests/fake_flags.py +++ b/nova/tests/fake_flags.py @@ -48,3 +48,4 @@ def set_defaults(conf): conf.set_default('api_paste_config', '$state_path/etc/nova/api-paste.ini') conf.set_default('rpc_response_timeout', 5) conf.set_default('rpc_cast_timeout', 5) + conf.set_default('lock_path', None) diff --git a/nova/tests/test_misc.py b/nova/tests/test_misc.py index 58d52290b65b..10443ecdeff7 100644 --- a/nova/tests/test_misc.py +++ b/nova/tests/test_misc.py @@ -108,47 +108,51 @@ class LockTestCase(test.TestCase): def test_nested_external_works(self): """We can nest external syncs""" - sentinel = object() + with utils.tempdir() as tempdir: + self.flags(lock_path=tempdir) + sentinel = object() - @utils.synchronized('testlock1', external=True) - def outer_lock(): + @utils.synchronized('testlock1', external=True) + def outer_lock(): - @utils.synchronized('testlock2', external=True) - def inner_lock(): - return sentinel - return inner_lock() + @utils.synchronized('testlock2', external=True) + def inner_lock(): + return sentinel + return inner_lock() - self.assertEqual(sentinel, outer_lock()) + self.assertEqual(sentinel, outer_lock()) def test_synchronized_externally(self): """We can lock across multiple processes""" - rpipe1, wpipe1 = os.pipe() - rpipe2, wpipe2 = os.pipe() + with utils.tempdir() as tempdir: + self.flags(lock_path=tempdir) + rpipe1, wpipe1 = os.pipe() + rpipe2, wpipe2 = os.pipe() - @utils.synchronized('testlock1', external=True) - def f(rpipe, wpipe): - try: - os.write(wpipe, "foo") - except OSError, e: - self.assertEquals(e.errno, errno.EPIPE) - return + @utils.synchronized('testlock1', external=True) + def f(rpipe, wpipe): + try: + os.write(wpipe, "foo") + except OSError, e: + self.assertEquals(e.errno, errno.EPIPE) + return - rfds, _wfds, _efds = select.select([rpipe], [], [], 1) - self.assertEquals(len(rfds), 0, "The other process, which was" - " supposed to be locked, " - "wrote on its end of the " - "pipe") - os.close(rpipe) + rfds, _wfds, _efds = select.select([rpipe], [], [], 1) + self.assertEquals(len(rfds), 0, "The other process, which was" + " supposed to be locked, " + "wrote on its end of the " + "pipe") + os.close(rpipe) - pid = os.fork() - if pid > 0: - os.close(wpipe1) - os.close(rpipe2) + pid = os.fork() + if pid > 0: + os.close(wpipe1) + os.close(rpipe2) - f(rpipe1, wpipe2) - else: - os.close(rpipe1) - os.close(wpipe2) + f(rpipe1, wpipe2) + else: + os.close(rpipe1) + os.close(wpipe2) - f(rpipe2, wpipe1) - os._exit(0) + f(rpipe2, wpipe1) + os._exit(0) diff --git a/nova/utils.py b/nova/utils.py index 180210dc8460..6360f9133064 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -715,14 +715,21 @@ def synchronized(name, external=False): LOG.debug(_('Attempting to grab file lock "%(lock)s" for ' 'method "%(method)s"...'), {'lock': name, 'method': f.__name__}) - lock_file_path = os.path.join(FLAGS.lock_path, - 'nova-%s' % name) + lock_path = FLAGS.lock_path or tempfile.mkdtemp() + lock_file_path = os.path.join(lock_path, 'nova-%s' % name) lock = InterProcessLock(lock_file_path) - with lock: - LOG.debug(_('Got file lock "%(lock)s" for ' - 'method "%(method)s"...'), - {'lock': name, 'method': f.__name__}) - retval = f(*args, **kwargs) + try: + with lock: + LOG.debug(_('Got file lock "%(lock)s" for ' + 'method "%(method)s"...'), + {'lock': name, 'method': f.__name__}) + retval = f(*args, **kwargs) + finally: + # NOTE(vish): This removes the tempdir if we needed + # to create one. This is used to cleanup + # the locks left behind by unit tests. + if not FLAGS.lock_path: + shutil.rmtree(lock_path) else: retval = f(*args, **kwargs) diff --git a/run_tests.sh b/run_tests.sh index 2784bb3742ec..0fd7f4169dfd 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -95,8 +95,6 @@ function run_tests { cat run_tests.log fi fi - # cleanup locks - not really needed, but stops pollution of the source tree - rm -f nova-ensure_bridge nova-ensure_vlan nova-iptables nova-testlock1 nova-testlock2 return $RESULT }