From a382620879519ba16ca46328157ffe76896fe937 Mon Sep 17 00:00:00 2001
From: balaji <balaji7@gmail.com>
Date: Tue, 18 Aug 2026 13:02:17 -0700
Subject: [PATCH 1/7] test(nvsnap): refuse to measure a cold start as a restore

The restore step never checked that the pod it launched was admitted as a
restore. When it was not, the pod cold-started, served normally, passed
every downstream check, and the harness printed restore timings that were
really cold-start timings.

Rootfs/cachedir restore manifests carry nvsnap.io/restore-from:
"__CAPTURE_HASH__". Applying such a template without substituting leaves
the webhook nothing to resolve, so it injects no cache mount and no cache
env, and the workload fetches its model again. A 70B TP=4 "restore"
measured this way spent 8m47s downloading weights; the pod had no
/opt/nvsnap mount and HF_HOME still pointed at /root/.cache/huggingface.
The number looked plausible and was used to reason about restore
performance before the mistake surfaced.

Add two guards:

  - refuse to apply a restore manifest that still contains __PLACEHOLDER__
  - after creating the restore pod, assert the captured cache is mounted
    and the cache env points into it; fail loudly when it is not

Both were checked against the saved spec of the pod that produced the bad
number, and both fire on it.

A restore benchmark that silently degrades to a cold start is worse than
one that fails, because the result is quotable.

Closes #964

Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
---
 .../nvsnap/scripts/test-e2e.sh                | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/src/compute-plane-services/nvsnap/scripts/test-e2e.sh b/src/compute-plane-services/nvsnap/scripts/test-e2e.sh
index d343134b6..4e8f31a6a 100755
--- a/src/compute-plane-services/nvsnap/scripts/test-e2e.sh
+++ b/src/compute-plane-services/nvsnap/scripts/test-e2e.sh
@@ -793,6 +793,18 @@ else
         "$RESTORE_MANIFEST_TEMPLATE" > "$RESTORE_MANIFEST"
 fi
 
+# A template placeholder that survives substitution is not a cosmetic problem.
+# nvsnap.io/restore-from: "__CAPTURE_HASH__" gives the webhook nothing to
+# resolve, so it injects no storage and no cache env -- and the pod cold-starts
+# while looking exactly like a slow restore, model download included. Every
+# timing measured from that point is a cold-start number wearing a restore
+# label. Refuse to launch instead.
+if grep -nE '__[A-Z_]+__' "$RESTORE_MANIFEST"; then
+    log_error "Unsubstituted placeholder(s) above in $RESTORE_MANIFEST."
+    log_error "The webhook would ignore this pod and it would COLD START, not restore."
+    exit 1
+fi
+
 # 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 +842,54 @@ 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..."
+    for _ in $(seq 1 30); do
+        kubectl get pod "$RESTORE_POD_NAME" -n "$NAMESPACE" -o json >/tmp/nvsnap-restore-pod.json 2>/dev/null && break
+        sleep 2
+    done
+    if ! python3 - /tmp/nvsnap-restore-pod.json "$RESTORE_CONTAINER_NAME" <<'PY'
+import json, sys
+pod = json.load(open(sys.argv[1]))
+want = sys.argv[2]
+containers = pod["spec"]["containers"]
+c = next((x for x in containers if x["name"] == want), containers[0])
+env = {e["name"]: e.get("value", "") for e in (c.get("env") or [])}
+mounts = [m["mountPath"] for m in (c.get("volumeMounts") or [])]
+
+problems = []
+if not [m for m in mounts if m.startswith("/opt/nvsnap")]:
+    problems.append(f"no captured-cache mount injected (mounts: {mounts})")
+for var in ("HF_HOME", "NIM_CACHE_PATH"):
+    val = env.get(var)
+    if val and not val.startswith("/opt/nvsnap"):
+        problems.append(f"{var}={val!r} points outside the restored cache")
+if problems:
+    for p in problems:
+        print(f"  {p}", file=sys.stderr)
+    sys.exit(1)
+PY
+    then
+        log_error "Restore pod was NOT decorated by the webhook — it will COLD START."
+        log_error "Any timing from this run would be a cold start labelled as a restore."
+        kubectl get pod "$RESTORE_POD_NAME" -n "$NAMESPACE" \
+            -o jsonpath='{.metadata.annotations.nvsnap\.io/restore-from}{"\n"}' 2>/dev/null \
+            | sed 's/^/  restore-from: /'
+        fail "Restore pod not admitted as a restore"
+    fi
+    log_info "  verified: captured cache 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

