From a9e5932a1c8e1ef4be159e21f925945fe91ad869 Mon Sep 17 00:00:00 2001 From: yxf Date: Wed, 16 Sep 2026 14:40:37 +0800 Subject: [PATCH] feat: record probed dependency versions in runner.py.json - probe the installed versions of a whitelisted set of Python packages inside every built image, and export them from the same buildx build - merge them into runner.py.json, refreshing them after a post operation - filter runners by PEP 440 version range on all three query entrypoints - guard the Dockerfile wiring with a test, since no pull request builds an image Signed-off-by: yxf --- .github/workflows/ci.yml | 6 +- .github/workflows/pack.yml | 110 +++- README.md | 149 +++++- gpustack_runner/runner.py | 155 ++++++ pack/.post_operation/README.md | 55 ++ pack/cann/Dockerfile.sglang | 22 + pack/cann/Dockerfile.vllm | 22 + pack/corex/Dockerfile | 22 + pack/cuda/Dockerfile.sglang | 22 + pack/cuda/Dockerfile.vllm | 22 + pack/dependencies.json | 22 + pack/dtk/Dockerfile.sglang | 22 + pack/dtk/Dockerfile.vllm | 22 + pack/hggc/Dockerfile.sglang | 22 + pack/hggc/Dockerfile.vllm | 22 + pack/maca/Dockerfile.sglang | 22 + pack/maca/Dockerfile.vllm | 22 + pack/merge_runner.sh | 150 +++++- pack/musa/Dockerfile | 44 ++ pack/rocm/Dockerfile.sglang | 22 + pack/rocm/Dockerfile.vllm | 22 + pack/shared/probe_dependencies.sh | 145 ++++++ .../test_list_runners_by_dependencies.json | 86 ++++ tests/gpustack_runner/test_runner.py | 484 +++++++++++++++++- tests/pack/test_dependencies.py | 76 +++ tests/pack/test_merge_runner.py | 214 ++++++++ tests/pack/test_probe_wiring.py | 361 +++++++++++++ 27 files changed, 2314 insertions(+), 29 deletions(-) create mode 100644 pack/dependencies.json create mode 100755 pack/shared/probe_dependencies.sh create mode 100644 tests/gpustack_runner/fixtures/test_list_runners_by_dependencies.json create mode 100644 tests/pack/test_dependencies.py create mode 100644 tests/pack/test_merge_runner.py create mode 100644 tests/pack/test_probe_wiring.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5d56220e..80f3c3d9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,13 +14,16 @@ defaults: on: workflow_dispatch: + # `pack/**` is deliberately NOT ignored here or below: it is the only path a + # broken dependency-probe wiring can arrive through, and the guard that catches + # it is tests/pack/test_probe_wiring.py. Ignoring it buys back a few runner + # minutes and gives up the guard entirely. push: branches: - "main" - "v*-dev" paths-ignore: - "!.github/workflows/ci.yml" - - "pack/**" - "docs/**" - "tools/**" - "**.md" @@ -36,7 +39,6 @@ on: - "v*-dev" paths-ignore: - "!.github/workflows/ci.yml" - - "pack/**" - "docs/**" - "tools/**" - "**.md" diff --git a/.github/workflows/pack.yml b/.github/workflows/pack.yml index d20c0568..ddbbf56e 100644 --- a/.github/workflows/pack.yml +++ b/.github/workflows/pack.yml @@ -221,11 +221,40 @@ jobs: DOCKER_CONTEXT="${{ github.workspace }}/pack${{ github.event.inputs.post_operation && format('/.post_operation/{0}', github.event.inputs.post_operation) || '' }}/${{ matrix.backend }}" echo "docker_context=${DOCKER_CONTEXT}" >> $GITHUB_OUTPUT - if [[ -f ${DOCKER_CONTEXT}/Dockerfile.${{ matrix.service }} ]]; then - echo "docker_file=${DOCKER_CONTEXT}/Dockerfile.${{ matrix.service }}" >> $GITHUB_OUTPUT - else - echo "docker_file=${DOCKER_CONTEXT}/Dockerfile" >> $GITHUB_OUTPUT + DOCKER_FILE="${DOCKER_CONTEXT}/Dockerfile" + if [[ -f ${DOCKER_FILE}.${{ matrix.service }} ]]; then + DOCKER_FILE="${DOCKER_FILE}.${{ matrix.service }}" fi + echo "docker_file=${DOCKER_FILE}" >> $GITHUB_OUTPUT + + # Report whether this Dockerfile exposes the dependency export stage. + # + # Only post operations are gated on this. The ones predating dependency + # probing have no `-deps` stage, and exporting it would fail the + # run -- after the image was already pushed. Skipping leaves the recorded + # dependencies stale but never wrong, since `pack/merge_runner.sh` only + # updates the entries it actually probed. The normal path is never gated, + # so a backend Dockerfile that drops the stage still fails loudly. + HAS_DEPENDENCIES_STAGE=false + if grep -qiE "^[[:space:]]*FROM[[:space:]]+.*[[:space:]]+AS[[:space:]]+${{ matrix.service }}-deps([[:space:]]|#|$)" "${DOCKER_FILE}"; then + HAS_DEPENDENCIES_STAGE=true + fi + echo "[INFO]: dependency export stage '${{ matrix.service }}-deps' present in ${DOCKER_FILE}: ${HAS_DEPENDENCIES_STAGE}" + echo "has_dependencies_stage=${HAS_DEPENDENCIES_STAGE}" >> $GITHUB_OUTPUT + + # Flatten the whitelist into the build argument consumed by + # pack/shared/probe_dependencies.sh. An empty one must fail the job here: + # the script treats empty as "skip probing", the escape hatch for manual + # local builds, which in CI would silently ship an unprobed image. + DEPENDENCY_PACKAGES="$(jq -er '[.[][]] | join(" ")' "${{ github.workspace }}/pack/dependencies.json")" + # Match on "has a non-whitespace character" rather than stripping spaces: + # `${VAR// /}` strips spaces only, so a tab would pass as non-empty. + if [[ ! "${DEPENDENCY_PACKAGES}" =~ [^[:space:]] ]]; then + echo "[ERROR]: empty dependency whitelist from 'pack/dependencies.json'" >&2 + exit 1 + fi + echo "[INFO]: dependency packages ($(echo "${DEPENDENCY_PACKAGES}" | wc -w | tr -d ' ')): ${DEPENDENCY_PACKAGES}" + echo "dependency_packages=${DEPENDENCY_PACKAGES}" >> $GITHUB_OUTPUT - name: Package timeout-minutes: 360 uses: docker/build-push-action@v6 @@ -242,16 +271,74 @@ jobs: sbom: false context: ${{ steps.metadata.outputs.docker_context }} file: ${{ steps.metadata.outputs.docker_file }} + # Carries pack/shared/, which every backend Dockerfile mounts to reach + # the probe script. The build context itself stays pack//. + build-contexts: | + shared=${{ github.workspace }}/pack/shared platforms: ${{ matrix.platform }} target: ${{ matrix.service }} tags: | ${{ steps.metadata.outputs.tags }} build-args: | ${{ steps.metadata.outputs.build_args }} + DEPENDENCY_PACKAGES=${{ steps.metadata.outputs.dependency_packages }} cache-from: | ${{ steps.metadata.outputs.cache_from }} cache-to: | ${{ steps.metadata.outputs.cache_to }} + # Pull the probed versions out of the build that just ran: a second build of + # the same Dockerfile targeting the `FROM scratch` export stage, because a + # build can only have one target. It runs on the same builder, so it hits the + # Package step's cache instead of re-running the business layers, and needs no + # `docker pull`. It exports no cache -- Package already wrote it. + # + # A dry run runs it as a `check` call, which is the only thing that verifies + # the `-deps` stage exists and is well formed. That produces no file + # and uploads nothing. + - name: Export Dependencies + if: ${{ github.event.inputs.post_operation == '' || steps.metadata.outputs.has_dependencies_stage == 'true' }} + uses: docker/build-push-action@v6 + with: + allow: | + network.host + security.insecure + call: ${{ github.event.inputs.dry_run == 'true' && 'check' || 'build' }} + # Must stay byte-identical to the Package step: these land in the LLB + # definition, so a mismatch changes every following RUN's digest, misses + # the cache Package just populated, and exports a dependencies.json + # describing an image that was never pushed. Change one, change both. + ulimit: | + nofile=65536:65536 + shm-size: '16G' + push: false + provenance: false + sbom: false + context: ${{ steps.metadata.outputs.docker_context }} + file: ${{ steps.metadata.outputs.docker_file }} + build-contexts: | + shared=${{ github.workspace }}/pack/shared + platforms: ${{ matrix.platform }} + target: ${{ matrix.service }}-deps + build-args: | + ${{ steps.metadata.outputs.build_args }} + DEPENDENCY_PACKAGES=${{ steps.metadata.outputs.dependency_packages }} + cache-from: | + ${{ steps.metadata.outputs.cache_from }} + outputs: ${{ github.event.inputs.dry_run == 'false' && format('type=local,dest={0}/dependencies', runner.temp) || '' }} + # The artifact name carries the platform tag, unique per (platform, tag) + # within a run: that is the key `merge_runner.sh` relates probes back with. + - name: Upload Dependencies + if: ${{ github.event.inputs.dry_run == 'false' && (github.event.inputs.post_operation == '' || steps.metadata.outputs.has_dependencies_stage == 'true') }} + uses: actions/upload-artifact@v4 + with: + name: dependencies-${{ matrix.platform_tag }} + path: ${{ runner.temp }}/dependencies/dependencies.json + # `upload-artifact@v4` answers a duplicate name with a non-retryable 409. + # `build` carries `timeout-minutes: 360`, so "Re-run all jobs" is routine + # here, and without this every re-run leg would fail after pushing. + overwrite: true + if-no-files-found: error + retention-days: 1 # Merge all architecture images into manifest list. manifest: @@ -288,8 +375,14 @@ jobs: done # Submit a PR to merge the runner. + # + # A post operation runs through here as well: it mutates a released image in + # place, so the dependency versions recorded for that tag have to follow. + # `for_release == 'true'` stays required, and doubles as the guard for it: a + # post operation is by definition applied to released tags, and without the + # flag every tag would carry the `-dev` suffix and address no released entry. merge-runner: - if: ${{ github.event.inputs.dry_run == 'false' && github.event.inputs.for_release == 'true' && github.event.inputs.post_operation == '' }} + if: ${{ github.event.inputs.dry_run == 'false' && github.event.inputs.for_release == 'true' }} needs: - expand-matrix - manifest @@ -300,9 +393,16 @@ jobs: with: fetch-depth: 1 persist-credentials: false + - name: Download Dependencies + uses: actions/download-artifact@v4 + with: + pattern: dependencies-* + path: ${{ runner.temp }}/dependencies - name: Merge Runner env: INPUT_BUILD_JOBS: ${{ needs.expand-matrix.outputs.build_jobs }} + INPUT_DEPENDENCIES_DIR: ${{ runner.temp }}/dependencies + INPUT_POST_OPERATION: ${{ github.event.inputs.post_operation }} run: ${{ github.workspace }}/pack/merge_runner.sh - name: Generate Pull Request Token id: generate-prt diff --git a/README.md b/README.md index debd1409..0bce5f41 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,7 @@ backends. - [Directory Structure](#directory-structure) - [Dockerfile Convention](#dockerfile-convention) - [Docker Image Naming Convention](#docker-image-naming-convention) +- [Dependency Versions](#dependency-versions) - [Integration Process](#integration-process) ## Onboard Services @@ -196,6 +197,36 @@ ENTRYPOINT [ "tini", "--" ] ``` +### Example Build Command + +Each Dockerfile is built with the backend directory as its build context: + +```bash +cd pack/cuda + +docker buildx build \ + --file Dockerfile.vllm \ + --target vllm \ + --build-context shared=../shared \ + --build-arg DEPENDENCY_PACKAGES="$(jq -er '[.[][]] | join(" ")' ../dependencies.json)" \ + --tag gpustack/runner:cuda13.0-vllm0.29.0 \ + . +``` + +Two of these flags are mandatory and one is optional: + +- `--target {SERVICE}` is **mandatory**. Every Dockerfile ends with an export-only + `FROM scratch AS {SERVICE}-deps` stage that carries nothing but the probed dependency manifest. Without + `--target`, Docker builds the *last* stage in the file and hands back an empty image. +- `--build-context shared=../shared` is **mandatory** as well. The dependency probe script lives in + [pack/shared](pack/shared) so that all backends share one copy, and the build context of `pack/{BACKEND}/` + cannot reach it with a plain `COPY`; a named build context is the only way in. Omitting the flag makes the + `RUN --mount=type=bind,from=shared` step resolve `shared` as an image name and fail the build. +- `--build-arg DEPENDENCY_PACKAGES=...`, in contrast, is optional and is the escape hatch for manual local + builds. Leaving it empty skips probing, and the resulting image then has **no** + `/etc/gpustack-runner/dependencies.json` — which is also how the data pipeline tells "never probed" apart + from "probed, nothing installed". + ## Docker Image Naming Convention The Docker image naming convention is as follows: @@ -225,6 +256,115 @@ The Docker image naming convention is as follows: `gpustack/runner:cann8.1-910b-vllm0.9.2-dev`. 3. After testing, rename the multi-architecture image to the final tag, e.g. `gpustack/runner:cann8.1-910b-vllm0.9.2`. +## Dependency Versions + +Besides the image tag, each entry of [runner.py.json](gpustack_runner/runner.py.json) may carry a +`dependencies` map, which records the versions of a whitelisted set of Python packages **as actually +installed in the built image**, not as declared by the Dockerfile `ARG`s. The whitelist lives in +[pack/dependencies.json](pack/dependencies.json) and the probe runs at build time. + +```json +{ + "docker_image": "gpustack/runner:cann9.1-a3-vllm0.23.0", + "dependencies": { + "lmcache": "0.4.3", + "lmcache-ascend": "0.4.3", + "ray": "2.54.0", + "torch": "2.10.0", + "torch-npu": "2.10.0rc1", + "vllm-ascend": "0.23.0" + } +} +``` + +### One Name, Several Distributions + +Keys are the dependency names of the whitelist. Most map one to one onto a distribution, but a name may +cover several, highest priority first: + +```json +{ "mooncake-transfer-engine": ["mooncake-transfer-engine-npu", "mooncake-transfer-engine-rocm", "mooncake-transfer-engine"] } +``` + +The probe reports raw distribution names, and `pack/merge_runner.sh` folds them onto the dependency name — +the first distribution of the list that is installed wins — so a consumer asking about +`mooncake-transfer-engine` never has to know the accelerator naming conventions. + +Two conditions must both hold before grouping distributions under one name: + +1. **They must be mutually exclusive** — at most one of them can be installed in any given image. Folding + keeps a single winner, so grouping distributions that *coexist* silently discards one of them. `torch` + and `torch-npu` look like such a pair by their names, but `torch-npu` pins `torch==` and is + the NPU backend *on top of* it: both are installed, with different versions that mean different things. + The same holds for `lmcache` and `lmcache-ascend` — a CANN image carries both, and they do not even track + the same version. +2. **Their versions must be comparable** — the same versioning scheme, ideally the same release line. A + grouped name yields one specifier for all of them, so a specifier that is meaningful for one and + meaningless for another gives a confidently wrong answer. `sglang-kernel` and `sgl-kernel-npu` *are* + mutually exclusive, yet they stay separate: the former is `0.4.6.post1` and the latter is a date version + `2026.6.1`, so `>=0.4.5` would match the NPU build for no reason at all. + +`mooncake-transfer-engine` satisfies both, which is why it is the one grouped entry: its `-npu`, `-rocm` and +generic builds are one per platform and share a release line (`0.3.11.post1` / `0.3.10.post2`). + +When in doubt, give each distribution its own name. That records both facts and asserts nothing. + +The raw, unfolded probe result stays inside the image at `/etc/gpustack-runner/dependencies.json`, so +`docker run --rm cat /etc/gpustack-runner/dependencies.json` still shows every distribution and +version for troubleshooting. + +### Absent Field vs. Absent Key + +The field has two levels of meaning, and conflating them leads to wrong conclusions: + +| State | Meaning | +|---------------------------------------|-------------------------------------------------------------------------------------| +| `dependencies` is absent | The image was **never probed** — built before probing existed, or built without the whitelist | +| `dependencies` is a map missing a key | The image **was** probed and the package is **not installed** | + +### Querying + +All three query entries — `list_runners`, `list_backend_runners` and `list_service_runners` — accept a +`dependencies` argument: a tuple of `(dependency name, PEP 440 specifier)` pairs, matched directly against +the `dependencies` map of each entry. + +```python +# Images whose lmcache is new enough. +list_runners(service="vllm", dependencies=(("lmcache", ">=0.4.6"),)) + +# Multiple conditions. +list_runners(backend="cuda", dependencies=(("lmcache", ">=0.4.6"), ("torch", ">=2.9"),)) + +# An empty specifier asks only whether the package is installed. +list_runners(backend="cuda", dependencies=(("vllm-omni", ""),)) + +# Strict mode: drop images that were never probed. +list_runners( + backend="cuda", + dependencies=(("lmcache", ">=0.4.6"),), + with_unknown_dependencies=False, +) +``` + +Five behaviors to keep in mind: + +1. **Conditions are ANDed.** A runner must satisfy every pair to be returned; there is no "any of" form. +2. **Unprobed runners are kept by default.** A runner without a `dependencies` field is never filtered out by + a dependency condition, so that adding a condition does not make every pre-existing image disappear at + once. Pass `with_unknown_dependencies=False` to tighten this to "only runners known to satisfy the + condition" — expect a much shorter list until the fleet has been rebuilt. +3. **Pre-releases match.** Matching is done with `prereleases=True`, because `rc` versions are routine here + (`vllm-ascend 0.20.2rc1`, `sglang 0.5.10rc0`); without it `>=0.20.0` would silently skip `0.20.2rc1`. Note + the converse, which is **not** a bug: `>=0.4.6` does not match `0.4.6rc1`, because PEP 440 orders + `0.4.6rc1 < 0.4.6`. The same rule applies to `dev` versions — `0.27.0rc2.dev25+g` does not satisfy + `>=0.27.0`. Write the bound you actually mean (`>=0.4.6rc1`) instead of "fixing" the comparison. +4. **An empty specifier is a pure existence check.** `("vllm-omni", "")` matches any image that has the + package, whatever its version. This is the right form for a package installed from a commit, whose + version string carries no information — comparing it would give a confidently wrong answer. +5. **An unknown name matches nothing; it does not raise.** The whitelist is a build-side file and is not + shipped with the library, so there is nothing to check a name against. A misspelled name simply yields an + empty result, which is indistinguishable from "no image qualifies" — callers own their spelling. + ## Integration Process ### Ingesting a New Accelerated Backend @@ -246,7 +386,14 @@ To add support for a new inference service: 2. Update [pack.yml](.github/workflows/pack.yml) to include the new service in the build matrix. 3. Update [matrix.yml](pack/matrix.yaml) to include the new service. 4. Update `_RE_DOCKER_IMAGE` in [runner.py](gpustack_runner/runner.py) to recognize the new service. -5. [Optional] Update [tests](tests/gpustack_runner) if necessary. +5. Review [pack/dependencies.json](pack/dependencies.json) for the key packages the new service brings + in. A package that is not listed there is never probed for any image, and consumers have no way to ask + about it. The bar for listing one is **it directly decides whether a model or the inference backend + starts** *and* **it is updated often or breaks compatibility** — every entry is recorded for every image, + so the list is meant to stay short. Before grouping accelerator-specific variants under one name, confirm + they are mutually exclusive — see [One Name, Several Distributions](#one-name-several-distributions); + when in doubt, give each its own name, which records both facts and asserts nothing. +6. [Optional] Update [tests](tests/gpustack_runner) if necessary. ## License diff --git a/gpustack_runner/runner.py b/gpustack_runner/runner.py index bdb0215d..b9a7c378 100644 --- a/gpustack_runner/runner.py +++ b/gpustack_runner/runner.py @@ -9,6 +9,7 @@ from typing import Any from dataclasses_json import dataclass_json +from packaging.specifiers import InvalidSpecifier, SpecifierSet from packaging.version import InvalidVersion, Version _RE_DOCKER_IMAGE = re.compile( @@ -206,6 +207,22 @@ class Runner: """ Deprecated runner or not. """ + dependencies: dict[str, str] | None = field( + default=None, + metadata={"dataclasses_json": {"exclude": lambda v: v is None}}, + ) + """ + The versions of whitelisted packages installed in the image, probed at build + time rather than declared in the Dockerfile. Keyed by the dependency names of + `pack/dependencies.json` and sorted; a name covering several accelerator + variants is already resolved to the one that applies. An absent key means the + package is not installed; the field being ``None`` means the image was never + probed. + + Do not mutate in place: `list_runners` is ``@lru_cache``-decorated and hands + out the same ``Runner`` instances on every call, so an edit corrupts the + process-wide cache for every future caller. + """ Runners = list[Runner] @@ -228,6 +245,106 @@ class Runner: """ +def _resolve_dependency_conditions( + conditions: tuple[tuple[str, str], ...], +) -> tuple[tuple[str, SpecifierSet], ...]: + """ + Validates dependency conditions and compiles their version specifiers. + + Compiling up front, rather than per runner entry, keeps an invalid specifier + an error even when no entry would be evaluated against it. + + Args: + conditions: + A tuple of `(dependency name, PEP 440 specifier)` pairs. + + Returns: + A tuple of `(dependency name, specifier set)` pairs. + + Raises: + ValueError: + If a pair is not a 2-item `(dependency name, specifier)` sequence, + or if a specifier is invalid. + + """ + resolved: list[tuple[str, SpecifierSet]] = [] + for condition in conditions: + if not isinstance(condition, (tuple, list)) or len(condition) != 2: + errmsg = ( + f"Invalid dependency condition {condition!r}: expected a " + f"(dependency name, PEP 440 specifier) pair." + ) + raise ValueError(errmsg) + name, specifier = condition + # A missing specifier means "no version constraint", matching how every + # other `list_runners` filter treats `None`. It is also how to ask + # whether a package is installed at all, whatever its version. + if specifier is None: + specifier = "" + try: + # Pre-releases are the norm here (``vllm-ascend 0.20.2rc1``), and the + # default would drop every image carrying one. + specifier_set = SpecifierSet(specifier, prereleases=True) + except InvalidSpecifier as e: + errmsg = f"Invalid specifier {specifier!r} for dependency {name!r}." + raise ValueError(errmsg) from e + resolved.append((name, specifier_set)) + + return tuple(resolved) + + +def _match_dependencies( + dependencies: dict[str, str] | None, + resolved_conditions: tuple[tuple[str, SpecifierSet], ...], + with_unknown_dependencies: bool, +) -> bool: + """ + Reports whether the probed dependencies of a runner satisfy all conditions. + + Args: + dependencies: + The probed dependency map of a runner, or None if the runner + predates dependency probing. + resolved_conditions: + The conditions returned by `_resolve_dependency_conditions`. + with_unknown_dependencies: + Whether to keep runners that have never been probed. + + Returns: + True if the runner satisfies every condition. + + """ + if not resolved_conditions: + return True + + # An absent map means "never probed"; an empty one means "probed, nothing + # whitelisted installed". Only the former is treated leniently, so this must + # check None rather than emptiness. + if dependencies is None: + return with_unknown_dependencies + + for name, specifier_set in resolved_conditions: + version = dependencies.get(name) + if version is None: + return False + # An empty specifier only asks whether the package is installed, which + # is already answered. Short-circuit before parsing, so a version string + # that is not PEP 440 does not turn an existence check into a miss. + if not specifier_set: + continue + try: + matched = specifier_set.contains(version) + except InvalidVersion: + # A recorded version that cannot be parsed can never be shown to + # satisfy a range, so it is a miss rather than a crash of the whole + # query. `version_sort_key` makes the same call for the same reason. + matched = False + if not matched: + return False + + return True + + def convert_runners_to_dict(runners: Runners) -> list[dict]: """ Converts a list of Runner objects to a list of dictionaries. @@ -255,6 +372,14 @@ def list_runners(**kwargs) -> Runners | list[dict]: - `data_path`: The path to the JSON data file. If not provided, uses the default data file. - `todict`: If True, returns a list of dictionaries instead of Runner objects. - `with_deprecated`: Whether to include deprecated runners, default is True. + - `dependencies`: A tuple of `(dependency name, PEP 440 specifier)` pairs, + e.g. `(("lmcache", ">=0.4.6"),)`, ANDed. Must be a tuple, not a list: it is + hashed by `@lru_cache`. Names are the keys of a runner's `dependencies` map; + an unknown one matches nothing rather than raising. An empty specifier asks + only whether the package is installed. See the Dependency Versions section + of README.md. Default is None. + - `with_unknown_dependencies`: Whether to keep runners that were never + dependency-probed, default is True. A no-op without `dependencies`. - `backend`: The backend name, default is None. - `backend_version`: The backend version, default is None. - `backend_version_prefix`: The prefix of the backend version, default is None. @@ -287,6 +412,14 @@ def list_runners(**kwargs) -> Runners | list[dict]: if with_deprecated is None: with_deprecated = True + with_unknown_dependencies = kwargs.pop("with_unknown_dependencies", True) + if with_unknown_dependencies is None: + with_unknown_dependencies = True + + resolved_dependencies = _resolve_dependency_conditions( + kwargs.pop("dependencies", None) or (), + ) + allowed_keys = { "backend", "backend_version", @@ -326,6 +459,12 @@ def list_runners(**kwargs) -> Runners | list[dict]: if match: if not with_deprecated and item.deprecated: continue + if not _match_dependencies( + item.dependencies, + resolved_dependencies, + with_unknown_dependencies, + ): + continue results.append(item) return convert_runners_to_dict(results) if todict else results @@ -668,6 +807,14 @@ def list_backend_runners(**kwargs) -> BackendRunners | list[dict]: - `data_path`: The path to the JSON data file. If not provided, uses the default data file. - `todict`: If True, returns a list of dictionaries instead of BackendRunner objects. - `with_deprecated`: Whether to include deprecated runners, default is True. + - `dependencies`: A tuple of `(dependency name, PEP 440 specifier)` pairs, + e.g. `(("lmcache", ">=0.4.6"),)`, ANDed. Must be a tuple, not a list: it is + hashed by `@lru_cache`. Names are the keys of a runner's `dependencies` map; + an unknown one matches nothing rather than raising. An empty specifier asks + only whether the package is installed. See the Dependency Versions section + of README.md. Default is None. + - `with_unknown_dependencies`: Whether to keep runners that were never + dependency-probed, default is True. A no-op without `dependencies`. - `backend`: The backend name, default is None. - `backend_version`: The backend version, default is None. - `backend_version_prefix`: The prefix of the backend version, default is None. @@ -838,6 +985,14 @@ def list_service_runners(**kwargs) -> ServiceRunners | list[dict]: - `data_path`: The path to the JSON data file. If not provided, uses the default data file. - `todict`: If True, returns a list of dictionaries instead of ServiceRunner objects. - `with_deprecated`: Whether to include deprecated runners, default is True. + - `dependencies`: A tuple of `(dependency name, PEP 440 specifier)` pairs, + e.g. `(("lmcache", ">=0.4.6"),)`, ANDed. Must be a tuple, not a list: it is + hashed by `@lru_cache`. Names are the keys of a runner's `dependencies` map; + an unknown one matches nothing rather than raising. An empty specifier asks + only whether the package is installed. See the Dependency Versions section + of README.md. Default is None. + - `with_unknown_dependencies`: Whether to keep runners that were never + dependency-probed, default is True. A no-op without `dependencies`. - `backend`: The backend name, default is None. - `backend_version`: The backend version, default is None. - `backend_version_prefix`: The prefix of the backend version, default is None. diff --git a/pack/.post_operation/README.md b/pack/.post_operation/README.md index d47db748..f56bfc54 100644 --- a/pack/.post_operation/README.md +++ b/pack/.post_operation/README.md @@ -9,6 +9,61 @@ However, for some needs, we have to modify the image's content while preserving We leverage the matrix expansion feature of GPUStack Runner to achieve this, and document here the operations we perform. +## Requirements for a New Operation + +`gpustack_runner/runner.py.json` records, per tag, the versions of the whitelisted +packages that the image ships. An operation rewrites the content behind an already +released tag, so one that touches the Python environment without probing it again +leaves those versions describing the image as it was *before* the operation. + +Every **new** operation's Dockerfile must therefore end with the dependency probe and +expose the export stage. Copy the shape verbatim from `pack//Dockerfile*`: + +```dockerfile +FROM gpustack/runner: AS vllm + +# ... the operation itself ... + +## Probe Dependencies + +ARG DEPENDENCY_PACKAGES="" +RUN --mount=type=bind,from=shared,source=probe_dependencies.sh,target=/tmp/probe_dependencies.sh \ + DEPENDENCY_PACKAGES="${DEPENDENCY_PACKAGES}" bash /tmp/probe_dependencies.sh + +## Entrypoint + +WORKDIR / +ENTRYPOINT [ "tini", "--" ] + +## Export Dependencies + +FROM scratch AS vllm-deps + +COPY --from=vllm /etc/gpustack-runner/dependencies.json / +``` + +Replace `vllm` with the service target of the operation. The export stage name must be +exactly `-deps`: `pack.yml` skips the export when it cannot find that stage, +so a misspelled name silently ships without refreshing the recorded versions. +`probe_dependencies.sh` arrives through the named build context `shared`, which +`pack.yml` supplies for post operations too, so nothing else needs wiring up. + +Running such an operation: + +- Run it with `for_release=true`. Otherwise every tag carries the `-dev` suffix and + addresses no released entry. +- `merge_runner.sh` then **only updates the `dependencies` of the entries that already + exist**; it never adds an entry, and never touches any other field. A probed tag that + does not address exactly one existing entry fails the job, printing the `platform`, + `docker_image` and `platform_tag` it looked for. +- The run opens the usual `chore: update runner` pull request, which also regenerates + `tests/gpustack_runner/fixtures/`. A fixture diff unrelated to the operation is + therefore possible and not, by itself, a sign that something went wrong. + +The operations recorded below predate this requirement and are deliberately left +unchanged. They do not refresh `dependencies`: the recorded versions for the tags they +mutated stay as they were until the next release rebuilds those images. + - [x] 2025-10-20: Install `lmcache` package for CANN/CUDA/ROCm released images. - [x] 2025-10-22: Install `ray[client]` package for CANN/CUDA/ROCm released images. - [x] 2025-10-22: Install `ray[default]` package for CUDA/ROCm released images. diff --git a/pack/cann/Dockerfile.sglang b/pack/cann/Dockerfile.sglang index 4126da79..ee4e9839 100644 --- a/pack/cann/Dockerfile.sglang +++ b/pack/cann/Dockerfile.sglang @@ -270,6 +270,18 @@ RUN <", and a platform tag +# is unique per (platform, tag) within a run, so it addresses exactly one record. +PROBED_DEPENDENCIES="{}" +if [[ -n "${INPUT_DEPENDENCIES_DIR}" ]]; then + for DEPENDENCIES_FILE in "${INPUT_DEPENDENCIES_DIR}"/dependencies-*/dependencies.json; do + [[ -f "${DEPENDENCIES_FILE}" ]] || continue + PLATFORM_TAG="$(basename "$(dirname "${DEPENDENCIES_FILE}")")" + PROBED_DEPENDENCIES="$(echo "${PROBED_DEPENDENCIES}" | jq -cr \ + --arg platform_tag "${PLATFORM_TAG#dependencies-}" \ + --slurpfile probed "${DEPENDENCIES_FILE}" \ + '.[$platform_tag] = $probed[0]')" + done +fi + +# Fold the probe results onto the dependency names of `pack/dependencies.json`. +# +# A probe reports raw distribution names, and one dependency may ship under +# several of them (``lmcache-ascend`` on CANN, ``lmcache`` elsewhere). The first +# name of the list that is installed wins, so `runner.py.json` records one +# version per dependency and the query side needs no second lookup. A dependency +# no image installed is simply absent, like any other uninstalled package. +# `-S` keeps the folded map sorted whatever order the names file is in, so the +# `runner.py.json` diff stays readable. +PROBED_DEPENDENCIES="$(echo "${PROBED_DEPENDENCIES}" | jq -cSr \ + --slurpfile dependencies "${INPUT_DEPENDENCIES_FILE}" \ + '$dependencies[0] as $names + | map_values( + . as $probed + | reduce ($names | to_entries[]) as $entry ({}; + ($entry.value | map(select($probed[.] != null)) | first) as $hit + | if $hit == null then . else . + {($entry.key): $probed[$hit]} end + ) + )')" + +# Review the probed dependencies. +echo "[INFO] Probed Dependencies:" +jq -r '.' <<<"${PROBED_DEPENDENCIES}" # Load existing runners if exists. ORIGINAL_RUNNERS="[]" @@ -39,11 +65,95 @@ if [[ -f "${OUTPUT_FILE}" ]]; then ORIGINAL_RUNNERS="$(jq -cr '.' "${OUTPUT_FILE}")" fi -# Merge new runners with original runners, and distinct by docker_image. -MERGED_RUNNERS="$(echo "${NEW_RUNNERS}" "${ORIGINAL_RUNNERS}" | jq -cs 'add | unique_by(.platform + .docker_image)')" - -# Normalize the merged runners by sorting them. -MERGED_RUNNERS="$(echo "${MERGED_RUNNERS}" | jq -cr 'sort_by([.backend, (.backend_variant | explode | map(-.)), (.backend_version | explode | map(-.)), .service, (.service_version | split(".") | map(tonumber?) | map(-.))])')" +if [[ -n "${INPUT_POST_OPERATION}" ]]; then + # Post operation mode: update in place, never add. + # + # An operation mutates an already released tag, so the only field it may + # change is the `dependencies` of the entries already describing those tags. + # Its matrix is pruned to just those tags -- building entries from it, the way + # the normal path does, would invent entries for tags that were never released + # and rewrite the other fields from a non-authoritative source. Nothing is + # reconstructed here, so nothing can be lost, and the normal path's carry-over + # step needs no counterpart. + + # Relate each probe result back to the entry it belongs to. + PROBED_ENTRIES="$(echo "${INPUT_BUILD_JOBS}" | jq -cr \ + --arg namespace "${INPUT_NAMESPACE}" \ + --arg repository "${INPUT_REPOSITORY}" \ + --argjson dependencies "${PROBED_DEPENDENCIES}" \ + '[.[] + | select($dependencies[.platform_tag] != null) + | { + platform_tag: .platform_tag, + platform: .platform, + docker_image: ($namespace + "/" + $repository + ":" + .tag), + dependencies: $dependencies[.platform_tag], + }]')" + + # Fail on anything that does not address exactly one existing entry. Only the + # jobs that actually probed are checked: one without a probe result has no + # data to write, so it can neither create an entry nor corrupt one. + UNLOCATABLE="$(jq -cn \ + --argjson probed "${PROBED_ENTRIES}" \ + --argjson original "${ORIGINAL_RUNNERS}" \ + '[$probed[] + | . as $p + | . + {matched: ([$original[] + | select(.platform == $p.platform and .docker_image == $p.docker_image)] | length)} + | select(.matched != 1)]')" + if [[ "$(echo "${UNLOCATABLE}" | jq -r 'length')" -ne 0 ]]; then + echo "[ERROR] Post operation '${INPUT_POST_OPERATION}': $(echo "${UNLOCATABLE}" | jq -r 'length') probed image(s) do not address exactly one existing entry of '${OUTPUT_FILE}'." >&2 + echo "${UNLOCATABLE}" | jq -r '.[] | "[ERROR] platform_tag=\"\(.platform_tag)\" platform=\"\(.platform)\" docker_image=\"\(.docker_image)\" matched=\(.matched)"' >&2 + echo "[ERROR] A post operation may only address already released tags. Check the tags of the operation's matrix.yaml, and make sure the workflow ran with for_release=true: otherwise the tags carry the '-dev' suffix and can never address a released entry." >&2 + exit 1 + fi + + # Update the addressed entries, and only their `dependencies`. + MERGED_RUNNERS="$(jq -cn \ + --argjson probed "${PROBED_ENTRIES}" \ + --argjson original "${ORIGINAL_RUNNERS}" \ + '($probed | INDEX([.platform, .docker_image] | tostring)) as $index + | $original + | map(($index[[.platform, .docker_image] | tostring] | .dependencies) as $d + | if $d then . + {dependencies: $d} else . end)')" + + echo "[INFO] Updated Runners: $(echo "${PROBED_ENTRIES}" | jq -r 'length') of $(echo "${ORIGINAL_RUNNERS}" | jq -r 'length')" +else + # Construct new runners from the input build jobs. + NEW_RUNNERS="$(echo "${INPUT_BUILD_JOBS}" | jq -cr \ + --arg namespace "${INPUT_NAMESPACE}" \ + --arg repository "${INPUT_REPOSITORY}" \ + --argjson dependencies "${PROBED_DEPENDENCIES}" \ + '.[] | { + backend: .backend, + backend_version: .backend_version, + original_backend_version: .original_backend_version, + backend_variant: .backend_variant, + service: .service, + service_version: .service_version, + platform: .platform, + docker_image: ($namespace + "/" + $repository + ":" + .tag), + deprecated: (.deprecated // false), + } + (if $dependencies[.platform_tag] then {dependencies: $dependencies[.platform_tag]} else {} end)' | jq -cs .)" + + # Carry over the dependencies of the entries this build did not probe. + # Otherwise rebuilding an unprobed image would replace an entry that carries + # dependencies with one that does not, and an absent `dependencies` means + # "never probed" -- erasing it is a lie only a rebuild can undo. + NEW_RUNNERS="$(echo "${NEW_RUNNERS}" | jq -cr \ + --argjson original "${ORIGINAL_RUNNERS}" \ + '($original | INDEX([.platform, .docker_image] | tostring)) as $index + | map(if has("dependencies") then . + else . + (($index[[.platform, .docker_image] | tostring] | .dependencies) as $d + | if $d then {dependencies: $d} else {} end) + end)')" + + # Merge new runners with original runners, and distinct by docker_image. + MERGED_RUNNERS="$(echo "${NEW_RUNNERS}" "${ORIGINAL_RUNNERS}" | jq -cs 'add | unique_by([.platform, .docker_image])')" + + # Normalize the merged runners by sorting them. + MERGED_RUNNERS="$(echo "${MERGED_RUNNERS}" | jq -cr 'sort_by([.backend, (.backend_variant | explode | map(-.)), (.backend_version | explode | map(-.)), .service, (.service_version | split(".") | map(tonumber?) | map(-.))])')" +fi # Review the merged runners. echo "[INFO] Merged Runners:" diff --git a/pack/musa/Dockerfile b/pack/musa/Dockerfile index 0cd43788..d3574478 100644 --- a/pack/musa/Dockerfile +++ b/pack/musa/Dockerfile @@ -315,6 +315,18 @@ RUN < skip, exit 0, NO file written. A missing file +# means "never probed", mirroring an absent `dependencies` field, and makes +# the export stage fail loudly rather than export an empty map. +# - probed, nothing matched -> exit 0, OUTPUT_PATH contains `{}`. +# - probe failed -> exit 1, the build fails. +# +# The environment is enumerated once with `pip list` rather than one `pip show` +# per package: `pip show` exits non-zero both when a package is missing and when +# the tool itself is broken (e.g. `uv pip` outside a virtualenv), so it cannot +# tell "not installed" from "probe failed" and would silently report `{}`. + +set -eo pipefail + +OUTPUT_PATH="${1:-/etc/gpustack-runner/dependencies.json}" + +if [[ -z "${DEPENDENCY_PACKAGES// /}" ]]; then + echo "[INFO]: DEPENDENCY_PACKAGES is empty, skipping dependency probing" + exit 0 +fi + +# Resolve a probing tool. Fail loudly rather than emitting an empty map. +if command -v uv >/dev/null 2>&1; then + PROBE_TOOL="uv" + # Most images set this themselves, but not necessarily in the stage this + # script runs in: without it `uv pip` refuses to work outside a virtualenv. + export UV_SYSTEM_PYTHON=1 + LIST_CMD=(uv pip list --format=json) +elif command -v pip >/dev/null 2>&1; then + PROBE_TOOL="pip" + LIST_CMD=(pip list --format=json) +elif command -v pip3 >/dev/null 2>&1; then + PROBE_TOOL="pip3" + LIST_CMD=(pip3 list --format=json) +else + echo "[ERROR]: neither uv nor pip is available, cannot probe dependencies" >&2 + exit 1 +fi + +if ! command -v python3 >/dev/null 2>&1; then + echo "[ERROR]: python3 is not available, cannot probe dependencies" >&2 + exit 1 +fi + +echo "[INFO]: probing with '${PROBE_TOOL}'" + +# Capture stderr rather than discarding it: the usual trigger is uv/pip pointed +# at the wrong environment in this stage, which only shows up there. +LIST_STDERR_FILE="$(mktemp)" +LIST_EXIT=0 +INSTALLED_JSON="$("${LIST_CMD[@]}" 2>"${LIST_STDERR_FILE}")" || LIST_EXIT=$? +LIST_STDERR="$(cat "${LIST_STDERR_FILE}")" +rm -f "${LIST_STDERR_FILE}" + +if [[ ${LIST_EXIT} -ne 0 || -z "${INSTALLED_JSON}" ]]; then + echo "[ERROR]: '${LIST_CMD[*]}' failed (exit ${LIST_EXIT}), cannot probe dependencies" >&2 + if [[ -n "${LIST_STDERR}" ]]; then + echo "[ERROR]: stderr was:" >&2 + echo "${LIST_STDERR}" | sed 's/^/[ERROR]: /' >&2 + fi + exit 1 +fi + +mkdir -p "$(dirname "${OUTPUT_PATH}")" + +INSTALLED_JSON="${INSTALLED_JSON}" \ +DEPENDENCY_PACKAGES="${DEPENDENCY_PACKAGES}" \ +OUTPUT_PATH="${OUTPUT_PATH}" \ +python3 - <<'PYTHON' +import json +import os +import re +import sys + + +def normalize(name): + # PEP 503 normalization. + return re.sub(r"[-_.]+", "-", name).lower() + + +try: + enumerated = json.loads(os.environ["INSTALLED_JSON"]) +except ValueError as e: + print(f"[ERROR]: cannot parse the installed package list: {e}", file=sys.stderr) + sys.exit(1) + +installed = {normalize(item["name"]): item["version"] for item in enumerated} + +# A real runner image always has packages installed. An empty enumeration means +# the probing tool pointed at the wrong Python environment, which must not be +# reported as "nothing from the whitelist is installed". +if not installed: + print("[ERROR]: enumerated 0 installed packages, the probing tool is looking at the wrong environment", file=sys.stderr) + sys.exit(1) + +wanted = sorted({normalize(n) for n in os.environ["DEPENDENCY_PACKAGES"].split()}) + +# A whitespace-only value (e.g. a stray tab) slips past the bash-side empty +# check but yields zero names here, and would be written out as a plausible +# `{}`. Hard failure instead -- see the three outcomes in the header. +if not wanted: + print("[ERROR]: DEPENDENCY_PACKAGES is whitespace-only, no package names to probe", file=sys.stderr) + sys.exit(1) + +packages = {} +for name in wanted: + version = installed.get(name) + if version is None: + print(f"[INFO]: {name}: not installed") + continue + print(f"[INFO]: {name}: {version}") + packages[name] = version + +with open(os.environ["OUTPUT_PATH"], "w", encoding="utf-8") as f: + json.dump(packages, f, sort_keys=True) + f.write("\n") + +# Build-log only: `installed_total` answers "is this the environment that runs +# the service?", `probed`/`hit` answers "did a sane whitelist arrive?". +print(f"[INFO]: installed_total {len(installed)}, probed {len(wanted)}, hit {len(packages)}") +PYTHON + +echo "[INFO]: wrote '${OUTPUT_PATH}'" +cat "${OUTPUT_PATH}" +echo diff --git a/tests/gpustack_runner/fixtures/test_list_runners_by_dependencies.json b/tests/gpustack_runner/fixtures/test_list_runners_by_dependencies.json new file mode 100644 index 00000000..6d185fdb --- /dev/null +++ b/tests/gpustack_runner/fixtures/test_list_runners_by_dependencies.json @@ -0,0 +1,86 @@ +[ + { + "backend": "cann", + "backend_version": "9.1", + "original_backend_version": "9.1.0", + "backend_variant": "a3", + "service": "vllm", + "service_version": "0.23.0", + "platform": "linux/amd64", + "docker_image": "gpustack/runner:cann9.1-a3-vllm0.23.0", + "deprecated": false, + "dependencies": { + "lmcache": "0.5.4", + "torch-npu": "2.10.0rc1", + "vllm-ascend": "0.20.2rc1" + } + }, + { + "backend": "cuda", + "backend_version": "13.0", + "original_backend_version": "13.0.2", + "backend_variant": "", + "service": "vllm", + "service_version": "0.29.0", + "platform": "linux/amd64", + "docker_image": "gpustack/runner:cuda13.0-vllm0.29.0", + "deprecated": false, + "dependencies": { + "lmcache": "0.5.4", + "torch": "2.9.0", + "transformers": "4.58.2" + } + }, + { + "backend": "cuda", + "backend_version": "13.0", + "original_backend_version": "13.0.2", + "backend_variant": "", + "service": "sglang", + "service_version": "0.5.10", + "platform": "linux/amd64", + "docker_image": "gpustack/runner:cuda13.0-sglang0.5.10", + "deprecated": false, + "dependencies": { + "lmcache": "0.4.6rc1", + "sglang-kernel": "0.4.6rc0" + } + }, + { + "backend": "rocm", + "backend_version": "7.0", + "original_backend_version": "7.0.2", + "backend_variant": "", + "service": "vllm", + "service_version": "0.29.0", + "platform": "linux/amd64", + "docker_image": "gpustack/runner:rocm7.0-vllm0.29.0", + "deprecated": false, + "dependencies": { + "torch": "2.8.0" + } + }, + { + "backend": "musa", + "backend_version": "4.1", + "original_backend_version": "4.1.0", + "backend_variant": "", + "service": "vllm", + "service_version": "0.9.2", + "platform": "linux/amd64", + "docker_image": "gpustack/runner:musa4.1-vllm0.9.2", + "deprecated": false + }, + { + "backend": "corex", + "backend_version": "4.3", + "original_backend_version": "4.3.0", + "backend_variant": "", + "service": "vllm", + "service_version": "0.11.0", + "platform": "linux/amd64", + "docker_image": "gpustack/runner:corex4.3-vllm0.11.0", + "deprecated": false, + "dependencies": {} + } +] diff --git a/tests/gpustack_runner/test_runner.py b/tests/gpustack_runner/test_runner.py index a290ecb2..0cb77a13 100644 --- a/tests/gpustack_runner/test_runner.py +++ b/tests/gpustack_runner/test_runner.py @@ -1,3 +1,5 @@ +from pathlib import Path + import pytest from fixtures import load @@ -7,7 +9,21 @@ list_runners, list_service_runners, ) -from gpustack_runner.runner import version_sort_key +from gpustack_runner.runner import ( + Runner, + _match_dependencies, + _resolve_dependency_conditions, + version_sort_key, +) + +DEPENDENCIES_CATALOG = str( + Path(__file__).parent / "fixtures" / "test_list_runners_by_dependencies.json", +) +""" +A synthetic runner catalog whose entries carry ``dependencies``, used to drive +the dependency filtering through the real query entrypoints. The bundled +catalog cannot serve that purpose: none of its entries has been probed yet. +""" @pytest.mark.parametrize( @@ -182,6 +198,472 @@ def test_list_service_runners(name, filters, expected): ) +def _matches( + dependencies, + conditions, + with_unknown_dependencies=True, +) -> bool: + """ + Resolve dependency conditions and match them against a probed dependency map. + + :param dependencies: The probed dependency map of a single runner entry. + :param conditions: A tuple of ``(dependency name, PEP 440 specifier)`` pairs. + :param with_unknown_dependencies: Whether unprobed entries are kept. + :return: True if the entry satisfies every condition. + """ + return _match_dependencies( + dependencies, + _resolve_dependency_conditions(conditions), + with_unknown_dependencies, + ) + + +@pytest.mark.parametrize( + "name, dependencies, conditions, with_unknown_dependencies, expected", + [ + # No condition at all never filters anything out, whatever the state of + # the dependency map is. + ("no condition, probed", {"lmcache": "0.4.3"}, (), True, True), + ("no condition, unprobed", None, (), True, True), + ("no condition, unprobed strict", None, (), False, True), + # Plain ranges. + ( + "lower bound hit", + {"lmcache": "0.5.4"}, + (("lmcache", ">=0.4.6"),), + True, + True, + ), + ( + "lower bound miss", + {"lmcache": "0.4.3"}, + (("lmcache", ">=0.4.6"),), + True, + False, + ), + ( + "interval hit", + {"lmcache": "0.5.4"}, + (("lmcache", ">=0.4.6,<0.6"),), + True, + True, + ), + ( + "interval miss above", + {"lmcache": "0.6.0"}, + (("lmcache", ">=0.4.6,<0.6"),), + True, + False, + ), + ( + "interval miss below", + {"lmcache": "0.4.3"}, + (("lmcache", ">=0.4.6,<0.6"),), + True, + False, + ), + ("exact hit", {"lmcache": "0.5.4"}, (("lmcache", "==0.5.4"),), True, True), + ("exact miss", {"lmcache": "0.5.5"}, (("lmcache", "==0.5.4"),), True, False), + # Pre-release regression locks: ``SpecifierSet`` excludes pre-releases by + # default, and dropping ``prereleases=True`` would filter out every rc + # image, which are the norm here. + ( + "prerelease matches lower bound (vllm-ascend)", + {"vllm-ascend": "0.20.2rc1"}, + (("vllm-ascend", ">=0.20.0"),), + True, + True, + ), + ( + "prerelease matches lower bound (sglang-kernel)", + {"sglang-kernel": "0.4.6rc0"}, + (("sglang-kernel", ">=0.4.5"),), + True, + True, + ), + ( + "prerelease of the bound itself stays below it", + # ``0.4.6rc1 < 0.4.6`` is correct PEP 440 ordering, not a defect. + # Locked so it is never "fixed" into a match. + {"lmcache": "0.4.6rc1"}, + (("lmcache", ">=0.4.6"),), + True, + False, + ), + ( + "empty specifier is a pure existence check", + # An empty specifier degrades to "installed at all", which is how a + # commit-pinned dev version stays matchable. + {"lmcache": "0.3.0.dev0+gaff7d64"}, + (("lmcache", ""),), + True, + True, + ), + # ``vllm-omni`` is installed from a commit, so its version (real output + # from ``gpustack/runner:cuda13.0-vllm0.27.1``) carries no information + # and sorts below its own rc2 under PEP 440. An empty specifier is how + # to ask about it without making a meaningless comparison. + ( + "empty specifier hits a commit-pinned dev version (vllm-omni)", + {"vllm-omni": "0.27.0rc2.dev25+gd77a35a32"}, + (("vllm-omni", ""),), + True, + True, + ), + ( + "empty specifier still misses when not installed", + # "No constraint" never degrades to "always true": absence is still + # absence. + {}, + (("vllm-omni", ""),), + True, + False, + ), + # A recorded version that is not PEP 440 must never crash the query. + # ``SpecifierSet.contains`` raises ``InvalidVersion`` on one, and the + # same class of input already took down the catalog once + # (gpustack/gpustack#5792), so both paths are locked here. + ( + "unparseable version misses a range instead of raising", + {"torch": "latest"}, + (("torch", ">=2.9"),), + True, + False, + ), + ( + "unparseable version still satisfies an existence check", + # The package *is* installed, which is the whole question an empty + # specifier asks; an unreadable version must not turn it into a miss. + {"torch": "latest"}, + (("torch", ""),), + True, + True, + ), + # Unknown names match nothing rather than raising: the whitelist is a + # build-side file and is not shipped with the library, so a name is just + # a key lookup into the probed map. + ( + "unknown name misses instead of raising", + {"lmcache": "0.5.4"}, + (("lmcahce", ">=0.4.6"),), + True, + False, + ), + ( + "unknown name misses even unconstrained", + {"lmcache": "0.5.4"}, + (("lmcahce", ""),), + True, + False, + ), + # Multiple conditions are ANDed. + ( + "multiple conditions all hit", + {"torch": "2.9.0", "transformers": "4.58.2"}, + (("torch", ">=2.9"), ("transformers", ">=4.58")), + True, + True, + ), + ( + "multiple conditions one misses", + {"torch": "2.8.0", "transformers": "4.58.2"}, + (("torch", ">=2.9"), ("transformers", ">=4.58")), + True, + False, + ), + # Unprobed entries (the whole field absent) are kept by default and + # dropped in strict mode. + ("unprobed is lenient by default", None, (("lmcache", ">=0.4.6"),), True, True), + ( + "unprobed is dropped when strict", + None, + (("lmcache", ">=0.4.6"),), + False, + False, + ), + # A probed entry missing the key means "not installed", a different + # state from "unprobed" that must not get the lenient default. + ( + "probed but package absent", + {"torch": "2.8.0"}, + (("lmcache", ">=0.4.6"),), + True, + False, + ), + ( + "probed with an empty map", + # ``{}`` is "probed, nothing whitelisted installed"; it must not be + # confused with the unprobed ``None``. + {}, + (("lmcache", ">=0.4.6"),), + True, + False, + ), + ], +) +def test_match_dependencies( + name, + dependencies, + conditions, + with_unknown_dependencies, + expected, +): + actual = _matches(dependencies, conditions, with_unknown_dependencies) + assert actual is expected, ( + f"case {name} expected {expected}, but got {actual} " + f"for dependencies: {dependencies} and conditions: {conditions}" + ) + + +def test_match_dependencies_invalid_specifier(): + with pytest.raises(ValueError, match=r"lmcache.*>=abc|>=abc.*lmcache") as excinfo: + _matches({"lmcache": "0.5.4"}, (("lmcache", ">=abc"),)) + message = str(excinfo.value) + assert "lmcache" in message, f"expected the package name in {message!r}" + assert ">=abc" in message, f"expected the specifier in {message!r}" + + +def test_match_dependencies_invalid_specifier_of_an_unknown_name(): + """ + The name is no longer validated, but the specifier still is: a malformed + specifier is a caller input error whatever the name. + """ + with pytest.raises(ValueError, match=r"lmcahce.*>>>1\.0|>>>1\.0.*lmcahce"): + _matches({"lmcache": "0.5.4"}, (("lmcahce", ">>>1.0"),)) + + +def test_match_dependencies_invalid_specifier_without_any_entry(): + """ + An invalid specifier is a caller error, so it must be reported even when no + entry would ever be evaluated against it. + """ + with pytest.raises(ValueError, match="lmcache"): + _resolve_dependency_conditions((("lmcache", ">=abc"),)) + + +def test_match_dependencies_malformed_condition_too_short(): + # A natural typo for a single-element condition. It must raise a useful + # ValueError, not leak "not enough values to unpack" from destructuring. + with pytest.raises(ValueError, match="lmcache") as excinfo: + _resolve_dependency_conditions((("lmcache",),)) + message = str(excinfo.value) + assert "pair" in message, f"expected a hint about the expected shape in {message!r}" + + +def test_match_dependencies_malformed_condition_too_long(): + with pytest.raises(ValueError, match="lmcache"): + _resolve_dependency_conditions((("lmcache", ">=0.4.6", "extra"),)) + + +def test_match_dependencies_none_specifier_is_unconstrained(): + # ``None`` is how every other ``list_runners`` filter spells "no + # constraint", rather than a ``TypeError`` out of ``SpecifierSet``. + assert _matches({"lmcache": "0.4.3"}, (("lmcache", None),)) is True + # Unconstrained still requires the package to be installed at all. + assert _matches({"torch": "2.9.0"}, (("lmcache", None),)) is False + + +def test_match_dependencies_does_not_validate_names(): + """ + A name the catalog never carries is a miss, not an error. + + The whitelist lives in ``pack/`` and is not shipped with the library, so + there is nothing to validate a name against. Callers own their spelling. + """ + assert _matches({"flashinfer-python": "0.2.0"}, (("flashinfer", ">=0.2"),)) is False + assert _matches({"flashinfer-python": "0.2.0"}, (("flashinfer-python", ">=0.2"),)) + + +@pytest.mark.parametrize( + "name, filters, expected", + [ + ( + "no dependency condition returns everything", + {}, + [ + "gpustack/runner:cann9.1-a3-vllm0.23.0", + "gpustack/runner:cuda13.0-vllm0.29.0", + "gpustack/runner:cuda13.0-sglang0.5.10", + "gpustack/runner:rocm7.0-vllm0.29.0", + "gpustack/runner:musa4.1-vllm0.9.2", + "gpustack/runner:corex4.3-vllm0.11.0", + ], + ), + ( + "lmcache lower bound keeps unprobed entries", + {"dependencies": (("lmcache", ">=0.4.6"),)}, + [ + "gpustack/runner:cann9.1-a3-vllm0.23.0", + "gpustack/runner:cuda13.0-vllm0.29.0", + # Unprobed, kept by the lenient default. + "gpustack/runner:musa4.1-vllm0.9.2", + ], + ), + ( + "lmcache lower bound in strict mode", + { + "dependencies": (("lmcache", ">=0.4.6"),), + "with_unknown_dependencies": False, + }, + [ + "gpustack/runner:cann9.1-a3-vllm0.23.0", + "gpustack/runner:cuda13.0-vllm0.29.0", + ], + ), + ( + "upper bound excludes the newer entries", + {"dependencies": (("lmcache", "<0.5"),)}, + [ + # ``lmcache 0.4.6rc1`` matches with prereleases enabled. + "gpustack/runner:cuda13.0-sglang0.5.10", + "gpustack/runner:musa4.1-vllm0.9.2", + ], + ), + ( + "prerelease matches the lower bound", + {"dependencies": (("sglang-kernel", ">=0.4.5"),)}, + [ + "gpustack/runner:cuda13.0-sglang0.5.10", + "gpustack/runner:musa4.1-vllm0.9.2", + ], + ), + ( + "multiple conditions are ANDed", + { + "dependencies": (("torch", ">=2.9"), ("transformers", ">=4.58")), + }, + [ + "gpustack/runner:cuda13.0-vllm0.29.0", + "gpustack/runner:musa4.1-vllm0.9.2", + ], + ), + ( + "dependency conditions combine with the existing criteria", + { + "backend": "cuda", + "dependencies": (("lmcache", ">=0.4.6"),), + }, + [ + "gpustack/runner:cuda13.0-vllm0.29.0", + ], + ), + ( + "nothing matches", + { + "dependencies": (("lmcache", ">=99.0"),), + "with_unknown_dependencies": False, + }, + [], + ), + ], +) +def test_list_runners_by_dependencies(name, filters, expected): + actual = [ + r.docker_image for r in list_runners(**filters, data_path=DEPENDENCIES_CATALOG) + ] + assert actual == expected, ( + f"case {name} expected {expected}, but got {actual} for filters: {filters}" + ) + + +def test_list_backend_runners_by_dependencies(): + """ + The dependency criteria must reach ``list_runners`` through the backend + entrypoint as well. + """ + actual = list_backend_runners( + dependencies=(("lmcache", ">=0.4.6"),), + data_path=DEPENDENCIES_CATALOG, + ) + assert [br.backend for br in actual] == ["cann", "cuda", "musa"] + + strict = list_backend_runners( + dependencies=(("lmcache", ">=99.0"),), + with_unknown_dependencies=False, + data_path=DEPENDENCIES_CATALOG, + ) + assert strict == [] + + +def test_list_service_runners_by_dependencies(): + """ + The dependency criteria must reach ``list_runners`` through the service + entrypoint as well. + """ + actual = list_service_runners( + dependencies=(("lmcache", ">=0.4.6"),), + data_path=DEPENDENCIES_CATALOG, + ) + # The only sglang entry carries ``lmcache 0.4.6rc1``, which ranks below the + # bound, so no sglang service survives. + assert [sr.service for sr in actual] == ["vllm"] + + strict = list_service_runners( + dependencies=(("lmcache", ">=99.0"),), + with_unknown_dependencies=False, + data_path=DEPENDENCIES_CATALOG, + ) + assert strict == [] + + +def test_list_runners_rejects_unknown_keys(): + with pytest.raises(ValueError, match="Invalid keys in kwargs"): + list_runners(unknown_key="whatever") + + +def test_runner_dependencies_is_optional(): + """ + Every bundled catalog entry predates dependency probing, so the field must + stay optional and must be omitted from the serialized form. + """ + item = { + "backend": "musa", + "backend_version": "4.1", + "original_backend_version": "4.1.0", + "backend_variant": "", + "service": "vllm", + "service_version": "0.9.2", + "platform": "linux/amd64", + "docker_image": "gpustack/runner:musa4.1-vllm0.9.2", + "deprecated": False, + } + runner = Runner.from_dict(item) + assert runner.dependencies is None + assert "dependencies" not in runner.to_dict() + + +def test_bundled_catalog_omits_absent_dependencies(): + actual = list_runners(todict=True) + assert actual, "expected a non-empty runner catalog" + assert all("dependencies" not in r for r in actual), ( + "expected unprobed entries to serialize without a dependencies key" + ) + + +def test_bundled_catalog_is_lenient_by_default(): + """ + No bundled entry has been probed yet, so a dependency condition must be a + no-op by default, and must exclude everything in strict mode. + """ + baseline = list_runners(backend="cuda", todict=True) + assert baseline, "expected a non-empty cuda runner catalog" + + lenient = list_runners( + backend="cuda", + dependencies=(("lmcache", ">=0.4.6"),), + todict=True, + ) + assert lenient == baseline + + strict = list_runners( + backend="cuda", + dependencies=(("lmcache", ">=0.4.6"),), + with_unknown_dependencies=False, + todict=True, + ) + assert strict == [] + + @pytest.mark.parametrize( "name, image, expected", load( diff --git a/tests/pack/test_dependencies.py b/tests/pack/test_dependencies.py new file mode 100644 index 00000000..63a880a7 --- /dev/null +++ b/tests/pack/test_dependencies.py @@ -0,0 +1,76 @@ +"""Self-consistency tests for pack/dependencies.json. + +The file maps a dependency name to the distribution names it may ship under, +highest priority first. It is read twice, both on the build side: `pack.yml` +flattens the values into the probe's build argument, and `pack/merge_runner.sh` +folds a probe result back onto the names. Nothing reads it at query time -- +`runner.py.json` already carries the folded result. +""" + +from __future__ import annotations + +import json +import re +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +DEPENDENCIES_FILE = REPO_ROOT / "pack" / "dependencies.json" + +DEPENDENCIES: dict[str, list[str]] = json.loads( + DEPENDENCIES_FILE.read_text(encoding="utf-8"), +) + + +def _normalize(name: str) -> str: + return re.sub(r"[-_.]+", "-", name).lower() + + +def test_discovery_found_dependencies(): + # If this ever comes back empty, every test below passes vacuously. + assert DEPENDENCIES, f"no dependency names found in {DEPENDENCIES_FILE}" + + +@pytest.mark.parametrize("name, packages", list(DEPENDENCIES.items())) +def test_packages_non_empty(name, packages): + assert packages, f"case {name} expected a non-empty package list" + + +@pytest.mark.parametrize( + "package", + [package for packages in DEPENDENCIES.values() for package in packages], +) +def test_package_names_are_pep503_normalized(package): + # The probe normalizes what `pip list` reports before intersecting, so a + # non-normalized name here could never match. + assert _normalize(package) == package, ( + f"case {package} expected an already PEP 503 normalized package name" + ) + + +def test_dependency_names_are_sorted(): + names = list(DEPENDENCIES.keys()) + assert names == sorted(names), ( + "expected top-level dependency names to be sorted lexicographically" + ) + + +def test_package_names_are_globally_unique(): + # A distribution belonging to two dependency names would be folded into + # both, so one of them would report a version for a package it does not + # actually describe. + seen: dict[str, str] = {} + duplicates = [] + for name, packages in DEPENDENCIES.items(): + for package in packages: + if package in seen: + duplicates.append( + f"{package} appears under both {seen[package]!r} and {name!r}", + ) + else: + seen[package] = name + assert not duplicates, ( + f"expected no duplicate package names across dependency names, " + f"found: {duplicates}" + ) diff --git a/tests/pack/test_merge_runner.py b/tests/pack/test_merge_runner.py new file mode 100644 index 00000000..880443e7 --- /dev/null +++ b/tests/pack/test_merge_runner.py @@ -0,0 +1,214 @@ +"""Behavioral tests for pack/merge_runner.sh. + +The script decides what lands in the published `runner.py.json`, and its two +riskiest behaviors are invisible until a release goes wrong: folding a probe +result onto the dependency names, and refusing to invent entries for a post +operation. Both are covered here. + +The real script is executed against a sandbox workspace, so nothing in the +repository is written to. +""" + +from __future__ import annotations + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +MERGE_RUNNER = REPO_ROOT / "pack" / "merge_runner.sh" +DEPENDENCIES_FILE = REPO_ROOT / "pack" / "dependencies.json" + +pytestmark = pytest.mark.skipif( + shutil.which("yq") is None or shutil.which("jq") is None, + reason="merge_runner.sh needs yq and jq", +) + +MATRIX_YAML = """\ +rules: + - backend: "cuda" + services: + - "vllm" +""" + +# One build job, shaped like an `expand_matrix.sh` entry. `platform_tag` is the +# key probe results are related back with. +BUILD_JOB = { + "backend": "cuda", + "backend_version": "13.0", + "original_backend_version": "13.0.2", + "backend_variant": "", + "service": "vllm", + "service_version": "0.29.0", + "platform": "linux/amd64", + "tag": "cuda13.0-vllm0.29.0", + "platform_tag": "linux-amd64-cuda13.0-vllm0.29.0", +} + +ENTRY = { + "backend": "cuda", + "backend_version": "13.0", + "original_backend_version": "13.0.2", + "backend_variant": "", + "service": "vllm", + "service_version": "0.29.0", + "platform": "linux/amd64", + "docker_image": "gpustack/runner:cuda13.0-vllm0.29.0", + "deprecated": False, +} + + +def _workspace(tmp_path: Path, entries: list[dict] | None) -> Path: + """Lay out the directories merge_runner.sh expects, and return the pack dir.""" + pack = tmp_path / "pack" + pack.mkdir() + (pack / "matrix.yaml").write_text(MATRIX_YAML) + (tmp_path / "gpustack_runner").mkdir() + (tmp_path / "tests" / "gpustack_runner" / "fixtures").mkdir(parents=True) + if entries is not None: + (tmp_path / "gpustack_runner" / "runner.py.json").write_text( + json.dumps(entries, indent=2), + ) + return pack + + +def _probe(tmp_path: Path, platform_tag: str, probed: dict[str, str]) -> Path: + """Write one downloaded artifact, laid out the way download-artifact does.""" + directory = tmp_path / "dependencies" / f"dependencies-{platform_tag}" + directory.mkdir(parents=True, exist_ok=True) + (directory / "dependencies.json").write_text(json.dumps(probed)) + return tmp_path / "dependencies" + + +def _run( + pack: Path, + build_jobs: list[dict], + dependencies_dir: Path | None = None, + post_operation: str = "", +) -> subprocess.CompletedProcess: + env = { + "PATH": "/usr/bin:/bin:/usr/local/bin:/opt/homebrew/bin", + "INPUT_WORKSPACE": str(pack), + "INPUT_BUILD_JOBS": json.dumps(build_jobs), + "INPUT_DEPENDENCIES_FILE": str(DEPENDENCIES_FILE), + "INPUT_DEPENDENCIES_DIR": str(dependencies_dir) if dependencies_dir else "", + "INPUT_POST_OPERATION": post_operation, + } + return subprocess.run( # noqa: S603 + ["bash", str(MERGE_RUNNER)], # noqa: S607 + env=env, + capture_output=True, + text=True, + check=False, + ) + + +def _merged(pack: Path) -> list[dict]: + path = pack.parent / "gpustack_runner" / "runner.py.json" + return json.loads(path.read_text(encoding="utf-8")) + + +def test_probe_result_is_folded_onto_dependency_names(tmp_path): + """A raw probe result is recorded under dependency names, not distributions.""" + pack = _workspace(tmp_path, []) + # ``mooncake-transfer-engine-rocm`` outranks the generic build of the same + # name, which is the whole point of a multi-distribution entry. + # ``unlisted-package`` is not whitelisted at all and is dropped. + dependencies_dir = _probe( + tmp_path, + BUILD_JOB["platform_tag"], + { + "mooncake-transfer-engine": "0.3.13", + "mooncake-transfer-engine-rocm": "0.3.13.post1", + "torch": "2.9.0", + "unlisted-package": "1.0.0", + }, + ) + + result = _run(pack, [BUILD_JOB], dependencies_dir) + assert result.returncode == 0, result.stderr + + merged = _merged(pack) + assert len(merged) == 1 + assert merged[0]["dependencies"] == { + "mooncake-transfer-engine": "0.3.13.post1", + "torch": "2.9.0", + } + + +def test_probed_nothing_installed_stays_an_empty_map(tmp_path): + """``{}`` means "probed, nothing installed" and must not become absent.""" + pack = _workspace(tmp_path, []) + dependencies_dir = _probe(tmp_path, BUILD_JOB["platform_tag"], {}) + + result = _run(pack, [BUILD_JOB], dependencies_dir) + assert result.returncode == 0, result.stderr + + merged = _merged(pack) + assert merged[0]["dependencies"] == {} + + +def test_unprobed_rebuild_carries_over_existing_dependencies(tmp_path): + """Rebuilding without a probe must not erase what an earlier build recorded. + + An absent ``dependencies`` means "never probed", so dropping it would be a + lie that only another rebuild could undo. + """ + existing = dict(ENTRY, dependencies={"lmcache": "0.5.4"}) + pack = _workspace(tmp_path, [existing]) + + result = _run(pack, [BUILD_JOB], dependencies_dir=None) + assert result.returncode == 0, result.stderr + + merged = _merged(pack) + assert len(merged) == 1 + assert merged[0]["dependencies"] == {"lmcache": "0.5.4"} + + +def test_post_operation_updates_in_place_without_adding(tmp_path): + """A post operation may only refresh ``dependencies`` of existing entries.""" + untouched = dict( + ENTRY, + service_version="0.28.0", + docker_image="gpustack/runner:cuda13.0-vllm0.28.0", + ) + existing = dict(ENTRY, dependencies={"lmcache": "0.4.3"}) + pack = _workspace(tmp_path, [existing, untouched]) + dependencies_dir = _probe( + tmp_path, + BUILD_JOB["platform_tag"], + {"lmcache": "0.5.4", "torch": "2.9.0"}, + ) + + result = _run(pack, [BUILD_JOB], dependencies_dir, post_operation="whatever") + assert result.returncode == 0, result.stderr + + merged = _merged(pack) + assert len(merged) == 2, "a post operation must never add or drop an entry" + + by_image = {entry["docker_image"]: entry for entry in merged} + updated = by_image["gpustack/runner:cuda13.0-vllm0.29.0"] + assert updated["dependencies"] == {"lmcache": "0.5.4", "torch": "2.9.0"} + # Every other field of the updated entry, and the other entry as a whole, + # must come through byte for byte. + assert {k: v for k, v in updated.items() if k != "dependencies"} == ENTRY + assert by_image["gpustack/runner:cuda13.0-vllm0.28.0"] == untouched + + +def test_post_operation_fails_on_a_tag_that_addresses_no_entry(tmp_path): + """The usual cause is running without ``for_release``, so the tags are dev tags.""" + pack = _workspace(tmp_path, []) + dependencies_dir = _probe( + tmp_path, + BUILD_JOB["platform_tag"], + {"lmcache": "0.5.4"}, + ) + + result = _run(pack, [BUILD_JOB], dependencies_dir, post_operation="whatever") + assert result.returncode != 0, ( + "expected a post operation addressing no existing entry to fail" + ) + assert "do not address exactly one existing entry" in result.stderr, result.stderr diff --git a/tests/pack/test_probe_wiring.py b/tests/pack/test_probe_wiring.py new file mode 100644 index 00000000..a2c9a598 --- /dev/null +++ b/tests/pack/test_probe_wiring.py @@ -0,0 +1,361 @@ +"""Structural invariant tests for the dependency-probe wiring in pack/. + +pack.yml is `workflow_dispatch` only, so no pull request ever builds an image: a +missing or mis-wired probe produces no CI signal at all, and surfaces only when +someone dispatches a build or cuts a release. This file is that missing signal. + +Coverage is derived, not hardcoded. The (backend, service) pairs come from +pack/matrix.yaml, and each pair is resolved to a Dockerfile the same way pack.yml +does -- `Dockerfile.` wins over the merged `Dockerfile` when it exists. +So only the targets that are actually buildable are required to carry the probe, +and a merged Dockerfile left stale by the per-service split is not. Adding a +backend, a service, or a matrix rule extends this coverage on its own. +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +PACK_DIR = REPO_ROOT / "pack" + +REPO_WORKFLOW = REPO_ROOT / ".github" / "workflows" / "pack.yml" +MATRIX_YAML = PACK_DIR / "matrix.yaml" + +_YAML_LIST_ITEM_RE = re.compile(r"^\s*-\s*\"?([A-Za-z0-9][A-Za-z0-9_.-]*)\"?\s*$") +_YAML_KEY_RE = re.compile(r"^(\s*)([A-Za-z0-9_-]+):\s*$") +_RULE_BACKEND_RE = re.compile(r"^\s*-\s+backend:\s*\"?([A-Za-z0-9_-]+)\"?\s*$") + +# The file-resolution rule this module mirrors, as it appears in pack.yml. +_WORKFLOW_DOCKERFILE_PREFERENCE = "if [[ -f ${DOCKER_FILE}.${{ matrix.service }} ]]" + + +def _yaml_list_items(lines: list[str], start: int) -> list[str]: + """Collect the `- item` entries of the YAML block sequence starting at `start`.""" + items: list[str] = [] + for line in lines[start:]: + if not line.strip() or line.lstrip().startswith("#"): + continue + m = _YAML_LIST_ITEM_RE.match(line) + if not m: + break + items.append(m.group(1)) + return items + + +def _matrix_build_pairs() -> set[tuple[str, str]]: + """Every (backend, service) pair pack/matrix.yaml expands into. + + matrix.yaml is where a service actually enters the build -- expand_matrix.sh + derives `matrix.backend` and `matrix.service` from `rules[]` -- so a target + absent from here is one no build can ever reach. + """ + lines = MATRIX_YAML.read_text(encoding="utf-8").splitlines() + pairs: set[tuple[str, str]] = set() + backend: str | None = None + for i, line in enumerate(lines): + m = _RULE_BACKEND_RE.match(line) + if m: + backend = m.group(1) + continue + km = _YAML_KEY_RE.match(line) + if backend and km and km.group(2) == "services": + pairs.update((backend, s) for s in _yaml_list_items(lines, i + 1)) + if not pairs: + errmsg = ( + f"no (backend, service) pairs found in {MATRIX_YAML} -- the matrix " + f"layout changed, and every test in this file would otherwise " + f"silently cover nothing" + ) + raise RuntimeError(errmsg) + return pairs + + +def _resolve_dockerfile(backend: str, service: str) -> Path: + """Pick the Dockerfile a build would use, mirroring pack.yml's rule.""" + split = PACK_DIR / backend / f"Dockerfile.{service}" + return split if split.is_file() else PACK_DIR / backend / "Dockerfile" + + +# The buildable (Dockerfile, service target) pairs -- the only ones that must +# carry the probe. A merged Dockerfile that every service has since outgrown a +# split file for resolves to nothing here and is left alone. +BUILD_TARGETS = sorted( + (_resolve_dockerfile(backend, service), service) + for backend, service in _matrix_build_pairs() +) + +# `AS` is matched case-insensitively even though the repo currently always uppercases +# it -- don't assume that stays true forever. +FROM_RE = re.compile(r"^FROM\s+(\S+)(?:\s+AS\s+(\S+))?\s*$", re.IGNORECASE) +ARG_DEPS_RE = re.compile(r"^ARG\s+DEPENDENCY_PACKAGES(?:=.*)?\s*$") +COPY_FROM_RE = re.compile(r"^COPY\s+--from=(\S+)\s") +BUILD_STEP_RE = re.compile(r"^(RUN|COPY|ADD)\b", re.IGNORECASE) +# Matching the mounted script name too, not just the shared context: mounting +# pack/shared/ to run something else is not a probe call. +PROBE_RUN_PREFIX = "RUN --mount=type=bind,from=shared,source=probe_dependencies.sh" + + +@dataclass +class Stage: + name: str + base: str + from_line: int # 1-indexed line number of the `FROM` instruction itself + body: list[tuple[int, str]] # (line_no, text) for lines strictly after FROM, + # up to (not including) the next FROM, or EOF for the last stage. + + +@dataclass +class ParsedDockerfile: + path: Path + preamble: list[tuple[int, str]] # lines before the first FROM + stages: list[Stage] + + def stage(self, name: str) -> Stage | None: + for s in self.stages: + if s.name == name: + return s + return None + + +def parse_dockerfile(path: Path) -> ParsedDockerfile: + lines = path.read_text(encoding="utf-8").splitlines() + froms: list[tuple[int, str, str | None]] = [] + for i, line in enumerate(lines, start=1): + m = FROM_RE.match(line) + if m: + froms.append((i, m.group(1), m.group(2))) + + first_from_line = froms[0][0] if froms else len(lines) + 1 + preamble = [(i, lines[i - 1]) for i in range(1, first_from_line)] + + stages = [] + for idx, (from_line, base, name) in enumerate(froms): + end = froms[idx + 1][0] if idx + 1 < len(froms) else len(lines) + 1 + body = [(j, lines[j - 1]) for j in range(from_line + 1, end)] + if name is not None: + stages.append(Stage(name=name, base=base, from_line=from_line, body=body)) + + return ParsedDockerfile(path=path, preamble=preamble, stages=stages) + + +def _rel(path: Path) -> str: + return str(path.relative_to(REPO_ROOT)) + + +def _probe_lines(body: list[tuple[int, str]]) -> list[int]: + return [ln for ln, text in body if text.lstrip().startswith(PROBE_RUN_PREFIX)] + + +def _arg_lines(body: list[tuple[int, str]]) -> list[int]: + return [ln for ln, text in body if ARG_DEPS_RE.match(text.strip())] + + +PACK_DOCKERFILES = sorted({path for path, _ in BUILD_TARGETS}) +PARSED = {p: parse_dockerfile(p) for p in PACK_DOCKERFILES} + +# The service targets each Dockerfile is actually built for. A file may define +# more stages than this -- only the buildable ones are held to the invariants. +TARGET_NAMES = { + path: {service for p, service in BUILD_TARGETS if p == path} + for path in PACK_DOCKERFILES +} + + +def _service_targets(parsed: ParsedDockerfile) -> list[Stage]: + names = TARGET_NAMES[parsed.path] + return [s for s in parsed.stages if s.name in names] + + +def _deps_stages(parsed: ParsedDockerfile) -> list[Stage]: + """The `-deps` export stages belonging to this file's buildable targets.""" + wanted = {f"{name}-deps" for name in TARGET_NAMES[parsed.path]} + return [s for s in parsed.stages if s.name in wanted] + + +def test_discovery_found_build_targets(): + # If this ever comes back empty, every other test here passes vacuously. + assert BUILD_TARGETS, ( + f"no buildable (Dockerfile, service) pair resolved from {MATRIX_YAML}" + ) + missing = sorted(_rel(p) for p in PACK_DOCKERFILES if not p.is_file()) + assert not missing, ( + f"matrix.yaml names backends whose Dockerfile does not exist: {missing}" + ) + + +def test_workflow_still_prefers_the_split_dockerfile(): + """The file-resolution rule `_resolve_dockerfile` mirrors still lives in pack.yml. + + Without this, a change to how the workflow picks a Dockerfile would silently + shift which targets get built, while this module kept checking the old set. + """ + # Via a local, so a failure does not dump the whole workflow file. + found = _WORKFLOW_DOCKERFILE_PREFERENCE in REPO_WORKFLOW.read_text( + encoding="utf-8", + ) + assert found, ( + f"{_rel(REPO_WORKFLOW)} no longer contains " + f"'{_WORKFLOW_DOCKERFILE_PREFERENCE}' -- the Dockerfile selection rule " + f"changed, so `_resolve_dockerfile` in this module must change with it" + ) + + +@pytest.mark.parametrize("path", PACK_DOCKERFILES, ids=_rel) +def test_every_service_target_has_a_deps_export_stage(path: Path): + """Invariant 1: every service target has a matching `-deps` stage.""" + parsed = PARSED[path] + for stage in _service_targets(parsed): + deps_name = f"{stage.name}-deps" + assert parsed.stage(deps_name) is not None, ( + f"{_rel(path)}:{stage.from_line}: service target '{stage.name}' has " + f"no matching export stage '{deps_name}' -- the dependency probe " + f"export is missing for this target" + ) + + +@pytest.mark.parametrize("path", PACK_DOCKERFILES, ids=_rel) +def test_deps_stage_is_scratch_and_names_a_real_target(path: Path): + """Invariant 2: every `-deps` stage is `FROM scratch` and names a real target.""" + parsed = PARSED[path] + service_names = {s.name for s in _service_targets(parsed)} + for stage in _deps_stages(parsed): + assert stage.base.lower() == "scratch", ( + f"{_rel(path)}:{stage.from_line}: export stage '{stage.name}' must " + f"be 'FROM scratch', found 'FROM {stage.base}'" + ) + target_name = stage.name[: -len("-deps")] + assert target_name in service_names, ( + f"{_rel(path)}:{stage.from_line}: export stage '{stage.name}' does " + f"not correspond to a real service target named '{target_name}' in " + f"this file (service targets found: {sorted(service_names)})" + ) + + +@pytest.mark.parametrize("path", PACK_DOCKERFILES, ids=_rel) +def test_deps_stage_copies_from_its_own_target(path: Path): + """Invariant 3: a `-deps` stage's `COPY --from=X` must be its own target. + + This guards against cross-target copy-paste in a multi-service file such as + pack/musa/Dockerfile, which defines vllm and sglang side by side. + """ + parsed = PARSED[path] + for stage in _deps_stages(parsed): + target_name = stage.name[: -len("-deps")] + copy_from = None + copy_line = None + for ln, text in stage.body: + m = COPY_FROM_RE.match(text.strip()) + if m: + copy_from, copy_line = m.group(1), ln + break + assert copy_from is not None, ( + f"{_rel(path)}:{stage.from_line}: export stage '{stage.name}' has " + f"no 'COPY --from=...' instruction" + ) + assert copy_from == target_name, ( + f"{_rel(path)}:{copy_line}: export stage '{stage.name}' copies " + f"from '{copy_from}', expected '{target_name}' -- looks like a " + f"cross-target copy-paste mistake" + ) + + +@pytest.mark.parametrize("path", PACK_DOCKERFILES, ids=_rel) +def test_probe_call_count_and_arg_placement(path: Path): + """Invariant 4: probe-call count == service-target count, each preceded by + its own `ARG DEPENDENCY_PACKAGES`.""" + parsed = PARSED[path] + targets = _service_targets(parsed) + + all_probe_lines = [ln for s in parsed.stages for ln in _probe_lines(s.body)] + assert len(all_probe_lines) == len(targets), ( + f"{_rel(path)}: found {len(all_probe_lines)} probe_dependencies.sh " + f"invocation(s) at line(s) {all_probe_lines}, but {len(targets)} " + f"service target(s) ({sorted(s.name for s in targets)}) -- every " + f"service target must call the probe exactly once, with no " + f"extra/missing calls" + ) + + for stage in targets: + probe_lines = _probe_lines(stage.body) + assert len(probe_lines) == 1, ( + f"{_rel(path)}:{stage.from_line}: service target '{stage.name}' " + f"has {len(probe_lines)} probe_dependencies.sh invocation(s), " + f"expected exactly 1" + ) + probe_line = probe_lines[0] + arg_lines = _arg_lines(stage.body) + assert (probe_line - 1) in arg_lines, ( + f"{_rel(path)}:{probe_line}: the probe call in service target " + f"'{stage.name}' is not immediately preceded (line " + f"{probe_line - 1}) by 'ARG DEPENDENCY_PACKAGES'" + ) + + +@pytest.mark.parametrize("path", PACK_DOCKERFILES, ids=_rel) +def test_no_file_level_dependency_packages_arg(path: Path): + """Invariant 5: no file-level (pre-first-FROM) `ARG DEPENDENCY_PACKAGES`. + + A file-level ARG would make every whitelist edit invalidate the Docker build + cache for the entire file, not just the probe step. + """ + parsed = PARSED[path] + bad_lines = _arg_lines(parsed.preamble) + assert not bad_lines, ( + f"{_rel(path)}: found file-level 'ARG DEPENDENCY_PACKAGES' before the " + f"first FROM at line(s) {bad_lines} -- this invalidates the cache for " + f"every stage in the file on every whitelist edit; declare it inside " + f"each service target instead, immediately before the probe RUN" + ) + + +@pytest.mark.parametrize("path", PACK_DOCKERFILES, ids=_rel) +def test_probe_call_is_last_build_step_in_its_stage(path: Path): + """Invariant 6: the probe call is the last RUN/COPY/ADD in its stage. + + The probe's mount cache key covers the script's content, which every backend + shares, so keeping it last makes editing the script cost a re-probe rather + than a rebuild of the business layers above it. + """ + parsed = PARSED[path] + for stage in _service_targets(parsed): + probe_lines = _probe_lines(stage.body) + if not probe_lines: + continue # already reported by test_probe_call_count_and_arg_placement + probe_line = probe_lines[0] + for ln, text in stage.body: + if ln <= probe_line: + continue + stripped = text.strip() + m = BUILD_STEP_RE.match(stripped) + if m: + pytest.fail( + f"{_rel(path)}:{ln}: found a '{m.group(1).upper()}' " + f"instruction after the probe call (line {probe_line}) in " + f"service target '{stage.name}' -- the probe must be the " + f"last build step in its stage", + ) + + +def test_every_buildable_target_is_wired(): + """Aggregate sanity check: total wired targets == total buildable targets. + + The count is deliberately not hardcoded, so a new matrix rule grows the + denominator on its own rather than failing on a stale magic number. + """ + unwired = [] + for path, service in BUILD_TARGETS: + parsed = PARSED[path] + if parsed.stage(service) is None: + unwired.append(f"{_rel(path)}: no '{service}' stage") + elif parsed.stage(f"{service}-deps") is None: + unwired.append(f"{_rel(path)}: '{service}' has no '{service}-deps' stage") + assert not unwired, ( + f"{len(unwired)} of {len(BUILD_TARGETS)} buildable target(s) across " + f"{len(PACK_DOCKERFILES)} Dockerfile(s) are not wired for dependency " + f"probing: {unwired}" + )