Do not allow starting next batch if the Koji is still building new repo.

module_build_service/builder.py

@@ -59,6 +59,8 @@ import module_build_service.utils
 import module_build_service.scheduler
 import module_build_service.scheduler.consumer
 
+from requests.exceptions import ConnectionError
+
 logging.basicConfig(level=logging.DEBUG)
 
 """
@@ -300,6 +302,7 @@ class GenericBuilder(six.with_metaclass(ABCMeta)):
         raise NotImplementedError()
 
     @classmethod
+    @module_build_service.utils.retry(wait_on=(ConnectionError))
     def default_buildroot_groups(cls, session, module):
         try:
             pdc_session = pdc.get_pdc_client_session(conf)
@@ -329,6 +332,12 @@ class GenericBuilder(six.with_metaclass(ABCMeta)):
         """
         raise NotImplementedError()
 
+    @abstractmethod
+    def is_waiting_for_repo_regen(self):
+        """
+        :return: True if there is repo regeneration pending for a module.
+        """
+        raise NotImplementedError()
 
 class KojiModuleBuilder(GenericBuilder):
     """ Koji specific builder class """
@@ -369,6 +378,20 @@ class KojiModuleBuilder(GenericBuilder):
         return "<KojiModuleBuilder module: %s, tag: %s>" % (
             self.module_str, self.tag_name)
 
+    def is_waiting_for_repo_regen(self):
+        """
+        Returns true when there is 'newRepo' task for our tag.
+        """
+        tasks = []
+        state = [koji.TASK_STATES[s] for s in ('FREE', 'OPEN', 'ASSIGNED')]
+        for task in self.koji_session.listTasks(opts={'state': state,
+                                                      'decode': True,
+                                                      'method': 'newRepo'}):
+            tag = task['request'][0]
+            if tag == self.tag_name:
+                return True
+        return False
+
     @module_build_service.utils.retry(wait_on=(IOError, koji.GenericError))
     def buildroot_ready(self, artifacts=None):
         """
@@ -1056,6 +1079,9 @@ class CoprModuleBuilder(GenericBuilder):
     def list_tasks_for_components(self, component_builds=None, state='active'):
         pass
 
+    def is_waiting_for_repo_regen(self):
+        return False
+
     def build(self, artifact_name, source):
         """
         :param artifact_name : A package name. We can't guess it since macros
@@ -1553,6 +1579,9 @@ mdpolicy=group:primary
     def list_tasks_for_components(self, component_builds=None, state='active'):
         pass
 
+    def is_waiting_for_repo_regen(self):
+        return True
+
 
 def build_from_scm(artifact_name, source, config, build_srpm,
                         data = None, stdout=None, stderr=None):

module_build_service/scheduler/handlers/repos.py

@@ -104,15 +104,17 @@ def done(config, session, msg):
     # So now we can either start a new batch if there are still some to build
     # or, if everything is built successfully, then we can bless the module as
     # complete.
-    unbuilt_components = [
-        c for c in module_build.component_builds
-        if (c.state != koji.BUILD_STATES['COMPLETE']
-            and c.state != koji.BUILD_STATES["FAILED"]
-            and c.state != koji.BUILD_STATES["CANCELED"])
-    ]
+    has_unbuilt_components = False
+    has_failed_components = False
+    for c in module_build.component_builds:
+        if c.state in [None, koji.BUILD_STATES['BUILDING']]:
+            has_unbuilt_components = True
+        elif (c.state in [koji.BUILD_STATES['FAILED'],
+                          koji.BUILD_STATES['CANCELED']]):
+            has_failed_components = True
 
     further_work = []
-    if unbuilt_components:
+    if has_unbuilt_components and not has_failed_components:
         # Try to start next batch build, because there are still unbuilt
         # components in a module.
         further_work += start_next_batch_build(
@@ -125,16 +127,7 @@ def done(config, session, msg):
             return further_work
 
     else:
-        # We know that there are no unbuilt components in this module build,
-        # so we can mark the state as "done" or "failed (in case there are
-        # some failed or canceled components in a build).
-        failed_component_present = False
-        for c in module_build.component_builds:
-            if c.state in (koji.BUILD_STATES['FAILED'],
-                           koji.BUILD_STATES["CANCELED"]):
-                failed_component_present = True
-                break
-        if failed_component_present:
+        if has_failed_components:
             module_build.transition(config, state=models.BUILD_STATES['failed'],
                                     state_reason="Some components failed to build.")
         else:

module_build_service/scheduler/producer.py

@@ -179,15 +179,7 @@ class MBSProducer(PollingProducer):
                     state=models.BUILD_STATES['build']).all():
             # If there are no components in the build state on the module build,
             # then no possible event will start off new component builds
