Bladeren bron

feat: health probes [mikrotik] devices via RouterOS API login (ping = on, dead/rejected = off, blocking); power-on routes through on_entity relay

Alexander Korolev 1 maand geleden
bovenliggende
commit
69ba71ad59
8 gewijzigde bestanden met toevoegingen van 227 en 87 verwijderingen
  1. 7 2
      AGENTS.md
  2. 3 55
      src/docker/build_dir.rs
  3. 48 0
      src/docker/build_dir_tests.rs
  4. 31 3
      src/ends.rs
  5. 101 16
      src/home_assistant.rs
  6. 16 0
      src/mikrotik.rs
  7. 8 2
      src/publish.rs
  8. 13 9
      src/web/probe.rs

+ 7 - 2
AGENTS.md

@@ -119,7 +119,10 @@ the model locally — no streaming weights over slow shared storage at load time
 
 **Cluster health is cluster-level** (`health`, web dashboard): a cluster is
 UP only when *all* its nodes are reachable AND its **blocking**
-`extra_entities` (HA switches) report `on`. One offline node — or a blocking
+`extra_entities` report `on`. HA entities are probed via HA REST; a
+`[mikrotik]` device referenced by name is probed through the **RouterOS API
+login** (logged in = on; unreachable or rejected login = off, like a dead
+switch — `health` warns with the reason). One offline node — or a blocking
 switch off — takes the whole cluster down: containers on surviving nodes are
 not reported (serve/stop/jobs views all ignore them; `power on` stays
 available). `extra_entities` entries can opt out of gating with
@@ -651,7 +654,9 @@ on_entity = "switch.lab_relay"   # optional: HA relay that powers it back on
   HA off phase; a failed API call warns, never blocks the rest of the flow).
 - `power on` → `on_entity` relay via HA if set, otherwise a
   “power on manually” reminder (the API cannot wake an off router).
-- `health` entity polling skips mikrotik names (they are not HA entities).
+- `health` / web probe / serve auto power-on: mikrotik names are dispatched
+  to the RouterOS API — ping for state, `on_entity` relay for `turn_on`
+  (manual reminder when unset); never HA.
 - Without a `[mikrotik]` section everything behaves exactly as before.
 
 ## NCCL_IB_GID_INDEX auto-detect

+ 3 - 55
src/docker/build_dir.rs

@@ -115,10 +115,7 @@ pub fn flatten_overlay(src_dir: &Path, dst: &Path, dockerfile: &str) -> Result<(
                 .unwrap_or(f)
                 .to_string_lossy()
                 .to_string();
-            exact
-                .get(dir)
-                .map(|s| s.contains(&rel))
-                .unwrap_or(false)
+            exact.get(dir).map(|s| s.contains(&rel)).unwrap_or(false)
                 || globs
                     .get(dir)
                     .map(|pats| pats.iter().any(|p| glob_match(p, &rel)))
@@ -459,54 +456,5 @@ pub fn resolve_build_dir(
 }
 
 #[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn glob_match_basics() {
-        assert!(glob_match("patch_vllm_*.py", "patch_vllm_topk.py"));
-        assert!(glob_match("patch_vllm_*.py", "patch_vllm_a.py"));
-        assert!(!glob_match("patch_vllm_*.py", "other.py"));
-        assert!(glob_match("a?c", "abc"));
-        assert!(!glob_match("a?c", "ac"));
-        assert!(glob_match("x", "x"));
-        assert!(glob_match("*", "anything/at-all"));
-    }
-
-    #[test]
-    fn parse_versions_env_quotes_are_normalized() {
-        let raw = "A=\"x:tag@sha256:abc\"\nB='y'\nC=z\nE=\"half\nF='weird\"\n";
-        let pins = parse_versions_env(raw);
-        assert_eq!(pins.get("A"), Some(&"x:tag@sha256:abc".to_string()));
-        assert_eq!(pins.get("B"), Some(&"y".to_string()));
-        assert_eq!(pins.get("C"), Some(&"z".to_string()));
-        assert_eq!(pins.get("E"), Some(&"\"half".to_string()));
-        assert_eq!(pins.get("F"), Some(&"'weird\"".to_string()));
-    }
-
-    #[test]
-    fn parse_versions_env_skips_comments_and_blanks() {
-        let raw = "# comment\n\nKEY=val\nOTHER=a=b\n  SPACED  =  x  \nnoeq\n";
-        let pins = parse_versions_env(raw);
-        assert_eq!(pins.get("KEY"), Some(&"val".to_string()));
-        assert_eq!(pins.get("OTHER"), Some(&"a=b".to_string()));
-        assert_eq!(pins.get("SPACED"), Some(&"x".to_string()));
-        assert!(!pins.contains_key("noeq"));
-        assert_eq!(pins.len(), 3);
-    }
-
-    #[test]
-    fn resolve_falls_back_to_bundle_layout() {
-        let base = std::env::temp_dir().join(format!("sm121-resolve-test-{}", std::process::id()));
-        let bundle = base.join("recipes").join("test-resolve-recipe-x");
-        std::fs::create_dir_all(&bundle).unwrap();
-        std::fs::write(bundle.join("build.sh"), "").unwrap();
-        std::fs::write(bundle.join("Dockerfile"), "").unwrap();
-        let (dir, src) =
-            resolve_build_dir(&base, "test-resolve-recipe-x", "test-resolve-recipe-x", &[])
-                .unwrap();
-        assert_eq!(src, BuildSource::Bundle);
-        assert_eq!(dir, bundle);
-        let _ = std::fs::remove_dir_all(&base);
-    }
-}
+#[path = "build_dir_tests.rs"]
+mod tests;

