diff --git a/module_build_service/models.py b/module_build_service/models.py index b3fae068..80c2f16a 100644 --- a/module_build_service/models.py +++ b/module_build_service/models.py @@ -460,7 +460,7 @@ class ModuleBuild(MBSBase): @staticmethod def get_last_builds_in_stream_version_lte( - session, name, stream_version=None, virtual_streams=None): + session, name, stream_version=None, virtual_streams=None, states=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(...)` @@ -475,10 +475,11 @@ class ModuleBuild(MBSBase): :param list virtual_streams: A list of the virtual streams to filter on. The filtering uses "or" logic. When falsy, no filtering occurs. """ + states = states or [BUILD_STATES["ready"]] query = ( session.query(ModuleBuild) .filter(ModuleBuild.name == name) - .filter(ModuleBuild.state == BUILD_STATES["ready"]) + .filter(ModuleBuild.state.in_(states)) .order_by(sqlalchemy.cast(ModuleBuild.version, db.BigInteger).desc()) ) diff --git a/module_build_service/resolver/DBResolver.py b/module_build_service/resolver/DBResolver.py index ad62bb32..ba6f59ed 100644 --- a/module_build_service/resolver/DBResolver.py +++ b/module_build_service/resolver/DBResolver.py @@ -93,8 +93,6 @@ class DBResolver(GenericResolver): version=None, context=None, strict=False, - stream_version_lte=False, - virtual_streams=None, ): """ Gets the module modulemds from the resolver. @@ -106,11 +104,6 @@ class DBResolver(GenericResolver): be returned. :kwarg strict: Normally this function returns [] if no module can be found. If strict=True, then a UnprocessableEntity is raised. - :kwarg stream_version_lte: If True and if the `stream` can be transformed to - "stream version", the returned list will include all the modules with stream version - less than or equal the stream version computed from `stream`. - :kwarg virtual_streams: a list of the virtual streams to filter on. The filtering uses "or" - logic. When falsy, no filtering occurs. :return: List of Modulemd metadata instances matching the query """ if version and context: @@ -121,17 +114,7 @@ class DBResolver(GenericResolver): with models.make_session(self.config) as session: if not version and not context: - if stream_version_lte and ( - len(str(models.ModuleBuild.get_stream_version(stream, right_pad=False))) >= 5 - ): - 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) + builds = models.ModuleBuild.get_last_builds_in_stream(session, name, stream) else: raise NotImplementedError( "This combination of name/stream/version/context is not implemented") @@ -141,6 +124,48 @@ class DBResolver(GenericResolver): "Cannot find any module builds for %s:%s" % (name, stream)) return [build.mmd() for build in builds] + def get_compatible_base_module_modulemds( + self, name, stream, stream_version_lte, virtual_streams, states + ): + """ + Returns the Modulemd metadata of base modules compatible with base module + defined by `name` and `stream`. The compatibility is found out using the + stream version in case the stream is in "x.y.z" format and is limited to + single major version of stream version. + + If `virtual_streams` are defined, the compatibility is also extended to + all base module streams which share the same virtual stream. + + :param name: Name of the base module. + :param stream: Stream of the base module. + :param stream_version_lte: If True, the compatible streams are limited + by the stream version computed from `stream`. If False, even the + modules with higher stream version are returned. + :param virtual_streams: List of virtual streams. If set, also modules + with incompatible stream version are returned in case they share + one of the virtual streams. + :param states: List of states the returned compatible modules should + be in. + """ + builds = [] + with models.make_session(self.config) as session: + if stream_version_lte: + stream_in_xyz_format = len(str(models.ModuleBuild.get_stream_version( + stream, right_pad=False))) >= 5 + if not stream_in_xyz_format: + log.warning( + "Cannot get compatible base modules, because stream_version_let is used, " + "but stream %r is not in x.y.z format." % stream) + return [] + stream_version = models.ModuleBuild.get_stream_version(stream) + builds = models.ModuleBuild.get_last_builds_in_stream_version_lte( + session, name, stream_version, virtual_streams, states) + else: + builds = models.ModuleBuild.get_last_builds_in_stream_version_lte( + session, name, None, virtual_streams, states) + + return [build.mmd() for build in builds] + def get_buildrequired_modulemds(self, name, stream, base_module_nsvc): """ Returns modulemd metadata of all module builds with `name` and `stream` buildrequiring diff --git a/module_build_service/resolver/MBSResolver.py b/module_build_service/resolver/MBSResolver.py index 3d8bda1d..c27633cc 100644 --- a/module_build_service/resolver/MBSResolver.py +++ b/module_build_service/resolver/MBSResolver.py @@ -46,7 +46,7 @@ class MBSResolver(GenericResolver): self.mbs_prod_url = config.mbs_url self._generic_error = "Failed to query MBS with query %r returned HTTP status %s" - def _query_from_nsvc(self, name, stream, version=None, context=None, state="ready"): + def _query_from_nsvc(self, name, stream, version=None, context=None, states=None): """ Generates dict with MBS params query. @@ -55,10 +55,11 @@ class MBSResolver(GenericResolver): :param str version/int: Version of the module to query. :param str context: Context of the module to query. """ + states = states or ["ready"] query = { "name": name, "stream": stream, - "state": state, + "state": states, "verbose": True, "order_desc_by": "version", } @@ -69,7 +70,7 @@ class MBSResolver(GenericResolver): return query def _get_modules( - self, name, stream, version=None, context=None, state="ready", strict=False, **kwargs + self, name, stream, version=None, context=None, states=None, strict=False, **kwargs ): """Query and return modules from MBS with specific info @@ -85,7 +86,7 @@ class MBSResolver(GenericResolver): :rtype: list[dict] :raises UnprocessableEntity: if no modules are found and ``strict`` is True. """ - query = self._query_from_nsvc(name, stream, version, context, state) + query = self._query_from_nsvc(name, stream, version, context, states) query["page"] = 1 query["per_page"] = 10 query.update(kwargs) @@ -118,8 +119,8 @@ class MBSResolver(GenericResolver): else: return modules - def get_module(self, name, stream, version, context, state="ready", strict=False): - rv = self._get_modules(name, stream, version, context, state, strict) + def get_module(self, name, stream, version, context, states=None, strict=False): + rv = self._get_modules(name, stream, version, context, states, strict) if rv: return rv[0] @@ -173,6 +174,7 @@ class MBSResolver(GenericResolver): strict=False, stream_version_lte=False, virtual_streams=None, + states=None, ): """ Gets the module modulemds from the resolver. @@ -207,7 +209,8 @@ class MBSResolver(GenericResolver): if virtual_streams: extra_args["virtual_stream"] = virtual_streams - modules = self._get_modules(name, stream, version, context, strict=strict, **extra_args) + modules = self._get_modules(name, stream, version, context, strict=strict, states=states, + **extra_args) if not modules: return [] @@ -226,6 +229,33 @@ class MBSResolver(GenericResolver): mmds.append(load_mmd(yaml)) return mmds + def get_compatible_base_module_modulemds( + self, name, stream, stream_version_lte, virtual_streams, states + ): + """ + Returns the Modulemd metadata of base modules compatible with base module + defined by `name` and `stream`. The compatibility is found out using the + stream version in case the stream is in "x.y.z" format and is limited to + single major version of stream version. + + If `virtual_streams` are defined, the compatibility is also extended to + all base module streams which share the same virtual stream. + + :param name: Name of the base module. + :param stream: Stream of the base module. + :param stream_version_lte: If True, the compatible streams are limited + by the stream version computed from `stream`. If False, even the + modules with higher stream version are returned. + :param virtual_streams: List of virtual streams. If set, also modules + with incompatible stream version are returned in case they share + one of the virtual streams. + :param states: List of states the returned compatible modules should + be in. + """ + return self.get_module_modulemds( + name, stream, stream_version_lte=stream_version_lte, virtual_streams=virtual_streams, + states=states) + def get_buildrequired_modulemds(self, name, stream, base_module_nsvc): """ Returns modulemd metadata of all module builds with `name` and `stream` buildrequiring diff --git a/module_build_service/resolver/base.py b/module_build_service/resolver/base.py index d1074a1a..7f8dbb6a 100644 --- a/module_build_service/resolver/base.py +++ b/module_build_service/resolver/base.py @@ -117,9 +117,13 @@ class GenericResolver(six.with_metaclass(ABCMeta)): stream, version=None, context=None, - strict=False, - stream_version_lte=None, - virtual_streams=None, + strict=False + ): + raise NotImplementedError() + + @abstractmethod + def get_compatible_base_module_modulemds( + self, name, stream, stream_version_lte, virtual_streams, states ): raise NotImplementedError() diff --git a/module_build_service/utils/mse.py b/module_build_service/utils/mse.py index 1ba956e5..b9b427ec 100644 --- a/module_build_service/utils/mse.py +++ b/module_build_service/utils/mse.py @@ -211,11 +211,12 @@ def _get_base_module_mmds(mmd): old versions of the base module based on the stream version. :param Modulemd mmd: Input modulemd metadata. - :rtype: list of Modulemd - :return: List of MMDs of base modules buildrequired by `mmd`. + :rtype: dict with lists of Modulemd + :return: Dict with "ready" or "garbage" state name as a key and list of MMDs of base modules + buildrequired by `mmd` as a value. """ seen = set() - ret = [] + ret = {"ready": [], "garbage": []} resolver = module_build_service.resolver.system_resolver for deps in mmd.get_dependencies(): @@ -258,30 +259,34 @@ def _get_base_module_mmds(mmd): # `stream_mmd`. if not virtual_streams: seen.add(ns) - ret.append(stream_mmd) + ret["ready"].append(stream_mmd) continue virtual_streams = xmd["mbs"]["virtual_streams"] if conf.allow_only_compatible_base_modules: stream_version_lte = True + states = ["ready"] else: stream_version_lte = False + states = ["ready", "garbage"] - mmds = resolver.get_module_modulemds( - 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 - for mmd_ in mmds: - 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 += ret_chunk + for state in states: + mmds = resolver.get_compatible_base_module_modulemds( + name, stream, 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 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) break @@ -322,7 +327,7 @@ def get_mmds_required_by_module_recursively( # Get the MMDs of all compatible base modules based on the buildrequires. base_module_mmds = _get_base_module_mmds(mmd) - if not base_module_mmds: + if not base_module_mmds["ready"]: base_module_choices = " or ".join(conf.base_module_names) raise UnprocessableEntity( "None of the base module ({}) streams in the buildrequires section could be found" @@ -330,16 +335,22 @@ def get_mmds_required_by_module_recursively( ) # Add base modules to `mmds`. - for base_module in base_module_mmds: + for base_module in base_module_mmds["ready"]: ns = ":".join([base_module.get_module_name(), base_module.get_stream_name()]) mmds.setdefault(ns, []) mmds[ns].append(base_module) + # The currently submitted module build must be build only against "ready" base modules, + # but its dependencies might have been built against some old platform which is already + # EOL ("garbage" state). In order to find such old module builds, we need to include + # also EOL platform streams. + all_base_module_mmds = base_module_mmds["ready"] + base_module_mmds["garbage"] + # Get all the buildrequires of the module of interest. for deps in mmd.get_dependencies(): deps_dict = deps_to_dict(deps, 'buildtime') mmds = _get_mmds_from_requires( - deps_dict, mmds, False, default_streams, raise_if_stream_ambigous, base_module_mmds) + deps_dict, mmds, False, default_streams, raise_if_stream_ambigous, all_base_module_mmds) # Now get the requires of buildrequires recursively. for mmd_key in list(mmds.keys()): @@ -352,7 +363,7 @@ def get_mmds_required_by_module_recursively( True, default_streams, raise_if_stream_ambigous, - base_module_mmds, + all_base_module_mmds, ) # Make single list from dict of lists. diff --git a/tests/test_resolver/test_db.py b/tests/test_resolver/test_db.py index 5180b4ea..0ca873c7 100644 --- a/tests/test_resolver/test_db.py +++ b/tests/test_resolver/test_db.py @@ -78,16 +78,18 @@ class TestDBModule: assert nsvcs == set(["testmodule:master:20170109091357:123"]) @pytest.mark.parametrize("stream_versions", [False, True]) - def test_get_module_modulemds_stream_versions(self, stream_versions): + def test_get_compatible_base_module_modulemds_stream_versions(self, stream_versions): tests.init_data(1, multiple_stream_versions=True) resolver = mbs_resolver.GenericResolver.create(tests.conf, backend="db") - result = resolver.get_module_modulemds( - "platform", "f29.1.0", stream_version_lte=stream_versions) + result = resolver.get_compatible_base_module_modulemds( + "platform", "f29.1.0", stream_version_lte=stream_versions, virtual_streams=["f29"], + states=[models.BUILD_STATES["ready"]]) nsvcs = set([mmd.get_nsvc() for mmd in result]) if stream_versions: assert nsvcs == set(["platform:f29.1.0:3:00000000", "platform:f29.0.0:3:00000000"]) else: - assert nsvcs == set(["platform:f29.1.0:3:00000000"]) + assert nsvcs == set(["platform:f29.1.0:3:00000000", "platform:f29.0.0:3:00000000", + "platform:f29.2.0:3:00000000"]) @pytest.mark.parametrize("empty_buildrequires", [False, True]) def test_get_module_build_dependencies(self, empty_buildrequires, db_session): diff --git a/tests/test_resolver/test_mbs.py b/tests/test_resolver/test_mbs.py index 24048a04..ff022886 100644 --- a/tests/test_resolver/test_mbs.py +++ b/tests/test_resolver/test_mbs.py @@ -73,7 +73,7 @@ class TestMBSModule: "order_desc_by": "version", "page": 1, "per_page": 10, - "state": "ready", + "state": ["ready"], "virtual_stream": ["f28"], } mock_session.get.assert_called_once_with(mbs_url, params=expected_query) @@ -129,7 +129,7 @@ class TestMBSModule: "order_desc_by": "version", "page": 1, "per_page": 10, - "state": "ready", + "state": ["ready"], } mock_session.get.assert_called_once_with(mbs_url, params=expected_query) assert nsvcs == expected @@ -187,7 +187,7 @@ class TestMBSModule: "order_desc_by": "version", "page": 1, "per_page": 10, - "state": "ready", + "state": ["ready"], }, { "name": "platform", @@ -198,7 +198,7 @@ class TestMBSModule: "order_desc_by": "version", "page": 1, "per_page": 10, - "state": "ready", + "state": ["ready"], }, ] @@ -260,7 +260,7 @@ class TestMBSModule: "order_desc_by": "version", "page": 1, "per_page": 10, - "state": "ready", + "state": ["ready"], } mock_session.get.assert_called_once_with(mbs_url, params=expected_query) assert set(result) == expected @@ -336,7 +336,7 @@ class TestMBSModule: "order_desc_by": "version", "page": 1, "per_page": 10, - "state": "ready", + "state": ["ready"], } mock_session.get.assert_called_once_with(mbs_url, params=expected_query) diff --git a/tests/test_utils/test_utils_mse.py b/tests/test_utils/test_utils_mse.py index d693a628..449cf496 100644 --- a/tests/test_utils/test_utils_mse.py +++ b/tests/test_utils/test_utils_mse.py @@ -22,7 +22,7 @@ from mock import patch, PropertyMock import pytest import module_build_service.utils -from module_build_service import Modulemd +from module_build_service import Modulemd, models from module_build_service.errors import StreamAmbigous from tests import db, clean_database, make_module, init_data, read_staged_data @@ -418,10 +418,10 @@ class TestUtilsModuleStreamExpansion: mmds = module_build_service.utils.mse._get_base_module_mmds(mmd) expected = set(["platform:f29.0.0", "platform:f29.1.0", "platform:f29.2.0"]) # Verify no duplicates were returned before doing set operations - assert len(mmds) == len(expected) + assert len(mmds["ready"]) == len(expected) # Verify the expected ones were returned actual = set() - for mmd_ in mmds: + for mmd_ in mmds["ready"]: actual.add("{}:{}".format(mmd_.get_module_name(), mmd_.get_stream_name())) assert actual == expected @@ -447,10 +447,10 @@ class TestUtilsModuleStreamExpansion: else: expected = set(["platform:f29.0.0", "platform:f29.1.0", "platform:f29.2.0"]) # Verify no duplicates were returned before doing set operations - assert len(mmds) == len(expected) + assert len(mmds["ready"]) == len(expected) # Verify the expected ones were returned actual = set() - for mmd_ in mmds: + for mmd_ in mmds["ready"]: actual.add("{}:{}".format(mmd_.get_module_name(), mmd_.get_stream_name())) assert actual == expected @@ -461,6 +461,13 @@ class TestUtilsModuleStreamExpansion: 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"]) + + # Mark platform:foo28 as garbage to test that it is still considered as compatible. + platform = models.ModuleBuild.query.filter_by(name="platform", stream="foo28").first() + platform.state = "garbage" + db.session.add(platform) + db.session.commit() + mmd = module_build_service.utils.load_mmd(read_staged_data("testmodule_v2")) deps = mmd.get_dependencies()[0] new_deps = Modulemd.Dependencies() @@ -471,12 +478,16 @@ class TestUtilsModuleStreamExpansion: mmd.add_dependencies(new_deps) mmds = module_build_service.utils.mse._get_base_module_mmds(mmd) - expected = set(["platform:foo28", "platform:foo29", "platform:foo30"]) + expected = {} + expected["ready"] = set(["platform:foo29", "platform:foo30"]) + expected["garbage"] = set(["platform:foo28"]) # 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 + for k in expected.keys(): + assert len(mmds[k]) == len(expected[k]) + # Verify the expected ones were returned + actual = set() + for mmd_ in mmds[k]: + actual.add("{}:{}".format(mmd_.get_module_name(), mmd_.get_stream_name())) + assert actual == expected[k]