Do not build module-build-macros when all the components will be reused from previous module build.

module_build_service/models.py

@@ -133,6 +133,26 @@ class ModuleBuild(MBSBase):
                 if component.batch == self.batch
             ]
 
+    def up_to_current_batch(self, state=None):
+        """
+        Returns all components of this module in the current batch and
+        in the previous batches.
+        """
+
+        if not self.batch:
+            raise ValueError("No batch is in progress: %r" % self.batch)
+
+        if state != None:
+            return [
+                component for component in self.component_builds
+                if component.batch <= self.batch and component.state == state
+            ]
+        else:
+            return [
+                component for component in self.component_builds
+                if component.batch <= self.batch
+            ]
+
     def mmd(self):
         mmd = _modulemd.ModuleMetadata()
         try:

module_build_service/scheduler/handlers/modules.py

@@ -28,6 +28,8 @@ import module_build_service.builder
 import module_build_service.pdc
 import module_build_service.utils
 import module_build_service.messaging
+from module_build_service.utils import (
+    start_next_batch_build, attempt_to_reuse_all_components)
 
 from requests.exceptions import ConnectionError
 
@@ -234,32 +236,42 @@ def wait(config, session, msg):
     log.debug("Adding dependencies %s into buildroot for module %s" % (dependencies, module_info))
     builder.buildroot_add_repos(dependencies)
 
-    # inject dist-tag into buildroot
-    srpm = builder.get_disttag_srpm(
-        disttag=".%s" % get_rpm_release_from_mmd(build.mmd()),
-        module_build=build)
+    # If all components in module build will be reused, we don't have to build
+    # module-build-macros, because there won't be any build done.
+    if attempt_to_reuse_all_components(builder, session, build):
+        log.info("All components have ben reused for module %r, "
+                 "skipping build" % build)
+        session.commit()
+        return []
+    else:
+        # Build the module-build-macros
+        # inject dist-tag into buildroot
+        srpm = builder.get_disttag_srpm(
+            disttag=".%s" % get_rpm_release_from_mmd(build.mmd()),
+            module_build=build)
 
-    log.debug("Starting build batch 1")
-    build.batch = 1
+        log.debug("Starting build batch 1")
+        build.batch = 1
+        session.commit()
 
-    artifact_name = "module-build-macros"
-    task_id, state, reason, nvr = builder.build(artifact_name=artifact_name, source=srpm)
+        artifact_name = "module-build-macros"
+        task_id, state, reason, nvr = builder.build(artifact_name=artifact_name, source=srpm)
 
-    component_build = models.ComponentBuild(
-        module_id=build.id,
-        package=artifact_name,
-        format="rpms",
-        scmurl=srpm,
-        task_id=task_id,
-        state=state,
-        state_reason=reason,
-        nvr=nvr,
-        batch=1,
-    )
-    session.add(component_build)
-    build.transition(config, state="build")
-    session.add(build)
-    session.commit()
+        component_build = models.ComponentBuild(
+            module_id=build.id,
+            package=artifact_name,
+            format="rpms",
+            scmurl=srpm,
+            task_id=task_id,
+            state=state,
+            state_reason=reason,
+            nvr=nvr,
+            batch=1,
+        )
+        session.add(component_build)
+        build.transition(config, state="build")
+        session.add(build)
+        session.commit()
 
     # If this build already exists and is done, then fake the repo change event
     # back to the scheduler

module_build_service/scheduler/handlers/tags.py

@@ -72,10 +72,10 @@ def tagged(config, session, msg):
                  "building components in a batch", tag)
         return []
 
-    # Get the list of untagged components in current batch which
+    # Get the list of untagged components in current/previous batches which
     # have been built successfully.
     untagged_components = [
-        c for c in module_build.current_batch()
+        c for c in module_build.up_to_current_batch()
         if not c.tagged and c.state == koji.BUILD_STATES['COMPLETE']
     ]
 

module_build_service/utils.py

@@ -152,12 +152,10 @@ def continue_batch_build(config, module, session, builder, components=None):
     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)
+        # instead of rebuilding it
+        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
@@ -166,37 +164,7 @@ def continue_batch_build(config, module, session, builder, components=None):
             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
-                )
-            )
+            further_work += reuse_component(c, previous_component_build)
             continue
 
         # We set state to BUILDING here, because we are going to build the
@@ -806,6 +774,96 @@ def module_build_state_from_msg(msg):
         % (state, type(state), list(models.BUILD_STATES.values())))
     return state
 
