From 11c300e520b5331b876c8bae894e19e981464e94 Mon Sep 17 00:00:00 2001 From: Jakub Kadlcik Date: Thu, 5 Sep 2024 09:43:09 +0200 Subject: [PATCH] backend, frontend: implement project and build deletion in Pulp Fix #3318 Fix #3319 --- backend/copr_backend/actions.py | 114 +++++------------ backend/copr_backend/pulp.py | 49 ++++++++ backend/copr_backend/storage.py | 117 +++++++++++++++++- backend/tests/test_action.py | 2 +- .../coprs/logic/actions_logic.py | 10 +- .../tests/test_logic/test_builds_logic.py | 5 +- 6 files changed, 207 insertions(+), 90 deletions(-) diff --git a/backend/copr_backend/actions.py b/backend/copr_backend/actions.py index effff12af..558d2a788 100644 --- a/backend/copr_backend/actions.py +++ b/backend/copr_backend/actions.py @@ -26,8 +26,7 @@ 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) @@ -221,72 +219,21 @@ 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) + return BackendResultEnum("success") class CompsUpdate(Action): @@ -322,7 +269,7 @@ def run(self): return result -class DeleteMultipleBuilds(Delete): +class DeleteMultipleBuilds(Action): def run(self): self.log.debug("Action delete multiple builds.") @@ -334,25 +281,20 @@ 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): + success = self.storage.delete_builds( + project_dirname, chroot_builddirs, build_ids) + 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 +305,27 @@ 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) + success = self.storage.delete_builds( + self.ext_data["project_dirname"], + self.ext_data["chroot_builddirs"], + [self.data['object_id']]) + 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 c794f1cdc..3ede6181c 100644 --- a/backend/copr_backend/pulp.py +++ b/backend/copr_backend/pulp.py @@ -163,6 +163,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 @@ -178,3 +188,42 @@ def delete_distribution(self, distribution): """ url = self.config["base_url"] + distribution return requests.delete(url, **self.request_params) + + def list_packages(self, repository_version): + """ + List packages for a given repository version + Ideally, we would list packages for a repository but that probably + isn't possible. + + TODO Since we don't care about repository versions, we should probably + set `--retain-repo-versions=1` + https://discourse.pulpproject.org/t/delete-artifact-with-pulp-cli/559/6 + + https://pulpproject.org/pulp_rpm/restapi/#tag/Content:-Packages/operation/content_rpm_packages_list + """ + url = self.url("api/v3/content/rpm/packages/?") + url += urlencode({"repository_version": repository_version}) + return requests.get(url, **self.request_params) + + def list_repositories(self, prefix): + """ + Get a list of repositories whose names match a given prefix + https://pulpproject.org/pulp_rpm/restapi/#tag/Repositories:-Rpm/operation/repositories_rpm_rpm_list + """ + url = self.url("api/v3/repositories/rpm/rpm/?") + url += urlencode({"name__startswith": prefix}) + return requests.get(url, **self.request_params) + + def get_latest_repository_version(self, name): + """ + Return the latest repository version + https://pulpproject.org/pulp_rpm/restapi/#tag/Repositories:-Rpm-Versions/operation/repositories_rpm_rpm_versions_list + """ + response = self.get_repository(name) + href = response.json()["results"][0]["versions_href"] + + url = self.config["base_url"] + href + "?" + url += urlencode({"offset": 0, "limit": 1}) + + response = requests.get(url, **self.request_params) + return response.json()["results"][0]["pulp_href"] diff --git a/backend/copr_backend/storage.py b/backend/copr_backend/storage.py index 182382750..d5c8752e4 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): + artifacts = [] for root, _, files in os.walk(results_dir): for name in files: if os.path.basename(root) == "prev_build_backup": @@ -171,6 +227,11 @@ def upload_build_results(self, chroot, results_dir, target_dir_name): self.log.info("Uploaded to Pulp: %s", path) + data = {"artifacts": artifacts} + path = os.path.join(results_dir, "pulp.json") + with open(path, "w+", encoding="utf8") as fp: + json.dump(data, fp) + def publish_repository(self, chroot, **kwargs): repository = self._get_repository(chroot) response = self.client.create_publication(repository) @@ -206,6 +267,60 @@ 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_repositories(prefix) + repositories = response.json()["results"] + for repository in repositories: + self.client.delete_repository(repository["pulp_href"]) + + def delete_builds(self, dirname, chroot_builddirs, build_ids): + # pylint: disable=too-many-locals + result = True + for chroot, subdirs in chroot_builddirs.items(): + 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) + reponame = self._repository_name(chroot, dirname=dirname) + repoversion = self.client.get_latest_repository_version(reponame) + + # It is currently not possible to set labels for Pulp content. + # https://github.com/pulp/pulpcore/issues/3338 + # Until it is implemented, we need to query all packages from a + # given repository(version) and manually filter only the artifacts + # for this build ID. Which we know from pulp.json in the resultdir. + packages = self.client.list_packages(repoversion).json()["results"] + a2c = {x["artifact"]: x["pulp_href"] for x in packages} + + for subdir in subdirs: + path = os.path.join(chroot_path, subdir, "pulp.json") + with open(path, "r", encoding="utf8") as fp: + pulp = json.load(fp) + + for artifact in pulp["artifacts"]: + content = a2c.get(artifact) + if not content: + msg = "Pulp content for artifact %s doesn't exist, skipping." + self.log.info(msg, artifact) + continue + + response = self.client.delete_content(repository, [content]) + if response.ok: + self.log.info("Successfully deleted Pulp content %s", content) + else: + result = False + self.log.info("Failed to delete Pulp content %s", content) + + 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):