diff --git a/module_build_service/resolver/KojiResolver.py b/module_build_service/resolver/KojiResolver.py index 6c627cc0..ad223eca 100644 --- a/module_build_service/resolver/KojiResolver.py +++ b/module_build_service/resolver/KojiResolver.py @@ -59,6 +59,59 @@ class KojiResolver(DBResolver): return result + def _filter_based_on_real_stream_name(self, koji_session, module_builds, stream): + """ + Query Koji for real stream name of each module and keep only those matching `stream`. + + This needs to be done, because MBS stores the stream name in the "version" field in Koji, + but the "version" field cannot contain "-" character. Therefore MBS replaces all "-" + with "_". This makes it impossible to reconstruct the original stream name from the + "version" field. + + We therefore need to ask for real original stream name here and filter out modules based + on this real stream name. + + :param KojiSession koji_session: Koji session. + :param list module_builds: List of builds as returned by KojiSession.listTagged method. + :param str stream: The requested stream name. + :return list: Filtered list of builds. + """ + # We need to import here because of circular dependencies. + from module_build_service.builder.KojiModuleBuilder import koji_multicall_map + + # Return early if there are no module builds. + if not module_builds: + return [] + + # Prepare list of build ids to pass them to Koji multicall later. + build_ids = [b["build_id"] for b in module_builds] + + # Get the Koji builds from Koji. + koji_builds = koji_multicall_map(koji_session, koji_session.getBuild, build_ids) + if not koji_builds: + raise RuntimeError("Error during Koji multicall when filtering KojiResolver builds.") + + # Filter out modules with different stream in the Koji build metadata. + ret = [] + for module_build, koji_build in zip(module_builds, koji_builds): + koji_build_stream = koji_build.get("extra", {}).get("typeinfo", {}).get("module", {}).\ + get("stream") + if not koji_build_stream: + log.warning( + "Not filtering out Koji build with id %d - it has no \"stream\" set in its " + "metadata." % koji_build["build_id"]) + ret.append(module_build) + continue + + if koji_build_stream == stream: + ret.append(module_build) + else: + log.info( + "Filtering out Koji build %d - its stream \"%s\" does not match the requested " + "stream \"%s\"" % (koji_build["build_id"], stream, koji_build_stream)) + + return ret + def get_buildrequired_koji_builds(self, name, stream, base_module_mmd): """ Returns list of Koji build dicts of all module builds with `name` and `stream` which are @@ -89,13 +142,23 @@ class KojiResolver(DBResolver): module_builds = koji_session.listTagged( tag, inherit=True, type="module", package=name, event=event["id"]) - # Filter out different streams + # Filter out different streams. Note that the stream name in the b["version"] is + # normalized. This makes it impossible to find out its original value. We therefore + # filter out only completely different stream names here to reduce the `module_builds` + # dramatically, but the resulting `module_builds` list might still contain unwanted + # streams. We will get rid of them using the `_filter_based_on_real_stream_name` method + # later. + # Example of such streams: "fedora-30" and "fedora_30". They will both be normalized to + # "fedora_30". normalized_stream = stream.replace("-", "_") module_builds = [b for b in module_builds if b["version"] == normalized_stream] # Filter out builds inherited from non-top tag module_builds = self._filter_inherited(koji_session, module_builds, tag, event) + # Filter out modules based on the real stream name. + module_builds = self._filter_based_on_real_stream_name(koji_session, module_builds, stream) + # Find the latest builds of all modules. This does the following: # - Sorts the module_builds descending by Koji NVR (which maps to NSV # for modules). Split release into modular version and context, and diff --git a/tests/test_resolver/test_koji.py b/tests/test_resolver/test_koji.py index 2e46c57e..b6bb178e 100644 --- a/tests/test_resolver/test_koji.py +++ b/tests/test_resolver/test_koji.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # SPDX-License-Identifier: MIT import pytest -from mock import patch +from mock import patch, MagicMock from datetime import datetime import module_build_service.resolver as mbs_resolver @@ -70,6 +70,7 @@ class TestLocalResolverModule: # No package with such name tagged. koji_session.listTagged.return_value = [] + koji_session.multiCall.return_value = [[]] self._create_test_modules(db_session) platform = db_session.query(ModuleBuild).filter_by(stream="f30.1.3").one() @@ -95,6 +96,9 @@ class TestLocalResolverModule: "release": "20170109091357.7c29193d", "tag_name": "foo-test" }] + koji_session.multiCall.return_value = [ + [build] for build in koji_session.listTagged.return_value] + self._create_test_modules(db_session) platform = db_session.query(ModuleBuild).filter_by(stream="f30.1.3").one() resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji") @@ -119,6 +123,9 @@ class TestLocalResolverModule: "release": "20170109091357.7c29193d", "tag_name": "foo-test" }] + koji_session.multiCall.return_value = [ + [build] for build in koji_session.listTagged.return_value] + self._create_test_modules(db_session) platform = db_session.query(ModuleBuild).filter_by(stream="f30.1.3").one() resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji") @@ -152,6 +159,9 @@ class TestLocalResolverModule: "release": "20160109091357.7c29193d", "tag_name": "foo-test" }] + koji_session.multiCall.return_value = [ + [build] for build in koji_session.listTagged.return_value] + self._create_test_modules(db_session) platform = db_session.query(ModuleBuild).filter_by(stream="f30.1.3").one() resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji") @@ -177,6 +187,9 @@ class TestLocalResolverModule: "release": "20170109091357.7c29193d", "tag_name": "foo-test" }] + koji_session.multiCall.return_value = [ + [build] for build in koji_session.listTagged.return_value] + self._create_test_modules(db_session) platform = db_session.query(ModuleBuild).filter_by(stream="f30.1.3").one() resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji") @@ -216,6 +229,44 @@ class TestLocalResolverModule: "testmodule-master-20170110091357.7c29193d", "testmodule-2-20180109091357.7c29193d"} + @patch("module_build_service.builder.KojiModuleBuilder.koji_multicall_map") + def test_filter_based_on_real_stream_name(self, koji_multicall_map, db_session): + koji_session = MagicMock() + koji_multicall_map.return_value = [ + {"build_id": 124, "extra": {"typeinfo": {"module": {"stream": "foo-test"}}}}, + {"build_id": 125, "extra": {"typeinfo": {"module": {"stream": "foo_test"}}}}, + {"build_id": 126, "extra": {"typeinfo": {"module": {"stream": "foo-test"}}}}, + {"build_id": 127, "extra": {"typeinfo": {"module": {}}}}, + ] + + builds = [ + {"build_id": 124, "name": "testmodule", "version": "foo_test"}, + {"build_id": 125, "name": "testmodule", "version": "foo_test"}, + {"build_id": 126, "name": "testmodule", "version": "foo_test"}, + {"build_id": 127, "name": "testmodule", "version": "foo_test"}, + ] + + resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji") + new_builds = resolver._filter_based_on_real_stream_name(koji_session, builds, "foo-test") + + build_ids = {b["build_id"] for b in new_builds} + assert build_ids == {124, 126, 127} + + @patch("module_build_service.builder.KojiModuleBuilder.koji_multicall_map") + def test_filter_based_on_real_stream_name_multicall_error( + self, koji_multicall_map, db_session): + koji_session = MagicMock() + koji_multicall_map.return_value = None + + builds = [ + {"build_id": 124, "name": "testmodule", "version": "foo_test"}, + ] + + expected_error = "Error during Koji multicall when filtering KojiResolver builds." + resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji") + with pytest.raises(RuntimeError, match=expected_error): + resolver._filter_based_on_real_stream_name(koji_session, builds, "foo-test") + def test_get_compatible_base_module_modulemds_fallback_to_dbresolver(self, db_session): tests.init_data(1, multiple_stream_versions=True) resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji") diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index 781e35f7..cce9c206 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -1694,6 +1694,9 @@ class TestUtilsModuleReuse: "release": "20170109091357.78e4a6fd", "tag_name": "module-fedora-27-build" }] + koji_session.multiCall.return_value = [ + [build] for build in koji_session.listTagged.return_value] + # Mark platform:f28 as KojiResolver ready by defining "koji_tag_with_modules". # Also define the "virtual_streams" to possibly confuse the get_reusable_module. platform_f28 = db_session.query(models.ModuleBuild).filter_by(name="platform").one()