Update lockutils and fixture in openstack.common

lockutils: included commits:
  8b2b0b7 Use hacking import_exceptions for gettextutils._
  6d0a6c3 Correct invalid docstrings
  12bcdb7 Remove vim header
  79e6bc6 fix lockutils.lock() to make it thread-safe
  ace5120 Add main() to lockutils that creates temp dir for locks
  537d8e2 Allow lockutils to get lock_path conf from envvar
  371fa42 Move LockFixture into a fixtures module
  d498c42 Fix to properly log when we release a semaphore
  29d387c Add LockFixture to lockutils
  3e3ac0c Modify lockutils.py due to dispose of eventlet
  90b6a65 Fix locking bug
  27d4b41 Move synchronized body to a first-class function
  15c17fb Make lock_file_prefix optional
  1a2df89 Enable H302 hacking check

fixture: created, included commits:
  45658e2 Fix violations of H302:import only modules
  12bcdb7 Remove vim header
  3970d46 Fix typos in oslo
  371fa42 Move LockFixture into a fixtures module
  f4a4855 Consolidate the use of stubs
  6111131 Make openstack.common.fixture.config Py3 compliant
  3906979 Add a fixture for dealing with config
  d332cca Add a fixture for dealing with mock patching.
  1bc3ecf Start adding reusable test fixtures.

Also tox.ini was corrected to let lockutils work in tests.

This change is needed for work on bp: db-sync-models-with-migrations

Closes-Bug: #1065531

Change-Id: I139f30b4767ff2c9d1f01ee728823859c09b3859

neutron/openstack/common/fixture/config.py

@@ -0,0 +1,45 @@
+#
+# Copyright 2013 Mirantis, Inc.
+# Copyright 2013 OpenStack Foundation
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+import fixtures
+from oslo.config import cfg
+import six
+
+
+class Config(fixtures.Fixture):
+    """Override some configuration values.
+
+    The keyword arguments are the names of configuration options to
+    override and their values.
+
+    If a group argument is supplied, the overrides are applied to
+    the specified configuration option group.
+
+    All overrides are automatically cleared at the end of the current
+    test by the reset() method, which is registered by addCleanup().
+    """
+
+    def __init__(self, conf=cfg.CONF):
+        self.conf = conf
+
+    def setUp(self):
+        super(Config, self).setUp()
+        self.addCleanup(self.conf.reset)
+
+    def config(self, **kw):
+        group = kw.pop('group', None)
+        for k, v in six.iteritems(kw):
+            self.conf.set_override(k, v, group)

neutron/openstack/common/fixture/lockutils.py

@@ -0,0 +1,51 @@
+# Copyright 2011 OpenStack Foundation.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import fixtures
+
+from neutron.openstack.common import lockutils
+
+
+class LockFixture(fixtures.Fixture):
+    """External locking fixture.
+
+    This fixture is basically an alternative to the synchronized decorator with
+    the external flag so that tearDowns and addCleanups will be included in
+    the lock context for locking between tests. The fixture is recommended to
+    be the first line in a test method, like so::
+
+        def test_method(self):
+            self.useFixture(LockFixture)
+                ...
+
+    or the first line in setUp if all the test methods in the class are
+    required to be serialized. Something like::
+
+        class TestCase(testtools.testcase):
+            def setUp(self):
+                self.useFixture(LockFixture)
+                super(TestCase, self).setUp()
+                    ...
+
+    This is because addCleanups are put on a LIFO queue that gets run after the
+    test method exits. (either by completing or raising an exception)
+    """
+    def __init__(self, name, lock_file_prefix=None):
+        self.mgr = lockutils.lock(name, lock_file_prefix, True)
+
+    def setUp(self):
+        super(LockFixture, self).setUp()
+        self.addCleanup(self.mgr.__exit__, None, None, None)
+        self.mgr.__enter__()

neutron/openstack/common/fixture/mockpatch.py

