diff --git a/module_build_service/builder/KojiModuleBuilder.py b/module_build_service/builder/KojiModuleBuilder.py index 6500701d..fdef3efa 100644 --- a/module_build_service/builder/KojiModuleBuilder.py +++ b/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 diff --git a/module_build_service/builder/MockModuleBuilder.py b/module_build_service/builder/MockModuleBuilder.py index 2bb5a457..19993e95 100644 --- a/module_build_service/builder/MockModuleBuilder.py +++ b/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 diff --git a/module_build_service/builder/base.py b/module_build_service/builder/base.py index 33aa14f3..b54e45a8 100644 --- a/module_build_service/builder/base.py +++ b/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 diff --git a/module_build_service/scheduler/handlers/modules.py b/module_build_service/scheduler/handlers/modules.py index e4c99bce..2d1ea2f9 100644 --- a/module_build_service/scheduler/handlers/modules.py +++ b/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: diff --git a/module_build_service/scheduler/handlers/repos.py b/module_build_service/scheduler/handlers/repos.py index 95cb994b..63586241 100644 --- a/module_build_service/scheduler/handlers/repos.py +++ b/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() diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py index a29d1c23..e2f46dbc 100644 --- a/tests/test_build/test_build.py +++ b/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) diff --git a/tests/test_scheduler/test_repo_done.py b/tests/test_scheduler/test_repo_done.py index df1b1cc6..e24ce3d9 100644 --- a/tests/test_scheduler/test_repo_done.py +++ b/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 diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index c259386e..7d8d8614 100644 --- a/tests/test_utils/test_utils.py +++ b/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