diff --git a/.buildkite/amd/scripts/bootstrap-amd-omni.sh b/.buildkite/amd/scripts/bootstrap-amd-omni.sh index 4eb7f6eb4..9eff6b6c7 100755 --- a/.buildkite/amd/scripts/bootstrap-amd-omni.sh +++ b/.buildkite/amd/scripts/bootstrap-amd-omni.sh @@ -14,6 +14,8 @@ set -euo pipefail # shellcheck disable=SC1091 source .buildkite/common/scripts/resolve_skip_ci.sh +AMD_SUITE_SELECTOR=".buildkite/amd/scripts/select_test_suites.py" + if [[ -z "${RUN_ALL:-}" ]]; then RUN_ALL=0 fi @@ -30,31 +32,20 @@ if [[ -z "${AMD_MIRROR_HW:-}" ]]; then AMD_MIRROR_HW="amdproduction" fi -fail_fast() { - DISABLE_LABEL="ci-no-fail-fast" - # If BUILDKITE_PULL_REQUEST != "false", then we check the PR labels using curl and jq - if [ "$BUILDKITE_PULL_REQUEST" != "false" ]; then - PR_LABELS=$(curl -s "https://api.github.com/repos/vllm-project/vllm-omni/pulls/$BUILDKITE_PULL_REQUEST" | jq -r '.labels[].name') - if [[ $PR_LABELS == *"$DISABLE_LABEL"* ]]; then - echo false - else - echo true - fi - else - echo false # not a PR or BUILDKITE_PULL_REQUEST not set +PR_LABELS="" +# Explicit debug suites do not depend on PR labels. +if [[ "${BUILDKITE_PULL_REQUEST:-false}" != "false" && -z "${DEBUG_TEST_YAML:-}" ]]; then + if ! PR_LABELS=$(curl -fsSL \ + "https://api.github.com/repos/vllm-project/vllm-omni/pulls/$BUILDKITE_PULL_REQUEST" \ + | jq -r '.labels[].name'); then + echo "ERROR: Could not read PR labels; refusing to select an incorrect AMD test tier." >&2 + exit 1 fi -} +fi -check_run_all_label() { - RUN_ALL_LABEL="ready-run-all-tests" - # If BUILDKITE_PULL_REQUEST != "false", then we check the PR labels using curl and jq - if [ "$BUILDKITE_PULL_REQUEST" != "false" ]; then - PR_LABELS=$(curl -s "https://api.github.com/repos/vllm-project/vllm-omni/pulls/$BUILDKITE_PULL_REQUEST" | jq -r '.labels[].name') - if [[ $PR_LABELS == *"$RUN_ALL_LABEL"* ]]; then - echo true - else - echo false - fi +fail_fast() { + if [[ "${BUILDKITE_PULL_REQUEST:-false}" != "false" ]]; then + echo true else echo false # not a PR or BUILDKITE_PULL_REQUEST not set fi @@ -64,6 +55,57 @@ if [[ -z "${COV_ENABLED:-}" ]]; then COV_ENABLED=0 fi +resolve_test_specs() { + local selected_specs + if ! selected_specs=$(python3 "$AMD_SUITE_SELECTOR" \ + --branch "$BUILDKITE_BRANCH" \ + --labels "$PR_LABELS" \ + --debug-test-yaml "${DEBUG_TEST_YAML:-}"); then + exit 1 + fi + + TEST_SPECS=() + while IFS= read -r suite_spec; do + [[ -z "$suite_spec" ]] && continue + TEST_SPECS+=("$suite_spec") + done <<< "$selected_specs" +} + +filter_test_specs_by_skip_ci() { + local suite_spec level decision + local skip_all=0 + local -a runnable_specs=() + + for suite_spec in "${TEST_SPECS[@]}"; do + case "$suite_spec" in + READY_TESTS:*) level="l2" ;; + MERGE_TESTS:*) level="l3" ;; + *) + echo "ERROR: unknown AMD test suite spec '$suite_spec'" >&2 + exit 1 + ;; + esac + + if decision=$(python3 "$SKIP_CI_PY" gate amd "$level"); then + echo "Skipping AMD $level suite due to $decision decision." + [[ "$decision" == "skip-all" ]] && skip_all=1 + else + runnable_specs+=("$suite_spec") + fi + done + + TEST_SPECS=("${runnable_specs[@]}") + if [[ ${#TEST_SPECS[@]} -eq 0 ]]; then + if [[ $skip_all -eq 1 ]]; then + buildkite-agent annotate \ + ":memo: CI skipped — docs or pytest skip-mark changes only" \ + --style "info" 2>/dev/null || true + fi + echo "No AMD L2/L3 suites remain after skip-ci filtering." + exit 0 + fi +} + upload_pipeline() { echo "Uploading pipeline..." # Install minijinja @@ -84,7 +126,7 @@ upload_pipeline() { # (WIP) Use pipeline generator instead of jinja template if [ -e ".buildkite/amd/pipeline_generator/pipeline_generator.py" ]; then python -m pip install click pydantic - python .buildkite/amd/pipeline_generator/pipeline_generator.py --run_all=$RUN_ALL --list_file_diff="$LIST_FILE_DIFF" --nightly="$NIGHTLY" --mirror_hw="$AMD_MIRROR_HW" + python .buildkite/amd/pipeline_generator/pipeline_generator.py --run_all="$RUN_ALL" --list_file_diff="$LIST_FILE_DIFF" --nightly="$NIGHTLY" --mirror_hw="$AMD_MIRROR_HW" buildkite-agent pipeline upload .buildkite/amd/pipeline.yaml exit 0 fi @@ -97,49 +139,12 @@ upload_pipeline() { cd .buildkite/amd - # Select test definition file: merge suite for main, ready suite for PRs. - # For debugging, DEBUG_TEST_YAML accepts a comma-separated list containing - # "merge" and/or "ready" (case-insensitive). Multiple suites are combined - # before rendering so they share one amd-build step. - if [[ -n "${DEBUG_TEST_YAML:-}" ]]; then - declare -a DEBUG_TEST_SPECS=() - declare -A SEEN_DEBUG_TESTS=() - IFS=',' read -ra REQUESTED_DEBUG_TESTS <<< "${DEBUG_TEST_YAML,,}" - - for requested_test in "${REQUESTED_DEBUG_TESTS[@]}"; do - # Permit readable values such as "ready, merge," and ignore the - # empty item produced by a trailing comma. - requested_test="${requested_test//[[:space:]]/}" - [[ -z "$requested_test" ]] && continue - - if [[ -n "${SEEN_DEBUG_TESTS[$requested_test]:-}" ]]; then - echo "ERROR: duplicate DEBUG_TEST_YAML suite '$requested_test'" >&2 - exit 1 - fi - SEEN_DEBUG_TESTS[$requested_test]=1 - - case "$requested_test" in - ready) - DEBUG_TEST_SPECS+=("READY_TESTS:test-amd-ready.yml") - ;; - merge) - DEBUG_TEST_SPECS+=("MERGE_TESTS:test-amd-merge.yml") - ;; - *) - echo "ERROR: DEBUG_TEST_YAML entries must be 'merge' or 'ready', got '$requested_test'" >&2 - exit 1 - ;; - esac - done - - if [[ ${#DEBUG_TEST_SPECS[@]} -eq 0 ]]; then - echo "ERROR: DEBUG_TEST_YAML did not contain a test suite" >&2 - exit 1 - elif [[ ${#DEBUG_TEST_SPECS[@]} -eq 1 ]]; then - TEST_YAML="${DEBUG_TEST_SPECS[0]#*:}" - else - TEST_YAML=$(mktemp "${TMPDIR:-/tmp}/amd-debug-tests.XXXXXX.yml") - python - "$TEST_YAML" "${DEBUG_TEST_SPECS[@]}" <<'PY' + # Multiple label-selected or debug-selected suites share one image build. + if [[ ${#TEST_SPECS[@]} -eq 1 ]]; then + TEST_YAML="${TEST_SPECS[0]#*:}" + else + TEST_YAML=$(mktemp "${TMPDIR:-/tmp}/amd-selected-tests.XXXXXX.yml") + python - "$TEST_YAML" "${TEST_SPECS[@]}" <<'PY' import sys import yaml @@ -172,13 +177,8 @@ for suite_spec in suite_specs: with open(output_path, "w", encoding="utf-8") as output_file: yaml.safe_dump(combined, output_file, sort_keys=False) PY - fi - echo "DEBUG_TEST_YAML override: using ${DEBUG_TEST_SPECS[*]}" - elif [[ $BUILDKITE_BRANCH == "main" ]]; then - TEST_YAML="test-amd-merge.yml" - else - TEST_YAML="test-amd-ready.yml" fi + echo "AMD test suites: ${TEST_SPECS[*]}" ( set -x @@ -198,7 +198,7 @@ PY > pipeline.yaml ) cat pipeline.yaml - if [[ "$TEST_YAML" == "${TMPDIR:-/tmp}/amd-debug-tests."*.yml ]]; then + if [[ "$TEST_YAML" == "${TMPDIR:-/tmp}/amd-selected-tests."*.yml ]]; then rm -f -- "$TEST_YAML" fi buildkite-agent artifact upload pipeline.yaml @@ -219,12 +219,10 @@ if [[ $BUILDKITE_BRANCH == "main" ]]; then file_diff=$(get_diff_main) fi -# Early exit: unified skip-ci (docs / skip marks) and CI-yaml-only level targeting. -if [[ $BUILDKITE_BRANCH == "main" ]]; then - gate_bootstrap_ci amd l3 -else - gate_bootstrap_ci amd l2 -fi +# Resolve PR tier labels before skip-ci so ready and merge suites can be +# filtered independently for CI-YAML-only changes. +resolve_test_specs +filter_test_specs_by_skip_ci patterns=( "docker/Dockerfile" @@ -274,14 +272,6 @@ for file in $file_diff; do fi done -# Check for ready-run-all-tests label -LABEL_RUN_ALL=$(check_run_all_label) -if [[ $LABEL_RUN_ALL == true ]]; then - RUN_ALL=1 - NIGHTLY=1 - echo "Found 'ready-run-all-tests' label. Running all tests including optional tests." -fi - # Decide whether to use precompiled wheels # Relies on existing patterns array as a basis. if [[ -n "${VLLM_USE_PRECOMPILED:-}" ]]; then diff --git a/.buildkite/amd/scripts/select_test_suites.py b/.buildkite/amd/scripts/select_test_suites.py new file mode 100644 index 000000000..b7c72f6e4 --- /dev/null +++ b/.buildkite/amd/scripts/select_test_suites.py @@ -0,0 +1,93 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: Apache-2.0 +# SPDX-FileCopyrightText: Copyright contributors to the vLLM-Omni project +"""Select AMD L2/L3 suites after Buildkite has created an AMD build. + +This repository-side selector does not decide which GitHub label events start +the external ``vllm-omni-amd-ci`` pipeline. Its Buildkite PR build condition +must admit the relevant label event before this code can inspect the labels. +""" + +from __future__ import annotations + +import argparse +import sys +from collections.abc import Iterable + +SUITE_SPECS = { + "ready": "READY_TESTS:test-amd-ready.yml", + "merge": "MERGE_TESTS:test-amd-merge.yml", +} + + +def _parse_debug_suites(value: str) -> tuple[str, ...]: + suites = tuple(item.strip().lower() for item in value.split(",") if item.strip()) + if not suites: + raise ValueError("DEBUG_TEST_YAML did not contain a test suite") + + seen: set[str] = set() + for suite in suites: + if suite not in SUITE_SPECS: + raise ValueError(f"DEBUG_TEST_YAML entries must be 'merge' or 'ready', got {suite!r}") + if suite in seen: + raise ValueError(f"duplicate DEBUG_TEST_YAML suite {suite!r}") + seen.add(suite) + return suites + + +def select_amd_test_suites( + *, + branch: str, + labels: Iterable[str], + debug_test_yaml: str = "", +) -> tuple[str, ...]: + """Return ordered suite names while preserving legacy unlabeled PRs.""" + + if debug_test_yaml: + return _parse_debug_suites(debug_test_yaml) + if branch == "main": + return ("merge",) + + label_set = {label.strip() for label in labels if label.strip()} + selected = tuple(suite for label, suite in (("ready", "ready"), ("merge-test", "merge")) if label in label_set) + # AMD historically uploaded L2 for every PR build. Keep that behavior for + # triggers such as amd-test while adding tier selection for ready/merge-test. + return selected or ("ready",) + + +def main() -> int: + parser = argparse.ArgumentParser() + parser.add_argument("--branch", required=True) + parser.add_argument("--labels", default="") + parser.add_argument("--debug-test-yaml", default="") + args = parser.parse_args() + + labels = tuple(args.labels.splitlines()) + try: + suites = select_amd_test_suites( + branch=args.branch, + labels=labels, + debug_test_yaml=args.debug_test_yaml, + ) + except ValueError as exc: + parser.error(str(exc)) + + label_set = {label.strip() for label in labels if label.strip()} + if not args.debug_test_yaml and args.branch != "main" and not ({"ready", "merge-test"} & label_set): + print( + "No AMD L2/L3 tier label found; preserving the legacy ready-suite fallback.", + file=sys.stderr, + ) + if "nightly-test" in label_set: + print( + "AMD nightly-test has no suite yet and does not affect L2/L3 selection.", + file=sys.stderr, + ) + + for suite in suites: + print(SUITE_SPECS[suite]) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/docs/contributing/ci/ci_settings.md b/docs/contributing/ci/ci_settings.md index 7348ad183..d578b4b02 100644 --- a/docs/contributing/ci/ci_settings.md +++ b/docs/contributing/ci/ci_settings.md @@ -161,7 +161,9 @@ Canonical layout (prefer these paths for new changes): **Bootstrap:** [`amd/scripts/bootstrap-amd-omni.sh`](https://github.com/vllm-project/vllm-omni/blob/main/.buildkite/amd/scripts/bootstrap-amd-omni.sh)—skip-ci, diff filtering, Jinja render, then `buildkite-agent pipeline upload`. - **Test YAML (data):** `amd/test-amd-ready.yml` (L2 / PR), `test-amd-merge.yml` (L3 / main). + **Test YAML (data):** `amd/test-amd-ready.yml` (L2 / `ready`), `test-amd-merge.yml` (L3 / `merge-test` and `main`). On a PR carrying both labels, AMD combines both suites behind one image build. PR builds without either tier label retain the legacy L2 fallback for compatibility with the `amd-test` trigger. `DEBUG_TEST_YAML` remains an explicit override. AMD has no L4 file yet, so `nightly-test` does not select an additional suite. + + **Trigger boundary:** AMD label handling has two layers. First, the external `vllm-omni-amd-ci` Buildkite pipeline condition decides whether a GitHub label event creates a build. Only after that build starts does this repository's bootstrap inspect all current PR labels and select L2, L3, or both. Repository-side selection therefore cannot make a `merge-test` event start AMD CI by itself. The external condition must admit both `ready` and `merge-test`, while preserving the legacy `amd-test` trigger and non-PR `main` / scheduled builds. For PR builds without a `DEBUG_TEST_YAML` override, an unreadable label set fails the bootstrap instead of silently selecting the wrong tier. Debug overrides skip label lookup and are validated directly; invalid values, including whitespace-only values, fail the bootstrap. Keep `nightly-test` out of the AMD condition until an AMD L4 suite exists; otherwise it would start a build without providing nightly coverage. **Rendering:** [`test-template-amd-omni.j2`](https://github.com/vllm-project/vllm-omni/blob/main/.buildkite/amd/test-template-amd-omni.j2) wraps data steps with `amd-build` image build and `amd_` queues. Do **not** hand-edit generated `pipeline.yaml`. diff --git a/tests/buildkite/test_amd_bootstrap.py b/tests/buildkite/test_amd_bootstrap.py new file mode 100644 index 000000000..593ba8b26 --- /dev/null +++ b/tests/buildkite/test_amd_bootstrap.py @@ -0,0 +1,156 @@ +# SPDX-License-Identifier: Apache-2.0 +# SPDX-FileCopyrightText: Copyright contributors to the vLLM-Omni project + +from __future__ import annotations + +import os +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +pytestmark = [pytest.mark.core_model, pytest.mark.cpu] + +REPO_ROOT = Path(__file__).resolve().parents[2] +BOOTSTRAP_PATH = REPO_ROOT / ".buildkite/amd/scripts/bootstrap-amd-omni.sh" + + +def _run_bootstrap( + tmp_path: Path, + *, + debug_test_yaml: str | None = None, + curl_status: int = 0, + jq_status: int = 0, + labels: tuple[str, ...] = (), + pull_request: bool = True, +) -> subprocess.CompletedProcess[str]: + for relative_path in ( + ".buildkite/common/scripts/resolve_skip_ci.sh", + ".buildkite/amd/scripts/select_test_suites.py", + ): + destination = tmp_path / relative_path + destination.parent.mkdir(parents=True, exist_ok=True) + shutil.copyfile(REPO_ROOT / relative_path, destination) + + # Stop after real suite selection, before rendering or uploading a pipeline. + (tmp_path / ".buildkite/common/scripts/skip_ci.py").write_text("print('test-skip')\n", encoding="utf-8") + env = { + "PATH": os.environ["PATH"], + "BUILDKITE_PULL_REQUEST": "123" if pull_request else "false", + "BUILDKITE_BRANCH": "feature" if pull_request else "main", + "TEST_PYTHON": sys.executable, + "TEST_CURL_STATUS": str(curl_status), + "TEST_JQ_STATUS": str(jq_status), + "TEST_PR_LABELS": "\n".join(labels), + } + if debug_test_yaml is not None: + env["DEBUG_TEST_YAML"] = debug_test_yaml + + # Source the unmodified bootstrap with subprocess-local command doubles. + script = r""" +curl() { + echo 'curl called' >&2 + return "$TEST_CURL_STATUS" +} +jq() { + echo 'jq called' >&2 + printf '%s\n' "$TEST_PR_LABELS" + return "$TEST_JQ_STATUS" +} +git() { :; } +python3() { "$TEST_PYTHON" "$@"; } +source "$1" +""" + return subprocess.run( + ["bash", "-c", script, "test-amd-bootstrap", str(BOOTSTRAP_PATH)], + cwd=tmp_path, + env=env, + capture_output=True, + text=True, + timeout=10, + check=False, + ) + + +@pytest.mark.parametrize(("curl_status", "jq_status"), [(22, 0), (0, 4)], ids=["request-failure", "parse-failure"]) +@pytest.mark.parametrize( + ("debug_test_yaml", "expected_levels"), + [ + ("ready", ["l2"]), + ("merge", ["l3"]), + ("ready,merge", ["l2", "l3"]), + (" MERGE, ready,", ["l3", "l2"]), + ], +) +def test_debug_override_skips_label_lookup(tmp_path, debug_test_yaml, expected_levels, curl_status, jq_status): + result = _run_bootstrap( + tmp_path, + debug_test_yaml=debug_test_yaml, + curl_status=curl_status, + jq_status=jq_status, + ) + + assert result.returncode == 0, result.stderr + assert "curl called" not in result.stderr + assert "jq called" not in result.stderr + assert "legacy ready-suite fallback" not in result.stderr + assert [ + line.split()[2] for line in result.stdout.splitlines() if line.startswith("Skipping AMD ") + ] == expected_levels + + +@pytest.mark.parametrize(("curl_status", "jq_status"), [(22, 0), (0, 4)], ids=["request-failure", "parse-failure"]) +@pytest.mark.parametrize("debug_test_yaml", [None, ""], ids=["unset", "empty"]) +def test_label_lookup_failure_without_override(tmp_path, debug_test_yaml, curl_status, jq_status): + result = _run_bootstrap( + tmp_path, + debug_test_yaml=debug_test_yaml, + curl_status=curl_status, + jq_status=jq_status, + ) + + assert result.returncode == 1 + assert "Could not read PR labels" in result.stderr + assert "Skipping AMD " not in result.stdout + + +@pytest.mark.parametrize( + ("labels", "expected_levels"), + [ + (("ready",), ["l2"]), + (("merge-test",), ["l3"]), + (("ready", "merge-test"), ["l2", "l3"]), + ((), ["l2"]), + ], +) +def test_label_lookup_selects_tiers(tmp_path, labels, expected_levels): + result = _run_bootstrap(tmp_path, labels=labels) + + assert result.returncode == 0, result.stderr + assert "curl called" in result.stderr + assert "jq called" in result.stderr + assert [ + line.split()[2] for line in result.stdout.splitlines() if line.startswith("Skipping AMD ") + ] == expected_levels + + +@pytest.mark.parametrize("debug_test_yaml", ["ready,ready", "nightly", ", ,", " \t"]) +def test_invalid_debug_override_fails_without_label_lookup(tmp_path, debug_test_yaml): + result = _run_bootstrap(tmp_path, debug_test_yaml=debug_test_yaml, curl_status=22) + + assert result.returncode == 1 + assert "DEBUG_TEST_YAML" in result.stderr + assert "curl called" not in result.stderr + assert "jq called" not in result.stderr + assert "Skipping AMD " not in result.stdout + + +def test_main_does_not_lookup_labels(tmp_path): + result = _run_bootstrap(tmp_path, pull_request=False, curl_status=22, jq_status=4) + + assert result.returncode == 0, result.stderr + assert "curl called" not in result.stderr + assert "jq called" not in result.stderr + assert "Skipping AMD l3 suite" in result.stdout diff --git a/tests/buildkite/test_amd_suite_selection.py b/tests/buildkite/test_amd_suite_selection.py new file mode 100644 index 000000000..b1b25aa26 --- /dev/null +++ b/tests/buildkite/test_amd_suite_selection.py @@ -0,0 +1,61 @@ +# SPDX-License-Identifier: Apache-2.0 +# SPDX-FileCopyrightText: Copyright contributors to the vLLM-Omni project + +from __future__ import annotations + +import importlib.util +from pathlib import Path + +import pytest + +pytestmark = [pytest.mark.core_model, pytest.mark.cpu] + +SELECTOR_PATH = Path(".buildkite/amd/scripts/select_test_suites.py") +SPEC = importlib.util.spec_from_file_location("select_test_suites", SELECTOR_PATH) +assert SPEC is not None and SPEC.loader is not None +SELECTOR = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(SELECTOR) + + +@pytest.mark.parametrize( + ("branch", "labels", "expected"), + [ + ("main", (), ("merge",)), + ("feature", ("ready",), ("ready",)), + ("feature", ("merge-test",), ("merge",)), + ("feature", ("merge-test", "ready"), ("ready", "merge")), + ("feature", ("amd-test",), ("ready",)), + ("feature", ("amd-test", "merge-test"), ("merge",)), + ("feature", ("amd-test", "ready", "merge-test"), ("ready", "merge")), + ("feature", ("nightly-test",), ("ready",)), + ("feature", ("not-ready", "merge-test-extra"), ("ready",)), + ], +) +def test_label_suite_selection(branch, labels, expected): + assert SELECTOR.select_amd_test_suites(branch=branch, labels=labels) == expected + + +def test_debug_override_takes_precedence_and_normalizes_input(): + assert SELECTOR.select_amd_test_suites( + branch="main", + labels=("ready",), + debug_test_yaml=" MERGE, ready,", + ) == ("merge", "ready") + + +def test_empty_debug_override_uses_normal_selection(): + assert SELECTOR.select_amd_test_suites( + branch="feature", + labels=("merge-test",), + debug_test_yaml="", + ) == ("merge",) + + +@pytest.mark.parametrize("value", ["ready,ready", "nightly", ", ,", " \t"]) +def test_invalid_debug_override(value): + with pytest.raises(ValueError): + SELECTOR.select_amd_test_suites( + branch="feature", + labels=(), + debug_test_yaml=value, + )