refactor(#2916): destroy submits a DAG instead of an imperative teardown
Destroy was a straight-line async fn with no queue node behind it, so
nothing in the graph could answer "is this container going down on
purpose?". That gap is why an imperative crash-watch suppression guard
existed: an RAII handle held for the operation's duration, a second way
to say what every other lifecycle op already says through its node.
Reuse the existing Stop node rather than teaching a new node to stop
things:
Stop -> DestroyContainer -> (PurgeState) -> DestroyBookkeeping
Stop already declares takes_container_down honestly, so the suppression
is now derived from the graph like every other op's. It also turns the
precondition into an edge: DestroyContainer runs only under a completed
Stop, so it operates on an already-stopped container and carries
takes_container_down = false permanently. A container still alive at
that point is a real bug and stays loud instead of being absorbed by a
flag -- which matters because a wrong true silently swallows a crash
while a wrong false only costs a spurious event.
Removes suppress_crash_watch, CrashWatchSuppression, crash_suppressed,
crash_watch_suppressed and NO_NODE_LABEL. The migration call sites went
with the obsolete startup migrations, so destroy was the last caller and
intent now has exactly one home.
destroy() becomes a submit-and-return, matching every sibling endpoint
(rebuild, kill, restart, start, pause, resume) -- it was the only
lifecycle op that awaited its work. The container rescan moves into the
bookkeeping tail, so ContainerRemoved now arrives after the 200 rather
than before it.
Also drops an orphaned doc-comment in coordinator.rs: two stacked blocks
where only the second described crash_suppressed, the first documenting
a field that no longer exists. Removing the field would have re-pointed
it at recent_transient.
This commit is contained in:
parent
2c7872841a
commit
6338939657
9 changed files with 326 additions and 194 deletions
|
|
@ -1714,6 +1714,97 @@ fn spawn_shape_provision_create_dropin_reconcile() {
|
|||
);
|
||||
}
|
||||
|
||||
/// The destroy chain, and the reason it is a chain: `Stop` is reused so the
|
||||
/// crash-watch answer comes from the node that actually stops the container.
|
||||
///
|
||||
/// Asserting the *edges* is the point. `destroy_container` runs `after_ok` a
|
||||
/// `stop`, which is what makes "the container is already down here" a
|
||||
/// structural fact rather than a convention — see the companion test below for
|
||||
/// why that matters.
|
||||
#[test]
|
||||
fn destroy_shape_stop_then_destroy_then_bookkeeping() {
|
||||
let q = JobQueue::new(1);
|
||||
insert(&q, |builder| {
|
||||
templates::destroy(builder, "doomed", false);
|
||||
});
|
||||
assert_eq!(
|
||||
declared_shape(&q),
|
||||
vec![
|
||||
row("stop", None, &[]),
|
||||
// No explicit edge to `stop`: `part_of` already gates the child on
|
||||
// its parent reaching `Finishing`, and declaring a dep on your own
|
||||
// parent is rejected outright (it would deadlock). The precondition
|
||||
// is the group membership.
|
||||
row("destroy_container", Some("stop"), &[]),
|
||||
row(
|
||||
"destroy_bookkeeping",
|
||||
Some("stop"),
|
||||
&[("destroy_container", "done")]
|
||||
),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
/// `purge` inserts the irreversible delete as its own node, between the destroy
|
||||
/// and the bookkeeping tail — so the meta sync that stops referencing the agent
|
||||
/// runs *after* its trees are actually gone, and a purge is visibly distinct
|
||||
/// from a plain destroy on the graph instead of being a hidden boolean.
|
||||
#[test]
|
||||
fn destroy_shape_purge_inserts_purge_state_before_the_tail() {
|
||||
let q = JobQueue::new(1);
|
||||
insert(&q, |builder| {
|
||||
templates::destroy(builder, "doomed", true);
|
||||
});
|
||||
assert_eq!(
|
||||
declared_shape(&q),
|
||||
vec![
|
||||
row("stop", None, &[]),
|
||||
row("destroy_container", Some("stop"), &[]),
|
||||
row(
|
||||
"purge_state",
|
||||
Some("stop"),
|
||||
&[("destroy_container", "done")]
|
||||
),
|
||||
row(
|
||||
"destroy_bookkeeping",
|
||||
Some("stop"),
|
||||
&[("purge_state", "done")]
|
||||
),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
/// The counter-case to `rebuild_chain_nodes_suppress_crash_watch`, and the one
|
||||
/// assertion in this file that exists to stop a *plausible* edit rather than a
|
||||
/// wrong one.
|
||||
///
|
||||
/// `destroy_container` is the most obvious candidate for `takes_container_down`
|
||||
/// on the whole list and must stay `false`. It is edged downstream of a `Stop`
|
||||
/// that already carries the flag, so the intentional stop is already accounted
|
||||
/// for; a container still alive when this node claims is a genuine bug. Since a
|
||||
/// wrong `true` **silently swallows a real crash** while a wrong `false` only
|
||||
/// costs a spurious event, this is the asymmetry that has to be pinned.
|
||||
#[test]
|
||||
fn destroy_container_must_not_suppress_crash_watch() {
|
||||
assert!(
|
||||
!NodeKind::DestroyContainer {
|
||||
agent: "a".to_owned()
|
||||
}
|
||||
.takes_container_down(),
|
||||
"destroy_container runs after a Stop that already declared the \
|
||||
container is going down; claiming it again would suppress the alert \
|
||||
for a container found unexpectedly alive"
|
||||
);
|
||||
// The upstream node is where the `true` lives — assert it here too, so the
|
||||
// pair reads as one property and moving the flag breaks this test.
|
||||
assert!(
|
||||
NodeKind::Stop {
|
||||
agent: "a".to_owned()
|
||||
}
|
||||
.takes_container_down()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn perm_change_shape_prefixes_rebuild_chain() {
|
||||
let q = JobQueue::new(1);
|
||||
|
|
|
|||
Loading…
Reference in a new issue