diff --git a/module_build_service/scheduler/default_modules.py b/module_build_service/scheduler/default_modules.py index 46f14ec2..2f15fdec 100644 --- a/module_build_service/scheduler/default_modules.py +++ b/module_build_service/scheduler/default_modules.py @@ -124,14 +124,9 @@ def add_default_modules(db_session, mmd, arches): buildrequires.update(resolved) defaults_added = True - # For now, we only want to run _handle_collisions if default modules were added, otherwise - # still rely on the old approach of running ursine.handle_stream_collision_modules. This is - # done in the init handler. if defaults_added: mmd.set_xmd(xmd) - # For now, only handle collisions when defaults are used. In the future, this can be enabled - # for all module builds when Ursa-Major is no longer supported. - _handle_collisions(mmd, arches) + return defaults_added def _get_default_modules(stream, default_modules_scm_url): @@ -208,7 +203,7 @@ def _get_rawhide_version(): return build_target["build_tag_name"].partition("-build")[0] -def _handle_collisions(mmd, arches): +def handle_collisions_with_base_module_rpms(mmd, arches): """ Find any RPMs in the buildrequired base modules that collide with the buildrequired modules. @@ -269,10 +264,14 @@ def _handle_collisions(mmd, arches): if rpm_name in name_to_nevras: conflicts = conflicts | name_to_nevras[rpm_name] - # Setting these values will keep ursine.handle_stream_collision_modules from running. - # These values are handled in KojiModuleBuilder.get_disttag_srpm. - xmd["mbs"]["ursine_rpms"] = list(conflicts) - xmd["mbs"]["stream_collision_modules"] = [] + if not conflicts: + return + + # Append the conflicting NEVRAs to `ursine_rpms`, so the Conflicts are later generated for them + # in the KojiModuleBuilder. + if not xmd["mbs"].get("ursine_rpms"): + xmd["mbs"]["ursine_rpms"] = [] + xmd["mbs"]["ursine_rpms"] = list(set(xmd["mbs"]["ursine_rpms"]).union(conflicts)) mmd.set_xmd(xmd) diff --git a/module_build_service/scheduler/handlers/modules.py b/module_build_service/scheduler/handlers/modules.py index e2687ef2..0ab00945 100644 --- a/module_build_service/scheduler/handlers/modules.py +++ b/module_build_service/scheduler/handlers/modules.py @@ -38,7 +38,8 @@ from module_build_service.utils import ( ) from module_build_service.errors import UnprocessableEntity, Forbidden, ValidationError from module_build_service.utils.greenwave import greenwave -from module_build_service.scheduler.default_modules import add_default_modules +from module_build_service.scheduler.default_modules import ( + add_default_modules, handle_collisions_with_base_module_rpms) from module_build_service.utils.submit import format_mmd from module_build_service.utils.ursine import handle_stream_collision_modules @@ -177,7 +178,7 @@ def init(config, db_session, msg): try: mmd = build.mmd() arches = [arch.name for arch in build.arches] - add_default_modules(db_session, mmd, arches) + defaults_added = add_default_modules(db_session, mmd, arches) record_module_build_arches(mmd, build, db_session) # Format the modulemd by putting in defaults and replacing streams that @@ -186,9 +187,16 @@ def init(config, db_session, msg): record_component_builds(db_session, mmd, build) # The ursine.handle_stream_collision_modules is Koji specific. - if conf.system in ["koji", "test"]: + # It is also run only when Ursa Prime is not enabled for the base + # module (`if not defaults_added`). + if conf.system in ["koji", "test"] and not defaults_added: handle_stream_collision_modules(db_session, mmd) + # Extends the xmd["mbs"]["ursine_rpms"] with RPMs from base module which conflict + # with the RPMs from module. We need to prefer modular RPMs over base module RPMs + # even if their NVR is lower. + handle_collisions_with_base_module_rpms(mmd, arches) + mmd = record_filtered_rpms(db_session, mmd) build.modulemd = mmd_to_str(mmd) build.transition(db_session, conf, models.BUILD_STATES["wait"]) diff --git a/tests/test_scheduler/test_default_modules.py b/tests/test_scheduler/test_default_modules.py index 4d99b724..582dee52 100644 --- a/tests/test_scheduler/test_default_modules.py +++ b/tests/test_scheduler/test_default_modules.py @@ -31,7 +31,7 @@ from module_build_service.utils.general import load_mmd, mmd_to_str from tests import clean_database, conf, make_module_in_db, read_staged_data -@patch("module_build_service.scheduler.default_modules._handle_collisions") +@patch("module_build_service.scheduler.default_modules.handle_collisions_with_base_module_rpms") @patch("module_build_service.scheduler.default_modules._get_default_modules") def test_add_default_modules(mock_get_dm, mock_hc, db_session): """ @@ -64,7 +64,7 @@ def test_add_default_modules(mock_get_dm, mock_hc, db_session): "python": "3", "ruby": "2.6", } - default_modules.add_default_modules(db_session, mmd, ["x86_64"]) + defaults_added = default_modules.add_default_modules(db_session, mmd, ["x86_64"]) # Make sure that the default modules were added. ruby:2.6 will be ignored since it's not in # the database assert set(mmd.get_xmd()["mbs"]["buildrequires"].keys()) == {"nodejs", "platform", "python"} @@ -72,7 +72,8 @@ def test_add_default_modules(mock_get_dm, mock_hc, db_session): "f28", "https://pagure.io/releng/fedora-module-defaults.git", ) - mock_hc.assert_called_once() + assert "ursine_rpms" not in mmd.get_xmd()["mbs"] + assert defaults_added is True @patch("module_build_service.scheduler.default_modules._get_default_modules") @@ -221,17 +222,24 @@ def test_get_rawhide_version(mock_koji_builder): assert default_modules._get_rawhide_version() == "f32" +@pytest.mark.parametrize("ursine_rpms", ([], ["httpd-0:2.4-5.el8.x86_64"])) @patch("module_build_service.scheduler.default_modules.KojiModuleBuilder.get_session") @patch("module_build_service.scheduler.default_modules._get_rpms_from_tags") -def test_handle_collisions(mock_grft, mock_get_session): +def test_handle_collisions_with_base_module_rpms( + mock_grft, mock_get_session, ursine_rpms): """ - Test that _handle_collisions will add conflicts for NEVRAs in the modulemd. + Test that handle_collisions_with_base_module_rpms will add conflicts for NEVRAs in the + modulemd. """ mmd = load_mmd(read_staged_data("formatted_testmodule.yaml")) xmd = mmd.get_xmd() xmd["mbs"]["buildrequires"]["platform"]["koji_tag"] = "module-el-build" xmd["mbs"]["buildrequires"]["python"] = {"koji_tag": "module-python27"} xmd["mbs"]["buildrequires"]["bash"] = {"koji_tag": "module-bash"} + if ursine_rpms: + # There might already be some ursine RPMs set from another methods. + # We must check they are not overwritten. + xmd["mbs"]["ursine_rpms"] = ursine_rpms mmd.set_xmd(xmd) bm_rpms = { @@ -249,15 +257,15 @@ def test_handle_collisions(mock_grft, mock_get_session): } mock_grft.side_effect = [bm_rpms, non_bm_rpms] - default_modules._handle_collisions(mmd, ["aarch64", "x86_64"]) + default_modules.handle_collisions_with_base_module_rpms(mmd, ["aarch64", "x86_64"]) mock_get_session.assert_called_once() xmd_mbs = mmd.get_xmd()["mbs"] - assert set(xmd_mbs["ursine_rpms"]) == { + assert set(xmd_mbs["ursine_rpms"]) == set(ursine_rpms).union({ "bash-0:4.4.19-7.el8.aarch64", "python2-tools-0:2.7.16-11.el8.aarch64", "python2-tools-0:2.7.16-11.el8.x86_64", - } + }) mock_grft.mock_calls == [ call( mock_get_session.return_value, diff --git a/tests/test_scheduler/test_module_init.py b/tests/test_scheduler/test_module_init.py index c61e7965..74d1bc13 100644 --- a/tests/test_scheduler/test_module_init.py +++ b/tests/test_scheduler/test_module_init.py @@ -59,8 +59,11 @@ class TestModuleInit: ) @patch("module_build_service.scm.SCM") @patch("module_build_service.scheduler.handlers.modules.handle_stream_collision_modules") + @patch( + "module_build_service.scheduler.handlers.modules.handle_collisions_with_base_module_rpms" + ) @patch("module_build_service.utils.submit.get_build_arches", return_value=["x86_64"]) - def init_basic(self, db_session, get_build_arches, rscm, mocked_scm, built_rpms): + def init_basic(self, db_session, get_build_arches, hcwbmr, rscm, mocked_scm, built_rpms): FakeSCM( mocked_scm, "testmodule", @@ -102,6 +105,7 @@ class TestModuleInit: "foo-0:2.4.48-3.el8+1308+551bfa71", "bar-0:2.5.48-3.el8+1308+551bfa71", ] + hcwbmr.assert_called_once() return build def test_init_called_twice(self, db_session):