From 2778c397657a0cfa0e129e38c22538244113dc76 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Jun 17 2019 05:25:50 +0000 Subject: Allow buildrequiring modules built against all platform streams. This commit fixes issue with following situation: Module `foo` has been built with `buildrequires: platform: [f28]` and `requires: platform: []`. It can therefore be used as a buildrequirement on any platform stream. But if you want to build module `app` which buildrequires `foo` on `platform:f30`, the MBS won't pull-in the `foo` module build, because MBS currently limits the modules by the platform they have been built for. This commit adds new config option to allow including modules built against any platform stream. --- diff --git a/module_build_service/config.py b/module_build_service/config.py index 1262b27..d0e62b0 100644 --- a/module_build_service/config.py +++ b/module_build_service/config.py @@ -433,6 +433,13 @@ class Config(object): "default": {}, "desc": "Per base-module name:stream Koji arches list.", }, + "allow_only_compatible_base_modules": { + "type": bool, + "default": True, + "desc": "When True, only modules built on top of compatible base modules are " + "considered by MBS as possible buildrequirement. When False, modules " + "built against any base module stream can be used as a buildrequire.", + }, "koji_cg_tag_build": { "type": bool, "default": True, diff --git a/module_build_service/models.py b/module_build_service/models.py index a7a5b5a..0434c7a 100644 --- a/module_build_service/models.py +++ b/module_build_service/models.py @@ -399,6 +399,9 @@ class ModuleBuild(MBSBase): :param int stream_version: the stream version to filter on :return: the query with the added stream version filter """ + if not stream_version: + return query + # Compute the minimal stream_version. For example, for `stream_version` 281234, # the minimal `stream_version` is 280000. min_stream_version = (stream_version // 10000) * 10000 @@ -436,7 +439,8 @@ class ModuleBuild(MBSBase): modules_with_virtual_streams, ModuleBuild.id == modules_with_virtual_streams.c.id) @staticmethod - def get_last_builds_in_stream_version_lte(session, name, stream_version, virtual_streams=None): + def get_last_builds_in_stream_version_lte( + session, name, stream_version=None, virtual_streams=None): """ Returns the latest builds in "ready" state for given name:stream limited by `stream_version`. The `stream_version` is int generated by `get_stream_version(...)` @@ -446,7 +450,8 @@ class ModuleBuild(MBSBase): :param session: SQLAlchemy session. :param str name: Name of the module to search builds for. - :param int stream_version: Maximum stream_version to search builds for. + :param int stream_version: Maximum stream_version to search builds for. When None, + the stream_version is not limited. :param list virtual_streams: A list of the virtual streams to filter on. The filtering uses "or" logic. When falsy, no filtering occurs. """ diff --git a/module_build_service/resolver/DBResolver.py b/module_build_service/resolver/DBResolver.py index 16b0804..f176abe 100644 --- a/module_build_service/resolver/DBResolver.py +++ b/module_build_service/resolver/DBResolver.py @@ -127,6 +127,9 @@ class DBResolver(GenericResolver): stream_version = models.ModuleBuild.get_stream_version(stream) builds = models.ModuleBuild.get_last_builds_in_stream_version_lte( session, name, stream_version, virtual_streams) + elif not stream_version_lte and virtual_streams: + builds = models.ModuleBuild.get_last_builds_in_stream_version_lte( + session, name, None, virtual_streams) else: builds = models.ModuleBuild.get_last_builds_in_stream(session, name, stream) else: diff --git a/module_build_service/utils/mse.py b/module_build_service/utils/mse.py index afd39f3..1ba956e 100644 --- a/module_build_service/utils/mse.py +++ b/module_build_service/utils/mse.py @@ -263,8 +263,14 @@ def _get_base_module_mmds(mmd): virtual_streams = xmd["mbs"]["virtual_streams"] + if conf.allow_only_compatible_base_modules: + stream_version_lte = True + else: + stream_version_lte = False + mmds = resolver.get_module_modulemds( - name, stream, stream_version_lte=True, virtual_streams=virtual_streams) + name, stream, stream_version_lte=stream_version_lte, + virtual_streams=virtual_streams) 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 diff --git a/tests/__init__.py b/tests/__init__.py index 5dccef9..ceb181e 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -112,20 +112,23 @@ def clean_database(add_platform_module=True): import_mmd(db.session, mmd) -def init_data(data_size=10, contexts=False, multiple_stream_versions=False, scratch=False): +def init_data(data_size=10, contexts=False, multiple_stream_versions=None, scratch=False): """ Creates data_size * 3 modules in database in different states and with different component builds. See _populate_data for more info. :param bool contexts: If True, multiple streams and contexts in each stream are generated for 'nginx' module. - :param bool multiple_stream_versions: If true, multiple base modules with - difference stream versions are generated. + :param list/bool multiple_stream_versions: If true, multiple base modules with + difference stream versions are generated. If set to list, the list defines + the generated base module streams. """ clean_database() if multiple_stream_versions: + if multiple_stream_versions is True: + multiple_stream_versions = ["f28.0.0", "f29.0.0", "f29.1.0", "f29.2.0"] mmd = load_mmd_file(os.path.join(base_dir, "staged_data", "platform.yaml")) - for stream in ["f28.0.0", "f29.0.0", "f29.1.0", "f29.2.0"]: + for stream in multiple_stream_versions: mmd = mmd.copy("platform", stream) # Set the virtual_streams based on "fXY" to mark the platform streams diff --git a/tests/test_mmd_resolver.py b/tests/test_mmd_resolver.py index 32d275b..6017530 100644 --- a/tests/test_mmd_resolver.py +++ b/tests/test_mmd_resolver.py @@ -386,3 +386,26 @@ class TestMMDResolver: ]) assert expanded == expected + + def test_solve_requires_any_platform(self,): + """ + Tests that MMDResolver finds out the buildrequired module `foo` even if + it was built on newer platform stream, but can run on any platform stream. + """ + modules = ( + ("platform:f28:0:c0", {}, {}), + ("platform:f29:0:c0", {}, {}), + ("platform:f30:0:c0", {}, {}), + ("foo:1:0:c8", {"platform": []}, ["platform:f29"]), + ) + for n, req, xmd_req in modules: + self.mmd_resolver.add_modules(self._make_mmd(n, req, xmd_req)) + + app = self._make_mmd("app:1:0", {"platform": ["f28"], "foo": ["1"]}) + expanded = self.mmd_resolver.solve(app) + + expected = set([ + frozenset(["foo:1:0:c8:x86_64", "app:1:0:0:src", "platform:f28:0:c0:x86_64"]), + ]) + + assert expanded == expected diff --git a/tests/test_utils/test_utils_mse.py b/tests/test_utils/test_utils_mse.py index aeaf2c7..d99bac6 100644 --- a/tests/test_utils/test_utils_mse.py +++ b/tests/test_utils/test_utils_mse.py @@ -20,6 +20,7 @@ import os +from mock import patch, PropertyMock import pytest import module_build_service.utils @@ -451,3 +452,31 @@ class TestUtilsModuleStreamExpansion: for mmd_ in mmds: actual.add("{}:{}".format(mmd_.get_module_name(), mmd_.get_stream_name())) assert actual == expected + + @patch( + "module_build_service.config.Config.allow_only_compatible_base_modules", + new_callable=PropertyMock, return_value=False + ) + def test__get_base_module_mmds_virtual_streams_only_major_versions(self, cfg): + """Ensure the correct results are returned without duplicates.""" + init_data(data_size=1, multiple_stream_versions=["foo28", "foo29", "foo30"]) + mmd = module_build_service.utils.load_mmd_file( + os.path.join(base_dir, "staged_data", "testmodule_v2.yaml")) + deps = mmd.get_dependencies()[0] + new_deps = Modulemd.Dependencies() + for stream in deps.get_runtime_streams("platform"): + new_deps.add_runtime_stream("platform", stream) + new_deps.add_buildtime_stream("platform", "foo29") + mmd.remove_dependencies(deps) + mmd.add_dependencies(new_deps) + + mmds = module_build_service.utils.mse._get_base_module_mmds(mmd) + expected = set(["platform:foo28", "platform:foo29", "platform:foo30"]) + + # Verify no duplicates were returned before doing set operations + assert len(mmds) == len(expected) + # Verify the expected ones were returned + actual = set() + for mmd_ in mmds: + actual.add("{}:{}".format(mmd_.get_module_name(), mmd_.get_stream_name())) + assert actual == expected