+ 48 - 0
src/docker/build_dir_tests.rs

@@ -0,0 +1,48 @@
+use super::*;
+
+#[test]
+fn glob_match_basics() {
+    assert!(glob_match("patch_vllm_*.py", "patch_vllm_topk.py"));
+    assert!(glob_match("patch_vllm_*.py", "patch_vllm_a.py"));
+    assert!(!glob_match("patch_vllm_*.py", "other.py"));
+    assert!(glob_match("a?c", "abc"));
+    assert!(!glob_match("a?c", "ac"));
+    assert!(glob_match("x", "x"));
+    assert!(glob_match("*", "anything/at-all"));
+}
+
+#[test]
+fn parse_versions_env_quotes_are_normalized() {
+    let raw = "A=\"x:tag@sha256:abc\"\nB='y'\nC=z\nE=\"half\nF='weird\"\n";
+    let pins = parse_versions_env(raw);
+    assert_eq!(pins.get("A"), Some(&"x:tag@sha256:abc".to_string()));
+    assert_eq!(pins.get("B"), Some(&"y".to_string()));
+    assert_eq!(pins.get("C"), Some(&"z".to_string()));
+    assert_eq!(pins.get("E"), Some(&"\"half".to_string()));
+    assert_eq!(pins.get("F"), Some(&"'weird\"".to_string()));
+}
+
+#[test]
+fn parse_versions_env_skips_comments_and_blanks() {
+    let raw = "# comment\n\nKEY=val\nOTHER=a=b\n  SPACED  =  x  \nnoeq\n";
+    let pins = parse_versions_env(raw);
+    assert_eq!(pins.get("KEY"), Some(&"val".to_string()));
+    assert_eq!(pins.get("OTHER"), Some(&"a=b".to_string()));
+    assert_eq!(pins.get("SPACED"), Some(&"x".to_string()));
+    assert!(!pins.contains_key("noeq"));
+    assert_eq!(pins.len(), 3);
+}
+
+#[test]
+fn resolve_falls_back_to_bundle_layout() {
+    let base = std::env::temp_dir().join(format!("sm121-resolve-test-{}", std::process::id()));
+    let bundle = base.join("recipes").join("test-resolve-recipe-x");
+    std::fs::create_dir_all(&bundle).unwrap();
+    std::fs::write(bundle.join("build.sh"), "").unwrap();
+    std::fs::write(bundle.join("Dockerfile"), "").unwrap();
+    let (dir, src) =
+        resolve_build_dir(&base, "test-resolve-recipe-x", "test-resolve-recipe-x", &[]).unwrap();
+    assert_eq!(src, BuildSource::Bundle);
+    assert_eq!(dir, bundle);
+    let _ = std::fs::remove_dir_all(&base);
+}

