diff options
| author | tv <tv@krebsco.de> | 2026-11-13 20:36:10 +0000 |
|---|---|---|
| committer | tv <tv@krebsco.de> | 2026-11-13 22:01:46 +0100 |
| commit | ce2cd2553a937d84f1a78386ab28502e89f247d6 (patch) | |
| tree | fd598a289771894a6eb82157d192a5fa4f784b3f | |
| parent | 7768c71fd8083ea21572de2859fed39c17524baf (diff) | |
populate: exit with the status of the command that failed
The shell implementation runs under set -e, so it exits with the status
of the command that failed, e.g. 255 if ssh can't reach the target,
which wrappers retrying on connection failures rely on, or 128 if git
fetch fails. The program does the same, instead of exiting with 1, and
with 128 plus the signal for a command a signal ended.
A phase's expect.status is the exit status it expects populate to fail
with; git-status checks git fetch's.
| -rw-r--r-- | pkgs/populate/docs/design.md | 6 | ||||
| -rw-r--r-- | pkgs/populate/src/main.rs | 2 | ||||
| -rw-r--r-- | pkgs/populate/src/run.rs | 26 | ||||
| -rw-r--r-- | tests/scenarios.nix | 13 | ||||
| -rw-r--r-- | tests/vm.nix | 4 |
5 files changed, 42 insertions, 9 deletions
diff --git a/pkgs/populate/docs/design.md b/pkgs/populate/docs/design.md index 6ad5eea..cc78a65 100644 --- a/pkgs/populate/docs/design.md +++ b/pkgs/populate/docs/design.md @@ -37,7 +37,11 @@ populate SPEC input. Exit status: - 0: populate delivered all sources. -- 1: a check of the spec failed, or delivering did. +- 1: a check of the spec failed, or delivering did, with a message of its + own. +- The exit status of the command that failed, if delivering failed with + that, like the shell implementation does: e.g. 255 if ssh can't reach + the target, or 128 if git fetch fails. - 2: the arguments are wrong, or the spec can't be read, isn't JSON, is of another version, or doesn't have the spec's shape. - 129, 130, 143: populate got SIGHUP, SIGINT, or SIGTERM, and stopped. diff --git a/pkgs/populate/src/main.rs b/pkgs/populate/src/main.rs index 17d9678..2884fd0 100644 --- a/pkgs/populate/src/main.rs +++ b/pkgs/populate/src/main.rs @@ -78,7 +78,7 @@ fn exit(failure: run::Failure) -> ExitCode { eprintln!("{message}"); ExitCode::FAILURE } - run::Failure::Command => ExitCode::FAILURE, + run::Failure::Command(status) => ExitCode::from(status), run::Failure::Signal(signal) => ExitCode::from(128 + signal as u8), } } diff --git a/pkgs/populate/src/run.rs b/pkgs/populate/src/run.rs index 528977c..4ed1d76 100644 --- a/pkgs/populate/src/run.rs +++ b/pkgs/populate/src/run.rs @@ -15,8 +15,10 @@ use std::sync::atomic::{AtomicI32, Ordering}; pub enum Failure { /// A message to print. Message(String), - /// A command failed, after printing why. - Command, + /// A command failed, after printing why, with its exit status, which + /// populate exits with: the shell implementation, running under set -e, + /// does, so e.g. 255 from ssh still tells a connection failure. + Command(u8), /// A signal arrived: SIGHUP, SIGINT, or SIGTERM. Signal(i32), } @@ -25,7 +27,7 @@ impl fmt::Display for Failure { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { Failure::Message(message) => f.write_str(message), - Failure::Command => f.write_str("a command failed"), + Failure::Command(status) => write!(f, "a command failed with status {status}"), Failure::Signal(signal) => write!(f, "got signal {signal}"), } } @@ -117,7 +119,7 @@ impl Context { .output() .map_err(|e| Failure::Message(format!("error: can't run mktemp: {e}")))?; if !output.status.success() { - return Err(Failure::Command); + return Err(failed(output.status)); } let tmpdir = PathBuf::from(String::from_utf8_lossy(&output.stdout).trim_end()); // Shared with the commands populate runs. @@ -319,13 +321,25 @@ fn spawn_error(command: &Command, e: std::io::Error) -> Failure { )) } +/// The failure of a command that exited with status, or, like in a shell, +/// with 128 plus the signal that ended it. +fn failed(status: ExitStatus) -> Failure { + use std::os::unix::process::ExitStatusExt; + let code = match (status.code(), status.signal()) { + (Some(code), _) => code, + (None, Some(signal)) => 128 + signal, + (None, None) => 1, + }; + Failure::Command(u8::try_from(code).ok().filter(|c| *c != 0).unwrap_or(1)) +} + /// Runs a command, failing if it fails, or if a signal arrived meanwhile. pub fn status(command: &mut Command) -> Result<()> { let result: std::io::Result<ExitStatus> = command.status(); check_signal()?; match result { Ok(status) if status.success() => Ok(()), - Ok(_) => Err(Failure::Command), + Ok(status) => Err(failed(status)), Err(e) => Err(spawn_error(command, e)), } } @@ -339,7 +353,7 @@ pub fn output(command: &mut Command) -> Result<String> { Ok(output) if output.status.success() => { Ok(String::from_utf8_lossy(&output.stdout).into_owned()) } - Ok(_) => Err(Failure::Command), + Ok(output) => Err(failed(output.status)), Err(e) => Err(spawn_error(command, e)), } } diff --git a/tests/scenarios.nix b/tests/scenarios.nix index 9f1e037..ca2fd20 100644 --- a/tests/scenarios.nix +++ b/tests/scenarios.nix @@ -109,7 +109,9 @@ let # evaluation fail. env sets environment variables for populate, as # arguments of env. interrupt ends populate after that many seconds, # which it's expected not to finish before. output lists what a - # succeeding populate is expected to print. + # succeeding populate is expected to print. expect.fail is what a + # failing populate is expected to print, and expect.status, if given, + # its exit status. phase = { sources, backup ? false, prune ? false, deploy ? null, targetPath ? path, env ? "", interrupt ? null, output ? [], @@ -149,6 +151,7 @@ let populate = toString (command sources targetPath); inherit env interrupt output; expect = if expect == "success" then null else expect.fail; + status = if expect == "success" then null else expect.status or null; # No phase gets around checking the canaries. check = script (check + "\ncanary_check\n"); mutate = script mutate; @@ -2252,6 +2255,14 @@ assert !(krops.isLocalTarget (targets.remote "/")); } ]; })) + # populate exits with the status of the command that failed, here git + # fetch's. + (s "git-status" { + phases = [{ + sources.x.git = { url = gitUrl; ref = "nonexistent"; }; + expect = { fail = "couldn't find remote ref"; status = 128; }; + }]; + }) # A .git that a materialized flake got, and one that a file source # keeps, which Nix uses for the git+file: URL of the flake there: writing # its lock file, Nix runs Git, with that repository's configuration. diff --git a/tests/vm.nix b/tests/vm.nix index d7b2f60..25a12a4 100644 --- a/tests/vm.nix +++ b/tests/vm.nix @@ -173,6 +173,10 @@ in pkgs.testers.runNixOSTest { raise Exception( f"populate {i} doesn't fail with {phase['expect']!r}:\n{out}" ) + elif phase["status"] is not None and status != phase["status"]: + raise Exception( + f"populate {i} exits with {status} instead of {phase['status']}:\n{out}" + ) run(node, f"KROPS_TEST_GC=1 sh {phase['check']}") if phase["checkDeployer"] is not None: run(deployer, f"sh {phase['checkDeployer']}") |
