[the-gigi-pplx]

Summary

Makes the cluster controller's node drain evacuate a stopped sandbox whose stay-stopped runtime upgrade is in flight or didn't finish, without minting a disk backup that a later boot of the upgrade would invalidate. It also pins down that no sweeper or GC loop takes such an upgrade's execution or pod. Nothing calls the upgrade yet, so merging changes no behavior until something does.

Terms

• Stay-stopped upgrade: SubstrateAdapter::upgrade_stopped_runtime, from the base pull request. It moves a stopped Kubernetes sandbox to the cluster's default runtime environment in an execution of its own, a row in sandbox_k8s_executions whose activation attempt targets stopped, and records the upgrade's operation on it (runtime_upgrade_operation_id). The sandbox row stays stopped throughout.
• Lease: the activation attempt's activation_attempt_expires_at. While it runs, the upgrade owns the execution; once it ends, a retry may take the execution over.
• Leftover upgrade: the execution an unfinished stay-stopped upgrade leaves, read by leftover_stopped_upgrade, often with its pod still there. Kubelet keeps retrying that pod's create after the lease ends, and each try boots the upgrade on the node.
• Stopped evacuation: the drain's path for a stopped resident of a force-draining node (evacuate_stopped_sandbox in crates/l40_api/cluster/controller/server/src/node_drain/store.rs). It claims the sandbox by setting its marker (evacuation_restore_target = 'stopped'), has the node mint a disk artifact and back it up, and once the backup is durable tears the node's copy down and releases the node.
• Settle: KubernetesSubstrate::settle_expired_stopped_upgrade, from the base. For a leftover upgrade whose lease ended, it reserves the execution, deletes the upgrade's pod and abandons the operation on the node, which stops a boot the upgrade verified but didn't stop.

The problem

The drain's stopped evacuation predates stay-stopped upgrades, and an unfinished one breaks it in three ways:

1. A boot after the mint wedges the drain. The evacuation mints its disk artifact on the node, waits for the backup and then tears the node's copy down. If the upgrade's pod is still there, a kubelet retry can boot the upgrade after the mint. That boot drops the node's pin on the artifact, so the teardown defers on every tick and the node is never released.
2. A verified boot that wasn't stopped blocks the node check. An upgrade whose lease ended may have booted and verified the sandbox without stopping it. The node then reports the sandbox running, and the evacuation's node check (step 0c) defers on it every tick, before any step that could settle the upgrade runs.
3. Opening an evacuation during a live lease races the upgrade. The upgrade may still be placing or booting its pod. Claiming the sandbox then would mint and tear down underneath it.

Separately, the sweepers and GC loops classify executions and pods against the sandbox row's lifecycle. An upgrade's execution exists while the row is stopped, and none of them had a test showing they leave it alone.

How this fixes it

• Open the evacuation through open_stopped_evacuation. It runs the existing claim_stopped_evacuation inside a transaction that first takes the sandbox row's lock, and refuses while a leftover upgrade holds a live lease. The sandbox defers until the lease ends; other sandboxes on the node go on evacuating. The upgrade's own claim and takeover take the same lock and read the marker under it, so once the evacuation is open no new upgrade claim or takeover can start.
• Settle an expired upgrade before checking the node (step 0a). On an unmarked row, the evacuation settles the expired upgrade and defers until its pod is gone, so a verified boot is stopped before step 0c reads the node. It runs after the cluster-pin check (step 0b), so a row the claim would refuse on its pin keeps its upgrade for the retry. It runs only when the drain's state read (stopped_evacuation_state, which gains a resident column) still finds the row a stopped resident of this force-draining node, so an undrain after the listing leaves the upgrade alone and the claim refuses the row.
• Remove the leftover pod before minting (step 1c). Once the evacuation is open it holds the execution: it deletes any pod the upgrade left and defers until the pod is gone, and only then mints. A resumed evacuation whose backup is already durable waits out the pod the same way before its teardown, which retires the upgrade's execution with the pre-stop one (the base's retire_executions_predating_current_stop). The backup the evacuation mints stands in for the one the upgrade owed.
• Report a refused abandon as needing an operator, and hold the node. When the node refuses, as invalid, to abandon the upgrade's operation (the base's settle and pod removal answer FailedPrecondition: sandbox-manager records the operation for another sandbox, or can't parse its ids), no retry changes it. The evacuation reports UpgradeAbandonRefused instead of a local_teardown failure. It counts node_drain.stopped_evacuation{outcome=upgrade_abandon_refused} on every tick, and logs a WARN naming the node, sandbox, operation and the node's reason on the tick the node refused. The sink remembers the refusal for the controller process's lifetime, so later ticks don't call the node again; a restart or leader change asks once more, and so does a new upgrade operation. So a repair of the node's record of the operation takes effect only once the drain asks again, and deleting the sandbox stops it holding the node once its row reaches deleted. The sandbox stays stopped and placed on its node, which sandbox_residency::active_resident_predicate counts. That predicate is what the drain's guarded draining → drained write reads (with the fourth gate term on, which the stopped evacuation requires) and what the terminator's drained → terminating write re-checks, so the node is neither drained nor terminated while the sandbox's only copy is there.
• Pin down the sweepers and GC loops. insert_unsettled_stopped_runtime_upgrade in lifecycle_writer_test_support.rs seeds what an unsettled upgrade leaves for its retry, and the orphaned-sandboxes scan, the transient lifecycle sweeper and the stuck-ContainerCreating GC are each shown to leave it alone. The orphaned-restore-pod GC's existing skip case gains a stopped row, whose live execution's pod is the one an upgrade boots on.

