Skip to content

fix(test): mount model-cache PVC on v1beta1 pod templates - #12419

Closed
galletas1712 wants to merge 2 commits into
mainfrom
ci/checkpoint-model-cache-v1beta1-mount
Closed

fix(test): mount model-cache PVC on v1beta1 pod templates#12419
galletas1712 wants to merge 2 commits into
mainfrom
ci/checkpoint-model-cache-v1beta1-mount

Conversation

@galletas1712

@galletas1712 galletas1712 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Follow-up to ci(checkpoint): mount shared model-cache PVC in DynamoCheckpoint tests #12400: DynamoCheckpoint CI uses v1beta1 DGDs, but mount_model_cache_pvc only wrote legacy volumeMounts, so checkpoint source pods never inherited the Azure model cache.
  • Mount the PVC onto native podTemplate volumes/volumeMounts (including non-main checkpoint target containers) and set HF_HOME accordingly.
  • Unit coverage for the v1beta1 mount path.

Depends on #12400 (stacked). Does not include the vLLM os._exit capture change — that stays in #12184.

Validation


Open in Devin Review

Summary by CodeRabbit

  • New Features

    • Added optional shared model-cache support for checkpoint and restore deployments.
    • Deployments can mount a shared model cache at a configurable path.
    • Improved support for checkpoint target containers when configuring model-cache mounts.
  • Bug Fixes

    • Preserved compatibility across deployment schema versions.
    • Added validation for missing checkpoint target containers to prevent invalid deployments.
  • Tests

    • Added coverage for model-cache mounting, schema compatibility, idempotent configuration, and validation errors.

Checkpoint deploy CI was downloading Qwen via ModelExpress/HF on every
run while regular deploy tests already used the Azure model-cache PVC.
Wire the same provision + mount path through, and honor --model-cache-pvc
in the checkpoint test harness (including skipping TRTLLM's container
HF_HOME override so it cannot shadow the shared cache).

Signed-off-by: Schwinn Saereesitthipitak <schwinns@nvidia.com>
DynamoCheckpoint CI uses v1beta1 DGDs. The shared model-cache helper only
wrote legacy volumeMounts, so checkpoint source pods never saw HF_HOME on
the Azure cache and fell back to HF downloads. Mount the PVC onto native
podTemplate volumes/volumeMounts (and non-main checkpoint targets) so the
cache is inherited by operator-generated checkpoint pods.

Signed-off-by: Schwinn Saereesitthipitak <schwinns@nvidia.com>
@galletas1712
galletas1712 requested review from a team as code owners July 30, 2026 06:40
@galletas1712

Copy link
Copy Markdown
Contributor Author

Not needed — #12400 was restored to the narrow PVC-only change; the expanded tip had inadvertently picked up #12184's snapshot exit fix (plus extra test harness work). Closing this split.

@galletas1712
galletas1712 deleted the ci/checkpoint-model-cache-v1beta1-mount branch July 30, 2026 06:42
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c76d0727-c14a-45cd-b04f-395cd5c21f37

📥 Commits

Reviewing files that changed from the base of the PR and between 0caba6a and 53dacc0.

📒 Files selected for processing (4)
  • .github/workflows/pr.yaml
  • tests/deploy/test_dynamocheckpoint.py
  • tests/utils/managed_deployment.py
  • tests/utils/test_managed_deployment.py

Walkthrough

Shared model-cache server, path, and PVC settings are now propagated through DynamoCheckpoint CI workflows and deployment tests. Deployment utilities support schema-specific PVC mounting, checkpoint target containers, idempotence, and validation for v1beta1 while retaining v1alpha1 behavior.

Changes

Shared model-cache checkpoint flow

Layer / File(s) Summary
Schema-aware PVC mounting
tests/utils/managed_deployment.py, tests/utils/test_managed_deployment.py
v1beta1 deployments mount the PVC through pod-template volumes and container mounts, set HF_HOME, validate checkpoint target containers, and preserve existing v1alpha1 fields.
Checkpoint deployment integration
tests/deploy/test_dynamocheckpoint.py
Checkpoint spec generation accepts optional PVC and mount settings, omits conflicting container-level HF_HOME, and forwards pytest configuration.
CI model-cache wiring
.github/workflows/pr.yaml
vLLM, SGLang, and TRTLLM setup and deploy steps pass model-cache endpoint settings and conditionally enable the model-cache PVC.

Estimated code review effort: 3 (Moderate) | ~20 minutes


Comment @coderabbitai help to get the list of available commands.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant