فهرست منبع

config: global-config mgmt commands + ssh creds from cluster sources

Addressing "the global config path is not transparent" + duplicated
ssh credentials:

- `sm121 config path`: shows the active config (./config.toml vs global
  vs --config) and where the global config lives (platform config dir,
  e.g. ~/Library/Application Support/sm121 on macOS) + whether it exists.
- `sm121 config init-global`: writes a template if missing (never
  overwrites; -d shows the target).
- `ssh_user`/`ssh_key_path` are now `serde(default)` on Config and may
  be carried by `cluster_sources` files: merged into the main config
  when it leaves them empty (first source wins, main wins over sources).
  The global config on this machine is now sources-only.
- `ssh_session_with_timeout` bails with a clean hint if credentials are
  still unset at connect time.
- `resolve_config_path` moved to the new src/gconfig.rs module.

AGENTS.md: config subcommand + updated resolution/source docs.
Verified: real SSH (health) from $HOME with a sources-only global config;
repo-dir runs unchanged.
Alexander Korolev 2 ماه پیش
والد
کامیت
4275e1cd7c
8فایلهای تغییر یافته به همراه178 افزوده شده و 52 حذف شده
  1. 12 9
      AGENTS.md
  2. 13 0
      src/cli.rs
  3. 12 0
      src/config.rs
  4. 12 0
      src/config/tests.rs
  5. 10 11
      src/config/types.rs
  6. 106 0
      src/gconfig.rs
  7. 7 32
      src/main.rs
  8. 6 0
      src/ssh.rs

+ 12 - 9
AGENTS.md

@@ -17,7 +17,7 @@ See [`DEVELOPMENT.md`](DEVELOPMENT.md) for architecture, module layout, and dev
 ## Commands
 
 ```
-serve/stop/logs/health/doctor/exec/power/tmux/init-host-keys
+serve/stop/logs/health/doctor/exec/power/tmux/init-host-keys/config
 ```
 
 Cluster: exact name, prefix (`4`→`4nodes-switch`), or `auto` (smallest with NCCL + matching `required_nodes`).
@@ -417,18 +417,21 @@ why each decision was made, and how to re-benchmark.
 
 Config resolution: `--config <path>` > `./config.toml` (cwd) > the global
 config (`~/.config/sm121/config.toml` on Linux, `~/Library/Application
-Support/sm121/config.toml` on macOS). The global config exists so `sm121`
-runs from **any cwd**: it holds `ssh_user`/`ssh_key_path` plus the two
-source lists below (it may carry no `[[clusters]]` of its own — clusters
-may come entirely from `cluster_sources`).
+Support/sm121/config.toml` on macOS — see `sm121 config path`).
+`sm121 config init-global` creates a template if missing. The global config
+exists so `sm121` runs from **any cwd**: it holds the two source lists below
+(and may omit `ssh_user`/`ssh_key_path` — a `cluster_sources` file provides
+them when the main config leaves them empty); it may carry no
+`[[clusters]]` of its own — clusters may come entirely from `cluster_sources`.
 
 ```toml
-ssh_user = "u"
-ssh_key_path = "/path"              # PathBuf
+ssh_user = "u"                   # may be empty — a cluster source can provide it
+ssh_key_path = "/path"            # PathBuf; same
 
 recipe_sources = ["/path/to/recipes"]       # extra recipe dirs (each `<name>/recipe.toml`); searched after ./recipes
-cluster_sources = ["/path/clusters.toml"]   # extra cluster def files: `[[clusters]]` (+ `[nccl.<name>]` /
-                                            # `[before_serve.<name>]` tables); merged by name, this file wins
+cluster_sources = ["/path/clusters.toml"]   # extra cluster def files: `ssh_user`/`ssh_key_path`,
+                                            # `[[clusters]]`, `[nccl.<name>]`, `[before_serve.<name>]`;
+                                            # merged by name, this file wins
 
 [[clusters]]
 name = "c"

+ 13 - 0
src/cli.rs

@@ -189,6 +189,10 @@ pub enum Commands {
         command: Vec<String>,
     },
 
+    /// Manage configuration (global config path, init)
+    #[command(subcommand)]
+    Config(ConfigCommands),
+
     /// Build Docker image for a recipe on a remote node
     Build {
         #[arg(long, required = true, help = "Recipe name")]
@@ -243,6 +247,15 @@ pub enum PowerAction {
     },
 }
 
