[CI] Add /ci cancel command (#51732)
Signed-off-by: khluu <khluu000@gmail.com> Co-authored-by: OpenAI Codex <noreply@openai.com>
This commit is contained in:
co-authored by
OpenAI Codex
parent
635dd6aae6
commit
3c6b2e9c08
@@ -82,7 +82,7 @@ jobs:
|
||||
'',
|
||||
'PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment `/ci run` whenever CI signals are needed.',
|
||||
'',
|
||||
'Once the PR is approved or has the `ready` label, the PR author can also use `/ci run` or `/ci retry`. New commits do not start CI automatically.',
|
||||
'Once the PR is approved or has the `ready` label, the PR author can also use `/ci run`, `/ci retry`, or `/ci cancel`. New commits do not start CI automatically.',
|
||||
'',
|
||||
'If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.',
|
||||
'',
|
||||
|
||||
@@ -20,7 +20,8 @@ jobs:
|
||||
(github.event.comment.body == '/ci run' ||
|
||||
github.event.comment.body == '/ci run all' ||
|
||||
github.event.comment.body == '/ci run nightly' ||
|
||||
github.event.comment.body == '/ci retry')
|
||||
github.event.comment.body == '/ci retry' ||
|
||||
github.event.comment.body == '/ci cancel')
|
||||
runs-on: [self-hosted, linux, x64, vllm-runners]
|
||||
steps:
|
||||
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
|
||||
|
||||
@@ -16,6 +16,7 @@ COMMAND_RUN_CI = "/ci run"
|
||||
COMMAND_RUN_CI_ALL = "/ci run all"
|
||||
COMMAND_RUN_CI_NIGHTLY = "/ci run nightly"
|
||||
COMMAND_RETRY_FAILED = "/ci retry"
|
||||
COMMAND_CANCEL_CI = "/ci cancel"
|
||||
RUN_CI_COMMAND_ENV = {
|
||||
COMMAND_RUN_CI: {},
|
||||
COMMAND_RUN_CI_ALL: {"RUN_ALL": "1"},
|
||||
@@ -35,6 +36,7 @@ ACTIVE_BUILD_STATES = {
|
||||
"waiting_failed",
|
||||
}
|
||||
RETRY_STATES = "failed,timed_out,expired"
|
||||
CANCELABLE_BUILD_STATES = ("scheduled", "running", "failing")
|
||||
SETUP_STEP_KEYS = {
|
||||
"ensure-ci-base-amd",
|
||||
"pre-commit",
|
||||
@@ -335,7 +337,9 @@ class BuildkiteClient:
|
||||
self,
|
||||
commit: str | None,
|
||||
*,
|
||||
branch: str | None = None,
|
||||
metadata: tuple[str, str] | None = None,
|
||||
states: Sequence[str] = (),
|
||||
) -> list[dict[str, Any]]:
|
||||
query = [
|
||||
("exclude_jobs", "true"),
|
||||
@@ -344,9 +348,12 @@ class BuildkiteClient:
|
||||
]
|
||||
if commit:
|
||||
query.append(("commit", commit))
|
||||
if branch:
|
||||
query.append(("branch", branch))
|
||||
if metadata:
|
||||
key, value = metadata
|
||||
query.append((f"meta_data[{key}]", value))
|
||||
query.extend(("state[]", state) for state in states)
|
||||
response = self._request(query=query)
|
||||
if not isinstance(response, list):
|
||||
raise ApiError(None, "Buildkite API returned an invalid build list.")
|
||||
@@ -367,6 +374,10 @@ class BuildkiteClient:
|
||||
path=f"/{number}/retry_failed_jobs",
|
||||
)
|
||||
|
||||
def cancel_build(self, build_number: int) -> dict[str, Any]:
|
||||
number = urllib.parse.quote(str(build_number), safe="")
|
||||
return self._request(method="PUT", path=f"/{number}/cancel")
|
||||
|
||||
def list_failed_jobs(self, build_number: int) -> list[dict[str, Any]]:
|
||||
number = urllib.parse.quote(str(build_number), safe="")
|
||||
query = [
|
||||
@@ -397,7 +408,7 @@ class BuildkiteClient:
|
||||
|
||||
|
||||
def parse_command(body: str) -> str | None:
|
||||
if body in {*RUN_CI_COMMAND_ENV, COMMAND_RETRY_FAILED}:
|
||||
if body in {*RUN_CI_COMMAND_ENV, COMMAND_RETRY_FAILED, COMMAND_CANCEL_CI}:
|
||||
return body
|
||||
return None
|
||||
|
||||
@@ -432,7 +443,7 @@ def authorize(
|
||||
if actor.casefold() != pr["user"]["login"].casefold():
|
||||
return (
|
||||
False,
|
||||
"Only reviewers with write access can run CI before it is "
|
||||
"Only reviewers with write access can use CI commands before CI is "
|
||||
"delegated to the PR author.",
|
||||
)
|
||||
if pr["draft"]:
|
||||
@@ -650,7 +661,9 @@ def notify_authorized(
|
||||
"- `/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 "
|
||||
"the latest earlier CI build for this PR.\n\n"
|
||||
"the latest earlier CI build for this PR.\n"
|
||||
"- `/ci cancel` cancels scheduled or running CI builds for this PR "
|
||||
"branch.\n\n"
|
||||
f"{CI_AUTHORIZED_COMMENT_MARKER}"
|
||||
),
|
||||
)
|
||||
@@ -837,6 +850,38 @@ def handle_retry_failed(
|
||||
)
|
||||
|
||||
|
||||
def handle_cancel_ci(
|
||||
*,
|
||||
buildkite: BuildkiteClient,
|
||||
pr: Mapping[str, Any],
|
||||
) -> str:
|
||||
branch = pr["head"]["ref"]
|
||||
builds = buildkite.list_builds(
|
||||
None,
|
||||
branch=branch,
|
||||
states=CANCELABLE_BUILD_STATES,
|
||||
)
|
||||
cancelable_builds = [
|
||||
build
|
||||
for build in builds
|
||||
if build.get("branch") == branch
|
||||
and is_build_for_pr(build, pr["number"])
|
||||
and build.get("state") in CANCELABLE_BUILD_STATES
|
||||
]
|
||||
if not cancelable_builds:
|
||||
return f"No cancelable CI build is running for branch `{branch}`."
|
||||
|
||||
for build in cancelable_builds:
|
||||
buildkite.cancel_build(build["number"])
|
||||
|
||||
links = ", ".join(
|
||||
f"[#{build['number']}]({build['web_url']})" for build in cancelable_builds
|
||||
)
|
||||
count = len(cancelable_builds)
|
||||
noun = "build" if count == 1 else "builds"
|
||||
return f"Requested cancellation of {count} CI {noun} for `{branch}`: {links}."
|
||||
|
||||
|
||||
def run(
|
||||
event: Mapping[str, Any],
|
||||
github: GitHubClient,
|
||||
@@ -904,7 +949,7 @@ def run(
|
||||
github=github,
|
||||
pr=pr,
|
||||
)
|
||||
else:
|
||||
elif command == COMMAND_RETRY_FAILED:
|
||||
message = handle_retry_failed(
|
||||
actor=actor,
|
||||
buildkite=buildkite,
|
||||
@@ -912,6 +957,11 @@ def run(
|
||||
github=github,
|
||||
pr=pr,
|
||||
)
|
||||
else:
|
||||
message = handle_cancel_ci(
|
||||
buildkite=buildkite,
|
||||
pr=pr,
|
||||
)
|
||||
add_reaction_safely(github, comment_id, "rocket")
|
||||
github.add_comment(issue_number, f"✅ {message}")
|
||||
except Exception:
|
||||
|
||||
@@ -9,7 +9,9 @@ from typing import Any
|
||||
from unittest.mock import patch
|
||||
|
||||
from run_ci_command import (
|
||||
CANCELABLE_BUILD_STATES,
|
||||
CI_AUTHORIZED_COMMENT_MARKER,
|
||||
COMMAND_CANCEL_CI,
|
||||
COMMAND_RETRY_FAILED,
|
||||
COMMAND_RUN_CI,
|
||||
COMMAND_RUN_CI_ALL,
|
||||
@@ -136,12 +138,15 @@ class FakeBuildkite:
|
||||
self.job_list_calls: list[int] = []
|
||||
self.list_calls: list[tuple[str | None, tuple[str, str] | None]] = []
|
||||
self.retry_calls: list[tuple[int, str]] = []
|
||||
self.cancel_calls: list[int] = []
|
||||
|
||||
def list_builds(
|
||||
self,
|
||||
commit: str | None,
|
||||
*,
|
||||
branch: str | None = None,
|
||||
metadata: tuple[str, str] | None = None,
|
||||
states: tuple[str, ...] = (),
|
||||
) -> list[dict[str, Any]]:
|
||||
self.list_calls.append((commit, metadata))
|
||||
return self.build_lists.pop(0)
|
||||
@@ -161,6 +166,10 @@ class FakeBuildkite:
|
||||
self.retry_calls.append((build_number, states))
|
||||
return {"retried_jobs_count": 3}
|
||||
|
||||
def cancel_build(self, build_number: int) -> dict[str, Any]:
|
||||
self.cancel_calls.append(build_number)
|
||||
return {"number": build_number, "state": "canceling"}
|
||||
|
||||
def list_failed_jobs(self, build_number: int) -> list[dict[str, Any]]:
|
||||
self.job_list_calls.append(build_number)
|
||||
return self.failed_job_lists.pop(0)
|
||||
@@ -292,8 +301,10 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
parse_command(COMMAND_RETRY_FAILED),
|
||||
COMMAND_RETRY_FAILED,
|
||||
)
|
||||
self.assertEqual(parse_command(COMMAND_CANCEL_CI), COMMAND_CANCEL_CI)
|
||||
self.assertIsNone(parse_command("/ci run please"))
|
||||
self.assertIsNone(parse_command("/ci run all please"))
|
||||
self.assertIsNone(parse_command("/ci cancel please"))
|
||||
self.assertIsNone(parse_command(" /ci run"))
|
||||
|
||||
def test_write_access_authorizes_reviewers_and_authors(self) -> None:
|
||||
@@ -531,6 +542,7 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
self.assertTrue(comment.startswith("✅ @author"))
|
||||
self.assertIn("`/ci run` starts a CI build", comment)
|
||||
self.assertIn("`/ci retry` retries failed jobs", comment)
|
||||
self.assertIn("`/ci cancel` cancels scheduled or running", comment)
|
||||
self.assertIn("CI build for the current PR head", comment)
|
||||
self.assertIn("only jobs that failed in the latest earlier CI build", comment)
|
||||
self.assertNotIn(COMMAND_RUN_CI_ALL, comment)
|
||||
@@ -854,6 +866,82 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
self.assertTrue(call["url"].endswith("/123/retry_failed_jobs"))
|
||||
self.assertEqual(call["body"], {"states": RETRY_STATES})
|
||||
|
||||
def test_ci_cancel_cancels_active_builds_for_pr_branch(self) -> None:
|
||||
github = FakeGitHub()
|
||||
buildkite = FakeBuildkite(
|
||||
[
|
||||
[
|
||||
{
|
||||
"branch": "feature",
|
||||
"number": 123,
|
||||
"pull_request": {"id": 42},
|
||||
"state": "running",
|
||||
"web_url": "https://buildkite.example/builds/123",
|
||||
},
|
||||
{
|
||||
"branch": "feature",
|
||||
"number": 124,
|
||||
"meta_data": {"github-pr-number": "42"},
|
||||
"state": "failing",
|
||||
"web_url": "https://buildkite.example/builds/124",
|
||||
},
|
||||
{
|
||||
"branch": "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",
|
||||
"number": 127,
|
||||
"pull_request": {"id": 42},
|
||||
"state": "passed",
|
||||
"web_url": "https://buildkite.example/builds/127",
|
||||
},
|
||||
]
|
||||
]
|
||||
)
|
||||
|
||||
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.assertIn("#123", github.comments[0])
|
||||
self.assertIn("#124", github.comments[0])
|
||||
|
||||
def test_ci_cancel_is_a_noop_without_active_builds(self) -> None:
|
||||
github = FakeGitHub()
|
||||
buildkite = FakeBuildkite([[]])
|
||||
|
||||
run(make_event(COMMAND_CANCEL_CI), github, buildkite)
|
||||
|
||||
self.assertEqual(buildkite.cancel_calls, [])
|
||||
self.assertIn("No cancelable CI build is running", github.comments[0])
|
||||
|
||||
def test_buildkite_cancel_uses_cancel_build_endpoint(self) -> None:
|
||||
transport = FakeTransport({"number": 123, "state": "canceling"})
|
||||
client = BuildkiteClient(
|
||||
"secret",
|
||||
"vllm",
|
||||
"ci",
|
||||
transport=transport,
|
||||
)
|
||||
|
||||
client.cancel_build(123)
|
||||
|
||||
call = transport.calls[0]
|
||||
self.assertEqual(call["method"], "PUT")
|
||||
self.assertTrue(call["url"].endswith("/123/cancel"))
|
||||
self.assertIsNone(call["body"])
|
||||
|
||||
def test_buildkite_list_builds_allows_query_on_builds_endpoint(self) -> None:
|
||||
transport = FakeTransport([])
|
||||
client = BuildkiteClient(
|
||||
@@ -865,14 +953,20 @@ class RunCiCommandTest(unittest.TestCase):
|
||||
|
||||
builds = client.list_builds(
|
||||
"current-commit",
|
||||
branch="feature",
|
||||
metadata=("github-pr-number", "42"),
|
||||
states=CANCELABLE_BUILD_STATES,
|
||||
)
|
||||
|
||||
self.assertEqual(builds, [])
|
||||
url = transport.calls[0]["url"]
|
||||
self.assertIn("?exclude_jobs=true", url)
|
||||
self.assertIn("commit=current-commit", url)
|
||||
self.assertIn("branch=feature", url)
|
||||
self.assertIn("meta_data%5Bgithub-pr-number%5D=42", url)
|
||||
self.assertIn("state%5B%5D=scheduled", url)
|
||||
self.assertIn("state%5B%5D=running", url)
|
||||
self.assertIn("state%5B%5D=failing", url)
|
||||
|
||||
def test_buildkite_failed_jobs_follow_cursor_pagination(self) -> None:
|
||||
next_url = (
|
||||
|
||||
Reference in New Issue
Block a user