From 69cc273819241907658c3c2947bd4d5410710554 Mon Sep 17 00:00:00 2001
From: balaji <balaji7@gmail.com>
Date: Tue, 18 Aug 2026 13:07:49 -0700
Subject: [PATCH 2/7] fix(nvsnap): declare cachedir, not rootfs, on every
 multi-GPU manifest

Four manifests declared nvsnap.io/path: "rootfs" while the agent, running
with cachedir mode on, captured only the pod's cache mount. The label
described a path that was not running.

That mismatch is not cosmetic. Reading the annotation is the natural way
to answer "which path did this use", and it gives the wrong answer, so
analysis built on it is wrong from the start -- including a benchmark
comparison this week that attributed a difference to capture path when
both runs used the same one.

Switch nim-qwen3-32b, vllm-tp2, vllm-70b and gpt-oss-120b to "cachedir",
add that value alongside criu, and mark rootfs deprecated: no workload
uses it. Also correct the descriptions that advertised whole-rootfs
behaviour -- vllm-70b claimed to capture the overlay upperdir, which
cachedir does not do.

The annotation feeds the demo catalog and the criu conformance check;
neither switches on "rootfs", so adding a value is safe.

Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
---
 .../k8s/benchmarks/gpt-oss-120b-restore.yaml      |  2 +-
 .../deploy/k8s/benchmarks/gpt-oss-120b.yaml       |  6 +++---
 .../deploy/k8s/workloads/nim-qwen3-32b.yaml       |  6 +++---
 .../nvsnap/deploy/k8s/workloads/vllm-70b.yaml     | 12 ++++++------
 .../nvsnap/deploy/k8s/workloads/vllm-tp2.yaml     |  2 +-
 .../nvsnap/internal/server/manifests.go           | 15 ++++++++++++++-
 6 files changed, 28 insertions(+), 15 deletions(-)

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/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"
 )
 

From acf41fe67cd373054aeb83fdbb8de6ca4759a74f Mon Sep 17 00:00:00 2001
From: balaji <balaji7@gmail.com>
Date: Tue, 18 Aug 2026 13:10:58 -0700
Subject: [PATCH 3/7] fix(nvsnap): refuse whole-rootfs capture unless
 explicitly allowed

Capture ran whole-rootfs whenever --pod-cache-dir was unset. That path
succeeds quietly: it produces a capture, restores work, and nothing looks
wrong -- so a cluster that lost its cachedir setting keeps running while
diverging from every workload and benchmark that assumes cachedir. The
difference only surfaces later as restores that behave unlike the ones
that were measured, which reads as a performance mystery rather than a
misconfiguration.

Refuse at startup instead, where an operator sees it, and say how to fix
it: set --pod-cache-dir, or pass --allow-whole-rootfs to run that path
deliberately. The override exists so this is a guard rather than a
removal; a deployment that genuinely needs whole-rootfs is not blocked
from a code change.

