From 5bb5733e8237b4b8b6399c2acda84b059fcfd9d1 Mon Sep 17 00:00:00 2001 From: Owen W. Taylor Date: Apr 26 2022 20:58:30 +0000 Subject: [PATCH 1/6] MockModuleBuilder: fix repo_name for local repositories --- diff --git a/module_build_service/builder/MockModuleBuilder.py b/module_build_service/builder/MockModuleBuilder.py index 0f28001..dcf239f 100644 --- a/module_build_service/builder/MockModuleBuilder.py +++ b/module_build_service/builder/MockModuleBuilder.py @@ -536,7 +536,7 @@ class MockModuleBuilder(GenericBuilder): # If source starts with mock_resultdir, it means it is path to local # module build repository. if source.startswith(conf.mock_resultsdir): - repo_name = os.path.basename(source) + repo_name = os.path.basename(os.path.dirname(source)) if repo_name.startswith("module-"): repo_name = repo_name[7:] repo_dir = source From c35730cb334652f5ec9a921a443850ed944271a2 Mon Sep 17 00:00:00 2001 From: Owen W. Taylor Date: Apr 26 2022 20:58:30 +0000 Subject: [PATCH 2/6] MockModuleBuilder: move get_average_build_time() to the right class --- diff --git a/module_build_service/builder/MockModuleBuilder.py b/module_build_service/builder/MockModuleBuilder.py index dcf239f..8175609 100644 --- a/module_build_service/builder/MockModuleBuilder.py +++ b/module_build_service/builder/MockModuleBuilder.py @@ -779,6 +779,26 @@ class MockModuleBuilder(GenericBuilder): nvrs = set(kobo.rpmlib.make_nvr(rpm, force_epoch=True) for rpm in rpms) return list(nvrs) + def get_average_build_time(self, component): + """ + Get the average build time of the component from Koji + :param component: a ComponentBuild object + :return: a float of the average build time in seconds + """ + # We currently don't track build times in MBS directly, so we can use Koji to get a decent + # estimate + if not self.koji_session: + # If Koji is not configured on the system, then just return 0.0 for components + try: + self.koji_session = get_session(self.config, login=False) + # If the component has not been built before, then None is returned. Instead, + # let's return 0.0 so the type is consistent + return self.koji_session.getAverageBuildDuration(component.package) or 0.0 + except Exception: + log.debug( + "The Koji call to getAverageBuildDuration failed. Is Koji properly configured?") + return 0.0 + class BaseBuilder(object): def __init__(self, config, resultsdir): @@ -846,23 +866,3 @@ class SCMBuilder(BaseBuilder): if source.startswith(host): return cmds raise KeyError("No defined commands for {}".format(source)) - - def get_average_build_time(self, component): - """ - Get the average build time of the component from Koji - :param component: a ComponentBuild object - :return: a float of the average build time in seconds - """ - # We currently don't track build times in MBS directly, so we can use Koji to get a decent - # estimate - if not self.koji_session: - # If Koji is not configured on the system, then just return 0.0 for components - try: - self.koji_session = get_session(self.config, login=False) - # If the component has not been built before, then None is returned. Instead, - # let's return 0.0 so the type is consistent - return self.koji_session.getAverageBuildDuration(component.package) or 0.0 - except Exception: - log.debug( - "The Koji call to getAverageBuildDuration failed. Is Koji properly configured?") - return 0.0 From 0b3df487005d4deff37beab85d0a04568a8d6b5b Mon Sep 17 00:00:00 2001 From: Owen W. Taylor Date: Apr 26 2022 20:58:30 +0000 Subject: [PATCH 3/6] MockModuleBuilder.SCMBuilder: remove unused method --- diff --git a/module_build_service/builder/MockModuleBuilder.py b/module_build_service/builder/MockModuleBuilder.py index 8175609..0991328 100644 --- a/module_build_service/builder/MockModuleBuilder.py +++ b/module_build_service/builder/MockModuleBuilder.py @@ -856,11 +856,6 @@ class SCMBuilder(BaseBuilder): src_dir = repo_path[len("file://"):] f.write("config_opts['scm_opts']['ext_src_dir'] = '{}'\n".format(src_dir)) - def _make_executable(self, path): - mode = os.stat(path).st_mode - mode |= (mode & 0o444) >> 2 # copy R bits to X - os.chmod(path, mode) - def _get_distgit_commands(self, source): for host, cmds in conf.distgits.items(): if source.startswith(host): From f5c7c6b9f442538141c30e5dcd1e1ffa410032e4 Mon Sep 17 00:00:00 2001 From: Owen W. Taylor Date: Apr 26 2022 20:58:30 +0000 Subject: [PATCH 4/6] MockModuleBuilder.py: remove stale code to remove -srpm-stdout.log This log is no longer generated, since the code was switched to use Mock SCM. --- diff --git a/module_build_service/builder/MockModuleBuilder.py b/module_build_service/builder/MockModuleBuilder.py index 0991328..5fb4cab 100644 --- a/module_build_service/builder/MockModuleBuilder.py +++ b/module_build_service/builder/MockModuleBuilder.py @@ -618,13 +618,6 @@ class MockModuleBuilder(GenericBuilder): if os.path.isfile(log_path) and os.path.getsize(log_path) == 0: os.remove(log_path) - # Remove other files containing useless information - elif logf.endswith("-srpm-stdout.log"): - with open(log_path) as f: - data = f.read(4096) - if re.match("Downloading [^\n]*\n\n\nWrote: [^\n]", data): - os.remove(log_path) - def build_srpm(self, artifact_name, source, build_id, builder): """ Builds the artifact from the SRPM. From 48224c7853353ad5b007a982f292b1674e0bfbcf Mon Sep 17 00:00:00 2001 From: Owen W. Taylor Date: Apr 26 2022 20:58:30 +0000 Subject: [PATCH 5/6] MockModuleBuilder: fix an encoding error running under Python 2.7 --- diff --git a/module_build_service/builder/MockModuleBuilder.py b/module_build_service/builder/MockModuleBuilder.py index 5fb4cab..df9a1b8 100644 --- a/module_build_service/builder/MockModuleBuilder.py +++ b/module_build_service/builder/MockModuleBuilder.py @@ -413,8 +413,8 @@ class MockModuleBuilder(GenericBuilder): # ...and inject modules.yaml there if asked. if include_module_yaml: mmd_path = os.path.join(path, "modules.yaml") - with open(mmd_path, "w") as f: - f.write(mmd_to_str(m1_mmd)) + with open(mmd_path, "wb") as f: + f.write(mmd_to_str(m1_mmd).encode("utf-8")) execute_cmd(["/usr/bin/modifyrepo_c", "--mdtype=modules", mmd_path, repodata_path]) def _add_repo(self, name, baseurl, extra=""): From 9c1fc386ff9f79f8afc90e683616d7fd402ef1e9 Mon Sep 17 00:00:00 2001 From: Owen W. Taylor Date: Apr 26 2022 20:58:30 +0000 Subject: [PATCH 6/6] Improve test coverage for MockModuleBuilder Test coverage for this file improves from 55% => 89% --- diff --git a/tests/test_builder/test_mock.py b/tests/test_builder/test_mock.py index 738b4e9..956c0c5 100644 --- a/tests/test_builder/test_mock.py +++ b/tests/test_builder/test_mock.py @@ -12,6 +12,7 @@ import mock import pytest from module_build_service.common.config import conf +from module_build_service.builder.base import GenericBuilder from module_build_service.builder.MockModuleBuilder import ( import_fake_base_module, import_builds_from_local_dnf_repos, @@ -20,13 +21,28 @@ from module_build_service.builder.MockModuleBuilder import ( ) from module_build_service.common import models from module_build_service.common.models import ModuleBuild, ComponentBuild -from module_build_service.common.utils import load_mmd, mmd_to_str +from module_build_service.common.utils import import_mmd, load_mmd, mmd_to_str from module_build_service.scheduler import events from module_build_service.scheduler.db_session import db_session from tests import make_module_in_db, read_staged_data, staged_data_filename -@pytest.mark.usefixtures("require_empty_database") +class TestConfig(object): + def __init__(self, resultsdir): + self.resultsdir = resultsdir + + +@pytest.fixture +def testconfig(): + resultsdir = tempfile.mkdtemp() + + with mock.patch.multiple("module_build_service.common.conf", + mock_resultsdir=resultsdir, + system="mock"): + yield TestConfig(resultsdir) + + +@pytest.mark.usefixtures("require_empty_database", "testconfig") class TestMockModuleBuilder: def setup_method(self, test_method): self.resultdir = tempfile.mkdtemp() @@ -121,7 +137,6 @@ class TestMockModuleBuilder: return module - @mock.patch("module_build_service.common.conf.system", new="mock") def test_createrepo_filter_last_batch(self): module = self._create_module_with_filters(db_session, 3, koji.BUILD_STATES["COMPLETE"]) @@ -148,7 +163,6 @@ class TestMockModuleBuilder: rpm_names = [kobo.rpmlib.parse_nvr(rpm)["name"] for rpm in pkglist.split("\n")] assert "ed" not in rpm_names - @mock.patch("module_build_service.common.conf.system", new="mock") def test_createrepo_not_last_batch(self): module = self._create_module_with_filters(db_session, 2, koji.BUILD_STATES["COMPLETE"]) @@ -173,7 +187,6 @@ class TestMockModuleBuilder: rpm_names = [kobo.rpmlib.parse_nvr(rpm)["name"] for rpm in pkglist.split("\n")] assert "ed" in rpm_names - @mock.patch("module_build_service.common.conf.system", new="mock") def test_createrepo_empty_rmp_list(self): module = self._create_module_with_filters(db_session, 3, koji.BUILD_STATES["COMPLETE"]) @@ -189,26 +202,160 @@ class TestMockModuleBuilder: assert not pkglist -@pytest.mark.usefixtures("require_empty_database") -class TestMockModuleBuilderAddRepos: +@pytest.mark.usefixtures("require_platform_and_default_arch", "testconfig") +class TestMockModuleBuilderBuild: + + @pytest.fixture(params=[{}]) + def mock_external_commands(self, request): + def mock_popen(cmd, *args, **kwargs): + stdout = kwargs.get("stdout", None) + + result = mock.MagicMock(name="Popen") + result.__enter__.return_value = result + result.communicate.return_value = (None, None) + result.poll.return_value = 0 + result.returncode = 0 + + command = os.path.basename(cmd[0]) + if command == "mock": + if "--init" in cmd: + return result + + result.returncode = 1 if "fail_build" in request.param else 0 + + resultdir = None + for arg in cmd: + if arg.startswith("--resultdir="): + resultdir = arg[len("--resultdir="):] + assert resultdir + + result_files = [ + "perl-Tangerine-0.22-2.module_1589+0def5cd9.src.rpm", + "perl-Tangerine-0.22-2.module_1589+0def5cd9.noarch.rpm" + ] + + if stdout: + stdout.write("Successfully built perl-Tangerine") + stdout.flush() + + for f in result_files: + with open(os.path.join(resultdir, f), "w") as f: + f.write("-") + + return result + elif command == "createrepo_c": + # Expecting /usr/bin/createrepo_c --pkglist + assert len(cmd) == 4 + assert cmd[0:2] == ["/usr/bin/createrepo_c", "--pkglist"] + path = cmd[3] + + repodata_path = os.path.join(path, "repodata") + if not os.path.exists(repodata_path): + os.mkdir(repodata_path) + with open(os.path.join(repodata_path, "repomd.xml"), "w") as f: + f.write("-") + + return result + elif command == "modifyrepo_c": + return result + elif command == "rpm": + assert cmd[0:4] == [ + "rpm", + "--queryformat", + "%{NAME} %{EPOCHNUM} %{VERSION} %{RELEASE} %{ARCH}\n", + "-qp" + ] + packages = cmd[4:] + ret = "" + for p in packages: + nvr, arch, ext = p.rsplit(".", 2) + n, v, r = nvr.rsplit("-", 2) + ret += "{} 0 {} {} {}\n".format(n, v, r, arch) + + result.communicate.return_value = (ret, None) + + return result + else: + raise RuntimeError("Unsupported cmd {}".format(cmd)) + + with mock.patch("subprocess.Popen") as popen_mock: + popen_mock.side_effect = mock_popen + yield + + @pytest.fixture + def builder(self, mock_external_commands, require_platform_and_default_arch): + mmd = load_mmd(read_staged_data("formatted_testmodule")) + xmd = mmd.get_xmd() + xmd["mbs"]["koji_tag"] = "module-testmodule-master-20180205135154-9c690d0e" + mmd.set_xmd(xmd) - @mock.patch("module_build_service.common.conf.system", new="mock") - @mock.patch( - "module_build_service.common.config.Config.base_module_repofiles", - new_callable=mock.PropertyMock, - return_value=["/etc/yum.repos.d/bar.repo", "/etc/yum.repos.d/bar-updates.repo"], - create=True, - ) - @mock.patch("module_build_service.builder.MockModuleBuilder.open", create=True) - @mock.patch( - "module_build_service.builder.MockModuleBuilder.MockModuleBuilder._load_mock_config" - ) - @mock.patch( - "module_build_service.builder.MockModuleBuilder.MockModuleBuilder._write_mock_config" - ) - def test_buildroot_add_repos( - self, write_config, load_config, patched_open, base_module_repofiles - ): + import_mmd(db_session, mmd) + + build = ModuleBuild.get_last_build_in_stream(db_session, "testmodule", "master") + build.batch = 2 + + comp_builds = [ + { + "module_id": build.id, + "state": koji.BUILD_STATES["BUILDING"], + "package": "perl-Tangerine", + "format": "rpms", + "scmurl": ( + "https://src.fedoraproject.org/rpms/perl-Tangerine?#master" + ), + "batch": 2, + "ref": "01234567812fea798671925cc537f2f29b0bd216", + }, + ] + + for build in comp_builds: + db_session.add(ComponentBuild(**build)) + db_session.commit() + + # This is used to identify the first build of a module + MockModuleBuilder._build_id = 1 + + build = ModuleBuild.get_last_build_in_stream(db_session, "testmodule", "master") + yield GenericBuilder.create_from_module(db_session, build, conf, buildroot_connect=True) + + def test_mock_module_builder_build(self, builder): + builder.buildroot_add_artifacts([ + "module-build-macros-0.1-1.module+f33+2+cea92c88.noarch.rpm" + ]) + + builder.build("perl-Tangerine", "https://src.fedoraproject.org/rpms/perl-Tangerine?#master") + + # Try building again to exercise code to clean up last build for this thread + builder.build("perl-Tangerine", "https://src.fedoraproject.org/rpms/perl-Tangerine?#master") + + builder.finalize(succeeded=True) + + @pytest.mark.parametrize("mock_external_commands", [{"fail_build": True}], indirect=True) + def test_mock_module_builder_build_failure(self, builder, caplog): + builder.build("perl-Tangerine", "file://opt/sources/fedora/perl-Tangerine?#master") + assert "Error while building artifact perl-Tangerine" in caplog.text + + def test_mock_module_builder_build_local_repo(self, builder): + builder.build("perl-Tangerine", "file://opt/sources/fedora/perl-Tangerine?#master") + + def test_mock_module_builder_build_srpm(self, builder): + builder.build("perl-Tangerine", "/opt/sources/perl-Tangerine-0.22-2.src.rpm") + + def test_mock_module_builder_build_stale_builddir(self, builder): + builder.build("perl-Tangerine", "/opt/sources/perl-Tangerine-0.22-2.src.rpm") + builder.finalize(succeeded=True) + + MockModuleBuilder._build_id = 1 + builder2 = GenericBuilder.create_from_module( + db_session, builder.module, conf, buildroot_connect=True + ) + builder2.build("perl-Tangerine", "/opt/sources/perl-Tangerine-0.22-2.src.rpm") + + +@pytest.mark.usefixtures("require_empty_database", "testconfig") +class TestMockModuleBuilderAddRepos: + @pytest.fixture + def testsetup(self): import_fake_base_module("platform:f29:1:000000") platform = ModuleBuild.get_last_build_in_stream(db_session, "platform", "f29") @@ -219,14 +366,37 @@ class TestMockModuleBuilderAddRepos: foo = make_module_in_db("foo:1:1:1", module_deps) app = make_module_in_db("app:1:1:1", module_deps) + builder = MockModuleBuilder(db_session, "user", app, conf, "module-app", []) + + with mock.patch.multiple("module_build_service.builder.MockModuleBuilder.MockModuleBuilder", + _load_mock_config=mock.MagicMock(), + _write_mock_config=mock.MagicMock()): + yield { + "builder": builder, + "platform": platform, + "app": app, + "foo": foo, + } + + @mock.patch( + "module_build_service.common.config.Config.base_module_repofiles", + new_callable=mock.PropertyMock, + return_value=["/etc/yum.repos.d/bar.repo", "/etc/yum.repos.d/bar-updates.repo"], + create=True, + ) + @mock.patch("module_build_service.builder.MockModuleBuilder.open", create=True) + def test_buildroot_add_repos_repofile(self, patched_open, base_module_repofiles, testsetup): + builder = testsetup["builder"] + platform = testsetup["platform"] + foo = testsetup["foo"] + app = testsetup["app"] + patched_open.side_effect = [ mock.mock_open(read_data="[fake]\nrepofile 1\n").return_value, mock.mock_open(read_data="[fake]\nrepofile 2\n").return_value, mock.mock_open(read_data="[fake]\nrepofile 3\n").return_value, ] - builder = MockModuleBuilder(db_session, "user", app, conf, "module-app", []) - dependencies = { "repofile://": [platform.mmd()], "repofile:///etc/yum.repos.d/foo.repo": [foo.mmd(), app.mmd()], @@ -240,6 +410,83 @@ class TestMockModuleBuilderAddRepos: assert set(builder.enabled_modules) == {"foo:1", "app:1"} + def test_buildroot_add_repos_local(self, testsetup): + builder = testsetup["builder"] + foo = testsetup["foo"] + + dependencies = { + os.path.join(conf.mock_resultsdir, "module-foo/results"): [foo.mmd()] + } + + builder.buildroot_add_repos(dependencies) + + assert "[foo]" in builder.yum_conf + + assert set(builder.enabled_modules) == set() + + @mock.patch("koji.ClientSession") + def test_buildroot_add_repos_koji_tag_with_repo(self, client_session, testsetup): + builder = testsetup["builder"] + foo = testsetup["foo"] + + client_session_mock = mock.MagicMock() + client_session.return_value = client_session_mock + + client_session_mock.getRepo.return_value = { + "id": 2471511 + } + + dependencies = { + "module-foo": [foo.mmd()] + } + + builder.buildroot_add_repos(dependencies) + + assert "[module-foo]" in builder.yum_conf + assert ("baseurl=https://kojipkgs.stg.fedoraproject.org" + "//repos/module-foo/2471511/x86_64/" in builder.yum_conf) + + assert set(builder.enabled_modules) == set() + + @pytest.mark.parametrize("should_add_repo", (True, False)) + @mock.patch("koji.ClientSession") + @mock.patch("module_build_service.builder.MockModuleBuilder.create_local_repo_from_koji_tag") + def test_buildroot_add_repos_koji_tag_create_local_repo( + self, create_local_repo, client_session, testsetup, should_add_repo + ): + builder = testsetup["builder"] + foo = testsetup["foo"] + + import_fake_base_module("platform:f29:1:000000") + + client_session_mock = mock.MagicMock() + client_session.return_value = client_session_mock + + client_session_mock.getRepo.return_value = None + client_session_mock.getTagExternalRepos.return_value = [{ + "external_repo_name": "extdep", + "url": "http://example.com/repos/extdep", + }] + + create_local_repo.return_value = should_add_repo + + dependencies = { + "module-foo": [foo.mmd()] + } + + builder.buildroot_add_repos(dependencies) + + if should_add_repo: + assert "[module-foo]" in builder.yum_conf + assert ("baseurl=file:///tmp/mbs/koji_tags/module-foo" in builder.yum_conf) + else: + assert "[module-foo]" not in builder.yum_conf + + assert "[extdep]" in builder.yum_conf + assert ("baseurl=http://example.com/repos/extdep" in builder.yum_conf) + + assert set(builder.enabled_modules) == set() + @pytest.mark.usefixtures("require_empty_database_cls") class TestOfflineLocalBuilds: