fix: deflake //rs/tests/nns:delete_subnet_test_local

[github-actions]

This PR was created by https://github.com/dfinity/ic/actions/runs/37827535031 following .claude/skills/fix-flaky-tests/SKILL.md to deflake:

• //rs/tests/nns:delete_subnet_test_local

---

Deflakes //rs/tests/nns:delete_subnet_test_local.

Flaky labels:

• //rs/tests/nns:delete_subnet_test_local

Root cause

The test flaked once in the last week, on master at commit 8d8880f (https://github.com/dfinity/ic/actions/runs/37752534307). Attempt 1 failed and attempt 2 passed. The test task failed on the assert!(metrics[&fs_trim_duration][0] > 0) in assert_node_is_unassigned_with_ssh_session (rs/tests/consensus/utils/src/node.rs). The test calls this helper at the end, once for each of the 6 nodes of the 3 deleted subnets.

The helper works in two steps:

1. It polls the node over SSH every 10 s until the node has deleted its state directory and its local CUP.
2. It fetches orchestrator_state_removal_failed_total and orchestrator_fstrim_duration_milliseconds in a retry_with_msg! loop and asserts that the fstrim duration is greater than 0.

The orchestrator does things in a different order (remove_state in rs/orchestrator/src/upgrade.rs). It first deletes the state and the CUP. Then it runs /opt/ic/bin/sync_fstrim.sh and waits for it. Only then does it set the fstrim duration gauge. In between, the node already looks unassigned but the gauge is still 0. In the failed attempt this window lasted 27 to 213 ms, depending on the node.

The failed attempt hit that window. On the failing node, the orchestrator removed the subnet state and started sync_fstrim.sh at 09:10:10.249. The test's next SSH poll saw the node as unassigned right after that. The test then fetched the metrics and found the gauge still at 0. The orchestrator set the gauge to 46 ms a moment later.

The assert! panics inside the retry closure instead of returning an Err, so retry never retried. To hit the race, a poll has to land in a window of tens to hundreds of milliseconds, and the helper polls every 10 s. That is why this failure is rare.

Fix

Replace the assert! with anyhow::ensure!. The retry loop then retries every 10 s, for up to 120 s, until the node has finished trimming its filesystem.

The assert_eq! on orchestrator_state_removal_failed_total stays. That counter never goes back to 0, so failing right away is correct.

node_reassignment_test and the subnet recovery tests use the same helper, so they get the same fix. None of them flaked or failed on this assertion in the last month.

Verification

• cargo check --all-targets --all-features -p ic_consensus_system_test_utils, cargo fmt and ./ci/scripts/rust-lint.sh all pass (exit 0). No dependencies changed.
• bazel build //rs/tests/consensus/utils:utils succeeds. This is the only direct reverse dependency of the changed file. The depth-2 query for affected tests returns no tests.
• bazel test --test_output=errors --runs_per_test=3 --local_test_jobs=2 //rs/tests/nns:delete_subnet_test_local: 3 of 3 runs passed (88.8 to 101.7 s). None of them happened to hit the race window.
• To exercise the race, I used a throwaway change that is not part of this PR. It made the helper poll for unassignment every 10 ms instead of every 10 s, and retry the metrics fetch every 100 ms. The test then checks each node right after the node removed its state. Results with --runs_per_test=2:
  • With this fix, 2 of 2 runs passed. In 3 of the 12 node checks, the first metrics fetch found that the node had not finished trimming its filesystem yet. Each time, the retry succeeded about 125 ms later.
  • Without this fix, 1 of 2 runs failed on the same assertion as on CI.

This PR was created following the steps in .claude/skills/fix-flaky-tests/SKILL.md.

🤖 Generated with Claude Code (https://claude.com/claude-code)

[Copilot]

🟢 Approval recommended

The focused change correctly addresses the identified race while preserving immediate failure for state-removal errors.

0 open findings

What changed in this PR

Updates the shared node-unassignment helper to retry while filesystem trimming is still in progress.

Changes:

• Replaces a premature assertion with a retryable ensure!.
• Documents the orchestrator timing race.

File	Description
rs/​tests/​consensus/​utils/​src/​node.rs	Makes the fstrim metric check retryable.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.