PR#1666: Set platform for repoclosure - pungi

pungi

#1666 Set platform for repoclosure

Opened a year ago by lsedlar. Modified 10 months ago

lsedlar/pungi repoclosure-auto-platform into master

Expose details in multiple platforms error

Lubomír Sedlář • a year ago

3466620

Set platform for repoclosure

Lubomír Sedlář • a year ago

8d1ea7b

pungi/phases/gather/methods/method_hybrid.py

file modified

+13 -57

		`@@ -13,7 +13,6 @@`
		`# You should have received a copy of the GNU General Public License`
		`# along with this program; if not, see <https://gnu.org/licenses/>.`

		`- import gzip`
		`import os`
		`from collections import defaultdict`
		`from fnmatch import fnmatch`
		`@@ -24,10 +23,15 @@`

		`import pungi.phases.gather.method`
		`from pungi import multilib_dnf`
		`- from pungi.module_util import Modulemd`
		`from pungi.arch import get_valid_arches, tree_arch_to_yum_arch`
		`from pungi.phases.gather import _mk_pkg_map`
		`- from pungi.util import get_arch_variant_data, pkg_is_debug, temp_dir, as_local_file`
		`+ from pungi.util import (`
		`+ as_local_file,`
		`+ get_arch_variant_data,`
		`+ get_platform,`
		`+ pkg_is_debug,`
		`+ temp_dir,`
		`+ )`
		`from pungi.wrappers import fus`
		`from pungi.wrappers.comps import CompsWrapper`

		`@@ -196,7 +200,12 @@`
		`set(p.name for p in self.expand_list(multilib_whitelist)),`
		`)`

		`- platform = get_platform(self.compose, variant, arch)`
		`+ platform = get_platform(`
		`+ self.compose,`
		`+ variant,`
		`+ arch,`
		`+ pungi.phases.gather.get_lookaside_repos(self.compose, arch, variant),`
		`+ )`

		`packages.update(`
		`expand_groups(self.compose, arch, variant, groups, set_pkg_arch=False)`
		`@@ -417,59 +426,6 @@`
		`return packages`


		`- def iter_platforms_in_repo(url):`
		`- """Find all platform streams that any module in give repo requires at runtime.`
		`- Yields lists of stream names (possible empty).`
		`- """`
		`- repomd = os.path.join(url, "repodata/repomd.xml")`
		`- with as_local_file(repomd) as url_:`
		`- repomd = cr.Repomd(url_)`
		`- for rec in repomd.records:`
		`- if rec.type != "modules":`
		`- continue`
		`- # No with statement on Python 2.6 for GzipFile...`
		`- record_url = os.path.join(url, rec.location_href)`
		`- with as_local_file(record_url) as url_:`
		`- gzipped_file = gzip.GzipFile(url_, "rb")`
		`- mod_index = Modulemd.ModuleIndex.new()`
		`- mod_index.update_from_string(gzipped_file.read().decode("utf-8"), False)`
		`- gzipped_file.close()`
		`- for module_name in mod_index.get_module_names():`
		`- module = mod_index.get_module(module_name)`
		`- for module_stream in module.get_all_streams():`
		`- module_stream = module_stream.upgrade(2)`
		`- for dep in module_stream.get_dependencies():`
		`- yield dep.get_runtime_streams("platform")`
		`-`
		`-`
		`- def get_platform_from_lookasides(compose, variant, arch):`
		`- """Find a set of all platform dependencies in all lookaside repos."""`
		`- platforms = set()`
		`- for repo in pungi.phases.gather.get_lookaside_repos(compose, arch, variant):`
		`- for ps in iter_platforms_in_repo(fus._prep_path(repo)):`
		`- platforms.update(ps)`
		`- return platforms`
		`-`
		`-`
		`- def get_platform(compose, variant, arch):`
		`- """Find platform stream for modules. Raises RuntimeError if there are`
		`- conflicting requests.`
		`- """`
		`- platforms = get_platform_from_lookasides(compose, variant, arch)`
		`-`
		`- for var in compose.all_variants.values():`
		`- for mmd in var.arch_mmds.get(arch, {}).values():`
		`- for dep in mmd.get_dependencies():`
		`- streams = dep.get_runtime_streams("platform")`
		`- if streams:`
		`- platforms.update(streams)`
		`-`
		`- if len(platforms) > 1:`
		`- raise RuntimeError("There are conflicting requests for platform.")`
		`-`
		`- return list(platforms)[0] if platforms else None`
		`-`
		`-`
		`def _fmt_pkg(pkg_name, arch):`
		`if arch:`
		`pkg_name += ".%s" % arch`

