[CI] Use PR head label for Buildkite branch to avoid main collision (#54895)
Signed-off-by: khluu <khluu000@gmail.com>
This commit is contained in:
@@ -606,7 +606,10 @@ def create_build_payload(
|
||||
}
|
||||
return {
|
||||
"commit": pr["head"]["sha"],
|
||||
"branch": pr["head"]["ref"],
|
||||
# Use the head label (owner:branch) rather than the bare ref so a PR
|
||||
# whose fork branch is named e.g. "main" does not collide with the
|
||||
# upstream branch of the same name.
|
||||
"branch": pr["head"].get("label") or pr["head"]["ref"],
|
||||
"message": f"PR #{pr['number']} {command} by @{actor}",
|
||||
"pull_request_id": pr["number"],
|
||||
"pull_request_base_branch": pr["base"]["ref"],
|
||||
@@ -969,12 +972,13 @@ def handle_cancel_ci(
|
||||
) -> str:
|
||||
ci_name = ci_name_for_command(command)
|
||||
branch = pr["head"]["ref"]
|
||||
# Builds are created under the head label (owner:branch); also match the
|
||||
# bare ref so builds created before that change can still be cancelled.
|
||||
branches = [branch]
|
||||
head_label = pr["head"].get("label")
|
||||
if head_label and head_label not in branches:
|
||||
branches.insert(0, str(head_label))
|
||||
states = CANCELABLE_BUILD_STATES
|
||||
if command in AMD_CI_COMMANDS:
|
||||
head_label = pr["head"].get("label")
|
||||
if head_label and head_label not in branches:
|
||||
branches.append(str(head_label))
|
||||
|
||||
builds_by_number: dict[int, dict[str, Any]] = {}
|
||||
for candidate_branch in branches:
|
||||
|
||||
@@ -45,6 +45,7 @@ def make_pr(**overrides: Any) -> dict[str, Any]:
|
||||
"base": {"ref": "main"},
|
||||
"draft": False,
|
||||
"head": {
|
||||
"label": "contributor:feature",
|
||||
"ref": "feature",
|
||||
"repo": {"clone_url": "https://github.com/contributor/vllm.git"},
|
||||
"sha": "0123456789abcdef",
|
||||
@@ -479,7 +480,7 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
payload,
|
||||
{
|
||||
"commit": "0123456789abcdef",
|
||||
"branch": "feature",
|
||||
"branch": "contributor:feature",
|
||||
"message": "PR #42 /ci run by @reviewer",
|
||||
"pull_request_id": 42,
|
||||
"pull_request_base_branch": "main",
|
||||
@@ -498,6 +499,23 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
},
|
||||
)
|
||||
|
||||
def test_build_payload_uses_head_label_to_avoid_branch_collision(self) -> None:
|
||||
pr = make_pr()
|
||||
pr["head"]["ref"] = "main"
|
||||
pr["head"]["label"] = "contributor:main"
|
||||
|
||||
payload = create_build_payload(actor="reviewer", comment_id=99, pr=pr)
|
||||
|
||||
self.assertEqual(payload["branch"], "contributor:main")
|
||||
|
||||
def test_build_payload_falls_back_to_ref_without_label(self) -> None:
|
||||
pr = make_pr()
|
||||
del pr["head"]["label"]
|
||||
|
||||
payload = create_build_payload(actor="reviewer", comment_id=99, pr=pr)
|
||||
|
||||
self.assertEqual(payload["branch"], "feature")
|
||||
|
||||
def test_write_reviewer_runs_ci_without_delegation(self) -> None:
|
||||
github = FakeGitHub()
|
||||
buildkite = FakeBuildkite([[], []])
|
||||
@@ -1039,56 +1057,65 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
github = FakeGitHub()
|
||||
buildkite = FakeBuildkite(
|
||||
[
|
||||
# Builds created under the head label (owner:branch).
|
||||
[
|
||||
{
|
||||
"branch": "feature",
|
||||
"branch": "contributor:feature",
|
||||
"number": 123,
|
||||
"pull_request": {"id": 42},
|
||||
"state": "running",
|
||||
"web_url": "https://buildkite.example/builds/123",
|
||||
},
|
||||
{
|
||||
"branch": "feature",
|
||||
"branch": "contributor:feature",
|
||||
"number": 124,
|
||||
"meta_data": {"github-pr-number": "42"},
|
||||
"state": "failing",
|
||||
"web_url": "https://buildkite.example/builds/124",
|
||||
},
|
||||
{
|
||||
"branch": "feature",
|
||||
"branch": "contributor:feature",
|
||||
"number": 125,
|
||||
"pull_request": {"id": 43},
|
||||
"state": "running",
|
||||
"web_url": "https://buildkite.example/builds/125",
|
||||
},
|
||||
{
|
||||
"branch": "other-branch",
|
||||
"number": 126,
|
||||
"pull_request": {"id": 42},
|
||||
"state": "running",
|
||||
"web_url": "https://buildkite.example/builds/126",
|
||||
},
|
||||
{
|
||||
"branch": "feature",
|
||||
"branch": "contributor:feature",
|
||||
"number": 127,
|
||||
"pull_request": {"id": 42},
|
||||
"state": "passed",
|
||||
"web_url": "https://buildkite.example/builds/127",
|
||||
},
|
||||
]
|
||||
],
|
||||
# Legacy builds created under the bare ref before the label change.
|
||||
[
|
||||
{
|
||||
"branch": "feature",
|
||||
"number": 126,
|
||||
"pull_request": {"id": 42},
|
||||
"state": "running",
|
||||
"web_url": "https://buildkite.example/builds/126",
|
||||
},
|
||||
],
|
||||
]
|
||||
)
|
||||
|
||||
run(make_event(COMMAND_CANCEL_CI), github, buildkite)
|
||||
|
||||
self.assertEqual(buildkite.cancel_calls, [123, 124])
|
||||
self.assertIn("Requested cancellation of 2 CI builds", github.comments[0])
|
||||
self.assertEqual(buildkite.cancel_calls, [123, 124, 126])
|
||||
self.assertEqual(
|
||||
[request["branch"] for request in buildkite.list_requests],
|
||||
["contributor:feature", "feature"],
|
||||
)
|
||||
self.assertIn("Requested cancellation of 3 CI builds", github.comments[0])
|
||||
self.assertIn("#123", github.comments[0])
|
||||
self.assertIn("#124", github.comments[0])
|
||||
self.assertIn("#126", github.comments[0])
|
||||
|
||||
def test_ci_cancel_is_a_noop_without_active_builds(self) -> None:
|
||||
github = FakeGitHub()
|
||||
buildkite = FakeBuildkite([[]])
|
||||
buildkite = FakeBuildkite([[], []])
|
||||
|
||||
run(make_event(COMMAND_CANCEL_CI), github, buildkite)
|
||||
|
||||
@@ -1101,15 +1128,6 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
github = FakeGitHub(pr=pr)
|
||||
buildkite = FakeBuildkite(
|
||||
[
|
||||
[
|
||||
{
|
||||
"branch": "feature",
|
||||
"number": 321,
|
||||
"pull_request": {"id": 42},
|
||||
"state": "running",
|
||||
"web_url": "https://buildkite.example/amd-ci/builds/321",
|
||||
}
|
||||
],
|
||||
[
|
||||
{
|
||||
"branch": "contributor:feature",
|
||||
@@ -1119,15 +1137,24 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
"web_url": "https://buildkite.example/amd-ci/builds/322",
|
||||
}
|
||||
],
|
||||
[
|
||||
{
|
||||
"branch": "feature",
|
||||
"number": 321,
|
||||
"pull_request": {"id": 42},
|
||||
"state": "running",
|
||||
"web_url": "https://buildkite.example/amd-ci/builds/321",
|
||||
}
|
||||
],
|
||||
]
|
||||
)
|
||||
|
||||
run(make_event(COMMAND_CANCEL_AMD_CI), github, buildkite)
|
||||
|
||||
self.assertEqual(buildkite.cancel_calls, [321, 322])
|
||||
self.assertEqual(buildkite.cancel_calls, [322, 321])
|
||||
self.assertEqual(
|
||||
[request["branch"] for request in buildkite.list_requests],
|
||||
["feature", "contributor:feature"],
|
||||
["contributor:feature", "feature"],
|
||||
)
|
||||
self.assertTrue(
|
||||
all(
|
||||
|
||||
Reference in New Issue
Block a user