#1765 backend: change prunerepo logic, use get_rpms_to_remove from prunerepo.helpers
Merged 4 years ago by frostyx. Opened 4 years ago by schlupov.
copr/ schlupov/copr change_copr_prune  into  main

file modified
+4 -2
@@ -2,7 +2,7 @@ 

  %global _pkgdocdir %{_docdir}/%{name}-%{version}

  %endif

  

- %global tests_version 1

+ %global tests_version 2

  %global tests_tar test-data-copr-backend

  

  Name:       copr-backend
@@ -54,6 +54,8 @@ 

  BuildRequires: python3-sphinx

  BuildRequires: python3-tabulate

  BuildRequires: modulemd-tools >= 0.6

+ BuildRequires: prunerepo >= 1.19

+ BuildRequires: dnf

  

  Requires:   (copr-selinux if selinux-policy-targeted)

  Requires:   ansible
@@ -66,7 +68,7 @@ 

  Requires:   mock

  Requires:   obs-signd

  Requires:   openssh-clients

- Requires:   prunerepo

+ Requires:   prunerepo >= 1.19

  Requires:   python3-copr

  Requires:   python3-copr-common >= 0.10.1.dev

  Requires:   python3-copr-messaging

@@ -1,10 +1,6 @@ 

  import json

  import os

- from subprocess import Popen, PIPE

  

- from shlex import split

- from setproctitle import getproctitle, setproctitle

- from oslo_concurrency import lockutils

  from copr_backend.helpers import get_redis_connection

  

  # todo: add logging here
@@ -12,171 +8,11 @@ 

  # opts = BackendConfigReader().read()

  # log = get_redis_logger(opts, "createrepo", "actions")

  

- from .exceptions import CreateRepoError

- 

- 

  # Some reasonable limit here for exceptional (probably buggy) situations.

  # This is here mostly to not overflow the execve() stack limits.

  MAX_IN_BATCH = 100

  

  

- def run_cmd_unsafe(comm_str, lock_name, lock_path="/var/lock/copr-backend"):

-     # log.info("Running command: {}".format(comm_str))

-     comm = split(comm_str)

-     title = getproctitle()

-     try:

-         # TODO change this to logger

-         setproctitle(title + " [locked] in createrepo")

-         with lockutils.lock(name=lock_name, external=True, lock_path=lock_path):

-             cmd = Popen(comm, stdout=PIPE, stderr=PIPE, encoding="utf-8")

-             out, err = cmd.communicate()

-     except Exception as err:

-         raise CreateRepoError(msg="Failed to execute: {}".format(err), cmd=comm_str)

-     setproctitle(title)

- 

-     if cmd.returncode != 0:

-         raise CreateRepoError(msg="exit code != 0",

-                               cmd=comm_str, exit_code=cmd.returncode,

-                               stdout=out, stderr=err)

-     return out

- 

- 

- def createrepo_unsafe(path, dest_dir=None, base_url=None):

-     """

-         Run createrepo_c on the given path

- 

-         Warning! This function doesn't check user preferences.

-         In most cases use `createrepo(...)`

- 

-     :param string path: target location to create repo

-     :param str dest_dir: [optional] relative to path location for repomd,

-             in most cases you should also provide base_url.

-     :param str base_url: optional parameter for createrepo_c, "--baseurl"

- 

-     :return tuple: (return_code,  stdout, stderr)

-     """

- 

-     comm = ['/usr/bin/createrepo_c', '--database', '--ignore-lock', '--local-sqlite',

-             '--cachedir', '/tmp/', '--workers', '8']

-     if os.path.exists(path + '/repodata/repomd.xml'):

-         comm.append("--update")

-     if "epel-5" in path or "rhel-5" in path:

-         # this is because rhel-5 doesn't know sha256

-         comm.extend(['-s', 'sha', '--checksum', 'md5'])

- 

-     if dest_dir:

-         dest_dir_path = os.path.join(path, dest_dir)

-         comm.extend(['--outputdir', dest_dir_path])

-         if not os.path.exists(dest_dir_path):

-             os.makedirs(dest_dir_path)

- 

-     if base_url:

-         comm.extend(['--baseurl', base_url])

- 

-     mb_comps_xml_path = os.path.join(path, "comps.xml")

-     if os.path.exists(mb_comps_xml_path):

-         comm.extend(['--groupfile', mb_comps_xml_path, '--keep-all-metadata'])

- 

-     comm.append(path)

- 

-     return run_cmd_unsafe(" ".join(map(str, comm)), os.path.join(path, "createrepo.lock"))

- 

- 

- APPDATA_CMD_TEMPLATE = \

-     """/usr/bin/timeout --kill-after=240 180 \

- /usr/bin/appstream-builder \

- --temp-dir={packages_dir}/tmp \

- --cache-dir={packages_dir}/cache \

- --packages-dir={packages_dir} \

- --output-dir={packages_dir}/appdata \

- --basename=appstream \

- --include-failed \

- --min-icon-size=48 \

- --veto-ignore=missing-parents \

- --enable-hidpi \

- --origin={username}/{projectname}

- """

- INCLUDE_APPSTREAM = \

-     """/usr/bin/modifyrepo_c \

- --no-compress \

- {packages_dir}/appdata/appstream.xml.gz \

- {packages_dir}/repodata

- """

- INCLUDE_ICONS = \

-     """/usr/bin/modifyrepo_c \

- --no-compress \

- {packages_dir}/appdata/appstream-icons.tar.gz \

- {packages_dir}/repodata

- """

- 

- 

- def add_appdata(path, username, projectname, lock=None):

-     out = ""

- 

-     # We need to have a possibility to disable an appstream builder for some projects

-     # because it doesn't properly scale up for a large ammount of packages

-     parent_dir = os.path.dirname(os.path.normpath(path))

- 

-     # We don't generate appstream metadata for anything else than the main

-     # directory.  If we reconsidered this in future, we would have to check the

-     # file ../../<main_dir>/.disable-appstream for existance.

-     if ":" in os.path.basename(parent_dir):

-         return out

- 

-     if os.path.exists(os.path.join(parent_dir, ".disable-appstream")):

