From 94e0853058680450773d1a865ce319fee587b3d4 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Jun 05 2018 09:33:06 +0000 Subject: Improve handling of content_sets with multiple repositories by merging them into single one when possible. --- diff --git a/server/odcs/server/backend.py b/server/odcs/server/backend.py index 98a12ad..0f517f9 100644 --- a/server/odcs/server/backend.py +++ b/server/odcs/server/backend.py @@ -477,7 +477,7 @@ enabled=1 gpgcheck=0 """ % (name, name, url) repofile += r - arches.add(repo_data["arch"]) + arches = arches.union(repo_data["arches"]) sigkeys = sigkeys.union(repo_data["sigkeys"]) _write_repo_file(compose, repofile) diff --git a/server/odcs/server/pulp.py b/server/odcs/server/pulp.py index f895860..24b84d3 100644 --- a/server/odcs/server/pulp.py +++ b/server/odcs/server/pulp.py @@ -20,7 +20,9 @@ # SOFTWARE. # # Written by Chenxiong Qi +# Jan Kaluza +import copy import json import requests @@ -43,6 +45,46 @@ 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 get_repos_from_content_sets(self, content_sets): """ Returns dictionary with URLs of all shipped repositories defined by @@ -56,7 +98,7 @@ class Pulp(object): { content_set_1: { "url": repo_url, - "arch": repo_arch, + "arches": set([repo_arch1, repo_arch2]), 'sigkeys': ['sigkey1', 'sigkey2', ...] }, ... @@ -74,20 +116,31 @@ class Pulp(object): } repos = self._rest_post('repositories/search/', query_data) - ret = {} + per_content_set_repos = {} for repo in repos: + notes = repo["notes"] url = "%s/%s" % (self.server_url.rstrip('/'), - repo['notes']['relative_url']) - arch = repo["notes"]["arch"] - sigkeys = repo["notes"]["signatures"].split(",") + notes['relative_url']) + arch = notes["arch"] + sigkeys = sorted(notes["signatures"].split(",")) # OSBS cannot verify https during the container image build, so # fallback to http for now. if url.startswith("https://"): url = "http://" + url[len("https://"):] - ret[repo["notes"]["content_set"]] = { + if notes["content_set"] not in per_content_set_repos: + per_content_set_repos[notes["content_set"]] = [] + per_content_set_repos[notes["content_set"]].append({ "url": url, - "arch": arch, + "arches": set([arch]), "sigkeys": sigkeys, - } + }) + + ret = {} + for cs, repos in per_content_set_repos.items(): + merged_repos = self._try_arch_merge(repos) + if merged_repos: + ret[cs] = merged_repos + else: + ret[cs] = repos[-1] return ret diff --git a/server/tests/test_pulp.py b/server/tests/test_pulp.py new file mode 100644 index 0000000..5af2a08 --- /dev/null +++ b/server/tests/test_pulp.py @@ -0,0 +1,79 @@ +# Copyright (c) 2016 Red Hat, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +# Written by Jan Kaluza + +import unittest +from mock import patch + +from odcs.server.pulp import Pulp + + +@patch("odcs.server.pulp.Pulp._rest_post") +class TestPulp(unittest.TestCase): + + 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. + """ + pulp_rest_post.return_value = [ + { + "notes": { + "relative_url": "content/1/x86_64/os", + "content_set": "foo-1", + "arch": "x86_64", + "signatures": "SIG1,SIG2", + }, + }, + { + "notes": { + "relative_url": "content/1/ppc64le/os", + "content_set": "foo-1", + "arch": "ppc64le", + "signatures": "SIG1,SIG2", + } + }, + { + "notes": { + "relative_url": "content/3/ppc64/os", + "content_set": "foo-2", + "arch": "ppc64", + "signatures": "SIG1,SIG3", + } + } + ] + + pulp = Pulp("http://localhost/", "user", "pass") + ret = pulp.get_repos_from_content_sets(["foo-1", "foo-2"]) + self.assertEqual( + ret, + { + "foo-1": { + "url": "http://localhost/content/1/$basearch/os", + "arches": set(["x86_64", "ppc64le"]), + "sigkeys": ["SIG1", "SIG2"], + }, + "foo-2": { + "url": "http://localhost/content/3/ppc64/os", + "arches": set(["ppc64"]), + "sigkeys": ["SIG1", "SIG3"], + } + })