From 7c81e5e33ab608da1f176c4d54643b525f8f3ae8 Mon Sep 17 00:00:00 2001 From: Qixiang Wan Date: Apr 27 2017 12:20:45 +0000 Subject: [PATCH 1/2] Refactor pdc.get_modules --- diff --git a/freshmaker/handlers/mbs.py b/freshmaker/handlers/mbs.py index 61aeb8d..f084de9 100644 --- a/freshmaker/handlers/mbs.py +++ b/freshmaker/handlers/mbs.py @@ -83,20 +83,11 @@ class MBS(BaseHandler): "in MBS", module_name, module_stream) pdc_session = pdc.get_client_session(conf) - depending_modules = pdc.get_modules(pdc_session, - build_dep_name=module_name, - build_dep_stream=module_stream, - active=True) - - # only rebuild the latest (by cmp variant_release) modules of - # (variant_name, variant_version) - latest_modules = [] - for (name, version) in set([(m.get('variant_name'), m.get('variant_version')) for m in depending_modules]): - mods = pdc.get_modules(pdc_session, name=name, version=version, active=True) - latest_modules.append(sorted(mods, key=lambda x: x['variant_release']).pop()) - - rebuild_modules = list(filter(lambda x: x in latest_modules, depending_modules)) - for mod in rebuild_modules: + modules = pdc.get_latest_modules(pdc_session, + build_dep_name=module_name, + build_dep_stream=module_stream, + active='true') + for mod in modules: module_name = mod['variant_name'] module_stream = mod['variant_version'] commitid = None diff --git a/freshmaker/pdc.py b/freshmaker/pdc.py index b052273..3e65fc8 100644 --- a/freshmaker/pdc.py +++ b/freshmaker/pdc.py @@ -50,22 +50,32 @@ def get_client_session(config): @freshmaker.utils.retry(wait_on=(requests.ConnectTimeout, requests.ConnectionError), logger=freshmaker.log) -def get_modules(pdc_session, name=None, version=None, build_dep_name=None, build_dep_stream=None, active=True): +def get_modules(pdc_session, **kwargs): """ + Query PDC with specified query parameters and return a list of modules. + :param pdc_session: PDCClient instance - :return: list of modules + :param kwargs: query parameters in keyword arguments + :return: a list of modules """ - query = {} - if name is not None: - query['variant_name'] = name - if version is not None: - query['variant_version'] = version - if build_dep_name is not None: - query['build_dep_name'] = build_dep_name - if build_dep_stream is not None: - query['build_dep_stream'] = build_dep_stream - if active: - query['active'] = 'true' - - modules = pdc_session['unreleasedvariants'](page_size=-1, **query) + modules = pdc_session['unreleasedvariants'](page_size=-1, **kwargs) return modules + + +def get_latest_modules(pdc_session, **kwargs): + """ + Query PDC with query parameters in kwargs and return a list of modules + which contains latest modules of each (module_name, module_version). + + :param pdc_session: PDCClient instance + :param kwargs: query parameters in keyword arguments, should only provide + valid query parameters supported by PDC's module query API. + :return: a list of modules + """ + modules = get_modules(pdc_session, **kwargs) + active = kwargs.get('active', 'true') + latest_modules = [] + for (name, version) in set([(m.get('variant_name'), m.get('variant_version')) for m in modules]): + mods = get_modules(pdc_session, variant_name=name, variant_version=version, active=active) + latest_modules.append(sorted(mods, key=lambda x: x['variant_release']).pop()) + return list(filter(lambda x: x in latest_modules, modules)) diff --git a/tests/helpers.py b/tests/helpers.py index 47a349b..c2d0467 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -43,10 +43,13 @@ class FedMsgFactory(object): self.source_name = 'unittest', self.source_version = '0.1.1', self.timestamp = time.time() - self.topic = 'org.fedoraproject.prod.mbs.module.state.change' + self.topic = '' self.username = 'freshmaker' self.i = random.randint(0, 100) - self.inner_msg = {} + + @property + def inner_msg(self): + return {} def produce(self): message_body = { @@ -69,24 +72,27 @@ class FedMsgFactory(object): class ModuleBuiltMessage(FedMsgFactory): def __init__(self, name, stream, state='ready', build_id=None, *args, **kwargs): super(ModuleBuiltMessage, self).__init__(*args, **kwargs) - states_dict = {} + self.topic = 'org.fedoraproject.prod.mbs.module.state.change' self.name = name self.stream = stream self.state = state self.build_id = build_id if build_id else random.randint(0, 1000) self.scmurl = "git://pkgs.fedoraproject.org/modules/%s?#%s" % (self.name, '123') + self._states_dict = {} for state, code in six.iteritems(BUILD_STATES): - states_dict[state] = {'state_name': state, 'state': code} + self._states_dict[state] = {'state_name': state, 'state': code} - inner_msg = { + @property + def inner_msg(self): + return { 'component_builds': [], 'id': self.build_id, 'modulemd': '', 'name': self.name, 'owner': 'freshmaker', 'scmurl': self.scmurl, - 'state': states_dict[self.state]['state'], + 'state': self._states_dict[self.state]['state'], 'state_name': self.state, 'state_reason': None, 'state_trace': [], @@ -98,7 +104,6 @@ class ModuleBuiltMessage(FedMsgFactory): 'time_submitted': time.time(), 'version': time.strftime("%Y%m%d%H%M%S"), } - self.inner_msg = inner_msg class PDCModuleInfoFactory(object): diff --git a/tests/test_mbs_handler.py b/tests/test_mbs_handler.py index d9dd52f..b30898c 100644 --- a/tests/test_mbs_handler.py +++ b/tests/test_mbs_handler.py @@ -62,10 +62,10 @@ class MBSHandlerTest(unittest.TestCase): handler = MBS() self.assertFalse(handler.can_handle(event)) + @mock.patch('freshmaker.pdc.get_modules') @mock.patch('freshmaker.handlers.mbs.utils') - @mock.patch('freshmaker.handlers.mbs.pdc') @mock.patch('freshmaker.handlers.mbs.conf') - def test_rebuild_depending_modules_on_module_built_event(self, conf, pdc, utils): + def test_rebuild_depending_modules_on_module_built_event(self, conf, utils, get_modules): """ Tests MBS handler can rebuild all modules which depend on the module in module built event. @@ -83,7 +83,9 @@ class MBSHandlerTest(unittest.TestCase): mod3_r1_info.add_build_dep('testmodule', 'master') mod3_r1 = mod3_r1_info.produce() - def get_modules(pdc_session, name=None, version=None, build_dep_name=None, build_dep_stream=None, active=True): + def mock_get_modules(pdc_session, **kwargs): + name = kwargs.get('variant_name', None) + version = kwargs.get('variant_version', None) if name == 'testmodule2' and version == 'master': return [mod2_r1] @@ -92,7 +94,7 @@ class MBSHandlerTest(unittest.TestCase): else: return [mod2_r1, mod3_r1] - pdc.get_modules.side_effect = get_modules + get_modules.side_effect = mock_get_modules conf.git_base_url = "git://pkgs.fedoraproject.org" utils.get_commit_hash.side_effect = [ "fae7848fa47a854f25b782aa64441040a6d86544", @@ -105,10 +107,10 @@ class MBSHandlerTest(unittest.TestCase): [mock.call(u'git://pkgs.fedoraproject.org/modules/testmodule2.git?#fae7848fa47a854f25b782aa64441040a6d86544', u'master'), mock.call(u'git://pkgs.fedoraproject.org/modules/testmodule3.git?#43ec03000d249231bc7135b11b810afc96e90efb', u'master')]) + @mock.patch('freshmaker.pdc.get_modules') @mock.patch('freshmaker.handlers.mbs.utils') - @mock.patch('freshmaker.handlers.mbs.pdc') @mock.patch('freshmaker.handlers.mbs.conf') - def test_only_rebuild_latest_depending_modules_on_module_built_event(self, conf, pdc, utils): + def test_only_rebuild_latest_depending_modules_on_module_built_event(self, conf, utils, get_modules): """ Tests MBS handler only rebuild latest depending modules. If there is a module only has old release depends on the module, it won't be rebuilt. @@ -130,7 +132,9 @@ class MBSHandlerTest(unittest.TestCase): mod3_r2_info.add_build_dep('testmodule', 'master') mod3_r2 = mod3_r2_info.produce() - def get_modules(pdc_session, name=None, version=None, build_dep_name=None, build_dep_stream=None, active=True): + def mock_get_modules(pdc_session, **kwargs): + name = kwargs.get('variant_name', None) + version = kwargs.get('variant_version', None) if name == 'testmodule2' and version == 'master': return [mod2_r1] @@ -141,7 +145,7 @@ class MBSHandlerTest(unittest.TestCase): # query for testmodule3 releases, get mod3_r1 and mod3_r2, # only mod3_r1 depends on testmodule, and r1 < r2. - pdc.get_modules.side_effect = get_modules + get_modules.side_effect = mock_get_modules conf.git_base_url = "git://pkgs.fedoraproject.org" utils.get_commit_hash.side_effect = [ "fae7848fa47a854f25b782aa64441040a6d86544", From af893f8eaeb4564d0a00e06c43a4acb0ecb37dab Mon Sep 17 00:00:00 2001 From: Qixiang Wan Date: Apr 27 2017 12:20:51 +0000 Subject: [PATCH 2/2] Rebuild module when spec file of rpm in module is updated --- diff --git a/freshmaker/events.py b/freshmaker/events.py index b152769..b6eee8a 100644 --- a/freshmaker/events.py +++ b/freshmaker/events.py @@ -149,6 +149,21 @@ class ModuleMetadataUpdated(BaseEvent): self.branch = branch +class RPMSpecUpdated(BaseEvent): + """ + Provides an event object for "RPM spec file in dist-git updated". + + :param rpm: RPM name, also known as the name of component or source package. + :param branch: Branch of updated RPM spec. + :param rev: revision. + """ + def __init__(self, msg_id, rpm, branch, rev): + super(RPMSpecUpdated, self).__init__(msg_id) + self.rpm = rpm + self.branch = branch + self.rev = rev + + class TestingEvent(BaseEvent): """ Event useds in unit-tests. diff --git a/freshmaker/handlers/mbs.py b/freshmaker/handlers/mbs.py index f084de9..725c9ae 100644 --- a/freshmaker/handlers/mbs.py +++ b/freshmaker/handlers/mbs.py @@ -23,9 +23,9 @@ import requests -from freshmaker import log, conf, pdc, utils +from freshmaker import log, conf, utils, pdc from freshmaker.handlers import BaseHandler -from freshmaker.events import ModuleBuilt, ModuleMetadataUpdated +from freshmaker.events import ModuleBuilt, ModuleMetadataUpdated, RPMSpecUpdated class MBS(BaseHandler): @@ -42,6 +42,9 @@ class MBS(BaseHandler): if isinstance(event, ModuleMetadataUpdated): return True + if isinstance(event, RPMSpecUpdated): + return True + return False def rebuild_module(self, scm_url, branch): @@ -64,6 +67,23 @@ class MBS(BaseHandler): log.error("Error when triggering rebuild of %s: %s", scm_url, data) return None + def bump_and_rebuild_module(self, name, branch, commit_msg=None): + """Bump module repo with an empty commit and submit a module build request to MBS""" + commitid = None + with utils.temp_dir(prefix='freshmaker-%s-' % name) as repodir: + try: + utils.clone_module_repo(name, repodir, branch=branch, user=conf.git_user, logger=log) + msg = commit_msg or "Bump" + utils.add_empty_commit(repodir, msg=msg, logger=log) + commitid = utils.get_commit_hash(repodir) + utils.push_repo(repodir) + except Exception: + log.exception("Failed to update module repo of '%s-%s'.", name, branch) + + if commitid is not None: + scm_url = conf.git_base_url + '/modules/%s.git?#%s' % (name, commitid) + self.rebuild_module(scm_url, branch) + def handle_metadata_update(self, event): log.info("Triggering rebuild of %s, metadata updated", event.scm_url) self.rebuild_module(event.scm_url, event.branch) @@ -88,21 +108,30 @@ class MBS(BaseHandler): build_dep_stream=module_stream, active='true') for mod in modules: + commit_msg = "Bump to rebuild because of %s update" % module_name + self.bump_and_rebuild_module(mod['variant_name'], + mod['variant_version'], + commit_msg=commit_msg) + return [] + + def handle_rpm_spec_updated(self, event): + """ + Rebuild module when spec file of rpm in module is updated. + """ + rpm = event.rpm + branch = event.branch + rev = event.rev + pdc_session = pdc.get_client_session(conf) + modules = pdc.get_latest_modules(pdc_session, + component_name=rpm, + component_branch=branch, + active='true') + for mod in modules: module_name = mod['variant_name'] - module_stream = mod['variant_version'] - commitid = None - with utils.temp_dir(prefix='freshmaker-%s-' % module_name) as repodir: - try: - utils.clone_module_repo(module_name, repodir, branch=module_stream, user=conf.git_user, logger=log) - utils.add_empty_commit(repodir, msg="Bumped to rebuild because of %s update" % module_name, logger=log) - commitid = utils.get_commit_hash(repodir) - utils.push_repo(repodir) - except Exception: - log.exception("Failed to update module repo for '%s-%s'.", module_name, module_stream) - - if commitid is not None: - scm_url = conf.git_base_url + '/modules/%s.git?#%s' % (module_name, commitid) - self.rebuild_module(scm_url, module_stream) + module_branch = mod['variant_version'] + log.info("Going to rebuild module '%s:%s'.", module_name, module_branch) + commit_msg = "Bump to rebuild because of %s rpm spec update (%s)." % (rpm, rev) + self.bump_and_rebuild_module(module_name, module_branch, commit_msg=commit_msg) return [] @@ -111,5 +140,7 @@ class MBS(BaseHandler): return self.handle_metadata_update(event) elif isinstance(event, ModuleBuilt): return self.handle_module_built(event) + elif isinstance(event, RPMSpecUpdated): + return self.handle_rpm_spec_updated(event) return [] diff --git a/freshmaker/parsers/gitreceive.py b/freshmaker/parsers/gitreceive.py index 4ba94e7..6982577 100644 --- a/freshmaker/parsers/gitreceive.py +++ b/freshmaker/parsers/gitreceive.py @@ -25,6 +25,7 @@ from freshmaker import log, conf from freshmaker.parsers import BaseParser from freshmaker.events import ModuleMetadataUpdated from freshmaker.events import DockerfileChanged +from freshmaker.events import RPMSpecUpdated class GitReceiveParser(BaseParser): @@ -78,5 +79,13 @@ class GitReceiveParser(BaseParser): repo_url = '{}/{}/{}.git'.format(conf.git_base_url, namespace, repo) return DockerfileChanged(msg_id, repo_url=repo_url, branch=branch, namespace=namespace, repo=repo, rev=rev) + elif namespace == 'rpms': + component = commit.get('repo') + branch = commit.get('branch') + rev = commit.get('rev') + changed_files = commit.get('stats', {}).get('files', {}).keys() + has_spec = any([i.endswith('.spec') for i in changed_files]) + if has_spec: + return RPMSpecUpdated(msg_id, component, branch, rev=rev) return None diff --git a/tests/helpers.py b/tests/helpers.py index c2d0467..2c60c96 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -106,6 +106,52 @@ class ModuleBuiltMessage(FedMsgFactory): } +class DistGitMessage(FedMsgFactory): + def __init__(self, namespace, repo, branch, rev, *args, **kwargs): + super(DistGitMessage, self).__init__(*args, **kwargs) + self.topic = 'org.fedoraproject.prod.git.receive' + self.namespace = namespace + self.repo = repo + self.branch = branch + self.rev = rev + self.stats = {'files': {}, + 'total': { + 'additions': 0, + 'deletions': 0, + 'files': 0, + 'lines': 0, + }} + + @property + def inner_msg(self): + return { + 'commit': { + 'repo': self.repo, + 'namespace': self.namespace, + 'branch': self.branch, + 'rev': self.rev, + 'agent': 'freshmaker', + 'name': 'freshmaker', + 'username': 'freshmaker', + 'email': 'freshmaker@example.com', + 'message': 'test message', + 'summary': 'test', + 'path': "/srv/git/repositories/%s/%s.git" % (self.namespace, self.repo), + 'seen': False, + 'stats': self.stats, + } + } + + def add_changed_file(self, filename, additions, deletions): + self.stats['files'].setdefault(filename, {})['additions'] = additions + self.stats['files'][filename]['deletions'] = deletions + self.stats['files'][filename]['lines'] = additions + deletions + self.stats['total']['additions'] += additions + self.stats['total']['deletions'] += deletions + self.stats['total']['files'] += 1 + self.stats['total']['lines'] += self.stats['files'][filename]['lines'] + + class PDCModuleInfoFactory(object): def __init__(self, name, version, release, active=True): self.variant_name = name @@ -119,6 +165,7 @@ class PDCModuleInfoFactory(object): self.build_deps = [] self.runtime_deps = [] self.koji_tag = 'module-%s' % ''.join([random.choice(string.ascii_letters[:6] + string.digits) for n in range(16)]) + self.rpms = [] def produce(self): module = { @@ -133,6 +180,7 @@ class PDCModuleInfoFactory(object): 'koji_tag': self.koji_tag, 'build_deps': self.build_deps, 'runtime_deps': self.runtime_deps, + 'rpms': self.rpms, } return module @@ -140,3 +188,6 @@ class PDCModuleInfoFactory(object): class PDCModuleInfo(PDCModuleInfoFactory): def add_build_dep(self, name, stream): self.build_deps.append({'dependency': name, 'stream': stream}) + + def add_rpm(self, rpm): + self.rpms.append(rpm) diff --git a/tests/test_mbs_handler.py b/tests/test_mbs_handler.py index b30898c..17e4a67 100644 --- a/tests/test_mbs_handler.py +++ b/tests/test_mbs_handler.py @@ -29,11 +29,13 @@ from tests import helpers from freshmaker import events from freshmaker.handlers.mbs import MBS from freshmaker.parsers.mbsmodule import MBSModuleParser +from freshmaker.parsers.gitreceive import GitReceiveParser class MBSHandlerTest(unittest.TestCase): def setUp(self): events.BaseEvent.register_parser(MBSModuleParser) + events.BaseEvent.register_parser(GitReceiveParser) def _get_event(self, message): event = events.BaseEvent.from_fedmsg(message['body']['topic'], message['body']) @@ -157,6 +159,64 @@ class MBSHandlerTest(unittest.TestCase): self.assertEqual(handler.rebuild_module.call_args_list, [mock.call(u'git://pkgs.fedoraproject.org/modules/testmodule2.git?#fae7848fa47a854f25b782aa64441040a6d86544', u'master')]) + def test_can_handle_rpm_spec_updated_event(self): + """ + Tests MBS handler can handle rpm spec updated event + """ + m = helpers.DistGitMessage('rpms', 'bash', 'master', '123') + m.add_changed_file('bash.spec', 1, 1) + msg = m.produce() + + event = self._get_event(msg) + + handler = MBS() + self.assertTrue(handler.can_handle(event)) + + def test_can_not_handle_no_spec_updated_dist_git_event(self): + """ + Tests RPMSpechandler can not handle dist git message that + spec file is not updated. + """ + + m = helpers.DistGitMessage('rpms', 'bash', 'master', '123') + m.add_changed_file('test.c', 1, 1) + msg = m.produce() + + event = self._get_event(msg) + + handler = MBS() + self.assertFalse(handler.can_handle(event)) + + @mock.patch('freshmaker.handlers.mbs.utils') + @mock.patch('freshmaker.handlers.mbs.pdc') + @mock.patch('freshmaker.handlers.mbs.conf') + def test_trigger_module_rebuild_when_rpm_spec_updated(self, conf, pdc, utils): + """ + Test RPMSpecHandler can trigger module rebuild when spec + file of rpm in module updated. + """ + conf.git_base_url = "git://pkgs.fedoraproject.org" + + m = helpers.DistGitMessage('rpms', 'bash', 'master', '123') + m.add_changed_file('bash.spec', 1, 1) + msg = m.produce() + + event = self._get_event(msg) + + mod_info = helpers.PDCModuleInfo('testmodule', 'master', '20170412010101') + mod_info.add_rpm("bash-1.2.3-4.f26.rpm") + mod = mod_info.produce() + pdc.get_latest_modules.return_value = [mod] + + commitid = '9287eb8eb4c4c60f73b4a59f228a673846d940c6' + utils.get_commit_hash.return_value = commitid + + handler = MBS() + handler.rebuild_module = mock.Mock() + handler.handle(event) + self.assertEqual(handler.rebuild_module.call_args_list, + [mock.call('git://pkgs.fedoraproject.org/modules/testmodule.git?#%s' % commitid, 'master')]) + if __name__ == '__main__': unittest.main()