-            unbuilt_components = [
-                c for c in module_build.component_builds
-                if (c.state != koji.BUILD_STATES['COMPLETE']
-                    and c.state != koji.BUILD_STATES["FAILED"]
-                    and c.state != koji.BUILD_STATES["CANCELED"])
-            ]
-
-            if (not module_build.current_batch(koji.BUILD_STATES['BUILDING'])
-                and unbuilt_components):
+            if not module_build.current_batch(koji.BUILD_STATES['BUILDING']):
                 # Initialize the builder...
                 builder = GenericBuilder.create_from_module(
                     session, module_build, config)

module_build_service/utils.py

@@ -131,6 +131,10 @@ def continue_batch_build(config, module, session, builder, components=None):
             and c.batch == module.batch)
     ]
 
+    if not unbuilt_components:
+        log.debug("Cannot continue building module %s. No component to build." % module)
+        return []
+
     # 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.
@@ -221,14 +225,52 @@ def start_next_batch_build(config, module, session, builder, components=None):
 
     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:
+    # Check the status of the module build and current batch so we can
+    # later decide if we can start new batch or not.
+    has_unbuilt_components = False
+    has_unbuilt_components_in_batch = False
+    has_building_components_in_batch = False
+    has_failed_components = False
+    for c in module.component_builds:
+        if c.state in [None, koji.BUILD_STATES['BUILDING']]:
+            has_unbuilt_components = True
+
+            if c.batch == module.batch:
+                if not c.state:
+                    has_unbuilt_components_in_batch = True
+                elif c.state == koji.BUILD_STATES['BUILDING']:
+                    has_building_components_in_batch = True
+        elif (c.state in [koji.BUILD_STATES['FAILED'],
+                          koji.BUILD_STATES['CANCELED']]):
+            has_failed_components = True
+
+    # Do not start new batch if there are no components to build.
+    if not has_unbuilt_components:
+        log.debug("Not starting new batch, there is no component to build "
+                  "for module %s" % module)
+        return []
+
+    # Check that there is something to build in current batch before starting
+    # the new one. If there is, continue building current batch.
+    if has_unbuilt_components_in_batch:
+        log.info("Continuing building batch %d", module.batch)
         return continue_batch_build(
             config, module, session, builder, components)
 
