From d61d1895728fb7816d4d060e805528d700810e4d Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Mon, 13 Jul 2026 00:08:29 +0000 Subject: [PATCH] node: split Running into Full/Degraded, add Failed health state Determine node health at start completion instead of unconditionally reaching Running. Zero transports up is now Failed (fatal): start() tears down cleanly and returns an error, and the daemon exits. Any configured optional child that failed to start - a transport beyond the first, Nostr, mDNS, TUN, DNS, or a worker pool - leaves the node Degraded but serving, with an operator warning naming what failed. All configured children up is Full. A child the node was never asked to run does not count against health. The published NodeState gains Degraded and Failed variants, both visible via control queries; Degraded is operational, Failed is not. The lifecycle FSM gains the health states plus the PublishState action that drives them - a health fork cannot be a single direct state write, which is why the earlier commits deferred it to here. Runtime child-exit health re-evaluation (a running child dying) is a separate liveness-monitoring mechanism left for a follow-up; this commit is start-time health only. --- src/node/lifecycle/mod.rs | 61 ++++- src/node/lifecycle/supervisor.rs | 424 ++++++++++++++++++++++++++----- src/node/mod.rs | 30 ++- src/node/tests/mod.rs | 19 ++ src/node/tests/unit.rs | 30 ++- testing/dns-resolver/test.sh | 7 + 6 files changed, 492 insertions(+), 79 deletions(-) diff --git a/src/node/lifecycle/mod.rs b/src/node/lifecycle/mod.rs index c0aa86f..ea8859f 100644 --- a/src/node/lifecycle/mod.rs +++ b/src/node/lifecycle/mod.rs @@ -1125,6 +1125,19 @@ impl Node { dns, }); + // The FSM resolves start-completion health (Full/Degraded/Failed) when + // `Starting.pending` empties and emits it as a `PublishState` action + // (design doc §6/§9.1). Capture that outcome — from the degenerate + // no-children path (published on the `Event::Start` step itself) or from + // the final `SubstrateUp`/`SubstrateFailed` below — to drive the + // start-completion behavior after the spawn loop. + let mut start_outcome: Option = None; + for action in &actions { + if let Action::PublishState(ns) = action { + start_outcome = Some(*ns); + } + } + // Execute each SpawnChild in order, reporting the outcome back so the // FSM's up-set tracks what actually came up. Optional failures are // warn/debug-and-continue (today's behavior); start still reaches @@ -1465,7 +1478,12 @@ impl Node { } }; - self.supervisor.fsm.step(feedback); + let feedback_actions = self.supervisor.fsm.step(feedback); + for action in &feedback_actions { + if let Action::PublishState(ns) = action { + start_outcome = Some(*ns); + } + } } // Seams that never triggered inside the loop: the "Transports @@ -1479,7 +1497,40 @@ impl Node { self.initiate_peer_connections().await; } - self.supervisor.state = NodeState::Running; + // Publish the FSM-resolved start-completion state (design doc §6/§9.1) + // instead of the old unconditional `Running`. + let outcome = start_outcome + .expect("supervisor publishes a start-completion state when bring-up resolves"); + self.supervisor.state = outcome; + + match outcome { + NodeState::Failed => { + // Zero transports came up — fatal. Tear down cleanly any + // children that DID come up (a failed start must not leave the + // node half-up), then return an error. `broadcast_disconnect = + // false`: there is nothing to gracefully disconnect on a start + // that never reached service. The daemon exits on this error. + warn!( + "Node start failed: no operational transports came up; tearing down partially-started children" + ); + let up = self.reconstruct_supervised_up(); + self.supervisor.fsm = SupervisorFsm::running_with(up); + let teardown = self.supervisor.fsm.step(Event::Stop); + self.execute_teardown(teardown, false).await; + return Err(NodeError::NoOperationalTransports); + } + NodeState::Degraded => { + // Operational but missing one or more configured optional + // children. Enumerate them for the operator, then proceed — + // a degraded node serves traffic. + warn!( + degraded_children = ?self.supervisor.fsm.failed(), + "Node started DEGRADED: one or more configured optional children failed to start" + ); + } + _ => {} + } + info!("Node started:"); info!(" state: {}", self.supervisor.state); info!(" transports: {}", self.transports.len()); @@ -1803,8 +1854,10 @@ impl Node { // Step 1b. Without it there is nothing to reconnect the peers // the drain closes, so the gate is implicitly satisfied. } - Action::SpawnChild(_) | Action::StopChild(_) => { - // Drain entry never emits child actions; ignore defensively. + Action::SpawnChild(_) | Action::StopChild(_) | Action::PublishState(_) => { + // Drain entry never emits child or publish-state actions + // (the `Draining` state is a direct write above); ignore + // defensively. } } } diff --git a/src/node/lifecycle/supervisor.rs b/src/node/lifecycle/supervisor.rs index b0edd22..30a1ba3 100644 --- a/src/node/lifecycle/supervisor.rs +++ b/src/node/lifecycle/supervisor.rs @@ -41,11 +41,32 @@ //! path is untouched. `Draining` and `Stop` share a single teardown-plan author //! (`begin_stopping`), so the teardown ordering is defined once. //! -//! What is deferred is only the FSM-owned `PublishState` *action* (published -//! state authored by the machine rather than the driver): it lands with the -//! `Running{Full|Degraded}` health split (design doc §6/§9.1), which needs it -//! because a single direct `self.state` write cannot express the health fork. -//! The `Draining` published state itself is **not** deferred — it is here. +//! ## Scope: the `Running{Full|Degraded}` + `Failed` health split (this commit) +//! +//! This commit lands the operator-visible start-completion health policy +//! (design doc §6/§9.1) and, with it, the FSM-owned [`Action::PublishState`]: +//! +//! - [`SupState::Running`] now carries a [`Health`] (`Full` or `Degraded`), and +//! [`SupState::Failed`] is the fatal path. When `Starting.pending` empties (or +//! the degenerate no-children path), the machine resolves health once +//! ([`SupervisorFsm::resolve_start_health`]): **zero transports up → `Failed`** +//! (fatal); **≥1 transport up but a configured optional child failed → +//! `Degraded`**; **everything configured came up → `Full`**. Not-configured +//! children never count (a node never asked to run DNS is not degraded for +//! lacking it); worker-pool failures are `Degraded` at most, never `Failed`. +//! - the health outcome is a fork that a single direct `self.state` write cannot +//! express, so the machine emits [`Action::PublishState`] carrying the resolved +//! [`NodeState`]; the driver writes it. The non-forking transitions +//! (`Starting`/`Draining`/`Stopping`/`Stopped`) keep their direct `self.state` +//! writes — only the start-completion health outcome routes through +//! `PublishState`, to minimize churn. +//! - the degenerate no-children path now resolves to `Failed` (zero transports), +//! **not** the old immediate-`Running`. +//! +//! Runtime child-liveness monitoring (a `ChildExited` event re-routing health +//! when a task/thread dies at runtime) is **deferred** (design doc §7): §9.1's +//! resolution is start-framed, and liveness monitoring is a substantial unbuilt +//! mechanism. This commit is start-time health only. use std::collections::HashSet; use std::sync::Arc; @@ -83,9 +104,10 @@ pub(crate) enum Child { /// An input to the supervisor. Results of executing [`Action`]s are fed back as /// `SubstrateUp` / `SubstrateFailed` / `ChildStopped`. /// -/// `Tick` and `ChildExited` (design doc §6) arrive with the `Degraded`/health -/// commit; the bounded-drain events (`Drain` / `DrainDeadlineElapsed`) are -/// present here. +/// `Tick` and `ChildExited` (design doc §6) are **deferred**: `ChildExited` +/// belongs to the runtime child-liveness monitoring follow-up (this commit is +/// start-time health only). The bounded-drain events (`Drain` / +/// `DrainDeadlineElapsed`) and the start/up/failed/stop events are present here. #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) enum Event { /// Begin bring-up. `transports` are the ids the driver has already created @@ -169,14 +191,25 @@ pub(crate) enum PeeringDesired { /// An effect the driver must perform. The core never performs I/O itself. /// -/// `PublishState` (design doc §6) is intentionally absent until the -/// `Running{Full|Degraded}` health commit: the driver keeps its verbatim -/// `self.state` writes, so no published-state action is needed yet. +/// `PublishState` (design doc §6) lands here, with the `Running{Full|Degraded}` +/// health split: the start-completion health outcome is a fork +/// (`Full`/`Degraded`/`Failed`) that a single direct `self.state` write cannot +/// express, so the machine authors it as an action. The driver keeps its direct +/// `self.state` writes for the non-forking transitions (`Starting`/`Draining`/ +/// `Stopping`/`Stopped`); only the start-completion health outcome routes through +/// `PublishState`. #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) enum Action { /// Bring up this child (the driver performs the spawn / start I/O and /// reports `SubstrateUp` or `SubstrateFailed`). SpawnChild(Child), + /// Publish the given operator-visible [`NodeState`]. Emitted at start + /// completion (when `Starting.pending` empties, or the degenerate + /// no-children path) to carry the resolved health outcome — + /// [`NodeState::Running`] (Full), [`NodeState::Degraded`], or + /// [`NodeState::Failed`] — to the driver, which writes it to the published + /// state (design doc §6/§9.1). + PublishState(NodeState), /// Tear down this child (the driver performs the stop / join I/O and /// reports `ChildStopped`). StopChild(Child), @@ -197,10 +230,35 @@ pub(crate) enum Action { SuspendReplenish, } +/// Start-completion health (design doc §9.1). Resolved once when +/// `Starting.pending` empties: `Full` iff every configured child came up, +/// `Degraded` iff ≥1 transport is up but some configured optional child failed. +/// Zero transports up is not a health — it is the fatal [`SupState::Failed`]. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum Health { + /// Every configured child came up. + Full, + /// ≥1 transport is up, but one or more configured optional children failed + /// to start (a transport beyond the first, Nostr, mDNS, TUN, DNS, or a + /// worker-pool spawn). The node is operational (serving) but degraded. + Degraded { + /// The configured children that failed to start. + reasons: HashSet, + }, +} + +/// Reason for the fatal [`SupState::Failed`] state (design doc §9.1). +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum FailReason { + /// Zero transports came up at start completion. Without a transport the node + /// cannot serve, so this is fatal (the driver tears down and returns an + /// error), unlike the degraded-but-serving optional-child failures. + NoTransports, +} + /// Internal supervisor state (design doc §6). Richer than the published /// [`NodeState`](crate::node::NodeState): `Starting`/`Stopping` carry the set of -/// children still resolving. `Draining{deadline}`, `Running{Full|Degraded}`, and -/// `Failed{reason}` are added by the later flagged commits. +/// children still resolving, and `Running` carries the resolved [`Health`]. #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) enum SupState { /// Constructed but not started. @@ -210,8 +268,18 @@ pub(crate) enum SupState { /// Children asked to spawn that have not yet reported up-or-failed. pending: HashSet, }, - /// All children resolved; node operational. - Running, + /// All children resolved and ≥1 transport up; node operational. Carries the + /// resolved [`Health`] (`Full` or `Degraded`). + Running { + /// Resolved start-completion health. + health: Health, + }, + /// Start completed with zero transports up (design doc §9.1) — fatal. The + /// driver tears down any children that did come up and returns an error. + Failed { + /// Why the start failed. + reason: FailReason, + }, /// Bounded graceful-drain window (design doc §6/§8). Broadcast Disconnect /// has gone out and the reconciler is gated off (desired peering set /// emptied, replenishment suspended); teardown begins when @@ -241,6 +309,9 @@ pub(crate) struct SupervisorFsm { state: SupState, /// Children currently up (present). Drives the teardown plan. up: HashSet, + /// Configured children that failed to start during the current bring-up. + /// Feeds the `Degraded` health determination when `pending` empties. + failed: HashSet, } impl SupervisorFsm { @@ -249,6 +320,7 @@ impl SupervisorFsm { Self { state: SupState::Created, up: HashSet::new(), + failed: HashSet::new(), } } @@ -262,8 +334,11 @@ impl SupervisorFsm { /// plan over exactly the present children. pub(crate) fn running_with(up: impl IntoIterator) -> Self { Self { - state: SupState::Running, + state: SupState::Running { + health: Health::Full, + }, up: up.into_iter().collect(), + failed: HashSet::new(), } } @@ -273,6 +348,13 @@ impl SupervisorFsm { &self.state } + /// The configured children that failed to start during bring-up. The driver + /// reads this on the `Degraded` start outcome to enumerate the degraded + /// children in an operator-visible `warn!`. + pub(in crate::node) fn failed(&self) -> &HashSet { + &self.failed + } + /// Whether the machine is in the bounded-drain window. The driver uses this /// after the rx loop returns to decide between the drain-teardown path and /// the immediate-`stop()` fallback. @@ -351,12 +433,13 @@ impl SupervisorFsm { } self.up.clear(); + self.failed.clear(); - // A node with no children at all still reaches `Running` (today: even - // zero started transports proceeds to `Running`). + // A node with no children at all resolves health immediately. Zero + // transports up → `Failed` (design doc §9.1; this is the behavioral + // change from the old immediate-`Running`). if order.is_empty() { - self.state = SupState::Running; - return Vec::new(); + return vec![Action::PublishState(self.resolve_start_health())]; } self.state = SupState::Starting { @@ -366,39 +449,89 @@ impl SupervisorFsm { } fn on_substrate_up(&mut self, child: Child) -> Vec { - if let SupState::Starting { pending } = &mut self.state { - pending.remove(&child); - self.up.insert(child); - if pending.is_empty() { - self.state = SupState::Running; - } + let SupState::Starting { pending } = &mut self.state else { + return Vec::new(); + }; + pending.remove(&child); + let emptied = pending.is_empty(); + self.up.insert(child); + if emptied { + vec![Action::PublishState(self.resolve_start_health())] + } else { + Vec::new() } - Vec::new() } fn on_substrate_failed(&mut self, child: Child) -> Vec { - // Behavior-neutral: warn/continue. The child drains from `pending` and - // does not join the up-set; start still reaches `Running`. - if let SupState::Starting { pending } = &mut self.state { - pending.remove(&child); - if pending.is_empty() { - self.state = SupState::Running; - } + // Record the failed child (design doc §9.1): a configured child that + // failed to start drives the `Degraded` determination when `pending` + // empties. It drains from `pending` and never joins the up-set. + let SupState::Starting { pending } = &mut self.state else { + return Vec::new(); + }; + pending.remove(&child); + let emptied = pending.is_empty(); + self.failed.insert(child); + if emptied { + vec![Action::PublishState(self.resolve_start_health())] + } else { + Vec::new() + } + } + + /// Resolve start-completion health (design doc §9.1) and set the resulting + /// state, returning the [`NodeState`] the driver should publish. Called once + /// when `Starting.pending` empties (or the degenerate no-children path): + /// + /// - zero transports up → [`SupState::Failed`] / [`NodeState::Failed`]; + /// - ≥1 transport up but some configured child failed → [`Health::Degraded`] + /// / [`NodeState::Degraded`]; + /// - everything configured came up → [`Health::Full`] / [`NodeState::Running`]. + /// + /// Worker-pool failures are captured in `failed` like any other optional + /// child, so they contribute `Degraded` (never `Failed`) — the inline crypto + /// fallback keeps the node correct without the pools (design doc §9.1). + /// + /// This is start-time health only. Runtime child-liveness monitoring (a + /// `ChildExited` event re-routing health when a task/thread dies at runtime) + /// is a deferred follow-up (design doc §7 / §9.1 is start-framed). + fn resolve_start_health(&mut self) -> NodeState { + let transports_up = self + .up + .iter() + .filter(|c| matches!(c, Child::Transport(_))) + .count(); + if transports_up == 0 { + self.state = SupState::Failed { + reason: FailReason::NoTransports, + }; + NodeState::Failed + } else if !self.failed.is_empty() { + self.state = SupState::Running { + health: Health::Degraded { + reasons: self.failed.clone(), + }, + }; + NodeState::Degraded + } else { + self.state = SupState::Running { + health: Health::Full, + }; + NodeState::Running } - Vec::new() } fn on_stop(&mut self) -> Vec { - if !matches!(self.state, SupState::Running) { + if !matches!(self.state, SupState::Running { .. }) { return Vec::new(); } self.begin_stopping() } fn on_drain(&mut self, deadline_ms: u64) -> Vec { - // Only a graceful drain from a running node. Inert otherwise (matching - // `Stop`'s guard). - if !matches!(self.state, SupState::Running) { + // Only a graceful drain from a running node (either health). Inert + // otherwise (matching `Stop`'s guard). + if !matches!(self.state, SupState::Running { .. }) { return Vec::new(); } self.state = SupState::Draining { deadline_ms }; @@ -614,23 +747,42 @@ mod tests { } #[test] - fn all_children_up_reaches_running() { + fn all_configured_up_reaches_full() { + // Everything configured came up (2 transports + all optional children) + // → Full; the pending-emptying step publishes `Running`. let mut s = SupervisorFsm::new(); let spawns = s.step(start_full()); - for a in spawns { - let child = match a { + let children: Vec = spawns + .into_iter() + .map(|a| match a { Action::SpawnChild(c) => c, _ => panic!("unexpected action"), - }; - assert_eq!(s.step(Event::SubstrateUp { child }), vec![]); + }) + .collect(); + let last = children.len() - 1; + for (i, child) in children.into_iter().enumerate() { + let out = s.step(Event::SubstrateUp { child }); + if i == last { + assert_eq!(out, vec![Action::PublishState(NodeState::Running)]); + } else { + assert_eq!(out, vec![]); + } } - assert_eq!(s.state(), &SupState::Running); + assert_eq!( + s.state(), + &SupState::Running { + health: Health::Full + } + ); + assert!(s.failed().is_empty()); } #[test] - fn failed_child_still_reaches_running_and_is_not_up() { - // Behavior-neutral: a failed optional child does not block Running and - // is excluded from teardown (never joined the up-set). + fn configured_optional_child_failure_is_degraded() { + // A configured optional child (mDNS) fails but ≥1 transport is up → + // Degraded, with the failed child in `reasons`. The node stays + // operational and tears down cleanly (the failed child never joined the + // up-set, so it is excluded from teardown; workers excluded by design). let mut s = SupervisorFsm::new(); s.step(start_full()); for child in [ @@ -640,13 +792,29 @@ mod tests { Child::DecryptWorkers, Child::Nostr, ] { - s.step(Event::SubstrateUp { child }); + assert_eq!(s.step(Event::SubstrateUp { child }), vec![]); } - // mdns fails, tun+dns come up - s.step(Event::SubstrateFailed { child: Child::Mdns }); - s.step(Event::SubstrateUp { child: Child::Tun }); - s.step(Event::SubstrateUp { child: Child::Dns }); - assert_eq!(s.state(), &SupState::Running); + // mdns fails, tun comes up, dns comes up last (empties pending). + assert_eq!( + s.step(Event::SubstrateFailed { child: Child::Mdns }), + vec![] + ); + assert_eq!(s.step(Event::SubstrateUp { child: Child::Tun }), vec![]); + assert_eq!( + s.step(Event::SubstrateUp { child: Child::Dns }), + vec![Action::PublishState(NodeState::Degraded)] + ); + let mut expected_reasons = HashSet::new(); + expected_reasons.insert(Child::Mdns); + assert_eq!( + s.state(), + &SupState::Running { + health: Health::Degraded { + reasons: expected_reasons.clone() + } + } + ); + assert_eq!(s.failed(), &expected_reasons); let stops = s.step(Event::Stop); // mdns must not appear in teardown; workers excluded by design. @@ -663,7 +831,109 @@ mod tests { } #[test] - fn no_children_reaches_running_immediately() { + fn worker_pool_failure_is_degraded_not_failed() { + // A worker-pool spawn failure is Degraded at most, never Failed + // (inline crypto fallback keeps the node correct). One transport is up. + let mut s = SupervisorFsm::new(); + s.step(Event::Start { + transports: vec![tid(1)], + encrypt_workers: true, + decrypt_workers: false, + nostr: false, + mdns: false, + tun: false, + dns: false, + }); + assert_eq!( + s.step(Event::SubstrateUp { + child: Child::Transport(tid(1)) + }), + vec![] + ); + assert_eq!( + s.step(Event::SubstrateFailed { + child: Child::EncryptWorkers + }), + vec![Action::PublishState(NodeState::Degraded)] + ); + let mut expected = HashSet::new(); + expected.insert(Child::EncryptWorkers); + assert_eq!( + s.state(), + &SupState::Running { + health: Health::Degraded { reasons: expected } + } + ); + } + + #[test] + fn not_configured_child_does_not_cause_degraded() { + // A node that never asked to run mDNS/TUN/DNS/Nostr is not degraded for + // lacking them: only a configured-and-failed child counts. One transport + // configured and up, nothing else configured → Full. + let mut s = SupervisorFsm::new(); + s.step(Event::Start { + transports: vec![tid(1)], + encrypt_workers: false, + decrypt_workers: false, + nostr: false, + mdns: false, + tun: false, + dns: false, + }); + assert_eq!( + s.step(Event::SubstrateUp { + child: Child::Transport(tid(1)) + }), + vec![Action::PublishState(NodeState::Running)] + ); + assert_eq!( + s.state(), + &SupState::Running { + health: Health::Full + } + ); + } + + #[test] + fn zero_transports_up_is_failed_via_child_failures() { + // Transports were configured but all failed → zero transports up → + // Failed (fatal), even though other children came up. Failed takes + // priority over Degraded. + let mut s = SupervisorFsm::new(); + s.step(Event::Start { + transports: vec![tid(1)], + encrypt_workers: false, + decrypt_workers: false, + nostr: true, + mdns: false, + tun: false, + dns: false, + }); + assert_eq!( + s.step(Event::SubstrateFailed { + child: Child::Transport(tid(1)) + }), + vec![] + ); + assert_eq!( + s.step(Event::SubstrateUp { + child: Child::Nostr + }), + vec![Action::PublishState(NodeState::Failed)] + ); + assert_eq!( + s.state(), + &SupState::Failed { + reason: FailReason::NoTransports + } + ); + } + + #[test] + fn no_children_is_failed_immediately() { + // The degenerate empty-`Start` path: zero transports → Failed (the + // behavioral change from the old immediate-Running). let mut s = SupervisorFsm::new(); let actions = s.step(Event::Start { transports: vec![], @@ -674,8 +944,13 @@ mod tests { tun: false, dns: false, }); - assert_eq!(actions, vec![]); - assert_eq!(s.state(), &SupState::Running); + assert_eq!(actions, vec![Action::PublishState(NodeState::Failed)]); + assert_eq!( + s.state(), + &SupState::Failed { + reason: FailReason::NoTransports + } + ); } #[test] @@ -714,9 +989,10 @@ mod tests { fn all_children_stopped_reaches_stopped() { let mut s = SupervisorFsm::new(); s.step(start_full()); - // Every spawned child reports an outcome: five come up, three fail - // (warn/continue). `pending` drains fully, so the node still reaches - // `Running` — as it does today. + // Every spawned child reports an outcome: five come up, three fail. + // `pending` drains fully; one transport (tid(1)) is up so the node + // reaches `Running`, but a configured transport (tid(2)) and two + // configured optional children failed → Degraded. for child in [ Child::Transport(tid(1)), Child::EncryptWorkers, @@ -729,7 +1005,12 @@ mod tests { for child in [Child::Transport(tid(2)), Child::DecryptWorkers, Child::Mdns] { s.step(Event::SubstrateFailed { child }); } - assert_eq!(s.state(), &SupState::Running); + assert!(matches!( + s.state(), + SupState::Running { + health: Health::Degraded { .. } + } + )); // Only the children that came up are torn down; the failed ones never // joined the up-set. @@ -768,7 +1049,12 @@ mod tests { s.step(Event::SubstrateUp { child: Child::Transport(tid(1)), }); - assert_eq!(s.state(), &SupState::Running); + assert_eq!( + s.state(), + &SupState::Running { + health: Health::Full + } + ); // A stray event in Running produces nothing and does not change state. assert_eq!( s.step(Event::SubstrateUp { @@ -776,7 +1062,12 @@ mod tests { }), vec![] ); - assert_eq!(s.state(), &SupState::Running); + assert_eq!( + s.state(), + &SupState::Running { + health: Health::Full + } + ); } #[test] @@ -873,6 +1164,11 @@ mod tests { // Inert from `Running` (no drain in progress). let mut s = SupervisorFsm::running_with([Child::Transport(tid(1))]); assert_eq!(s.step(Event::DrainDeadlineElapsed), vec![]); - assert_eq!(s.state(), &SupState::Running); + assert_eq!( + s.state(), + &SupState::Running { + health: Health::Full + } + ); } } diff --git a/src/node/mod.rs b/src/node/mod.rs index 94054ec..cbbcfc5 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -157,6 +157,9 @@ pub enum NodeError { #[error("bootstrap handoff failed: {0}")] BootstrapHandoff(String), + + #[error("node start failed: no operational transports")] + NoOperationalTransports, } /// Node operational state. @@ -166,14 +169,23 @@ pub enum NodeState { Created, /// Starting up (initializing transports). Starting, - /// Fully operational. + /// Fully operational — every configured child came up (design doc §9.1). Running, + /// Operational but degraded (design doc §9.1): ≥1 transport is up and the + /// node is serving, but one or more configured optional children (a + /// transport beyond the first, Nostr, mDNS, TUN, DNS, or a worker pool) + /// failed to start. Still operational — a degraded node serves traffic. + Degraded, /// Bounded graceful drain in progress (design doc §6): a shutdown /// `Disconnect` has been broadcast and the node is waiting for peers to /// clear (bounded by `node.drain_timeout_secs`) before teardown. Not /// operational; the daemon drain path advances to `Stopping` via the /// supervisor's `DrainDeadlineElapsed`, never through `stop()`. Draining, + /// Start failed fatally: zero transports came up (design doc §9.1). The + /// driver tears down any children that did come up and `start()` returns + /// an error. Not operational and not restartable in-process. + Failed, /// Shutting down. Stopping, /// Stopped. @@ -181,19 +193,23 @@ pub enum NodeState { } impl NodeState { - /// Check if node is operational. + /// Check if node is operational. A `Degraded` node is operational — it is + /// serving, just missing an optional child (design doc §9.1). pub fn is_operational(&self) -> bool { - matches!(self, NodeState::Running) + matches!(self, NodeState::Running | NodeState::Degraded) } - /// Check if node can be started. + /// Check if node can be started. A `Failed` node is not restartable + /// in-process (design doc §9.1) — only a fresh `Created` or a cleanly + /// `Stopped` node can start. pub fn can_start(&self) -> bool { matches!(self, NodeState::Created | NodeState::Stopped) } - /// Check if node can be stopped. + /// Check if node can be stopped. Both `Running` and `Degraded` nodes are + /// operational and can be stopped or drained. pub fn can_stop(&self) -> bool { - matches!(self, NodeState::Running) + matches!(self, NodeState::Running | NodeState::Degraded) } } @@ -203,7 +219,9 @@ impl fmt::Display for NodeState { NodeState::Created => "created", NodeState::Starting => "starting", NodeState::Running => "running", + NodeState::Degraded => "degraded", NodeState::Draining => "draining", + NodeState::Failed => "failed", NodeState::Stopping => "stopping", NodeState::Stopped => "stopped", }; diff --git a/src/node/tests/mod.rs b/src/node/tests/mod.rs index 645aea4..b23d8d2 100644 --- a/src/node/tests/mod.rs +++ b/src/node/tests/mod.rs @@ -30,6 +30,25 @@ pub(super) fn make_node() -> Node { make_node_with(Config::new()) } +/// A test node that reaches `Full` health on `start()`. +/// +/// A default [`make_node`] configures no transports, so its `start()` now +/// resolves to `NodeState::Failed` (zero transports up, design doc §9.1) and +/// returns `NoOperationalTransports`. Lifecycle-state tests that need a running +/// node build one with a single loopback UDP transport (ephemeral port) as the +/// sole configured child — DNS disabled — so bring-up has exactly one +/// configured child and it comes up (`Full`). Mirrors the udp config in +/// `test_node_start_does_not_wait_for_nostr_relay_startup`. +pub(super) fn make_healthy_node() -> Node { + let mut config = Config::new(); + config.transports.udp = crate::config::TransportInstances::Single(crate::config::UdpConfig { + bind_addr: Some("127.0.0.1:0".to_string()), + ..Default::default() + }); + config.dns.enabled = false; + make_node_with(config) +} + /// Build a test node from an explicit `Config`. Immutable state lives solely in /// the shared `NodeContext`, built once at construction — there is no /// post-construction field to poke, so set limits/config on the `Config` here. diff --git a/src/node/tests/unit.rs b/src/node/tests/unit.rs index e9775f2..8e783f2 100644 --- a/src/node/tests/unit.rs +++ b/src/node/tests/unit.rs @@ -151,13 +151,16 @@ async fn test_try_peer_addresses_races_all_concrete_udp_candidates() { #[tokio::test] async fn test_node_state_transitions() { - let mut node = make_node(); + // A transport-less node now resolves to `Failed` on start (design doc + // §9.1), so exercise state transitions with a genuinely healthy node. + let mut node = make_healthy_node(); assert!(!node.is_running()); assert!(node.state().can_start()); node.start().await.unwrap(); assert!(node.is_running()); + assert_eq!(node.state(), NodeState::Running); assert!(!node.state().can_start()); node.stop().await.unwrap(); @@ -166,8 +169,25 @@ async fn test_node_state_transitions() { } #[tokio::test] -async fn test_drain_publishes_draining_state() { +async fn test_transportless_start_fails_and_publishes_failed() { + // The intended behavioral change (design doc §9.1): a node with zero + // transports up cannot serve, so `start()` returns `NoOperationalTransports` + // and leaves the published state at `Failed` (not operational, not + // restartable in-process). let mut node = make_node(); + assert!(node.state().can_start()); + + let result = node.start().await; + assert!(matches!(result, Err(NodeError::NoOperationalTransports))); + assert_eq!(node.state(), NodeState::Failed); + assert!(!node.is_running()); + assert!(!node.state().is_operational()); + assert!(!node.state().can_start()); +} + +#[tokio::test] +async fn test_drain_publishes_draining_state() { + let mut node = make_healthy_node(); node.start().await.unwrap(); assert_eq!(node.state(), NodeState::Running); assert!(node.state().is_operational()); @@ -189,7 +209,7 @@ async fn test_drain_publishes_draining_state() { #[tokio::test] async fn test_immediate_stop_never_publishes_draining() { - let mut node = make_node(); + let mut node = make_healthy_node(); node.start().await.unwrap(); assert_eq!(node.state(), NodeState::Running); @@ -231,7 +251,7 @@ async fn test_node_start_does_not_wait_for_nostr_relay_startup() { #[tokio::test] async fn test_node_double_start() { - let mut node = make_node(); + let mut node = make_healthy_node(); node.start().await.unwrap(); let result = node.start().await; @@ -578,7 +598,7 @@ async fn test_node_rx_loop_requires_start() { #[tokio::test] async fn test_node_rx_loop_takes_channel() { - let mut node = make_node(); + let mut node = make_healthy_node(); node.start().await.unwrap(); // packet_rx should be available after start diff --git a/testing/dns-resolver/test.sh b/testing/dns-resolver/test.sh index 218f978..f5df0b0 100755 --- a/testing/dns-resolver/test.sh +++ b/testing/dns-resolver/test.sh @@ -661,11 +661,18 @@ DOCKERFILE # Write a minimal fips.yaml that exercises the new defaults. # tun.enabled: true so the daemon creates fips0 itself; identity # persistent so /etc/fips/fips.pub gives us a stable npub to query. + # A UDP transport is configured so at least one transport comes up: + # a node with zero operational transports is Failed and refuses to + # start. This test exercises the .fips DNS responder, not mesh + # connectivity, so any bound transport suffices. docker exec "$name" bash -c 'cat > /etc/fips/fips.yaml <