[CI] Merge the target branch into stale PRs on /ci run (#61592)
Signed-off-by: khluu <khluu000@gmail.com> Co-authored-by: Misha Goin <mgoin64@gmail.com>
This commit is contained in:
co-authored by
Misha Goin
parent
bb1438a93f
commit
d36158f919
@@ -39,6 +39,7 @@ jobs:
|
||||
BUILDKITE_ORGANIZATION: vllm
|
||||
BUILDKITE_PIPELINE: ci
|
||||
CI_TRUSTED_USERS: ${{ vars.CI_TRUSTED_USERS }}
|
||||
GH_BRANCH_UPDATE_TOKEN: ${{ secrets.CI_BRANCH_UPDATE_TOKEN }}
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
- name: Report failure on the PR
|
||||
if: failure()
|
||||
|
||||
@@ -62,6 +62,8 @@ CI_COMMAND_PREFIXES = frozenset({"/ci", "/amd-ci"})
|
||||
CI_AUTHORIZED_COMMENT_MARKER = "<!-- vllm-ci-authorized -->"
|
||||
READY_LABELS = {"ready", "ready-run-all-tests"}
|
||||
TRUSTED_PERMISSIONS = {"admin", "maintain", "write"}
|
||||
BRANCH_UPDATE_POLL_SECONDS = 3
|
||||
BRANCH_UPDATE_POLL_ATTEMPTS = 40
|
||||
ACTIVE_BUILD_STATES = {
|
||||
"blocked",
|
||||
"creating",
|
||||
@@ -259,6 +261,13 @@ class GitHubClient:
|
||||
raise ApiError(None, "GitHub returned an invalid commits-behind count.")
|
||||
return behind
|
||||
|
||||
def update_branch(self, number: int, expected_head_sha: str) -> None:
|
||||
self._request(
|
||||
self._repo_path(f"/pulls/{number}/update-branch"),
|
||||
body={"expected_head_sha": expected_head_sha},
|
||||
method="PUT",
|
||||
)
|
||||
|
||||
def list_pulls_for_commit(self, commit: str) -> list[dict[str, Any]]:
|
||||
commit = urllib.parse.quote(commit, safe="")
|
||||
response = self._request(self._repo_path(f"/commits/{commit}/pulls"))
|
||||
@@ -778,10 +787,11 @@ def notify_authorized(
|
||||
(
|
||||
f"✅ @{author}, CI is now available for this PR.\n\n"
|
||||
"- `/ci run` starts upstream CI; `/amd-ci run` starts AMD CI only.\n"
|
||||
"- Your branch must contain every commit currently on its upstream "
|
||||
"target branch. Merge or rebase onto the latest target branch, then "
|
||||
"rerun the command. Append `--allow-stale` to a run command to test "
|
||||
"an outdated branch at your own risk.\n"
|
||||
"- Run commands test your branch with every commit currently on its "
|
||||
"upstream target branch. If it is behind, the bot merges the target "
|
||||
"branch into it first (pull before pushing again). Append "
|
||||
"`--allow-stale` to a run command to test an outdated branch as is, "
|
||||
"at your own risk.\n"
|
||||
"- `/ci retry` retries failed jobs in the CI build for the current "
|
||||
"PR head. If the current head has no CI build, it starts a new CI "
|
||||
"build for the current head containing only jobs that failed in "
|
||||
@@ -829,10 +839,55 @@ def resolve_workflow_run_pr(
|
||||
return find_matching_pr(github.list_pulls_for_commit(head_sha))
|
||||
|
||||
|
||||
def merge_base_into_pr(
|
||||
github: GitHubClient,
|
||||
branch_updater: GitHubClient,
|
||||
pr: Mapping[str, Any],
|
||||
command: str,
|
||||
lag: str,
|
||||
) -> tuple[dict[str, Any], str]:
|
||||
base_ref = pr["base"]["ref"]
|
||||
|
||||
def merge_failed(reason: str) -> CiPreparationError:
|
||||
return CiPreparationError(
|
||||
f"{lag} The bot could not merge upstream `{base_ref}` into this "
|
||||
f"branch ({reason}). No new CI build was started. Either merge or "
|
||||
f"rebase onto the latest `{base_ref}` yourself and rerun `{command}`, "
|
||||
f"or use `{command}{ALLOW_STALE_SUFFIX}` to test this branch as is."
|
||||
)
|
||||
|
||||
try:
|
||||
branch_updater.update_branch(pr["number"], pr["head"]["sha"])
|
||||
for _ in range(BRANCH_UPDATE_POLL_ATTEMPTS):
|
||||
time.sleep(BRANCH_UPDATE_POLL_SECONDS)
|
||||
current_pr = github.get_pr(pr["number"])
|
||||
if current_pr["head"]["sha"] != pr["head"]["sha"]:
|
||||
break
|
||||
else:
|
||||
raise merge_failed("the merge commit did not appear in time")
|
||||
if (
|
||||
current_pr["state"] != "open"
|
||||
or current_pr["base"]["ref"] != base_ref
|
||||
or github.get_commits_behind_base(base_ref, current_pr["head"]["sha"])
|
||||
):
|
||||
raise merge_failed("the PR changed during the merge")
|
||||
except ApiError as error:
|
||||
raise merge_failed(str(error)) from error
|
||||
|
||||
note = (
|
||||
f"ℹ️ {lag} Merged upstream `{base_ref}` into this branch as "
|
||||
f"`{current_pr['head']['sha'][:12]}` before starting CI. "
|
||||
"Pull the branch before pushing more changes."
|
||||
)
|
||||
print(note)
|
||||
return current_pr, note
|
||||
|
||||
|
||||
def prepare_pr_for_ci(
|
||||
github: GitHubClient,
|
||||
pr: Mapping[str, Any],
|
||||
command: str,
|
||||
branch_updater: GitHubClient | None = None,
|
||||
) -> tuple[dict[str, Any], str]:
|
||||
base_ref = pr["base"]["ref"]
|
||||
no_action = "No new CI build was started."
|
||||
@@ -868,6 +923,8 @@ def prepare_pr_for_ci(
|
||||
print(warning)
|
||||
return current_pr, warning
|
||||
|
||||
if behind and branch_updater is not None:
|
||||
return merge_base_into_pr(github, branch_updater, current_pr, command, lag)
|
||||
if behind:
|
||||
raise CiPreparationError(
|
||||
f"{lag} Your branch must contain every commit currently on upstream "
|
||||
@@ -887,6 +944,8 @@ def handle_run_ci(
|
||||
command: str,
|
||||
github: GitHubClient,
|
||||
pr: Mapping[str, Any],
|
||||
branch_updater: GitHubClient | None = None,
|
||||
allow_repeat_run: bool = False,
|
||||
) -> str:
|
||||
ci_name = ci_name_for_command(command)
|
||||
duplicate_builds = buildkite.list_builds(
|
||||
@@ -913,7 +972,19 @@ def handle_run_ci(
|
||||
f"{ci_name} is already running for this commit: {active_build['web_url']}"
|
||||
)
|
||||
|
||||
current_pr, warning = prepare_pr_for_ci(github, pr, command)
|
||||
tested_build = select_latest_build(
|
||||
[build for build in current_builds if build.get("state") != "canceled"],
|
||||
pr["number"],
|
||||
)
|
||||
if tested_build and not allow_repeat_run:
|
||||
raise CiPreparationError(
|
||||
f"{ci_name} already ran on this commit: {tested_build['web_url']}. "
|
||||
"No new CI build was started. Push new changes to run CI again, use "
|
||||
f"`{retry_command_for_command(command)}` to rerun failed jobs, or ask "
|
||||
"a reviewer with write access to rerun it."
|
||||
)
|
||||
|
||||
current_pr, warning = prepare_pr_for_ci(github, pr, command, branch_updater)
|
||||
|
||||
build = buildkite.create_build(
|
||||
create_build_payload(
|
||||
@@ -1120,6 +1191,7 @@ def run(
|
||||
github: GitHubClient,
|
||||
buildkite: BuildkiteClient,
|
||||
trusted_users_value: str = "",
|
||||
branch_updater: GitHubClient | None = None,
|
||||
) -> None:
|
||||
command = parse_command(event["comment"]["body"])
|
||||
if not command or "pull_request" not in event["issue"]:
|
||||
@@ -1182,6 +1254,8 @@ def run(
|
||||
command=command,
|
||||
github=github,
|
||||
pr=pr,
|
||||
branch_updater=branch_updater,
|
||||
allow_repeat_run=is_trusted_permission(permission),
|
||||
)
|
||||
elif command in RETRY_COMMANDS:
|
||||
message = handle_retry_failed(
|
||||
@@ -1263,11 +1337,15 @@ def main() -> None:
|
||||
os.environ.get("BUILDKITE_ORGANIZATION", "vllm"),
|
||||
pipeline,
|
||||
)
|
||||
branch_update_token = os.environ.get("GH_BRANCH_UPDATE_TOKEN", "")
|
||||
run(
|
||||
event,
|
||||
github,
|
||||
buildkite,
|
||||
os.environ.get("CI_TRUSTED_USERS", ""),
|
||||
GitHubClient(branch_update_token, os.environ["GITHUB_REPOSITORY"])
|
||||
if branch_update_token
|
||||
else None,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -144,6 +144,23 @@ class FakeGitHub:
|
||||
self.comments.append(body)
|
||||
|
||||
|
||||
class FakeBranchUpdater:
|
||||
def __init__(self, github: FakeGitHub, error: ApiError | None = None) -> None:
|
||||
self.calls: list[tuple[int, str]] = []
|
||||
self.error = error
|
||||
self.github = github
|
||||
|
||||
def update_branch(self, number: int, expected_head_sha: str) -> None:
|
||||
self.calls.append((number, expected_head_sha))
|
||||
if self.error:
|
||||
raise self.error
|
||||
self.github.prs[number] = {
|
||||
**self.github.prs[number],
|
||||
"head": {**self.github.prs[number]["head"], "sha": "merge-commit-sha"},
|
||||
}
|
||||
self.github.behind = 0
|
||||
|
||||
|
||||
class FakeBuildkite:
|
||||
def __init__(
|
||||
self,
|
||||
@@ -616,6 +633,64 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
)
|
||||
self.assertIn(command, github.comments[0])
|
||||
|
||||
def test_run_merges_base_into_outdated_branch_before_building(self) -> None:
|
||||
github = FakeGitHub(behind=5)
|
||||
updater = FakeBranchUpdater(github)
|
||||
buildkite = FakeBuildkite([[], []])
|
||||
|
||||
with patch("run_ci_command.time.sleep"):
|
||||
run(make_event(COMMAND_RUN_CI), github, buildkite, "", updater)
|
||||
|
||||
self.assertEqual(updater.calls, [(42, "0123456789abcdef")])
|
||||
self.assertEqual(buildkite.created_builds[0]["commit"], "merge-commit-sha")
|
||||
self.assertTrue(github.comments[0].startswith("✅ "))
|
||||
self.assertIn("5 commits behind", github.comments[0])
|
||||
self.assertIn("Merged upstream `main`", github.comments[0])
|
||||
|
||||
def test_only_committers_can_rerun_a_commit_that_already_ran_ci(self) -> None:
|
||||
finished_build = {
|
||||
"created_at": "2026-07-28T01:00:00Z",
|
||||
"pull_request": {"id": 42},
|
||||
"state": "failed",
|
||||
"web_url": "https://buildkite.example/builds/122",
|
||||
}
|
||||
for actor, permission, command, allowed in (
|
||||
("author", "read", COMMAND_RUN_CI, False),
|
||||
("author", "read", f"{COMMAND_RUN_CI} --allow-stale", False),
|
||||
("reviewer", "write", COMMAND_RUN_CI, True),
|
||||
):
|
||||
with self.subTest(actor=actor, command=command):
|
||||
github = FakeGitHub(
|
||||
behind=5,
|
||||
permission=permission,
|
||||
pr=make_pr(labels=[{"name": "ready"}]),
|
||||
)
|
||||
updater = FakeBranchUpdater(github)
|
||||
buildkite = FakeBuildkite([[], [finished_build]])
|
||||
|
||||
with patch("run_ci_command.time.sleep"):
|
||||
run(make_event(command, actor), github, buildkite, "", updater)
|
||||
|
||||
self.assertEqual(bool(buildkite.created_builds), allowed)
|
||||
if not allowed:
|
||||
self.assertEqual(updater.calls, [])
|
||||
self.assertIn("CI already ran on this commit", github.comments[0])
|
||||
self.assertIn("`/ci retry`", github.comments[0])
|
||||
|
||||
def test_run_reports_failed_merge_without_building(self) -> None:
|
||||
github = FakeGitHub(behind=5)
|
||||
updater = FakeBranchUpdater(github, error=ApiError(422, "merge conflict"))
|
||||
buildkite = FakeBuildkite([[], []])
|
||||
|
||||
run(make_event(COMMAND_RUN_CI), github, buildkite, "", updater)
|
||||
|
||||
self.assertEqual(buildkite.created_builds, [])
|
||||
self.assertTrue(github.comments[0].startswith("❌ "))
|
||||
self.assertIn("could not merge upstream `main`", github.comments[0])
|
||||
self.assertIn("merge conflict", github.comments[0])
|
||||
self.assertIn("No new CI build was started.", github.comments[0])
|
||||
self.assertIn("`/ci run --allow-stale`", github.comments[0])
|
||||
|
||||
def test_allow_stale_preserves_author_permissions_and_run_configuration(
|
||||
self,
|
||||
) -> None:
|
||||
|
||||
Reference in New Issue
Block a user