diff --git a/src/compute-plane-services/nvsnap/cmd/agent/main.go b/src/compute-plane-services/nvsnap/cmd/agent/main.go index 55b85a587..86317c021 100644 --- a/src/compute-plane-services/nvsnap/cmd/agent/main.go +++ b/src/compute-plane-services/nvsnap/cmd/agent/main.go @@ -116,6 +116,8 @@ func main() { "Directory the Local backend writes captures into") flag.StringVar(&config.RootfsCapture.PodCacheDir, "pod-cache-dir", os.Getenv("NVSNAP_POD_CACHE_DIR"), "In-pod cache mount path (e.g. /opt/nvsnap) for cachedir mode: capture ONLY this dir as the PVC root, restore RO-mounts the rox here (no overlayfs). Empty = standard whole-rootfs capture. Must match the webhook's cacheDir.") + flag.BoolVar(&config.RootfsCapture.AllowWholeRootfs, "allow-whole-rootfs", os.Getenv("NVSNAP_ALLOW_WHOLE_ROOTFS") == "1", + "Permit capture without --pod-cache-dir, i.e. capture the whole container rootfs. Off by default: whole-rootfs capture succeeds silently and only diverges later, at restore, from the cachedir behaviour every workload and benchmark assumes. Set only to run that path deliberately.") flag.StringVar(&config.RootfsCapture.PodCacheEnvFile, "cachedir-env-file", os.Getenv("NVSNAP_CACHEDIR_ENV_FILE"), "Path to a mounted ConfigMap file with the cachedir env template (NAME=value lines; {root}/{cache}/{model} placeholders). Read on capture inject only — edit the ConfigMap to add/remove cache env vars without an agent rebuild. Empty/unreadable = built-in default. Restore replays the env stamped in the manifest.") flag.StringVar(&config.OverlayRoot, "overlay-root", "/var/lib/nvsnap/overlays", diff --git a/src/compute-plane-services/nvsnap/deploy/k8s/benchmarks/gpt-oss-120b-restore.yaml b/src/compute-plane-services/nvsnap/deploy/k8s/benchmarks/gpt-oss-120b-restore.yaml index d15d614ee..75e4427f1 100644 --- a/src/compute-plane-services/nvsnap/deploy/k8s/benchmarks/gpt-oss-120b-restore.yaml +++ b/src/compute-plane-services/nvsnap/deploy/k8s/benchmarks/gpt-oss-120b-restore.yaml @@ -1,7 +1,7 @@ # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -# Restore pod for PDF-bench gpt-oss-120b (TP=4, rootfs path). +# Restore pod for PDF-bench gpt-oss-120b (TP=4, cachedir path). # # Customer-shape: no CRIU init ladder, no nodeName pin. The nvsnap # webhook injects PVC mounts and (for Local backend) nodeAffinity diff --git a/src/compute-plane-services/nvsnap/deploy/k8s/benchmarks/gpt-oss-120b.yaml b/src/compute-plane-services/nvsnap/deploy/k8s/benchmarks/gpt-oss-120b.yaml index d4cb1120a..6c3a462bf 100644 --- a/src/compute-plane-services/nvsnap/deploy/k8s/benchmarks/gpt-oss-120b.yaml +++ b/src/compute-plane-services/nvsnap/deploy/k8s/benchmarks/gpt-oss-120b.yaml @@ -20,7 +20,7 @@ # --no-enable-prefix-caching # Hardware : 4x H100 (one node, TP=4) # -# Multi-GPU → rootfs capture path per CLAUDE.md rule 20. No CRIU init +# Multi-GPU → cachedir capture path per CLAUDE.md rule 20. No CRIU init # container ladder; just the workload + nvsnap.io/capture=true label. # The agent's rootfsonly.Watcher snapshots the overlay upperdir after # the readiness probe passes + 60s warmup. @@ -33,10 +33,10 @@ metadata: app: bench-gpt-oss-120b nvsnap.io/bench: "pdf-matrix" nvsnap.io/bench-row: "llm-medium" - nvsnap.io/capture: "true" # opt-in to rootfs capture (multi-GPU only) + nvsnap.io/capture: "true" # opt-in to cachedir capture (multi-GPU only) annotations: nvsnap.io/desc: "PDF bench: openai/gpt-oss-120b TP=4 on vllm:v0.20.0" - nvsnap.io/path: "rootfs" + nvsnap.io/path: "cachedir" spec: tolerations: - key: "nvidia.com/gpu" diff --git a/src/compute-plane-services/nvsnap/deploy/k8s/workloads/nim-qwen3-32b.yaml b/src/compute-plane-services/nvsnap/deploy/k8s/workloads/nim-qwen3-32b.yaml index aad280130..7fccccc95 100644 --- a/src/compute-plane-services/nvsnap/deploy/k8s/workloads/nim-qwen3-32b.yaml +++ b/src/compute-plane-services/nvsnap/deploy/k8s/workloads/nim-qwen3-32b.yaml @@ -15,14 +15,14 @@ metadata: labels: app: nim-qwen3-32b nvsnap.io/demo: "true" - nvsnap.io/capture: "true" # opt-in to rootfs capture (multi-GPU only) + nvsnap.io/capture: "true" # opt-in to cachedir capture (multi-GPU only) annotations: nvsnap.io/demo-name: "NIM" - nvsnap.io/desc: "Qwen3-32B multi-GPU TP=2 (rootfs)" + nvsnap.io/desc: "Qwen3-32B multi-GPU TP=2 (cachedir)" nvsnap.io/model: "qwen/qwen3-32b" nvsnap.io/port: "8000" nvsnap.io/gpus: "2" - nvsnap.io/path: "rootfs" + nvsnap.io/path: "cachedir" nvsnap.io/ckpt-size: "61 GB" spec: tolerations: diff --git a/src/compute-plane-services/nvsnap/deploy/k8s/workloads/vllm-70b.yaml b/src/compute-plane-services/nvsnap/deploy/k8s/workloads/vllm-70b.yaml index d769f074b..646df7b2c 100644 --- a/src/compute-plane-services/nvsnap/deploy/k8s/workloads/vllm-70b.yaml +++ b/src/compute-plane-services/nvsnap/deploy/k8s/workloads/vllm-70b.yaml @@ -6,9 +6,9 @@ # nvsnap agent's rootfs watcher captures this pod after the warmup window. # # Customer-shape minimal yaml — no CRIU init container ladder. The -# rootfs path captures the pod's overlay upperdir + Hugging Face cache -# + compiled engine caches and replays them via webhook-injected bind -# mounts on the corresponding `vllm-70b-fresh` pod. +# cachedir path captures ONLY the pod's cache mount (Hugging Face model +# + compiled engine caches), not the whole container rootfs, and replays +# it via webhook-injected mounts on the corresponding restore pod. apiVersion: v1 kind: Pod metadata: @@ -17,14 +17,14 @@ metadata: labels: app: vllm-70b nvsnap.io/demo: "true" - nvsnap.io/capture: "true" # opt-in to rootfs capture (multi-GPU only) + nvsnap.io/capture: "true" # opt-in to cachedir capture (multi-GPU only) annotations: nvsnap.io/demo-name: "vLLM" - nvsnap.io/desc: "Llama-3.1-70B multi-GPU tensor-parallel (TP=4, rootfs)" + nvsnap.io/desc: "Llama-3.1-70B multi-GPU tensor-parallel (TP=4, cachedir)" nvsnap.io/model: "meta-llama/Llama-3.1-70B-Instruct" nvsnap.io/port: "8000" nvsnap.io/gpus: "4" - nvsnap.io/path: "rootfs" + nvsnap.io/path: "cachedir" nvsnap.io/ckpt-size: "132 GB" spec: tolerations: diff --git a/src/compute-plane-services/nvsnap/deploy/k8s/workloads/vllm-tp2.yaml b/src/compute-plane-services/nvsnap/deploy/k8s/workloads/vllm-tp2.yaml index 7f44d325e..99b5589c3 100644 --- a/src/compute-plane-services/nvsnap/deploy/k8s/workloads/vllm-tp2.yaml +++ b/src/compute-plane-services/nvsnap/deploy/k8s/workloads/vllm-tp2.yaml @@ -33,7 +33,7 @@ metadata: # anything requesting >= 2 GPUs here. Declaring "criu" made the manifest # generator emit a criu-v2 restore placeholder that nothing ever drives, # so the restore pod idled until the readiness timeout. - nvsnap.io/path: "rootfs" + nvsnap.io/path: "cachedir" spec: automountServiceAccountToken: false tolerations: diff --git a/src/compute-plane-services/nvsnap/internal/agent/BUILD.bazel b/src/compute-plane-services/nvsnap/internal/agent/BUILD.bazel index 4fcfefc3e..48881a4dc 100644 --- a/src/compute-plane-services/nvsnap/internal/agent/BUILD.bazel +++ b/src/compute-plane-services/nvsnap/internal/agent/BUILD.bazel @@ -107,6 +107,7 @@ go_test( "restoreoverlay_integration_test.go", "restoreoverlay_test.go", "rootfs_diff_test.go", + "rootfs_wholerootfs_guard_test.go", "rootfsonly_integration_test.go", ], embed = [":agent"], diff --git a/src/compute-plane-services/nvsnap/internal/agent/rootfs_wholerootfs_guard_test.go b/src/compute-plane-services/nvsnap/internal/agent/rootfs_wholerootfs_guard_test.go new file mode 100644 index 000000000..407843964 --- /dev/null +++ b/src/compute-plane-services/nvsnap/internal/agent/rootfs_wholerootfs_guard_test.go @@ -0,0 +1,67 @@ +/* +SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package agent + +import ( + "context" + "strings" + "testing" +) + +// Whole-rootfs capture must not start by accident. It succeeds quietly -- +// producing a capture that restores -- so a cluster whose cachedir setting was +// dropped keeps running while diverging from every workload and benchmark that +// assumes cachedir. The agent refuses at startup instead. +func TestStartRootfsCaptureRefusesWholeRootfs(t *testing.T) { + a := &Agent{} + _, err := a.startRootfsCapture(context.Background(), RootfsCaptureConfig{ + Enabled: true, // no PodCacheDir, no override + }) + if err == nil { + t.Fatal("expected refusal when --pod-cache-dir is unset; whole-rootfs must be opt-in") + } + for _, want := range []string{"pod-cache-dir", "allow-whole-rootfs"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error should tell the operator about %q, got: %v", want, err) + } + } +} + +// The override exists so an operator can still run that path deliberately. +// It must get past the guard -- failing later for an unrelated reason (no +// kube client in a unit test) is fine; failing *at the guard* is not. +func TestStartRootfsCaptureAllowsExplicitOptIn(t *testing.T) { + a := &Agent{} + _, err := a.startRootfsCapture(context.Background(), RootfsCaptureConfig{ + Enabled: true, + AllowWholeRootfs: true, + }) + if err != nil && strings.Contains(err.Error(), "whole-rootfs capture is not supported") { + t.Fatalf("explicit opt-in must pass the guard, got: %v", err) + } +} + +// Disabled stays a clean no-op: the guard must not turn "capture off" into an +// error for every agent that does not run capture at all. +func TestStartRootfsCaptureDisabledIsNoop(t *testing.T) { + a := &Agent{} + b, err := a.startRootfsCapture(context.Background(), RootfsCaptureConfig{Enabled: false}) + if err != nil || b != nil { + t.Fatalf("disabled capture should be a no-op, got backend=%v err=%v", b, err) + } +} diff --git a/src/compute-plane-services/nvsnap/internal/agent/rootfsonly_integration.go b/src/compute-plane-services/nvsnap/internal/agent/rootfsonly_integration.go index b2800d7db..cb92f32e3 100644 --- a/src/compute-plane-services/nvsnap/internal/agent/rootfsonly_integration.go +++ b/src/compute-plane-services/nvsnap/internal/agent/rootfsonly_integration.go @@ -57,6 +57,16 @@ type RootfsCaptureConfig struct { // replays the env stamped into the manifest. PodCacheEnvFile string + // AllowWholeRootfs permits capture to run with PodCacheDir empty, i.e. + // capturing the whole container rootfs instead of just the cache mount. + // + // Off by default, and the agent refuses to start capture without it, + // because the failure mode is silent: whole-rootfs looks like a working + // capture, and the difference only surfaces later as restores that + // behave unlike the ones that were benchmarked. Requiring an explicit + // opt-in makes running it a decision rather than an oversight. + AllowWholeRootfs bool + // CMNamespace is the K8s namespace ConfigMaps are written to so // any node's webhook can resolve a hash. Default "nvsnap-system". CMNamespace string @@ -84,6 +94,19 @@ func (a *Agent) startRootfsCapture(ctx context.Context, cfg RootfsCaptureConfig) if !cfg.Enabled { return nil, nil } + // Refuse whole-rootfs capture unless explicitly allowed. Capturing the + // entire container rootfs still "works" -- it produces a capture, restores + // succeed, and nothing looks wrong -- so a cluster that lost its cachedir + // setting would keep running and silently diverge from every workload and + // benchmark that assumes cachedir. Fail at startup, where an operator sees + // it, rather than at restore time, where it looks like a performance + // mystery. + if cfg.PodCacheDir == "" && !cfg.AllowWholeRootfs { + return nil, fmt.Errorf("rootfs capture is enabled without --pod-cache-dir: " + + "whole-rootfs capture is not supported for normal use. Set --pod-cache-dir " + + "(e.g. /opt/nvsnap) to capture the cache mount, or pass --allow-whole-rootfs " + + "to override deliberately") + } if cfg.CacheDir == "" { cfg.CacheDir = "/var/lib/nvsnap/cache" } diff --git a/src/compute-plane-services/nvsnap/internal/agent/rootfsonly_integration_test.go b/src/compute-plane-services/nvsnap/internal/agent/rootfsonly_integration_test.go index e0e76957c..188d70e4d 100644 --- a/src/compute-plane-services/nvsnap/internal/agent/rootfsonly_integration_test.go +++ b/src/compute-plane-services/nvsnap/internal/agent/rootfsonly_integration_test.go @@ -19,6 +19,7 @@ package agent import ( "context" + "strings" "testing" "github.com/sirupsen/logrus" @@ -38,8 +39,17 @@ func TestStartRootfsCapture_EnabledFailsWithoutKubeConfig(t *testing.T) { t.Setenv("KUBECONFIG", "/nonexistent/kubeconfig") t.Setenv("HOME", t.TempDir()) // hide any ~/.kube/config the test runner has a := &Agent{config: Config{}, log: logrus.New()} - _, err := a.startRootfsCapture(context.Background(), RootfsCaptureConfig{Enabled: true}) + // PodCacheDir is set so the whole-rootfs guard does not answer first. + // Without it this test would still fail -- but on the guard, not on the + // kube client it is named for, and the coverage would be gone silently. + _, err := a.startRootfsCapture(context.Background(), RootfsCaptureConfig{ + Enabled: true, + PodCacheDir: "/opt/nvsnap", + }) if err == nil { t.Fatal("expected kube client construction error when no config available") } + if strings.Contains(err.Error(), "whole-rootfs") { + t.Fatalf("guard fired instead of the kube client path; this test no longer covers what it claims: %v", err) + } } diff --git a/src/compute-plane-services/nvsnap/internal/server/manifests.go b/src/compute-plane-services/nvsnap/internal/server/manifests.go index 1785fbc83..29024ba7e 100644 --- a/src/compute-plane-services/nvsnap/internal/server/manifests.go +++ b/src/compute-plane-services/nvsnap/internal/server/manifests.go @@ -84,7 +84,20 @@ type CapturePath string // Capture path identifiers. const ( - CapturePathCRIU CapturePath = "criu" + CapturePathCRIU CapturePath = "criu" + + // CapturePathCacheDir captures only the pod's cache mount (model + + // compile caches) rather than the whole container rootfs. This is what + // multi-GPU workloads use. + CapturePathCacheDir CapturePath = "cachedir" + + // CapturePathRootfs captures the whole container rootfs. + // + // Deprecated: not used by any workload. It was the original multi-GPU + // path, and manifests kept declaring "rootfs" long after the agent's + // cachedir mode meant they were really capturing only the cache dir -- + // so the label described a path that was not running. That mismatch + // cost real debugging time. Kept only so an older manifest still parses. CapturePathRootfs CapturePath = "rootfs" ) diff --git a/src/compute-plane-services/nvsnap/scripts/README.md b/src/compute-plane-services/nvsnap/scripts/README.md index 4ccbc5d80..970e85555 100644 --- a/src/compute-plane-services/nvsnap/scripts/README.md +++ b/src/compute-plane-services/nvsnap/scripts/README.md @@ -2,40 +2,125 @@ SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. SPDX-License-Identifier: Apache-2.0 --> -# scripts/ - -Build, deploy, and test automation. Prefer these over ad-hoc `kubectl`/`docker` -commands — they encode setup that's easy to get wrong by hand. Key entry points -(grouped by purpose): - -## Versions -- [`versions.sh`](versions.sh) — **single source of truth** for image tags, - registry, and fork repos/refs. Sourced by every build/deploy script. -- [`sync-versions.sh`](sync-versions.sh) — stamp the current tags into the K8s - manifests. - -## Build -- [`build-agent.sh`](build-agent.sh) — agent base/app images (`base`, `app`, - `push-*`, `deploy`). -- [`build-deps.sh`](build-deps.sh), `build-libzmq-image.sh`, - `build-uvloop-wheel.sh`, `build-pyzmq-wheel.sh` — patched dependency images. -- CI builds every image via [`../ci/build-image.sh`](../ci/build-image.sh). - -## Deploy -- [`install-nvsnap.sh`](install-nvsnap.sh) — one-command cluster bootstrap - (namespace + pull secret + helm; `--without-webhook` to skip cert-manager). - -## Test / validate -- [`test-e2e.sh`](test-e2e.sh) `` — deploy → warm → capture → restore → - verify inference. The merge gate for capture/restore changes (`CLAUDE.md` - rule 10). `CAPTURE_PATH=rootfs` forces the rootfs/cachedir path. -- [`test-bench.sh`](test-bench.sh) `` — same flow with cold/capture/ - restore timings for the benchmark matrix. -- [`checkpoint.sh`](checkpoint.sh) — checkpoint-creation helper (exit 42 on a - rootfs redirect). - -## Rules - -Scripts live on disk and are version-controlled — never type build commands -ad-hoc in a terminal (`CLAUDE.md` rule 13). Keep `DOCKER_HOST` unset; never -`sudo` docker. + +# nvsnap test suite + +Two entry points: + +| Script | Answers | Writes | +|---|---|---| +| `test-e2e.sh ` | does capture/restore work for this workload | pass/fail + step timings to stdout | +| `test-bench.sh ` | how long does cold vs warm vs restore take | a row in `docs/PDF-BENCH-RESULTS.md` | + +Use `test-e2e.sh` to check correctness, `test-bench.sh` to produce numbers. + +## Before you run anything + +```sh +export KUBECONFIG= +kubectl get nodes # must work; expired credentials are the #1 cause of confusing failures +./scripts/test-e2e.sh # no args: prints the workload list +``` + +The suite refuses to start unless the deployed agent matches +`scripts/versions.sh`. That is deliberate: a run against a different build +produces numbers attributed to the wrong version. To test a build that is +already deployed, override rather than editing the file: + +```sh +NVSNAP_APP_VERSION=v0.2.46 ./scripts/test-e2e.sh vllm-small +``` + +## Running + +```sh +./scripts/test-e2e.sh vllm-small # single GPU, CRIU path, ~5 min +./scripts/test-e2e.sh vllm-70b # 4 GPUs, cachedir path, ~30 min +./scripts/test-bench.sh gpt-oss-120b # benchmark row instead of pass/fail +``` + +Both leave the source and restored pods in place on failure so you can inspect +them. On success they clean up. + +## Capture paths + +Which path runs is decided by GPU count, not by the manifest: + +- **1 GPU** -> `criu-v2`: CRIU + cuda-checkpoint of the live process, GPU state + included. +- **2+ GPUs** -> `cachedir`: capture the pod's cache mount (model weights, + compiled kernels). No process state. Multi-GPU CRIU does not work. + +Override with `CAPTURE_PATH=criu-v2` or `CAPTURE_PATH=rootfs` when you need the +other one. + +The `nvsnap.io/path` annotation in a workload manifest documents the intent; it +does not select the path. The agent's `--pod-cache-dir` flag is what decides +whether a `rootfs`-family capture is really cachedir. If those two disagree, +believe the flag. + +## Guards, and why a run may refuse to start + +These exist because each one has silently produced a wrong result before. +If a guard fires, it is telling you the run would have measured something other +than what you asked for. + +| Guard | Refuses when | Why | +|---|---|---| +| agent version | deployed image != `versions.sh` | numbers would be attributed to the wrong build | +| image exists | tag missing from the registry | catches a failed push before a 30 min run | +| placeholder | `__CAPTURE_HASH__` survived substitution | the webhook ignores the pod and it cold-starts | +| restore admitted | cache dir not mounted, or cache env points outside it | the pod cold-starts while looking like a restore | + +The restore-admitted guard is the one worth understanding. A pod the webhook +declined still starts, still serves, and passes every functional check -- it +just fetches its model again. Without the guard the run reports a restore time +that is really a cold start, and in `test-bench.sh` that number is published. + +Shared implementation: `scripts/lib/restore-guard.sh`, sourced by both scripts +so the contract cannot drift between them. + +## When a run fails + +```sh +kubectl get pods -n nvsnap-system # both pods are left behind +kubectl logs -n nvsnap-system --tail=100 +kubectl logs -n nvsnap-system -l app=nvsnap-agent -c agent --since=30m | grep -i capture +``` + +Capture and restore logs land next to the checkpoint on the node: + +```sh +AGENT=$(kubectl get pods -n nvsnap-system -l app=nvsnap-agent -o name | head -1) +kubectl exec -n nvsnap-system ${AGENT#pod/} -c agent -- \ + sh -c 'ls -1dt /var/lib/containerd/nvsnap-cache/*/ | head -3' +``` + +Copy anything you need out before re-running: a second run may reuse or replace +the capture, and the evidence goes with it. + +## Re-capturing + +Captures are content-addressed. A second run with the same pod identity reuses +the existing capture rather than making a new one -- normally what you want, and +confusing when you are trying to test the capture path itself. + +To force a fresh capture, remove what claims the hash: + +```sh +kubectl delete cm -n nvsnap-system nvsnap-capture- # manifest tier +kubectl delete pvc -n nvsnap-system rox- # L2 tier +``` + +Deleting only one tier is not enough: the agent skips the capture if any tier +claims the hash. A schema change bumps `CaptureFormatVersion`, which changes the +hash and re-captures everything automatically. + +## Adding a workload + +1. `deploy/k8s/workloads/.yaml` plus `-restore.yaml`. +2. Restore manifest carries `nvsnap.io/restore-from: "__CAPTURE_HASH__"`; the + scripts substitute it. +3. Add a `case` arm in both scripts with the pod names, port, model, and + inference payloads. +4. Annotate `nvsnap.io/gpus` accurately -- it selects the capture path. diff --git a/src/compute-plane-services/nvsnap/scripts/lib/restore-guard.sh b/src/compute-plane-services/nvsnap/scripts/lib/restore-guard.sh new file mode 100644 index 000000000..c6a085585 --- /dev/null +++ b/src/compute-plane-services/nvsnap/scripts/lib/restore-guard.sh @@ -0,0 +1,118 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Guards that stop a restore test from measuring a cold start. +# +# A pod the webhook declined still starts, still serves, and still passes every +# functional check -- it just fetches its model again. The timings then describe +# a cold start wearing a restore label, and nothing in the run says so. That is +# worse than a failure, because the number is plausible and gets quoted: in +# test-bench.sh it is written straight into the published results table. +# +# Sourced by test-e2e.sh and test-bench.sh so both enforce the same contract. + +# assert_no_placeholders +# +# Restore templates carry nvsnap.io/restore-from: "__CAPTURE_HASH__". If a +# placeholder survives substitution the webhook has nothing to resolve, injects +# nothing, and the pod cold-starts. +assert_no_placeholders() { + local manifest="$1" hits + # Comments are excluded deliberately. Templates name their own placeholders + # in explanatory comments ("test-e2e.sh substitutes __NODE_NAME__ from ..."), + # and matching those fails a correctly substituted manifest -- a guard that + # blocks good runs is worse than the problem it was added for. sed keeps the + # line count, so grep -n still reports true line numbers. + hits=$(sed 's/#.*//' "$manifest" | grep -nE '__[A-Z_]+__') + if [ -n "$hits" ]; then + printf '%s\n' "$hits" >&2 + echo "ERROR: unsubstituted placeholder(s) above in $manifest" >&2 + echo "ERROR: the webhook would ignore this pod and it would COLD START, not restore" >&2 + return 1 + fi + return 0 +} + +# agent_pod_cache_dir +# +# The cache path is the agent's, not ours to guess. Echoes the deployed +# --pod-cache-dir so callers follow a cluster configured differently instead of +# asserting a hardcoded default. Empty output means cachedir capture is not +# configured, which callers should treat as fatal for a restore test. +agent_pod_cache_dir() { + kubectl get ds nvsnap-agent -n nvsnap-system \ + -o jsonpath='{.spec.template.spec.containers[0].args}' 2>/dev/null \ + | tr ',' '\n' | sed -n 's|.*--pod-cache-dir=\([^"]*\).*|\1|p' | head -1 +} + +# assert_restore_admitted +# +# Proves the webhook decorated the pod as a restore: the configured cache dir is +# mounted, and the cache env points into it. Returns non-zero with the reasons +# on stderr otherwise. +assert_restore_admitted() { + local pod="$1" ns="$2" container="$3" cache_dir="$4" + local json rc + + if [ -z "$cache_dir" ]; then + echo "ERROR: agent has no --pod-cache-dir; cachedir capture is not configured" >&2 + return 1 + fi + + json=$(mktemp -t nvsnap-restore-pod.XXXXXX.json) || return 1 + local i + for i in $(seq 1 30); do + kubectl get pod "$pod" -n "$ns" -o json >"$json" 2>/dev/null && break + sleep 2 + done + + python3 - "$json" "$container" "$cache_dir" <<'PY' +import json, sys, posixpath + +pod_json, want, cache_dir = sys.argv[1], sys.argv[2], sys.argv[3].rstrip("/") +pod = json.load(open(pod_json)) +containers = pod["spec"]["containers"] + +# Fail closed on the container: falling back to containers[0] would let a +# decorated sidecar vouch for a workload that is cold-starting. +c = next((x for x in containers if x["name"] == want), None) +if c is None: + print(f" container {want!r} not found (have: {[x['name'] for x in containers]})", file=sys.stderr) + sys.exit(1) + +env = {e["name"]: e.get("value", "") for e in (c.get("env") or [])} +mounts = {m["mountPath"].rstrip("/") for m in (c.get("volumeMounts") or [])} + +def at_or_under(path, root): + # Exact match or a genuine child. Prefix matching alone would accept + # "/opt/nvsnap-other" for root "/opt/nvsnap". + path = path.rstrip("/") + return path == root or path.startswith(root + posixpath.sep) + +problems = [] +if cache_dir not in mounts: + problems.append(f"cache dir {cache_dir} is not mounted (mounts: {sorted(mounts)})") +# A restore pod that inherited none of the stamped cache env is not restoring +# from anything, so absent counts as a failure rather than "nothing to check". +if not any(v in env for v in ("HF_HOME", "NIM_CACHE_PATH")): + problems.append("no cache env (HF_HOME / NIM_CACHE_PATH) injected") +for var in ("HF_HOME", "NIM_CACHE_PATH"): + val = env.get(var) + if val and not at_or_under(val, cache_dir): + problems.append(f"{var}={val!r} points outside {cache_dir}") + +for p in problems: + print(f" {p}", file=sys.stderr) +sys.exit(1 if problems else 0) +PY + rc=$? + rm -f "$json" + if [ $rc -ne 0 ]; then + echo "ERROR: restore pod was NOT decorated by the webhook - it will COLD START" >&2 + echo "ERROR: any timing from this run would be a cold start labelled as a restore" >&2 + kubectl get pod "$pod" -n "$ns" \ + -o jsonpath='{.metadata.annotations.nvsnap\.io/restore-from}{"\n"}' 2>/dev/null \ + | sed 's/^/ restore-from: /' >&2 + fi + return $rc +} diff --git a/src/compute-plane-services/nvsnap/scripts/test-bench.sh b/src/compute-plane-services/nvsnap/scripts/test-bench.sh index 88c397ea1..2f2e4b192 100755 --- a/src/compute-plane-services/nvsnap/scripts/test-bench.sh +++ b/src/compute-plane-services/nvsnap/scripts/test-bench.sh @@ -32,6 +32,7 @@ set -euo pipefail SCRIPT_DIR="$(dirname "$(readlink -f "$0")")" PROJECT_ROOT="$(dirname "$SCRIPT_DIR")" +source "$SCRIPT_DIR/lib/restore-guard.sh" source "$SCRIPT_DIR/versions.sh" # ─── Global temp-file cleanup ──────────────────────────────────────────────── @@ -679,7 +680,16 @@ if [ "$SKIP_RESTORE" -eq 0 ] && [ -n "$CHECKPOINT_ID" ]; then -e "s|nodeName: __NODE_NAME__|nodeName: $NODE|" \ "$RESTORE_MANIFEST_TEMPLATE" > "$R_MANIFEST" fi + # A cold start measured here is published into PDF-BENCH-RESULTS.md as a + # restore row, so verify before spending the run rather than after. + assert_no_placeholders "$R_MANIFEST" || exit 1 kubectl apply -f "$R_MANIFEST" >/dev/null + if [ "$CAPTURE_PATH" = "rootfs" ]; then + POD_CACHE_DIR=$(agent_pod_cache_dir) + assert_restore_admitted "$RESTORE_POD_NAME" "$NAMESPACE" \ + "$RESTORE_CONTAINER_NAME" "$POD_CACHE_DIR" || exit 1 + log_info " verified: $POD_CACHE_DIR mounted, cache env points into it" + fi wait_ready "$RESTORE_POD_NAME" "$POD_READY_TIMEOUT" || { log_error "restore pod didn't ready"; exit 1; } verify_infer "$RESTORE_POD_NAME" "$RESTORE_CONTAINER_NAME" || log_warn " post-restore inference probe failed (continuing)" IFS=':' read -r RESTORE_CDL RESTORE_MDL RESTORE_INIT RESTORE_TOTAL <<<"$(measure_phase "$RESTORE_POD_NAME" "$RESTORE_CONTAINER_NAME" restore)" diff --git a/src/compute-plane-services/nvsnap/scripts/test-e2e.sh b/src/compute-plane-services/nvsnap/scripts/test-e2e.sh index d343134b6..aa8bc01e5 100755 --- a/src/compute-plane-services/nvsnap/scripts/test-e2e.sh +++ b/src/compute-plane-services/nvsnap/scripts/test-e2e.sh @@ -32,6 +32,7 @@ fi # Verify deployed agent matches expected version source "$SCRIPT_DIR/versions.sh" source "$SCRIPT_DIR/lib/agent-auth.sh" +source "$SCRIPT_DIR/lib/restore-guard.sh" DEPLOYED=$(kubectl get ds nvsnap-agent -n nvsnap-system -o jsonpath='{.spec.template.spec.containers[0].image}' 2>/dev/null) EXPECTED="${NVSNAP_REGISTRY}/nvsnap-agent:${NVSNAP_APP_VERSION}" if [ "$DEPLOYED" != "$EXPECTED" ]; then @@ -793,6 +794,10 @@ else "$RESTORE_MANIFEST_TEMPLATE" > "$RESTORE_MANIFEST" fi +# See scripts/lib/restore-guard.sh: a surviving placeholder leaves the webhook +# nothing to resolve, so the pod cold-starts while looking like a slow restore. +assert_no_placeholders "$RESTORE_MANIFEST" || exit 1 + # Phase 5d: restore goes back to the simple hostPath mount on the # capture-source node (or the agent's EnsureLocal cascade materializes # it locally on a different target node before the placeholder reads @@ -830,6 +835,26 @@ if [ "$CAPTURE_PATH" = "criu-v2" ]; then log_info "criu-v2: agent restore returned: $RESTORE_RESP" fi +# Prove the webhook admitted this pod AS A RESTORE before timing anything. +# +# A pod the webhook declined still starts, still serves, and still passes every +# check below -- it just cold-starts, downloading the model again. The timings +# then describe a cold start wearing a restore label, and nothing in the run +# says so. (Measured: a 70B "restore" that spent 8m47s downloading weights, +# because the pod carried no injected cache at all.) +# +# The observable signature of a real restore on the rootfs/cachedir path is the +# injected cache mount plus a cache env that points into it. +if [ "$CAPTURE_PATH" = "rootfs" ]; then + log_info "Verifying the restore pod was admitted as a restore..." + POD_CACHE_DIR=$(agent_pod_cache_dir) + if ! assert_restore_admitted "$RESTORE_POD_NAME" "$NAMESPACE" \ + "$RESTORE_CONTAINER_NAME" "$POD_CACHE_DIR"; then + fail "Restore pod not admitted as a restore" + fi + log_info " verified: $POD_CACHE_DIR is mounted and the cache env points into it" +fi + log_info "Waiting for restore pod ready (up to ${RESTORE_READY_TIMEOUT}s)..." log_info " (readiness probe polls /v1/models — succeeds only when serving)" if kubectl wait --for=condition=ready pod/$RESTORE_POD_NAME -n $NAMESPACE --timeout=${RESTORE_READY_TIMEOUT}s; then