From b569fa6c21b45cc70b1d1d82bcddf28d5eb76a0b Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Thu, 9 Mar 2017 08:59:09 +0100 Subject: [PATCH] Fix #405 - Fix num_consecutive_builds. --- module_build_service/manage.py | 2 +- module_build_service/utils.py | 135 ++++++++++++++++++--------------- tests/test_build/test_build.py | 19 ++++- 3 files changed, 89 insertions(+), 67 deletions(-) diff --git a/module_build_service/manage.py b/module_build_service/manage.py index 85ffff47..ae226d42 100644 --- a/module_build_service/manage.py +++ b/module_build_service/manage.py @@ -172,7 +172,7 @@ def build_module_locally(url): db.create_all() username = getpass.getuser() - submit_module_build_from_scm(username, url, allow_local_url=True) + submit_module_build_from_scm(username, url, "master", allow_local_url=True) stop = module_build_service.scheduler.make_simple_stop_condition(db.session) initial_messages = [MBSModule("local module build", 1, 1)] diff --git a/module_build_service/utils.py b/module_build_service/utils.py index d5072c09..37232309 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -91,6 +91,25 @@ def at_concurrent_component_threshold(config, session): return False +def start_build_component(builder, c): + """ + Submits single component build to builder. Called in thread + by QueueBasedThreadPool in start_build_batch. + """ + try: + c.task_id, c.state, c.state_reason, c.nvr = builder.build( + artifact_name=c.package, source=c.scmurl) + except Exception as e: + c.state = koji.BUILD_STATES['FAILED'] + c.state_reason = "Failed to build artifact %s: %s" % (c.package, str(e)) + return + + if not c.task_id and c.state == koji.BUILD_STATES['BUILDING']: + c.state = koji.BUILD_STATES['FAILED'] + c.state_reason = ("Failed to build artifact %s: " + "Builder did not return task ID" % (c.package)) + return + def start_build_batch(config, module, session, builder, components=None): """ Starts a round of the build cycle for a module. @@ -142,76 +161,66 @@ def start_build_batch(config, module, session, builder, components=None): log.info("Starting build of next batch %d, %s" % (module.batch, unbuilt_components)) + # Get the list of components to be build in this batch. We are not + # building all `unbuilt_components`, because we can a) meet + # the num_consecutive_builds threshold or b) reuse previous build. further_work = [] + components_to_build = [] + for c in unbuilt_components: + previous_component_build = None + # Check to see if we can reuse a previous component build + # instead of rebuilding it if the builder is Koji + if conf.system == 'koji': + previous_component_build = get_reusable_component( + session, module, c.package) + # If a component build can't be reused, we need to check + # the concurrent threshold + if not previous_component_build and \ + at_concurrent_component_threshold(config, session): + log.info('Concurrent build threshold met') + break - def start_build_component(c): - """ - Submits single component build to builder. Called in thread - by QueueBasedThreadPool later. - """ - try: - with models.make_session(conf) as s: - previous_component_build = None - # Check to see if we can reuse a previous component build - # instead of rebuilding it if the builder is Koji - if conf.system == 'koji': - previous_component_build = get_reusable_component( - s, module, c.package) - # If a component build can't be reused, we need to check - # the concurrent threshold - if not previous_component_build and \ - at_concurrent_component_threshold(config, s): - log.info('Concurrent build threshold met') - return - - if previous_component_build: - log.info( - 'Reusing component "{0}" from a previous module ' - 'build with the nvr "{1}"'.format( - c.package, previous_component_build.nvr)) - c.reused_component_id = previous_component_build.id - c.task_id = previous_component_build.task_id - c.state = previous_component_build.state - c.reused_component_id = previous_component_build.id - c.task_id = previous_component_build.task_id - c.state = previous_component_build.state - c.state_reason = \ - 'Reused component from previous module build' - c.nvr = previous_component_build.nvr - nvr_dict = kobo.rpmlib.parse_nvr(c.nvr) - # Add this message to further_work so that the reused - # component will be tagged properly - further_work.append( - module_build_service.messaging.KojiBuildChange( - msg_id='start_build_batch: fake msg', - build_id=None, - task_id=c.task_id, - build_new_state=c.state, - build_name=c.package, - build_version=nvr_dict['version'], - build_release=nvr_dict['release'], - module_build_id=c.module_id, - state_reason=c.state_reason - ) + if previous_component_build: + log.info( + 'Reusing component "{0}" from a previous module ' + 'build with the nvr "{1}"'.format( + c.package, previous_component_build.nvr)) + c.reused_component_id = previous_component_build.id + c.task_id = previous_component_build.task_id + c.state = previous_component_build.state + c.reused_component_id = previous_component_build.id + c.task_id = previous_component_build.task_id + c.state = previous_component_build.state + c.state_reason = \ + 'Reused component from previous module build' + c.nvr = previous_component_build.nvr + nvr_dict = kobo.rpmlib.parse_nvr(c.nvr) + # Add this message to further_work so that the reused + # component will be tagged properly + further_work.append( + module_build_service.messaging.KojiBuildChange( + msg_id='start_build_batch: fake msg', + build_id=None, + task_id=c.task_id, + build_new_state=c.state, + build_name=c.package, + build_version=nvr_dict['version'], + build_release=nvr_dict['release'], + module_build_id=c.module_id, + state_reason=c.state_reason ) - return + ) + continue - c.task_id, c.state, c.state_reason, c.nvr = builder.build( - artifact_name=c.package, source=c.scmurl) - except Exception as e: - c.state = koji.BUILD_STATES['FAILED'] - c.state_reason = "Failed to build artifact %s: %s" % (c.package, str(e)) - return - - if not c.task_id and c.state == koji.BUILD_STATES['BUILDING']: - c.state = koji.BUILD_STATES['FAILED'] - c.state_reason = ("Failed to build artifact %s: " - "Builder did not return task ID" % (c.package)) - return + # We set state to BUILDING here, because we are going to build the + # component anyway and at_concurrent_component_threshold() works + # by counting components in BUILDING state. + c.state = koji.BUILD_STATES['BUILDING'] + components_to_build.append(c) # Start build of components in this batch. with concurrent.futures.ThreadPoolExecutor(max_workers=config.num_consecutive_builds) as executor: - futures = {executor.submit(start_build_component, c): c for c in unbuilt_components} + futures = {executor.submit(start_build_component, builder, c): c for c in components_to_build} concurrent.futures.wait(futures) # If all components in this batch are already done, it can mean that they diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py index 8939e790..b8d0b841 100644 --- a/tests/test_build/test_build.py +++ b/tests/test_build/test_build.py @@ -31,6 +31,7 @@ from nose.tools import timed import module_build_service.messaging import module_build_service.scheduler.handlers.repos +import module_build_service.utils from module_build_service import db, models, conf from mock import patch @@ -208,7 +209,7 @@ class TestBuild(unittest.TestCase): GenericBuilder.register_backend_class(TestModuleBuilder) self.client = app.test_client() self._prev_system = conf.system - self._prev_num_consencutive_builds = conf.num_consecutive_builds + self._prev_num_consecutive_builds = conf.num_consecutive_builds conf.set_item("system", "mock") @@ -222,7 +223,7 @@ class TestBuild(unittest.TestCase): def tearDown(self): conf.set_item("system", self._prev_system) - conf.set_item("num_consecutive_builds", self._prev_num_consencutive_builds) + conf.set_item("num_consecutive_builds", self._prev_num_consecutive_builds) TestModuleBuilder.reset() # Necessary to restart the twisted reactor for the next test. @@ -434,12 +435,24 @@ class TestBuild(unittest.TestCase): data = json.loads(rv.data) module_build_id = data['id'] + def stop(message): + """ + Stop the scheduler when the module is built or when we try to build + more components than the num_consecutive_builds. + """ + main_stop = module_build_service.scheduler.make_simple_stop_condition(db.session) + over_threshold = conf.num_consecutive_builds < \ + db.session.query(models.ComponentBuild).filter_by( + state=koji.BUILD_STATES['BUILDING']).count() + return main_stop(message) or over_threshold + msgs = [] - stop = module_build_service.scheduler.make_simple_stop_condition(db.session) module_build_service.scheduler.main(msgs, stop) # All components should be built and module itself should be in "done" # or "ready" state. for build in models.ComponentBuild.query.filter_by(module_id=module_build_id).all(): self.assertEqual(build.state, koji.BUILD_STATES['COMPLETE']) + # When this fails, it can mean that num_consecutive_builds + # threshold has been met. self.assertTrue(build.module_build.state in [models.BUILD_STATES["done"], models.BUILD_STATES["ready"]] )