From 6981449853aab655d404ce0a27aa5151af600dbb Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Jun 25 2018 21:05:22 +0000 Subject: Fix local builds. I discovered that local builds have been broken by recent (good) changes in dnf. At the end of every batch, we regenerate the local repo with createrepo and we also call modifyrepo to include the modulemd file as we progress. This was added in #467. What we really want, is for the modulemd file to be present at the *end* of the build. Recently, dnf started respecting the modulemd file natively, such that builds we built in batch0 would not show up in batch1. They would be present in the repo, but they would be marked as belonging to a module which was not enabled, and so could not be pulled in. Every module build would fail because module-srpm-macros was in a disabled module (the module being built at the time). This change makes it so that the module metadata is only added to the repo at the very end of the build. I moved it into a `finalize` method on the builder which the copr builder was using, and for symmetry's sake I moved the koji content generator code to the same method on that builder. There was a circular import issue to solve between the koji module builder and the koji content generator modules that generated a larger diff, but is mostly cosmetic and mock changes. --- diff --git a/module_build_service/builder/KojiContentGenerator.py b/module_build_service/builder/KojiContentGenerator.py index c04a1ba..e41ef9b 100644 --- a/module_build_service/builder/KojiContentGenerator.py +++ b/module_build_service/builder/KojiContentGenerator.py @@ -39,11 +39,15 @@ from six import text_type import koji from module_build_service import log, build_logs -from module_build_service.builder.KojiModuleBuilder import KojiModuleBuilder logging.basicConfig(level=logging.DEBUG) +def get_session(config, owner): + from module_build_service.builder.KojiModuleBuilder import KojiModuleBuilder + KojiModuleBuilder.get_session(config, owner) + + class KojiContentGenerator(object): """ Class for handling content generator imports of module builds into Koji """ @@ -184,7 +188,7 @@ class KojiContentGenerator(object): def _koji_rpms_in_tag(self, tag): """ Return the list of koji rpms in a tag. """ log.debug("Listing rpms in koji tag %s", tag) - session = KojiModuleBuilder.get_session(self.config, self.owner) + session = get_session(self.config, self.owner) try: rpms, builds = session.listTaggedRPMS(tag, latest=True) @@ -227,7 +231,7 @@ class KojiContentGenerator(object): } } } - session = KojiModuleBuilder.get_session(self.config, None) + session = get_session(self.config, None) # Only add the CG build owner if the user exists in Koji if session.getUser(self.owner): ret[u'owner'] = self.owner @@ -373,7 +377,7 @@ class KojiContentGenerator(object): """ Tags the Content Generator build to module.cg_build_koji_tag. """ - session = KojiModuleBuilder.get_session(self.config, self.owner) + session = get_session(self.config, self.owner) tag_name = self.module.cg_build_koji_tag if not tag_name: @@ -409,7 +413,7 @@ class KojiContentGenerator(object): a content generator based build Raises an exception when error is encountered during import""" - session = KojiModuleBuilder.get_session(self.config, self.owner) + session = get_session(self.config, self.owner) file_dir = self._prepare_file_directory() metadata = self._get_content_generator_metadata(file_dir) diff --git a/module_build_service/builder/KojiModuleBuilder.py b/module_build_service/builder/KojiModuleBuilder.py index 335381d..b98c01c 100644 --- a/module_build_service/builder/KojiModuleBuilder.py +++ b/module_build_service/builder/KojiModuleBuilder.py @@ -50,6 +50,7 @@ from module_build_service.builder.utils import execute_cmd from module_build_service.errors import ProgrammingError from module_build_service.builder.base import GenericBuilder +from module_build_service.builder.KojiContentGenerator import KojiContentGenerator logging.basicConfig(level=logging.DEBUG) @@ -1057,3 +1058,9 @@ chmod 644 %buildroot/etc/rpm/macros.zz-modules weights[component_name] = weight return weights + + def finalize(self): + # Only import to koji CG if the module is "done". + if self.config.koji_enable_content_generator and self.module.state == 3: + cg = KojiContentGenerator(self.module, self.config) + cg.koji_import() diff --git a/module_build_service/builder/MockModuleBuilder.py b/module_build_service/builder/MockModuleBuilder.py index b4f7847..c3bb557 100644 --- a/module_build_service/builder/MockModuleBuilder.py +++ b/module_build_service/builder/MockModuleBuilder.py @@ -153,7 +153,7 @@ class MockModuleBuilder(GenericBuilder): # Workaround koji specific code in modules.py return {"name": self.tag_name} - def _createrepo(self): + def _createrepo(self, include_module_yaml=False): """ Creates the repository using "createrepo_c" command in the resultsdir. """ @@ -205,12 +205,14 @@ class MockModuleBuilder(GenericBuilder): pkglist_f.close() m1_mmd.set_rpm_artifacts(artifacts) - mmd_path = os.path.join(path, "modules.yaml") - m1_mmd.dump(mmd_path) - - # Generate repo and inject modules.yaml there. + # Generate repo. execute_cmd(['/usr/bin/createrepo_c', '--pkglist', pkglist, path]) - execute_cmd(['/usr/bin/modifyrepo_c', '--mdtype=modules', mmd_path, repodata_path]) + + # ...and inject modules.yaml there if asked. + if include_module_yaml: + mmd_path = os.path.join(path, "modules.yaml") + m1_mmd.dump(mmd_path) + execute_cmd(['/usr/bin/modifyrepo_c', '--mdtype=modules', mmd_path, repodata_path]) def _add_repo(self, name, baseurl, extra=""): """ @@ -512,6 +514,10 @@ class MockModuleBuilder(GenericBuilder): def repo_from_tag(cls, config, tag_name, arch): pass + def finalize(self): + # One last createrepo, to include the module metadata. + self._createrepo(include_module_yaml=True) + class BaseBuilder(object): def __init__(self, config, resultsdir): diff --git a/module_build_service/builder/base.py b/module_build_service/builder/base.py index 69d7a95..2574c81 100644 --- a/module_build_service/builder/base.py +++ b/module_build_service/builder/base.py @@ -269,6 +269,7 @@ class GenericBuilder(six.with_metaclass(ABCMeta)): """ raise NotImplementedError() + @abstractmethod def finalize(self): """ :return: None diff --git a/module_build_service/scheduler/handlers/components.py b/module_build_service/scheduler/handlers/components.py index 627e05a..291ff1f 100644 --- a/module_build_service/scheduler/handlers/components.py +++ b/module_build_service/scheduler/handlers/components.py @@ -103,7 +103,6 @@ def _finalize(config, session, msg, state): parent.transition(config, state=models.BUILD_STATES['failed'], state_reason="Some components failed to build.") session.commit() - builder.finalize() return [] elif not built_components_in_batch: # If there are no successfully built components in a batch, there is nothing to tag. diff --git a/module_build_service/scheduler/handlers/modules.py b/module_build_service/scheduler/handlers/modules.py index 896b302..1e1759c 100644 --- a/module_build_service/scheduler/handlers/modules.py +++ b/module_build_service/scheduler/handlers/modules.py @@ -34,7 +34,6 @@ from module_build_service.utils import ( get_rpm_release, generate_koji_tag) from module_build_service.errors import UnprocessableEntity, Forbidden, ValidationError -from module_build_service.builder.KojiContentGenerator import KojiContentGenerator from requests.exceptions import ConnectionError @@ -88,6 +87,9 @@ def failed(config, session, msg): component.state = koji.BUILD_STATES['FAILED'] component.state_reason = build.state_reason session.add(component) + + # Tell the external buildsystem to wrap up (copr API) + builder.finalize() else: # Do not overwrite state_reason set by Frontend if any. if not build.state_reason: @@ -100,7 +102,9 @@ def failed(config, session, msg): # Don't transition it again if it's already been transitioned if build.state != models.BUILD_STATES["failed"]: build.transition(config, state="failed") + session.commit() + build_logs.stop(build) module_build_service.builder.GenericBuilder.clear_cache(build) @@ -121,10 +125,11 @@ def done(config, session, msg): # This is ok.. it's a race condition we can ignore. pass - if config.system == 'koji' and config.koji_enable_content_generator: - # KojiContentGenerator import - cg = KojiContentGenerator(build, config) - cg.koji_import() + builder = module_build_service.builder.GenericBuilder.create_from_module( + session, build, config) + + # Tell the external buildsystem to wrap up (CG import, copr API, createrepo, etc.) + builder.finalize() build.transition(config, state="ready") session.commit() diff --git a/module_build_service/scheduler/handlers/repos.py b/module_build_service/scheduler/handlers/repos.py index 0d1eac0..7eab9d9 100644 --- a/module_build_service/scheduler/handlers/repos.py +++ b/module_build_service/scheduler/handlers/repos.py @@ -146,6 +146,5 @@ def done(config, session, msg): else: module_build.transition(config, state=models.BUILD_STATES['done']) session.commit() - builder.finalize() return further_work diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py index a209856..f758944 100644 --- a/tests/test_build/test_build.py +++ b/tests/test_build/test_build.py @@ -277,6 +277,9 @@ class FakeModuleBuilder(GenericBuilder): component_build.module_build.koji_tag + '-build', component_build.package)) return msgs + def finalize(self): + pass + def cleanup_moksha(): # Necessary to restart the twisted reactor for the next test. diff --git a/tests/test_content_generator.py b/tests/test_content_generator.py index 533f7dd..3953b46 100644 --- a/tests/test_content_generator.py +++ b/tests/test_content_generator.py @@ -72,7 +72,7 @@ class TestBuild: except OSError: pass - @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session") + @patch("module_build_service.builder.KojiContentGenerator.get_session") @patch("subprocess.Popen") @patch("subprocess.check_output", return_value='1.4') @patch("pkg_resources.get_distribution") @@ -118,7 +118,7 @@ class TestBuild: rpms_in_tag.assert_called_once() assert expected_output == ret - @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session") + @patch("module_build_service.builder.KojiContentGenerator.get_session") @patch("subprocess.Popen") @patch("subprocess.check_output", return_value='1.4') @patch("pkg_resources.get_distribution") @@ -165,7 +165,7 @@ class TestBuild: with open(path.join(dir_path, "modulemd.txt")) as mmd: assert len(mmd.read()) == 1134 - @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session") + @patch("module_build_service.builder.KojiContentGenerator.get_session") def test_tag_cg_build(self, get_session): """ Test that the CG build is tagged. """ koji_session = MagicMock() @@ -178,7 +178,7 @@ class TestBuild: koji_session.getTag.assert_called_once_with(self.cg.module.cg_build_koji_tag) koji_session.tagBuild.assert_called_once_with(123, "nginx-0-2.10e50d06") - @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session") + @patch("module_build_service.builder.KojiContentGenerator.get_session") def test_tag_cg_build_fallback_to_default_tag(self, get_session): """ Test that the CG build is tagged to default tag. """ koji_session = MagicMock() @@ -193,7 +193,7 @@ class TestBuild: call(conf.koji_cg_default_build_tag)] koji_session.tagBuild.assert_called_once_with(123, "nginx-0-2.10e50d06") - @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session") + @patch("module_build_service.builder.KojiContentGenerator.get_session") def test_tag_cg_build_no_tag_set(self, get_session): """ Test that the CG build is not tagged when no tag set. """ koji_session = MagicMock() @@ -206,7 +206,7 @@ class TestBuild: koji_session.tagBuild.assert_not_called() - @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session") + @patch("module_build_service.builder.KojiContentGenerator.get_session") def test_tag_cg_build_no_tag_available(self, get_session): """ Test that the CG build is not tagged when no tag available. """ koji_session = MagicMock() diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index 6f2efee..1c494b2 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -619,6 +619,9 @@ class DummyModuleBuilder(GenericBuilder): def repo_from_tag(self, config, tag_name, arch): pass + def finalize(self): + pass + @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups", return_value={'build': [], 'srpm-build': []})