distgit, backend, common: unify file locking code in copr-common

Fix #3367

As a consequence, stop using `python3-oslo-concurrency` on Copr DistGit and
start using `python3-filelock` as the rest of the stack.

backend/copr-backend.spec

@@ -6,7 +6,7 @@
 %global tests_version 5
 %global tests_tar test-data-copr-backend
 
-%global copr_common_version 0.24.1~~dev0
+%global copr_common_version 0.25.1~~dev0
 
 Name:       copr-backend
 Version:    1.177
@@ -41,7 +41,6 @@ BuildRequires: python3-copr-common >= %copr_common_version
 BuildRequires: python3-daemon
 BuildRequires: python3-dateutil
 BuildRequires: python3-distro
-BuildRequires: python3-filelock
 BuildRequires: python3-gobject
 BuildRequires: python3-httpretty
 BuildRequires: python3-humanize
@@ -81,7 +80,6 @@ Recommends: python3-copr-messaging
 Requires:   python3-daemon
 Requires:   python3-dateutil
 Recommends: python3-fedmsg
-Requires:   python3-filelock
 Requires:   python3-gobject
 Requires:   python3-humanize
 Requires:   python3-jinja2

backend/run/copr-repo

@@ -10,7 +10,6 @@ mess up everything around.
 """
 
 import argparse
-import contextlib
 import datetime
 import logging
 import os
@@ -19,8 +18,7 @@ import shutil
 import subprocess
 import sys
 
-import filelock
-
+from copr_common.lock import lock, LockTimeout
 from copr_backend.constants import CHROOTS_USING_SQLITE_REPODATA
 from copr_backend.createrepo import BatchedCreaterepo
 from copr_backend.helpers import (
@@ -313,24 +311,6 @@ def assert_new_createrepo():
     assert b'--recycle-pkglist' in out
 
 
-class LockTimeout(OSError):
-    """ Raised for lock() timeout, if timeout= option is set to value >= 0 """
-
-@contextlib.contextmanager
-def lock(opts, timeout=-1):
-    lock_path = os.environ.get('COPR_TESTSUITE_LOCKPATH', "/var/lock/copr-backend")
-    lock_basename = opts.directory.replace("/", "_@_") + '.lock'
-    lock_filename = os.path.join(lock_path, lock_basename)
-    opts.log.debug("acquiring lock")
-    try:
-        with filelock.FileLock(lock_filename, timeout=timeout):
-            opts.log.debug("acquired lock")
-            yield
-    except filelock.Timeout as err:
-        opts.log.debug("lock timeouted")
-        raise LockTimeout("Timeouted on lock file: {}".format(lock_path)) from err
-
-
 def main_locked(opts, batch, log):
     """
     Main method, executed under lock.
@@ -411,7 +391,9 @@ def main_try_lock(opts, batch):
             return
 
         try:
-            with lock(opts, timeout=5):
+            lockdir = os.environ.get(
+                "COPR_TESTSUITE_LOCKPATH", "/var/lock/copr-backend")
+            with lock(opts.directory, lockdir=lockdir, timeout=5, log=opts.log):
                 main_locked(opts, batch, opts.log)
                 # skip commit if main_locked() raises exception
                 batch.commit()

backend/tests/test_modifyrepo.py

@@ -47,7 +47,8 @@ def _lock(directory="non-existent"):
     opts.log = logging.getLogger()
     opts.directory = directory
     lock = filedict['lock']
-    with lock(opts):
+    lockdir = os.environ.get("COPR_TESTSUITE_LOCKPATH")
+    with lock(opts.directory, lockdir=lockdir, timeout=-1, log=opts.log):
         yield opts
 
 class TestModifyRepo(object):

common/copr_common/lock.py

@@ -0,0 +1,43 @@
+"""
+File locking for multithreading
+"""
+
+import os
+import contextlib
+import filelock
+from setproctitle import getproctitle, setproctitle
+
+
+@contextlib.contextmanager
+def lock(path, lockdir, timeout, log):
+    """
+    Create a lock file that can be accessed only by one thread at the time.
+    A practical use-case for this is to lock a repository so multiple versions
+    of the same package cannot be imported in paralel.
+
+    We want to pass a simple `path` parameter specifying what file or directory
+    should be locked. This however isn't where the lockfile is going to be
+    created. It will be created in the `lockdir`.
+    """
+    filename = path.replace("/", "_@_") + ".lock"
+    lockfile = os.path.join(lockdir, filename)
+
+    title = getproctitle()
+    setproctitle("{0} [locking]".format(title))
+    log.debug("acquiring lock")
+    try:
+        with filelock.FileLock(lockfile, timeout=timeout):
+            setproctitle("{0} [locked]".format(title))
+            log.debug("acquired lock")
+            yield
+    except filelock.Timeout as err:
+        log.debug("lock timeouted")
+        raise LockTimeout("Timeouted on lock for: {}".format(path)) from err
+    finally:
+        setproctitle("{0} [locking]".format(title))
+
+
+class LockTimeout(OSError):
+    """
+    Raised for lock() timeout, if timeout= option is set to value >= 0
+    """