@@ -0,0 +1,49 @@
+# Copyright 2010 United States Government as represented by the
+# Administrator of the National Aeronautics and Space Administration.
+# Copyright 2013 Hewlett-Packard Development Company, L.P.
+# All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import fixtures
+import mock
+
+
+class PatchObject(fixtures.Fixture):
+    """Deal with code around mock."""
+
+    def __init__(self, obj, attr, **kwargs):
+        self.obj = obj
+        self.attr = attr
+        self.kwargs = kwargs
+
+    def setUp(self):
+        super(PatchObject, self).setUp()
+        _p = mock.patch.object(self.obj, self.attr, **self.kwargs)
+        self.mock = _p.start()
+        self.addCleanup(_p.stop)
+
+
+class Patch(fixtures.Fixture):
+
+    """Deal with code around mock.patch."""
+
+    def __init__(self, obj, **kwargs):
+        self.obj = obj
+        self.kwargs = kwargs
+
+    def setUp(self):
+        super(Patch, self).setUp()
+        _p = mock.patch(self.obj, **self.kwargs)
+        self.mock = _p.start()
+        self.addCleanup(_p.stop)

neutron/openstack/common/fixture/moxstubout.py

@@ -0,0 +1,32 @@
+# Copyright 2010 United States Government as represented by the
+# Administrator of the National Aeronautics and Space Administration.
+# Copyright 2013 Hewlett-Packard Development Company, L.P.
+# All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import fixtures
+import mox
+
+
+class MoxStubout(fixtures.Fixture):
+    """Deal with code around mox and stubout as a fixture."""
+
+    def setUp(self):
+        super(MoxStubout, self).setUp()
+        # emulate some of the mox stuff, we can't use the metaclass
+        # because it screws with our generators
+        self.mox = mox.Mox()
+        self.stubs = self.mox.stubs
+        self.addCleanup(self.mox.UnsetStubs)
+        self.addCleanup(self.mox.VerifyAll)

neutron/openstack/common/lockutils.py

@@ -1,5 +1,3 @@
-# vim: tabstop=4 shiftwidth=4 softtabstop=4
-
 # Copyright 2011 OpenStack Foundation.
 # All Rights Reserved.
 #
@@ -16,15 +14,18 @@
 #    under the License.
 
 
+import contextlib
 import errno
 import functools
 import os
 import shutil
+import subprocess
+import sys
 import tempfile
+import threading
 import time
 import weakref
 
-from eventlet import semaphore
 from oslo.config import cfg
 
 from neutron.openstack.common import fileutils
@@ -40,8 +41,8 @@ util_opts = [
     cfg.BoolOpt('disable_process_locking', default=False,
                 help='Whether to disable inter-process locks'),
     cfg.StrOpt('lock_path',
-               help=('Directory to use for lock files. Default to a '
+               default=os.environ.get("NEUTRON_LOCK_PATH"),
-                     'temp directory'))
+               help=('Directory to use for lock files.'))
 ]
 
 
@@ -133,9 +134,88 @@ else:
     InterProcessLock = _PosixLock
 
 _semaphores = weakref.WeakValueDictionary()
