PR#1568: Skip git ref checks for rpm components with srpm overrides - fm-orchestrator

fm-orchestrator

#1568 Skip git ref checks for rpm components with srpm overrides

Merged 4 years ago by mprahl. Opened 4 years ago by merlinm.

Unknown source skip-ref-check-for-custom-srpms into master

Skip git ref checks for rpm components with srpm overrides

Merlin Mathesius • 4 years ago

de2a776

Clean up unused test code to avoid confusion

Merlin Mathesius • 4 years ago

ab90c7b

module_build_service/scheduler/handlers/modules.py

file modified

+4 -2

		`@@ -31,7 +31,7 @@`
		`add_default_modules, handle_collisions_with_base_module_rpms)`
		`from module_build_service.scheduler.greenwave import greenwave`
		`from module_build_service.scheduler.reuse import attempt_to_reuse_all_components`
		`- from module_build_service.scheduler.submit import format_mmd`
		`+ from module_build_service.scheduler.submit import format_mmd, get_module_srpm_overrides`
		`from module_build_service.scheduler.ursine import handle_stream_collision_modules`

		`logging.basicConfig(level=logging.DEBUG)`
		`@@ -183,9 +183,11 @@`
		`arches = [arch.name for arch in build.arches]`
		`defaults_added = add_default_modules(mmd)`

		`+ # Get map of packages that have SRPM overrides`
		`+ srpm_overrides = get_module_srpm_overrides(build)`
		`# Format the modulemd by putting in defaults and replacing streams that`
		`# are branches with commit hashes`
		`- format_mmd(mmd, build.scmurl, build, db_session)`
		`+ format_mmd(mmd, build.scmurl, build, db_session, srpm_overrides)`
		`record_component_builds(mmd, build)`

		`# The ursine.handle_stream_collision_modules is Koji specific.`

module_build_service/scheduler/submit.py

file modified

+18 -8

		`@@ -182,7 +182,7 @@`
		`return {"pkg_name": pkg.get_name(), "pkg_ref": pkgref, "error": None}`


		`- def format_mmd(mmd, scmurl, module=None, db_session=None):`
		`+ def format_mmd(mmd, scmurl, module=None, db_session=None, srpm_overrides=None):`
		`"""`
		`Prepares the modulemd for the MBS. This does things such as replacing the`
		`branches of components with commit hashes and adding metadata in the xmd`
		`@@ -192,7 +192,11 @@`
		:param module: When specified together with `session`, the time_modified
		`of a module is updated regularly in case this method takes lot of time.`
		:param db_session: Database session to update the `module`.
		`+ :param dict srpm_overrides: Mapping of package names to SRPM links for all`
		`+ component packages which have custom SRPM overrides specified.`
		`"""`
		`+ srpm_overrides = srpm_overrides or {}`
		`+`
		`xmd = mmd.get_xmd()`
		`if "mbs" not in xmd:`
		`xmd["mbs"] = {}`
		`@@ -263,12 +267,18 @@`
		`pool = ThreadPool(20)`
		`try:`
		`# Filter out the packages which we have already resolved in possible`
		`- # previous runs of this method (can be caused by module build resubmition).`
		`- pkgs_to_resolve = [`
		`- mmd.get_rpm_component(name)`
		`- for name in mmd.get_rpm_component_names()`
		`- if name not in xmd["mbs"]["rpms"]`
		`- ]`
		`+ # previous runs of this method (can be caused by module build resubmition)`
		`+ # or which have custom SRPMs and shouldn't be resolved.`
		`+ pkgs_to_resolve = []`
		`+ for name in mmd.get_rpm_component_names():`
		`+ if name not in xmd["mbs"]["rpms"]:`
		`+ if name in srpm_overrides:`
		`+ # If this package has a custom SRPM, store an empty`
		`+ # ref entry so no further verification takes place.`
		`+ xmd["mbs"]["rpms"][name] = {"ref": None}`
		`+ else:`
		`+ pkgs_to_resolve.append(mmd.get_rpm_component(name))`
		`+`
		`async_result = pool.map_async(_scm_get_latest, pkgs_to_resolve)`

		`# For modules with lot of components, the _scm_get_latest can take a lot of time.`
		`@@ -436,7 +446,7 @@`
		`# It is OK to whitelist all URLs here, because the validity`
		`# of every URL have been already checked in format_mmd(...).`
		`included_mmd = fetch_mmd(full_url, whitelist_url=True)[0]`
		`- format_mmd(included_mmd, module.scmurl, module, db_session)`
		`+ format_mmd(included_mmd, module.scmurl, module, db_session, srpm_overrides)`
		`batch = record_component_builds(`
		`included_mmd, module, batch, previous_buildorder, main_mmd)`
		`continue`