Tests cover all three states: refusal without the flag, the opt-in
getting past the guard, and capture-disabled staying a clean no-op so the
guard cannot turn "no capture" into a startup failure.

Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
---
 .../nvsnap/cmd/agent/main.go                  |  2 +
 .../nvsnap/internal/agent/BUILD.bazel         |  1 +
 .../agent/rootfs_wholerootfs_guard_test.go    | 67 +++++++++++++++++++
 .../internal/agent/rootfsonly_integration.go  | 23 +++++++
 4 files changed, 93 insertions(+)
 create mode 100644 src/compute-plane-services/nvsnap/internal/agent/rootfs_wholerootfs_guard_test.go

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/internal/agent/BUILD.bazel b/src/compute-plane-services/nvsnap/internal/agent/BUILD.bazel
index 4fcfefc3e..c94ea2a02 100644
--- a/src/compute-plane-services/nvsnap/internal/agent/BUILD.bazel
+++ b/src/compute-plane-services/nvsnap/internal/agent/BUILD.bazel
@@ -82,6 +82,7 @@ go_library(
 go_test(
     name = "agent_test",
     srcs = [
+        "rootfs_wholerootfs_guard_test.go",
         "advertise_test.go",
         "auth_test.go",
         "blob_uploader_test.go",
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"
 	}

From a55553211e048197bfdcd36b86e957cb3cfa0b31 Mon Sep 17 00:00:00 2001
From: balaji <balaji7@gmail.com>
Date: Tue, 18 Aug 2026 13:44:49 -0700
Subject: [PATCH 4/7] fix(nvsnap): harden the restore-verification guard

Review found the guard could pass while the workload cold-started.

It fell back to containers[0] when the named container was absent, so a
decorated sidecar could vouch for a workload that was not restoring. It
prefix-matched the cache path, so /opt/nvsnap-other satisfied a check for
/opt/nvsnap. It treated missing cache env as acceptable, though a pod that
inherited none of the stamped env is not restoring from anything. And it
hardcoded /opt/nvsnap rather than reading the agent's configured
--pod-cache-dir, so it asserted a default instead of the cluster's setup.

Now: require the exact named container, require the configured cache dir
to be mounted, require at least one cache variable, and accept a cache
path only when it equals that dir or lies beneath it.

Also use mktemp with an exit trap instead of a fixed /tmp path, which was
open to a symlink swap, and drop a non-ASCII character from a log line.

TestStartRootfsCapture_EnabledFailsWithoutKubeConfig was passing on the
new whole-rootfs guard rather than the kube client it is named for. Give
it a PodCacheDir so it reaches the kube client again, and assert it did
not stop at the guard, so the coverage cannot vanish silently a second
time.

Verified against the saved spec of the pod that produced the bad
measurement: still aborts, now for both the missing mount and the env
pointing outside the cache dir.

Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
---
 .../nvsnap/internal/agent/BUILD.bazel         |  2 +-
 .../agent/rootfsonly_integration_test.go      | 12 +++-
 .../nvsnap/scripts/test-e2e.sh                | 61 +++++++++++++++----
 3 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/src/compute-plane-services/nvsnap/internal/agent/BUILD.bazel b/src/compute-plane-services/nvsnap/internal/agent/BUILD.bazel
index c94ea2a02..48881a4dc 100644
--- a/src/compute-plane-services/nvsnap/internal/agent/BUILD.bazel
+++ b/src/compute-plane-services/nvsnap/internal/agent/BUILD.bazel
@@ -82,7 +82,6 @@ go_library(
 go_test(
     name = "agent_test",
     srcs = [
-        "rootfs_wholerootfs_guard_test.go",
         "advertise_test.go",
         "auth_test.go",
         "blob_uploader_test.go",
@@ -108,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/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/scripts/test-e2e.sh b/src/compute-plane-services/nvsnap/scripts/test-e2e.sh
index 4e8f31a6a..03d08ee4d 100755
--- a/src/compute-plane-services/nvsnap/scripts/test-e2e.sh
+++ b/src/compute-plane-services/nvsnap/scripts/test-e2e.sh
@@ -854,40 +854,75 @@ fi
 # 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..."
+
+    # The cache path is the agent's, not ours to guess: read the deployed
+    # --pod-cache-dir so this check follows a cluster that was configured
+    # differently instead of asserting a hardcoded default.
+    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)
+    if [ -z "$POD_CACHE_DIR" ]; then
+        log_error "Agent has no --pod-cache-dir; cachedir capture is not configured on this cluster."
+        fail "Restore verification cannot run without a configured cache dir"
+    fi
+
+    RESTORE_POD_JSON=$(mktemp -t nvsnap-restore-pod.XXXXXX.json)
+    trap 'rm -f "$RESTORE_POD_JSON"' EXIT
     for _ in $(seq 1 30); do
-        kubectl get pod "$RESTORE_POD_NAME" -n "$NAMESPACE" -o json >/tmp/nvsnap-restore-pod.json 2>/dev/null && break
+        kubectl get pod "$RESTORE_POD_NAME" -n "$NAMESPACE" -o json >"$RESTORE_POD_JSON" 2>/dev/null && break
         sleep 2
     done
-    if ! python3 - /tmp/nvsnap-restore-pod.json "$RESTORE_CONTAINER_NAME" <<'PY'
-import json, sys
-pod = json.load(open(sys.argv[1]))
-want = sys.argv[2]
+    if ! python3 - "$RESTORE_POD_JSON" "$RESTORE_CONTAINER_NAME" "$POD_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"]
-c = next((x for x in containers if x["name"] == want), containers[0])
+
+# 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"] for m in (c.get("volumeMounts") or [])]
+mounts = {m["mountPath"].rstrip("/"): m.get("name", "") 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 not [m for m in mounts if m.startswith("/opt/nvsnap")]:
-    problems.append(f"no captured-cache mount injected (mounts: {mounts})")
+if cache_dir not in mounts:
+    problems.append(f"cache dir {cache_dir} is not mounted (mounts: {sorted(mounts)})")
+
+# The cache env is what makes the engine reuse the capture instead of
+# fetching. Absent counts as a failure: a restore pod that inherited none
+# of the stamped env is not restoring from anything.
+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 val.startswith("/opt/nvsnap"):
-        problems.append(f"{var}={val!r} points outside the restored cache")
+    if val and not at_or_under(val, cache_dir):
+        problems.append(f"{var}={val!r} points outside {cache_dir}")
+
 if problems:
     for p in problems:
         print(f"  {p}", file=sys.stderr)
     sys.exit(1)
 PY
     then
-        log_error "Restore pod was NOT decorated by the webhook — it will COLD START."
+        log_error "Restore pod was NOT decorated by the webhook - it will COLD START."
         log_error "Any timing from this run would be a cold start labelled as a restore."
         kubectl get pod "$RESTORE_POD_NAME" -n "$NAMESPACE" \
             -o jsonpath='{.metadata.annotations.nvsnap\.io/restore-from}{"\n"}' 2>/dev/null \
             | sed 's/^/  restore-from: /'
         fail "Restore pod not admitted as a restore"
     fi
-    log_info "  verified: captured cache is mounted and the cache env points into it"
+    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)..."

From 0469cdaa757804f42173363da033aa3d4eadc14f Mon Sep 17 00:00:00 2001
From: balaji <balaji7@gmail.com>
Date: Tue, 18 Aug 2026 14:00:54 -0700
Subject: [PATCH 5/7] test(nvsnap): share the restore guard with test-bench.sh

test-bench.sh had the same blind spot as test-e2e.sh: it substituted the
capture hash, applied the manifest, and measured whatever started. A pod
the webhook declined cold-starts, serves, and passes every check, so the
run produces restore timings for a cold start.

It matters more here. test-bench.sh appends its numbers to
docs/PDF-BENCH-RESULTS.md, including a "Restore: Model DL" column -- so a
cold start does not just mislead the operator, it gets published as a
benchmark row and quoted later.

Move the checks into scripts/lib/restore-guard.sh and source it from both
scripts, so the contract has one implementation rather than two that can
drift:

  - assert_no_placeholders: refuse a manifest still carrying __PLACEHOLDER__
  - agent_pod_cache_dir:    read the deployed --pod-cache-dir instead of
                            assuming a default
  - assert_restore_admitted: require the exact named container, the
                            configured cache dir mounted, and cache env
                            pointing at or beneath it

Verified against the saved spec of the pod that produced the bad
measurement: refuses for the missing mount and for the env pointing
outside, and refuses on an unconfigured cache dir or a wrong container
name.

Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
---
 .../nvsnap/scripts/lib/restore-guard.sh       | 111 ++++++++++++++++++
 .../nvsnap/scripts/test-bench.sh              |  10 ++
 .../nvsnap/scripts/test-e2e.sh                |  70 +----------
 3 files changed, 125 insertions(+), 66 deletions(-)
 create mode 100644 src/compute-plane-services/nvsnap/scripts/lib/restore-guard.sh

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..45edbd3b0
--- /dev/null
+++ b/src/compute-plane-services/nvsnap/scripts/lib/restore-guard.sh
@@ -0,0 +1,111 @@
+# 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 <rendered-manifest>
+#
+# 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"
+    if grep -nE '__[A-Z_]+__' "$manifest"; then
+        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 <pod> <namespace> <container> <pod-cache-dir>
+#
+# 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 03d08ee4d..39963eac9 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
@@ -854,72 +855,9 @@ fi
 # 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..."
-
-    # The cache path is the agent's, not ours to guess: read the deployed
-    # --pod-cache-dir so this check follows a cluster that was configured
-    # differently instead of asserting a hardcoded default.
-    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)
-    if [ -z "$POD_CACHE_DIR" ]; then
-        log_error "Agent has no --pod-cache-dir; cachedir capture is not configured on this cluster."
-        fail "Restore verification cannot run without a configured cache dir"
-    fi
-
-    RESTORE_POD_JSON=$(mktemp -t nvsnap-restore-pod.XXXXXX.json)
-    trap 'rm -f "$RESTORE_POD_JSON"' EXIT
-    for _ in $(seq 1 30); do
-        kubectl get pod "$RESTORE_POD_NAME" -n "$NAMESPACE" -o json >"$RESTORE_POD_JSON" 2>/dev/null && break
-        sleep 2
-    done
-    if ! python3 - "$RESTORE_POD_JSON" "$RESTORE_CONTAINER_NAME" "$POD_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("/"): m.get("name", "") 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)})")
-
-# The cache env is what makes the engine reuse the capture instead of
-# fetching. Absent counts as a failure: a restore pod that inherited none
-# of the stamped env is not restoring from anything.
-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}")
-
-if problems:
-    for p in problems:
-        print(f"  {p}", file=sys.stderr)
-    sys.exit(1)
-PY
-    then
-        log_error "Restore pod was NOT decorated by the webhook - it will COLD START."
-        log_error "Any timing from this run would be a cold start labelled as a restore."
-        kubectl get pod "$RESTORE_POD_NAME" -n "$NAMESPACE" \
-            -o jsonpath='{.metadata.annotations.nvsnap\.io/restore-from}{"\n"}' 2>/dev/null \
-            | sed 's/^/  restore-from: /'
+    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"

