Fix stdout/stderr ordering in ShutdownPrepCheck
Build with clawstor cache / Cargo build (clawstor-cached) (pull_request) Failing after 3s
Build with clawstor cache / Cargo build (clawstor-cached) (pull_request) Failing after 3s
Command::output() captures stdout and stderr as two separate buffers. check() was concatenating stdout-then-stderr, which throws away chronological order entirely -- every stderr line (e.g. "Error: send_to_host not set" from `claw-store replicate` on a node with no downstream replication target, which is normal and expected on architect) landed at the very end of the report regardless of when it actually printed, making a mid-script, already-handled condition look like a failure that happened after "DRY RUN COMPLETE". Fix: invoke via `bash -c "script --dry-run 2>&1"` so stderr merges into stdout inside the shell, before either stream reaches us -- true chronological order preserved, single buffer to read. Verified on architect: the send_to_host message now appears exactly where it happens, inside the "taking final snapshot + replicating" step, with DRY RUN COMPLETE correctly last. Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
@@ -87,16 +87,24 @@ pub async fn check() -> Result<ShutdownPrepCheckReply> {
|
||||
if !script.exists() {
|
||||
bail!("shutdown-prep script not found at {}", script.display());
|
||||
}
|
||||
// `2>&1` inside the shell merges stderr into stdout *before*
|
||||
// either stream is piped back to us, preserving true
|
||||
// chronological order. Capturing stdout/stderr separately (as
|
||||
// `Command::output()` does by default) and concatenating them
|
||||
// after the fact loses interleaving entirely — every stderr line
|
||||
// lands at the very end regardless of when it was actually
|
||||
// printed, which makes a mid-script warning (e.g. "replicate not
|
||||
// configured on this node") look like a failure that happened
|
||||
// after "DRY RUN COMPLETE".
|
||||
let run = Command::new("bash")
|
||||
.arg(&script)
|
||||
.arg("--dry-run")
|
||||
.arg("-c")
|
||||
.arg(format!("{} --dry-run 2>&1", script.display()))
|
||||
.output();
|
||||
let output = timeout(CHECK_TIMEOUT, run)
|
||||
.await
|
||||
.context("shutdown-prep --dry-run timed out")?
|
||||
.context("spawning shutdown-prep --dry-run")?;
|
||||
let mut combined = String::from_utf8_lossy(&output.stdout).into_owned();
|
||||
combined.push_str(&String::from_utf8_lossy(&output.stderr));
|
||||
let combined = String::from_utf8_lossy(&output.stdout).into_owned();
|
||||
Ok(ShutdownPrepCheckReply {
|
||||
ready: output.status.success(),
|
||||
output: combined,
|
||||
|
||||
Reference in New Issue
Block a user