From f56b5c5a7b4acc1c8dd692f2c6a3b1a2ae9d501e Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 1 Jun 2026 19:28:29 +0200 Subject: [PATCH] fix(#991): start nginx when unit is in failed state, not just reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit nginx -s reload signals a running master process. When nginx enters failed state (start-limit-hit from repeated nginx -t failures on a bad agents.conf), there is no master and the reload is a silent no-op. c0re kept re-firing the same no-op reload forever via RELOAD_PENDING. Fix: probe the nginx unit's ActiveState before sending the reload: - active → nginx -s reload (existing zero-downtime path) - failed → systemctl reset-failed nginx + systemctl start nginx - other → systemctl start nginx This makes c0re self-healing: once a corrected agents.conf is published, the next reload_gateway_nginx call clears the start-limit and restarts nginx automatically without operator intervention. New helpers: nginx_active_state() (systemctl show --property=ActiveState --value) and gateway_systemctl() (host-side systemctl --machine=hive-gateway). --- hive-c0re/src/gateway_nginx.rs | 165 +++++++++++++++++++++++++-------- 1 file changed, 126 insertions(+), 39 deletions(-) diff --git a/hive-c0re/src/gateway_nginx.rs b/hive-c0re/src/gateway_nginx.rs index e00d57b7..223d399f 100644 --- a/hive-c0re/src/gateway_nginx.rs +++ b/hive-c0re/src/gateway_nginx.rs @@ -193,8 +193,7 @@ pub fn write(names: &[String]) -> Result<()> { .with_context(|| format!("create dir {}", parent.display()))?; } let tmp = path.with_extension("conf.tmp"); - std::fs::write(&tmp, &body) - .with_context(|| format!("write tmp {}", tmp.display()))?; + std::fs::write(&tmp, &body).with_context(|| format!("write tmp {}", tmp.display()))?; std::fs::rename(&tmp, &path).with_context(|| { format!( "rename {} -> {} (atomic publish)", @@ -219,52 +218,134 @@ pub fn reload_if_pending() { } } -/// Send `nginx -s reload` to the gateway container via systemd-run. -/// Uses `--wait` so the exit code reflects whether nginx received the -/// signal; clears `RELOAD_PENDING` on success so `reload_if_pending` -/// stops retrying. Best-effort: errors are logged, not bubbled. -fn reload_gateway_nginx() { - // `--machine=hive-gateway` targets the container by its nspawn - // machine name (same as the nixos-container name). `--quiet` - // suppresses the transient unit name echo. `--wait` blocks until - // the transient job exits so the exit code tells us whether - // `nginx -s reload` ran at all (RELOAD_PENDING is only cleared on - // success — a failed attempt is retried next tick). `--` - // separates systemd-run args from the command. - let status = std::process::Command::new("systemd-run") +/// Query the nginx unit's `ActiveState` inside the gateway container. +/// Returns the raw state string from `systemctl show --property=ActiveState +/// --value` (e.g. `"active"`, `"failed"`, `"inactive"`, `"activating"`). +/// Returns `"unknown"` on any error so callers can branch safely. +fn nginx_active_state() -> String { + let out = std::process::Command::new("systemctl") .args([ "--machine=hive-gateway", - "--quiet", - "--wait", - "--", + "show", + "--property=ActiveState", + "--value", "nginx", - "-s", - "reload", ]) - .status(); - match status { - Ok(s) if s.success() => { - // nginx -s reload ran successfully (SIGHUP sent to master). - // The actual worker replacement is async but the signal was - // delivered; clear the pending flag. - RELOAD_PENDING.store(false, Ordering::Relaxed); - tracing::debug!("gateway nginx reload signal sent"); - } - Ok(s) => { + .output(); + match out { + Ok(o) if o.status.success() => String::from_utf8_lossy(&o.stdout).trim().to_owned(), + Ok(o) => { tracing::warn!( - exit_code = ?s.code(), - "gateway nginx reload exited non-zero — will retry next poll tick" + exit_code = ?o.status.code(), + "systemctl show ActiveState exited non-zero" ); + "unknown".to_owned() } Err(e) => { - tracing::warn!( - error = %e, - "failed to invoke systemd-run for gateway nginx reload — will retry next poll tick" - ); + tracing::warn!(error = %e, "systemctl show ActiveState failed"); + "unknown".to_owned() } } } +/// Send a `systemctl --machine=hive-gateway ` command and +/// return whether it succeeded. Best-effort: errors are logged. +fn gateway_systemctl(args: &[&str]) -> bool { + let mut cmd = std::process::Command::new("systemctl"); + cmd.arg("--machine=hive-gateway"); + cmd.args(args); + match cmd.status() { + Ok(s) if s.success() => true, + Ok(s) => { + tracing::warn!( + args = ?args, + exit_code = ?s.code(), + "gateway systemctl exited non-zero" + ); + false + } + Err(e) => { + tracing::warn!(args = ?args, error = %e, "gateway systemctl invocation failed"); + false + } + } +} + +/// Synchronise the gateway nginx unit with the current agents.conf: +/// +/// - **active**: send `nginx -s reload` (SIGHUP to master, zero-downtime +/// worker replacement). Keeps RELOAD_PENDING set on failure so the +/// next poll tick retries. +/// - **failed / start-limit-hit**: run `systemctl reset-failed nginx` +/// then `systemctl start nginx`. This is the self-healing path: a +/// transient bad agents.conf that causes five instant `nginx -t` +/// failures trips systemd's start-limit. Once c0re publishes a correct +/// config, the next reload attempt clears the failure and restarts. +/// - **inactive / other**: run `systemctl start nginx` directly (no +/// reset-failed needed when the unit isn't in a failed state). +/// +/// `RELOAD_PENDING` is cleared only after a successful operation so +/// `reload_if_pending` keeps retrying on failure. +fn reload_gateway_nginx() { + let state = nginx_active_state(); + let success = match state.as_str() { + "active" => { + // nginx master is running — SIGHUP is the zero-downtime path. + // `systemd-run --machine=hive-gateway --quiet --wait -- nginx + // -s reload` runs the signal inside the container and exits + // with the nginx exit code. `--` separates systemd-run flags + // from the command. + let status = std::process::Command::new("systemd-run") + .args([ + "--machine=hive-gateway", + "--quiet", + "--wait", + "--", + "nginx", + "-s", + "reload", + ]) + .status(); + match status { + Ok(s) if s.success() => { + tracing::debug!("gateway nginx reload signal sent"); + true + } + Ok(s) => { + tracing::warn!( + exit_code = ?s.code(), + "gateway nginx reload exited non-zero — will retry next poll tick" + ); + false + } + Err(e) => { + tracing::warn!( + error = %e, + "failed to invoke systemd-run for gateway nginx reload — will retry" + ); + false + } + } + } + "failed" => { + // Unit hit start-limit (e.g. repeated nginx -t failures from + // a bad agents.conf). reset-failed clears the rate-limit so + // start can proceed. + tracing::info!("gateway nginx unit in failed state — resetting and starting"); + gateway_systemctl(&["reset-failed", "nginx"]) && gateway_systemctl(&["start", "nginx"]) + } + other => { + // inactive, deactivating, activating, unknown — just try start. + tracing::info!(state = other, "gateway nginx unit not active — starting"); + gateway_systemctl(&["start", "nginx"]) + } + }; + + if success { + RELOAD_PENDING.store(false, Ordering::Relaxed); + } +} + #[cfg(test)] mod tests { use super::*; @@ -341,7 +422,8 @@ mod tests { // ── split mode (frontend_dir = Some) ─────────────────────────────── - const FAKE_FRONTEND: &str = "/nix/store/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-frontend/agent"; + const FAKE_FRONTEND: &str = + "/nix/store/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-frontend/agent"; #[test] fn split_mode_has_immutable_cache_for_static_assets() { @@ -406,7 +488,9 @@ mod tests { // The named-location proxy_pass must have no URI part (no // trailing slash or path). Extract the `@iris_dynamic { ... }` // block and check every proxy_pass directive in it. - let block_start = body.find("location @iris_dynamic {").expect("named location"); + let block_start = body + .find("location @iris_dynamic {") + .expect("named location"); let block = &body[block_start..]; let block_end = block.find("\n}\n").expect("block close"); let block = &block[..block_end]; @@ -456,7 +540,10 @@ mod tests { // Each agent gets exactly one named dynamic location block. let damocles_count = body.matches("@damocles_dynamic {").count(); let iris_count = body.matches("@iris_dynamic {").count(); - assert_eq!(damocles_count, 1, "expected exactly one damocles_dynamic block"); + assert_eq!( + damocles_count, 1, + "expected exactly one damocles_dynamic block" + ); assert_eq!(iris_count, 1, "expected exactly one iris_dynamic block"); } }