From a3f4f838fda945f292b7ccab2b46fbe8f24b4065 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20K=C4=99ska?= <372403+keskad@users.noreply.github.com> Date: Wed, 26 Aug 2026 20:05:11 +0200 Subject: [PATCH 1/2] fix: wait liveness probes through the PID-1 reaper Cmd probes used Child::try_wait while waitpid(-1) already reaped the child, so micronet check failed with ECHILD and recycled network. Wait on the exit registry when the reaper is running, and add livenessProbe.failureThreshold so a single failed probe does not restart. Co-authored-by: Cursor --- README.md | 3 +- docs/configuration.md | 4 +- docs/operator.md | 2 +- docs/service-lifecycle.md | 3 +- go/config/types.go | 33 +++---- man/man5/microinit.json.5.mdoc | 13 ++- src/config.rs | 14 +++ src/constants.rs | 6 ++ src/liveness.rs | 1 + src/reaper.rs | 21 +++++ src/service.rs | 158 +++++++++++++++++++++------------ src/supervisor.rs | 34 ++++++- tests/config_test.rs | 47 ++++++++++ tests/service_test.rs | 16 ++++ tests/supervisor_test.rs | 121 ++++++++++++++++++++++++- 15 files changed, 389 insertions(+), 87 deletions(-) diff --git a/README.md b/README.md index a11d7a0..f81d0d3 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,8 @@ Works in embedded systems based on Linux as well as in containers. Lightning fast and solid-rock reliable. Inspired by supervisord and Kubernetes, handles dependencies, able to self-heal. -The services are health-checked with liveness probes and restarted. Cascade services depending on others are started as soon as dependency is started. +The services are health-checked with liveness probes and restarted after +`failureThreshold` consecutive failures (default 1). Cascade services depending on others are started as soon as dependency is started. There are two modes - `microinit init` and `microinit supervise`. Init replaces `/sbin/init`, and supervise replaces `supervisord`. diff --git a/docs/configuration.md b/docs/configuration.md index 16aea7d..d7837cf 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -198,7 +198,7 @@ Success → `succeeded`. Failure → `failed`. | `background` | Parallel start at boot | | `orderPriority` | Among ready services, lower starts earlier (default `100`; equal → alphabetical name) | | `dependsOn` | These must be `running` or `succeeded` first | -| `livenessProbe` | Optional health check; failure triggers restart | +| `livenessProbe` | Optional health check; consecutive failures reaching `failureThreshold` trigger restart | ### Liveness probe @@ -213,7 +213,7 @@ Exactly **one** of `cmd`, `httpUrl`, or `tcpAddr`: } ``` -Defaults: `interval` 60 s, `timeout` 5 s. +Defaults: `interval` 60 s, `timeout` 5 s, `failureThreshold` 1 (restart on the first failed probe). --- diff --git a/docs/operator.md b/docs/operator.md index b02dee5..2de152e 100644 --- a/docs/operator.md +++ b/docs/operator.md @@ -127,7 +127,7 @@ If `startCmd` is set, it is used instead of `cmd start`. Prefer **`exec` of the | `orderPriority` | Among ready services, lower starts earlier (default `100`; equal → name A–Z). See [Service ordering](configuration.md#service-ordering) | | `dependsOn` | Other service names that must be `running` or `succeeded` first | | `env` / `cwd` | Extra environment and working directory | -| `livenessProbe` | Optional periodic check. Exactly one of `cmd`, `httpUrl`, or `tcpAddr`. Shared: `interval` (default `60`), `timeout` (default `5`). `cmd` uses `successExitCodes` (default `[0]`); `httpUrl` uses `httpMethod` (default `GET`) and `httpAcceptedCodes` (default `[200]`); `tcpAddr` is `host:port`. Runs while `running` / `succeeded` / `failed`; failure re-runs start | +| `livenessProbe` | Optional periodic check. Exactly one of `cmd`, `httpUrl`, or `tcpAddr`. Shared: `interval` (default `60`), `timeout` (default `5`), `failureThreshold` (default `1` — restart after that many consecutive failures). `cmd` uses `successExitCodes` (default `[0]`); `httpUrl` uses `httpMethod` (default `GET`) and `httpAcceptedCodes` (default `[200]`); `tcpAddr` is `host:port`. Runs while `running` / `succeeded` / `failed`; failure re-runs start | | `securityContext` | Optional privilege drop (`runAsUser` / `runAsGroup`) and Linux capabilities. See [Security context](#security-context). Disabled on Android | Example one-shot with recovery (network bring-up): diff --git a/docs/service-lifecycle.md b/docs/service-lifecycle.md index 717c50d..9f00c6c 100644 --- a/docs/service-lifecycle.md +++ b/docs/service-lifecycle.md @@ -161,7 +161,8 @@ If **`webapp`** is already **`running`** and **`database`** crashes: - database goes through restarting / running (if `restart: true`); - **webapp is not stopped automatically** — microinit does not cascade-stop dependents when a dependency dies. -If webapp must die with the database, that belongs in the app or a `livenessProbe` on webapp. +If webapp must die with the database, that belongs in the app or a `livenessProbe` on webapp +(`failureThreshold` consecutive failed probes before a restart; default 1). --- diff --git a/go/config/types.go b/go/config/types.go index 13c9ea7..57a05e0 100644 --- a/go/config/types.go +++ b/go/config/types.go @@ -2,24 +2,24 @@ package config // ServiceDef is one service entry in microinit.json or a drop-in file. type ServiceDef struct { - Name string `json:"name"` - Enabled *bool `json:"enabled,omitempty"` - Daemon *bool `json:"daemon,omitempty"` + Name string `json:"name"` + Enabled *bool `json:"enabled,omitempty"` + Daemon *bool `json:"daemon,omitempty"` // RestartPolicy is "always", "onError" (default), or "none". - RestartPolicy string `json:"restartPolicy,omitempty"` - RestartBackoff *int `json:"restartBackoff,omitempty"` - StartWaitSecs *int `json:"startWaitSecs,omitempty"` - ShutdownWaitSecs *int `json:"shutdownWaitSecs,omitempty"` + RestartPolicy string `json:"restartPolicy,omitempty"` + RestartBackoff *int `json:"restartBackoff,omitempty"` + StartWaitSecs *int `json:"startWaitSecs,omitempty"` + ShutdownWaitSecs *int `json:"shutdownWaitSecs,omitempty"` // OrderPriority: among ready services, lower starts earlier (default 100). - OrderPriority *int `json:"orderPriority,omitempty"` - DependsOn []string `json:"dependsOn,omitempty"` - StartCmd string `json:"startCmd,omitempty"` - StopCmd string `json:"stopCmd,omitempty"` - Cmd string `json:"cmd,omitempty"` - Cwd string `json:"cwd,omitempty"` - LivenessProbe *LivenessProbe `json:"livenessProbe,omitempty"` - Labels map[string]string `json:"labels,omitempty"` - SecurityContext *SecurityContext `json:"securityContext,omitempty"` + OrderPriority *int `json:"orderPriority,omitempty"` + DependsOn []string `json:"dependsOn,omitempty"` + StartCmd string `json:"startCmd,omitempty"` + StopCmd string `json:"stopCmd,omitempty"` + Cmd string `json:"cmd,omitempty"` + Cwd string `json:"cwd,omitempty"` + LivenessProbe *LivenessProbe `json:"livenessProbe,omitempty"` + Labels map[string]string `json:"labels,omitempty"` + SecurityContext *SecurityContext `json:"securityContext,omitempty"` } // SecurityContext drops privileges and optionally keeps Linux capabilities. @@ -46,6 +46,7 @@ type LivenessProbe struct { SuccessExitCodes []int `json:"successExitCodes,omitempty"` Interval int `json:"interval,omitempty"` Timeout int `json:"timeout,omitempty"` + FailureThreshold int `json:"failureThreshold,omitempty"` } // DropinFile is the JSON envelope for files under microinit.d/services/. diff --git a/man/man5/microinit.json.5.mdoc b/man/man5/microinit.json.5.mdoc index 9f4e78c..abda802 100644 --- a/man/man5/microinit.json.5.mdoc +++ b/man/man5/microinit.json.5.mdoc @@ -196,11 +196,14 @@ must be set. Shared fields: .Cm interval in seconds -.Pq default 60 -and +.Pq default 60 , .Cm timeout in seconds -.Pq default 5 . +.Pq default 5 , +and +.Cm failureThreshold +consecutive failed probes before a restart +.Pq default 1 . .Cm cmd uses .Cm successExitCodes @@ -223,7 +226,9 @@ or .Cm failed , microinit probes every .Cm interval -seconds; failure re-runs start. +seconds; after +.Cm failureThreshold +consecutive failures, microinit re-runs start. .It Cm securityContext Optional object. On Linux: drops the service to another user/group and optionally keeps Linux capabilities across diff --git a/src/config.rs b/src/config.rs index db3bc52..0524fb4 100644 --- a/src/config.rs +++ b/src/config.rs @@ -221,6 +221,9 @@ pub struct LivenessProbe { /// Seconds before a probe attempt is aborted. Default 5. #[serde(default = "default_liveness_timeout")] pub timeout: u64, + /// Consecutive failed probes before a restart. Default 1 (restart on first failure). + #[serde(default = "default_liveness_failure_threshold")] + pub failure_threshold: u32, } fn default_liveness_interval() -> u64 { @@ -231,6 +234,10 @@ fn default_liveness_timeout() -> u64 { 5 } +fn default_liveness_failure_threshold() -> u32 { + 1 +} + fn default_http_accepted_codes() -> Vec { vec![200] } @@ -662,6 +669,12 @@ impl Config { svc.name ))); } + if probe.failure_threshold < 1 { + return Err(Error::Config(format!( + "service '{}': livenessProbe.failureThreshold must be >= 1", + svc.name + ))); + } } validate_labels(&svc.name, &svc.labels)?; if let Some(ref sec) = svc.security_context { @@ -977,6 +990,7 @@ pub fn example_config() -> Config { http_method: "GET".into(), interval: 30, timeout: 5, + failure_threshold: 1, }), labels: BTreeMap::new(), security_context: None, diff --git a/src/constants.rs b/src/constants.rs index cd9479b..bb19e5c 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -41,6 +41,12 @@ pub const MAX_WATCH_FOLLOWERS: usize = 8; pub const WATCH_HEARTBEAT: Duration = Duration::from_secs(10); /// Poll interval while waiting for a process to exit after SIGTERM. pub const TERMINATE_POLL: Duration = Duration::from_millis(100); +/// Poll interval while waiting on an owned `Child` (no PID-1 reaper). +pub const CHILD_WAIT_POLL: Duration = Duration::from_millis(50); +/// After SIGKILL, wait this long for the central reaper to publish the exit. +pub const REAP_AFTER_KILL: Duration = Duration::from_millis(500); +/// Slice used when waiting forever on the exit registry (avoids `Instant` overflow). +pub const CHILD_WAIT_SLICE: Duration = Duration::from_secs(60); /// Per-service lifecycle event ring capacity (bounded memory). /// Same as [`EVENT_RETURN`]: the ring only exists to feed `describe`. pub const EVENT_RING_CAP: usize = 16; diff --git a/src/liveness.rs b/src/liveness.rs index d8d0ce0..e028c8f 100644 --- a/src/liveness.rs +++ b/src/liveness.rs @@ -177,6 +177,7 @@ mod tests { http_method: "GET".into(), interval: 1, timeout: 2, + failure_threshold: 1, } } diff --git a/src/reaper.rs b/src/reaper.rs index e98b4e4..b9113cd 100644 --- a/src/reaper.rs +++ b/src/reaper.rs @@ -82,6 +82,15 @@ pub fn global_exits() -> Arc { .clone() } +/// True once [`ensure_reaper_thread`] has started the `waitpid(-1)` loop. +/// +/// Callers that spawn short-lived children MUST NOT use `Child::wait` / +/// `try_wait` while this is true — the reaper already owns `waitpid(-1)`. +#[must_use] +pub fn is_running() -> bool { + REAPER_STARTED.load(Ordering::SeqCst) +} + /// Ensure a single background `waitpid(-1)` thread publishes into [`global_exits`]. pub fn ensure_reaper_thread() { if REAPER_STARTED.swap(true, Ordering::SeqCst) { @@ -97,3 +106,15 @@ pub fn ensure_reaper_thread() { thread::sleep(CTL_POLL); }); } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn is_running_false_until_started() { + // This crate's unit-test binary never starts the reaper; integration + // tests that call `ensure_reaper_thread` live in a separate process. + assert!(!is_running()); + } +} diff --git a/src/service.rs b/src/service.rs index 69c6327..77bfb8d 100644 --- a/src/service.rs +++ b/src/service.rs @@ -6,9 +6,10 @@ use std::process::{Child, Command, Stdio}; use std::time::Duration; use crate::config::ServiceConfig; -use crate::constants::TERMINATE_POLL; +use crate::constants::{CHILD_WAIT_POLL, CHILD_WAIT_SLICE, REAP_AFTER_KILL, TERMINATE_POLL}; use crate::error::{Error, Result}; use crate::protocol::RunningIdentity; +use crate::reaper; /// Shell used to run service `cmd` / probes / stop scripts. #[cfg(target_os = "android")] @@ -113,38 +114,115 @@ pub fn spawn_shell(cmd: &str, cfg: &ServiceConfig) -> Result { .map_err(|e| Error::Service(cfg.name.clone(), e.to_string())) } -/// Run a shell command to completion (for stop/restart scripts that are short-lived). -pub fn run_shell( +enum ChildWait { + Forever, + Timeout(Duration), +} + +fn spawn_shell_cmd( cmd: &str, cfg: &ServiceConfig, env_extra: &HashMap, -) -> Result { + quiet: bool, +) -> Result { #[cfg(not(target_os = "android"))] let ident = resolve_sec(cfg)?; #[cfg(not(target_os = "android"))] - let status = build_shell_command(cmd, cfg, env_extra, ident.as_ref()) - .status() - .map_err(|e| Error::Service(cfg.name.clone(), e.to_string()))?; + let mut cmd_built = build_shell_command(cmd, cfg, env_extra, ident.as_ref()); #[cfg(target_os = "android")] - let status = build_shell_command(cmd, cfg, env_extra) - .status() - .map_err(|e| Error::Service(cfg.name.clone(), e.to_string()))?; - Ok(status.code().unwrap_or(1)) + let mut cmd_built = build_shell_command(cmd, cfg, env_extra); + if quiet { + cmd_built.stdout(Stdio::null()).stderr(Stdio::null()); + } + cmd_built + .spawn() + .map_err(|e| Error::Service(cfg.name.clone(), e.to_string())) +} + +/// Wait for a spawned child without racing the PID-1 reaper. +/// +/// While [`reaper::is_running`] the central `waitpid(-1)` thread already reaps +/// every child. `Child::wait` / `try_wait` then return `ECHILD`. Forget the +/// `Child` and wait on [`reaper::global_exits`] instead. Unit tests that never +/// start the reaper keep the owned-wait path. +fn wait_spawned_child(mut child: Child, name: &str, bound: ChildWait) -> Result> { + let pid = child.id() as i32; + if reaper::is_running() { + drop(child.stdout.take()); + drop(child.stderr.take()); + std::mem::forget(child); + wait_via_registry(pid, bound) + } else { + wait_via_owned(child, name, bound) + } +} + +fn wait_via_registry(pid: i32, bound: ChildWait) -> Result> { + let exits = reaper::global_exits(); + match bound { + ChildWait::Forever => loop { + if let Some(code) = exits.wait_take(pid, CHILD_WAIT_SLICE) { + return Ok(Some(code)); + } + }, + ChildWait::Timeout(timeout) => { + if let Some(code) = exits.wait_take(pid, timeout) { + return Ok(Some(code)); + } + terminate_pid(nix::unistd::Pid::from_raw(pid), 0); + let _ = exits.wait_take(pid, REAP_AFTER_KILL); + Ok(None) + } + } +} + +fn wait_via_owned(mut child: Child, name: &str, bound: ChildWait) -> Result> { + match bound { + ChildWait::Forever => { + let status = child + .wait() + .map_err(|e| Error::Service(name.to_string(), e.to_string()))?; + Ok(Some(status.code().unwrap_or(1))) + } + ChildWait::Timeout(timeout) => { + use std::thread; + use std::time::Instant; + let deadline = Instant::now() + timeout; + loop { + match child.try_wait() { + Ok(Some(status)) => return Ok(Some(status.code().unwrap_or(1))), + Ok(None) => { + if Instant::now() >= deadline { + let pid = child.id() as i32; + terminate_pid(nix::unistd::Pid::from_raw(pid), 0); + let _ = child.wait(); + return Ok(None); + } + thread::sleep(CHILD_WAIT_POLL); + } + Err(e) => { + return Err(Error::Service(name.to_string(), e.to_string())); + } + } + } + } + } +} + +/// Run a shell command to completion (for stop/restart scripts that are short-lived). +pub fn run_shell( + cmd: &str, + cfg: &ServiceConfig, + env_extra: &HashMap, +) -> Result { + let child = spawn_shell_cmd(cmd, cfg, env_extra, false)?; + Ok(wait_spawned_child(child, &cfg.name, ChildWait::Forever)?.unwrap_or(1)) } /// Like [`run_shell`], but discard stdout/stderr (liveness probes must stay cheap/quiet). pub fn run_shell_quiet(cmd: &str, cfg: &ServiceConfig) -> Result { - #[cfg(not(target_os = "android"))] - let ident = resolve_sec(cfg)?; - #[cfg(not(target_os = "android"))] - let mut c = build_shell_command(cmd, cfg, &HashMap::new(), ident.as_ref()); - #[cfg(target_os = "android")] - let mut c = build_shell_command(cmd, cfg, &HashMap::new()); - c.stdout(Stdio::null()).stderr(Stdio::null()); - let status = c - .status() - .map_err(|e| Error::Service(cfg.name.clone(), e.to_string()))?; - Ok(status.code().unwrap_or(1)) + let child = spawn_shell_cmd(cmd, cfg, &HashMap::new(), true)?; + Ok(wait_spawned_child(child, &cfg.name, ChildWait::Forever)?.unwrap_or(1)) } /// Quiet shell command with a hard timeout; kills the process group on expiry. @@ -155,40 +233,8 @@ pub fn run_shell_quiet_timeout( cfg: &ServiceConfig, timeout: Duration, ) -> Result> { - use std::thread; - use std::time::Instant; - - #[cfg(not(target_os = "android"))] - let ident = resolve_sec(cfg)?; - #[cfg(not(target_os = "android"))] - let mut cmd_built = build_shell_command(cmd, cfg, &HashMap::new(), ident.as_ref()); - #[cfg(target_os = "android")] - let mut cmd_built = build_shell_command(cmd, cfg, &HashMap::new()); - - let mut child = cmd_built - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn() - .map_err(|e| Error::Service(cfg.name.clone(), e.to_string()))?; - - let deadline = Instant::now() + timeout; - loop { - match child.try_wait() { - Ok(Some(status)) => return Ok(Some(status.code().unwrap_or(1))), - Ok(None) => { - if Instant::now() >= deadline { - let pid = child.id() as i32; - terminate_pid(nix::unistd::Pid::from_raw(pid), 0); - let _ = child.wait(); - return Ok(None); - } - thread::sleep(Duration::from_millis(50)); - } - Err(e) => { - return Err(Error::Service(cfg.name.clone(), e.to_string())); - } - } - } + let child = spawn_shell_cmd(cmd, cfg, &HashMap::new(), true)?; + wait_spawned_child(child, &cfg.name, ChildWait::Timeout(timeout)) } /// Read the real uid/gid of a live process from `/proc//status`. diff --git a/src/supervisor.rs b/src/supervisor.rs index 6d19bae..fde70fa 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -920,6 +920,7 @@ impl Supervisor { fn monitor_loop(self: Arc, name: String, rx: std::sync::mpsc::Receiver) { let mut tracked: Option = None; let mut next_liveness: Option = None; + let mut liveness_streak: u32 = 0; loop { if self.shared.stop_all.load(Ordering::SeqCst) { @@ -956,6 +957,7 @@ impl Supervisor { self.apply_state(&name, ServiceState::Stopping, None); self.stop_tracked(&cfg, &mut tracked); next_liveness = None; + liveness_streak = 0; let enabled = self.shared.is_enabled(&name).unwrap_or(true); if enabled { self.apply_state(&name, ServiceState::Stopped, None); @@ -967,6 +969,7 @@ impl Supervisor { CtlMsg::Restart => { self.hub .emit(INIT_SERVICE, LogLevel::Info, format!("restarting {name}")); + liveness_streak = 0; self.stop_tracked(&cfg, &mut tracked); if let Ok(restart) = cfg.resolve_restart() { let code = run_shell(&restart, &cfg, &HashMap::new()).unwrap_or(1); @@ -988,6 +991,7 @@ impl Supervisor { if tracked.is_some() { continue; } + liveness_streak = 0; if let Err(e) = self.do_start(&cfg, &mut tracked, force) { self.hub .emit(INIT_SERVICE, LogLevel::Error, format!("{name}: {e}")); @@ -1002,6 +1006,7 @@ impl Supervisor { if let Some(pid) = tracked { if let Some(code) = self.exits.take(pid) { tracked = None; + liveness_streak = 0; if let Ok(cfg) = self.service_cfg(&name) { self.on_process_exit(&cfg, &mut tracked, code); next_liveness = Self::schedule_liveness(&cfg); @@ -1026,7 +1031,12 @@ impl Supervisor { if !self.shared.stop_all.load(Ordering::SeqCst) { if let Ok(cfg) = self.service_cfg(&name) { - self.maybe_liveness(&cfg, &mut tracked, &mut next_liveness); + self.maybe_liveness( + &cfg, + &mut tracked, + &mut next_liveness, + &mut liveness_streak, + ); } } } @@ -1038,12 +1048,14 @@ impl Supervisor { .map(|p| Instant::now() + Duration::from_secs(p.interval)) } - /// Periodic health check: on failure, stop and re-run start. + /// Periodic health check: on consecutive failures reaching `failureThreshold`, + /// stop and re-run start. fn maybe_liveness( self: &Arc, cfg: &ServiceConfig, tracked: &mut Option, next_liveness: &mut Option, + streak: &mut u32, ) { let Some(probe) = cfg.liveness_probe.as_ref() else { *next_liveness = None; @@ -1079,6 +1091,7 @@ impl Supervisor { *next_liveness = Some(Instant::now() + Duration::from_secs(probe.interval)); if outcome.is_ok() { + *streak = 0; if matches!(state, ServiceState::Failed) && !cfg.daemon { self.apply_state(&cfg.name, ServiceState::Succeeded, None); } @@ -1089,13 +1102,28 @@ impl Supervisor { ProbeResult::Fail(r) => r, ProbeResult::Ok => unreachable!(), }; + *streak = streak.saturating_add(1); + let threshold = probe.failure_threshold.max(1); + self.shared + .bump_liveness_failures(&cfg.name, Some(reason.clone())); + if *streak < threshold { + self.hub.emit( + INIT_SERVICE, + LogLevel::Warn, + format!( + "{}: livenessProbe failed ({reason}) ({streak}/{threshold})", + cfg.name + ), + ); + return; + } + *streak = 0; self.hub.emit( INIT_SERVICE, LogLevel::Warn, format!("{}: livenessProbe failed ({reason}), restarting", cfg.name), ); self.apply_state(&cfg.name, ServiceState::Restarting, None); - self.shared.bump_liveness_failures(&cfg.name, Some(reason)); self.shared.bump_restarts(&cfg.name); self.stop_tracked(cfg, tracked); if let Err(e) = self.do_start(cfg, tracked, false) { diff --git a/tests/config_test.rs b/tests/config_test.rs index 4c2465a..3e1d6f0 100644 --- a/tests/config_test.rs +++ b/tests/config_test.rs @@ -379,6 +379,7 @@ fn parses_liveness_probe_with_defaults() { assert_eq!(probe.success_exit_codes, vec![0]); assert_eq!(probe.interval, 60); assert_eq!(probe.timeout, 5); + assert_eq!(probe.failure_threshold, 1); assert_eq!(probe.http_method, "GET"); assert_eq!(probe.http_accepted_codes, vec![200]); } @@ -406,6 +407,7 @@ fn parses_http_and_tcp_liveness_probes() { assert_eq!(p.http_method, "HEAD"); assert_eq!(p.http_accepted_codes, vec![200, 204]); assert!(p.cmd.is_none()); + assert_eq!(p.failure_threshold, 1); let tcp = r#"{ "version": 1, @@ -444,6 +446,7 @@ fn rejects_empty_liveness_probe_cmd() { http_method: "GET".into(), interval: 30, timeout: 5, + failure_threshold: 1, }); cfg.services.push(svc); assert!(cfg.validate().is_err()); @@ -467,6 +470,50 @@ fn rejects_multiple_liveness_probe_kinds() { assert!(err.contains("exactly one"), "{err}"); } +#[test] +fn parses_liveness_failure_threshold() { + let raw = r#"{ + "version": 1, + "services": [{ + "name": "net", + "cmd": "/bin/true", + "livenessProbe": { + "cmd": "true", + "failureThreshold": 3 + } + }] + }"#; + let cfg: Config = serde_json::from_str(raw).unwrap(); + cfg.validate().unwrap(); + assert_eq!( + cfg.get("net") + .unwrap() + .liveness_probe + .as_ref() + .unwrap() + .failure_threshold, + 3 + ); +} + +#[test] +fn rejects_zero_liveness_failure_threshold() { + let raw = r#"{ + "version": 1, + "services": [{ + "name": "net", + "cmd": "/bin/true", + "livenessProbe": { + "cmd": "true", + "failureThreshold": 0 + } + }] + }"#; + let cfg: Config = serde_json::from_str(raw).unwrap(); + let err = cfg.validate().unwrap_err().to_string(); + assert!(err.contains("failureThreshold"), "{err}"); +} + #[test] fn dropins_merge_lex_later_wins() { let dir = temp_dir("dropins"); diff --git a/tests/service_test.rs b/tests/service_test.rs index e475b6f..48d59ae 100644 --- a/tests/service_test.rs +++ b/tests/service_test.rs @@ -1,6 +1,7 @@ //! Unit/integration tests for microinit::service use std::collections::{BTreeMap, HashMap}; +use std::time::Duration; use microinit::config::{RestartPolicy, ServiceConfig}; use microinit::service::*; @@ -185,3 +186,18 @@ fn path_precedence_default_file_cfg_extra() { envfile::install(HashMap::new()); } + +#[test] +fn run_shell_quiet_timeout_ok_and_expiry() { + let c = cfg(); + let code = run_shell_quiet_timeout("exit 0", &c, Duration::from_secs(2)) + .unwrap() + .expect("true should finish"); + assert_eq!(code, 0); + assert!( + run_shell_quiet_timeout("sleep 30", &c, Duration::from_millis(200)) + .unwrap() + .is_none(), + "sleep should time out" + ); +} diff --git a/tests/supervisor_test.rs b/tests/supervisor_test.rs index 9582cb6..d97021d 100644 --- a/tests/supervisor_test.rs +++ b/tests/supervisor_test.rs @@ -6,7 +6,9 @@ use std::sync::{Arc, Mutex}; use std::thread; use std::time::Duration; -use microinit::config::{Config, EarlyBootConfig, LogsConfig, RestartPolicy, ServiceConfig}; +use microinit::config::{ + Config, EarlyBootConfig, LivenessProbe, LogsConfig, RestartPolicy, ServiceConfig, +}; use microinit::console::Console; use microinit::error::Error; use microinit::logs::LogHub; @@ -424,8 +426,6 @@ fn start_force_bypasses_waiting_for_dependency() { #[test] fn liveness_probe_restarts_oneshot_on_failure() { - use microinit::config::LivenessProbe; - let marker = std::env::temp_dir().join(format!( "microinit-live-{}-{}", std::process::id(), @@ -449,6 +449,7 @@ fn liveness_probe_restarts_oneshot_on_failure() { http_method: "GET".into(), interval: 1, timeout: 5, + failure_threshold: 1, }); let (sup, dir) = make_sup(vec![svc]); @@ -507,6 +508,120 @@ fn liveness_probe_restarts_oneshot_on_failure() { let _ = std::fs::remove_dir_all(dir); } +fn cmd_probe(cmd: String, interval: u64, timeout: u64, threshold: u32) -> LivenessProbe { + LivenessProbe { + cmd: Some(cmd), + http_url: None, + tcp_addr: None, + success_exit_codes: vec![0], + http_accepted_codes: vec![200], + http_method: "GET".into(), + interval, + timeout, + failure_threshold: threshold, + } +} + +#[test] +fn liveness_probe_true_does_not_fail_under_reaper() { + let mut svc = job("net", "true", &[], true); + svc.liveness_probe = Some(cmd_probe("true".into(), 1, 5, 1)); + let (sup, dir) = make_sup(vec![svc]); + sup.boot().unwrap(); + thread::sleep(Duration::from_millis(2500)); + let st = sup.status("net").unwrap(); + assert_eq!(st.state, ServiceState::Succeeded); + assert_eq!( + st.liveness_failures, 0, + "successful cmd probe must not report ECHILD / fail under the reaper" + ); + assert_eq!(st.restarts, 0); + let _ = std::fs::remove_dir_all(dir); +} + +#[test] +fn liveness_probe_below_threshold_does_not_restart() { + let marker = std::env::temp_dir().join(format!( + "microinit-live-below-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let _ = std::fs::remove_file(&marker); + let path = marker.to_string_lossy(); + let mut svc = job("net", &format!("touch {path}"), &[], true); + svc.liveness_probe = Some(cmd_probe(format!("test -f {path}"), 1, 5, 3)); + let (sup, dir) = make_sup(vec![svc]); + sup.boot().unwrap(); + assert!(marker.exists()); + std::fs::remove_file(&marker).unwrap(); + + let deadline = std::time::Instant::now() + Duration::from_secs(3); + let mut saw_fail = false; + while std::time::Instant::now() < deadline { + let st = sup.status("net").unwrap(); + if st.liveness_failures >= 1 { + saw_fail = true; + break; + } + thread::sleep(Duration::from_millis(100)); + } + let st = sup.status("net").unwrap(); + assert!( + saw_fail, + "expected a liveness failure after removing marker" + ); + assert_eq!(st.restarts, 0, "must not restart below failureThreshold=3"); + assert!( + !marker.exists(), + "start must not have re-run (would recreate marker)" + ); + + let _ = std::fs::remove_file(&marker); + let _ = std::fs::remove_dir_all(dir); +} + +#[test] +fn liveness_probe_reaches_threshold_restarts() { + let marker = std::env::temp_dir().join(format!( + "microinit-live-thr-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let _ = std::fs::remove_file(&marker); + let path = marker.to_string_lossy(); + let mut svc = job("net", &format!("touch {path}"), &[], true); + svc.liveness_probe = Some(cmd_probe(format!("test -f {path}"), 1, 5, 2)); + let (sup, dir) = make_sup(vec![svc]); + sup.boot().unwrap(); + assert!(marker.exists()); + std::fs::remove_file(&marker).unwrap(); + + let deadline = std::time::Instant::now() + Duration::from_secs(6); + while std::time::Instant::now() < deadline { + let st = sup.status("net").unwrap(); + if marker.exists() && st.restarts >= 1 { + break; + } + thread::sleep(Duration::from_millis(100)); + } + let st = sup.status("net").unwrap(); + assert!( + marker.exists(), + "start should recreate marker after reaching failureThreshold" + ); + assert!(st.restarts >= 1, "expected restart at threshold 2"); + assert!(st.liveness_failures >= 2); + + let _ = std::fs::remove_file(&marker); + let _ = std::fs::remove_dir_all(dir); +} + #[test] fn describe_deps_and_reverse_deps() { let (sup, dir) = make_sup(vec![ From 7fdf12d3746a0c1fcd6c14af7f51512d24b74f8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20K=C4=99ska?= <372403+keskad@users.noreply.github.com> Date: Wed, 26 Aug 2026 20:07:57 +0200 Subject: [PATCH 2/2] test: drop flaky 200ms sleep timeout assertion cmd_timeout already covers run_shell_quiet_timeout expiry; the 200ms sleep check returned a code on CI instead of None. Co-authored-by: Cursor --- tests/service_test.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/tests/service_test.rs b/tests/service_test.rs index 48d59ae..e475b6f 100644 --- a/tests/service_test.rs +++ b/tests/service_test.rs @@ -1,7 +1,6 @@ //! Unit/integration tests for microinit::service use std::collections::{BTreeMap, HashMap}; -use std::time::Duration; use microinit::config::{RestartPolicy, ServiceConfig}; use microinit::service::*; @@ -186,18 +185,3 @@ fn path_precedence_default_file_cfg_extra() { envfile::install(HashMap::new()); } - -#[test] -fn run_shell_quiet_timeout_ok_and_expiry() { - let c = cfg(); - let code = run_shell_quiet_timeout("exit 0", &c, Duration::from_secs(2)) - .unwrap() - .expect("true should finish"); - assert_eq!(code, 0); - assert!( - run_shell_quiet_timeout("sleep 30", &c, Duration::from_millis(200)) - .unwrap() - .is_none(), - "sleep should time out" - ); -}