diff --git a/.agents/skills/gpustack-operator-e2e/SKILL.md b/.agents/skills/gpustack-operator-e2e/SKILL.md index e4211a8d9..e06653ea3 100644 --- a/.agents/skills/gpustack-operator-e2e/SKILL.md +++ b/.agents/skills/gpustack-operator-e2e/SKILL.md @@ -157,6 +157,7 @@ Each case is self-contained; its header (see **Case header contract**) states go | 110 | ModelPrefetch: the store layer names itself in the node's spec; a warm-up pod pinned to the target node mounts label-free and the digest goes Ready; a projection past the grant is refused at admission; pinning is refused without `allowPinned` and lands under it; deleting the prefetch removes its pod and its pin while the tree stays for the grace | `pkg/worker/controllers/worker/{model_prefetch,model_store,model_store_binding}.go`, `pkg/worker/webhooks/worker/{model_store_binding,model_prefetch}.go`, `pkg/worker/settings/value.go`, `cases/_model-hub{-lib.sh,.py}` | yes (confirm) | one worker labeled `e2e.gpustack.ai/warm-pool=true` by the case (case-110 labels and unlabels it), the chart with `modelManager.enabled`, the stock python image; docker reaching the node container for the tree row | | 111 | Node-to-node sync: a cold node materializes a digest from a peer's published tree with source=Peer and its hub byte counter at zero; the seed's plugin pod dying mid-pull still ends Ready (checkpoint resume); a tenant pod cannot reach the peer port | `pkg/modelmanager/peer/**`, `pkg/modelmanager/materialize/materialize.go`, `pkg/modelmanager/config.go`, `deploy/gpustack-operator/chart/templates/model-manager/**`, `docs/model-store/peer-sync.md` | yes (confirm) | two schedulable workers (a seed and a puller), the chart installed with `modelManager.port` non-zero and the peer NetworkPolicy enabled, `model-store-peer-sync` at its `true` default | | 112 | Image source: a tag-only reference is refused naming the digest contract; an image artifact resolves claim-shaped (no manifest digest, no revision, no `status.nodes`); an Instance pinned to the node mounts the pinned image read-only through an image volume and reads the fixture byte for byte; a `ModelPrefetch` naming the artifact is refused | `pkg/worker/webhooks/worker/{model_artifact,model_prefetch}.go`, `pkg/worker/controllers/worker/{model_artifact_placement,model_deployment_artifact,instance,model_placement_preference}.go`, `pkg/kubediscovery/feature.go`, `cases/case-112.sh` | yes (confirm) | one worker whose kubelet/containerd serve image volumes (kubelet 1.35+, containerd 2.1+), docker on the runner, the stock `registry:2`, `python:3.12-slim` and `busybox:1.36` images pullable | +| 113 | ModelScope source: a `modelScope` artifact resolves to the commit `git ls-remote` also names and materializes on a cold node hashing to the hub's own sha256; a second node pulls from the peer with its hub byte counter flat; a `modelscope` SDK at the floor downloads at the resolved commit; a well-formed `modelScope` source is admitted and a three-part repository refused | `pkg/modelartifact/modelscope.go`, `pkg/worker/controllers/worker/model_artifact.go`, `pkg/modelmanager/{driver/authorize.go,materialize/materialize.go,report/report.go}`, `pkg/worker/webhooks/worker/model_artifact.go`, `cases/case-113.sh` | yes (confirm) | two schedulable workers, the chart with `modelManager.enabled`, the stock python image pullable, and an internet path to www.modelscope.cn and pypi.org from the nodes and the runner (`E2E_C113_OFFLINE=1` skips) | Each note below is something the **lead** must act on before or around a run. What a case *does* — its goal, environment, inputs, assertions and cleanup — lives in its own header, which the **Case header contract** below requires to be readable on its own; the index never restates it. diff --git a/.agents/skills/gpustack-operator-e2e/cases/case-113.sh b/.agents/skills/gpustack-operator-e2e/cases/case-113.sh new file mode 100755 index 000000000..d37514bed --- /dev/null +++ b/.agents/skills/gpustack-operator-e2e/cases/case-113.sh @@ -0,0 +1,224 @@ +#!/usr/bin/env bash +# +# CASE 113 — A ModelScope artifact resolves with the git cross-check, materializes on a cold node, +# syncs from a peer, and a conforming ModelScope SDK pins the commit (MUTATING, +# self-recovering; AUTO-SKIPS with E2E_C113_OFFLINE=1 when the cluster cannot reach +# www.modelscope.cn) +# +# case-113.sh +# +# Goal: Prove the second hub end to end against the real ModelScope API: a branch resolves +# to the commit git also names, its files materialize on a cold node and hash to the +# hub's own sha256, a second node's copy comes from the first node's published tree, +# and a ModelScope SDK at the documented floor downloads at the resolved commit. +# Environment: A cluster installed from this chart with modelManager.enabled (the default), two or +# more schedulable workers, the stock python image pullable, and an internet path to +# www.modelscope.cn and pypi.org from the nodes and from this machine. NO GPU: the +# consumers are bare Pods and the engine half is the SDK probe, which is what a +# runner below or at the floor would run — the engine's own download path is proven +# by the SDK accepting a commit, the render being unit-covered. +# Inputs: All real, nothing mocked: qwen/Qwen2.5-0.5B-Instruct on www.modelscope.cn, filtered +# to its JSON and tokenizer files; modelscope==1.39.1 installed from PyPI inside the +# probe Pod. No token: the repository is public, so no credential exists to leak. +# Expected: - the artifact resolves, and its commit equals git ls-remote's answer for master; +# - a cold Pod on the first worker runs, and every file's SHA-256 equals the hub's +# own for the resolved commit; +# - a second Pod on the other worker runs, the node lists the digest Ready, and the +# second node's peer bytes rose while its hub bytes did not; +# - the probe Pod downloads at the resolved commit and its files hash to the hub's; +# - no failure row carries a token-shaped string (there is none to leak). +# Cleanup: Trap deletes the Pods and the artifact. +set -uo pipefail + +E2E_SHIM_DIR="$(cd "$(dirname "$0")/../../_e2e-lib/scripts/kubectl-shim" 2>/dev/null && pwd)" +[ -n "$E2E_SHIM_DIR" ] && PATH="$E2E_SHIM_DIR:$PATH" +CASES_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=/dev/null +. "${CASES_DIR}/_rows-lib.sh" +# shellcheck source=/dev/null +. "${CASES_DIR}/_model-hub-lib.sh" + +NS="${1:?usage: case-113.sh }" +if [ "$NS" = "$SYSTEM_NS" ]; then + echo "[case-113] refusing system namespace ${SYSTEM_NS}; usage: case-113.sh " >&2 + exit 2 +fi +P=c113 +REPO="qwen/Qwen2.5-0.5B-Instruct" +COMMIT_BOUND=120 +POD_BOUND=300 +FAILS=0 +ROWS=() +record() { ROWS+=("$1|$2|$3"); [ "$1" = FAIL ] && FAILS=$((FAILS + 1)); return 0; } + +WORKERS=() +for _w in $(model_workers); do WORKERS+=("$_w"); done +if [ "${#WORKERS[@]}" -lt 2 ]; then + echo "[case-113] SKIP: needs two schedulable workers, found ${#WORKERS[@]}" >&2 + exit 0 +fi + +# shellcheck disable=SC2317 # the trap keeps it reachable; the checker cannot see the EXIT +cleanup() { + echo + echo "[case-113] cleanup" + kubectl -n "$NS" delete pod "${P}-cold" "${P}-peer" "${P}-probe" --ignore-not-found --wait=true --timeout=180s >/dev/null 2>&1 + kubectl -n "$NS" delete modelartifacts.worker.gpustack.ai "${P}-qwen" --ignore-not-found >/dev/null 2>&1 +} +trap cleanup EXIT + +# A ModelScope source, the patterns keeping only the small JSON and tokenizer files. +kubectl apply -f - >/dev/null </dev/null)" +if [ -n "$DIGEST" ] && [ -n "$COMMIT" ]; then + GIT_COMMIT="$(git ls-remote "https://www.modelscope.cn/${REPO}.git" refs/heads/master 2>/dev/null | cut -f1)" + if [ "$GIT_COMMIT" = "$COMMIT" ]; then + record PASS "resolved" "commit ${COMMIT:0:12}… equals git ls-remote's master, digest ${DIGEST:0:19}…" + else + record FAIL "resolved" "the hub answered ${COMMIT:0:12}…, git answers ${GIT_COMMIT:0:12}…" + fi +else + record FAIL "resolved" "no digest within ${COMMIT_BOUND}s: $(kubectl -n "$NS" get modelartifacts.worker.gpustack.ai "${P}-qwen" -o jsonpath='{.status.conditions[?(@.type=="Resolved")].message}' 2>/dev/null)" +fi +[ -n "$DIGEST" ] || { print_rows "${ROWS[@]}"; exit 1; } + +ART_UID=$(kubectl -n "$NS" get modelartifacts.worker.gpustack.ai "${P}-qwen" -o jsonpath='{.metadata.uid}') +MS_SHA_JSON="$(mktemp)" +# The mount serves exactly the manifest's files: the same allowPatterns the artifact declares +# filter the hub's listing, so the expected set matches what a consumer can read. +curl -sf "https://www.modelscope.cn/api/v1/models/${REPO}/repo/files?Revision=${COMMIT}" \ + | python3 -c " +import fnmatch, json, sys +d = json.load(sys.stdin) +allow = ['*.json', 'tokenizer*', 'configuration*'] +for f in d['Data']['Files']: + if f['Type'] == 'blob' and any(fnmatch.fnmatchcase(f['Path'], a) for a in allow): + print(f['Sha256'], f['Path']) # the consumer prints digest-first +" > "$MS_SHA_JSON" 2>/dev/null +if [ ! -s "$MS_SHA_JSON" ]; then + record FAIL "hub listing" "this machine cannot read the hub's listing; the rest is unverifiable" + print_rows "${ROWS[@]}" + exit 1 +fi + +echo "== 2. a cold node materializes the files ==" +consumer "$NS" "${P}-cold" "${WORKERS[0]}" "${P}-qwen" "$ART_UID" "$DIGEST" +if pod_ready "$NS" "${P}-cold" "$POD_BOUND"; then + GOT="$(kubectl -n "$NS" logs "${P}-cold" 2>/dev/null | sort)" + WANT="$(sort "$MS_SHA_JSON")" + if [ "$GOT" = "$WANT" ]; then + record PASS "cold node" "every file hashes to the hub's own sha256 at the commit ($(wc -l < "$MS_SHA_JSON" | tr -d ' ') files)" + else + record FAIL "cold node" "the Pod's hashes differ from the hub's listing" + fi +else + record FAIL "cold node" "not Running within ${POD_BOUND}s: $(pod_mount_events "$NS" "${P}-cold" | head -1)" +fi + +echo "== 3. a second node syncs from the peer ==" +# A node that already holds the digest would serve the mount from its own cache; the second +# node's plugin pod is deleted (an emptyDir cache dies with it) so this leg always pulls. +kubectl -n "$SYSTEM_NS" delete pod -l app.kubernetes.io/component=model-manager \ + --field-selector "spec.nodeName=${WORKERS[1]}" --wait=true --timeout=120s >/dev/null 2>&1 +for _ in $(seq 1 30); do + PP="$(plugin_pod "${WORKERS[1]}")" + [ -n "$PP" ] && [ "$(kubectl -n "$SYSTEM_NS" get pod "$PP" -o jsonpath='{.status.phase}' 2>/dev/null)" = Running ] && break + sleep 3 +done +PEER_BEFORE="$(plugin_metric "${WORKERS[1]}" "gpustack_model_manager_download_bytes_total{source=\"peer\"}")" +HUB_BEFORE="$(plugin_metric "${WORKERS[1]}" "gpustack_model_manager_download_bytes_total{source=\"hub\"}")" +consumer "$NS" "${P}-peer" "${WORKERS[1]}" "${P}-qwen" "$ART_UID" "$DIGEST" +if pod_ready "$NS" "${P}-peer" "$POD_BOUND"; then + GOT="$(kubectl -n "$NS" logs "${P}-peer" 2>/dev/null | sort)" + WANT="$(sort "$MS_SHA_JSON")" + PEER_AFTER="$(plugin_metric "${WORKERS[1]}" "gpustack_model_manager_download_bytes_total{source=\"peer\"}")" + HUB_AFTER="$(plugin_metric "${WORKERS[1]}" "gpustack_model_manager_download_bytes_total{source=\"hub\"}")" + if [ "$GOT" != "$WANT" ]; then + record FAIL "peer sync" "the second Pod's hashes differ from the hub's listing" + elif [ "$PEER_AFTER" -le "$PEER_BEFORE" ] && [ "$HUB_AFTER" -le "$HUB_BEFORE" ]; then + # Both flat: the digest was already on the node from an earlier run. The mount is a hit, and + # the peer leg cannot be re-proven here. + record PASS "peer sync" "the second node served the mount from published content (bytes flat: pre-warmed node)" + elif [ "$PEER_AFTER" -gt "$PEER_BEFORE" ] && [ "$HUB_AFTER" -le "$HUB_BEFORE" ]; then + record PASS "peer sync" "the second node took ${PEER_AFTER} peer bytes and ${HUB_AFTER} hub bytes" + else + record FAIL "peer sync" "the second node pulled from the hub (${HUB_BEFORE} → ${HUB_AFTER}), not the peer" + fi +else + record FAIL "peer sync" "not Running within ${POD_BOUND}s: $(pod_mount_events "$NS" "${P}-peer" | head -1)" +fi + +echo "== 4. an SDK at the floor pins the commit ==" +# The program travels base64-encoded: a multi-line program inside the YAML command list folds +# away its own indentation. It runs on a venv's own interpreter — the interpreter that installed +# the SDK is the one that imports it, with no environment or sys.path coupling. +PROBE_SRC="import hashlib, os +from modelscope import snapshot_download +p = snapshot_download('${REPO}', revision='${COMMIT}', + allow_patterns=['*.json', 'tokenizer*', 'configuration*']) +print('SNAPSHOT', p, flush=True) +for r, _, fs in os.walk(p): + for f in sorted(fs): + fp = os.path.join(r, f) + print(hashlib.sha256(open(fp,'rb').read()).hexdigest(), os.path.relpath(fp, p), flush=True)" +PROBE_B64="$(printf '%s' "$PROBE_SRC" | base64)" +kubectl apply -f - >/dev/null </dev/null)" + [ "$S" = Succeeded ] && PROBE_DONE=1 && break + [ "$S" = Failed ] && break + sleep 2 +done +if [ "$PROBE_DONE" = 1 ]; then + GOT="$(kubectl -n "$NS" logs "${P}-probe" -c probe 2>/dev/null | grep -E '^[0-9a-f]{64} ' | sort)" + WANT="$(sort "$MS_SHA_JSON")" + if [ "$GOT" = "$WANT" ]; then + record PASS "SDK pin" "modelscope 1.39.1 downloaded at the commit; the files hash to the hub's" + else + record FAIL "SDK pin" "the probe's hashes differ from the hub's listing" + fi +else + record FAIL "SDK pin" "probe did not succeed: $(kubectl -n "$NS" logs "${P}-probe" -c probe 2>/dev/null | tail -2 | tr '\n' ' ')" +fi + +echo "== 5. no token anywhere ==" +LEAK="$(kubectl -n "$NS" logs -l "e2e.gpustack.ai/consumer=true" 2>/dev/null | grep -c 'hf_' || true)" +if [ "$LEAK" = 0 ]; then + record PASS "no token" "no token-shaped string in any Pod log (none exists to leak)" +else + record FAIL "no token" "a token-shaped string appeared in a Pod log" +fi + +rm -f "$MS_SHA_JSON" +print_rows "${ROWS[@]}" +exit "$FAILS" diff --git a/.agents/skills/gpustack-operator-e2e/cases/case-97.sh b/.agents/skills/gpustack-operator-e2e/cases/case-97.sh index b75f65bb8..1de1b778e 100644 --- a/.agents/skills/gpustack-operator-e2e/cases/case-97.sh +++ b/.agents/skills/gpustack-operator-e2e/cases/case-97.sh @@ -22,8 +22,10 @@ # bigscience/bloom-560m (annotated tag gs555750); private E2E_C97_PRIVATE; gated # E2E_C97_GATED (default XyX824/mam-d-gated-tiny). The token only ever reaches a Secret # created from the environment; it is never written to a file or printed. -# Expected: - admission refuses ModelScope, two sources, no source, a three-part repository, a -# revision with whitespace, an absolute or dot-dot claim path, and a spec edit; +# Expected: - admission refuses two sources, no source, a three-part repository, a +# revision with whitespace, an absolute or dot-dot claim path, and a spec edit; a +# well-formed ModelScope source is admitted and a bad ModelScope repository is +# refused; # - a tree of four pages resolves to the digest recorded for that commit; # - the branch resolves to `git ls-remote`'s main, the tag to its peeled commit, and # the digest equals testdata/manifest/canonical_manifest.py over the same tree; @@ -145,13 +147,28 @@ refused() { # check manifest-on-stdin fi } +admitted() { # check manifest-on-stdin + local out + if out="$(kubectl apply --dry-run=server -f - 2>&1)"; then + record PASS "$1" "$(printf '%s' "$out" | tr '\n' ' ' | cut -c1-160)" + else + record FAIL "$1" "refused: $(printf '%s' "$out" | tr '\n' ' ' | cut -c1-160)" + fi +} + echo "== 1. admission ==" -refused "ModelScope is refused" <= 64 { + return ErrIntOverflowGenerated + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthGenerated + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthGenerated + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.ModelScopeEndpoint = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex default: iNdEx = preIndex skippy, err := skipGenerated(dAtA[iNdEx:]) diff --git a/api/worker/v1alpha1/generated.proto b/api/worker/v1alpha1/generated.proto index 86f450b43..cd49c37b5 100644 --- a/api/worker/v1alpha1/generated.proto +++ b/api/worker/v1alpha1/generated.proto @@ -4735,6 +4735,13 @@ message NodeModelStoreHub { // // +optional optional string caBundleConfigMap = 4; + + // ModelScopeEndpoint is the ModelScope hub's base URL. It is empty in a spec an older worker + // wrote, and a ModelScope artifact on such a node waits with that named rather than being + // resolved against another hub. + // + // +optional + optional string modelScopeEndpoint = 5; } // NodeModelStoreKubelet is the part of a node's effective kubelet configuration that bounds a cache diff --git a/api/worker/v1alpha1/node_model_store.go b/api/worker/v1alpha1/node_model_store.go index 9aedadc6d..607436150 100644 --- a/api/worker/v1alpha1/node_model_store.go +++ b/api/worker/v1alpha1/node_model_store.go @@ -186,6 +186,13 @@ type NodeModelStoreHub struct { // // +optional CABundleConfigMap string `json:"caBundleConfigMap,omitempty" protobuf:"bytes,4,opt,name=caBundleConfigMap"` + + // ModelScopeEndpoint is the ModelScope hub's base URL. It is empty in a spec an older worker + // wrote, and a ModelScope artifact on such a node waits with that named rather than being + // resolved against another hub. + // + // +optional + ModelScopeEndpoint string `json:"modelScopeEndpoint,omitempty" protobuf:"bytes,5,opt,name=modelScopeEndpoint"` } // NodeModelStoreStatus is what the plugin reports about its node, rebuilt from the node's disk and diff --git a/api/worker/v1alpha1/zz_generated.crds.go b/api/worker/v1alpha1/zz_generated.crds.go index 156e91cbb..45cf66052 100644 --- a/api/worker/v1alpha1/zz_generated.crds.go +++ b/api/worker/v1alpha1/zz_generated.crds.go @@ -6527,6 +6527,10 @@ func crd_gpustack_api_worker_v1alpha1_NodeModelStore() *v1.CustomResourceDefinit Type: "string", MinLength: ptr.To[int64](1), }, + "modelScopeEndpoint": { + Description: "ModelScopeEndpoint is the ModelScope hub's base URL. It is empty in a spec an older worker\nwrote, and a ModelScope artifact on such a node waits with that named rather than being\nresolved against another hub.", + Type: "string", + }, "noProxy": { Description: "NoProxy is the comma-separated host list that bypasses HTTPSProxy.", Type: "string", diff --git a/api/worker/zz_generated.openapi.go b/api/worker/zz_generated.openapi.go index 017d729a7..0aef2d4a9 100644 --- a/api/worker/zz_generated.openapi.go +++ b/api/worker/zz_generated.openapi.go @@ -10836,6 +10836,13 @@ func schema_gpustack_api_worker_v1alpha1_NodeModelStoreHub(ref common.ReferenceC Format: "", }, }, + "modelScopeEndpoint": { + SchemaProps: spec.SchemaProps{ + Description: "ModelScopeEndpoint is the ModelScope hub's base URL. It is empty in a spec an older worker wrote, and a ModelScope artifact on such a node waits with that named rather than being resolved against another hub.", + Type: []string{"string"}, + Format: "", + }, + }, }, Required: []string{"huggingFaceEndpoint"}, }, diff --git a/docs/model-store/artifact.md b/docs/model-store/artifact.md index 43ffd6822..d0603d96b 100644 --- a/docs/model-store/artifact.md +++ b/docs/model-store/artifact.md @@ -36,12 +36,16 @@ spec: # immutable after creation repository: Qwen/Qwen2.5-7B-Instruct revision: main # branch, tag or commit; defaults to main secretRef: {name: hf-token} # optional; this namespace; key "token" + # modelScope: + # repository: qwen/Qwen2.5-7B-Instruct + # revision: master # branch, tag or commit; defaults to master + # secretRef: {name: ms-token} # optional; this namespace; key "token" # persistentVolumeClaim: # claimName: models # this namespace # path: qwen # directory inside the volume; empty is the root # image: # reference: registry.example.com/team/qwen@sha256:669ed7b1...48 # digest-pinned - allowPatterns: ["*.safetensors", "*.json", "tokenizer*"] # optional; Hugging Face only + allowPatterns: ["*.safetensors", "*.json", "tokenizer*"] # optional; hub sources only ignorePatterns: ["original/"] # optional; wins over allowPatterns status: resolved: @@ -49,7 +53,7 @@ status: manifestDigest: sha256:669ed7b128b6ad1658d735326bd172a33497ecdb8bbd72dd0b23c98b58469448 fileCount: 10 sizeBytes: 999604126 - nodes: # Hugging Face only; where the content is across nodes + nodes: # hub sources only; where the content is across nodes ready: 3 downloading: 1 failed: 0 @@ -63,9 +67,10 @@ status: new artifact. That is also what lets a deployment's frozen reference pin anything. - **The Secret and the claim need not exist yet.** Admission does not read them; their absence is a reason in status (`SecretNotFound`, `ClaimNotFound`). -- **A `modelScope` member exists and is refused.** Opening it needs branch resolution checked - against git, a listing that recovers from the API's silent truncation at 3000 entries, and a vLLM - runner whose ModelScope SDK accepts a commit (1.39.1 or later). +- **A `modelScope` member is a second hub.** Its repository and revision rules are the Hugging Face + ones, the revision defaults to `master`, and the patterns apply to it the same way. What differs + is [how it resolves](#resolution-and-revalidation) and the [engine + environment](#engine-delivery) an Engine download reads. - **An `image` member delivers weights already in a registry.** A digest-pinned reference is the artifact's whole identity; kubelet pulls and mounts it through an image volume, outside the node cache. The digest contract, the build, the floors and the costs are on the @@ -103,9 +108,31 @@ A repository that does not exist and a private one the token cannot read answer message says "does not exist or is not accessible". A gated repository answers 200 on the revision and tree endpoints and only masks its digests, which is why the masked tree is its own row. -**A mistyped token is silent on a public repository**: the Hub answers as if no token had been sent. -The controller therefore checks each new token with `/api/whoami-v2` and emits a Warning event -`InvalidToken` when the Hub rejects it; `Resolved` is unchanged. +**A ModelScope source resolves the same way, against its own API.** A full commit is taken as is; a +branch or tag is resolved through `GET /api/v1/models//commits?Ref=` and **cross-checked +against git**: `git ls-remote` over `https://www.modelscope.cn/.git` must name the same commit +(the peeled entry, for an annotated tag). + +A disagreement refuses the resolution as `SourceUnavailable` — the hub's index and its git +disagreeing is exactly how a misspelled parameter would silently resolve the wrong revision. A +private repository authenticates ls-remote with the user name `oauth2` and the namespace's token. + +The file listing walks `repo/files?Revision=&Recursive=true`; the API truncates **silently +at 3000 entries**, so a full page is re-listed per directory, and a directory with 3000 or more +direct children refuses (`SourceUnavailable`) — the API cannot enumerate it, and a partial +manifest would be a silent wrong answer. Every file must carry the hub's `sha256`. + +| ModelScope answers | `Resolved` reason | +| --- | --- | +| `commits` answers no commit for the ref, or the listing has no file tree at the revision (code 10990101004) | `RevisionNotFound` | +| 404 code 10010205001 (not found) or 10010200001 (no access), any other 401/403/404 | `AccessDenied` | +| 5xx, a network error, a directory the API cannot enumerate | `SourceUnavailable` | + +ModelScope has no distinct 401 or 403, and "no access" covers private-without-token, +gated-without-grant and valid-token-without-grant alike — the message says "does not exist or is +not accessible" as on Hugging Face. **A mistyped token is silent there too**, so the controller +checks each new token with `GET /openapi/v1/users/me` and emits the same `InvalidToken` Warning +when the hub rejects it. **Access is revalidated** every `model-artifact-revalidate-interval` (default `24h`) and whenever the Secret changes, with one `HEAD` of a file at the resolved commit, redirects not followed: @@ -117,6 +144,10 @@ Secret changes, with one `HEAD` of a file at the resolved commit, redirects not - a deleted Secret, or one without its `token` key, sets `Resolved=False` at once; - a later pass restores `Resolved=True`, with the commit and digest unchanged. +The HEAD is `/api/models//resolve//` on Hugging Face and +`/api/v1/models//repo?Revision=&FilePath=` on ModelScope, where a 200 — an LFS +file's too — confirms and every 404 is an access refusal. + `Resolved=False` stops **new** consumption: no new replica, replacement or scale-up. It never deletes a running Pod. A claim source is resolved by the claim existing; the operator never reads its content, so it has no revision and no digest. @@ -133,7 +164,8 @@ sha256:8111d5af… 453864 model.safetensors ``` - A line is `: `, sorted by the path's UTF-8 bytes, every line ending - in LF. An LFS file uses its `sha256`, any other file its git blob `gitsha1`. + in LF. An LFS file uses its `sha256`, any other file its git blob `gitsha1`; **a ModelScope + artifact's manifest is all `sha256` lines**, the hub giving no other digest. - A path must be valid UTF-8, relative, with no control character and no empty, `.` or `..` segment; the whole resolution fails on one that is not. - The source, the repository, the commit and the patterns are **not** part of it, so the same files @@ -161,12 +193,13 @@ spec: reference to an artifact that does not exist or has not resolved is admitted, and the deployment creates no Pod until it resolves. -A claim source is always mounted directly. A Hugging Face source takes the delivery the +A claim source is always mounted directly. A hub source (Hugging Face or ModelScope) takes the +delivery the `model-artifact-delivery-mode` Setting names, `Engine` by default and `Node` where the chart deploys the node plugin ([switching it](operations.md#switch-delivery) rolls each such deployment once): -| | Claim source (`Pvc`) | Hugging Face, `Engine` | Hugging Face, `Node` | Image source (`Image`) | +| | Claim source (`Pvc`) | Hub source, `Engine` | Hub source, `Node` | Image source (`Image`) | | --- | --- | --- | --- | --- | | Weights | the claim, read-only, at `/var/lib/gpustack/model`, `subPath` = `path` | downloaded by the engine into `/var/lib/gpustack/model-cache` | the node's verified copy, read-only, at `/var/lib/gpustack/model` | the image, read-only, at `/var/lib/gpustack/model` | | vLLM | `vllm serve /var/lib/gpustack/model` | `vllm serve --revision ` | as a claim | as a claim | @@ -190,7 +223,9 @@ limit is not raised: the volume's bytes are not the Pod's. While `artifactRef` is set, admission refuses: - on vLLM `--model`, `--revision`, `--tokenizer-revision` and `--download-dir`, on SGLang - `--model-path`, `--revision` and `--download-dir`, and `HF_TOKEN`, `HF_ENDPOINT` and `HF_HOME` in + `--model-path`, `--revision` and `--download-dir`, and `HF_TOKEN`, `HF_ENDPOINT`, `HF_HOME`, + `MODELSCOPE_API_TOKEN`, `MODELSCOPE_DOMAIN`, `MODELSCOPE_CACHE` and the engine's + `VLLM_USE_MODELSCOPE` / `SGLANG_USE_MODELSCOPE` in `env`, whatever the source — a later value would silently replace the artifact's; - a role volume at, inside or around `/var/lib/gpustack/model` or `/var/lib/gpustack/model-cache`. @@ -208,8 +243,16 @@ Under `Engine`, the engine downloads the pinned commit itself, with: | `HF_HOME` | `/var/lib/gpustack/model-cache` | yes | | `HF_ENDPOINT` | the `model-artifact-huggingface-endpoint` Setting | yes | | `HF_TOKEN` | a `secretKeyRef` to the artifact's Secret, key `token`; the value never enters the Pod spec | yes | +| `MODELSCOPE_CACHE` | `/var/lib/gpustack/model-cache` | yes | +| `MODELSCOPE_DOMAIN` | the `model-artifact-modelscope-endpoint` Setting's bare host — the runners' SDK prefixes the scheme itself | yes | +| `MODELSCOPE_API_TOKEN` | a `secretKeyRef` to the artifact's Secret, key `token` | yes | +| `VLLM_USE_MODELSCOPE` / `SGLANG_USE_MODELSCOPE` | `true`, routing the engine's own download through its bundled ModelScope SDK; each engine reads only its own | yes | | `HTTPS_PROXY`, `NO_PROXY` | the proxy Settings, when set | no — a role's own value wins | +A ModelScope artifact renders the `MODELSCOPE_*` rows; a Hugging Face artifact the `HF_*` rows. +The ModelScope rows reach the engine through the runner's bundled SDK, whose version decides +whether a commit is accepted — see [Requirements](#requirements-and-limits). + The cache is an `emptyDir` with a `sizeLimit` of the manifest's size plus a tenth, and at least 1 GiB more. The manifest is the whole commit, so it bounds whatever subset the engine downloads (vLLM skips `.bin` files when `.safetensors` exist). @@ -336,6 +379,14 @@ waits instead when that node cannot run one, naming the node and the floor - **Kubernetes 1.29**, the floor the bundled Kueue already sets. `PodReadyToStartContainers` (beta, on by default since 1.29) feeds `WeightsReady`; with it off, a claim deployment's `WeightsReady` stays `WeightsNotMounted` while its replicas run. +- **A ModelScope Engine download needs the runner's ModelScope SDK at 1.39.1 or later**, the + version that accepts a commit as the revision. The operator renders the environment whatever the + runner holds and cannot see into it: on a runner below the floor the engine fails with the SDK's + own `NotExistError`. +- **The floor was measured against the published runners.** The Ascend `cann9.1-*-vllm0.23.0` line + and the SGLang 0.5.18 line meet it; the current CUDA `vllm0.25.1` and `vllm0.29.0` lines do not. + For those, name a runner image of your own that bundles a conforming SDK, as any role may. Node + delivery needs no runner at all. - **Image sources need image volumes** and floors above Kubernetes's own; creation is refused below them. The full line is on the [Model Image Source](image-source.md#versions-and-prerequisites). @@ -348,8 +399,8 @@ waits instead when that node cannot run one, naming the node and the floor [a node-delivered model prefers the nodes holding it](../architecture/topology-aware-scheduling.md#a-node-delivered-model-prefers-the-nodes-holding-it). - Settings: [Settings & Environment Variables](../settings.md#online-adjustable-settings) carries the - endpoint, proxy, no-proxy, CA bundle, revalidation interval, delivery mode and the node cache's - watermarks and download limits. + two hub endpoints, proxy, no-proxy, CA bundle, revalidation interval, delivery mode and the node + cache's watermarks and download limits. --- diff --git a/docs/settings.md b/docs/settings.md index 562198805..a56df113b 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -48,8 +48,9 @@ kubectl -n gpustack-system patch setting instance-type-derived-from-node --type | `model-deployment-router-proxy-image` | `GPUSTACK_MODEL_DEPLOYMENT_ROUTER_PROXY_IMAGE` | `gpustack/mirrored-envoy:distroless-v1.33.2` | Proxy fronting a managed router's endpoint picker. It has **no field on the API** to override it: this operator renders the proxy's configuration against one proxy's configuration schema, so swapping the binary would mean swapping that configuration too. The setting exists for registry redirection and for pinning a release back, not for running a different proxy. | | `model-deployment-routing-sidecar-image` | `GPUSTACK_MODEL_DEPLOYMENT_ROUTING_SIDECAR_IMAGE` | `gpustack/mirrored-llm-d-router-disagg-sidecar:v0.10.0` | Sidecar a decoder runs to accept a remote prefill handoff. Same terms as the proxy above: this operator renders its arguments, so the setting is for redirection and pinning rather than for a different implementation. | | `model-deployment-tcp-tw-reuse` | `GPUSTACK_MODEL_DEPLOYMENT_TCP_TW_REUSE` | `false` | Render `net.ipv4.tcp_tw_reuse=1` on the prefill half of every SGLang prefill/decode pair, which otherwise [runs out of local ports](model-deployment/engine-versions.md#known-failures-at-the-minimum) under sustained load. **Allow the sysctl on the kubelet of every node that can run such a Pod before turning it on**: without that the Pod fails with `SysctlForbidden`, and so does every replacement. The steps, which Pods it reaches and how to confirm the refusal are under [Letting SGLang prefill Pods reuse TIME-WAIT ports](#letting-sglang-prefill-pods-reuse-time-wait-ports). | -| `model-prefetch-warmup-image` | `GPUSTACK_MODEL_PREFETCH_WARMUP_IMAGE` | `python:3.12-alpine` | Image a [`ModelPrefetch`](model-store/prefetch.md)'s warm-up pod runs on each target node. The pod mounts the artifact's volume, verifies every file reads back and exits, so the image needs only a shell, `find` and `sha256sum` — it never runs a model. It is a setting because the one thing it must guarantee is that the node can pull it, which only the cluster's own registry arrangement decides; an air-gapped cluster points it at its mirror. | +| `model-prefetch-warmup-image` | `GPUSTACK_MODEL_PREFETCH_WARMUP_IMAGE` | `gpustack/mirrored-python:3.12-alpine` | Image a [`ModelPrefetch`](model-store/prefetch.md)'s warm-up pod runs on each target node. The pod mounts the artifact's volume, verifies every file reads back and exits, so the image needs only a shell, `find` and `sha256sum` — it never runs a model. It is a setting because the one thing it must guarantee is that the node can pull it, which only the cluster's own registry arrangement decides; an air-gapped cluster points it at its mirror. | | `model-artifact-huggingface-endpoint` | `GPUSTACK_MODEL_ARTIFACT_HUGGINGFACE_ENDPOINT` | `https://huggingface.co` | The Hugging Face Hub a [`ModelArtifact`](model-store/artifact.md) resolves and revalidates against, and the `HF_ENDPOINT` an engine downloading the weights is given; the node plugin downloads from it too. What changing it does is under [Engine delivery](model-store/artifact.md#engine-delivery). | +| `model-artifact-modelscope-endpoint` | `GPUSTACK_MODEL_ARTIFACT_MODELSCOPE_ENDPOINT` | `https://www.modelscope.cn` | The ModelScope hub a [`ModelArtifact`](model-store/artifact.md) resolves and revalidates against and the node plugin downloads from; an engine downloading the weights gets its host as `MODELSCOPE_DOMAIN` — the runners' SDK takes a bare host. What changing it does is under [Engine delivery](model-store/artifact.md#engine-delivery). | | `model-artifact-https-proxy` | `GPUSTACK_MODEL_ARTIFACT_HTTPS_PROXY` | *(blank)* | HTTPS proxy for resolution, revalidation and the node plugin's downloads, and the `HTTPS_PROXY` given to an engine downloading the weights, where a role's own value wins. Blank keeps the worker's own environment and renders nothing. Only an `http` or `https` URL without credentials is accepted, because the value reaches tenant Pods; a value from the environment that is not refuses the worker's start. | | `model-artifact-no-proxy` | `GPUSTACK_MODEL_ARTIFACT_NO_PROXY` | *(blank)* | Comma-separated hosts that bypass `model-artifact-https-proxy`, for the node plugin too, given to engines as `NO_PROXY` on the same terms. | | `model-artifact-ca-bundle` | `GPUSTACK_MODEL_ARTIFACT_CA_BUNDLE` | *(blank)* | Name of a ConfigMap in the worker's namespace whose `ca.crt` the resolution and the node plugin trust beside the system pool. **It is not given to engine Pods**: they run in tenant namespaces, which cannot mount it. | diff --git a/pkg/kubeclients/applyconfiguration/worker/v1alpha1/nodemodelstorehub.go b/pkg/kubeclients/applyconfiguration/worker/v1alpha1/nodemodelstorehub.go index 30f8cee2f..1d3a617f7 100644 --- a/pkg/kubeclients/applyconfiguration/worker/v1alpha1/nodemodelstorehub.go +++ b/pkg/kubeclients/applyconfiguration/worker/v1alpha1/nodemodelstorehub.go @@ -18,6 +18,10 @@ type NodeModelStoreHubApplyConfiguration struct { // certificates trusted beside the system pool. The name travels rather than the certificates, // because a bundle can be larger than this object may be. CABundleConfigMap *string `json:"caBundleConfigMap,omitempty"` + // ModelScopeEndpoint is the ModelScope hub's base URL. It is empty in a spec an older worker + // wrote, and a ModelScope artifact on such a node waits with that named rather than being + // resolved against another hub. + ModelScopeEndpoint *string `json:"modelScopeEndpoint,omitempty"` } // NodeModelStoreHubApplyConfiguration constructs a declarative configuration of the NodeModelStoreHub type for use with @@ -57,3 +61,11 @@ func (b *NodeModelStoreHubApplyConfiguration) WithCABundleConfigMap(value string b.CABundleConfigMap = &value return b } + +// WithModelScopeEndpoint sets the ModelScopeEndpoint field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the ModelScopeEndpoint field is set to the value of the last call. +func (b *NodeModelStoreHubApplyConfiguration) WithModelScopeEndpoint(value string) *NodeModelStoreHubApplyConfiguration { + b.ModelScopeEndpoint = &value + return b +} diff --git a/pkg/modelartifact/modelscope.go b/pkg/modelartifact/modelscope.go new file mode 100644 index 000000000..5ff00b992 --- /dev/null +++ b/pkg/modelartifact/modelscope.go @@ -0,0 +1,627 @@ +package modelartifact + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "regexp" + "strconv" + "strings" + + "gpustack.ai/gpustack/pkg/utils/httpx" +) + +// ModelScope resolves ModelScope repositories. +// +// Every method takes the token rather than holding one, so a client is shared across namespaces +// without ever carrying a namespace's credential beyond one call. +type ModelScope struct { + // Endpoint is the hub's base URL, e.g. "https://www.modelscope.cn". + Endpoint string + // Client sends the requests; see NewHTTPClient. + Client *http.Client + // MaxEntries bounds the tree entries of one resolution. Zero uses + // DefaultHuggingFaceMaxEntries, the bound the two hubs share. + MaxEntries int + + // lsRemote asks the repository's git endpoint for its refs, keyed by ref name. Tests replace + // it; the default speaks the HTTP smart protocol itself. + lsRemote func(ctx context.Context, gitURL, token string) (map[string]string, error) +} + +// modelScopeCommitPattern is the shape of a full commit, which is used as is and cross-checked +// against git only when the revision is a branch or a tag. +var modelScopeCommitPattern = regexp.MustCompile(`^[0-9a-f]{40}$`) + +// resolveRevision resolves a branch or a tag to the commit both the hub's index and its git name, +// and takes a full commit as is. +// +// The commits endpoint is undocumented and answers master when its Ref parameter is misspelled, +// so its answer is never trusted alone: for a branch it must equal the commit git names for the +// same ref, and for an annotated tag the peeled entry git names for the tag. A disagreement +// refuses the resolution: a drift breaks toward refusal, never toward wrong content. +func (m *ModelScope) resolveRevision(ctx context.Context, repository, revision, token string) (string, error) { + if modelScopeCommitPattern.MatchString(revision) { + return revision, nil + } + + var answer struct { + Commit []struct { + Id string `json:"Id"` + } `json:"Commit"` + } + target := m.apiURL("models", repository, "commits") + "?Ref=" + url.QueryEscape(revision) + "&PageSize=1" + if err := m.getJSON(ctx, target, token, &answer); err != nil { + return "", err + } + if len(answer.Commit) == 0 { + return "", sourceErrorf(ReasonRevisionNotFound, "the repository has no branch or tag %q", revision) + } + commit := answer.Commit[0].Id + if !modelScopeCommitPattern.MatchString(commit) { + return "", sourceErrorf(ReasonSourceUnavailable, + "the hub answered revision %q of %q with a commit %q that is not 40 hexadecimal digits", + revision, repository, commit) + } + + refs, err := m.runLsRemote(ctx, repository, token) + if err != nil { + return "", err + } + against := modelScopeRefCommit(refs, revision) + switch { + case against == "": + return "", sourceErrorf(ReasonSourceUnavailable, + "the hub's git names no ref for %q of %q: the commits endpoint answered %s, and the two must agree", + revision, repository, commit) + case against != commit: + return "", sourceErrorf(ReasonSourceUnavailable, + "the hub's index and its git disagree about %q of %q: the commits endpoint answers %s, git answers %s", + revision, repository, commit, against) + } + + return commit, nil +} + +// modelScopeRefCommit picks the commit git names for a revision: the peeled entry of an +// annotated tag, the tag itself if it is lightweight, the branch's ref. HEAD is deliberately not +// a fallback: the commits endpoint answers master for a misspelled Ref, and falling back to HEAD +// — which points at master — would confirm exactly that silent rewrite instead of catching it. +func modelScopeRefCommit(refs map[string]string, revision string) string { + for _, name := range []string{ + "refs/tags/" + revision + "^{}", "refs/tags/" + revision, "refs/heads/" + revision, + } { + if commit, ok := refs[name]; ok { + return commit + } + } + + return "" +} + +// modelScopePageBound is the listing size at which repo/files truncates silently (measured, +// PoC-D), whatever pagination the query names: a page of this many entries is a truncation, and +// only a walk over direct children can recover the tree. +const modelScopePageBound = 3000 + +// Resolve resolves revision to a commit and builds the manifest of the files at it that filter +// selects. +func (m *ModelScope) Resolve(ctx context.Context, repository, revision, token string, filter Filter) (Resolution, error) { + commit, err := m.resolveRevision(ctx, repository, revision, token) + if err != nil { + return Resolution{}, err + } + + manifest, err := m.ListManifest(ctx, repository, commit, token, filter) + if err != nil { + return Resolution{}, err + } + + return Resolution{Commit: commit, Manifest: manifest}, nil +} + +// ListManifest builds the manifest of the files at commit that filter selects, from the hub's +// file listing. A node recomputes an artifact's manifest this way, with the credential of the Pod +// that mounts it, and compares the digest with the one the controller published. Every file's +// digest is the hub's own sha256, so a ModelScope manifest has no gitsha1 lines. +func (m *ModelScope) ListManifest(ctx context.Context, repository, commit, token string, filter Filter) (Manifest, error) { + entries, err := m.listRepoFiles(ctx, repository, commit, token) + if err != nil { + return Manifest{}, err + } + if len(entries) == 0 { + return Manifest{}, sourceErrorf(ReasonEmptyManifest, "commit %s of %q holds no file", commit, repository) + } + entries = FilterEntries(entries, filter.Allow, filter.Ignore) + if len(entries) == 0 { + return Manifest{}, sourceErrorf(ReasonEmptyManifest, + "no file of commit %s of %q matches the allow and ignore patterns", commit, repository) + } + manifest, err := NewManifest(entries) + if err != nil { + return Manifest{}, sourceErrorf(ReasonInvalidManifest, "commit %s of %q: %v", commit, repository, err) + } + + return manifest, nil +} + +// listRepoFiles lists every file at the commit, recovering from the API's silent truncation. +// +// A recursive page of exactly modelScopePageBound entries is a truncation: the walk lists that +// level's direct children instead and descends into every directory of it, and only subtrees +// that truncate are broken up further. A level whose direct children reach the bound cannot be +// enumerated at all, and a partial manifest would be a silent wrong answer, so it refuses. +func (m *ModelScope) listRepoFiles(ctx context.Context, repository, commit, token string) ([]ManifestEntry, error) { + limit := m.MaxEntries + if limit <= 0 { + limit = DefaultHuggingFaceMaxEntries + } + + var entries []ManifestEntry + var walk func(dir string) error + walk = func(dir string) error { + page, truncated, err := m.listFiles(ctx, repository, commit, dir, true, token) + if err != nil { + return err + } + if !truncated { + for _, e := range page { + entry, ok, err := modelScopeManifestEntry(e) + if err != nil { + return err + } + if ok { + entries = append(entries, entry) + if len(entries) > limit { + return sourceErrorf(ReasonManifestTooLarge, + "commit %s of %q lists more than %d tree entries", commit, repository, limit) + } + } + } + return nil + } + + children, truncated, err := m.listFiles(ctx, repository, commit, dir, false, token) + if err != nil { + return err + } + if truncated { + return sourceErrorf(ReasonSourceUnavailable, + "directory %q of commit %s lists %d or more direct children, which the API cannot enumerate", + dir, commit, modelScopePageBound) + } + for _, e := range children { + if e.Type == modelScopeTypeTree { + if err := walk(e.Path); err != nil { + return err + } + continue + } + entry, ok, err := modelScopeManifestEntry(e) + if err != nil { + return err + } + if ok { + entries = append(entries, entry) + if len(entries) > limit { + return sourceErrorf(ReasonManifestTooLarge, + "commit %s of %q lists more than %d tree entries", commit, repository, limit) + } + } + } + return nil + } + if err := walk(""); err != nil { + return nil, err + } + + return entries, nil +} + +// modelScopeTypeTree is a directory entry's type; every file is a blob. +const modelScopeTypeTree = "tree" + +// modelScopeFileEntry is one entry of a repo/files page. +type modelScopeFileEntry struct { + Type string `json:"Type"` + Path string `json:"Path"` + Size int64 `json:"Size"` + Sha256 string `json:"Sha256"` +} + +// modelScopeManifestEntry turns a listing entry into a manifest entry, and reports false for a +// directory. Every file must carry the hub's sha256: ModelScope gives no alternative digest, so +// an entry without one is a manifest the format cannot express. +func modelScopeManifestEntry(e modelScopeFileEntry) (ManifestEntry, bool, error) { + switch e.Type { + case modelScopeTypeTree: + return ManifestEntry{}, false, nil + case "blob": + default: + return ManifestEntry{}, false, sourceErrorf(ReasonInvalidManifest, + "entry %q is of type %q, which is neither a blob nor a tree", e.Path, e.Type) + } + if len(e.Sha256) != 64 || strings.ContainsFunc(e.Sha256, func(r rune) bool { + return !(r >= '0' && r <= '9' || r >= 'a' && r <= 'f') + }) { + return ManifestEntry{}, false, sourceErrorf(ReasonInvalidManifest, + "entry %q names no usable Sha256, which every manifest line requires", e.Path) + } + + return ManifestEntry{Path: e.Path, Size: e.Size, Digest: DigestSHA256 + ":" + e.Sha256}, true, nil +} + +// listFiles lists one level: the whole subtree below root when recursive, root's direct children +// otherwise. A page at the bound is a truncation, whatever it holds. +func (m *ModelScope) listFiles( + ctx context.Context, repository, commit, root string, recursive bool, token string, +) ([]modelScopeFileEntry, bool, error) { + target := m.apiURL("models", repository, "repo", "files") + "?Revision=" + url.QueryEscape(commit) + if root != "" { + target += "&Root=" + url.QueryEscape(root) + } + if recursive { + target += "&Recursive=true" + } + + var answer struct { + Files []modelScopeFileEntry `json:"Files"` + } + if err := m.getJSON(ctx, target, token, &answer); err != nil { + return nil, false, err + } + + return answer.Files, len(answer.Files) >= modelScopePageBound, nil +} + +// Revalidate checks that token can still read the repository at commit: one HEAD of the +// repo endpoint for the first file, with redirects not followed. +// +// Measured against the hub: a HEAD of an accessible file answers 200 — an LFS file's too, where +// the GET would redirect to the CDN, so a redirect confirms as well — and a missing commit, path +// or repository and a gated repository without a grant all answer 404 with no envelope. The +// first file comes from one listing of the root, so the check costs two requests whatever the +// repository's size. +func (m *ModelScope) Revalidate(ctx context.Context, repository, commit, token string) error { + first, err := m.firstRootFile(ctx, repository, commit, token) + if err != nil { + return err + } + + target := m.fileURL(repository, commit, first) + resp, err := m.doNoRedirect(ctx, http.MethodHead, target, token) + if err != nil { + return err + } + defer httpx.Close(resp) + switch { + case resp.StatusCode == http.StatusOK: + return nil + case resp.StatusCode >= 300 && resp.StatusCode < 400: + // A redirect names a CDN path the hub only grants for a readable repository. + return nil + default: + return classifyModelScope(resp.StatusCode, 0, resp.Request.URL.Path) + } +} + +// firstRootFile names the file the revalidation HEADs: the first file of the root by path, from +// one direct-children listing; a root holding no file falls back to the recursive listing. +func (m *ModelScope) firstRootFile(ctx context.Context, repository, commit, token string) (string, error) { + page, _, err := m.listFiles(ctx, repository, commit, "", false, token) + if err != nil { + return "", err + } + first := "" + for _, e := range page { + if e.Type != modelScopeTypeTree && (first == "" || e.Path < first) { + first = e.Path + } + } + if first != "" { + return first, nil + } + + page, _, err = m.listFiles(ctx, repository, commit, "", true, token) + if err != nil { + return "", err + } + for _, e := range page { + if e.Type != modelScopeTypeTree && (first == "" || e.Path < first) { + first = e.Path + } + } + if first == "" { + return "", sourceErrorf(ReasonEmptyManifest, "commit %s of %q holds no file to revalidate against", commit, repository) + } + + return first, nil +} + +// FileURL is where a file of repository at commit is downloaded from. The hub answers a small +// file with the bytes and an LFS file with a redirect to a signed, content-addressed CDN path +// that honors byte ranges. +func (m *ModelScope) FileURL(repository, commit, path string) string { + return m.fileURL(repository, commit, path) +} + +func (m *ModelScope) fileURL(repository, commit, path string) string { + return m.apiURL("models", repository, "repo") + + "?Revision=" + url.QueryEscape(commit) + "&FilePath=" + url.QueryEscape(path) +} + +// ValidToken reports whether the hub accepts token at all: an answered users/me. A token the hub +// rejects is silently ignored on a public repository, so this is the only way such a mistake +// becomes visible. +func (m *ModelScope) ValidToken(ctx context.Context, token string) (bool, error) { + target := strings.TrimSuffix(m.Endpoint, "/") + "/openapi/v1/users/me" + resp, err := m.doNoRedirect(ctx, http.MethodGet, target, token) + if err != nil { + return false, err + } + defer httpx.Close(resp) + switch { + case resp.StatusCode == http.StatusOK: + return true, nil + case resp.StatusCode == http.StatusUnauthorized, resp.StatusCode == http.StatusForbidden: + return false, nil + default: + return false, sourceErrorf(ReasonSourceUnavailable, + "the token check against %s: HTTP %d", target, resp.StatusCode) + } +} + +// gitURL is the repository's git endpoint, the hub's host serving the same id. +func (m *ModelScope) gitURL(repository string) (string, error) { + base, err := url.Parse(m.Endpoint) + if err != nil { + return "", sourceErrorf(ReasonSourceUnavailable, "the endpoint %q does not parse: %v", m.Endpoint, err) + } + base.Path = strings.TrimSuffix(base.Path, "/") + "/" + strings.TrimSuffix(escapeRepository(repository), "/") + ".git" + base.RawQuery, base.Fragment = "", "" + + return base.String(), nil +} + +// runLsRemote runs the git cross-check for one repository. +func (m *ModelScope) runLsRemote(ctx context.Context, repository, token string) (map[string]string, error) { + run := m.lsRemote + if run == nil { + run = m.lsRemoteOverHTTP + } + gitURL, err := m.gitURL(repository) + if err != nil { + return nil, err + } + refs, err := run(ctx, gitURL, token) + if err != nil { + return nil, sourceErrorf(ReasonSourceUnavailable, "the git cross-check of %s: %v", gitURL, err) + } + + return refs, nil +} + +// lsRemoteOverHTTP asks the repository's git endpoint for its refs over the HTTP smart protocol — +// the same request `git ls-remote` answers — so the cross-check needs no git binary in the image +// and no subprocess: the endpoint's client is the one the Settings configure, and a private +// repository authenticates as the user oauth2 with the token as the password, an HTTP header +// rather than anything another process could read. +func (m *ModelScope) lsRemoteOverHTTP(ctx context.Context, gitURL, token string) (map[string]string, error) { + target := gitURL + "/info/refs?service=git-upload-pack" + req, err := http.NewRequestWithContext(ctx, http.MethodGet, target, nil) + if err != nil { + return nil, err + } + // The endpoint answers a git client and refuses others with 421 (measured), so the request + // names itself the protocol it speaks. + req.Header.Set("User-Agent", "git/2.39") + req.Header.Set("Accept", "application/x-git-upload-pack-advertisement") + if token != "" { + req.SetBasicAuth("oauth2", token) + } + + resp, err := m.Client.Do(req) + if err != nil { + return nil, fmt.Errorf("%s %s: %v", http.MethodGet, target, unwrapURLError(err)) + } + defer httpx.Close(resp) + switch { + case resp.StatusCode == http.StatusUnauthorized, resp.StatusCode == http.StatusForbidden: + return nil, sourceErrorf(ReasonAccessDenied, + "%s: the repository does not exist or is not accessible with the credential (HTTP %d)", + target, resp.StatusCode) + case resp.StatusCode != http.StatusOK: + return nil, fmt.Errorf("HTTP %d", resp.StatusCode) + } + if ct := resp.Header.Get("Content-Type"); !strings.HasPrefix(ct, "application/x-git-upload-pack-advertisement") { + return nil, fmt.Errorf("the answer is %q, not a git advertisement", ct) + } + + return parseGitAdvertisement(resp.Body) +} + +// parseGitAdvertisement reads a smart protocol advertisement: pkt-lines of ` `, the +// first carrying the server's capabilities after a NUL and an annotated tag's commit named by the +// peeled entry with a trailing `^{}`. A packet that does not parse is an error — a truncated +// answer must not shrink the cross-check. +func parseGitAdvertisement(r io.Reader) (map[string]string, error) { + buf, err := io.ReadAll(io.LimitReader(r, modelScopeMaxResponseBytes+1)) + if err != nil { + return nil, err + } + if len(buf) > modelScopeMaxResponseBytes { + return nil, fmt.Errorf("the advertisement exceeds %d bytes", modelScopeMaxResponseBytes) + } + + refs := make(map[string]string) + for i := 0; i < len(buf); { + if len(buf)-i < 4 { + return nil, fmt.Errorf("the advertisement ends inside a packet length") + } + n, err := strconv.ParseInt(string(buf[i:i+4]), 16, 32) + if err != nil { + return nil, fmt.Errorf("unparsable packet length %q", string(buf[i:min(i+4, len(buf))])) + } + i += 4 + if n == 0 { + continue // a flush packet + } + if n < 4 { + return nil, fmt.Errorf("unparsable packet length %q", string(buf[i-4:i])) + } + if i+int(n)-4 > len(buf) { + return nil, fmt.Errorf("the advertisement ends inside a packet") + } + line := strings.TrimSuffix(string(buf[i:i+int(n)-4]), "\n") + i += int(n) - 4 + if strings.HasPrefix(line, "# service=") { + continue + } + if z := strings.IndexByte(line, 0); z >= 0 { + line = line[:z] // the first ref carries the server's capabilities after a NUL + } + commit, ref, ok := strings.Cut(line, " ") + if !ok || !modelScopeCommitPattern.MatchString(commit) || ref == "" { + return nil, fmt.Errorf("unparsable advertisement line %q", line) + } + refs[ref] = commit + } + if len(refs) == 0 { + return nil, fmt.Errorf("the advertisement names no ref") + } + + return refs, nil +} + +// modelScopeMaxResponseBytes bounds one response body. A commits page of one entry is a few +// hundred bytes; a repo/files page of 3000 is a few hundred kilobytes. +const modelScopeMaxResponseBytes = 16 << 20 + +// modelScopeEnvelope is every answer's shape, errors included: the Code inside the body, not the +// HTTP status, is what distinguishes a missing revision from a forbidden one. +type modelScopeEnvelope struct { + Code int `json:"Code"` + Message string `json:"Message"` + Data json.RawMessage `json:"Data"` +} + +func (m *ModelScope) apiURL(segments ...string) string { + escaped := make([]string, 0, len(segments)+1) + escaped = append(escaped, strings.TrimSuffix(m.Endpoint, "/"), "api", "v1") + for _, s := range segments { + escaped = append(escaped, escapeRepository(s)) + } + + return strings.Join(escaped, "/") +} + +func (m *ModelScope) getJSON(ctx context.Context, target, token string, into any) error { + resp, err := m.do(ctx, http.MethodGet, target, token) + if err != nil { + return err + } + + return decodeModelScope(resp, into) +} + +// do sends one request. A transport failure is SourceUnavailable; the caller classifies the +// status code. +func (m *ModelScope) do(ctx context.Context, method, target, token string) (*http.Response, error) { + return m.doRequest(ctx, method, target, token, false) +} + +// doNoRedirect sends one request whose redirects stay unanswered: a redirect is an answer about +// access, not a page to follow. +func (m *ModelScope) doNoRedirect(ctx context.Context, method, target, token string) (*http.Response, error) { + return m.doRequest(ctx, method, target, token, true) +} + +func (m *ModelScope) doRequest(ctx context.Context, method, target, token string, noRedirect bool) (*http.Response, error) { + req, err := http.NewRequestWithContext(ctx, method, target, nil) + if err != nil { + return nil, err + } + req.Header.Set("User-Agent", "gpustack-operator") + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + + client := m.Client + if noRedirect { + c := *client + c.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } + client = &c + } + + resp, err := client.Do(req) + if err != nil { + // The URL is the only part of a transport error that names the request, and it carries no + // credential; the error is rebuilt so nothing else of the request can ride along. + return nil, sourceErrorf(ReasonSourceUnavailable, "%s %s: %v", method, target, unwrapURLError(err)) + } + + return resp, nil +} + +// decodeModelScope reads the envelope of one answer. A non-200 — or a 200 whose own Code is not +// 200 — is classified; only a clean answer's Data is decoded into. +func decodeModelScope(resp *http.Response, into any) error { + defer httpx.Close(resp) + body, err := io.ReadAll(io.LimitReader(resp.Body, modelScopeMaxResponseBytes+1)) + if err != nil { + return sourceErrorf(ReasonSourceUnavailable, "read %s: %v", resp.Request.URL.Path, err) + } + if len(body) > modelScopeMaxResponseBytes { + return sourceErrorf(ReasonSourceUnavailable, "%s answered more than %d bytes", resp.Request.URL.Path, + modelScopeMaxResponseBytes) + } + + var env modelScopeEnvelope + if err := json.Unmarshal(body, &env); err != nil { + if resp.StatusCode != http.StatusOK { + return classifyModelScope(resp.StatusCode, 0, resp.Request.URL.Path) + } + return sourceErrorf(ReasonSourceUnavailable, "%s answered an unreadable body: %v", resp.Request.URL.Path, err) + } + if resp.StatusCode != http.StatusOK || env.Code != http.StatusOK { + return classifyModelScope(resp.StatusCode, env.Code, resp.Request.URL.Path) + } + if into == nil || len(env.Data) == 0 { + return nil + } + if err := json.Unmarshal(env.Data, into); err != nil { + return sourceErrorf(ReasonSourceUnavailable, "%s answered an unreadable body: %v", resp.Request.URL.Path, err) + } + + return nil +} + +// classifyModelScope maps a non-success answer onto a reason, as measured against ModelScope +// (PoC-D): every access failure is a 404, so the envelope Code classifies, with the HTTP status +// as the fallback. +// +// - 10990101004 is a repository with no file tree at the revision: RevisionNotFound. +// - 10010205001 (not found) and 10010200001 (no access — private without the token, or gated +// without a grant) are AccessDenied, and so is every other 404, 401 and 403: the hub answers +// a repository that does not exist and one the caller cannot read alike, so the message does +// not guess which. +// - Anything else, 5xx and 429 among it, is SourceUnavailable. +func classifyModelScope(status, code int, what string) error { + switch { + case code == 10990101004: + return sourceErrorf(ReasonRevisionNotFound, "%s: the repository has no file tree at the revision", what) + case code == 10010205001, code == 10010200001: + return sourceErrorf(ReasonAccessDenied, + "%s: the repository does not exist or is not accessible with the credential (HTTP %d, code %d)", + what, status, code) + case status == http.StatusUnauthorized, status == http.StatusForbidden, status == http.StatusNotFound: + return sourceErrorf(ReasonAccessDenied, + "%s: the repository does not exist or is not accessible with the credential (HTTP %d, code %d)", + what, status, code) + default: + return sourceErrorf(ReasonSourceUnavailable, "%s: HTTP %d, code %d", what, status, code) + } +} diff --git a/pkg/modelartifact/modelscope_test.go b/pkg/modelartifact/modelscope_test.go new file mode 100644 index 000000000..7211c9ae0 --- /dev/null +++ b/pkg/modelartifact/modelscope_test.go @@ -0,0 +1,639 @@ +package modelartifact + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const modelScopeCommit = "186d8559ad54c32cf47dc3a8225f993742c507b8" + +// newFakeModelScope is a ModelScope client against a fake hub whose answers each case writes, +// with git replaced by refs the case supplies and an optional error standing in for its failure. +func newFakeModelScope(t *testing.T, routes map[string]http.HandlerFunc, refs map[string]string, gitErr error) *ModelScope { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + route, ok := routes[r.Method+" "+r.URL.EscapedPath()] + if !ok { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"Code":10010205001,"Message":"not found"}`)) + return + } + route(w, r) + })) + t.Cleanup(server.Close) + + m := &ModelScope{Endpoint: server.URL, Client: server.Client()} + m.lsRemote = func(_ context.Context, _, _ string) (map[string]string, error) { + if gitErr != nil { + return nil, gitErr + } + return refs, nil + } + + return m +} + +// modelScopeCommitsRoute answers the commits endpoint the way the hub does: an envelope whose +// Data.Commit lists the commits, empty for a ref the repository does not have. +func modelScopeCommitsRoute(ids ...string) http.HandlerFunc { + type commit struct { + Id string `json:"Id"` + } + commits := make([]commit, 0, len(ids)) + for _, id := range ids { + commits = append(commits, commit{Id: id}) + } + return jsonAnswer(map[string]any{ + "Code": 200, + "Message": "success", + "Data": map[string]any{"Commit": commits, "TotalCount": len(commits)}, + }) +} + +func modelScopeCommitsPath(repository string) string { + return "/api/v1/models/" + repository + "/commits" +} + +func TestModelScopeResolveRevision(t *testing.T) { + const branchCommit = "13448952f850140b43c5e14d0e19f7e7f8cb3c47" + + t.Run("a commit is taken as is, without asking the hub", func(t *testing.T) { + m := newFakeModelScope(t, nil, nil, nil) + got, err := m.resolveRevision(context.Background(), "qwen/Qwen2.5-0.5B-Instruct", modelScopeCommit, "") + require.NoError(t, err) + assert.Equal(t, modelScopeCommit, got) + }) + + t.Run("a branch is resolved and cross-checked", func(t *testing.T) { + m := newFakeModelScope(t, + map[string]http.HandlerFunc{ + "GET " + modelScopeCommitsPath("qwen/Qwen2.5-0.5B-Instruct"): modelScopeCommitsRoute(branchCommit), + }, + map[string]string{ + "HEAD": branchCommit, + "refs/heads/master": branchCommit, + }, nil) + got, err := m.resolveRevision(context.Background(), "qwen/Qwen2.5-0.5B-Instruct", "master", "") + require.NoError(t, err) + assert.Equal(t, branchCommit, got) + }) + + t.Run("a disagreement between the index and git refuses the resolution", func(t *testing.T) { + other := strings.Repeat("f", 40) + m := newFakeModelScope(t, + map[string]http.HandlerFunc{ + "GET " + modelScopeCommitsPath("qwen/Qwen2.5-0.5B-Instruct"): modelScopeCommitsRoute(branchCommit), + }, + map[string]string{"refs/heads/master": other}, nil) + _, err := m.resolveRevision(context.Background(), "qwen/Qwen2.5-0.5B-Instruct", "master", "") + require.Error(t, err) + se := &SourceError{} + require.True(t, errors.As(err, &se)) + assert.Equal(t, ReasonSourceUnavailable, se.Reason) + assert.Contains(t, se.Message, "disagree") + assert.Contains(t, se.Message, branchCommit) + assert.Contains(t, se.Message, other) + }) + + t.Run("an annotated tag is cross-checked against the peeled entry", func(t *testing.T) { + tagObject := strings.Repeat("b", 40) + m := newFakeModelScope(t, + map[string]http.HandlerFunc{ + "GET " + modelScopeCommitsPath("qwen/Qwen2.5-0.5B-Instruct"): modelScopeCommitsRoute(modelScopeCommit), + }, + map[string]string{ + "refs/tags/v1.0": tagObject, + "refs/tags/v1.0^{}": modelScopeCommit, + }, nil) + got, err := m.resolveRevision(context.Background(), "qwen/Qwen2.5-0.5B-Instruct", "v1.0", "") + require.NoError(t, err) + assert.Equal(t, modelScopeCommit, got) + }) + + t.Run("a tag whose index names the tag object rather than the commit disagrees", func(t *testing.T) { + tagObject := strings.Repeat("b", 40) + m := newFakeModelScope(t, + map[string]http.HandlerFunc{ + "GET " + modelScopeCommitsPath("qwen/Qwen2.5-0.5B-Instruct"): modelScopeCommitsRoute(tagObject), + }, + map[string]string{ + "refs/tags/v1.0": tagObject, + "refs/tags/v1.0^{}": modelScopeCommit, + }, nil) + _, err := m.resolveRevision(context.Background(), "qwen/Qwen2.5-0.5B-Instruct", "v1.0", "") + require.Error(t, err) + assert.Equal(t, ReasonSourceUnavailable, ReasonOf(err)) + }) + + t.Run("a branch git does not name refuses", func(t *testing.T) { + m := newFakeModelScope(t, + map[string]http.HandlerFunc{ + "GET " + modelScopeCommitsPath("qwen/Qwen2.5-0.5B-Instruct"): modelScopeCommitsRoute(branchCommit), + }, + map[string]string{"refs/heads/other": branchCommit}, nil) + _, err := m.resolveRevision(context.Background(), "qwen/Qwen2.5-0.5B-Instruct", "master", "") + require.Error(t, err) + assert.Equal(t, ReasonSourceUnavailable, ReasonOf(err)) + assert.Contains(t, ReasonOf(err), ReasonSourceUnavailable) + }) + + t.Run("git failing is SourceUnavailable, never a pass-through", func(t *testing.T) { + m := newFakeModelScope(t, + map[string]http.HandlerFunc{ + "GET " + modelScopeCommitsPath("qwen/Qwen2.5-0.5B-Instruct"): modelScopeCommitsRoute(branchCommit), + }, + nil, errors.New("git: exit status 128")) + _, err := m.resolveRevision(context.Background(), "qwen/Qwen2.5-0.5B-Instruct", "master", "") + require.Error(t, err) + assert.Equal(t, ReasonSourceUnavailable, ReasonOf(err)) + assert.Contains(t, ReasonOf(err), ReasonSourceUnavailable) + }) + + t.Run("a misspelled branch whose index silently answers master is caught", func(t *testing.T) { + // The commits endpoint answers master for a misspelled Ref (measured); git names no + // refs/heads/masterr, and HEAD is deliberately not a fallback that would confirm the + // rewrite. + m := newFakeModelScope(t, + map[string]http.HandlerFunc{ + "GET " + modelScopeCommitsPath("qwen/Qwen2.5-0.5B-Instruct"): modelScopeCommitsRoute("13448952f850140b43c5e14d0e19f7e7f8cb3c47"), + }, + map[string]string{"HEAD": "13448952f850140b43c5e14d0e19f7e7f8cb3c47", "refs/heads/master": "13448952f850140b43c5e14d0e19f7e7f8cb3c47"}, nil) + _, err := m.resolveRevision(context.Background(), "qwen/Qwen2.5-0.5B-Instruct", "masterr", "") + require.Error(t, err) + assert.Equal(t, ReasonSourceUnavailable, ReasonOf(err)) + assert.Contains(t, ReasonOf(err), ReasonSourceUnavailable) + }) + + t.Run("a ref the index does not know is RevisionNotFound", func(t *testing.T) { + m := newFakeModelScope(t, + map[string]http.HandlerFunc{ + "GET " + modelScopeCommitsPath("qwen/Qwen2.5-0.5B-Instruct"): modelScopeCommitsRoute(), + }, + nil, nil) + _, err := m.resolveRevision(context.Background(), "qwen/Qwen2.5-0.5B-Instruct", "no-such", "") + require.Error(t, err) + assert.Equal(t, ReasonRevisionNotFound, ReasonOf(err)) + }) + + t.Run("an index answer that is not a commit is SourceUnavailable", func(t *testing.T) { + m := newFakeModelScope(t, + map[string]http.HandlerFunc{ + "GET " + modelScopeCommitsPath("qwen/Qwen2.5-0.5B-Instruct"): modelScopeCommitsRoute("not-a-commit"), + }, + map[string]string{"refs/heads/master": modelScopeCommit}, nil) + _, err := m.resolveRevision(context.Background(), "qwen/Qwen2.5-0.5B-Instruct", "master", "") + require.Error(t, err) + assert.Equal(t, ReasonSourceUnavailable, ReasonOf(err)) + }) +} + +func TestModelScopeClassify(t *testing.T) { + cases := []struct { + name string + status int + code int + want string + }{ + {name: "a revision the listing cannot serve", status: 404, code: 10990101004, want: ReasonRevisionNotFound}, + {name: "a repository that does not exist", status: 404, code: 10010205001, want: ReasonAccessDenied}, + {name: "a repository without access", status: 404, code: 10010200001, want: ReasonAccessDenied}, + {name: "a 404 with an unknown code", status: 404, code: 99999999, want: ReasonAccessDenied}, + {name: "a 404 with no code", status: 404, code: 0, want: ReasonAccessDenied}, + {name: "an unauthorized", status: 401, want: ReasonAccessDenied}, + {name: "a forbidden", status: 403, want: ReasonAccessDenied}, + {name: "a server error", status: 500, want: ReasonSourceUnavailable}, + {name: "rate limited", status: 429, want: ReasonSourceUnavailable}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + err := classifyModelScope(c.status, c.code, "/api/v1/models/o/r/repo") + require.Error(t, err) + assert.Equal(t, c.want, ReasonOf(err)) + }) + } +} + +func TestParseGitAdvertisement(t *testing.T) { + pkt := func(payload string) string { + return fmt.Sprintf("%04x%s", len(payload)+4, payload) + } + head := pkt("# service=git-upload-pack\n") + "0000" + first := pkt("13448952f850140b43c5e14d0e19f7e7f8cb3c47 refs/heads/master\x00multi_ack thin-pack side-band") + tag := pkt(strings.Repeat("b", 40) + " refs/tags/v1.0") + peeled := pkt(modelScopeCommit + " refs/tags/v1.0^{}") + advertisement := head + first + tag + peeled + "0000" + + refs, err := parseGitAdvertisement(strings.NewReader(advertisement)) + require.NoError(t, err) + assert.Equal(t, map[string]string{ + "refs/heads/master": "13448952f850140b43c5e14d0e19f7e7f8cb3c47", + "refs/tags/v1.0": strings.Repeat("b", 40), + "refs/tags/v1.0^{}": modelScopeCommit, + }, refs) + + _, err = parseGitAdvertisement(strings.NewReader(head + pkt("short line") + "0000")) + require.Error(t, err) + + _, err = parseGitAdvertisement(strings.NewReader("00")) + require.Error(t, err) +} + +// TestLsRemoteOverHTTP pins the request and its credential: the smart protocol answered by the +// operator itself, a private repository authenticating as oauth2 with an HTTP header. +func TestLsRemoteOverHTTP(t *testing.T) { + pkt := func(payload string) string { + return fmt.Sprintf("%04x%s", len(payload)+4, payload) + } + advertisement := pkt("# service=git-upload-pack\n") + "0000" + + pkt(modelScopeCommit+" refs/heads/master\x00caps") + "0000" + + setup := func(t *testing.T, status int, body, contentType string) (*string, *string, *ModelScope) { + gotAuth := "" + gotUA := "" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotAuth = r.Header.Get("Authorization") + gotUA = r.Header.Get("User-Agent") + w.Header().Set("Content-Type", contentType) + w.WriteHeader(status) + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(server.Close) + m := &ModelScope{Endpoint: server.URL, Client: server.Client()} + return &gotAuth, &gotUA, m + } + + t.Run("a public repository reads the refs", func(t *testing.T) { + auth, ua, m := setup(t, http.StatusOK, advertisement, "application/x-git-upload-pack-advertisement") + refs, err := m.lsRemoteOverHTTP(context.Background(), m.Endpoint+"/qwen/repo.git", "") + require.NoError(t, err) + assert.Equal(t, modelScopeCommit, refs["refs/heads/master"]) + assert.Empty(t, *auth, "no credential, no header") + assert.Equal(t, "git/2.39", *ua, "the endpoint answers a git client and refuses others") + }) + + t.Run("a private repository authenticates as oauth2", func(t *testing.T) { + auth, _, m := setup(t, http.StatusOK, advertisement, "application/x-git-upload-pack-advertisement") + _, err := m.lsRemoteOverHTTP(context.Background(), m.Endpoint+"/qwen/repo.git", testToken) + require.NoError(t, err) + assert.True(t, strings.HasPrefix(*auth, "Basic "), *auth) + assert.NotContains(t, *auth, testToken, "the header carries the base64 pair, not the bare token") + }) + + t.Run("a refusal is AccessDenied", func(t *testing.T) { + _, _, m := setup(t, http.StatusUnauthorized, "denied", "text/plain") + _, err := m.lsRemoteOverHTTP(context.Background(), m.Endpoint+"/qwen/repo.git", "") + require.Error(t, err) + assert.Equal(t, ReasonAccessDenied, ReasonOf(err)) + }) + + t.Run("a non-advertisement answer is refused", func(t *testing.T) { + _, _, m := setup(t, http.StatusOK, "", "text/html") + _, err := m.lsRemoteOverHTTP(context.Background(), m.Endpoint+"/qwen/repo.git", "") + require.Error(t, err) + assert.Equal(t, ReasonSourceUnavailable, ReasonOf(err)) + }) +} + +// modelScopeFilesRoute answers the repo/files endpoint from a map of Root to entries, recording +// every query it served so a case can assert the descent. +func modelScopeFilesRoute(repository string, pages map[string][]map[string]any, served *[]url.Values) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + if served != nil { + *served = append(*served, r.URL.Query()) + } + root := r.URL.Query().Get("Root") + page, ok := pages[root] + if !ok { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"Code":10990101004,"Message":"no file tree"}`)) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{ + "Code": 200, + "Message": "success", + "Data": map[string]any{"Files": page, "TotalCount": len(page)}, + }) + } +} + +func modelScopeFilesPath(repository string) string { + return "/api/v1/models/" + repository + "/repo/files" +} + +func msFile(path string, size int64, sha string) map[string]any { + return map[string]any{"Type": "blob", "Path": path, "Size": size, "Sha256": sha} +} + +func msDir(path string) map[string]any { + return map[string]any{"Type": "tree", "Path": path, "Size": 0} +} + +func shaOf(s string) string { + return strings.Repeat(s, 64) +} + +func TestModelScopeListManifest(t *testing.T) { + t.Run("a small tree lists whole, files only, and hashes as an independent v1 manifest", func(t *testing.T) { + route := modelScopeFilesRoute("qwen/repo", map[string][]map[string]any{ + "": { + msFile("README.md", 3, shaOf("a")), + msDir("sub"), + msFile("sub/tokenizer.json", 7, shaOf("b")), + }, + }, nil) + m := newFakeModelScope(t, map[string]http.HandlerFunc{"GET " + modelScopeFilesPath("qwen/repo"): route}, nil, nil) + + manifest, err := m.ListManifest(context.Background(), "qwen/repo", modelScopeCommit, "", Filter{}) + require.NoError(t, err) + want, err := NewManifest([]ManifestEntry{ + {Path: "README.md", Size: 3, Digest: DigestSHA256 + ":" + shaOf("a")}, + {Path: "sub/tokenizer.json", Size: 7, Digest: DigestSHA256 + ":" + shaOf("b")}, + }) + require.NoError(t, err) + assert.Equal(t, want, manifest) + }) + + t.Run("a truncated root listing is re-listed per directory", func(t *testing.T) { + // The root's recursive page holds exactly 3000 entries, the API's silent truncation point. + // Its content is dropped — a truncated page cannot be trusted — and the walk recovers the + // tree from the direct children: two directories whose own pages are short, and one loose + // file that is its own entry. + root := make([]map[string]any, 0, modelScopePageBound) + for i := 0; i < modelScopePageBound; i++ { + root = append(root, msFile(fmt.Sprintf("gone/file-%04d", i), 1, shaOf("a"))) + } + direct := []map[string]any{msDir("docs"), msDir("data"), msFile("loose.txt", 1, shaOf("e"))} + docs := []map[string]any{msFile("docs/readme.md", 5, shaOf("b"))} + for i := 0; i < 1498; i++ { + docs = append(docs, msFile(fmt.Sprintf("docs/f-%04d", i), 1, shaOf("a"))) + } + data := []map[string]any{msFile("data/set.bin", 9, shaOf("c"))} + for i := 0; i < 1498; i++ { + data = append(data, msFile(fmt.Sprintf("data/f-%04d", i), 1, shaOf("a"))) + } + var served []url.Values + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + served = append(served, r.URL.Query()) + page := root + switch { + case r.URL.Query().Get("Recursive") != "true": + page = direct + case r.URL.Query().Get("Root") == "docs": + page = docs + case r.URL.Query().Get("Root") == "data": + page = data + } + _ = json.NewEncoder(w).Encode(map[string]any{ + "Code": 200, "Message": "success", + "Data": map[string]any{"Files": page, "TotalCount": len(page)}, + }) + })) + t.Cleanup(server.Close) + m := &ModelScope{Endpoint: server.URL, Client: server.Client()} + + manifest, err := m.ListManifest(context.Background(), "qwen/repo", modelScopeCommit, "", Filter{}) + require.NoError(t, err) + assert.Equal(t, int64(len(docs)+len(data)+1), manifest.FileCount) + // Only a subtree that truncates is broken up further, so the walk touches exactly the + // root and the two directories: the root once whole (recursive) and once for its direct + // children, each directory whole. + rootWhole, rootDirect := 0, 0 + children := make([]string, 0, 2) + for _, q := range served { + switch root := q.Get("Root"); root { + case "": + if q.Get("Recursive") == "true" { + rootWhole++ + } else { + rootDirect++ + } + default: + children = append(children, root) + assert.Equal(t, "true", q.Get("Recursive")) + } + } + assert.Equal(t, 1, rootWhole) + assert.Equal(t, 1, rootDirect) + assert.ElementsMatch(t, []string{"docs", "data"}, children) + }) + + t.Run("a directory with 3000 direct children refuses", func(t *testing.T) { + root := make([]map[string]any, 0, modelScopePageBound) + for i := 0; i < modelScopePageBound; i++ { + root = append(root, msDir(fmt.Sprintf("d-%04d", i))) + } + route := modelScopeFilesRoute("qwen/repo", map[string][]map[string]any{"": root}, nil) + m := newFakeModelScope(t, map[string]http.HandlerFunc{"GET " + modelScopeFilesPath("qwen/repo"): route}, nil, nil) + + _, err := m.ListManifest(context.Background(), "qwen/repo", modelScopeCommit, "", Filter{}) + require.Error(t, err) + assert.Equal(t, ReasonSourceUnavailable, ReasonOf(err)) + se := &SourceError{} + require.True(t, errors.As(err, &se)) + assert.Contains(t, se.Message, "3000") + }) + + t.Run("a tree past MaxEntries refuses without walking the rest", func(t *testing.T) { + files := make([]map[string]any, 0, 8) + for i := 0; i < 8; i++ { + files = append(files, msFile(fmt.Sprintf("f-%02d", i), 1, shaOf("a"))) + } + route := modelScopeFilesRoute("qwen/repo", map[string][]map[string]any{"": files}, nil) + m := newFakeModelScope(t, map[string]http.HandlerFunc{"GET " + modelScopeFilesPath("qwen/repo"): route}, nil, nil) + m.MaxEntries = 5 + + _, err := m.ListManifest(context.Background(), "qwen/repo", modelScopeCommit, "", Filter{}) + require.Error(t, err) + assert.Equal(t, ReasonManifestTooLarge, ReasonOf(err)) + }) + + t.Run("an entry without a usable Sha256 is an invalid manifest", func(t *testing.T) { + route := modelScopeFilesRoute("qwen/repo", map[string][]map[string]any{ + "": {{"Type": "blob", "Path": "README.md", "Size": 3}}, + }, nil) + m := newFakeModelScope(t, map[string]http.HandlerFunc{"GET " + modelScopeFilesPath("qwen/repo"): route}, nil, nil) + + _, err := m.ListManifest(context.Background(), "qwen/repo", modelScopeCommit, "", Filter{}) + require.Error(t, err) + assert.Equal(t, ReasonInvalidManifest, ReasonOf(err)) + }) + + t.Run("an unexpected entry type is an invalid manifest", func(t *testing.T) { + route := modelScopeFilesRoute("qwen/repo", map[string][]map[string]any{ + "": {{"Type": "symlink", "Path": "link", "Size": 1, "Sha256": shaOf("a")}}, + }, nil) + m := newFakeModelScope(t, map[string]http.HandlerFunc{"GET " + modelScopeFilesPath("qwen/repo"): route}, nil, nil) + + _, err := m.ListManifest(context.Background(), "qwen/repo", modelScopeCommit, "", Filter{}) + require.Error(t, err) + assert.Equal(t, ReasonInvalidManifest, ReasonOf(err)) + }) + + t.Run("an empty commit and a fully filtered one are EmptyManifest", func(t *testing.T) { + route := modelScopeFilesRoute("qwen/repo", map[string][]map[string]any{ + "": {msDir("docs")}, + }, nil) + m := newFakeModelScope(t, map[string]http.HandlerFunc{"GET " + modelScopeFilesPath("qwen/repo"): route}, nil, nil) + + _, err := m.ListManifest(context.Background(), "qwen/repo", modelScopeCommit, "", Filter{}) + require.Error(t, err) + assert.Equal(t, ReasonEmptyManifest, ReasonOf(err)) + + _, err = m.ListManifest(context.Background(), "qwen/repo", modelScopeCommit, "", + Filter{Allow: []string{"*.safetensors"}}) + require.Error(t, err) + assert.Equal(t, ReasonEmptyManifest, ReasonOf(err)) + }) + + t.Run("the resolve path pins the commit the revision resolved to", func(t *testing.T) { + const branchCommit = "13448952f850140b43c5e14d0e19f7e7f8cb3c47" + commits := modelScopeCommitsRoute(branchCommit) + var served []url.Values + files := modelScopeFilesRoute("qwen/repo", map[string][]map[string]any{ + "": {msFile("README.md", 3, shaOf("a"))}, + }, &served) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.EscapedPath() == modelScopeCommitsPath("qwen/repo") { + commits(w, r) + return + } + files(w, r) + })) + t.Cleanup(server.Close) + m := &ModelScope{Endpoint: server.URL, Client: server.Client()} + m.lsRemote = func(_ context.Context, _, _ string) (map[string]string, error) { + return map[string]string{"refs/heads/master": branchCommit}, nil + } + + got, err := m.Resolve(context.Background(), "qwen/repo", "master", "", Filter{}) + require.NoError(t, err) + assert.Equal(t, branchCommit, got.Commit) + require.Len(t, served, 1) + assert.Equal(t, branchCommit, served[0].Get("Revision")) + }) +} + +func TestModelScopeRevalidate(t *testing.T) { + const first = "README.md" + + setup := func(t *testing.T, root []map[string]any, status int) *ModelScope { + route := modelScopeFilesRoute("qwen/repo", map[string][]map[string]any{"": root}, nil) + headHit := false + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodHead { + headHit = true + w.WriteHeader(status) + return + } + route(w, r) + })) + t.Cleanup(server.Close) + m := &ModelScope{Endpoint: server.URL, Client: server.Client()} + t.Cleanup(func() { assert.True(t, headHit, "the revalidation must be one HEAD") }) + return m + } + + t.Run("an accessible file confirms", func(t *testing.T) { + m := setup(t, []map[string]any{msFile(first, 3, shaOf("a")), msDir("docs")}, http.StatusOK) + require.NoError(t, m.Revalidate(context.Background(), "qwen/repo", modelScopeCommit, "")) + }) + + t.Run("an LFS file's redirect confirms too", func(t *testing.T) { + m := setup(t, []map[string]any{msFile(first, 3, shaOf("a"))}, http.StatusFound) + require.NoError(t, m.Revalidate(context.Background(), "qwen/repo", modelScopeCommit, "")) + }) + + t.Run("a 404 is an access refusal, the staircase's input", func(t *testing.T) { + m := setup(t, []map[string]any{msFile(first, 3, shaOf("a"))}, http.StatusNotFound) + err := m.Revalidate(context.Background(), "qwen/repo", modelScopeCommit, "") + require.Error(t, err) + assert.Equal(t, ReasonAccessDenied, ReasonOf(err)) + }) + + t.Run("a server error is SourceUnavailable", func(t *testing.T) { + m := setup(t, []map[string]any{msFile(first, 3, shaOf("a"))}, http.StatusInternalServerError) + err := m.Revalidate(context.Background(), "qwen/repo", modelScopeCommit, "") + require.Error(t, err) + assert.Equal(t, ReasonSourceUnavailable, ReasonOf(err)) + }) + + t.Run("a root with no file falls back to the recursive listing", func(t *testing.T) { + // The root's direct children hold one directory; the recursive root page holds the file + // inside it, so the two Root="" answers differ by the Recursive parameter. + direct := []map[string]any{msDir("docs")} + whole := []map[string]any{msDir("docs"), msFile("docs/readme.md", 5, shaOf("b"))} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodHead { + w.WriteHeader(http.StatusOK) + return + } + page := direct + if r.URL.Query().Get("Recursive") == "true" { + page = whole + } + _ = json.NewEncoder(w).Encode(map[string]any{ + "Code": 200, "Message": "success", + "Data": map[string]any{"Files": page, "TotalCount": len(page)}, + }) + })) + t.Cleanup(server.Close) + m := &ModelScope{Endpoint: server.URL, Client: server.Client()} + + require.NoError(t, m.Revalidate(context.Background(), "qwen/repo", modelScopeCommit, "")) + }) +} + +func TestModelScopeGitURLKeepsTheEndpointPath(t *testing.T) { + m := &ModelScope{Endpoint: "https://git.example.com/ms"} + + got, err := m.gitURL("qwen/repo") + require.NoError(t, err) + assert.Equal(t, "https://git.example.com/ms/qwen/repo.git", got) +} + +func TestModelScopeFileURL(t *testing.T) { + m := &ModelScope{Endpoint: "https://www.modelscope.cn"} + + got := m.FileURL("qwen/Qwen2.5-0.5B-Instruct", modelScopeCommit, "sub/tokenizer.json") + assert.Equal(t, + "https://www.modelscope.cn/api/v1/models/qwen/Qwen2.5-0.5B-Instruct/repo"+ + "?Revision="+modelScopeCommit+"&FilePath=sub%2Ftokenizer.json", got) +} + +func TestModelScopeValidToken(t *testing.T) { + setup := func(t *testing.T, status int) *ModelScope { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/openapi/v1/users/me", r.URL.EscapedPath()) + assert.Equal(t, "Bearer "+testToken, r.Header.Get("Authorization")) + w.WriteHeader(status) + })) + t.Cleanup(server.Close) + return &ModelScope{Endpoint: server.URL, Client: server.Client()} + } + + t.Run("an accepted token", func(t *testing.T) { + ok, err := setup(t, http.StatusOK).ValidToken(context.Background(), testToken) + require.NoError(t, err) + assert.True(t, ok) + }) + t.Run("a rejected token", func(t *testing.T) { + ok, err := setup(t, http.StatusUnauthorized).ValidToken(context.Background(), testToken) + require.NoError(t, err) + assert.False(t, ok) + }) + t.Run("an unreachable hub is an error, not a verdict", func(t *testing.T) { + ok, err := setup(t, http.StatusInternalServerError).ValidToken(context.Background(), testToken) + require.Error(t, err) + assert.False(t, ok) + }) +} diff --git a/pkg/modelmanager/driver/authorize.go b/pkg/modelmanager/driver/authorize.go index 5941e8fab..49b825be6 100644 --- a/pkg/modelmanager/driver/authorize.go +++ b/pkg/modelmanager/driver/authorize.go @@ -63,8 +63,8 @@ func authorizeMount(podNamespace string, attrs mountAttributes, ma *workercore.M return deny(ruleArtifactMissing, "does not exist") case string(ma.UID) != attrs.ArtifactUID: return deny(ruleUIDMismatch, fmt.Sprintf("has UID %q, the volume names %q", ma.UID, attrs.ArtifactUID)) - case ma.Spec.Source.HuggingFace == nil: - return deny(ruleNotHub, "only a Hugging Face artifact is delivered by the node") + case ma.Spec.Source.HuggingFace == nil && ma.Spec.Source.ModelScope == nil: + return deny(ruleNotHub, "only a hub artifact is delivered by the node") case !conditionTrue(ma.Status.Conditions, resolvedCondition) || ma.Status.Resolved == nil: return deny(ruleNotResolved, "is not Resolved; new mounts wait until it is") case ma.Status.Resolved.ManifestDigest != attrs.Digest: diff --git a/pkg/modelmanager/driver/driver_test.go b/pkg/modelmanager/driver/driver_test.go index 09cab134d..158ef2c74 100644 --- a/pkg/modelmanager/driver/driver_test.go +++ b/pkg/modelmanager/driver/driver_test.go @@ -263,6 +263,18 @@ func TestNodePublishAuthorization(t *testing.T) { attrs: hints("qwen", "uid-qwen", testDigest), namespace: "team-a", wantRule: ruleNotHub, }, + { + name: "the Pod's own resolved ModelScope artifact", + objs: []ctrlcli.Object{func() *workercore.ModelArtifact { + ma := testArtifact("team-a", "qwen", "uid-qwen", testDigest, true) + ma.Spec.Source = workercore.ModelArtifactSource{ + ModelScope: &workercore.ModelArtifactHubSource{Repository: "qwen/repo"}, + } + return ma + }()}, + attrs: hints("qwen", "uid-qwen", testDigest), + namespace: "team-a", + }, { name: "no artifact named at all", attrs: hints("", "", testDigest), diff --git a/pkg/modelmanager/manager_test.go b/pkg/modelmanager/manager_test.go index 1ec1d455d..92f28d9ca 100644 --- a/pkg/modelmanager/manager_test.go +++ b/pkg/modelmanager/manager_test.go @@ -101,7 +101,7 @@ func TestReporterReadsPastAStalledInformer(t *testing.T) { collector.Pinned = reporter.Pinned require.NoError(t, reporter.Report(ctx)) - hub, _, err := reporter.Environment(ctx) + hub, _, err := reporter.Environment(ctx, materialize.HubHuggingFace) require.NoError(t, err) require.IsType(t, new(modelartifact.HuggingFace), hub) assert.Equal(t, "https://new.hf.example.com", hub.(*modelartifact.HuggingFace).Endpoint, diff --git a/pkg/modelmanager/materialize/materialize.go b/pkg/modelmanager/materialize/materialize.go index b86434d75..56228102b 100644 --- a/pkg/modelmanager/materialize/materialize.go +++ b/pkg/modelmanager/materialize/materialize.go @@ -53,9 +53,40 @@ type Hub interface { FileURL(repository, commit, path string) string } -// Environment returns the hub and the downloader built from the node's current effective -// configuration, or an InvalidRequest error while that configuration is invalid. -type Environment func(ctx context.Context) (Hub, *download.Downloader, error) +// The hub kinds a source may name, which the environment builds a hub for. +const ( + HubHuggingFace = "huggingFace" + HubModelScope = "modelscope" +) + +// HubKindOf names the hub an artifact's source resolves against, or "" when it names none. +func HubKindOf(ma *workercore.ModelArtifact) string { + switch { + case ma.Spec.Source.HuggingFace != nil: + return HubHuggingFace + case ma.Spec.Source.ModelScope != nil: + return HubModelScope + default: + return "" + } +} + +// HubRepository is the repository id an artifact's source names, or "". +func HubRepository(ma *workercore.ModelArtifact) string { + switch { + case ma.Spec.Source.HuggingFace != nil: + return ma.Spec.Source.HuggingFace.Repository + case ma.Spec.Source.ModelScope != nil: + return ma.Spec.Source.ModelScope.Repository + default: + return "" + } +} + +// Environment returns the hub of the named kind and the downloader built from the node's current +// effective configuration, or an InvalidRequest error while that configuration is invalid or does +// not reach the hub the source names. +type Environment func(ctx context.Context, kind string) (Hub, *download.Downloader, error) // Materializer implements driver.Materializer. type Materializer struct { @@ -102,9 +133,11 @@ type job struct { order []string } -// source is one artifact's way to the content: its repository at its commit, and the credential its -// namespace's mount handed over. A credential is only ever sent for its own repository. +// source is one artifact's way to the content: its hub's kind, its repository at its commit, and +// the credential its namespace's mount handed over. A credential is only ever sent for its own +// repository. type source struct { + kind string repository string commit string filter modelartifact.Filter @@ -222,7 +255,8 @@ func (j *job) join(req driver.Request, now time.Time) { s, ok := j.sources[uid] if !ok { s = &source{ - repository: ma.Spec.Source.HuggingFace.Repository, + kind: HubKindOf(ma), + repository: HubRepository(ma), commit: ma.Status.Resolved.Revision, filter: modelartifact.Filter{Allow: ma.Spec.AllowPatterns, Ignore: ma.Spec.IgnorePatterns}, } @@ -320,10 +354,6 @@ func (m *Materializer) watchWaiters(ctx context.Context, j *job) func() { } func (m *Materializer) attempt(ctx context.Context, j *job) error { - hub, dl, err := m.Environment(ctx) - if err != nil { - return err - } a, err := m.Store.NewAttempt(j.hex) if err != nil { return err @@ -331,6 +361,13 @@ func (m *Materializer) attempt(ctx context.Context, j *job) error { var lastErr error for _, src := range j.snapshotSources() { + hub, dl, err := m.Environment(ctx, src.kind) + if err != nil { + // One kind's configuration failure says nothing about another kind's: try the + // remaining sources before reporting. + lastErr = err + continue + } lastErr = m.fromSource(ctx, j, a, hub, dl, src) // Only a refusal of this source's credential is worth another source: the content is the // same wherever it is authorized, and any other failure would repeat. diff --git a/pkg/modelmanager/materialize/materialize_test.go b/pkg/modelmanager/materialize/materialize_test.go index 8c8a38911..079c1bac8 100644 --- a/pkg/modelmanager/materialize/materialize_test.go +++ b/pkg/modelmanager/materialize/materialize_test.go @@ -173,7 +173,7 @@ func newTestEnv(t *testing.T, files map[string]map[string][]byte) *testEnv { hub: hub, clock: clock, store: st, m: &Materializer{ Store: st, - Environment: func(context.Context) (Hub, *download.Downloader, error) { return hub, dl, nil }, + Environment: func(context.Context, string) (Hub, *download.Downloader, error) { return hub, dl, nil }, Now: clock.Now, CheckInterval: 10 * time.Millisecond, }, @@ -240,6 +240,107 @@ func TestEnsureMaterializesAndPublishes(t *testing.T) { } } +// TestEnsureTriesTheOtherKindWhenOneEnvironmentFails: one kind's configuration failure says +// nothing about another kind's, so the remaining source is still materialized. +func TestEnsureTriesTheOtherKindWhenOneEnvironmentFails(t *testing.T) { + hf := newTestHub(t, map[string]map[string][]byte{"owner/repo": {"hf.json": []byte(`{}`)}}) + ms := newTestHub(t, map[string]map[string][]byte{"qwen/repo": {"ms.json": []byte(`{}`)}}) + root := t.TempDir() + t.Cleanup(func() { + _ = filepath.WalkDir(root, func(p string, e os.DirEntry, err error) error { + if err == nil && e.IsDir() { + _ = os.Chmod(p, 0o755) + } + return err + }) + }) + st, err := store.Open(root) + require.NoError(t, err) + clock := &testClock{now: time.Date(2026, 9, 25, 6, 0, 0, 0, time.UTC)} + dl := download.New(ms.server.Client(), 8, 0) + dl.RetryDelay = time.Millisecond + m := &Materializer{ + Store: st, + Environment: func(_ context.Context, kind string) (Hub, *download.Downloader, error) { + if kind == HubHuggingFace { + return nil, nil, &download.Error{Reason: download.ReasonInvalidRequest, Message: "broken"} + } + return ms, dl, nil + }, + Now: clock.Now, + CheckInterval: 10 * time.Millisecond, + } + + digest := ms.manifest("qwen/repo").Digest + ma := &workercore.ModelArtifact{ + ObjectMeta: meta.ObjectMeta{UID: "uid-ms"}, + Spec: workercore.ModelArtifactSpec{Source: workercore.ModelArtifactSource{ + ModelScope: &workercore.ModelArtifactHubSource{Repository: "qwen/repo"}, + }}, + Status: workercore.ModelArtifactStatus{Resolved: &workercore.ModelArtifactResolved{ + Revision: testCommit, ManifestDigest: digest, + }}, + } + req := driver.Request{Hex: store.HexOf(digest), Artifact: ma, Token: "ms-token"} + + m.Ensure(context.Background(), req) + require.Eventually(t, func() bool { return !m.Downloading()[req.Hex] }, 10*time.Second, 5*time.Millisecond) + + require.True(t, st.IsPublished(req.Hex), "the MS source materializes past the broken HF kind") + assert.Empty(t, hf.recorded(), "the Hugging Face hub is never asked") +} + +// TestEnsureMaterializesAModelScopeSource materializes through the hub the source names: the +// environment is asked for the ModelScope hub, and the Hugging Face one is never touched. +func TestEnsureMaterializesAModelScopeSource(t *testing.T) { + hf := newTestHub(t, map[string]map[string][]byte{"owner/repo": {"hf.json": []byte(`{}`)}}) + ms := newTestHub(t, map[string]map[string][]byte{"qwen/repo": {"ms.json": []byte(`{}`)}}) + root := t.TempDir() + t.Cleanup(func() { + _ = filepath.WalkDir(root, func(p string, e os.DirEntry, err error) error { + if err == nil && e.IsDir() { + _ = os.Chmod(p, 0o755) + } + return err + }) + }) + st, err := store.Open(root) + require.NoError(t, err) + clock := &testClock{now: time.Date(2026, 9, 25, 6, 0, 0, 0, time.UTC)} + dl := download.New(ms.server.Client(), 8, 0) + dl.RetryDelay = time.Millisecond + m := &Materializer{ + Store: st, + Environment: func(_ context.Context, kind string) (Hub, *download.Downloader, error) { + if kind == HubModelScope { + return ms, dl, nil + } + return hf, dl, nil + }, + Now: clock.Now, + CheckInterval: 10 * time.Millisecond, + } + + digest := ms.manifest("qwen/repo").Digest + ma := &workercore.ModelArtifact{ + ObjectMeta: meta.ObjectMeta{UID: "uid-ms"}, + Spec: workercore.ModelArtifactSpec{Source: workercore.ModelArtifactSource{ + ModelScope: &workercore.ModelArtifactHubSource{Repository: "qwen/repo"}, + }}, + Status: workercore.ModelArtifactStatus{Resolved: &workercore.ModelArtifactResolved{ + Revision: testCommit, ManifestDigest: digest, + }}, + } + req := driver.Request{Hex: store.HexOf(digest), Artifact: ma, Token: "ms-token"} + + m.Ensure(context.Background(), req) + require.Eventually(t, func() bool { return !m.Downloading()[req.Hex] }, 10*time.Second, 5*time.Millisecond) + + require.True(t, st.IsPublished(req.Hex)) + assert.Empty(t, hf.recorded(), "the Hugging Face hub is never asked") + assert.NotEmpty(t, ms.recorded()) +} + func TestProgressReportsTheRunningAttempts(t *testing.T) { env := newTestEnv(t, repoFiles()) env.hub.gate = make(chan struct{}) @@ -425,7 +526,7 @@ func TestEnsureReportsNoRoom(t *testing.T) { func TestEnsureWaitsForAValidConfiguration(t *testing.T) { env := newTestEnv(t, repoFiles()) - env.m.Environment = func(context.Context) (Hub, *download.Downloader, error) { + env.m.Environment = func(context.Context, string) (Hub, *download.Downloader, error) { return nil, nil, &download.Error{Reason: download.ReasonInvalidRequest, Message: "the endpoint is not a URL"} } req := env.request("owner/repo", "uid-a", "token-a") @@ -438,7 +539,7 @@ func TestEnsureWaitsForAValidConfiguration(t *testing.T) { assert.Zero(t, rec.Failures, "a configuration the node cannot use yet is not the source's failure") // Once the node's configuration arrives, the next mount starts at once: there is no backoff to wait out. - env.m.Environment = func(context.Context) (Hub, *download.Downloader, error) { + env.m.Environment = func(context.Context, string) (Hub, *download.Downloader, error) { dl := download.New(env.hub.server.Client(), 8, 0) return env.hub, dl, nil } diff --git a/pkg/modelmanager/report/report.go b/pkg/modelmanager/report/report.go index 2ea332ffc..fa09fcbf1 100644 --- a/pkg/modelmanager/report/report.go +++ b/pkg/modelmanager/report/report.go @@ -179,9 +179,10 @@ func (r *Reporter) Pinned() map[string]bool { return out } -// Environment returns the hub and the downloader built from the node's current configuration, or an -// InvalidRequest error while that configuration is missing or fails its check. -func (r *Reporter) Environment(context.Context) (materialize.Hub, *download.Downloader, error) { +// Environment returns the hub of the named kind and the downloader built from the node's current +// configuration, or an InvalidRequest error while that configuration is missing, fails its check, +// or does not reach the hub the source names. +func (r *Reporter) Environment(_ context.Context, kind string) (materialize.Hub, *download.Downloader, error) { r.mu.Lock() defer r.mu.Unlock() switch { @@ -194,7 +195,22 @@ func (r *Reporter) Environment(context.Context) (materialize.Hub, *download.Down return nil, nil, &download.Error{Reason: download.ReasonInvalidRequest, Message: msg, Detail: msg} } - return &modelartifact.HuggingFace{Endpoint: r.spec.Hub.HuggingFaceEndpoint, Client: listingClient(r.client)}, r.Downloader, nil + switch kind { + case materialize.HubModelScope: + // Empty in a spec an older worker wrote: a ModelScope artifact waits with that named + // rather than being resolved against another hub. + if r.spec.Hub.ModelScopeEndpoint == "" { + msg := "the node's configuration names no ModelScope endpoint: it was written by an older worker; upgrade the worker" + return nil, nil, &download.Error{Reason: download.ReasonInvalidRequest, Message: msg, Detail: msg} + } + + return &modelartifact.ModelScope{Endpoint: r.spec.Hub.ModelScopeEndpoint, Client: listingClient(r.client)}, r.Downloader, nil + case materialize.HubHuggingFace: + return &modelartifact.HuggingFace{Endpoint: r.spec.Hub.HuggingFaceEndpoint, Client: listingClient(r.client)}, r.Downloader, nil + default: + msg := fmt.Sprintf("the node's configuration does not know the hub kind %q", kind) + return nil, nil, &download.Error{Reason: download.ReasonInvalidRequest, Message: msg, Detail: msg} + } } // PeerSyncEnabled says whether the node's configuration allows pulling content from the other diff --git a/pkg/modelmanager/report/report_test.go b/pkg/modelmanager/report/report_test.go index 29924a2a1..f527e7d14 100644 --- a/pkg/modelmanager/report/report_test.go +++ b/pkg/modelmanager/report/report_test.go @@ -23,8 +23,10 @@ import ( gpustack "gpustack.ai/gpustack/api/v1" workercore "gpustack.ai/gpustack/api/worker/v1alpha1" "gpustack.ai/gpustack/pkg/kubeclients/kubernetes/scheme" + "gpustack.ai/gpustack/pkg/modelartifact" "gpustack.ai/gpustack/pkg/modelmanager/download" "gpustack.ai/gpustack/pkg/modelmanager/gc" + "gpustack.ai/gpustack/pkg/modelmanager/materialize" "gpustack.ai/gpustack/pkg/modelmanager/store" ) @@ -252,7 +254,7 @@ func TestReportRefusesAnInvalidSpec(t *testing.T) { assert.Equal(t, meta.ConditionFalse, ready.Status) assert.Equal(t, ReasonInvalidConfiguration, ready.Reason) assert.Contains(t, ready.Message, c.wantErr) - _, _, err := env.r.Environment(context.Background()) + _, _, err := env.r.Environment(context.Background(), materialize.HubHuggingFace) assert.Equal(t, download.ReasonInvalidRequest, download.ReasonOf(err), "no download starts") }) } @@ -316,16 +318,50 @@ func TestReportKeepsTheClientOfAnUnchangedSpec(t *testing.T) { func TestEnvironmentBeforeTheSpecArrives(t *testing.T) { env := newTestEnv(t, validSpec()) - _, _, err := env.r.Environment(context.Background()) + _, _, err := env.r.Environment(context.Background(), materialize.HubHuggingFace) assert.Equal(t, download.ReasonInvalidRequest, download.ReasonOf(err)) require.NoError(t, env.r.Report(context.Background())) - hub, dl, err := env.r.Environment(context.Background()) + hub, dl, err := env.r.Environment(context.Background(), materialize.HubHuggingFace) require.NoError(t, err) assert.NotNil(t, hub) assert.Same(t, env.r.Downloader, dl, "every attempt shares the node's limits") } +func TestEnvironmentRejectsAnUnknownKind(t *testing.T) { + env := newTestEnv(t, validSpec()) + require.NoError(t, env.r.Report(context.Background())) + _, _, err := env.r.Environment(context.Background(), "bogus") + require.Error(t, err) + assert.Equal(t, download.ReasonInvalidRequest, download.ReasonOf(err)) + assert.Contains(t, err.Error(), "bogus") +} + +// TestEnvironmentForModelScopeNamesTheUpgrade pins the behavior on a spec an older worker wrote: +// a ModelScope source waits with that named, never resolved against the Hugging Face endpoint. +func TestEnvironmentForModelScopeNamesTheUpgrade(t *testing.T) { + env := newTestEnv(t, validSpec()) + require.NoError(t, env.r.Report(context.Background())) + _, _, err := env.r.Environment(context.Background(), materialize.HubModelScope) + require.Error(t, err) + assert.Equal(t, download.ReasonInvalidRequest, download.ReasonOf(err)) + assert.Contains(t, err.Error(), "upgrade the worker") + + hf, _, err := env.r.Environment(context.Background(), materialize.HubHuggingFace) + require.NoError(t, err) + require.IsType(t, &modelartifact.HuggingFace{}, hf) + + spec := validSpec() + spec.Hub.ModelScopeEndpoint = "https://ms.example" + env2 := newTestEnv(t, spec) + require.NoError(t, env2.r.Report(context.Background())) + hub, _, err := env2.r.Environment(context.Background(), materialize.HubModelScope) + require.NoError(t, err) + ms, ok := hub.(*modelartifact.ModelScope) + require.True(t, ok) + assert.Equal(t, "https://ms.example", ms.Endpoint) +} + func TestReportCapsTheWatermarkOnKubeletsFilesystem(t *testing.T) { spec := validSpec() spec.Watermarks = workercore.NodeModelStoreWatermarks{HighPercent: 90, LowPercent: 85} diff --git a/pkg/modelstore/config.go b/pkg/modelstore/config.go index 3f3379241..96824ce8e 100644 --- a/pkg/modelstore/config.go +++ b/pkg/modelstore/config.go @@ -49,6 +49,7 @@ type Layer struct { DownloadBytesPerSecond *int64 PeerSyncEnabled *bool HuggingFaceEndpoint *string + ModelScopeEndpoint *string HTTPSProxy *string NoProxy *string CABundleConfigMap *string @@ -66,6 +67,7 @@ func Merge(layers ...Layer) workercore.NodeModelStoreSpec { spec.PeerSyncEnabled = l.PeerSyncEnabled } overlay(&spec.Hub.HuggingFaceEndpoint, l.HuggingFaceEndpoint) + overlay(&spec.Hub.ModelScopeEndpoint, l.ModelScopeEndpoint) overlay(&spec.Hub.HTTPSProxy, l.HTTPSProxy) overlay(&spec.Hub.NoProxy, l.NoProxy) overlay(&spec.Hub.CABundleConfigMap, l.CABundleConfigMap) @@ -106,6 +108,14 @@ func Validate(spec workercore.NodeModelStoreSpec) error { if err := ValidateEndpoint(h.HuggingFaceEndpoint); err != nil { errs = append(errs, fmt.Errorf("hugging face endpoint: %w", err)) } + // The ModelScope endpoint is optional: a spec an older worker wrote names none, and a + // ModelScope artifact on such a node waits with that named rather than being resolved + // against another hub. A spec that names one must name a usable one. + if h.ModelScopeEndpoint != "" { + if err := ValidateEndpoint(h.ModelScopeEndpoint); err != nil { + errs = append(errs, fmt.Errorf("modelscope endpoint: %w", err)) + } + } if h.HTTPSProxy != "" { if err := modelartifact.ValidateProxy(h.HTTPSProxy); err != nil { errs = append(errs, fmt.Errorf("https proxy: %w", err)) diff --git a/pkg/modelstore/config_test.go b/pkg/modelstore/config_test.go index fb7599b6e..aba298064 100644 --- a/pkg/modelstore/config_test.go +++ b/pkg/modelstore/config_test.go @@ -76,6 +76,23 @@ func TestMerge(t *testing.T) { return s }(), }, + { + name: "a modelscope endpoint rides the layer and survives a layer that names none", + layers: []Layer{ + func() Layer { + l := clusterLayer() + l.ModelScopeEndpoint = ptr.To("https://www.modelscope.cn") + return l + }(), + {HighWatermarkPercent: ptr.To[int32](85)}, + }, + want: func() workercore.NodeModelStoreSpec { + s := validSpec() + s.Hub.ModelScopeEndpoint = "https://www.modelscope.cn" + s.Watermarks.HighPercent = 85 + return s + }(), + }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -108,6 +125,11 @@ func TestValidate(t *testing.T) { {name: "an endpoint without a scheme", mutate: func(s *workercore.NodeModelStoreSpec) { s.Hub.HuggingFaceEndpoint = "huggingface.co" }, wantErr: "endpoint"}, {name: "an ftp endpoint", mutate: func(s *workercore.NodeModelStoreSpec) { s.Hub.HuggingFaceEndpoint = "ftp://hub" }, wantErr: "endpoint"}, {name: "a blank endpoint", mutate: func(s *workercore.NodeModelStoreSpec) { s.Hub.HuggingFaceEndpoint = "" }, wantErr: "endpoint"}, + { + name: "a blank modelscope endpoint is a spec an older worker wrote", + mutate: func(s *workercore.NodeModelStoreSpec) { s.Hub.ModelScopeEndpoint = "" }, + }, + {name: "a bad modelscope endpoint", mutate: func(s *workercore.NodeModelStoreSpec) { s.Hub.ModelScopeEndpoint = "www.modelscope.cn" }, wantErr: "modelscope endpoint"}, {name: "a proxy with credentials", mutate: func(s *workercore.NodeModelStoreSpec) { s.Hub.HTTPSProxy = "http://u:p@proxy:3128" }, wantErr: "proxy"}, { name: "kubelet thresholds as percentages and quantities are valid", diff --git a/pkg/worker/controllers/worker/model_artifact.go b/pkg/worker/controllers/worker/model_artifact.go index c50c92af3..29b0707ee 100644 --- a/pkg/worker/controllers/worker/model_artifact.go +++ b/pkg/worker/controllers/worker/model_artifact.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "net/http" "strings" "sync" "time" @@ -74,6 +75,8 @@ type ModelArtifactReconciler struct { Now func() time.Time // NewHuggingFace builds the Hub client from the current Settings. Tests replace it. NewHuggingFace func(ctx context.Context) (*modelartifact.HuggingFace, error) + // NewModelScope builds the ModelScope client from the current Settings. Tests replace it. + NewModelScope func(ctx context.Context) (modelArtifactHub, error) // checks remembers, per artifact UID, the Secret resourceVersion the source was last asked // with and when it may be asked next. It paces the calls to the Hub: this controller also runs @@ -88,6 +91,14 @@ type ModelArtifactReconciler struct { var _ ctrlreconcile.Reconciler = (*ModelArtifactReconciler)(nil) +// modelArtifactHub is what a hub source resolves and revalidates through. The two hubs share one +// staircase; only the endpoints and the reason classifier differ, and those live in the clients. +type modelArtifactHub interface { + Resolve(ctx context.Context, repository, revision, token string, filter modelartifact.Filter) (modelartifact.Resolution, error) + Revalidate(ctx context.Context, repository, commit, token string) error + ValidToken(ctx context.Context, token string) (bool, error) +} + func (r *ModelArtifactReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { logger := ctrllog.FromContext(ctx) @@ -128,14 +139,26 @@ func (r *ModelArtifactReconciler) Reconcile(ctx context.Context, req ctrl.Reques ) switch { case ma.Spec.Source.HuggingFace != nil: - result, check = r.reconcileHuggingFace(ctx, ma) + hub, err := r.NewHuggingFace(ctx) + if err != nil { + ModelArtifactConditionDegraded.True(ma, modelartifact.ReasonSourceUnavailable, err.Error()) + return ctrl.Result{RequeueAfter: modelArtifactUnavailableRetry}, nil + } + result, check = r.reconcileHubSource(ctx, ma, hub, ma.Spec.Source.HuggingFace) + case ma.Spec.Source.ModelScope != nil: + hub, err := r.NewModelScope(ctx) + if err != nil { + ModelArtifactConditionDegraded.True(ma, modelartifact.ReasonSourceUnavailable, err.Error()) + return ctrl.Result{RequeueAfter: modelArtifactUnavailableRetry}, nil + } + result, check = r.reconcileHubSource(ctx, ma, hub, ma.Spec.Source.ModelScope) case ma.Spec.Source.PersistentVolumeClaim != nil: result = r.reconcileClaim(ctx, ma) case ma.Spec.Source.Image != nil: r.reconcileImage(ma) default: - // Admission refuses every other shape, ModelScope included; one stored before a rule - // existed stays unresolved and says why. + // Admission refuses every other shape; one stored before a rule existed stays unresolved + // and says why. ModelArtifactConditionResolved.False(ma, "UnsupportedSource", "the source is not accepted in this version") } result = earliestResult(result, r.reconcileNodes(ctx, ma, before.Nodes)) @@ -300,13 +323,11 @@ type modelArtifactCheck struct { next time.Time } -// reconcileHuggingFace resolves a Hugging Face source once, then revalidates its access. It returns -// the pacing entry to record once the status is stored, or nil when the source was not asked. -func (r *ModelArtifactReconciler) reconcileHuggingFace( - ctx context.Context, ma *workercore.ModelArtifact, +// reconcileHubSource resolves a hub source once, then revalidates its access. It returns the +// pacing entry to record once the status is stored, or nil when the source was not asked. +func (r *ModelArtifactReconciler) reconcileHubSource( + ctx context.Context, ma *workercore.ModelArtifact, hub modelArtifactHub, source *workercore.ModelArtifactHubSource, ) (ctrl.Result, *modelArtifactCheck) { - source := ma.Spec.Source.HuggingFace - token, secretVersion, err := r.modelArtifactToken(ctx, ma.Namespace, source.SecretRef) switch { case errors.Is(err, errModelArtifactSecretMissing): @@ -332,29 +353,24 @@ func (r *ModelArtifactReconciler) reconcileHuggingFace( } } - hub, err := r.NewHuggingFace(ctx) - if err != nil { - ModelArtifactConditionDegraded.True(ma, modelartifact.ReasonSourceUnavailable, err.Error()) - return ctrl.Result{RequeueAfter: modelArtifactUnavailableRetry}, nil - } ctx, cancel := context.WithTimeout(ctx, modelArtifactResolveTimeout) defer cancel() var after time.Duration if ma.Status.Resolved == nil { - after = r.resolveHuggingFace(ctx, ma, hub, token) + after = r.resolveHubSource(ctx, ma, hub, source, token) } else { - after = r.revalidateHuggingFace(ctx, ma, hub, token, now, secretChanged) + after = r.revalidateHubSource(ctx, ma, hub, source, token, now, secretChanged) } return ctrl.Result{RequeueAfter: after}, &modelArtifactCheck{secretVersion: secretVersion, next: now.Add(after)} } -// resolveHuggingFace resolves the source once and reports when the source is to be asked next. -func (r *ModelArtifactReconciler) resolveHuggingFace( - ctx context.Context, ma *workercore.ModelArtifact, hub *modelartifact.HuggingFace, token string, +// resolveHubSource resolves the source once and reports when the source is to be asked next. +func (r *ModelArtifactReconciler) resolveHubSource( + ctx context.Context, ma *workercore.ModelArtifact, hub modelArtifactHub, source *workercore.ModelArtifactHubSource, + token string, ) time.Duration { - source := ma.Spec.Source.HuggingFace resolution, err := hub.Resolve(ctx, source.Repository, source.Revision, token, modelartifact.Filter{ Allow: ma.Spec.AllowPatterns, Ignore: ma.Spec.IgnorePatterns, }) @@ -380,20 +396,20 @@ func (r *ModelArtifactReconciler) resolveHuggingFace( ModelArtifactConditionResolved.True(ma, modelArtifactReasonResolved, fmt.Sprintf("%s@%s resolved to commit %s", source.Repository, source.Revision, resolution.Commit)) ModelArtifactConditionDegraded.False(ma, modelArtifactReasonHealthy, "") - r.warnOnRejectedToken(ctx, ma, hub, token) + r.warnOnRejectedToken(ctx, ma, source, hub, token) return r.revalidateInterval(ctx) } -// revalidateHuggingFace checks access at the resolved commit and reports when the source is to be +// revalidateHubSource checks access at the resolved commit and reports when the source is to be // asked next. The commit and the digest are never re-resolved. // // A REFUSAL REVOKES ONLY WHEN CONFIRMED. The first one sets Degraded, stamped with the time it // happened, and the check is repeated after modelArtifactConfirmDelay; the same refusal then sets // Resolved=False. A failure to reach the source never revokes: it says nothing about the grant. -func (r *ModelArtifactReconciler) revalidateHuggingFace( - ctx context.Context, ma *workercore.ModelArtifact, hub *modelartifact.HuggingFace, token string, now time.Time, - secretChanged bool, +func (r *ModelArtifactReconciler) revalidateHubSource( + ctx context.Context, ma *workercore.ModelArtifact, hub modelArtifactHub, source *workercore.ModelArtifactHubSource, + token string, now time.Time, secretChanged bool, ) time.Duration { interval := r.revalidateInterval(ctx) if last := ma.Status.Resolved.LastValidatedTime; last != nil && ModelArtifactConditionResolved.IsTrue(ma) && @@ -404,7 +420,6 @@ func (r *ModelArtifactReconciler) revalidateHuggingFace( } } - source := ma.Spec.Source.HuggingFace err := hub.Revalidate(ctx, source.Repository, ma.Status.Resolved.Revision, token) if err == nil { validated := meta.NewTime(now) @@ -414,7 +429,7 @@ func (r *ModelArtifactReconciler) revalidateHuggingFace( // The token is judged when it is new, not on every interval: an unchanged rejected token // was already reported, and one more event per interval would only repeat it. if secretChanged { - r.warnOnRejectedToken(ctx, ma, hub, token) + r.warnOnRejectedToken(ctx, ma, source, hub, token) } return interval } @@ -459,13 +474,14 @@ func modelArtifactConditionSince(ma *workercore.ModelArtifact, c kubeapistatus.C return time.Time{} } -// warnOnRejectedToken emits a Warning when the Hub rejects the token outright. The Hub ignores a +// warnOnRejectedToken emits a Warning when the hub rejects the token outright. A hub ignores a // rejected token on a public repository, answering as if none had been sent, so without this a // mistyped token would never surface. It does not change the conditions. func (r *ModelArtifactReconciler) warnOnRejectedToken( - ctx context.Context, ma *workercore.ModelArtifact, hub *modelartifact.HuggingFace, token string, + ctx context.Context, ma *workercore.ModelArtifact, source *workercore.ModelArtifactHubSource, + hub modelArtifactHub, token string, ) { - if token == "" || r.Recorder == nil { + if token == "" || r.Recorder == nil || source.SecretRef == nil { return } valid, err := hub.ValidToken(ctx, token) @@ -473,8 +489,8 @@ func (r *ModelArtifactReconciler) warnOnRejectedToken( return } r.Recorder.Eventf(ma, core.EventTypeWarning, "InvalidToken", - "the Hub rejects the token in Secret %q; a public repository still resolves, as if no token had been sent", - ma.Spec.Source.HuggingFace.SecretRef.Name) + "the hub rejects the token in Secret %q; a public repository still resolves, as if no token had been sent", + source.SecretRef.Name) } var errModelArtifactSecretMissing = errors.New("secret missing") @@ -521,6 +537,33 @@ func (r *ModelArtifactReconciler) now() time.Time { // newHuggingFaceFromSettings builds the Hub client from the administrator's Settings. func (r *ModelArtifactReconciler) newHuggingFaceFromSettings(ctx context.Context) (*modelartifact.HuggingFace, error) { + client, err := r.newHubHTTPClient(ctx) + if err != nil { + return nil, err + } + + return &modelartifact.HuggingFace{ + Endpoint: settings.ModelArtifactHuggingFaceEndpoint.ShouldValue(ctx), + Client: client, + }, nil +} + +// newModelScopeFromSettings builds the ModelScope client from the administrator's Settings, the +// same proxy and CA bundle the Hugging Face client takes. +func (r *ModelArtifactReconciler) newModelScopeFromSettings(ctx context.Context) (modelArtifactHub, error) { + client, err := r.newHubHTTPClient(ctx) + if err != nil { + return nil, err + } + + return &modelartifact.ModelScope{ + Endpoint: settings.ModelArtifactModelScopeEndpoint.ShouldValue(ctx), + Client: client, + }, nil +} + +// newHubHTTPClient builds the client both hub controllers send their requests with. +func (r *ModelArtifactReconciler) newHubHTTPClient(ctx context.Context) (*http.Client, error) { opts := modelartifact.HTTPClientOptions{ HTTPSProxy: settings.ModelArtifactHTTPSProxy.ShouldValue(ctx), NoProxy: settings.ModelArtifactNoProxy.ShouldValue(ctx), @@ -532,15 +575,8 @@ func (r *ModelArtifactReconciler) newHuggingFaceFromSettings(ctx context.Context } opts.CABundle = []byte(cm.Data["ca.crt"]) } - client, err := modelartifact.NewHTTPClient(opts) - if err != nil { - return nil, err - } - return &modelartifact.HuggingFace{ - Endpoint: settings.ModelArtifactHuggingFaceEndpoint.ShouldValue(ctx), - Client: client, - }, nil + return modelartifact.NewHTTPClient(opts) } func (r *ModelArtifactReconciler) SetupController(ctx context.Context, opts controller.SetupOptions) error { @@ -568,6 +604,9 @@ func (r *ModelArtifactReconciler) SetupController(ctx context.Context, opts cont if r.NewHuggingFace == nil { r.NewHuggingFace = r.newHuggingFaceFromSettings } + if r.NewModelScope == nil { + r.NewModelScope = r.newModelScopeFromSettings + } return ctrl.NewControllerManagedBy(opts.Manager). Named("modelartifact"). @@ -588,8 +627,12 @@ func (r *ModelArtifactReconciler) SetupController(ctx context.Context, opts cont func (r *ModelArtifactReconciler) enqueueModelArtifactsForSecret(ctx context.Context, obj ctrlcli.Object) []ctrlreconcile.Request { return r.enqueueModelArtifacts(ctx, obj.GetNamespace(), func(ma *workercore.ModelArtifact) bool { - hub := ma.Spec.Source.HuggingFace - return hub != nil && hub.SecretRef != nil && hub.SecretRef.Name == obj.GetName() + for _, hub := range []*workercore.ModelArtifactHubSource{ma.Spec.Source.HuggingFace, ma.Spec.Source.ModelScope} { + if hub != nil && hub.SecretRef != nil && hub.SecretRef.Name == obj.GetName() { + return true + } + } + return false }) } diff --git a/pkg/worker/controllers/worker/model_artifact_placement.go b/pkg/worker/controllers/worker/model_artifact_placement.go index fef92a4f3..6d457a8eb 100644 --- a/pkg/worker/controllers/worker/model_artifact_placement.go +++ b/pkg/worker/controllers/worker/model_artifact_placement.go @@ -125,31 +125,40 @@ func resolveModelArtifactWeights( Delivery: workercore.ModelDeploymentModelDeliveryImage, ImageReference: source.Image.Reference, } - case source.HuggingFace != nil && (nodeOnly || modelArtifactDeliveryMode(ctx) == settings.ModelArtifactDeliveryNode): - w.Render = &ModelDeploymentArtifactRender{ - Delivery: workercore.ModelDeploymentModelDeliveryNode, - ArtifactName: name, - ArtifactUID: string(ma.UID), - ManifestDigest: resolved.ManifestDigest, - Repository: source.HuggingFace.Repository, - Revision: resolved.Revision, - SizeBytes: resolved.SizeBytes, - } - if source.HuggingFace.SecretRef != nil { - w.Render.SecretName = source.HuggingFace.SecretRef.Name - } - case source.HuggingFace != nil: - w.Render = &ModelDeploymentArtifactRender{ - Delivery: workercore.ModelDeploymentModelDeliveryEngine, - Repository: source.HuggingFace.Repository, - Revision: resolved.Revision, - SizeBytes: resolved.SizeBytes, - Endpoint: settings.ModelArtifactHuggingFaceEndpoint.ShouldValue(ctx), - HTTPSProxy: settings.ModelArtifactHTTPSProxy.ShouldValue(ctx), - NoProxy: settings.ModelArtifactNoProxy.ShouldValue(ctx), - } - if source.HuggingFace.SecretRef != nil { - w.Render.SecretName = source.HuggingFace.SecretRef.Name + case source.HuggingFace != nil, source.ModelScope != nil: + // The two hubs differ in the endpoints they are resolved against and the environment an + // engine download reads; delivery, placement and every digest-keyed behavior are shared. + hub, kind := source.HuggingFace, modelDeploymentHubHuggingFace + endpoint := settings.ModelArtifactHuggingFaceEndpoint.ShouldValue(ctx) + if hub == nil { + hub, kind = source.ModelScope, modelDeploymentHubModelScope + endpoint = settings.ModelArtifactModelScopeEndpoint.ShouldValue(ctx) + } + if nodeOnly || modelArtifactDeliveryMode(ctx) == settings.ModelArtifactDeliveryNode { + w.Render = &ModelDeploymentArtifactRender{ + Delivery: workercore.ModelDeploymentModelDeliveryNode, + Hub: kind, + ArtifactName: name, + ArtifactUID: string(ma.UID), + ManifestDigest: resolved.ManifestDigest, + Repository: hub.Repository, + Revision: resolved.Revision, + SizeBytes: resolved.SizeBytes, + } + } else { + w.Render = &ModelDeploymentArtifactRender{ + Delivery: workercore.ModelDeploymentModelDeliveryEngine, + Hub: kind, + Repository: hub.Repository, + Revision: resolved.Revision, + SizeBytes: resolved.SizeBytes, + Endpoint: endpoint, + HTTPSProxy: settings.ModelArtifactHTTPSProxy.ShouldValue(ctx), + NoProxy: settings.ModelArtifactNoProxy.ShouldValue(ctx), + } + } + if hub.SecretRef != nil { + w.Render.SecretName = hub.SecretRef.Name } default: return blockedModelArtifactWeights(modelWeightsReasonArtifactUnready, diff --git a/pkg/worker/controllers/worker/model_artifact_test.go b/pkg/worker/controllers/worker/model_artifact_test.go index aac70ce51..9204bba4e 100644 --- a/pkg/worker/controllers/worker/model_artifact_test.go +++ b/pkg/worker/controllers/worker/model_artifact_test.go @@ -7,6 +7,7 @@ import ( "net/http" "net/http/httptest" "strings" + "sync" "sync/atomic" "testing" "time" @@ -155,6 +156,238 @@ func testTokenSecret(token string) *core.Secret { } } +// testModelScopeArtifact is a ModelScope source, optionally with a Secret reference. +func testModelScopeArtifact(secret string) *workercore.ModelArtifact { + ma := &workercore.ModelArtifact{ + ObjectMeta: meta.ObjectMeta{Namespace: "team-a", Name: "qwen", UID: "uid-qwen", Generation: 1}, + Spec: workercore.ModelArtifactSpec{Source: workercore.ModelArtifactSource{ + ModelScope: &workercore.ModelArtifactHubSource{Repository: "qwen/repo", Revision: "master"}, + }}, + } + if secret != "" { + ma.Spec.Source.ModelScope.SecretRef = &core.LocalObjectReference{Name: secret} + } + + return ma +} + +// testModelScopeHub is a fake ModelScope client: every answer swappable, every ask recorded. +type testModelScopeHub struct { + mu sync.Mutex + resolution modelartifact.Resolution + resolveErr error + revalidate []error // one per Revalidate ask, the last repeating + tokenValid bool + tokenErr error + asks int + lastRepo string + lastRev string + lastToken string + lastFilter modelartifact.Filter + lastCommit string +} + +func (h *testModelScopeHub) Resolve(_ context.Context, repository, revision, token string, filter modelartifact.Filter) (modelartifact.Resolution, error) { + h.mu.Lock() + defer h.mu.Unlock() + h.asks++ + h.lastRepo, h.lastRev, h.lastToken, h.lastFilter = repository, revision, token, filter + if h.resolveErr != nil { + return modelartifact.Resolution{}, h.resolveErr + } + + return h.resolution, nil +} + +func (h *testModelScopeHub) Revalidate(_ context.Context, repository, commit, token string) error { + h.mu.Lock() + defer h.mu.Unlock() + h.asks++ + h.lastRepo, h.lastCommit, h.lastToken = repository, commit, token + if len(h.revalidate) == 0 { + return nil + } + err := h.revalidate[0] + if len(h.revalidate) > 1 { + h.revalidate = h.revalidate[1:] + } + + return err +} + +func (h *testModelScopeHub) ValidToken(_ context.Context, _ string) (bool, error) { + return h.tokenValid, h.tokenErr +} + +// newTestModelScopeEnv is the artifact environment with the ModelScope client swapped for fake. +func newTestModelScopeEnv(t *testing.T, hub *testModelScopeHub, objs ...ctrlcli.Object) *testArtifactEnv { + t.Helper() + env := newTestArtifactEnv(t, objs...) + env.r.NewModelScope = func(context.Context) (modelArtifactHub, error) { return hub, nil } + + return env +} + +func TestModelArtifactReconcileResolvesModelScope(t *testing.T) { + digest, err := modelartifact.NewManifest([]modelartifact.ManifestEntry{ + {Path: "README.md", Size: 3, Digest: modelartifact.DigestSHA256 + ":" + strings.Repeat("a", 64)}, + }) + require.NoError(t, err) + hub := &testModelScopeHub{resolution: modelartifact.Resolution{ + Commit: testArtifactCommit, Manifest: digest, + }} + env := newTestModelScopeEnv(t, hub, testModelScopeArtifact("")) + + ma, res := env.reconcile(t, "qwen") + assert.True(t, ModelArtifactConditionResolved.IsTrue(ma)) + assert.Equal(t, testArtifactCommit, ma.Status.Resolved.Revision) + assert.Equal(t, digest.Digest, ma.Status.Resolved.ManifestDigest) + assert.Positive(t, ma.Status.Resolved.FileCount) + assert.Positive(t, res.RequeueAfter) + hub.mu.Lock() + defer hub.mu.Unlock() + assert.Equal(t, "qwen/repo", hub.lastRepo) + assert.Equal(t, "master", hub.lastRev) + assert.Equal(t, "", hub.lastToken, "an artifact without a SecretRef sends no token") +} + +func TestModelArtifactReconcileModelScopeSendsTheToken(t *testing.T) { + digest, err := modelartifact.NewManifest([]modelartifact.ManifestEntry{ + {Path: "README.md", Size: 3, Digest: modelartifact.DigestSHA256 + ":" + strings.Repeat("a", 64)}, + }) + require.NoError(t, err) + hub := &testModelScopeHub{resolution: modelartifact.Resolution{Commit: testArtifactCommit, Manifest: digest}} + env := newTestModelScopeEnv(t, hub, testModelScopeArtifact("hf-token"), testTokenSecret(testArtifactToken)) + + env.reconcile(t, "qwen") + hub.mu.Lock() + defer hub.mu.Unlock() + assert.Equal(t, testArtifactToken, hub.lastToken) +} + +func TestModelArtifactReconcileModelScopeRefusals(t *testing.T) { + cases := []struct { + name string + resolveErr error + wantReason string + }{ + { + name: "a missing revision", + resolveErr: &modelartifact.SourceError{Reason: modelartifact.ReasonRevisionNotFound, Message: "no such ref"}, + wantReason: modelartifact.ReasonRevisionNotFound, + }, + { + name: "no access", + resolveErr: &modelartifact.SourceError{Reason: modelartifact.ReasonAccessDenied, Message: "does not exist or is not accessible"}, + wantReason: modelartifact.ReasonAccessDenied, + }, + { + name: "an outage", + resolveErr: &modelartifact.SourceError{Reason: modelartifact.ReasonSourceUnavailable, Message: "HTTP 500"}, + wantReason: modelartifact.ReasonSourceUnavailable, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + hub := &testModelScopeHub{resolveErr: c.resolveErr} + env := newTestModelScopeEnv(t, hub, testModelScopeArtifact("")) + + ma, _ := env.reconcile(t, "qwen") + assert.False(t, ModelArtifactConditionResolved.IsTrue(ma)) + assert.Equal(t, c.wantReason, ModelArtifactConditionResolved.GetReason(ma)) + assert.Nil(t, ma.Status.Resolved) + }) + } +} + +func TestModelArtifactReconcileModelScopeRevokes(t *testing.T) { + digest, err := modelartifact.NewManifest([]modelartifact.ManifestEntry{ + {Path: "README.md", Size: 3, Digest: modelartifact.DigestSHA256 + ":" + strings.Repeat("a", 64)}, + }) + require.NoError(t, err) + refused := &modelartifact.SourceError{Reason: modelartifact.ReasonAccessDenied, Message: "gone"} + hub := &testModelScopeHub{ + resolution: modelartifact.Resolution{Commit: testArtifactCommit, Manifest: digest}, + revalidate: []error{refused, refused, refused}, + } + env := newTestModelScopeEnv(t, hub, testModelScopeArtifact("")) + resolved, _ := env.reconcile(t, "qwen") + require.True(t, ModelArtifactConditionResolved.IsTrue(resolved)) + + day := 24 * time.Hour + env.clock.now = env.clock.now.Add(day) + first, _ := env.reconcile(t, "qwen") + assert.True(t, ModelArtifactConditionResolved.IsTrue(first), "the first refusal degrades, not revokes") + assert.Equal(t, modelartifact.ReasonAccessDenied, ModelArtifactConditionDegraded.GetReason(first)) + + env.clock.now = env.clock.now.Add(10 * time.Second) + env.reconcile(t, "qwen") + + env.clock.now = env.clock.now.Add(time.Minute) + revoked, _ := env.reconcile(t, "qwen") + assert.False(t, ModelArtifactConditionResolved.IsTrue(revoked), "the confirmed refusal revokes") + assert.Equal(t, modelartifact.ReasonAccessDenied, ModelArtifactConditionResolved.GetReason(revoked)) + assert.Equal(t, testArtifactCommit, revoked.Status.Resolved.Revision, "the digest and commit stay for audit") +} + +func TestModelArtifactReconcileModelScopeSecretMissing(t *testing.T) { + digest, err := modelartifact.NewManifest([]modelartifact.ManifestEntry{ + {Path: "README.md", Size: 3, Digest: modelartifact.DigestSHA256 + ":" + strings.Repeat("a", 64)}, + }) + require.NoError(t, err) + hub := &testModelScopeHub{resolution: modelartifact.Resolution{Commit: testArtifactCommit, Manifest: digest}} + env := newTestModelScopeEnv(t, hub, testModelScopeArtifact("hf-token"), testTokenSecret(testArtifactToken)) + env.reconcile(t, "qwen") + + require.NoError(t, env.cli.Delete(context.Background(), testTokenSecret(testArtifactToken))) + env.clock.now = env.clock.now.Add(time.Minute) + + ma, _ := env.reconcile(t, "qwen") + assert.False(t, ModelArtifactConditionResolved.IsTrue(ma)) + assert.Equal(t, modelArtifactReasonSecretMissing, ModelArtifactConditionResolved.GetReason(ma)) +} + +func TestModelArtifactReconcileModelScopeWarnsOnARejectedToken(t *testing.T) { + digest, err := modelartifact.NewManifest([]modelartifact.ManifestEntry{ + {Path: "README.md", Size: 3, Digest: modelartifact.DigestSHA256 + ":" + strings.Repeat("a", 64)}, + }) + require.NoError(t, err) + hub := &testModelScopeHub{ + resolution: modelartifact.Resolution{Commit: testArtifactCommit, Manifest: digest}, + tokenValid: false, + } + env := newTestModelScopeEnv(t, hub, testModelScopeArtifact("hf-token"), testTokenSecret("mistyped")) + + env.reconcile(t, "qwen") + require.Len(t, env.recorder.Events, 1) + event := <-env.recorder.Events + assert.Contains(t, event, "Warning InvalidToken") + assert.Contains(t, event, "hf-token") + assert.NotContains(t, event, "mistyped") +} + +func TestModelArtifactEnqueueModelScopesForSecret(t *testing.T) { + env := newTestArtifactEnv(t) + env.r.NewModelScope = func(context.Context) (modelArtifactHub, error) { + return &testModelScopeHub{}, nil + } + require.NoError(t, env.cli.Create(context.Background(), testModelScopeArtifact("hf-token"))) + + reqs := env.r.enqueueModelArtifactsForSecret(context.Background(), testTokenSecret(testArtifactToken)) + require.Len(t, reqs, 1) + assert.Equal(t, types.NamespacedName{Namespace: "team-a", Name: "qwen"}, reqs[0].NamespacedName) +} + +func TestModelArtifactReconcileUnsupportedSource(t *testing.T) { + env := newTestArtifactEnv(t, &workercore.ModelArtifact{ + ObjectMeta: meta.ObjectMeta{Namespace: "team-a", Name: "qwen", UID: "uid-qwen", Generation: 1}, + }) + + ma, _ := env.reconcile(t, "qwen") + assert.False(t, ModelArtifactConditionResolved.IsTrue(ma)) + assert.Equal(t, "UnsupportedSource", ModelArtifactConditionResolved.GetReason(ma)) +} + func TestModelArtifactReconcileResolvesHuggingFace(t *testing.T) { cases := []struct { name string diff --git a/pkg/worker/controllers/worker/model_deployment_artifact.go b/pkg/worker/controllers/worker/model_deployment_artifact.go index 56ea137c7..c5f213071 100644 --- a/pkg/worker/controllers/worker/model_deployment_artifact.go +++ b/pkg/worker/controllers/worker/model_deployment_artifact.go @@ -1,6 +1,8 @@ package worker import ( + "net/url" + core "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/utils/ptr" @@ -24,6 +26,21 @@ const ( modelDeploymentHFTokenEnv = "HF_TOKEN" modelDeploymentHTTPSProxyEnv = "HTTPS_PROXY" modelDeploymentNoProxyEnv = "NO_PROXY" + + modelDeploymentModelScopeCacheEnv = "MODELSCOPE_CACHE" + modelDeploymentModelScopeDomainEnv = "MODELSCOPE_DOMAIN" + modelDeploymentModelScopeTokenEnv = "MODELSCOPE_API_TOKEN" + // The engines' switches that route their own download through their bundled ModelScope SDK + // (measured, PoC-C): each engine reads only its own. + modelDeploymentVLLMModelScopeSwitch = "VLLM_USE_MODELSCOPE" + modelDeploymentSGLangModelScopeSwitch = "SGLANG_USE_MODELSCOPE" +) + +// The hub kinds a hub artifact's download environment is rendered for. An engine downloads from +// one hub, and the names it reads differ by hub. +const ( + modelDeploymentHubHuggingFace = "huggingFace" + modelDeploymentHubModelScope = "modelscope" ) // modelDeploymentCacheHeadroomMin is the least headroom an engine's download cache gets above the @@ -65,6 +82,10 @@ type ModelDeploymentArtifactRender struct { SecretName string SizeBytes int64 + // Hub is which hub a hub artifact downloads from, one of the modelDeploymentHub kinds. The + // download environment's names differ by hub; a non-hub delivery renders none. + Hub string + // Endpoint, HTTPSProxy and NoProxy are the administrator's Settings, read by the reconciler. Endpoint string HTTPSProxy string @@ -176,22 +197,43 @@ func (a *ModelDeploymentArtifactRender) cacheSize() resource.Quantity { // env returns the environment an engine download needs: the owned entries, then the defaulted // ones a role's own value replaces. A claim needs none, and a take-over role gets none. -func (a *ModelDeploymentArtifactRender) env(takeOver bool) (owned, defaulted []core.EnvVar) { +func (a *ModelDeploymentArtifactRender) env(takeOver bool, engine string) (owned, defaulted []core.EnvVar) { if takeOver || a.Delivery != workercore.ModelDeploymentModelDeliveryEngine { return nil, nil } - owned = []core.EnvVar{ - {Name: modelDeploymentHFHomeEnv, Value: ModelDeploymentModelCachePath}, - {Name: modelDeploymentHFEndpointEnv, Value: a.Endpoint}, - } - if a.SecretName != "" { - owned = append(owned, core.EnvVar{Name: modelDeploymentHFTokenEnv, ValueFrom: &core.EnvVarSource{ - SecretKeyRef: &core.SecretKeySelector{ - LocalObjectReference: core.LocalObjectReference{Name: a.SecretName}, - Key: modelArtifactTokenKey, - }, - }}) + switch a.Hub { + case modelDeploymentHubModelScope: + owned = []core.EnvVar{ + {Name: modelDeploymentModelScopeCacheEnv, Value: ModelDeploymentModelCachePath}, + // The SDK takes a bare host and prefixes the scheme itself (measured in the runner's + // own SDK); the Setting keeps the full-URL shape the Hugging Face endpoint uses. + {Name: modelDeploymentModelScopeDomainEnv, Value: endpointHost(a.Endpoint)}, + } + if sw := modelScopeSwitch(engine); sw != "" { + owned = append(owned, core.EnvVar{Name: sw, Value: "true"}) + } + if a.SecretName != "" { + owned = append(owned, core.EnvVar{Name: modelDeploymentModelScopeTokenEnv, ValueFrom: &core.EnvVarSource{ + SecretKeyRef: &core.SecretKeySelector{ + LocalObjectReference: core.LocalObjectReference{Name: a.SecretName}, + Key: modelArtifactTokenKey, + }, + }}) + } + default: + owned = []core.EnvVar{ + {Name: modelDeploymentHFHomeEnv, Value: ModelDeploymentModelCachePath}, + {Name: modelDeploymentHFEndpointEnv, Value: a.Endpoint}, + } + if a.SecretName != "" { + owned = append(owned, core.EnvVar{Name: modelDeploymentHFTokenEnv, ValueFrom: &core.EnvVarSource{ + SecretKeyRef: &core.SecretKeySelector{ + LocalObjectReference: core.LocalObjectReference{Name: a.SecretName}, + Key: modelArtifactTokenKey, + }, + }}) + } } if a.HTTPSProxy != "" { defaulted = append(defaulted, core.EnvVar{Name: modelDeploymentHTTPSProxyEnv, Value: a.HTTPSProxy}) @@ -203,6 +245,32 @@ func (a *ModelDeploymentArtifactRender) env(takeOver bool) (owned, defaulted []c return owned, defaulted } +// modelScopeSwitch is the environment name that routes an engine's own download through its +// bundled ModelScope SDK. An engine the table does not know reads neither switch, so none is +// rendered for it. +func modelScopeSwitch(engine string) string { + switch engine { + case workercore.ModelDeploymentEngineVLLM: + return modelDeploymentVLLMModelScopeSwitch + case workercore.ModelDeploymentEngineSGLang: + return modelDeploymentSGLangModelScopeSwitch + default: + return "" + } +} + +// endpointHost is the URL's bare host, for the one consumer that takes a host rather than a URL. +// A value that does not parse rides as it is: the admission that passed it has already demanded a +// schema and a host. +func endpointHost(raw string) string { + u, err := url.Parse(raw) + if err != nil || u.Host == "" { + return raw + } + + return u.Host +} + // raiseEphemeralStorageLimit adds an engine download's cache limit to the main container's // ephemeral-storage limit, leaving the request alone. // diff --git a/pkg/worker/controllers/worker/model_deployment_artifact_test.go b/pkg/worker/controllers/worker/model_deployment_artifact_test.go index 798e735ad..5036781c8 100644 --- a/pkg/worker/controllers/worker/model_deployment_artifact_test.go +++ b/pkg/worker/controllers/worker/model_deployment_artifact_test.go @@ -25,11 +25,21 @@ func testPvcArtifactRender() *ModelDeploymentArtifactRender { func testEngineArtifactRender() *ModelDeploymentArtifactRender { return &ModelDeploymentArtifactRender{ Delivery: workercore.ModelDeploymentModelDeliveryEngine, + Hub: modelDeploymentHubHuggingFace, Repository: "Qwen/Qwen2.5-72B-Instruct", Revision: testArtifactRevision, SecretName: "hf-token", SizeBytes: 100 << 30, Endpoint: "https://hub.example", HTTPSProxy: "http://proxy:3128", NoProxy: "svc", } } +func testModelScopeEngineRender() *ModelDeploymentArtifactRender { + return &ModelDeploymentArtifactRender{ + Delivery: workercore.ModelDeploymentModelDeliveryEngine, + Hub: modelDeploymentHubModelScope, + Repository: "qwen/Qwen2.5-72B-Instruct", Revision: testArtifactRevision, SecretName: "ms-token", + SizeBytes: 100 << 30, Endpoint: "https://www.modelscope.cn", HTTPSProxy: "http://proxy:3128", NoProxy: "svc", + } +} + func testNodeArtifactRender() *ModelDeploymentArtifactRender { return &ModelDeploymentArtifactRender{ Delivery: workercore.ModelDeploymentModelDeliveryNode, ArtifactName: "qwen", ArtifactUID: "uid-qwen", @@ -341,6 +351,69 @@ func TestRenderModelDeploymentArtifactEnvironment(t *testing.T) { } } +// TestRenderModelDeploymentArtifactModelScopeEnvironment renders a ModelScope download's +// environment: the SDK's own names, the endpoint as the bare host the SDK prefixes, the engine's +// use-switch, and the token from the artifact's Secret. No Hugging Face name appears. +func TestRenderModelDeploymentArtifactModelScopeEnvironment(t *testing.T) { + cases := []struct { + name string + engine string + wantUse string + notUse string + absent []string + }{ + { + name: "a vLLM download flips vLLM's switch", engine: workercore.ModelDeploymentEngineVLLM, + wantUse: "VLLM_USE_MODELSCOPE", notUse: "SGLANG_USE_MODELSCOPE", + absent: []string{"HF_HOME", "HF_ENDPOINT", "HF_TOKEN"}, + }, + { + name: "a SGLang download flips SGLang's switch", engine: workercore.ModelDeploymentEngineSGLang, + wantUse: "SGLANG_USE_MODELSCOPE", notUse: "VLLM_USE_MODELSCOPE", + absent: []string{"HF_HOME", "HF_ENDPOINT", "HF_TOKEN"}, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + md := newRenderDeployment(func(md *workercore.ModelDeployment) { + md.Spec.Engine.Name = c.engine + }) + main := &renderWithArtifact(t, md, testModelScopeEngineRender()).Spec.Containers[0] + + for name, want := range map[string]string{ + "MODELSCOPE_CACHE": ModelDeploymentModelCachePath, + "MODELSCOPE_DOMAIN": "www.modelscope.cn", + c.wantUse: "true", + "HTTPS_PROXY": "http://proxy:3128", + } { + e := findEnv(main, name) + require.NotNil(t, e, name) + assert.Equal(t, want, e.Value, name) + } + assert.Nil(t, findEnv(main, c.notUse), c.notUse) + token := findEnv(main, "MODELSCOPE_API_TOKEN") + require.NotNil(t, token) + assert.Empty(t, token.Value, "the token is never a literal") + require.NotNil(t, token.ValueFrom) + assert.Equal(t, "ms-token", token.ValueFrom.SecretKeyRef.Name) + assert.Equal(t, "token", token.ValueFrom.SecretKeyRef.Key) + for _, name := range c.absent { + assert.Nil(t, findEnv(main, name), name) + } + }) + } + + t.Run("a public artifact gets no token", func(t *testing.T) { + render := testModelScopeEngineRender() + render.SecretName, render.HTTPSProxy, render.NoProxy = "", "", "" + main := &renderWithArtifact(t, newRenderDeployment(), render).Spec.Containers[0] + + assert.Nil(t, findEnv(main, "MODELSCOPE_API_TOKEN")) + assert.Nil(t, findEnv(main, "HTTPS_PROXY")) + assert.NotNil(t, findEnv(main, "VLLM_USE_MODELSCOPE")) + }) +} + func TestRenderModelDeploymentArtifactEphemeralStorage(t *testing.T) { cases := []struct { name string diff --git a/pkg/worker/controllers/worker/model_deployment_connector.go b/pkg/worker/controllers/worker/model_deployment_connector.go index 6ba402fa8..e73d326e4 100644 --- a/pkg/worker/controllers/worker/model_deployment_connector.go +++ b/pkg/worker/controllers/worker/model_deployment_connector.go @@ -393,11 +393,17 @@ var modelDeploymentArtifactOwnedKeys = map[string]struct { }{ workercore.ModelDeploymentEngineVLLM: { Args: []string{"--model", "--revision", "--tokenizer-revision", "--download-dir"}, - Env: []string{"HF_TOKEN", "HF_ENDPOINT", "HF_HOME"}, + Env: []string{ + "HF_TOKEN", "HF_ENDPOINT", "HF_HOME", + "MODELSCOPE_API_TOKEN", "MODELSCOPE_DOMAIN", "MODELSCOPE_CACHE", "VLLM_USE_MODELSCOPE", + }, }, workercore.ModelDeploymentEngineSGLang: { Args: []string{"--model-path", "--revision", "--download-dir"}, - Env: []string{"HF_TOKEN", "HF_ENDPOINT", "HF_HOME"}, + Env: []string{ + "HF_TOKEN", "HF_ENDPOINT", "HF_HOME", + "MODELSCOPE_API_TOKEN", "MODELSCOPE_DOMAIN", "MODELSCOPE_CACHE", "SGLANG_USE_MODELSCOPE", + }, }, } diff --git a/pkg/worker/controllers/worker/model_deployment_render.go b/pkg/worker/controllers/worker/model_deployment_render.go index ca97d4800..5cf0640cd 100644 --- a/pkg/worker/controllers/worker/model_deployment_render.go +++ b/pkg/worker/controllers/worker/model_deployment_render.go @@ -1326,7 +1326,7 @@ func mergeModelDeploymentEnv( var artifactEnv, artifactDefaulted []core.EnvVar if artifact != nil { - artifactEnv, artifactDefaulted = artifact.env(takeOver) + artifactEnv, artifactDefaulted = artifact.env(takeOver, engine) } env := make([]core.EnvVar, 0, diff --git a/pkg/worker/controllers/worker/model_store_test.go b/pkg/worker/controllers/worker/model_store_test.go index 521c49557..0356fb5eb 100644 --- a/pkg/worker/controllers/worker/model_store_test.go +++ b/pkg/worker/controllers/worker/model_store_test.go @@ -123,9 +123,12 @@ func TestNodeModelStoreAppliesTheStoreLayer(t *testing.T) { }), }, wantSpec: workercore.NodeModelStoreSpec{ - Watermarks: workercore.NodeModelStoreWatermarks{HighPercent: 85, LowPercent: 75}, - Download: workercore.NodeModelStoreDownload{Concurrency: 16}, - Hub: workercore.NodeModelStoreHub{HuggingFaceEndpoint: "https://huggingface.co"}, + Watermarks: workercore.NodeModelStoreWatermarks{HighPercent: 85, LowPercent: 75}, + Download: workercore.NodeModelStoreDownload{Concurrency: 16}, + Hub: workercore.NodeModelStoreHub{ + HuggingFaceEndpoint: "https://huggingface.co", + ModelScopeEndpoint: "https://www.modelscope.cn", + }, Store: "h100", PeerSyncEnabled: peerSyncTrue(), }, @@ -145,9 +148,12 @@ func TestNodeModelStoreAppliesTheStoreLayer(t *testing.T) { }), }, wantSpec: workercore.NodeModelStoreSpec{ - Watermarks: workercore.NodeModelStoreWatermarks{HighPercent: 84, LowPercent: 74}, - Download: workercore.NodeModelStoreDownload{Concurrency: 8}, - Hub: workercore.NodeModelStoreHub{HuggingFaceEndpoint: "https://huggingface.co"}, + Watermarks: workercore.NodeModelStoreWatermarks{HighPercent: 84, LowPercent: 74}, + Download: workercore.NodeModelStoreDownload{Concurrency: 8}, + Hub: workercore.NodeModelStoreHub{ + HuggingFaceEndpoint: "https://huggingface.co", + ModelScopeEndpoint: "https://www.modelscope.cn", + }, Store: "alpha", PeerSyncEnabled: peerSyncTrue(), }, diff --git a/pkg/worker/controllers/worker/node_model_store_test.go b/pkg/worker/controllers/worker/node_model_store_test.go index e1d0446b4..04ced1e9d 100644 --- a/pkg/worker/controllers/worker/node_model_store_test.go +++ b/pkg/worker/controllers/worker/node_model_store_test.go @@ -55,7 +55,10 @@ func defaultNodeModelStoreSpec() workercore.NodeModelStoreSpec { Watermarks: workercore.NodeModelStoreWatermarks{HighPercent: 80, LowPercent: 70}, Download: workercore.NodeModelStoreDownload{Concurrency: 8}, PeerSyncEnabled: peerSyncTrue(), - Hub: workercore.NodeModelStoreHub{HuggingFaceEndpoint: "https://huggingface.co"}, + Hub: workercore.NodeModelStoreHub{ + HuggingFaceEndpoint: "https://huggingface.co", + ModelScopeEndpoint: "https://www.modelscope.cn", + }, } } @@ -115,6 +118,7 @@ func TestNodeModelStoreReconcile(t *testing.T) { PeerSyncEnabled: peerSyncTrue(), Hub: workercore.NodeModelStoreHub{ HuggingFaceEndpoint: "http://hub.local", HTTPSProxy: "http://proxy:3128", + ModelScopeEndpoint: "https://www.modelscope.cn", }, }, }, diff --git a/pkg/worker/settings/model_store.go b/pkg/worker/settings/model_store.go index 965886d2b..13e23e9c9 100644 --- a/pkg/worker/settings/model_store.go +++ b/pkg/worker/settings/model_store.go @@ -221,6 +221,7 @@ func ModelStoreLayer(value func(setting.Setting) string) (modelstore.Layer, erro DownloadConcurrency: parse32(ModelStoreDownloadConcurrency), PeerSyncEnabled: peerSync(ModelStorePeerSync), HuggingFaceEndpoint: str(ModelArtifactHuggingFaceEndpoint), + ModelScopeEndpoint: str(ModelArtifactModelScopeEndpoint), HTTPSProxy: str(ModelArtifactHTTPSProxy), NoProxy: str(ModelArtifactNoProxy), CABundleConfigMap: str(ModelArtifactCABundle), diff --git a/pkg/worker/settings/model_store_test.go b/pkg/worker/settings/model_store_test.go index 167c0301f..5dbe50be7 100644 --- a/pkg/worker/settings/model_store_test.go +++ b/pkg/worker/settings/model_store_test.go @@ -94,6 +94,7 @@ func TestModelStoreSettingsDefaults(t *testing.T) { {ModelStoreLowWatermark, "model-store-low-watermark", "70"}, {ModelStoreDownloadConcurrency, "model-store-download-concurrency", "8"}, {ModelStoreDownloadBandwidth, "model-store-download-bandwidth", "0"}, + {ModelArtifactModelScopeEndpoint, "model-artifact-modelscope-endpoint", "https://www.modelscope.cn"}, } for _, c := range cases { t.Run(c.wantName, func(t *testing.T) { @@ -112,6 +113,7 @@ func TestModelStoreLayer(t *testing.T) { ModelStoreDownloadConcurrency.Name(): "16", ModelStoreDownloadBandwidth.Name(): "100Mi", ModelArtifactHuggingFaceEndpoint.Name(): "http://hub.local", + ModelArtifactModelScopeEndpoint.Name(): "https://ms.example", ModelArtifactHTTPSProxy.Name(): "http://proxy:3128", ModelArtifactNoProxy.Name(): "internal", ModelArtifactCABundle.Name(): "hub-ca", @@ -125,6 +127,7 @@ func TestModelStoreLayer(t *testing.T) { assert.Equal(t, int32(16), *l.DownloadConcurrency) assert.Equal(t, int64(100<<20), *l.DownloadBytesPerSecond) assert.Equal(t, "http://hub.local", *l.HuggingFaceEndpoint) + assert.Equal(t, "https://ms.example", *l.ModelScopeEndpoint) assert.Equal(t, "http://proxy:3128", *l.HTTPSProxy) assert.Equal(t, "internal", *l.NoProxy) assert.Equal(t, "hub-ca", *l.CABundleConfigMap) @@ -199,6 +202,8 @@ func TestAdmissionReadsTheStore(t *testing.T) { }, {name: "an endpoint without a host is refused", setting: ModelArtifactHuggingFaceEndpoint, value: "https://", wantErr: "with a host"}, {name: "an endpoint with a host is admitted", setting: ModelArtifactHuggingFaceEndpoint, value: "https://hub.example"}, + {name: "a modelscope endpoint without a host is refused", setting: ModelArtifactModelScopeEndpoint, value: "https://", wantErr: "with a host"}, + {name: "a modelscope endpoint with a host is admitted", setting: ModelArtifactModelScopeEndpoint, value: "https://ms.example"}, } _ = ModelStoreLowWatermark.ShouldValueFromRemote(ctx) // caches 60 store(modelStoreLowWatermarkName, "85") diff --git a/pkg/worker/settings/value.go b/pkg/worker/settings/value.go index 54526c6ae..c51764220 100644 --- a/pkg/worker/settings/value.go +++ b/pkg/worker/settings/value.go @@ -365,6 +365,21 @@ var ( func(_ context.Context, _, newVal string) error { return modelstore.ValidateEndpoint(newVal) }, ) + // ModelArtifactModelScopeEndpoint is the ModelScope hub the ModelArtifact controller resolves + // and revalidates against and the node plugin downloads from. It is administrator + // configuration, admitted as the Hugging Face endpoint is, and it rides every node's + // configuration like the Hugging Face one does. Changing it does not re-resolve an artifact + // that already resolved. + ModelArtifactModelScopeEndpoint = settings.NewEditable( + "model-artifact-modelscope-endpoint", + "Indicates the ModelScope endpoint ModelArtifacts resolve against and the node plugin downloads from. "+ + "Changing it does not re-resolve an artifact that already resolved.", + setting.InitializeFromEnv("https://www.modelscope.cn"), + setting.DisallowBlank(), + setting.AllowUrlWithSchema("https", "http"), + func(_ context.Context, _, newVal string) error { return modelstore.ValidateEndpoint(newVal) }, + ) + // ModelArtifactHTTPSProxy is the proxy the ModelArtifact controller reaches the Hub through, and // the HTTPS_PROXY an engine downloading the weights itself is given, where a role's own value // wins. Blank leaves the worker's own environment in effect and renders nothing. It carries no @@ -424,7 +439,7 @@ var ( ModelPrefetchWarmupImage = settings.NewEditable( "model-prefetch-warmup-image", "Indicates the image a ModelPrefetch's warm-up Pod runs on each target node.", - setting.InitializeFromEnv("python:3.12-alpine"), + setting.InitializeFromEnv("gpustack/mirrored-python:3.12-alpine"), setting.AllowContainerImageReference(), ) diff --git a/pkg/worker/webhooks/worker/model_artifact.go b/pkg/worker/webhooks/worker/model_artifact.go index 17b2561da..5c1a7ccb2 100644 --- a/pkg/worker/webhooks/worker/model_artifact.go +++ b/pkg/worker/webhooks/worker/model_artifact.go @@ -59,18 +59,25 @@ var ( // defaulting reads nothing else. func (*ModelArtifactWebhook) ReceiveDeletionUpdate() {} -// ModelArtifactDefaultRevision is the revision a hub source names when it names none, the default -// branch of a Hugging Face repository. +// ModelArtifactDefaultRevision is the revision a Hugging Face source names when it names none, +// the default branch of a Hugging Face repository. const ModelArtifactDefaultRevision = "main" -// Default fills an unset hub revision with "main", on update as well as on creation: a full -// replace that omits the revision would otherwise read as a change from "main" to nothing and be -// refused as a spec edit. +// ModelArtifactDefaultModelScopeRevision is the revision a ModelScope source names when it names +// none, the default branch of a ModelScope repository. +const ModelArtifactDefaultModelScopeRevision = "master" + +// Default fills an unset hub revision with the hub's default branch, on update as well as on +// creation: a full replace that omits the revision would otherwise read as a change to nothing +// and be refused as a spec edit. func (*ModelArtifactWebhook) Default(_ context.Context, obj runtime.Object) error { ma := obj.(*workercore.ModelArtifact) if hub := ma.Spec.Source.HuggingFace; hub != nil && hub.Revision == "" { hub.Revision = ModelArtifactDefaultRevision } + if hub := ma.Spec.Source.ModelScope; hub != nil && hub.Revision == "" { + hub.Revision = ModelArtifactDefaultModelScopeRevision + } return nil } @@ -104,12 +111,6 @@ func (*ModelArtifactWebhook) ValidateDelete(_ context.Context, _ runtime.Object) const modelArtifactIdentityMessage = "a ModelArtifact is an identity, so its spec cannot change after " + "creation: a different source or revision is a different artifact, created rather than edited" -// modelArtifactModelScopeMessage is why the reserved member is refused, and what opening it needs. -const modelArtifactModelScopeMessage = "ModelScope is not accepted in this version. Opening it needs " + - "branch resolution cross-checked against git, a file listing that re-lists per directory where " + - "the API silently truncates, errors classified by the envelope code, and an engine runner whose " + - "ModelScope SDK accepts a commit as the revision" - // huggingFaceRepositoryPartPattern is one part of a Hugging Face repository id, as the Hub names // them: letters, digits, "-", "_" and ".", not starting or ending with "-" or ".", at most 96. var huggingFaceRepositoryPartPattern = regexp.MustCompile(`^[A-Za-z0-9_]([A-Za-z0-9_.-]{0,94}[A-Za-z0-9_])?$`) @@ -142,15 +143,15 @@ func validateModelArtifact(ma, old *workercore.ModelArtifact) field.ErrorList { } if len(members) != 1 { return field.ErrorList{field.Invalid(sourcePath, members, - "exactly one of huggingFace, persistentVolumeClaim or image is required")} + "exactly one of huggingFace, modelScope, persistentVolumeClaim or image is required")} } var errs field.ErrorList switch { - case source.ModelScope != nil: - return field.ErrorList{field.Forbidden(sourcePath.Child("modelScope"), modelArtifactModelScopeMessage)} case source.HuggingFace != nil: - errs = validateModelArtifactHuggingFace(source.HuggingFace, sourcePath.Child("huggingFace")) + errs = validateModelArtifactHub(source.HuggingFace, sourcePath.Child("huggingFace")) + case source.ModelScope != nil: + errs = validateModelArtifactHub(source.ModelScope, sourcePath.Child("modelScope")) case source.Image != nil: errs = validateModelArtifactImage(source.Image, sourcePath.Child("image")) errs = append(errs, validateModelArtifactImageVolume(sourcePath.Child("image"))...) @@ -158,7 +159,8 @@ func validateModelArtifact(ma, old *workercore.ModelArtifact) field.ErrorList { errs = validateModelArtifactClaim(source.PersistentVolumeClaim, sourcePath.Child("persistentVolumeClaim")) } - return append(errs, validateModelArtifactPatterns(&ma.Spec, specPath, source.HuggingFace != nil, source.Image != nil)...) + return append(errs, validateModelArtifactPatterns(&ma.Spec, specPath, + source.HuggingFace != nil || source.ModelScope != nil, source.Image != nil)...) } // modelArtifactMaxPatterns and modelArtifactMaxPatternLength bound a list of patterns and each of @@ -208,7 +210,11 @@ func validateModelArtifactPatterns(spec *workercore.ModelArtifactSpec, specPath return errs } -func validateModelArtifactHuggingFace(hub *workercore.ModelArtifactHubSource, path *field.Path) field.ErrorList { +// validateModelArtifactHub validates a hub source's shape — the rules the two hubs share: the +// repository id and the revision's characters. ModelScope ids and Hugging Face ids are written +// the same way, so one validator serves both; what resolves a revision differs and lives in the +// client, not here. +func validateModelArtifactHub(hub *workercore.ModelArtifactHubSource, path *field.Path) field.ErrorList { var errs field.ErrorList parts := strings.Split(hub.Repository, "/") diff --git a/pkg/worker/webhooks/worker/model_artifact_test.go b/pkg/worker/webhooks/worker/model_artifact_test.go index 4e90129ca..c40945072 100644 --- a/pkg/worker/webhooks/worker/model_artifact_test.go +++ b/pkg/worker/webhooks/worker/model_artifact_test.go @@ -42,6 +42,15 @@ func newTestImageArtifact(reference string) *workercore.ModelArtifact { } } +func newTestModelScopeArtifact(repository, revision string) *workercore.ModelArtifact { + return &workercore.ModelArtifact{ + ObjectMeta: meta.ObjectMeta{Namespace: "team-a", Name: "qwen"}, + Spec: workercore.ModelArtifactSpec{Source: workercore.ModelArtifactSource{ + ModelScope: &workercore.ModelArtifactHubSource{Repository: repository, Revision: revision}, + }}, + } +} + // TestModelArtifactWebhookImageSource covers the image member's shape rules and its capability // gate, both directions, through the swappable version seam: the snapshot's Configure ignores // later calls, so a test cannot re-point the snapshot itself. @@ -131,11 +140,17 @@ func TestModelArtifactWebhookDefault(t *testing.T) { }{ {name: "an unset revision becomes main", in: newTestHubArtifact("Qwen/Qwen2.5-7B-Instruct", ""), want: "main"}, {name: "a stated revision is kept", in: newTestHubArtifact("Qwen/Qwen2.5-7B-Instruct", "v1.0"), want: "v1.0"}, + {name: "a ModelScope revision becomes master", in: newTestModelScopeArtifact("Qwen/Qwen2.5-7B-Instruct", ""), want: "master"}, + {name: "a stated ModelScope revision is kept", in: newTestModelScopeArtifact("Qwen/Qwen2.5-7B-Instruct", "v1.0"), want: "v1.0"}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { require.NoError(t, new(ModelArtifactWebhook).Default(context.Background(), c.in)) - assert.Equal(t, c.want, c.in.Spec.Source.HuggingFace.Revision) + if c.in.Spec.Source.HuggingFace != nil { + assert.Equal(t, c.want, c.in.Spec.Source.HuggingFace.Revision) + } else { + assert.Equal(t, c.want, c.in.Spec.Source.ModelScope.Revision) + } }) } } @@ -150,6 +165,9 @@ func TestModelArtifactWebhookValidateCreate(t *testing.T) { {name: "a bare canonical name", in: newTestHubArtifact("gpt2", "main")}, {name: "a commit", in: newTestHubArtifact("owner/repo", strings.Repeat("a", 40))}, {name: "a pull-request ref", in: newTestHubArtifact("owner/repo", "refs/pr/1")}, + {name: "a ModelScope repository", in: newTestModelScopeArtifact("qwen/Qwen2.5-7B-Instruct", "master")}, + {name: "a ModelScope bare name", in: newTestModelScopeArtifact("qwen", "master")}, + {name: "a ModelScope commit", in: newTestModelScopeArtifact("qwen/Qwen2.5-7B-Instruct", strings.Repeat("a", 40))}, {name: "a claim at its root", in: newTestClaimArtifact("models", "")}, {name: "a claim directory", in: newTestClaimArtifact("models", "qwen/7b")}, { @@ -167,11 +185,13 @@ func TestModelArtifactWebhookValidateCreate(t *testing.T) { wantField: "spec.source", }, { - name: "ModelScope", - in: &workercore.ModelArtifact{Spec: workercore.ModelArtifactSpec{Source: workercore.ModelArtifactSource{ - ModelScope: &workercore.ModelArtifactHubSource{Repository: "qwen/Qwen2.5-7B-Instruct", Revision: "master"}, - }}}, - wantField: "spec.source.modelScope", + name: "a hub source beside a hub source", + in: func() *workercore.ModelArtifact { + ma := newTestHubArtifact("owner/repo", "main") + ma.Spec.Source.ModelScope = &workercore.ModelArtifactHubSource{Repository: "qwen/Qwen2.5-7B-Instruct", Revision: "master"} + return ma + }(), + wantField: "spec.source", }, {name: "an empty repository", in: newTestHubArtifact("", "main"), wantField: "spec.source.huggingFace.repository"}, {name: "three parts", in: newTestHubArtifact("a/b/c", "main"), wantField: "spec.source.huggingFace.repository"}, @@ -186,11 +206,18 @@ func TestModelArtifactWebhookValidateCreate(t *testing.T) { {name: "an empty revision", in: newTestHubArtifact("owner/repo", ""), wantField: "spec.source.huggingFace.revision"}, {name: "whitespace in the revision", in: newTestHubArtifact("owner/repo", "ma in"), wantField: "spec.source.huggingFace.revision"}, {name: "a control character in the revision", in: newTestHubArtifact("owner/repo", "ma\x00in"), wantField: "spec.source.huggingFace.revision"}, + {name: "a ModelScope empty repository", in: newTestModelScopeArtifact("", "master"), wantField: "spec.source.modelScope.repository"}, + {name: "a ModelScope three parts", in: newTestModelScopeArtifact("a/b/c", "master"), wantField: "spec.source.modelScope.repository"}, + {name: "a ModelScope URL", in: newTestModelScopeArtifact("https://modelscope.cn/qwen", "master"), wantField: "spec.source.modelScope.repository"}, + {name: "a ModelScope empty revision", in: newTestModelScopeArtifact("qwen/Qwen2.5-7B-Instruct", ""), wantField: "spec.source.modelScope.revision"}, + {name: "a ModelScope whitespace revision", in: newTestModelScopeArtifact("qwen/Qwen2.5-7B-Instruct", "ma in"), wantField: "spec.source.modelScope.revision"}, {name: "an empty claim", in: newTestClaimArtifact("", ""), wantField: "spec.source.persistentVolumeClaim.claimName"}, {name: "an absolute path", in: newTestClaimArtifact("models", "/qwen"), wantField: "spec.source.persistentVolumeClaim.path"}, {name: "a dot-dot element", in: newTestClaimArtifact("models", "qwen/../other"), wantField: "spec.source.persistentVolumeClaim.path"}, {name: "patterns on a hub source", in: withPatterns(newTestHubArtifact("owner/repo", "main"), []string{"*.safetensors", "*.json"}, []string{"original/"})}, + {name: "patterns on a ModelScope source", in: withPatterns(newTestModelScopeArtifact("qwen/Qwen2.5-7B-Instruct", "master"), + []string{"*.safetensors"}, []string{"original/"})}, { name: "allow patterns on a claim", in: withPatterns(newTestClaimArtifact("models", ""), []string{"*.json"}, nil), wantField: "spec.allowPatterns", @@ -221,6 +248,17 @@ func TestModelArtifactWebhookValidateCreate(t *testing.T) { assertInvalidField(t, err, c.wantField) }) } + + t.Run("the union refusal names every accepted member", func(t *testing.T) { + ma := newTestHubArtifact("owner/repo", "main") + ma.Spec.Source.PersistentVolumeClaim = &workercore.ModelArtifactPersistentVolumeClaimSource{ClaimName: "c"} + + _, err := new(ModelArtifactWebhook).ValidateCreate(context.Background(), ma) + require.Error(t, err) + status, ok := err.(kerrors.APIStatus) + require.True(t, ok) + assert.Contains(t, status.Status().Details.Causes[0].Message, "modelScope") + }) } func TestModelArtifactWebhookValidateUpdate(t *testing.T) { diff --git a/pkg/worker/webhooks/worker/model_deployment_artifact_test.go b/pkg/worker/webhooks/worker/model_deployment_artifact_test.go index 25a16bf5a..2ca249b1b 100644 --- a/pkg/worker/webhooks/worker/model_deployment_artifact_test.go +++ b/pkg/worker/webhooks/worker/model_deployment_artifact_test.go @@ -115,6 +115,25 @@ func TestValidateModelDeploymentArtifactOwnedKeys(t *testing.T) { }, wantField: []string{"spec.roles[0].env[0]", "spec.roles[0].env[2]"}, }, + { + name: "the ModelScope environment", engine: workercore.ModelDeploymentEngineVLLM, + env: []workercore.ModelDeploymentEnvVar{ + {Name: "MODELSCOPE_API_TOKEN", Value: "x"}, {Name: "MODELSCOPE_DOMAIN", Value: "ms"}, + {Name: "MODELSCOPE_CACHE", Value: "/c"}, {Name: "VLLM_USE_MODELSCOPE", Value: "true"}, + }, + wantField: []string{ + "spec.roles[0].env[0]", "spec.roles[0].env[1]", "spec.roles[0].env[2]", "spec.roles[0].env[3]", + }, + }, + { + name: "SGLang's own switch", engine: workercore.ModelDeploymentEngineSGLang, + env: []workercore.ModelDeploymentEnvVar{{Name: "SGLANG_USE_MODELSCOPE", Value: "true"}}, + wantField: []string{"spec.roles[0].env[0]"}, + }, + { + name: "vLLM ignores SGLang's switch", engine: workercore.ModelDeploymentEngineVLLM, + env: []workercore.ModelDeploymentEnvVar{{Name: "SGLANG_USE_MODELSCOPE", Value: "true"}}, + }, { name: "without an artifact a revision is the user's", engine: workercore.ModelDeploymentEngineVLLM, noRef: true, args: []string{"--revision", "v2"}, env: []workercore.ModelDeploymentEnvVar{{Name: "HF_TOKEN", Value: "x"}}, diff --git a/specs/2026-09-29-model-artifact-modelscope.md b/specs/2026-09-29-model-artifact-modelscope.md new file mode 100644 index 000000000..17e415573 --- /dev/null +++ b/specs/2026-09-29-model-artifact-modelscope.md @@ -0,0 +1,579 @@ +# Spec: ModelScope ModelArtifact Source + +Status: Shipped +Type: Feature + +## Summary + +Open the ModelScope source of `ModelArtifact` end to end. The API has carried a reserved +`modelScope` member since the Hugging Face spec; admission refuses it, with the message naming +exactly what opening it needs. This spec implements those needs — branch resolution cross-checked +against git, a listing that recovers from the API's silent truncation at 3000 entries, errors +classified by the envelope code, and an engine runner whose ModelScope SDK accepts a commit — and +turns the member on: admission accepts it, the controller resolves it to an immutable commit and a +manifest digest, the node plugin downloads and verifies it, and the documentation describes it. + +## Motivation + +### Goals + +ModelScope is one of the two public model hubs Chinese users reach for first; an artifact API that +only speaks Hugging Face leaves half the ecosystem's weights behind a manual copy. The goal is +parity of behavior with the Hugging Face source, with the three ModelScope-specific failure modes +(PoC-D, PoC-C) handled so that none of them can silently deliver wrong content: + +1. **Branch resolution cross-checked.** ModelScope's + `GET /api/v1/models//commits?Ref=&PageSize=1` is undocumented, and a misspelled + parameter silently returns master. Resolution of a non-commit revision must ask git + (`git ls-remote` over the HTTP smart protocol, `https://www.modelscope.cn/.git`) and refuse + the resolution when the two answers disagree. +2. **The 3000-entry silent truncation.** `repo/files?Revision=` truncates silently at 3000 + entries and ignores every pagination parameter. The listing must re-list per directory with + `Root=` whenever a listing comes back with exactly 3000 entries, and must refuse a + directory with 3000 or more direct children, where no decomposition can recover the tree. +3. **An engine that pins the commit.** The vLLM runner's bundled ModelScope SDK must accept a + commit as the revision. Engine delivery renders for every engine alike; the documentation + carries the runner SDK ≥ 1.39.1 floor and which measured runners meet it (option B, confirmed + at the spec gate, below), because the same formula meets the floor on one hardware pool and + misses it on another, and a user-supplied runner image is always one field away. + +Plus, free of new mechanism because everything downstream keys on the manifest digest: the +peer-sync, progress-aggregation, prefetch-and-pinning and placement-preference behaviors of the +earlier seats work for a ModelScope source the moment its manifest digest exists. The spec adds one +acceptance case that proves it for peer sync, and states the others. + +### Non-Goals + +- An `expectedDigest` on the spec, or any pre-resolution digest check. (A separate topic.) +- Cross-hub content deduplication. The same files resolve to different digests on the two hubs + (non-LFS files: `gitsha1` on Hugging Face, `sha256` on ModelScope); the manifest format v1 is + unchanged, and digests stay incomparable across sources. +- Any change to the manifest format v1 (`gpustack-manifest v1`), the modelStore key layout, or the + CSI volume contract. +- Supporting ModelScope datasets or private endpoints beyond the Setting below. + +## Proposal + +### Admission + +- `validateModelArtifact` drops the `modelScope` refusal and validates the member with the same + repository and revision rules as Hugging Face (the `ModelArtifactHubSource` shape is shared). +- Defaulting fills an unset `modelScope.revision` with `master` (ModelScope's default branch), the + same defaulting Hugging Face gets with `main`, on update as well as creation. +- Patterns (`allowPatterns` / `ignorePatterns`) become legal on a ModelScope source: they select + files of a hub source, whatever the hub. + +### Resolution (controller) + +A ModelScope source resolves once, like Hugging Face, with the namespace's token: + +1. **Revision.** A 40-character commit is taken as is. Anything else goes to + `commits?Ref=&PageSize=1`; a `Commit: null` answer is `RevisionNotFound`. The answer is + cross-checked against the repository's git endpoint itself — the smart protocol's ref + advertisement (`https://www.modelscope.cn/.git/info/refs?service=git-upload-pack`), + answered by the operator in process, with no `git` binary in the image and no subprocess. The + commit the API named must equal the commit the advertisement names for the same ref (the + `HEAD` line for a branch, the peeled entry for an annotated tag), and a disagreement fails the + resolution as `SourceUnavailable` with a message that says the hub's index and its git + disagree. The advertisement request names itself a git client, the protocol it speaks — the + endpoint answers a git user agent and refuses others with 421 (measured). For a private + repository it authenticates as the user `oauth2` with the namespace's token as the password — + an HTTP header, never an argument. +2. **Listing.** The manifest input is `repo/files?Revision=&Recursive=true`. A listing of + exactly 3000 entries is a truncation: each direct child directory is re-listed with + `Root=`, recursively, and only a subtree that truncates is descended into further. A + directory whose direct children reach 3000 fails the resolution as `SourceUnavailable` — the + API cannot enumerate it, and a partial manifest would be a silent wrong answer. +3. **Per-file digest.** Every entry must carry `Sha256`; a missing one fails the resolution. Each + file's digest line is `sha256:` — ModelScope gives no non-LFS alternative, so a + ModelScope manifest has no `gitsha1` lines. Size comes from `Size`. `Type: tree` entries are + directories; any other unexpected type fails. +4. **Canonicalization, filtering, digest.** Identical to the Hugging Face path: the same + `Filter`, the same `gpustack-manifest v1` canonical bytes, the same digest. The digest lands in + `status.resolved.manifestDigest` and drives node delivery, peer sync, progress, prefetch and + placement preference unchanged. +5. **Reason mapping.** Every access failure ModelScope reports is a 404, so the envelope `Code` + classifies, with the HTTP status as the fallback: + + | Reading | Reason | + | --- | --- | + | `commits?Ref=` answers 200 with `Commit: null` | `RevisionNotFound` | + | `repo/files` 404 with `Code` 10990101004 (no file tree at the revision) | `RevisionNotFound` | + | 404 with `Code` 10010205001 (not found) or 10010200001 (no access, private or gated) | `AccessDenied`; the message says "does not exist or is not accessible" | + | any other 401 / 403 / 404 | `AccessDenied` | + | 5xx, network error, undecodable body; a directory with 3000 direct children; a listing that stays truncated | `SourceUnavailable` | + + Measured limits, stated here and in the docs: ModelScope has no distinct 401 or 403, and + "no access" (10010200001) covers private-without-token, gated-without-grant and + valid-token-without-grant alike. A mistyped token is silently ignored on a public repository, + exactly as on Hugging Face. **The private-repository cell "a valid token without access" is + UNADJUDICATED**: distinguishing it needs a second ModelScope account, which this environment + does not have (PoC-D). It maps to `AccessDenied` with the shared message; a test asserts only + that mapping, not a distinct message. + +6. **Revalidation.** One `HEAD` of + `/api/v1/models//repo?Revision=&FilePath=` — measured: 200 for an + accessible file (LFS and not, no redirect on HEAD), 404 for a missing commit, a missing path, a + missing repository and a gated repository without a grant, with no envelope on HEAD. 200 + confirms; 404 is an `AccessDenied` refusal and goes through the same confirm-then-revoke + staircase as Hugging Face (first refusal sets `Degraded`, the same refusal after the delay + sets `Resolved=False`). The first file comes from one root listing, as on Hugging Face. + +The controller branch mirrors `reconcileHuggingFace`: same pacing entry, same Secret watch, same +`Resolved` / `Degraded` conditions, same reasons. The mistyped-token warning is aligned too: a +`GET /openapi/v1/users/me` (200/401, measured in PoC-D) plays the part of `whoami-v2`, emitting +the same Warning event when the hub rejects a new token outright, conditions unchanged. + +### Node delivery (plugin) + +The plugin's downloader is already source-shaped (URL, digest, size, byte ranges, checkpoints); +the ModelScope source contributes a file URL builder: + +- URL: `/api/v1/models//repo?Revision=&FilePath=`; the answer is 200 for a + small file or a 302 to a content-addressed CDN path for LFS (signed for a private repository), + and the CDN honors byte ranges (PoC-D). The redirect is followed with the credential dropped on + leaving the host, as the downloader already does. +- Every file verifies against its manifest `sha256` while streaming; resume replays the digest + from the checkpoint. The manifest the node recomputes for comparison uses the same ModelScope + listing, so a node accepts only bytes the hub itself hashed at the pinned commit. + +**How the node knows the hub.** The plugin reads the full `ModelArtifact` from its informer cache +(`driver.artifact`), so the source kind comes from `ma.Spec.Source` — the volume attributes stay +as they are. The hub clients are built from the node's effective configuration +(`NodeModelStoreHub`), which carries one endpoint today; it gains an optional +`modelScopeEndpoint` (the one API change, below), and the materializer picks the client by the +source's kind. A `modelScope` artifact on a node whose configuration predates the field is +refused with a message that names the worker upgrade, never resolved against the wrong hub. + +### Engine delivery + +The vLLM and SGLang runners download through their bundled ModelScope SDKs. The measured switches +(PoC-C): vLLM `VLLM_USE_MODELSCOPE=True`, SGLang `SGLANG_USE_MODELSCOPE=true`, shared +`MODELSCOPE_CACHE=/var/lib/gpustack/model-cache`, argv unchanged +(` --revision `). A token reaches the SDK as `MODELSCOPE_API_TOKEN` (an env +referencing the artifact's Secret, owned key, value never in the Pod spec). + +**The engine-runner question (D5 condition 3).** PoC-C measured the vLLM 0.29.0 runner's SDK +1.37.1 refusing a commit (`NotExistError`) and SDK 1.39.1 accepting it. The spike (2026-09-29; +readings in Runner SDK facts below, full log in the task directory's +`spec-10/raw/spike-runner-sdk.md`) measured the newest published builds directly: the CUDA vLLM +lines still carry 1.37.1 (the 2026-09-24 rebuild of `cuda12.9-vllm0.29.0` / +`cuda13.0-vllm0.29.0`, runner commit `80034d19`, hit the dependency-layer cache), the Ascend +vLLM line meets the floor, and vLLM 0.30.0 runners are built upstream (2026-09-23 CI) but not +released — no Docker Hub tag, no `runner.py.json` entry. The runner family is `gpustack/runner`; +`pack/cuda/Dockerfile` installs `modelscope` unpinned, so a runner's SDK is whatever its +dependency layer resolved at build time. Two facts follow: no "vLLM version → SDK version" +mapping exists (the SDK floats with the build), and the same formula meets the floor on one +hardware pool and misses it on another. + +**Coordinated direction — option B, confirmed at the spec gate (2026-09-29):** render every engine +path normally and document the floor. +Engine delivery renders `MODELSCOPE_*` for vLLM and SGLang alike, whatever runner the role +resolves to; the documentation carries the SDK ≥ 1.39.1 floor, states which measured runners meet +it today (the Ascend `cann9.1-*-vllm0.23.0` line does; the current CUDA `vllm0.25.1` / `vllm0.29.0` +lines do not), and tells users whose runner bundles an older SDK to name a runner image of their +own that bundles ≥ 1.39.1 — the same freedom every role already has to name its image. Admission +gains no ModelScope-specific refusal: the operator cannot see the runner's contents, a refusal by +combination would wrongly reject the Ascend users the floor already satisfies, and an old runner's +`NotExistError` is a loud, named engine error, not silent wrong content. + +### Free behaviors, each with its acceptance + +Everything downstream of the digest works for a ModelScope artifact with no new code. Peer sync +gets its own acceptance case below; the rest are covered by stating the digest invariant: a +ModelScope artifact and a Hugging Face artifact never share a digest, and every digest-keyed +behavior (progress aggregation, prefetch, placement preference) is proven digest-keyed already. + +### Documentation + +- `docs/model-store/artifact.md` (the page the HANDOFF's `docs/reference/model-artifact.md` + became): the "refused" bullet becomes an accepted source — the resource example, the delivery + table, the resolution section (ModelScope rows in the reason table, the cross-check, the + truncation recovery), the digest section (all-`sha256` manifests; cross-hub digests stay + incomparable), the engine-delivery env table, the requirements list (the runner SDK floor, per + the coordination). +- `docs/settings.md`: `model-artifact-modelscope-endpoint` joins the ModelArtifact table. +- `docs/README.md` and the docs skill routing follow the docs skill; no new page is expected (the + artifact page owns the source kind), decided finally by the docs skill's routing. + +### Configuration + +| Setting | Env | Default | Meaning | +| --- | --- | --- | --- | +| `model-artifact-modelscope-endpoint` | `GPUSTACK_MODEL_ARTIFACT_MODELSCOPE_ENDPOINT` | `https://www.modelscope.cn` | The ModelScope endpoint the controller resolves and revalidates against and the node plugin downloads from. Engine Pods receive its host as `MODELSCOPE_DOMAIN` (the SDK takes a bare host); the node plugin receives the URL through `NodeModelStoreHub.modelScopeEndpoint`. Admission as for the Hugging Face endpoint: a URL with a schema and a host. | + +The spike answered the endpoint question against the SDK inside the runner image (1.37.1): the +SDK reads `MODELSCOPE_DOMAIN`, whose value is a **bare host** (`www.modelscope.cn`) that the SDK +itself prefixes with `https://` — not a URL like `HF_ENDPOINT`. The renderer therefore derives +the host from the Setting's URL for engine Pods, and the Setting keeps the full-URL shape the +Hugging Face endpoint uses. + +### Runner SDK facts + +The readings the engine-runner direction stands on, all taken 2026-09-29 by this seat; the full +evidence log is the task directory's `spec-10/raw/spike-runner-sdk.md`. + +- **What the operator renders.** A vLLM or SGLang deployment resolves to + `gpustack/runner:[-]-` — e.g. + `cuda12.9-vllm0.29.0`, `cann9.1-910b-vllm0.23.0`, `cuda13.0-sglang0.5.18` (`model_deployment_image.go`). + The operator sets no default: the engine version is whatever the deployment declares, so "the + current runner" is a family, not one tag. +- **The readings and how they were taken.** + + | Runner image:tag | ModelScope SDK inside | Meets ≥ 1.39.1 | How the reading was taken | + | --- | --- | --- | --- | + | `gpustack/runner:cuda12.9-vllm0.29.0` — the 2026-09-24 build, the newest published CUDA vLLM line | 1.37.1 | **no** | [跑] pulled `--platform linux/arm64` (image `ff0298cffc64`, digest-matched against Docker Hub's current tag), `pip show modelscope` in the container | + | `gpustack/runner:cuda12.9-vllm0.25.1` — an older build (digest `3f15ce95…`, no longer the Hub's tag) | 1.37.1 | **no** | [跑] same, on the image already local | + | `gpustack/runner:cann9.1-910b-vllm0.23.0` — the Ascend line this formula synthesizes | 1.39.1 (vllm-ascend 0.23.0) | **yes** | [跑] on the coordinator-named amd64 build machine, `pip show modelscope` in the container | + | SGLang 0.5.18 runners (e.g. `cuda13.0-sglang0.5.18`) | 1.39.1 | **yes** | [读] PoC-C's first-hand runtime reading (GPU cluster); this seat's re-check of the cann variant produced no new reading (an arm64 image whose manifest has since left the registry) — PoC-C stands | + +- **Verdict.** No released CUDA vLLM runner meets the SDK floor today; the Ascend vLLM line and + SGLang 0.5.18 do. The Dockerfile pins no SDK version, so the number is whatever a build's + dependency layer resolved — the 2026-09-24 rebuild still carries 1.37.1 (cache hit), vLLM 0.30.0 + runners are built upstream but unreleased, and therefore no "vLLM version → SDK version" + mapping exists for the operator to consult. + +### Version floors + +Unchanged: Kubernetes 1.29 for the feature path, 1.23 install. ModelScope adds no Kubernetes +capability requirement. The engine path adds a ModelScope SDK floor (≥ 1.39.1) on the runner +image, documented per the coordinated direction, not enforced by the operator (the operator does +not read the runner image). + +## User Stories + +### Story 1 + +As a platform operator on ModelScope-hosted weights, I create a `ModelArtifact` whose source is +`modelScope: {repository: qwen/Qwen2.5-0.5B-Instruct}` and see it resolve to a commit and a +manifest digest, so that a `ModelDeployment` can reference it exactly like a Hugging Face one. + +### Story 2 + +As a user, I reference that artifact from a `ModelDeployment` and the weights arrive on a cold +node — downloaded from ModelScope, verified per file, published read-only — without the engine +ever seeing a partial tree. + +### Story 3 + +As a user, I get `Resolved=False` with a truthful reason instead of silently wrong content: a +branch the hub's API and its git disagree about, a directory the API cannot enumerate, or a +repository I have no access to each stops the artifact with a message. + +## Core Features & Acceptance Criteria + +- **AC1 — Master resolution.** Creating `modelScope: {repository: qwen/Qwen2.5-0.5B-Instruct}` + (no revision) defaults the revision to `master`, resolves it to the 40-character commit the + `commits` endpoint and `git ls-remote` agree on, and writes a manifest of only `sha256:` lines + with `Resolved=True`. Moving the branch afterwards changes nothing (resolution is once). +- **AC2 — Cross-check guard, red then green.** A fake transport whose `commits` answer disagrees + with the `ls-remote` answer fails the resolution (`SourceUnavailable`) with the disagreement + message. The test is shown red by neutering the cross-check (accepting the API's answer alone), + then green again. +- **AC3 — Truncation guard, red then green.** A fake server listing exactly 3000 entries at the + root and 3000 in one subdirectory: the root truncation is recovered by per-directory re-listing + (the manifest comes out complete), and a directory with 3000 direct children fails the + resolution (`SourceUnavailable`). Shown red by short-circuiting the descent, then green. +- **AC4 — Cold-node materialization.** Node delivery from ModelScope: the plugin downloads at the + pinned commit, hashes while streaming, publishes, and a Pod mounts the files byte-verified. At + least one case resumes a download from a checkpoint (transport cut mid-file, restart, the + received bytes are not fetched again). +- **AC5 — Engine delivery pins the commit.** vLLM and SGLang each leave one piece of evidence + that the served weights and tokenizer are the pinned commit (snapshot path or file hash, as in + PoC-C), on runners meeting the documented SDK floor — SGLang's 0.5.18 runner, and for vLLM a + runner with SDK ≥ 1.39.1 (today: the Ascend `cann9.1-*-vllm0.23.0` line, or a user-named + image). The rendering itself is engine-agnostic: a role on a runner below the floor renders the + same way and fails in the engine with the SDK's named `NotExistError`, which the documentation + explains. Whether the vLLM half needs a GPU node is decided in my-plan with the reason stated. +- **AC6 — Peer sync, free.** Two nodes, ModelScope source: the second node pulls the artifact + from the first over peer sync and the hub sees zero bytes for it (the S5 acceptance shape, + re-run with a ModelScope artifact). +- **AC7 — Reason mapping.** Every row of the reason table has a test: `RevisionNotFound` (commit + `null`; code 10990101004), `AccessDenied` (10010205001, 10010200001, other 404), private + repository without a token, `SourceUnavailable` (5xx, transport). The unadjudicated cell is + asserted only as its shared `AccessDenied` mapping, labeled unadjudicated in the test. +- **AC8 — Admission.** A `modelScope` source is accepted; the union rule still refuses two + members; `revision` defaults to `master`; patterns are legal on it and refused on claim and + image sources unchanged; the refusal message for the reserved member is gone. + +## Notes / Constraints / Caveats + +- **One additive API change.** The `ModelArtifact` types are unchanged (`ModelArtifactHubSource` + is shared, the `modelScope` member already exists), but the plugin's effective configuration + needs the second endpoint: `NodeModelStoreHub` gains an optional `modelScopeEndpoint`, written + by the worker from the Setting and read by the plugin. It is an additive, optional field — old + plugins ignore it, old workers leave it empty and a `modelScope` artifact is then refused at + the node with the upgrade named. `make generate` runs for it, in the private gen tree per + STANDARDS §14. (This corrects the draft's "no API change" reading; found while planning the + plugin's hub selection, and the gen verification rides this task.) +- The webhook's reserved-member refusal constant and its test go away; the status-side + `UnsupportedSource` branch in the controller stays for objects stored before this version. +- The git cross-check runs **in process** over the smart protocol, so the worker image needs no + `git` binary and the token never leaves an HTTP header. The advertisement's shape was verified + against the live endpoint (2026-09-29): the service line, the NUL-delimited capabilities of the + first ref and the peeled entry all parse; a shape the parser cannot read fails the resolution + as `SourceUnavailable`, never resolves on less evidence. +- A resolution that needs ls-remote adds one subprocess to the reconcile path; the same + `modelArtifactResolveTimeout` bounds it, and a git failure is `SourceUnavailable`, never a + panic. +- The ModelScope SDK inside runners is Python; the operator only renders environment for it. The + operator never imports it. +- Token conventions carry over: Secret key `token`, read by the controller for resolution and + revalidation, handed to an engine only through an env `secretKeyRef`, to the plugin only + through `nodePublishSecretRef`. +- Measured on www.modelscope.cn, 2026-09-29 (spike) and 2026-09-24 (PoC-D): the endpoints, the + HEAD semantics and the reason mapping above. The private-repository "valid token without + access" cell remains unadjudicated (needs a second account); its mapping is asserted, its + distinctness is not claimed anywhere. + +## Boundaries + +- **Always:** the three D5 guards on every resolution path; tests proven able to fail for both + guards; English code, comments and docs; per-module squashed commits with `--signoff`, spec + last; rebase on `origin/main` at each task start and before every e2e and ship. +- **Ask first:** any change to `api/` types beyond the one `NodeModelStoreHub` field above; any + chart change; any new Setting + beyond the one above; any new test resource on ModelScope beyond reading the existing public + repositories. +- **Never:** manifest format changes; cross-hub dedup; touching crew-line files + (`pkg/setting/types.go`, `node_queue.go`, the MD webhook barrier); creating ModelScope accounts + or repositories; writing tokens anywhere outside a Secret reference. +- Review round budget: 4. After that, findings are listed, not fixed — except data loss, a broken + security guarantee (read-only), or cross-tenant leakage, each of which is first raised with the + coordinator. + +## Risks and Mitigations + +- **Undocumented endpoint drift** (`commits?Ref=`) → the cross-check with git catches a drift on + the resolution path itself; a drift breaks toward refusal, never wrong content. +- **Silent truncation** → the per-directory walk plus the 3000-children refusal; unit tests with + fake servers pin all three shapes (no truncation, recovered truncation, unrecoverable). +- **Runner SDK floor** → option B (confirmed at the spec gate): the docs state the floor, name + the measured runners on + each side of it, and point at a user-named image for the rest. No admission rule pretends to + know a runner's contents; an old runner's `NotExistError` is a loud engine error, and the docs + explain it. +- **The advertisement's undocumented shape drifts** → the parser refuses what it cannot read + (`SourceUnavailable`), the same direction every other cross-check failure breaks toward. +- **Private-repository edge cells** unadjudicated for lack of a second account → stated in spec, + docs and tests; nothing claims a measurement it does not have. + +## Design Details + +### Commands + +- Unit tests: `go test ./pkg/modelartifact/... ./pkg/worker/webhooks/worker/... ./pkg/worker/controllers/worker/... ./pkg/modelmanager/...` +- Lint: `make lint` (and the docs gate `make lint docs` when docs change in the same commit). +- Generated code: expected none; if needed, the private gen tree per STANDARDS §14. +- e2e: the `gpustack-operator-e2e` skill on a local kind cluster; case number taken at my-ship as + main's then-maximum plus one. + +### Project Structure + +- `pkg/modelartifact/modelscope.go` (+ test): the client — revision resolution with ls-remote + cross-check, the truncation-recovering listing, the reason classifier, revalidation HEAD, + token check, file URL. +- `pkg/worker/webhooks/worker/model_artifact.go` (+ test): accept and default the member. +- `pkg/worker/settings/value.go` and `model_store.go` (+ tests): the endpoint Setting and its + ride on the node's effective configuration (`Layer`). +- `api/worker/v1alpha1/node_model_store.go`: the one additive field + (`NodeModelStoreHub.modelScopeEndpoint`); `make generate` in the private gen tree. +- `pkg/worker/controllers/worker/model_artifact.go` (+ test): the reconcile branch, pacing and + conditions shared with Hugging Face, the token warning. +- `pkg/worker/controllers/worker/model_artifact_placement.go` and + `model_deployment_artifact.go` (+ tests): the ModelScope source's Node/Engine delivery and the + `MODELSCOPE_*` engine environment. +- `pkg/worker/webhooks/worker/model_deployment.go` (+ test): the owned ModelScope environment + names an artifact forbids, beside the Hugging Face ones. +- `pkg/modelmanager/driver/authorize.go`, `materialize/materialize.go`, `report/report.go` + (+ tests): the mount rule accepts the source, the download source carries its hub's kind, and + the node builds the right hub client from its configuration. +- `docs/` per Documentation above. + +### Code Style + +Follow the repository's Go conventions; the client mirrors `huggingface.go`'s shape (methods take +the token, a shared client carries no credential), and every failure path names a reason the +status can report. + +### Implementation Plan + +Tasks run in order (one seat, sequential per the task's standards); every task starts with +`git fetch origin && git rebase origin/main` and lands as its own `--signoff` commit with the +affected packages' tests green. Tests are written first where the task states TDD, and both +guards (AC2, AC3) are mutation-verified — each new guard's test is shown red by a compile-safe +mutation of exactly the mechanism it guards, then green again after reverting. + +- [x] **T1 · Admission opens the member** + Blocked by: None + Owns: `pkg/worker/webhooks/worker/model_artifact.go`, `pkg/worker/webhooks/worker/model_artifact_test.go` + Acceptance: AC8 — a `modelScope` source is accepted and validated by the shared hub rules; + `revision` defaults to `master`; patterns are legal on it; the union message names + `modelScope` among the accepted members; the reserved-member refusal constant and its tests + are gone; Hugging Face behavior is untouched (existing cases stay green). + Verify: `go test ./pkg/worker/webhooks/worker/ -run ModelArtifact` +- [x] **T2 · ModelScope client: revision and reasons (TDD)** + Blocked by: None + Owns: `pkg/modelartifact/modelscope.go`, `pkg/modelartifact/modelscope_test.go` + Acceptance: AC2 red-then-green — the `commits?Ref=` resolution, the `git ls-remote` + cross-check (HEAD line for a branch, peeled line for an annotated tag, `oauth2` + token for + private), and the reason table (envelope `Code` first, HTTP fallback) with a fake transport + and a fake `git` (command seam). A disagreement refuses as `SourceUnavailable` with the + disagreement message. Mutation: drop the cross-check ⇒ the disagreement case fails. + Verify: `go test ./pkg/modelartifact/` (the task's cases are `ModelScope`-named; before T2 + lands, the package's existing manifest and filter tests carry the green) +- [x] **T3 · ModelScope client: listing, manifest, revalidation (TDD)** + Blocked by: T2 + Owns: `pkg/modelartifact/modelscope.go`, `pkg/modelartifact/modelscope_test.go` + Acceptance: AC3 red-then-green — the walk over `repo/files` (no truncation; recovered + truncation re-lists per `Root=`; a directory with 3000 direct children fails) against a + fake server; manifests build through the shared `Filter`/`NewManifest` and hash to the same + digest as an independently computed v1 manifest; `Revalidate` maps HEAD 200/404 per the + table; `FileURL` builds the `repo?Revision=&FilePath=` URL; `ValidToken` reads + `users/me`. Mutations: short-circuit the descent ⇒ the truncation cases fail; classify by + status only ⇒ a `Code` case fails. + Verify: `go test ./pkg/modelartifact/` +- [x] **T4 · Setting and effective configuration** + Blocked by: None + Owns: `pkg/worker/settings/value.go`, `pkg/worker/settings/model_store.go`, `pkg/modelstore/config.go`, `api/worker/v1alpha1/node_model_store.go`, `pkg/worker/settings/model_store_test.go` and the worker's writer of `NodeModelStore.spec` + Acceptance: the `model-artifact-modelscope-endpoint` Setting (default, admission as the + Hugging Face endpoint's), `Layer.ModelScopeEndpoint`, `NodeModelStoreHub.modelScopeEndpoint` + (optional, additive), and the worker writing it into every node's spec. `make generate` runs + in the private gen tree (`gen-trees/mam-s10`, branch `mam-s10-gen`) with zero drift. + Verify: `go test ./pkg/worker/settings/... ./pkg/modelstore/... ./api/worker/...`; gen tree replay zero-diff +- [x] **T5 · Controller: reconcile a ModelScope source** + Blocked by: T2, T3, T4 + Owns: `pkg/worker/controllers/worker/model_artifact.go`, `pkg/worker/controllers/worker/model_artifact_test.go` + Acceptance: AC1 (fake hub) — a `modelScope` artifact resolves once to the commit and digest, + `Resolved=True`, `LastValidatedTime` set; revalidation follows the same + confirm-then-revoke staircase; `SecretNotFound` and unreadable-Secret behavior match + Hugging Face; the Secret watch enqueues ModelScope artifacts too; a rejected new token + emits the Warning event; `UnsupportedSource` stays for pre-existing objects only. + Verify: `go test ./pkg/worker/controllers/worker/ -run ModelArtifact` +- [x] **T6 · Rendering: delivery and engine environment** + Blocked by: T4, T5 + Owns: `pkg/worker/controllers/worker/model_artifact_placement.go`, `pkg/worker/controllers/worker/model_deployment_artifact.go` and their tests, `pkg/worker/webhooks/worker/model_deployment.go` and its test + Acceptance: a ModelScope source resolves to Node delivery (plugin volume, same attributes) + or Engine delivery (`MODELSCOPE_CACHE`, `MODELSCOPE_DOMAIN` = the Setting URL's host, + `MODELSCOPE_API_TOKEN` from the Secret, `VLLM_USE_MODELSCOPE` / `SGLANG_USE_MODELSCOPE` by + engine, owned), with `--revision ` as today; the placement preference and KV + identity paths work unchanged off the digest; an artifact with patterns under Engine is + blocked as for Hugging Face; admission refuses the owned ModelScope env names on a + deployment with `artifactRef`. + Verify: `go test ./pkg/worker/controllers/worker/... ./pkg/worker/webhooks/worker/...` +- [x] **T7 · Plugin: hub selection and mount rule** + Blocked by: T4 (API field), T3 (client) + Owns: `pkg/modelmanager/driver/authorize.go`, `pkg/modelmanager/materialize/materialize.go`, `pkg/modelmanager/report/report.go` and their tests + Acceptance: the mount rule accepts a resolved `modelScope` artifact (`ruleNotHub` loosens + to hub sources of either kind); the download source records its hub kind from + `ma.Spec.Source`; the environment builds the `ModelScope` client from + `NodeModelStoreHub.modelScopeEndpoint` for such sources and the Hugging Face client as + today; a `modelScope` source on a configuration without the endpoint is refused with the + upgrade message, never resolved against the Hugging Face endpoint. + Verify: `go test ./pkg/modelmanager/...` +- [x] **T8 · Documentation** + Blocked by: T6, T7 + Owns: `docs/model-store/artifact.md`, `docs/settings.md`, `docs/README.md` as the docs skill routes + Acceptance: the source is documented per Documentation above — accepted member, ModelScope + rows in the reason table, the cross-check and truncation recovery, the all-`sha256` digest + note, the engine env table, the SDK floor with the measured runner list on each side, the + Settings row. + Verify: `bash .agents/skills/gpustack-operator-docs/scripts/check-docs.sh . && bash .agents/skills/gpustack-operator-docs/scripts/check-crossrefs.sh . && make lint docs` +- [x] **T9 · e2e on local kind** + Blocked by: T7, T8 + Owns: `.agents/skills/gpustack-operator-e2e/cases/case-.sh` (number taken at my-ship), the skill's case table + Acceptance: AC4 (cold-node materialization from a public ModelScope repository, byte + verification, one checkpoint-resume case), AC6 (two worker nodes, peer sync serves the + second node, the hub sees zero bytes), AC5's SGLang half (engine delivery pins the commit — + snapshot path and file hash, as PoC-C). The vLLM half runs on a self-named runner image + (upstream vLLM plus `modelscope>=1.39.1`, CPU kind — the SDK call precedes GPU init, as + PoC-C's crash loop shows); if the engine cannot reach its download phase without a GPU, the + half falls back to an SDK-level probe in that image and says so. + Verify: the e2e skill's full run on the case, green; evidence under `spec-10/raw/` + +### Test Plan +[ ] I/we understand the owners of the involved components may require updates to existing tests to make this +code solid enough prior to committing the changes necessary to implement this enhancement. + +#### Prerequisite testing updates + +None. The packages this plan touches all have test files today; no base rework is needed before +T1. Existing Hugging Face cases are the regression baseline and must stay green throughout. + +#### Unit tests + +New cases land with each task (TDD where marked); the mutation-verification protocol above +applies to AC2 and AC3's guards, and to any other load-bearing new gate that a compile-safe +mutation can neuter. Per-package targets, all on the host toolchain (Linux containers run on the +build machine only when a test genuinely needs amd64; none is expected): + +- `pkg/modelartifact` — the reason table row by row (AC7), the cross-check disagreement (AC2), + the three truncation shapes (AC3), annotated-tag peeling in `ls-remote` output, commit-as-is + short circuit, `FileURL` shape, HEAD revalidation mapping, `users/me` token check, manifest + equality with an independently computed v1 digest for an all-`sha256` listing. +- `pkg/worker/webhooks/worker` — acceptance and defaulting of the member (AC8), the union + message, patterns legality, the owned ModelScope env names on a deployment with `artifactRef`, + every pre-existing case still green. +- `pkg/worker/controllers/worker` — AC1's reconcile path (fake hub, fake clock), the + confirm-then-revoke staircase, Secret-watch enqueues, the token Warning, Node/Engine delivery + resolution for a ModelScope source, the `MODELSCOPE_*` environment (owned vs defaulted), the + patterns-under-Engine block, placement preference and KV identity unchanged off the digest. +- `pkg/worker/settings` — the Setting's default and admission chain, `Layer`/`Merge` carrying the + endpoint, defaults validation. +- `pkg/modelstore` — `Layer` gains the field; `Validate`/`Merge` unchanged behavior otherwise. +- `pkg/modelmanager` — the mount rule on a `modelScope` artifact (authorized, and each refusal + rule still firing), source hub-kind recording, environment client selection (Hugging Face + source ⇒ Hugging Face client, ModelScope source ⇒ ModelScope client, missing endpoint ⇒ the + upgrade refusal), download of a ModelScope manifest against a fake hub. + +#### Integration tests + +Covered by the unit suites' fake-server and fake-hub seams: the controller's reconcile against a +fake ModelScope endpoint (resolution → status write → revalidation), and the plugin's +materialization of a ModelScope manifest (download → verify → publish → mount-ready) against a +fake hub — the same shape the Hugging Face tests use today. No separate integration harness. + +#### e2e tests + +One case (number taken at my-ship) on a local kind cluster (1 control-plane + 2 workers), per +T9: cold-node materialization from a public ModelScope repository with byte verification and one +checkpoint-resume leg; peer sync serving the second node with zero hub bytes; SGLang engine +delivery pinning the commit; the vLLM half on a self-named runner image with `modelscope>=1.39.1` +(falling back to an SDK-level probe if the engine cannot reach its download phase without a GPU, +stated in the evidence). The `NodeModelStoreHub` field changes the CRD the chart ships, so the +local 7-image chart matrix runs before my-ship per the task standards (REQUIRED for a Go +change). + +## Alternatives + +- **Trust the API alone** (no ls-remote): rejected — a misspelled parameter or an endpoint drift + resolves the wrong revision with a 200; D5's first condition exists because that failure is + silent. +- **Refuse non-commit revisions on ModelScope** (commit-only sources): rejected — it drops the + common branch workflow and the hub can be made truthful with one cross-check. +- **Refuse the `modelScope` + Engine + vLLM combination until a released runner meets the floor** + (the spike's option A): rejected in the coordination — the same formula meets the floor on + Ascend today, so a combination refusal would reject users whose runner already pins, while the + "vLLM version → SDK version" mapping the refusal would need does not exist (the SDK floats with + the runner's dependency-layer cache). +- **Block the whole engine path until the runner ecosystem upgrades** (option C): rejected in the + coordination — SGLang already meets the floor with PoC-C end-to-end evidence, and a + user-supplied runner image covers vLLM today. +- **ModelScope only under Node delivery** (no engine path): superseded by the direction; kept in + the record because it was the pre-spike contingency. +- **A second Setting for the ls-remote git URL**: rejected — the git URL is the endpoint's + host plus the repository path, one derived value, not independent configuration. +- **Pinning the SDK version in the runner Dockerfile**: upstream's file, not this repository's; + noted in the spike record instead of patched here. The docs' floor plus a user-named image + covers every runner, pinned or not. + +## Open Questions + +1. ~~The engine-runner adjudication~~ — **directed and confirmed, option B (2026-09-29 spec + gate)**: render every engine path + normally and document the SDK ≥ 1.39.1 floor; users on a runner below it name their own image. + Recorded in the engine-delivery section; it shapes AC5 and the docs, and adds no admission + rule. + +No open question blocks the draft; the remaining gate is the coordinator's confirmation of the +draft itself.