+/// `sm121 config` subcommands — manage the global config file.
+#[derive(Subcommand)]
+pub enum ConfigCommands {
+    /// Show the active config path and where the global config lives
+    Path,
+    /// Create the global config file (template) if it does not exist
+    InitGlobal,
+}
+
 #[derive(Subcommand)]
 pub enum TmuxCommands {
     /// Launch cluster via tmux

+ 12 - 0
src/config.rs

@@ -59,6 +59,12 @@ pub fn resolve_cluster_name<'a>(clusters: &'a [Cluster], name: &str) -> Result<&
 /// the main [`Config`] by [`merge_cluster_sources`].
 #[derive(Deserialize, Default)]
 pub struct ClusterSource {
+    /// SSH credentials for this source's clusters; merged into the main
+    /// config only when it leaves them empty (first source wins).
+    #[serde(default)]
+    pub ssh_user: Option<String>,
+    #[serde(default)]
+    pub ssh_key_path: Option<std::path::PathBuf>,
     #[serde(default)]
     pub clusters: Vec<Cluster>,
     #[serde(default)]
@@ -78,6 +84,12 @@ pub async fn merge_cluster_sources(config: &mut Config) -> Result<()> {
             .with_context(|| format!("cluster source '{}'", path.display()))?;
         let src: ClusterSource = toml::from_str(&content)
             .with_context(|| format!("invalid cluster source '{}'", path.display()))?;
+        if config.ssh_user.is_empty() {
+            config.ssh_user = src.ssh_user.clone().unwrap_or_default();
+        }
+        if config.ssh_key_path.as_os_str().is_empty() {
+            config.ssh_key_path = src.ssh_key_path.clone().unwrap_or_default();
+        }
         for c in src.clusters {
             if !config.clusters.iter().any(|x| x.name == c.name) {
                 config.clusters.push(c);

+ 12 - 0
src/config/tests.rs

@@ -333,6 +333,9 @@ async fn test_merge_cluster_sources_adds_new_keeps_main() {
     std::fs::write(
         &src_path,
         r#"
+ssh_user = "srcuser"
+ssh_key_path = "/src/key"
+
 [[clusters]]
 name = "extra"
 nodes = [{ name = "e0", ssh_ip = "10.9.0.1", rank = 0 }]
@@ -353,7 +356,11 @@ commands = ["echo hi"]
     .unwrap();
     let mut cfg = tcfg();
     cfg.cluster_sources = vec![src_path.to_string_lossy().into_owned()];
+    cfg.ssh_user = String::new();
+    cfg.ssh_key_path = std::path::PathBuf::new();
     merge_cluster_sources(&mut cfg).await.unwrap();
+    assert_eq!(cfg.ssh_user, "srcuser", "empty ssh_user filled from source");
+    assert_eq!(cfg.ssh_key_path, std::path::PathBuf::from("/src/key"));
     assert_eq!(
         cfg.clusters.len(),
         2,
@@ -371,6 +378,11 @@ commands = ["echo hi"]
     assert!(cfg.clusters.iter().any(|c| c.name == "extra"));
     assert!(cfg.nccl.contains_key("extra"));
     assert!(cfg.before_serve.contains_key("extra"));
+    // A main-config ssh_user is never overridden by a source.
+    let mut cfg2 = tcfg();
+    cfg2.cluster_sources = vec![src_path.to_string_lossy().into_owned()];
+    merge_cluster_sources(&mut cfg2).await.unwrap();
+    assert_eq!(cfg2.ssh_user, "u", "main ssh_user wins over source");
 }
 
 #[tokio::test]

+ 10 - 11
src/config/types.rs

@@ -55,7 +55,10 @@ pub struct Cluster {
 
 #[derive(Clone, Deserialize)]
 pub struct Config {
+    /// SSH user; may be empty — a `cluster_sources` file can provide it.
+    #[serde(default)]
     pub ssh_user: String,
+    #[serde(default)]
     pub ssh_key_path: std::path::PathBuf,
     #[serde(default)]
     pub clusters: Vec<Cluster>,
@@ -76,11 +79,8 @@ pub struct Config {
     /// Drop page cache before starting containers.
     #[serde(default = "default_drop_caches")]
     pub drop_caches: bool,
-    /// Named mount-alias registry — host paths referenced by name from recipe
-    /// `[[extra_mounts]]` entries. Lets recipes stay portable (reference an
-    /// alias like `jit_cache`) while the operator sets the real path here.
-    /// A recipe may also carry a `host` default; the alias, when present,
-    /// overrides it.
+    /// Mount-alias registry: recipe `[[extra_mounts]]` `alias` resolves here;
+    /// the alias wins over the recipe's `host` default. Keeps recipes portable.
     #[serde(default)]
     pub mount_aliases: HashMap<String, String>,
     /// Extra recipe dirs (each `<name>/recipe.toml`) searched after `./recipes`; `~` expanded.
@@ -237,9 +237,8 @@ pub struct Recipe {
     pub extra_mounts: Vec<ExtraMount>,
 }
 
-/// Recipe-level build configuration. Currently only `versions` (a map of
-/// `versions.env` pin overrides) is supported; the launcher merges these
-/// over the image's own `versions.env` before building.
+/// Recipe-level build config. Only `versions` (a map of `versions.env` pin
+/// overrides) is supported; merged over the image's `versions.env` before build.
 #[derive(Clone, Debug, Default, Deserialize)]
 pub struct RecipeBuild {
     /// Pins merged over the image's `versions.env` (recipe wins). Keys are
@@ -251,9 +250,9 @@ pub struct RecipeBuild {
 
 /// A composed model directory: the launcher builds a `/tmp` dir per-node
 /// with symlinks into the HF cache (container-side absolute paths) +
-/// generated files, then bind-mounts it into the container. Lets a recipe
-/// combine multiple HF repos (e.g. a base checkpoint + an expert overlay)
-/// without mutating the HF cache.
+/// generated files, then bind-mounts it into the container — combining
+/// multiple HF repos (e.g. a base checkpoint + an expert overlay) without
+/// mutating the HF cache.
 ///
 /// Symlinks point to `/root/.cache/huggingface/...` (the container-side HF
 /// cache mount), not the host-side path. They are dangling on the host but

+ 106 - 0
src/gconfig.rs

@@ -0,0 +1,106 @@
+//! Global config management: path resolution, `config path` display,
+//! `config init-global` scaffold.
+
+use crate::config::default_config_dir;
+use std::path::PathBuf;
+
+/// Global config template written by `config init-global`.
+const GLOBAL_TEMPLATE: &str = r#"# Global sm121 config (created by `sm121 config init-global`).
+# Used when ./config.toml is absent, so `sm121` works from any cwd.
+# `ssh_user` / `ssh_key_path` may instead be provided by a cluster source.
+# `~` is expanded.
+#
+# recipe_sources = ["/path/to/recipes"]       # extra recipe dirs
+# cluster_sources = ["/path/to/clusters.toml"] # extra cluster def files
+"#;
+
+/// Resolve the path to the config file.
+///
+/// Search order:
+/// 1. `--config <path>` from CLI (always wins; error if missing)
+/// 2. `./config.toml` in cwd
+/// 3. `<default_config_dir>/config.toml`
+///
+/// Returns the explicit path if nothing is found; the loader reports a
+/// clean error.
+pub fn resolve_config_path(cli_config: &str) -> PathBuf {
+    let explicit = PathBuf::from(cli_config);
+    // User passed a non-default path: use it as-is.
+    if cli_config != "config.toml" {
+        return explicit;
+    }
+    // Default value (or no flag): try cwd first, then default folder.
+    if explicit.exists() {
+        return explicit;
+    }
+    if let Some(dir) = default_config_dir() {
+        let candidate = dir.join("config.toml");
+        if candidate.exists() {
+            return candidate;
+        }
+    }
+    explicit
+}
+
+/// `sm121 config path`: print the active config and the global config
+/// location (the part that is not transparent — it lives in the
+/// platform config dir, e.g. `~/Library/Application Support/sm121` on macOS).
+pub fn show_paths(cli_config: &str) {
+    let active = resolve_config_path(cli_config);
+    let origin = if cli_config != "config.toml" {
+        "--config"
+    } else if std::path::Path::new("config.toml").exists() {
+        "./config.toml (cwd)"
+    } else {
+        "global config dir"
+    };
+    println!("active config:  {}  [{}]", active.display(), origin);
+    if let Some(dir) = default_config_dir() {
+        let global = dir.join("config.toml");
+        let state = if global.exists() {
+            "exists"
+        } else {
+            "missing — `sm121 config init-global` creates a template"
+        };
+        println!("global config:  {}  [{}]", global.display(), state);
+    }
+}
+
+/// `sm121 config init-global`: create the global config (template) if
+/// missing. Never overwrites an existing file.
+pub fn init_global(dry_run: bool) -> anyhow::Result<()> {
+    let dir = default_config_dir().ok_or_else(|| {
+        anyhow::anyhow!("cannot determine the platform config dir (dirs::config_dir)")
+    })?;
+    let global = dir.join("config.toml");
+    if global.exists() {
+        println!("global config already exists: {}", global.display());
+        return Ok(());
+    }
+    if dry_run {
+        println!("would write: {}", global.display());
+        return Ok(());
+    }
+    std::fs::create_dir_all(&dir)?;
+    std::fs::write(&global, GLOBAL_TEMPLATE)?;
+    println!("created global config template: {}", global.display());
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::resolve_config_path;
+
+    #[test]
+    fn explicit_config_path_wins() {
+        assert_eq!(
+            resolve_config_path("/tmp/explicit-sm121.toml"),
+            std::path::PathBuf::from("/tmp/explicit-sm121.toml")
+        );
+    }
+
+    #[test]
+    fn default_config_dir_is_some() {
+        assert!(crate::config::default_config_dir().is_some());
+    }
+}

+ 7 - 32
src/main.rs

@@ -17,6 +17,7 @@ mod doctor;
 mod ends;
 mod engine_flags;
 mod exec;
+mod gconfig;
 mod health;
 mod home_assistant;
 mod host_keys;
@@ -29,41 +30,11 @@ mod tmux;
 
 use crate::cli::{Cli, Commands, PowerAction};
 use crate::config::{
-    Cluster, Config, DeployTarget, default_config_dir, load_recipe, resolve_cluster,
-    select_cluster_by_name,
+    Cluster, Config, DeployTarget, load_recipe, resolve_cluster, select_cluster_by_name,
 };
 use crate::ssh::fetch_host_key;
 use std::collections::HashSet;
 
-/// Resolve the path to the config file.
-///
-/// Search order:
-/// 1. `--config <path>` from CLI (always wins; error if missing)
-/// 2. `./config.toml` in cwd
-/// 3. `<default_config_dir>/config.toml`
-///
-/// Returns an error if nothing is found (hint: copy the example config).
-fn resolve_config_path(cli_config: &str) -> std::path::PathBuf {
-    use std::path::PathBuf;
-    let explicit = PathBuf::from(cli_config);
-    // User passed a non-default path: use it as-is.
-    if cli_config != "config.toml" {
-        return explicit;
-    }
-    // Default value (or no flag): try cwd first, then default folder.
-    if explicit.exists() {
-        return explicit;
-    }
-    if let Some(dir) = default_config_dir() {
-        let candidate = dir.join("config.toml");
-        if candidate.exists() {
-            return candidate;
-        }
-    }
-    // Fall through; the loader will report a clean error.
-    explicit
-}
-
 #[tokio::main]
 async fn main() -> Result<()> {
     // Locale: check LANG env var first (macOS sys-locale ignores it).
@@ -83,7 +54,7 @@ async fn main() -> Result<()> {
         .init();
     let cli = Cli::parse();
 
-    let config_path = resolve_config_path(&cli.config);
+    let config_path = crate::gconfig::resolve_config_path(&cli.config);
     let raw_config = tokio::fs::read_to_string(&config_path).await?;
     let mut config: Config = toml::from_str(&raw_config)?;
     crate::config::merge_cluster_sources(&mut config).await?;
@@ -189,6 +160,10 @@ async fn main() -> Result<()> {
             )
             .await?;
         }
+        Commands::Config(cmd) => match cmd {
+            crate::cli::ConfigCommands::Path => crate::gconfig::show_paths(&cli.config),
+            crate::cli::ConfigCommands::InitGlobal => crate::gconfig::init_global(dry_run)?,
+        },
         Commands::Exec {
             cluster,
             recipe,

+ 6 - 0
src/ssh.rs

@@ -34,6 +34,12 @@ async fn ssh_session_with_timeout(
     node: &Node,
     inactivity_timeout: Option<Duration>,
 ) -> Result<client::Handle<VerifyingHandler>> {
+    if config.ssh_user.is_empty() || config.ssh_key_path.as_os_str().is_empty() {
+        anyhow::bail!(
+            "ssh_user/ssh_key_path not set — no ./config.toml, and neither the global \
+             config nor a cluster source provides them (see `sm121 config path`)"
+        );
+    }
     let key = load_secret_key(&config.ssh_key_path)?;
     let ssh_config = russh::client::Config {
         inactivity_timeout,