From 5fb6a2f28a1376466f9d77c0a78e966c4625b0fa Mon Sep 17 00:00:00 2001 From: Valerij Maljulin Date: Jul 31 2019 09:23:52 +0000 Subject: Check dependencies in reuse Signed-off-by: Valerij Maljulin --- diff --git a/module_build_service/models.py b/module_build_service/models.py index df1f849..1da845d 100644 --- a/module_build_service/models.py +++ b/module_build_service/models.py @@ -30,7 +30,7 @@ import contextlib import hashlib import json import re -from collections import OrderedDict +from collections import OrderedDict, namedtuple from datetime import datetime import sqlalchemy @@ -85,6 +85,9 @@ INVERSE_BUILD_STATES = {v: k for k, v in BUILD_STATES.items()} FAILED_STATES = (BUILD_STATES["failed"], BUILD_STATES["garbage"]) +Contexts = namedtuple("Contexts", "ref_build_context build_context runtime_context context") + + def _utc_datetime_to_iso(datetime_object): """ Takes a UTC datetime object and returns an ISO formatted string @@ -619,8 +622,8 @@ class ModuleBuild(MBSBase): else: raise ValueError("%r is not a module message." % type(event).__name__) - @staticmethod - def contexts_from_mmd(mmd_str): + @classmethod + def contexts_from_mmd(cls, mmd_str): """ Returns tuple (ref_build_context, build_context, runtime_context, context) with hashes: @@ -630,8 +633,8 @@ class ModuleBuild(MBSBase): - context - Hash of combined hashes of build_context and runtime_context. :param str mmd_str: String with Modulemd metadata. - :rtype: tuple of strings - :return: Tuple with build_context, strem_build_context, runtime_context and + :rtype: Contexts + :return: Named tuple with build_context, strem_build_context, runtime_context and context hashes. """ from module_build_service.utils.general import load_mmd @@ -640,47 +643,81 @@ class ModuleBuild(MBSBase): mmd = load_mmd(mmd_str) except UnprocessableEntity: raise ValueError("Invalid modulemd") - mbs_xmd = mmd.get_xmd().get("mbs", {}) - rv = [] + mbs_xmd_buildrequires = mmd.get_xmd()["mbs"]["buildrequires"] + mmd_deps = mmd.get_dependencies() + + build_context = cls.calculate_build_context(mbs_xmd_buildrequires) + runtime_context = cls.calculate_runtime_context(mmd_deps) + return Contexts( + cls.calculate_ref_build_context(mbs_xmd_buildrequires), + build_context, + runtime_context, + cls.calculate_module_context(build_context, runtime_context) + ) + + @staticmethod + def calculate_ref_build_context(mbs_xmd_buildrequires): + """ + Returns the hash of commit hashes of expanded buildrequires. + :param mbs_xmd_buildrequires: xmd["mbs"]["buildrequires"] from Modulemd + :rtype: str + :return: ref_build_context hash + """ # Get the buildrequires from the XMD section, because it contains # all the buildrequires as we resolved them using dependency resolver. - if "buildrequires" not in mbs_xmd: - raise ValueError("The module's modulemd hasn't been formatted by MBS") mmd_formatted_buildrequires = { - dep: info["ref"] for dep, info in mbs_xmd["buildrequires"].items() + dep: info["ref"] for dep, info in mbs_xmd_buildrequires.items() } property_json = json.dumps(OrderedDict(sorted(mmd_formatted_buildrequires.items()))) - rv.append(hashlib.sha1(property_json.encode("utf-8")).hexdigest()) + return hashlib.sha1(property_json.encode("utf-8")).hexdigest() - # Get the streams of buildrequires and hash it. + @staticmethod + def calculate_build_context(mbs_xmd_buildrequires): + """ + Returns the hash of stream names of expanded buildrequires + :param mbs_xmd_buildrequires: xmd["mbs"]["buildrequires"] from Modulemd + :rtype: str + :return: build_context hash + """ mmd_formatted_buildrequires = { - dep: info["stream"] for dep, info in mbs_xmd["buildrequires"].items() + dep: info["stream"] for dep, info in mbs_xmd_buildrequires.items() } property_json = json.dumps(OrderedDict(sorted(mmd_formatted_buildrequires.items()))) - build_context = hashlib.sha1(property_json.encode("utf-8")).hexdigest() - rv.append(build_context) + return hashlib.sha1(property_json.encode("utf-8")).hexdigest() - # Get the requires from the real "dependencies" section in MMD. + @staticmethod + def calculate_runtime_context(mmd_dependencies): + """ + Returns the hash of stream names of expanded runtime requires + :param mmd_dependencies: dependencies from Modulemd + :rtype: str + :return: runtime_context hash + """ mmd_requires = {} - for deps in mmd.get_dependencies(): + for deps in mmd_dependencies: for name in deps.get_runtime_modules(): streams = deps.get_runtime_streams(name) - if name not in mmd_requires: - mmd_requires[name] = set() - mmd_requires[name] = mmd_requires[name].union(streams) + mmd_requires[name] = mmd_requires.get(name, set()).union(streams) # Sort the streams for each module name and also sort the module names. mmd_requires = {dep: sorted(list(streams)) for dep, streams in mmd_requires.items()} property_json = json.dumps(OrderedDict(sorted(mmd_requires.items()))) - runtime_context = hashlib.sha1(property_json.encode("utf-8")).hexdigest() - rv.append(runtime_context) + return hashlib.sha1(property_json.encode("utf-8")).hexdigest() + @staticmethod + def calculate_module_context(build_context, runtime_context): + """ + Returns the hash of combined hashes of build_context and runtime_context + :param build_context: hash returned by calculate_build_context + :type build_context: str + :param runtime_context: hash returned by calculate_runtime_context + :type runtime_context: str + :rtype: str + :return: module context hash + """ combined_hashes = "{0}:{1}".format(build_context, runtime_context) - context = hashlib.sha1(combined_hashes.encode("utf-8")).hexdigest()[:8] - rv.append(context) - - return tuple(rv) + return hashlib.sha1(combined_hashes.encode("utf-8")).hexdigest()[:8] def siblings(self, db_session): query = db_session.query(ModuleBuild).filter( diff --git a/module_build_service/utils/mse.py b/module_build_service/utils/mse.py index 004be0e..fc663f8 100644 --- a/module_build_service/utils/mse.py +++ b/module_build_service/utils/mse.py @@ -207,7 +207,7 @@ def _get_mmds_from_requires( return mmds -def _get_base_module_mmds(db_session, mmd): +def get_base_module_mmds(db_session, mmd): """ Returns list of MMDs of base modules buildrequired by `mmd` including the compatible old versions of the base module based on the stream version. @@ -329,7 +329,7 @@ def get_mmds_required_by_module_recursively( mmds = {} # Get the MMDs of all compatible base modules based on the buildrequires. - base_module_mmds = _get_base_module_mmds(db_session, mmd) + base_module_mmds = get_base_module_mmds(db_session, mmd) if not base_module_mmds["ready"]: base_module_choices = " or ".join(conf.base_module_names) raise UnprocessableEntity( @@ -526,7 +526,7 @@ def generate_expanded_mmds(db_session, mmd, raise_if_stream_ambigous=False, defa mmd_copy.set_xmd(xmd) # Now we have all the info to actually compute context of this module. - _, _, _, context = models.ModuleBuild.contexts_from_mmd(mmd_to_str(mmd_copy)) + context = models.ModuleBuild.contexts_from_mmd(mmd_to_str(mmd_copy)).context mmd_copy.set_context(context) mmds.append(mmd_copy) diff --git a/module_build_service/utils/reuse.py b/module_build_service/utils/reuse.py index 4119bab..b5bd49a 100644 --- a/module_build_service/utils/reuse.py +++ b/module_build_service/utils/reuse.py @@ -26,6 +26,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 def reuse_component(component, previous_component_build, change_state_now=False): @@ -90,24 +91,40 @@ def get_reusable_module(db_session, module): return module.reused_module mmd = module.mmd() - # Find the latest module that is in the done or 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=module.build_context) - .order_by(models.ModuleBuild.time_completed.desc()) - ) - # 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": - previous_module_build = previous_module_build.filter( - models.ModuleBuild.rebuild_strategy.in_(["all", "changed-and-after"])) - previous_module_build = previous_module_build.filter_by( - ref_build_context=module.ref_build_context) - previous_module_build = previous_module_build.first() + previous_module_build = None + + base_mmds = get_base_module_mmds(db_session, mmd)["ready"] + 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())) + + # 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": + previous_module_build = previous_module_build.filter( + models.ModuleBuild.rebuild_strategy.in_(["all", "changed-and-after"]) + ) + previous_module_build = previous_module_build.filter_by( + ref_build_context=module.ref_build_context + ) + previous_module_build = previous_module_build.first() + + if previous_module_build: + break + # The component can't be reused if there isn't a previous build in the done # or ready state if not previous_module_build: diff --git a/tests/conftest.py b/tests/conftest.py index 262c5c1..ec824ca 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -129,6 +129,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 db_session.add(build_one) db_session.commit() @@ -226,6 +229,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 db_session.add(build_two) db_session.commit() @@ -305,6 +311,15 @@ def reuse_shared_userspace_init_data(db_session): rebuild_strategy="changed-and-after", ) + xmd = mmd.get_xmd() + xmd["mbs"]["scmurl"] = module_build.scmurl + xmd["mbs"]["commit"] = "55f4a0a2e6cc255c88712a905157ab39315b8fd8" + mmd.set_xmd(xmd) + module_build.modulemd = mmd_to_str(mmd) + module_build.build_context = module_build_service.models.ModuleBuild.contexts_from_mmd( + module_build.modulemd + ).build_context + components = [ mmd.get_rpm_component(rpm) for rpm in mmd.get_rpm_component_names() @@ -358,6 +373,15 @@ def reuse_shared_userspace_init_data(db_session): rebuild_strategy="changed-and-after", ) + xmd = mmd2.get_xmd() + xmd["mbs"]["scmurl"] = module_build.scmurl + xmd["mbs"]["commit"] = "55f4a0a2e6cc255c88712a905157ab39315b8fd8" + mmd2.set_xmd(xmd) + module_build.modulemd = mmd_to_str(mmd2) + module_build.build_context = module_build_service.models.ModuleBuild.contexts_from_mmd( + module_build.modulemd + ).build_context + components2 = [ mmd2.get_rpm_component(rpm) for rpm in mmd2.get_rpm_component_names() diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index 49e36a8..0b7e592 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -233,9 +233,18 @@ class TestUtilsComponentReuse: mmd = second_module_build.mmd() xmd = mmd.get_xmd() xmd["mbs"]["buildrequires"]["platform"]["stream"] = "different" + deps = Modulemd.Dependencies() + deps.add_buildtime_stream("platform", "different") + deps.add_runtime_stream("platform", "different") + mmd.clear_dependencies() + mmd.add_dependencies(deps) + mmd.set_xmd(xmd) second_module_build.modulemd = mmd_to_str(mmd) - second_module_build.build_context = "37c6c57bedf4305ef41249c1794760b5cb8fad17" + second_module_build.build_context = \ + module_build_service.models.ModuleBuild.contexts_from_mmd( + second_module_build.modulemd + ).build_context second_module_build.rebuild_strategy = rebuild_strategy db_session.commit() @@ -614,8 +623,11 @@ class TestUtils: current `new_module`. In this case, reuse code should still be able to reuse the components. """ + old_module = models.ModuleBuild.get_by_id(db_session, 2) new_module = models.ModuleBuild.get_by_id(db_session, 3) - rv = module_build_service.utils.get_reusable_component(db_session, new_module, "llvm") + rv = module_build_service.utils.get_reusable_component( + db_session, new_module, "llvm", previous_module_build=old_module + ) assert rv.package == "llvm" def test_validate_koji_tag_wrong_tag_arg_during_programming(self): @@ -1499,17 +1511,14 @@ class TestOfflineLocalBuilds: assert module_build +@pytest.mark.usefixtures("reuse_component_init_data") class TestUtilsModuleReuse: - def setup_method(self, test_method): - init_data() - - def teardown_method(self, test_method): - clean_database() - def test_get_reusable_module_when_reused_module_not_set(self, db_session): - module = db_session.query(models.ModuleBuild).filter_by( - name="nginx").order_by(models.ModuleBuild.id.desc()).first() + module = db_session.query(models.ModuleBuild)\ + .filter_by(name="testmodule")\ + .order_by(models.ModuleBuild.id.desc())\ + .first() module.state = models.BUILD_STATES["build"] db_session.commit() @@ -1522,8 +1531,10 @@ class TestUtilsModuleReuse: assert reusable_module.id == module.reused_module_id def test_get_reusable_module_when_reused_module_already_set(self, db_session): - modules = db_session.query(models.ModuleBuild).filter_by( - name="nginx").order_by(models.ModuleBuild.id.desc()).limit(2).all() + modules = db_session.query(models.ModuleBuild)\ + .filter_by(name="testmodule")\ + .order_by(models.ModuleBuild.id.desc())\ + .limit(2).all() build_module = modules[0] reused_module = modules[1] build_module.state = models.BUILD_STATES["build"] diff --git a/tests/test_utils/test_utils_mse.py b/tests/test_utils/test_utils_mse.py index 0475740..de16062 100644 --- a/tests/test_utils/test_utils_mse.py +++ b/tests/test_utils/test_utils_mse.py @@ -503,7 +503,7 @@ class TestUtilsModuleStreamExpansion: mmd.remove_dependencies(deps) mmd.add_dependencies(new_deps) - mmds = module_build_service.utils.mse._get_base_module_mmds(db_session, mmd) + mmds = module_build_service.utils.mse.get_base_module_mmds(db_session, 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["ready"]) == len(expected) @@ -530,7 +530,7 @@ class TestUtilsModuleStreamExpansion: "platform:lp29.1.1:12:c11", db_session=db_session, virtual_streams=virtual_streams) - mmds = module_build_service.utils.mse._get_base_module_mmds(db_session, mmd) + mmds = module_build_service.utils.mse.get_base_module_mmds(db_session, mmd) if virtual_streams == ["f29"]: expected = set( ["platform:f29.0.0", "platform:f29.1.0", "platform:f29.2.0", "platform:lp29.1.1"]) @@ -568,7 +568,7 @@ class TestUtilsModuleStreamExpansion: mmd.remove_dependencies(deps) mmd.add_dependencies(new_deps) - mmds = module_build_service.utils.mse._get_base_module_mmds(db_session, mmd) + mmds = module_build_service.utils.mse.get_base_module_mmds(db_session, mmd) expected = {} expected["ready"] = set(["platform:foo29", "platform:foo30"]) expected["garbage"] = set(["platform:foo28"])