From 135d5443e375e6c3e1aa16e1b95d2237dd0619a3 Mon Sep 17 00:00:00 2001 From: Ann Kamyshnikova Date: Fri, 20 Sep 2013 15:48:37 +0400 Subject: [PATCH] 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/__init__.py | 0 neutron/openstack/common/fixture/config.py | 45 ++++ neutron/openstack/common/fixture/lockutils.py | 51 +++++ neutron/openstack/common/fixture/mockpatch.py | 49 ++++ .../openstack/common/fixture/moxstubout.py | 32 +++ neutron/openstack/common/lockutils.py | 214 ++++++++++-------- neutron/tests/unit/__init__.py | 3 - openstack-common.conf | 1 + tox.ini | 4 +- 9 files changed, 299 insertions(+), 100 deletions(-) create mode 100644 neutron/openstack/common/fixture/__init__.py create mode 100644 neutron/openstack/common/fixture/config.py create mode 100644 neutron/openstack/common/fixture/lockutils.py create mode 100644 neutron/openstack/common/fixture/mockpatch.py create mode 100644 neutron/openstack/common/fixture/moxstubout.py diff --git a/neutron/openstack/common/fixture/__init__.py b/neutron/openstack/common/fixture/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/neutron/openstack/common/fixture/config.py b/neutron/openstack/common/fixture/config.py new file mode 100644 index 0000000000..0bf90ff7a0 --- /dev/null +++ b/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) diff --git a/neutron/openstack/common/fixture/lockutils.py b/neutron/openstack/common/fixture/lockutils.py new file mode 100644 index 0000000000..90932c56e2 --- /dev/null +++ b/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__() diff --git a/neutron/openstack/common/fixture/mockpatch.py b/neutron/openstack/common/fixture/mockpatch.py new file mode 100644 index 0000000000..858e77cd06 --- /dev/null +++ b/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) diff --git a/neutron/openstack/common/fixture/moxstubout.py b/neutron/openstack/common/fixture/moxstubout.py new file mode 100644 index 0000000000..e8c031f08a --- /dev/null +++ b/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) diff --git a/neutron/openstack/common/lockutils.py b/neutron/openstack/common/lockutils.py index ea80e73c59..f85b8c1307 100644 --- a/neutron/openstack/common/lockutils.py +++ b/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 ' - 'temp directory')) + default=os.environ.get("NEUTRON_LOCK_PATH"), + 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: - LOG.debug(_('Attempting to grab file lock "%(lock)s" ' - 'for method "%(method)s"...'), - {'lock': name, 'method': f.__name__}) - 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 ' - '%(path)s for method "%(method)s"...'), - {'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 + try: + with lock(name, lock_file_prefix, external, lock_path): + LOG.debug(_('Got semaphore / lock "%(function)s"'), + {'function': f.__name__}) + return f(*args, **kwargs) + finally: + LOG.debug(_('Semaphore / lock released "%(function)s"'), + {'function': f.__name__}) 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 + + 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)) diff --git a/neutron/tests/unit/__init__.py b/neutron/tests/unit/__init__.py index a0e30b7480..96b119faf6 100644 --- a/neutron/tests/unit/__init__.py +++ b/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 diff --git a/openstack-common.conf b/openstack-common.conf index 57606c67ae..e548fd774f 100644 --- a/openstack-common.conf +++ b/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 diff --git a/tox.ini b/tox.ini index 1644ad960b..fbb07f9faa 100644 --- a/tox.ini +++ b/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}