From 392117e77bef6977e9849a123613b352be54ff88 Mon Sep 17 00:00:00 2001 From: balaji Date: Mon, 17 Aug 2026 20:54:55 -0700 Subject: [PATCH 1/2] fix(nvsnap): keep captures on node-local disk, not the boot volume The agent wrote captures, staging and overlay upperdirs under /var/lib/nvsnap/*. On a typical cloud GPU node that is the boot volume, while containerd sits on the instance-local NVMe array -- so a capture was read from fast local disk and written to a slow network-backed one. Only hostPaths.checkpoints had already been pointed under the containerd root. A 70B cachedir capture is ~132 GB and took ~28 min, which matches that volume class's baseline throughput rather than local disk. The same bytes also accumulate on the root filesystem, so a couple of large captures on one node approaches kubelet disk-pressure eviction, which affects every pod on the node. Point nvsnapCache, nvsnapStaging, nvsnapOverlays and the L2 hostBundleRoot under the containerd root, and align the Go fallbacks (RootfsCaptureConfig.CacheDir, DefaultHostBundleRoot) with the chart so a chart-less deployment behaves the same. Operators whose containerd is not on the fastest local disk can still override. Putting source and destination on one filesystem is also what makes a future reflink possible. Also fix the e2e capture wait, which was a flat 15 min against a quantity that scales with model size: it failed the test while the agent was still succeeding. It now scales with the declared GPU count and honors NVSNAP_CAPTURE_TIMEOUT. The bundle-root test asserted a hardcoded literal, so it pinned the old path; it now derives from DefaultHostBundleRoot. Closes #936 Co-Authored-By: Balaji Ganesan --- .../nvsnap/templates/agent-daemonset.yaml | 12 +++++----- .../nvsnap/deploy/helm/nvsnap/values.yaml | 23 ++++++++++++++++--- .../internal/agent/rootfsonly_integration.go | 8 +++++-- .../internal/webhook/restore_entrypoint.go | 4 +++- .../webhook/restore_entrypoint_test.go | 13 +++++++---- .../nvsnap/scripts/test-e2e.sh | 13 +++++++++-- 6 files changed, 55 insertions(+), 18 deletions(-) diff --git a/src/compute-plane-services/nvsnap/deploy/helm/nvsnap/templates/agent-daemonset.yaml b/src/compute-plane-services/nvsnap/deploy/helm/nvsnap/templates/agent-daemonset.yaml index 644e08294..a9fc51e89 100644 --- a/src/compute-plane-services/nvsnap/deploy/helm/nvsnap/templates/agent-daemonset.yaml +++ b/src/compute-plane-services/nvsnap/deploy/helm/nvsnap/templates/agent-daemonset.yaml @@ -62,12 +62,12 @@ spec: - /criu-bundle/restore-bundle-init.sh env: - name: NVSNAP_BUNDLE_TOOLS_DST - value: {{ printf "%s/nvsnap" (.Values.agent.l2.hostBundleRoot | default "/var/lib/nvsnap/bundle") | quote }} + value: {{ printf "%s/nvsnap" (.Values.agent.l2.hostBundleRoot | default "/var/lib/containerd/nvsnap-bundle") | quote }} volumeMounts: - name: host-bundle # Mount the parent of {tools,lib} so the script can rename # siblings under it atomically. - mountPath: {{ .Values.agent.l2.hostBundleRoot | default "/var/lib/nvsnap/bundle" | quote }} + mountPath: {{ .Values.agent.l2.hostBundleRoot | default "/var/lib/containerd/nvsnap-bundle" | quote }} resources: requests: cpu: 100m @@ -118,7 +118,7 @@ spec: # path further down — the webhook patches kubelet pod # specs with paths built off this root, and kubelet runs # in host namespace. - - --overlay-root={{ .Values.agent.hostPaths.nvsnapOverlays | default "/var/lib/nvsnap/overlays" }} + - --overlay-root={{ .Values.agent.hostPaths.nvsnapOverlays | default "/var/lib/containerd/nvsnap-overlays" }} {{- if .Values.webhook.enabled }} # In-agent admission webhook. The Service in webhook.yaml # load-balances admission requests across agent pods. @@ -310,7 +310,7 @@ spec: # namespace — host path and container path must match # exactly or kubelet's hostPath type check fails. - name: nvsnap-overlays - mountPath: {{ .Values.agent.hostPaths.nvsnapOverlays | default "/var/lib/nvsnap/overlays" }} + mountPath: {{ .Values.agent.hostPaths.nvsnapOverlays | default "/var/lib/containerd/nvsnap-overlays" }} mountPropagation: Bidirectional {{- if .Values.webhook.enabled }} - name: webhook-tls @@ -384,7 +384,7 @@ spec: # vs 5.5TB local SSD). - name: nvsnap-overlays hostPath: - path: {{ .Values.agent.hostPaths.nvsnapOverlays | default "/var/lib/nvsnap/overlays" }} + path: {{ .Values.agent.hostPaths.nvsnapOverlays | default "/var/lib/containerd/nvsnap-overlays" }} type: DirectoryOrCreate # nvsnap#184: bundle-staging hostPath. The nvsnap-bundle-stage # initContainer writes /nvsnap (tools) + /nvsnap-lib (intercept @@ -393,7 +393,7 @@ spec: # restore_entrypoint.go for the inject patches. - name: host-bundle hostPath: - path: {{ .Values.agent.l2.hostBundleRoot | default "/var/lib/nvsnap/bundle" | quote }} + path: {{ .Values.agent.l2.hostBundleRoot | default "/var/lib/containerd/nvsnap-bundle" | quote }} type: DirectoryOrCreate {{- if .Values.webhook.enabled }} - name: webhook-tls diff --git a/src/compute-plane-services/nvsnap/deploy/helm/nvsnap/values.yaml b/src/compute-plane-services/nvsnap/deploy/helm/nvsnap/values.yaml index 4dfcc9496..bc5a67fca 100644 --- a/src/compute-plane-services/nvsnap/deploy/helm/nvsnap/values.yaml +++ b/src/compute-plane-services/nvsnap/deploy/helm/nvsnap/values.yaml @@ -228,8 +228,20 @@ agent: checkpoints: /var/lib/containerd/nvsnap-checkpoints containerdSock: /run/containerd/containerd.sock containerdStorage: /var/lib/containerd - nvsnapCache: /var/lib/nvsnap/cache - nvsnapStaging: /var/lib/nvsnap/staging + # Cache, staging and overlays sit under the containerd root on purpose. + # They are the bulk writers -- a 70B cachedir capture is ~132 GB -- and on + # a typical GPU node /var/lib is the boot volume (network-backed, ~125 + # MB/s) while containerd lives on the instance-local NVMe array. Writing + # captures to the boot volume was both slow (~28 min for that 132 GB) and + # a node-stability risk, since filling the root disk triggers kubelet disk + # pressure. Keeping these on containerd's filesystem also puts source and + # destination on one filesystem, which is what makes reflink possible. + # Override per-cluster if containerd is not on the fastest local disk. + nvsnapCache: /var/lib/containerd/nvsnap-cache + nvsnapStaging: /var/lib/containerd/nvsnap-staging + # Same reasoning, plus overlayfs needs an upperdir on a filesystem with + # d_type support -- containerd's own snapshotter filesystem qualifies. + nvsnapOverlays: /var/lib/containerd/nvsnap-overlays nvidiaRun: /run/nvidia hostUsr: /usr @@ -351,7 +363,12 @@ agent: # mirrored by the nvsnap-bundle-stage initContainer + the # host-bundle hostPath volume in the DaemonSet template (Helm # threads the same value into all three). - hostBundleRoot: "" + # Restore-bundle staging. Same reasoning as agent.hostPaths above: this is + # a bulk path on the restore side, so it belongs on containerd's local + # disk rather than the boot volume. Set explicitly (not left empty) so the + # chart passes --webhook-host-bundle-root and the agent, the DaemonSet + # mount and the webhook-injected function-pod mounts all agree on one path. + hostBundleRoot: "/var/lib/containerd/nvsnap-bundle" # Opt-in cross-cluster replication to a managed object store (provider: # gcs today, s3 planned). Empty provider/homeBucket disables it entirely. 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..59fb39ee8 100644 --- a/src/compute-plane-services/nvsnap/internal/agent/rootfsonly_integration.go +++ b/src/compute-plane-services/nvsnap/internal/agent/rootfsonly_integration.go @@ -40,7 +40,7 @@ type RootfsCaptureConfig struct { Enabled bool // CacheDir is the root the Local Backend writes captured trees into. - // Default "/var/lib/nvsnap/cache". + // Default "/var/lib/containerd/nvsnap-cache". CacheDir string // PodCacheDir, when non-empty (e.g. "/opt/nvsnap"), switches capture to @@ -85,7 +85,11 @@ func (a *Agent) startRootfsCapture(ctx context.Context, cfg RootfsCaptureConfig) return nil, nil } if cfg.CacheDir == "" { - cfg.CacheDir = "/var/lib/nvsnap/cache" + // Under the containerd root, not /var/lib directly: on a typical GPU + // node the latter is the boot volume while containerd sits on local + // NVMe, and captures are large enough that the difference is both a + // throughput and a disk-pressure problem. + cfg.CacheDir = "/var/lib/containerd/nvsnap-cache" } if cfg.CMNamespace == "" { cfg.CMNamespace = "nvsnap-system" diff --git a/src/compute-plane-services/nvsnap/internal/webhook/restore_entrypoint.go b/src/compute-plane-services/nvsnap/internal/webhook/restore_entrypoint.go index 8bf13bd43..7efe909c3 100644 --- a/src/compute-plane-services/nvsnap/internal/webhook/restore_entrypoint.go +++ b/src/compute-plane-services/nvsnap/internal/webhook/restore_entrypoint.go @@ -112,7 +112,9 @@ const ( // knob, by design: changing the path requires coordinated // edits in both places, which a single config value can't // safely express. - DefaultHostBundleRoot = "/var/lib/nvsnap/bundle" + // Under the containerd root so the bundle lands on the node's local disk + // rather than the boot volume, matching agent.hostPaths in the chart. + DefaultHostBundleRoot = "/var/lib/containerd/nvsnap-bundle" // envOrigCommand and envOrigArgs are read by restore-entrypoint's // cold-start fallback path (cmd/restore-entrypoint/main.go). diff --git a/src/compute-plane-services/nvsnap/internal/webhook/restore_entrypoint_test.go b/src/compute-plane-services/nvsnap/internal/webhook/restore_entrypoint_test.go index 35e2d8c2d..c515051c1 100644 --- a/src/compute-plane-services/nvsnap/internal/webhook/restore_entrypoint_test.go +++ b/src/compute-plane-services/nvsnap/internal/webhook/restore_entrypoint_test.go @@ -114,8 +114,12 @@ func TestRestoreBundle_HostPathInjected(t *testing.T) { } if v.Name == nvsnapToolsVolumeName { saw.volumeTools = true - if v.HostPath.Path != "/var/lib/nvsnap/bundle/nvsnap" { - t.Errorf("nvsnap-tools hostPath = %q, want /var/lib/nvsnap/bundle/nvsnap", v.HostPath.Path) + // Derived from the constant, not a literal: the point of the + // assertion is that the injected path tracks the configured + // bundle root, not that it equals one particular directory. + want := DefaultHostBundleRoot + "/nvsnap" + if v.HostPath.Path != want { + t.Errorf("nvsnap-tools hostPath = %q, want %q", v.HostPath.Path, want) } if v.HostPath.Type == nil || *v.HostPath.Type != corev1.HostPathDirectory { t.Errorf("nvsnap-tools hostPath.type = %v, want Directory (fail-fast if agent hasn't staged)", v.HostPath.Type) @@ -123,8 +127,9 @@ func TestRestoreBundle_HostPathInjected(t *testing.T) { } if v.Name == nvsnapLibVolumeName { saw.volumeLib = true - if v.HostPath.Path != "/var/lib/nvsnap/bundle/nvsnap-lib" { - t.Errorf("nvsnap-lib hostPath = %q, want /var/lib/nvsnap/bundle/nvsnap-lib", v.HostPath.Path) + want := DefaultHostBundleRoot + "/nvsnap-lib" + if v.HostPath.Path != want { + t.Errorf("nvsnap-lib hostPath = %q, want %q", v.HostPath.Path, want) } if v.HostPath.Type == nil || *v.HostPath.Type != corev1.HostPathDirectory { t.Errorf("nvsnap-lib hostPath.type = %v, want Directory", v.HostPath.Type) diff --git a/src/compute-plane-services/nvsnap/scripts/test-e2e.sh b/src/compute-plane-services/nvsnap/scripts/test-e2e.sh index d343134b6..b2d522ce5 100755 --- a/src/compute-plane-services/nvsnap/scripts/test-e2e.sh +++ b/src/compute-plane-services/nvsnap/scripts/test-e2e.sh @@ -639,7 +639,16 @@ if [ "$CAPTURE_PATH" = "rootfs" ]; then log_info "Step 7: Waiting for rootfs capture (watcher auto-fires post-Ready + warmup)..." POD_UID=$(kubectl get pod $POD_NAME -n $NAMESPACE -o jsonpath='{.metadata.uid}') HASH="" - DEADLINE=$(( $(date +%s) + 900 )) # 15 min — large captures take time + # Capture time scales with what has to be written, not with wall-clock + # patience: a 70B TP=4 cache dir is ~132 GB and takes ~28 min, so the old + # flat 15 min failed the test while the agent was still succeeding. Scale + # off the declared GPU count (a good enough proxy for model size) and let + # the caller override outright. + CAPTURE_GPUS=$(kubectl get pod $POD_NAME -n $NAMESPACE \ + -o jsonpath='{.metadata.annotations.nvsnap\.io/gpus}' 2>/dev/null) + [ -n "$CAPTURE_GPUS" ] || CAPTURE_GPUS=1 + CAPTURE_TIMEOUT="${NVSNAP_CAPTURE_TIMEOUT:-$(( 900 + 900 * (CAPTURE_GPUS - 1) ))}" + DEADLINE=$(( $(date +%s) + CAPTURE_TIMEOUT )) while [ "$(date +%s)" -lt "$DEADLINE" ]; do # Pick the most-recent ConfigMap whose manifest.json source_pod_meta # matches this pod (by name + namespace). Multiple may exist if @@ -672,7 +681,7 @@ if matches: sleep 10 done if [ -z "$HASH" ]; then - fail "Rootfs capture (no manifest CM appeared within 15min)" + fail "Rootfs capture (no manifest CM appeared within $(( CAPTURE_TIMEOUT / 60 ))min)" fi CHECKPOINT_ID="$HASH" # downstream uses this name uniformly log_info "Capture hash: $HASH" From c260435d02eae6a8b16255ce7dedf4a82e6ef9c9 Mon Sep 17 00:00:00 2001 From: balaji Date: Tue, 18 Aug 2026 07:17:51 -0700 Subject: [PATCH 2/2] fix(nvsnap): validate the capture-timeout inputs before arithmetic Bash evaluates a non-numeric string as 0 inside $(( )), so a hand-edited or malformed nvsnap.io/gpus annotation ("abc", "1.5", "0", "-1") made the timeout 900 + 900*(0-1) = 0. The capture wait then expired immediately and failed the step while the agent was still working -- the same test-fails-while-product-works trap this timeout was added to fix. Validate both the annotation and NVSNAP_CAPTURE_TIMEOUT, falling back to the single-GPU default and warning when the override is unusable. Co-Authored-By: Balaji Ganesan --- .../nvsnap/scripts/test-e2e.sh | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/compute-plane-services/nvsnap/scripts/test-e2e.sh b/src/compute-plane-services/nvsnap/scripts/test-e2e.sh index b2d522ce5..e5222e32f 100755 --- a/src/compute-plane-services/nvsnap/scripts/test-e2e.sh +++ b/src/compute-plane-services/nvsnap/scripts/test-e2e.sh @@ -646,8 +646,21 @@ if [ "$CAPTURE_PATH" = "rootfs" ]; then # the caller override outright. CAPTURE_GPUS=$(kubectl get pod $POD_NAME -n $NAMESPACE \ -o jsonpath='{.metadata.annotations.nvsnap\.io/gpus}' 2>/dev/null) - [ -n "$CAPTURE_GPUS" ] || CAPTURE_GPUS=1 + # Both inputs are attacker-of-convenience: a hand-edited annotation or a + # typo'd env var. Validate rather than feed them to $(( )), which + # evaluates a non-numeric string as 0 -- that would make the timeout + # 900 + 900*(0-1) = 0 and fail this step instantly, the same + # test-fails-while-product-works trap this timeout was added to fix. + case "$CAPTURE_GPUS" in + '' | *[!0-9]* | 0) CAPTURE_GPUS=1 ;; + esac CAPTURE_TIMEOUT="${NVSNAP_CAPTURE_TIMEOUT:-$(( 900 + 900 * (CAPTURE_GPUS - 1) ))}" + case "$CAPTURE_TIMEOUT" in + '' | *[!0-9]* | 0) + log_warn "NVSNAP_CAPTURE_TIMEOUT=${CAPTURE_TIMEOUT} is not a positive integer; using 900s" + CAPTURE_TIMEOUT=900 + ;; + esac DEADLINE=$(( $(date +%s) + CAPTURE_TIMEOUT )) while [ "$(date +%s)" -lt "$DEADLINE" ]; do # Pick the most-recent ConfigMap whose manifest.json source_pod_meta