tests/test_scheduler/test_submit.py

file modified

+22 -8

		`@@ -97,14 +97,17 @@`
		`None,`
		`],`
		`)`
		`+ @pytest.mark.parametrize(`
		`+ "srpm_overrides",`
		`+ [`
		`+ {"perl-List-Compare": "/path/to/perl-List-Compare.src.rpm"},`
		`+ None,`
		`+ ],`
		`+ )`
		`@mock.patch("module_build_service.common.scm.SCM")`
		`- def test_format_mmd(self, mocked_scm, scmurl):`
		`+ def test_format_mmd(self, mocked_scm, srpm_overrides, scmurl):`
		`mocked_scm.return_value.commit = "620ec77321b2ea7b0d67d82992dda3e1d67055b4"`
		`# For all the RPMs in testmodule, get_latest is called`
		`- mocked_scm.return_value.get_latest.side_effect = [`
		`- "4ceea43add2366d8b8c5a622a2fb563b625b9abf",`
		`- "fbed359411a1baa08d4a88e0d12d426fbf8f602c",`
		`- ]`
		`hashes_returned = {`
		`"master": "fbed359411a1baa08d4a88e0d12d426fbf8f602c",`
		`"f28": "4ceea43add2366d8b8c5a622a2fb563b625b9abf",`
		`@@ -112,24 +115,33 @@`
		`}`

		`def mocked_get_latest(ref="master"):`
		`- return hashes_returned[ref]`
		`+ if ref in hashes_returned:`
		`+ return hashes_returned[ref]`
		`+ raise RuntimeError("ref %s not found." % ref)`

		`mocked_scm.return_value.get_latest = mocked_get_latest`
		`mmd = load_mmd(read_staged_data("testmodule"))`
		`# Modify the component branches so we can identify them later on`
		`mmd.get_rpm_component("perl-Tangerine").set_ref("f28")`
		`mmd.get_rpm_component("tangerine").set_ref("f27")`
		`- format_mmd(mmd, scmurl)`
		`+ if srpm_overrides:`
		`+ # Set a bogus ref that will raise an exception if not properly ignored.`
		`+ mmd.get_rpm_component("perl-List-Compare").set_ref("bogus")`
		`+ format_mmd(mmd, scmurl, srpm_overrides=srpm_overrides)`

		`# Make sure that original refs are not changed.`
		`mmd_pkg_refs = [`
		`mmd.get_rpm_component(pkg_name).get_ref()`
		`for pkg_name in mmd.get_rpm_component_names()`
		`]`
		`- assert set(mmd_pkg_refs) == set(hashes_returned.keys())`
		`+ if srpm_overrides:`
		`+ assert set(mmd_pkg_refs) == {'f27', 'f28', 'bogus'}`
		`+ else:`
		`+ assert set(mmd_pkg_refs) == {'f27', 'f28', 'master'}`
		`deps = mmd.get_dependencies()[0]`
		`assert deps.get_buildtime_modules() == ["platform"]`
		`assert deps.get_buildtime_streams("platform") == ["f28"]`
		`+ match_anything = type('eq_any', (), {"__eq__": lambda left, right: True})()`
		`xmd = {`
		`"mbs": {`
		`"commit": "",`
		`@@ -144,6 +156,8 @@`
		`if scmurl:`
		`xmd["mbs"]["commit"] = "620ec77321b2ea7b0d67d82992dda3e1d67055b4"`
		`xmd["mbs"]["scmurl"] = scmurl`
		`+ if srpm_overrides:`
		`+ xmd["mbs"]["rpms"]["perl-List-Compare"]["ref"] = match_anything`
		`mmd_xmd = mmd.get_xmd()`
		`assert mmd_xmd == xmd`

merlinm commented 4 years ago

When attempting to submit scratch module builds that include custom SRPMs, I have occasionally run into the situation where my modulemd file includes a new package that is not yet published to a distgit repo, or it contains a git 'ref' to a commit in an existing repo that doesn't yet (or no longer) exist. In these cases, the scratch module build promptly fails with a error such as the following:

Reason: Failed to get the latest commit for git+https://src.fedoraproject.org/rpms/cool-new-package#master

For a non-scratch build, it is critical that MBS validates all of the git repos and refs. For a scratch build, however, as long as a corresponding SRPM has been provided, such checks should be skipped. This PR implements the skipping of such checks for rpm components with srpm overrides.

I also started out this PR with a commit that removes a bunch of unused code in tests/test_utils/test_utils.py that was causing me confusion while updating test_format_mmd() to include tests for the revised ref checking.