From 5c2e888148e3978282612c1a6250c0830c4872d4 Mon Sep 17 00:00:00 2001 From: amd-mkarvir <272370325+amd-mkarvir@users.noreply.github.com> Date: Mon, 31 Aug 2026 22:36:59 -0700 Subject: [PATCH 1/2] Harden serving recipe cache freshness --- .github/workflows/validate.yml | 18 ++ skills/serving-llms-on-instinct/SKILL.md | 66 +++++-- .../serving-llms-on-instinct/evals/evals.json | 2 + .../scripts/sync_recipes.py | 186 +++++++++++++----- .../scripts/tests/test_sync_recipes.py | 97 +++++++++ 5 files changed, 304 insertions(+), 65 deletions(-) create mode 100644 skills/serving-llms-on-instinct/scripts/tests/test_sync_recipes.py diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 2a72866..d4ccadc 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -88,6 +88,24 @@ jobs: - name: Test the eval runner run: uv run --with pyyaml python eval/test_evals.py + # The serving recipe cache controls model compatibility and the container + # image agents deploy. Exercise its fail-closed freshness and immutable-image + # selection without making network requests. + test-serving-recipe-cache: + name: Test serving recipe cache + runs-on: ubuntu-latest + steps: + - name: Check out repository + uses: actions/checkout@v4 + + - name: Set up uv + uses: astral-sh/setup-uv@v7 + + - name: Test recipe cache + run: >- + uv run --with pyyaml python -m unittest discover + -s skills/serving-llms-on-instinct/scripts/tests -v + # Repo-wide checks that aren't tied to a single skill: the generated plugin # manifests and internal markdown references. validate-manifests: diff --git a/skills/serving-llms-on-instinct/SKILL.md b/skills/serving-llms-on-instinct/SKILL.md index ba45b09..003bdf7 100644 --- a/skills/serving-llms-on-instinct/SKILL.md +++ b/skills/serving-llms-on-instinct/SKILL.md @@ -23,6 +23,8 @@ Get a vLLM endpoint running on AMD Instinct GPU hardware. - ROCm driver and `amd-smi` installed on the GPU host - Docker running and accessible (check with `docker ps`) - `/dev/kfd` and `/dev/dri` present on the GPU host +- Python 3, Git, curl, and PyYAML available where the helper scripts run + (`python3 -m pip install PyYAML` if `import yaml` fails) - HuggingFace token in `HF_TOKEN` env var (required for gated models; not required for Qwen3 or Gemma). For gated models (Llama 3.2, Gemma, etc.), the HF token must belong to an account that has accepted the model's license @@ -36,12 +38,13 @@ Get a vLLM endpoint running on AMD Instinct GPU hardware. Read these files directly to get model and GPU configuration: -- **`data/recipes_cache.json`** -- model configs synced from +- **`data/recipes_cache.json`** -- bundled fallback model configs synced from [vllm-project/recipes](https://github.com/vllm-project/recipes). Each entry under `models..recipe` contains the full recipe with `model.base_args`, `model.base_env`, `features.tool_calling.args`, `features.reasoning.args`, `hardware_overrides.amd.extra_args`, `hardware_overrides.amd.extra_env`. - The top-level `docker_image` field has the latest resolved vLLM ROCm image. + The top-level `docker_image` field has the resolved vLLM ROCm image. Do not + treat this bundled file as current until Step 3 verifies freshness. - **`data/gpu_overrides.json`** -- GPU-specific configuration. Contains `docker_flags` (mandatory for all AMD Instinct), `gpu_configs` keyed by @@ -88,22 +91,35 @@ python3 scripts/validate.py --auto-fix --host user@hostname Returns JSON with `ready` (bool), `errors`, `warnings`, `fixes_applied`. Do not proceed if `ready` is `false`. -## Step 3: Refresh recipes (if stale) +## Step 3: Establish a fresh recipe source -Check `fetched_at` in `data/recipes_cache.json`. If older than 24 hours or -the file is missing, refresh: +Before selecting a model, image, precision, or arguments, check the writable +runtime cache: ```bash -python3 scripts/sync_recipes.py +python3 scripts/sync_recipes.py --check || python3 scripts/sync_recipes.py +python3 scripts/sync_recipes.py --check ``` -This shallow-clones vllm-project/recipes from GitHub and fetches the latest -Docker tag from Docker Hub. Takes ~10 seconds. If it fails, the existing -cache still works. +The first successful refresh shallow-clones `vllm-project/recipes`, resolves +the highest stable vLLM ROCm image and immutable manifest digest, and writes a +runtime cache outside the installed skill package. Read the `cache` path from +the JSON output and use that file in the remaining steps. + +Both checks and refreshes return nonzero on missing, stale, malformed, or +unrefreshable data. Never silently present the bundled cache as current. If +refresh fails: + +- If the requested model is absent from the bundled cache, stop before launch + and explain that current framework support and arguments could not be + established. +- If the model is present, report the bundled cache's `fetched_at`, image tag, + and refresh error in the Step 5 summary. Use it only after the user explicitly + accepts the stale recipe risk. ## Step 4: Construct the Docker command -Read `data/recipes_cache.json` and `data/gpu_overrides.json` directly. +Read the fresh runtime cache selected in Step 3 and `data/gpu_overrides.json`. Build the Docker command by combining: 1. **Docker flags** from `gpu_overrides.json > docker_flags` (mandatory for all AMD GPUs) @@ -116,11 +132,14 @@ Build the Docker command by combining: 4. **Environment variables**: merge `gpu_configs..env_defaults` with the recipe's `model.base_env` and `hardware_overrides.amd.extra_env`. Always add `--env HF_TOKEN=${HF_TOKEN}`. -5. **Docker image**: use `docker_image` from `recipes_cache.json` top level - (unless the model needs a pinned image, e.g. GLM-4.5 needs `v0.15.1`). - If the user specifies a Docker image version, check it against the recipe's - `model.min_vllm_version`. Warn if the image is older -- the model may crash - on startup with an opaque "Engine core initialization failed" error. +5. **Docker image**: use `docker_image_pinned` from the runtime cache when + available; otherwise disclose that only a mutable `docker_image` tag is + available. Always compare the selected image against + `model.min_vllm_version`, even when the user did not specify the image. + Use a model-specific pinned image instead when the recipe requires one + (for example, GLM-4.5 requires `v0.15.1`). + Do not launch with an older image; it may fail with an opaque "Engine core + initialization failed" error. 6. **Model ID**: `--model ` 7. **vLLM args**: combine the recipe's `model.base_args` + `hardware_overrides.amd.extra_args` + `features.tool_calling.args` + @@ -129,13 +148,17 @@ Build the Docker command by combining: For MoE models on multi-GPU, also add `--distributed-executor-backend mp`. 8. **Port arg**: `--port ` -If the exact model ID is not in `recipes_cache.json`, check for a base model -match by stripping date/version suffixes (e.g., `Kimi-K2-Instruct` matches -`Kimi-K2-Instruct-0905`). Use the base model's recipe if found. +If the exact model ID is not in the fresh runtime cache, do not infer support +from a similarly named older generation. A date/version-suffixed fine-tune may +use its exact base model's recipe only after its Hugging Face `config.json` +confirms the same `model_type` and `architectures`. If no recipe match, check `legacy_models` in `gpu_overrides.json`. If not -there either, use a generic config with -`--enable-auto-tool-choice --trust-remote-code --tool-call-parser hermes`. +there either, inspect the model provider's official serving instructions, the +current vLLM supported-model list and release notes, and the model config before +proposing an experimental generic launch. Do not add `--trust-remote-code` or +assume the `hermes` tool parser without model-specific evidence. Present the +generic configuration and uncertainty in Step 5 and wait for confirmation. **Precision variant selection:** Recipes may offer variants (default, fp8, nvfp4). Check `gpu_configs..precision.native` in @@ -221,6 +244,9 @@ Before launching, present a summary and ask the user to confirm: - **TP**: tensor parallelism degree (1, 2, 4, 8) - **Context**: max achievable context length (and whether it's limited) - **Port**: which port the endpoint will be on +- **Recipe provenance**: cache timestamp, vLLM recipes commit, immutable image + digest, and whether the target GPU is merely supported or actually verified + by the upstream recipe If a quantized alternative was selected (Step 4 fit check), explain that the original model doesn't fit and which alternative is being used. diff --git a/skills/serving-llms-on-instinct/evals/evals.json b/skills/serving-llms-on-instinct/evals/evals.json index cf02de8..c81c8af 100644 --- a/skills/serving-llms-on-instinct/evals/evals.json +++ b/skills/serving-llms-on-instinct/evals/evals.json @@ -7,11 +7,13 @@ "prompt": "Serve Qwen/Qwen3-0.6B on this AMD Instinct GPU with vLLM. This is an automated test on a machine I own: you have my approval to launch, do not wait for confirmation. Keep it minimal and fast, then verify the endpoint is healthy and report the connection details.", "expected_behavior": [ "Detect the AMD Instinct GPU before configuring vLLM", + "Verify recipe cache freshness before selecting the vLLM image and arguments, and surface a refresh failure rather than silently treating stale bundled data as current", "Leave Qwen/Qwen3-0.6B served by vLLM in a Docker container on the AMD GPU, whether by launching one or by reusing a container already serving that model", "Verify the vLLM endpoint is healthy before reporting the connection details" ], "unexpected_behavior": [ "Fall back to a cloud LLM provider or an NVIDIA/CUDA code path", + "Describe a stale or failed-to-refresh recipe cache as the latest supported configuration", "Serve a different, larger model than the one that was requested" ] }, diff --git a/skills/serving-llms-on-instinct/scripts/sync_recipes.py b/skills/serving-llms-on-instinct/scripts/sync_recipes.py index 02bd420..58888e5 100644 --- a/skills/serving-llms-on-instinct/scripts/sync_recipes.py +++ b/skills/serving-llms-on-instinct/scripts/sync_recipes.py @@ -7,13 +7,16 @@ 2. Reads all model YAML files from models//.yaml 3. Latest stable Docker image tag from Docker Hub API -Writes output to: data/recipes_cache.json +Writes output to a writable runtime cache. The path is reported as JSON. Usage: - python3 scripts/sync_recipes.py # refresh cache - python3 scripts/sync_recipes.py --verbose # show progress + python3 scripts/sync_recipes.py # refresh runtime cache + python3 scripts/sync_recipes.py --check # require a fresh cache + python3 scripts/sync_recipes.py --verbose # show progress + python3 scripts/sync_recipes.py --cache-file F # explicit cache path -Exit 0 always (callers fall back to existing cache on failure). +Refresh failures exit nonzero. The bundled cache remains a fallback for an +agent that explicitly discloses its age; it is never overwritten at runtime. """ import argparse @@ -36,8 +39,17 @@ REPO_URL = "https://github.com/vllm-project/recipes.git" DOCKERHUB_URL = "https://hub.docker.com/v2/repositories/vllm/vllm-openai-rocm/tags" -CACHE_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "..", "data") -CACHE_FILE = os.path.join(CACHE_DIR, "recipes_cache.json") +SKILL_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) +BUNDLED_CACHE_FILE = os.path.join(SKILL_DIR, "data", "recipes_cache.json") +DEFAULT_CACHE_FILE = os.environ.get( + "AMD_SKILLS_RECIPE_CACHE", + os.path.join( + tempfile.gettempdir(), "amd-skills", "serving-llms-on-instinct", + "recipes_cache.json", + ), +) +DEFAULT_MAX_AGE_HOURS = 24.0 +_STABLE_TAG = re.compile(r"^v(\d+)\.(\d+)\.(\d+)$") def _log(msg, verbose): @@ -47,12 +59,12 @@ def _log(msg, verbose): def _parse_yaml(path): """Parse a YAML file. Requires PyYAML.""" - with open(path) as f: + with open(path, encoding="utf-8") as f: return yaml.safe_load(f) def _clone_recipes(verbose=False): - """Shallow clone the recipes repo into a temp directory. Returns path.""" + """Shallow clone the recipes repo. Return ``(path, commit)``.""" tmpdir = tempfile.mkdtemp(prefix="vllm-recipes-") _log(f"Cloning {REPO_URL} (shallow)...", verbose) r = subprocess.run( @@ -63,7 +75,14 @@ def _clone_recipes(verbose=False): if r.returncode != 0: shutil.rmtree(tmpdir, ignore_errors=True) raise RuntimeError(f"git clone failed: {r.stderr[:200]}") - return tmpdir + commit = subprocess.run( + ["git", "-C", tmpdir, "rev-parse", "HEAD"], + stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, timeout=10, + ) + if commit.returncode != 0: + shutil.rmtree(tmpdir, ignore_errors=True) + raise RuntimeError(f"git rev-parse failed: {commit.stderr[:200]}") + return tmpdir, commit.stdout.strip() def _read_all_recipes(repo_dir, verbose=False): @@ -110,8 +129,31 @@ def _read_all_recipes(repo_dir, verbose=False): return recipes +def _select_docker_tag(tags): + """Select the highest stable semantic version and its manifest digest.""" + candidates = [] + for tag in tags: + match = _STABLE_TAG.fullmatch(tag.get("name", "")) + if match: + candidates.append((tuple(map(int, match.groups())), tag)) + if not candidates: + raise RuntimeError("Docker Hub returned no stable vLLM ROCm tag") + + selected = max(candidates, key=lambda item: item[0])[1] + digest = selected.get("digest", "") + if not digest: + images = selected.get("images") or [] + digest = next((image.get("digest", "") for image in images + if image.get("digest")), "") + if not digest: + raise RuntimeError( + f"Docker Hub returned no digest for {selected['name']}" + ) + return selected["name"], selected.get("last_updated", ""), digest + + def _fetch_docker_tag(verbose=False): - """Fetch the latest stable vllm-openai-rocm tag from Docker Hub.""" + """Fetch the highest stable vLLM ROCm tag and digest from Docker Hub.""" _log("Fetching Docker Hub tags...", verbose) url = f"{DOCKERHUB_URL}?page_size=50&ordering=last_updated" r = subprocess.run( @@ -119,28 +161,66 @@ def _fetch_docker_tag(verbose=False): stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, timeout=10, ) if r.returncode != 0: - return "latest", "" + raise RuntimeError(f"Docker Hub request failed: {r.stderr[:200]}") data = json.loads(r.stdout) - for tag in data.get("results", []): - name = tag["name"] - if "nightly" in name or "base" in name: - continue - if name.startswith("v") and re.match(r"v\d+\.\d+", name): - return name, tag.get("last_updated", "") - if name == "latest": - return name, tag.get("last_updated", "") + return _select_docker_tag(data.get("results", [])) - return "latest", "" + +def _cache_status(cache_file, max_age_hours=DEFAULT_MAX_AGE_HOURS, now=None): + """Return machine-readable cache freshness and whether it is usable.""" + result = {"cache": os.path.abspath(cache_file)} + if not os.path.isfile(cache_file): + return {**result, "status": "missing"}, False + try: + with open(cache_file, encoding="utf-8") as f: + cache = json.load(f) + fetched_at = datetime.fromisoformat(cache["fetched_at"].replace("Z", "+00:00")) + if fetched_at.tzinfo is None: + fetched_at = fetched_at.replace(tzinfo=timezone.utc) + except (OSError, ValueError, KeyError, TypeError, json.JSONDecodeError) as e: + return {**result, "status": "invalid", "error": str(e)}, False + + now = now or datetime.now(timezone.utc) + age_hours = max(0.0, (now - fetched_at).total_seconds() / 3600) + fresh = age_hours <= max_age_hours + return { + **result, + "status": "fresh" if fresh else "stale", + "fetched_at": fetched_at.isoformat(), + "age_hours": round(age_hours, 2), + "max_age_hours": max_age_hours, + "recipes_commit": cache.get("recipes_commit", ""), + "docker_image": cache.get("docker_image_pinned", + cache.get("docker_image", "")), + }, fresh + + +def _write_cache(cache_file, cache): + """Atomically replace a cache so interruption cannot leave partial JSON.""" + cache_dir = os.path.dirname(os.path.abspath(cache_file)) + os.makedirs(cache_dir, exist_ok=True) + fd, tmp_path = tempfile.mkstemp(prefix=".recipes-cache-", suffix=".tmp", + dir=cache_dir) + try: + with os.fdopen(fd, "w", encoding="utf-8") as f: + json.dump(cache, f, indent=2, default=str) + f.write("\n") + os.replace(tmp_path, cache_file) + finally: + if os.path.exists(tmp_path): + os.unlink(tmp_path) -def sync(verbose=False): +def sync(verbose=False, cache_file=DEFAULT_CACHE_FILE): if not HAS_YAML: - print("WARN: PyYAML not installed, cannot sync recipes", file=sys.stderr) - return False + raise RuntimeError( + "PyYAML is required to sync recipes; install it with " + "`python3 -m pip install PyYAML`" + ) # Step 1: Clone the repo - repo_dir = _clone_recipes(verbose) + repo_dir, recipes_commit = _clone_recipes(verbose) try: # Step 2: Read all YAML recipes @@ -150,51 +230,67 @@ def sync(verbose=False): shutil.rmtree(repo_dir, ignore_errors=True) if not recipes: - print("WARN: No recipes found in cloned repo", file=sys.stderr) - return False + raise RuntimeError("No recipes found in cloned repo") # Step 3: Fetch Docker Hub tag - docker_tag, docker_date = "latest", "" - try: - docker_tag, docker_date = _fetch_docker_tag(verbose) - _log(f"Latest stable ROCm tag: {docker_tag} ({docker_date})", verbose) - except Exception as e: - _log(f"Docker Hub fetch failed: {e}", verbose) + docker_tag, docker_date, docker_digest = _fetch_docker_tag(verbose) + _log(f"Latest stable ROCm tag: {docker_tag} ({docker_date})", verbose) # Step 4: Write cache cache = { "fetched_at": datetime.now(timezone.utc).isoformat(), + "recipes_source": REPO_URL, + "recipes_commit": recipes_commit, "docker_image": f"vllm/vllm-openai-rocm:{docker_tag}", + "docker_image_pinned": ( + f"vllm/vllm-openai-rocm@{docker_digest}" + ), + "docker_digest": docker_digest, "docker_tag": docker_tag, "docker_tag_date": docker_date, "model_count": len(recipes), "models": recipes, } - os.makedirs(CACHE_DIR, exist_ok=True) - with open(CACHE_FILE, "w") as f: - json.dump(cache, f, indent=2, default=str) - + _write_cache(cache_file, cache) _log(f"Cache written: {len(recipes)} models, tag={docker_tag}", verbose) - return True + return cache def main(): parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) parser.add_argument("--verbose", "-v", action="store_true") + parser.add_argument("--check", action="store_true", + help="exit nonzero unless the cache is fresh") + parser.add_argument("--max-age-hours", type=float, + default=DEFAULT_MAX_AGE_HOURS) + parser.add_argument("--cache-file", default=DEFAULT_CACHE_FILE) args = parser.parse_args() + if args.check: + status, fresh = _cache_status(args.cache_file, args.max_age_hours) + print(json.dumps(status)) + return 0 if fresh else 1 + try: - ok = sync(verbose=args.verbose) - if ok: - print(json.dumps({"status": "ok", "cache": CACHE_FILE})) - else: - print(json.dumps({"status": "partial", "cache": CACHE_FILE})) + cache = sync(verbose=args.verbose, cache_file=args.cache_file) + print(json.dumps({ + "status": "ok", + "cache": os.path.abspath(args.cache_file), + "recipes_commit": cache["recipes_commit"], + "docker_image": cache["docker_image_pinned"], + })) + return 0 except Exception as e: print(f"WARN: sync_recipes failed: {e}", file=sys.stderr) - print(json.dumps({"status": "failed", "error": str(e)})) - sys.exit(0) + print(json.dumps({ + "status": "failed", + "error": str(e), + "cache": os.path.abspath(args.cache_file), + "bundled_fallback": os.path.abspath(BUNDLED_CACHE_FILE), + })) + return 1 if __name__ == "__main__": - main() + sys.exit(main()) diff --git a/skills/serving-llms-on-instinct/scripts/tests/test_sync_recipes.py b/skills/serving-llms-on-instinct/scripts/tests/test_sync_recipes.py new file mode 100644 index 0000000..b0a4c81 --- /dev/null +++ b/skills/serving-llms-on-instinct/scripts/tests/test_sync_recipes.py @@ -0,0 +1,97 @@ +#!/usr/bin/env python3 +"""Regression tests for recipe-cache freshness and provenance selection.""" + +from __future__ import annotations + +import importlib.util +import json +import tempfile +import unittest +from datetime import datetime, timedelta, timezone +from pathlib import Path + + +SCRIPT = Path(__file__).resolve().parents[1] / "sync_recipes.py" +SPEC = importlib.util.spec_from_file_location("sync_recipes", SCRIPT) +sync_recipes = importlib.util.module_from_spec(SPEC) +assert SPEC.loader is not None +SPEC.loader.exec_module(sync_recipes) + + +class CacheStatusTests(unittest.TestCase): + def test_missing_cache_is_not_fresh(self): + with tempfile.TemporaryDirectory() as tmp: + status, fresh = sync_recipes._cache_status( + str(Path(tmp) / "missing.json") + ) + self.assertFalse(fresh) + self.assertEqual(status["status"], "missing") + + def test_fresh_and_stale_cache_are_distinguished(self): + now = datetime(2026, 8, 30, tzinfo=timezone.utc) + with tempfile.TemporaryDirectory() as tmp: + cache = Path(tmp) / "recipes.json" + cache.write_text(json.dumps({ + "fetched_at": (now - timedelta(hours=2)).isoformat(), + "recipes_commit": "abc123", + "docker_image_pinned": "vllm/image@sha256:123", + }), encoding="utf-8") + + status, fresh = sync_recipes._cache_status( + str(cache), max_age_hours=24, now=now + ) + self.assertTrue(fresh) + self.assertEqual(status["status"], "fresh") + + status, fresh = sync_recipes._cache_status( + str(cache), max_age_hours=1, now=now + ) + self.assertFalse(fresh) + self.assertEqual(status["status"], "stale") + + def test_malformed_cache_is_not_fresh(self): + with tempfile.TemporaryDirectory() as tmp: + cache = Path(tmp) / "recipes.json" + cache.write_text("not json", encoding="utf-8") + status, fresh = sync_recipes._cache_status(str(cache)) + self.assertFalse(fresh) + self.assertEqual(status["status"], "invalid") + + +class RecipeParsingTests(unittest.TestCase): + @unittest.skipUnless(sync_recipes.HAS_YAML, "PyYAML is not installed") + def test_yaml_is_read_as_utf8(self): + with tempfile.TemporaryDirectory() as tmp: + recipe = Path(tmp) / "recipe.yaml" + recipe.write_text("meta:\n description: 日本語\n", encoding="utf-8") + parsed = sync_recipes._parse_yaml(str(recipe)) + self.assertEqual(parsed["meta"]["description"], "日本語") + + +class DockerTagTests(unittest.TestCase): + def test_highest_stable_semver_wins_over_latest_and_nightly(self): + tags = [ + {"name": "latest", "digest": "sha256:latest"}, + {"name": "nightly", "digest": "sha256:nightly"}, + {"name": "v0.9.2", "digest": "sha256:old"}, + { + "name": "v0.28.0", + "last_updated": "2026-08-26T00:00:00Z", + "images": [{"digest": "sha256:new"}], + }, + {"name": "v0.22.0", "digest": "sha256:middle"}, + ] + tag, updated, digest = sync_recipes._select_docker_tag(tags) + self.assertEqual(tag, "v0.28.0") + self.assertEqual(updated, "2026-08-26T00:00:00Z") + self.assertEqual(digest, "sha256:new") + + def test_missing_stable_tag_fails(self): + with self.assertRaises(RuntimeError): + sync_recipes._select_docker_tag([ + {"name": "latest", "digest": "sha256:latest"} + ]) + + +if __name__ == "__main__": + unittest.main() From 43159c7243157743a53683cb8cf22f4146fda564 Mon Sep 17 00:00:00 2001 From: amd-mkarvir <272370325+amd-mkarvir@users.noreply.github.com> Date: Mon, 31 Aug 2026 22:37:00 -0700 Subject: [PATCH 2/2] Address recipe cache review feedback --- skills/serving-llms-on-instinct/SKILL.md | 7 +- .../scripts/sync_recipes.py | 90 +++++++++++++------ .../scripts/tests/test_sync_recipes.py | 86 ++++++++++++++++++ 3 files changed, 156 insertions(+), 27 deletions(-) diff --git a/skills/serving-llms-on-instinct/SKILL.md b/skills/serving-llms-on-instinct/SKILL.md index 003bdf7..ad2bd59 100644 --- a/skills/serving-llms-on-instinct/SKILL.md +++ b/skills/serving-llms-on-instinct/SKILL.md @@ -97,11 +97,16 @@ Before selecting a model, image, precision, or arguments, check the writable runtime cache: ```bash +# Refresh only when the existing cache is missing, invalid, or stale. python3 scripts/sync_recipes.py --check || python3 scripts/sync_recipes.py + +# Reopen the cache from disk, confirm it is fresh, and print its path. python3 scripts/sync_recipes.py --check ``` -The first successful refresh shallow-clones `vllm-project/recipes`, resolves +The final check is intentional: it validates the file written by a refresh, +rather than trusting the refresh command's exit status. A successful refresh +shallow-clones `vllm-project/recipes`, resolves the highest stable vLLM ROCm image and immutable manifest digest, and writes a runtime cache outside the installed skill package. Read the `cache` path from the JSON output and use that file in the remaining steps. diff --git a/skills/serving-llms-on-instinct/scripts/sync_recipes.py b/skills/serving-llms-on-instinct/scripts/sync_recipes.py index 58888e5..b0ae19b 100644 --- a/skills/serving-llms-on-instinct/scripts/sync_recipes.py +++ b/skills/serving-llms-on-instinct/scripts/sync_recipes.py @@ -41,17 +41,27 @@ SKILL_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) BUNDLED_CACHE_FILE = os.path.join(SKILL_DIR, "data", "recipes_cache.json") -DEFAULT_CACHE_FILE = os.environ.get( - "AMD_SKILLS_RECIPE_CACHE", - os.path.join( - tempfile.gettempdir(), "amd-skills", "serving-llms-on-instinct", - "recipes_cache.json", - ), +DEFAULT_CACHE_FILE = os.path.join( + tempfile.gettempdir(), "amd-skills", "serving-llms-on-instinct", + "recipes_cache.json", ) DEFAULT_MAX_AGE_HOURS = 24.0 +MAX_DOCKERHUB_PAGES = 100 _STABLE_TAG = re.compile(r"^v(\d+)\.(\d+)\.(\d+)$") +def _resolve_cache_file(path): + """Expand user/environment syntax and return an absolute cache path.""" + return os.path.abspath(os.path.expandvars(os.path.expanduser(path))) + + +def _runtime_cache_file(): + """Resolve the environment override at call time, not module import time.""" + return _resolve_cache_file( + os.environ.get("AMD_SKILLS_RECIPE_CACHE", DEFAULT_CACHE_FILE) + ) + + def _log(msg, verbose): if verbose: print(f" [sync] {msg}", file=sys.stderr, flush=True) @@ -155,16 +165,35 @@ def _select_docker_tag(tags): def _fetch_docker_tag(verbose=False): """Fetch the highest stable vLLM ROCm tag and digest from Docker Hub.""" _log("Fetching Docker Hub tags...", verbose) - url = f"{DOCKERHUB_URL}?page_size=50&ordering=last_updated" - r = subprocess.run( - ["curl", "-sf", "--max-time", "5", url], - stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, timeout=10, + url = f"{DOCKERHUB_URL}?page_size=100&ordering=last_updated" + tags = [] + for page in range(1, MAX_DOCKERHUB_PAGES + 1): + r = subprocess.run( + ["curl", "-sf", "--max-time", "5", url], + stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, + timeout=10, + ) + if r.returncode != 0: + raise RuntimeError( + f"Docker Hub request failed on page {page}: {r.stderr[:200]}" + ) + + data = json.loads(r.stdout) + results = data.get("results", []) + if not isinstance(results, list): + raise RuntimeError(f"Docker Hub returned invalid results on page {page}") + tags.extend(results) + + next_url = data.get("next") + if not next_url: + return _select_docker_tag(tags) + if not isinstance(next_url, str) or not next_url.startswith(DOCKERHUB_URL): + raise RuntimeError("Docker Hub returned an invalid pagination URL") + url = next_url + + raise RuntimeError( + f"Docker Hub pagination exceeded {MAX_DOCKERHUB_PAGES} pages" ) - if r.returncode != 0: - raise RuntimeError(f"Docker Hub request failed: {r.stderr[:200]}") - - data = json.loads(r.stdout) - return _select_docker_tag(data.get("results", [])) def _cache_status(cache_file, max_age_hours=DEFAULT_MAX_AGE_HOURS, now=None): @@ -200,19 +229,23 @@ def _write_cache(cache_file, cache): """Atomically replace a cache so interruption cannot leave partial JSON.""" cache_dir = os.path.dirname(os.path.abspath(cache_file)) os.makedirs(cache_dir, exist_ok=True) - fd, tmp_path = tempfile.mkstemp(prefix=".recipes-cache-", suffix=".tmp", - dir=cache_dir) + tmp = tempfile.NamedTemporaryFile( + mode="w", encoding="utf-8", prefix=".recipes-cache-", suffix=".tmp", + dir=cache_dir, delete=False, + ) + tmp_path = tmp.name try: - with os.fdopen(fd, "w", encoding="utf-8") as f: - json.dump(cache, f, indent=2, default=str) - f.write("\n") + with tmp: + json.dump(cache, tmp, indent=2, default=str) + tmp.write("\n") os.replace(tmp_path, cache_file) finally: if os.path.exists(tmp_path): os.unlink(tmp_path) -def sync(verbose=False, cache_file=DEFAULT_CACHE_FILE): +def sync(verbose=False, cache_file=None): + cache_file = _resolve_cache_file(cache_file or _runtime_cache_file()) if not HAS_YAML: raise RuntimeError( "PyYAML is required to sync recipes; install it with " @@ -264,19 +297,24 @@ def main(): help="exit nonzero unless the cache is fresh") parser.add_argument("--max-age-hours", type=float, default=DEFAULT_MAX_AGE_HOURS) - parser.add_argument("--cache-file", default=DEFAULT_CACHE_FILE) + parser.add_argument("--cache-file") args = parser.parse_args() + cache_file = ( + _resolve_cache_file(args.cache_file) + if args.cache_file + else _runtime_cache_file() + ) if args.check: - status, fresh = _cache_status(args.cache_file, args.max_age_hours) + status, fresh = _cache_status(cache_file, args.max_age_hours) print(json.dumps(status)) return 0 if fresh else 1 try: - cache = sync(verbose=args.verbose, cache_file=args.cache_file) + cache = sync(verbose=args.verbose, cache_file=cache_file) print(json.dumps({ "status": "ok", - "cache": os.path.abspath(args.cache_file), + "cache": cache_file, "recipes_commit": cache["recipes_commit"], "docker_image": cache["docker_image_pinned"], })) @@ -286,7 +324,7 @@ def main(): print(json.dumps({ "status": "failed", "error": str(e), - "cache": os.path.abspath(args.cache_file), + "cache": cache_file, "bundled_fallback": os.path.abspath(BUNDLED_CACHE_FILE), })) return 1 diff --git a/skills/serving-llms-on-instinct/scripts/tests/test_sync_recipes.py b/skills/serving-llms-on-instinct/scripts/tests/test_sync_recipes.py index b0a4c81..46c1155 100644 --- a/skills/serving-llms-on-instinct/scripts/tests/test_sync_recipes.py +++ b/skills/serving-llms-on-instinct/scripts/tests/test_sync_recipes.py @@ -4,11 +4,16 @@ from __future__ import annotations import importlib.util +import io import json +import os +import sys import tempfile import unittest +from contextlib import redirect_stdout from datetime import datetime, timedelta, timezone from pathlib import Path +from unittest import mock SCRIPT = Path(__file__).resolve().parents[1] / "sync_recipes.py" @@ -58,6 +63,56 @@ def test_malformed_cache_is_not_fresh(self): self.assertEqual(status["status"], "invalid") +class CachePathTests(unittest.TestCase): + def test_environment_cache_path_is_resolved_at_runtime(self): + with tempfile.TemporaryDirectory() as tmp: + env = { + "AMD_SKILLS_RECIPE_CACHE": "$RECIPE_CACHE_ROOT/env-cache.json", + "RECIPE_CACHE_ROOT": tmp, + } + with mock.patch.dict(os.environ, env, clear=False): + resolved = sync_recipes._runtime_cache_file() + self.assertEqual(resolved, str(Path(tmp, "env-cache.json").resolve())) + + def test_cache_file_cli_override_wins_and_expands_variables(self): + with tempfile.TemporaryDirectory() as tmp: + explicit = "$RECIPE_CACHE_ROOT/cli-cache.json" + expected = str(Path(tmp, "cli-cache.json").resolve()) + env = { + "AMD_SKILLS_RECIPE_CACHE": str(Path(tmp, "env-cache.json")), + "RECIPE_CACHE_ROOT": tmp, + } + argv = ["sync_recipes.py", "--check", "--cache-file", explicit] + with ( + mock.patch.dict(os.environ, env, clear=False), + mock.patch.object(sys, "argv", argv), + mock.patch.object( + sync_recipes, + "_cache_status", + return_value=({"status": "fresh"}, True), + ) as cache_status, + redirect_stdout(io.StringIO()), + ): + result = sync_recipes.main() + + self.assertEqual(result, 0) + cache_status.assert_called_once_with( + expected, sync_recipes.DEFAULT_MAX_AGE_HOURS + ) + + +class CacheWriteTests(unittest.TestCase): + def test_cache_is_replaced_and_temporary_file_is_removed(self): + with tempfile.TemporaryDirectory() as tmp: + cache_file = Path(tmp, "recipes.json") + sync_recipes._write_cache(str(cache_file), {"fetched_at": "now"}) + written = json.loads(cache_file.read_text(encoding="utf-8")) + leftovers = list(Path(tmp).glob(".recipes-cache-*.tmp")) + + self.assertEqual(written, {"fetched_at": "now"}) + self.assertEqual(leftovers, []) + + class RecipeParsingTests(unittest.TestCase): @unittest.skipUnless(sync_recipes.HAS_YAML, "PyYAML is not installed") def test_yaml_is_read_as_utf8(self): @@ -92,6 +147,37 @@ def test_missing_stable_tag_fails(self): {"name": "latest", "digest": "sha256:latest"} ]) + def test_fetch_follows_pagination_before_selecting(self): + next_url = f"{sync_recipes.DOCKERHUB_URL}?page=2" + responses = [ + sync_recipes.subprocess.CompletedProcess( + args=[], returncode=0, + stdout=json.dumps({ + "results": [{"name": "v0.22.0", "digest": "sha256:old"}], + "next": next_url, + }), + stderr="", + ), + sync_recipes.subprocess.CompletedProcess( + args=[], returncode=0, + stdout=json.dumps({ + "results": [{"name": "v0.28.0", "digest": "sha256:new"}], + "next": None, + }), + stderr="", + ), + ] + with mock.patch.object( + sync_recipes.subprocess, "run", side_effect=responses + ) as run: + tag, _, digest = sync_recipes._fetch_docker_tag() + + self.assertEqual(tag, "v0.28.0") + self.assertEqual(digest, "sha256:new") + self.assertEqual(run.call_count, 2) + self.assertIn("page_size=100", run.call_args_list[0].args[0][-1]) + self.assertEqual(run.call_args_list[1].args[0][-1], next_url) + if __name__ == "__main__": unittest.main()