diff --git a/crates/mind-body/src/cli.rs b/crates/mind-body/src/cli.rs index bb2f4b2..7c80658 100644 --- a/crates/mind-body/src/cli.rs +++ b/crates/mind-body/src/cli.rs @@ -4,10 +4,14 @@ //! //! ```text //! mind-control [--root ] --instance show -//! mind-control --instance brake hold +//! mind-control --instance brake hold (also stops her running units) //! mind-control --instance brake release //! mind-control --instance dial set --heat H --base-cooldown-s S --base-run-usd U //! ``` +//! +//! `brake hold` writes the brake held and then, whatever the write's outcome, +//! stops the instance's running units through the `stop_units` parameter. It +//! fails if either step failed. No other command stops anything. use std::io::Write; use std::path::Path; @@ -26,18 +30,22 @@ const USAGE: &str = /// Who made the change: `SUDO_USER`, else `USER`, else `unknown`. An empty /// variable counts as unset. pub fn operator_name(sudo_user: Option, user: Option) -> String { - [sudo_user, user] - .into_iter() - .flatten() - .find(|name| !name.is_empty()) - .unwrap_or_else(|| "unknown".to_string()) + [sudo_user, user].into_iter().flatten().find(|name| !name.is_empty()).unwrap_or_else(|| "unknown".to_string()) } /// Runs one command against `//control.cc`. `root` is the /// default; a leading `--root ` argument replaces it (argv, not environment, -/// because `sudo -E` carries the environment). The operator's name, the clock and the output are parameters so a test drives -/// the whole command without touching `/etc`. -pub fn run(args: &[String], root: &Path, set_by: &str, now: DateTime, out: &mut impl Write) -> Result<()> { +/// because `sudo -E` carries the environment). The operator's name, the clock, the output and the stopper are parameters so a test drives +/// the whole command without touching `/etc` or systemd. `stop_units` is +/// called only by `brake hold`, after the write, whatever the write's outcome. +pub fn run( + args: &[String], + root: &Path, + set_by: &str, + now: DateTime, + out: &mut impl Write, + stop_units: &mut dyn FnMut(&Slug) -> Result<()>, +) -> Result<()> { let (root, args) = match args { [flag, dir, rest @ ..] if flag == "--root" => (Path::new(dir), rest), _ => (root, args), @@ -50,7 +58,19 @@ pub fn run(args: &[String], root: &Path, set_by: &str, now: DateTime, out: let words: Vec<&str> = command.iter().map(String::as_str).collect(); match words.as_slice() { ["show"] => show(root, &instance, out), - ["brake", "hold"] => ControlWriter::open(&path)?.set_brake(false, now, set_by), + ["brake", "hold"] => { + let written = ControlWriter::open(&path).and_then(|mut writer| writer.set_brake(false, now, set_by)); + let stopped = stop_units(&instance); + match (written, stopped) { + (Ok(()), Ok(())) => { + writeln!(out, "units: stopped mind-self@{0}:* mind-persona@{0}:*", instance.0)?; + Ok(()) + } + (Err(write), Ok(())) => Err(write.context("the brake was not written; the units were stopped")), + (Ok(()), Err(stop)) => Err(stop.context("the brake is held; the units were not stopped")), + (Err(write), Err(stop)) => Err(anyhow!("the brake was not written ({write:#}) and the units were not stopped ({stop:#})")), + } + } ["brake", "release"] => ControlWriter::open(&path)?.set_brake(true, now, set_by), ["dial", "set", flags @ ..] => { let dial = parse_dial(flags, now, set_by)?; @@ -77,9 +97,7 @@ fn parse_dial(flags: &[&str], now: DateTime, set_by: &str) -> Result, pub stdout: String, } @@ -71,7 +73,11 @@ struct RealSystemctl; impl Systemctl for RealSystemctl { fn run(&self, args: &[&str]) -> Result { let output = std::process::Command::new("systemctl").args(args).output().map_err(|_| anyhow!("systemctl could not be run"))?; - Ok(SystemctlReply { success: output.status.success(), stdout: String::from_utf8_lossy(&output.stdout).into_owned() }) + Ok(SystemctlReply { + success: output.status.success(), + code: output.status.code(), + stdout: String::from_utf8_lossy(&output.stdout).into_owned(), + }) } } @@ -101,16 +107,50 @@ impl Default for SystemdLauncher { } } +/// The unit template of one turn. Start, alive and stop all name units from +/// these two. +fn template(turn: RunTurn) -> &'static str { + match turn { + RunTurn::PersonaTurn => "mind-persona", + RunTurn::SelfRun => "mind-self", + } +} + /// The unit of one turn: the template by turn, then the instance, held to the /// leaf's grammar first so it cannot escape the name. The error names nothing /// of the value. Start and alive both name the unit here. pub(crate) fn unit(instance: &Slug, turn: RunTurn) -> Result { instance.validate_slug().map_err(|_| anyhow!("the instance is not a unit name"))?; - let template = match turn { - RunTurn::PersonaTurn => "mind-persona", - RunTurn::SelfRun => "mind-self", - }; - Ok(format!("{template}@{}.service", instance.0)) + Ok(format!("{}@{}.service", template(turn), instance.0)) +} + +/// The arguments of the one synchronous stop: every unit of both turns of this +/// instance, matched by glob. The `:` after the instance keeps `eureka` from +/// matching `eureka-two`. No `--no-block`: `systemctl stop` returns when the +/// units are down. The instance is not validated here; `stop_all` does. +pub(crate) fn stop_args(instance: &Slug) -> Vec { + let mut args = vec!["stop".to_string(), "--".to_string()]; + args.extend([RunTurn::SelfRun, RunTurn::PersonaTurn].map(|turn| format!("{}@{}:*.service", template(turn), instance.0))); + args +} + +impl SystemdLauncher { + /// Stops every unit of both turns of one instance and waits for them. An + /// inherent method, not on `Launcher`: only the operator's brake hold + /// calls it. A glob that matches no loaded unit is a success. + pub fn stop_all(&self, instance: &Slug) -> Result<()> { + instance.validate_slug().map_err(|_| anyhow!("the instance is not a unit name"))?; + let args = stop_args(instance); + let args: Vec<&str> = args.iter().map(String::as_str).collect(); + let reply = self.systemctl.run(&args)?; + if reply.success { + return Ok(()); + } + match reply.code { + Some(code) => Err(anyhow!("systemctl stop failed with exit status {code}")), + None => Err(anyhow!("systemctl stop was ended by a signal")), + } + } } impl Launcher for SystemdLauncher { @@ -1082,18 +1122,20 @@ mod tests { struct Spy { calls: RefCell>>, success: Cell, + code: Cell>, stdout: RefCell, } impl Systemctl for &'static Spy { fn run(&self, args: &[&str]) -> Result { self.calls.borrow_mut().push(args.iter().map(|arg| arg.to_string()).collect()); - Ok(SystemctlReply { success: self.success.get(), stdout: self.stdout.borrow().clone() }) + Ok(SystemctlReply { success: self.success.get(), code: self.code.get(), stdout: self.stdout.borrow().clone() }) } } fn spied() -> (SystemdLauncher, &'static Spy) { - let spy: &'static Spy = Box::leak(Box::new(Spy { calls: RefCell::default(), success: Cell::new(true), stdout: RefCell::default() })); + let spy: &'static Spy = + Box::leak(Box::new(Spy { calls: RefCell::default(), success: Cell::new(true), code: Cell::new(Some(0)), stdout: RefCell::default() })); (SystemdLauncher::over(Box::new(spy)), spy) } @@ -1126,6 +1168,45 @@ mod tests { assert_eq!(spy.calls.borrow()[0], vec!["is-active", "mind-self@eureka.service"]); } + #[test] + fn stop_args_is_one_synchronous_stop_scoped_to_one_instance() { + let args = stop_args(&eureka()); + assert_eq!(args, ["stop", "--", "mind-self@eureka:*.service", "mind-persona@eureka:*.service"]); + // The text before each glob star must not be a prefix of another instance's unit. + for (pattern, other) in [(&args[2], "mind-self@eureka-two:x.service"), (&args[3], "mind-persona@eureka-two:x.service")] { + assert!(!other.starts_with(pattern.split('*').next().unwrap()), "{pattern}"); + } + } + + #[test] + fn stop_all_runs_the_one_stop_and_waits() { + let (launcher, spy) = spied(); + launcher.stop_all(&eureka()).unwrap(); + assert_eq!(*spy.calls.borrow(), vec![stop_args(&eureka())]); + } + + #[test] + fn stop_all_with_an_invalid_instance_is_an_error_and_runs_no_systemctl() { + let (launcher, spy) = spied(); + for bad in ["canary-../evil-7f3a91", "canary instance", "", "canary*"] { + let error = launcher.stop_all(&Slug(bad.into())).unwrap_err(); + assert!(!format!("{error:#}").contains("canary"), "{error:#}"); + } + assert!(spy.calls.borrow().is_empty()); + } + + #[test] + fn a_failed_stop_is_an_error_naming_the_exit_status_and_no_instance() { + let (launcher, spy) = spied(); + spy.success.set(false); + spy.code.set(Some(5)); + let shown = format!("{:#}", launcher.stop_all(&Slug("canary-instance-7f3a91".into())).unwrap_err()); + assert!(shown.contains("exit status 5") && !shown.contains("canary"), "{shown}"); + spy.code.set(None); + let shown = format!("{:#}", launcher.stop_all(&eureka()).unwrap_err()); + assert!(shown.contains("signal"), "{shown}"); + } + #[test] fn a_failed_start_names_no_instance() { let (launcher, spy) = spied(); diff --git a/crates/mind-body/src/main.rs b/crates/mind-body/src/main.rs index 014faf2..a37f9b7 100644 --- a/crates/mind-body/src/main.rs +++ b/crates/mind-body/src/main.rs @@ -32,6 +32,7 @@ fn main() { &set_by, chrono::Utc::now(), &mut std::io::stdout(), + &mut |instance| mind_body::launch::SystemdLauncher::new().stop_all(instance), ); if let Err(error) = result { eprintln!("mind-control: {error:#}"); diff --git a/crates/mind-body/tests/cli.rs b/crates/mind-body/tests/cli.rs index ff6e8ea..1529015 100644 --- a/crates/mind-body/tests/cli.rs +++ b/crates/mind-body/tests/cli.rs @@ -3,14 +3,32 @@ //! status. Nothing touches `/etc`. use chrono::{TimeZone, Utc}; +use eureka_pipeline::Slug; use mind_body::cli::{operator_name, run}; fn go(root: &std::path::Path, args: &[&str], set_by: &str, secs: i64) -> (anyhow::Result<()>, String) { + let (result, out, _) = go_stopping(root, args, set_by, secs, |_| Ok(())); + (result, out) +} + +/// Runs one command with a stopper that records the instance of every call and +/// answers with `stop`. `stop` also sees the call before it returns. +fn go_stopping( + root: &std::path::Path, + args: &[&str], + set_by: &str, + secs: i64, + mut stop: impl FnMut(&Slug) -> anyhow::Result<()>, +) -> (anyhow::Result<()>, String, Vec) { let args: Vec = args.iter().map(|a| a.to_string()).collect(); let mut out = Vec::new(); + let mut calls = Vec::new(); let now = Utc.timestamp_opt(1_800_000_000 + secs, 0).unwrap(); - let result = run(&args, root, set_by, now, &mut out); - (result, String::from_utf8(out).unwrap()) + let result = run(&args, root, set_by, now, &mut out, &mut |instance| { + calls.push(instance.0.clone()); + stop(instance) + }); + (result, String::from_utf8(out).unwrap(), calls) } #[test] @@ -49,13 +67,12 @@ fn instances_do_not_share_a_store() { fn a_dial_set_outside_bounds_refuses_and_leaves_the_store_unchanged() { let root = tempfile::tempdir().unwrap(); go(root.path(), &["--instance", "eureka", "brake", "release"], "op", 0).0.unwrap(); - go(root.path(), &["--instance", "eureka", "dial", "set", "--heat", "1", "--base-cooldown-s", "60", "--base-run-usd", "1"], "op", 1) - .0 - .unwrap(); + go(root.path(), &["--instance", "eureka", "dial", "set", "--heat", "1", "--base-cooldown-s", "60", "--base-run-usd", "1"], "op", 1).0.unwrap(); let path = root.path().join("eureka").join("control.cc"); let before = std::fs::read(&path).unwrap(); for heat in ["2.0001", "0.0499", "0", "-1"] { - let (r, _) = go(root.path(), &["--instance", "eureka", "dial", "set", "--heat", heat, "--base-cooldown-s", "60", "--base-run-usd", "1"], "op", 9); + let (r, _) = + go(root.path(), &["--instance", "eureka", "dial", "set", "--heat", heat, "--base-cooldown-s", "60", "--base-run-usd", "1"], "op", 9); assert!(r.is_err(), "heat {heat} must be refused"); assert_eq!(std::fs::read(&path).unwrap(), before, "heat {heat} changed the store"); } @@ -134,3 +151,75 @@ fn a_root_flag_without_a_directory_or_an_instance_is_refused_and_writes_nothing( assert_eq!(std::fs::read_dir(default.path()).unwrap().count(), 0); assert!(!std::path::Path::new("/nonexistent-root-for-test").exists()); } + +fn shown(root: &std::path::Path) -> String { + go(root, &["--instance", "eureka", "show"], "op", 0).1 +} + +#[test] +fn brake_hold_stops_once_with_the_instance_and_the_store_already_held() { + let root = tempfile::tempdir().unwrap(); + go(root.path(), &["--instance", "eureka", "brake", "release"], "op", 0).0.unwrap(); + let mut seen_at_stop = String::new(); + let (result, out, calls) = go_stopping(root.path(), &["--instance", "eureka", "brake", "hold"], "alice", 1, |_| { + seen_at_stop = shown(root.path()); + Ok(()) + }); + result.unwrap(); + assert_eq!(calls, ["eureka"]); + assert!(seen_at_stop.contains("BrakeHeld"), "the write comes before the stop: {seen_at_stop}"); + assert!(out.contains("units: stopped mind-self@eureka:* mind-persona@eureka:*"), "{out}"); +} + +#[test] +fn only_brake_hold_calls_the_stopper() { + let root = tempfile::tempdir().unwrap(); + let dial = ["--instance", "eureka", "dial", "set", "--heat", "1", "--base-cooldown-s", "60", "--base-run-usd", "1"]; + let commands: &[&[&str]] = &[ + &["--instance", "eureka", "brake", "release"], + &dial, + &["--instance", "eureka", "show"], + &["--instance", "eureka", "brake"], + &["--instance", "eureka", "brake", "toggle"], + &["--instance", "eureka", "dial", "set", "--heat", "99", "--base-cooldown-s", "60", "--base-run-usd", "1"], + &["--instance", "../evil", "brake", "release"], + &["--instance", "eureka", "brake", "hold", "now"], + &[], + ]; + for args in commands { + let (_, _, calls) = go_stopping(root.path(), args, "op", 0, |_| Ok(())); + assert!(calls.is_empty(), "{args:?} called the stopper"); + } + let (_, _, calls) = go_stopping(root.path(), &["--instance", "eureka", "brake", "hold"], "op", 1, |_| Ok(())); + assert_eq!(calls.len(), 1); +} + +#[test] +fn a_failing_stopper_fails_the_hold_and_leaves_the_brake_held() { + let root = tempfile::tempdir().unwrap(); + go(root.path(), &["--instance", "eureka", "brake", "release"], "op", 0).0.unwrap(); + let (result, out, calls) = go_stopping(root.path(), &["--instance", "eureka", "brake", "hold"], "op", 1, |_| { + Err(anyhow::anyhow!("systemctl stop failed with exit status 5")) + }); + let error = format!("{:#}", result.unwrap_err()); + assert_eq!(calls.len(), 1); + assert!(error.contains("units were not stopped") && error.contains("exit status 5"), "{error}"); + assert!(!out.contains("stopped"), "a failed stop printed success: {out}"); + assert!(shown(root.path()).contains("BrakeHeld")); +} + +#[test] +fn a_failing_write_still_stops_and_the_hold_fails() { + let root = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(root.path().join("eureka").join("control.cc")).unwrap(); + let (result, out, calls) = go_stopping(root.path(), &["--instance", "eureka", "brake", "hold"], "op", 1, |_| Ok(())); + let error = format!("{:#}", result.unwrap_err()); + assert_eq!(calls, ["eureka"], "stopping does not wait on the write"); + assert!(error.contains("brake was not written"), "{error}"); + assert!(!out.contains("stopped"), "{out}"); + + let (result, _, calls) = go_stopping(root.path(), &["--instance", "eureka", "brake", "hold"], "op", 2, |_| Err(anyhow::anyhow!("stop failed"))); + let error = format!("{:#}", result.unwrap_err()); + assert_eq!(calls.len(), 1); + assert!(error.contains("brake was not written") && error.contains("units were not stopped"), "{error}"); +} diff --git a/crates/mind-body/tests/control.rs b/crates/mind-body/tests/control.rs index 6c9c159..73ac41a 100644 --- a/crates/mind-body/tests/control.rs +++ b/crates/mind-body/tests/control.rs @@ -328,9 +328,14 @@ mod unsafe_installs { .unwrap(); let bin = env!("CARGO_BIN_EXE_mind-control"); for round in 0..100 { - let verb = if round % 2 == 0 { "release" } else { "hold" }; + let verb: &[&str] = if round % 2 == 0 { + &["brake", "release"] + } else { + &["dial", "set", "--heat", "1", "--base-cooldown-s", "60", "--base-run-usd", "1"] + }; let status = Command::new("sh") - .args(["-c", "umask 000; exec \"$@\"", "sh", bin, "--root", minds.to_str().unwrap(), "--instance", &instance, "brake", verb]) + .args(["-c", "umask 000; exec \"$@\"", "sh", bin, "--root", minds.to_str().unwrap(), "--instance", &instance]) + .args(verb) .status() .unwrap(); assert!(status.success());