+def reuse_component(component, previous_component_build,
+                    change_state_now=False):
+    """
+    Reuses component build `previous_component_build` instead of building
+    component `component`
+
+    Returns the list of BaseMessage instances to be handled later by the
+    scheduler.
+    """
+
+    import koji
+
+    log.info(
+        'Reusing component "{0}" from a previous module '
+        'build with the nvr "{1}"'.format(
+            component.package, previous_component_build.nvr))
+    component.reused_component_id = previous_component_build.id
+    component.task_id = previous_component_build.task_id
+    if change_state_now:
+        component.state = previous_component_build.state
+    else:
+        # 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.
+        component.state = koji.BUILD_STATES['BUILDING']
+    component.state_reason = \
+        'Reused component from previous module build'
+    component.nvr = previous_component_build.nvr
+    nvr_dict = kobo.rpmlib.parse_nvr(component.nvr)
+    # Add this message to further_work so that the reused
+    # component will be tagged properly
+    return [
+        module_build_service.messaging.KojiBuildChange(
+            msg_id='reuse_component: fake msg',
+            build_id=None,
+            task_id=component.task_id,
+            build_new_state=previous_component_build.state,
+            build_name=component.package,
+            build_version=nvr_dict['version'],
+            build_release=nvr_dict['release'],
+            module_build_id=component.module_id,
+            state_reason=component.state_reason
+        )
+    ]
+
+def attempt_to_reuse_all_components(builder, session, module):
+    """
+    Tries to reuse all the components in a build. The components are also
+    tagged to the tags using the `builder`.
+
+    Returns True if all components could be reused, otherwise False. When
+    False is returned, no component has been reused.
+    """
+
+    # [(component, component_to_reuse), ...]
+    component_pairs = []
+
+    # Find out if we can reuse all components and cache component and
+    # component to reuse pairs.
+    for c in module.component_builds:
+        if c.package == "module-build-macros":
+            continue
+        component_to_reuse = get_reusable_component(
+            session, module, c.package)
+        if not component_to_reuse:
+            return False
+
+        component_pairs.append((c, component_to_reuse))
+
+    # Stores components we will tag to buildroot and final tag.
+    components_to_tag = []
+
+    # Reuse all components.
+    for c, component_to_reuse in component_pairs:
+        # Set the module.batch to the last batch we have.
+        if c.batch > module.batch:
+            module.batch = c.batch
+
+        # Reuse the component
+        reuse_component(c, component_to_reuse, True)
+        components_to_tag.append(c.nvr)
+
+    # Tag them
+    builder.buildroot_add_artifacts(components_to_tag, install=False)
+    builder.tag_artifacts(components_to_tag)
+
+    return True
+
 def get_reusable_component(session, module, component_name):
     """
     Returns the component (RPM) build of a module that can be reused
@@ -818,6 +876,11 @@ def get_reusable_component(session, module, component_name):
     :return: the component (RPM) build SQLAlchemy object, if one is not found,
     None is returned
     """
+
+    # We support components reusing only for koji and test backend.
+    if conf.system not in ['koji', 'test']:
+        return None
+
     mmd = module.mmd()
     # Find the latest module that is in the done or ready state
     previous_module_build = session.query(models.ModuleBuild)\

tests/test_build/test_build.py

@@ -36,13 +36,14 @@ from module_build_service import db, models, conf
 
 from mock import patch, PropertyMock
 
-from tests import app, init_data
+from tests import app, init_data, test_reuse_component_init_data
 import os
 import json
 import itertools
 
 from module_build_service.builder import KojiModuleBuilder, GenericBuilder
 import module_build_service.scheduler.consumer
+from module_build_service.messaging import MBSModule
 
 base_dir = dirname(dirname(__file__))
 cassette_dir = base_dir + '/vcr-request-data/'
@@ -628,3 +629,107 @@ class TestBuild(unittest.TestCase):
             # We should end up with batch 2 and never start batch 3, because
             # there were failed components in batch 2.
             self.assertEqual(c.module_build.batch, 2)