pungi/phases/repoclosure.py

file modified

+7 -3

		`@@ -24,7 +24,7 @@`
		`from pungi.arch import get_valid_arches`
		`from pungi.phases.base import PhaseBase`
		`from pungi.phases.gather import get_lookaside_repos, get_gather_methods`
		`- from pungi.util import is_arch_multilib, temp_dir, get_arch_variant_data`
		`+ from pungi.util import is_arch_multilib, temp_dir, get_arch_variant_data, get_platform`


		`class RepoclosurePhase(PhaseBase):`
		`@@ -85,7 +85,10 @@`
		`fus_logs = sorted(glob.glob(pattern))`
		`repoclosure.extract_from_fus_logs(fus_logs, logfile)`
		`else:`
		`- _run_repoclosure_cmd(compose, repos, lookaside, arches, logfile)`
		`+ platform = get_platform(compose, variant, arch, lookaside.values())`
		`+ _run_repoclosure_cmd(`
		`+ compose, repos, lookaside, arches, logfile, platform`
		`+ )`
		`except RuntimeError as exc:`
		`if conf and conf[-1] == "fatal":`
		`raise`
		`@@ -124,12 +127,13 @@`
		`os.remove(cache_path)`


		`- def _run_repoclosure_cmd(compose, repos, lookaside, arches, logfile):`
		`+ def _run_repoclosure_cmd(compose, repos, lookaside, arches, logfile, platform):`
		`cmd = repoclosure.get_repoclosure_cmd(`
		`backend=compose.conf["repoclosure_backend"],`
		`repos=repos,`
		`lookaside=lookaside,`
		`arch=arches,`
		`+ platform=platform,`
		`)`
		`# Use temp working directory directory as workaround for`
		`# https://bugzilla.redhat.com/show_bug.cgi?id=795137`

pungi/util.py

file modified

+59

		`@@ -14,6 +14,7 @@`
		`# along with this program; if not, see <https://gnu.org/licenses/>.`

		`import argparse`
		`+ import gzip`
		`import json`
		`import subprocess`
		`import os`
		`@@ -30,11 +31,14 @@`
		`import functools`
		`from six.moves import urllib, range, shlex_quote`

		`+ import createrepo_c as cr`
		`+`
		`import kobo.conf`
		`from kobo.shortcuts import run, force_list`
		`from kobo.threads import WorkerThread, ThreadPool`
		`from productmd.common import get_major_version`
		`from pungi.module_util import Modulemd`
		`+ from pungi.wrappers import fus`

		`# Patterns that match all names of debuginfo packages`
		`DEBUG_PATTERNS = ["-debuginfo", "-debuginfo-", "-debugsource"]`
		`@@ -1137,3 +1141,58 @@`
		`"""A helper function to read a JSON file."""`
		`with open(file_path) as f:`
		`return json.load(f)`
		`+`
		`+`
		`+ def iter_platforms_in_repo(url):`
		`+ """Find all platform streams that any module in give repo requires at runtime.`
		`+ Yields lists of stream names (possible empty).`
		`+ """`
		`+ repomd = os.path.join(url, "repodata/repomd.xml")`
		`+ with as_local_file(repomd) as url_:`
		`+ repomd = cr.Repomd(url_)`
		`+ for rec in repomd.records:`
		`+ if rec.type != "modules":`
		`+ continue`
		`+ # No with statement on Python 2.6 for GzipFile...`
		`+ record_url = os.path.join(url, rec.location_href)`
		`+ with as_local_file(record_url) as url_:`
		`+ gzipped_file = gzip.GzipFile(url_, "rb")`
		`+ mod_index = Modulemd.ModuleIndex.new()`
		`+ mod_index.update_from_string(gzipped_file.read().decode("utf-8"), False)`
		`+ gzipped_file.close()`
		`+ for module_name in mod_index.get_module_names():`
		`+ module = mod_index.get_module(module_name)`
		`+ for module_stream in module.get_all_streams():`
		`+ module_stream = module_stream.upgrade(2)`
		`+ for dep in module_stream.get_dependencies():`
		`+ yield dep.get_runtime_streams("platform")`
		`+`
		`+`
		`+ def get_platform_from_repos(repos):`
		`+ """Find a set of all platform dependencies in all lookaside repos."""`
		`+ platforms = set()`
		`+ for repo in repos:`
		`+ for ps in iter_platforms_in_repo(fus._prep_path(repo)):`
		`+ platforms.update(ps)`
		`+ return platforms`
		`+`
		`+`
		`+ def get_platform(compose, variant, arch, repos):`
		`+ """Find platform stream for modules. Raises RuntimeError if there are`
		`+ conflicting requests.`
		`+ """`
		`+ platforms = get_platform_from_repos(repos)`
		`+`
		`+ for var in compose.all_variants.values():`
		`+ for mmd in var.arch_mmds.get(arch, {}).values():`
		`+ for dep in mmd.get_dependencies():`
		`+ streams = dep.get_runtime_streams("platform")`
		`+ if streams:`
		`+ platforms.update(streams)`
		`+`
		`+ if len(platforms) > 1:`
		`+ raise RuntimeError(`
		`+ "There are conflicting requests for platform: %r" % platforms`
		`+ )`
		`+`
		`+ return list(platforms)[0] if platforms else None`

