-
Notifications
You must be signed in to change notification settings - Fork 1.5k
ci(checkpoint): cherry-pick mount shared model-cache PVC in DynamoCheckpoint tests (#12400) #12593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -168,9 +168,10 @@ class CheckpointBackendConfig: | |
| "--free-gpu-memory-fraction", | ||
| "0.10", | ||
| ), | ||
| # Keep the raw DGD PVC-free: the checkpoint operator mounts | ||
| # snapshot-pvc at /checkpoints for checkpoint/restore pods, so HF_HOME | ||
| # there preserves model files across restore without a model-cache PVC. | ||
| # UCX_TLS is always set. HF_HOME defaults to the snapshot PVC so restore | ||
| # pods keep weights without a model-cache PVC; when CI passes | ||
| # --model-cache-pvc, _new_checkpoint_spec skips this HF_HOME so the | ||
| # shared cache mount can own it (same as regular deploy tests). | ||
| env=(("UCX_TLS", "tcp,self"), ("HF_HOME", TRTLLM_HF_HOME)), | ||
| # Match the base TRTLLM snapshot recipe and avoid cold-worker/restore | ||
| # rollout overlap during initial DGD startup. | ||
|
|
@@ -233,6 +234,9 @@ def _new_checkpoint_spec( | |
| namespace: str, | ||
| image: str, | ||
| frontend_image: str, | ||
| *, | ||
| model_cache_pvc: str | None = None, | ||
| model_cache_mount: str | None = None, | ||
| ) -> DeploymentSpec: | ||
| spec_path = Path(_get_workspace_dir()).joinpath(*backend.manifest) | ||
| deployment_spec = DeploymentSpec(str(spec_path)) | ||
|
|
@@ -269,6 +273,10 @@ def _new_checkpoint_spec( | |
| if backend.env: | ||
| env = container.setdefault("env", []) | ||
| for name, value in backend.env: | ||
| # Container HF_HOME would shadow the deployment-level value that | ||
| # mount_model_cache_pvc sets; skip it when the shared cache is used. | ||
| if name == "HF_HOME" and model_cache_pvc: | ||
| continue | ||
| for item in env: | ||
| if item.get("name") == name: | ||
| item["value"] = value | ||
|
|
@@ -281,6 +289,11 @@ def _new_checkpoint_spec( | |
| checkpoint["targetContainerName"] = backend.target_container | ||
| if backend.checkpoint_startup_policy is not None: | ||
| checkpoint["startupPolicy"] = backend.checkpoint_startup_policy | ||
|
|
||
| if model_cache_pvc: | ||
| mount = model_cache_mount or "/models" | ||
| deployment_spec.mount_model_cache_pvc(model_cache_pvc, mount) | ||
|
Comment on lines
+293
to
+295
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Shared model cache is never actually attached in checkpoint tests, and TensorRT-LLM loses its model-storage setting The shared model cache is attached using an older manifest format ( Schema mismatch: mount_model_cache_pvc emits v1alpha1-only fields into a v1beta1 DGD
Unknown fields are pruned by the CRD structural schema, so the PVC is never referenced or mounted. Meanwhile the new skip at The pre-existing users of this helper ( Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| return deployment_spec | ||
|
|
||
|
|
||
|
|
@@ -594,6 +607,8 @@ async def test_dgd_checkpoint_restore_deploy( | |
| namespace=namespace, | ||
| image=image, | ||
| frontend_image=frontend_image, | ||
| model_cache_pvc=request.config.getoption("--model-cache-pvc") or None, | ||
| model_cache_mount=request.config.getoption("--model-cache-mount") or None, | ||
| ) | ||
|
|
||
| async with ManagedDeployment( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔍 Shared cache mount is read-only NFS while HF_HOME points at it
mount_model_cache_pvc(tests/utils/managed_deployment.py:494-512) setsHF_HOMEto the mount path, and the PV created by.github/actions/setup-dynamo-operator/action.yml:337-377is an NFS export shared across CI runs. If the HuggingFace client needs to write locks/metadata under HF_HOME (typical when a model is not fully present in the cache), checkpoint pods will attempt writes into the shared export. Worth confirming the export is writable or that the cache always contains Qwen3-0.6B, otherwise the checkpoint jobs may fail differently than the regular deploy tests, which have longer soak history with this mount.Was this helpful? React with 👍 or 👎 to provide feedback.