Skip to main content

max / makenotwork

sando: fail the promote on red post-deploy gates, stop rollback rolling forward Two prod-risk defects on the path to production, from the 2026-07-21 audit. Rollback swapped current_version and previous_version, so a second /rollback walked back onto the build the operator was escaping. previous_version now goes NULL: Sando tracks one step of history, and the second call refuses loudly rather than guessing. Post-deploy gate results were discarded into a tracing::warn! and the promote returned 200. The next promote was correctly blocked by the missing evidence, but the operator only found out then, with nothing on /state to explain it. The tier still advances (the nodes genuinely run that version, and a stale current_version would aim a later rollback at the wrong artifact), but it is flagged partial with the failing gate names and the handler returns 409. run_all returns the failing gate kinds instead of a bool so the promote path can name them. Also fixes a moved-value break in the reset_scratch test that had stopped the suite compiling.
Author: Max Johnson <me@maxj.phd> · 2026-07-21 19:03 UTC
Commit: d897b6eddec6ba4ad4231589a263fe6fd60e7045
Parent: 68f44d7
5 files changed, +195 insertions, -27 deletions
M sando/README.md +2 -2
@@ -106,8 +106,8 @@ curl -X POST http://127.0.0.1:7766/promote/a \
106 106 | POST | `/rebuild` | `{sha?: string}` | Force a build; if `sha` is absent, resolves the configured deploy branch. Aborts any in-flight build (latest wins). Returns `{accepted, sha, run_id}`. |
107 107 | GET | `/runs/{id}` | — | Build-status of the run a `/rebuild` returned: `{run_id, sha, version, phase, result, failure_summary, gates[], started_at, finished_at}`. The pollable resource for a non-TUI driver — `/state` only reflects the last *successful* version. |
108 108 | GET | `/runs/{id}/wait` | `?timeout_ms=` | Long-poll: blocks until the run settles or `timeout_ms` (default 30s, cap 120s) elapses, then returns the same `RunView`. Fire `/rebuild` → block on `/wait`. |
109 - | POST | `/promote/{tier}` | `{version?, hotfix?, reset_burn_in?}` | Verify predecessor gates, deploy to tier nodes, advance state. `version` defaults to the predecessor tier's `current_version`. |
110 - | POST | `/rollback/{tier}` | — | Swap `current` symlink to `previous_version` on every node in the tier |
109 + | POST | `/promote/{tier}` | `{version?, hotfix?, reset_burn_in?}` | Verify predecessor gates, deploy to tier nodes, advance state. `version` defaults to the predecessor tier's `current_version`. Red post-deploy gates advance the tier (the nodes really are running it) but return 409 and flag the tier `partial` — the rollout landed, the tier cannot promote onward. |
110 + | POST | `/rollback/{tier}` | — | Swap `current` symlink to `previous_version` on every node in the tier. One step only: `previous_version` is cleared afterwards, so a second `/rollback` returns 409 rather than rolling forward onto the version you just escaped. |
111 111 | POST | `/confirm/{tier}` | — | Insert a passing `manual_confirm` gate row for the tier's `current_version`. Replaces hand-SQL. |
112 112 | POST | `/backup/fetch` | — | Pull the prod backup. Supports `file://`, `rsync://`, `ssh://user@host[:port]/path`. |
113 113 | GET | `/metrics` | — | Prometheus exposition |
M sando/daemon/src/build.rs +2 -2
@@ -319,9 +319,9 @@ pub async fn build_and_run_host(
319 319 // nodes to probe.
320 320 nodes: Vec::new(),
321 321 };
322 - let ok = gates::run_all(&ctx, &host.gates).await?;
322 + let failed = gates::run_all(&ctx, &host.gates).await?;
323 323
324 - if ok {
324 + if failed.is_empty() {
325 325 // Advance the host tier through the single sealed forward-advance op, under
326 326 // deploy_lock so this can't interleave with a concurrent `/rollback host`
327 327 // (the old fetch-then-write here was the one CF3 site outside the lock —
M sando/daemon/src/gates.rs +14 -10
@@ -149,20 +149,24 @@ pub async fn run(ctx: &GateCtx, gate: &Gate) -> Result<GateOutcome> {
149 149 Ok(outcome)
150 150 }
151 151
152 - /// Run a sequence of gates; stops on the first failure (no point running the
153 - /// Run every gate in order and return true iff all passed. We deliberately do
154 - /// NOT short-circuit on first failure — every gate's outcome is recorded in
155 - /// `gate_runs`, which is the operator's only visibility into pipeline health.
156 - /// Hiding later gates because an earlier one failed makes diagnosis worse.
157 - pub async fn run_all(ctx: &GateCtx, gates: &[Gate]) -> Result<bool> {
158 - let mut all_ok = true;
152 + /// Run every gate in order and return the kinds that did not pass (empty means
153 + /// green). We deliberately do NOT short-circuit on first failure — every gate's
154 + /// outcome is recorded in `gate_runs`, which is the operator's only visibility
155 + /// into pipeline health. Hiding later gates because an earlier one failed makes
156 + /// diagnosis worse.
157 + ///
158 + /// Returning the failing kinds rather than a bare bool is what lets the promote
159 + /// path name them in the tier's `partial_reason` and in the error it returns to
160 + /// the operator, instead of a generic "something was red".
161 + pub async fn run_all(ctx: &GateCtx, gates: &[Gate]) -> Result<Vec<GateKind>> {
162 + let mut failed = Vec::new();
159 163 for g in gates {
160 164 let o = run(ctx, g).await?;
161 165 if !o.is_passed() {
162 - all_ok = false;
166 + failed.push(g.kind());
163 167 }
164 168 }
165 - Ok(all_ok)
169 + Ok(failed)
166 170 }
167 171
168 172 // ---- individual gate runners ----
@@ -1523,7 +1527,7 @@ mod tests {
1523 1527 );
1524 1528
1525 1529 let pool = PgPoolOptions::new().max_connections(1).connect(&url).await.unwrap();
1526 - pool.execute(sqlx::raw_sql(sqlx::AssertSqlSafe(drop_role))).await.unwrap();
1530 + pool.execute(sqlx::raw_sql(sqlx::AssertSqlSafe(drop_role.clone()))).await.unwrap();
1527 1531 pool.close().await;
1528 1532
1529 1533 reset_scratch(&url, role).await.expect("reset_scratch creates the owner role");
M sando/daemon/src/routes/mod.rs +135 -2
@@ -322,12 +322,17 @@ async fn rollback(
322 322 }
323 323 }
324 324
325 + // `previous_version` becomes NULL, not `current`. Sando tracks exactly one
326 + // step of history, so writing the version we just rolled *off* into the
327 + // rollback slot makes a second /rollback roll FORWARD onto the build the
328 + // operator was escaping. NULL makes the second call fail loudly with "no
329 + // previous_version to roll back to" — honest about the depth we keep.
330 + // Stepping back further is the deploys-table walk we deliberately don't do.
325 331 sqlx::query(
326 - "UPDATE tier_state SET current_version = ?, previous_version = ?, burn_in_started_at = NULL
332 + "UPDATE tier_state SET current_version = ?, previous_version = NULL, burn_in_started_at = NULL
327 333 WHERE tier = ?",
328 334 )
329 335 .bind(&previous)
330 - .bind(&current)
331 336 .bind(&tier)
332 337 .execute(&s.pool).await.map_err(crate::error::Error::Db)?;
333 338
@@ -1430,6 +1435,134 @@ mod tests {
1430 1435 assert_eq!(restored, 0, "no artifact to roll back to -> nothing restored, no panic");
1431 1436 }
1432 1437
1438 + /// Build a one-node tier "a" on a tempdir release root, pre-seeded as if a
1439 + /// promote had flipped it to `current` with `prev` still staged on disk.
1440 + /// Returns the state (topology rewired to the tempdir node) and the tempdir,
1441 + /// which the caller must keep alive.
1442 + async fn rollback_fixture(prev: &str, current: &str) -> (AppState, tempfile::TempDir) {
1443 + use crate::topology::{default_actuate, default_observe, Node};
1444 + let tmp = tempfile::tempdir().unwrap();
1445 + let rr = tmp.path().join("a-local");
1446 + for v in [prev, current] {
1447 + tokio::fs::create_dir_all(rr.join("releases").join(v)).await.unwrap();
1448 + }
1449 + tokio::fs::symlink(format!("releases/{current}"), rr.join("current")).await.unwrap();
1450 + let node = Node {
1451 + name: "a-local".into(),
1452 + ssh_target: "local".into(),
1453 + release_root: rr.to_string_lossy().into_owned(),
1454 + service_name: "x.service".into(),
1455 + health_url: None,
1456 + config_check_env_file: None,
1457 + actuate: default_actuate(),
1458 + observe: default_observe(),
1459 + companions: Vec::new(),
1460 + };
1461 +
1462 + let mut state = test_state().await;
1463 + let mut topo = (*state.topo).clone();
1464 + topo.tiers[1].nodes = vec![node];
1465 + state.executors = Arc::new(crate::state::build_executors(&topo));
1466 + state.topo = Arc::new(topo);
1467 +
1468 + // deploys.node FKs into `nodes`, so the promote path needs the row.
1469 + sqlx::query("INSERT INTO nodes (name, tier, ssh_target, release_root) VALUES ('a-local', 'a', 'local', ?)")
1470 + .bind(rr.to_string_lossy().into_owned())
1471 + .execute(&state.pool).await.unwrap();
1472 + for v in [prev, current] {
1473 + sqlx::query("INSERT INTO versions (version, git_sha, built_at, artifact_path) VALUES (?, 'sha', datetime('now'), '/tmp/staged/makenotwork')")
1474 + .bind(v).execute(&state.pool).await.unwrap();
1475 + }
1476 + sqlx::query("UPDATE tier_state SET current_version = ?, previous_version = ? WHERE tier = 'a'")
1477 + .bind(current).bind(prev)
1478 + .execute(&state.pool).await.unwrap();
1479 + (state, tmp)
1480 + }
1481 +
1482 + async fn tier_versions(pool: &SqlitePool, tier: &str) -> (Option<String>, Option<String>) {
1483 + sqlx::query_as("SELECT current_version, previous_version FROM tier_state WHERE tier = ?")
1484 + .bind(tier)
1485 + .fetch_one(pool).await.unwrap()
1486 + }
1487 +
1488 + #[tokio::test]
1489 + async fn rollback_clears_previous_version_rather_than_swapping_it() {
1490 + // The swap bug: writing the version we just rolled OFF into
1491 + // previous_version made a second /rollback roll FORWARD onto the broken
1492 + // build the operator was escaping. previous_version must go NULL.
1493 + let (state, _tmp) = rollback_fixture("1.0.0", "2.0.0").await;
1494 + let pool = state.pool.clone();
1495 +
1496 + let _ = rollback(State(state), Path("a".to_string())).await.unwrap();
1497 +
1498 + let (cur, prev) = tier_versions(&pool, "a").await;
1499 + assert_eq!(cur.as_deref(), Some("1.0.0"), "rolled back to the previous version");
1500 + assert_eq!(prev, None, "the version we rolled off must NOT become the rollback target");
1501 + }
1502 +
1503 + #[tokio::test]
1504 + async fn second_rollback_refuses_instead_of_rolling_forward() {
1505 + let (state, _tmp) = rollback_fixture("1.0.0", "2.0.0").await;
1506 + let pool = state.pool.clone();
1507 + let release_root = state.topo.tiers[1].nodes[0].release_root.clone();
1508 +
1509 + let _ = rollback(State(state.clone()), Path("a".to_string())).await.unwrap();
1510 + let err = rollback(State(state), Path("a".to_string())).await
1511 + .expect_err("only one step of history is tracked; a second rollback must refuse");
1512 + assert!(
1513 + matches!(&err, crate::error::Error::GateBlocked(m) if m.contains("no previous_version")),
1514 + "expected a loud refusal, got: {err:?}",
1515 + );
1516 +
1517 + // The refusal is total: neither the DB nor the node moved back to 2.0.0.
1518 + let (cur, _) = tier_versions(&pool, "a").await;
1519 + assert_eq!(cur.as_deref(), Some("1.0.0"));
1520 + let link = tokio::fs::read_link(std::path::Path::new(&release_root).join("current"))
1521 + .await.unwrap();
1522 + assert_eq!(link.to_string_lossy(), "releases/1.0.0", "node stays on the rolled-back version");
1523 + }
1524 +
1525 + #[tokio::test]
1526 + async fn promote_fails_and_flags_the_tier_when_post_deploy_gates_are_red() {
1527 + // The deploy reached every node, so tier_state advances (a stale
1528 + // current_version would aim a later rollback at the wrong artifact), but
1529 + // the promote must NOT report success: the tier is flagged partial and
1530 + // the handler returns the gate failure.
1531 + use crate::topology::Gate;
1532 + let (mut state, _tmp) = rollback_fixture("1.0.0", "2.0.0").await;
1533 + // node_health is the tier's only gate, and it fails closed with no
1534 + // probes — an empty executor map gives it nothing to probe while
1535 + // deploy_node still falls back to a built executor and succeeds.
1536 + let mut topo = (*state.topo).clone();
1537 + topo.tiers[1].gates = vec![Gate::NodeHealth];
1538 + state.topo = Arc::new(topo);
1539 + state.executors = Arc::new(crate::state::ExecutorMap::new());
1540 + let pool = state.pool.clone();
1541 + // Promote 3.0.0 up from host, which configures no gates of its own.
1542 + sqlx::query("INSERT INTO versions (version, git_sha, built_at, artifact_path) VALUES ('3.0.0','sha',datetime('now'),'/tmp/staged/makenotwork')")
1543 + .execute(&pool).await.unwrap();
1544 + sqlx::query("UPDATE tier_state SET current_version = '3.0.0' WHERE tier = 'host'")
1545 + .execute(&pool).await.unwrap();
1546 +
1547 + let err = promote_inner(state, "a".into(), PromoteBody::default()).await
1548 + .expect_err("red post-deploy gates must fail the promote");
1549 + assert!(
1550 + matches!(&err, crate::error::Error::GateBlocked(m) if m.contains("node_health")),
1551 + "the failure must name the red gate, got: {err:?}",
1552 + );
1553 +
1554 + let (cur, _) = tier_versions(&pool, "a").await;
1555 + assert_eq!(cur.as_deref(), Some("3.0.0"), "tier_state tracks what the nodes actually run");
1556 + let reason: Option<String> = sqlx::query_scalar(
1557 + "SELECT partial_reason FROM tier_state WHERE tier = 'a'",
1558 + )
1559 + .fetch_one(&pool).await.unwrap();
1560 + assert!(
1561 + reason.as_deref().is_some_and(|r| r.contains("node_health")),
1562 + "the tier must be flagged for /state and the TUI, got: {reason:?}",
1563 + );
1564 + }
1565 +
1433 1566 #[tokio::test]
1434 1567 async fn set_partial_then_clear_roundtrips() {
1435 1568 let state = test_state().await;
M sando/daemon/src/routes/promotion.rs +42 -11
@@ -194,8 +194,16 @@ pub(super) async fn promote_inner(
194 194 // live / by the operator at the next promote. A failed gate does not
195 195 // unwind this deploy (the artifact is already live on the tier); it
196 196 // blocks the next promote, which is the fail-closed behavior we want.
197 + //
198 + // It does, however, fail *this* promote's response. tier_state still
199 + // advances below — the nodes genuinely run this version and a stale
200 + // `current_version` would send a later rollback to the wrong artifact —
201 + // but the tier is flagged partial and the handler returns the failure,
202 + // so the operator learns at the promote instead of discovering it as an
203 + // unexplained block on the next one.
197 204 let post_deploy: Vec<crate::topology::Gate> =
198 205 target.gates.iter().filter(|g| g.runs_post_deploy()).cloned().collect();
206 + let mut post_deploy_failure: Option<String> = None;
199 207 if !post_deploy.is_empty() {
200 208 // node_health probes each node the deploy just shipped to, over the same
201 209 // executor the deploy used. Build the probe set from the tier's nodes and
@@ -225,17 +233,30 @@ pub(super) async fn promote_inner(
225 233 events: s.events.clone(),
226 234 nodes,
227 235 };
228 - match crate::gates::run_all(&ctx, &post_deploy).await {
229 - Ok(true) => {}
230 - Ok(false) => tracing::warn!(
231 - tier = %target.name, version = %version,
232 - "post-deploy gate(s) failed; tier advanced but promotion to the next tier will be blocked",
233 - ),
234 - Err(e) => tracing::error!(
235 - tier = %target.name, version = %version, error = %e,
236 - "post-deploy gate execution errored; promotion to the next tier will be blocked",
237 - ),
238 - }
236 + post_deploy_failure = match crate::gates::run_all(&ctx, &post_deploy).await {
237 + Ok(failed) if failed.is_empty() => None,
238 + Ok(failed) => {
239 + let names = failed.iter().map(|k| k.as_str()).collect::<Vec<_>>().join(", ");
240 + tracing::warn!(
241 + tier = %target.name, version = %version, gates = %names,
242 + "post-deploy gate(s) failed; tier advanced but promotion to the next tier is blocked",
243 + );
244 + Some(format!(
245 + "post-deploy gate(s) failed on {version}: {names}; \
246 + the tier is serving {version} but cannot promote onward until they pass"
247 + ))
248 + }
249 + Err(e) => {
250 + tracing::error!(
251 + tier = %target.name, version = %version, error = %e,
252 + "post-deploy gate execution errored; promotion to the next tier is blocked",
253 + );
254 + Some(format!(
255 + "post-deploy gate execution errored on {version}: {e}; \
256 + the tier is serving {version} but cannot promote onward until the gates pass"
257 + ))
258 + }
259 + };
239 260 }
240 261
241 262 // 4. Advance tier_state through the single sealed forward-advance op (atomic
@@ -254,6 +275,16 @@ pub(super) async fn promote_inner(
254 275 .execute(&s.pool).await.map_err(crate::error::Error::Db)?;
255 276 }
256 277
278 + // Red post-deploy gates: the rollout itself reached every node, but the tier
279 + // is not fit to promote onward. Flag it so /state and the TUI say so, and
280 + // return the failure rather than a 200 the operator would read as "shipped,
281 + // all good". tier_state has already advanced above — it describes what the
282 + // nodes are running, not whether we're happy about it.
283 + if let Some(reason) = post_deploy_failure {
284 + set_partial(&s, &target.name, &reason).await;
285 + return Err(crate::error::Error::GateBlocked(reason));
286 + }
287 +
257 288 // A clean full rollout to every node clears any prior partial flag on this tier.
258 289 clear_partial(&s, &target.name).await;
259 290