From ea340ebcb7263ba7d8ecd44cbc230cc14ef0ce2d Mon Sep 17 00:00:00 2001
From: balaji <balaji7@gmail.com>
Date: Tue, 18 Aug 2026 14:11:49 -0700
Subject: [PATCH 6/7] docs(nvsnap): add a README for the test suite

Someone handed this suite had to read both scripts to learn which one to
run, what the capture paths mean, why a run refuses to start, and how to
force a re-capture. Write it down.

The guards table is the part worth having: each entry exists because that
check silently produced a wrong result before, so a guard firing is
information rather than an obstacle. Documents only what this branch
carries -- the capture-timeout guard belongs to a separate change and is
left out until it lands.

Also finish the refactor: test-e2e.sh still had its own copy of the
placeholder check rather than calling the shared one, which was the
duplication the shared lib was meant to remove.

Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
---
 .../nvsnap/scripts/README.md                  | 159 ++++++++++++++----
 .../nvsnap/scripts/test-e2e.sh                |  14 +-
 2 files changed, 125 insertions(+), 48 deletions(-)

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) `<workload>` — 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) `<workload>` — 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 <workload>` | does capture/restore work for this workload | pass/fail + step timings to stdout |
+| `test-bench.sh <workload>` | 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=<your cluster 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 <pod> -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-<hash-prefix>   # manifest tier
+kubectl delete pvc -n nvsnap-system rox-<hash>                    # 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/<id>.yaml` plus `<id>-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/test-e2e.sh b/src/compute-plane-services/nvsnap/scripts/test-e2e.sh
index 39963eac9..aa8bc01e5 100755
--- a/src/compute-plane-services/nvsnap/scripts/test-e2e.sh
+++ b/src/compute-plane-services/nvsnap/scripts/test-e2e.sh
@@ -794,17 +794,9 @@ else
         "$RESTORE_MANIFEST_TEMPLATE" > "$RESTORE_MANIFEST"
 fi
 
-# A template placeholder that survives substitution is not a cosmetic problem.
-# nvsnap.io/restore-from: "__CAPTURE_HASH__" gives the webhook nothing to
-# resolve, so it injects no storage and no cache env -- and the pod cold-starts
-# while looking exactly like a slow restore, model download included. Every
-# timing measured from that point is a cold-start number wearing a restore
-# label. Refuse to launch instead.
-if grep -nE '__[A-Z_]+__' "$RESTORE_MANIFEST"; then
-    log_error "Unsubstituted placeholder(s) above in $RESTORE_MANIFEST."
-    log_error "The webhook would ignore this pod and it would COLD START, not restore."
-    exit 1
-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

From f72f3f77428a231f17136ef9e54e4798d73f1299 Mon Sep 17 00:00:00 2001
From: balaji <balaji7@gmail.com>
Date: Tue, 18 Aug 2026 14:39:53 -0700
Subject: [PATCH 7/7] fix(nvsnap): do not treat a placeholder named in a
 comment as unsubstituted

The placeholder guard scanned the whole manifest, so a template that names
its own placeholder in an explanatory comment failed even when every value
was substituted correctly:

  25:  # test-e2e.sh substitutes __NODE_NAME__ from the source pod's status.
  ERROR: unsubstituted placeholder(s) above

That rejected a correct vllm-small restore and failed a run that would
otherwise have passed. A guard that blocks good runs is worse than the
problem it was added for, and this one was caught by the first suite run
rather than by me.

Strip comments before matching. Verified both directions: a placeholder
named only in a comment passes, a real unsubstituted value still fails,
including when a comment on the same line mentions one.

Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
---
 .../nvsnap/scripts/lib/restore-guard.sh               | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/compute-plane-services/nvsnap/scripts/lib/restore-guard.sh b/src/compute-plane-services/nvsnap/scripts/lib/restore-guard.sh
index 45edbd3b0..c6a085585 100644
--- a/src/compute-plane-services/nvsnap/scripts/lib/restore-guard.sh
+++ b/src/compute-plane-services/nvsnap/scripts/lib/restore-guard.sh
@@ -17,8 +17,15 @@
 # placeholder survives substitution the webhook has nothing to resolve, injects
 # nothing, and the pod cold-starts.
 assert_no_placeholders() {
-    local manifest="$1"
-    if grep -nE '__[A-Z_]+__' "$manifest"; then
+    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