|
|
@@ -25,16 +25,15 @@ pub(crate) struct DockerCommandCtx {
|
|
|
pub init_port: u16,
|
|
|
pub mods_remote_path: String,
|
|
|
/// Static `extra_mounts` (alias/host) with host paths already resolved
|
|
|
- /// once against `Config.mount_aliases` / `ssh_user`. Model-subfolder mounts
|
|
|
- /// are NOT here — `start_cluster` resolves those per node and merges the
|
|
|
- /// results into this `Vec` (as `ResolvedMount`s) before calling
|
|
|
- /// [`build_docker_command`].
|
|
|
+ /// once against `Config.mount_aliases` / `ssh_user`. Path references
|
|
|
+ /// (`repo`/`subfolder`) are NOT here — `start_cluster` resolves those
|
|
|
+ /// per-node and substitutes `{{<name>}}` into the recipe args/env before
|
|
|
+ /// calling [`build_docker_command`].
|
|
|
pub extra_mounts: Vec<ResolvedMount>,
|
|
|
}
|
|
|
|
|
|
/// A recipe `ExtraMount` after host-path resolution (alias + tilde), ready to
|
|
|
-/// turn into a `-v` flag / `-e` env pair. Also the final form a model-subfolder
|
|
|
-/// mount takes after per-node resolution.
|
|
|
+/// turn into a `-v` flag / `-e` env pair.
|
|
|
#[derive(Clone)]
|
|
|
pub(crate) struct ResolvedMount {
|
|
|
pub host: String,
|
|
|
@@ -43,29 +42,29 @@ pub(crate) struct ResolvedMount {
|
|
|
pub env: Option<String>,
|
|
|
}
|
|
|
|
|
|
-/// A model-subfolder mount spec (`repo`+`subfolder`) before per-node
|
|
|
-/// resolution. The host path is resolved at serve time from each node's HF
|
|
|
-/// cache (so it can differ across nodes); `container` is the fixed path the
|
|
|
-/// recipe references in args.
|
|
|
+/// A model path reference (`repo`+`subfolder`) before per-node resolution.
|
|
|
+/// At serve time, the launcher resolves the path on each node (reading
|
|
|
+/// `refs/main`) and substitutes `{{<name>}}` in the recipe args/env with the
|
|
|
+/// resolved full HF cache path. The HF cache is already mounted at
|
|
|
+/// `/root/.cache/huggingface`, so the resolved path's symlinks resolve
|
|
|
+/// correctly (a separate bind-mount would break them).
|
|
|
#[derive(Clone)]
|
|
|
-pub(crate) struct ModelMountSpec {
|
|
|
+pub(crate) struct PathRef {
|
|
|
pub repo: String,
|
|
|
pub subfolder: Option<String>,
|
|
|
- pub container: String,
|
|
|
- pub read_only: bool,
|
|
|
- pub env: Option<String>,
|
|
|
+ pub name: String,
|
|
|
}
|
|
|
|
|
|
/// Partition a recipe's `extra_mounts` into static (resolved now) and
|
|
|
-/// model-subfolder (deferred to per-node resolution). Validates that a mount
|
|
|
-/// is one or the other — `repo` set means `alias`/`host` must be empty, and
|
|
|
-/// `subfolder` without `repo` is an error.
|
|
|
+/// path references (deferred to per-node resolution + substitution).
|
|
|
+/// Validates that a mount is one or the other — `repo` set means
|
|
|
+/// `alias`/`host` must be empty, and `subfolder` without `repo` is an error.
|
|
|
pub(crate) fn resolve_extra_mounts(
|
|
|
config: &Config,
|
|
|
recipe: &Recipe,
|
|
|
) -> Result<ResolvedExtraMounts> {
|
|
|
let mut static_mounts = Vec::new();
|
|
|
- let mut model_mounts = Vec::new();
|
|
|
+ let mut path_refs = Vec::new();
|
|
|
for m in &recipe.extra_mounts {
|
|
|
if m.repo.as_deref().is_some_and(|r| !r.trim().is_empty()) {
|
|
|
if m.alias.is_some() || m.host.is_some() {
|
|
|
@@ -74,16 +73,14 @@ pub(crate) fn resolve_extra_mounts(
|
|
|
m.container
|
|
|
);
|
|
|
}
|
|
|
- model_mounts.push(ModelMountSpec {
|
|
|
+ path_refs.push(PathRef {
|
|
|
repo: m.repo.as_ref().unwrap().trim().to_string(),
|
|
|
subfolder: m
|
|
|
.subfolder
|
|
|
.as_deref()
|
|
|
.filter(|s| !s.trim().is_empty())
|
|
|
.map(|s| s.trim().to_string()),
|
|
|
- container: m.container.clone(),
|
|
|
- read_only: m.read_only,
|
|
|
- env: m.env.clone(),
|
|
|
+ name: m.container.clone(),
|
|
|
});
|
|
|
} else if m.subfolder.as_deref().is_some_and(|s| !s.trim().is_empty()) {
|
|
|
anyhow::bail!("extra_mount '{}': `subfolder` requires `repo`", m.container);
|
|
|
@@ -105,33 +102,42 @@ pub(crate) fn resolve_extra_mounts(
|
|
|
}
|
|
|
Ok(ResolvedExtraMounts {
|
|
|
static_mounts,
|
|
|
- model_mounts,
|
|
|
+ path_refs,
|
|
|
})
|
|
|
}
|
|
|
|
|
|
/// Result of [`resolve_extra_mounts`]: static mounts ready to clone per node,
|
|
|
-/// model-subfolder specs deferred to per-node resolution.
|
|
|
+/// path references deferred to per-node resolution + substitution.
|
|
|
pub(crate) struct ResolvedExtraMounts {
|
|
|
pub static_mounts: Vec<ResolvedMount>,
|
|
|
- pub model_mounts: Vec<ModelMountSpec>,
|
|
|
+ pub path_refs: Vec<PathRef>,
|
|
|
+}
|
|
|
+
|
|
|
+/// Substitute `{{<name>}}` placeholders in `args` and `env` with resolved paths.
|
|
|
+/// `path_refs` maps placeholder name → resolved path (per-node). Done before
|
|
|
+/// building the docker command so vLLM args reference the correct snapshot.
|
|
|
+pub(crate) fn substitute_path_refs(recipe: &Recipe, path_refs: &[(String, String)]) -> Recipe {
|
|
|
+ let mut r = recipe.clone();
|
|
|
+ for (name, path) in path_refs {
|
|
|
+ let placeholder = format!("{{{{{}}}}}", name);
|
|
|
+ r.args = r.args.substitute(&placeholder, path);
|
|
|
+ for v in r.env.values_mut() {
|
|
|
+ *v = v.replace(&placeholder, path);
|
|
|
+ }
|
|
|
+ }
|
|
|
+ r
|
|
|
}
|
|
|
|
|
|
-/// Dry-run placeholder for a model-subfolder mount: nodes may be off, so no
|
|
|
+/// Dry-run placeholder for a model path reference: nodes may be off, so no
|
|
|
/// HF cache to read. Emits a descriptive path so the printed `docker run`
|
|
|
-/// command shows *what* would be mounted, not a broken lookup.
|
|
|
-pub(crate) fn model_mount_placeholder(spec: &ModelMountSpec) -> ResolvedMount {
|
|
|
+/// command shows *what* would be substituted, not a broken lookup.
|
|
|
+pub(crate) fn path_ref_placeholder(spec: &PathRef) -> String {
|
|
|
let sub = spec.subfolder.as_deref().unwrap_or("");
|
|
|
- let host = format!(
|
|
|
+ format!(
|
|
|
"<hf_cache>/hub/{}/snapshots/<refs/main>/{}",
|
|
|
crate::config::model_cache_dir_name(&spec.repo),
|
|
|
sub
|
|
|
- );
|
|
|
- ResolvedMount {
|
|
|
- host,
|
|
|
- container: spec.container.clone(),
|
|
|
- read_only: spec.read_only,
|
|
|
- env: spec.env.clone(),
|
|
|
- }
|
|
|
+ )
|
|
|
}
|
|
|
|
|
|
/// Collect NCCL cluster env vars for multi-node deployments.
|
|
|
@@ -393,16 +399,16 @@ mod tests {
|
|
|
host: Some("~/.cache/models".into()),
|
|
|
repo: None,
|
|
|
subfolder: None,
|
|
|
- container: "/models:ro".into(),
|
|
|
+ container: "/models".into(),
|
|
|
read_only: true,
|
|
|
env: Some("MODEL_DIR".into()),
|
|
|
});
|
|
|
let cmd = run_with(&r);
|
|
|
// alias present in minimal_config() → alias host wins
|
|
|
- assert!(cmd.contains("-v /mnt/nas/models:/models:ro:ro"));
|
|
|
+ assert!(cmd.contains("-v /mnt/nas/models:/models:ro"));
|
|
|
assert!(cmd.contains("-v /home/user/.cache/huggingface:/root/.cache/huggingface"));
|
|
|
// env injected pointing at the container path, before recipe env
|
|
|
- assert!(cmd.contains("-e MODEL_DIR=/models:ro"));
|
|
|
+ assert!(cmd.contains("-e MODEL_DIR=/models"));
|
|
|
}
|
|
|
|
|
|
#[test]
|
|
|
@@ -424,40 +430,66 @@ mod tests {
|
|
|
}
|
|
|
|
|
|
#[test]
|
|
|
- fn model_subfolder_mount_uses_placeholder_in_dry_run() {
|
|
|
- // In build_docker_command (no per-node SSH resolution here), a model
|
|
|
- // mount shows the descriptive placeholder host so the printed command
|
|
|
- // documents what would be mounted. start_cluster replaces this with
|
|
|
- // the real resolved path at serve time (or keeps it under dry-run).
|
|
|
+ fn model_path_ref_is_partitioned_not_mounted() {
|
|
|
+ // repo-based mounts are path references, NOT bind-mounts. They are
|
|
|
+ // partitioned into path_refs and resolved per-node via substitution.
|
|
|
let mut r = base_recipe();
|
|
|
r.extra_mounts.push(ExtraMount {
|
|
|
alias: None,
|
|
|
host: None,
|
|
|
repo: Some("aidendle94/GLM-5.2-Hybrid-FP8-MXFP4".into()),
|
|
|
subfolder: Some("mtp-draft".into()),
|
|
|
- container: "/models/mtp-draft".into(),
|
|
|
+ container: "mtp_draft".into(),
|
|
|
read_only: true,
|
|
|
- env: Some("MTP_DRAFT_PATH".into()),
|
|
|
+ env: None,
|
|
|
});
|
|
|
- let cmd = run_with(&r);
|
|
|
- // model_mounts are NOT in extra_mounts here (run_with doesn't resolve
|
|
|
- // them per-node), so the -v flag for the model mount is absent — only
|
|
|
- // start_cluster emits it. Verify the spec is parsed + the env is NOT
|
|
|
- // injected yet (env injection follows extra_mounts, which is empty of
|
|
|
- // the model mount here). This test guards the partition.
|
|
|
- assert!(!cmd.contains("-v /models/mtp-draft"));
|
|
|
- assert!(!cmd.contains("MTP_DRAFT_PATH"));
|
|
|
- // But the recipe parsed into a model_mount spec:
|
|
|
let resolved = resolve_extra_mounts(&minimal_config(), &r).unwrap();
|
|
|
- assert_eq!(resolved.model_mounts.len(), 1);
|
|
|
+ assert_eq!(resolved.path_refs.len(), 1);
|
|
|
assert_eq!(resolved.static_mounts.len(), 0);
|
|
|
- let ph = model_mount_placeholder(&resolved.model_mounts[0]);
|
|
|
- assert_eq!(ph.container, "/models/mtp-draft");
|
|
|
- assert!(
|
|
|
- ph.host
|
|
|
- .contains("models--aidendle94--GLM-5.2-Hybrid-FP8-MXFP4")
|
|
|
+ let pr = &resolved.path_refs[0];
|
|
|
+ assert_eq!(pr.repo, "aidendle94/GLM-5.2-Hybrid-FP8-MXFP4");
|
|
|
+ assert_eq!(pr.subfolder.as_deref(), Some("mtp-draft"));
|
|
|
+ assert_eq!(pr.name, "mtp_draft");
|
|
|
+ // No -v flag for path refs in build_docker_command (substitution only)
|
|
|
+ let cmd = run_with(&r);
|
|
|
+ assert!(!cmd.contains("-v /models/mtp-draft"));
|
|
|
+ }
|
|
|
+
|
|
|
+ #[test]
|
|
|
+ fn substitute_path_refs_replaces_placeholders() {
|
|
|
+ let mut r = base_recipe();
|
|
|
+ r.args = RecipeArgs::List(vec![
|
|
|
+ "--speculative-config".into(),
|
|
|
+ "{\"model\":\"{{mtp_draft}}\"}".into(),
|
|
|
+ ]);
|
|
|
+ let path_refs = vec![(
|
|
|
+ "mtp_draft".to_string(),
|
|
|
+ "/root/.cache/huggingface/hub/models--aidendle94--GLM-5.2-Hybrid-FP8-MXPF4/snapshots/bb0d0858/mtp-draft".to_string(),
|
|
|
+ )];
|
|
|
+ let substituted = substitute_path_refs(&r, &path_refs);
|
|
|
+ if let RecipeArgs::List(args) = &substituted.args {
|
|
|
+ assert_eq!(
|
|
|
+ args[1],
|
|
|
+ "{\"model\":\"/root/.cache/huggingface/hub/models--aidendle94--GLM-5.2-Hybrid-FP8-MXPF4/snapshots/bb0d0858/mtp-draft\"}"
|
|
|
+ );
|
|
|
+ } else {
|
|
|
+ panic!("expected list args");
|
|
|
+ }
|
|
|
+ }
|
|
|
+
|
|
|
+ #[test]
|
|
|
+ fn substitute_path_refs_in_env() {
|
|
|
+ let mut r = base_recipe();
|
|
|
+ r.env
|
|
|
+ .insert("MTP_PATH".to_string(), "{{mtp_draft}}".to_string());
|
|
|
+ let path_refs = vec![(
|
|
|
+ "mtp_draft".to_string(),
|
|
|
+ "/root/.cache/huggingface/hub/models--aidendle94--GLM-5.2-Hybrid-FP8-MXPF4/snapshots/bb0d0858/mtp-draft".to_string(),
|
|
|
+ )];
|
|
|
+ let substituted = substitute_path_refs(&r, &path_refs);
|
|
|
+ assert_eq!(
|
|
|
+ substituted.env["MTP_PATH"],
|
|
|
+ "/root/.cache/huggingface/hub/models--aidendle94--GLM-5.2-Hybrid-FP8-MXPF4/snapshots/bb0d0858/mtp-draft"
|
|
|
);
|
|
|
- assert!(ph.host.contains("mtp-draft"));
|
|
|
- assert!(ph.read_only);
|
|
|
}
|
|
|
}
|