This leaves the claim's own guards, the mint, the backup and the teardown's authorization as they were. On a sandbox with no leftover upgrade, the new reads find nothing and the evacuation runs exactly as before.

Validation

At this head, stacked on its base, on a Linux build box:

• cargo clippy --workspace --locked --all-targets --all-features -- -D warnings: clean.
• The repository's own test lanes, ci/scripts/run-rust-checks.sh tests-unprivileged and then tests-privileged as root inside ci/scripts/with-nocow-test-filesystem.sh, non-incremental:
  • Unprivileged: 13073 passed, 78 skipped, plus five smaller nextest runs in the same lane (302 tests), all passing.
  • Privileged: 72 passed, 6 failed. The six fail on that build box whatever the change, and none is in a crate this pull request touches: three pplx-space-node-runtime-core tests need fs-verity, which the box's kernel doesn't have; two pplx-space-node-runtime-supervisor contained_launcher tests refuse a temp directory whose ancestors aren't root-owned there; and pplx-space-spaced-guest-worker::embedding needs the private cgroup namespace CI sets up.
• The api and internal-api crates are unchanged from the base, whose own validation covers them.

New tests, against Postgres:

• node_drain::store::stopped_evacuation_upgrade_tests:

  • a_sandbox_whose_upgrade_retired_is_still_offered_for_evacuation
  • a_leftover_upgrade_pod_is_gone_before_the_evacuation_mints
  • an_upgrade_claimed_before_the_drain_opens_finishes_or_ends_first
  • a_live_upgrade_defers_only_its_own_sandbox
  • a_resumed_durable_evacuation_waits_out_a_leftover_upgrade_pod
  • an_undrained_node_never_settles_an_expired_upgrade
  • a_row_refused_on_its_pin_keeps_its_expired_upgrade
  • an_open_evacuation_deletes_a_leftover_upgrade_pod_before_it_mints
  • a_refused_abandon_needs_an_operator_and_holds_the_node: the node refuses to abandon an expired upgrade's operation. The evacuation reports the refusal twice, from one call to the node; a neighbour on the same node still evacuates; and neither the drain's draining → drained write nor the terminator's drained → terminating write goes through until the refused sandbox leaves the node.
  • an_open_evacuations_refused_abandon_is_asked_once: the same refusal on an evacuation already open, met at step 1c.

  The faked substrate settles an expired upgrade as production does, against the real leftover-upgrade read and a scripted pod. Each of seven mutations fails at least one of these tests: dropping step 0a, running it before the pin check, dropping its residency check, dropping step 1c on an open evacuation, treating a refused abandon as an ordinary failure, and dropping the recorded refusal at step 0a or at step 1c. The settle's reservation isn't faked; the api crate's own settle tests cover it.

• orphaned_sandboxes: a_stopped_runtime_upgrade_execution_is_never_an_orphan_candidate and an_unsettled_stopped_runtime_upgrade_row_is_left_for_its_retry.

• transient_lifecycle_sweeper::tests::stopped_runtime_upgrade: no_sweep_pass_takes_a_stopped_runtime_upgrade_execution and an_unsettled_stopped_runtime_upgrade_row_is_left_for_its_retry.

• stuck_container_creating_gc: a_stopped_runtime_upgrade_pod_is_never_reaped and an_unsettled_stopped_runtime_upgrade_pod_is_left_for_its_retry. This GC reads only pods, so neither test needs Postgres.

Not validated here: a Kind run. Nothing calls upgrade_stopped_runtime in this pull request, so no Kind scenario reaches these paths. The pull request above this one, which adds the test-only trigger, carries the bench run.

Not covered

• An undrain that commits between the drain's state read and step 0a's settle can still have an expired stay-stopped upgrade's leftover pod deleted and its operation abandoned on the undrained node. The settle's reservation, in the base, checks the execution's expired claim, not the node's drain state. That costs only that upgrade attempt: the settle doesn't clear the upgrade's record on the execution, so the backup it owes stays owed, the next activation's owed-backup path takes the same steps before it claims the execution, and a retry of the operation reads the node's abandonment as an upgrade that never started.