+    # Check that there are no components in BUILDING state in current batch.
+    # If there are, wait until they are built.
+    if has_building_components_in_batch:
+        log.debug("Not starting new batch, there are still components in "
+                  "BUILDING state in current batch for module %s", module)
+        return []
+
+    # Check that there are no failed components in this batch. If there are,
+    # do not start the new batch.
+    if has_failed_components:
+        log.info("Not starting new batch, there are failed components for "
+                 "module %s", module)
+        return []
+
     # Identify active tasks which might contain relicts of previous builds
     # and fail the module build if this^ happens.
     active_tasks = builder.list_tasks_for_components(module.component_builds,
@@ -239,12 +281,20 @@ def start_next_batch_build(config, module, session, builder, components=None):
         module.transition(config, state=models.BUILD_STATES['failed'],
                           state_reason=state_reason)
         session.commit()
-        return
+        return []
 
     else:
         log.debug("Builder {} doesn't provide information about active tasks."
                   .format(builder))
 
+    # Find out if there is repo regeneration in progress for this module.
+    # If there is, wait until the repo is regenerated before starting a new
+    # batch.
+    if builder.is_waiting_for_repo_regen():
+        log.debug("Not starting new batch, repo regeneration is in progress "
+                  "for module %s" % module)
+        return []
+
     module.batch += 1
 
     # The user can either pass in a list of components to 'seed' the batch, or

tests/test_build/test_build.py

@@ -145,6 +145,9 @@ class TestModuleBuilder(GenericBuilder):
         if TestModuleBuilder.on_tag_artifacts_cb:
             TestModuleBuilder.on_tag_artifacts_cb(self, artifacts)
 
+    def is_waiting_for_repo_regen(self):
+        return False
+
     @property
     def module_build_tag(self):
         return {"name": self.tag_name + "-build"}
@@ -526,3 +529,59 @@ class TestBuild(unittest.TestCase):
         # does not work correctly.
         num_builds = [k for k, g in itertools.groupby(TestBuild._global_var)]
         self.assertEqual(num_builds.count(1), 2)
+
+    @timed(30)
+    @patch('module_build_service.auth.get_user', return_value=user)
+    @patch('module_build_service.scm.SCM')
+    @patch("module_build_service.config.Config.num_consecutive_builds",
+           new_callable=PropertyMock, return_value = 1)
+    def test_build_in_batch_fails(self, conf_num_consecutive_builds, mocked_scm,
+                                  mocked_get_user, conf_system, dbg):
+        """
+        Tests that if the build in batch fails, other components in a batch
+        are still build, but next batch is not started.
+        """
+        MockedSCM(mocked_scm, 'testmodule', 'testmodule.yaml',
+                  '620ec77321b2ea7b0d67d82992dda3e1d67055b4')
+
+        rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps(
+            {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/'
+                'testmodule.git?#68932c90de214d9d13feefbd35246a81b6cb8d49'}))
+
+        data = json.loads(rv.data)
+        module_build_id = data['id']
+
+        def on_build_cb(cls, artifact_name, source):
+            # Next component *after* the module-build-macros will fail
+            # to build.
+            if artifact_name.startswith("module-build-macros"):
+                TestModuleBuilder.BUILD_STATE = "FAILED"
+            else:
+                TestModuleBuilder.BUILD_STATE = "COMPLETE"
+            print artifact_name
+
+        TestModuleBuilder.on_build_cb = on_build_cb
+
+        msgs = []
+        stop = module_build_service.scheduler.make_simple_stop_condition(db.session)
+        module_build_service.scheduler.main(msgs, stop)
+
+        for c in models.ComponentBuild.query.filter_by(module_id=module_build_id).all():
+            # perl-Tangerine is expected to fail as configured in on_build_cb.
+            if c.package == "perl-Tangerine":
+                self.assertEqual(c.state, koji.BUILD_STATES['FAILED'])
+            # tangerine is expected to fail, because it is in batch 3, but
+            # we had a failing component in batch 2.
+            elif c.package == "tangerine":
+                self.assertEqual(c.state, koji.BUILD_STATES['FAILED'])
+                self.assertEqual(c.state_reason, "Some components failed to build.")
+            else:
+                self.assertEqual(c.state, koji.BUILD_STATES['COMPLETE'])
+
+            # Whole module should be failed.
+            self.assertEqual(c.module_build.state, models.BUILD_STATES['failed'])
+            self.assertEqual(c.module_build.state_reason, "Some components failed to build.")
+
+            # 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)

tests/test_utils/test_utils.py

@@ -396,6 +396,9 @@ class DummyModuleBuilder(GenericBuilder):
     def tag_artifacts(self, artifacts):
         pass
 
+    def is_waiting_for_repo_regen(self):
+        return False
+
     @property
     def module_build_tag(self):
         return {"name": self.tag_name + "-build"}
@@ -443,6 +446,7 @@ class TestBatches(unittest.TestCase):
         module_build.batch = 1
 
         builder = mock.MagicMock()
+        builder.is_waiting_for_repo_regen.return_value = False
         further_work = module_build_service.utils.start_next_batch_build(
             conf, module_build, db.session, builder)
 
@@ -498,5 +502,18 @@ class TestBatches(unittest.TestCase):
         self.assertEqual(len(further_work), 2)
         self.assertEqual(further_work[0].build_name, "perl-List-Compare")
 
+    def test_start_next_batch_build_repo_building(self, default_buildroot_groups):
+        """
+        Test that start_next_batch_build does not start new batch when
+        builder.is_waiting_for_repo_regen() returns True.
+        """
+        module_build = models.ModuleBuild.query.filter_by(id=2).one()
+        module_build.batch = 1
 
+        builder = mock.MagicMock()
+        builder.is_waiting_for_repo_regen.return_value = True
+        further_work = module_build_service.utils.start_next_batch_build(
+            conf, module_build, db.session, builder)
 
+        # Batch number should not increase.
+        self.assertEqual(module_build.batch, 1)