From f559c18079e38adafcddb2b2c5614e18ec78ac58 Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Mon, 14 Feb 2022 17:13:06 +0000 Subject: [PATCH] plugins: support for repo package sets This adds support for specifing the package sets for repositories; on the command line this can be done via `--repo-package-set` with and argument of `;` separated package set names. This will result in repo information being transported via dict instead of plain strings. Thus the hub plugin's schema was modified accordingly. Last but not least, the builder plugin now can decode these dicts and setup the repos accordingly. Test were added for plugins as well as the integration test changed to use this new feature. The first upstream commit that supports this feature is pinned. --- plugins/builder/osbuild.py | 20 +++++++++--- plugins/cli/osbuild.py | 34 +++++++++++++++++++- plugins/hub/osbuild.py | 22 ++++++++++++- schutzbot/deploy.sh | 2 +- test/integration/test_koji.py | 14 ++++++--- test/unit/test_builder.py | 51 ++++++++++++++++++++++++++++++ test/unit/test_cli.py | 59 +++++++++++++++++++++++++++++++++++ 7 files changed, 191 insertions(+), 11 deletions(-) diff --git a/plugins/builder/osbuild.py b/plugins/builder/osbuild.py index 367c52b..bcced48 100644 --- a/plugins/builder/osbuild.py +++ b/plugins/builder/osbuild.py @@ -24,7 +24,7 @@ import time import urllib.parse from string import Template -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Union import requests import koji @@ -80,11 +80,21 @@ class OSTreeOptions: class Repository: - def __init__(self, baseurl: str, gpgkey: str = None): + def __init__(self, baseurl: str): self.baseurl = baseurl - self.gpgkey = gpgkey + self.gpgkey = None + self.package_sets: List[str] = None self.rhsm = False + @classmethod + def from_data(cls, data: Union[str, Dict]) -> "Repository": + if isinstance(data, str): + return cls(data) + baseurl = data["baseurl"] + repo = cls(baseurl) + repo.package_sets = data.get("package_sets") + return repo + def as_dict(self, arch: str = ""): tmp = Template(self.baseurl) url = tmp.substitute(arch=arch) @@ -95,6 +105,8 @@ class Repository: if self.gpgkey: res["gpg_key"] = self.gpgkey res["check_gpg"] = True + if self.package_sets: + res["package_sets"] = self.package_sets return res @@ -532,7 +544,7 @@ class OSBuildImage(BaseTaskHandler): def make_repos_for_user(self, repos): self.logger.debug("user repo override: %s", str(repos)) - return [Repository(r) for r in repos] + return [Repository.from_data(r) for r in repos] def map_koji_api_image_type(self, image_type: str) -> str: mapped = KOJIAPI_IMAGE_TYPES.get(image_type) diff --git a/plugins/cli/osbuild.py b/plugins/cli/osbuild.py index 71412fd..e485108 100755 --- a/plugins/cli/osbuild.py +++ b/plugins/cli/osbuild.py @@ -6,6 +6,7 @@ is provided by the koji osbuild plugin for the koji hub. """ +import optparse # pylint: disable=deprecated-module from pprint import pprint import koji @@ -13,6 +14,32 @@ import koji_cli.lib as kl from koji.plugin import export_cli +def parse_repo(_option, _opt, value, parser): + repo = parser.values.repo + if repo and isinstance(repo[0], dict): + repo.append({"baseurl": value}) + return + + if not repo: + parser.values.repo = repo = [] + repo.append(value) + + +def parse_repo_package_set(_option, opt, value, parser): + if not parser.values.repo: + raise optparse.OptionValueError(f"Need '--repo' for {opt}") + + repo = parser.values.repo.pop() + if not isinstance(repo, dict): + repo = { + "baseurl": repo + } + ps = repo.get("package_sets", []) + vals = set(map(lambda x: x.strip(), value.split(";"))) + repo["package_sets"] = list(sorted(set(ps).union(vals))) + parser.values.repo.append(repo) + + def parse_args(argv): usage = ("usage: %prog osbuild-image [options] " " [ ...]") @@ -28,10 +55,15 @@ def parse_args(argv): parser.add_option("--ostree-url", type=str, dest="ostree_url", help="URL to the OSTree repo for OSTree commit image types") parser.add_option("--release", help="Forcibly set the release field") - parser.add_option("--repo", action="append", + parser.add_option("--repo", action="callback", callback=parse_repo, nargs=1, type=str, help=("Specify a repo that will override the repo used to install " "RPMs in the image. May be used multiple times. The " "build tag repo associated with the target is the default.")) + parser.add_option("--repo-package-sets", dest="repo", nargs=1, type=str, + action="callback", callback=parse_repo_package_set, + help=("Specify the package sets for the last repository. " + "Individual set items are separated by ';'. " + "Maybe be used multiple times")) parser.add_option("--image-type", metavar="TYPE", help='Request an image-type [default: guest-image]', type=str, action="append", default=[]) diff --git a/plugins/hub/osbuild.py b/plugins/hub/osbuild.py index de6c006..1e954d1 100644 --- a/plugins/hub/osbuild.py +++ b/plugins/hub/osbuild.py @@ -49,6 +49,23 @@ OSBUILD_IMAGE_SCHEMA = { "$ref": "#/definitions/options" }], "definitions": { + "repo": { + "title": "Repository options", + "type": "object", + "additionalProperties": False, + "properties": { + "baseurl": { + "type": "string" + }, + "package_sets": { + "type": "array", + "description": "Repositories", + "items": { + "type": "string" + } + } + } + }, "ostree": { "title": "OSTree specific options", "type": "object", @@ -78,7 +95,10 @@ OSBUILD_IMAGE_SCHEMA = { "type": "array", "description": "Repositories", "items": { - "type": "string" + "oneOf": [ + {"type": "string"}, + {"$ref": "#/definitions/repo"} + ] } }, "release": { diff --git a/schutzbot/deploy.sh b/schutzbot/deploy.sh index b186b9c..4beb040 100755 --- a/schutzbot/deploy.sh +++ b/schutzbot/deploy.sh @@ -21,7 +21,7 @@ function retry { # Variables for where to find osbuild-composer RPMs to test against DNF_REPO_BASEURL=http://osbuild-composer-repos.s3-website.us-east-2.amazonaws.com OSBUILD_COMMIT=bb30ffa0629e16ecff103aaaeb7e931f3f8ff79e # release 46 -OSBUILD_COMPOSER_COMMIT=631bd21ffeea03e7d4849f4d34430bde5a1b9db9 # commit that contains the cloud API integration +OSBUILD_COMPOSER_COMMIT=346486cd3f06856efee5e982553e28fb387558e6 # commit that contains repo package sets # Get OS details. source /etc/os-release diff --git a/test/integration/test_koji.py b/test/integration/test_koji.py index 29a6c9e..50e728f 100644 --- a/test/integration/test_koji.py +++ b/test/integration/test_koji.py @@ -12,11 +12,13 @@ import subprocess REPOS = { "fedora": [ - "http://download.fedoraproject.org/pub/fedora/linux/releases/$release/Everything/$arch/os" + {"url": "http://download.fedoraproject.org/pub/fedora/linux/releases/$release/Everything/$arch/os"} ], "rhel": [ - "http://download.devel.redhat.com/released/RHEL-8/$release/BaseOS/x86_64/os/", - "http://download.devel.redhat.com/released/RHEL-8/$release/AppStream/x86_64/os/", + {"url": "http://download.devel.redhat.com/released/RHEL-8/$release/BaseOS/x86_64/os/", + "package_sets": "blueprint; build; packages"}, + {"url": "http://download.devel.redhat.com/released/RHEL-8/$release/AppStream/x86_64/os/", + "package_sets": "blueprint; build; packages"}, ] } @@ -98,9 +100,13 @@ class TestIntegration(unittest.TestCase): repos = [] for repo in REPOS[name]: - tpl = string.Template(repo) + baseurl = repo["url"] + package_sets = repo.get("package_sets") + tpl = string.Template(baseurl) url = tpl.safe_substitute({"release": release}) repos += ["--repo", url] + if package_sets: + repos += ["--repo-package-sets", package_sets] package = f"{name.lower()}-guest" diff --git a/test/unit/test_builder.py b/test/unit/test_builder.py index bc5aade..7a604e2 100644 --- a/test/unit/test_builder.py +++ b/test/unit/test_builder.py @@ -2,6 +2,8 @@ # koji hub plugin unit tests # +#pylint: disable=too-many-lines + import configparser import json import os @@ -960,3 +962,52 @@ class TestBuilderPlugin(PluginTest): ireq_refs = [i["ostree"]["ref"] for i in ireqs] diff = set(f"osbuild/{a}/r" for a in arches) ^ set(ireq_refs) self.assertEqual(diff, set()) + + @httpretty.activate + def test_compose_repo_complex(self): + # Check we properly handle ostree compose requests + session = self.mock_session() + handler = self.make_handler(session=session) + + arches = ["x86_64", "s390x"] + repos = [ + {"baseurl": "https://first.repo/$arch", + "package_sets": ["a", "b", "c", "d"]}, + {"baseurl": "https://second.repo/$arch", + "package_sets": ["alpha"]}, + {"baseurl": "https://third.repo/$arch"} + ] + args = ["name", "version", "distro", + ["image_type"], + "fedora-candidate", + arches, + {"repo": repos}] + + url = self.plugin.DEFAULT_COMPOSER_URL + composer = MockComposer(url, architectures=arches) + composer.httpretty_regsiter() + + res = handler.handler(*args) + assert res, "invalid compose result" + compose_id = res["composer"]["id"] + compose = composer.composes.get(compose_id) + self.assertIsNotNone(compose) + + ireqs = compose["request"]["image_requests"] + + # Check we got all the requested architectures + ireq_arches = [i["architecture"] for i in ireqs] + diff = set(arches) ^ set(ireq_arches) + self.assertEqual(diff, set()) + + for ir in ireqs: + arch = ir["architecture"] + repos = ir["repositories"] + assert len(repos) == 3 + + for r in repos: + baseurl = r["baseurl"] + assert baseurl.endswith(arch) + if baseurl.startswith("https://first.repo"): + ps = r.get("package_sets") + assert ps and ps == ["a", "b", "c", "d"] diff --git a/test/unit/test_cli.py b/test/unit/test_cli.py index 96b62a2..b713d5b 100644 --- a/test/unit/test_cli.py +++ b/test/unit/test_cli.py @@ -182,6 +182,65 @@ class TestCliPlugin(PluginTest): r = self.plugin.handle_osbuild_image(options, session, argv) self.assertEqual(r, 0) + def test_repo_package_sets(self): + # Check we properly handle ostree specific options + + argv = [ + # the required positional arguments + "name", "version", "distro", "target", "arch1", + # optional keyword arguments + "--repo", "https://first.repo", + "--repo-package-sets", "a; b; c", + "--repo-package-sets", "d", + "--repo", "https://second.repo", + "--repo-package-sets", "alpha", + "--repo", "https://third.repo", # NB: no `--repo-package-set` + "--release", "20200202.n2", + ] + + expected_args = ["name", "version", "distro", + ['guest-image'], # the default image type + "target", + ['arch1']] + + expected_opts = { + "release": "20200202.n2", + "repo": [ + {"baseurl": "https://first.repo", + "package_sets": ["a", "b", "c", "d"]}, + {"baseurl": "https://second.repo", + "package_sets": ["alpha"]}, + {"baseurl": "https://third.repo"} + ], + } + + task_result = {"compose_id": "42", "build_id": 23} + task_id = 1 + koji_lib = self.mock_koji_lib() + + options = self.mock_options() + session = flexmock() + + self.mock_session_add_valid_tag(session) + + session.should_receive("osbuildImage") \ + .with_args(*expected_args, opts=expected_opts) \ + .and_return(task_id) \ + .once() + + session.should_receive("logout") \ + .with_args() \ + .once() + + session.should_receive("getTaskResult") \ + .with_args(task_id) \ + .and_return(task_result) \ + .once() + + setattr(self.plugin, "kl", koji_lib) + r = self.plugin.handle_osbuild_image(options, session, argv) + self.assertEqual(r, 0) + def test_target_check(self): # unknown build target session = flexmock()