pungi/wrappers/repoclosure.py

file modified

+6 -1

		`@@ -19,7 +19,9 @@`
		`from kobo.shortcuts import force_list`


		`- def get_repoclosure_cmd(backend="yum", arch=None, repos=None, lookaside=None):`
		`+ def get_repoclosure_cmd(`
		`+ backend="yum", arch=None, repos=None, lookaside=None, platform=None`
		`+ ):`
		`cmds = {`
		`"yum": {`
		`"cmd": ["/usr/bin/repoclosure", "--tempcache"],`
		`@@ -62,6 +64,9 @@`
		`cmd.append("--repofrompath=%s,%s" % (repo_id, _to_url(repo_path)))`
		`cmd.append(cmds[backend]["lookaside"] % repo_id)`

		`+ if backend == "dnf" and platform:`
		`+ cmd.append("--setopt=module_platform_id=platform:%s" % platform)`
		`+`
		`return cmd`

tests/helpers.py

file modified

+37

		`@@ -334,3 +334,40 @@`
		`"""Like run_in_threads from Kobo, but actually runs tasks serially."""`
		`for num, param in enumerate(params):`
		`func(None, param, num)`
		`+`
		`+`
		`+ class MockModule(object):`
		`+ def __init__(`
		`+ self, name, platform=None, stream=None, version=None, context=None, rpms=None`
		`+ ):`
		`+ self.name = name`
		`+ self.platform = platform`
		`+ self.stream = stream`
		`+ self.version = version`
		`+ self.context = context`
		`+ self.rpms = rpms or ["pkg-1.0-1.x86_64"]`
		`+`
		`+ def get_name(self):`
		`+ return self.name`
		`+`
		`+ def get_module_name(self):`
		`+ return self.name`
		`+`
		`+ def get_stream_name(self):`
		`+ return self.stream`
		`+`
		`+ def get_version(self):`
		`+ return self.version`
		`+`
		`+ def get_context(self):`
		`+ return self.context`
		`+`
		`+ def get_dependencies(self):`
		`+ def get_runtime_streams(platform):`
		`+ assert platform == "platform"`
		`+ return [self.platform]`
		`+`
		`+ return [mock.Mock(get_runtime_streams=get_runtime_streams)]`
		`+`
		`+ def get_rpm_artifacts(self):`
		`+ return self.rpms`

tests/test_gather_method_hybrid.py

file modified