+_semaphores_lock = threading.Lock()
 
 
-def synchronized(name, lock_file_prefix, external=False, lock_path=None):
+@contextlib.contextmanager
+def lock(name, lock_file_prefix=None, external=False, lock_path=None):
+    """Context based lock
+
+    This function yields a `threading.Semaphore` instance (if we don't use
+    eventlet.monkey_patch(), else `semaphore.Semaphore`) unless external is
+    True, in which case, it'll yield an InterProcessLock instance.
+
+    :param lock_file_prefix: The lock_file_prefix argument is used to provide
+      lock files on disk with a meaningful prefix.
+
+    :param external: The external keyword argument denotes whether this lock
+      should work across multiple processes. This means that if two different
+      workers both run a a method decorated with @synchronized('mylock',
+      external=True), only one of them will execute at a time.
+
+    :param lock_path: The lock_path keyword argument is used to specify a
+      special location for external lock files to live. If nothing is set, then
+      CONF.lock_path is used as a default.
+    """
+    with _semaphores_lock:
+        try:
+            sem = _semaphores[name]
+        except KeyError:
+            sem = threading.Semaphore()
+            _semaphores[name] = sem
+
+    with sem:
+        LOG.debug(_('Got semaphore "%(lock)s"'), {'lock': name})
+
+        # NOTE(mikal): I know this looks odd
+        if not hasattr(local.strong_store, 'locks_held'):
+            local.strong_store.locks_held = []
+        local.strong_store.locks_held.append(name)
+
+        try:
+            if external and not CONF.disable_process_locking:
+                LOG.debug(_('Attempting to grab file lock "%(lock)s"'),
+                          {'lock': name})
+
+                # We need a copy of lock_path because it is non-local
+                local_lock_path = lock_path or CONF.lock_path
+                if not local_lock_path:
+                    raise cfg.RequiredOptError('lock_path')
+
+                if not os.path.exists(local_lock_path):
+                    fileutils.ensure_tree(local_lock_path)
+                    LOG.info(_('Created lock path: %s'), local_lock_path)
+
+                def add_prefix(name, prefix):
+                    if not prefix:
+                        return name
+                    sep = '' if prefix.endswith('-') else '-'
+                    return '%s%s%s' % (prefix, sep, name)
+
+                # NOTE(mikal): the lock name cannot contain directory
+                # separators
+                lock_file_name = add_prefix(name.replace(os.sep, '_'),
+                                            lock_file_prefix)
+
+                lock_file_path = os.path.join(local_lock_path, lock_file_name)
+
+                try:
+                    lock = InterProcessLock(lock_file_path)
+                    with lock as lock:
+                        LOG.debug(_('Got file lock "%(lock)s" at %(path)s'),
+                                  {'lock': name, 'path': lock_file_path})
+                        yield lock
+                finally:
+                    LOG.debug(_('Released file lock "%(lock)s" at %(path)s'),
+                              {'lock': name, 'path': lock_file_path})
+            else:
+                yield sem
+
+        finally:
+            local.strong_store.locks_held.remove(name)
+
+
+def synchronized(name, lock_file_prefix=None, external=False, lock_path=None):
     """Synchronization decorator.
 
     Decorating a method like so::
@@ -157,99 +237,19 @@ def synchronized(name, lock_file_prefix, external=False, lock_path=None):
            ...
 
     This way only one of either foo or bar can be executing at a time.
-
-    :param lock_file_prefix: The lock_file_prefix argument is used to provide
-    lock files on disk with a meaningful prefix. The prefix should end with a
-    hyphen ('-') if specified.
-
-    :param external: The external keyword argument denotes whether this lock
-    should work across multiple processes. This means that if two different
-    workers both run a a method decorated with @synchronized('mylock',
-    external=True), only one of them will execute at a time.
-
-    :param lock_path: The lock_path keyword argument is used to specify a
-    special location for external lock files to live. If nothing is set, then
-    CONF.lock_path is used as a default.
     """
 
     def wrap(f):
         @functools.wraps(f)
         def inner(*args, **kwargs):
-            # NOTE(soren): If we ever go natively threaded, this will be racy.
-            #              See http://stackoverflow.com/questions/5390569/dyn
-            #              amically-allocating-and-destroying-mutexes
-            sem = _semaphores.get(name, semaphore.Semaphore())
-            if name not in _semaphores:
-                # this check is not racy - we're already holding ref locally
-                # so GC won't remove the item and there was no IO switch
-                # (only valid in greenthreads)
-                _semaphores[name] = sem
-
-            with sem:
-                LOG.debug(_('Got semaphore "%(lock)s" for method '
-                            '"%(method)s"...'), {'lock': name,
-                                                 'method': f.__name__})
-
-                # NOTE(mikal): I know this looks odd
-                if not hasattr(local.strong_store, 'locks_held'):
-                    local.strong_store.locks_held = []
-                local.strong_store.locks_held.append(name)
-
             try:
