Handle the conflicts between base module modular Koji tags everytime.

Currently, we generate `Conflicts` for ursine RPMs conflicting with
modular RPMs only when Ursa Prime is used for the base module. This
commit changes it, so these Conflicts are generated everytime.

The reason is that modular RPMs should always be preferred in the
buildroot over the ursine RPMs no matter what is their NVR. So far,
this has been guarded on Koji side by using external repos, but
we need to move away from external repo or at least use "bare"
merge mode which basically means we won't get this feature for free
from Koji.

The reason why we need to move away from external repos or use "bare"
merge mode is that without this, the Koji removes RPMs sharing the same
name but different version/release from the buildroot and only keeps
the latest one. This is an issue in situation when you need two
versions of single RPM in a buildroot comming from two modules.

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)
 
 

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"])

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,

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):