+6 -68

		`@@ -61,7 +61,9 @@`
		`)`

		`self.assertEqual(res, ep.return_value)`
		`- self.assertEqual(gp.call_args_list, [mock.call(compose, variant, arch)])`
		`+ self.assertEqual(`
		`+ gp.call_args_list, [mock.call(compose, variant, arch, glr.return_value)]`
		`+ )`
		`self.assertEqual(`
		`m.run_solver.call_args_list,`
		`[`
		`@@ -213,70 +215,6 @@`
		`six.assertCountEqual(self, [p.name for p in expanded], ["foo", "foo-en"])`


		`- class MockModule(object):`
		`- def __init__(`
		`- self, name, platform=None, stream=None, version=None, context=None, rpms=None`
		`- ):`
		`- self.name = name`
		`- self.platform = platform`
		`- self.stream = stream`
		`- self.version = version`
		`- self.context = context`
		`- self.rpms = rpms or ["pkg-1.0-1.x86_64"]`
		`-`
		`- def get_name(self):`
		`- return self.name`
		`-`
		`- def get_module_name(self):`
		`- return self.name`
		`-`
		`- def get_stream_name(self):`
		`- return self.stream`
		`-`
		`- def get_version(self):`
		`- return self.version`
		`-`
		`- def get_context(self):`
		`- return self.context`
		`-`
		`- def get_dependencies(self):`
		`- def get_runtime_streams(platform):`
		`- assert platform == "platform"`
		`- return [self.platform]`
		`-`
		`- return [mock.Mock(get_runtime_streams=get_runtime_streams)]`
		`-`
		`- def get_rpm_artifacts(self):`
		`- return self.rpms`
		`-`
		`-`
		`- class HelperMixin(object):`
		`- def _repo(self, name):`
		`- return os.path.join(self.compose.topdir, "work/x86_64/%s" % name)`
		`-`
		`-`
		`- class TestGetPlatform(HelperMixin, helpers.PungiTestCase):`
		`- def setUp(self):`
		`- super(TestGetPlatform, self).setUp()`
		`- self.compose = helpers.DummyCompose(self.topdir, {})`
		`- self.variant = self.compose.variants["Server"]`
		`-`
		`- def test_no_modules(self):`
		`- plat = hybrid.get_platform(self.compose, self.variant, "x86_64")`
		`- self.assertIsNone(plat)`
		`-`
		`- def test_more_than_one_platform(self):`
		`- self.variant.arch_mmds["x86_64"] = {`
		`- "mod:1": MockModule("mod", platform="f29"),`
		`- "mod:2": MockModule("mod", platform="f30"),`
		`- }`
		`-`
		`- with self.assertRaises(RuntimeError) as ctx:`
		`- hybrid.get_platform(self.compose, self.variant, "x86_64")`
		`-`
		`- self.assertIn("conflicting requests for platform", str(ctx.exception))`
		`-`
		`-`
		`class ModifiedMagicMock(mock.MagicMock):`
		`"""Like MagicMock, but remembers original values or mutable arguments."""`

		`@@ -290,7 +228,7 @@`
		`@mock.patch("pungi.wrappers.fus.parse_output")`
		`@mock.patch("pungi.wrappers.fus.get_cmd", new_callable=ModifiedMagicMock)`
		`@mock.patch("pungi.phases.gather.methods.method_hybrid.run")`
		`- class TestRunSolver(HelperMixin, helpers.PungiTestCase):`
		`+ class TestRunSolver(helpers.PungiTestCase):`
		`def setUp(self):`
		`super(TestRunSolver, self).setUp()`
		`self.compose = helpers.DummyCompose(self.topdir, {})`
		`@@ -994,8 +932,8 @@`
		`self.compose = helpers.DummyCompose(self.topdir, {})`
		`self.variant = self.compose.variants["Server"]`
		`self.variant.arch_mmds["x86_64"] = {`
		`- "mod:1": MockModule("mod", platform="f29"),`
		`- "mod:2": MockModule("mod", platform="f30"),`
		`+ "mod:1": helpers.MockModule("mod", platform="f29"),`
		`+ "mod:2": helpers.MockModule("mod", platform="f30"),`
		`}`

		`hybrid.filter_modules(self.variant, "x86_64", ["mod:1"])`

tests/test_repoclosure_phase.py

file modified

