Avoid race condition using parallel fuel-devops env manipulations

If multiple processes are creating/erasing different fuel-devops
environments at the same time, there can be race condition during
libvirt object creation/deletion like:
'bridge virbr3 already exists' and others.

This patch allows to use a lock file to avoid such situation:
export DEVOPS_LOCK_FILE=/run/lock/devops_lock

If the environment variable DEVOPS_LOCK_FILE is not set, then
the lock file is not used (backward compatibility to avoid any
errors caused by using the lock file by default).

Change-Id: Id28f442959594aa5d6bb5c1d15e4f0880653070d

.gitignore

@@ -36,6 +36,7 @@ pip-delete-this-directory.txt
 htmlcov/
 .tox/
 .coverage
+cover/
 .cache
 nosetests.xml
 coverage.xml

devops/helpers/decorators.py

@@ -22,10 +22,12 @@ import sys
 import threading
 import time
 
+import fasteners
 import six
 
 from devops import error
 from devops import logger
+from devops import settings
 
 
 def threaded(name=None, started=False, daemon=False):
@@ -317,3 +319,41 @@ def logwrap(log=logger, log_level=logging.DEBUG, exc_level=logging.ERROR):
         return real_decorator(func)
 
     return real_decorator
+
+
+def proc_lock(path=settings.DEVOPS_LOCK_FILE, timeout=300):
+    """Process lock based on fcntl.lockf
+
+    Avoid race condition between different processes which
+    use fuel-devops at the same time during the resources
+    creation/modification/erase.
+
+    :param path: str, path to the lock file
+    :param timeout: int, timeout in second for waiting the lock file
+    """
+    def real_decorator(func):
+        @functools.wraps(func)
+        def wrapped(*args, **kwargs):
+            acquired = False
+            if path is not None:
+                logger.debug('Acquiring lock file {0} for {1}'
+                             .format(path, func.__name__))
+                lock = fasteners.InterProcessLock(path)
+                acquired = lock.acquire(blocking=True,
+                                        delay=5, timeout=timeout)
+                logger.debug('Acquired the lock file {0} for {1}'
+                             .format(path, func.__name__))
+                if not acquired:
+                    raise error.DevopsException(
+                        'Failed to aquire lock file in {0} sec'
+                        .format(timeout))
+            try:
+                result = func(*args, **kwargs)
+            finally:
+                if acquired:
+                    logger.debug('Releasing the lock file {0} for {1}'
+                                 .format(path, func.__name__))
+                    lock.release()
+            return result
+        return wrapped
+    return real_decorator

devops/models/environment.py

@@ -22,6 +22,7 @@ import netaddr
 import paramiko
 
 from devops import error
+from devops.helpers import decorators
 from devops.helpers import network as network_helpers
 from devops.helpers import ssh_client
 from devops import logger
@@ -183,6 +184,7 @@ class Environment(base.BaseModel):
         else:
             return False
 
+    @decorators.proc_lock()
     def define(self):
         for grp in self.get_groups():
             grp.define_networks()
@@ -201,6 +203,7 @@ class Environment(base.BaseModel):
         for grp in self.get_groups():
             grp.destroy()
 
+    @decorators.proc_lock()
     def erase(self):
         for grp in self.get_groups():
             grp.erase()
@@ -214,6 +217,7 @@ class Environment(base.BaseModel):
         for nod in self.get_nodes():
             nod.resume()
 
+    @decorators.proc_lock()
     def snapshot(self, name=None, description=None, force=False, suspend=True):
         """Snapshot the environment
 
@@ -237,6 +241,7 @@ class Environment(base.BaseModel):
             nod.snapshot(name=name, description=description, force=force,
                          external=settings.SNAPSHOTS_EXTERNAL)
 
+    @decorators.proc_lock()
     def revert(self, name=None, flag=True, resume=True):
         """Revert the environment from snapshot
 
@@ -312,6 +317,7 @@ class Environment(base.BaseModel):
             return dclient.create_env()
 
     @classmethod
+    @decorators.proc_lock()
     def create_environment(cls, full_config):
         """Create a new environment using full_config object
 

devops/settings.py

@@ -60,6 +60,8 @@ DATABASES = {
     }
 }
 
+DEVOPS_LOCK_FILE = os.environ.get('DEVOPS_LOCK_FILE', None)
+
 KEYSTONE_CREDS = {'username': os.environ.get('KEYSTONE_USERNAME', 'admin'),
                   'password': os.environ.get('KEYSTONE_PASSWORD', 'admin'),
                   'tenant_name': os.environ.get('KEYSTONE_TENANT', 'admin')}

devops/tests/helpers/test_decorators.py

@@ -503,3 +503,75 @@ class TestLogWrap(unittest.TestCase):
                 exc_info=True
             ),
         ))
+
+
+class TestProcLock(unittest.TestCase):
+
+    def patch(self, *args, **kwargs):
+        patcher = mock.patch(*args, **kwargs)
+        m = patcher.start()
+        self.addCleanup(patcher.stop)
+        return m
+
+    def setUp(self):
+        self.sleep_mock = self.patch(
+            'time.sleep')
+
+    def create_class_with_proc_lock(self, path, timeout):
+        class MyClass(object):
+            def __init__(self, method):
+                self.m = method
+
+            @decorators.proc_lock(path=path, timeout=timeout)
+            def method(self):
+                return self.m()
+
+        return MyClass
+
+    @mock.patch('fasteners.InterProcessLock.acquire')
+    @mock.patch('fasteners.InterProcessLock.release')
+    def test_default_no_proc_lock(self, release, acquire):
+        method_mock = mock.Mock()
+
+        # noinspection PyPep8Naming
+        MyClass = self.create_class_with_proc_lock(None, 10)
+        c = MyClass(method_mock)
+
+        c.method()
+
+        acquire.assert_not_called()
+        method_mock.assert_called_once()
+        release.assert_not_called()
+
+    @mock.patch('fasteners.InterProcessLock.acquire')
+    @mock.patch('fasteners.InterProcessLock.release')
+    def test_passed_proc_lock(self, release, acquire):
+        acquire.return_value = True
+        method_mock = mock.Mock()
+
+        # noinspection PyPep8Naming
+        MyClass = self.create_class_with_proc_lock('/run/lock/devops_lock', 20)
+        c = MyClass(method_mock)
+
+        c.method()
+
+        acquire.assert_called_once()
+        method_mock.assert_called_once()
+        release.assert_called_once()
+
+    @mock.patch('fasteners.InterProcessLock.acquire')
+    @mock.patch('fasteners.InterProcessLock.release')
+    def test_acquire_timeout(self, release, acquire):
+        acquire.return_value = False
+        method_mock = mock.Mock()
+
+        # noinspection PyPep8Naming
+        MyClass = self.create_class_with_proc_lock('/run/lock/devops_lock', 30)
+        c = MyClass(method_mock)
+
+        with self.assertRaises(error.DevopsException):
+            c.method()
+
+        acquire.assert_called_once()
+        method_mock.assert_not_called()
+        release.assert_not_called()

setup.py

@@ -51,7 +51,8 @@ setuptools.setup(
         'six>=1.9.0',
         'python-dateutil>=2.4.2',
         'lxml',
-        'enum34' if sys.version_info.major == 2 else ''
+        'enum34' if sys.version_info.major == 2 else '',
+        'fasteners>=0.7.0'
     ],
     tests_require=[
         'pytest>=2.7.1',