From 4f09e5c6eb09d96b2ff844b8d678af0b2bc5ce7b Mon Sep 17 00:00:00 2001 From: Anton Bobrov Date: Fri, 7 Aug 2026 18:23:18 +0200 Subject: [PATCH 1/3] Use the same z-stream branch creation logic as rhpkg implementation --- ymir/common/tests/unit/test_utils.py | 47 +++ ymir/common/utils.py | 25 ++ ymir/tools/privileged/distgit.py | 31 +- .../privileged/tests/unit/test_distgit.py | 304 +++++++++++------- 4 files changed, 278 insertions(+), 129 deletions(-) diff --git a/ymir/common/tests/unit/test_utils.py b/ymir/common/tests/unit/test_utils.py index 523420203..650bb83f7 100644 --- a/ymir/common/tests/unit/test_utils.py +++ b/ymir/common/tests/unit/test_utils.py @@ -12,6 +12,7 @@ from ymir.common.utils import ( _is_connection_error, get_latest_candidate_build, + get_latest_z_build, get_latest_z_pending_build, mcp_tools, ) @@ -546,3 +547,49 @@ async def test_get_latest_z_pending_build_no_builds(): ) with pytest.raises(RuntimeError, match="no builds"): await get_latest_z_pending_build("bash", "rhel-9.6.0") + + +# ============================================================================ +# get_latest_z_build +# ============================================================================ + + +@pytest.mark.asyncio +async def test_get_latest_z_build_prefers_pending(): + _mock_koji_session( + { + "rhel-9.6.0-z-pending": [ + {"build_id": 1, "nvr": "bash-1.0-1.el9", "epoch": 0, "version": "1.0", "release": "1.el9"}, + ], + }, + {"source": "git+https://pkgs.example.com/rpms/bash#abc123"}, + ) + evr, ref = await get_latest_z_build("bash", "rhel-9.6.0") + assert evr == EVR(epoch=0, version="1.0", release="1.el9") + assert ref == "abc123" + + +@pytest.mark.asyncio +async def test_get_latest_z_build_falls_back_to_released(): + _mock_koji_session( + { + "rhel-9.6.0-z-pending": [], + "rhel-9.6.0-z": [ + {"build_id": 2, "nvr": "bash-1.0-2.el9", "epoch": 0, "version": "1.0", "release": "2.el9"}, + ], + }, + {"source": "git+https://pkgs.example.com/rpms/bash#def456"}, + ) + evr, ref = await get_latest_z_build("bash", "rhel-9.6.0") + assert evr == EVR(epoch=0, version="1.0", release="2.el9") + assert ref == "def456" + + +@pytest.mark.asyncio +async def test_get_latest_z_build_no_builds(): + _mock_koji_session( + {"rhel-9.6.0-z-pending": [], "rhel-9.6.0-z": []}, + None, + ) + with pytest.raises(RuntimeError, match="no builds"): + await get_latest_z_build("bash", "rhel-9.6.0") diff --git a/ymir/common/utils.py b/ymir/common/utils.py index 5c402b4e2..f7dd4897b 100644 --- a/ymir/common/utils.py +++ b/ymir/common/utils.py @@ -240,6 +240,31 @@ async def get_latest_z_pending_build(package: str, dist_git_branch: str) -> tupl ) +async def get_latest_z_build(package: str, dist_git_branch: str) -> tuple[EVR, str]: + """Look up the latest build for z-stream branch creation. + + Tries ``{branch}-z-pending`` first (builds approved for release but not + yet shipped — equivalent to rhpkg's ``--allow-pending``), then falls back + to ``{branch}-z`` (released builds). Both queries use Koji + ``inherit=True`` so they also see parent tags. + """ + pending_tag = f"{dist_git_branch}-z-pending" + released_tag = f"{dist_git_branch}-z" + build = await asyncio.to_thread(_get_latest_koji_build, BREWHUB_URL, pending_tag, package) + tag_used = pending_tag + if build is None: + build = await asyncio.to_thread(_get_latest_koji_build, BREWHUB_URL, released_tag, package) + tag_used = released_tag + if build is None: + raise RuntimeError(f"There are no builds of {package} in {pending_tag} or {released_tag}") + evr = _evr_from_build(build) + session = koji.ClientSession(BREWHUB_URL) + metadata = await asyncio.to_thread(session.getBuild, build["build_id"], strict=True) + source_ref = metadata["source"].split("#")[-1] + logger.info(f"Found {package} build {build['nvr']} in {tag_used} (commit {source_ref[:12]})") + return evr, source_ref + + def _resolve_buildroot_checks( target_branch: str, fix_version: str, rhel_config: dict | None = None ) -> list[tuple[str, str]]: diff --git a/ymir/tools/privileged/distgit.py b/ymir/tools/privileged/distgit.py index ab1bad636..865696a73 100644 --- a/ymir/tools/privileged/distgit.py +++ b/ymir/tools/privileged/distgit.py @@ -16,8 +16,8 @@ from specfile import Specfile from ymir.common.base_utils import KerberosError, init_kerberos_ticket -from ymir.common.utils import get_latest_candidate_build, get_latest_z_pending_build -from ymir.common.version_utils import is_older_zstream, parse_zstream_branch_name +from ymir.common.utils import get_latest_z_build +from ymir.common.version_utils import parse_zstream_branch_name from ymir.tools.base import CloneableTool as Tool from ymir.tools.privileged.utils import sanitize_url @@ -36,7 +36,6 @@ "no route to host", "broken pipe", "ssh_exchange_identification", - "failed to push some refs", ) _T = TypeVar("_T") @@ -222,24 +221,30 @@ async def _clone(): "skipping push and waiting for mirror sync" ) else: - if await is_older_zstream(branch): - _, ref = await get_latest_z_pending_build(package, branch) - else: - _, ref = await get_latest_candidate_build(package, branch) - if source_branch := self._find_source_branch(repo, branch): + _, ref = await get_latest_z_build(package, branch) + source_branch = self._find_source_branch(repo, branch) + if source_branch and source_branch.endswith("-main"): ref = await self._find_latest_same_nvr_ref( repo, package, ref, source_branch, ) - push_infos = await _retry_transient( - lambda: asyncio.to_thread(repo.remotes.origin.push, f"{ref}:refs/heads/{branch}"), + try: + await asyncio.to_thread(repo.commit, ref) + except Exception: + raise RuntimeError( + f"Commit {ref} (from latest Brew build) not found in dist-git clone of {package}" + ) from None + await _retry_transient( + lambda: asyncio.to_thread(repo.git.push, "origin", f"{ref}:refs/heads/{branch}"), f"push {branch} to dist-git", ) - for info in push_infos: - if info.flags & git.remote.PushInfo.ERROR: - raise RuntimeError(f"Push rejected: {info.summary.strip()}") + if not await asyncio.to_thread(repo.git.ls_remote, "--heads", "origin", branch): + raise RuntimeError( + f"Push appeared to succeed but branch {branch} not found " + f"on dist-git — possible silent rejection by server ACL" + ) start_time = time.monotonic() while time.monotonic() - start_time < SYNC_TIMEOUT: try: diff --git a/ymir/tools/privileged/tests/unit/test_distgit.py b/ymir/tools/privileged/tests/unit/test_distgit.py index bb02b8626..dcdcc44a2 100644 --- a/ymir/tools/privileged/tests/unit/test_distgit.py +++ b/ymir/tools/privileged/tests/unit/test_distgit.py @@ -12,13 +12,6 @@ ) -def _mock_is_older_zstream(result): - async def _mock(branch): - return result - - return _mock - - @pytest.mark.parametrize( "branch_exists", [False, True], @@ -38,30 +31,24 @@ async def init_kerberos_ticket(): gitcmd = flexmock().should_receive("ls_remote").and_return(branch_exists).and_return(True).mock() flexmock(git.cmd.Git).new_instances(gitcmd) - flexmock(git.Repo).should_receive("clone_from").and_return( - flexmock( - git=gitcmd, - remotes=flexmock( - origin=flexmock(refs=[]) - .should_receive("push") - .with_args(f"{ref}:refs/heads/{branch}") - .times(0 if branch_exists else 1) - .and_return([]) - .mock(), - ), + gitcmd.should_receive("push").with_args("origin", f"{ref}:refs/heads/{branch}").times( + 0 if branch_exists else 1 + ).and_return("") + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[]), ), ) + mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).times(0 if branch_exists else 1) + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) - flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( - _mock_is_older_zstream(False) - ).times(0 if branch_exists else 1) - - async def mock_get_latest_candidate_build(package, dist_git_branch): + async def mock_get_latest_z_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), ref - flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( - mock_get_latest_candidate_build - ).times(0 if branch_exists else 1) + flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).times( + 0 if branch_exists else 1 + ) monkeypatch.setenv("GITLAB_TOKEN", "") @@ -108,8 +95,8 @@ async def init_kerberos_ticket(): @pytest.mark.asyncio -async def test_create_zstream_branch_push_rejected(monkeypatch): - """Push is silently rejected by gitolite — ToolError must be raised immediately.""" +async def test_create_zstream_branch_push_silently_rejected(monkeypatch): + """Push exits 0 but branch doesn't appear on dist-git — silent rejection by server ACL.""" package = "bash" branch = "rhel-10.0" user = "bot" @@ -120,38 +107,27 @@ async def init_kerberos_ticket(): flexmock(distgit_tools).should_receive("init_kerberos_ticket").replace_with(init_kerberos_ticket).once() - gitcmd = flexmock().should_receive("ls_remote").and_return(False).mock() + gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return("").mock() + gitcmd.should_receive("push").with_args("origin", f"{ref}:refs/heads/{branch}").once().and_return("") flexmock(git.cmd.Git).new_instances(gitcmd) - mock_push_info = flexmock(flags=git.remote.PushInfo.ERROR, summary="access denied") - flexmock(git.Repo).should_receive("clone_from").and_return( - flexmock( - git=gitcmd, - remotes=flexmock( - origin=flexmock(refs=[]) - .should_receive("push") - .with_args(f"{ref}:refs/heads/{branch}") - .once() - .and_return([mock_push_info]) - .mock(), - ), + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[]), ), ) + mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).once() + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) - flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( - _mock_is_older_zstream(False) - ).once() - - async def mock_get_latest_candidate_build(package, dist_git_branch): + async def mock_get_latest_z_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), ref - flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( - mock_get_latest_candidate_build - ).once() + flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).once() monkeypatch.setenv("GITLAB_TOKEN", "") - with pytest.raises(ToolError, match="Push rejected"): + with pytest.raises(ToolError, match="not found on dist-git"): await CreateZstreamBranchTool().run(input={"package": package, "branch": branch}) @@ -161,7 +137,7 @@ async def mock_get_latest_candidate_build(package, dist_git_branch): ("Connection closed by 10.2.32.39 port 22\nfatal: Could not read from remote repository.", True), ("Connection reset by peer", True), ("ssh_exchange_identification: Connection closed by remote host", True), - ("error: failed to push some refs to 'ssh://pkgs.devel.redhat.com/rpms/ruby'", True), + ("error: failed to push some refs to 'ssh://pkgs.devel.redhat.com/rpms/ruby'", False), ("Permission denied (publickey)", False), ("fatal: Authentication failed for 'https://example.com/'", False), ("fatal: Could not read from remote repository.", False), @@ -225,10 +201,10 @@ def test_find_source_branch(branch, remote_branches, expected): @pytest.mark.asyncio -async def test_create_zstream_branch_advances_ref(monkeypatch): - """When a source branch exists, the ref is advanced to the latest same-NVR commit.""" +async def test_create_zstream_branch_advances_ref_on_main(monkeypatch): + """NVR walk advances the ref when the source branch is rhel-X-main.""" package = "bash" - branch = "rhel-10.0" + branch = "rhel-10.2" user = "bot" build_ref = "aaa111" # pragma: allowlist secret advanced_ref = "bbb222" # pragma: allowlist secret @@ -241,36 +217,28 @@ async def init_kerberos_ticket(): gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return(True).mock() flexmock(git.cmd.Git).new_instances(gitcmd) - flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( - _mock_is_older_zstream(False) - ).once() - - async def mock_get_latest_candidate_build(package, dist_git_branch): + async def mock_get_latest_z_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), build_ref - flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( - mock_get_latest_candidate_build - ).once() + flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).once() - mock_higher_ref = flexmock(name="origin/rhel-10.1") - flexmock(git.Repo).should_receive("clone_from").and_return( - flexmock( - git=gitcmd, - remotes=flexmock( - origin=flexmock(refs=[mock_higher_ref]) - .should_receive("push") - .with_args(f"{advanced_ref}:refs/heads/{branch}") - .once() - .and_return([]) - .mock(), - ), + mock_main_ref = flexmock(name="origin/rhel-10-main") + gitcmd.should_receive("push").with_args( + "origin", f"{advanced_ref}:refs/heads/{branch}" + ).once().and_return("") + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[mock_main_ref]), ), ) + mock_repo.should_receive("commit").with_args(advanced_ref).and_return(flexmock()).once() + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) async def mock_find_latest_same_nvr_ref(repo, pkg, ref, source): assert pkg == package assert ref == build_ref - assert source == "rhel-10.1" + assert source == "rhel-10-main" return advanced_ref flexmock(CreateZstreamBranchTool).should_receive("_find_latest_same_nvr_ref").replace_with( @@ -283,6 +251,50 @@ async def mock_find_latest_same_nvr_ref(repo, pkg, ref, source): assert result.startswith("Successfully") +@pytest.mark.asyncio +async def test_create_zstream_branch_skips_nvr_walk_on_zstream_source(monkeypatch): + """NVR walk is skipped when source branch is a z-stream branch (not -main).""" + package = "isns-utils" + branch = "rhel-10.0" + user = "bot" + build_ref = "a3276f38" # pragma: allowlist secret + + async def init_kerberos_ticket(): + return f"{user}@EXAMPLE.COM" + + flexmock(distgit_tools).should_receive("init_kerberos_ticket").replace_with(init_kerberos_ticket).once() + + gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return(True).mock() + flexmock(git.cmd.Git).new_instances(gitcmd) + + async def mock_get_latest_z_build(package, dist_git_branch): + return EVR(version="0.103", release="1.el10"), build_ref + + flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).once() + + # Source branch is rhel-10.2 (a z-stream, not -main) → NVR walk must be skipped + mock_higher_ref = flexmock(name="origin/rhel-10.2") + gitcmd.should_receive("push").with_args("origin", f"{build_ref}:refs/heads/{branch}").once().and_return( + "" + ) + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[mock_higher_ref]), + ), + ) + mock_repo.should_receive("commit").with_args(build_ref).and_return(flexmock()).once() + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) + + # _find_latest_same_nvr_ref must NOT be called + flexmock(CreateZstreamBranchTool).should_receive("_find_latest_same_nvr_ref").times(0) + + monkeypatch.setenv("GITLAB_TOKEN", "") + + result = (await CreateZstreamBranchTool().run(input={"package": package, "branch": branch})).result + assert result.startswith("Successfully") + + def _mock_spec_commit(hexsha, spec_content): """Create a mock commit whose tree contains a spec file with given content.""" @@ -375,31 +387,21 @@ async def init_kerberos_ticket(): gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return(True).mock() flexmock(git.cmd.Git).new_instances(gitcmd) - flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( - _mock_is_older_zstream(False) - ).once() - - async def mock_get_latest_candidate_build(package, dist_git_branch): + async def mock_get_latest_z_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), ref - flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( - mock_get_latest_candidate_build - ).once() + flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).once() # No higher branches and no rhel-X-main - flexmock(git.Repo).should_receive("clone_from").and_return( - flexmock( - git=gitcmd, - remotes=flexmock( - origin=flexmock(refs=[]) - .should_receive("push") - .with_args(f"{ref}:refs/heads/{branch}") - .once() - .and_return([]) - .mock(), - ), + gitcmd.should_receive("push").with_args("origin", f"{ref}:refs/heads/{branch}").once().and_return("") + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[]), ), ) + mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).once() + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) monkeypatch.setenv("GITLAB_TOKEN", "") @@ -408,8 +410,8 @@ async def mock_get_latest_candidate_build(package, dist_git_branch): @pytest.mark.asyncio -async def test_create_zstream_branch_older_zstream_uses_z_pending(monkeypatch): - """Older z-stream branches use get_latest_z_pending_build instead of candidate.""" +async def test_create_zstream_branch_older_zstream_uses_z_tag(monkeypatch): + """Older z-stream branches use get_latest_z_build (the -z tag), same as current ones.""" package = "bash" branch = "rhel-9.6.0" user = "bot" @@ -423,33 +425,103 @@ async def init_kerberos_ticket(): gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return(True).mock() flexmock(git.cmd.Git).new_instances(gitcmd) - flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( - _mock_is_older_zstream(True) - ).once() - flexmock(distgit_tools).should_receive("get_latest_candidate_build").never() - - async def mock_get_latest_z_pending_build(package, dist_git_branch): + async def mock_get_latest_z_build(package, dist_git_branch): return EVR(version="1.0", release="1.el9"), ref - flexmock(distgit_tools).should_receive("get_latest_z_pending_build").replace_with( - mock_get_latest_z_pending_build - ).once() + flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).once() - flexmock(git.Repo).should_receive("clone_from").and_return( - flexmock( - git=gitcmd, - remotes=flexmock( - origin=flexmock(refs=[]) - .should_receive("push") - .with_args(f"{ref}:refs/heads/{branch}") - .once() - .and_return([]) - .mock(), - ), + gitcmd.should_receive("push").with_args("origin", f"{ref}:refs/heads/{branch}").once().and_return("") + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[]), ), ) + mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).once() + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) monkeypatch.setenv("GITLAB_TOKEN", "") result = (await CreateZstreamBranchTool().run(input={"package": package, "branch": branch})).result assert result.startswith("Successfully") + + +@pytest.mark.asyncio +async def test_create_zstream_branch_commit_not_in_clone(monkeypatch): + """ToolError with clear message when the Brew build commit is missing from the dist-git clone.""" + package = "bash" + branch = "rhel-10.0" + user = "bot" + ref = "deadbeef1234" # pragma: allowlist secret + + async def init_kerberos_ticket(): + return f"{user}@EXAMPLE.COM" + + flexmock(distgit_tools).should_receive("init_kerberos_ticket").replace_with(init_kerberos_ticket).once() + + gitcmd = flexmock().should_receive("ls_remote").and_return(False).mock() + flexmock(git.cmd.Git).new_instances(gitcmd) + + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[]).should_receive("push").times(0).mock(), + ), + ) + mock_repo.should_receive("commit").with_args(ref).and_raise( + git.exc.GitCommandError(["git", "rev-parse"], status=128, stderr="bad object") + ).once() + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) + + async def mock_get_latest_z_build(package, dist_git_branch): + return EVR(version="1.0", release="1.el10"), ref + + flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).once() + + monkeypatch.setenv("GITLAB_TOKEN", "") + + with pytest.raises(ToolError, match="not found in dist-git clone"): + await CreateZstreamBranchTool().run(input={"package": package, "branch": branch}) + + +@pytest.mark.asyncio +async def test_create_zstream_branch_push_hook_rejection(monkeypatch): + """Push rejected by server hook — full stderr surfaces in ToolError.""" + package = "bash" + branch = "rhel-10.0" + user = "bot" + ref = "123456abcdef" # pragma: allowlist secret + + async def init_kerberos_ticket(): + return f"{user}@EXAMPLE.COM" + + flexmock(distgit_tools).should_receive("init_kerberos_ticket").replace_with(init_kerberos_ticket).once() + + gitcmd = flexmock().should_receive("ls_remote").and_return(False).mock() + gitcmd.should_receive("push").with_args("origin", f"{ref}:refs/heads/{branch}").once().and_raise( + git.exc.GitCommandError( + ["git", "push"], + status=1, + stderr="remote: error: hook declined to update refs/heads/rhel-10.0", + ) + ) + flexmock(git.cmd.Git).new_instances(gitcmd) + + mock_repo = flexmock( + git=gitcmd, + remotes=flexmock( + origin=flexmock(refs=[]), + ), + ) + mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).once() + flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) + + async def mock_get_latest_z_build(package, dist_git_branch): + return EVR(version="1.0", release="1.el10"), ref + + flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).once() + + monkeypatch.setenv("GITLAB_TOKEN", "") + + with pytest.raises(ToolError, match="hook declined"): + await CreateZstreamBranchTool().run(input={"package": package, "branch": branch}) From 9ef66dbda9afb12ced86aa69d990bdfa38adc09b Mon Sep 17 00:00:00 2001 From: Anton Bobrov Date: Mon, 10 Aug 2026 12:53:47 +0200 Subject: [PATCH 2/3] Revert leading z-stream changes and only apply to older z-streams --- ymir/common/tests/unit/test_utils.py | 47 ----------- ymir/common/utils.py | 25 ------ ymir/tools/privileged/distgit.py | 14 +++- .../privileged/tests/unit/test_distgit.py | 84 +++++++++++++++---- 4 files changed, 76 insertions(+), 94 deletions(-) diff --git a/ymir/common/tests/unit/test_utils.py b/ymir/common/tests/unit/test_utils.py index 650bb83f7..523420203 100644 --- a/ymir/common/tests/unit/test_utils.py +++ b/ymir/common/tests/unit/test_utils.py @@ -12,7 +12,6 @@ from ymir.common.utils import ( _is_connection_error, get_latest_candidate_build, - get_latest_z_build, get_latest_z_pending_build, mcp_tools, ) @@ -547,49 +546,3 @@ async def test_get_latest_z_pending_build_no_builds(): ) with pytest.raises(RuntimeError, match="no builds"): await get_latest_z_pending_build("bash", "rhel-9.6.0") - - -# ============================================================================ -# get_latest_z_build -# ============================================================================ - - -@pytest.mark.asyncio -async def test_get_latest_z_build_prefers_pending(): - _mock_koji_session( - { - "rhel-9.6.0-z-pending": [ - {"build_id": 1, "nvr": "bash-1.0-1.el9", "epoch": 0, "version": "1.0", "release": "1.el9"}, - ], - }, - {"source": "git+https://pkgs.example.com/rpms/bash#abc123"}, - ) - evr, ref = await get_latest_z_build("bash", "rhel-9.6.0") - assert evr == EVR(epoch=0, version="1.0", release="1.el9") - assert ref == "abc123" - - -@pytest.mark.asyncio -async def test_get_latest_z_build_falls_back_to_released(): - _mock_koji_session( - { - "rhel-9.6.0-z-pending": [], - "rhel-9.6.0-z": [ - {"build_id": 2, "nvr": "bash-1.0-2.el9", "epoch": 0, "version": "1.0", "release": "2.el9"}, - ], - }, - {"source": "git+https://pkgs.example.com/rpms/bash#def456"}, - ) - evr, ref = await get_latest_z_build("bash", "rhel-9.6.0") - assert evr == EVR(epoch=0, version="1.0", release="2.el9") - assert ref == "def456" - - -@pytest.mark.asyncio -async def test_get_latest_z_build_no_builds(): - _mock_koji_session( - {"rhel-9.6.0-z-pending": [], "rhel-9.6.0-z": []}, - None, - ) - with pytest.raises(RuntimeError, match="no builds"): - await get_latest_z_build("bash", "rhel-9.6.0") diff --git a/ymir/common/utils.py b/ymir/common/utils.py index f7dd4897b..5c402b4e2 100644 --- a/ymir/common/utils.py +++ b/ymir/common/utils.py @@ -240,31 +240,6 @@ async def get_latest_z_pending_build(package: str, dist_git_branch: str) -> tupl ) -async def get_latest_z_build(package: str, dist_git_branch: str) -> tuple[EVR, str]: - """Look up the latest build for z-stream branch creation. - - Tries ``{branch}-z-pending`` first (builds approved for release but not - yet shipped — equivalent to rhpkg's ``--allow-pending``), then falls back - to ``{branch}-z`` (released builds). Both queries use Koji - ``inherit=True`` so they also see parent tags. - """ - pending_tag = f"{dist_git_branch}-z-pending" - released_tag = f"{dist_git_branch}-z" - build = await asyncio.to_thread(_get_latest_koji_build, BREWHUB_URL, pending_tag, package) - tag_used = pending_tag - if build is None: - build = await asyncio.to_thread(_get_latest_koji_build, BREWHUB_URL, released_tag, package) - tag_used = released_tag - if build is None: - raise RuntimeError(f"There are no builds of {package} in {pending_tag} or {released_tag}") - evr = _evr_from_build(build) - session = koji.ClientSession(BREWHUB_URL) - metadata = await asyncio.to_thread(session.getBuild, build["build_id"], strict=True) - source_ref = metadata["source"].split("#")[-1] - logger.info(f"Found {package} build {build['nvr']} in {tag_used} (commit {source_ref[:12]})") - return evr, source_ref - - def _resolve_buildroot_checks( target_branch: str, fix_version: str, rhel_config: dict | None = None ) -> list[tuple[str, str]]: diff --git a/ymir/tools/privileged/distgit.py b/ymir/tools/privileged/distgit.py index 865696a73..f1e00d775 100644 --- a/ymir/tools/privileged/distgit.py +++ b/ymir/tools/privileged/distgit.py @@ -16,8 +16,8 @@ from specfile import Specfile from ymir.common.base_utils import KerberosError, init_kerberos_ticket -from ymir.common.utils import get_latest_z_build -from ymir.common.version_utils import parse_zstream_branch_name +from ymir.common.utils import get_latest_candidate_build, get_latest_z_pending_build +from ymir.common.version_utils import is_older_zstream, parse_zstream_branch_name from ymir.tools.base import CloneableTool as Tool from ymir.tools.privileged.utils import sanitize_url @@ -221,7 +221,10 @@ async def _clone(): "skipping push and waiting for mirror sync" ) else: - _, ref = await get_latest_z_build(package, branch) + if await is_older_zstream(branch): + _, ref = await get_latest_z_pending_build(package, branch) + else: + _, ref = await get_latest_candidate_build(package, branch) source_branch = self._find_source_branch(repo, branch) if source_branch and source_branch.endswith("-main"): ref = await self._find_latest_same_nvr_ref( @@ -240,7 +243,10 @@ async def _clone(): lambda: asyncio.to_thread(repo.git.push, "origin", f"{ref}:refs/heads/{branch}"), f"push {branch} to dist-git", ) - if not await asyncio.to_thread(repo.git.ls_remote, "--heads", "origin", branch): + if not await _retry_transient( + lambda: asyncio.to_thread(repo.git.ls_remote, "--heads", "origin", branch), + f"verify {branch} on dist-git", + ): raise RuntimeError( f"Push appeared to succeed but branch {branch} not found " f"on dist-git — possible silent rejection by server ACL" diff --git a/ymir/tools/privileged/tests/unit/test_distgit.py b/ymir/tools/privileged/tests/unit/test_distgit.py index dcdcc44a2..ea164c558 100644 --- a/ymir/tools/privileged/tests/unit/test_distgit.py +++ b/ymir/tools/privileged/tests/unit/test_distgit.py @@ -43,12 +43,18 @@ async def init_kerberos_ticket(): mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).times(0 if branch_exists else 1) flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) - async def mock_get_latest_z_build(package, dist_git_branch): + async def mock_is_older_zstream(branch): + return False + + async def mock_get_latest_candidate_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), ref - flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).times( + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).times( 0 if branch_exists else 1 ) + flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( + mock_get_latest_candidate_build + ).times(0 if branch_exists else 1) monkeypatch.setenv("GITLAB_TOKEN", "") @@ -120,10 +126,16 @@ async def init_kerberos_ticket(): mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).once() flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) - async def mock_get_latest_z_build(package, dist_git_branch): + async def mock_is_older_zstream(branch): + return False + + async def mock_get_latest_candidate_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), ref - flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).once() + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).once() + flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( + mock_get_latest_candidate_build + ).once() monkeypatch.setenv("GITLAB_TOKEN", "") @@ -217,10 +229,16 @@ async def init_kerberos_ticket(): gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return(True).mock() flexmock(git.cmd.Git).new_instances(gitcmd) - async def mock_get_latest_z_build(package, dist_git_branch): + async def mock_is_older_zstream(branch): + return False + + async def mock_get_latest_candidate_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), build_ref - flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).once() + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).once() + flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( + mock_get_latest_candidate_build + ).once() mock_main_ref = flexmock(name="origin/rhel-10-main") gitcmd.should_receive("push").with_args( @@ -267,10 +285,16 @@ async def init_kerberos_ticket(): gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return(True).mock() flexmock(git.cmd.Git).new_instances(gitcmd) - async def mock_get_latest_z_build(package, dist_git_branch): + async def mock_is_older_zstream(branch): + return False + + async def mock_get_latest_candidate_build(package, dist_git_branch): return EVR(version="0.103", release="1.el10"), build_ref - flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).once() + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).once() + flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( + mock_get_latest_candidate_build + ).once() # Source branch is rhel-10.2 (a z-stream, not -main) → NVR walk must be skipped mock_higher_ref = flexmock(name="origin/rhel-10.2") @@ -387,10 +411,16 @@ async def init_kerberos_ticket(): gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return(True).mock() flexmock(git.cmd.Git).new_instances(gitcmd) - async def mock_get_latest_z_build(package, dist_git_branch): + async def mock_is_older_zstream(branch): + return False + + async def mock_get_latest_candidate_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), ref - flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).once() + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).once() + flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( + mock_get_latest_candidate_build + ).once() # No higher branches and no rhel-X-main gitcmd.should_receive("push").with_args("origin", f"{ref}:refs/heads/{branch}").once().and_return("") @@ -410,8 +440,8 @@ async def mock_get_latest_z_build(package, dist_git_branch): @pytest.mark.asyncio -async def test_create_zstream_branch_older_zstream_uses_z_tag(monkeypatch): - """Older z-stream branches use get_latest_z_build (the -z tag), same as current ones.""" +async def test_create_zstream_branch_older_zstream_uses_z_pending(monkeypatch): + """Older z-stream branches use get_latest_z_pending_build (the -z-pending tag).""" package = "bash" branch = "rhel-9.6.0" user = "bot" @@ -425,10 +455,16 @@ async def init_kerberos_ticket(): gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return(True).mock() flexmock(git.cmd.Git).new_instances(gitcmd) - async def mock_get_latest_z_build(package, dist_git_branch): + async def mock_is_older_zstream(branch): + return True + + async def mock_get_latest_z_pending_build(package, dist_git_branch): return EVR(version="1.0", release="1.el9"), ref - flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).once() + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).once() + flexmock(distgit_tools).should_receive("get_latest_z_pending_build").replace_with( + mock_get_latest_z_pending_build + ).once() gitcmd.should_receive("push").with_args("origin", f"{ref}:refs/heads/{branch}").once().and_return("") mock_repo = flexmock( @@ -473,10 +509,16 @@ async def init_kerberos_ticket(): ).once() flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) - async def mock_get_latest_z_build(package, dist_git_branch): + async def mock_is_older_zstream(branch): + return False + + async def mock_get_latest_candidate_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), ref - flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).once() + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).once() + flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( + mock_get_latest_candidate_build + ).once() monkeypatch.setenv("GITLAB_TOKEN", "") @@ -516,10 +558,16 @@ async def init_kerberos_ticket(): mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).once() flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) - async def mock_get_latest_z_build(package, dist_git_branch): + async def mock_is_older_zstream(branch): + return False + + async def mock_get_latest_candidate_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), ref - flexmock(distgit_tools).should_receive("get_latest_z_build").replace_with(mock_get_latest_z_build).once() + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).once() + flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( + mock_get_latest_candidate_build + ).once() monkeypatch.setenv("GITLAB_TOKEN", "") From 685d0ad37354db5fddd8ee315c812d73bfa76cd2 Mon Sep 17 00:00:00 2001 From: Anton Bobrov Date: Tue, 11 Aug 2026 10:57:57 +0200 Subject: [PATCH 3/3] revert unnecessary test changes --- .../privileged/tests/unit/test_distgit.py | 60 ++++++++++--------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/ymir/tools/privileged/tests/unit/test_distgit.py b/ymir/tools/privileged/tests/unit/test_distgit.py index ea164c558..6b7d34837 100644 --- a/ymir/tools/privileged/tests/unit/test_distgit.py +++ b/ymir/tools/privileged/tests/unit/test_distgit.py @@ -12,6 +12,13 @@ ) +def _mock_is_older_zstream(result): + async def _mock(branch): + return result + + return _mock + + @pytest.mark.parametrize( "branch_exists", [False, True], @@ -43,15 +50,13 @@ async def init_kerberos_ticket(): mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).times(0 if branch_exists else 1) flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) - async def mock_is_older_zstream(branch): - return False + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( + _mock_is_older_zstream(False) + ).times(0 if branch_exists else 1) async def mock_get_latest_candidate_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), ref - flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).times( - 0 if branch_exists else 1 - ) flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( mock_get_latest_candidate_build ).times(0 if branch_exists else 1) @@ -126,13 +131,13 @@ async def init_kerberos_ticket(): mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).once() flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) - async def mock_is_older_zstream(branch): - return False + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( + _mock_is_older_zstream(False) + ).once() async def mock_get_latest_candidate_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), ref - flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).once() flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( mock_get_latest_candidate_build ).once() @@ -229,13 +234,13 @@ async def init_kerberos_ticket(): gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return(True).mock() flexmock(git.cmd.Git).new_instances(gitcmd) - async def mock_is_older_zstream(branch): - return False + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( + _mock_is_older_zstream(False) + ).once() async def mock_get_latest_candidate_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), build_ref - flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).once() flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( mock_get_latest_candidate_build ).once() @@ -285,13 +290,13 @@ async def init_kerberos_ticket(): gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return(True).mock() flexmock(git.cmd.Git).new_instances(gitcmd) - async def mock_is_older_zstream(branch): - return False + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( + _mock_is_older_zstream(False) + ).once() async def mock_get_latest_candidate_build(package, dist_git_branch): return EVR(version="0.103", release="1.el10"), build_ref - flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).once() flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( mock_get_latest_candidate_build ).once() @@ -411,13 +416,13 @@ async def init_kerberos_ticket(): gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return(True).mock() flexmock(git.cmd.Git).new_instances(gitcmd) - async def mock_is_older_zstream(branch): - return False + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( + _mock_is_older_zstream(False) + ).once() async def mock_get_latest_candidate_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), ref - flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).once() flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( mock_get_latest_candidate_build ).once() @@ -441,7 +446,7 @@ async def mock_get_latest_candidate_build(package, dist_git_branch): @pytest.mark.asyncio async def test_create_zstream_branch_older_zstream_uses_z_pending(monkeypatch): - """Older z-stream branches use get_latest_z_pending_build (the -z-pending tag).""" + """Older z-stream branches use get_latest_z_pending_build instead of candidate.""" package = "bash" branch = "rhel-9.6.0" user = "bot" @@ -455,13 +460,14 @@ async def init_kerberos_ticket(): gitcmd = flexmock().should_receive("ls_remote").and_return(False).and_return(True).mock() flexmock(git.cmd.Git).new_instances(gitcmd) - async def mock_is_older_zstream(branch): - return True + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( + _mock_is_older_zstream(True) + ).once() + flexmock(distgit_tools).should_receive("get_latest_candidate_build").never() async def mock_get_latest_z_pending_build(package, dist_git_branch): return EVR(version="1.0", release="1.el9"), ref - flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).once() flexmock(distgit_tools).should_receive("get_latest_z_pending_build").replace_with( mock_get_latest_z_pending_build ).once() @@ -509,13 +515,13 @@ async def init_kerberos_ticket(): ).once() flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) - async def mock_is_older_zstream(branch): - return False + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( + _mock_is_older_zstream(False) + ).once() async def mock_get_latest_candidate_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), ref - flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).once() flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( mock_get_latest_candidate_build ).once() @@ -558,13 +564,13 @@ async def init_kerberos_ticket(): mock_repo.should_receive("commit").with_args(ref).and_return(flexmock()).once() flexmock(git.Repo).should_receive("clone_from").and_return(mock_repo) - async def mock_is_older_zstream(branch): - return False + flexmock(distgit_tools).should_receive("is_older_zstream").replace_with( + _mock_is_older_zstream(False) + ).once() async def mock_get_latest_candidate_build(package, dist_git_branch): return EVR(version="1.0", release="1.el10"), ref - flexmock(distgit_tools).should_receive("is_older_zstream").replace_with(mock_is_older_zstream).once() flexmock(distgit_tools).should_receive("get_latest_candidate_build").replace_with( mock_get_latest_candidate_build ).once()