From e15924306a4cad0733b785159a299e466193842f Mon Sep 17 00:00:00 2001 From: Aidan McMahon-Smith Date: Thu, 17 Oct 2024 14:50:25 +0200 Subject: [PATCH] Add --all-repos argument to deletion task [RHELDST-13644] There are ocassions where we would like to remove a package from all repos its associated with. This can be tedious as every repo has to be listed out. This change adds a convenient --all-repos argument to address this issue. --- src/pubtools/_pulp/tasks/delete.py | 46 +++-- tests/delete/test_delete_packages.py | 178 ++++++++++++++++++ ...st_delete_rpms_conflicting_repo_args.jsonl | 1 + ...test_delete_rpms_conflicting_repo_args.txt | 2 + .../test_no_repo_provided.txt | 2 +- 5 files changed, 215 insertions(+), 14 deletions(-) create mode 100644 tests/logs/delete/test_delete_packages/test_delete_rpms_conflicting_repo_args.jsonl create mode 100644 tests/logs/delete/test_delete_packages/test_delete_rpms_conflicting_repo_args.txt diff --git a/src/pubtools/_pulp/tasks/delete.py b/src/pubtools/_pulp/tasks/delete.py index e2c02369..3cdc3aaf 100644 --- a/src/pubtools/_pulp/tasks/delete.py +++ b/src/pubtools/_pulp/tasks/delete.py @@ -78,10 +78,19 @@ def add_args(self): self.parser.add_argument( "--repo", - help="remove content from these comma-seperated repositories ", + help="remove content from these comma-seperated repositories. " + "This argument is mutually exclusive with --all-repos.", type=str, action=SplitAndExtend, split_on=",", + default=[] + ) + + self.parser.add_argument( + "--all-repos", + help="remove content from all repos it belongs to. " + "This argument is mutually exclusive with --repo.", + action="store_true" ) self.parser.add_argument( @@ -130,8 +139,11 @@ def run(self): if not (self.args.file or self.args.advisory): self.fail("One of --file or --advisory is required") - if self.args.file and not self.args.repo: - self.fail("Repository names in --repo is required") + if self.args.repo and self.args.all_repos: + self.fail("Arguments --repos and --all-repos are mutually exclusive.") + + if self.args.file and not (self.args.repo or self.args.all_repos): + self.fail("Repository names in --repo or --all-repos is required") if not signing_keys and self.args.allow_unsigned: signing_keys = [None] @@ -564,21 +576,29 @@ def map_to_repo(self, units, repos, unit_attr): repo_map = {} unit_map = {} repos = sorted(repos) - for unit in sorted(units): unit_name = getattr(unit, unit_attr) unit_map.setdefault(unit_name, RemoveUnitItem(unit=unit, repos=[])) - for repo in repos: - if repo not in unit.repository_memberships: - LOG.warning( - "%s is not present in %s", - unit_name, - repo, - ) - else: + if repos: + for repo in repos: + if repo not in unit.repository_memberships: + LOG.warning( + "%s is not present in %s", + unit_name, + repo, + ) + else: + repo_map.setdefault(repo, []).append(unit) + unit_map.get(unit_name).repos.append(repo) + else: + for repo in unit.repository_memberships: + # If an RPM package is removed from it's all-rpm-content + # repo, it will be an orphan and get garbage collected. + # This saves it from needing to be reuploaded later. + if re.match("all-rpm-content-.*", repo): + continue repo_map.setdefault(repo, []).append(unit) unit_map.get(unit_name).repos.append(repo) - missing = set(repos) - set(repo_map.keys()) if missing: missing = ", ".join(sorted(list(missing))) diff --git a/tests/delete/test_delete_packages.py b/tests/delete/test_delete_packages.py index a6091b72..2b0cbda1 100644 --- a/tests/delete/test_delete_packages.py +++ b/tests/delete/test_delete_packages.py @@ -801,3 +801,181 @@ def test_delete_rpms_skip_publish(command_tester, fake_collector, monkeypatch): # All the files exist on Pulp files_search = list(client.search_content(criteria1).result()) assert len(files_search) == 2 + + +def test_delete_rpms_with_all_repos(command_tester, fake_collector, monkeypatch): + """Deleting RPMs from repos succeeds""" + + repo1 = YumRepository( + id="some-yumrepo", relative_url="some/publish/url", mutable_urls=["repomd.xml"] + ) + repo2 = YumRepository( + id="other-yumrepo", + relative_url="other/publish/url", + mutable_urls=["repomd.xml"], + ) + all_rpm_content_repo = YumRepository( + id="all-rpm-content-gg", + relative_url="other/publish/url", + mutable_urls=["repomd.xml"], + ) + + files1 = [ + RpmUnit( + name="bash", + version="1.23", + release="1.test8", + arch="x86_64", + filename="bash-1.23-1.test8_x86_64.rpm", + sha256sum="a" * 64, + md5sum="b" * 32, + signing_key="aabbcc", + unit_id="file1_rpm1", + ), + RpmUnit( + name="dash", + version="2.25", + release="1.test8", + arch="x86_64", + filename="dash-2.25-1.test8_x86_64.rpm", + sha256sum="a" * 64, + md5sum="b" * 32, + signing_key="aabbcc", + unit_id="file1_rpm2", + ), + ] + + files2 = [ + RpmUnit( + name="crash", + version="3.30", + release="1.test8", + arch="s390x", + filename="crash-3.30-1.test8_s390x.rpm", + sha256sum="a" * 64, + md5sum="b" * 32, + signing_key="aabbcc", + unit_id="file2_rpm1", + ) + ] + + + with FakeDeletePackages() as task_instance: + task_instance.pulp_client_controller.insert_repository(repo1) + task_instance.pulp_client_controller.insert_repository(repo2) + task_instance.pulp_client_controller.insert_repository(all_rpm_content_repo) + task_instance.pulp_client_controller.insert_units(repo1, files1) + task_instance.pulp_client_controller.insert_units(repo2, files2) + task_instance.pulp_client_controller.insert_units(repo2, [files1[0]]) + task_instance.pulp_client_controller.insert_units(all_rpm_content_repo, files1) + task_instance.pulp_client_controller.insert_units(all_rpm_content_repo, files2) + + # It should run with expected output. + command_tester.test( + task_instance.main, + [ + "test-delete", + "--pulp-url", + "https://pulp.example.com/", + "--all-repos", + "--file", + "bash-1.23-1.test8_x86_64.rpm", + "--file", + "dash-2.25-1.test8_x86_64.rpm", + "--signing-key", + "aabbcc", + ] + ) + # It should record that it removed these push items: + assert sorted(fake_collector.items, key=lambda pi: pi["filename"]) == [ + { + "origin": "pulp", + "src": None, + "dest": "other-yumrepo", + "signing_key": None, + "filename": "bash-1.23-1.test8.x86_64.rpm", + "state": "DELETED", + "build": None, + "checksums": {"sha256": "a" * 64}, + }, + { + "origin": "pulp", + "src": None, + "dest": "some-yumrepo", + "signing_key": None, + "filename": "bash-1.23-1.test8.x86_64.rpm", + "state": "DELETED", + "build": None, + "checksums": {"sha256": "a" * 64}, + }, + { + "origin": "pulp", + "src": None, + "dest": "some-yumrepo", + "signing_key": None, + "filename": "dash-2.25-1.test8.x86_64.rpm", + "state": "DELETED", + "build": None, + "checksums": {"sha256": "a" * 64}, + }, + ] + # + # verify whether files were deleted on Pulp + client = task_instance.pulp_client + + # get the repo where the files were deleted + repos = sorted( + list( + client.search_repository( + Criteria.with_id(["all-rpm-content-gg", "some-yumrepo", "other-yumrepo"]) + ).result() + ), + key=lambda r: r.id, + ) + assert len(repos) == 3 + r_all_content, r2, r1 = repos + + assert r_all_content.id == all_rpm_content_repo.id + assert r1.id == repo1.id + assert r2.id == repo2.id + # + # # criteria with the unit_ids + # # critera1 for files1 in repo1 + unit_ids = [] + for f in files1: + unit_ids.append(f.unit_id) + + for f in files2: + unit_ids.append(f.unit_id) + search_criteria = Criteria.with_field("unit_id", Matcher.in_(unit_ids)) + + # No files should be in repo1 + result1 =list(r1.search_content(search_criteria).result()) + assert len(result1) == 0 + + result2 = list(r2.search_content(search_criteria).result()) + assert len(result2) == 1 + assert result2[0].unit_id == files2[0].unit_id + + # --all-repos arg should skip over all-rpm-content repos + result3 = list(r_all_content.search_content(search_criteria).result()) + assert len(result3) == 3 + + +def test_delete_rpms_conflicting_repo_args(command_tester, fake_collector, monkeypatch): + """If --repos and --all-repos are used, we should fail""" + command_tester.test( + lambda: entry_point(FakeDeletePackages), + [ + "test-delete", + "--pulp-url", + "https://pulp.example.com/", + "--repo", + "some-yumrepo", + "--all-repos", + "--file", + "bash-1.23-1.test8_x86_64.rpm", + "--signing-key", + "aabbcc", + ], + ) \ No newline at end of file diff --git a/tests/logs/delete/test_delete_packages/test_delete_rpms_conflicting_repo_args.jsonl b/tests/logs/delete/test_delete_packages/test_delete_rpms_conflicting_repo_args.jsonl new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/tests/logs/delete/test_delete_packages/test_delete_rpms_conflicting_repo_args.jsonl @@ -0,0 +1 @@ + diff --git a/tests/logs/delete/test_delete_packages/test_delete_rpms_conflicting_repo_args.txt b/tests/logs/delete/test_delete_packages/test_delete_rpms_conflicting_repo_args.txt new file mode 100644 index 00000000..e589e786 --- /dev/null +++ b/tests/logs/delete/test_delete_packages/test_delete_rpms_conflicting_repo_args.txt @@ -0,0 +1,2 @@ +[ ERROR] Arguments --repos and --all-repos are mutually exclusive. +# Raised: 30 diff --git a/tests/logs/delete/test_delete_packages/test_no_repo_provided.txt b/tests/logs/delete/test_delete_packages/test_no_repo_provided.txt index bea5a620..838c4225 100644 --- a/tests/logs/delete/test_delete_packages/test_no_repo_provided.txt +++ b/tests/logs/delete/test_delete_packages/test_no_repo_provided.txt @@ -1,2 +1,2 @@ -[ ERROR] Repository names in --repo is required +[ ERROR] Repository names in --repo or --all-repos is required # Raised: 30