From 8d88e4a2bea49e57590d4989a4d48b3ddc5e005b Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Mar 11 2019 10:01:43 +0000 Subject: Allow filtering on buildrequires With this patch, Ursa-Major could be configured to track modules by both buildrequires and requires. There is an example in README. Meanwhile, just same as the requires, buildrequires is optional as well. Signed-off-by: Chenxiong Qi --- diff --git a/README.rst b/README.rst index 2cf9a8b..e840eb5 100644 --- a/README.rst +++ b/README.rst @@ -22,6 +22,9 @@ An example tag config file is: { "name": "httpd", "priority": 10, + "buildrequires": { + "platform": "f30" + }, "requires": { "platform": "f30" }, @@ -57,10 +60,11 @@ An example tag config file is: A valid module config should contains: -* `name` (required): module name -* `stream` (required): module stream -* `priority` (required): add module's tag to tag inheritance with this priority -* `requires` (optional): module's runtime dependency +* ``name`` (required): module name +* ``stream`` (required): module stream +* ``priority`` (required): add module's tag to tag inheritance with this priority +* ``requires`` (optional): module's runtime dependencies. +* ``buildrequires`` (optional): module's build time dependencies. For each tag, ``owners`` can be set with email addresses. diff --git a/tests/test_add_tag_handler.py b/tests/test_add_tag_handler.py index 7a2c3a2..aeeafba 100644 --- a/tests/test_add_tag_handler.py +++ b/tests/test_add_tag_handler.py @@ -234,75 +234,121 @@ class TestAddTagHandler(AddTagHandlerTestCase): # messages. self.assertGreaterEqual(log.info.call_count, 2) - def test_get_module_config_match_name_stream(self): - configs = [ - {'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10}, - {'name': 'testmodule2', 'stream': 'rhel-8.0', 'priority': 20} - ] - message = self.load_json_from_file("testmodule_ready_message.json") - mock_mbs = mock.MagicMock() - fake_requires = {"platform": ["el8"]} - fake_mmd = self.make_mmd("testmodule", "rhel-8.0", - "20180409051516", "9e5fe74b", - requires=fake_requires) - mock_mbs.get_module_mmd.return_value = fake_mmd - modinfo = ModuleInfo.from_mbs_message(mock_mbs, message) - matched_config = AddTagHandler.get_module_config(configs, modinfo) - expected = {'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10} - self.assertEqual(matched_config, expected) - - def test_get_module_config_match_requires(self): - configs = [ - {'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10, - 'requires': {'platform': 'el8'}}, - {'name': 'testmodule2', 'stream': 'rhel-8.0', 'priority': 20} - ] + def assert_get_module_config(self, fake_configs, expected_result): message_file = "testmodule_with_requires_ready_message.json" message = self.load_json_from_file(message_file) mock_mbs = mock.MagicMock() fake_requires = {"platform": ["el8"]} fake_mmd = self.make_mmd("testmodule", "rhel-8.0", "20180409051516", "9e5fe74b", - requires=fake_requires) - mock_mbs.get_module_mmd.return_value = fake_mmd - modinfo = ModuleInfo.from_mbs_message(mock_mbs, message) - matched_config = AddTagHandler.get_module_config(configs, modinfo) - expected = {'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10, - 'requires': {'platform': 'el8'}} - self.assertEqual(matched_config, expected) - - def test_get_module_config_unmatch_stream(self): - configs = [ - {'name': 'testmodule', 'stream': 'f28', 'priority': 10}, - {'name': 'testmodule2', 'stream': 'rhel-8.0', 'priority': 20} - ] - message = self.load_json_from_file("testmodule_ready_message.json") - mock_mbs = mock.MagicMock() - fake_requires = {"platform": ["el8"]} - fake_mmd = self.make_mmd("testmodule", "rhel-8.0", - "20180409051516", "9e5fe74b", - requires=fake_requires) + requires=fake_requires, + buildrequires=fake_requires) mock_mbs.get_module_mmd.return_value = fake_mmd modinfo = ModuleInfo.from_mbs_message(mock_mbs, message) - matched_config = AddTagHandler.get_module_config(configs, modinfo) - self.assertEqual(matched_config, None) - - def test_get_module_config_unmatch_requires(self): - configs = [ - {'name': 'testmodule', 'stream': 'f28', 'priority': 10, - 'requires': {'platform': 'el8'}}, - {'name': 'testmodule2', 'stream': 'rhel-8.0', 'priority': 20} - ] - message = self.load_json_from_file("testmodule_ready_message.json") - mock_mbs = mock.MagicMock() - fake_requires = {"platform": ["el8"]} - fake_mmd = self.make_mmd("testmodule", "rhel-8.0", - "20180409051516", "9e5fe74b", - requires=fake_requires) - mock_mbs.get_module_mmd.return_value = fake_mmd - modinfo = ModuleInfo.from_mbs_message(mock_mbs, message) - matched_config = AddTagHandler.get_module_config(configs, modinfo) - self.assertEqual(matched_config, None) + matched_config = AddTagHandler.get_module_config(fake_configs, modinfo) + self.assertEqual(matched_config, expected_result) + + def test_get_module_config_match_requires(self): + # ((fake_module_configs, expected result), ...) + test_matrix = ( + ( + [ + { + 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10, + 'requires': {'platform': 'el8'} + }, + # This noisy config should not impact to choose the correct one. + { + 'name': 'testmodule2', 'stream': 'rhel-8.0', 'priority': 20, + } + ], + { + 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10, + 'requires': {'platform': 'el8'} + }, + ), + ( + [ + # buildrequires could be specified in config as well. + { + 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10, + 'buildrequires': {'platform': 'el8'} + }, + ], + { + 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10, + 'buildrequires': {'platform': 'el8'} + }, + ), + ( + [ + # Both requires and buildrequires are specified to match the + # module metadata. + { + 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10, + 'requires': {'platform': 'el8'}, + 'buildrequires': {'platform': 'el8'}, + }, + ], + { + 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10, + 'requires': {'platform': 'el8'}, + 'buildrequires': {'platform': 'el8'}, + }, + ), + # Either requires or buildrequires is not included in module metadata. + ( + [ + { + 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10, + 'requires': {'platform': 'f30'}, + }, + ], + None, + ), + ( + [ + # Both requires and buildrequires are specified to match the + # module metadata. + { + 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10, + 'buildrequires': {'platform': 'f30'}, + }, + ], + None, + ), + ( + [ + { + 'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 10, + 'requires': {'platform': 'f30'}, + 'buildrequires': {'platform': 'f30'}, + }, + ], + None, + ), + # Either name or stream is not matched module metadata. + ( + # Module name is not matched in this config + [{'name': 'module-xxxx', 'stream': 'rhel-8.0', 'priority': 20}], + None, + ), + ( + # Module stream is not matched in this config + [{'name': 'testmodule', 'stream': '100', 'priority': 20}], + None, + ), + # Module name and stream are matched + ( + [ + {'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 20}, + {'name': 'testmodule2', 'stream': 'rhel-8.0', 'priority': 20}, + ], + {'name': 'testmodule', 'stream': 'rhel-8.0', 'priority': 20}, + ), + ) + for config, expected_result in test_matrix: + self.assert_get_module_config(config, expected_result) @mock.patch('ursa_major.mbs.requests.get') def test_get_match_tags_in_inheritance(self, requests_get): diff --git a/ursa_major/handlers/add_tag.py b/ursa_major/handlers/add_tag.py index 2ac22c7..b75ec30 100644 --- a/ursa_major/handlers/add_tag.py +++ b/ursa_major/handlers/add_tag.py @@ -28,7 +28,7 @@ import traceback from ursa_major.logger import log from ursa_major.mail import MailAPI -from ursa_major.utils import get_env_var, mmd_has_requires +from ursa_major.utils import get_env_var, mmd_has_requires, mmd_has_buildrequires from ursa_major.handlers.base import BaseHandler @@ -99,8 +99,13 @@ class AddTagHandler(BaseHandler): by the modinfo. :params module_configs: a list of module config + :type module_configs: list[dict] :params modinfo: instance of ModuleInfo - :return: an item in module configs, it's a dict. + :return: the matched module config. None is returned if no module + config matches the module metadata. + :rtype: dict + :raises RuntimeError: if more than one module configs match the + specified module metadata. """ matched = [] for config in module_configs: @@ -108,6 +113,9 @@ class AddTagHandler(BaseHandler): stream = config['stream'] if not (name == modinfo.name and stream == modinfo.stream): continue + dep_requires = config.get('buildrequires') + if dep_requires and not mmd_has_buildrequires(modinfo.mmd, dep_requires): + continue requires = config.get('requires', {}) if requires and not mmd_has_requires(modinfo.mmd, requires): continue diff --git a/ursa_major/utils.py b/ursa_major/utils.py index 8536e2a..9406c0d 100644 --- a/ursa_major/utils.py +++ b/ursa_major/utils.py @@ -88,19 +88,20 @@ def load_mmd(yaml, is_file=False): return mmd -def mmd_has_requires(mmd, requires): - """ - Check whether a module represent by the mmd has requires. - - :param mmd: Modulemd.Module object - :param requires: dict of requires, example: - {'platform': 'f28', 'python3': 'master'} +def requires_included(mmd_requires, config_requires): + """Test if requires defined in config is included in module metadata + + :param dict mmd_requires: a mapping representing either buildrequires or + requires, which is generally converted from module metadata. For + example, ``{"platform": "f29"}``. + :param dict config_requires: a mapping representing either buildrequires or + requires defined in config file. This is what to check if it is + included in ``mmd_requires``. + :return: True if all requires inside ``config_requires`` are included in + module metadata. Otherwise, False is returned. + :rtype: bool """ - deps_list = mmd.peek_dependencies() - mmd_requires = deps_list[0].peek_requires() if deps_list else {} - - neg_reqs = pos_reqs = [] - for req_name, req_streams in requires.items(): + for req_name, req_streams in config_requires.items(): if req_name not in mmd_requires.keys(): return False @@ -118,3 +119,34 @@ def mmd_has_requires(mmd, requires): if pos_reqs and not (set(streams) & set(pos_reqs)): return False return True + + +def mmd_has_requires(mmd, requires): + """ + Check whether a module represent by the mmd has requires. + + :param mmd: Modulemd.Module object + :param requires: dict of requires, example: + {'platform': 'f28', 'python3': 'master'} + """ + deps_list = mmd.peek_dependencies() + mmd_requires = deps_list[0].peek_requires() if deps_list else {} + return requires_included(mmd_requires, requires) + + +def mmd_has_buildrequires(mmd, config_buildrequires): + """ + Check if a module metadata represented by the mmd has buildrequires. + + :param mmd: a module metadata. + :type mmd: Modulemd.Module + :param dict config_buildrequires: a mapping of buildrequires defined in + config file to match the module metadata, for example: + ``{'platform': 'f28', 'python3': 'master'}``. + :return: True if the specified module metadata has the buildrequires + defined in config. + :rtype: bool + """ + deps_list = mmd.peek_dependencies() + mmd_requires = deps_list[0].peek_buildrequires() if deps_list else {} + return requires_included(mmd_requires, config_buildrequires)