-                    if external and not CONF.disable_process_locking:
+                with lock(name, lock_file_prefix, external, lock_path):
-                        LOG.debug(_('Attempting to grab file lock "%(lock)s" '
+                    LOG.debug(_('Got semaphore / lock "%(function)s"'),
-                                    'for method "%(method)s"...'),
+                              {'function': f.__name__})
-                                  {'lock': name, 'method': f.__name__})
+                    return f(*args, **kwargs)
-                        cleanup_dir = False
-
-                        # We need a copy of lock_path because it is non-local
-                        local_lock_path = lock_path
-                        if not local_lock_path:
-                            local_lock_path = CONF.lock_path
-
-                        if not local_lock_path:
-                            cleanup_dir = True
-                            local_lock_path = tempfile.mkdtemp()
-
-                        if not os.path.exists(local_lock_path):
-                            fileutils.ensure_tree(local_lock_path)
-
-                        # NOTE(mikal): the lock name cannot contain directory
-                        # separators
-                        safe_name = name.replace(os.sep, '_')
-                        lock_file_name = '%s%s' % (lock_file_prefix, safe_name)
-                        lock_file_path = os.path.join(local_lock_path,
-                                                      lock_file_name)
-
-                        try:
-                            lock = InterProcessLock(lock_file_path)
-                            with lock:
-                                LOG.debug(_('Got file lock "%(lock)s" at '
-                                            '%(path)s for method '
-                                            '"%(method)s"...'),
-                                          {'lock': name,
-                                           'path': lock_file_path,
-                                           'method': f.__name__})
-                                retval = f(*args, **kwargs)
             finally:
-                            LOG.debug(_('Released file lock "%(lock)s" at '
+                LOG.debug(_('Semaphore / lock released "%(function)s"'),
-                                        '%(path)s for method "%(method)s"...'),
+                          {'function': f.__name__})
-                                      {'lock': name,
-                                       'path': lock_file_path,
-                                       'method': f.__name__})
-                            # 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 cleanup_dir:
-                                shutil.rmtree(local_lock_path)
-                    else:
-                        retval = f(*args, **kwargs)
-
-                finally:
-                    local.strong_store.locks_held.remove(name)
-
-            return retval
         return inner
     return wrap
 
@@ -273,7 +273,31 @@ def synchronized_with_prefix(lock_file_prefix):
            ...
 
     The lock_file_prefix argument is used to provide lock files on disk with a
-    meaningful prefix. The prefix should end with a hyphen ('-') if specified.
+    meaningful prefix.
     """
 
     return functools.partial(synchronized, lock_file_prefix=lock_file_prefix)
+
+
+def main(argv):
+    """Create a dir for locks and pass it to command from arguments
+
+    If you run this:
+    python -m openstack.common.lockutils python setup.py testr <etc>
+
+    a temporary directory will be created for all your locks and passed to all
+    your tests in an environment variable. The temporary dir will be deleted
+    afterwards and the return value will be preserved.
+    """
+
+    lock_dir = tempfile.mkdtemp()
+    os.environ["NEUTRON_LOCK_PATH"] = lock_dir
+    try:
+        ret_val = subprocess.call(argv[1:])
+    finally:
+        shutil.rmtree(lock_dir, ignore_errors=True)
+    return ret_val
+
+
+if __name__ == '__main__':
+    sys.exit(main(sys.argv))

neutron/tests/unit/__init__.py

@@ -23,7 +23,4 @@ from oslo.config import cfg
 reldir = os.path.join(os.path.dirname(__file__), '..', '..', '..')
 absdir = os.path.abspath(reldir)
 cfg.CONF.state_path = absdir
-# An empty lock path forces lockutils.synchronized to use a temporary
-# location for lock files that will be cleaned up automatically.
-cfg.CONF.lock_path = ''
 cfg.CONF.use_stderr = False

openstack-common.conf

@@ -6,6 +6,7 @@ module=db.sqlalchemy
 module=eventlet_backdoor
 module=excutils
 module=fileutils
+module=fixture
 module=gettextutils
 module=importutils
 module=install_venv_common

tox.ini

@@ -7,7 +7,7 @@ deps = -r{toxinidir}/requirements.txt
        -r{toxinidir}/test-requirements.txt
        setuptools_git>=0.4
 commands =
-  python setup.py testr --slowest --testr-args='{posargs}'
+  python -m neutron.openstack.common.lockutils python setup.py testr --slowest --testr-args='{posargs}'
 
 [tox:jenkins]
 sitepackages = True
@@ -22,7 +22,7 @@ commands = python ./tools/check_i18n.py ./neutron ./tools/i18n_cfg.py
 
 [testenv:cover]
 commands =
-  python setup.py testr --coverage --testr-args='{posargs}'
+  python -m neutron.openstack.common.lockutils python setup.py testr --coverage --testr-args='{posargs}'
 
 [testenv:venv]
 commands = {posargs}