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

@@ -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)

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)
+            @utils.synchronized('testlock1', external=True)
-        def outer_lock():
+            def outer_lock():
 
-            @utils.synchronized('testlock2', external=True)
+                @utils.synchronized('testlock2', external=True)
-            def inner_lock():
+                def inner_lock():
-                return sentinel
+                    return sentinel
-            return inner_lock()
+                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()
+        with utils.tempdir() as tempdir:
-        rpipe2, wpipe2 = os.pipe()
+            self.flags(lock_path=tempdir)
+            rpipe1, wpipe1 = os.pipe()
+            rpipe2, wpipe2 = os.pipe()
 
-        @utils.synchronized('testlock1', external=True)
+            @utils.synchronized('testlock1', external=True)
-        def f(rpipe, wpipe):
+            def f(rpipe, wpipe):
-            try:
+                try:
-                os.write(wpipe, "foo")
+                    os.write(wpipe, "foo")
-            except OSError, e:
+                except OSError, e:
-                self.assertEquals(e.errno, errno.EPIPE)
+                    self.assertEquals(e.errno, errno.EPIPE)
-                return
+                    return
 
-            rfds, _wfds, _efds = select.select([rpipe], [], [], 1)
+                rfds, _wfds, _efds = select.select([rpipe], [], [], 1)
-            self.assertEquals(len(rfds), 0, "The other process, which was"
+                self.assertEquals(len(rfds), 0, "The other process, which was"
-                                            " supposed to be locked, "
+                                                " supposed to be locked, "
-                                            "wrote on its end of the "
+                                                "wrote on its end of the "
-                                            "pipe")
+                                                "pipe")
-            os.close(rpipe)
+                os.close(rpipe)
 
-        pid = os.fork()
+            pid = os.fork()
-        if pid > 0:
+            if pid > 0:
-            os.close(wpipe1)
+                os.close(wpipe1)
-            os.close(rpipe2)
+                os.close(rpipe2)
 
-            f(rpipe1, wpipe2)
+                f(rpipe1, wpipe2)
-        else:
+            else:
-            os.close(rpipe1)
+                os.close(rpipe1)
-            os.close(wpipe2)
+                os.close(wpipe2)
 
-            f(rpipe2, wpipe1)
+                f(rpipe2, wpipe1)
-            os._exit(0)
+                os._exit(0)

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,
+                    lock_path = FLAGS.lock_path or tempfile.mkdtemp()
-                                                  'nova-%s' % name)
+                    lock_file_path = os.path.join(lock_path, 'nova-%s' % name)
                     lock = InterProcessLock(lock_file_path)
-                    with lock:
+                    try:
-                        LOG.debug(_('Got file lock "%(lock)s" for '
+                        with lock:
-                                    'method "%(method)s"...'),
+                            LOG.debug(_('Got file lock "%(lock)s" for '
-                                  {'lock': name, 'method': f.__name__})
+                                        'method "%(method)s"...'),
-                        retval = f(*args, **kwargs)
+                                      {'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)
 

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
 }