From 6fd6de91ae13b6d74719092e6bada97e3b8768da Mon Sep 17 00:00:00 2001 From: Haibo Lin Date: Nov 13 2023 02:55:22 +0000 Subject: Rework handling Pulp content sets JIRA: RHELCMP-12628 Signed-off-by: Haibo Lin --- diff --git a/server/odcs/server/backend.py b/server/odcs/server/backend.py index 7c03ffc..9f93a2c 100644 --- a/server/odcs/server/backend.py +++ b/server/odcs/server/backend.py @@ -840,20 +840,19 @@ def generate_pulp_compose(compose): log.error(err) raise ValueError(err) - merged_repos = pulp.merge_repos_by_arch(repos) + all_repos = list(itertools.chain(*repos.values())) # Get all repos that are used via content sets. - used_repo_ids = set(item.get("id") for item in merged_repos.values()) + used_repo_ids = set(item.get("id") for item in all_repos) # Add direct repos to the result, as long as the same repo is not already # used by a content set. for key, repo in direct_repos.items(): if repo["id"] not in used_repo_ids: - merged_repos[key] = repo + all_repos.append(repo) repofile = "" arches = set() sigkeys = set() - for name in sorted(merged_repos.keys()): - repo_data = merged_repos[name] + for repo_data in sorted(all_repos, key=lambda r: r["id"]): if compose.flags & COMPOSE_FLAGS["use_only_compatible_arch"]: url = repo_data["url"].replace("".join(repo_data["arches"]), "$basearch") else: @@ -866,7 +865,7 @@ def generate_pulp_compose(compose): enabled=1 gpgcheck=0 """.format( - name, url + repo_data["id"], url ) ) if compose.flags & COMPOSE_FLAGS["use_only_compatible_arch"]: @@ -875,7 +874,7 @@ def generate_pulp_compose(compose): arches = arches.union(repo_data["arches"]) sigkeys = sigkeys.union(repo_data["sigkeys"]) - if not merged_repos: + if not all_repos: log.info("Creating empty repository for %r", compose) arches = conf.allowed_arches odcs.server.utils.write_empty_repo(compose, arches) diff --git a/server/odcs/server/pulp.py b/server/odcs/server/pulp.py index 082a8c7..046e451 100644 --- a/server/odcs/server/pulp.py +++ b/server/odcs/server/pulp.py @@ -22,15 +22,12 @@ # Written by Chenxiong Qi # Jan Kaluza -import copy import json -import re import requests import requests.exceptions from requests.models import ProtocolError from odcs.server import conf, log -from odcs.server.mergerepo import MergeRepo from odcs.server.utils import retry @@ -78,87 +75,6 @@ class Pulp(object): r.raise_for_status() return r.json() - def _try_arch_merge(self, content_set_repos): - """ - Tries replacing arch string (e.g. "x86_64" or "ppc64le") in each "url" - in content_set_repos with "$basearch" and if this results in the same - repository URL for each repo in the content_set_repos and also - the sigkeys are the same, returns the single repo with $basearch. - If not, returns an empty dict. - - The "arches" value of returned repo is set to union of merged "arches". - - For example, for following input: - [{"url": "http://localhost/x86_64/os", "arches": ["x86_64"]}, - {"url": "http://localhost/ppc64le/os", "arches": ["ppc64le"]}] - This method returns: - {"url": "http://localhost/$basearch/os", - "arches": ["x86_64", "ppc64le"]} - """ - # For no or exactly one repo, there is nothing to merge. - if len(content_set_repos) < 2: - return {} - - first_repo = None - for repo in content_set_repos: - if len(repo["arches"]) != 1: - # This should not happen normally, because each repo has just - # single arch in Pulp, but be defensive. - raise ValueError( - "Content set repository %s does not have exactly 1 arch: " - "%r." % (repo["url"], repo["arches"]) - ) - url = repo["url"].replace(list(repo["arches"])[0], "$basearch") - if first_repo is None: - first_repo = copy.deepcopy(repo) - first_repo["url"] = url - continue - if first_repo["url"] != url or first_repo["sigkeys"] != repo["sigkeys"]: - return {} - first_repo["arches"] = first_repo["arches"].union(repo["arches"]) - return first_repo - - def _merge_repos(self, content_set, content_set_repos): - """ - Merges the repositories of the same arch from `content_set_repos` - and returns the new repository dict pointing to the newly created - merged repository. - - In case there is just single (or none) repository in - `content_set_repos`, returns empty dict. - """ - # For no or exactly one repo, there is nothing to merge. - if len(content_set_repos) < 2: - return {} - - # We must merge repos of the same arch only, so group them by arch - # at first. - per_arch_repos = {} - for repo in content_set_repos: - if len(repo["arches"]) != 1: - # This should not happen normally, because each repo has just - # single arch in Pulp, but be defensive. - raise ValueError( - "Content set repository %s does not have exactly 1 arch: " - "%r." % (repo["url"], repo["arches"]) - ) - arch = list(repo["arches"])[0] - if arch not in per_arch_repos: - per_arch_repos[arch] = [] - per_arch_repos[arch].append(repo) - - merge_repo = MergeRepo(self.compose) - - for arch, repos in per_arch_repos.items(): - urls = [repo["url"] for repo in repos] - merge_repo.run(arch, urls, content_set) - - return { - "url": "%s/%s/$basearch" % (self.compose.result_repo_url, content_set), - "arches": set(per_arch_repos.keys()), - "sigkeys": content_set_repos[0]["sigkeys"], - } - def _make_repo_info(self, raw_repo): """ Convert the raw repo info returned from Pulp to a simple repo object @@ -221,80 +137,6 @@ class Pulp(object): ) return per_content_set_repos - def merge_repos_by_arch(self, repos): - """ - Merge repositories by arch and the repo with highest version within - a content set will be selected. - - :param repos: a mapping from a content set to its associated repositories. - :type repos: dict[str, list[dict]] - :return: a new mapping from a content set to the merged repository by arch. - :rtype: dict[str, dict] - """ - ret = {} - for cs, repos in repos.items(): - can_sort = True - version_len = None - arches = None - for repo in repos: - if arches and arches != repo["arches"]: - log.debug("Skipping sorting repos: arch mismatch.") - can_sort = False - break - arches = repo["arches"] - try: - product_versions = json.loads(repo["product_versions"]) - except ValueError: - log.debug( - "Skipping sorting repos: invalid JSON in product_versions." - ) - can_sort = False - break - if not isinstance(product_versions, list): - log.debug("Skipping sorting repos: no product versions.") - can_sort = False - break - if len(product_versions) != 1: - log.debug( - "Skipping sorting repos: more than one version for a repo." - ) - can_sort = False - break - if not re.match(r"\d+(\.\d+)*$", product_versions[0]): - log.debug("Skipping sorting repos: unparsable version string.") - can_sort = False - break - # Parse the version into a tuple. - ver_len = product_versions[0].count(".") - if version_len and version_len != ver_len: - log.debug("Skipping sorting repos: incompatible versions.") - can_sort = False - break - version_len = ver_len - if can_sort: - repos = [ - sorted( - repos, - key=lambda k: tuple( - json.loads(k["product_versions"])[0].split(".") - ), - reverse=True, - )[0] - ] - # In case there are multiple repos, at first try merging them - # by replacing arch in repository baseurl with $basearch. - merged_repos = self._try_arch_merge(repos) - if not merged_repos: - # In case we cannot merge repositories by replacing the arch - # with $basearch, call mergerepo_c. - merged_repos = self._merge_repos(cs, repos) - if merged_repos: - ret[cs] = merged_repos - else: - ret[cs] = repos[-1] - - return ret - def get_repos_by_id(self, repo_ids, include_unpublished_repos=False): """Get repositories by id diff --git a/server/tests/test_backend.py b/server/tests/test_backend.py index 7e9ce6d..d791daf 100644 --- a/server/tests/test_backend.py +++ b/server/tests/test_backend.py @@ -873,20 +873,20 @@ class TestBackend(ModelsBaseTest): expected_repofile = dedent( """ - [foo-1] - name=foo-1 + [repo1] + name=repo1 baseurl=https://localhost/content/1/x86_64/os enabled=1 gpgcheck=0 - [foo-2] - name=foo-2 + [repo2] + name=repo2 baseurl=https://localhost/content/2/x86_64/os enabled=1 gpgcheck=0 - [foo-3] - name=foo-3 + [repo3] + name=repo3 baseurl=https://localhost/content/3/ppc64/os enabled=1 gpgcheck=0 @@ -1083,8 +1083,8 @@ class TestBackend(ModelsBaseTest): pulp_rest_post.assert_called_once_with("repositories/search/", expected_query) expected_repofile = dedent( """ - [foo-1] - name=foo-1 + [repo1] + name=repo1 baseurl=https://localhost/content/1/x86_64/os enabled=1 gpgcheck=0 diff --git a/server/tests/test_pulp.py b/server/tests/test_pulp.py index 1f4794f..84b50ce 100644 --- a/server/tests/test_pulp.py +++ b/server/tests/test_pulp.py @@ -24,7 +24,7 @@ from mock import patch from odcs.server.pulp import Pulp from odcs.server.pungi import PungiSourceType -from odcs.server import db, conf +from odcs.server import db from odcs.server.models import Compose from .utils import ModelsBaseTest @@ -70,241 +70,3 @@ class TestPulp(ModelsBaseTest): } }, ) - - def test_generate_pulp_compose_arch_merge(self, pulp_rest_post): - """ - Tests that multiple repos in single content_set are merged into - single one by replacing arch with $basearch variable if possible. - """ - c = Compose.create(db.session, "me", PungiSourceType.PULP, "foo-1", 0, 3600) - db.session.commit() - - pulp_rest_post.return_value = [ - { - "notes": { - "relative_url": "content/1/x86_64/os", - "content_set": "foo-1", - "arch": "x86_64", - "signatures": "SIG1,SIG2", - "product_versions": "", - }, - "id": "repo1", - }, - { - "notes": { - "relative_url": "content/1/ppc64le/os", - "content_set": "foo-1", - "arch": "ppc64le", - "signatures": "SIG1,SIG2", - "product_versions": "", - }, - "id": "repo2", - }, - { - "notes": { - "relative_url": "content/3/ppc64/os", - "content_set": "foo-2", - "arch": "ppc64", - "signatures": "SIG1,SIG3", - "product_versions": "", - }, - "id": "repo3", - }, - ] - - pulp = Pulp("http://localhost/", "user", "pass", c) - repos = pulp.get_repos_from_content_sets(["foo-1", "foo-2"]) - ret = pulp.merge_repos_by_arch(repos) - self.assertEqual( - ret, - { - "foo-1": { - "id": "repo1", - "url": "http://localhost/content/1/$basearch/os", - "arches": {"x86_64", "ppc64le"}, - "sigkeys": ["SIG1", "SIG2"], - "product_versions": "", - }, - "foo-2": { - "id": "repo3", - "url": "http://localhost/content/3/ppc64/os", - "arches": {"ppc64"}, - "sigkeys": ["SIG1", "SIG3"], - "product_versions": "", - }, - }, - ) - - @patch("odcs.server.mergerepo.execute_cmd") - @patch("odcs.server.mergerepo.makedirs") - @patch("odcs.server.mergerepo.Lock") - @patch("odcs.server.mergerepo.MergeRepo._download_repodata") - def test_pulp_compose_merge_repos( - self, download_repodata, lock, makedirs, execute_cmd, pulp_rest_post - ): - c = Compose.create(db.session, "me", PungiSourceType.PULP, "foo-1", 0, 3600) - db.session.commit() - - pulp_rest_post.return_value = [ - { - "notes": { - "relative_url": "content/1.0/x86_64/os", - "content_set": "foo-1", - "arch": "x86_64", - "signatures": "SIG1,SIG2", - "product_versions": "", - }, - "id": "repo1", - }, - # Test two same relative_urls here. - { - "notes": { - "relative_url": "content/1.0/x86_64/os", - "content_set": "foo-1", - "arch": "x86_64", - "signatures": "SIG1,SIG2", - "product_versions": "", - }, - "id": "repo2", - }, - { - "notes": { - "relative_url": "content/1.1/x86_64/os", - "content_set": "foo-1", - "arch": "x86_64", - "signatures": "SIG1,SIG2", - "product_versions": "", - }, - "id": "repo3", - }, - { - "notes": { - "relative_url": "content/1.0/ppc64le/os", - "content_set": "foo-1", - "arch": "ppc64le", - "signatures": "SIG1,SIG2", - "product_versions": "", - }, - "id": "repo4", - }, - ] - - pulp = Pulp("http://localhost/", "user", "pass", c) - repos = pulp.get_repos_from_content_sets(["foo-1", "foo-2"]) - ret = pulp.merge_repos_by_arch(repos) - - self.assertEqual( - ret, - { - "foo-1": { - "url": "http://localhost/odcs/odcs-1/compose/Temporary/foo-1/$basearch", - "arches": {"x86_64", "ppc64le"}, - "sigkeys": ["SIG1", "SIG2"], - } - }, - ) - - makedirs.assert_any_call(c.result_repo_dir + "/foo-1/x86_64") - makedirs.assert_any_call(c.result_repo_dir + "/foo-1/ppc64le") - - repo_prefix = "%s/pulp_repo_cache/content/" % conf.target_dir - execute_cmd.assert_any_call( - [ - "/usr/bin/mergerepo_c", - "--method", - "nvr", - "-o", - c.result_repo_dir + "/foo-1/x86_64", - "--repo-prefix-search", - "%s/pulp_repo_cache" % conf.target_dir, - "--repo-prefix-replace", - "http://localhost/", - "-r", - repo_prefix + "1.0/x86_64/os", - "-r", - repo_prefix + "1.1/x86_64/os", - ], - timeout=1800, - ) - execute_cmd.assert_any_call( - [ - "/usr/bin/mergerepo_c", - "--method", - "nvr", - "-o", - c.result_repo_dir + "/foo-1/ppc64le", - "--repo-prefix-search", - "%s/pulp_repo_cache" % conf.target_dir, - "--repo-prefix-replace", - "http://localhost/", - "-r", - repo_prefix + "1.0/ppc64le/os", - ], - timeout=1800, - ) - - download_repodata.assert_any_call( - repo_prefix + "1.0/x86_64/os", "http://localhost/content/1.0/x86_64/os" - ) - download_repodata.assert_any_call( - repo_prefix + "1.1/x86_64/os", "http://localhost/content/1.1/x86_64/os" - ) - download_repodata.assert_any_call( - repo_prefix + "1.0/ppc64le/os", "http://localhost/content/1.0/ppc64le/os" - ) - - @patch("odcs.server.mergerepo.execute_cmd") - @patch("odcs.server.mergerepo.makedirs") - @patch("odcs.server.mergerepo.Lock") - @patch("odcs.server.mergerepo.MergeRepo._download_repodata") - def test_pulp_compose_find_latest_version( - self, download_repodata, lock, makedirs, execute_cmd, pulp_rest_post - ): - c = Compose.create(db.session, "me", PungiSourceType.PULP, "foo-1", 0, 3600) - db.session.commit() - - pulp_rest_post.return_value = [ - { - "notes": { - "relative_url": "content/1.0/x86_64/os", - "content_set": "foo-1", - "arch": "x86_64", - "signatures": "SIG1,SIG2", - "product_versions": '["1.0"]', - }, - "id": "repo1", - }, - { - "notes": { - "relative_url": "content/1.1/x86_64/os", - "content_set": "foo-1", - "arch": "x86_64", - "signatures": "SIG1,SIG2", - "product_versions": '["1.1"]', - }, - "id": "repo2", - }, - ] - - pulp = Pulp("http://localhost/", "user", "pass", c) - repos = pulp.get_repos_from_content_sets(["foo-1"]) - ret = pulp.merge_repos_by_arch(repos) - - self.assertEqual( - ret, - { - "foo-1": { - "id": "repo2", - "url": "http://localhost/content/1.1/x86_64/os", - "arches": {"x86_64"}, - "sigkeys": ["SIG1", "SIG2"], - "product_versions": '["1.1"]', - } - }, - ) - - makedirs.assert_not_called() - - execute_cmd.assert_not_called() - - download_repodata.assert_not_called()