From 96ca7fd00ba0d2240e12e00557f6da2d7574c253 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Wed, 30 Oct 2019 14:22:33 +0100 Subject: [PATCH] Search for default modules built also against compatible base modules. --- .../scheduler/default_modules.py | 43 ++++++- module_build_service/utils/mse.py | 113 +++++++++++------- tests/test_scheduler/test_default_modules.py | 69 ++++++++++- 3 files changed, 174 insertions(+), 51 deletions(-) diff --git a/module_build_service/scheduler/default_modules.py b/module_build_service/scheduler/default_modules.py index 2bced951..6adbdb8d 100644 --- a/module_build_service/scheduler/default_modules.py +++ b/module_build_service/scheduler/default_modules.py @@ -17,6 +17,8 @@ from module_build_service.builder.KojiModuleBuilder import ( from module_build_service.errors import UnprocessableEntity from module_build_service.resolver.base import GenericResolver from module_build_service.utils import retry +from module_build_service.utils.mse import ( + get_compatible_base_module_mmds, expand_single_mse_streams) def add_default_modules(db_session, mmd, arches): @@ -86,18 +88,47 @@ def add_default_modules(db_session, mmd, arches): # Query for the latest default module that was built against this base module resolver = GenericResolver.create(db_session, conf) - default_module_mmds = resolver.get_buildrequired_modulemds(name, stream, bm_mmd) - if not default_module_mmds: + base_mmds = get_compatible_base_module_mmds(resolver, bm_mmd) + base_mmds = base_mmds["ready"] + base_mmds["garbage"] + base_mmds.sort( + key=lambda mmd: models.ModuleBuild.get_stream_version(mmd.get_stream_name(), False), + reverse=True) + for base_mmd in base_mmds: + default_module_mmds = resolver.get_buildrequired_modulemds(name, stream, base_mmd) + if not default_module_mmds: + continue + + # We need to ensure that module built against compatible base module stream + # really contains runtime-dependency on the current base module stream. + # For example in Fedora, we can have platform:f30 and platform:f31 base module + # streams. There can be foo:1 module built against platform:f30 which can work with + # any platform ("requires: platform: []"). This module can be configured as default + # module for platform:f28 and we need to support this case, but in the same time we + # cannot simply add any platform:f27 based module to platform:f28. + module_found = False + for default_module_mmd in default_module_mmds: + for deps in default_module_mmd.get_dependencies(): + streams = deps.get_runtime_streams(module_name) + if streams is None: + continue + streams = expand_single_mse_streams(db_session, module_name, streams) + if bm_info["stream"] in streams: + module_found = True + break + else: + log.info( + "Not using module %s as default module, because it does not " + "contain runtime dependency on %s", default_module_mmd.get_nsvc(), + bm_nsvc) + if module_found: + break + else: log.warning( "The default module %s from %s is not in the database and couldn't be added as " "a buildrequire", ns, bm_nsvc, ) continue - # Since a default module entry only has the name and stream, there's no way to know - # which context to pick from if multiple are present. In this case, just pick the first - # one, which is the latest version but potentially a random context. - default_module_mmd = default_module_mmds[0] # Use resolve_requires since it provides the exact format that is needed for # mbs.xmd.buildrequires resolved = resolver.resolve_requires([default_module_mmd.get_nsvc()]) diff --git a/module_build_service/utils/mse.py b/module_build_service/utils/mse.py index 96da48fc..ea460f8e 100644 --- a/module_build_service/utils/mse.py +++ b/module_build_service/utils/mse.py @@ -8,7 +8,8 @@ from module_build_service.utils.general import deps_to_dict, mmd_to_str from module_build_service.resolver import GenericResolver -def _expand_mse_streams(db_session, name, streams, default_streams, raise_if_stream_ambigous): +def expand_single_mse_streams( + db_session, name, streams, default_streams=None, raise_if_stream_ambigous=False): """ Helper method for `expand_mse_stream()` expanding single name:[streams]. Returns list of expanded streams. @@ -80,7 +81,7 @@ def expand_mse_streams(db_session, mmd, default_streams=None, raise_if_stream_am new_deps = Modulemd.Dependencies() for name in deps.get_runtime_modules(): streams = deps.get_runtime_streams(name) - new_streams = _expand_mse_streams( + new_streams = expand_single_mse_streams( db_session, name, streams, default_streams, raise_if_stream_ambigous) if not new_streams: @@ -91,7 +92,7 @@ def expand_mse_streams(db_session, mmd, default_streams=None, raise_if_stream_am for name in deps.get_buildtime_modules(): streams = deps.get_buildtime_streams(name) - new_streams = _expand_mse_streams( + new_streams = expand_single_mse_streams( db_session, name, streams, default_streams, raise_if_stream_ambigous) if not new_streams: @@ -184,6 +185,67 @@ def _get_mmds_from_requires( return mmds +def get_compatible_base_module_mmds(resolver, base_mmd, ignore_ns=None): + """ + Returns dict containing the base modules compatible with `base_mmd` grouped by their state. + + :param GenericResolver resolver: GenericResolver instance. + :param Modulemd base_mmd: Modulemd instant to return compatible modules for. + :param set ignore_ns: When set, defines the name:stream of base modules which will be ignored + by this function and therefore not returned. + :return dict: Dictionary with module's state name as a key and list of Modulemd objects for + each compatible base module in that state. For example: + { + "ready": [base_mmd_1, base_mmd_2] + "garbage": [base_mmd_3] + } + The input `base_mmd` is always included in the result in "ready" state. + """ + # Add the module to `seen` and `ret`. + ret = {"ready": [], "garbage": []} + ret["ready"].append(base_mmd) + ns = ":".join([base_mmd.get_module_name(), base_mmd.get_stream_name()]) + seen = set() if not ignore_ns else set(ignore_ns) + seen.add(ns) + + # Get the list of compatible virtual streams. + xmd = base_mmd.get_xmd() + virtual_streams = xmd.get("mbs", {}).get("virtual_streams") + + # In case there are no virtual_streams in the buildrequired name:stream, + # it is clear that there are no compatible streams, so return just this + # `base_mmd`. + if not virtual_streams: + return ret + + if conf.allow_only_compatible_base_modules: + stream_version_lte = True + states = ["ready"] + else: + stream_version_lte = False + states = ["ready", "garbage"] + + for state in states: + mmds = resolver.get_compatible_base_module_modulemds( + base_mmd, stream_version_lte, virtual_streams, + [models.BUILD_STATES[state]]) + ret_chunk = [] + # Add the returned mmds to the `seen` set to avoid querying those + # individually if they are part of the buildrequire streams for this + # base module. + for mmd_ in mmds: + mmd_ns = ":".join([mmd_.get_module_name(), mmd_.get_stream_name()]) + # An extra precaution to ensure there are no duplicates. This can happen when there + # are two modules with the same name:stream - one in ready state and one in garbage + # state. + if mmd_ns not in seen: + ret_chunk.append(mmd_) + seen.add(mmd_ns) + ret[state] += ret_chunk + + return ret + + def get_base_module_mmds(db_session, mmd): """ Returns list of MMDs of base modules buildrequired by `mmd` including the compatible @@ -227,47 +289,14 @@ def get_base_module_mmds(db_session, mmd): mmds = resolver.get_module_modulemds(name, stream) if not mmds: continue - stream_mmd = mmds[0] + base_mmd = mmds[0] - # Add the module to `seen` and `ret`. - seen.add(ns) - ret["ready"].append(stream_mmd) - - # Get the list of compatible virtual streams. - xmd = stream_mmd.get_xmd() - virtual_streams = xmd.get("mbs", {}).get("virtual_streams") - - # In case there are no virtual_streams in the buildrequired name:stream, - # it is clear that there are no compatible streams, so return just this - # `stream_mmd`. - if not virtual_streams: - continue - - if conf.allow_only_compatible_base_modules: - stream_version_lte = True - states = ["ready"] - else: - stream_version_lte = False - states = ["ready", "garbage"] - - for state in states: - mmds = resolver.get_compatible_base_module_modulemds( - stream_mmd, stream_version_lte, virtual_streams, - [models.BUILD_STATES[state]]) - ret_chunk = [] - # Add the returned mmds to the `seen` set to avoid querying those - # individually if they are part of the buildrequire streams for this - # base module. - for mmd_ in mmds: + new_ret = get_compatible_base_module_mmds(resolver, base_mmd, ignore_ns=seen) + for state in new_ret.keys(): + for mmd_ in new_ret[state]: mmd_ns = ":".join([mmd_.get_module_name(), mmd_.get_stream_name()]) - # An extra precaution to ensure there are no duplicates in the event the - # sorting above wasn't flawless - if mmd_ns not in seen: - ret_chunk.append(mmd_) - seen.add(mmd_ns) - ret[state] += ret_chunk - # Just in case it was queried for but MBS didn't find it - seen.add(ns) + seen.add(mmd_ns) + ret[state] += new_ret[state] break return ret diff --git a/tests/test_scheduler/test_default_modules.py b/tests/test_scheduler/test_default_modules.py index 46d53f78..3b6edca9 100644 --- a/tests/test_scheduler/test_default_modules.py +++ b/tests/test_scheduler/test_default_modules.py @@ -11,7 +11,7 @@ from module_build_service.errors import UnprocessableEntity from module_build_service.models import ModuleBuild from module_build_service.scheduler import default_modules 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 +from tests import clean_database, conf, make_module_in_db, read_staged_data, import_mmd @patch("module_build_service.scheduler.default_modules.handle_collisions_with_base_module_rpms") @@ -39,8 +39,13 @@ def test_add_default_modules(mock_get_dm, mock_hc, db_session): platform_mmd.set_xmd(platform_xmd) platform.modulemd = mmd_to_str(platform_mmd) - make_module_in_db("python:3:12345:1", base_module=platform, db_session=db_session) - make_module_in_db("nodejs:11:2345:2", base_module=platform, db_session=db_session) + dependencies = [ + {"requires": {"platform": ["f28"]}, + "buildrequires": {"platform": ["f28"]}}] + make_module_in_db("python:3:12345:1", base_module=platform, db_session=db_session, + dependencies=dependencies) + make_module_in_db("nodejs:11:2345:2", base_module=platform, db_session=db_session, + dependencies=dependencies) db_session.commit() mock_get_dm.return_value = { @@ -87,6 +92,64 @@ def test_add_default_modules_platform_not_available(db_session): default_modules.add_default_modules(db_session, mmd, ["x86_64"]) +@patch("module_build_service.scheduler.default_modules._get_default_modules") +def test_add_default_modules_compatible_platforms(mock_get_dm, db_session): + """ + Test that default modules built against compatible base module streams are added. + """ + clean_database(add_platform_module=False) + + # Create compatible base modules. + mmd = load_mmd(read_staged_data("platform")) + for stream in ["f27", "f28"]: + mmd = mmd.copy("platform", stream) + + # Set the virtual stream to "fedora" to make these base modules compatible. + xmd = mmd.get_xmd() + xmd["mbs"]["virtual_streams"] = ["fedora"] + xmd["mbs"]["use_default_modules"] = True + mmd.set_xmd(xmd) + import_mmd(db_session, mmd) + + mmd = load_mmd(read_staged_data("formatted_testmodule.yaml")) + xmd_brs = mmd.get_xmd()["mbs"]["buildrequires"] + assert set(xmd_brs.keys()) == {"platform"} + + platform_f27 = ModuleBuild.get_build_from_nsvc( + db_session, "platform", "f27", "3", "00000000") + assert platform_f27 + + # Create python default module which requires platform:f27 and therefore cannot be used + # as default module for platform:f28. + dependencies = [ + {"requires": {"platform": ["f27"]}, + "buildrequires": {"platform": ["f27"]}}] + make_module_in_db("python:3:12345:1", base_module=platform_f27, db_session=db_session, + dependencies=dependencies) + + # Create nodejs default module which requries any platform stream and therefore can be used + # as default module for platform:f28. + dependencies[0]["requires"]["platform"] = [] + make_module_in_db("nodejs:11:2345:2", base_module=platform_f27, db_session=db_session, + dependencies=dependencies) + db_session.commit() + + mock_get_dm.return_value = { + "nodejs": "11", + "python": "3", + "ruby": "2.6", + } + 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"} + mock_get_dm.assert_called_once_with( + "f28", + "https://pagure.io/releng/fedora-module-defaults.git", + ) + assert defaults_added is True + + @patch("module_build_service.scheduler.default_modules._get_default_modules") def test_add_default_modules_request_failed(mock_get_dm, db_session): """