From 486dc3989824eddf8c7261fb1cd795199bb3631a Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Oct 03 2019 13:14:29 +0000 Subject: Add support for KojiResolver in component reuse code. In this commit, when component reuse code finds out that the base module uses KojiResolver, it uses the `KojiResolver.get_buildrequired_modules` method to find out possible modules to reuse and limits the original query just by the IDs of these modules. In order to do that, this commit splits the original `KojiResolver.get_buildrequired_modulemds` into two methods: - The `get_buildrequired_modules` returning the ModuleBuilds. - The `get_buildrequired_modulemds` calling the `get_buildrequired_modules` and returning modulemd metadata. --- diff --git a/module_build_service/resolver/KojiResolver.py b/module_build_service/resolver/KojiResolver.py index 6bda681..6f0d844 100644 --- a/module_build_service/resolver/KojiResolver.py +++ b/module_build_service/resolver/KojiResolver.py @@ -80,24 +80,21 @@ class KojiResolver(DBResolver): return result - def get_buildrequired_modulemds(self, name, stream, base_module_mmd): + def get_buildrequired_modules(self, name, stream, base_module_mmd): """ - Returns modulemd metadata of all module builds with `name` and `stream` which are tagged + Returns ModuleBuild objects of all module builds with `name` and `stream` which are tagged in the Koji tag defined in `base_module_mmd`. :param str name: Name of module to return. :param str stream: Stream of module to return. :param Modulemd base_module_mmd: Base module metadata. - :return list: List of modulemd metadata. + :return list: List of ModuleBuilds. """ # Get the `koji_tag_with_modules`. If the `koji_tag_with_modules` is not configured for # the base module, fallback to DBResolver. tag = base_module_mmd.get_xmd().get("mbs", {}).get("koji_tag_with_modules") if not tag: - log.info( - "The %s does not define 'koji_tag_with_modules'. Falling back to DBResolver." % ( - base_module_mmd.get_nsvc())) - return DBResolver.get_buildrequired_modulemds(self, name, stream, base_module_mmd) + return [] # Create KojiSession. We need to import here because of circular dependencies. from module_build_service.builder.KojiModuleBuilder import KojiModuleBuilder @@ -146,9 +143,8 @@ class KojiResolver(DBResolver): latest_builds += list(nsv_builds) break - # For each latest module build, find the matching ModuleBuild and store its modulemd - # in `mmds`. - mmds = [] + # For each latest module build, find the matching ModuleBuild and store it into `ret`. + ret = [] for build in latest_builds: version, context = build["release"].split(".") module = models.ModuleBuild.get_build_from_nsvc( @@ -157,9 +153,29 @@ class KojiResolver(DBResolver): raise ValueError( "Module %s is tagged in the %s Koji tag, but does not exist " "in MBS DB." % (":".join([name, stream, version, context]), tag)) - mmds.append(module.mmd()) + ret.append(module) + + return ret + + def get_buildrequired_modulemds(self, name, stream, base_module_mmd): + """ + Returns modulemd metadata of all module builds with `name` and `stream` which are tagged + in the Koji tag defined in `base_module_mmd`. + + :param str name: Name of module to return. + :param str stream: Stream of module to return. + :param Modulemd base_module_mmd: Base module metadata. + :return list: List of modulemd metadata. + """ + tag = base_module_mmd.get_xmd().get("mbs", {}).get("koji_tag_with_modules") + if not tag: + log.info( + "The %s does not define 'koji_tag_with_modules'. Falling back to DBResolver." % + (base_module_mmd.get_nsvc())) + return DBResolver.get_buildrequired_modulemds(self, name, stream, base_module_mmd) - return mmds + modules = self.get_buildrequired_modules(name, stream, base_module_mmd) + return [module.mmd() for module in modules] def get_compatible_base_module_modulemds(self, *args, **kwargs): """ diff --git a/module_build_service/utils/mse.py b/module_build_service/utils/mse.py index 086e91a..eccb201 100644 --- a/module_build_service/utils/mse.py +++ b/module_build_service/utils/mse.py @@ -251,6 +251,10 @@ def get_base_module_mmds(db_session, mmd): continue stream_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") @@ -259,8 +263,6 @@ def get_base_module_mmds(db_session, mmd): # it is clear that there are no compatible streams, so return just this # `stream_mmd`. if not virtual_streams: - seen.add(ns) - ret["ready"].append(stream_mmd) continue virtual_streams = xmd["mbs"]["virtual_streams"] diff --git a/module_build_service/utils/reuse.py b/module_build_service/utils/reuse.py index 3f44f1a..ec7e07e 100644 --- a/module_build_service/utils/reuse.py +++ b/module_build_service/utils/reuse.py @@ -27,6 +27,7 @@ import kobo.rpmlib import module_build_service.messaging from module_build_service import log, models, conf from module_build_service.utils.mse import get_base_module_mmds +from module_build_service.resolver import GenericResolver def reuse_component(component, previous_component_build, change_state_now=False): @@ -93,6 +94,22 @@ def get_reusable_module(db_session, module): mmd = module.mmd() previous_module_build = None + # The `base_mmds` will contain the list of base modules against which the possible modules + # to reuse are built. There are three options how these base modules are found: + # + # 1) The `conf.allow_only_compatible_base_modules` is False. This means that MBS should + # not try to find any compatible base modules in its DB and simply use the buildrequired + # base module as it is. + # 2) The `conf.allow_only_compatible_base_modules` is True and DBResolver is used. This means + # that MBS should try to find the compatible modules using its database. + # The `get_base_module_mmds` finds out the list of compatible modules and returns mmds of + # all of them. + # 3) The `conf.allow_only_compatible_base_modules` is True and KojiResolver is used. This + # means that MBS should *not* try to find any compatible base modules in its DB, but + # instead just query Koji using KojiResolver later to find out the module to + # reuse. The list of compatible base modules is defined by Koji tag inheritance directly + # in Koji. + # The `get_base_module_mmds` in this case returns just the buildrequired base module. if conf.allow_only_compatible_base_modules: log.debug("Checking for compatible base modules") base_mmds = get_base_module_mmds(db_session, mmd)["ready"] @@ -108,22 +125,43 @@ def get_reusable_module(db_session, module): base_mmds.append(br.mmd()) for base_mmd in base_mmds: - mbs_xmd = mmd.get_xmd()["mbs"] - if base_mmd.get_module_name() not in mbs_xmd["buildrequires"]: - continue - mbs_xmd["buildrequires"][base_mmd.get_module_name()]["stream"] \ - = base_mmd.get_stream_name() - build_context = module.calculate_build_context(mbs_xmd["buildrequires"]) - # Find the latest module that is in the ready state previous_module_build = ( db_session.query(models.ModuleBuild) .filter_by(name=mmd.get_module_name()) .filter_by(stream=mmd.get_stream_name()) .filter_by(state=models.BUILD_STATES["ready"]) .filter(models.ModuleBuild.scmurl.isnot(None)) - .filter_by(build_context=build_context) .order_by(models.ModuleBuild.time_completed.desc())) + koji_resolver_enabled = base_mmd.get_xmd().get("mbs", {}).get("koji_tag_with_modules") + if koji_resolver_enabled: + # Find ModuleBuilds tagged in the Koji tag using KojiResolver. + resolver = GenericResolver.create(db_session, conf, backend="koji") + possible_modules_to_reuse = resolver.get_buildrequired_modules( + module.name, module.stream, base_mmd) + + # Limit the query to these modules. + possible_module_ids = [m.id for m in possible_modules_to_reuse] + previous_module_build = previous_module_build.filter( + models.ModuleBuild.id.in_(possible_module_ids)) + + # Limit the query to modules sharing the same `build_context_no_bms`. That means they + # have the same buildrequirements. + previous_module_build = previous_module_build.filter_by( + build_context_no_bms=module.build_context_no_bms) + else: + # Recompute the build_context with compatible base module stream. + mbs_xmd = mmd.get_xmd()["mbs"] + if base_mmd.get_module_name() not in mbs_xmd["buildrequires"]: + previous_module_build = None + continue + mbs_xmd["buildrequires"][base_mmd.get_module_name()]["stream"] \ + = base_mmd.get_stream_name() + build_context = module.calculate_build_context(mbs_xmd["buildrequires"]) + + # Limit the query to find only modules sharing the same build_context. + previous_module_build = previous_module_build.filter_by(build_context=build_context) + # If we are rebuilding with the "changed-and-after" option, then we can't reuse # components from modules that were built more liberally if module.rebuild_strategy == "changed-and-after": diff --git a/tests/conftest.py b/tests/conftest.py index 774bd3a..7639f75 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -128,9 +128,9 @@ def reuse_component_init_data(db_session): xmd["mbs"]["commit"] = "ff1ea79fc952143efeed1851aa0aa006559239ba" mmd.set_xmd(xmd) build_one.modulemd = mmd_to_str(mmd) - build_one.build_context = module_build_service.models.ModuleBuild.contexts_from_mmd( - build_one.modulemd - ).build_context + contexts = module_build_service.models.ModuleBuild.contexts_from_mmd(build_one.modulemd) + build_one.build_context = contexts.build_context + build_one.build_context_no_bms = contexts.build_context_no_bms db_session.add(build_one) db_session.commit() @@ -227,9 +227,9 @@ def reuse_component_init_data(db_session): xmd["mbs"]["commit"] = "55f4a0a2e6cc255c88712a905157ab39315b8fd8" mmd.set_xmd(xmd) build_two.modulemd = mmd_to_str(mmd) - build_two.build_context = module_build_service.models.ModuleBuild.contexts_from_mmd( - build_two.modulemd - ).build_context + contexts = module_build_service.models.ModuleBuild.contexts_from_mmd(build_two.modulemd) + build_two.build_context = contexts.build_context + build_two.build_context_no_bms = contexts.build_context_no_bms db_session.add(build_two) db_session.commit() diff --git a/tests/test_resolver/test_koji.py b/tests/test_resolver/test_koji.py index 7fe3b49..7d5be6d 100644 --- a/tests/test_resolver/test_koji.py +++ b/tests/test_resolver/test_koji.py @@ -98,7 +98,7 @@ class TestLocalResolverModule: assert result == [] koji_session.listTagged.assert_called_with( - 'foo-test', inherit=True, package='testmodule', type='module', event=123) + "foo-test", inherit=True, package="testmodule", type="module", event=123) @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession") def test_get_buildrequired_modulemds_multiple_streams(self, ClientSession, db_session): @@ -107,12 +107,12 @@ class TestLocalResolverModule: # We will ask for testmodule:master, but there is also testmodule:2 in a tag. koji_session.listTagged.return_value = [ { - 'build_id': 123, 'name': 'testmodule', 'version': '2', - 'release': '820181219174508.9edba152', 'tag_name': 'foo-test' + "build_id": 123, "name": "testmodule", "version": "2", + "release": "820181219174508.9edba152", "tag_name": "foo-test" }, { - 'build_id': 124, 'name': 'testmodule', 'version': 'master', - 'release': '20170109091357.7c29193d', 'tag_name': 'foo-test' + "build_id": 124, "name": "testmodule", "version": "master", + "release": "20170109091357.7c29193d", "tag_name": "foo-test" }] self._create_test_modules(db_session) @@ -131,12 +131,12 @@ class TestLocalResolverModule: # ValueError later. koji_session.listTagged.return_value = [ { - 'build_id': 123, 'name': 'testmodule', 'version': '2', - 'release': '820181219174508.9edba152', 'tag_name': 'foo-test' + "build_id": 123, "name": "testmodule", "version": "2", + "release": "820181219174508.9edba152", "tag_name": "foo-test" }, { - 'build_id': 124, 'name': 'testmodule', 'version': 'master', - 'release': '20170109091357.7c29193d', 'tag_name': 'foo-test' + "build_id": 124, "name": "testmodule", "version": "master", + "release": "20170109091357.7c29193d", "tag_name": "foo-test" }] self._create_test_modules(db_session) @@ -156,20 +156,20 @@ class TestLocalResolverModule: # ValueError later. koji_session.listTagged.return_value = [ { - 'build_id': 124, 'name': 'testmodule', 'version': 'master', - 'release': '20160110091357.7c29193d', 'tag_name': 'foo-test' + "build_id": 124, "name": "testmodule", "version": "master", + "release": "20160110091357.7c29193d", "tag_name": "foo-test" }, { - 'build_id': 124, 'name': 'testmodule', 'version': 'master', - 'release': '20170109091357.7c29193d', 'tag_name': 'foo-test' + "build_id": 124, "name": "testmodule", "version": "master", + "release": "20170109091357.7c29193d", "tag_name": "foo-test" }, { - 'build_id': 124, 'name': 'testmodule', 'version': 'master', - 'release': '20170109091357.7c29193e', 'tag_name': 'foo-test' + "build_id": 124, "name": "testmodule", "version": "master", + "release": "20170109091357.7c29193e", "tag_name": "foo-test" }, { - 'build_id': 124, 'name': 'testmodule', 'version': 'master', - 'release': '20160109091357.7c29193d', 'tag_name': 'foo-test' + "build_id": 124, "name": "testmodule", "version": "master", + "release": "20160109091357.7c29193d", "tag_name": "foo-test" }] self._create_test_modules(db_session) @@ -183,6 +183,29 @@ class TestLocalResolverModule: "testmodule:master:20170109091357:7c29193e"} @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession") + def test_get_buildrequired_modules(self, ClientSession, db_session): + koji_session = ClientSession.return_value + + # We will ask for testmodule:master, but there is also testmodule:2 in a tag. + koji_session.listTagged.return_value = [ + { + "build_id": 123, "name": "testmodule", "version": "2", + "release": "820181219174508.9edba152", "tag_name": "foo-test" + }, + { + "build_id": 124, "name": "testmodule", "version": "master", + "release": "20170109091357.7c29193d", "tag_name": "foo-test" + }] + + self._create_test_modules(db_session) + platform = db_session.query(ModuleBuild).filter_by(stream="f30.1.3").one() + resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji") + result = resolver.get_buildrequired_modules("testmodule", "master", platform.mmd()) + + nvrs = {m.nvr_string for m in result} + assert nvrs == {"testmodule-master-20170109091357.7c29193d"} + + @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession") def test_filter_inherited(self, ClientSession, db_session): koji_session = ClientSession.return_value @@ -193,16 +216,16 @@ class TestLocalResolverModule: builds = [ { - 'build_id': 124, 'name': 'testmodule', 'version': 'master', - 'release': '20170110091357.7c29193d', 'tag_name': 'foo-test' + "build_id": 124, "name": "testmodule", "version": "master", + "release": "20170110091357.7c29193d", "tag_name": "foo-test" }, { - 'build_id': 125, 'name': 'testmodule', 'version': 'master', - 'release': '20180109091357.7c29193d', 'tag_name': 'foo-test-parent' + "build_id": 125, "name": "testmodule", "version": "master", + "release": "20180109091357.7c29193d", "tag_name": "foo-test-parent" }, { - 'build_id': 126, 'name': 'testmodule', 'version': '2', - 'release': '20180109091357.7c29193d', 'tag_name': 'foo-test-parent' + "build_id": 126, "name": "testmodule", "version": "2", + "release": "20180109091357.7c29193d", "tag_name": "foo-test-parent" }] resolver = mbs_resolver.GenericResolver.create(db_session, tests.conf, backend="koji") diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index 6cd5a08..2482427 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -1685,3 +1685,81 @@ class TestUtilsModuleReuse: first_module = db_session.query(models.ModuleBuild).filter_by( name="testmodule", state=models.BUILD_STATES["ready"]).first() assert reusable_module.id == first_module.id + + @pytest.mark.parametrize("allow_ocbm", (True, False)) + @patch( + "module_build_service.config.Config.allow_only_compatible_base_modules", + new_callable=mock.PropertyMock, + ) + @patch("module_build_service.builder.KojiModuleBuilder.KojiClientSession") + @patch( + "module_build_service.config.Config.resolver", + new_callable=mock.PropertyMock, return_value="koji" + ) + def test_get_reusable_module_koji_resolver( + self, resolver, ClientSession, cfg, db_session, allow_ocbm): + """ + Test that get_reusable_module works with KojiResolver. + """ + cfg.return_value = allow_ocbm + + # Mock the listTagged so the testmodule:master is listed as tagged in the + # module-fedora-27-build Koji tag. + koji_session = ClientSession.return_value + koji_session.listTagged.return_value = [ + { + "build_id": 123, "name": "testmodule", "version": "master", + "release": "20170109091357.78e4a6fd", "tag_name": "module-fedora-27-build" + }] + + # Mark platform:f28 as KojiResolver ready by defining "koji_tag_with_modules". + # Also define the "virtual_streams" to possibly confuse the get_reusable_module. + platform_f28 = db_session.query(models.ModuleBuild).filter_by(name="platform").one() + mmd = platform_f28.mmd() + xmd = mmd.get_xmd() + xmd["mbs"]["virtual_streams"] = ["fedora"] + xmd["mbs"]["koji_tag_with_modules"] = "module-fedora-27-build" + mmd.set_xmd(xmd) + platform_f28.modulemd = mmd_to_str(mmd) + platform_f28.update_virtual_streams(db_session, ["fedora"]) + + # Create platform:f27 without KojiResolver support. + mmd = load_mmd(read_staged_data("platform")) + mmd = mmd.copy("platform", "f27") + xmd = mmd.get_xmd() + xmd["mbs"]["virtual_streams"] = ["fedora"] + mmd.set_xmd(xmd) + platform_f27 = module_build_service.utils.import_mmd(db_session, mmd)[0] + + # Change the reusable testmodule:master to buildrequire platform:f27. + latest_module = db_session.query(models.ModuleBuild).filter_by( + name="testmodule", state=models.BUILD_STATES["ready"]).one() + mmd = latest_module.mmd() + xmd = mmd.get_xmd() + xmd["mbs"]["buildrequires"]["platform"]["stream"] = "f27" + mmd.set_xmd(xmd) + latest_module.modulemd = mmd_to_str(mmd) + latest_module.buildrequires = [platform_f27] + + # Recompute the build_context and ensure that `build_context` changed while + # `build_context_no_bms` did not change. + contexts = module_build_service.models.ModuleBuild.contexts_from_mmd( + latest_module.modulemd) + + assert latest_module.build_context_no_bms == contexts.build_context_no_bms + assert latest_module.build_context != contexts.build_context + + latest_module.build_context = contexts.build_context + latest_module.build_context_no_bms = contexts.build_context_no_bms + db_session.commit() + + # Get the module we want to build. + module = db_session.query(models.ModuleBuild)\ + .filter_by(name="testmodule")\ + .filter_by(state=models.BUILD_STATES["build"])\ + .one() + + reusable_module = module_build_service.utils.get_reusable_module( + db_session, module) + + assert reusable_module.id == latest_module.id