Dependencies

Stacked on #1714, this pull request's base. The chain below it merges first, in this order:

1. [sbx-mgr] answer a backup of a sandbox not yet at rest as retryable #1750
2. [sbx-mgr] hold suspends while a runtime upgrade that ends stopped runs #1751
3. [sbx-mgr] publish the execution a rollback restores while a runtime upgrade fences a sandbox #1752
4. [sbx-mgr] add the per-attempt runtime upgrade fields and AbandonRuntimeUpgrade to the wire #1753
5. [sbx-mgr] record runtime upgrade outcomes and refused attempts per attempt id in the store #1754
6. [sbx-mgr] make a runtime upgrade rollback or refusal final for its attempt #1755
7. [sbx-mgr] add AbandonRuntimeUpgrade, so no late call for an abandoned operation fences or boots #1756
8. [sbx-mgr] keep a pod's runtime upgrade finality when its sandbox row is deleted #1707: sandbox-manager's per-pod outcomes and AbandonRuntimeUpgrade.
9. [api] boot a start's or resume's runtime upgrade in its pod and read its outcome from the node #1708: a start's or resume's upgrade boots in its pod and reads the outcome from the node.
10. [api] upgrade a stopped sandbox's runtime in place without starting it, and owe its disk backup #1714: the stay-stopped upgrade on the api and internal-api side.

Then this pull request. #1322, the test-only admin trigger, Kind settings, typed smoke scenario and docs, is stacked on this one and merges after it.

Severity and risk

Severity: medium. Without this, once anything calls the stay-stopped upgrade, a node drain could mint and release a stopped sandbox's disk while an unfinished upgrade can still boot it, or wedge on that sandbox forever.

Risk: low. Until something calls upgrade_stopped_runtime, no execution records a stay-stopped upgrade. open_stopped_evacuation then runs the same claim_stopped_evacuation, inside a transaction that first takes the sandbox row's lock, and the leftover-upgrade reads and the settle find nothing, costing a few more reads per evacuation tick. Roll back by reverting; no migration is involved.

Docs

None. This pull request changes no docs.

AI assistance: Claude Code split this pull request out of PR #1322, made the review fixes in it, and drafted this description.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Pre-merge checks: 5 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Title check		This assessment has not completed. The check will be retried on the next pre-merge check run.

Passed checks: 5 passed