+18 -3

		`@@ -50,9 +50,10 @@`
		`self.assertEqual(mock_grc.call_args_list, [])`

		`@unittest.skipUnless(HAS_YUM, "YUM is not available")`
		`+ @mock.patch("pungi.phases.repoclosure.get_platform")`
		`@mock.patch("pungi.wrappers.repoclosure.get_repoclosure_cmd")`
		`@mock.patch("pungi.phases.repoclosure.run")`
		`- def test_repoclosure_default_backend(self, mock_run, mock_grc):`
		`+ def test_repoclosure_default_backend(self, mock_run, mock_grc, mock_get_platform):`
		`with mock.patch("six.PY2", new=True):`
		`compose = DummyCompose(self.topdir, {})`

		`@@ -67,38 +68,44 @@`
		`arch=["amd64", "x86_64", "noarch"],`
		`lookaside={},`
		`repos=self._get_repo(compose.compose_id, "Everything", "amd64"),`
		`+ platform=mock_get_platform.return_value,`
		`),`
		`mock.call(`
		`backend="yum",`
		`arch=["amd64", "x86_64", "noarch"],`
		`lookaside={},`
		`repos=self._get_repo(compose.compose_id, "Client", "amd64"),`
		`+ platform=mock_get_platform.return_value,`
		`),`
		`mock.call(`
		`backend="yum",`
		`arch=["amd64", "x86_64", "noarch"],`
		`lookaside={},`
		`repos=self._get_repo(compose.compose_id, "Server", "amd64"),`
		`+ platform=mock_get_platform.return_value,`
		`),`
		`mock.call(`
		`backend="yum",`
		`arch=["x86_64", "noarch"],`
		`lookaside={},`
		`repos=self._get_repo(compose.compose_id, "Server", "x86_64"),`
		`+ platform=mock_get_platform.return_value,`
		`),`
		`mock.call(`
		`backend="yum",`
		`arch=["x86_64", "noarch"],`
		`lookaside={},`
		`repos=self._get_repo(compose.compose_id, "Everything", "x86_64"),`
		`+ platform=mock_get_platform.return_value,`
		`),`
		`],`
		`)`

		`@unittest.skipUnless(HAS_DNF, "DNF is not available")`
		`+ @mock.patch("pungi.phases.repoclosure.get_platform")`
		`@mock.patch("pungi.wrappers.repoclosure.get_repoclosure_cmd")`
		`@mock.patch("pungi.phases.repoclosure.run")`
		`- def test_repoclosure_dnf_backend(self, mock_run, mock_grc):`
		`+ def test_repoclosure_dnf_backend(self, mock_run, mock_grc, get_platform):`
		`compose = DummyCompose(self.topdir, {"repoclosure_backend": "dnf"})`
		`repoclosure_phase.run_repoclosure(compose)`

		`@@ -111,30 +118,35 @@`
		`arch=["amd64", "x86_64", "noarch"],`
		`lookaside={},`
		`repos=self._get_repo(compose.compose_id, "Everything", "amd64"),`
		`+ platform=get_platform.return_value,`
		`),`
		`mock.call(`
		`backend="dnf",`
		`arch=["amd64", "x86_64", "noarch"],`
		`lookaside={},`
		`repos=self._get_repo(compose.compose_id, "Client", "amd64"),`
		`+ platform=get_platform.return_value,`
		`),`
		`mock.call(`
		`backend="dnf",`
		`arch=["amd64", "x86_64", "noarch"],`
		`lookaside={},`
		`repos=self._get_repo(compose.compose_id, "Server", "amd64"),`
		`+ platform=get_platform.return_value,`
		`),`
		`mock.call(`
		`backend="dnf",`
		`arch=["x86_64", "noarch"],`
		`lookaside={},`
		`repos=self._get_repo(compose.compose_id, "Server", "x86_64"),`
		`+ platform=get_platform.return_value,`
		`),`
		`mock.call(`
		`backend="dnf",`
		`arch=["x86_64", "noarch"],`
		`lookaside={},`
		`repos=self._get_repo(compose.compose_id, "Everything", "x86_64"),`
		`+ platform=get_platform.return_value,`
		`),`
		`],`
		`)`
		`@@ -180,10 +192,11 @@`
		`repoclosure_phase.run_repoclosure(compose)`

		`@unittest.skipUnless(HAS_DNF, "DNF is not available")`
		`+ @mock.patch("pungi.phases.repoclosure.get_platform")`
		`@mock.patch("pungi.wrappers.repoclosure.get_repoclosure_cmd")`
		`@mock.patch("pungi.phases.repoclosure.run")`
		`def test_repoclosure_overwrite_options_creates_correct_commands(`
		`- self, mock_run, mock_grc`
		`+ self, mock_run, mock_grc, mock_get_platform`
		`):`
		`compose = DummyCompose(`
		`self.topdir,`
		`@@ -206,12 +219,14 @@`
		`arch=["amd64", "x86_64", "noarch"],`
		`lookaside={},`
		`repos=self._get_repo(compose.compose_id, "Server", "amd64"),`
		`+ platform=mock_get_platform.return_value,`
		`),`
		`mock.call(`
		`backend="dnf",`
		`arch=["x86_64", "noarch"],`
		`lookaside={},`
		`repos=self._get_repo(compose.compose_id, "Server", "x86_64"),`
		`+ platform=mock_get_platform.return_value,`
		`),`
		`],`
		`)`

