From 706e5be55de210918b3db56550668399e74dfee3 Mon Sep 17 00:00:00 2001 From: Qixiang Wan Date: Jun 08 2017 06:50:44 +0000 Subject: [PATCH 1/3] Record dep_of when a module build is triggered When a module build is triggered by the event of module built, we add the triggerred build in db with dep of the build in event. --- diff --git a/freshmaker/handlers/mbs/module_state_change.py b/freshmaker/handlers/mbs/module_state_change.py index f347407..fa907b5 100644 --- a/freshmaker/handlers/mbs/module_state_change.py +++ b/freshmaker/handlers/mbs/module_state_change.py @@ -22,7 +22,7 @@ # Written by Jan Kaluza from freshmaker import log, conf, utils, db, models -from freshmaker.types import ArtifactType +from freshmaker.types import ArtifactType, ArtifactBuildState from freshmaker.mbs import MBS from freshmaker.pdc import PDC from freshmaker.handlers import BaseHandler @@ -50,19 +50,21 @@ class MBSModuleStateChangeHandler(BaseHandler): build_id = event.build_id build_state = event.build_state + module_build = None # update build state if the build is submitted by Freshmaker builds = db.session.query(models.ArtifactBuild).filter_by(build_id=build_id, type=ArtifactType.MODULE.value).all() if len(builds) > 1: raise RuntimeError("Found duplicate module build '%s' in db" % build_id) if len(builds) == 1: - build = builds.pop() + # we can find this build in DB + module_build = builds.pop() if build_state in [MBS.BUILD_STATES['ready'], MBS.BUILD_STATES['failed']]: log.info("Module build '%s' state changed in MBS, updating it in db.", build_id) if build_state == MBS.BUILD_STATES['ready']: - build.state = models.BUILD_STATES['done'] + module_build.state = ArtifactBuildState.DONE.value if build_state == MBS.BUILD_STATES['failed']: - build.state = models.BUILD_STATES['failed'] + module_build.state = ArtifactBuildState.FAILED.value db.session.commit() # Rebuild depending modules when state of MBSModuleStateChangeEvent is 'ready' @@ -85,8 +87,8 @@ class MBSModuleStateChangeHandler(BaseHandler): # bump module repo first commit_msg = "Bump to rebuild because of %s update" % module_name rev = utils.bump_distgit_repo('modules', name, branch=version, commit_msg=commit_msg, logger=log) - build_id = self.build_module(name, version, rev) - if build_id is not None: - self.record_build(event, name, 'module', build_id) + new_build_id = self.build_module(name, version, rev) + if new_build_id is not None: + self.record_build(event, name, 'module', new_build_id, dep_of=module_build) return [] From eb6f45bd4b2fd27cdc04da70801a6aebc4518d7d Mon Sep 17 00:00:00 2001 From: Qixiang Wan Date: Jun 08 2017 06:53:19 +0000 Subject: [PATCH 2/3] New api get_root_dep_of to get the root dep build Look through the builds to find out the root dep of a build. For example, in the following case: 1. build_4's dep_of is build_3 2. build_3's dep_of is build_2 3. build_2's dep_of is build_1 4. build_1 doesn't has dep_of Then: 1. Root dep of build_4 is build1 2. Root dep of build_3 is build1 3. Root dep of build_2 is build1 4. Root dep of build_1 is None --- diff --git a/freshmaker/models.py b/freshmaker/models.py index 25daf1a..cf0d207 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -149,3 +149,13 @@ class ArtifactBuild(FreshmakerBase): return "" % ( self.name, ArtifactType(self.type).name, ArtifactBuildState(self.state).name, self.event.message_id) + + def get_root_dep_of(self): + dep_of = self.dep_of + while dep_of: + dep = dep_of.dep_of + if dep: + dep_of = dep + else: + break + return dep_of diff --git a/tests/test_models.py b/tests/test_models.py index db9b385..c45113b 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -63,3 +63,16 @@ class TestModels(unittest.TestCase): self.assertEqual(e.builds[1].state, 0) self.assertEqual(e.builds[1].build_id, 1235) self.assertEqual(e.builds[1].dep_of.name, "ed") + + def test_get_root_dep_of(self): + event = Event.create(db.session, "test_msg_id", "test", TestingEvent) + build1 = ArtifactBuild.create(db.session, event, "ed", "module", 1234) + build2 = ArtifactBuild.create(db.session, event, "mksh", "module", 1235, build1) + build3 = ArtifactBuild.create(db.session, event, "runtime", "module", 1236, build2) + build4 = ArtifactBuild.create(db.session, event, "perl-runtime", "module", 1237, build3) + db.session.commit() + db.session.expire_all() + self.assertEqual(build1.get_root_dep_of(), None) + self.assertEqual(build2.get_root_dep_of(), build1) + self.assertEqual(build3.get_root_dep_of(), build1) + self.assertEqual(build4.get_root_dep_of(), build1) From aa04fc036da8d8c1daad35570ae7ebde22340427 Mon Sep 17 00:00:00 2001 From: Qixiang Wan Date: Jun 08 2017 06:53:28 +0000 Subject: [PATCH 3/3] Prevent cyclic rebuild of modules by checking db records --- diff --git a/freshmaker/handlers/mbs/module_state_change.py b/freshmaker/handlers/mbs/module_state_change.py index fa907b5..118fcb4 100644 --- a/freshmaker/handlers/mbs/module_state_change.py +++ b/freshmaker/handlers/mbs/module_state_change.py @@ -72,6 +72,15 @@ class MBSModuleStateChangeHandler(BaseHandler): log.info("Triggering rebuild of modules depending on %s:%s " "in MBS", module_name, module_stream) + if module_build: + # we have this build recorded in DB, check to prevent + # cyclic build loop + root_dep = module_build.get_root_dep_of() + if root_dep and root_dep.name == module_name: + log.info("Skipping the rebuild triggered by %s:%s as it will" + "result in cyclic build loop.", module_name, module_stream) + return [] + pdc = PDC(conf) modules = pdc.get_latest_modules(build_dep_name=module_name, build_dep_stream=module_stream, diff --git a/tests/test_mbs_module_state_change_handler.py b/tests/test_mbs_module_state_change_handler.py index 3e56983..1871c17 100644 --- a/tests/test_mbs_module_state_change_handler.py +++ b/tests/test_mbs_module_state_change_handler.py @@ -175,6 +175,93 @@ class MBSModuleStateChangeHandlerTest(helpers.FreshmakerTestCase): handler.build_module.assert_not_called() + @mock.patch('freshmaker.handlers.mbs.module_state_change.PDC') + @mock.patch('freshmaker.handlers.mbs.module_state_change.utils') + @mock.patch('freshmaker.handlers.mbs.module_state_change.log') + def test_handler_not_fall_into_cyclic_rebuild_loop(self, log, utils, PDC): + """ + Tests handler will not fall into cyclic rebuild loop when there is + build dep loop of modules. + """ + # in this case, we have: + # 1. module2 depends on module1 + # 2. module3 depends on module2 + # 3. module1 depends on module3 + # + # when we receives a modult built event of module1, the expect result is: + # 1. module2 get rebuild because module1 is built + # 2. module3 get rebuild because module2 is built + # 3. module1 get rebuild because module3 is built + # 4. stop here + + utils.bump_distgit_repo.return_value = 'abcd' + + mod1_info = helpers.PDCModuleInfo('module1', 'master', '20170412010101') + mod1_info.add_build_dep('module3', 'master') + mod1 = mod1_info.produce() + + mod2_info = helpers.PDCModuleInfo('module2', 'master', '20170412010102') + mod2_info.add_build_dep('module1', 'master') + mod2 = mod2_info.produce() + + mod3_info = helpers.PDCModuleInfo('module3', 'master', '20170412010103') + mod3_info.add_build_dep('module2', 'master') + mod3 = mod3_info.produce() + + pdc = PDC.return_value + handler = MBSModuleStateChangeHandler() + + # Assume we have build of module1 recorded in DB already, it doesn't has + # any dep_of as it was initial triggered by an event which is not + # associated with any build in our DB. + event = models.Event.create(db.session, "initial_msg_id") + models.ArtifactBuild.create(db.session, event, "module1", "module", '123') + db.session.commit() + + # we received module built event of module1 + msg = helpers.ModuleStateChangeMessage('module1', 'master', state='ready', build_id=123).produce() + event = self.get_event_from_msg(msg) + pdc.get_latest_modules.return_value = [mod2] + handler.build_module = mock.Mock() + handler.build_module.return_value = 124 + + # this will trigger module rebuild of module2 + handler.handle(event) + handler.build_module.assert_called_once_with('module2', 'master', 'abcd') + + # we received module built event of module2 + msg = helpers.ModuleStateChangeMessage('module2', 'master', state='ready', build_id=124).produce() + event = self.get_event_from_msg(msg) + pdc.get_latest_modules.return_value = [mod3] + handler.build_module = mock.Mock() + handler.build_module.return_value = 125 + + # this will trigger module rebuild of module3 + handler.handle(event) + handler.build_module.assert_called_once_with('module3', 'master', 'abcd') + + # we received module built event of module3 + msg = helpers.ModuleStateChangeMessage('module3', 'master', state='ready', build_id=125).produce() + event = self.get_event_from_msg(msg) + pdc.get_latest_modules.return_value = [mod1] + handler.build_module = mock.Mock() + handler.build_module.return_value = 126 + + # this will trigger module rebuild of module1 + handler.handle(event) + handler.build_module.assert_called_once_with('module1', 'master', 'abcd') + + # we received module built event of module1 + msg = helpers.ModuleStateChangeMessage('module1', 'master', state='ready', build_id=126).produce() + event = self.get_event_from_msg(msg) + pdc.get_latest_modules.return_value = [mod2] + handler.build_module = mock.Mock() + + # but this time we should not rebuild module2 + handler.handle(event) + handler.build_module.assert_not_called() + log.info.assert_has_calls([mock.call('Skipping the rebuild triggered by %s:%s as it willresult in cyclic build loop.', 'module1', u'master')]) + if __name__ == '__main__': unittest.main()