-         return out

- 

-     kwargs = {

-         "packages_dir": path,

-         "username": username,

-         "projectname": projectname

-     }

-     try:

-         out += "\n" + run_cmd_unsafe(

-             APPDATA_CMD_TEMPLATE.format(**kwargs), os.path.join(path, "createrepo.lock"))

- 

-         if os.path.exists(os.path.join(path, "appdata", "appstream.xml.gz")):

-             out += "\n" + run_cmd_unsafe(

-                 INCLUDE_APPSTREAM.format(**kwargs), os.path.join(path, "createrepo.lock"))

- 

-         if os.path.exists(os.path.join(path, "appdata", "appstream-icons.tar.gz")):

-             out += "\n" + run_cmd_unsafe(

-                 INCLUDE_ICONS.format(**kwargs), os.path.join(path, "createrepo.lock"))

- 

-         # appstream builder provide strange access rights to result dir

-         # fix them, so that lighttpd could serve appdata dir

-         out += "\n" + run_cmd_unsafe("chmod -R +rX {packages_dir}/appdata"

-                                      .format(**kwargs), os.path.join(path, "createrepo.lock"))

-     except CreateRepoError as err:

-         err.stdout = out + "\nLast command\n" + err.stdout

-         raise

-     return out

- 

- 

- def createrepo(path, username, projectname, devel=False, base_url=None):

-     """

-     Creates repodata.  Depending on the "auto_createrepo" parameter it either

-     creates the repodata directory in `path`, or in `path/devel`.

- 

-     :param path: directory with rpms

-     :param username: copr project owner username

-     :param projectname: copr project name

-     :param devel: create the repository in 'devel' subdirectory

-     :param base_url: base_url to access rpms independently of repomd location

- 

-     :return: tuple(returncode, stdout, stderr) produced by `createrepo_c`

-     """

-     # TODO: add means of logging

-     if not devel:

-         out_cr = createrepo_unsafe(path)

-         out_ad = add_appdata(path, username, projectname)

-         return "\n".join([out_cr, out_ad])

- 

-     # Automatic createrepo disabled.  Even so, we still need to createrepo in

-     # special "devel" directory so we can later build packages against it.

-     return createrepo_unsafe(path, base_url=base_url, dest_dir="devel")

- 

- 

  class BatchedCreaterepo:

      """

      Group a "group-able" set of pending createrepo tasks, and execute
@@ -204,8 +40,9 @@ 

         don't have to duplicate the efforts and waste resources.

      """

      # pylint: disable=too-many-instance-attributes

+     # pylint: disable=too-many-arguments

  

-     def __init__(self, dirname, full, add, delete, log,

+     def __init__(self, dirname, full, add, delete, rpms_to_remove, log,

                   devel=False,

                   appstream=True,

                   backend_opts=None,
@@ -215,7 +52,7 @@ 

          self.dirname = dirname

          self.devel = devel

          self.appstream = appstream

- 

+         self.rpms_to_remove = rpms_to_remove

  

          if not backend_opts:

              self.log.error("can't get access to redis, batch disabled")
@@ -229,6 +66,7 @@ 

              "add": add,

              "delete": delete,

              "full": full,

+             "rpms_to_remove": rpms_to_remove,

          })

  

          self.notify_keys = []
@@ -292,10 +130,11 @@ 

          """

          add = set()

          delete = set()

+         rpms_to_remove = set()

          full = False

  

          if self.noop:

-             return (full, add, delete)

+             return (full, add, delete, rpms_to_remove)

  

          for key in self.redis.keys(self.key_pattern):

              assert key != self.key
@@ -335,12 +174,14 @@ 

              # always process the delete requests

              delete.update(task_opts["delete"])

  

+             rpms_to_remove.update(task_opts["rpms_to_remove"])

+ 

              if len(self.notify_keys) >= MAX_IN_BATCH:

                  self.log.info("Batch copr-repo limit %s reached, skip the rest",

                                MAX_IN_BATCH)

                  break

  

-         return (full, add, delete)

+         return (full, add, delete, rpms_to_remove)

  

      def commit(self):

          """

@@ -577,7 +577,7 @@ 

      return "{}-{}-{}.{}".format(name, version, release, arch)

  

  