+
+    @timed(30)
+    @patch('module_build_service.auth.get_user', return_value=user)
+    @patch('module_build_service.scm.SCM')
+    def test_submit_build_reuse_all(self, mocked_scm, mocked_get_user,
+                                    conf_system, dbg):
+        """
+        Tests that we do not try building module-build-macros when reusing all
+        components in a module build.
+        """
+        test_reuse_component_init_data()
+
+        def on_build_cb(cls, artifact_name, source):
+            raise ValueError("All components should be reused, not build.")
+        TestModuleBuilder.on_build_cb = on_build_cb
+
+        # Check that components are tagged after the batch is built.
+        tag_groups = []
+        tag_groups.append(set(
+            ['perl-Tangerine-0.23-1.module_testmodule_master_20170109091357',
+            'perl-List-Compare-0.53-5.module_testmodule_master_20170109091357',
+            'tangerine-0.22-3.module_testmodule_master_20170109091357']))
+
+        def on_tag_artifacts_cb(cls, artifacts):
+            self.assertEqual(tag_groups.pop(0), set(artifacts))
+        TestModuleBuilder.on_tag_artifacts_cb = on_tag_artifacts_cb
+
+        buildtag_groups = []
+        buildtag_groups.append(set(
+            ['perl-Tangerine-0.23-1.module_testmodule_master_20170109091357',
+            'perl-List-Compare-0.53-5.module_testmodule_master_20170109091357',
+            'tangerine-0.22-3.module_testmodule_master_20170109091357']))
+        def on_buildroot_add_artifacts_cb(cls, artifacts, install):
+            self.assertEqual(buildtag_groups.pop(0), set(artifacts))
+        TestModuleBuilder.on_buildroot_add_artifacts_cb = on_buildroot_add_artifacts_cb
+
+        msgs = [MBSModule("local module build", 2, 1)]
+        stop = module_build_service.scheduler.make_simple_stop_condition(db.session)
+        module_build_service.scheduler.main(msgs, stop)
+
+        reused_component_ids = {"module-build-macros": None, "tangerine": 3,
+                                "perl-Tangerine": 1, "perl-List-Compare": 2}
+
+        # 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=2).all():
+            self.assertEqual(build.state, koji.BUILD_STATES['COMPLETE'])
+            self.assertTrue(build.module_build.state in [models.BUILD_STATES["done"], models.BUILD_STATES["ready"]])
+
+            self.assertEqual(build.reused_component_id,
+                             reused_component_ids[build.package])
+
+    @timed(30)
+    @patch('module_build_service.auth.get_user', return_value=user)
+    @patch('module_build_service.scm.SCM')
+    def test_submit_build_reuse_all_without_build_macros(self, mocked_scm, mocked_get_user,
+                                    conf_system, dbg):
+        """
+        Tests that we can reuse components even when the reused module does
+        not have module-build-macros component.
+        """
+        test_reuse_component_init_data()
+
+        models.ComponentBuild.query.filter_by(package="module-build-macros").delete()
+        self.assertEqual(len(models.ComponentBuild.query.filter_by(
+            package="module-build-macros").all()), 0)
+
+        db.session.commit()
+
+        def on_build_cb(cls, artifact_name, source):
+            raise ValueError("All components should be reused, not build.")
+        TestModuleBuilder.on_build_cb = on_build_cb
+
+        # Check that components are tagged after the batch is built.
+        tag_groups = []
+        tag_groups.append(set(
+            ['perl-Tangerine-0.23-1.module_testmodule_master_20170109091357',
+            'perl-List-Compare-0.53-5.module_testmodule_master_20170109091357',
+            'tangerine-0.22-3.module_testmodule_master_20170109091357']))
+
+        def on_tag_artifacts_cb(cls, artifacts):
+            self.assertEqual(tag_groups.pop(0), set(artifacts))
+        TestModuleBuilder.on_tag_artifacts_cb = on_tag_artifacts_cb
+
+        buildtag_groups = []
+        buildtag_groups.append(set(
+            ['perl-Tangerine-0.23-1.module_testmodule_master_20170109091357',
+            'perl-List-Compare-0.53-5.module_testmodule_master_20170109091357',
+            'tangerine-0.22-3.module_testmodule_master_20170109091357']))
+        def on_buildroot_add_artifacts_cb(cls, artifacts, install):
+            self.assertEqual(buildtag_groups.pop(0), set(artifacts))
+        TestModuleBuilder.on_buildroot_add_artifacts_cb = on_buildroot_add_artifacts_cb
+
+        msgs = [MBSModule("local module build", 2, 1)]
+        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=2).all():
+            self.assertEqual(build.state, koji.BUILD_STATES['COMPLETE'])
+            self.assertTrue(build.module_build.state in [models.BUILD_STATES["done"], models.BUILD_STATES["ready"]] )
+            self.assertNotEqual(build.package, "module-build-macros")
+

tests/test_scheduler/test_tag_tagged.py

@@ -84,6 +84,13 @@ class TestTagTagged(unittest.TestCase):
         create_builder.return_value = builder
 
         module_build = module_build_service.models.ModuleBuild.query.filter_by(id=2).one()
+
+        # Set previous components as COMPLETE and tagged.
+        module_build.batch = 1
+        for c in module_build.up_to_current_batch():
+            c.state = koji.BUILD_STATES["COMPLETE"]
+            c.tagged = True
+
         module_build.batch = 2
         for c in module_build.current_batch():
             c.state = koji.BUILD_STATES["COMPLETE"]
@@ -173,6 +180,13 @@ class TestTagTagged(unittest.TestCase):
         create_builder.return_value = builder
 
         module_build = module_build_service.models.ModuleBuild.query.filter_by(id=2).one()
+
+        # Set previous components as COMPLETE and tagged.
+        module_build.batch = 1
+        for c in module_build.up_to_current_batch():
+            c.state = koji.BUILD_STATES["COMPLETE"]
+            c.tagged = True
+
         module_build.batch = 2
         component = module_build_service.models.ComponentBuild.query\
             .filter_by(package='perl-Tangerine', module_id=module_build.id).one()
@@ -199,3 +213,68 @@ class TestTagTagged(unittest.TestCase):
         # 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)
+
+    @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_multiple_batches_tagged(
+            self, create_builder, koji_get_session, dbg):
+        """
+        Test that newRepo is called just once and only when all components
+        are tagged even if we tag components from the multiple batches in the
+        same time.
+        """
+        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
+        for c in module_build.current_batch():
+            c.state = koji.BUILD_STATES["COMPLETE"]
+        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 not be called, because there are still components
+        # to tag.
+        self.assertTrue(not koji_session.newRepo.called)
+
+        # Tag the component from first batch to the buildroot.
+        msg = module_build_service.messaging.KojiTagChange(
+            'id', 'module-testmodule-build', "module-build-macros")
+        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)