+ 31 - 3
src/ends.rs

@@ -60,20 +60,48 @@ impl NodeProber for SshProber {
 
 pub struct HaPowerController {
     ha: HaClient,
+    config: Config,
 }
 
 impl HaPowerController {
-    pub fn new(ha: HaClient) -> Self {
-        Self { ha }
+    pub fn new(ha: HaClient, config: Config) -> Self {
+        Self { ha, config }
     }
 }
 
 impl PowerController for HaPowerController {
     async fn entity_is_on(&self, entity: &str) -> Option<bool> {
+        // `[mikrotik]` device: liveness through the RouterOS API login
+        // (logged in = on; unreachable/rejected = off, like a dead switch).
+        if let Some(dev) = self
+            .config
+            .mikrotik
+            .as_ref()
+            .and_then(|m| m.devices.iter().find(|d| d.name == entity))
+        {
+            return Some(crate::mikrotik::ping_device(dev).await.is_ok());
+        }
         self.ha.get_state(entity).await.map(|s| s == "on")
     }
 
     async fn turn_on(&self, entity: &str) -> Result<()> {
+        // RouterOS has no power-ON API: route through the device's
+        // `on_entity` HA relay if configured; otherwise the operator plugs
+        // it in manually (same semantics as `power on`), never an error.
+        if let Some(dev) = self
+            .config
+            .mikrotik
+            .as_ref()
+            .and_then(|m| m.devices.iter().find(|d| d.name == entity))
+        {
+            match &dev.on_entity {
+                Some(relay) => self.ha.turn_on(relay).await?,
+                None => {
+                    warn!("RouterOS {entity} has no on_entity relay — power it on manually");
+                }
+            }
+            return Ok(());
+        }
         self.ha.turn_on(entity).await
     }
 }
@@ -98,7 +126,7 @@ pub fn ha_controller_from_config(config: &Config) -> Option<HaPowerController> {
         }
     };
     warn_if_token_looks_short(&ha_cfg.token);
-    Some(HaPowerController::new(ha))
+    Some(HaPowerController::new(ha, config.clone()))
 }
 
 /// Collect all entity IDs (per-node + cluster extras) for a cluster.

+ 101 - 16
src/home_assistant.rs

@@ -143,35 +143,42 @@ pub struct EntityStates {
     pub soft_off: Vec<String>,
 }
 
