Browse Source

flare: honour the pinned orbit controller and match orbit node names by name (0.2.6)

- serve/stop now use [flare.clusters.<c>].controller_node (or the flat pin) as
  the FLARE CLI host, instead of always our cluster rank 0. They diverge once
  the orbit is re-paired or reordered: prod4's rank 0 is now dgx4 (its 2 TB is
  the only disk holding the 488 GB store plus the 488 GB agent cache).
- orbit_node_name matched by NAME: orbit_nodes is in the orbit's order, so
  indexing it with our rank named gx10-dgx3 for a dgx4 controller.
- dry-run prints the package-path shape instead of a bogus '.' source.
- split orbit/cluster mapping into native/flare_orbit.rs (500-line gate).
Alexander Korolev 1 day ago
parent
commit
f3548d85e8
5 changed files with 308 additions and 108 deletions
  1. 13 0
      CHANGELOG.md
  2. 15 107
      src/native/flare.rs
  3. 169 0
      src/native/flare_orbit.rs
  4. 110 1
      src/native/flare_tests.rs
  5. 1 0
      src/native/mod.rs

+ 13 - 0
CHANGELOG.md

@@ -22,6 +22,19 @@ own version and its own `sm121/CHANGELOG.md`; do not mix them.
   serves the wrong model silently).
   serves the wrong model silently).
 - Companion probing is done inside the package's own directory in both layouts
 - Companion probing is done inside the package's own directory in both layouts
   (a blob store has no `weights/` at the mirror root).
   (a blob store has no `weights/` at the mirror root).
+- **`serve`/`stop` honour the pinned FLARE controller.** `[flare.clusters.<c>]
+  .controller_node` (or the flat `[flare].controller_node`) now decides which
+  node runs `flare download` / `orbit run`, instead of always our cluster rank 0.
+  The two diverge as soon as the orbit is re-paired or reordered — dgx4 became
+  the orbit's rank 0 for `prod4` because its 2 TB disk is the only one that
+  holds the 488 GB package store *plus* the 488 GB agent cache, and the old
+  behaviour reached the wrong machine, failing with
+  "… is not an authorized controller of this node".
+- **Orbit node names are matched by name, not by our rank.** `orbit_nodes` lists
+  the orbit's ranks in the orbit's order (`gx10-dgx4` first when dgx4 is the
+  orbit's rank 0), so indexing it with our rank named the wrong node
+  (`gx10-dgx3` for a dgx4 controller). A list that does not name our nodes
+  keeps the positional fallback.
 - Detail + measurements: `specs/tech-architecture/flare-bringup.md`
 - Detail + measurements: `specs/tech-architecture/flare-bringup.md`
   ("Package distribution: CLI store vs agent cache").
   ("Package distribution: CLI store vs agent cache").
 
 

+ 15 - 107
src/native/flare.rs

@@ -25,8 +25,13 @@
 use anyhow::{Result, bail};
 use anyhow::{Result, bail};
 use tracing::info;
 use tracing::info;
 
 
-use crate::config::{Config, Node, Recipe};
+use crate::config::{Config, Recipe};
 use crate::docker::{ServeOpts, shq};
 use crate::docker::{ServeOpts, shq};
+use crate::native::flare_orbit::{controller_for, extra_flags_for};
+// Re-exported: the driver is the FLARE entry point the rest of the launcher uses.
+pub use crate::native::flare_orbit::{
+    local_dir_for, orbit_cluster_for, orbit_cluster_paired, orbit_node_name,
+};
 
 
 /// FLARE API port on the controller (recipe `[flare].http_port`, else the
 /// FLARE API port on the controller (recipe `[flare].http_port`, else the
 /// cluster default 30000 — the same port every other lane serves on).
 /// cluster default 30000 — the same port every other lane serves on).
@@ -38,101 +43,6 @@ pub fn http_port(recipe: &Recipe) -> u16 {
         .unwrap_or(30000)
         .unwrap_or(30000)
 }
 }
 
 
