orchestrator: Leave retry and terminal decisions to the orchestrator
WalkVerdict now reports observation only: Failed{checkpoint, cause}
replaces Retry/Dead/retries_left — the state machine's
ComponentStatus.retry and Recovering→RecoveryFailed path already own
those decisions, and a second counter could only agree or disagree
with the first. max_retries leaves the table for the same reason: a
retry re-resets the device and re-runs the whole walk, so budgets are
per boot attempt, owned where boot attempts are owned. The device's
own judgment still flows up as FailureCause::{TimedOut,
DeviceRetriable, DeviceFatal} — the one input the retry decision
needs.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Christina Quast <christina.quast@9elements.com>
diff --git a/services/orchestrator/capabilities/src/boot_watch.rs b/services/orchestrator/capabilities/src/boot_watch.rs
index 28de726..6c05cc8 100644
--- a/services/orchestrator/capabilities/src/boot_watch.rs
+++ b/services/orchestrator/capabilities/src/boot_watch.rs
@@ -19,9 +19,15 @@
/// Everything the orchestrator needs to know about a boot walk.
///
-/// Deliberately free of device and error types: the orchestrator acts the
-/// same whatever the cause, so the concrete detail is logged by the walk
-/// while it is still in scope, not carried across the seam.
+/// Observation only: the walk judges checkpoint windows, never lives.
+/// Retry counts and terminal calls belong to the orchestrator state
+/// machine (`ComponentStatus.retry`, the `Recovering` → `RecoveryFailed`
+/// path) — a verdict that carried a retry budget would be a second owner
+/// for the same decision, free to disagree with the first.
+///
+/// Deliberately free of device and error types: the concrete detail is
+/// logged by the walk while it is still in scope, not carried across the
+/// seam.
///
/// Intentionally exhaustive (not `#[non_exhaustive]`): adding a verdict is
/// a breaking change, so the compiler forces every consumer — in particular
@@ -35,29 +41,31 @@
},
/// Every checkpoint passed — the device is up.
Complete,
- /// The attempt failed — a window expired, or the device reported
- /// [`FailedRetriable`](crate::BootStatus::FailedRetriable) (which ends
- /// the wait early) — and retry budget remains; the window is re-armed
- /// from the poll that judged it. The caller re-resets the device and
- /// keeps polling — what a retry re-runs is the caller's policy.
- Retry {
- /// The checkpoint that failed.
+ /// This boot attempt failed at `checkpoint`; the walk is over.
+ /// Whether to try again, recover, or give up is the orchestrator's
+ /// decision — a retry re-resets the device and starts a fresh walk.
+ Failed {
+ /// The checkpoint the attempt died at.
checkpoint: &'static str,
- /// Attempts left after this one.
- retries_left: u8,
- /// When the re-armed window expires: the judging poll's
- /// `now_millis` plus the checkpoint's `timeout`. The caller
- /// schedules against this exactly as it does for `Waiting` —
- /// no deadline arithmetic of its own.
- deadline_millis: u64,
+ /// Why it died — the one input the retry decision needs.
+ cause: FailureCause,
},
- /// This boot is dead: retry budget exhausted, or the device reported
- /// [`FailedFatal`](crate::BootStatus::FailedFatal) — a verdict no
- /// remaining budget can overturn. Recovery is the caller's move.
- Dead {
- /// The checkpoint the boot died at.
- checkpoint: &'static str,
- },
+}
+
+/// Why a boot attempt failed at a checkpoint.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum FailureCause {
+ /// The window expired; the device reported nothing.
+ TimedOut,
+ /// The device reported a failure worth another attempt
+ /// ([`FailedRetriable`](crate::BootStatus::FailedRetriable)) — the
+ /// wait ended early.
+ DeviceRetriable,
+ /// The device reported a terminal failure
+ /// ([`FailedFatal`](crate::BootStatus::FailedFatal)) — re-running the
+ /// same image cannot change the verdict, whatever retry budget the
+ /// orchestrator has left.
+ DeviceFatal,
}
#[cfg(test)]
@@ -94,13 +102,13 @@
};
let mut nic = ScriptedWalk {
verdicts: &[
- WalkVerdict::Retry {
+ WalkVerdict::Failed {
checkpoint: "heartbeat",
- retries_left: 1,
- deadline_millis: 30_000,
+ cause: FailureCause::TimedOut,
},
- WalkVerdict::Dead {
+ WalkVerdict::Failed {
checkpoint: "heartbeat",
+ cause: FailureCause::DeviceFatal,
},
],
next: 0,
@@ -117,10 +125,9 @@
WalkVerdict::Waiting {
deadline_millis: 90_000
},
- WalkVerdict::Retry {
+ WalkVerdict::Failed {
checkpoint: "heartbeat",
- retries_left: 1,
- deadline_millis: 30_000
+ cause: FailureCause::TimedOut
},
]
);
@@ -128,8 +135,9 @@
second,
[
WalkVerdict::Complete,
- WalkVerdict::Dead {
- checkpoint: "heartbeat"
+ WalkVerdict::Failed {
+ checkpoint: "heartbeat",
+ cause: FailureCause::DeviceFatal
},
]
);
diff --git a/services/orchestrator/capabilities/src/lib.rs b/services/orchestrator/capabilities/src/lib.rs
index 8a65574..85f7019 100644
--- a/services/orchestrator/capabilities/src/lib.rs
+++ b/services/orchestrator/capabilities/src/lib.rs
@@ -33,5 +33,5 @@
pub use boot_control::BootControl;
pub use boot_status::BootStatus;
-pub use boot_watch::{BootWatch, WalkVerdict};
+pub use boot_watch::{BootWatch, FailureCause, WalkVerdict};
pub use evidence::EvidenceReader;
diff --git a/services/orchestrator/config/src/lib.rs b/services/orchestrator/config/src/lib.rs
index f4144dc..ffc34ab 100644
--- a/services/orchestrator/config/src/lib.rs
+++ b/services/orchestrator/config/src/lib.rs
@@ -7,8 +7,10 @@
#![cfg_attr(not(test), no_std)]
-/// One boot checkpoint: a signal the orchestrator waits for, how long it
-/// waits per attempt, and how many failed attempts it tolerates.
+/// One boot checkpoint: a signal the orchestrator waits for, and how long
+/// it waits. Retry policy is deliberately not table data: a retry
+/// re-resets the device and re-runs the whole walk, so budgets are
+/// per boot attempt and owned by the orchestrator state machine.
///
/// `signal` is a board-defined id — the schema attaches no meaning to it
/// and names no signal kinds. Each board defines its own vocabulary (a
@@ -28,9 +30,6 @@
/// Window for one attempt at this checkpoint. Expiry is the
/// orchestrator's own judgment; hung devices report nothing.
pub timeout: core::time::Duration,
- /// Attempts allowed beyond the first before the failure is final.
- /// `0` means the one attempt is all the device gets.
- pub max_retries: u8,
}
/// One managed downstream device, as declared by the board config.
@@ -55,7 +54,8 @@
pub reset_signal: R,
/// Boot checkpoints, in the order the device passes them. The device
/// counts as booted when the last one is reached; a checkpoint whose
- /// window and retry budget are exhausted fails the boot.
+ /// window expires fails the attempt — whether to retry or recover is
+ /// the orchestrator's decision, not table data.
pub checkpoints: &'static [BootCheckpoint<G>],
}
@@ -133,7 +133,6 @@
name: "boot-complete",
signal: 0,
timeout: Duration::from_secs(1),
- max_retries: 1,
};
const DEVICE: DeviceConfig<u8, u8> = DeviceConfig {
diff --git a/target/mock/devices.rs b/target/mock/devices.rs
index a5fa71a..6008e1b 100644
--- a/target/mock/devices.rs
+++ b/target/mock/devices.rs
@@ -39,7 +39,6 @@
name: "boot-complete",
signal: MockSignal::Gpio(12),
timeout: Duration::from_secs(90),
- max_retries: 1,
}],
},
// PLDM device (NIC archetype): self-updating, SPDM-capable. Two
@@ -53,13 +52,11 @@
name: "mctp-ready",
signal: MockSignal::MctpReady,
timeout: Duration::from_secs(20),
- max_retries: 2,
},
BootCheckpoint {
name: "heartbeat",
signal: MockSignal::Heartbeat,
timeout: Duration::from_secs(10),
- max_retries: 0,
},
],
},