-/// Probe the cluster's `extra_entities`. No HA config, no entities, or an
-/// unparseable HA URL → default (nothing to check ≠ off).
+/// Probe the cluster's `extra_entities`. No HA config and no `[mikrotik]`
+/// backend for an entity → that entity is "nothing to check", not off
+/// (legacy semantics). Mikrotik devices (exact name match from
+/// [`crate::config`]) are probed through the RouterOS API instead of HA:
+/// logged in = on, unreachable/rejected login = off (a dead router takes
+/// the cluster down like any other blocking switch).
 pub async fn extra_entities_states(config: &Config, cluster: &Cluster) -> EntityStates {
-    let Some(ha_cfg) = &config.home_assistant else {
-        return EntityStates::default();
-    };
     if cluster.extra_entities.is_empty() {
         return EntityStates::default();
     }
-    let Ok(client) = Client::new(ha_cfg) else {
-        return EntityStates::default();
-    };
+    let ha_client: Option<Client> = config
+        .home_assistant
+        .as_ref()
+        .and_then(|ha| Client::new(ha).ok());
+    let has_ha = ha_client.is_some();
     let mut handles = Vec::new();
     for e in &cluster.extra_entities {
-        // Mikrotik devices (referenced by exact name from [mikrotik]) are
-        // not HA entities — probing them would report "off" and take the
-        // cluster down for nothing.
+        let id = e.entity().to_string();
         let is_mikrotik = config
             .mikrotik
             .as_ref()
-            .is_some_and(|m| m.devices.iter().any(|d| d.name == e.entity()));
-        if is_mikrotik {
+            .is_some_and(|m| m.devices.iter().any(|d| d.name == id.as_str()));
+        // A non-mikrotik entity without an HA backend is "nothing to check",
+        // not off — only entities with a configured backend flip the verdict.
+        if !is_mikrotik && !has_ha {
             continue;
         }
-        let client = client.clone();
-        let id = e.entity().to_string();
+        let config = config.clone();
+        let client = ha_client.clone();
         let blocks = e.blocks();
         handles.push(crate::locale::spawn_with_locale(async move {
-            (id.clone(), blocks, client.get_state(&id).await)
+            (
+                id.clone(),
+                blocks,
+                entity_state(&config, client.as_ref(), &id).await,
+            )
         }));
     }
     let mut st = EntityStates::default();
@@ -191,6 +198,37 @@ pub async fn extra_entities_states(config: &Config, cluster: &Cluster) -> Entity
     st
 }
 
+/// The state of one entity through the correct backend: a `[mikrotik]`
+/// device is probed with a RouterOS API login (logged in = "on";
+/// unreachable or rejected login = "off", like a switch that does not
+/// answer), everything else goes through HA REST.
+/// Returns `None` only when the entity has no configured backend at all
+/// (unknown — callers keep the "nothing to check ≠ off" semantics).
+pub async fn entity_state(
+    config: &Config,
+    client: Option<&Client>,
+    entity_id: &str,
+) -> Option<String> {
+    if let Some(dev) = config
+        .mikrotik
+        .as_ref()
+        .and_then(|m| m.devices.iter().find(|d| d.name == entity_id))
+    {
+        return match crate::mikrotik::ping_device(dev).await {
+            Ok(()) => Some("on".to_string()),
+            Err(e) => {
+                warn!("RouterOS ping {entity_id} failed: {e}");
+                Some("off".to_string())
+            }
+        };
+    }
+    let client = client?;
+    client
+        .get_state(entity_id)
+        .await
+        .map(|s| if s == "on" { "on".into() } else { "off".into() })
+}
+
 /// Parse `switch.dgx1` into `("switch", "dgx1")` so callers can build
 /// their own endpoints if needed.  Exposed for the `power` subcommand
 /// and for tests.
@@ -224,6 +262,53 @@ pub fn warn_if_token_looks_short(token: &str) {
 mod tests {
     use super::*;
 
+    #[tokio::test]
+    async fn mikrotik_entity_unreachable_is_blocking_off() {
+        // A `[mikrotik]` device referenced from `extra_entities` is probed
+        // through the RouterOS API: unreachable (closed port) = off, and a
+        // blocking entity takes the cluster down — the r18-class "dead
+        // entity silently skipped" failure mode must not exist.
+        let toml = r#"
+ssh_user = "u"
+[[clusters]]
+name = "c1"
+nodes = [{ name = "n1", ssh_ip = "10.0.0.1", rank = 0 }]
+extra_entities = ["rt"]
+[mikrotik]
+[[mikrotik.devices]]
+name = "rt"
+address = "127.0.0.1"
+username = "x"
+password = "y"
+api_port = 1
+"#;
+        let config: crate::config::Config = toml::from_str(toml).unwrap();
+        let cluster = &config.clusters[0];
+        let st = extra_entities_states(&config, cluster).await;
+        assert!(
+            st.blocking_off,
+            "unreachable mikrotik device must be blocking-off"
+        );
+    }
+
+    #[tokio::test]
+    async fn mikrotik_device_without_backend_not_reported_off() {
+        // No [mikrotik] section, no HA → the entity has no backend at all:
+        // "nothing to check", not off (legacy semantics).
+        let toml = r#"
+ssh_user = "u"
+[[clusters]]
+name = "c1"
+nodes = [{ name = "n1", ssh_ip = "10.0.0.1", rank = 0 }]
+extra_entities = ["rt"]
+"#;
+        let config: crate::config::Config = toml::from_str(toml).unwrap();
+        let cluster = &config.clusters[0];
+        let st = extra_entities_states(&config, cluster).await;
+        assert!(!st.blocking_off);
+        assert!(st.soft_off.is_empty());
+    }
+
     #[test]
     fn split_entity_basic() {
         assert_eq!(split_entity("switch.dgx1").unwrap(), ("switch", "dgx1"));

+ 16 - 0
src/mikrotik.rs

@@ -17,6 +17,22 @@ use tracing::info;
 
 use crate::config::MikrotikDevice;
 
+/// Liveness probe for `health`: connect + login, nothing else.
+/// `Ok` = the router is up AND the configured credentials work;
+/// any failure (unreachable, timeout, rejected login) = the entity is off,
+/// exactly like an HA switch that does not answer.
+pub async fn ping_device(dev: &MikrotikDevice) -> Result<()> {
+    let target = format!("{}:{}", dev.address, dev.api_port);
+    let mut s = tokio::time::timeout(Duration::from_secs(5), TcpStream::connect(&target))
+        .await
+        .with_context(|| format!("connecting to RouterOS at {}", target))?
+        .with_context(|| format!("connect to RouterOS at {}", target))?;
+    s.set_nodelay(true)?;
+    tokio::time::timeout(Duration::from_secs(10), login_stream(&mut s, dev))
+        .await
+        .map_err(|_| anyhow::anyhow!("RouterOS login timed out ({})", dev.name))?
+}
+
 /// Connect + login, then shut the router down via `/system/shutdown`.
 /// One connection per call — the router is about to die anyway.
 pub async fn shutdown_device(dev: &MikrotikDevice) -> Result<()> {

+ 8 - 2
src/publish.rs

@@ -240,9 +240,15 @@ mod tests {
         let dst = tmp.join("dst");
         std::fs::create_dir_all(&dst).unwrap();
         let (flat, nested) = crate::docker::build_dir::flatten_overlay(&src, &dst, df).unwrap();
-        assert!(!nested, "nothing nests anymore — unreferenced files are dropped");
+        assert!(
+            !nested,
+            "nothing nests anymore — unreferenced files are dropped"
+        );
         // collision rename for the referenced locks file
-        assert!(dst.join("locks-apt-sources.list").exists(), "collision rename");
+        assert!(
+            dst.join("locks-apt-sources.list").exists(),
+            "collision rename"
+        );
         assert!(dst.join("apt-sources.list").exists(), "root file shipped");
         assert_eq!(
             std::fs::read_to_string(dst.join("apt-packages.txt")).unwrap(),

+ 13 - 9
src/web/probe.rs

@@ -35,14 +35,13 @@ pub struct Fleet {
 }
 
 /// HA state per entity id (node power plugs + extra entities): on/off,
-/// `None` when HA is unconfigured or the entity is unknown.
+/// `None` when no backend is configured or the entity is unknown.
+/// Mikrotik devices are probed through the RouterOS API (not HA).
 async fn ha_states(config: &Config) -> HashMap<String, Option<bool>> {
-    let Some(ha_cfg) = &config.home_assistant else {
-        return HashMap::new();
-    };
-    let Ok(client) = crate::home_assistant::Client::new(ha_cfg) else {
-        return HashMap::new();
-    };
+    let ha_client: Option<crate::home_assistant::Client> = config
+        .home_assistant
+        .as_ref()
+        .and_then(|ha| crate::home_assistant::Client::new(ha).ok());
     let mut ents: Vec<String> = Vec::new();
     for c in &config.clusters {
         for n in &c.nodes {
@@ -60,10 +59,15 @@ async fn ha_states(config: &Config) -> HashMap<String, Option<bool>> {
         }
     }
     let handles = ents.iter().map(|id| {
-        let client = client.clone();
+        let config = config.clone();
+        let client = ha_client.clone();
         let id = id.clone();
         crate::locale::spawn_with_locale(async move {
-            let on = client.get_state(&id).await.map(|s| s == "on");
+            // `None` state = no backend configured (unknown, not off) —
+            // kept tri-state so a missing HA config never renders entities off.
+            let on = crate::home_assistant::entity_state(&config, client.as_ref(), &id)
+                .await
+                .map(|s| s == "on");
             (id, on)
         })
     });