Merge pull request #887 from jberry-suse/lock-needed

osc-staging: skip lock for observation commands (and other unlock fix).

osc-staging.py

@@ -76,6 +76,12 @@ def _full_project_name(self, project):
     warnings.warn('%s project not recognized.' % project)
     return project
 
+def lock_needed(cmd, opts):
+    return not(
+        cmd in ('acheck', 'check', 'frozenage', 'rebuild', 'unlock') or
+        (cmd == 'list' and not opts.supersede)
+    )
+
 
 @cmdln.option('--move', action='store_true',
               help='force the selection to become a move')
@@ -352,12 +358,8 @@ def do_staging(self, subcmd, opts, *args):
     if opts.wipe_cache:
         Cache.delete_all()
 
-    lock = OBSLock(opts.apiurl, opts.project, reason=cmd)
+    needed = lock_needed(cmd, opts)
-    if cmd == 'unlock':
+    with OBSLock(opts.apiurl, opts.project, reason=cmd, needed=needed) as lock:
-        lock.release(force=True)
-        return
-
-    with lock:
         api = StagingAPI(opts.apiurl, opts.project)
         config.apply_remote(api)
 
@@ -578,3 +580,5 @@ def do_staging(self, subcmd, opts, *args):
             PrioCommand(api).perform(args[1:])
         elif cmd == 'supersede':
             SupersedeCommand(api).perform(args[1:])
+        elif cmd == 'unlock':
+            lock.release(force=True)

osclib/obslock.py

@@ -28,7 +28,7 @@ from osc.core import http_POST
 class OBSLock(object):
     """Implement a distributed lock using a shared OBS resource."""
 
-    def __init__(self, apiurl, project, ttl=3600, reason=None):
+    def __init__(self, apiurl, project, ttl=3600, reason=None, needed=True):
         self.apiurl = apiurl
         self.project = project
         self.lock = conf.config[project]['lock']
@@ -39,6 +39,7 @@ class OBSLock(object):
         self.reason = reason
         self.reason_sub = None
         self.locked = False
+        self.needed = needed
 
     def _signature(self):
         """Create a signature with a timestamp."""
@@ -83,6 +84,8 @@ class OBSLock(object):
         http_POST(url, data=data)
 
     def acquire(self):
+        if not self.needed: return self
+
         # If the project doesn't have locks configured, raise a
         # Warning (but continue the operation)
         if not self.lock:
@@ -121,20 +124,30 @@ class OBSLock(object):
         return self
 
     def release(self, force=False):
+        if not force and not self.needed: return
+
         # If the project do not have locks configured, simply ignore
         # the operation.
         if not self.lock:
             return
 
         user, reason, reason_sub, _ = self._parse(self._read())
+        clear = False
         if user == self.user:
             if reason_sub:
                 self.reason = reason_sub
                 self.reason_sub = None
                 self._write(self._signature())
             elif not reason.startswith('hold') or force:
-                self._write('')
+                # Only clear a command lock as hold can only be cleared by force.
-                self.locked = False
+                clear = True
+        elif user is not None and force:
+            # Clear if a lock is present and force.
+            clear = True
+
+        if clear:
+            self._write('')
+            self.locked = False
 
     def hold(self, message=None):
         self.reason = 'hold'

tests/obslock_tests.py

@@ -122,3 +122,23 @@ class TestOBSLock(unittest.TestCase):
         with lock:
             _, reason, _, _ = lock._parse(lock._read())
             self.assertEqual(reason, 'some reason at hashnight')
+
+    def test_needed(self):
+        lock1 = self.obs_lock()
+        lock2 = self.obs_lock('unlock')
+        lock2.user = 'user2'
+        lock2.needed = False
+
+        self.assertFalse(lock1.locked)
+        self.assertFalse(lock2.locked)
+
+        with lock1:
+            self.assertTrue(lock1.locked)
+            with lock2:
+                self.assertFalse(lock2.locked)
+                user, _, _, _ = lock2._parse(lock2._read())
+                self.assertEqual(user, lock1.user, 'lock1 remains')
+
+                lock2.release(force=True)
+                user, _, _, _ = lock2._parse(lock2._read())
+                self.assertEqual(user, None, 'unlocked')