- def call_copr_repo(directory, devel=False, add=None, delete=None, timeout=None,

+ def call_copr_repo(directory, rpms_to_remove=None, devel=False, add=None, delete=None, timeout=None,

                     logger=None):

      """

      Execute 'copr-repo' tool, and return True if the command succeeded.
@@ -598,6 +598,8 @@ 

      cmd += subdirs('--delete', delete)

      if devel:

          cmd += ['--devel']

+     if rpms_to_remove:

+         cmd += subdirs('--rpms-to-remove', rpms_to_remove)

  

      try:

          if logger:

file modified
+20 -1
@@ -77,6 +77,8 @@ 

                          help="Try try to batch this request with requests "

                               "from other processes.  When specified, the "

                               "process needs an access to Redis DB.")

+     parser.add_argument('--rpms-to-remove', action='append', default=[],

+                         help="list of (s)RPM path names that should be removed")

      parser.add_argument('directory')

      return parser

  
@@ -137,6 +139,10 @@ 

          # this is because rhel-5 doesn't know sha256

          createrepo_cmd.extend(['-s', 'sha', '--checksum', 'md5'])

  

+     mb_comps_xml_path = os.path.join(opts.directory, "repodata", "comps.xml")

+     if os.path.exists(mb_comps_xml_path):

+         createrepo_cmd += ['--groupfile', mb_comps_xml_path, '--keep-all-metadata']

+ 

      # we assume that we could skip the call at the beginning

      createrepo_run_needed = False

  
@@ -164,6 +170,10 @@ 

          createrepo_run_needed = True

          createrepo_cmd += ['--excludes', '*{}/*'.format(subdir)]

  

+     for rpm in opts.rpms_to_remove:

+         createrepo_run_needed = True

+         createrepo_cmd += ['--excludes', '{}'.format(rpm)]

+ 

      filelist = os.path.join(opts.directory, '.copr-createrepo-pkglist')

      if opts.add:

          # assure createrepo is run after each addition
@@ -267,6 +277,13 @@ 

          except:

              opts.log.exception("can't remove %s subdirectory", subdir)

  

+     for rpm in opts.rpms_to_remove:

+         opts.log.info("removing %s", rpm)

+         try:

+             os.unlink(os.path.join(opts.directory, rpm))

+         except OSError:

+             opts.log.exception("can't remove %s", rpm)

+ 

  

  def assert_new_createrepo():

      sp = subprocess.Popen(['/usr/bin/createrepo_c', '--help'],
@@ -293,7 +310,7 @@ 

          return

  

      # Merge others' tasks with ours (if any).

-     (batch_full, batch_add, batch_delete) = batch.options()

+     (batch_full, batch_add, batch_delete, batch_rpms_to_remove) = batch.options()

  

      if batch_full:

          log.info("Others requested full createrepo")
@@ -310,6 +327,7 @@ 

              opts.add += list(batch_add)

  

      opts.delete += list(batch_delete)

+     opts.rpms_to_remove += list(batch_rpms_to_remove)

  

      dont_add = set(opts.delete).intersection(opts.add)

      if dont_add:
@@ -369,6 +387,7 @@ 

          opts.full,

          opts.add,

          opts.delete,

+         opts.rpms_to_remove,

          log=opts.log,

          devel=opts.devel,

          appstream=opts.appstream,

@@ -1,75 +0,0 @@ 

- #!/usr/bin/python3

- 

- import logging

- import os

- 

- import sys

- import pwd

- 

- logging.basicConfig(

-     filename="/var/log/copr-backend/copr_createrepo.log",

-     format='[%(asctime)s][%(levelname)6s]: %(message)s',

-     level=logging.INFO)

- log = logging.getLogger(__name__)

- 

- 

- from copr_backend.createrepo import createrepo

- from copr_backend.helpers import SortedOptParser, BackendConfigReader

- 

- 

- def main(args):

- 

-     parser = SortedOptParser(

-         "copr_create_repo")

- 

-     parser.add_option("-u", "--user", dest="user",

-                       help="copr project owner username")

- 

-     parser.add_option("-p", "--project", dest="project",

-                       help="copr project name")

- 

-     parser.add_option("-f", "--front_url", dest="front_url",

-                       help="copr frontend url")

- 

-     cli_opts, args = parser.parse_args(args)

- 

-     if not cli_opts.user:

-         print("No user was specified, exiting", file=sys.stderr)

-         sys.exit(1)

- 

-     if not cli_opts.project:

-         print("No project was specified, exiting", file=sys.stderr)

-         sys.exit(1)

- 

-     opts = BackendConfigReader().read()

- 

-     front_url = cli_opts.front_url or opts.frontend_base_url

-     project_path = os.path.join(opts.destdir, cli_opts.user, cli_opts.project)

- 

-     log.info("start processing {}/{}".format(cli_opts.user, cli_opts.project))

-     for subdir in os.listdir(project_path):

-         if os.path.isdir(subdir):

-             path = os.path.join(project_path, subdir)

-             log.info("entering dir: {}".format(subdir))

-             createrepo(path=path, front_url=front_url,

-                        username=cli_opts.user, projectname=cli_opts.project)

-             log.info("done dir: {}".format(subdir))

-     log.info("finished processing {}/{}".format(cli_opts.user, cli_opts.project))

- 

- if __name__ == "__main__":

-     """

-         Provides cli interface for conditional execution of createrepo_c

-               depending on user setting `auto_createrepo`

-     """

-     if pwd.getpwuid(os.getuid())[0] != "copr":

-         print("This script should be executed under the `copr` user")

-         sys.exit(1)

-     else:

-         main(sys.argv[1:])

- 

- 

- #with open("coprs_to_cr.txt") as inp:

- #    with open("cr_cmd_list.txt", "w") as outp:

- #        for line in inp:

- #            copr, user = line.strip().split("/")

- #            outp.write("{} -u {} -p {}\n".format("/usr/bin/copr_createrepo.py", copr, user))

file modified
+3 -7
@@ -6,9 +6,8 @@ 

  import argparse

  import pwd

  

- from copr_backend.helpers import BackendConfigReader

+ from copr_backend.helpers import BackendConfigReader, call_copr_repo

  from copr_backend.sign import get_pubkey, unsign_rpms_in_dir, sign_rpms_in_dir, create_user_keys, create_gpg_email

- from copr_backend.createrepo import createrepo_unsafe, add_appdata

  

  logging.basicConfig(

      filename="/var/log/copr-backend/fix_gpg.log",
@@ -66,11 +65,8 @@ 

                  log.exception(str(e))

                  continue

  

-         log.info("> > Running createrepo_unsafe for {}".format(dir_path))

-         createrepo_unsafe(dir_path)

- 

-         log.info("> > Running add_appdata for {}".format(dir_path))

-         add_appdata(dir_path, owner, coprname)

+         log.info("> > Running add_appdata for %s", dir_path)

+         call_copr_repo(dir_path)

  

  

  def main():

@@ -12,13 +12,15 @@ 

  import json

  import multiprocessing

  

+ from prunerepo.helpers import get_rpms_to_remove

+ 

  from copr.exceptions import CoprException

  from copr.exceptions import CoprRequestException

  

  from copr_backend.helpers import BackendConfigReader, get_redis_logger

- from copr_backend.helpers import uses_devel_repo, get_persistent_status, get_auto_prune_status

+ from copr_backend.helpers import uses_devel_repo, get_persistent_status, get_auto_prune_status, call_copr_repo

  from copr_backend.frontend import FrontendClient

- from copr_backend.createrepo import createrepo

+ 

  

  LOG = multiprocessing.log_to_stderr()

  LOG.setLevel(logging.INFO)
@@ -50,7 +52,7 @@ 

          raise Exception("Got non-zero return code ({0}) from prunerepo with stderr: {1}".format(process.returncode, stderr))

      return stdout

  

- def run_prunerepo(chroot_path, username, projectname, projectdir, sub_dir_name, prune_days):

+ def run_prunerepo(chroot_path, username, projectdir, sub_dir_name, prune_days):

      """

      Running prunerepo in background worker.  We don't check the return value, so

      the best we can do is that we return useful success/error message that will
@@ -58,11 +60,8 @@ 

      """

      try:

          LOG.info("Pruning of %s/%s/%s started", username, projectdir, sub_dir_name)

-         cmd = ['prunerepo', '--verbose', '--days', str(prune_days), '--nocreaterepo', chroot_path]

-         stdout = runcmd(cmd)

-         LOG.info("Prunerepo stdout:\n%s", stdout)

-         createrepo(path=chroot_path, username=username,

-                    projectname=projectname)

+         rpms = get_rpms_to_remove(chroot_path, days=prune_days, log=LOG)

+         call_copr_repo(directory=chroot_path, rpms_to_remove=rpms)

          clean_copr(chroot_path, prune_days, verbose=True)

      except Exception as err:  # pylint: disable=broad-except

          LOG.exception(err)
@@ -172,7 +171,7 @@ 

                      continue

  

              self.pool.apply_async(run_prunerepo,

-                                   (chroot_path, username, projectname,

+                                   (chroot_path, username,

                                     projectdir, sub_dir_name, self.prune_days))

  

  

@@ -10,6 +10,11 @@ 

  import logging

  import pwd

  

+ from copr_backend.helpers import (BackendConfigReader, create_file_logger,

+                              uses_devel_repo, call_copr_repo)

+ from copr_backend.sign import get_pubkey, sign_rpms_in_dir, create_user_keys

+ from copr_backend.exceptions import CoprSignNoKeyError

+ 

  

  logging.basicConfig(

      filename="/var/log/copr-backend/onetime_signer.log",
@@ -18,32 +23,15 @@ 

  log = logging.getLogger(__name__)

  

  

- from copr_backend.helpers import (BackendConfigReader, create_file_logger,

-                              uses_devel_repo)

- from copr_backend.sign import get_pubkey, sign_rpms_in_dir, create_user_keys

- from copr_backend.exceptions import CoprSignNoKeyError

- from copr_backend.createrepo import createrepo

- 

- 

- def check_signed_rpms_in_pkg_dir(pkg_dir, user, project, chroot, chroot_dir, opts, devel):

+ def check_signed_rpms_in_pkg_dir(pkg_dir, user, project, opts, chroot_dir, devel):

      success = True

  

      logger = create_file_logger("run.check_signed_rpms_in_pkg_dir",

                                  "/tmp/copr_check_signed_rpms.log")

      try:

          sign_rpms_in_dir(user, project, pkg_dir, opts, log=logger)

- 

          log.info("running createrepo for {}".format(pkg_dir))

-         base_url = "/".join([opts.results_baseurl, user,

-                              project, chroot])

-         createrepo(

-             path=chroot_dir,

-             base_url=base_url,

-             username=user,

-             projectname=project,

-             devel=devel,

-         )

- 

+         call_copr_repo(directory=chroot_dir, devel=devel)

      except Exception as err:

          success = False

          log.error(">>> Failed to check/sign rpm in dir pkg_dir")
@@ -77,7 +65,7 @@ 

              log.debug(">> Stepping into package: {}".format(mb_pkg_path))

  

              if not check_signed_rpms_in_pkg_dir(mb_pkg_path, user, project,

-                                                 chroot, chroot_path, opts,

+                                                 opts, chroot_path,

                                                  devel):

                  success = False

  

file modified
+15
@@ -129,3 +129,18 @@ 

          os.mkdir(chdir)

          shutil.copy(source, chdir)

      yield ctx

+ 

+ 

+ @fixture

+ def f_builds_to_prune(f_empty_repos):

+     """

+     Prepare repositories to test prunerepo.

+     """

+     ctx = f_empty_repos

+     source = os.path.join(os.environ["TEST_DATA_DIRECTORY"],

+                           "to_prune")

+     for chroot in ctx.chroots:

+         chdir = os.path.join(ctx.empty_dir, chroot)

+         shutil.rmtree(chdir)

+         shutil.copytree(source, chdir)

+     yield ctx

file modified
+13 -235
@@ -1,246 +1,21 @@ 

  import os

- import copy

  import json

  import logging

- import tarfile

  import tempfile

  import shutil

- import time

- import pytest

  

- from unittest import mock

- from unittest.mock import MagicMock

+ import testlib

+ from testlib import assert_logs_exist, AsyncCreaterepoRequestFactory

  

  from copr_backend.createrepo import (

      BatchedCreaterepo,

-     createrepo,

-     createrepo_unsafe,

      MAX_IN_BATCH,

-     run_cmd_unsafe,

  )

  

  from copr_backend.helpers import BackendConfigReader, get_redis_connection

- from copr_backend.exceptions import CreateRepoError

- 

- import testlib

- from testlib import assert_logs_exist, AsyncCreaterepoRequestFactory

  

  # pylint: disable=attribute-defined-outside-init

  

- @mock.patch('copr_backend.createrepo.createrepo_unsafe')

- @mock.patch('copr_backend.createrepo.add_appdata')

- def test_createrepo_conditional_true(mc_add_appdata, mc_create_unsafe):

-     mc_create_unsafe.return_value = ""

-     mc_add_appdata.return_value = ""

- 

-     createrepo(path="/tmp/", username="foo", projectname="bar")

-     mc_create_unsafe.reset_mock()

- 

-     createrepo(path="/tmp/", username="foo", projectname="bar")

-     mc_create_unsafe.reset_mock()

- 

- 

- @mock.patch('copr_backend.createrepo.createrepo_unsafe')

- def test_createrepo_conditional_false(mc_create_unsafe):

-     base_url = "http://example.com/repo/"

-     createrepo(path="/tmp/", username="foo", projectname="bar", base_url=base_url, devel=True)

-     assert mc_create_unsafe.call_args == mock.call('/tmp/', dest_dir='devel', base_url=base_url)

- 

- 

- @pytest.yield_fixture

- def mc_popen():

-     with mock.patch('copr_backend.createrepo.Popen') as handle:

-         yield handle

- 

- 

- @pytest.yield_fixture

- def mc_run_cmd_unsafe():

-     with mock.patch('copr_backend.createrepo.run_cmd_unsafe') as handle:

-         yield handle

- 

- 

- class TestCreaterepo(object):

-     def setup_method(self, method):

-         self.tmp_dir_name = self.make_temp_dir()

-         self.test_time = time.time()

-         self.base_url = "http://example.com/repo/"

- 

-         self.username = "foo"

-         self.projectname = "bar"

- 

-     # def unpack_resource(self, resource_name):

-     #     if self.tmp_dir_name is None:

-     #         self.make_temp_dir()

-     #

-     #     src_path = os.path.join(os.path.dirname(__file__),

-     #                             "_resources", resource_name)

-     #

-     #     with tarfile.open(src_path, "r:gz") as tfile:

-     #         tfile.extractall(os.path.join(self.tmp_dir_name, "old_dir"))

- 

-     def teardown_method(self, method):

-         self.rm_tmp_dir()

- 

-     def rm_tmp_dir(self):

-         if self.tmp_dir_name:

-             shutil.rmtree(self.tmp_dir_name)

-             self.tmp_dir_name = None

- 

-     def make_temp_dir(self):

-         root_tmp_dir = tempfile.gettempdir()

-         subdir = "test_createrepo_{}".format(time.time())

-         self.tmp_dir_name = os.path.join(root_tmp_dir, subdir)

-         os.mkdir(self.tmp_dir_name)

-         return self.tmp_dir_name

- 

-     #def test_add_appdata(self, mc_run_cmd_unsafe):

-     #    todo: implement, need to test behaviour with/withou produced appstream files

-     #    for lock in [None, MagicMock()]:

-     #        add_appdata(self.tmp_dir_name, self.username, self.projectname, lock=lock)

-     #        print mc_run_cmd_unsafe.call_args_list

-     #        mc_run_cmd_unsafe.reset()

- 

-     def test_run_cmd_unsafe_ok(self, mc_popen):

-         cmd = "foo --bar"

-         mc_popen.return_value.communicate.return_value = ("stdout", "stderr")

-         mc_popen.return_value.returncode = 0

- 

-         assert run_cmd_unsafe(cmd, self.tmp_dir_name, lock_path="/tmp") == "stdout"

-         mc_popen.reset()

- 

-     def test_run_cmd_unsafe_err_popen(self, mc_popen):

-         cmd = "foo --bar"

-         mc_popen.side_effect = IOError()

- 

-         with pytest.raises(CreateRepoError) as err:

-             run_cmd_unsafe(cmd, self.tmp_dir_name, lock_path="/tmp") == "stdout"

- 

-         assert err.value.cmd == cmd

-         assert mc_popen.call_args[0][0] == ["foo", "--bar"]

-         mc_popen.reset()

- 

-     def test_run_cmd_unsafe_err_return_code(self, mc_popen):

-         cmd = "foo --bar"

-         mc_popen.return_value.communicate.return_value = ("stdout", "stderr")

-         mc_popen.return_value.returncode = 1

- 

- 

-         with pytest.raises(CreateRepoError) as err:

-             run_cmd_unsafe(cmd, self.tmp_dir_name, lock_path="/tmp") == "stdout"

- 

-         assert err.value.cmd == cmd

-         assert err.value.stdout == "stdout"

-         assert err.value.stderr == "stderr"

-         assert err.value.exit_code == 1

-         assert mc_popen.call_args[0][0] == ["foo", "--bar"]

-         mc_popen.reset()

- 

-     def test_run_cmd_unsafe_err_communicate(self, mc_popen):

-         cmd = "foo --bar"

-         mc_handle = MagicMock()

-         mc_popen.return_value = MagicMock()

-         mc_handle.returncode = 0

-         mc_handle.side_effect = RuntimeError()

- 

-         with pytest.raises(CreateRepoError) as err:

-             run_cmd_unsafe(cmd, self.tmp_dir_name, lock_path="/tmp") == "stdout"

- 

-         assert err.value.cmd == cmd

-         assert mc_popen.call_args[0][0] == ["foo", "--bar"]

-         mc_popen.reset()

- 

-     def test_createrepo_generated_commands_existing_repodata(self, mc_run_cmd_unsafe):

-         path_epel_5 = os.path.join(self.tmp_dir_name, "epel-5")

-         expected_epel_5 = ('/usr/bin/createrepo_c --database --ignore-lock --local-sqlite --cachedir /tmp/ --workers 8 '

-                            '--update -s sha --checksum md5 ' + path_epel_5)

-         path_fedora = os.path.join(self.tmp_dir_name, "fedora-21")

-         expected_fedora = ('/usr/bin/createrepo_c --database --ignore-lock --local-sqlite --cachedir /tmp/ --workers 8 '

-                            '--update ' + path_fedora)

-         for path, expected in [(path_epel_5, expected_epel_5), (path_fedora, expected_fedora)]:

-             os.makedirs(path)

- 

-             repo_path = os.path.join(path, "repodata")

-             os.makedirs(repo_path)

-             with open(os.path.join(repo_path, "repomd.xml"), "w") as handle:

-                 handle.write("1")

- 

-             createrepo_unsafe(path)

-             assert mc_run_cmd_unsafe.call_args[0][0] == expected

- 

-     def test_createrepo_generated_commands_comps_xml(self, mc_run_cmd_unsafe):

-         path_epel_5 = os.path.join(self.tmp_dir_name, "epel-5")

-         path_fedora = os.path.join(self.tmp_dir_name, "fedora-21")

-         for path in [path_epel_5, path_fedora]:

-             for add_comps in [True, False]:

-                 os.makedirs(path)

- 

-                 comps_path = os.path.join(path, "comps.xml")

-                 if add_comps:

-                     with open(comps_path, "w") as handle:

-                         handle.write("1")

- 

-                 repo_path = os.path.join(path, "repodata")

-                 os.makedirs(repo_path)

-                 with open(os.path.join(repo_path, "repomd.xml"), "w") as handle:

-                     handle.write("1")

- 

-                 createrepo_unsafe(path)

-                 if add_comps:

-                     assert "--groupfile" in mc_run_cmd_unsafe.call_args[0][0]

-                 else:

-                     assert "--groupfile" not in mc_run_cmd_unsafe.call_args[0][0]

-                 mc_run_cmd_unsafe.mock_reset()

-                 shutil.rmtree(path, ignore_errors=True)

- 

-     def test_createrepo_devel_generated_commands_existing_repodata(self, mc_run_cmd_unsafe):

-         path_epel_5 = os.path.join(self.tmp_dir_name, "epel-5")

-         expected_epel_5 = ("/usr/bin/createrepo_c --database --ignore-lock --local-sqlite --cachedir /tmp/ --workers 8 "

-                            "-s sha --checksum md5 "

-                            "--outputdir " + os.path.join(path_epel_5, "devel") + " "

-                            "--baseurl " + self.base_url + " " + path_epel_5)

-         path_fedora = os.path.join(self.tmp_dir_name, "fedora-21")

-         expected_fedora = ("/usr/bin/createrepo_c --database --ignore-lock --local-sqlite --cachedir /tmp/ --workers 8 "

-                            "--outputdir " + os.path.join(path_fedora, "devel") + " "

-                            "--baseurl " + self.base_url + " " + path_fedora)

-         for path, expected in [(path_epel_5, expected_epel_5), (path_fedora, expected_fedora)]:

-             os.makedirs(path)

- 

-             repo_path = os.path.join(path, "devel", "repodata")

-             os.makedirs(repo_path)

-             with open(os.path.join(repo_path, "repomd.xml"), "w") as handle:

-                 handle.write("1")

- 

-             createrepo_unsafe(path, base_url=self.base_url, dest_dir="devel")

-             assert mc_run_cmd_unsafe.call_args[0][0] == expected

- 

-     def test_createrepo_devel_generated_commands(self, mc_run_cmd_unsafe):

-         path_epel_5 = os.path.join(self.tmp_dir_name, "epel-5")

-         expected_epel_5 = ("/usr/bin/createrepo_c --database --ignore-lock --local-sqlite --cachedir /tmp/ --workers 8 "

-                            "-s sha --checksum md5 "

-                            "--outputdir " + os.path.join(path_epel_5, "devel") + " "

-                            "--baseurl " + self.base_url + " " + path_epel_5)

-         path_fedora = os.path.join(self.tmp_dir_name, "fedora-21")

-         expected_fedora = ("/usr/bin/createrepo_c --database --ignore-lock --local-sqlite --cachedir /tmp/ --workers 8 "

-                            "--outputdir " + os.path.join(path_fedora, "devel") + " "

-                            "--baseurl " + self.base_url + " " + path_fedora)

-         for path, expected in [(path_epel_5, expected_epel_5), (path_fedora, expected_fedora)]:

-             os.makedirs(path)

- 

-             createrepo_unsafe(path, base_url=self.base_url, dest_dir="devel")

-             assert os.path.exists(os.path.join(path, "devel"))

-             assert mc_run_cmd_unsafe.call_args[0][0] == expected

-             # assert mc_popen.call_args == mock.call(expected, stderr=-1, stdout=-1)

- 

-     def test_createrepo_devel_creates_folder(self, mc_run_cmd_unsafe):

-         path_epel_5 = os.path.join(self.tmp_dir_name, "epel-5")

-         path_fedora = os.path.join(self.tmp_dir_name, "fedora-21")

- 

-         for path in [path_epel_5, path_fedora]:

-             os.makedirs(path)

- 

-             createrepo_unsafe(path, base_url=self.base_url, dest_dir="devel")

-             assert os.path.exists(os.path.join(path, "devel"))

- 

  

  class TestBatchedCreaterepo:

      def setup_method(self):
@@ -259,12 +34,13 @@ 

          shutil.rmtree(self.workdir)

          self.redis.flushdb()

  

-     def _prep_batched_repo(self, some_dir, full=False, add=None, delete=None):

+     def _prep_batched_repo(self, some_dir, full=False, add=None, delete=None, rpms_to_remove=None):

          self.bcr = BatchedCreaterepo(

              some_dir,

              full,

              add if add is not None else ["subdir_add_1", "subdir_add_2"],

              delete if delete is not None else ["subdir_del_1", "subdir_del_2"],

+             rpms_to_remove if rpms_to_remove is not None else [],

              logging.getLogger(),

              backend_opts=self.config,

          )
@@ -287,6 +63,7 @@ 

              "add": ["subdir_add_1", "subdir_add_2"],

              "delete": ["subdir_del_1", "subdir_del_2"],

              "full": False,

+             "rpms_to_remove": [],

          }

          self.request_createrepo.get(some_dir)

          # appstream=False has no effect, others beat it, appstream=False
@@ -297,7 +74,8 @@ 

          assert not bcr.check_processed()

          assert bcr.options() == (False,

                                   set(["add_1"]),

-                                  set(["del_1", "del_2"]))

+                                  set(["del_1", "del_2"]),

+                                  set([]))

          assert len(bcr.notify_keys) == 3

  

          our_key = keys[0]
@@ -338,7 +116,7 @@ 

          assert not bcr.check_processed()

  

          # we only process the first other request

-         assert bcr.options() == (False, set(["add_1"]), set())

+         assert bcr.options() == (False, set(["add_1"]), set(), set())

          assert len(bcr.notify_keys) == 1  # still one to notify

          assert self.redis.hgetall(key) == {}

          assert len(caplog.record_tuples) == 2
@@ -357,7 +135,7 @@ 

          assert not bcr.check_processed()

  

          # we only process the first other request

-         assert bcr.options() == (False, set(["add_1"]), set())

+         assert bcr.options() == (False, set(["add_1"]), set(), set())

          assert len(bcr.notify_keys) == 1  # still one to notify

          assert self.redis.hgetall(key) == {}

          assert len(caplog.record_tuples) == 2
@@ -382,7 +160,7 @@ 

          assert not bcr.check_processed()

          assert len(self.redis.keys()) == 3

  

-         assert bcr.options() == (False, {"add_1"}, {"del_1"})

+         assert bcr.options() == (False, {"add_1"}, {"del_1"}, set())

          assert len(bcr.notify_keys) == 2

  

      def test_batched_createrepo_full_others_take_us(self):
@@ -400,7 +178,7 @@ 

          self.request_createrepo.get(some_dir, {"add": [], "delete": [], "full": True})

          self.request_createrepo.get(some_dir, {"add": [], "delete": ["del_1"]})

  

-         assert bcr.options() == (True, set(), {"del_1"})

+         assert bcr.options() == (True, set(), {"del_1"}, set())

          assert len(bcr.notify_keys) == 3

  

      def test_batched_createrepo_task_limit(self, caplog):
@@ -423,8 +201,8 @@ 

          expected.remove("add_2")

          assert len(expected) == MAX_IN_BATCH

  

-         full, add, remove = bcr.options()

-         assert (full, remove) == (False, set())

+         full, add, remove, rpms_to_remove = bcr.options()

+         assert (full, remove, rpms_to_remove) == (False, set(), set())

          assert len(add) == MAX_IN_BATCH

  

          # The redis.keys() isn't sorted, and even if it was - PID would make the

@@ -5,18 +5,13 @@ 

  import shutil

  import subprocess

  import tempfile

+ import glob

  from unittest import mock

  

  import pytest

  from packaging import version

  import munch

  

- from copr_backend.helpers import (

-     BackendConfigReader,

-     call_copr_repo,

-     get_redis_connection,

- )

- 

  from testlib.repodata import load_primary_xml

  from testlib import (

      assert_files_in_dir,
@@ -24,9 +19,19 @@ 

      minimal_be_config,

  )

  

+ from copr_prune_results import run_prunerepo

+ 

+ from copr_backend.helpers import (

+     BackendConfigReader,

+     call_copr_repo,

+     get_redis_connection,

+ )

+ 

+ 

  modifyrepo = 'run/copr-repo'

  

  # pylint: disable=attribute-defined-outside-init

+ # pylint: disable=too-many-public-methods

  

  @contextlib.contextmanager

  def _lock(directory="non-existent"):
@@ -420,3 +425,76 @@ 

          assert len(keys) == 1

          task_dict = self.redis.hgetall(keys[0])

          assert task_dict["status"] == "success"

+ 

+     @mock.patch("copr_backend.helpers.subprocess.call")

+     def test_copr_repo_rpms_to_remove_in_call(self, call):

+         """ check that list of rpm files to be removed is added to copr-repo call """

+         _unused = self

+         call.return_value = 0  # exit status 0

+         assert call_copr_repo('/some/dir', rpms_to_remove=['xxx.rpm'])

+         assert len(call.call_args_list) == 1

+         call = call.call_args_list[0]

+         assert call[0][0] == ['copr-repo', '--batched', '/some/dir', '--rpms-to-remove', 'xxx.rpm']

+ 

+     def test_copr_repo_rpms_to_remove_passes(self, f_third_build):

+         _unused = self

+         ctx = f_third_build

+         chroot = ctx.chroots[0]

+         chrootdir = os.path.join(ctx.empty_dir, chroot)

+ 

+         assert_files_in_dir(chrootdir, ["00000002-example/example-1.0.4-1.fc23.x86_64.rpm"], [])

+         assert call_copr_repo(chrootdir, rpms_to_remove=["00000002-example/example-1.0.4-1.fc23.x86_64.rpm"])

+         assert_files_in_dir(chrootdir, [], ["00000002-example/example-1.0.4-1.fc23.x86_64.rpm"])

+ 

+     def test_copr_repo_rpms_to_remove_passes_2(self, f_third_build):

+         _unused = self

+         ctx = f_third_build

+         chroot = ctx.chroots[0]

+         chrootdir = os.path.join(ctx.empty_dir, chroot)

+ 

+         assert_files_in_dir(chrootdir, ["00000002-example/example-1.0.4-1.fc23.x86_64.rpm"], [])

+         assert call_copr_repo(chrootdir, rpms_to_remove=[])

+         assert_files_in_dir(chrootdir, ["00000002-example/example-1.0.4-1.fc23.x86_64.rpm"], [])

+ 

+     def test_copr_repo_rpms_to_remove_passes_3(self, f_third_build):

+         _unused = self

+         ctx = f_third_build

+         chroot = ctx.chroots[0]

+         chrootdir = os.path.join(ctx.empty_dir, chroot)

+ 

+         assert call_copr_repo(chrootdir, add=[ctx.builds[0]])

+         assert_files_in_dir(chrootdir, ["00000001-prunerepo", "00000002-example"], [])

+         assert call_copr_repo(chrootdir,

+                               rpms_to_remove=["00000002-example/example-1.0.4-1.fc23.x86_64.rpm"])

+         assert_files_in_dir(chrootdir,

+                             [],

+                             ["00000002-example/example-1.0.4-1.fc23.x86_64.rpm"])

+         assert_files_in_dir(chrootdir,

+                             ["00000001-prunerepo/prunerepo-1.1-1.fc23.noarch.rpm"],

+                             [])

+ 

+     def test_comps_present(self, f_third_build):

+         _unused = self

+         ctx = f_third_build

+         chroot = ctx.chroots[0]

+         chrootdir = os.path.join(ctx.empty_dir, chroot)

+         copms_path = os.path.join(chrootdir, "repodata", "comps.xml")

+         with open(copms_path, "w"):

+             pass

+ 

+         assert call_copr_repo(chrootdir)

+         name = glob.glob(os.path.join(chrootdir, "repodata", "*-comps.xml.gz"))

+         assert os.path.exists(name[0])

+ 

+     def test_run_prunerepo(self, f_builds_to_prune):

+         _unused = self

+         ctx = f_builds_to_prune

+         chroot = ctx.chroots[0]

+         chrootdir = os.path.join(ctx.empty_dir, chroot)

+         assert_files_in_dir(os.path.join(chrootdir, '00999999-dummy-pkg'),

+                             ["dummy-pkg-1-1.fc34.x86_64.rpm"],

+                             [])

+         run_prunerepo(chrootdir, 'john', 'empty', chroot, 0)

+         assert_files_in_dir(os.path.join(chrootdir, '00999999-dummy-pkg'),

+                             [],

+                             ["dummy-pkg-1-1.fc34.x86_64.rpm"])

@@ -243,6 +243,7 @@ 

              "add": ["add_1"],

              "delete": [],

              "full": False,

+             "rpms_to_remove": [],

          }

          self.pid += 1

          if overrides:

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

Please use dashes, not underscores here - like --rpms-to-remove.

I love to see the huge code removal! :-) But there's still copr_fix_gpg.py that relies on createrepo_unsafe. Either that code needs to be fixed as well, or postpone the removal to
a different PR.

Perhaps we could just use unlink?

Metadata Update from @praiskup:
- Pull-request tagged with: needs-tests, needs-work

4 years ago

We also need a new test-case for in test_modifyrepo.py for the new option...
Not sure about the coverage report, did not check so far.

Thank you for this PR!

rebased onto 6516dc28c76f302daee4540a18882f322c1f2f1d

4 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto 97c267cd1a7ddd36f697281d8ed842497063ceca

4 years ago

Metadata Update from @schlupov:
- Pull-request untagged with: needs-work

4 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

There's a CI build failure, and here I'd be much more confident with "green" Zuul, too.

There's a CI build failure, and here I'd be much more confident with "green" Zuul, too.

To that CI build failure, I'm just waiting for this PR https://pagure.io/prunerepo/pull-request/10 to be merged and released because I'm using the new get_rpms_to_remove function from the pull request.

rebased onto 51721541c9b4ae481b6e63b5baecacac52384fb8

4 years ago

Build succeeded.

I'm just waiting for this PR

Feel free to build the updated prunerepo package in @copr/copr-dev, so it is available at CI time.

rebased onto d417e7c172ecaa44c500cc9c392eddb7adf63491

4 years ago

Build succeeded.

I updated the prunerepo package in copr-dev, though you probably have to put prunerepo to BuildRequires now.

rebased onto a0aabc222f2902f5174e26e0f09aa81c5c2ee629

4 years ago

Build succeeded.

rebased onto 66a68f36320b74800169d3b71d5936fecc10290a

4 years ago

rebased onto 8402e5c14fac1716611ef74baee3fd8e09a6913b

4 years ago

Build succeeded.

For some reason, it only knows the prunerepo package in fedora-32-x86_64.

The default Fedora packages in F33 have higher NVR, so those are installed instead. We should release the new version, and build v1.19 ...

Please add prunerepo >= 1.19 here.

And the same should be in Requires:.

rebased onto 4dd24b8ff5c42cd0c0ec951cf1304432e80d6ff6

4 years ago

Metadata Update from @schlupov:
- Pull-request untagged with: needs-tests

4 years ago

Build succeeded.

Can you please pass down the LOG object?

rebased onto 624d3e958875a2dbba35cc5af361ab3bc0c9667a

4 years ago

Build succeeded.

rebased onto cfc18e7ecc55fe6789dce619f3f07c069bc88bc7

4 years ago

Build succeeded.

Ooops, this whole logic needs to go through BatchedCreaterepo, and we need to merge the options from other copr-repo runs.

Metadata Update from @praiskup:
- Pull-request tagged with: needs-work

4 years ago

Could we change the logic of call_copr_repo so the delete option would be reused also for this? It now accepts only subdirectories, so I'm not sure ...

tests: backend: change prunerepo logic, use get_rpms_to_remove from
prunerepo.helpers

The tests: prefix is weird. We use get_rpms_to_remove from just prunerepo.

While we are touching this line, use just log.info("... %s ...", dir_path).

This rpms_to_path is normally a relative path, so just something like 00000002-example/example-1.0.4-1.fc23.x86_64.rpm, can you change the test?

Same here, the call_copr_repo should work with relative paths, as got from get_rpms_to_remove.

Nice catch here. This is probably causes #1431, but we need to have a test-case for this, please.

rebased onto d93f63528211363e4e739c61c9595f70298a9140

4 years ago

Build succeeded.

Not literally :-) sorry, I used ellipse ... to make it easier to write, but "> > Running add_appdata for %s", dir_path) is correct

rebased onto 35106ec4673255350551ce7dbce1ea28b886d1b3

4 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto cba3c35b269e75322acaaf11c88398f9eb0c2c0d

4 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto bca712285e8fc197c53d8a963b839f6bb05dd549

4 years ago

Build succeeded.

Could we change the logic of call_copr_repo so the delete option would be reused also for this? It now accepts only subdirectories, so I'm not sure ...

I wouldn't mix it together, the delete option is for subdirectories and rpms-to-remove is only for some files in a directory.

PTAL, I resolved problems with BatchedCreaterepo.

Metadata Update from @schlupov:
- Pull-request untagged with: needs-work

4 years ago

This check doesn't seem to be necessary. Just try to unlink, and ignore the FileNotFoundError?

This .get() change is not necessary.

We call it like rpms_to_remove=['00000002-example/example-1.0.4-1.fc23.x86_64.rpm'], can you change it in the tests, too?

Actually, I would be much more confident if we also tested the run_prunerepo method directly, I'll prepare a new tests tarball for this purpose ... give me a while...

pylint didn't like the fact that the class accepts 11 arguments, and I also thought it is a lot.

We can do such refactorings later ... feel free to silence PyLint on that line/block.

Take a look at #1785, that fixture creates a directory tree that can be used to test this.

rebased onto 975b05f2a74e996bbf43e2f16cfb8f1a6c54fe63

4 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

Please also check that one RPM is removed.

rebased onto 54958fcf740ffc3b448cc203976a4a9e470ee1b4

4 years ago

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.

Build succeeded.

rebased onto 00344cc

4 years ago

Build succeeded.

Those two LOG output's don't provide context. I'd prefer to remove them entirely.

2 new commits added

  • test: backend: change prunerepo logic, use get_rpms_to_remove from
  • backend: new fixture for testing prunerepo
4 years ago

Those two LOG output's don't provide context. I'd prefer to remove them entirely.

Removed, I forgot it there, it was just for debugging purposes.

Build succeeded.

Pull-Request has been merged by frostyx

4 years ago