GenericBuilder: Add a boolean 'succeeded' parameter to finalize

Previously MockModuleBuilder was checking the module state to see if
it should run a final createrepo, but since eafa93037f, finalize() is
called before changing the module state; add an explicit boolean to
GenericBuilder.finalize() to avoid worrying about ordering.

module_build_service/builder/KojiModuleBuilder.py

@@ -1209,7 +1209,7 @@ chmod 644 %buildroot/etc/rpm/macros.zz-modules
             nvrs = set(kobo.rpmlib.make_nvr(rpm, force_epoch=True) for rpm in rpms)
             return list(nvrs)
 
-    def finalize(self):
+    def finalize(self, succeeded=True):
         # Only import to koji CG if the module is "build" and not scratch.
         if (not self.module.scratch and
                 self.config.koji_enable_content_generator and

module_build_service/builder/MockModuleBuilder.py

@@ -542,11 +542,11 @@ class MockModuleBuilder(GenericBuilder):
     def repo_from_tag(cls, config, tag_name, arch):
         pass
 
-    def finalize(self):
-        # If the state is "done", run one last createrepo, to include
+    def finalize(self, succeeded=True):
+        # For successful builds, do one last createrepo, to include
         # the module metadata. We don't want to do this for failed builds,
         # since that makes it impossible to retry a build manually.
-        if self.module.state == models.BUILD_STATES["done"]:
+        if succeeded:
             self._createrepo(include_module_yaml=True)
 
     @classmethod

module_build_service/builder/base.py

@@ -262,12 +262,13 @@ class GenericBuilder(six.with_metaclass(ABCMeta)):
         raise NotImplementedError()
 
     @abstractmethod
-    def finalize(self):
+    def finalize(self, succeeded=True):
         """
+        :param succeeded: True if all module builds were successful
         :return: None
 
         This method is supposed to be called after all module builds are
-        successfully finished.
+        finished.
 
         It could be utilized for various purposes such as cleaning or
         running additional build-system based operations on top of

module_build_service/scheduler/handlers/modules.py

@@ -92,7 +92,7 @@ def failed(config, session, msg):
             session.add(component)
 
         # Tell the external buildsystem to wrap up
-        builder.finalize()
+        builder.finalize(succeeded=False)
     else:
         # Do not overwrite state_reason set by Frontend if any.
         if not build.state_reason:

module_build_service/scheduler/handlers/repos.py

@@ -151,7 +151,7 @@ def done(config, session, msg):
         else:
             # Tell the external buildsystem to wrap up (CG import, createrepo, etc.)
             module_build.time_completed = datetime.utcnow()
-            builder.finalize()
+            builder.finalize(succeeded=True)
 
             module_build.transition(config, state=models.BUILD_STATES['done'])
         session.commit()

tests/test_build/test_build.py

@@ -103,6 +103,7 @@ class FakeModuleBuilder(GenericBuilder):
 
     on_build_cb = None
     on_cancel_cb = None
+    on_finalize_cb = None
     on_buildroot_add_artifacts_cb = None
     on_tag_artifacts_cb = None
 
@@ -118,6 +119,7 @@ class FakeModuleBuilder(GenericBuilder):
         FakeModuleBuilder.INSTANT_COMPLETE = False
         FakeModuleBuilder.on_build_cb = None
         FakeModuleBuilder.on_cancel_cb = None
+        FakeModuleBuilder.on_finalize_cb = None
         FakeModuleBuilder.on_buildroot_add_artifacts_cb = None
         FakeModuleBuilder.on_tag_artifacts_cb = None
         FakeModuleBuilder.DEFAULT_GROUPS = None
@@ -281,8 +283,9 @@ class FakeModuleBuilder(GenericBuilder):
                 component_build.nvr))
         return msgs
 
-    def finalize(self):
-        pass
+    def finalize(self, succeeded=None):
+        if FakeModuleBuilder.on_finalize_cb:
+            FakeModuleBuilder.on_finalize_cb(self, succeeded)
 
 
 def cleanup_moksha():
@@ -354,9 +357,13 @@ class TestBuild:
         tag_groups.append(set(['perl-Tangerine-1-1', 'perl-List-Compare-1-1']))
         tag_groups.append(set(['tangerine-1-1']))
 
+        def on_finalize_cb(cls, succeeded):
+            assert succeeded is True
+
         def on_tag_artifacts_cb(cls, artifacts, dest_tag=True):
             assert tag_groups.pop(0) == set(artifacts)
 
+        FakeModuleBuilder.on_finalize_cb = on_finalize_cb
         FakeModuleBuilder.on_tag_artifacts_cb = on_tag_artifacts_cb
 
         # Check that the components are added to buildroot after the batch
@@ -505,11 +512,15 @@ class TestBuild:
         def on_cancel_cb(cls, task_id):
             cancelled_tasks.append(task_id)
 
+        def on_finalize_cb(cls, succeeded):
+            assert succeeded is False
+
         # We do not want the builds to COMPLETE, but instead we want them
         # to be in the BULDING state after the FakeModuleBuilder.build().
         FakeModuleBuilder.BUILD_STATE = "BUILDING"
         FakeModuleBuilder.on_build_cb = on_build_cb
         FakeModuleBuilder.on_cancel_cb = on_cancel_cb
+        FakeModuleBuilder.on_finalize_cb = on_finalize_cb
 
         msgs = []
         stop = module_build_service.scheduler.make_simple_stop_condition(db.session)

tests/test_scheduler/test_repo_done.py

@@ -107,9 +107,10 @@ class TestRepoDone:
         module_build.time_completed = None
         db.session.commit()
 
-        def mocked_finalizer():
+        def mocked_finalizer(succeeded=None):
             # Check that the time_completed is set in the time when
             # finalizer is called.
+            assert succeeded is True
             module_build = module_build_service.models.ModuleBuild.query.get(2)
             assert module_build.time_completed is not None
 

tests/test_utils/test_utils.py

@@ -866,7 +866,7 @@ class DummyModuleBuilder(GenericBuilder):
     def repo_from_tag(self, config, tag_name, arch):
         pass
 
-    def finalize(self):
+    def finalize(self, succeeded=True):
         pass