Check name	Status	Explanation
Description Follows The Pr Template		All four required items are present with real content in the PR description (#1733, author the-gigi-pplx). This check only tests the description text, so no code inspection was neede…
Bug Fixes Name A Regression Test		The check applies only when the PR fixes a defect in application behavior under crates/, shim/, or ui/. This PR changes the cluster controller under crates/l40_api/cluster/controller/. Its "Problem" s…
Weakened Safeguards Are Justified		The diff for HEAD a5c55cd against HEAD~1 touches 12 Rust files and no CI, lint, deny.toml, nextest, AGENTS.md, CLAUDE.md, CONTRIBUTING.md, or docs/rust/ files. I found no weakened safeguard: - **No n…
Agent Tooling Runs No New Commands		No agent plugin manifests, skills, command files, or guideline files are changed in this pull request. I compared the PR's base (b5ba1b5) with HEAD (a5c55cd). The diff touches 12 files, all Rust sou…
Public-Safe Content		The PR's own commit (a5c55cd, 12 files, 1645 added lines) contains no public-safety violations. - Credentials and keys: no private-key blocks, AWS key IDs, ARNs, GitHub or Slack tokens, or password/s…

Full details: Description Follows The Pr Template

Explanation

All four required items are present with real content in the PR description (#1733, author the-gigi-pplx). This check only tests the description text, so no code inspection was needed. 1. "## Summary": Present. It states that the drain's stopped evacuation now handles a stopped sandbox with an unfinished stay-stopped runtime upgrade. It also says that merging changes no behavior until something calls upgrade_stopped_runtime. 2. "## Validation": Present. It names the commands and test lanes that were run, with results. It lists the new tests and the Postgres-backed runs. It also states what was not validated (no Kind run). 3. "## Severity and risk": Present. It gives "Severity: medium" and "Risk: low", each with a justification. 4. "AI assistance:" line: Present. It reads "AI assistance: Claude Code split this pull request out of PR #1322, made the review fixes in it, and drafted this description." This names the tool and the parts it changed. No required item is missing or limited to template HTML comments. The check judges presence only, so the content quality is not assessed.

Full details: Bug Fixes Name A Regression Test

Explanation

The check applies only when the PR fixes a defect in application behavior under crates/, shim/, or ui/. This PR changes the cluster controller under crates/l40_api/cluster/controller/. Its "Problem" section lists three ways the node drain's stopped evacuation fails when a stay-stopped upgrade is unfinished. The PR describes this as hardening, because no code path calls upgrade_stopped_runtime yet. Read either way, the check passes, for these reasons: - The Validation section names regression tests. They are node_drain::store::stopped_evacuation_upgrade_tests (ten tests, including a_refused_abandon_needs_an_operator_and_holds_the_node and a_leftover_upgrade_pod_is_gone_before_the_evacuation_mints), orphaned_sandboxes (two tests), transient_lifecycle_sweeper::tests::stopped_runtime_upgrade (two tests), and stuck_container_creating_gc (two tests). - Validation also reports seven mutation runs. Each mutation removes one part of the fix: dropping step 0a, running it before the pin check, dropping its residency check, dropping step 1c on an open evacuation, treating a refused abandon as an ordinary failure, and dropping the recorded refusal at step 0a or at step 1c. The description states that each mutation fails at least one named test. That is the fail-without-fix evidence the check asks for. - The named tests pass at the PR head, per the Validation section. I did not re-run the tests. Linked-repository inspection was unavailable, and the check turns on what the Validation section states.

Full details: Weakened Safeguards Are Justified

Explanation

The diff for HEAD a5c55cd against HEAD~1 touches 12 Rust files and no CI, lint, deny.toml, nextest, AGENTS.md, CLAUDE.md, CONTRIBUTING.md, or docs/rust/ files. I found no weakened safeguard: - No new suppressions or skips. No added #[allow], #[expect], #[ignore], skip, xfail, or continue-on-error, and no [lints] or allowlist edits. - Removed lines are doc comments, imports, or visibility changes. The - lines in store.rs are doc-comment rewording, import reshuffles, and an extended StoppedEvacuationState query tuple that gains a resident column. The test-file removals are only async fn → pub(super) async fn visibility changes (stopped_evacuation_tests.rs, episode_deadline.rs) and one comment edit in orphaned_restore_pod_gc/tests.rs. - No assertions removed. orphaned_restore_pod_gc/tests.rs (current_generation_pod_is_skipped) adds SandboxLifecycle::Stopped to the loop, which strengthens the test. The new assert_eq!(skip_count(...), 1) lines are additions. - New tests added. stopped_evacuation_upgrade_tests.rs (+644), stopped_runtime_upgrade.rs (+163), and new cases in orphaned_sandboxes, stuck_container_creating_gc, and lifecycle_writer_test_support.rs. - Guard changes are additions. store.rs adds step 0a settle and the open_stopped_evacuation gate. These add checks and do not remove any existing guard. - No review-steering text. No changed file is a review-guidance file, and none gains text about reviews, merge-risk ratings, or approvals.

Full details: Agent Tooling Runs No New Commands

Explanation

No agent plugin manifests, skills, command files, or guideline files are changed in this pull request. I compared the PR's base (b5ba1b5) with HEAD (a5c55cd). The diff touches 12 files, all Rust sources and Rust test modules under crates/l40_api/cluster/controller/server/. None of the changed paths match plugin.json, plugin.yaml, SKILL.md, skills/, commands/, CLAUDE.md, AGENTS.md, .claude/, or .md files. A search of the added lines for ! followed by a backtick, hooks, mcp, allowed-tools, lsp, plugin, or SKILL found no matches. The check's failure conditions do not apply, so the check passes.

Full details: Public-Safe Content

Explanation

The PR's own commit (a5c55cd, 12 files, 1645 added lines) contains no public-safety violations. - Credentials and keys: no private-key blocks, AWS key IDs, ARNs, GitHub or Slack tokens, or password/secret assignments were found in the added lines. Matches for CancellationToken, route_token and similar identifiers are code names only. - Absolute personal paths: none. No /Users/ or /home/ paths were added. - URLs and internal hosts: the description links only to PRs in the same public repository (#1714, #1322, #1707 to #1756). No internal hostnames, trackers, chats or documents were linked. The only private-range address added is 10.0.0.2 in lifecycle_writer_test_support.rs (line 225), a generic SQL test fixture inside a CASE expression. It is not a hostname and does not identify a real environment. - Cloud account IDs and resource names: no 12-digit account IDs or real ARNs were found. - Environment names, production data and incidents: no cluster or environment names, production logs, query output, measurements or incident details were found. The description's "Severity and risk" section is a generic impact statement. - Security vulnerabilities: the description covers correctness and liveness defects (drain wedging, a race between evacuation and upgrade claims). It does not describe a security vulnerability. The "Not covered" section describes a narrow ordering gap in node drain state. It is not framed as a security issue, and it is not a sensitive disclosure under this check. Context: the main...HEAD diff includes stacked base PRs. This verdict relies on the PR-only diff (HEAD^..HEAD).