Allow using lockfile per jenkins master

When a jjb run is thrown when another jjb is already running, it can
cause corruption of cache. Start using a fasteners to ensure this
won't be happening and run securely on automated systems.

Ensure unlock is called from only the destructor, so that it is only
called when the JJB process is guaranteed to be finished using it.

Make it obvious that _lock is intended to be internal to the cache
storage implementation.

Potentially we may not need to call it at all, as python might unlock
it for us on exit by closing the file when no longer needed. However
better to make it explicit.

Change-Id: I53a1f92cf2bfbbe87c9ea205c377f93869353620

3 files changed, 28 insertions, 2 deletions

diff --git a/jenkins_jobs/cache.py b/jenkins_jobs/cache.py
index ee293d46..dfcb9998 100644
--- a/jenkins_jobs/cache.py
+++ b/jenkins_jobs/cache.py
@@ -21,8 +21,11 @@ import logging
 import os
 import re
 import tempfile
+
+import fasteners
 import yaml
 
+from jenkins_jobs import errors
 
 logger = logging.getLogger(__name__)
 
@@ -43,6 +46,13 @@ class JobCache(object):
         host_vary = re.sub('[^A-Za-z0-9\-\~]', '_', jenkins_url)
         self.cachefilename = os.path.join(
             cache_dir, 'cache-host-jobs-' + host_vary + '.yml')
+
+        # generate named lockfile if none exists, and lock it
+        self._locked = self._lock()
+        if not self._locked:
+            raise errors.JenkinsJobsException(
+                "Unable to lock cache for '%s'" % jenkins_url)
+
         if flush or not os.path.isfile(self.cachefilename):
             self.data = {}
         else:
@@ -50,6 +60,18 @@ class JobCache(object):
                 self.data = yaml.load(yfile)
         logger.debug("Using cache: '{0}'".format(self.cachefilename))
 
+    def _lock(self):
+        self._fastener = fasteners.InterProcessLock("%s.lock" %
+                                                    self.cachefilename)
+
+        return self._fastener.acquire(delay=1, max_delay=2, timeout=60)
+
+    def _unlock(self):
+        if getattr(self, '_locked', False):
+            if getattr(self, '_fastener', None) is not None:
+                self._fastener.release()
+            self._locked = None
+
     @staticmethod
     def get_cache_dir():
         home = os.path.expanduser('~')
@@ -115,3 +137,4 @@ class JobCache(object):
             except Exception as e:
                 self._logger.error("Failed to write to cache file '%s' on "
                                    "exit: %s" % (self.cachefilename, e))
+        self._unlock()
diff --git a/requirements.txt b/requirements.txt
index 4f19904e..07e73a21 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -6,3 +6,4 @@ PyYAML>=3.10.0 # MIT
 pbr>=1.8 # Apache-2.0
 stevedore>=1.17.1 # Apache-2.0
 python-jenkins>=0.4.8
+fasteners
diff --git a/tests/cachestorage/test_cachestorage.py b/tests/cachestorage/test_cachestorage.py
index 8a9f533d..138f1406 100644
--- a/tests/cachestorage/test_cachestorage.py
+++ b/tests/cachestorage/test_cachestorage.py
@@ -31,7 +31,8 @@ class TestCaseJobCache(base.BaseTestCase):
 
         with mock.patch('jenkins_jobs.builder.JobCache.save') as save_mock:
             with mock.patch('os.path.isfile', return_value=False):
-                jenkins_jobs.builder.JobCache("dummy")
+                with mock.patch('jenkins_jobs.builder.JobCache._lock'):
+                    jenkins_jobs.builder.JobCache("dummy")
             save_mock.assert_called_with()
 
     @mock.patch('jenkins_jobs.builder.JobCache.get_cache_dir',
@@ -43,4 +44,5 @@ class TestCaseJobCache(base.BaseTestCase):
         test_file = os.path.abspath(__file__)
         with mock.patch('os.path.join', return_value=test_file):
             with mock.patch('yaml.load'):
-                jenkins_jobs.builder.JobCache("dummy").data = None
+                with mock.patch('jenkins_jobs.builder.JobCache._lock'):
+                    jenkins_jobs.builder.JobCache("dummy").data = None