Merge #451 Remove is_waiting_for_repo_regen and use buildroot_ready instead. Do not get buildroot dependencies from the buildrequires recursively. Do not validate Koji tags to inherit.

module_build_service/builder.py

@@ -332,12 +332,6 @@ 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 """
@@ -378,20 +372,6 @@ 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):
         """
@@ -750,7 +730,7 @@ chmod 644 %buildroot/%_rpmconfigdir/macros.d/macros.modules
                 raise SystemError("Unknown tag: %s" % tag)
         return taginfo
 
-    @module_build_service.utils.validate_koji_tag(['tag_name', 'parent_tags'], post='')
+    @module_build_service.utils.validate_koji_tag(['tag_name'], post='')
     def _koji_add_many_tag_inheritance(self, tag_name, parent_tags):
         tag = self._get_tag(tag_name)
         # highest priority num is at the end
@@ -1079,9 +1059,6 @@ 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
@@ -1579,8 +1556,6 @@ 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,

module_build_service/pdc.py

@@ -274,54 +274,40 @@ def module_depsolving_wrapper(session, modules, strict=True):
     """
     log.debug("module_depsolving_wrapper(%r, strict=%r)" % (modules, strict))
 
-    # We're going to modify this list, so don't screw with the passed in value.
-    module_list = copy.deepcopy(modules)
-    # Name this, just for readability...
-    original_modules = modules
-
-    # Keep track of which modules we've queried for before, to avoid that.
-    seen = list()
-
     # This is the set we're going to build up and return.
     module_tags = set()
 
-    while not all([d in seen for d in module_list]):
-        for module_dict in module_list:
-            # Make sure not to spam PDC with unnecessary queries.
-            if module_dict in seen:
-                continue
-            seen.append(module_dict)
+    # We want to take only single 
+    for module_dict in modules:
+        # Get enhanced info on this module from pdc.
+        info = get_module(session, module_dict, strict)
 
-            # Get enhanced info on this module from pdc.
-            info = get_module(session, module_dict, strict)
+        # Take note of the tag of this module, but only if it is a dep and
+        # not in the original list.
+         # XXX - But, for now go ahead and include it because that's how this
+        # code used to work.
+        module_tags.add(info['koji_tag'])
 
-            # Take note of the tag of this module, but only if it is a dep and
-            # not in the original list.
-            #if module_dict not in original_modules:
-            #    module_tags.add(info['koji_tag'])
-            # XXX - But, for now go ahead and include it because that's how this
-            # code used to work.
+        # Now, when we look for the deps of this module, use the mmd.xmd
+        # attributes because they contain the promise of *exactly* which
+        # versions of which deps we say we're going to build *this* module
+        # against.
+        if not info['modulemd']:
+            raise ValueError("No PDC modulemd found for %r" % info)
+        mmd = _extract_modulemd(info['modulemd'])
+
+        # Queue up the next tier of deps that we should look at..
+        for name, details in mmd.xmd['mbs']['buildrequires'].items():
+            modified_dep = {
+                'name': name,
+                'version': details['stream'],
+                'release': details['version'],
+                # Only return details about module builds that finished
+                'active': True,
+            }
+            info = get_module(session, modified_dep, strict)
             module_tags.add(info['koji_tag'])
 
-            # Now, when we look for the deps of this module, use the mmd.xmd
-            # attributes because they contain the promise of *exactly* which
-            # versions of which deps we say we're going to build *this* module
-            # against.
-            if not info['modulemd']:
-                raise ValueError("No PDC modulemd found for %r" % info)
-            mmd = _extract_modulemd(info['modulemd'])
-
-            # Queue up the next tier of deps that we should look at..
-            for name, details in mmd.xmd['mbs']['buildrequires'].items():
-                modified_dep = {
-                    'name': name,
-                    'version': details['stream'],
-                    'release': details['version'],
-                    # Only return details about module builds that finished
-                    'active': True,
-                }
-                module_list.append(modified_dep)
-
     return list(module_tags)
 
 

module_build_service/utils.py

@@ -290,9 +290,10 @@ def start_next_batch_build(config, module, session, builder, components=None):
     # 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)
+    artifacts = [c.nvr for c in module.current_batch()]
+    if not builder.buildroot_ready(artifacts):
+        log.info("Not starting new batch, not all of %r are in the buildroot. "
+                 "Waiting." % artifacts)
         return []
 
     module.batch += 1

tests/test_build/test_build.py

@@ -145,9 +145,6 @@ 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"}

tests/test_pdc.py

@@ -68,6 +68,22 @@ class TestPDCModule(unittest.TestCase):
         ]
         self.assertEqual(set(result), set(expected))
 
+    def test_get_module_depsolving_wrapper_recursive(self):
+        query = [{
+            'name': 'testmodule',
+            'version': 'master',
+            'release': '20170322155247',
+            'active': False,
+        }]
+        result = mbs_pdc.module_depsolving_wrapper(self.pdc, query)
+        expected = [
+            u'module-base-runtime-master-20170315134803',
+            # Should the list of deps should not include the original tag?
+            # Probably not.
+            u'module-testmodule-master-20170322155247'
+        ]
+        self.assertEqual(set(result), set(expected))
+
     def test_resolve_profiles(self):
         current_dir = os.path.dirname(__file__)
         yaml_path = os.path.join(

tests/test_utils/test_utils.py

@@ -396,9 +396,6 @@ 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"}
@@ -446,7 +443,6 @@ 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)
 
@@ -505,13 +501,13 @@ class TestBatches(unittest.TestCase):
     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.
+        builder.buildroot_ready() returns False.
         """
         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
+        builder.buildroot_ready.return_value = False
         further_work = module_build_service.utils.start_next_batch_build(
             conf, module_build, db.session, builder)