From ff5288dc0bb2db526be5995f3d55c638e13ade2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Such=C3=BD?= Date: Wed, 11 Sep 2024 08:19:41 +0200 Subject: [PATCH 01/18] common: cleanup - remove six dependency --- common/copr_common/enums.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/common/copr_common/enums.py b/common/copr_common/enums.py index 5f831fdd9..b68fc2495 100644 --- a/common/copr_common/enums.py +++ b/common/copr_common/enums.py @@ -1,8 +1,6 @@ import random import string -from six import with_metaclass - # We don't know how to define the enums without `class`. # pylint: disable=too-few-public-methods @@ -22,7 +20,7 @@ def __getattr__(cls, attr): return cls._wrap(attr) -class ActionTypeEnum(with_metaclass(EnumType, object)): +class ActionTypeEnum(metaclass=EnumType): vals = { "delete": 0, "rename": 1, @@ -39,7 +37,7 @@ class ActionTypeEnum(with_metaclass(EnumType, object)): } -class ActionResult(with_metaclass(EnumType, object)): +class ActionResult(metaclass=EnumType): vals = { 'WAITING': 0, 'SUCCESS': 1, @@ -47,7 +45,7 @@ class ActionResult(with_metaclass(EnumType, object)): } -class DefaultActionPriorityEnum(with_metaclass(EnumType, object)): +class DefaultActionPriorityEnum(metaclass=EnumType): """ The higher the 'priority' is, the later the task is taken. Keep actions priority in range -100 to 100 @@ -64,7 +62,7 @@ class DefaultActionPriorityEnum(with_metaclass(EnumType, object)): } -class ActionPriorityEnum(with_metaclass(EnumType, object)): +class ActionPriorityEnum(metaclass=EnumType): """ Naming/assigning the values is a little bit tricky because how the current implementation works (i.e. it is inverted). @@ -74,15 +72,15 @@ class ActionPriorityEnum(with_metaclass(EnumType, object)): vals = {"highest": -99, "lowest": 99} -class BackendResultEnum(with_metaclass(EnumType, object)): +class BackendResultEnum(metaclass=EnumType): vals = {"waiting": 0, "success": 1, "failure": 2} -class RoleEnum(with_metaclass(EnumType, object)): +class RoleEnum(metaclass=EnumType): vals = {"user": 0, "admin": 1} -class StatusEnum(with_metaclass(EnumType, object)): +class StatusEnum(metaclass=EnumType): vals = { "failed": 0, # build failed "succeeded": 1, # build succeeded @@ -111,7 +109,7 @@ class ModuleStatusEnum(StatusEnum): "failed", "succeeded", "waiting", "unknown"]) -class BuildSourceEnum(with_metaclass(EnumType, object)): +class BuildSourceEnum(metaclass=EnumType): vals = {"unset": 0, "link": 1, # url "upload": 2, # pkg, tmp, url @@ -123,7 +121,7 @@ class BuildSourceEnum(with_metaclass(EnumType, object)): } -class FailTypeEnum(with_metaclass(EnumType, object)): +class FailTypeEnum(metaclass=EnumType): vals = {"unset": 0, # General errors mixed with errors for SRPM URL/upload: "unknown_error": 1, @@ -139,7 +137,7 @@ class FailTypeEnum(with_metaclass(EnumType, object)): } -class StorageEnum(with_metaclass(EnumType, object)): +class StorageEnum(metaclass=EnumType): """ Supported storages """ From 19eff0c8e0490a3c540fdc2844be4d57f660002e Mon Sep 17 00:00:00 2001 From: Jakub Kadlcik Date: Sun, 8 Sep 2024 12:03:15 +0200 Subject: [PATCH 02/18] copr: wait until Pulp publication is finished --- backend/copr_backend/pulp.py | 16 ++++++++++++++++ backend/copr_backend/storage.py | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/backend/copr_backend/pulp.py b/backend/copr_backend/pulp.py index c794f1cdc..5cf6956db 100644 --- a/backend/copr_backend/pulp.py +++ b/backend/copr_backend/pulp.py @@ -3,6 +3,7 @@ """ import os +import time import tomllib from urllib.parse import urlencode import requests @@ -178,3 +179,18 @@ def delete_distribution(self, distribution): """ url = self.config["base_url"] + distribution return requests.delete(url, **self.request_params) + + def wait_for_finished_task(self, task): + """ + Pulp task (e.g. creating a publication) can be running for an + unpredictably long time. We need to wait until it is finished to know + what it actually did. + """ + while True: + response = self.get_task(task) + if not response.ok: + break + if response.json()["state"] != "waiting": + break + time.sleep(5) + return response diff --git a/backend/copr_backend/storage.py b/backend/copr_backend/storage.py index 182382750..4ec9a7236 100644 --- a/backend/copr_backend/storage.py +++ b/backend/copr_backend/storage.py @@ -180,7 +180,7 @@ def publish_repository(self, chroot, **kwargs): return False task = response.json()["task"] - response = self.client.get_task(task) + response = self.client.wait_for_finished_task(task) if not response.ok: self.log.error("Failed to get Pulp task %s because of %s", task, response.text) From c5166a13337042a11d474aa7f3666263606c618f Mon Sep 17 00:00:00 2001 From: Jakub Kadlcik Date: Thu, 5 Sep 2024 09:43:09 +0200 Subject: [PATCH 03/18] backend, frontend: implement project and build deletion in Pulp Fix #3318 Fix #3319 --- backend/copr_backend/actions.py | 138 +++++++----------- backend/copr_backend/pulp.py | 21 ++- backend/copr_backend/storage.py | 129 +++++++++++++++- backend/tests/test_action.py | 2 +- .../coprs/logic/actions_logic.py | 10 +- .../tests/test_logic/test_builds_logic.py | 5 +- 6 files changed, 211 insertions(+), 94 deletions(-) diff --git a/backend/copr_backend/actions.py b/backend/copr_backend/actions.py index effff12af..d98d91947 100644 --- a/backend/copr_backend/actions.py +++ b/backend/copr_backend/actions.py @@ -20,14 +20,13 @@ from copr_common.worker_manager import WorkerManager from copr_backend.worker_manager import BackendQueueTask -from copr_backend.storage import storage_for_enum +from copr_backend.storage import storage_for_enum, BackendStorage from .sign import create_user_keys, CoprKeygenRequestError from .exceptions import CreateRepoError, CoprSignError, FrontendClientException from .helpers import (get_redis_logger, silent_remove, ensure_dir_exists, get_chroot_arch, format_filename, - uses_devel_repo, call_copr_repo, build_chroot_log_name, - copy2_but_hardlink_rpms) + call_copr_repo, copy2_but_hardlink_rpms) from .sign import sign_rpms_in_dir, unsign_rpms_in_dir, get_pubkey @@ -64,7 +63,6 @@ def get_action_class(cls, action): ActionTypeEnum("rawhide_to_release"): RawhideToRelease, ActionTypeEnum("fork"): Fork, ActionTypeEnum("build_module"): BuildModule, - ActionTypeEnum("delete"): Delete, ActionTypeEnum("remove_dirs"): RemoveDirs, }.get(action_type, None) @@ -102,8 +100,15 @@ def __init__(self, opts, action, log=None): project = self.ext_data.get("projectname") devel = self.ext_data.get("devel") appstream = self.ext_data.get("appstream") - self.storage = storage_for_enum(enum, owner, project, appstream, - devel, self.opts, self.log) + args = [owner, project, appstream, devel, self.opts, self.log] + self.storage = storage_for_enum(enum, *args) + + # Even though we already have `self.storage` which uses an + # appropriate storage for the project (e.g. Pulp), the project + # still has some data on backend (logs, srpm-builds chroot, etc). + # Many actions need to be performed on `self.storage` and + # `self.backend_storage` at the same time + self.backend_storage = storage_for_enum(StorageEnum.backend, *args) def __str__(self): return "<{}(Action): {}>".format(self.__class__.__name__, self.data) @@ -221,72 +226,24 @@ def run(self): return result -class Delete(Action): - """ - Abstract class for all other Delete* classes. - """ - # pylint: disable=abstract-method - def _handle_delete_builds(self, ownername, projectname, project_dirname, - chroot_builddirs, build_ids, appstream): - """ call /bin/copr-repo --delete """ - devel = uses_devel_repo(self.front_url, ownername, projectname) - result = BackendResultEnum("success") - for chroot, subdirs in chroot_builddirs.items(): - chroot_path = os.path.join(self.destdir, ownername, project_dirname, - chroot) - if not os.path.exists(chroot_path): - self.log.error("%s chroot path doesn't exist", chroot_path) - result = BackendResultEnum("failure") - continue - - self.log.info("Deleting subdirs [%s] in %s", - ", ".join(subdirs), chroot_path) - - # Run createrepo first and then remove the files (to avoid old - # repodata temporarily pointing at non-existing files)! - if chroot != "srpm-builds": - # In srpm-builds we don't create repodata at all - if not call_copr_repo(chroot_path, delete=subdirs, devel=devel, appstream=appstream, - logger=self.log): - result = BackendResultEnum("failure") - - for build_id in build_ids or []: - log_paths = [ - os.path.join(chroot_path, build_chroot_log_name(build_id)), - # we used to create those before - os.path.join(chroot_path, 'build-{}.rsync.log'.format(build_id)), - os.path.join(chroot_path, 'build-{}.log'.format(build_id))] - for log_path in log_paths: - try: - os.unlink(log_path) - except OSError: - self.log.debug("can't remove %s", log_path) - return result - - -class DeleteProject(Delete): +class DeleteProject(Action): def run(self): self.log.debug("Action delete copr") - result = BackendResultEnum("success") - - ext_data = json.loads(self.data["data"]) - ownername = ext_data["ownername"] - project_dirnames = ext_data["project_dirnames"] + project_dirnames = self.ext_data["project_dirnames"] - if not ownername: + if not self.storage.owner: self.log.error("Received empty ownername!") - result = BackendResultEnum("failure") - return result + return BackendResultEnum("failure") for dirname in project_dirnames: if not dirname: self.log.warning("Received empty dirname!") continue - path = os.path.join(self.destdir, ownername, dirname) - if os.path.exists(path): - self.log.info("Removing copr dir %s", path) - shutil.rmtree(path) - return result + self.storage.delete_project(dirname) + if not isinstance(self.storage, BackendStorage): + self.backend_storage.delete_project(dirname) + + return BackendResultEnum("success") class CompsUpdate(Action): @@ -322,7 +279,7 @@ def run(self): return result -class DeleteMultipleBuilds(Delete): +class DeleteMultipleBuilds(Action): def run(self): self.log.debug("Action delete multiple builds.") @@ -334,25 +291,24 @@ def run(self): # srpm-builds: [00849545, 00849546] # fedora-30-x86_64: [00849545-example, 00849545-foo] # [...] - ext_data = json.loads(self.data["data"]) - ownername = ext_data["ownername"] - projectname = ext_data["projectname"] - project_dirnames = ext_data["project_dirnames"] - build_ids = ext_data["build_ids"] - appstream = ext_data["appstream"] + project_dirnames = self.ext_data["project_dirnames"] + build_ids = self.ext_data["build_ids"] result = BackendResultEnum("success") for project_dirname, chroot_builddirs in project_dirnames.items(): - if BackendResultEnum("failure") == \ - self._handle_delete_builds(ownername, projectname, - project_dirname, chroot_builddirs, - build_ids, appstream): + args = [project_dirname, chroot_builddirs, build_ids] + success = self.storage.delete_builds(*args) + + if not isinstance(self.storage, BackendStorage): + success = self.backend_storage.delete_builds(*args) and success + + if not success: result = BackendResultEnum("failure") return result -class DeleteBuild(Delete): +class DeleteBuild(Action): def run(self): self.log.info("Action delete build.") @@ -363,25 +319,31 @@ def run(self): # chroot_builddirs: # srpm-builds: [00849545] # fedora-30-x86_64: [00849545-example] - ext_data = json.loads(self.data["data"]) - try: - ownername = ext_data["ownername"] - build_ids = [self.data['object_id']] - projectname = ext_data["projectname"] - project_dirname = ext_data["project_dirname"] - chroot_builddirs = ext_data["chroot_builddirs"] - appstream = ext_data["appstream"] - except KeyError: + valid = "object_id" in self.data + keys = {"ownername", "projectname", "project_dirname", + "chroot_builddirs", "appstream"} + for key in keys: + if key not in self.ext_data: + valid = False + break + + if not valid: self.log.exception("Invalid action data") return BackendResultEnum("failure") - return self._handle_delete_builds(ownername, projectname, - project_dirname, chroot_builddirs, - build_ids, appstream) + args = [ + self.ext_data["project_dirname"], + self.ext_data["chroot_builddirs"], + [self.data['object_id']], + ] + success = self.storage.delete_builds(*args) + if not isinstance(self.storage, BackendStorage): + success = self.backend_storage.delete_builds(*args) and success + return BackendResultEnum("success" if success else "failure") -class DeleteChroot(Delete): +class DeleteChroot(Action): def run(self): self.log.info("Action delete project chroot.") chroot = self.ext_data["chrootname"] diff --git a/backend/copr_backend/pulp.py b/backend/copr_backend/pulp.py index 5cf6956db..6dc19042b 100644 --- a/backend/copr_backend/pulp.py +++ b/backend/copr_backend/pulp.py @@ -164,6 +164,16 @@ def create_content(self, repository, path): return requests.post( url, data=data, files=files, **self.request_params) + def delete_content(self, repository, artifacts): + """ + Delete a list of artifacts from a repository + https://pulpproject.org/pulp_rpm/restapi/#tag/Repositories:-Rpm/operation/repositories_rpm_rpm_modify + """ + path = os.path.join(repository, "modify/") + url = self.config["base_url"] + path + data = {"remove_content_units": artifacts} + return requests.post(url, json=data, **self.request_params) + def delete_repository(self, repository): """ Delete an RPM repository @@ -190,7 +200,16 @@ def wait_for_finished_task(self, task): response = self.get_task(task) if not response.ok: break - if response.json()["state"] != "waiting": + if response.json()["state"] not in ["waiting", "running"]: break time.sleep(5) return response + + def list_distributions(self, prefix): + """ + Get a list of distributions whose names match a given prefix + https://pulpproject.org/pulp_rpm/restapi/#tag/Distributions:-Rpm/operation/distributions_rpm_rpm_list + """ + url = self.url("api/v3/distributions/rpm/rpm/?") + url += urlencode({"name__startswith": prefix}) + return requests.get(url, **self.request_params) diff --git a/backend/copr_backend/storage.py b/backend/copr_backend/storage.py index 4ec9a7236..fdb683815 100644 --- a/backend/copr_backend/storage.py +++ b/backend/copr_backend/storage.py @@ -3,9 +3,10 @@ """ import os +import json import shutil from copr_common.enums import StorageEnum -from copr_backend.helpers import call_copr_repo +from copr_backend.helpers import call_copr_repo, build_chroot_log_name from copr_backend.pulp import PulpClient @@ -71,6 +72,18 @@ def delete_repository(self, chroot): """ raise NotImplementedError + def delete_project(self, dirname): + """ + Delete the whole project and all of its repositories and builds + """ + raise NotImplementedError + + def delete_builds(self, dirname, chroot_builddirs, build_ids): + """ + Delete multiple builds from the storage + """ + raise NotImplementedError + class BackendStorage(Storage): """ @@ -116,6 +129,48 @@ def delete_repository(self, chroot): return shutil.rmtree(chroot_path) + def delete_project(self, dirname): + path = os.path.join(self.opts.destdir, self.owner, dirname) + if os.path.exists(path): + self.log.info("Removing copr dir %s", path) + shutil.rmtree(path) + + def delete_builds(self, dirname, chroot_builddirs, build_ids): + result = True + for chroot, subdirs in chroot_builddirs.items(): + chroot_path = os.path.join( + self.opts.destdir, self.owner, dirname, chroot) + if not os.path.exists(chroot_path): + self.log.error("%s chroot path doesn't exist", chroot_path) + result = False + continue + + self.log.info("Deleting subdirs [%s] in %s", + ", ".join(subdirs), chroot_path) + + # Run createrepo first and then remove the files (to avoid old + # repodata temporarily pointing at non-existing files)! + # In srpm-builds we don't create repodata at all + if chroot != "srpm-builds": + repo = call_copr_repo( + chroot_path, delete=subdirs, devel=self.devel, + appstream=self.appstream, logger=self.log) + if not repo: + result = False + + for build_id in build_ids or []: + log_paths = [ + os.path.join(chroot_path, build_chroot_log_name(build_id)), + # we used to create those before + os.path.join(chroot_path, 'build-{}.rsync.log'.format(build_id)), + os.path.join(chroot_path, 'build-{}.log'.format(build_id))] + for log_path in log_paths: + try: + os.unlink(log_path) + except OSError: + self.log.debug("can't remove %s", log_path) + return result + class PulpStorage(Storage): """ @@ -148,6 +203,7 @@ def init_project(self, dirname, chroot): return response.ok def upload_build_results(self, chroot, results_dir, target_dir_name): + resources = [] for root, _, files in os.walk(results_dir): for name in files: if os.path.basename(root) == "prev_build_backup": @@ -169,8 +225,23 @@ def upload_build_results(self, chroot, results_dir, target_dir_name): path, response.text) continue + # This involves a lot of unnecessary waiting until every + # RPM content is created. Once we can reliably label Pulp + # content with Copr build ID, we should drop this code and stop + # creating the `pulp.json` file + task = response.json()["task"] + response = self.client.wait_for_finished_task(task) + created = response.json()["created_resources"] + resources.extend(created) + self.log.info("Uploaded to Pulp: %s", path) + data = {"resources": resources} + path = os.path.join(results_dir, "pulp.json") + with open(path, "w+", encoding="utf8") as fp: + json.dump(data, fp) + self.log.info("Pulp resources: %s", resources) + def publish_repository(self, chroot, **kwargs): repository = self._get_repository(chroot) response = self.client.create_publication(repository) @@ -206,6 +277,62 @@ def delete_repository(self, chroot): self.client.delete_repository(repository) self.client.delete_distribution(distribution) + def delete_project(self, dirname): + prefix = "{0}/{1}".format(self.owner, dirname) + response = self.client.list_distributions(prefix) + distributions = response.json()["results"] + for distribution in distributions: + self.client.delete_distribution(distribution["pulp_href"]) + if distribution["repository"]: + self.client.delete_repository(distribution["repository"]) + + def delete_builds(self, dirname, chroot_builddirs, build_ids): + # pylint: disable=too-many-locals + result = True + for chroot, subdirs in chroot_builddirs.items(): + # We don't upload results of source builds to Pulp + if chroot == "srpm-builds": + continue + + chroot_path = os.path.join( + self.opts.destdir, self.owner, dirname, chroot) + if not os.path.exists(chroot_path): + self.log.error("%s chroot path doesn't exist", chroot_path) + result = False + continue + + repository = self._get_repository(chroot) + for subdir in subdirs: + # It is currently not possible to set labels for Pulp content. + # https://github.com/pulp/pulpcore/issues/3338 + # Until it is implemented, we need read all Pulp resources that + # a copr build created from our `pulp.json` in the resultdir. + path = os.path.join(chroot_path, subdir, "pulp.json") + with open(path, "r", encoding="utf8") as fp: + pulp = json.load(fp) + + for resource in pulp["resources"]: + is_package = resource.split("/api/v3/")[1].startswith( + "content/rpm/packages") + if not is_package: + self.log.info("Not deleting %s", resource) + continue + + # TODO We can make performance improvements here by deleting + # all content at once + response = self.client.delete_content(repository, [resource]) + if response.ok: + self.log.info("Successfully deleted Pulp content %s", resource) + else: + result = False + self.log.info("Failed to delete Pulp content %s", resource) + + published = self.publish_repository(chroot) + if not published: + result = False + + return result + def _repository_name(self, chroot, dirname=None): return "/".join([ self.owner, diff --git a/backend/tests/test_action.py b/backend/tests/test_action.py index 49806a176..bc541e32a 100644 --- a/backend/tests/test_action.py +++ b/backend/tests/test_action.py @@ -488,7 +488,7 @@ def test_delete_build_acr_reflected(self, mc_devel, mc_time, devel): assert new_primary['names'] == set(['prunerepo']) assert len(new_primary_devel['names']) == 3 - @mock.patch("copr_backend.actions.call_copr_repo") + @mock.patch("copr_backend.storage.call_copr_repo") @mock.patch("copr_backend.actions.uses_devel_repo") def test_delete_build_succeeded_createrepo_error(self, mc_devel, mc_call_repo, mc_time): diff --git a/frontend/coprs_frontend/coprs/logic/actions_logic.py b/frontend/coprs_frontend/coprs/logic/actions_logic.py index 321d62d9f..e7a7c43c3 100644 --- a/frontend/coprs_frontend/coprs/logic/actions_logic.py +++ b/frontend/coprs_frontend/coprs/logic/actions_logic.py @@ -145,6 +145,7 @@ def send_delete_copr(cls, copr): data_dict = { "ownername": copr.owner_name, "project_dirnames": [copr_dir.name for copr_dir in copr.dirs], + "storage": copr.storage, } action = models.Action(action_type=ActionTypeEnum("delete"), object_type="copr", @@ -190,6 +191,8 @@ def get_build_delete_data(cls, build): build.copr_dirname if build.copr_dir else build.copr_name, "chroot_builddirs": cls.get_chroot_builddirs(build), "appstream": build.appstream, + "devel": build.copr.devel_mode, + "storage": build.copr.storage, } @classmethod @@ -216,7 +219,12 @@ def send_delete_multiple_builds(cls, builds): :type build: list of models.Build """ project_dirnames = {} - data = {'project_dirnames': project_dirnames} + data = { + "project_dirnames": project_dirnames, + # We can pick any random build because the assumption is, they are + # all from the same project + "storage": builds[0].copr.storage if builds else None, + } build_ids = [] for build in builds: diff --git a/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py b/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py index 24beeed02..faf46529d 100644 --- a/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py +++ b/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py @@ -12,7 +12,7 @@ from coprs import models from coprs.request import NAMED_FILE_FROM_BYTES -from copr_common.enums import StatusEnum +from copr_common.enums import StatusEnum, StorageEnum from coprs.exceptions import (ActionInProgressException, InsufficientRightsException, MalformedArgumentException, @@ -310,7 +310,8 @@ def test_delete_mulitple_builds_no_resultdir( 'fedora-17-x86_64': ['0000PR-pr-package'], } }, - 'build_ids': [1, 2, 5] + 'build_ids': [1, 2, 5], + 'storage': StorageEnum.backend, } with pytest.raises(NoResultFound): From 23a2fa32e93108ccbab187e4279112a25e93a89c Mon Sep 17 00:00:00 2001 From: Jakub Kadlcik Date: Mon, 22 Jul 2024 11:23:53 +0200 Subject: [PATCH 04/18] backend: actions don't call uses_devel_repo function anymore --- backend/tests/test_action.py | 44 ++++++++++++------------------------ 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/backend/tests/test_action.py b/backend/tests/test_action.py index bc541e32a..7d728c751 100644 --- a/backend/tests/test_action.py +++ b/backend/tests/test_action.py @@ -353,10 +353,8 @@ def test_action_run_delete_copr_remove_folders(self, mc_time): assert not os.path.exists(os.path.join(tmp_dir, "old_dir")) - @mock.patch("copr_backend.actions.uses_devel_repo") @mock.patch("copr_backend.actions.call_copr_repo") - def test_delete_no_chroot_dirs(self, mc_call, mc_devel, mc_time): - mc_devel.return_value = False + def test_delete_no_chroot_dirs(self, mc_call, mc_time): mc_time.time.return_value = self.test_time mc_front_cb = MagicMock() @@ -377,9 +375,7 @@ def test_delete_no_chroot_dirs(self, mc_call, mc_devel, mc_time): assert len(mc_call.call_args_list) == 0 assert result == BackendResultEnum("failure") - @mock.patch("copr_backend.actions.uses_devel_repo") - def test_delete_build_succeeded(self, mc_devel, mc_time): - mc_devel.return_value = False + def test_delete_build_succeeded(self, mc_time): mc_time.time.return_value = self.test_time mc_front_cb = MagicMock() @@ -421,13 +417,11 @@ def test_delete_build_succeeded(self, mc_devel, mc_time): assert os.path.exists(chroot_2_dir) @pytest.mark.parametrize('devel', [False, True]) - @mock.patch("copr_backend.actions.uses_devel_repo") - def test_delete_build_acr_reflected(self, mc_devel, mc_time, devel): + def test_delete_build_acr_reflected(self, mc_time, devel): """ When build is deleted, we want to remove it from both devel and normal (production) repodata """ - mc_devel.return_value = devel self.unpack_resource("testresults.tar.gz") chroot = os.path.join(self.test_project_dir, 'fedora-23-x86_64') @@ -472,6 +466,7 @@ def test_delete_build_acr_reflected(self, mc_devel, mc_time, devel): "chroot_builddirs": { "fedora-23-x86_64": [builddir], }, + "devel": devel, }), }, ) @@ -489,8 +484,7 @@ def test_delete_build_acr_reflected(self, mc_devel, mc_time, devel): assert len(new_primary_devel['names']) == 3 @mock.patch("copr_backend.storage.call_copr_repo") - @mock.patch("copr_backend.actions.uses_devel_repo") - def test_delete_build_succeeded_createrepo_error(self, mc_devel, + def test_delete_build_succeeded_createrepo_error(self, mc_call_repo, mc_time): mc_time.time.return_value = self.test_time mc_front_cb = MagicMock() @@ -522,12 +516,10 @@ def test_delete_build_succeeded_createrepo_error(self, mc_devel, # just fail assert test_action.run() == BackendResultEnum("failure") - @mock.patch("copr_backend.actions.uses_devel_repo") - def test_delete_two_chroots(self, mc_devel, mc_time): + def test_delete_two_chroots(self, mc_time): """ Regression test, https://bugzilla.redhat.com/show_bug.cgi?id=1171796 """ - mc_devel.return_value = 0 self.unpack_resource("1171796.tar.gz") chroot_20_path = os.path.join(self.tmp_dir_name, "foo", "bar", "fedora-20-x86_64") @@ -589,13 +581,11 @@ def test_delete_two_chroots(self, mc_devel, mc_time): assert os.path.exists(chroot_20_path) assert os.path.exists(chroot_21_path) - @mock.patch("copr_backend.actions.uses_devel_repo") - def test_delete_two_chroots_two_remain(self, mc_devel, mc_time): + def test_delete_two_chroots_two_remain(self, mc_time): """ Regression test, https://bugzilla.redhat.com/show_bug.cgi?id=1171796 extended: we also put two more chroots, which should be unaffected """ - mc_devel.return_value = 0 self.unpack_resource("1171796_doubled.tar.gz") subdir = os.path.join(self.tmp_dir_name, "foo", "bar") @@ -703,10 +693,8 @@ def test_delete_build_with_bad_pkg_name(self, mc_time): assert os.path.exists(chroot_20_path) assert os.path.exists(chroot_21_path) - @mock.patch("copr_backend.actions.uses_devel_repo") - def test_delete_multiple_builds_succeeded(self, mc_build_devel, mc_time): + def test_delete_multiple_builds_succeeded(self, mc_time): mc_time.time.return_value = self.test_time - mc_build_devel.return_value = False tmp_dir = self.make_temp_dir() @@ -723,6 +711,7 @@ def test_delete_multiple_builds_succeeded(self, mc_build_devel, mc_time): "ownername": "foo", "projectname": "bar", "appstream": True, + "devel": False, "project_dirnames": { 'bar': { "fedora-20": ["01-foo", "02-foo"], @@ -754,11 +743,9 @@ def test_delete_multiple_builds_succeeded(self, mc_build_devel, mc_time): # createrepo always works with non-devel directory. @pytest.mark.parametrize('devel', [False, True]) @mock.patch("copr_backend.helpers.subprocess.Popen") - @mock.patch("copr_backend.actions.uses_devel_repo") - def test_handle_createrepo_ok(self, mc_devel, mc_sp_popen, mc_time, devel): + def test_handle_createrepo_ok(self, mc_sp_popen, mc_time, devel): mc_sp_popen.return_value.communicate.return_value = ("", "") mc_sp_popen.return_value.returncode = 0 - mc_devel.return_value = devel tmp_dir = self.make_temp_dir() @@ -768,7 +755,7 @@ def test_handle_createrepo_ok(self, mc_devel, mc_sp_popen, mc_time, devel): "projectname": "bar", "appstream": True, "project_dirnames": ["bar"], - "devel": False, + "devel": devel, "storage": StorageEnum.backend, }) self.opts.destdir = tmp_dir @@ -786,14 +773,15 @@ def test_handle_createrepo_ok(self, mc_devel, mc_sp_popen, mc_time, devel): for chroot in ['fedora-20-x86_64', 'epel-6-i386']: cmd = ["copr-repo", "--batched", os.path.join(self.test_project_dir, chroot)] + if devel: + cmd.append("--devel") exp_call = mock.call(cmd, stdout=-1, stderr=-1, shell=False, encoding='utf-8') assert exp_call in mc_sp_popen.call_args_list assert len(mc_sp_popen.call_args_list) == 2 @mock.patch("copr_backend.storage.call_copr_repo") - @mock.patch("copr_backend.actions.uses_devel_repo") - def test_handle_createrepo_failure_1(self, mc_devel, mc_call, mc_time): + def test_handle_createrepo_failure_1(self, mc_call, mc_time): tmp_dir = self.make_temp_dir() mc_call.return_value = 0 # failure @@ -894,9 +882,7 @@ def test_request_exception_is_taken_care_of_when_posting_to_frontend(self, mc_ti except Exception as e: assert False - @mock.patch("copr_backend.actions.uses_devel_repo") - def test_delete_chroot(self, mc_devel, mc_time): - mc_devel.return_value = False + def test_delete_chroot(self, mc_time): mc_time.time.return_value = self.test_time mc_front_cb = MagicMock() From 286a9136b071583be90ca1342a43010a4116bb31 Mon Sep 17 00:00:00 2001 From: Jakub Kadlcik Date: Tue, 10 Sep 2024 08:36:23 +0200 Subject: [PATCH 05/18] backend: add a timeout for waiting until a Pulp task finishes --- backend/copr_backend/pulp.py | 5 ++++- backend/copr_backend/storage.py | 13 +++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/backend/copr_backend/pulp.py b/backend/copr_backend/pulp.py index 6dc19042b..3a93a1765 100644 --- a/backend/copr_backend/pulp.py +++ b/backend/copr_backend/pulp.py @@ -190,18 +190,21 @@ def delete_distribution(self, distribution): url = self.config["base_url"] + distribution return requests.delete(url, **self.request_params) - def wait_for_finished_task(self, task): + def wait_for_finished_task(self, task, timeout=86400): """ Pulp task (e.g. creating a publication) can be running for an unpredictably long time. We need to wait until it is finished to know what it actually did. """ + start = time.time() while True: response = self.get_task(task) if not response.ok: break if response.json()["state"] not in ["waiting", "running"]: break + if time.time() > start + timeout: + break time.sleep(5) return response diff --git a/backend/copr_backend/storage.py b/backend/copr_backend/storage.py index fdb683815..1d816b6c1 100644 --- a/backend/copr_backend/storage.py +++ b/backend/copr_backend/storage.py @@ -8,6 +8,7 @@ from copr_common.enums import StorageEnum from copr_backend.helpers import call_copr_repo, build_chroot_log_name from copr_backend.pulp import PulpClient +from copr_backend.exceptions import CoprBackendError def storage_for_job(job, opts, log): @@ -231,7 +232,10 @@ def upload_build_results(self, chroot, results_dir, target_dir_name): # creating the `pulp.json` file task = response.json()["task"] response = self.client.wait_for_finished_task(task) - created = response.json()["created_resources"] + created = response.json().get("created_resources") + if not created: + raise CoprBackendError( + "Pulp task {0} didn't create any resources".format(task)) resources.extend(created) self.log.info("Uploaded to Pulp: %s", path) @@ -257,7 +261,12 @@ def publish_repository(self, chroot, **kwargs): task, response.text) return False - publication = response.json()["created_resources"][0] + resources = response.json()["created_resources"] + if not resources: + raise CoprBackendError( + "Pulp task {0} didn't create any resources".format(task)) + + publication = resources[0] distribution_name = self._distribution_name(chroot) distribution = self._get_distribution(chroot) From ad36b8b95ba7822e61da1c6b985ed23317e9a95b Mon Sep 17 00:00:00 2001 From: Jakub Kadlcik Date: Thu, 5 Sep 2024 10:36:46 +0200 Subject: [PATCH 06/18] frontend: make the default storage for new projects configurable See #2533 --- frontend/coprs_frontend/coprs/config.py | 4 ++++ frontend/coprs_frontend/coprs/logic/coprs_logic.py | 8 +++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/frontend/coprs_frontend/coprs/config.py b/frontend/coprs_frontend/coprs/config.py index 9797d43c8..2ff757e5c 100644 --- a/frontend/coprs_frontend/coprs/config.py +++ b/frontend/coprs_frontend/coprs/config.py @@ -201,6 +201,10 @@ class Config(object): ROLLING_CHROOTS_INACTIVITY_WARNING = 180 ROLLING_CHROOTS_INACTIVITY_REMOVAL = 180 + # What storage should be set for new projects. + # Possible options are "backend" and "pulp" + DEFAULT_STORAGE = "backend" + class ProductionConfig(Config): DEBUG = False diff --git a/frontend/coprs_frontend/coprs/logic/coprs_logic.py b/frontend/coprs_frontend/coprs/logic/coprs_logic.py index 17c00ec70..2922297ea 100644 --- a/frontend/coprs_frontend/coprs/logic/coprs_logic.py +++ b/frontend/coprs_frontend/coprs/logic/coprs_logic.py @@ -17,7 +17,12 @@ from sqlalchemy.orm.exc import NoResultFound from sqlalchemy.orm.attributes import get_history -from copr_common.enums import ActionTypeEnum, BackendResultEnum, ActionPriorityEnum +from copr_common.enums import ( + ActionTypeEnum, + BackendResultEnum, + ActionPriorityEnum, + StorageEnum, +) from coprs import app, db from coprs import exceptions from coprs import helpers @@ -310,6 +315,7 @@ def add(cls, user, name, selected_chroots, repos=None, description=None, isolation=isolation, follow_fedora_branching=follow_fedora_branching, appstream=appstream, + storage=StorageEnum(app.config["DEFAULT_STORAGE"]), **kwargs) From 14faa50b52406082a739b44308e1718c5ec69731 Mon Sep 17 00:00:00 2001 From: Jakub Kadlcik Date: Thu, 5 Sep 2024 11:10:15 +0200 Subject: [PATCH 07/18] frontend, python, cli: allow admins to set storage for new projects See #2533 This will be useful for beaker tests where we can now add basic tests for every supported storage. --- cli/copr_cli/main.py | 8 ++++++++ cli/tests/test_cli.py | 2 ++ frontend/coprs_frontend/coprs/forms.py | 6 ++++++ frontend/coprs_frontend/coprs/logic/coprs_logic.py | 8 ++++++-- .../coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py | 1 + .../coprs_frontend/coprs/views/apiv3_ns/schema/schemas.py | 7 +++++++ frontend/coprs_frontend/tests/test_apiv3/test_projects.py | 5 +---- python/copr/v3/proxies/project.py | 4 +++- 8 files changed, 34 insertions(+), 7 deletions(-) diff --git a/cli/copr_cli/main.py b/cli/copr_cli/main.py index 605cf5cc8..54348cf85 100644 --- a/cli/copr_cli/main.py +++ b/cli/copr_cli/main.py @@ -485,6 +485,7 @@ def action_create(self, args): runtime_dependencies=args.runtime_dependencies, packit_forge_projects_allowed=args.packit_forge_projects_allowed, repo_priority=args.repo_priority, + storage=args.storage, ) owner_part = username.replace('@', "g/") @@ -1192,6 +1193,13 @@ def setup_parser(): help=("Use the priority= config option for repositories in this " "project, see man dnf.conf(5) for more info.")) + parser_create.add_argument( + "--storage", + choices=["backend", "pulp"], + help=("What storage should be used for this project. " + "This option can only be specified by a COPR admin.") + ) + create_and_modify_common_opts(parser_create) parser_create.set_defaults(func="action_create") diff --git a/cli/tests/test_cli.py b/cli/tests/test_cli.py index 98b7a70c7..3a98fe174 100644 --- a/cli/tests/test_cli.py +++ b/cli/tests/test_cli.py @@ -480,6 +480,7 @@ def test_create_project(config_from_file, project_proxy_add, capsys): "runtime_dependencies": None, "packit_forge_projects_allowed": None, "repo_priority": None, + "storage": None, } assert stdout == "New project was successfully created: http://copr/coprs/jdoe/foo/\n" @@ -577,6 +578,7 @@ def test_create_multilib_project(config_from_file, project_proxy_add, capsys): "runtime_dependencies": None, "packit_forge_projects_allowed": None, "repo_priority": None, + "storage": None, } assert stdout == "New project was successfully created: http://copr/coprs/jdoe/foo/\n" diff --git a/frontend/coprs_frontend/coprs/forms.py b/frontend/coprs_frontend/coprs/forms.py index cb5b457a3..894a86e29 100644 --- a/frontend/coprs_frontend/coprs/forms.py +++ b/frontend/coprs_frontend/coprs/forms.py @@ -676,6 +676,12 @@ class CoprForm(BaseForm): default=None, ) + storage = wtforms.SelectField( + "Admin only - what storage should be used for this project", + choices=[(x, x) for x in ["backend", "pulp"]], + validators=[wtforms.validators.Optional()], + ) + @property def errors(self): """ diff --git a/frontend/coprs_frontend/coprs/logic/coprs_logic.py b/frontend/coprs_frontend/coprs/logic/coprs_logic.py index 2922297ea..ed804f92b 100644 --- a/frontend/coprs_frontend/coprs/logic/coprs_logic.py +++ b/frontend/coprs_frontend/coprs/logic/coprs_logic.py @@ -287,7 +287,7 @@ def get_multiple_fulltext(cls, fulltext=None, projectname=None, def add(cls, user, name, selected_chroots, repos=None, description=None, instructions=None, check_for_duplicates=False, group=None, persistent=False, auto_prune=True, bootstrap=None, follow_fedora_branching=False, isolation=None, - appstream=False, **kwargs): + appstream=False, storage=None, **kwargs): if not flask.g.user.admin and flask.g.user != user: msg = ("You were authorized as '{0}' user without permissions to access " @@ -300,6 +300,10 @@ def add(cls, user, name, selected_chroots, repos=None, description=None, if not flask.g.user.admin and not auto_prune: raise exceptions.NonAdminCannotDisableAutoPrunning() + if not flask.g.user.admin and storage: + raise exceptions.AccessRestricted("Non-admin cannot set storage") + storage = StorageEnum(storage or app.config["DEFAULT_STORAGE"]) + # form validation checks for duplicates cls.new(user, name, group, check_for_duplicates=check_for_duplicates) @@ -315,7 +319,7 @@ def add(cls, user, name, selected_chroots, repos=None, description=None, isolation=isolation, follow_fedora_branching=follow_fedora_branching, appstream=appstream, - storage=StorageEnum(app.config["DEFAULT_STORAGE"]), + storage=storage, **kwargs) diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py index 681016197..2084d0b1c 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py @@ -262,6 +262,7 @@ def _form_field_repos(form_field): form.packit_forge_projects_allowed ), repo_priority=form.repo_priority.data, + storage=form.storage.data, ) db.session.commit() except ( diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/schemas.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/schemas.py index cbb68bfd2..0a5eb706c 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/schemas.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/schemas.py @@ -477,6 +477,13 @@ class _ProjectGetAddFields: description="Build and project is immune against deletion", ) additional_repos: List = fields.additional_repos + storage: String = String( + description=( + "Admin only - what storage should be used for this project. " + "Possible values are 'backend' or 'pulp'." + ), + ) + @dataclass diff --git a/frontend/coprs_frontend/tests/test_apiv3/test_projects.py b/frontend/coprs_frontend/tests/test_apiv3/test_projects.py index 89779c3d3..34ecf7927 100644 --- a/frontend/coprs_frontend/tests/test_apiv3/test_projects.py +++ b/frontend/coprs_frontend/tests/test_apiv3/test_projects.py @@ -5,7 +5,6 @@ import pytest -from copr_common.enums import StorageEnum from coprs.models import User, Copr from tests.coprs_test_case import CoprsTestCase, TransactionDecorator @@ -153,8 +152,6 @@ def test_update_copr_api3(self): }, { }, { "appstream": True, - }, { - "storage": StorageEnum("backend"), }] for setup in easy_changes: @@ -168,7 +165,7 @@ def test_update_copr_api3(self): "created_on", "deleted", "scm_api_auth_json", "scm_api_type", "scm_repo_url", "id", "name", "user_id", "group_id", "webhook_secret", "forked_from_id", "latest_indexed_data_update", - "copr_id", "persistent", "playground", + "copr_id", "persistent", "playground", "storage", ]: should_test.remove(item) diff --git a/python/copr/v3/proxies/project.py b/python/copr/v3/proxies/project.py index ece6d13cc..69ba81d5f 100644 --- a/python/copr/v3/proxies/project.py +++ b/python/copr/v3/proxies/project.py @@ -72,7 +72,7 @@ def add(self, ownername, projectname, chroots, description=None, instructions=No delete_after_days=None, multilib=False, module_hotfixes=False, bootstrap=None, bootstrap_image=None, isolation=None, follow_fedora_branching=True, fedora_review=None, appstream=False, runtime_dependencies=None, packit_forge_projects_allowed=None, - repo_priority=None, exist_ok=False): + repo_priority=None, exist_ok=False, storage=None): """ Create a project @@ -110,6 +110,7 @@ def add(self, ownername, projectname, chroots, description=None, instructions=No enabled together with this project repository. :param list packit_forge_projects_allowed: List of forge projects that will be allowed to build in the project via Packit + :param str storage: Admin only - What storage should be used for this project :return: Munch """ endpoint = "/project/add/{ownername}" @@ -142,6 +143,7 @@ def add(self, ownername, projectname, chroots, description=None, instructions=No "runtime_dependencies": runtime_dependencies, "packit_forge_projects_allowed": packit_forge_projects_allowed, "repo_priority": repo_priority, + "storage": storage, } _compat_use_bootstrap_container(data, use_bootstrap_container) From 6aeb686190ab8b6193a0701357be48c87bba39b4 Mon Sep 17 00:00:00 2001 From: Jiri Kyjovsky Date: Mon, 16 Sep 2024 10:15:37 +0200 Subject: [PATCH 08/18] docker: set hard ulimits for docker container Because of bug in python3-daemon [1] we need to set ulimits inside docker container, otherwise backend and dist-git ooms. [1] - https://bugzilla.redhat.com/show_bug.cgi?id=2307635 --- docker-compose.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docker-compose.yaml b/docker-compose.yaml index 6aca9c1a3..b717b09d2 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -14,6 +14,17 @@ # 5009:5432 -- PostgreSQL database version: '3' + +# this should be dolved by python3-daemon >= 3.0.0 +# https://bugzilla.redhat.com/show_bug.cgi?id=2307635 +x-ulimits: &ulimits_settings + ulimits: + nproc: 65535 + nofile: + soft: 32767 + hard: 65535 + + services: # @TODO Probably not all backend services should use the same Dockerfile @@ -30,6 +41,7 @@ services: volumes: - .:/opt/copr:z - results:/var/lib/copr/public_html/results:z + <<: *ulimits_settings backend-build: build: @@ -43,6 +55,7 @@ services: volumes: - .:/opt/copr:z - results:/var/lib/copr/public_html/results:z + <<: *ulimits_settings backend-action: build: @@ -56,6 +69,7 @@ services: volumes: - .:/opt/copr:z - results:/var/lib/copr/public_html/results:z + <<: *ulimits_settings resalloc: build: @@ -147,6 +161,7 @@ services: volumes: - .:/opt/copr:z - dist-git:/var/lib/dist-git:z + <<: *ulimits_settings distgit-httpd: build: @@ -160,6 +175,7 @@ services: - .:/opt/copr:z - dist-git:/var/lib/dist-git:z command: /usr/sbin/httpd -DFOREGROUND + <<: *ulimits_settings keygen-signd: build: From 446dcb3739ef436ab1500bd3e7891001d7508ba8 Mon Sep 17 00:00:00 2001 From: Jiri Kyjovsky Date: Wed, 18 Sep 2024 16:49:52 +0200 Subject: [PATCH 09/18] beaker: use podman for testing inside container if installed --- beaker-tests/DockerTestEnv/Makefile | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/beaker-tests/DockerTestEnv/Makefile b/beaker-tests/DockerTestEnv/Makefile index 8fd770832..d2c60cb27 100644 --- a/beaker-tests/DockerTestEnv/Makefile +++ b/beaker-tests/DockerTestEnv/Makefile @@ -3,14 +3,15 @@ mkfile_path:=$(shell dirname $(realpath $(lastword $(MAKEFILE_LIST)))) CONTAINER_NAME ?= test-env +CONTAINER_ENGINE ?= $(shell command -v podman 2> /dev/null || echo docker) build: - docker build -t test-env-image . + $(CONTAINER_ENGINE) build -t test-env-image . bld: build run: - docker run \ + $(CONTAINER_ENGINE) run \ -dit \ -v $(mkfile_path)/../..:/root/copr \ --name="$(CONTAINER_NAME)" \ @@ -19,14 +20,14 @@ run: test-env-image sh: - docker exec $(CONTAINER_NAME) rm -f /run/nologin - docker exec -u root -it $(CONTAINER_NAME) script -qc 'bash' /dev/null + $(CONTAINER_ENGINE) exec $(CONTAINER_NAME) rm -f /run/nologin + $(CONTAINER_ENGINE) exec -u root -it $(CONTAINER_NAME) script -qc 'bash' /dev/null start: - docker start $(CONTAINER_NAME) + $(CONTAINER_ENGINE) start $(CONTAINER_NAME) stop: - docker stop $(CONTAINER_NAME) + $(CONTAINER_ENGINE) stop $(CONTAINER_NAME) del: - docker rm -f $(CONTAINER_NAME) + $(CONTAINER_ENGINE) rm -f $(CONTAINER_NAME) From 609d36988fb307193c77c872b726323bfca0647b Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Fri, 20 Sep 2024 14:14:31 +0200 Subject: [PATCH 10/18] frontend: fix the 500 for racy creation attempts This is TOCTOU issue. The other checks for duplications (on so many places) seem kinda redundant because nothing but try/except for commit() may catch these concurrency problems. Fixes: #3372 --- .../coprs/views/apiv3_ns/apiv3_projects.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py index 2084d0b1c..70ae60a9b 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py @@ -5,6 +5,7 @@ import flask from flask_restx import Namespace, Resource +from sqlalchemy.exc import IntegrityError from coprs.views.apiv3_ns import ( get_copr, @@ -15,7 +16,7 @@ editable_copr, ) from coprs.views.apiv3_ns.json2form import get_form_compatible_data, get_input_dict -from coprs import db, models, forms, db_session_scope +from coprs import app, db, models, forms, db_session_scope from coprs.views.misc import api_login_required from coprs.views.apiv3_ns import rename_fields_helper, api, query_to_parameters from coprs.views.apiv3_ns.schema.schemas import ( @@ -228,13 +229,14 @@ def post(self, ownername, exist_ok=False): if form.bootstrap.data is not None: bootstrap = form.bootstrap.data + projectname = form.name.data.strip() try: def _form_field_repos(form_field): return " ".join(form_field.data.split()) copr = CoprsLogic.add( - name=form.name.data.strip(), + name=projectname, repos=_form_field_repos(form.repos), user=user, selected_chroots=form.selected_chroots, @@ -265,6 +267,16 @@ def _form_field_repos(form_field): storage=form.storage.data, ) db.session.commit() + except IntegrityError as ierr: + app.log.debug("Racy attempt to create %s/%s", ownername, projectname) + db.session.rollback() + if exist_ok: + copr = get_copr(ownername, projectname) + return to_dict(copr) + raise DuplicateException( + f"Copr '{ownername}/{projectname}' has not been created " + "(race condition)" + ) from ierr except ( DuplicateException, NonAdminCannotCreatePersistentProject, From c1927266d16f277e8ccb96c3087b0776ea6dc0a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Bompard?= Date: Mon, 29 Jul 2024 10:38:34 +0200 Subject: [PATCH 11/18] Message schemas: one-line descriptions should be the summary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Aurélien Bompard --- messaging/copr_messaging/private/hierarchy.py | 3 ++- messaging/copr_messaging/schema.py | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/messaging/copr_messaging/private/hierarchy.py b/messaging/copr_messaging/private/hierarchy.py index 4d792cc85..c846fa6ec 100644 --- a/messaging/copr_messaging/private/hierarchy.py +++ b/messaging/copr_messaging/private/hierarchy.py @@ -33,7 +33,8 @@ def __init__(self, *args, **kwargs): """ Base class that all Copr messages should inherit from. """ - def __str__(self): + def summary(self): + """A one-line, human-readable representation of this message.""" return "Unspecified Copr message" def _str_prefix(self): diff --git a/messaging/copr_messaging/schema.py b/messaging/copr_messaging/schema.py index 6a61beac5..ab8b18023 100644 --- a/messaging/copr_messaging/schema.py +++ b/messaging/copr_messaging/schema.py @@ -39,7 +39,8 @@ def status(self): """ raise NotImplementedError - def __str__(self): + def summary(self): + """A one-line, human-readable representation of this message.""" return '{0}: chroot "{1}" ended as "{2}".'.format( super(BuildChrootEnded, self)._str_prefix(), self.chroot, @@ -61,7 +62,8 @@ class BuildChrootStarted(_BuildChrootMessage): Representation of a message sent by Copr build system right before some Copr worker starts working on a build in a particular mock chroot. """ - def __str__(self): + def summary(self): + """A one-line, human-readable representation of this message.""" return '{0}: chroot "{1}" started.'.format( super(BuildChrootStarted, self)._str_prefix(), self.chroot, From f3b16431f32f26d0f59617ee799d1fb37c243ab2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Bompard?= Date: Mon, 29 Jul 2024 10:40:13 +0200 Subject: [PATCH 12/18] Message schemas: set chroot message severity to DEBUG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Aurélien Bompard --- messaging/copr_messaging/schema.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/messaging/copr_messaging/schema.py b/messaging/copr_messaging/schema.py index ab8b18023..67f90449d 100644 --- a/messaging/copr_messaging/schema.py +++ b/messaging/copr_messaging/schema.py @@ -20,6 +20,8 @@ import copy +from fedora_messaging import message + from copr_common.enums import StatusEnum from .private.hierarchy import _BuildChrootMessage, _CoprMessage @@ -100,6 +102,12 @@ class BuildChrootStartedV1DontUse(_PreFMBuildMessage, BuildChrootStarted): """ topic = 'copr.chroot.start' + # Set the chroot message severity to DEBUG, which will not generate a notification in FMN by + # default. Those are always paired with a build message, so it makes more sense to notify on + # that one. + # Ref: https://fedora-messaging.readthedocs.io/en/stable/user-guide/messages.html#useful-accessors + severity = message.DEBUG + class BuildChrootStartedV1Stomp(schema_stomp_old._OldStompChrootMessage, BuildChrootStarted): From 1559b85066fb3f4ee3f975f43820086ae33084ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Bompard?= Date: Mon, 29 Jul 2024 10:54:02 +0200 Subject: [PATCH 13/18] Use `super()` without argument to make pylint happy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Aurélien Bompard --- messaging/copr_messaging/private/hierarchy.py | 6 +++--- messaging/copr_messaging/schema.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/messaging/copr_messaging/private/hierarchy.py b/messaging/copr_messaging/private/hierarchy.py index c846fa6ec..061f90052 100644 --- a/messaging/copr_messaging/private/hierarchy.py +++ b/messaging/copr_messaging/private/hierarchy.py @@ -28,7 +28,7 @@ def __init__(self, *args, **kwargs): body = body['msg'] kwargs['body'] = body - super(_CoprMessage, self).__init__(*args, **kwargs) + super().__init__(*args, **kwargs) """ Base class that all Copr messages should inherit from. @@ -54,7 +54,7 @@ def app_name(self): class _CoprProjectMessage(_CoprMessage): def _str_prefix(self): return '{0} in project "{1}"'.format( - super(_CoprProjectMessage, self)._str_prefix(), + super()._str_prefix(), self.project_full_name, ) @@ -85,7 +85,7 @@ def project_full_name(self): class _BuildMessage(_CoprProjectMessage): def _str_prefix(self): return '{0}: build {1}'.format( - super(_BuildMessage, self)._str_prefix(), + super()._str_prefix(), self.build_id, ) diff --git a/messaging/copr_messaging/schema.py b/messaging/copr_messaging/schema.py index 67f90449d..c381a0e38 100644 --- a/messaging/copr_messaging/schema.py +++ b/messaging/copr_messaging/schema.py @@ -44,7 +44,7 @@ def status(self): def summary(self): """A one-line, human-readable representation of this message.""" return '{0}: chroot "{1}" ended as "{2}".'.format( - super(BuildChrootEnded, self)._str_prefix(), + super()._str_prefix(), self.chroot, self.status, ) @@ -67,7 +67,7 @@ class BuildChrootStarted(_BuildChrootMessage): def summary(self): """A one-line, human-readable representation of this message.""" return '{0}: chroot "{1}" started.'.format( - super(BuildChrootStarted, self)._str_prefix(), + super()._str_prefix(), self.chroot, ) From 6ae7c6c584e47139e13313b63e5c01b6179258bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Such=C3=BD?= Date: Mon, 23 Sep 2024 22:04:11 +0200 Subject: [PATCH 14/18] rpmbuild: do not require qemu-user-static on rhel Resolves: RHBZ#2313879 --- rpmbuild/copr-rpmbuild.spec | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rpmbuild/copr-rpmbuild.spec b/rpmbuild/copr-rpmbuild.spec index 605e90da9..0917d9950 100644 --- a/rpmbuild/copr-rpmbuild.spec +++ b/rpmbuild/copr-rpmbuild.spec @@ -63,7 +63,9 @@ Requires: git Requires: git-svn # for the /bin/unbuffer binary Requires: expect -%if !0%{?openEuler} +%if 0%{?openEuler} > 0 || 0%{?rhel} > 0 +# qemu-user-static is not supported +%else Requires: qemu-user-static %endif Requires: sed From d6a3472e34ab8a6f2cc2418b2754acd5eee80fcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Such=C3=BD?= Date: Mon, 23 Sep 2024 22:12:43 +0200 Subject: [PATCH 15/18] rpmbuild: do not require rpkg,pyp2rpm,pyp2spec,gem2rpm and fedora-review on rhel Resolves: RHBZ#2313878 --- rpmbuild/copr-rpmbuild.spec | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rpmbuild/copr-rpmbuild.spec b/rpmbuild/copr-rpmbuild.spec index 0917d9950..291edf1f7 100644 --- a/rpmbuild/copr-rpmbuild.spec +++ b/rpmbuild/copr-rpmbuild.spec @@ -106,7 +106,9 @@ Requires: nosync %endif Requires: openssh-clients Requires: podman -%if !0%{?openEuler} +%if 0%{?openEuler} > 0 || 0%{?rhel} > 0 +# not supported +%else Requires: pyp2rpm Requires: pyp2spec Requires: rubygem-gem2rpm From 658230d0f6287eed51ee3249c0b7027175ccdb5a Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Mon, 23 Sep 2024 15:20:09 +0200 Subject: [PATCH 16/18] backend: unknown resalloc tickets helper cleanup If no tickets are taken (which often happens in the staging environment), this script encountered corner case issues. --- .../copr-backend-unknown-resalloc-tickets.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/backend/run/copr-backend-unknown-resalloc-tickets.py b/backend/run/copr-backend-unknown-resalloc-tickets.py index b7d8df60d..15ba839e1 100755 --- a/backend/run/copr-backend-unknown-resalloc-tickets.py +++ b/backend/run/copr-backend-unknown-resalloc-tickets.py @@ -22,7 +22,8 @@ def used_ids(): """ - Return a set of ticket_ids that can be found in the "ps aux" output. + Return a `set()` of ticket IDs currently in use by + `copr-backend-process-build`, based on the output of the `ps aux` command. """ cmd = ( "ps aux | " @@ -33,7 +34,9 @@ def used_ids(): output = output.strip() tickets = set() for ticket_id in output.split("\n"): - assert all(c.isdigit() for c in ticket_id) + if ticket_id: # ignore empty lines + continue + assert ticket_id.isdigit() tickets.add(int(ticket_id)) return tickets @@ -61,11 +64,13 @@ def print_once(message): print(message) storage[message] = True - -if __name__ == "__main__": +def _main(): script_requires_user("resalloc") used = used_ids() + if not used: + print("no tickets used") + return # This is the oldest ticket that Copr Backend currently uses. min_used = min(used) @@ -80,3 +85,7 @@ def print_once(message): else: print_once("These tickets are relatively new for closing blindly, double check!") print(f"resalloc ticket-check {unknown_id}") + + +if __name__ == "__main__": + _main() From 5d77d36198ef74aff87468c36960fd03e1d74eff Mon Sep 17 00:00:00 2001 From: Jiri Kyjovsky Date: Tue, 17 Sep 2024 14:35:28 +0200 Subject: [PATCH 17/18] rpmbuild: specify snippets to mock config via copr-rpmbuild config file This allows us to specify tpm fs size to rpmbuild in order to be able to automatically generate its size for performance builders. See #3268 --- rpmbuild/copr-rpmbuild.spec | 4 +++ rpmbuild/copr-rpmbuild.yml | 14 +++++++++++ rpmbuild/copr_rpmbuild/builders/mock.py | 11 +++++++- rpmbuild/copr_rpmbuild/config.py | 29 ++++++++++++++++++++++ rpmbuild/copr_rpmbuild/helpers.py | 17 +++++++++++++ rpmbuild/copr_rpmbuild/providers/base.py | 15 +++++++++-- rpmbuild/copr_rpmbuild/providers/custom.py | 1 + rpmbuild/mock-custom-build.cfg.j2 | 3 +++ rpmbuild/mock.cfg.j2 | 3 +++ rpmbuild/tests/test_mock.py | 27 ++++++++++++++++++++ 10 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 rpmbuild/copr-rpmbuild.yml create mode 100644 rpmbuild/copr_rpmbuild/config.py diff --git a/rpmbuild/copr-rpmbuild.spec b/rpmbuild/copr-rpmbuild.spec index 291edf1f7..95ad2701e 100644 --- a/rpmbuild/copr-rpmbuild.spec +++ b/rpmbuild/copr-rpmbuild.spec @@ -40,6 +40,7 @@ BuildRequires: %{python}-requests BuildRequires: %{python_pfx}-jinja2 BuildRequires: %{python_pfx}-specfile >= 0.21.0 BuildRequires: python3-backoff >= 1.9.0 +BuildRequires: python3-pyyaml BuildRequires: /usr/bin/argparse-manpage BuildRequires: python-rpm-macros @@ -57,6 +58,7 @@ Requires: %{python_pfx}-munch Requires: %{python}-requests Requires: %{python_pfx}-specfile >= 0.21.0 Requires: python3-backoff >= 1.9.0 +Requires: python3-pyyaml Requires: mock >= 5.0 Requires: git @@ -217,6 +219,7 @@ install -m 644 mock.cfg.j2 %{buildroot}%{_sysconfdir}/copr-rpmbuild/mock.cfg.j2 install -m 644 rpkg.conf.j2 %{buildroot}%{_sysconfdir}/copr-rpmbuild/rpkg.conf.j2 install -m 644 mock-source-build.cfg.j2 %{buildroot}%{_sysconfdir}/copr-rpmbuild/ install -m 644 mock-custom-build.cfg.j2 %{buildroot}%{_sysconfdir}/copr-rpmbuild/ +install -m 644 copr-rpmbuild.yml %{buildroot}%{_sysconfdir}/copr-rpmbuild/copr-rpmbuild.yml cat < %buildroot%mock_config_overrides/README Contents of this directory is used by %_bindir/copr-update-builder script. @@ -268,6 +271,7 @@ install -p -m 755 copr-update-builder %buildroot%_bindir %config(noreplace) %{_sysconfdir}/copr-rpmbuild/rpkg.conf.j2 %config(noreplace) %{_sysconfdir}/copr-rpmbuild/mock-source-build.cfg.j2 %config(noreplace) %{_sysconfdir}/copr-rpmbuild/mock-custom-build.cfg.j2 +%config(noreplace) %{_sysconfdir}/copr-rpmbuild/copr-rpmbuild.yml %files -n copr-builder %license LICENSE diff --git a/rpmbuild/copr-rpmbuild.yml b/rpmbuild/copr-rpmbuild.yml new file mode 100644 index 000000000..fcfffc104 --- /dev/null +++ b/rpmbuild/copr-rpmbuild.yml @@ -0,0 +1,14 @@ +--- +# Configure special mock configuration snippets per given set of tags. +# tags_to_mock_snippet: +# - tagset: +# - on_demand_powerful +# - arch_x86_64 +# snippet: config_opts['plugin_conf']['tmpfs_opts']['max_fs_size'] = '280g' +# - tagset: +# - on_demand_powerful +# - something +# snippet: | +# cute +# multiline +# snippet diff --git a/rpmbuild/copr_rpmbuild/builders/mock.py b/rpmbuild/copr_rpmbuild/builders/mock.py index b3cc31461..04a6a6abd 100644 --- a/rpmbuild/copr_rpmbuild/builders/mock.py +++ b/rpmbuild/copr_rpmbuild/builders/mock.py @@ -6,12 +6,14 @@ import subprocess from jinja2 import Environment, FileSystemLoader -from ..helpers import ( +from copr_rpmbuild.config import Config +from copr_rpmbuild.helpers import ( locate_spec, CONF_DIRS, get_mock_uniqueext, GentlyTimeoutedPopen, macros_for_task, + mock_snippet_for_tags, ) log = logging.getLogger("__main__") @@ -42,6 +44,10 @@ def __init__(self, task, sourcedir, resultdir, config): self.macros = macros_for_task(task, config) self.uniqueext = get_mock_uniqueext() self.allow_user_ssh = task.get("allow_user_ssh") + self.tags = task.get("tags", []) + + self.copr_rpmbuild_config = Config() + self.copr_rpmbuild_config.load_config() def run(self): open(self.logfile, 'w').close() # truncate logfile @@ -82,6 +88,9 @@ def render_config_template(self): copr_build_id=self.build_id, isolation=self.isolation, macros=self.macros, + mock_snippet=mock_snippet_for_tags( + self.copr_rpmbuild_config.tags_to_mock_snippet, self.tags + ), ) def produce_srpm(self, spec, sources, resultdir): diff --git a/rpmbuild/copr_rpmbuild/config.py b/rpmbuild/copr_rpmbuild/config.py new file mode 100644 index 000000000..ebdcee715 --- /dev/null +++ b/rpmbuild/copr_rpmbuild/config.py @@ -0,0 +1,29 @@ +""" +Configuration class for copr-rpmbuild +""" + +import yaml + + +CONFIG_PATH = "/etc/copr-rpmbuild/copr-rpmbuild.yml" + + +class Config: + """ + Configuration class for copr-rpmbuild + """ + def __init__(self): + self.tags_to_mock_snippet = [] + + def load_config(self): + """ + Load configuration from the config file + """ + config_data = {} + try: + with open(CONFIG_PATH, "r", encoding="utf-8") as file: + config_data = yaml.safe_load(file) or {} + except FileNotFoundError: + pass + + self.tags_to_mock_snippet = config_data.get("tags_to_mock_snippet", []) diff --git a/rpmbuild/copr_rpmbuild/helpers.py b/rpmbuild/copr_rpmbuild/helpers.py index f18cba23f..8ff30b0b9 100644 --- a/rpmbuild/copr_rpmbuild/helpers.py +++ b/rpmbuild/copr_rpmbuild/helpers.py @@ -469,3 +469,20 @@ def package_version(name): return pkg_resources.require(name)[0].version except pkg_resources.DistributionNotFound: return "git" + + +def mock_snippet_for_tags(tags_to_mock_snippet, tags): + """ + Return mock snippets as string separated by newlines for a given + list of tags. + """ + if not tags or not tags_to_mock_snippet: + return "" + + tags_set = set(tags) + snippets = [] + for item in tags_to_mock_snippet: + if set(item["tagset"]).issubset(tags_set): + snippets.append(item["snippet"]) + + return "\n".join(snippets) diff --git a/rpmbuild/copr_rpmbuild/providers/base.py b/rpmbuild/copr_rpmbuild/providers/base.py index 8250e4036..b08b6241b 100644 --- a/rpmbuild/copr_rpmbuild/providers/base.py +++ b/rpmbuild/copr_rpmbuild/providers/base.py @@ -8,7 +8,8 @@ from copr_common.request import SafeRequest from copr_rpmbuild.helpers import CONF_DIRS -from copr_rpmbuild.helpers import run_cmd +from copr_rpmbuild.helpers import run_cmd, mock_snippet_for_tags +from copr_rpmbuild.config import Config log = logging.getLogger("__main__") @@ -51,6 +52,16 @@ def __init__(self, source_dict, config, macros=None, task=None): if e.errno != errno.EEXIST: raise + self.copr_rpmbuild_config = Config() + self.copr_rpmbuild_config.load_config() + + # Mock snippets to render in the mock config + self.mock_snippet = "" + if self.task is not None: + self.mock_snippet = mock_snippet_for_tags( + self.copr_rpmbuild_config.tags_to_mock_snippet, self.task.get("tags") + ) + # Change home directory to workdir and create .rpmmacros there os.environ["HOME"] = self.workdir self.create_rpmmacros() @@ -130,7 +141,7 @@ def render_mock_config_template(self, template_name): """ jinja_env = Environment(loader=FileSystemLoader(CONF_DIRS)) template = jinja_env.get_template(template_name) - return template.render(macros=self.macros) + return template.render(macros=self.macros, mock_snippet=self.mock_snippet) def produce_srpm(self): """ diff --git a/rpmbuild/copr_rpmbuild/providers/custom.py b/rpmbuild/copr_rpmbuild/providers/custom.py index ce5469a62..2bf93cb9e 100644 --- a/rpmbuild/copr_rpmbuild/providers/custom.py +++ b/rpmbuild/copr_rpmbuild/providers/custom.py @@ -51,6 +51,7 @@ def render_mock_config_template(self, *_args): chroot=self.chroot, repos=self.repos, macros=self.macros, + mock_snippet=self.mock_snippet, ) def produce_srpm(self): diff --git a/rpmbuild/mock-custom-build.cfg.j2 b/rpmbuild/mock-custom-build.cfg.j2 index c71e85609..1c6d98615 100644 --- a/rpmbuild/mock-custom-build.cfg.j2 +++ b/rpmbuild/mock-custom-build.cfg.j2 @@ -16,6 +16,9 @@ config_opts["root"] = "copr-custom-" + config_opts["root"] # /bin/mock calls (when tmpfs_enable is on). config_opts['plugin_conf']['tmpfs_opts']['keep_mounted'] = True +# Custom mock snippets configured in copr-crpmbuild config file - can be empty +{{ mock_snippet }} + {%- for key, value in macros.items() %} config_opts['macros']['{{ key }}'] = '{{ value }}' {%- endfor %} diff --git a/rpmbuild/mock.cfg.j2 b/rpmbuild/mock.cfg.j2 index 25f3bbf19..c0010056c 100644 --- a/rpmbuild/mock.cfg.j2 +++ b/rpmbuild/mock.cfg.j2 @@ -4,6 +4,9 @@ config_opts.setdefault('plugin_conf', {}) config_opts['plugin_conf'].setdefault('tmpfs_opts', {}) config_opts['plugin_conf']['tmpfs_opts']['keep_mounted'] = True +# Custom mock snippets configured in copr-crpmbuild config file - can be empty +{{ mock_snippet }} + {% if buildroot_pkgs %} config_opts['chroot_additional_packages'] = '{{ buildroot_pkgs| join(" ") }}' {% endif %} diff --git a/rpmbuild/tests/test_mock.py b/rpmbuild/tests/test_mock.py index fe43e0bda..11c41f193 100644 --- a/rpmbuild/tests/test_mock.py +++ b/rpmbuild/tests/test_mock.py @@ -147,6 +147,9 @@ def test_mock_config(self, call, f_mock_calls): config_opts['plugin_conf'].setdefault('tmpfs_opts', {}) config_opts['plugin_conf']['tmpfs_opts']['keep_mounted'] = True +# Custom mock snippets configured in copr-crpmbuild config file - can be empty + + config_opts['chroot_additional_packages'] = 'pkg1 pkg2 pkg3' @@ -164,6 +167,30 @@ def test_mock_config(self, call, f_mock_calls): """ # TODO: make the output nicer + @mock.patch("copr_rpmbuild.builders.mock.subprocess.call") + # pylint: disable-next=unused-argument, redefined-outer-name + def test_mock_config_hp_fs_size(self, call, f_mock_calls): + """ test that fs_size for performance builders is correctly set """ + self.task["tags"] = ["blabla_tag", "on_demand_powerful", "aarch64"] + builder = MockBuilder( + self.task, self.sourcedir, self.resultdir, self.config + ) + mock_snippet = "config_opts['plugin_conf']['tmpfs_opts']['max_fs_size'] = '280g'" + builder.copr_rpmbuild_config.tags_to_mock_snippet = [ + { + 'tagset': ['on_demand_powerful', "aarch64"], + 'snippet': mock_snippet, + }, + ] + builder.run() + + with open(self.child_config, "r", encoding="utf-8") as f: + config = f.readlines() + + config = ''.join(config) + assert mock_snippet in config + + @mock.patch("copr_rpmbuild.builders.mock.MockBuilder.prepare_configs") @mock.patch("copr_rpmbuild.builders.mock.MockBuilder.archive_configs") def test_mock_options(self, archive_configs, prep_configs, f_mock_calls): From 8b0977a85018bd8e95e204a2cc32466c9c2f16f5 Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Tue, 24 Sep 2024 21:09:24 +0200 Subject: [PATCH 18/18] rpmbuild: unblock testsuite --- rpmbuild/tests/test_base.py | 2 +- rpmbuild/tests/test_mock.py | 2 +- rpmbuild/tests/test_pypi.py | 4 ++-- rpmbuild/tests/test_rubygems.py | 6 +++--- rpmbuild/tests/test_scm.py | 8 ++++---- rpmbuild/tests/test_spec.py | 6 +++--- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/rpmbuild/tests/test_base.py b/rpmbuild/tests/test_base.py index 5ecbd1e60..fc098b3e7 100644 --- a/rpmbuild/tests/test_base.py +++ b/rpmbuild/tests/test_base.py @@ -18,7 +18,7 @@ def setUp(self): super(TestProvider, self).setUp() self.source_json = {} - @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open()) + @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open) @mock.patch('copr_rpmbuild.providers.base.os.mkdir') def test_create_rpmmacros(self, mock_mkdir, mock_open): task = { diff --git a/rpmbuild/tests/test_mock.py b/rpmbuild/tests/test_mock.py index 11c41f193..964ef637c 100644 --- a/rpmbuild/tests/test_mock.py +++ b/rpmbuild/tests/test_mock.py @@ -227,7 +227,7 @@ def test_produce_rpm(self, popen_mock, get_mock_uniqueext_mock, '--scrub', 'root-cache', '--quiet']) assert get_mock_uniqueext_mock.call_count == 1 - @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open()) + @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open) def test_touch_success_file(self, mock_open): builder = MockBuilder(self.task, self.sourcedir, self.resultdir, self.config) builder.touch_success_file() diff --git a/rpmbuild/tests/test_pypi.py b/rpmbuild/tests/test_pypi.py index 34443bfe9..8543e8017 100644 --- a/rpmbuild/tests/test_pypi.py +++ b/rpmbuild/tests/test_pypi.py @@ -19,7 +19,7 @@ def setUp(self): "python_versions": [2, 3]} self.resultdir = "/path/to/resultdir" - @mock.patch("{0}.open".format(builtins)) + @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open) @mock.patch('copr_rpmbuild.providers.base.os.mkdir') def test_init(self, mock_mkdir, mock_open): provider = PyPIProvider(self.source_json, self.config) @@ -29,7 +29,7 @@ def test_init(self, mock_mkdir, mock_open): self.assertEqual(provider.python_versions, [2, 3]) @mock.patch("copr_rpmbuild.providers.pypi.run_cmd") - @mock.patch("{0}.open".format(builtins)) + @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open) @mock.patch('copr_rpmbuild.providers.base.os.mkdir') def test_produce_srpm(self, mock_mkdir, mock_open, run_cmd): provider = PyPIProvider(self.source_json, self.config) diff --git a/rpmbuild/tests/test_rubygems.py b/rpmbuild/tests/test_rubygems.py index 495bd2ca6..303812b05 100644 --- a/rpmbuild/tests/test_rubygems.py +++ b/rpmbuild/tests/test_rubygems.py @@ -18,14 +18,14 @@ def setUp(self): super(TestRubyGemsProvider, self).setUp() self.source_json = {"gem_name": "A_123"} - @mock.patch("{0}.open".format(builtins)) + @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open) @mock.patch('copr_rpmbuild.providers.base.os.mkdir') def test_init(self, mock_mkdir, mock_open): provider = RubyGemsProvider(self.source_json, self.config) self.assertEqual(provider.gem_name, "A_123") @mock.patch("copr_rpmbuild.providers.rubygems.run_cmd") - @mock.patch("{0}.open".format(builtins)) + @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open) @mock.patch('copr_rpmbuild.providers.base.os.mkdir') def test_produce_srpm(self, mock_mkdir, mock_open, run_cmd): provider = RubyGemsProvider(self.source_json, self.config) @@ -35,7 +35,7 @@ def test_produce_srpm(self, mock_mkdir, mock_open, run_cmd): run_cmd.assert_called_with(assert_cmd) @mock.patch("copr_rpmbuild.providers.rubygems.run_cmd") - @mock.patch("{0}.open".format(builtins)) + @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open) @mock.patch('copr_rpmbuild.providers.base.os.mkdir') def test_empty_license(self, mock_mkdir, mock_open, run_cmd): stderr = ("error: line 8: Empty tag: License:" diff --git a/rpmbuild/tests/test_scm.py b/rpmbuild/tests/test_scm.py index 801d42202..003a91146 100644 --- a/rpmbuild/tests/test_scm.py +++ b/rpmbuild/tests/test_scm.py @@ -34,7 +34,7 @@ def setUp(self): "srpm_build_method": "rpkg", } - @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open()) + @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open) @mock.patch('copr_rpmbuild.providers.base.os.mkdir') def test_init(self, mock_mkdir, mock_open): source_json = self.source_json.copy() @@ -90,7 +90,7 @@ def test_generate_rpkg_config(self): shutil.rmtree(tmpdir) - @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open()) + @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open) @mock.patch('copr_rpmbuild.providers.base.os.mkdir') def test_get_rpkg_command(self, mock_mkdir, mock_open): provider = ScmProvider(self.source_json, self.config) @@ -100,7 +100,7 @@ def test_get_rpkg_command(self, mock_mkdir, mock_open): "--spec", provider.spec_path] self.assertEqual(provider.get_rpkg_command(), assert_cmd) - @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open()) + @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open) @mock.patch('copr_rpmbuild.providers.base.os.mkdir') def test_get_tito_command(self, mock_mkdir, mock_open): provider = ScmProvider(self.source_json, self.config) @@ -110,7 +110,7 @@ def test_get_tito_command(self, mock_mkdir, mock_open): @mock.patch("copr_rpmbuild.helpers.run_cmd") - @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open()) + @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open) @mock.patch('copr_rpmbuild.providers.base.os.mkdir') def test_get_tito_test_command(self, mock_mkdir, mock_open, run_cmd_mock): provider = ScmProvider(self.source_json, self.config) diff --git a/rpmbuild/tests/test_spec.py b/rpmbuild/tests/test_spec.py index 62d64cbec..5f38ccee3 100644 --- a/rpmbuild/tests/test_spec.py +++ b/rpmbuild/tests/test_spec.py @@ -24,7 +24,7 @@ class TestUrlProvider(TestCase): def auto_test_setup(self): self.source_json = {"url": u"http://foo.ex/somepackage.spec"} - @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open()) + @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open) @mock.patch('copr_rpmbuild.providers.base.os.mkdir') def test_init(self, mock_mkdir, mock_open): provider = UrlProvider(self.source_json, self.config) @@ -32,7 +32,7 @@ def test_init(self, mock_mkdir, mock_open): @mock.patch('copr_common.request.SafeRequest.get') @mock.patch("copr_rpmbuild.providers.base.run_cmd") - @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open()) + @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open) @mock.patch("copr_rpmbuild.providers.spec.UrlProvider.create_rpmmacros") @mock.patch("copr_rpmbuild.providers.spec.UrlProvider.generate_mock_config") @mock.patch('copr_rpmbuild.providers.base.os.mkdir') @@ -52,7 +52,7 @@ def test_produce_srpm(self, mock_mkdir, mock_generate_mock_config, run_cmd.assert_called_with(args, cwd=provider.workdir) @mock.patch('copr_common.request.SafeRequest.get') - @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open()) + @mock.patch('{0}.open'.format(builtins), new_callable=mock.mock_open) @mock.patch('copr_rpmbuild.providers.base.os.mkdir') def test_save_spec(self, mock_mkdir, mock_open, mock_get): provider = UrlProvider(self.source_json, self.config)