tests/test_repoclosure_wrapper.py

file modified

+6 -1

		`@@ -59,7 +59,11 @@`
		`lookaside = {"fedora": "http://kojipkgs.fp.o/repo"}`

		`cmd = rc.get_repoclosure_cmd(`
		`- backend="dnf", arch="x86_64", repos=repos, lookaside=lookaside`
		`+ backend="dnf",`
		`+ arch="x86_64",`
		`+ repos=repos,`
		`+ lookaside=lookaside,`
		`+ platform="el8",`
		`)`
		`self.assertEqual(cmd[:2], ["dnf", "repoclosure"])`
		`six.assertCountEqual(`
		`@@ -73,6 +77,7 @@`
		`"--repo=my-repo",`
		`"--check=my-repo",`
		`"--repo=fedora",`
		`+ "--setopt=module_platform_id=platform:el8",`
		`],`
		`)`

tests/test_util.py

file modified

+23 -1

		`@@ -16,7 +16,7 @@`
		`from pungi import compose`
		`from pungi import util`

		`- from tests.helpers import touch, PungiTestCase, mk_boom`
		`+ from tests.helpers import touch, PungiTestCase, mk_boom, DummyCompose, MockModule`


		`class TestGitRefResolver(unittest.TestCase):`
		`@@ -1065,3 +1065,25 @@`
		`with util.as_local_file("file:///tmp/foo") as fn:`
		`self.assertEqual(fn, "/tmp/foo")`
		`self.assertEqual(urlretrieve.call_args_list, [])`
		`+`
		`+`
		`+ class TestGetPlatform(PungiTestCase):`
		`+ def setUp(self):`
		`+ super(TestGetPlatform, self).setUp()`
		`+ self.compose = DummyCompose(self.topdir, {})`
		`+ self.variant = self.compose.variants["Server"]`
		`+`
		`+ def test_no_modules(self):`
		`+ plat = util.get_platform(self.compose, self.variant, "x86_64", [])`
		`+ self.assertIsNone(plat)`
		`+`
		`+ def test_more_than_one_platform(self):`
		`+ self.variant.arch_mmds["x86_64"] = {`
		`+ "mod:1": MockModule("mod", platform="f29"),`
		`+ "mod:2": MockModule("mod", platform="f30"),`
		`+ }`
		`+`
		`+ with self.assertRaises(RuntimeError) as ctx:`
		`+ util.get_platform(self.compose, self.variant, "x86_64", [])`
		`+`
		`+ self.assertIn("conflicting requests for platform", str(ctx.exception))`

lsedlar commented a year ago

With dnf repoclosure backend, try to guess what platform should be provided for modular dependencies.

lsedlar commented a year ago

The motivation is that on RHEL 7, if there are modules in any of the repos, dnf will spit out a couple warnings per module. It can add up to a lot.

The behaviour on RHEL 8 is strange. dnf repoclosure seems to complain also about modules that are installed on the host, and not present in the checked repo.

So checking a repo with no modules still produces warnings, and adding the option makes those go away.

I think there's a repoclosure bug involved here.

ktdreyer commented on line 78 of pungi/util.py a year ago

You could raise the full list here to make troubleshooting easier:

raise RuntimeError("Multiple conflicting platforms: %s" % platforms)

ktdreyer commented a year ago

I've tested this on RHEL 7. It works for composing RH Ceph Storage 5 with RHEL 8.

lsedlar commented a year ago

I have submitted https://bugzilla.redhat.com/show_bug.cgi?id=2183091
I feel like the approach here might be correct, but there's not good way to get repoclosure working on RHEL 8 at the moment. It's mixing modules installed on the host into the checking.