From c139e4cb6068f019e2a9e901ef99d0d2c9e767c3 Mon Sep 17 00:00:00 2001 From: Martin Curlej Date: Sep 03 2020 07:22:08 +0000 Subject: Added the ability to build modules with static contexts This is a fix and feature in one. This patch enables to use static contexts instead of module stream expansion. This fixes the issue with upgrade path issue in dnf. Signed-off-by: Martin Curlej --- diff --git a/module_build_service/web/mse.py b/module_build_service/web/mse.py index d9966c7..dd67119 100644 --- a/module_build_service/web/mse.py +++ b/module_build_service/web/mse.py @@ -3,7 +3,7 @@ from __future__ import absolute_import from module_build_service.common import conf, log, models -from module_build_service.common.errors import StreamAmbigous, UnprocessableEntity +from module_build_service.common.errors import StreamAmbigous, UnprocessableEntity, ValidationError from module_build_service.common.modulemd import Modulemd from module_build_service.common.resolve import expand_single_mse_streams, get_base_module_mmds from module_build_service.common.utils import mmd_to_str @@ -229,7 +229,8 @@ def get_mmds_required_by_module_recursively( return res -def generate_expanded_mmds(db_session, mmd, raise_if_stream_ambigous=False, default_streams=None): +def generate_expanded_mmds(db_session, mmd, raise_if_stream_ambigous=False, default_streams=None, + static_context=False): """ Returns list with MMDs with buildrequires and requires set according to module stream expansion rules. These module metadata can be directly @@ -243,6 +244,9 @@ def generate_expanded_mmds(db_session, mmd, raise_if_stream_ambigous=False, defa :param dict default_streams: Dict in {module_name: module_stream, ...} format defining the default stream to choose for module in case when there are multiple streams to choose from. + :param bool static_context: When True will use a predefined context from the mmd yaml file + and will not generate a new one. Also it will set `static_context` property in the `mbs` + property to True. """ if not default_streams: default_streams = {} @@ -251,6 +255,10 @@ def generate_expanded_mmds(db_session, mmd, raise_if_stream_ambigous=False, defa # which would change the module. current_mmd = mmd.copy() + # if a static context is set the generated context will be overridden by the one + # defined in the mmd of the module. + if static_context: + context = current_mmd.get_context() # MMDResolver expects the input MMD to have no context. current_mmd.set_context(None) @@ -376,14 +384,130 @@ def generate_expanded_mmds(db_session, mmd, raise_if_stream_ambigous=False, defa xmd["mbs"] = {} resolver = GenericResolver.create(db_session, conf) xmd["mbs"]["buildrequires"] = resolver.resolve_requires(br_list) + xmd["mbs"]["mse"] = True + # if static context is used the mse is expanded beforehand. This means that the + # mmd set as a parameter to this function has only one context per stream. + # we set a `static_context` property to True to mark module streams with static contexts. + if static_context: + xmd["mbs"]["static_context"] = True 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 + # we only compute the context if static context is False + if not static_context: + # 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 + mmd_copy.set_context(context) mmds.append(mmd_copy) return mmds + + +def generate_mmds_from_static_contexts(mmd): + """ + This function preexpands the MSE when static contexts which are defined by the `contexts` + property under `mbs_options` in the `xmd` property of the initial modulemd file. + + :param Modulemd.ModuleStream mmd: Modulemd metadata with the `contexts` property. + :return list mmds: list of distinct mmds identified by context. + """ + + xmd = mmd.get_xmd() + if not xmd: + raise ValidationError("The 'xmd' property of the modulemd yaml file is empty!") + + if not xmd["mbs_options"].get("contexts"): + raise ValidationError(("The 'xmd' property of the modulemd yaml file does not contain the" + " 'contexts' key.")) + + contexts = xmd["mbs_options"].get("contexts") + if not type(contexts) is dict: + raise ValidationError("The value of the 'contexts' property needs to be a dict.") + + # remove the information about the `contexts` property from the `mbs_options` as we do not + # needed anymore + xmd["mbs_options"].pop("contexts") + + # we check if the `mbs_options` dict is empty after we remove the `contexts` key. + # if yes we remove `the mbs_options`. + if not xmd["mbs_options"]: + xmd.pop("mbs_options") + + mmd.set_xmd(xmd) + + mmds = [] + for context, dependencies in contexts.items(): + # we copy the mmd so we a get a fresh module for each context. + mmd_copy = mmd.copy() + mmd_copy.set_context(context) + + if "buildrequires" not in dependencies: + raise ValidationError(("The context '{context}' is missing the" + " 'buildrequires' key!").format(context=context)) + + if "requires" not in dependencies: + raise ValidationError("The context '{context}' is missing the 'requires' key!".format( + context=context)) + + # remove the old deps from the mmd + old_module_deps = mmd_copy.get_dependencies() + for deps in old_module_deps: + mmd_copy.remove_dependencies(deps) + + module_deps = Modulemd.Dependencies.new() + + # populate the new requires and buildrequires according to the `contexts` property + for module, stream in dependencies["buildrequires"].items(): + _validate_stream_name_for_static_context(module, stream, context, "buildrequires") + module_deps.add_buildtime_stream(module, stream) + + for module, stream in dependencies["requires"].items(): + _validate_stream_name_for_static_context(module, stream, context, "requires") + module_deps.add_runtime_stream(module, stream) + + mmd_copy.add_dependencies(module_deps) + + mmds.append(mmd_copy) + + return mmds + + +def _validate_stream_name_for_static_context(module, stream, context, dep_type): + """ + Validates the the streams of static contexts. + + :param str module: name of the module + :param str stream: name of the stream + :param str context: name of the context + :param str dep_type: type of the dependencies the module stream belongs to i. e. requires, + buildrequires. + :return None + """ + + if not stream: + raise ValidationError(("The '{module}' module in '{dep_type}' of the '{context}' static " + "context has no stream defined.").format(module=module, + dep_type=dep_type, + context=context)) + + stream_type = type(stream) + if stream_type is not str: + raise ValidationError(("The module '{module}' in '{dep_type}' of the '{context}' " + "static context is of type '{stream_type}' should be " + "'str' type.").format(stream=stream, + module=module, + dep_type=dep_type, + context=context, + stream_type=stream_type)) + + if stream.startswith("-"): + raise ValidationError(("The '{stream}' stream of module '{module}' in '{dep_type}' of the" + "'{context}' static contexts is using stream expansion. Usage of " + "stream expansion with static context is forbidden." + ).format(stream=stream, + module=module, + dep_type=dep_type, + context=context)) diff --git a/module_build_service/web/submit.py b/module_build_service/web/submit.py index b4fb527..61bc3bb 100644 --- a/module_build_service/web/submit.py +++ b/module_build_service/web/submit.py @@ -17,7 +17,7 @@ from module_build_service.common.messaging import notify_on_module_state_change from module_build_service.common.modulemd import Modulemd from module_build_service.common.submit import fetch_mmd from module_build_service.common.utils import load_mmd, mmd_to_str, to_text_type -from module_build_service.web.mse import generate_expanded_mmds +from module_build_service.web.mse import generate_expanded_mmds, generate_mmds_from_static_contexts from module_build_service.web.utils import deps_to_dict @@ -537,7 +537,6 @@ def submit_module_build(db_session, username, mmd, params): mmd.get_stream_name(), mmd.get_version(), ) - validate_mmd(mmd) raise_if_stream_ambigous = False default_streams = {} @@ -548,11 +547,22 @@ def submit_module_build(db_session, username, mmd, params): # Get the default_streams if set. if "default_streams" in params: default_streams = params["default_streams"] - _apply_dep_overrides(mmd, params) - _modify_buildtime_streams(db_session, mmd, resolve_base_module_virtual_streams) - _process_support_streams(db_session, mmd, params) - mmds = generate_expanded_mmds(db_session, mmd, raise_if_stream_ambigous, default_streams) + xmd = mmd.get_xmd() + # we check if static contexts are enabled by the `contexts` property defined by the user i + # as an build option. + static_context = "mbs_options" in xmd and "contexts" in xmd["mbs_options"] + input_mmds = generate_mmds_from_static_contexts(mmd) if static_context else [mmd] + + mmds = [] + for mmd in input_mmds: + validate_mmd(mmd) + _apply_dep_overrides(mmd, params) + _modify_buildtime_streams(db_session, mmd, resolve_base_module_virtual_streams) + _process_support_streams(db_session, mmd, params) + mmds += generate_expanded_mmds(db_session, mmd, raise_if_stream_ambigous, + default_streams, static_context=static_context) + if not mmds: raise ValidationError( "No dependency combination was satisfied. Please verify the " @@ -651,6 +661,11 @@ def submit_module_build(db_session, username, mmd, params): ) module.build_context, module.runtime_context, module.context, \ module.build_context_no_bms = module.contexts_from_mmd(module.modulemd) + + xmd = mmd.get_xmd() + if xmd["mbs"].get("static_context"): + module.context = mmd.get_context() + module.context += context_suffix db_session.commit() diff --git a/tests/test_web/test_mse.py b/tests/test_web/test_mse.py index 0403536..b8111f1 100644 --- a/tests/test_web/test_mse.py +++ b/tests/test_web/test_mse.py @@ -4,10 +4,11 @@ from __future__ import absolute_import import pytest -from module_build_service.common.errors import StreamAmbigous +from module_build_service.common.errors import StreamAmbigous, ValidationError from module_build_service.scheduler.db_session import db_session from module_build_service.web.mse import ( - expand_mse_streams, generate_expanded_mmds, get_mmds_required_by_module_recursively + expand_mse_streams, generate_expanded_mmds, get_mmds_required_by_module_recursively, + generate_mmds_from_static_contexts ) from tests import make_module_in_db @@ -461,3 +462,242 @@ class TestModuleStreamExpansion: self._generate_default_modules_modules_multiple_stream_versions() nsvcs = self._get_mmds_required_by_module_recursively(module_build, db_session) assert set(nsvcs) == set(expected) + + def test_generate_expanded_mmds_static_context(self): + """ + Tests if generate_expanded_mmds will not change the context of a module if provided + with a static one. + """ + module_deps = [{ + "requires": {"gtk": ["1"], "foo": ["1"]}, + "buildrequires": {"platform": ["f28"], "gtk": ["1"], "foo": ["1"]}, + }] + self._generate_default_modules() + module_build = make_module_in_db("app:1:0:static", module_deps) + + mmds = generate_expanded_mmds(db_session, module_build.mmd(), static_context=True) + + assert type(mmds) is list + assert len(mmds) == 1 + + current_context = mmds[0].get_context() + + assert current_context == "static" + + def test_generate_mmds_from_static_context(self): + self._generate_default_modules() + module_build = make_module_in_db( + "app:1:0:c1", + dependencies=[{ + "requires": {"gtk": ["1", "2"]}, + "buildrequires": { + "platform": ["f28"], + "gtk": ["1", "2"] + }}], + xmd={ + "mbs": {}, + "mbs_options": { + "contexts": { + "context1": { + "requires": { + "gtk": "1" + }, + "buildrequires": { + "platform": "f28", + "gtk": "1", + } + }, + "context2": { + "requires": { + "gtk": "2" + }, + "buildrequires": { + "platform": "f28", + "gtk": "2", + }, + } + } + } + } + ) + + mmds = generate_mmds_from_static_contexts(module_build.mmd()) + + expected_contexts = ["context1", "context2"] + expected_deps = { + "context1": { + "buildrequires": { + "platform": ["f28"], + "gtk": ["1"], + }, + "requires": { + "gtk": ["1"], + }, + }, + "context2": { + "buildrequires": { + "platform": ["f28"], + "gtk": ["2"], + }, + "requires": { + "gtk": ["2"], + }, + }, + } + + assert type(mmds) is list + assert len(mmds) == 2 + + for mmd in mmds: + current_context = mmd.get_context() + current_xmd = mmd.get_xmd() + + assert current_context in expected_contexts + assert "contexts" not in current_xmd + + deps = mmd.get_dependencies() + + assert len(deps) == 1 + + buildrequires = deps[0].get_buildtime_modules() + + for module in buildrequires: + current_stream = deps[0].get_buildtime_streams(module) + assert len(current_stream) == 1 + assert expected_deps[current_context]["buildrequires"][module] == current_stream + + requires = deps[0].get_runtime_modules() + + for module in requires: + current_stream = deps[0].get_runtime_streams(module) + assert len(current_stream) == 1 + assert expected_deps[current_context]["requires"][module] == current_stream + + def test_generate_expanded_mmds_static_context_empty_xmd(self): + module_build = make_module_in_db( + "app:1:0:c1", + xmd={}) + mmd = module_build.mmd() + mmd.set_xmd({}) + with pytest.raises(ValidationError): + generate_mmds_from_static_contexts(mmd) + + def test_generate_expanded_mmds_static_context_no_contexts(self): + module_build = make_module_in_db( + "app:1:0:c1", + xmd={"mbs": {}, "mbs_options": {}}) + + with pytest.raises(ValidationError): + generate_mmds_from_static_contexts(module_build.mmd()) + + def test_generate_expanded_mmds_static_context_missing_buildrequires(self): + xmd = { + "mbs": {}, + "mbs_options": { + "contexts": {"context1": {"requires": {"gtk": ["1"]}}} + } + } + + module_build = make_module_in_db( + "app:1:0:c1", + xmd=xmd) + + with pytest.raises(ValidationError): + generate_mmds_from_static_contexts(module_build.mmd()) + + def test_generate_expanded_mmds_static_context_missing_requires(self): + xmd = { + "mbs": {}, + "mbs_options": { + "contexts": {"context1": {"buildrequires": {"platform": ["f28"]}}} + } + } + + module_build = make_module_in_db( + "app:1:0:c1", + xmd=xmd) + + with pytest.raises(ValidationError): + generate_mmds_from_static_contexts(module_build.mmd()) + + def test_generate_expanded_mmds_static_context_wrong_type(self): + xmd = {"mbs": {}, "mbs_options": {"contexts": []}} + module_build = make_module_in_db( + "app:1:0:c1", + xmd=xmd) + + with pytest.raises(ValidationError): + generate_mmds_from_static_contexts(module_build.mmd()) + + def test_generate_expanded_mmds_static_context_missing_stream(self): + xmd = { + "mbs": {}, + "mbs_options": { + "contexts": { + "context1": { + "buildrequires": {"platform": "f28"}, + "requires": {"gtk": None}, + } + } + } + } + + module_build = make_module_in_db( + "app:1:0:c1", xmd=xmd) + + with pytest.raises(ValidationError) as ex: + generate_mmds_from_static_contexts(module_build.mmd()) + err_msg = ex.value.args[0] + assert "gtk" in err_msg + assert "requires" in err_msg + assert "context1" in err_msg + + def test_generate_expanded_mmds_static_context_stream_invalid_type(self): + xmd = { + "mbs": {}, + "mbs_options": { + "contexts": { + "context1": { + "buildrequires": {"platform": "f28"}, + "requires": {"gtk": {"dict": "1"}}, + } + } + } + } + + module_build = make_module_in_db( + "app:1:0:c1", + xmd=xmd) + + with pytest.raises(ValidationError) as ex: + generate_mmds_from_static_contexts(module_build.mmd()) + err_msg = ex.value.args[0] + assert "gtk" in err_msg + assert "requires" in err_msg + assert "context1" in err_msg + assert "dict" in err_msg + assert "str" in err_msg + + def test_generate_expanded_mmds_static_context_used_mse(self): + xmd = { + "mbs": {}, + "mbs_options": { + "contexts": { + "context1": { + "buildrequires": {"platform": "f28"}, + "requires": {"gtk": "-f28"}, + } + } + } + } + module_build = make_module_in_db( + "app:1:0:c1", + xmd=xmd) + + with pytest.raises(ValidationError) as ex: + generate_mmds_from_static_contexts(module_build.mmd()) + err_msg = ex.value.args[0] + assert "gtk" in err_msg + assert "requires" in err_msg + assert "context1" in err_msg + assert "-f28" in err_msg diff --git a/tests/test_web/test_submit.py b/tests/test_web/test_submit.py index 5be4570..415c67c 100644 --- a/tests/test_web/test_submit.py +++ b/tests/test_web/test_submit.py @@ -12,7 +12,7 @@ from werkzeug.datastructures import FileStorage from module_build_service.common import models from module_build_service.common.errors import ValidationError -from module_build_service.common.utils import mmd_to_str +from module_build_service.common.utils import mmd_to_str, load_mmd from module_build_service.scheduler.db_session import db_session from module_build_service.web.submit import ( get_prefixed_version, submit_module_build, submit_module_build_from_yaml @@ -25,7 +25,6 @@ from tests import ( class TestSubmit: - def test_get_prefixed_version_f28(self): scheduler_init_data(1) build_one = models.ModuleBuild.get_by_id(db_session, 2) @@ -42,6 +41,110 @@ class TestSubmit: v = get_prefixed_version(mmd) assert v == 7000120180205135154 + def test_submit_build_static_context(self): + """ + Test that we can now build modules with static contexts. The contexts are defined in + the `xmd` property by the `contexts` property of the initial modulemd yaml file. The + `contexts` is not pressent in the resulting module build. The generated contexts of a + module is overridden by the static context defined by the user and the `mse` property + is set to False. + """ + + yaml_str = """ +document: modulemd +version: 2 +data: + name: app + stream: test + summary: "A test module" + description: > + "A test module stream" + license: + module: [ MIT ] + dependencies: + - buildrequires: + platform: [] + gtk: [] + requires: + platform: [] + gtk: [] + xmd: + mbs_options: + contexts: + context1: + buildrequires: + platform: f28 + requires: + platform: f28 + gtk: 1 + context2: + buildrequires: + platform: f28 + requires: + platform: f28 + gtk: 2 + """ + mmd = load_mmd(yaml_str) + + builds = submit_module_build(db_session, "app", mmd, {}) + + expected_context = ["context1", "context2"] + + assert len(builds) == 2 + + for build in builds: + assert build.context in expected_context + mmd = build.mmd() + xmd = mmd.get_xmd() + assert "mbs_options" not in xmd + assert xmd["mbs"]["static_context"] + + def test_submit_build_static_context_preserve_mbs_options(self): + """ + This tests that the `mbs_options` will be preserved after static context build if there + are more options configured then `contexts` option.. + """ + + yaml_str = """ +document: modulemd +version: 2 +data: + name: app + stream: test1 + summary: "A test module" + description: > + "A test module stream" + license: + module: [ MIT ] + dependencies: + - buildrequires: + platform: [] + gtk: [] + requires: + platform: [] + gtk: [] + xmd: + mbs_options: + contexts: + context1: + buildrequires: + platform: f28 + requires: + platform: f28 + gtk: 1 + another_option: "test" + """ + mmd = load_mmd(yaml_str) + + builds = submit_module_build(db_session, "app", mmd, {}) + + assert len(builds) == 1 + mmd = builds[0].mmd() + xmd = mmd.get_xmd() + assert "mbs_options" in xmd + assert "another_option" in xmd["mbs_options"] + assert "test" == xmd["mbs_options"]["another_option"] + @pytest.mark.usefixtures("reuse_component_init_data") class TestUtilsComponentReuse: