From 8bbe2d359f6829135abfef71e7367e4e571094e9 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Wed, 5 Apr 2017 12:46:44 +0200 Subject: [PATCH] Trigger newRepo by MBS instead of waiting on Kojira in case when we have sucessfully tagged all the components --- module_build_service/messaging.py | 22 +++- .../migrations/versions/3b17cd6dc583_.py | 28 +++++ module_build_service/models.py | 16 +++ module_build_service/scheduler/consumer.py | 5 + .../scheduler/handlers/tags.py | 78 ++++++++++++ module_build_service/scheduler/producer.py | 28 +++++ tests/test_messaging.py | 25 ++++ tests/test_scheduler/test_poller.py | 70 +++++++++++ tests/test_scheduler/test_tag_tagged.py | 115 ++++++++++++++++++ 9 files changed, 384 insertions(+), 3 deletions(-) create mode 100644 module_build_service/migrations/versions/3b17cd6dc583_.py create mode 100644 module_build_service/scheduler/handlers/tags.py create mode 100644 tests/test_scheduler/test_tag_tagged.py diff --git a/module_build_service/messaging.py b/module_build_service/messaging.py index cfc29880..52b32ba6 100644 --- a/module_build_service/messaging.py +++ b/module_build_service/messaging.py @@ -159,9 +159,9 @@ class BaseMessage(object): topic_categories = _messaging_backends['fedmsg']['services'] categories_re = '|'.join(map(re.escape, topic_categories)) regex_pattern = re.compile( - (r'(?P' + categories_re + r')(?:\.)' - r'(?Pbuild|repo|module)(?:(?:\.)' - r'(?Pstate|build))?(?:\.)(?Pchange|done|end)$')) + (r'(?P' + categories_re + r')(?:(?:\.)' + r'(?Pbuild|repo|module))?(?:(?:\.)' + r'(?Pstate|build))?(?:\.)(?Pchange|done|end|tag)$')) regex_results = re.search(regex_pattern, topic) if regex_results: @@ -210,6 +210,11 @@ class BaseMessage(object): repo_tag = msg_inner_msg.get('tag') msg_obj = KojiRepoChange(msg_id, repo_tag) + elif category == 'buildsys' and event == 'tag': + tag = msg_inner_msg.get('tag') + artifact = msg_inner_msg.get('name') + msg_obj = KojiTagChange(msg_id, tag, artifact) + elif category == 'mbs' and object == 'module' and \ subobject == 'state' and event == 'change': msg_obj = MBSModule( @@ -260,6 +265,17 @@ class KojiBuildChange(BaseMessage): self.module_build_id = module_build_id self.state_reason = state_reason +class KojiTagChange(BaseMessage): + """ + A class that inherits from BaseMessage to provide a message + object for a buildsys.tag info (in fedmsg this replaces the msg dictionary) + :param tag: the name of tag (e.g. module-123456789-build) + :param artifact: the name of tagged artifact (e.g. module-build-macros) + """ + def __init__(self, msg_id, tag, artifact): + super(KojiTagChange, self).__init__(msg_id) + self.tag = tag + self.artifact = artifact class KojiRepoChange(BaseMessage): """ A class that inherits from BaseMessage to provide a message diff --git a/module_build_service/migrations/versions/3b17cd6dc583_.py b/module_build_service/migrations/versions/3b17cd6dc583_.py new file mode 100644 index 00000000..69f8c96c --- /dev/null +++ b/module_build_service/migrations/versions/3b17cd6dc583_.py @@ -0,0 +1,28 @@ +"""Add new_repo_task_id and tagged columns. + +Revision ID: 3b17cd6dc583 +Revises: 335455a30585 +Create Date: 2017-04-05 16:15:13.613851 + +""" + +# revision identifiers, used by Alembic. +revision = '3b17cd6dc583' +down_revision = '335455a30585' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + with op.batch_alter_table('component_builds') as b: + b.add_column(sa.Column('tagged', sa.Boolean(), nullable=True)) + with op.batch_alter_table('module_builds') as b: + b.add_column(sa.Column('new_repo_task_id', sa.Integer(), nullable=True)) + + +def downgrade(): + with op.batch_alter_table('component_builds') as b: + b.drop_column('tagged') + with op.batch_alter_table('module_builds') as b: + b.drop_column('new_repo_task_id') diff --git a/module_build_service/models.py b/module_build_service/models.py index c0d25a0b..23507eb5 100644 --- a/module_build_service/models.py +++ b/module_build_service/models.py @@ -109,6 +109,7 @@ class ModuleBuild(MBSBase): time_submitted = db.Column(db.DateTime, nullable=False) time_modified = db.Column(db.DateTime) time_completed = db.Column(db.DateTime) + new_repo_task_id = db.Column(db.Integer) # A monotonically increasing integer that represents which batch or # iteration this module is currently on for successive rebuilds of its @@ -233,6 +234,19 @@ class ModuleBuild(MBSBase): return query.first() + @classmethod + def from_tag_change_event(cls, session, event): + tag = event.tag.strip('-build') + query = session.query(cls)\ + .filter(cls.koji_tag == tag)\ + .filter(cls.state == BUILD_STATES["build"]) + + count = query.count() + if count > 1: + raise RuntimeError("%r module builds in flight for %r" % (count, tag)) + + return query.first() + def json(self): return { 'id': self.id, @@ -361,6 +375,8 @@ class ComponentBuild(MBSBase): state_reason = db.Column(db.String) # This stays as None until the build completes. nvr = db.Column(db.String) + # True when this component build is tagged into buildroot. + tagged = db.Column(db.Boolean, default=False) # A monotonically increasing integer that represents which batch or # iteration this *component* is currently in. This relates to the owning diff --git a/module_build_service/scheduler/consumer.py b/module_build_service/scheduler/consumer.py index 76e0bce6..01d68ef0 100644 --- a/module_build_service/scheduler/consumer.py +++ b/module_build_service/scheduler/consumer.py @@ -37,6 +37,7 @@ import module_build_service.messaging import module_build_service.scheduler.handlers.repos import module_build_service.scheduler.handlers.components import module_build_service.scheduler.handlers.modules +import module_build_service.scheduler.handlers.tags from module_build_service import models, log, conf @@ -97,6 +98,7 @@ class MBSConsumer(fedmsg.consumers.FedmsgConsumer): } # Only one kind of repo change event, though... self.on_repo_change = module_build_service.scheduler.handlers.repos.done + self.on_tag_change = module_build_service.scheduler.handlers.tags.tagged self.sanity_check() def shutdown(self): @@ -186,6 +188,9 @@ class MBSConsumer(fedmsg.consumers.FedmsgConsumer): elif type(msg) == module_build_service.messaging.KojiRepoChange: handler = self.on_repo_change build = models.ModuleBuild.from_repo_done_event(session, msg) + elif type(msg) == module_build_service.messaging.KojiTagChange: + handler = self.on_tag_change + build = models.ModuleBuild.from_tag_change_event(session, msg) elif type(msg) == module_build_service.messaging.MBSModule: handler = self.on_module_change[module_build_state_from_msg(msg)] build = models.ModuleBuild.from_module_event(session, msg) diff --git a/module_build_service/scheduler/handlers/tags.py b/module_build_service/scheduler/handlers/tags.py new file mode 100644 index 00000000..b2b381be --- /dev/null +++ b/module_build_service/scheduler/handlers/tags.py @@ -0,0 +1,78 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2017 Red Hat, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +# Written by Jan Kaluza + +""" Handlers for repo change events on the message bus. """ + +import module_build_service.builder +import module_build_service.pdc +import logging +import koji +from module_build_service import models, log +from module_build_service.utils import start_next_batch_build + +logging.basicConfig(level=logging.DEBUG) + + +def tagged(config, session, msg): + """ Called whenever koji tags a build to tag. """ + + if not config.system == "koji": + return [] + + # Find our ModuleBuild associated with this tagged artifact. + tag = msg.tag + if not tag.endswith('-build'): + log.debug("Tag %r does not end with '-build' suffix, ignoring" % tag) + return + module_build = models.ModuleBuild.from_tag_change_event(session, msg) + if not module_build: + log.debug("No module build found associated with koji tag %r" % tag) + return + + # Find tagged component. + component = models.ComponentBuild.from_component_name( + session, msg.artifact, module_build.id) + if not component: + log.error("No component %s in module %r", msg.artifact, module_build) + return + + # Mark the component as tagged + component.tagged = True + session.commit() + + # Get the list of untagged components in current batch. + untagged_components = [ + c for c in module_build.current_batch() + if not c.tagged + ] + + # If all components are tagged, start newRepo task. + if not untagged_components: + log.info("All components tagged, regenerating repo for tag %s", tag) + builder = module_build_service.builder.GenericBuilder.create_from_module( + session, module_build, config) + task_id = builder.koji_session.newRepo(tag) + module_build.new_repo_task_id = task_id + session.commit() + + return [] diff --git a/module_build_service/scheduler/producer.py b/module_build_service/scheduler/producer.py index 116d035d..e7d39d49 100644 --- a/module_build_service/scheduler/producer.py +++ b/module_build_service/scheduler/producer.py @@ -48,6 +48,7 @@ class MBSProducer(PollingProducer): self.process_open_component_builds(session) self.fail_lost_builds(session) self.process_paused_module_builds(conf, session) + self.trigger_new_repo_when_stalled(conf, session) log.info('Poller will now sleep for "{}" seconds' .format(conf.polling_interval)) @@ -194,3 +195,30 @@ class MBSProducer(PollingProducer): if module_build_service.utils.at_concurrent_component_threshold( config, session): break + + def trigger_new_repo_when_stalled(self, config, session): + """ + Sometimes the Koji repo regeneration stays in "init" state without + doing anything and our module build stucks. In case the module build + gets stuck on that, we trigger newRepo again to rebuild it. + """ + if config.system != 'koji': + return + + koji_session = module_build_service.builder.KojiModuleBuilder\ + .get_session(config, None) + + for module_build in session.query(models.ModuleBuild).filter_by( + state=models.BUILD_STATES['build']).all(): + if not module_build.new_repo_task_id: + continue + + task_info = koji_session.getTaskInfo(module_build.new_repo_task_id) + if (task_info["state"] in [koji.TASK_STATES['CANCELED'], + koji.TASK_STATES['FAILED']]): + log.info("newRepo task %s for %r failed, starting another one", + str(module_build.new_repo_task_id), module_build) + taginfo = koji_session.getTag(module_build.koji_tag + "-build") + module_build.new_repo_task_id = koji_session.newRepo(taginfo["name"]) + + session.commit() diff --git a/tests/test_messaging.py b/tests/test_messaging.py index 97043f55..3e408a68 100644 --- a/tests/test_messaging.py +++ b/tests/test_messaging.py @@ -90,3 +90,28 @@ class TestFedmsgMessaging(unittest.TestCase): self.assertEqual(msg.build_release, '1.20150203.git.c8504a8a.fc21') self.assertEqual(msg.state_reason, 'build end: user:fatka copr:mutt-kz build:100 ip:172.16.3.3 pid:12010 status:1') + + def test_buildsys_tag(self): + # https://fedora-fedmsg.readthedocs.io/en/latest/topics.html#id134 + buildsys_tag_msg = { + "msg": { + "build_id": 875961, + "name": "module-build-macros", + "tag_id": 619, + "instance": "primary", + "tag": "module-debugging-tools-master-20170405115403-build", + "user": "mbs/mbs.fedoraproject.org", + "version": "0.1", + "owner": "mbs/mbs.fedoraproject.org", + "release": "1.module_0c3d13fd" + }, + 'msg_id': '2015-51be4c8e-8ab6-4dcb-ac0d-37b257765c71', + 'timestamp': 1424789698.0, + 'topic': 'org.fedoraproject.prod.buildsys.tag' + } + + topic = 'org.fedoraproject.prod.buildsys.tag' + msg = messaging.BaseMessage.from_fedmsg(topic, buildsys_tag_msg) + + self.assertEqual(msg.tag, "module-debugging-tools-master-20170405115403-build") + self.assertEqual(msg.artifact, "module-build-macros") diff --git a/tests/test_scheduler/test_poller.py b/tests/test_scheduler/test_poller.py index 71bf15fd..aeba94a0 100644 --- a/tests/test_scheduler/test_poller.py +++ b/tests/test_scheduler/test_poller.py @@ -88,3 +88,73 @@ class TestPoller(unittest.TestCase): components = module_build.current_batch() for component in components: self.assertEqual(component.state, koji.BUILD_STATES["BUILDING"]) + + def test_trigger_new_repo_when_failed(self, crete_builder, + koji_get_session, global_consumer, + dbg): + """ + Tests that we call koji_sesion.newRepo when newRepo task failed. + """ + consumer = mock.MagicMock() + consumer.incoming = queue.Queue() + global_consumer.return_value = consumer + + koji_session = mock.MagicMock() + koji_session.getTag = lambda tag_name: {'name': tag_name} + koji_session.getTaskInfo.return_value = {'state': koji.TASK_STATES['FAILED']} + koji_session.newRepo.return_value = 123456 + koji_get_session.return_value = koji_session + + builder = mock.MagicMock() + builder.buildroot_ready.return_value = False + crete_builder.return_value = builder + + # Change the batch to 2, so the module build is in state where + # it is not building anything, but the state is "build". + module_build = models.ModuleBuild.query.filter_by(id=2).one() + module_build.batch = 2 + module_build.new_repo_task_id = 123456 + db.session.commit() + + hub = mock.MagicMock() + poller = MBSProducer(hub) + poller.poll() + + koji_session.newRepo.assert_called_once_with("module-testmodule-build") + + + def test_trigger_new_repo_when_succeded(self, crete_builder, + koji_get_session, global_consumer, + dbg): + """ + Tests that we do not call koji_sesion.newRepo when newRepo task + succeeded. + """ + consumer = mock.MagicMock() + consumer.incoming = queue.Queue() + global_consumer.return_value = consumer + + koji_session = mock.MagicMock() + koji_session.getTag = lambda tag_name: {'name': tag_name} + koji_session.getTaskInfo.return_value = {'state': koji.TASK_STATES['CLOSED']} + koji_session.newRepo.return_value = 123456 + koji_get_session.return_value = koji_session + + builder = mock.MagicMock() + builder.buildroot_ready.return_value = False + crete_builder.return_value = builder + + # Change the batch to 2, so the module build is in state where + # it is not building anything, but the state is "build". + module_build = models.ModuleBuild.query.filter_by(id=2).one() + module_build.batch = 2 + module_build.new_repo_task_id = 123456 + db.session.commit() + + hub = mock.MagicMock() + poller = MBSProducer(hub) + poller.poll() + + self.assertTrue(not koji_session.newRepo.called) + + diff --git a/tests/test_scheduler/test_tag_tagged.py b/tests/test_scheduler/test_tag_tagged.py new file mode 100644 index 00000000..eb810cc5 --- /dev/null +++ b/tests/test_scheduler/test_tag_tagged.py @@ -0,0 +1,115 @@ +# Copyright (c) 2016 Red Hat, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +# Written by Jan Kaluza + +from os.path import dirname +import unittest +import mock +import vcr + +from mock import patch + +import module_build_service.messaging +import module_build_service.scheduler.handlers.repos +import module_build_service.models +from tests import test_reuse_component_init_data +from tests import conf, db, app + +import koji + +class TestTagTagged(unittest.TestCase): + + def setUp(self): + test_reuse_component_init_data() + + def tearDown(self): + pass + + @mock.patch('module_build_service.models.ModuleBuild.from_tag_change_event') + def test_no_matching_module(self, from_tag_change_event): + """ Test that when a tag msg hits us and we have no match, + that we do nothing gracefully. + """ + from_tag_change_event.return_value = None + msg = module_build_service.messaging.KojiTagChange( + 'no matches for this...', '2016-some-nonexistent-build', "artifact") + module_build_service.scheduler.handlers.tags.tagged( + config=conf, session=db.session, msg=msg) + + def test_no_matching_artifact(self): + """ Test that when a tag msg hits us and we have no match, + that we do nothing gracefully. + """ + msg = module_build_service.messaging.KojiTagChange( + 'id', 'module-testmodule-build', "artifact") + module_build_service.scheduler.handlers.tags.tagged( + config=conf, session=db.session, msg=msg) + + + @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups", + return_value = {'build': [], 'srpm-build': []}) + @patch("module_build_service.builder.KojiModuleBuilder.get_session") + @patch("module_build_service.builder.GenericBuilder.create_from_module") + def test_newrepo(self, create_builder, koji_get_session, dbg): + """ + Test that newRepo is called in the expected times. + """ + koji_session = mock.MagicMock() + koji_session.getTag = lambda tag_name: {'name': tag_name} + koji_session.getTaskInfo.return_value = {'state': koji.TASK_STATES['CLOSED']} + koji_session.newRepo.return_value = 123456 + koji_get_session.return_value = koji_session + + builder = mock.MagicMock() + builder.koji_session = koji_session + builder.buildroot_ready.return_value = False + create_builder.return_value = builder + + module_build = module_build_service.models.ModuleBuild.query.filter_by(id=2).one() + module_build.batch = 2 + db.session.commit() + + # Tag the first component to the buildroot. + msg = module_build_service.messaging.KojiTagChange( + 'id', 'module-testmodule-build', "perl-Tangerine") + module_build_service.scheduler.handlers.tags.tagged( + config=conf, session=db.session, msg=msg) + + # newRepo should not be called, because there are still components + # to tag. + self.assertTrue(not koji_session.newRepo.called) + + # Tag the second component to the buildroot. + msg = module_build_service.messaging.KojiTagChange( + 'id', 'module-testmodule-build', "perl-List-Compare") + module_build_service.scheduler.handlers.tags.tagged( + config=conf, session=db.session, msg=msg) + + # newRepo should be called now - all components have been tagged. + koji_session.newRepo.assert_called_once_with("module-testmodule-build") + + # Refresh our module_build object. + db.session.expunge(module_build) + module_build = module_build_service.models.ModuleBuild.query.filter_by(id=2).one() + + # newRepo task_id should be stored in database, so we can check its + # status later in poller. + self.assertEqual(module_build.new_repo_task_id, 123456)