ci(checkpoint): mount shared model-cache PVC in DynamoCheckpoint tests - #12400
Merged
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
WalkthroughCheckpoint deployment specifications and CI workflows now support an optional shared model-cache server, path, and PVC. The test helper mounts the PVC and adjusts ChangesShared model cache integration
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/deploy/test_dynamocheckpoint.py (1)
276-279: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression test for cache environment precedence.
Verify that a PVC removes container-level
HF_HOME, sets deployment-levelHF_HOMEto the mount path, and adds the PVC mount; also verify that the no-PVC path preserves TRTLLM’s snapshotHF_HOME.Also applies to: 293-296
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/deploy/test_dynamocheckpoint.py` around lines 276 - 279, Extend the deployment tests around the container environment filtering and mount_model_cache_pvc flow to cover both cache modes: with a model cache PVC, assert container-level HF_HOME is removed, deployment-level HF_HOME points to the mount path, and the PVC mount is present; without a PVC, assert TRTLLM’s snapshot HF_HOME remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/deploy/test_dynamocheckpoint.py`:
- Around line 276-279: Extend the deployment tests around the container
environment filtering and mount_model_cache_pvc flow to cover both cache modes:
with a model cache PVC, assert container-level HF_HOME is removed,
deployment-level HF_HOME points to the mount path, and the PVC mount is present;
without a PVC, assert TRTLLM’s snapshot HF_HOME remains unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 98c98466-4bd2-4626-9b3e-6bc21560cac9
📒 Files selected for processing (2)
.github/workflows/pr.yamltests/deploy/test_dynamocheckpoint.py
This comment has been minimized.
This comment has been minimized.
galletas1712
force-pushed
the
ci/checkpoint-model-cache-pvc
branch
from
July 30, 2026 00:42
5980355 to
4c11947
Compare
Contributor
Author
|
/ok to test 4c11947 |
galletas1712
enabled auto-merge (squash)
July 30, 2026 00:45
Contributor
Author
|
/ok to test 4c11947 |
galletas1712
force-pushed
the
ci/checkpoint-model-cache-pvc
branch
from
July 30, 2026 01:52
4c11947 to
10a25da
Compare
Contributor
Author
|
/ok to test 10a25da |
galletas1712
force-pushed
the
ci/checkpoint-model-cache-pvc
branch
from
July 30, 2026 02:37
10a25da to
1884688
Compare
galletas1712
changed the title
Contributor
Author
|
/ok to test 1884688 |
github-actions
Bot
added
fix
backend::vllm
Contributor
Author
|
/ok to test 1884688 |
galletas1712
force-pushed
the
ci/checkpoint-model-cache-pvc
branch
from
July 30, 2026 06:40
1884688 to
3c7936f
Compare
galletas1712
changed the title
2 tasks
Contributor
Author
|
/ok to test 3c7936f |
github-actions
Bot
added
ci
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>
galletas1712
force-pushed
the
ci/checkpoint-model-cache-pvc
branch
from
July 30, 2026 19:13
3c7936f to
f2fbb78
Compare
Contributor
Author
|
/ok to test f2fbb78 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
model-cachePVC path regular deploy tests already use (setup-dynamo-operator+--model-cache-pvc).--model-cache-pvcintest_dynamocheckpoint.py, skipping TRTLLM containerHF_HOMEso it cannot shadow the shared cache.Validation
model-cacheand stop cold-fetching Qwen via HF/ModelExpress when the cache is populated.