-/// Orbit cluster name for one of OUR clusters: the per-cluster mapping wins,
-/// then the flat `[flare].orbit_cluster`, then the default. This is what makes
-/// one launcher config work on `dgx58-switch` and `prod4` alike.
-pub fn orbit_cluster_for(config: &Config, cluster: &str) -> String {
-    let f = config.flare.as_ref();
-    f.and_then(|f| f.clusters.get(cluster))
-        .and_then(|c| c.orbit_cluster.clone())
-        .or_else(|| f.and_then(|f| f.orbit_cluster.clone()))
-        .unwrap_or_else(|| "sparks".to_string())
-}
-
-/// The orbit cluster a **specific** cluster is paired with, or `None` when the
-/// config does not say (the flat `[flare].orbit_cluster` is then only a
-/// fallback for `serve`, not evidence of a pairing).
-///
-/// This is the identification rule the dashboard and `health` use: a cluster
-/// whose own nodes were never paired with an orbit must not be probed with the
-/// flat default (`sparks`) — `6nodes-switch`, `1node-*`, `ds4` and `qwen38`
-/// share nodes with the paired clusters, so asking one of them `--cluster
-/// sparks` made the sparks lane look like a lane of *that* cluster.
-pub fn orbit_cluster_paired(config: &Config, cluster: &str) -> Option<String> {
-    let f = config.flare.as_ref()?;
-    if let Some(own) = f.clusters.get(cluster) {
-        // A per-cluster block without an orbit name inherits the flat one —
-        // the operator already said "this cluster speaks FLARE".
-        return own
-            .orbit_cluster
-            .clone()
-            .or_else(|| f.orbit_cluster.clone());
-    }
-    // Once the map exists it is the source of truth: the flat block describes
-    // the machine it was written for, and our clusters share nodes (dgx5 is
-    // rank 0 of `dgx58-switch` AND rank 4 of `6nodes-switch`), so a flat
-    // `sparks` would pair `6nodes-switch` too.
-    if !f.clusters.is_empty() {
-        return None;
-    }
-    let flat = f.orbit_cluster.clone()?;
-    match f.controller_node.as_deref() {
-        // Legacy flat block: it speaks for the cluster that holds its
-        // controller node.
-        Some(ctl) => config
-            .clusters
-            .iter()
-            .find(|c| c.name == cluster)
-            .is_some_and(|c| c.nodes.iter().any(|n| n.name == ctl))
-            .then_some(flat),
-        // No controller either: unambiguous only with a single cluster.
-        None => (config.clusters.len() == 1 && config.clusters[0].name == cluster).then_some(flat),
-    }
-}
-
-/// Extra `orbit run` flags for a cluster (per-cluster map first, then flat).
-/// Fabric shape is a cluster property. The prod 4-node switch uses no extra
-/// flag: FLARE finds the star links itself once the legacy CX7 ports are down;
-/// `--roce-ring` was a symptom-level fix and is retired.
-pub fn extra_flags_for(config: &Config, cluster: &str) -> Vec<String> {
-    let Some(f) = config.flare.as_ref() else {
-        return Vec::new();
-    };
-    let per = f
-        .clusters
-        .get(cluster)
-        .map(|c| c.extra_flags.clone())
-        .unwrap_or_default();
-    if per.is_empty() {
-        f.extra_flags.clone()
-    } else {
-        per
-    }
-}
-
-/// Local package mirror dir for a cluster (per-cluster map first).
-pub(crate) fn local_dir_for(config: &Config, cluster: &str) -> Option<String> {
-    let f = config.flare.as_ref()?;
-    f.clusters
-        .get(cluster)
-        .and_then(|c| c.local_dir.clone())
-        .or_else(|| f.local_dir.clone())
-}
-
-/// The orbit-side name for one of our nodes (rank order → `orbit_nodes`,
-/// falling back to the cluster's own name).
-pub(crate) fn orbit_node_name(config: &Config, cluster: &str, node: &Node) -> String {
-    let f = config.flare.as_ref();
-    let per_cluster = f
-        .and_then(|f| f.clusters.get(cluster))
-        .map(|c| c.orbit_nodes.as_slice())
-        .filter(|v| !v.is_empty());
-    per_cluster
-        .or_else(|| f.map(|f| f.orbit_nodes.as_slice()))
-        .and_then(|v| v.get(node.rank as usize).cloned())
-        .unwrap_or_else(|| node.name.clone())
-}
-
 pub(crate) fn binary(config: &Config) -> String {
 pub(crate) fn binary(config: &Config) -> String {
     config
     config
         .flare
         .flare
@@ -325,14 +235,6 @@ pub fn parse_package_path(stdout: &str) -> Option<String> {
         .map(str::to_string)
         .map(str::to_string)
 }
 }
 
 
-fn head(cluster: &crate::config::Cluster) -> Result<&Node> {
-    cluster
-        .nodes
-        .iter()
-        .find(|n| n.rank == 0)
-        .ok_or_else(|| anyhow::anyhow!("no head node (rank 0) in cluster"))
-}
-
 /// Launch a FLARE recipe: preflight, package download on the controller, then
 /// Launch a FLARE recipe: preflight, package download on the controller, then
 /// `orbit run` detached. Readiness (`/health`, `/v1/models`) is polled by the
 /// `orbit run` detached. Readiness (`/health`, `/v1/models`) is polled by the
 /// caller, like every other engine.
 /// caller, like every other engine.
@@ -342,7 +244,7 @@ pub async fn serve(
     cluster: &crate::config::Cluster,
     cluster: &crate::config::Cluster,
     dry_run: bool,
     dry_run: bool,
 ) -> Result<()> {
 ) -> Result<()> {
-    let controller = head(cluster)?;
+    let controller = controller_for(config, cluster)?;
     let orbit = orbit_cluster_for(config, &cluster.name);
     let orbit = orbit_cluster_for(config, &cluster.name);
     if dry_run {
     if dry_run {
         println!(
         println!(
@@ -422,7 +324,11 @@ pub async fn serve(
         // Dry-run: print the download and mirror commands, no SSH.
         // Dry-run: print the download and mirror commands, no SSH.
         let dl = download_cmd(config, recipe)?;
         let dl = download_cmd(config, recipe)?;
         package = crate::ssh::run_ssh_long_running(config, controller, dl, true).await?;
         package = crate::ssh::run_ssh_long_running(config, controller, dl, true).await?;
-        package = parse_package_path(&package).unwrap_or_default();
+        // The path is printed by a real download; in a dry run there is none,
+        // so show the shape (a store-relative `models/<version>/model.safetensors`,
+        // which is what the mirror shape depends on).
+        package = parse_package_path(&package)
+            .unwrap_or_else(|| "<package-store>/models/<version>/model.safetensors".into());
         if let Some(dir) = local_dir_for(config, &cluster.name) {
         if let Some(dir) = local_dir_for(config, &cluster.name) {
             let dir = dir.replace('~', "$HOME");
             let dir = dir.replace('~', "$HOME");
             let cmd = mirror_cmd(&dir, &package);
             let cmd = mirror_cmd(&dir, &package);
@@ -475,7 +381,9 @@ pub async fn stop(
     cluster: &crate::config::Cluster,
     cluster: &crate::config::Cluster,
     dry_run: bool,
     dry_run: bool,
 ) -> Result<()> {
 ) -> Result<()> {
-    let controller = head(cluster)?;
+    // `orbit stop` must reach the orbit's controller, which the config may pin
+    // to a node other than our rank 0.
+    let controller = controller_for(config, cluster)?;
     let cmd = stop_cmd(config, recipe, &orbit_cluster_for(config, &cluster.name))?;
     let cmd = stop_cmd(config, recipe, &orbit_cluster_for(config, &cluster.name))?;
     crate::ssh::run_ssh(config, controller, cmd, dry_run).await?;
     crate::ssh::run_ssh(config, controller, cmd, dry_run).await?;
     Ok(())
     Ok(())

+ 169 - 0
src/native/flare_orbit.rs

@@ -0,0 +1,169 @@
+//! Which orbit and which nodes a FLARE lane uses.
+//!
+//! One launcher config serves FLARE on several of our clusters, and the orbit
+//! is not our cluster: it is a separate pairing (`orbit.toml` on the
+//! controller) with its own names and — importantly — its **own rank order**.
+//! Everything that maps "our cluster" onto "their orbit" lives here so the
+//! driver itself stays a sequence of commands.
+
+use anyhow::Result;
+
+use crate::config::{Config, Node};
+
+/// Orbit cluster name for one of OUR clusters: the per-cluster mapping wins,
+/// then the flat `[flare].orbit_cluster`, then the default. This is what makes
+/// one launcher config work on `dgx58-switch` and `prod4` alike.
+pub fn orbit_cluster_for(config: &Config, cluster: &str) -> String {
+    let f = config.flare.as_ref();
+    f.and_then(|f| f.clusters.get(cluster))
+        .and_then(|c| c.orbit_cluster.clone())
+        .or_else(|| f.and_then(|f| f.orbit_cluster.clone()))
+        .unwrap_or_else(|| "sparks".to_string())
+}
+
+/// The orbit cluster a **specific** cluster is paired with, or `None` when the
+/// config does not say (the flat `[flare].orbit_cluster` is then only a
+/// fallback for `serve`, not evidence of a pairing).
+///
+/// This is the identification rule the dashboard and `health` use: a cluster
+/// whose own nodes were never paired with an orbit must not be probed with the
+/// flat default (`sparks`) — `6nodes-switch`, `1node-*`, `ds4` and `qwen38`
+/// share nodes with the paired clusters, so asking one of them `--cluster
+/// sparks` made the sparks lane look like a lane of *that* cluster.
+pub fn orbit_cluster_paired(config: &Config, cluster: &str) -> Option<String> {
+    let f = config.flare.as_ref()?;
+    if let Some(own) = f.clusters.get(cluster) {
+        // A per-cluster block without an orbit name inherits the flat one —
+        // the operator already said "this cluster speaks FLARE".
+        return own
+            .orbit_cluster
+            .clone()
+            .or_else(|| f.orbit_cluster.clone());
+    }
+    // Once the map exists it is the source of truth: the flat block describes
+    // the machine it was written for, and our clusters share nodes (dgx5 is
+    // rank 0 of `dgx58-switch` AND rank 4 of `6nodes-switch`), so a flat
+    // `sparks` would pair `6nodes-switch` too.
+    if !f.clusters.is_empty() {
+        return None;
+    }
+    let flat = f.orbit_cluster.clone()?;
+    match f.controller_node.as_deref() {
+        // Legacy flat block: it speaks for the cluster that holds its
+        // controller node.
+        Some(ctl) => config
+            .clusters
+            .iter()
+            .find(|c| c.name == cluster)
+            .is_some_and(|c| c.nodes.iter().any(|n| n.name == ctl))
+            .then_some(flat),
+        // No controller either: unambiguous only with a single cluster.
+        None => (config.clusters.len() == 1 && config.clusters[0].name == cluster).then_some(flat),
+    }
+}
+
+/// Extra `orbit run` flags for a cluster (per-cluster map first, then flat).
+/// Fabric shape is a cluster property. The prod 4-node switch uses no extra
+/// flag: FLARE finds the star links itself once the legacy CX7 ports are down;
+/// `--roce-ring` was a symptom-level fix and is retired.
+pub fn extra_flags_for(config: &Config, cluster: &str) -> Vec<String> {
+    let Some(f) = config.flare.as_ref() else {
+        return Vec::new();
+    };
+    let per = f
+        .clusters
+        .get(cluster)
+        .map(|c| c.extra_flags.clone())
+        .unwrap_or_default();
+    if per.is_empty() {
+        f.extra_flags.clone()
+    } else {
+        per
+    }
+}
+
+/// Local package mirror dir for a cluster (per-cluster map first).
+pub fn local_dir_for(config: &Config, cluster: &str) -> Option<String> {
+    let f = config.flare.as_ref()?;
+    f.clusters
+        .get(cluster)
+        .and_then(|c| c.local_dir.clone())
+        .or_else(|| f.local_dir.clone())
+}
+
+/// The orbit-side name for one of our nodes (rank order → `orbit_nodes`,
+/// falling back to the cluster's own name).
+pub fn orbit_node_name(config: &Config, cluster: &str, node: &Node) -> String {
+    let f = config.flare.as_ref();
+    let per_cluster = f
+        .and_then(|f| f.clusters.get(cluster))
+        .map(|c| c.orbit_nodes.as_slice())
+        .filter(|v| !v.is_empty());
+    let names = per_cluster.or_else(|| f.map(|f| f.orbit_nodes.as_slice()));
+    let Some(names) = names else {
+        return node.name.clone();
+    };
+    // Name match first: `orbit_nodes` lists the *orbit's* ranks in the orbit's
+    // order (`gx10-dgx4` first when dgx4 is the orbit's rank 0), which is not
+    // our rank order — indexing by our rank then named the wrong node.
+    let want = bare_name(&node.name);
+    if let Some(hit) = names.iter().find(|n| bare_name(n) == want) {
+        return hit.clone();
+    }
+    // Legacy positional mapping (an orbit list that does not name our nodes).
+    names
+        .get(node.rank as usize)
+        .cloned()
+        .unwrap_or_else(|| node.name.clone())
+}
+
+/// The identifying tail of a node name: `dgx4.lan` and `gx10-dgx4` → `dgx4`.
+fn bare_name(name: &str) -> &str {
+    let host = name.strip_suffix(".lan").unwrap_or(name);
+    host.rsplit('-').next().unwrap_or(host)
+}
+
+fn head(cluster: &crate::config::Cluster) -> Result<&Node> {
+    cluster
+        .nodes
+        .iter()
+        .find(|n| n.rank == 0)
+        .ok_or_else(|| anyhow::anyhow!("no head node (rank 0) in cluster"))
+}
+
+/// The node that runs the FLARE CLI for this cluster: `controller_node` from
+/// the config when it names one of the cluster's nodes, else rank 0.
+///
+/// The two are not the same thing. `controller_node` is the orbit's controller
+/// — the machine whose key the agents trust and whose disk holds the package —
+/// and the orbit's rank order lives in `orbit.toml`. A re-pair or a disk-driven
+/// reshuffle (dgx4 became rank 0 because its 2 TB disk is the only one that
+/// fits the 488 GB store *plus* the 488 GB agent cache) moves the orbit's rank
+/// 0 without touching our cluster definition. Ignoring the pin sent
+/// `flare download`/`orbit run` to the wrong machine, which fails with
+/// "… is not an authorized controller of this node".
+pub fn controller_for<'a>(
+    config: &Config,
+    cluster: &'a crate::config::Cluster,
+) -> Result<&'a Node> {
+    let pinned = config.flare.as_ref().and_then(|f| {
+        f.clusters
+            .get(&cluster.name)
+            .and_then(|c| c.controller_node.clone())
+            .or_else(|| f.controller_node.clone())
+    });
+    let Some(name) = pinned else {
+        return head(cluster);
+    };
+    let bare = name.strip_suffix(".lan").unwrap_or(&name).to_string();
+    cluster
+        .nodes
+        .iter()
+        .find(|n| n.name.strip_suffix(".lan").unwrap_or(&n.name) == bare)
+        .ok_or_else(|| {
+            anyhow::anyhow!(
+                "[flare] controller_node '{name}' is not a node of cluster '{}'",
+                cluster.name
+            )
+        })
+}

+ 110 - 1
src/native/flare_tests.rs

@@ -1,7 +1,7 @@
 //! Tests for the FLARE driver (`native::flare`).
 //! Tests for the FLARE driver (`native::flare`).
 
 
 use super::*;
 use super::*;
-use crate::config::{Config, Recipe};
+use crate::config::{Config, Node, Recipe};
 
 
 use crate::config::FlareRecipe;
 use crate::config::FlareRecipe;
 
 
@@ -320,3 +320,112 @@ fn two_candidates_are_refused() {
     ]);
     ]);
     assert_eq!(select_cached("/store", &v, "gb10-tp4"), None);
     assert_eq!(select_cached("/store", &v, "gb10-tp4"), None);
 }
 }
+
+// ── which node runs the FLARE CLI ───────────────────────────────────────────
+
+use crate::config::Cluster;
+
+fn prod4() -> Cluster {
+    Cluster {
+        name: "4nodes-switch".into(),
+        master_ib_ip: None,
+        extra_entities: Vec::new(),
+        nodes: (1..=4)
+            .map(|i| Node {
+                name: format!("dgx{i}.lan"),
+                ssh_ip: format!("10.0.0.{i}"),
+                ib_ip: None,
+                rank: i - 1,
+                host_key: None,
+                entity: None,
+            })
+            .collect(),
+    }
+}
+
+#[test]
+fn without_a_pin_the_controller_is_rank_zero() {
+    let c = cfg();
+    let name = controller_for(&c, &prod4()).unwrap().name.clone();
+    assert_eq!(name, "dgx1.lan");
+}
+
+#[test]
+fn a_pinned_controller_wins_over_rank_zero() {
+    // the orbit's rank 0 moved to dgx4 (2 TB disk fits store + agent cache)
+    let mut c = cfg();
+    c.flare.as_mut().unwrap().clusters.insert(
+        "4nodes-switch".into(),
+        crate::config::flare_cluster::FlareCluster {
+            controller_node: Some("dgx4.lan".into()),
+            ..Default::default()
+        },
+    );
+    assert_eq!(controller_for(&c, &prod4()).unwrap().name, "dgx4.lan");
+    // a bare name resolves too
+    c.flare
+        .as_mut()
+        .unwrap()
+        .clusters
+        .get_mut("4nodes-switch")
+        .unwrap()
+        .controller_node = Some("dgx3".into());
+    assert_eq!(controller_for(&c, &prod4()).unwrap().name, "dgx3.lan");
+}
+
+#[test]
+fn a_pin_outside_the_cluster_is_an_error() {
+    let mut c = cfg();
+    c.flare.as_mut().unwrap().clusters.insert(
+        "4nodes-switch".into(),
+        crate::config::flare_cluster::FlareCluster {
+            controller_node: Some("dgx8.lan".into()),
+            ..Default::default()
+        },
+    );
+    let err = controller_for(&c, &prod4()).unwrap_err().to_string();
+    assert!(err.contains("not a node of cluster"), "{err}");
+}
+
+#[test]
+fn orbit_names_are_matched_by_node_name_not_by_rank() {
+    // The orbit's own rank order puts dgx4 first (its 2 TB disk fits the
+    // package store plus the agent cache); indexing that list by OUR rank
+    // named gx10-dgx3 for a controller that is dgx4.
+    let mut c = cfg();
+    c.flare.as_mut().unwrap().clusters.insert(
+        "4nodes-switch".into(),
+        crate::config::flare_cluster::FlareCluster {
+            orbit_nodes: vec![
+                "gx10-dgx4".into(),
+                "gx10-dgx1".into(),
+                "gx10-dgx2".into(),
+                "gx10-dgx3".into(),
+            ],
+            ..Default::default()
+        },
+    );
+    let cl = prod4();
+    let by_name = |n: &str| {
+        let node = cl.nodes.iter().find(|x| x.name == n).unwrap();
+        orbit_node_name(&c, "4nodes-switch", node)
+    };
+    assert_eq!(by_name("dgx4.lan"), "gx10-dgx4");
+    assert_eq!(by_name("dgx1.lan"), "gx10-dgx1");
+    assert_eq!(by_name("dgx3.lan"), "gx10-dgx3");
+}
+
+#[test]
+fn an_orbit_list_that_does_not_name_our_nodes_stays_positional() {
+    // legacy configs list bare orbit names; rank order is all they have
+    let mut c = cfg();
+    c.flare.as_mut().unwrap().orbit_nodes = vec![
+        "spark1".into(),
+        "spark2".into(),
+        "spark3".into(),
+        "spark4".into(),
+    ];
+    let cl = prod4();
+    let node = cl.nodes.iter().find(|x| x.rank == 2).unwrap();
+    assert_eq!(orbit_node_name(&c, "4nodes-switch", node), "spark3");
+}

+ 1 - 0
src/native/mod.rs

@@ -16,6 +16,7 @@ use crate::config::{Cluster, Config, DgppConfig, Node, Recipe};
 use crate::docker::ServeOpts;
 use crate::docker::ServeOpts;
 
 
 pub mod flare;
 pub mod flare;
+pub mod flare_orbit;
 pub mod flare_package;
 pub mod flare_package;
 mod provision;
 mod provision;