diff --git a/module_build_service/scheduler/handlers/components.py b/module_build_service/scheduler/handlers/components.py index 093ddee4..c0a1026f 100644 --- a/module_build_service/scheduler/handlers/components.py +++ b/module_build_service/scheduler/handlers/components.py @@ -111,11 +111,11 @@ def _finalize(config, session, msg, state): for c in unbuilt_components_in_batch])): # We are not in the middle of the batch building and # we have some unbuilt components in this batch. We might hit the - # concurrent builds threshold in previous call of start_build_batch + # concurrent builds threshold in previous call of continue_batch_build # done in repos.py:done(...), but because we have just finished one - # build, try to call start_build_batch again so in case we hit the + # build, try to call continue_batch_build again so in case we hit the # threshold previously, we will submit another build from this batch. - further_work = module_build_service.utils.start_build_batch( + further_work = module_build_service.utils.continue_batch_build( config, parent, session, builder) return further_work diff --git a/module_build_service/scheduler/handlers/repos.py b/module_build_service/scheduler/handlers/repos.py index 9bdfb5ab..33db66cc 100644 --- a/module_build_service/scheduler/handlers/repos.py +++ b/module_build_service/scheduler/handlers/repos.py @@ -28,6 +28,7 @@ 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) @@ -111,16 +112,9 @@ def done(config, session, msg): further_work = [] if unbuilt_components: - # Increment the build batch when no components are being built and all - # have at least attempted a build (even failures) in the current batch - unbuilt_components_in_batch = [ - c for c in module_build.current_batch() - if c.state == koji.BUILD_STATES['BUILDING'] or not c.state - ] - if not unbuilt_components_in_batch: - module_build.batch += 1 - - further_work += module_build_service.utils.start_build_batch( + # Try to start next batch build, because there are still unbuilt + # components in a module. + further_work += start_next_batch_build( config, module_build, session, builder) # We don't have copr implementation finished yet, Let's fake the repo change event, diff --git a/module_build_service/utils.py b/module_build_service/utils.py index 4b8da1d7..3c75cc02 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -94,7 +94,7 @@ def at_concurrent_component_threshold(config, session): def start_build_component(builder, c): """ Submits single component build to builder. Called in thread - by QueueBasedThreadPool in start_build_batch. + by QueueBasedThreadPool in continue_batch_build. """ try: c.task_id, c.state, c.state_reason, c.nvr = builder.build( @@ -110,15 +110,141 @@ def start_build_component(builder, c): "Builder did not return task ID" % (c.package)) return -def start_build_batch(config, module, session, builder, components=None): +def continue_batch_build(config, module, session, builder, components=None): """ - Starts a round of the build cycle for a module. + Continues building current batch. Submits next components in the batch + until it hits concurrent builds limit. Returns list of BaseMessage instances which should be scheduled by the scheduler. """ import koji # Placed here to avoid py2/py3 conflicts... + # The user can either pass in a list of components to 'seed' the batch, or + # if none are provided then we just select everything that hasn't + # successfully built yet or isn't currently being built. + unbuilt_components = components or [ + c for c in module.component_builds + if (c.state != koji.BUILD_STATES['COMPLETE'] + and c.state != koji.BUILD_STATES['BUILDING'] + and c.state != koji.BUILD_STATES['FAILED'] + and c.batch == module.batch) + ] + + # 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 + + 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 + # Use BUILDING state here, because we want the state to change to + # COMPLETE by the fake KojiBuildChange message we are generating + # few lines below. If we would set it to the right state right + # here, we would miss the code path handling the KojiBuildChange + # which works only when switching from BUILDING to COMPLETE. + c.state = koji.BUILD_STATES['BUILDING'] + 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=previous_component_build.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 + ) + ) + continue + + # 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, 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 + # have been built in the past and have been skipped in this module build. + # We therefore have to generate fake KojiRepoChange message, because the + # repo has been also done in the past and build system will not send us + # any message now. + if (all(c.state in [koji.BUILD_STATES['COMPLETE'], koji.BUILD_STATES['FAILED']] + or c.reused_component_id + for c in unbuilt_components) and builder.module_build_tag): + further_work += [module_build_service.messaging.KojiRepoChange( + 'start_build_batch: fake msg', builder.module_build_tag['name'])] + + session.commit() + return further_work + +def start_next_batch_build(config, module, session, builder, components=None): + """ + Tries to start the build of next batch. In case there are still unbuilt + components in a batch, tries to submit more components until it hits + concurrent builds limit. Otherwise Increments module.batch and submits component + builds from the next batch. + + :return: a list of BaseMessage instances to be handled by the MBSConsumer. + """ + + import koji # Placed here to avoid py2/py3 conflicts... + + unbuilt_components_in_batch = [ + c for c in module.current_batch() + if c.state == koji.BUILD_STATES['BUILDING'] or not c.state + ] + if unbuilt_components_in_batch: + return continue_batch_build( + config, module, session, builder, components) + + module.batch += 1 + + # The user can either pass in a list of components to 'seed' the batch, or + # if none are provided then we just select everything that hasn't + # successfully built yet or isn't currently being built. + unbuilt_components = components or [ + c for c in module.component_builds + if (c.state != koji.BUILD_STATES['COMPLETE'] + and c.state != koji.BUILD_STATES['BUILDING'] + and c.state != koji.BUILD_STATES['FAILED'] + and c.batch == module.batch) + ] + + log.info("Starting build of next batch %d, %s" % (module.batch, + unbuilt_components)) + # Local check for component relicts if any([c.state == koji.BUILD_STATES['BUILDING'] and c.batch != module.batch @@ -148,94 +274,8 @@ def start_build_batch(config, module, session, builder, components=None): log.debug("Builder {} doesn't provide information about active tasks." .format(builder)) - # The user can either pass in a list of components to 'seed' the batch, or - # if none are provided then we just select everything that hasn't - # successfully built yet or isn't currently being built. - unbuilt_components = components or [ - c for c in module.component_builds - if (c.state != koji.BUILD_STATES['COMPLETE'] - and c.state != koji.BUILD_STATES['BUILDING'] - and c.state != koji.BUILD_STATES['FAILED'] - and c.batch == module.batch) - ] - - 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 - - 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 - ) - ) - continue - - # 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, 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 - # have been built in the past and have been skipped in this module build. - # We therefore have to generate fake KojiRepoChange message, because the - # repo has been also done in the past and build system will not send us - # any message now. - if (all(c.state == koji.BUILD_STATES['COMPLETE'] for c in unbuilt_components) - and builder.module_build_tag): - further_work += [module_build_service.messaging.KojiRepoChange( - 'start_build_batch: fake msg', builder.module_build_tag['name'])] - - session.commit() - return further_work + return continue_batch_build( + config, module, session, builder, unbuilt_components) def pagination_metadata(p_query): """ diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index f122176c..e9d8fae1 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -25,9 +25,14 @@ import modulemd from mock import patch import module_build_service.utils import module_build_service.scm -from module_build_service import models +from module_build_service import models, conf from module_build_service.errors import ProgrammingError, ValidationError -from tests import test_resuse_component_init_data, db +from tests import test_resuse_component_init_data, init_data, db +import mock +from mock import PropertyMock +import koji +import module_build_service.scheduler.handlers.components +from module_build_service.builder import GenericBuilder, KojiModuleBuilder BASE_DIR = path.abspath(path.dirname(__file__)) CASSETTES_DIR = path.join( @@ -278,3 +283,113 @@ class TestUtils(unittest.TestCase): validate_koji_tag_is_None(None) self.assertTrue(str(cm.exception).endswith(' No value provided.')) + + +class DummyModuleBuilder(GenericBuilder): + """ + Dummy module builder + """ + + backend = "koji" + _build_id = 0 + + TAGGED_COMPONENTS = [] + + @module_build_service.utils.validate_koji_tag('tag_name') + def __init__(self, owner, module, config, tag_name, components): + self.module_str = module + self.tag_name = tag_name + self.config = config + + + def buildroot_connect(self, groups): + pass + + def buildroot_prep(self): + pass + + def buildroot_resume(self): + pass + + def buildroot_ready(self, artifacts=None): + return True + + def buildroot_add_dependency(self, dependencies): + pass + + def buildroot_add_artifacts(self, artifacts, install=False): + DummyModuleBuilder.TAGGED_COMPONENTS += artifacts + + def buildroot_add_repos(self, dependencies): + pass + + def tag_artifacts(self, artifacts): + pass + + @property + def module_build_tag(self): + return {"name": self.tag_name + "-build"} + + def build(self, artifact_name, source): + DummyModuleBuilder._build_id += 1 + state = koji.BUILD_STATES['COMPLETE'] + reason = "Submitted %s to Koji" % (artifact_name) + return DummyModuleBuilder._build_id, state, reason, None + + @staticmethod + def get_disttag_srpm(disttag): + # @FIXME + return KojiModuleBuilder.get_disttag_srpm(disttag) + + def cancel_build(self, task_id): + pass + + def list_tasks_for_components(self, component_builds=None, state='active'): + pass + +@patch("module_build_service.builder.GenericBuilder.default_buildroot_groups", return_value = {'build': [], 'srpm-build': []}) +class TestBatches(unittest.TestCase): + + def setUp(self): + test_resuse_component_init_data() + GenericBuilder.register_backend_class(DummyModuleBuilder) + + def tearDown(self): + init_data() + DummyModuleBuilder.TAGGED_COMPONENTS = [] + GenericBuilder.register_backend_class(KojiModuleBuilder) + + def test_start_next_batch_build_reuse(self, default_buildroot_groups): + module_build = models.ModuleBuild.query.filter_by(id=2).one() + module_build.batch = 1 + + builder = mock.MagicMock() + further_work = module_build_service.utils.start_next_batch_build( + conf, module_build, db.session, builder) + + # Batch number should increase. + self.assertEqual(module_build.batch, 2) + + # KojiBuildChange messages in further_work should have build_new_state + # set to COMPLETE, but the current component build state should be set + # to BUILDING, so KojiBuildChange message handler handles the change + # properly. + for msg in further_work: + if type(msg) == module_build_service.messaging.KojiBuildChange: + self.assertEqual(msg.build_new_state, koji.BUILD_STATES['COMPLETE']) + component_build = models.ComponentBuild.from_component_event(db.session, msg) + self.assertEqual(component_build.state, koji.BUILD_STATES['BUILDING']) + + # When we handle these KojiBuildChange messages, MBS should tag all + # the components just once. + for msg in further_work: + if type(msg) == module_build_service.messaging.KojiBuildChange: + module_build_service.scheduler.handlers.components.complete( + conf, db.session, msg) + + # Since we have reused all the components in the batch, there should + # be fake KojiRepoChange message. + self.assertEqual(type(further_work[-1]), module_build_service.messaging.KojiRepoChange) + + # Check that packages have been tagged just once. + self.assertEqual(len(DummyModuleBuilder.TAGGED_COMPONENTS), 2)