common/python-copr-common.spec

@@ -1,7 +1,7 @@
 %global srcname copr-common
 
 Name:       python-copr-common
-Version:    0.25
+Version:    0.25.1.dev0
 Release:    1%{?dist}
 Summary:    Python code used by Copr
 
@@ -19,6 +19,8 @@ BuildRequires: python3-devel
 BuildRequires: python3-setuptools
 BuildRequires: python3-pytest
 BuildRequires: python3-requests
+BuildRequires: python3-filelock
+BuildRequires: python3-setproctitle
 
 %global _description\
 COPR is lightweight build system. It allows you to create new project in WebUI,\

common/setup.py

@@ -18,9 +18,15 @@
 __url__ = "https://github.com/fedora-copr/copr"
 
 
+requires = [
+    "filelock",
+    "setproctitle",
+]
+
+
 setup(
     name='copr-common',
-    version="0.25",
+    version="0.25.1.dev0",
     description=__description__,
     long_description=long_description,
     author=__author__,
@@ -30,6 +36,7 @@
     classifiers=[
         "License :: OSI Approved :: GNU General Public License v2 or later (GPLv2+)",
     ],
+    install_requires=requires,
     packages=['copr_common'],
     include_package_data=True,
     zip_safe=False

dist-git/copr-dist-git.spec

@@ -1,4 +1,4 @@
-%global copr_common_version 0.20.1.dev1
+%global copr_common_version 0.25.1~~dev0
 
 Name:       copr-dist-git
 Version:    0.70
@@ -22,7 +22,6 @@ BuildRequires: python3-requests
 BuildRequires: python3-rpkg >= 1.67-1
 BuildRequires: python3-pytest
 BuildRequires: python3-copr-common >= %copr_common_version
-BuildRequires: python3-oslo-concurrency
 BuildRequires: python3-redis
 BuildRequires: python3-setproctitle
 
@@ -37,7 +36,6 @@ Requires: python3-copr-common >= %copr_common_version
 Requires: python3-requests
 Requires: python3-rpkg >= 1.66-6
 Requires: python3-munch
-Requires: python3-oslo-concurrency
 Requires: python3-setproctitle
 Requires: python3-daemon
 Requires: python3-redis

dist-git/copr_dist_git/helpers.py

@@ -2,37 +2,18 @@
 import sys
 import logging
 import subprocess
-import munch
+from configparser import ConfigParser
 
-from oslo_concurrency import lockutils
-from setproctitle import getproctitle, setproctitle
-from copr_common.request import SafeRequest, RequestError
+import munch
 from munch import Munch
+from copr_common.request import SafeRequest, RequestError
 from .exceptions import FileDownloadException, RunCommandException
 
-from contextlib import contextmanager
-from configparser import ConfigParser
 
 log = logging.getLogger(__name__)
 LOCK_PATH = "/var/lock/copr-dist-git"
 
 
-@contextmanager
-def lock(name):
-    """
-    Create a lock file that can be accessed only by one thread at the time.
-    A practical use-case for this is to lock a repository so multiple versions
-    of the same package cannot be imported in paralel.
-    """
-    title = getproctitle()
-    setproctitle("{0} [locking]".format(title))
-    with lockutils.lock(name=name, external=True, lock_path=LOCK_PATH,
-                        fair=True, delay=0):
-        setproctitle("{0} [locked]".format(title))
-        yield
-    setproctitle(title)
-
-
 def _get_conf(cp, section, option, default, mode=None):
     """
     To make returning items from config parser less irritating

dist-git/copr_dist_git/importer.py

@@ -8,6 +8,7 @@
 from requests import get, post
 
 from copr_common.worker_manager import WorkerManager
+from copr_common.lock import lock
 
 from .package_import import import_package
 from .process_pool import Worker, Pool, SingleThreadWorker
@@ -82,8 +83,7 @@ def do_import(self, task):
             )
 
             repo = os.path.join(self.opts.lookaside_location, task.reponame)
-            lockfile = os.path.join(repo, "import.lock")
-            with helpers.lock(lockfile):
+            with lock(repo, lockdir=helpers.LOCK_PATH, timeout=-1, log=log):
                 result.update(import_package(
                     self.opts,
                     task.repo_namespace,

dist-git/tests/test_importer.py

@@ -7,6 +7,7 @@
 import copy
 import pytest
 
+from tempfile import TemporaryDirectory
 from munch import Munch
 from base import Base
 
@@ -104,6 +105,9 @@ def test_post_back_safe(self, mc_post):
         assert mc_post.called
 
     def test_do_import(self, mc_import_package, mc_helpers):
+        tmp = TemporaryDirectory(prefix="copr-dist-git-test-")
+        mc_helpers.LOCK_PATH = tmp.name
+
         mc_helpers.download_file = MagicMock(return_value='somepath.src.rpm')
         mc_import_package.return_value = Munch(
             pkg_name='foo',