From 40c3e342fce69f4763b659d285158e96c012217e Mon Sep 17 00:00:00 2001 From: Pierre Tenedero Date: Tue, 27 Oct 2026 21:56:15 +0800 Subject: [PATCH] Extend recovery supervision --- .mise/config.js.toml | 6 +- .mise/config.rust.toml | 30 +- .mise/config.toml | 17 +- CHANGELOG.md | 9 + Cargo.lock | 3 + generated/rust-rest/Cargo.toml | 15 +- generated/rust-rest/src/lib.rs | 34 +- generated/rust-rest/src/retry.rs | 156 +++++ generated/rust-rest/tests/unit/retry.rs | 372 +++++++++++ services/recovery-supervisor/Cargo.toml | 6 + services/recovery-supervisor/HELP.md | 6 + services/recovery-supervisor/README.md | 71 +- services/recovery-supervisor/src/config.rs | 6 + services/recovery-supervisor/src/health.rs | 34 +- .../recovery-supervisor/src/kubernetes.rs | 29 +- services/recovery-supervisor/src/lib.rs | 1 + .../recovery-supervisor/src/persistence.rs | 20 +- services/recovery-supervisor/src/policy.rs | 622 ++++++++++++++++-- .../recovery-supervisor/src/resilience.rs | 226 +++++++ services/recovery-supervisor/src/runtime.rs | 142 +++- services/recovery-supervisor/tests/config.rs | 86 +++ services/recovery-supervisor/tests/health.rs | 32 +- .../recovery-supervisor/tests/kubernetes.rs | 118 +++- .../recovery-supervisor/tests/persistence.rs | 228 ++++++- services/recovery-supervisor/tests/policy.rs | 118 +++- .../recovery-supervisor/tests/resilience.rs | 538 +++++++++++++++ services/recovery-supervisor/tests/runtime.rs | 164 ++++- .../tests/unit/policy_guards.rs | 144 ++++ services/storage/Cargo.toml | 1 + services/storage/tests/s3_backend.rs | 49 +- .../cli/src/deployment_types/k3s_recovery.rs | 36 +- utilities/cli/src/recovery.rs | 4 + utilities/cli/tests/recovery.rs | 24 + utilities/int-gen/src/openapi.rs | 51 +- .../local/output/pyo3-math1-recovery/k3s.yaml | 2 + 35 files changed, 3152 insertions(+), 248 deletions(-) create mode 100644 generated/rust-rest/src/retry.rs create mode 100644 generated/rust-rest/tests/unit/retry.rs create mode 100644 services/recovery-supervisor/src/resilience.rs create mode 100644 services/recovery-supervisor/tests/resilience.rs create mode 100644 services/recovery-supervisor/tests/unit/policy_guards.rs diff --git a/.mise/config.js.toml b/.mise/config.js.toml index 3f387061..9f0fb66d 100644 --- a/.mise/config.js.toml +++ b/.mise/config.js.toml @@ -119,13 +119,13 @@ dir = "services/ws-modules/har1" # Chain with `&&` on one line rather than a multi-line `bash` block. # wasm-pack shells out to `cargo metadata`, which can't find `cargo` under Git Bash on Windows; the default # shell (cmd there) resolves it. web_pack_cov folds in the coverage wrapper prefix so this line stays under 120. -run = "{{ vars.web_pack_cov }} {{ vars.no_opt }}{{ vars.web_cov_feat }} && {{ vars.et_cli }} module-package-json" +run = "{{ vars.web_pack_cov }} {{ vars.no_opt }}{{ vars.web_cargo_args }} && {{ vars.et_cli }} module-package-json" [tasks.build-ws-face-detection-module] depends = ["build-et-cli", "build-wasm-cov-wrapper"] description = "Build the face detection workflow WASM module" dir = "services/ws-modules/face-detection" -run = "{{ vars.web_pack_cov }} {{ vars.no_opt }}{{ vars.web_cov_feat }} && {{ vars.et_cli }} module-package-json" +run = "{{ vars.web_pack_cov }} {{ vars.no_opt }}{{ vars.web_cargo_args }} && {{ vars.et_cli }} module-package-json" [tasks.vendor-llm1-ort] description = "Stage transformers.js's pinned ORT wasm runtime into llm1's pkg/ort/" @@ -177,7 +177,7 @@ shell = "{{ vars.task_shell }}" depends = ["build-et-cli", "build-wasm-cov-wrapper", "vendor-llm1-ort"] description = "Build the llm1 chat WASM module" dir = "services/ws-modules/llm1" -run = "{{ vars.web_pack_cov }} {{ vars.no_opt }}{{ vars.web_cov_feat }} && {{ vars.et_cli }} module-package-json" +run = "{{ vars.web_pack_cov }} {{ vars.no_opt }}{{ vars.web_cargo_args }} && {{ vars.et_cli }} module-package-json" [tasks.build-ws-js-data1-module] description = "Bundle the js-data1 module (AWS SDK for JS v3) into pkg/ with esbuild" diff --git a/.mise/config.rust.toml b/.mise/config.rust.toml index 00cf07b0..c29c898d 100644 --- a/.mise/config.rust.toml +++ b/.mise/config.rust.toml @@ -87,7 +87,7 @@ shell = "{{ vars.task_shell }}" # Browser wasm-pack builds are coverage-instrumented when ET_TEST_COVERAGE is set. # The web_cov_wrapper RUSTC_WORKSPACE_WRAPPER (built by build-wasm-cov-wrapper) instruments only workspace # crates -- never dependency cdylibs such as wasm-streams that don't link minicov's -# __llvm_profile_runtime -- while web_cov_feat enables et-web/coverage to supply minicov's runtime. et-web's +# __llvm_profile_runtime -- while web_cargo_args enables et-web/coverage to supply minicov's runtime. et-web's # __et_capture_coverage export (folded by wasm-bindgen into every module's glue) lets the web-runner pull each # module's .profraw for wasm-cov. [tasks.build-wasm-cov-wrapper] @@ -98,85 +98,85 @@ run = "cargo build -p int-wasm-cov-wrapper" depends = ["build-wasm-cov-wrapper"] description = "Build the data1 workflow WASM module" dir = "services/ws-modules/data1" -run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cov_feat }}" +run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cargo_args }}" [tasks.build-ws-math1-module] depends = ["build-wasm-cov-wrapper"] description = "Build the math1 FedAvg WASM module" dir = "services/ws-modules/math1" -run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cov_feat }}" +run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cargo_args }}" [tasks.build-ws-math1-sender-module] depends = ["build-wasm-cov-wrapper"] description = "Build the math1-sender trigger WASM module" dir = "services/ws-modules/math1-sender" -run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cov_feat }}" +run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cargo_args }}" [tasks.build-ws-comm1-module] depends = ["build-wasm-cov-wrapper"] description = "Build the comm1 workflow WASM module" dir = "services/ws-modules/comm1" -run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cov_feat }}" +run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cargo_args }}" [tasks.build-ws-pic-viewer-module] depends = ["build-wasm-cov-wrapper"] description = "Build the pic-viewer broadcast picture viewer WASM module" dir = "services/ws-modules/pic-viewer" -run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cov_feat }}" +run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cargo_args }}" [tasks.build-ws-except1-module] depends = ["build-wasm-cov-wrapper"] description = "Build the except1 exception-handling demo WASM module" dir = "services/ws-modules/except1" -run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cov_feat }}" +run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cargo_args }}" [tasks.build-ws-sensor1-module] depends = ["build-wasm-cov-wrapper"] description = "Build the sensor1 workflow WASM module" dir = "services/ws-modules/sensor1" -run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cov_feat }}" +run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cargo_args }}" [tasks.build-ws-audio1-module] depends = ["build-wasm-cov-wrapper"] description = "Build the audio1 workflow WASM module" dir = "services/ws-modules/audio1" -run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cov_feat }}" +run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cargo_args }}" [tasks.build-ws-video1-module] depends = ["build-wasm-cov-wrapper"] description = "Build the video1 workflow WASM module" dir = "services/ws-modules/video1" -run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cov_feat }}" +run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cargo_args }}" [tasks.build-ws-bluetooth-module] depends = ["build-wasm-cov-wrapper"] description = "Build the bluetooth workflow WASM module" dir = "services/ws-modules/bluetooth" -run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cov_feat }}" +run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cargo_args }}" [tasks.build-ws-geolocation-module] depends = ["build-wasm-cov-wrapper"] description = "Build the geolocation workflow WASM module" dir = "services/ws-modules/geolocation" -run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cov_feat }}" +run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cargo_args }}" [tasks.build-ws-graphics-info-module] depends = ["build-wasm-cov-wrapper"] description = "Build the graphics info workflow WASM module" dir = "services/ws-modules/graphics-info" -run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cov_feat }}" +run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cargo_args }}" [tasks.build-ws-speech-recognition-module] depends = ["build-wasm-cov-wrapper"] description = "Build the speech recognition workflow WASM module" dir = "services/ws-modules/speech-recognition" -run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cov_feat }}" +run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cargo_args }}" [tasks.build-ws-nfc-module] depends = ["build-wasm-cov-wrapper"] description = "Build the nfc workflow WASM module" dir = "services/ws-modules/nfc" -run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cov_feat }}" +run = "{{ vars.web_pack_cov }}{{ vars.no_opt }}{{ vars.web_cargo_args }}" [tasks.build-ws-wasi-data1-module] depends = ["build-et-cli"] diff --git a/.mise/config.toml b/.mise/config.toml index a8d2a87e..552700b1 100644 --- a/.mise/config.toml +++ b/.mise/config.toml @@ -648,13 +648,18 @@ a_wasm_cov_instr = "-Cinstrument-coverage -Zcoverage-options=branch" # RUSTFLAGS for the wasm coverage builds: instrument + emit LLVM IR, no profiler runtime (minicov provides it). # Single codegen unit + LTO off so each crate emits one .ll the wasm-cov task can gut and feed to llc/llvm-cov. wasm_cov_rustflags = "--emit=llvm-ir {{ vars.a_wasm_cov_instr }} -Ccodegen-units=1 -Clto=off -Zno-profiler-runtime" -# Coverage fragments the wasm module build commands append unconditionally (empty unless ET_TEST_COVERAGE is set). +# WASI coverage arguments are empty unless ET_TEST_COVERAGE is set. # wasm_cov is an inline RUSTFLAGS env prefix; the nightly toolchain + conda clang come from the coverage [env]. -# The *_feat vars turn on minicov: cargo `--features` for the WASI cdylibs, wasm-pack `-- --features` for browsers -# (comm1 pulls et-web only under this feature). +# Browser builds explicitly select Rust's wasm target, independently of Windows' native CARGO_BUILD_TARGET. +# wasm-pack 0.15.1 otherwise builds native output and reports: +# "Expected exactly one .wasm file in the compiler artifact, but found []". +# Observed on https://github.com/edge-toolkit/core/actions/runs/41204094871/job/124545540158. +# Coverage adds minicov through et-web to the same Cargo argument list. wasi_cov_feat = '{% if env?.ET_TEST_COVERAGE == "true" %} --features coverage{% endif %}' wasm_cov = '{% if env?.ET_TEST_COVERAGE == "true" %}RUSTFLAGS="{{ vars.wasm_cov_rustflags }}"{% endif %}' -web_cov_feat = '{% if env?.ET_TEST_COVERAGE == "true" %} -- --features et-web/coverage{% endif %}' +web_cargo_args = """ -- --target wasm32-unknown-unknown +{%- if env?.ET_TEST_COVERAGE == "true" %} --features et-web/coverage{% endif -%} +""" # Command prefix routing a browser module's wasm-pack build through the workspace-only coverage wrapper. # Instruments our crates, never dependency cdylibs. Empty unless ET_TEST_COVERAGE is set, so normal builds get # no wrapper (no fingerprint churn). The wrapper binary is built by the build-wasm-cov-wrapper task dependency. @@ -675,7 +680,7 @@ wasm_scope = "--scope edge-toolkit " hub_root_module = "@edge-toolkit/et-ws-server-static" # Override-free head of a browser module's wasm-pack build: coverage wrapper prefix plus fixed build args. # Used by the two modules that also run et-cli module-package-json (har1, face-detection). It excludes no_opt / -# web_cov_feat -- those are appended inline at the call site because no_opt carries a per-platform override that a +# web_cargo_args -- those are appended inline at the call site because no_opt carries a per-platform override that a # nested var would freeze at this file's value. web_pack_cov = '{{ vars.web_cov_wrapper }}wasm-pack build . --target web {{ vars.wasm_scope }}' # Extra flag for the wasm-pack module builds; empty so wasm-opt runs. @@ -2681,7 +2686,7 @@ dir = "services/ws-wasm-agent" # so other globally-loaded tasks -- prefetch-ci's transitive deps via `prefetch:node` -- can reference it # without forcing `rust` into MISE_ENV. A bash `shell` for the same base-config PATH-dropout reason # `prefetch:rust` carries one. -run = "wasm-pack build . --target web {{ vars.wasm_scope }}{{ vars.no_opt }}" +run = "wasm-pack build . --target web {{ vars.wasm_scope }}{{ vars.no_opt }} -- --target wasm32-unknown-unknown" shell = "{{ vars.task_shell }}" [tasks."prefetch:node"] diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d36225b..de7e16d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,12 @@ Changelog 1.1.0. - `et-ws-wasi-runner` builds as `no_std` + `alloc` without its new default `std` feature, and its `board` module runs a guest on a microcontroller: compiled ahead of time with `engine::precompile` for the board's Pulley target, and hosted through the firmware's `board::Platform` and `board::Transport`. +- Native REST requests retry transient HTTP 408/429/500/502/503/504 responses as well as transport failures, using a + bounded backoff policy for replayable idempotent operations. Permission errors, non-idempotent methods and streaming + request bodies are not retried. Storage-call timings include the retry/backoff interval; retry attempts are logged. +- Recovery supervision reconnects after hub interruptions and restores incident context with explicit observation gaps. + Optional dependency checks, fleet action limits, bounded backoff with jitter and sustained-recovery assessment extend + the existing restart policy. Reports distinguish observed failure conditions from reasons for withholding actions. - An opt-in `et-recovery-supervisor` service observes application progress and coordinates bounded Kubernetes recovery. - The Python SDK provides recovery lifecycle messages and integrity-checked checkpoint storage helpers. - A scenario can list `machines:` to span several hosts. The first is the server, running the hub and the collector, @@ -112,6 +118,9 @@ Changelog 1.1.0. name a shell would need quoted or that some filesystem could not hold, such as `my mod`, `mod$(id)` or `con`; rename the directory. Generated deployments no longer escape these paths. The check is the new `et-sanitise` crate, which accepts a name only when both sanitize-filename, with its Windows rules, and shlex leave it unchanged. +- **Breaking:** Recovery state is bound to its complete configuration. Changing configuration requires a new state + location or an explicitly provisioned new job. Legacy reports retain their restart budget and are archived in the + new context. - The workspace requires actix-web 4.17. - **Breaking:** the minimum supported Rust version is 1.99.0, up from 1.91.1, for every published crate. - **Breaking:** an agent's IP address is always known. `AgentSummary::last_known_ip` and diff --git a/Cargo.lock b/Cargo.lock index 03d4aec7..521149d0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5962,9 +5962,11 @@ dependencies = [ "reqwest", "retry-policies", "serde", + "tempfile", "tokio", "tracing", "tracing-opentelemetry", + "tracing-subscriber", "web-sys", ] @@ -5994,6 +5996,7 @@ dependencies = [ "image", "log", "object_store", + "reqwest", "schemars 1.2.3", "serde", "serde_default", diff --git a/generated/rust-rest/Cargo.toml b/generated/rust-rest/Cargo.toml index a08e8b23..ed4d338a 100644 --- a/generated/rust-rest/Cargo.toml +++ b/generated/rust-rest/Cargo.toml @@ -1,12 +1,17 @@ [package] name = "et-rest-client" publish = true -description = "Rust client for ws-server's REST API. src/lib.rs regenerated; this Cargo.toml is hand-maintained." +description = "Rust REST client for ws-server. src/lib.rs is regenerated; retry.rs and Cargo.toml are hand-maintained." version = "0.2.0" edition.workspace = true license.workspace = true repository.workspace = true +# Private retry tests need log capture and temporary files. +# The compiler loads tests/unit/retry.rs through src/retry.rs's #[path] module, but cargo-shear misses its uses. +[package.metadata.cargo-shear] +ignored = ["tempfile", "tracing-subscriber"] + [lib] # src/lib.rs is generated by progenitor; its /** */ doc comments contain # markdown backtick prose that Rust's doctest runner tries to compile as code. @@ -30,8 +35,7 @@ tracing-opentelemetry = { workspace = true, optional = true } # `rustls` and `stream` are native-only -- WASM reqwest dispatches via the # browser's `fetch()` and has no notion of a TLS stack. # -# `retry-policies` + `tokio` back the generator-injected `ClientHooks::exec` -# backoff retry (see `utilities/int-gen/src/openapi.rs::inject_retry_exec`). +# `retry-policies` + `tokio` back the hand-maintained native request retry policy invoked by the generated hook. # Native-only: that hook is `#[cfg(not(wasm32))]`, and `tokio::time::sleep` / # `SystemTime` don't work under `wasm32-unknown-unknown`. [target.'cfg(not(target_arch = "wasm32"))'.dependencies] @@ -47,6 +51,11 @@ reqwest = { workspace = true, features = ["json"] } # Deno runtime's stubbed `globalThis.location`). web-sys = { workspace = true, features = ["Location", "Window"] } +[target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies] +tempfile.workspace = true +tokio = { workspace = true, features = ["fs", "io-util", "macros", "net", "rt", "test-util"] } +tracing-subscriber.workspace = true + # Every rustc warning here is an error, as it is in every hand-written crate. # This was the one workspace member with no [lints] table at all, so its warnings stayed warnings -- which is how # six unreachable calls sat in `cargo-doc-check` output instead of failing it. The full `[workspace.lints]` set is diff --git a/generated/rust-rest/src/lib.rs b/generated/rust-rest/src/lib.rs index c59c3606..fa795a82 100644 --- a/generated/rust-rest/src/lib.rs +++ b/generated/rust-rest/src/lib.rs @@ -116,40 +116,12 @@ impl ClientInfo<()> for Client { &() } } +#[cfg(not(target_arch = "wasm32"))] +mod retry; impl ClientHooks<()> for &Client { - // Injected by `utilities/int-gen` (inject_retry_exec): retry request execution with exponential backoff. reqwest's - // native retry has no backoff yet -- remove this and use `ClientBuilder::retries` once it does. #[cfg(not(target_arch = "wasm32"))] async fn exec(&self, request: ::reqwest::Request, _info: &OperationInfo) -> ::reqwest::Result<::reqwest::Response> { - use ::retry_policies::policies::ExponentialBackoff; - use ::retry_policies::{RetryDecision, RetryPolicy as _}; - let policy = ExponentialBackoff::builder() - .retry_bounds( - ::core::time::Duration::from_millis(250), - ::core::time::Duration::from_secs(5), - ) - .build_with_total_retry_duration(::core::time::Duration::from_secs(30)); - let started = ::std::time::SystemTime::now(); - let mut n_past_retries: u32 = 0; - loop { - // Retry only when the request can be replayed (no streaming body). - let Some(attempt) = request.try_clone() else { - return self.client().execute(request).await; - }; - match self.client().execute(attempt).await { - Ok(response) => return Ok(response), - Err(err) => match policy.should_retry(started, n_past_retries) { - RetryDecision::Retry { execute_after } => { - let wait = execute_after - .duration_since(::std::time::SystemTime::now()) - .unwrap_or_default(); - ::tokio::time::sleep(wait).await; - n_past_retries = n_past_retries.saturating_add(1); - } - RetryDecision::DoNotRetry => return Err(err), - }, - } - } + crate::retry::execute(self.client(), request).await } } #[allow(clippy::all)] diff --git a/generated/rust-rest/src/retry.rs b/generated/rust-rest/src/retry.rs new file mode 100644 index 00000000..4b860126 --- /dev/null +++ b/generated/rust-rest/src/retry.rs @@ -0,0 +1,156 @@ +//! Native retry policy for replayable, idempotent REST requests. +//! +//! Kept separate from the generated operations so regeneration cannot discard the policy. An SDK call includes +//! retry/backoff time; each retry is logged without request bodies, URLs or credentials. There are at most 32 +//! attempts and no retry starts beyond a 90-second window. The first attempt retains the caller's HTTP timeout; +//! later attempts also stop at the retry deadline. The policy does not replay a response body that fails after +//! response headers have already been returned. Numeric Retry-After delays are honoured; unsupported values stop +//! retry rather than risking an earlier request than the server permits. Browser WASM keeps its original policy. + +use std::time::{Duration, SystemTime}; + +use reqwest::{Client, Method, Request, Response}; +use retry_policies::policies::{ExponentialBackoff, ExponentialBackoffTimed}; +use retry_policies::{RetryDecision, RetryPolicy as _}; +use tokio::time::Instant; + +struct Limits { + attempts: u32, + window: Duration, + minimum_backoff: Duration, + maximum_backoff: Duration, +} + +enum Delay { + Wait(Duration), + Stop, +} + +/// A streaming body cannot be copied for another request attempt. +#[derive(Debug)] +struct NonReplayableBody; + +/// Clone a replayable request without manufacturing a transport error for a non-replayable body. +fn clone_request(request: &Request) -> Result { + request.try_clone().ok_or(NonReplayableBody) +} + +/// Retry temporary request failures while preserving the final HTTP/error outcome on exhaustion. +pub(crate) async fn execute(client: &Client, request: Request) -> reqwest::Result { + run( + client, + request, + Limits { + attempts: 32, + window: Duration::from_secs(90), + minimum_backoff: Duration::from_millis(250), + maximum_backoff: Duration::from_secs(5), + }, + ) + .await +} + +async fn run(client: &Client, request: Request, limits: Limits) -> reqwest::Result { + if !matches!( + *request.method(), + Method::GET | Method::HEAD | Method::PUT | Method::DELETE | Method::OPTIONS + ) { + return client.execute(request).await; + } + // Streaming bodies cannot be replayed. The original request must still be sent exactly once. + let Ok(first) = clone_request(&request) else { + return client.execute(request).await; + }; + let deadline = Instant::now() + limits.window; + let started = SystemTime::now(); + let policy = ExponentialBackoff::builder() + .retry_bounds(limits.minimum_backoff, limits.maximum_backoff) + .build_with_total_retry_duration(limits.window); + // The first attempt retains the caller's HTTP timeout. Subsequent attempts are also bounded by the window. + let mut outcome = client.execute(first).await; + let mut attempts = 1; + while attempts < limits.attempts && retryable(&outcome) { + let Delay::Wait(wait) = next_delay(&policy, started, attempts, &outcome, deadline) else { + break; + }; + let method = request.method(); + let http_status = outcome.as_ref().map_or(0, |response| response.status().as_u16()); + let retry_after_seconds = wait.as_secs_f64(); + tracing::warn!( + method = %method, + http_status, + attempt = attempts, + retry_after_seconds, + "retrying replayable REST request" + ); + tokio::time::sleep(wait).await; + if Instant::now() >= deadline { + break; + } + let Ok(next) = clone_request(&request) else { + // The immutable request already cloned successfully. + // Retain the last failure if future body behaviour prevents replay; clone failure is tested in the helper. + // skipcq: TCV-001 + break; + }; + attempts += 1; + match tokio::time::timeout_at(deadline, client.execute(next)).await { + Ok(result) => outcome = result, + // Retain the previous concrete failure rather than inventing a successful/empty response. + Err(_) => break, + } + } + outcome +} + +/// Combine policy backoff and Retry-After without scheduling an attempt beyond the retry window. +fn next_delay( + policy: &ExponentialBackoffTimed, + started: SystemTime, + attempts: u32, + outcome: &reqwest::Result, + deadline: Instant, +) -> Delay { + let RetryDecision::Retry { execute_after } = policy.should_retry(started, attempts - 1) else { + return Delay::Stop; + }; + let fallback = execute_after.duration_since(SystemTime::now()).unwrap_or_default(); + let Delay::Wait(wait) = retry_delay(outcome, fallback) else { + return Delay::Stop; + }; + if wait >= deadline.saturating_duration_since(Instant::now()) { + return Delay::Stop; + } + Delay::Wait(wait) +} + +fn retryable(outcome: &reqwest::Result) -> bool { + match outcome { + Ok(response) => matches!(response.status().as_u16(), 408 | 429 | 500 | 502 | 503 | 504), + Err(error) => error.is_timeout() || error.is_connect() || error.is_request() || error.is_body(), + } +} + +fn retry_delay(outcome: &reqwest::Result, fallback: Duration) -> Delay { + let Ok(response) = outcome else { + return Delay::Wait(fallback); + }; + // An absent Retry-After header is normal. An unsupported/malformed value conservatively prevents retry. + let Some(value) = response.headers().get(reqwest::header::RETRY_AFTER) else { + return Delay::Wait(fallback); + }; + let Ok(text) = value.to_str() else { + return Delay::Stop; + }; + let Ok(seconds) = text.parse::() else { + return Delay::Stop; + }; + Delay::Wait(fallback.max(Duration::from_secs(seconds))) +} + +#[cfg(test)] +// The compiler resolves this test module against this source file, independently of runtime working directories. +// nosemgrep: no-semgrep-mentions, no-ast-grep-mentions -- Required compile-time path annotation. +// ast-grep-ignore: no-relative-path-literal +#[path = "../tests/unit/retry.rs"] +mod tests; diff --git a/generated/rust-rest/tests/unit/retry.rs b/generated/rust-rest/tests/unit/retry.rs new file mode 100644 index 00000000..5c3d05be --- /dev/null +++ b/generated/rust-rest/tests/unit/retry.rs @@ -0,0 +1,372 @@ +//! Retry policy and HTTP request-capture fixtures preserve payloads and failure outcomes. +#![cfg(test)] + +use std::collections::BTreeMap; +use std::error::Error; +use std::io::{Seek as _, Write as _}; +use std::sync::{Arc, Mutex}; +use std::time::Duration; + +use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _}; +use tokio::net::{TcpListener, TcpStream}; +use tracing::instrument::WithSubscriber as _; +use tracing_subscriber::layer::SubscriberExt as _; + +use super::{Limits, clone_request, run}; + +type TestError = Box; +type ServerTask = tokio::task::JoinHandle>, TestError>>; + +/// Create a genuine non-replayable file body without depending on source paths or leaving files behind. +fn streamed_body(bytes: &[u8]) -> Result { + let mut file = tempfile::tempfile()?; + file.write_all(bytes)?; + file.rewind()?; + Ok(tokio::fs::File::from_std(file).into()) +} + +/// Capture retry diagnostics and signal exactly when the request enters its backoff wait. +#[derive(Clone, Default)] +struct RetryLog { + events: Arc>>>, + waiting: Arc, +} + +impl tracing_subscriber::Layer for RetryLog { + fn on_event(&self, event: &tracing::Event<'_>, _context: tracing_subscriber::layer::Context<'_, S>) { + if event.metadata().level() != &tracing::Level::WARN || !event.metadata().target().ends_with("::retry") { + return; + } + let mut fields = BTreeMap::new(); + event.record(&mut |field: &tracing::field::Field, value: &dyn std::fmt::Debug| { + fields.insert(field.name().to_owned(), format!("{value:?}")); + }); + self.events.lock().expect("retry log mutex poisoned").push(fields); + self.waiting.notify_one(); + } +} + +/// Retry diagnostics identify the operation and delay without exposing its URL, credentials or payload. +#[tokio::test] +async fn retry_logs_safe_operation_fields() -> Result<(), TestError> { + let (url, task) = server(vec!["408 Request Timeout", "200 OK"], "").await?; + let client = reqwest::Client::new(); + let request = client + .put(format!("{url}/private-object?token=secret")) + .body("private payload") + .build()?; + let log = RetryLog::default(); + let response = run(&client, request, limits(4)) + .with_subscriber(tracing_subscriber::registry().with(log.clone())) + .await?; + assert_eq!(response.status(), reqwest::StatusCode::OK); + assert_eq!(task.await??.len(), 2); + let events = log.events.lock().expect("retry log mutex poisoned"); + assert_eq!(events.len(), 1); + let fields = &events[0]; + assert_eq!(fields["method"], "PUT"); + assert_eq!(fields["http_status"], "408"); + assert_eq!(fields["attempt"], "1"); + assert!(fields["retry_after_seconds"].parse::()? >= 0.0); + assert!(fields["message"].contains("retrying replayable REST request")); + assert_eq!(fields.len(), 5); + assert!(!format!("{fields:?}").contains("private")); + assert!(!format!("{fields:?}").contains("secret")); + Ok(()) +} + +/// A task resumed after its backoff deadline returns the original failure without sending another request. +#[tokio::test] +async fn deadline_expiring_during_backoff_prevents_replay() -> Result<(), TestError> { + let (url, task) = server(vec!["408 Request Timeout"], "").await?; + let client = reqwest::Client::new(); + let log = RetryLog::default(); + let waiting = Arc::clone(&log.waiting); + let clock = tokio::spawn(async move { + waiting.notified().await; + // Advance only after the real HTTP response schedules a retry, avoiding simulated network timeouts. + tokio::time::pause(); + tokio::time::advance(Duration::from_secs(3)).await; + }); + let response = run(&client, client.get(url).build()?, limits(4)) + .with_subscriber(tracing_subscriber::registry().with(log.clone())) + .await?; + clock.await?; + assert_eq!(response.status(), reqwest::StatusCode::REQUEST_TIMEOUT); + assert_eq!(task.await??.len(), 1); + assert_eq!(log.events.lock().expect("retry log mutex poisoned").len(), 1); + Ok(()) +} + +fn limits(attempts: u32) -> Limits { + Limits { + attempts, + window: Duration::from_secs(2), + minimum_backoff: Duration::from_millis(1), + maximum_backoff: Duration::from_millis(2), + } +} + +async fn server(statuses: Vec<&'static str>, header: &'static str) -> Result<(String, ServerTask), TestError> { + let listener = TcpListener::bind("127.0.0.1:0").await?; + let url = format!("http://{}", listener.local_addr()?); + let task = tokio::spawn(async move { + let mut bodies = Vec::new(); + for status in statuses { + let (mut socket, _) = listener.accept().await?; + bodies.push(request_body(&mut socket).await?); + if status == "DROP" { + continue; + } + let status = if status == "SLOW" { + tokio::time::sleep(Duration::from_millis(25)).await; + "503 Service Unavailable" + } else { + status + }; + if status == "HANG" { + tokio::time::sleep(Duration::from_secs(60)).await; + break; + } + socket + .write_all( + format!("HTTP/1.1 {status}\r\nContent-Length: 0\r\nConnection: close\r\n{header}\r\n").as_bytes(), + ) + .await?; + } + Ok(bodies) + }); + Ok((url, task)) +} + +/// Read one complete buffered HTTP request so tests can compare the payload across retries. +async fn request_body(socket: &mut TcpStream) -> Result, TestError> { + let mut input = Vec::new(); + let mut buffer = [0; 4096]; + let boundary = loop { + let read = socket.read(&mut buffer).await?; + if read == 0 { + return Err("request ended before headers".into()); + } + input.extend_from_slice(&buffer[..read]); + if let Some(index) = input.windows(4).position(|bytes| bytes == b"\r\n\r\n") { + break index + 4; + } + }; + let headers = String::from_utf8_lossy(&input[..boundary]).to_ascii_lowercase(); + let length = headers + .lines() + .find_map(|line| line.strip_prefix("content-length:")) + .unwrap_or("0") + .trim() + .parse::()?; + while input.len() < boundary + length { + let read = socket.read(&mut buffer).await?; + if read == 0 { + return Err("request ended before body".into()); + } + input.extend_from_slice(&buffer[..read]); + } + Ok(input[boundary..boundary + length].to_vec()) +} + +/// The fixture must reject truncated uploads rather than report a successful payload comparison. +#[tokio::test] +async fn request_capture_rejects_incomplete_or_invalid_requests() -> Result<(), TestError> { + for (request, expected) in [ + (b"PUT / HTTP/1.1\r\nContent-Length: 4\r\n".as_slice(), "before headers"), + ( + b"PUT / HTTP/1.1\r\nContent-Length: 4\r\n\r\nab".as_slice(), + "before body", + ), + ( + b"PUT / HTTP/1.1\r\nContent-Length: bad\r\n\r\n".as_slice(), + "invalid digit", + ), + ] { + let listener = TcpListener::bind("127.0.0.1:0").await?; + let address = listener.local_addr()?; + let capture = tokio::spawn(async move { + let (mut socket, _) = listener.accept().await?; + request_body(&mut socket).await + }); + let mut client = TcpStream::connect(address).await?; + client.write_all(request).await?; + client.shutdown().await?; + let error = tokio::time::timeout(Duration::from_secs(2), capture) + .await?? + .unwrap_err(); + assert!(error.to_string().contains(expected), "{error}"); + } + Ok(()) +} + +/// A checkpoint upload spanning several socket reads is replayed without losing or duplicating bytes. +#[tokio::test] +async fn buffered_put_replays_identical_bytes_after_http_408() -> Result<(), TestError> { + let (url, task) = server(vec!["408 Request Timeout", "200 OK"], "").await?; + let client = reqwest::Client::new(); + let payload = vec![0x5a; 64 * 1024]; + let request = client.put(url).body(payload.clone()).build()?; + let response = run(&client, request, limits(4)).await?; + assert_eq!(response.status(), reqwest::StatusCode::OK); + assert_eq!(task.await??, vec![payload.clone(), payload]); + Ok(()) +} + +/// A non-replayable file stream is sent once and preserves HTTP 408 without attempting to reuse its consumed body. +#[tokio::test] +async fn streaming_put_is_sent_once_without_retrying_http_408() -> Result<(), TestError> { + let expected = b"a file-backed update must be sent exactly once"; + let (url, task) = server(vec!["408 Request Timeout"], "").await?; + let client = reqwest::Client::new(); + let request = client + .put(url) + .header(reqwest::header::CONTENT_LENGTH, expected.len().to_string()) + .body(streamed_body(expected)?) + .build()?; + assert!( + request.try_clone().is_none(), + "the fixture must exercise a non-replayable body" + ); + let response = run(&client, request, limits(4)).await?; + assert_eq!(response.status(), reqwest::StatusCode::REQUEST_TIMEOUT); + assert_eq!(task.await??, vec![expected.to_vec()]); + Ok(()) +} + +/// Cloning retains a buffered operation's identity and payload, while a real file stream is explicitly rejected. +#[tokio::test] +async fn request_clone_preserves_buffered_requests_and_rejects_streams() -> Result<(), TestError> { + let client = reqwest::Client::new(); + let buffered = client + .put("http://127.0.0.1/immutable-update") + .header("x-update-id", "same-update") + .body("immutable payload") + .build()?; + let cloned = clone_request(&buffered).expect("buffered bodies must remain replayable"); + assert_eq!(cloned.method(), buffered.method()); + assert_eq!(cloned.url(), buffered.url()); + assert_eq!(cloned.headers(), buffered.headers()); + assert_eq!(cloned.body().unwrap().as_bytes(), Some(b"immutable payload".as_slice())); + let streamed = client + .put(buffered.url().clone()) + .body(streamed_body(b"streamed update")?) + .build()?; + assert!(clone_request(&streamed).is_err()); + Ok(()) +} + +#[tokio::test] +async fn permanent_error_and_post_are_not_retried() -> Result<(), TestError> { + for (method, status) in [ + (reqwest::Method::PUT, "403 Forbidden"), + (reqwest::Method::POST, "503 Service Unavailable"), + ] { + let (url, task) = server(vec![status], "").await?; + let client = reqwest::Client::new(); + let request = client.request(method, url).body("one operation").build()?; + let response = run(&client, request, limits(4)).await?; + assert!(response.status().is_client_error() || response.status().is_server_error()); + assert_eq!(task.await??.len(), 1); + } + Ok(()) +} + +#[tokio::test] +async fn exhaustion_returns_last_http_failure() -> Result<(), TestError> { + let (url, task) = server(vec!["503 Service Unavailable"; 3], "").await?; + let client = reqwest::Client::new(); + let response = run(&client, client.get(url).build()?, limits(3)).await?; + assert_eq!(response.status(), reqwest::StatusCode::SERVICE_UNAVAILABLE); + assert_eq!(task.await??.len(), 3); + Ok(()) +} + +#[tokio::test] +async fn retry_after_beyond_budget_preserves_response() -> Result<(), TestError> { + let (url, task) = server(vec!["429 Too Many Requests"], "Retry-After: 60\r\n").await?; + let client = reqwest::Client::new(); + let response = run(&client, client.get(url).build()?, limits(4)).await?; + assert_eq!(response.status(), reqwest::StatusCode::TOO_MANY_REQUESTS); + assert_eq!(task.await??.len(), 1); + Ok(()) +} + +/// A lost response logs the absent HTTP status and retries the same buffered update without changing its payload. +#[tokio::test] +async fn a_connection_closed_before_response_retries_the_same_payload() -> Result<(), TestError> { + let (url, task) = server(vec!["DROP", "200 OK"], "").await?; + let client = reqwest::Client::new(); + let payload = b"immutable update after a transport error".to_vec(); + let request = client.put(url).body(payload.clone()).build()?; + let log = RetryLog::default(); + let response = run(&client, request, limits(4)) + .with_subscriber(tracing_subscriber::registry().with(log.clone())) + .await?; + assert_eq!(response.status(), reqwest::StatusCode::OK); + assert_eq!(task.await??, vec![payload.clone(), payload]); + let events = log.events.lock().expect("retry log mutex poisoned"); + assert_eq!(events.len(), 1); + assert_eq!(events[0]["http_status"], "0"); + assert_eq!(events[0]["method"], "PUT"); + Ok(()) +} + +/// Unsupported server delays stop replay, including non-ASCII values and integers outside the supported range. +#[tokio::test] +async fn malformed_retry_after_headers_preserve_the_response_without_replay() -> Result<(), TestError> { + for header in [ + "Retry-After: tomorrow\r\n", + "Retry-After: \u{e9}\r\n", + "Retry-After: 18446744073709551616\r\n", + ] { + let (url, task) = server(vec!["503 Service Unavailable"], header).await?; + let client = reqwest::Client::new(); + let response = run(&client, client.get(url).build()?, limits(4)).await?; + assert_eq!(response.status(), reqwest::StatusCode::SERVICE_UNAVAILABLE); + assert_eq!(task.await??.len(), 1); + } + Ok(()) +} + +/// The first request retains its own timeout, but a slow failure cannot start retries after the budget expires. +#[tokio::test] +async fn a_slow_first_attempt_does_not_extend_the_retry_window() -> Result<(), TestError> { + let (url, task) = server(vec!["SLOW"], "").await?; + let client = reqwest::Client::builder().timeout(Duration::from_secs(2)).build()?; + let budget = Limits { + window: Duration::from_millis(1), + ..limits(4) + }; + let response = run(&client, client.get(url).build()?, budget).await?; + assert_eq!(response.status(), reqwest::StatusCode::SERVICE_UNAVAILABLE); + assert_eq!(task.await??.len(), 1); + Ok(()) +} + +#[tokio::test] +async fn generated_storage_operation_uses_the_shared_policy() -> Result<(), TestError> { + let (url, task) = server(vec!["408 Request Timeout", "200 OK"], "").await?; + let client = crate::Client::new(&url); + let payload = b"same immutable update".to_vec(); + client.put_file("agent", "update.pt", payload.clone()).await?; + assert_eq!(task.await??, vec![payload.clone(), payload]); + Ok(()) +} + +#[tokio::test] +async fn a_stalled_retry_stops_at_the_window_and_preserves_failure() -> Result<(), TestError> { + let (url, task) = server(vec!["408 Request Timeout", "HANG"], "").await?; + let client = reqwest::Client::new(); + let budget = Limits { + window: Duration::from_millis(200), + ..limits(4) + }; + let response = + tokio::time::timeout(Duration::from_secs(2), run(&client, client.get(url).build()?, budget)).await??; + assert_eq!(response.status(), reqwest::StatusCode::REQUEST_TIMEOUT); + task.abort(); + let _result = task.await; + Ok(()) +} diff --git a/services/recovery-supervisor/Cargo.toml b/services/recovery-supervisor/Cargo.toml index 30ee3f1f..fe30ab39 100644 --- a/services/recovery-supervisor/Cargo.toml +++ b/services/recovery-supervisor/Cargo.toml @@ -8,6 +8,12 @@ edition.workspace = true license.workspace = true repository.workspace = true +# The compiler links this private unit-test module through src/policy.rs's #[path] attribute. +# cargo-shear reports it as unlinked even though cargo test --lib executes its tests; keep the exception limited +# to that exact file. +[package.metadata.cargo-shear] +ignored-paths = ["tests/unit/policy_guards.rs"] + [package.metadata.docs.rs] all-features = true diff --git a/services/recovery-supervisor/HELP.md b/services/recovery-supervisor/HELP.md index c47faf8b..ff3849a3 100644 --- a/services/recovery-supervisor/HELP.md +++ b/services/recovery-supervisor/HELP.md @@ -76,6 +76,12 @@ Type: integer. Required. Time between infrastructure observation passes. +### `RECOVERY_RESILIENCE` + +Type: string. Default: `{}`. + +Optional connection, dependency, coordination and stability safeguards. + ### `RECOVERY_STATE_FILE` Type: string. Required. diff --git a/services/recovery-supervisor/README.md b/services/recovery-supervisor/README.md index fb816b82..bd7361a9 100644 --- a/services/recovery-supervisor/README.md +++ b/services/recovery-supervisor/README.md @@ -28,6 +28,14 @@ recovery: stall_seconds: 180 deadline_seconds: 240 max_restarts: 2 + resilience: + max_concurrent_recoveries: 1 + action_interval_seconds: 5 + backoff_multiplier: 2 + jitter_seconds: 2 + stability_seconds: 10 + dependencies: + example-worker: [ws-server] workloads: - deployment: example-worker agent_id: example-worker @@ -52,7 +60,7 @@ need the SDK installed in their runner image; scenario generation does not insta Supervisor and supervised Deployments use `Recreate` replacement. The supervisor has one replica and persistent budget state; it is not a highly available controller. A different job cannot reuse an old job's state: archive the old report and deliberately provision fresh state before changing the job. Do not run a second recovery controller over the same -workloads. Hub or physical-node loss is not covered by this deployment integration. +workloads. The supervisor reconnects when a hub becomes available again; it does not repair hubs or physical nodes. ## Supervisor health @@ -75,8 +83,7 @@ Docker runs the probe every ten seconds, with a sixty-second startup grace and t unhealthy container. Adjust the container's startup grace for unusually large workload sets. Generated k3s deployments use the same executable for startup, readiness and liveness. Their startup budget includes registration and a bounded cycle; after startup, three failed liveness probes can restart the supervisor. Kubernetes does not inherit Docker's -health check. Such a restart preserves the durable action budget but begins a new observer session, not a continuation -of its incident history. +health check. A replacement restores recovery context, retaining an explicit gap in observations. ## Standalone example @@ -176,7 +183,7 @@ operations: training rounds, completed batches, or completed inference tasks, fo The exact configured sender, module source, and job must match. The first observed instance can be adopted mid-run, with its existing counter establishing a baseline rather than proving recovery. Replacement instances announce -`started` first; replayed sequence numbers and retired instances are rejected within the observer session. These +`started` first; replayed sequence numbers and retired instances are rejected across restored observer sessions. These bindings reduce accidental cross-talk, but hub agent IDs and CloudEvent sources are not authenticated identities. Use controller-assisted mode only on a trusted hub/network. This is not malicious-node detection or a security boundary against another hub participant claiming an allowlisted agent ID. Kubernetes credentials belong to the supervisor, @@ -220,10 +227,58 @@ Incident detection and recovery timestamps are null until observed, rather than Action records contain a `kind` (`restart-reserved`, `restart-requested`, or `restart-failed`); failed actions also contain an `error` description. Checkpoint records retain their operation kind, instance, progress, size and duration. -Budget counters survive supervisor restarts. Lifecycle sequence tracking and incident histories belong to one observer -session and are not restored; archive reports before restarting if historical measurements are needed. A lost hub -connection stops the service and its actions rather than continuing to act on stale application evidence. Restart the -service using the same state file after connectivity returns; this first version does not reconnect its live session. +## Resilient supervision + +`resilience` is optional. Standalone deployments supply it as a JSON object in `RECOVERY_RESILIENCE`. +By default, lost hub connections are retried using Core's existing bounded connect/register backoff. Recovery actions +are suspended during disconnection. After an initial completed observation cycle, the local health check remains current +while reconnection is progressing; it does not claim the applications are healthy. Registration alone does not pass +startup health. Setting `reconnect: false` restores exit-on-disconnection behaviour. + +Reports retain incident baselines, application instance and sequence checks, consumed budgets, cooldowns and outstanding +recovery requests across supervisor replacement. State is bound to the complete configuration and job identity; +changing either requires deliberately provisioning new state. Legacy state retains its budget and archives its report +without mixing its observer clock into new measurements. Invalid or mismatched state fails closed. + +Within a process, time uses a monotonic clock. Across process replacement, elapsed time includes the wall-clock interval +since the last saved context. If `saved_at` is ahead of the system clock, startup fails closed and identifies that +timestamp. Correct the system clock before retrying; the state file is retained, and downtime is never replaced with +zero to bypass the check. Reports identify that clock basis and record gaps explicitly. +Active workloads must supply renewed lifecycle evidence after reconnection or restoration before further +interventions or verified recovery. Queued events still pass instance and sequence checks; their arrival does not +establish continuous health during the gap. Pending stability assessments become incomplete when observations are +unavailable. + +Dependencies are named Deployments in the same namespace. They receive read-only permissions and are not automatically +restarted. A required dependency that is unavailable or unknown withholds consumer interventions, including transitively +through the configured acyclic graph. This waiting consumes the original recovery deadline. No application event can +add a dependency or authorize a new target. + +Fleet coordination counts outstanding useful-recovery requests until recovery, command failure or deadline expiry. +`max_concurrent_recoveries: 0` and `action_interval_seconds: 0` disable the extra fleet limits. An existing recovery may +retry within its own slot. Failed commands release their slot for other workloads. Retries remain subject to cooldown, +backoff, fleet limits and the workload's durable budget; every attempted restart consumes budget. Limits are checked +again when an action is reserved. + +The first intervention retains the configured detection delay. Subsequent cooldowns use `backoff_multiplier`, +bounded by `backoff_max_seconds`, with up to `jitter_seconds` added variation. The original cooldown is never shortened. +Defaults of multiplier one and zero jitter preserve fixed spacing. Delays are not subtracted from recovery durations +and cannot extend the recovery deadline or reset consumed attempts. + +With `stability_seconds` enabled, first useful recovery is followed by a separate healthy observation window and renewed +progress, or a completed-work indication. Recurrent failure or instance replacement makes the assessment unstable; +missing observations make it incomplete. Completed work is not required to run again. Stability never changes the +first-recovery timestamp or MTTR. Zero disables this additional assessment. + +`assessments` reports observed failure conditions and the policy reason for acting or waiting, including unavailable +dependencies, insufficient evidence, cooldown, fleet limits and exhausted budgets or deadlines. These are evidence-based +categories, not hardware root-cause diagnoses. No operator confirmation, external incident service or rollback is used. + +Recovery evidence uses the existing direct-message retention policy. With `latest-per-sender`, producers needing every +transition must wait for acknowledgements; a later unacknowledged event can otherwise replace an earlier one. The SDK's +`Lifecycle.emit` does not implement a durable outbox. `WS_MESSAGE_RETENTION=all` retains all unacknowledged transitions, +but has no backlog bound. Configure delivery and producer rate deliberately; reconnection cannot reconstruct discarded +events or promote missing evidence into verified recovery. ## Permissions and verification diff --git a/services/recovery-supervisor/src/config.rs b/services/recovery-supervisor/src/config.rs index 921d4fe4..a156b82d 100644 --- a/services/recovery-supervisor/src/config.rs +++ b/services/recovery-supervisor/src/config.rs @@ -83,6 +83,10 @@ pub struct Config { #[serde(deserialize_with = "deserialize_workloads")] #[cfg_attr(feature = "env-schema", schemars(with = "String"))] pub workloads: Vec, + /// Optional connection, dependency, coordination and stability safeguards. + #[serde(default, deserialize_with = "crate::resilience::deserialize")] + #[cfg_attr(feature = "env-schema", schemars(with = "String", extend("default" = "{}")))] + pub resilience: crate::resilience::Settings, } /// Environment configuration grouped under the `RECOVERY_*` prefix. @@ -150,6 +154,7 @@ impl Config { state_file: PathBuf::from("/app/state/recovery.json"), policy, workloads, + resilience: crate::resilience::Settings::default(), } } @@ -157,6 +162,7 @@ impl Config { pub fn validate(&self) -> Result<(), RecoveryError> { self.validate_identity()?; self.policy.validate()?; + self.resilience.validate(self)?; if self.poll_seconds == 0 || self.command_timeout_seconds == 0 || self.workloads.is_empty() { return Err(RecoveryError::Invalid( "positive polling and command deadlines and at least one workload are required".into(), diff --git a/services/recovery-supervisor/src/health.rs b/services/recovery-supervisor/src/health.rs index 6f99807d..c64f4b3c 100644 --- a/services/recovery-supervisor/src/health.rs +++ b/services/recovery-supervisor/src/health.rs @@ -25,10 +25,11 @@ struct Evidence { #[derive(Debug)] pub struct Session { // This guard holds the exclusive budget lock until the supervisor exits, including cancelled futures. - _lease: fs::File, + lease: std::sync::Arc, store: StateStore, job_id: String, identity: String, + observed: std::sync::atomic::AtomicBool, } impl Session { @@ -42,18 +43,26 @@ impl Session { .write(true) .open(config.state_file.with_extension("lock"))?; lease.try_lock()?; + let lease = std::sync::Arc::new(lease); let identity = uuid::Uuid::new_v4().to_string(); // Windows mandatory locks reject reads through another handle with "os error 33". Keep the identity outside the // locked file, and replace it only after acquiring ownership. fs::write(identity_path(config), identity.as_bytes())?; Ok(Self { - _lease: lease, - store: StateStore::new(evidence_path(config)), + store: StateStore::owned(evidence_path(config), std::sync::Arc::clone(&lease)), + lease, job_id: config.job_id.clone(), identity, + observed: std::sync::atomic::AtomicBool::new(false), }) } + /// Bind a context writer to this session so a replacement cannot overtake an unfinished committed save. + #[must_use] + pub fn state_store(&self, path: PathBuf) -> StateStore { + StateStore::owned(path, std::sync::Arc::clone(&self.lease)) + } + /// Refresh evidence only when the policy loop has finished observing, acting, and persisting its report. pub async fn advance(&self) -> Result<(), RecoveryError> { self.store @@ -63,7 +72,17 @@ impl Session { session: self.identity.clone(), completed_at: SystemTime::now(), }) - .await + .await?; + self.observed.store(true, std::sync::atomic::Ordering::Release); + Ok(()) + } + + /// Keep an established observer alive during reconnection without making registration alone pass startup health. + pub async fn maintain(&self) -> Result<(), RecoveryError> { + if self.observed.load(std::sync::atomic::Ordering::Acquire) { + self.advance().await?; + } + Ok(()) } } @@ -72,9 +91,14 @@ impl Session { /// Includes polling, one observation and potential restart per workload, and ten seconds for persistence. #[must_use] pub fn silence_limit(config: &Config) -> Duration { - let commands = config.workloads.iter().fold(0_u64, |budget, _workload| { + let mut commands = config.workloads.iter().fold(0_u64, |budget, _workload| { budget.saturating_add(config.command_timeout_seconds.saturating_mul(2)) }); + for name in config.resilience.dependency_names() { + if !config.workloads.iter().any(|workload| workload.deployment == name) { + commands = commands.saturating_add(config.command_timeout_seconds); + } + } Duration::from_secs(config.poll_seconds.saturating_add(commands).saturating_add(10)) } diff --git a/services/recovery-supervisor/src/kubernetes.rs b/services/recovery-supervisor/src/kubernetes.rs index d8d12f61..0c145b0a 100644 --- a/services/recovery-supervisor/src/kubernetes.rs +++ b/services/recovery-supervisor/src/kubernetes.rs @@ -32,12 +32,14 @@ impl Kubernetes { /// Execute kubectl directly, without shell interpolation, bounding both the API request and child lifetime. pub async fn execute(&self, workload: &Workload, arguments: &[&str]) -> Result, RecoveryError> { - if !self + let supervised = self .config .workloads .iter() - .any(|item| item.deployment == workload.deployment) - { + .any(|item| item.deployment == workload.deployment); + let dependency_read = arguments == ["get", "-o", "json"] + && self.config.resilience.dependency_names().contains(&workload.deployment); + if !supervised && !dependency_read { return Err(RecoveryError::Invalid("Kubernetes target is not allowlisted".into())); } let target = format!("deployment/{}", workload.deployment); @@ -99,8 +101,13 @@ struct DeploymentStatus { /// Decode readiness without assuming that an HTTP-available process has resumed application work. pub fn deployment_health(bytes: &[u8]) -> Result { + readiness(bytes, true) +} + +/// Supporting deployments can have several replicas; application recovery still requires a single writer. +fn readiness(bytes: &[u8], single_writer: bool) -> Result { let deployment: Deployment = serde_json::from_slice(bytes)?; - if deployment.spec.replicas != 1 { + if single_writer && deployment.spec.replicas != 1 { return Err(RecoveryError::Invalid( "supervised deployments must have exactly one desired replica".into(), )); @@ -108,9 +115,10 @@ pub fn deployment_health(bytes: &[u8]) -> Result { let status = deployment.status; Ok( if status.observed_generation >= deployment.metadata.generation - && status.available_replicas == 1 - && status.updated_replicas == 1 - && status.replicas == 1 + && deployment.spec.replicas > 0 + && status.available_replicas == deployment.spec.replicas + && status.updated_replicas == deployment.spec.replicas + && status.replicas == deployment.spec.replicas && status.unavailable_replicas == 0 { Health::Ready @@ -123,7 +131,12 @@ pub fn deployment_health(bytes: &[u8]) -> Result { impl Backend for Kubernetes { /// Query and decode readiness for one allowlisted deployment. async fn health(&mut self, workload: &Workload) -> Result { - deployment_health(&self.execute(workload, &["get", "-o", "json"]).await?) + let single_writer = self + .config + .workloads + .iter() + .any(|item| item.deployment == workload.deployment); + readiness(&self.execute(workload, &["get", "-o", "json"]).await?, single_writer) } /// Request a bounded rollout restart for one allowlisted deployment. diff --git a/services/recovery-supervisor/src/lib.rs b/services/recovery-supervisor/src/lib.rs index 11846d5c..930e3e18 100644 --- a/services/recovery-supervisor/src/lib.rs +++ b/services/recovery-supervisor/src/lib.rs @@ -10,6 +10,7 @@ pub mod health; pub mod kubernetes; pub mod persistence; pub mod policy; +pub mod resilience; pub mod runtime; /// Errors preserve the failed operation instead of treating missing observations as healthy workloads. diff --git a/services/recovery-supervisor/src/persistence.rs b/services/recovery-supervisor/src/persistence.rs index 787c7d79..de36fccd 100644 --- a/services/recovery-supervisor/src/persistence.rs +++ b/services/recovery-supervisor/src/persistence.rs @@ -4,7 +4,6 @@ use std::io::Write as _; use std::path::{Path, PathBuf}; use std::sync::Arc; -#[cfg(unix)] use fs_err as fs; use serde::Serialize; use tokio::sync::Mutex; @@ -16,6 +15,13 @@ use crate::RecoveryError; pub struct StateStore { path: PathBuf, serial: Arc>, + ownership: Ownership, +} + +#[derive(Clone, Debug)] +enum Ownership { + Unbound, + Guard { _lease: Arc }, } impl StateStore { @@ -25,6 +31,16 @@ impl StateStore { Self { path, serial: Arc::new(Mutex::new(())), + ownership: Ownership::Unbound, + } + } + + /// Keep the supervisor's lease until blocking persistence completes, even when its async caller is cancelled. + #[must_use] + pub fn owned(path: PathBuf, lease: Arc) -> Self { + Self { + ownership: Ownership::Guard { _lease: lease }, + ..Self::new(path) } } @@ -36,8 +52,10 @@ impl StateStore { // The blocking task owns the permit: cancelling its waiter cannot let a later save overtake this write. let permit = Arc::clone(&self.serial).lock_owned().await; let path = self.path.clone(); + let ownership = self.ownership.clone(); tokio::task::spawn_blocking(move || { let _permit = permit; + let _ownership = ownership; persist(&path, &value) }) .await? diff --git a/services/recovery-supervisor/src/policy.rs b/services/recovery-supervisor/src/policy.rs index 7da8531f..25624c7e 100644 --- a/services/recovery-supervisor/src/policy.rs +++ b/services/recovery-supervisor/src/policy.rs @@ -2,15 +2,24 @@ use std::collections::{BTreeMap, BTreeSet}; use std::time::Duration; +use std::time::SystemTime; use edge_toolkit::ws::CloudEvent; use serde::{Deserialize, Serialize}; +#[cfg(test)] +// Compile-time inclusion keeps private guard tests under tests/ without exposing policy internals. +// nosemgrep: no-semgrep-mentions, no-ast-grep-mentions -- Required compile-time path annotation. +// ast-grep-ignore: no-relative-path-literal +#[path = "../tests/unit/policy_guards.rs"] +mod guard_tests; + use crate::RecoveryError; use crate::config::{Config, Mode, Workload}; +use crate::resilience::{Assessment, Classification, ObservationGap, Reason, Stability}; /// Infrastructure observations distinguish API failure from an unhealthy deployment. -#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] #[non_exhaustive] pub enum Health { /// The desired single replica is available at its current generation. @@ -111,7 +120,7 @@ pub struct CheckpointMeasurement { } /// An unavailable measurement must not be interpreted as a zero-duration success. -#[derive(Clone, Copy, Debug, Serialize)] +#[derive(Clone, Copy, Debug, Deserialize, Serialize)] #[non_exhaustive] #[serde(untagged)] pub enum MeanSeconds { @@ -122,7 +131,7 @@ pub enum MeanSeconds { } /// Owned report snapshot, safe to serialize off the async control task. -#[derive(Clone, Debug, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize)] #[non_exhaustive] pub struct Report { /// Persisted report format version. @@ -132,7 +141,7 @@ pub struct Report { /// Whether external recovery actions are permitted. pub mode: Mode, /// Time coordinate used for incident observations. - pub clock_basis: &'static str, + pub clock_basis: String, /// Incidents recorded during this observer session. pub incidents: Vec, /// Failed infrastructure observations, distinct from workload failures. @@ -153,6 +162,12 @@ pub struct Report { pub detected_incidents: usize, /// Application-reported checkpoint operations. pub checkpoint_measurements: Vec, + /// Latest condition and policy disposition per configured workload. + pub assessments: BTreeMap, + /// Periods without supervision, excluded from claims of continuous healthy observation. + pub observation_gaps: Vec, + /// Versioned restoration context owned by this supervisor, not an application checkpoint. + pub context: Snapshot, } /// Incident times are elapsed observer seconds within one supervisor process, not producer timestamps. @@ -173,16 +188,19 @@ pub struct Incident { pub status: Status, /// Executed action outcomes, distinct from Kubernetes' own process recovery. pub actions: Vec, + /// Sustained health after first useful recovery; does not change the recovery timestamp. + #[serde(default)] + pub stability: Stability, } -#[derive(Debug, Default)] +#[derive(Clone, Debug, Default, Deserialize, Serialize)] enum Phase { #[default] Healthy, Incident(usize), } -#[derive(Debug, Default)] +#[derive(Clone, Debug, Default, Deserialize, Serialize)] struct State { active: bool, failed: bool, @@ -194,6 +212,24 @@ struct State { sequence: u64, phase: Phase, last_action: u64, + fresh: bool, + in_flight: bool, +} + +/// Durable context is validated against the exact operator configuration before it can authorize actions. +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +#[non_exhaustive] +pub struct Snapshot { + schema_version: u32, + configuration: Config, + states: BTreeMap, + elapsed_seconds: u64, + saved_at: SystemTime, + last_reservation: ObservationTime, + jitter_seed: u64, + restored_sessions: u64, + archived_reports: Vec, } /// Durable action budget. An attempted restart consumes budget even if the backend fails or the process exits. @@ -233,6 +269,16 @@ pub struct Supervisor { /// API failures counted separately from workload failures. unknown_observations: u64, checkpoints: Vec, + assessments: BTreeMap, + dependency_health: BTreeMap, + gaps: Vec, + observed_at: u64, + time_offset: u64, + last_reservation: ObservationTime, + jitter_seed: u64, + restored_sessions: u64, + archived_reports: Vec, + connected: bool, } #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -291,9 +337,148 @@ impl Supervisor { budget, unknown_observations: 0, checkpoints: Vec::new(), + assessments: BTreeMap::new(), + dependency_health: BTreeMap::new(), + gaps: Vec::new(), + observed_at: 0, + time_offset: 0, + last_reservation: ObservationTime::Pending, + jitter_seed: uuid::Uuid::now_v7().as_u64_pair().1, + restored_sessions: 0, + archived_reports: Vec::new(), + connected: true, }) } + /// Restore committed policy state; legacy reports retain their budget and are archived without mixing clocks. + pub fn restore(config: Config, value: serde_json::Value) -> Result { + let budget = serde_json::from_value( + value + .get("restart_budget") + .cloned() + .ok_or_else(|| RecoveryError::Invalid("state document has no restart budget".into()))?, + )?; + let mut supervisor = Self::new(config, budget)?; + if value.get("context").is_none() { + supervisor.archived_reports.push(value); + return Ok(supervisor); + } + let report: Report = serde_json::from_value(value)?; + supervisor.validate_context(&report)?; + let snapshot = report.context; + let Ok(downtime) = SystemTime::now().duration_since(snapshot.saved_at) else { + return Err(RecoveryError::Invalid(format!( + "future recovery saved_at {:?}; correct the system clock before retrying", + snapshot.saved_at + ))); + }; + let downtime = downtime.as_secs(); + supervisor.states = snapshot.states; + supervisor.incidents = report.incidents; + supervisor.checkpoints = report.checkpoint_measurements; + supervisor.unknown_observations = report.unknown_observations; + supervisor.assessments = report.assessments; + supervisor.gaps = report.observation_gaps; + supervisor.time_offset = snapshot.elapsed_seconds.saturating_add(downtime); + supervisor.observed_at = supervisor.time_offset; + supervisor.last_reservation = snapshot.last_reservation; + supervisor.jitter_seed = snapshot.jitter_seed; + supervisor.restored_sessions = snapshot.restored_sessions.saturating_add(1); + supervisor.archived_reports = snapshot.archived_reports; + supervisor.transport_lost(snapshot.elapsed_seconds, "supervisor restart"); + supervisor.transport_connected(supervisor.time_offset); + Ok(supervisor) + } + + /// Reject saved state from a different configuration before restoring any live policy state. + fn validate_context(&self, report: &Report) -> Result<(), RecoveryError> { + let snapshot = &report.context; + if report.schema_version != 1 + || snapshot.schema_version != 1 + || report.job_id != self.config.job_id + || report.mode != self.config.mode + || serde_json::to_value(&snapshot.configuration)? != serde_json::to_value(&self.config)? + || snapshot.states.keys().ne(self.states.keys()) + { + return Err(RecoveryError::Invalid( + "saved recovery context does not match configuration".into(), + )); + } + Self::validate_saved_incidents(report) + } + + /// Verify that each restored workload phase refers to its own recorded incident. + #[expect( + clippy::single_call_fn, + reason = "incident references are a distinct persisted-state invariant" + )] + fn validate_saved_incidents(report: &Report) -> Result<(), RecoveryError> { + for (name, state) in &report.context.states { + if let Phase::Incident(index) = state.phase { + let incident = report + .incidents + .get(index) + .ok_or_else(|| RecoveryError::Invalid("saved incident index is invalid".into()))?; + if &incident.workload != name { + return Err(RecoveryError::Invalid("saved incident workload is inconsistent".into())); + } + } + } + Ok(()) + } + + /// Advance the job clock monotonically within a process, retaining measured downtime across replacement. + #[must_use] + pub const fn elapsed(&self, session_seconds: u64) -> u64 { + self.time_offset.saturating_add(session_seconds) + } + + /// Suspend actions while retaining budgets, incident baselines and accepted sequence numbers. + pub fn transport_lost(&mut self, now: u64, reason: &str) { + self.observed_at = self.observed_at.max(now); + self.connected = false; + if self + .gaps + .last() + .is_none_or(|gap| gap.ended_at != ObservationTime::Pending) + { + self.gaps.push(ObservationGap { + started_at: now, + ended_at: ObservationTime::Pending, + reason: reason.into(), + }); + } + for state in self.states.values_mut() { + state.fresh = false; + } + for incident in &mut self.incidents { + if matches!(incident.stability, Stability::Pending { .. }) { + incident.stability = Stability::Incomplete; + } + } + self.dependency_health.clear(); + } + + /// Resume observation after registration; active workloads must supply renewed lifecycle evidence before action. + pub fn transport_connected(&mut self, now: u64) { + self.observed_at = self.observed_at.max(now); + self.connected = true; + if let Some(gap) = self.gaps.last_mut() { + gap.ended_at = ObservationTime::Recorded(now); + } + } + + /// Refresh the read-only dependency snapshot before evaluating any workload in an observation cycle. + pub fn dependency_observation(&mut self, name: &str, health: Health) -> Result<(), RecoveryError> { + if !self.config.resilience.dependency_names().contains(name) { + return Err(RecoveryError::Invalid( + "dependency observation is not configured".into(), + )); + } + let _previous = self.dependency_health.insert(name.into(), health); + Ok(()) + } + /// Consume an allowlisted sender's lifecycle `CloudEvent`, rejecting invalid, replayed, or foreign evidence. pub fn event(&mut self, sender: &str, event: &CloudEvent, now: u64) -> Result<(), RecoveryError> { let workload = self @@ -308,6 +493,8 @@ impl Supervisor { .get_mut(&workload.deployment) .ok_or_else(|| RecoveryError::Invalid("workload state is absent".into()))?; state.accept_evidence(kind, &evidence, &mut self.incidents, now)?; + state.fresh = true; + self.observed_at = self.observed_at.max(now); if let EventKind::Checkpoint(kind) = kind { self.checkpoints.push(CheckpointMeasurement { workload: workload.deployment.clone(), @@ -324,30 +511,33 @@ impl Supervisor { /// Observe one workload at monotonic elapsed seconds. Readiness alone never closes a detected incident. pub fn observe(&mut self, workload: &Workload, health: Health, now: u64) -> Result { + self.observed_at = self.observed_at.max(now); + self.update_stability(workload, health, now)?; let state = self .states .get_mut(&workload.deployment) .ok_or_else(|| RecoveryError::Invalid("workload is not configured".into()))?; if health == Health::Unknown { self.unknown_observations = self.unknown_observations.saturating_add(1); + self.assess( + workload, + Classification::ObservationUnknown, + Reason::ObservationUnknown, + now, + ); return Ok(Decision::Observe); } // An idle module is not stalled, and completing a job does not create another work obligation. let unhealthy = state.unhealthy(health, &self.config.policy, now); + let classification = state.classify(health, unhealthy); if matches!(state.phase, Phase::Healthy) && unhealthy { let index = self.incidents.len(); - self.incidents.push(Incident { - workload: workload.deployment.clone(), - onset: now, - detected: ObservationTime::Pending, - recovered: ObservationTime::Pending, - baseline: state.progress, - status: Status::Suspected, - actions: Vec::new(), - }); + self.incidents + .push(Incident::new(workload.deployment.clone(), now, state.progress)); state.phase = Phase::Incident(index); } let Phase::Incident(index) = state.phase else { + self.assess(workload, classification, Reason::Healthy, now); return Ok(Decision::Observe); }; let incident = self @@ -355,17 +545,68 @@ impl Supervisor { .get_mut(index) .ok_or_else(|| RecoveryError::Invalid("incident state is absent".into()))?; state.advance(incident, health, unhealthy, &self.config.policy, now); - let attempts = self - .budget - .restarts + if matches!( + incident.status, + Status::Recovered | Status::Transient | Status::Unrecovered + ) { + state.in_flight = false; + } + if incident.status == Status::Recovered && self.config.resilience.stability_seconds > 0 { + incident.stability = Stability::Pending { + since: now, + progress: state.progress, + instance: state.instance.clone(), + }; + } + let reason = self.eligibility(workload, now)?; + self.assess(workload, classification, reason, now); + Ok(if reason == Reason::RestartEligible { + Decision::Restart + } else { + Decision::Observe + }) + } + + /// Update sustained recovery independently of incident detection and intervention eligibility. + fn update_stability(&mut self, workload: &Workload, health: Health, now: u64) -> Result<(), RecoveryError> { + let state = self + .states .get(&workload.deployment) - .copied() - .unwrap_or_default(); - Ok(state.decision(incident, &self.config, attempts, now)) + .ok_or_else(|| RecoveryError::Invalid("workload is not configured".into()))?; + let incident = self + .incidents + .iter_mut() + .rev() + .find(|item| item.workload == workload.deployment && item.status == Status::Recovered); + if let Some(incident) = incident { + if health == Health::Unknown && matches!(incident.stability, Stability::Pending { .. }) { + incident.stability = Stability::Incomplete; + } else { + state.verify_stability( + incident, + state.unhealthy(health, &self.config.policy, now), + now, + self.config.resilience.stability_seconds, + ); + } + } + Ok(()) } /// Reserve an action before persisting the budget and executing the backend operation. pub fn reserve_restart(&mut self, workload: &Workload, now: u64) -> Result<(), RecoveryError> { + if self.eligibility(workload, now)? != Reason::RestartEligible { + return Err(RecoveryError::Invalid( + "restart is not eligible under the configured policy".into(), + )); + } + self.reserve_eligible_restart(workload, now) + } + + /// Commit an admitted action while defending against missing or inconsistent reservation state. + fn reserve_eligible_restart(&mut self, workload: &Workload, now: u64) -> Result<(), RecoveryError> { + // Eligibility validated the target, phase and limits under this exclusive borrow. The following checks + // remain fail-closed invariant guards if that validation or the reservation sequence changes later. let state = self .states .get_mut(&workload.deployment) @@ -378,18 +619,40 @@ impl Supervisor { .incidents .get_mut(index) .ok_or_else(|| RecoveryError::Invalid("incident state is absent".into()))?; - if self.config.mode != Mode::ControllerAssisted - || *count >= self.config.policy.max_restarts - || now.saturating_sub(state.last_action) < self.config.policy.restart_after_seconds + Self::validate_reservation(&self.config, state, incident, *count, now)?; + *count = count.saturating_add(1); + state.last_action = now; + self.observed_at = self.observed_at.max(now); + state.in_flight = true; + self.last_reservation = ObservationTime::Recorded(now); + incident.actions.push(Action::RestartReserved); + Ok(()) + } + + /// Recheck reservation invariants before consuming budget or changing the incident's action history. + #[cfg_attr( + not(test), + expect( + clippy::single_call_fn, + reason = "independently tested fail-closed boundary before a restart reservation mutates state" + ) + )] + fn validate_reservation( + config: &Config, + state: &State, + incident: &Incident, + attempts: u32, + now: u64, + ) -> Result<(), RecoveryError> { + if config.mode != Mode::ControllerAssisted + || attempts >= config.policy.max_restarts + || now.saturating_sub(state.last_action) < config.policy.restart_after_seconds || incident.status != Status::Detected { return Err(RecoveryError::Invalid( "restart is not eligible under the configured policy".into(), )); } - *count = count.saturating_add(1); - state.last_action = now; - incident.actions.push(Action::RestartReserved); Ok(()) } @@ -397,19 +660,218 @@ impl Supervisor { pub fn action_result(&mut self, workload: &Workload, outcome: Action) -> Result<(), RecoveryError> { let state = self .states - .get(&workload.deployment) + .get_mut(&workload.deployment) .ok_or_else(|| RecoveryError::Invalid("action target is not configured".into()))?; let Phase::Incident(index) = state.phase else { return Err(RecoveryError::Invalid("action has no incident".into())); }; - self.incidents + let incident = self + .incidents .get_mut(index) - .ok_or_else(|| RecoveryError::Invalid("incident state is absent".into()))? - .actions - .push(outcome); + .ok_or_else(|| RecoveryError::Invalid("incident state is absent".into()))?; + if matches!(outcome, Action::RestartFailed { .. }) { + state.in_flight = false; + } + incident.actions.push(outcome); Ok(()) } + /// Evaluate safeguards again at reservation so a decision cannot bypass a newly consumed fleet limit. + fn eligibility(&self, workload: &Workload, now: u64) -> Result { + let state = self + .states + .get(&workload.deployment) + .ok_or_else(|| RecoveryError::Invalid("restart target is not configured".into()))?; + let Phase::Incident(index) = state.phase else { + return Ok(Reason::Healthy); + }; + let incident = self + .incidents + .get(index) + .ok_or_else(|| RecoveryError::Invalid("incident state is absent".into()))?; + let reason = self.incident_eligibility(state, incident, now); + if reason != Reason::Healthy { + return Ok(reason); + } + let dependency = self.dependency_reason(workload); + if dependency != Reason::Healthy { + return Ok(dependency); + } + Ok(self.restart_eligibility(workload, state, incident, now)) + } + + /// Require a detected incident with current evidence and time left for recovery. + fn incident_eligibility(&self, state: &State, incident: &Incident, now: u64) -> Reason { + if incident.status == Status::Unrecovered + || now.saturating_sub(incident.onset) >= self.config.policy.deadline_seconds + { + return Reason::DeadlineExceeded; + } + if incident.status != Status::Detected { + return Reason::DetectionGrace; + } + if !self.connected || (state.active && !state.completed && !state.fresh) { + return Reason::EvidenceRequired; + } + Reason::Healthy + } + + /// Apply intervention mode, per-workload limits and fleet coordination after evidence checks pass. + fn restart_eligibility(&self, workload: &Workload, state: &State, incident: &Incident, now: u64) -> Reason { + if self.config.mode != Mode::ControllerAssisted { + return Reason::ObserveOnly; + } + let attempts = self + .budget + .restarts + .get(&workload.deployment) + .copied() + .unwrap_or_default(); + if attempts >= self.config.policy.max_restarts { + return Reason::BudgetExhausted; + } + let cooldown = self.cooldown(incident); + if now.saturating_sub(state.last_action) < cooldown { + return if state.in_flight { + Reason::RecoveryInProgress + } else { + Reason::Cooldown + }; + } + self.fleet_eligibility(state, now) + } + + /// Count outstanding useful-recovery requests, not merely concurrent kubectl child processes. + fn fleet_eligibility(&self, state: &State, now: u64) -> Reason { + let active = self + .states + .values() + .filter(|candidate| { + let Phase::Incident(index) = candidate.phase else { + return false; + }; + candidate.in_flight + && self.incidents.get(index).is_some_and(|incident| { + // A restored terminal incident can retain a stale in-flight marker. + incident.status == Status::Detected + && now.saturating_sub(incident.onset) < self.config.policy.deadline_seconds + }) + }) + .count(); + if !state.in_flight + && self.config.resilience.max_concurrent_recoveries > 0 + && u32::try_from(active).unwrap_or(u32::MAX) >= self.config.resilience.max_concurrent_recoveries + { + return Reason::ConcurrencyLimit; + } + if let ObservationTime::Recorded(previous) = self.last_reservation + && now.saturating_sub(previous) < self.config.resilience.action_interval_seconds + { + return Reason::ActionInterval; + } + Reason::RestartEligible + } + + /// Unknown dependencies suppress interventions without being labelled as confirmed failures. + fn dependency_reason(&self, workload: &Workload) -> Reason { + self.dependency_condition(&workload.deployment) + } + + /// Evaluate the validated dependency graph transitively, using only this cycle's read-only observations. + fn dependency_condition(&self, name: &str) -> Reason { + let Some(dependencies) = self.config.resilience.dependencies.get(name) else { + return Reason::Healthy; + }; + let mut reason = Reason::Healthy; + for dependency in dependencies { + match self.dependency_health.get(dependency) { + Some(Health::Ready) => { + let nested = self.dependency_condition(dependency); + if nested == Reason::DependencyUnavailable { + return nested; + } + if nested == Reason::DependencyUnknown { + reason = nested; + } + if self + .states + .get(dependency) + .is_some_and(|state| state.unhealthy(Health::Ready, &self.config.policy, self.observed_at)) + { + return Reason::DependencyUnavailable; + } + } + Some(Health::Unready) => return Reason::DependencyUnavailable, + Some(Health::Unknown) | None => reason = Reason::DependencyUnknown, + } + } + reason + } + + /// Publish a bounded latest assessment without changing incident or first-recovery measurements. + fn assess(&mut self, workload: &Workload, mut classification: Classification, reason: Reason, now: u64) { + if reason == Reason::DependencyUnavailable { + classification = Classification::DependencyUnavailable; + } + let next_action_at = self.next_action_time(workload, reason); + let _previous = self.assessments.insert( + workload.deployment.clone(), + Assessment { + classification, + reason, + observed_at: now, + next_action_at, + dependencies: self + .config + .resilience + .dependencies + .get(&workload.deployment) + .cloned() + .unwrap_or_default(), + }, + ); + } + + /// Report a scheduled retry only when evidence permits it; dependency and capacity waits have no promised time. + fn next_action_time(&self, workload: &Workload, reason: Reason) -> ObservationTime { + if !matches!( + reason, + Reason::Cooldown | Reason::RecoveryInProgress | Reason::ActionInterval | Reason::RestartEligible + ) { + return ObservationTime::Pending; + } + // These reasons follow successful incident validation. Retain defensive fallbacks for future call-site + // changes rather than promising a retry time for inconsistent state. + let Some(state) = self.states.get(&workload.deployment) else { + return ObservationTime::Pending; + }; + let Phase::Incident(index) = state.phase else { + return ObservationTime::Pending; + }; + let Some(incident) = self.incidents.get(index) else { + return ObservationTime::Pending; + }; + let cooldown = self.cooldown(incident); + let mut next = state.last_action.saturating_add(cooldown); + if let ObservationTime::Recorded(previous) = self.last_reservation { + next = next.max(previous.saturating_add(self.config.resilience.action_interval_seconds)); + } + if next >= incident.onset.saturating_add(self.config.policy.deadline_seconds) { + return ObservationTime::Pending; + } + ObservationTime::Recorded(next) + } + + /// Use the same persisted jitter sample for eligibility and the reported retry schedule. + fn cooldown(&self, incident: &Incident) -> u64 { + self.config.resilience.cooldown( + self.config.policy.restart_after_seconds, + incident.attempts(), + &incident.workload, + self.jitter_seed ^ incident.onset, + ) + } + /// Serialize evidence with explicit unavailable measurements rather than zero-duration successful recoveries. #[must_use] pub fn report(&self) -> Report { @@ -433,7 +895,11 @@ impl Supervisor { schema_version: 1, job_id: self.config.job_id.clone(), mode: self.config.mode, - clock_basis: "observer-monotonic-seconds", + clock_basis: if self.restored_sessions == 0 { + "observer-monotonic-seconds".into() + } else { + "observer-elapsed-seconds-with-restart-gaps".into() + }, incidents: self.incidents.clone(), unknown_observations: self.unknown_observations, recovered_incidents: durations.len(), @@ -444,6 +910,19 @@ impl Supervisor { failure_detection_seconds: detection, recovery_seconds: durations, checkpoint_measurements: self.checkpoints.clone(), + assessments: self.assessments.clone(), + observation_gaps: self.gaps.clone(), + context: Snapshot { + schema_version: 1, + configuration: self.config.clone(), + states: self.states.clone(), + elapsed_seconds: self.observed_at, + saved_at: SystemTime::now(), + last_reservation: self.last_reservation, + jitter_seed: self.jitter_seed, + restored_sessions: self.restored_sessions, + archived_reports: self.archived_reports.clone(), + }, } } } @@ -507,6 +986,37 @@ impl EventKind { } impl State { + /// Describe only conditions supported by the current deployment and application observations. + fn classify(&self, health: Health, unhealthy: bool) -> Classification { + if self.failed { + Classification::ApplicationFailure + } else if health == Health::Unready { + Classification::DeploymentUnavailable + } else if unhealthy { + Classification::ProgressStalled + } else { + Classification::Healthy + } + } + + /// A quiet active workload needs additional progress; completed work needs only continued healthy observations. + fn verify_stability(&self, incident: &mut Incident, unhealthy: bool, now: u64, window: u64) { + let Stability::Pending { + since, + progress, + instance, + } = &incident.stability + else { + return; + }; + if unhealthy || instance != &self.instance { + incident.stability = Stability::Unstable { observed_at: now }; + return; + } + if now.saturating_sub(*since) >= window && (self.completed || self.progress > *progress) { + incident.stability = Stability::Stable { verified_at: now }; + } + } /// Adopt a first instance's baseline only after validating its sequence and identity. fn accept_evidence( &mut self, @@ -574,19 +1084,6 @@ impl State { || (self.active && !self.completed && now.saturating_sub(self.progress_at) >= policy.stall_seconds) } - /// Permit a restart only within the configured mode, action budget and cooldown. - fn decision(&self, incident: &Incident, config: &Config, attempts: u32, now: u64) -> Decision { - if incident.status == Status::Detected - && config.mode == Mode::ControllerAssisted - && attempts < config.policy.max_restarts - && now.saturating_sub(self.last_action) >= config.policy.restart_after_seconds - { - Decision::Restart - } else { - Decision::Observe - } - } - /// Advance an incident through detection, useful recovery or deadline expiry using observer time. fn advance( &mut self, @@ -612,7 +1109,7 @@ impl State { if now.saturating_sub(incident.onset) >= policy.deadline_seconds { incident.status = Status::Unrecovered; } - if health == Health::Ready && !unhealthy && self.progress > incident.baseline { + if self.fresh && health == Health::Ready && !unhealthy && self.progress > incident.baseline { // Late progress closes the active phase without rewriting a missed recovery deadline as success. if incident.status != Status::Unrecovered { incident.recovered = ObservationTime::Recorded(now); @@ -622,3 +1119,34 @@ impl State { } } } + +impl Incident { + /// Capture an incident baseline once, before detection and interventions can advance. + #[expect( + clippy::single_call_fn, + reason = "incident creation captures its immutable initial measurements" + )] + const fn new(workload: String, onset: u64, baseline: u64) -> Self { + Self { + workload, + onset, + baseline, + detected: ObservationTime::Pending, + recovered: ObservationTime::Pending, + status: Status::Suspected, + actions: Vec::new(), + stability: Stability::Disabled, + } + } + + /// Backoff counts attempts for this incident while the durable budget remains job-wide. + fn attempts(&self) -> u32 { + u32::try_from( + self.actions + .iter() + .filter(|action| matches!(action, Action::RestartReserved)) + .count(), + ) + .unwrap_or(u32::MAX) + } +} diff --git a/services/recovery-supervisor/src/resilience.rs b/services/recovery-supervisor/src/resilience.rs new file mode 100644 index 00000000..51f4ae3c --- /dev/null +++ b/services/recovery-supervisor/src/resilience.rs @@ -0,0 +1,226 @@ +//! Optional recovery coordination and explicit, evidence-based decision reporting. + +use std::collections::{BTreeMap, BTreeSet}; +use std::hash::{DefaultHasher, Hasher as _}; + +use serde::{Deserialize, Serialize}; + +use crate::RecoveryError; +use crate::config::{Config, valid_name}; +use crate::policy::ObservationTime; + +/// Additional safeguards; zero-valued limits preserve unrestricted fleet coordination. +#[derive(Clone, Debug, Deserialize, Serialize)] +#[cfg_attr(feature = "env-schema", derive(schemars::JsonSchema))] +#[serde(default, deny_unknown_fields)] +#[non_exhaustive] +pub struct Settings { + /// Retry a lost supervisor connection rather than exit; no interventions occur while disconnected. + pub reconnect: bool, + /// Bound each connect/register attempt; the shared transport supplies its retry backoff. + pub connect_timeout_seconds: u64, + /// Maximum workloads with an outstanding recovery request; zero disables this additional limit. + pub max_concurrent_recoveries: u32, + /// Minimum interval between restart reservations across all workloads; zero disables the limit. + pub action_interval_seconds: u64, + /// Multiply the per-workload cooldown after successive attempts; one preserves a fixed cooldown. + pub backoff_multiplier: u32, + /// Upper bound on the cooldown including jitter; never shortens the original cooldown. + pub backoff_max_seconds: u64, + /// Random additional cooldown, bounded by `backoff_max_seconds`; zero disables jitter. + pub jitter_seconds: u64, + /// Healthy observation window after first useful recovery; zero disables stability assessment. + pub stability_seconds: u64, + /// Required Deployment dependencies per supervised workload; dependencies are observed, never restarted. + pub dependencies: BTreeMap>, +} + +impl Settings { + /// Reject unsafe dependency names, cycles and cooldown bounds that weaken the base policy. + pub fn validate(&self, config: &Config) -> Result<(), RecoveryError> { + self.validate_timings()?; + for (workload, dependencies) in &self.dependencies { + if !config.workloads.iter().any(|item| &item.deployment == workload) { + return Err(RecoveryError::Invalid( + "dependency owner is not a supervised workload".into(), + )); + } + let unique: BTreeSet<_> = dependencies.iter().collect(); + if unique.len() != dependencies.len() + || dependencies.iter().any(|name| !valid_name(name) || name == workload) + { + return Err(RecoveryError::Invalid( + "dependencies must have unique, valid Deployment names".into(), + )); + } + self.validate_path(workload, &mut BTreeSet::new())?; + } + Ok(()) + } + + /// Check retry timing independently of the deployment dependency graph. + fn validate_timings(&self) -> Result<(), RecoveryError> { + if self.connect_timeout_seconds == 0 + || !(1..=16).contains(&self.backoff_multiplier) + || self.backoff_max_seconds == 0 + || self.jitter_seconds > self.backoff_max_seconds + { + return Err(RecoveryError::Invalid( + "invalid resilience timing or backoff bounds".into(), + )); + } + Ok(()) + } + + /// Validate a path rather than a global visited set so shared dependencies remain valid. + fn validate_path(&self, workload: &str, path: &mut BTreeSet) -> Result<(), RecoveryError> { + if !path.insert(workload.into()) { + return Err(RecoveryError::Invalid("recovery dependencies contain a cycle".into())); + } + if let Some(dependencies) = self.dependencies.get(workload) { + for dependency in dependencies { + self.validate_path(dependency, path)?; + } + } + let _removed = path.remove(workload); + Ok(()) + } + + /// Return all additional read-only deployment targets in a deterministic order. + #[must_use] + pub fn dependency_names(&self) -> BTreeSet { + self.dependencies.values().flatten().cloned().collect() + } + + /// Derive a bounded delay with a persisted per-supervisor seed and a stable per-attempt sample. + #[must_use] + pub fn cooldown(&self, base: u64, attempts: u32, workload: &str, seed: u64) -> u64 { + if attempts == 0 { + return base; + } + let exponent = attempts.saturating_sub(1).min(64); + let multiplier = u64::from(self.backoff_multiplier).saturating_pow(exponent); + let delay = base.saturating_mul(multiplier).min(self.backoff_max_seconds); + let mut sample = DefaultHasher::new(); + sample.write_u64(seed); + sample.write(workload.as_bytes()); + sample.write_u32(attempts); + let jitter = sample + .finish() + .checked_rem(self.jitter_seconds.saturating_add(1)) + .unwrap_or(0); + // skipcq: RS-W1069 -- base can exceed the cap; preserve it without clamp's inverted-bound panic + delay.saturating_add(jitter).min(self.backoff_max_seconds).max(base) + } +} + +impl Default for Settings { + fn default() -> Self { + Self { + reconnect: true, + connect_timeout_seconds: 30, + max_concurrent_recoveries: 0, + action_interval_seconds: 0, + backoff_multiplier: 1, + backoff_max_seconds: 300, + jitter_seconds: 0, + stability_seconds: 0, + dependencies: BTreeMap::new(), + } + } +} + +/// Accept the same settings as a scenario mapping or a single JSON environment value. +pub fn deserialize<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + #[derive(Deserialize)] + #[serde(untagged)] + enum Input { + Encoded(String), + Structured(Settings), + } + match Input::deserialize(deserializer)? { + Input::Encoded(value) => match serde_json::from_str(&value) { + Ok(settings) => Ok(settings), + Err(error) => Err(serde::de::Error::custom(error)), + }, + Input::Structured(settings) => Ok(settings), + } +} + +/// Observed condition, not a speculative diagnosis of hardware or application internals. +#[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(rename_all = "kebab-case")] +#[non_exhaustive] +pub enum Classification { + Healthy, + DeploymentUnavailable, + ApplicationFailure, + ProgressStalled, + DependencyUnavailable, + ObservationUnknown, +} + +/// Why policy permits or suppresses an intervention at its most recent assessment. +#[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(rename_all = "kebab-case")] +#[non_exhaustive] +pub enum Reason { + Healthy, + ObservationUnknown, + EvidenceRequired, + DependencyUnavailable, + DependencyUnknown, + DetectionGrace, + ObserveOnly, + Cooldown, + ConcurrencyLimit, + ActionInterval, + BudgetExhausted, + DeadlineExceeded, + RestartEligible, + RecoveryInProgress, +} + +/// Latest assessment per workload; bounded by the configured workload set. +#[derive(Clone, Debug, Deserialize, Serialize)] +#[non_exhaustive] +pub struct Assessment { + pub classification: Classification, + pub reason: Reason, + pub observed_at: u64, + pub next_action_at: ObservationTime, + pub dependencies: Vec, +} + +/// Sustained recovery is measured separately from the first useful recovery and MTTR. +#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] +#[serde(tag = "status", rename_all = "kebab-case")] +#[non_exhaustive] +pub enum Stability { + #[default] + Disabled, + Pending { + since: u64, + progress: u64, + instance: String, + }, + Stable { + verified_at: u64, + }, + Unstable { + observed_at: u64, + }, + Incomplete, +} + +/// Explicit periods when application health could not be observed. +#[derive(Clone, Debug, Deserialize, Serialize)] +#[non_exhaustive] +pub struct ObservationGap { + pub started_at: u64, + pub ended_at: ObservationTime, + pub reason: String, +} diff --git a/services/recovery-supervisor/src/runtime.rs b/services/recovery-supervisor/src/runtime.rs index 0930282a..b0df92ad 100644 --- a/services/recovery-supervisor/src/runtime.rs +++ b/services/recovery-supervisor/src/runtime.rs @@ -32,6 +32,17 @@ pub fn load_budget(config: &Config) -> Result { } } +/// Restore a complete committed context, creating a new supervisor only when no state file exists. +pub fn load_supervisor(config: &Config) -> Result { + match fs::read(&config.state_file) { + Ok(bytes) => Supervisor::restore(config.clone(), serde_json::from_slice(&bytes)?), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + Supervisor::new(config.clone(), Budget::new(config.job_id.clone())) + } + Err(error) => Err(error.into()), + } +} + /// Apply one observation cycle, recording a budget reservation before requesting an external action. pub async fn cycle( config: &Config, @@ -43,19 +54,33 @@ pub async fn cycle( where B: Backend, { - for (workload, health) in observations(config, backend).await { + for (workload, health) in observations(config, backend, supervisor).await? { apply_observation(store, supervisor, backend, &workload, health, now).await?; } store.persist(supervisor.report()).await } -/// Collect backend observations without borrowing application policy state. -async fn observations(config: &Config, backend: &mut B) -> Vec<(Workload, Health)> +/// Refresh all dependencies before yielding supervised targets eligible for policy evaluation. +async fn observations( + config: &Config, + backend: &mut B, + supervisor: &mut Supervisor, +) -> Result, RecoveryError> where B: Backend, { let mut observations = Vec::new(); - for workload in &config.workloads { + let mut targets = config.workloads.clone(); + for name in config.resilience.dependency_names() { + if !targets.iter().any(|item| item.deployment == name) { + targets.push(Workload { + deployment: name, + agent_id: String::default(), + source: String::default(), + }); + } + } + for workload in &targets { let health = match backend.health(workload).await { Ok(health) => health, Err(error) => { @@ -65,7 +90,19 @@ where }; observations.push((workload.clone(), health)); } - observations + let dependencies = config.resilience.dependency_names(); + for (workload, health) in &observations { + if dependencies.contains(&workload.deployment) { + supervisor.dependency_observation(&workload.deployment, *health)?; + } + } + observations.retain(|(workload, _health)| { + config + .workloads + .iter() + .any(|item| item.deployment == workload.deployment) + }); + Ok(observations) } /// Apply current application evidence to an infrastructure observation, reserving budget before any restart. @@ -114,33 +151,104 @@ where }; #[cfg(not(unix))] let termination = std::future::pending::<()>(); - let store = StateStore::new(config.state_file.clone()); - let mut supervisor = Supervisor::new(config.clone(), load_budget(&config)?)?; + let store = session.state_store(config.state_file.clone()); + let mut supervisor = load_supervisor(&config)?; // Fail closed on an unwritable budget path before registering or inspecting deployments. store.persist(supervisor.report()).await?; - let socket = connect(&config).await?; - let (sender, receiver) = mpsc::channel::<(String, CloudEvent)>(256); let started = Instant::now(); let outcome = tokio::select! { - result = hub(socket, sender) => result, - result = control(&config, &mut supervisor, &mut backend, &store, &session, receiver, started) => result, + result = supervise(&config, &mut supervisor, &mut backend, &store, &session, started) => result, signal = tokio::signal::ctrl_c() => { signal?; Ok(()) }, () = termination => Ok(()), }; + supervisor.transport_lost(supervisor.elapsed(started.elapsed().as_secs()), "supervision stopped"); store.persist(supervisor.report()).await?; outcome } -/// Register the configured supervisor identity before accepting lifecycle evidence. +/// Keep policy state and its ownership lease while re-establishing transport; persistence errors remain fatal. +#[expect( + clippy::integer_division_remainder_used, + clippy::single_call_fn, + reason = "connection ownership is separate from shutdown; tokio::select! expands to remainder operations" +)] +async fn supervise( + config: &Config, + supervisor: &mut Supervisor, + backend: &mut B, + store: &StateStore, + session: &Session, + started: Instant, +) -> Result<(), RecoveryError> +where + B: Backend, +{ + loop { + let socket = Box::pin(reconnect(config, supervisor, store, session, started)).await?; + supervisor.transport_connected(supervisor.elapsed(started.elapsed().as_secs())); + store.persist(supervisor.report()).await?; + let (sender, receiver) = mpsc::channel::<(String, CloudEvent)>(256); + let outcome = tokio::select! { + result = hub(socket, sender) => result, + result = control(config, supervisor, backend, store, session, receiver, started) => return result, + }; + if !config.resilience.reconnect { + return outcome; + } + let reason = outcome.map_or_else(|error| error.to_string(), |()| "hub transport ended".into()); + warn!(%reason, "recovery transport unavailable; interventions suspended"); + supervisor.transport_lost(supervisor.elapsed(started.elapsed().as_secs()), &reason); + store.persist(supervisor.report()).await?; + } +} + +/// Bound registration attempts and maintain established observer health while recording unavailable evidence. #[expect( + clippy::integer_division_remainder_used, clippy::single_call_fn, - reason = "registration validates hub identity before supervision can begin" + reason = "registration is separate from connected observation; tokio::select! expands to remainder operations" )] +async fn reconnect( + config: &Config, + supervisor: &mut Supervisor, + store: &StateStore, + session: &Session, + started: Instant, +) -> Result { + if !config.resilience.reconnect { + return connect(config).await; + } + loop { + let attempt = connect(config); + tokio::pin!(attempt); + let mut interval = tokio::time::interval(Duration::from_secs(config.poll_seconds)); + loop { + tokio::select! { + result = &mut attempt => match result { + Ok(socket) => return Ok(socket), + Err(error @ RecoveryError::Invalid(_)) => return Err(error), + Err(error) => { + supervisor.transport_lost(supervisor.elapsed(started.elapsed().as_secs()), &error.to_string()); + store.persist(supervisor.report()).await?; + break; + } + }, + _ = interval.tick() => { + supervisor.transport_lost(supervisor.elapsed(started.elapsed().as_secs()), "hub registration"); + store.persist(supervisor.report()).await?; + session.maintain().await?; + } + } + } + } +} + +/// Register the configured supervisor identity before accepting lifecycle evidence. async fn connect(config: &Config) -> Result { let (socket, identity, _status) = et_ws_runner_common::connect_and_register( &config.ws_url, Some(config.agent_id.clone()), - Some(Duration::from_secs(30)), + Some(Duration::from_secs(config.resilience.connect_timeout_seconds)), ) .await?; if identity != config.agent_id { @@ -203,7 +311,7 @@ async fn poll( where B: Backend, { - for (workload, health) in observations(config, backend).await { + for (workload, health) in observations(config, backend, supervisor).await? { // Apply queued progress without letting a continuous producer starve observation and persistence. for _ in 0..receiver.len() { if let Ok((identity, event)) = receiver.try_recv() { @@ -216,7 +324,7 @@ where backend, &workload, health, - started.elapsed().as_secs(), + supervisor.elapsed(started.elapsed().as_secs()), ) .await?; } @@ -225,7 +333,7 @@ where /// Log rejected lifecycle evidence without stopping supervision of valid workloads. fn accept(supervisor: &mut Supervisor, identity: &str, event: &CloudEvent, started: Instant) { - if let Err(error) = supervisor.event(identity, event, started.elapsed().as_secs()) { + if let Err(error) = supervisor.event(identity, event, supervisor.elapsed(started.elapsed().as_secs())) { warn!(%error, "ignored recovery evidence"); } } diff --git a/services/recovery-supervisor/tests/config.rs b/services/recovery-supervisor/tests/config.rs index bae0f72d..5804f082 100644 --- a/services/recovery-supervisor/tests/config.rs +++ b/services/recovery-supervisor/tests/config.rs @@ -7,6 +7,92 @@ use et_recovery_supervisor::policy::{Budget, Health, Supervisor}; use et_test_recovery::{config, event, supervisor}; use fs_err as fs; +/// A dependency cycle spanning deployments is rejected, while shared dependencies remain valid. +#[test] +fn dependency_cycles_are_rejected_without_rejecting_shared_dependencies() { + let mut settings = config(); + let mut second = settings.workloads[0].clone(); + second.deployment = "second-worker".into(); + second.agent_id = "second-worker".into(); + settings.workloads.push(second); + settings.resilience.dependencies = [ + ("example-worker".into(), vec!["second-worker".into()]), + ("second-worker".into(), vec!["example-worker".into()]), + ] + .into(); + let error = settings.validate().unwrap_err(); + assert!(error.to_string().contains("cycle")); + settings.resilience.dependencies = [ + ("example-worker".into(), vec!["second-worker".into(), "storage".into()]), + ("second-worker".into(), vec!["storage".into()]), + ] + .into(); + settings.validate().unwrap(); +} + +/// Structured scenarios and encoded settings preserve options and reject malformed or unknown values. +#[test] +fn resilience_deserialization_preserves_settings_and_rejects_invalid_encodings() { + let settings = serde_json::json!({"dependencies": {"example-worker": ["storage"]}, "stability_seconds": 10_u64}); + for input in [settings.clone(), serde_json::json!(settings.to_string())] { + let mut value = config_for_serialization(); + value["resilience"] = input; + let parsed: Config = serde_json::from_value(value).unwrap(); + parsed.validate().unwrap(); + assert_eq!(parsed.resilience.stability_seconds, 10); + assert_eq!(parsed.resilience.dependencies["example-worker"], vec!["storage"]); + } + for input in [ + serde_json::json!("not json"), + serde_json::json!(r#"{"unexpected": true}"#), + serde_json::json!(r#"{"connect_timeout_seconds": "invalid"}"#), + serde_json::json!(42_u64), + ] { + let mut value = config_for_serialization(); + value["resilience"] = input; + let _invalid = serde_json::from_value::(value).unwrap_err(); + } +} + +/// Resilience options accept structured scenarios and JSON environment values, rejecting ambiguous dependencies. +#[test] +fn resilience_configuration_round_trips_and_rejects_invalid_dependency_graphs() { + let mut settings = config(); + let _previous = settings + .resilience + .dependencies + .insert("example-worker".into(), vec!["storage".into()]); + settings.validate().unwrap(); + let mut env = et_test_recovery::environment(); + let _previous = env.insert( + "RECOVERY_RESILIENCE".into(), + serde_json::to_string(&settings.resilience).unwrap(), + ); + let parsed: et_recovery_supervisor::config::EnvConfig = serde_env::from_iter(env).unwrap(); + assert_eq!( + parsed.recovery.resilience.dependencies, + settings.resilience.dependencies + ); + for dependencies in [vec!["example-worker"], vec!["bad/name"], vec!["storage", "storage"]] { + let _previous = settings.resilience.dependencies.insert( + "example-worker".into(), + dependencies.into_iter().map(str::to_owned).collect(), + ); + assert!(settings.validate().is_err()); + } + settings.resilience.dependencies.clear(); + let _previous = settings + .resilience + .dependencies + .insert("foreign".into(), vec!["storage".into()]); + assert!(settings.validate().is_err()); + for field in ["connect_timeout_seconds", "backoff_multiplier"] { + let mut value = config_for_serialization(); + value["resilience"][field] = serde_json::json!(0_u64); + assert!(serde_json::from_value::(value).unwrap().validate().is_err()); + } +} + /// Typed construction preserves the complete in-cluster configuration without encoding it through JSON. #[test] fn typed_in_cluster_settings_match_the_expected_runtime_configuration() { diff --git a/services/recovery-supervisor/tests/health.rs b/services/recovery-supervisor/tests/health.rs index 19244175..30cf4bcc 100644 --- a/services/recovery-supervisor/tests/health.rs +++ b/services/recovery-supervisor/tests/health.rs @@ -41,8 +41,15 @@ fn probe(config: &Config) -> std::process::Output { .unwrap() } -/// Require both the library and executable health checks to reject a stopped owner. -fn assert_stopped(config: &Config) { +/// Allow an in-flight blocking save to release its lease, then require both health checks to reject the owner. +async fn assert_stopped(config: &Config) { + tokio::time::timeout(Duration::from_secs(5), async { + while !check(config).is_err_and(|error| error.to_string().contains("not running")) { + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); assert!(check(config).unwrap_err().to_string().contains("not running")); assert!(!probe(config).status.success()); } @@ -66,14 +73,16 @@ async fn startup_waits_for_a_completed_cycle_and_a_read_only_probe_cannot_spend_ let _missing = check(&config).unwrap_err(); assert!(!config.state_file.with_extension("lock").exists()); let session = Session::acquire(&config).unwrap(); + session.maintain().await.unwrap(); let _starting = check(&config).unwrap_err(); assert!(!probe(&config).status.success()); session.advance().await.unwrap(); + session.maintain().await.unwrap(); check(&config).unwrap(); assert!(probe(&config).status.success()); assert!(!config.state_file.exists()); drop(session); - assert_stopped(&config); + assert_stopped(&config).await; } /// Prevent a replacement supervisor from inheriting another process's healthy status. @@ -88,6 +97,7 @@ async fn a_new_owner_cannot_reuse_a_previous_sessions_fresh_heartbeat() { assert_eq!(fs::read(config.state_file.with_extension("session")).unwrap(), identity); check(&config).unwrap(); drop(first); + assert_stopped(&config).await; let second = Session::acquire(&config).unwrap(); assert_ne!(fs::read(config.state_file.with_extension("session")).unwrap(), identity); assert!( @@ -120,14 +130,14 @@ async fn session_identity_remains_readable_without_reading_the_locked_lease() { } /// Release ownership if readable session metadata cannot be published, without advertising old evidence as healthy. -#[test] -fn unavailable_session_storage_releases_the_budget_lease() { +#[tokio::test] +async fn unavailable_session_storage_releases_the_budget_lease() { let directory = tempfile::tempdir().unwrap(); let config = configured(directory.path()); let path = config.state_file.with_extension("session"); fs::create_dir(&path).unwrap(); let _unavailable = Session::acquire(&config).unwrap_err(); - assert_stopped(&config); + assert_stopped(&config).await; fs::remove_dir(path).unwrap(); let _owner = Session::acquire(&config).unwrap(); } @@ -271,8 +281,8 @@ async fn a_blocked_loop_cannot_refresh_health_and_api_failure_is_not_a_superviso release.notify_one(); wait_for_health(&config).await; task.abort(); - let _cancelled = task.await; - assert!(check(&config).is_err()); + assert!(task.await.unwrap_err().is_cancelled()); + assert_stopped(&config).await; } /// Release health ownership on transport loss instead of trusting a recently written heartbeat. @@ -293,6 +303,7 @@ async fn assert_transport_failure(graceful: bool) { let mut config = configured(directory.path()); let (url, close, server) = fake_hub("recovery-supervisor", graceful).await; config.ws_url = url; + config.resilience.reconnect = false; let backend = PausedBackend::default(); let task = tokio::spawn(et_recovery_supervisor::runtime::run(config.clone(), backend)); wait_for_health(&config).await; @@ -304,7 +315,7 @@ async fn assert_transport_failure(graceful: bool) { .unwrap_err(); assert!(error.to_string().contains("hub connection")); server.await.unwrap(); - assert_stopped(&config); + assert_stopped(&config).await; } /// Serve a loopback protocol peer with controllable registration and shutdown behavior. @@ -365,6 +376,7 @@ async fn registration_and_heartbeat_persistence_failures_never_leave_a_healthy_o }; let (url, close, server) = fake_hub(identity, false).await; config.ws_url = url; + config.resilience.reconnect = bad_identity; if !bad_identity { fs::create_dir_all(config.state_file.with_extension("health.json")).unwrap(); } @@ -384,7 +396,7 @@ async fn registration_and_heartbeat_persistence_failures_never_leave_a_healthy_o assert!(error.to_string().contains(expected), "{error}"); close.notify_one(); server.await.unwrap(); - assert_stopped(&config); + assert_stopped(&config).await; } } diff --git a/services/recovery-supervisor/tests/kubernetes.rs b/services/recovery-supervisor/tests/kubernetes.rs index 66a8fa13..381fca92 100644 --- a/services/recovery-supervisor/tests/kubernetes.rs +++ b/services/recovery-supervisor/tests/kubernetes.rs @@ -5,6 +5,7 @@ use std::path::{Path, PathBuf}; use std::time::{Duration, Instant}; use et_recovery_supervisor::RecoveryError; +use et_recovery_supervisor::config::Workload; use et_recovery_supervisor::kubernetes::{Backend as _, Kubernetes}; use et_recovery_supervisor::policy::Health; use et_test_helpers::temp_env; @@ -17,6 +18,17 @@ use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _}; use tokio::net::{TcpListener, TcpStream}; use tracing::subscriber::set_default; +const DEPENDENCY_HEALTH: [(&str, Health); 8] = [ + ("storage", Health::Ready), + ("storage-empty", Health::Unready), + ("storage-stale", Health::Unready), + ("storage-insufficient", Health::Unready), + ("storage-updating", Health::Unready), + ("storage-rolling", Health::Unready), + ("storage-unavailable", Health::Unready), + ("storage-missing", Health::Unready), +]; + /// Timestamp fixture activity without changing the command or request deadlines under investigation. #[derive(Clone, Copy)] struct Diagnostics { @@ -63,7 +75,14 @@ fn response(path: &str) -> Value { "resources":[{"name":"deployments", "singularName":"deployment", "namespaced":true, "kind":"Deployment", "verbs":["get","patch"]}] }), - "/apis/apps/v1/namespaces/example/deployments/example-worker" => deployment(), + "/apis/apps/v1/namespaces/example/deployments/example-worker" => deployment("example-worker", 1), + "/apis/apps/v1/namespaces/example/deployments/storage-forbidden" => json!({ + "kind": "Status", "apiVersion": "v1", "status": "Failure", "reason": "Forbidden", "code": 403_u32, + "message": "fixture permission denied" + }), + path if path.starts_with("/apis/apps/v1/namespaces/example/deployments/storage") => { + dependency_deployment(path.rsplit('/').next().unwrap()) + } // kubectl 1.37 reads the server version before discovery, to warn about client/server skew. "/version" => serde_json::to_value(Info { git_version: "v1.37.0".into(), @@ -77,33 +96,52 @@ fn response(path: &str) -> Value { } /// Serialize an actual Kubernetes Deployment so discovery and restart tests use the API's real wire schema. -#[expect( - clippy::single_call_fn, - reason = "typed deployment data is separate from API discovery routing" -)] -fn deployment() -> Value { +fn deployment(name: &str, replicas: i32) -> Value { serde_json::to_value(Deployment { metadata: ObjectMeta { - name: Some("example-worker".into()), + name: Some(name.into()), namespace: Some("example".into()), generation: Some(1), ..ObjectMeta::default() }, spec: Some(DeploymentSpec { - replicas: Some(1), + replicas: Some(replicas), ..DeploymentSpec::default() }), status: Some(DeploymentStatus { observed_generation: Some(1), - replicas: Some(1), - available_replicas: Some(1), - updated_replicas: Some(1), + replicas: Some(replicas), + available_replicas: Some(replicas), + updated_replicas: Some(replicas), ..DeploymentStatus::default() }), }) .unwrap() } +/// Supply multi-replica dependency health without weakening the application's single-writer requirement. +#[expect( + clippy::single_call_fn, + reason = "dependency fixtures are distinct from HTTP discovery routing" +)] +fn dependency_deployment(name: &str) -> Value { + let replicas = if name == "storage-empty" { 0_i32 } else { 3_i32 }; + let mut value = deployment(name, replicas); + match name { + "storage" | "storage-empty" => {} + "storage-stale" => value["status"]["observedGeneration"] = json!(0_u32), + "storage-insufficient" => value["status"]["availableReplicas"] = json!(2_u32), + "storage-updating" => value["status"]["updatedReplicas"] = json!(2_u32), + "storage-rolling" => value["status"]["replicas"] = json!(4_u32), + "storage-unavailable" => value["status"]["unavailableReplicas"] = json!(1_u32), + "storage-missing" => { + let _status = value.as_object_mut().unwrap().remove("status"); + } + _ => panic!("unexpected dependency fixture: {name}"), + } + value +} + /// Read a complete request, including the rollout patch body, before returning a close-delimited response. #[expect( clippy::single_call_fn, @@ -153,8 +191,13 @@ async fn serve(listener: TcpListener, seen: tokio::sync::mpsc::UnboundedSender, +) { + for &(name, expected) in dependencies { + let mut dependency = workload.clone(); + dependency.deployment = name.into(); + dependency.agent_id.clear(); + dependency.source.clear(); + assert_eq!( + diagnostics.command(name, backend.health(&dependency).await, task), + expected + ); + let restart = backend.restart(&dependency).await.unwrap_err(); + let other_read = backend.execute(&dependency, &["get", "-o", "yaml"]).await.unwrap_err(); + for error in [restart, other_read] { + assert!(error.to_string().contains("not allowlisted")); + } + } + let mut denied = workload.clone(); + denied.deployment = "storage-forbidden".into(); + let error = backend.health(&denied).await.unwrap_err(); + assert!(error.to_string().contains("fixture permission denied")); +} + /// Verify that the recorded restart patch targets only the configured namespaced deployment. #[expect( clippy::single_call_fn, reason = "request assertions are distinct from command execution and fixture setup" )] fn assert_restart_request(mut received: tokio::sync::mpsc::UnboundedReceiver) { - let mut patch = String::default(); + let mut patches = Vec::new(); while let Ok(text) = received.try_recv() { if text.starts_with("PATCH ") { - patch = text; + patches.push(text); } } - assert!(patch.starts_with("PATCH /apis/apps/v1/namespaces/example/deployments/example-worker")); - assert!(patch.contains("kubectl.kubernetes.io/restartedAt")); + assert_eq!(patches.len(), 1); + assert!(patches[0].starts_with("PATCH /apis/apps/v1/namespaces/example/deployments/example-worker")); + assert!(patches[0].contains("kubectl.kubernetes.io/restartedAt")); } /// Keep credentials isolated while making kubeconfig preparation distinct from the timed command exchange. diff --git a/services/recovery-supervisor/tests/persistence.rs b/services/recovery-supervisor/tests/persistence.rs index 974de2cb..44d3282b 100644 --- a/services/recovery-supervisor/tests/persistence.rs +++ b/services/recovery-supervisor/tests/persistence.rs @@ -1,14 +1,63 @@ -//! Ordered blocking writes preserve state on errors and do not race cancelled async callers. +//! Durable recovery context survives restoration, malformed input and cancelled asynchronous writes. #![cfg(test)] use std::sync::Arc; -use std::time::Duration; +use std::time::{Duration, SystemTime}; +use et_recovery_supervisor::config::Mode; use et_recovery_supervisor::persistence::StateStore; +use et_recovery_supervisor::policy::{Action, Budget, Decision, Health, Status, Supervisor}; +use et_test_recovery::{config, event, report, supervisor}; use fs_err as fs; use serde::{Serialize, Serializer}; use tokio::sync::Notify; +/// Future saved timestamps reject startup with actionable context and leave the durable state unchanged. +#[test] +fn future_saved_at_names_the_timestamp_and_preserves_the_state_file() { + let directory = tempfile::tempdir().unwrap(); + let mut settings = config(); + settings.state_file = directory.path().join("recovery.json"); + let policy = Supervisor::new(settings.clone(), Budget::new(settings.job_id.clone())).unwrap(); + let future = SystemTime::now().checked_add(Duration::from_secs(3600)).unwrap(); + let mut saved = report(&policy); + saved["context"]["saved_at"] = serde_json::to_value(future).unwrap(); + let bytes = serde_json::to_vec(&saved).unwrap(); + fs::write(&settings.state_file, &bytes).unwrap(); + for _ in 0..2_u8 { + let error = et_recovery_supervisor::runtime::load_supervisor(&settings).unwrap_err(); + assert!(error.to_string().contains(&format!("{future:?}"))); + assert!(error.to_string().contains("saved_at")); + assert!(error.to_string().contains("correct the system clock")); + assert_eq!(fs::read(&settings.state_file).unwrap(), bytes); + } +} + +/// Time spent stopped counts toward the original incident deadline and cannot grant another restart attempt. +#[test] +fn restored_downtime_expires_the_original_recovery_deadline() { + let mut settings = config(); + settings.mode = Mode::ControllerAssisted; + let worker = settings.workloads[0].clone(); + let mut policy = Supervisor::new(settings.clone(), Budget::new(settings.job_id.clone())).unwrap(); + for now in [0, 3] { + let _observed = policy.observe(&worker, Health::Unready, now).unwrap(); + } + policy.reserve_restart(&worker, 23).unwrap(); + let mut saved = report(&policy); + saved["context"]["saved_at"] = serde_json::to_value(SystemTime::now() - Duration::from_secs(300)).unwrap(); + let mut restored = Supervisor::restore(settings, saved).unwrap(); + let now = restored.elapsed(0); + assert!(now >= 323); + assert_eq!( + restored.observe(&worker, Health::Unready, now).unwrap(), + Decision::Observe + ); + assert_eq!(restored.incidents()[0].status, Status::Unrecovered); + assert!(restored.reserve_restart(&worker, now).is_err()); + assert_eq!(restored.budget().restarts[&worker.deployment], 1); +} + struct PausedDocument { started: Arc, release: std::sync::Mutex>, @@ -38,7 +87,22 @@ async fn cancellation_cannot_let_an_older_save_overwrite_the_final_report() { let directory = tempfile::tempdir().unwrap(); let path = directory.path().join("state.json"); let store = StateStore::new(path.clone()); - let writer = store.clone(); + let release = cancelled_write(store.clone()).await; + let mut final_save = tokio::spawn(async move { store.persist(2_u64).await }); + let _blocked = tokio::time::timeout(Duration::from_millis(20), &mut final_save) + .await + .unwrap_err(); + release.send(()).unwrap(); + tokio::time::timeout(Duration::from_secs(3), final_save) + .await + .unwrap() + .unwrap() + .unwrap(); + assert_eq!(fs::read(&path).unwrap(), b"2"); +} + +/// Cancel the async waiter only after serialization has entered the owned blocking write. +async fn cancelled_write(writer: StateStore) -> std::sync::mpsc::Sender<()> { let started = Arc::new(Notify::new()); let (release, receiver) = std::sync::mpsc::channel(); let paused = PausedDocument { @@ -52,21 +116,40 @@ async fn cancellation_cannot_let_an_older_save_overwrite_the_final_report() { .unwrap(); first.abort(); assert!(first.await.unwrap_err().is_cancelled()); - let mut final_save = tokio::spawn(async move { store.persist(2_u64).await }); - let _blocked = tokio::time::timeout(Duration::from_millis(20), &mut final_save) - .await - .unwrap_err(); - release.send(()).unwrap(); - tokio::time::timeout(Duration::from_secs(3), final_save) - .await - .unwrap() - .unwrap() - .unwrap(); - assert_eq!(fs::read(&path).unwrap(), b"2"); + release } struct InvalidDocument; +/// An unfinished cancelled save retains ownership, preventing an old report from overwriting a replacement's context. +#[tokio::test] +async fn cancelled_persistence_keeps_the_owner_until_the_blocking_write_finishes() { + let directory = tempfile::tempdir().unwrap(); + let mut config = et_test_recovery::config(); + config.state_file = directory.path().join("state.json"); + let session = et_recovery_supervisor::health::Session::acquire(&config).unwrap(); + let store = session.state_store(config.state_file.clone()); + let release = cancelled_write(store).await; + drop(session); + let _owned = et_recovery_supervisor::health::Session::acquire(&config).unwrap_err(); + release.send(()).unwrap(); + let owner = tokio::time::timeout(Duration::from_secs(3), async { + loop { + match et_recovery_supervisor::health::Session::acquire(&config) { + Ok(owner) => break owner, + Err(et_recovery_supervisor::RecoveryError::Lease(_)) => { + tokio::time::sleep(Duration::from_millis(10)).await; + } + Err(error) => panic!("replacement could not acquire ownership: {error}"), + } + } + }) + .await + .unwrap(); + assert_eq!(fs::read(&config.state_file).unwrap(), b"1"); + drop(owner); +} + impl Serialize for InvalidDocument { /// Inject a serialization failure before a replacement document can be committed. fn serialize(&self, _serializer: S) -> Result @@ -90,3 +173,120 @@ async fn failed_serialization_preserves_the_last_document_and_releases_the_write store.persist(2_u64).await.unwrap(); assert_eq!(fs::read(&path).unwrap(), b"2"); } + +/// Durable context retains incidents, consumed attempts, identities and cooldowns across a process replacement. +#[test] +fn restored_context_keeps_policy_history_and_rejects_replayed_or_mismatched_evidence() { + let mut settings = config(); + settings.mode = Mode::ControllerAssisted; + let worker = settings.workloads[0].clone(); + let mut policy = supervisor(settings.mode); + policy + .event(&worker.agent_id, &event("started", 0, 1, "one"), 0) + .unwrap(); + let _suspected = policy.observe(&worker, Health::Unready, 0).unwrap(); + let _detected = policy.observe(&worker, Health::Unready, 3).unwrap(); + policy.reserve_restart(&worker, 23).unwrap(); + policy.action_result(&worker, Action::RestartRequested).unwrap(); + let value = report(&policy); + let mut restored = Supervisor::restore(settings.clone(), value.clone()).unwrap(); + assert_eq!(restored.incidents()[0].actions, policy.incidents()[0].actions); + assert_eq!(restored.budget().restarts[&worker.deployment], 1); + assert!( + restored + .event(&worker.agent_id, &event("started", 0, 1, "one"), restored.elapsed(1)) + .is_err() + ); + assert_eq!( + restored.observe(&worker, Health::Unready, restored.elapsed(1)).unwrap(), + Decision::Observe + ); + assert_eq!(restored.report().observation_gaps.len(), 1); + settings.policy.max_restarts = 3; + let _mismatched = Supervisor::restore(settings, value).unwrap_err(); +} + +/// Legacy budgets survive migration; malformed contexts and impossible observer clocks fail closed. +#[test] +fn restoration_archives_legacy_reports_and_rejects_invalid_contexts() { + let settings = config(); + let policy = supervisor(Mode::ObserveOnly); + let mut legacy = report(&policy); + let _context = legacy.as_object_mut().unwrap().remove("context"); + let restored = Supervisor::restore(settings.clone(), legacy).unwrap(); + assert_eq!( + report(&restored)["context"]["archived_reports"] + .as_array() + .unwrap() + .len(), + 1 + ); + for (field, value) in [ + ("schema_version", serde_json::json!(2_u32)), + ("states", serde_json::json!({})), + ( + "saved_at", + serde_json::json!({"secs_since_epoch": u64::MAX, "nanos_since_epoch": 0_u32}), + ), + ] { + let mut invalid = report(&policy); + invalid["context"][field] = value; + let _invalid = Supervisor::restore(settings.clone(), invalid).unwrap_err(); + } + let _missing = Supervisor::restore(settings, serde_json::json!({})).unwrap_err(); +} + +/// Corrupt saved budgets fail closed instead of creating a fresh supervisor with a reset restart allowance. +#[test] +fn restoration_rejects_malformed_restart_budgets() { + for budget in [ + serde_json::Value::Null, + serde_json::json!({"restarts": {"example-worker": 1_u32}}), + ] { + let error = Supervisor::restore(config(), serde_json::json!({"restart_budget": budget})).unwrap_err(); + assert!(matches!(error, et_recovery_supervisor::RecoveryError::Json(_))); + } +} + +/// A valid budget cannot authorise restoration of an unsupported report version or a different policy. +#[test] +fn restoration_checks_report_schema_and_configuration_even_with_matching_identity() { + let policy = supervisor(Mode::ObserveOnly); + let mut invalid_schema = report(&policy); + invalid_schema["schema_version"] = serde_json::json!(2_u32); + let mut changed_configuration = report(&policy); + changed_configuration["context"]["configuration"]["policy"]["max_restarts"] = serde_json::json!(3_u32); + for invalid in [invalid_schema, changed_configuration] { + let error = Supervisor::restore(config(), invalid).unwrap_err(); + assert!(error.to_string().contains("does not match configuration")); + } +} + +/// Contradictory report identity cannot be restored even when its embedded configuration is valid. +#[test] +fn restoration_rejects_a_report_from_another_job_or_recovery_mode() { + let policy = supervisor(Mode::ObserveOnly); + for (field, value) in [("job_id", "foreign"), ("mode", "controller-assisted")] { + let mut invalid = report(&policy); + invalid[field] = serde_json::json!(value); + let _foreign = Supervisor::restore(config(), invalid).unwrap_err(); + } +} + +/// A persisted workload phase cannot refer outside the incident list or to another workload's incident. +#[test] +fn restoration_rejects_invalid_incident_references() { + let mut policy = supervisor(Mode::ObserveOnly); + let worker = config().workloads.remove(0); + let _suspected = policy.observe(&worker, Health::Unready, 0).unwrap(); + let _detected = policy.observe(&worker, Health::Unready, 3).unwrap(); + let valid = report(&policy); + let mut invalid_index = valid.clone(); + invalid_index["context"]["states"][&worker.deployment]["phase"]["Incident"] = serde_json::json!(999_u64); + let error = Supervisor::restore(config(), invalid_index).unwrap_err(); + assert!(error.to_string().contains("saved incident index is invalid")); + let mut invalid_workload = valid; + invalid_workload["incidents"][0]["workload"] = serde_json::json!("foreign-worker"); + let error = Supervisor::restore(config(), invalid_workload).unwrap_err(); + assert!(error.to_string().contains("saved incident workload is inconsistent")); +} diff --git a/services/recovery-supervisor/tests/policy.rs b/services/recovery-supervisor/tests/policy.rs index 76974af5..473bb4aa 100644 --- a/services/recovery-supervisor/tests/policy.rs +++ b/services/recovery-supervisor/tests/policy.rs @@ -1,10 +1,98 @@ //! Recovery policy verifies useful work, enforces action bounds, and rejects stale or foreign evidence. #![cfg(test)] -use et_recovery_supervisor::config::Mode; +use et_recovery_supervisor::config::{Mode, Workload}; use et_recovery_supervisor::policy::{Action, Decision, Health, ObservationTime, Status, Supervisor}; +use et_recovery_supervisor::resilience::{Classification, Reason}; use et_test_recovery::{config, event, report, supervisor}; +/// Start two detected workloads with one failed restart and a single fleet recovery slot. +fn policy_after_failed_restart() -> (Supervisor, Workload, Workload) { + let mut settings = config(); + settings.mode = Mode::ControllerAssisted; + settings.policy.max_restarts = 3; + settings.resilience.max_concurrent_recoveries = 1; + settings.resilience.backoff_multiplier = 2; + let first = settings.workloads[0].clone(); + let mut second = first.clone(); + second.deployment = "second-worker".into(); + second.agent_id = "second-worker".into(); + settings.workloads.push(second.clone()); + let budget = et_recovery_supervisor::policy::Budget::new(settings.job_id.clone()); + let mut policy = Supervisor::new(settings, budget).unwrap(); + for worker in [&first, &second] { + for now in [0, 3] { + let _observed = policy.observe(worker, Health::Unready, now).unwrap(); + } + } + policy.reserve_restart(&first, 23).unwrap(); + let failed = Action::RestartFailed { + error: "API request failed".into(), + }; + policy.action_result(&first, failed).unwrap(); + (policy, first, second) +} + +/// Failed restart commands release fleet capacity while retaining the original cooldown and consumed budget. +#[test] +fn failed_restart_releases_the_slot_without_bypassing_retry_limits() { + let (mut policy, first, second) = policy_after_failed_restart(); + assert_eq!(policy.observe(&first, Health::Unready, 24).unwrap(), Decision::Observe); + assert_eq!(policy.report().assessments[&first.deployment].reason, Reason::Cooldown); + assert_eq!( + policy.report().assessments[&first.deployment].next_action_at, + ObservationTime::Recorded(43) + ); + assert_eq!(policy.observe(&second, Health::Unready, 24).unwrap(), Decision::Restart); + policy.reserve_restart(&second, 24).unwrap(); + assert_eq!(policy.observe(&first, Health::Unready, 43).unwrap(), Decision::Observe); + assert_eq!( + policy.report().assessments[&first.deployment].reason, + Reason::ConcurrencyLimit + ); + assert!(policy.reserve_restart(&first, 43).is_err()); + assert_eq!(policy.budget().restarts[&first.deployment], 1); + policy + .action_result( + &second, + Action::RestartFailed { + error: "API request failed".into(), + }, + ) + .unwrap(); + policy.reserve_restart(&first, 43).unwrap(); + assert_eq!(policy.budget().restarts[&first.deployment], 2); +} + +/// A failed retry retains its longer backoff and cannot reset the lifetime restart allowance. +#[test] +fn failed_retry_preserves_backoff_and_eventual_budget_exhaustion() { + let (mut policy, first, _second) = policy_after_failed_restart(); + policy.reserve_restart(&first, 43).unwrap(); + policy + .action_result( + &first, + Action::RestartFailed { + error: "API request failed".into(), + }, + ) + .unwrap(); + assert_eq!(policy.observe(&first, Health::Unready, 82).unwrap(), Decision::Observe); + assert_eq!(policy.report().assessments[&first.deployment].reason, Reason::Cooldown); + assert_eq!( + policy.report().assessments[&first.deployment].next_action_at, + ObservationTime::Recorded(83) + ); + assert_eq!(policy.observe(&first, Health::Unready, 83).unwrap(), Decision::Restart); + policy.reserve_restart(&first, 83).unwrap(); + assert_eq!(policy.observe(&first, Health::Unready, 84).unwrap(), Decision::Observe); + assert_eq!( + policy.report().assessments[&first.deployment].reason, + Reason::BudgetExhausted + ); + assert_eq!(policy.budget().restarts[&first.deployment], 3); +} + /// Require renewed useful progress rather than deployment readiness alone to close an incident. #[test] fn readiness_does_not_establish_recovery_but_progress_does() { @@ -333,3 +421,31 @@ fn report_preserves_typed_checkpoint_measurements_and_distinguishes_pending_time assert_eq!(serde_json::from_value::(serialized).unwrap(), time); } } + +/// Classification reports observed conditions without asserting an unsupported root cause. +#[test] +fn classification_distinguishes_explicit_failure_stall_and_missing_observations() { + let worker = config().workloads[0].clone(); + let mut policy = supervisor(Mode::ObserveOnly); + policy + .event(&worker.agent_id, &event("started", 0, 1, "one"), 0) + .unwrap(); + let _stalled = policy.observe(&worker, Health::Ready, 180).unwrap(); + assert_eq!( + policy.report().assessments[&worker.deployment].classification, + Classification::ProgressStalled + ); + policy + .event(&worker.agent_id, &event("failed", 0, 2, "one"), 181) + .unwrap(); + let _failed = policy.observe(&worker, Health::Ready, 182).unwrap(); + assert_eq!( + policy.report().assessments[&worker.deployment].classification, + Classification::ApplicationFailure + ); + let _unknown = policy.observe(&worker, Health::Unknown, 183).unwrap(); + assert_eq!( + policy.report().assessments[&worker.deployment].classification, + Classification::ObservationUnknown + ); +} diff --git a/services/recovery-supervisor/tests/resilience.rs b/services/recovery-supervisor/tests/resilience.rs new file mode 100644 index 00000000..04e15e5d --- /dev/null +++ b/services/recovery-supervisor/tests/resilience.rs @@ -0,0 +1,538 @@ +//! Recovery coordination, durable context and stability preserve bounded actions and honest measurements. +#![cfg(test)] + +use et_recovery_supervisor::config::{Config, Mode, Workload}; +use et_recovery_supervisor::policy::{Action, Budget, Decision, Health, ObservationTime, Status, Supervisor}; +use et_recovery_supervisor::resilience::{Classification, Reason, Stability}; +use et_test_recovery::{config, event, report}; + +/// Create an active workload whose observed deployment failure has satisfied detection grace. +fn detected_policy(settings: Config) -> (Supervisor, Workload) { + let worker = settings.workloads[0].clone(); + let budget = Budget::new(settings.job_id.clone()); + let mut policy = Supervisor::new(settings, budget).unwrap(); + policy + .event(&worker.agent_id, &event("started", 0, 1, "one"), 0) + .unwrap(); + observe_unready_at(&mut policy, &worker, &[0, 3]); + (policy, worker) +} + +/// Apply a deterministic sequence of deployment-health observations without changing lifecycle evidence. +fn observe_unready_at(policy: &mut Supervisor, worker: &Workload, times: &[u64]) { + for &now in times { + let _observed = policy.observe(worker, Health::Unready, now).unwrap(); + } +} + +/// Configure two independently supervised deployments for fleet-admission tests. +fn two_worker_policy(mut settings: Config) -> (Supervisor, Workload, Workload) { + let first = settings.workloads[0].clone(); + let mut second = first.clone(); + second.deployment = "second".into(); + second.agent_id = "second".into(); + settings.workloads.push(second.clone()); + let budget = Budget::new(settings.job_id.clone()); + (Supervisor::new(settings, budget).unwrap(), first, second) +} + +/// Retry schedules respect both cooldowns and fleet spacing, and promise no action at or beyond the deadline. +#[test] +fn retry_schedule_is_pending_when_fleet_spacing_exhausts_the_incident_deadline() { + for (interval, expected) in [ + (0, ObservationTime::Recorded(43)), + (30, ObservationTime::Recorded(53)), + (217, ObservationTime::Pending), + (300, ObservationTime::Pending), + ] { + let mut settings = config(); + settings.mode = Mode::ControllerAssisted; + settings.resilience.action_interval_seconds = interval; + let (mut policy, worker) = detected_policy(settings); + policy.reserve_restart(&worker, 23).unwrap(); + let _waiting = policy.observe(&worker, Health::Unready, 24).unwrap(); + let assessment = &policy.report().assessments[&worker.deployment]; + assert_eq!(assessment.reason, Reason::RecoveryInProgress); + assert_eq!(assessment.next_action_at, expected); + assert_eq!(policy.budget().restarts[&worker.deployment], 1); + } +} + +/// Restore one useful work unit, leaving the separate stability assessment pending. +fn recovered_policy(settings: Config) -> (Supervisor, Workload) { + let (mut policy, worker) = detected_policy(settings); + policy + .event(&worker.agent_id, &event("progress", 1, 2, "one"), 4) + .unwrap(); + let _recovered = policy.observe(&worker, Health::Ready, 5).unwrap(); + (policy, worker) +} + +/// Dependency failure and missing observations suppress actions without resetting the incident deadline. +#[test] +fn dependencies_gate_restarts_and_are_reassessed_without_budget_consumption() { + let mut settings = config(); + settings.mode = Mode::ControllerAssisted; + let worker = settings.workloads[0].clone(); + let _previous = settings + .resilience + .dependencies + .insert(worker.deployment.clone(), vec!["storage".into()]); + let mut policy = Supervisor::new(settings, Budget::new("example-job".into())).unwrap(); + let _suspected = policy.observe(&worker, Health::Unready, 0).unwrap(); + assert_eq!(policy.observe(&worker, Health::Unready, 23).unwrap(), Decision::Observe); + assert_eq!( + report(&policy)["assessments"][&worker.deployment]["reason"], + "dependency-unknown" + ); + policy.dependency_observation("storage", Health::Unready).unwrap(); + assert_eq!(policy.observe(&worker, Health::Unready, 43).unwrap(), Decision::Observe); + assert_eq!( + policy.report().assessments[&worker.deployment].classification, + Classification::DependencyUnavailable + ); + policy.dependency_observation("storage", Health::Ready).unwrap(); + assert_eq!(policy.observe(&worker, Health::Unready, 44).unwrap(), Decision::Restart); + assert!(policy.budget().restarts.is_empty()); + assert!(policy.dependency_observation("foreign", Health::Ready).is_err()); + assert_eq!( + policy.report().assessments[&worker.deployment].next_action_at, + ObservationTime::Recorded(43) + ); + let _expired = policy.observe(&worker, Health::Unready, 240).unwrap(); + assert_eq!(policy.incidents()[0].status, Status::Unrecovered); +} + +/// Build a two-level dependency graph with a supervised intermediate deployment and an external storage service. +fn transitive_policy() -> (Supervisor, Workload, Workload) { + let mut settings = config(); + settings.mode = Mode::ControllerAssisted; + let mut dependency = settings.workloads[0].clone(); + dependency.deployment = "dependency".into(); + dependency.agent_id = "dependency".into(); + settings.workloads.push(dependency.clone()); + settings.resilience.dependencies = [ + ("example-worker".into(), vec!["dependency".into()]), + ("dependency".into(), vec!["storage".into()]), + ] + .into(); + let (policy, worker) = detected_policy(settings); + (policy, worker, dependency) +} + +/// Ready intermediate deployments cannot hide unavailable or unknown dependencies further down the graph. +#[test] +fn transitive_dependencies_block_reservations_until_the_entire_path_is_healthy() { + let (mut policy, worker, dependency) = transitive_policy(); + policy + .dependency_observation(&dependency.deployment, Health::Ready) + .unwrap(); + let _unknown = policy.observe(&worker, Health::Unready, 23).unwrap(); + assert_eq!( + policy.report().assessments[&worker.deployment].reason, + Reason::DependencyUnknown + ); + for (health, now, expected) in [ + (Health::Unready, 24, Reason::DependencyUnavailable), + (Health::Unknown, 25, Reason::DependencyUnknown), + (Health::Ready, 26, Reason::RestartEligible), + ] { + policy.dependency_observation("storage", health).unwrap(); + let _decision = policy.observe(&worker, Health::Unready, now).unwrap(); + let assessment = &policy.report().assessments[&worker.deployment]; + assert_eq!(assessment.reason, expected); + if expected != Reason::RestartEligible { + assert_eq!(assessment.next_action_at, ObservationTime::Pending); + let _blocked = policy.reserve_restart(&worker, now).unwrap_err(); + assert!(policy.budget().restarts.is_empty()); + assert_eq!(policy.incidents()[0].actions, Vec::::default()); + } + } + policy.reserve_restart(&worker, 26).unwrap(); + assert_eq!(policy.budget().restarts[&worker.deployment], 1); +} + +/// Deployment readiness does not make a supervised dependency healthy when its application fails or stalls. +#[test] +fn failed_or_stalled_dependencies_block_restarts_until_useful_work_resumes() { + for (failed, now) in [(true, 24), (false, 180)] { + let (mut policy, worker, dependency) = transitive_policy(); + policy + .dependency_observation(&dependency.deployment, Health::Ready) + .unwrap(); + policy.dependency_observation("storage", Health::Ready).unwrap(); + policy + .event(&dependency.agent_id, &event("started", 0, 1, "dependency-instance"), 0) + .unwrap(); + if failed { + policy + .event(&dependency.agent_id, &event("failed", 0, 2, "dependency-instance"), now) + .unwrap(); + } + assert_eq!( + policy.observe(&worker, Health::Unready, now).unwrap(), + Decision::Observe + ); + assert_eq!( + policy.report().assessments[&worker.deployment].reason, + Reason::DependencyUnavailable + ); + let _blocked = policy.reserve_restart(&worker, now).unwrap_err(); + assert!(policy.budget().restarts.is_empty()); + policy + .event( + &dependency.agent_id, + &event("progress", 1, 3, "dependency-instance"), + now + 1, + ) + .unwrap(); + assert_eq!( + policy.observe(&worker, Health::Unready, now + 1).unwrap(), + Decision::Restart + ); + policy.reserve_restart(&worker, now + 1).unwrap(); + assert_eq!(policy.budget().restarts[&worker.deployment], 1); + } +} + +/// Reservations serialize fleet admission even when several workloads were eligible in the same observation cycle. +#[test] +fn concurrent_recovery_and_action_interval_limits_apply_at_reservation() { + let mut settings = config(); + settings.mode = Mode::ControllerAssisted; + settings.resilience.max_concurrent_recoveries = 1; + settings.resilience.action_interval_seconds = 30; + let (mut policy, first, second) = two_worker_policy(settings); + for worker in [&first, &second] { + let _suspected = policy.observe(worker, Health::Unready, 0).unwrap(); + let _detected = policy.observe(worker, Health::Unready, 3).unwrap(); + assert_eq!(policy.observe(worker, Health::Unready, 23).unwrap(), Decision::Restart); + } + policy.reserve_restart(&first, 23).unwrap(); + policy.action_result(&first, Action::RestartRequested).unwrap(); + assert!(policy.reserve_restart(&second, 23).is_err()); + let _limited = policy.observe(&second, Health::Unready, 24).unwrap(); + assert_eq!(policy.report().assessments["second"].reason, Reason::ConcurrencyLimit); + policy + .event(&first.agent_id, &event("started", 0, 1, "one"), 24) + .unwrap(); + policy + .event(&first.agent_id, &event("progress", 1, 2, "one"), 25) + .unwrap(); + let _recovered = policy.observe(&first, Health::Ready, 25).unwrap(); + let _delayed = policy.observe(&second, Health::Unready, 26).unwrap(); + assert_eq!(policy.report().assessments["second"].reason, Reason::ActionInterval); + assert_eq!(policy.observe(&second, Health::Unready, 53).unwrap(), Decision::Restart); + policy.reserve_restart(&second, 53).unwrap(); +} + +/// An expired recovery on an unpolled workload must not retain the fleet's only admission slot. +#[test] +fn expired_in_flight_recovery_does_not_block_a_later_incident() { + let mut settings = config(); + settings.mode = Mode::ControllerAssisted; + settings.resilience.max_concurrent_recoveries = 1; + let (mut policy, first, second) = two_worker_policy(settings); + observe_unready_at(&mut policy, &first, &[0, 3]); + policy.reserve_restart(&first, 23).unwrap(); + for now in [200, 203, 223] { + let _observed = policy.observe(&second, Health::Unready, now).unwrap(); + } + assert_eq!(policy.report().assessments["second"].reason, Reason::ConcurrencyLimit); + assert_eq!( + policy.observe(&second, Health::Unready, 240).unwrap(), + Decision::Restart + ); + assert_eq!(policy.incidents()[0].status, Status::Detected); + policy.reserve_restart(&second, 240).unwrap(); + assert_eq!(policy.budget().restarts["second"], 1); +} + +/// A terminal incident in an external report cannot consume capacity through its stale reservation marker. +#[test] +fn restored_terminal_incident_with_a_stale_reservation_does_not_block_the_fleet() { + let mut settings = config(); + settings.mode = Mode::ControllerAssisted; + settings.resilience.max_concurrent_recoveries = 1; + let deadline = settings.policy.deadline_seconds; + let (mut policy, first, second) = two_worker_policy(settings.clone()); + settings.workloads.push(second.clone()); + observe_unready_at(&mut policy, &first, &[0, 3]); + policy.reserve_restart(&first, 23).unwrap(); + for now in [24, 27] { + let _observed = policy.observe(&second, Health::Unready, now).unwrap(); + } + let mut saved = policy.report(); + saved.incidents[0].status = Status::Unrecovered; + let mut restored = Supervisor::restore(settings, serde_json::to_value(saved).unwrap()).unwrap(); + let now = restored.elapsed(20); + assert!(now < deadline, "the status guard must act independently of expiry"); + assert_eq!( + restored.observe(&second, Health::Unready, now).unwrap(), + Decision::Restart + ); + restored.reserve_restart(&second, now).unwrap(); + assert_eq!(restored.incidents()[0].status, Status::Unrecovered); + assert_eq!(restored.budget().restarts[&first.deployment], 1); + assert_eq!(restored.budget().restarts[&second.deployment], 1); +} + +/// The action boundary enforces the deadline even before the next observation can mark an incident unrecovered. +#[test] +fn restart_reservation_cannot_outlive_an_unpolled_incident_deadline() { + let mut settings = config(); + settings.mode = Mode::ControllerAssisted; + let deadline = settings.policy.deadline_seconds; + let (mut policy, worker) = detected_policy(settings); + assert_eq!(policy.incidents()[0].status, Status::Detected); + let error = policy.reserve_restart(&worker, deadline).unwrap_err(); + assert!(error.to_string().contains("not eligible")); + assert!(policy.budget().restarts.is_empty()); + assert_eq!(policy.incidents()[0].actions, Vec::::default()); + let _expired = policy.observe(&worker, Health::Unready, deadline).unwrap(); + assert_eq!(policy.incidents()[0].status, Status::Unrecovered); +} + +/// Reconnection needs fresh evidence for unfinished work, while completed jobs need no new training activity. +#[test] +fn reconnect_evidence_requirements_distinguish_completed_and_unfinished_work() { + for completed in [false, true] { + let mut settings = config(); + settings.mode = Mode::ControllerAssisted; + let (mut policy, worker) = detected_policy(settings); + if completed { + policy + .event(&worker.agent_id, &event("completed", 0, 2, "one"), 4) + .unwrap(); + } + policy.transport_lost(5, "hub disconnected"); + assert!(policy.reserve_restart(&worker, 23).is_err()); + policy.transport_connected(24); + let decision = policy.observe(&worker, Health::Unready, 24).unwrap(); + if completed { + assert_eq!(decision, Decision::Restart); + policy.reserve_restart(&worker, 24).unwrap(); + assert_eq!(policy.budget().restarts[&worker.deployment], 1); + } else { + assert_eq!(decision, Decision::Observe); + assert_eq!( + policy.report().assessments[&worker.deployment].reason, + Reason::EvidenceRequired + ); + assert!(policy.budget().restarts.is_empty()); + } + } +} + +/// Backoff changes later interventions, keeps the initial delay and never exceeds the configured cap. +#[test] +fn exponential_backoff_and_jitter_are_bounded_and_do_not_reset_budgets() { + let mut settings = config(); + settings.mode = Mode::ControllerAssisted; + settings.policy.max_restarts = 3; + settings.resilience.backoff_multiplier = 2; + settings.resilience.backoff_max_seconds = 60; + settings.resilience.jitter_seconds = 3; + for seed in 0..32 { + assert_eq!(settings.resilience.cooldown(20, 0, "worker", seed), 20); + assert!((20..=23).contains(&settings.resilience.cooldown(20, 1, "worker", seed))); + assert!((40..=43).contains(&settings.resilience.cooldown(20, 2, "worker", seed))); + assert_eq!(settings.resilience.cooldown(20, 20, "worker", seed), 60); + } + let (mut policy, worker) = detected_policy(settings); + policy.reserve_restart(&worker, 23).unwrap(); + policy + .action_result( + &worker, + Action::RestartFailed { + error: "temporary API failure".into(), + }, + ) + .unwrap(); + assert_eq!(policy.observe(&worker, Health::Unready, 42).unwrap(), Decision::Observe); + policy.reserve_restart(&worker, 46).unwrap(); + policy.action_result(&worker, Action::RestartRequested).unwrap(); + assert_eq!(policy.observe(&worker, Health::Unready, 66).unwrap(), Decision::Observe); + assert_eq!(policy.observe(&worker, Health::Unready, 89).unwrap(), Decision::Restart); + assert_eq!(policy.budget().restarts[&worker.deployment], 2); +} + +/// First useful recovery remains the MTTR boundary while stability requires subsequent progress or completion. +#[test] +fn stability_does_not_inflate_mttr_or_require_completed_work_to_run_again() { + for completed in [false, true] { + let mut settings = config(); + settings.resilience.stability_seconds = 5; + let (mut policy, worker) = recovered_policy(settings); + assert_eq!(policy.incidents()[0].recovered, ObservationTime::Recorded(5)); + let _quiet = policy.observe(&worker, Health::Ready, 10).unwrap(); + assert!(matches!(policy.incidents()[0].stability, Stability::Pending { .. })); + let action = if completed { "completed" } else { "progress" }; + let progress = if completed { 1 } else { 2 }; + policy + .event(&worker.agent_id, &event(action, progress, 3, "one"), 11) + .unwrap(); + let _stable = policy.observe(&worker, Health::Ready, 12).unwrap(); + assert_eq!(policy.incidents()[0].stability, Stability::Stable { verified_at: 12 }); + assert_eq!(policy.report().recovery_seconds, vec![5]); + } +} + +/// Missing observations cannot certify stability; recurrent failure remains a separate incident. +#[test] +fn stability_records_unknown_observations_and_recurrent_failures() { + for health in [Health::Unknown, Health::Unready] { + let mut settings = config(); + settings.resilience.stability_seconds = 5; + let (mut policy, worker) = recovered_policy(settings); + let _next = policy.observe(&worker, health, 6).unwrap(); + let expected = if health == Health::Unknown { + Stability::Incomplete + } else { + Stability::Unstable { observed_at: 6 } + }; + assert_eq!(policy.incidents()[0].stability, expected); + assert_eq!(policy.incidents()[0].recovered, ObservationTime::Recorded(5)); + } +} + +/// Recover useful progress while retaining a five-second stability window for later observations. +fn pending_stability_policy() -> (Supervisor, Workload) { + let mut settings = config(); + settings.resilience.stability_seconds = 5; + recovered_policy(settings) +} + +/// A ready deployment cannot certify stability after an application failure or an unexpected process replacement. +#[test] +fn stability_rejects_application_failure_and_replacement_even_when_the_deployment_is_ready() { + for (kind, progress, sequence, instance) in [("failed", 1, 3, "one"), ("started", 1, 1, "replacement")] { + let (mut policy, worker) = pending_stability_policy(); + policy + .event(&worker.agent_id, &event(kind, progress, sequence, instance), 6) + .unwrap(); + let _observed = policy.observe(&worker, Health::Ready, 6).unwrap(); + assert_eq!(policy.incidents()[0].stability, Stability::Unstable { observed_at: 6 }); + assert_eq!(policy.incidents()[0].status, Status::Recovered); + assert_eq!(policy.incidents()[0].recovered, ObservationTime::Recorded(5)); + assert_eq!(policy.report().recovery_seconds, vec![5]); + } +} + +/// Fresh progress or completion cannot shorten the stability window; the exact boundary can certify recovery. +#[test] +fn early_progress_and_completion_wait_for_the_full_stability_window() { + for (kind, progress) in [("progress", 2), ("completed", 1)] { + let (mut policy, worker) = pending_stability_policy(); + policy + .event(&worker.agent_id, &event(kind, progress, 3, "one"), 6) + .unwrap(); + for now in [6, 9] { + let _early = policy.observe(&worker, Health::Ready, now).unwrap(); + assert!(matches!( + policy.incidents()[0].stability, + Stability::Pending { since: 5, .. } + )); + } + let _boundary = policy.observe(&worker, Health::Ready, 10).unwrap(); + assert_eq!(policy.incidents()[0].stability, Stability::Stable { verified_at: 10 }); + assert_eq!(policy.report().recovery_seconds, vec![5]); + } +} + +/// A hub disconnect or supervisor replacement cannot certify stability across a missing-observation interval. +#[test] +fn transport_loss_interrupts_stability_without_changing_the_recovery_time() { + for restore in [false, true] { + let mut settings = config(); + settings.resilience.stability_seconds = 5; + let (mut policy, worker) = recovered_policy(settings.clone()); + assert!(matches!(policy.incidents()[0].stability, Stability::Pending { .. })); + if restore { + policy = Supervisor::restore(settings, report(&policy)).unwrap(); + } else { + policy.transport_lost(6, "hub disconnected during stability verification"); + } + assert_eq!(policy.incidents()[0].stability, Stability::Incomplete); + assert_eq!(policy.incidents()[0].status, Status::Recovered); + assert_eq!(policy.report().recovery_seconds, vec![5]); + assert_eq!(policy.report().observation_gaps.len(), 1); + let now = policy.elapsed(10); + policy.transport_connected(now); + policy + .event(&worker.agent_id, &event("progress", 2, 3, "one"), now + 1) + .unwrap(); + let _ready = policy.observe(&worker, Health::Ready, now + 2).unwrap(); + assert_eq!(policy.incidents()[0].stability, Stability::Incomplete); + assert_eq!(policy.incidents().len(), 1); + assert_eq!(policy.report().recovery_seconds, vec![5]); + } +} + +/// Transport loss fences interventions until renewed evidence, retaining sequence and instance protections. +#[test] +fn reconnect_requires_renewed_evidence_and_keeps_the_original_incident() { + let mut settings = config(); + settings.mode = Mode::ControllerAssisted; + let (mut policy, worker) = detected_policy(settings); + policy.transport_lost(10, "hub disconnected"); + policy.transport_connected(30); + assert_eq!(policy.observe(&worker, Health::Unready, 31).unwrap(), Decision::Observe); + assert_eq!( + policy.report().assessments[&worker.deployment].reason, + Reason::EvidenceRequired + ); + assert!( + policy + .event(&worker.agent_id, &event("started", 0, 1, "one"), 31) + .is_err() + ); + policy + .event(&worker.agent_id, &event("progress", 0, 2, "one"), 32) + .unwrap(); + assert_eq!(policy.observe(&worker, Health::Unready, 33).unwrap(), Decision::Restart); + assert_eq!(policy.incidents().len(), 1); + assert_eq!(policy.incidents()[0].onset, 0); + assert_eq!( + policy.report().observation_gaps[0].ended_at, + ObservationTime::Recorded(30) + ); + policy + .event(&worker.agent_id, &event("progress", 1, 3, "one"), 34) + .unwrap(); + policy.transport_lost(35, "another interruption"); + policy.transport_connected(36); + let _stale = policy.observe(&worker, Health::Ready, 37).unwrap(); + assert_eq!(policy.incidents()[0].status, Status::Detected); + policy + .event(&worker.agent_id, &event("progress", 2, 4, "one"), 38) + .unwrap(); + let _renewed = policy.observe(&worker, Health::Ready, 39).unwrap(); + assert_eq!(policy.incidents()[0].status, Status::Recovered); +} + +/// Dependency permissions allow read-only observation but never authorize rollout mutation of supporting services. +#[tokio::test] +async fn supporting_dependency_is_not_an_intervention_target() { + let mut settings = config(); + let _previous = settings + .resilience + .dependencies + .insert("example-worker".into(), vec!["storage".into()]); + let mut target = settings.workloads[0].clone(); + target.deployment = "storage".into(); + let backend = et_recovery_supervisor::kubernetes::Kubernetes::new(settings).unwrap(); + let error = backend.execute(&target, &["rollout", "restart"]).await.unwrap_err(); + assert!(error.to_string().contains("allowlisted")); +} + +/// Additional read-only dependencies increase the allowed cycle duration without weakening ownership probes. +#[test] +fn health_budget_includes_dependency_observations() { + let mut settings = config(); + let _previous = settings.resilience.dependencies.insert( + "example-worker".into(), + vec!["storage".into(), "example-worker-other".into()], + ); + assert_eq!(et_recovery_supervisor::health::silence_limit(&settings).as_secs(), 52); +} diff --git a/services/recovery-supervisor/tests/runtime.rs b/services/recovery-supervisor/tests/runtime.rs index 3ba0bf17..1383f8c5 100644 --- a/services/recovery-supervisor/tests/runtime.rs +++ b/services/recovery-supervisor/tests/runtime.rs @@ -9,7 +9,7 @@ use et_recovery_supervisor::config::{Mode, Workload}; use et_recovery_supervisor::kubernetes::{Backend, Kubernetes}; use et_recovery_supervisor::persistence::StateStore; use et_recovery_supervisor::policy::{Action, Health, Supervisor}; -use et_recovery_supervisor::runtime::{cycle, hub, load_budget, run}; +use et_recovery_supervisor::runtime::{cycle, hub, load_budget, load_supervisor, run}; use et_test_recovery::{config, environment, event, report, supervisor}; use fs_err as fs; use futures_util::{SinkExt as _, StreamExt as _}; @@ -68,6 +68,97 @@ impl Backend for FakeBackend { } } +/// Only an absent file starts a fresh supervisor; unreadable or invalid saved state fails closed. +#[tokio::test] +async fn supervisor_loading_preserves_committed_state_and_rejects_invalid_storage() { + let directory = tempfile::tempdir().unwrap(); + let mut settings = config(); + settings.state_file = directory.path().join("state.json"); + let fresh = load_supervisor(&settings).unwrap(); + assert!(fresh.budget().restarts.is_empty()); + let mut saved = report(&fresh); + saved["restart_budget"]["restarts"]["example-worker"] = serde_json::json!(1_u32); + StateStore::new(settings.state_file.clone()) + .persist(saved) + .await + .unwrap(); + let restored = load_supervisor(&settings).unwrap(); + assert_eq!(restored.budget().restarts["example-worker"], 1); + for bytes in [b"invalid json".as_slice(), b"{}".as_slice()] { + fs::write(&settings.state_file, bytes).unwrap(); + let _invalid = load_supervisor(&settings).unwrap_err(); + } + fs::remove_file(&settings.state_file).unwrap(); + fs::create_dir(&settings.state_file).unwrap(); + let _unreadable = load_supervisor(&settings).unwrap_err(); +} + +struct DependencyBackend { + dependency_health: Health, + observed: Vec, + restarted: Vec, +} + +impl Backend for DependencyBackend { + /// Record every target so supporting services cannot be silently omitted or observed twice. + fn health(&mut self, workload: &Workload) -> impl Future> + Send { + self.observed.push(workload.deployment.clone()); + std::future::ready(Ok(if workload.deployment == "storage" { + self.dependency_health + } else { + Health::Unready + })) + } + + /// Record action targets to distinguish dependency observation from permission to restart it. + fn restart(&mut self, workload: &Workload) -> impl Future> + Send { + self.restarted.push(workload.deployment.clone()); + std::future::ready(Ok(())) + } +} + +/// Read dependencies before deciding on restarts, whether or not they are themselves supervised. +#[tokio::test] +async fn cycles_observe_dependencies_once_and_gate_actions_on_their_current_health() { + for supervised in [false, true] { + for health in [Health::Ready, Health::Unready, Health::Unknown] { + let directory = tempfile::tempdir().unwrap(); + let mut settings = config(); + settings.mode = Mode::ControllerAssisted; + settings.state_file = directory.path().join("state.json"); + let _previous = settings + .resilience + .dependencies + .insert("example-worker".into(), vec!["storage".into()]); + if supervised { + let mut dependency = settings.workloads[0].clone(); + dependency.deployment = "storage".into(); + dependency.agent_id = "storage".into(); + settings.workloads.push(dependency); + } + let mut policy = Supervisor::new(settings.clone(), load_budget(&settings).unwrap()).unwrap(); + let store = StateStore::new(settings.state_file.clone()); + let mut backend = DependencyBackend { + dependency_health: health, + observed: Vec::new(), + restarted: Vec::new(), + }; + for now in [0, 3, 23] { + cycle(&settings, &store, &mut policy, &mut backend, now).await.unwrap(); + } + assert_eq!(backend.observed.iter().filter(|name| *name == "storage").count(), 3); + assert_eq!( + backend.restarted.contains(&"example-worker".into()), + health == Health::Ready + ); + if !supervised { + assert!(!backend.restarted.contains(&"storage".into())); + assert!(!policy.report().assessments.contains_key("storage")); + } + } + } +} + /// Persist restart reservations before backend calls and retain attempts even when execution fails. #[tokio::test] async fn cycle_persists_budget_before_action_and_retains_failed_attempts() { @@ -291,7 +382,8 @@ impl Backend for ReadyBackend { async fn idle_control_rejects_invalid_evidence_without_losing_subsequent_valid_work() { let directory = tempfile::tempdir().unwrap(); let server = et_ws_test_server::start(); - let config = connected_config(&server, directory.path(), 3600); + let mut config = connected_config(&server, directory.path(), 3600); + config.resilience.reconnect = false; let (sender, mut rejected) = tokio::sync::mpsc::unbounded_channel(); let subscriber = tracing_subscriber::registry().with(RejectedEvidence(sender)); let task = tokio::spawn(run(config.clone(), ReadyBackend).with_subscriber(subscriber)); @@ -321,6 +413,74 @@ async fn idle_control_rejects_invalid_evidence_without_losing_subsequent_valid_w assert_eq!(saved["incidents"], serde_json::json!([])); } +/// A replacement hub at the same endpoint restores supervision without replacing its owner or resetting state. +#[tokio::test] +async fn supervisor_reconnects_to_a_replacement_hub_and_retains_lifecycle_context() { + let directory = tempfile::tempdir().unwrap(); + let server = et_ws_test_server::start(); + let port = url::Url::parse(&server.ws_url).unwrap().port().unwrap(); + let mut config = connected_config(&server, directory.path(), 1); + config.resilience.connect_timeout_seconds = 1; + let task = tokio::spawn(run(config.clone(), ReadyBackend)); + let (mut producer, _) = et_ws_test_server::connect_agent_as(&server.ws_url, Some("example-worker")).await; + wait_for_agent(&mut producer, &config.agent_id).await; + send_evidence(&mut producer, &config.agent_id, event("started", 0, 1, "one")).await; + wait_for_sequence(&config, 1).await; + server.stop().await; + wait_for_registration_retries(&config).await; + assert!(!task.is_finished()); + let replacement = et_ws_test_server::start_on(port); + let (mut producer, _) = et_ws_test_server::connect_agent_as(&replacement.ws_url, Some("example-worker")).await; + wait_for_agent(&mut producer, &config.agent_id).await; + send_evidence(&mut producer, &config.agent_id, event("progress", 1, 2, "one")).await; + wait_for_sequence(&config, 2).await; + assert!(!task.is_finished()); + let saved: serde_json::Value = serde_json::from_slice(&fs::read(&config.state_file).unwrap()).unwrap(); + assert!(saved["observation_gaps"].as_array().unwrap().last().unwrap()["ended_at"].is_number()); + task.abort(); + assert!(task.await.unwrap_err().is_cancelled()); +} + +/// Leave the hub unavailable across bounded registration attempts and observe their persisted gap. +#[expect( + clippy::single_call_fn, + reason = "registration retry waiting is distinct from sequence advancement" +)] +async fn wait_for_registration_retries(config: &et_recovery_supervisor::config::Config) { + tokio::time::timeout(Duration::from_secs(10), async { + loop { + let saved: serde_json::Value = serde_json::from_slice(&fs::read(&config.state_file).unwrap()).unwrap(); + if saved["context"]["elapsed_seconds"].as_u64().unwrap() >= 3 + && saved["observation_gaps"] + .as_array() + .unwrap() + .last() + .is_some_and(|gap| gap["ended_at"].is_null()) + { + break; + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + }) + .await + .unwrap(); +} + +/// Observe persisted sequence advancement rather than assume that a transport acknowledgement committed policy state. +async fn wait_for_sequence(config: &et_recovery_supervisor::config::Config, expected: u64) { + tokio::time::timeout(Duration::from_secs(10), async { + loop { + let saved: serde_json::Value = serde_json::from_slice(&fs::read(&config.state_file).unwrap()).unwrap(); + if saved["context"]["states"]["example-worker"]["sequence"] == expected { + break; + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + }) + .await + .unwrap(); +} + /// Complete registration and the initial poll before sending events that must be handled by the idle control loop. #[expect( clippy::single_call_fn, diff --git a/services/recovery-supervisor/tests/unit/policy_guards.rs b/services/recovery-supervisor/tests/unit/policy_guards.rs new file mode 100644 index 00000000..a09865a0 --- /dev/null +++ b/services/recovery-supervisor/tests/unit/policy_guards.rs @@ -0,0 +1,144 @@ +//! Private invariant checks remain fail-closed even when callers supply inconsistent scheduling state. +#![cfg(test)] + +use super::{Budget, Decision, Health, ObservationTime, Reason, Status, Supervisor, Workload}; +use crate::RecoveryError; +use crate::config::{EnvConfig, Mode}; + +/// Establish a real detected incident through the public observation API before testing its private guards. +fn detected_supervisor() -> (Supervisor, Workload) { + let settings: EnvConfig = serde_env::from_iter(et_test_recovery::environment()).unwrap(); + let mut config = settings.recovery; + config.mode = Mode::ControllerAssisted; + let workload = config.workloads[0].clone(); + let budget = Budget::new(config.job_id.clone()); + let mut supervisor = Supervisor::new(config, budget).unwrap(); + assert_eq!( + supervisor.observe(&workload, Health::Unready, 0).unwrap(), + Decision::Observe + ); + assert_eq!( + supervisor.observe(&workload, Health::Unready, 3).unwrap(), + Decision::Observe + ); + (supervisor, workload) +} + +/// Each reservation invariant independently rejects the action, including every non-detected incident status. +#[test] +fn reservation_recheck_rejects_disabled_mode_exhausted_budget_cooldown_and_closed_incidents() { + for (mode, attempts, now, status) in [ + (Mode::ObserveOnly, 0, 23, Status::Detected), + (Mode::ControllerAssisted, 2, 23, Status::Detected), + (Mode::ControllerAssisted, 0, 22, Status::Detected), + (Mode::ControllerAssisted, 0, 2, Status::Detected), + (Mode::ControllerAssisted, 0, 23, Status::Suspected), + (Mode::ControllerAssisted, 0, 23, Status::Recovered), + (Mode::ControllerAssisted, 0, 23, Status::Transient), + (Mode::ControllerAssisted, 0, 23, Status::Unrecovered), + ] { + let (mut supervisor, workload) = detected_supervisor(); + supervisor.config.mode = mode; + supervisor.incidents[0].status = status; + let error = Supervisor::validate_reservation( + &supervisor.config, + &supervisor.states[&workload.deployment], + &supervisor.incidents[0], + attempts, + now, + ) + .unwrap_err(); + assert!(matches!(error, RecoveryError::Invalid(_))); + assert!(error.to_string().contains("restart is not eligible")); + assert!(supervisor.budget.restarts.is_empty()); + assert_eq!(supervisor.incidents[0].actions, Vec::::new()); + } +} + +/// The exact cooldown boundary permits reservation while the last available restart remains in budget. +#[test] +fn reservation_recheck_accepts_the_cooldown_boundary_and_remaining_budget() { + let (supervisor, workload) = detected_supervisor(); + for attempts in [0, 1] { + Supervisor::validate_reservation( + &supervisor.config, + &supervisor.states[&workload.deployment], + &supervisor.incidents[0], + attempts, + 23, + ) + .unwrap(); + } +} + +/// A scheduling reason cannot promise a retry time for a target absent from the configured state map. +#[test] +fn unknown_workload_has_no_promised_action_time() { + let (supervisor, mut workload) = detected_supervisor(); + workload.deployment = "unconfigured-worker".into(); + assert_eq!( + supervisor.next_action_time(&workload, Reason::RestartEligible), + ObservationTime::Pending + ); +} + +/// A stale cooldown assessment cannot promise an action after the workload has returned to its healthy phase. +#[test] +fn healthy_workload_has_no_promised_action_time() { + let (supervisor, workload) = detected_supervisor(); + let healthy = Supervisor::new(supervisor.config, supervisor.budget).unwrap(); + assert_eq!( + healthy.next_action_time(&workload, Reason::Cooldown), + ObservationTime::Pending + ); +} + +/// An inconsistent incident reference fails closed instead of reporting an action time or panicking. +#[test] +fn missing_incident_has_no_promised_action_time() { + let (mut supervisor, workload) = detected_supervisor(); + assert_eq!( + supervisor.next_action_time(&workload, Reason::RestartEligible), + ObservationTime::Recorded(23) + ); + supervisor.incidents.clear(); + assert_eq!( + supervisor.next_action_time(&workload, Reason::RestartEligible), + ObservationTime::Pending + ); +} + +/// The post-eligibility step cannot reserve a deployment absent from the configured state map. +#[test] +fn reservation_step_rejects_unknown_workloads() { + let (mut supervisor, mut workload) = detected_supervisor(); + workload.deployment = "unconfigured-worker".into(); + let error = supervisor.reserve_eligible_restart(&workload, 23).unwrap_err(); + assert!(error.to_string().contains("restart target is not configured")); + assert!(supervisor.budget.restarts.is_empty()); + assert_eq!(supervisor.incidents[0].actions, Vec::::new()); +} + +/// A stale eligibility decision cannot reserve a restart after the workload returns to its healthy phase. +#[test] +fn reservation_step_rejects_workloads_without_an_incident() { + let (supervisor, workload) = detected_supervisor(); + let mut healthy = Supervisor::new(supervisor.config, supervisor.budget).unwrap(); + let error = healthy.reserve_eligible_restart(&workload, 23).unwrap_err(); + assert!(error.to_string().contains("no incident permits this restart")); + assert!(healthy.budget.restarts.is_empty()); + assert_eq!(healthy.last_reservation, ObservationTime::Pending); +} + +/// An invalid incident reference cannot consume budget, reserve fleet capacity or move the action clock. +#[test] +fn reservation_step_rejects_missing_incident_state() { + let (mut supervisor, workload) = detected_supervisor(); + supervisor.incidents.clear(); + let error = supervisor.reserve_eligible_restart(&workload, 23).unwrap_err(); + assert!(error.to_string().contains("incident state is absent")); + assert_eq!(supervisor.budget.restarts[&workload.deployment], 0); + assert!(!supervisor.states[&workload.deployment].in_flight); + assert_eq!(supervisor.states[&workload.deployment].last_action, 3); + assert_eq!(supervisor.last_reservation, ObservationTime::Pending); +} diff --git a/services/storage/Cargo.toml b/services/storage/Cargo.toml index 1e24d6bd..99e2c959 100644 --- a/services/storage/Cargo.toml +++ b/services/storage/Cargo.toml @@ -61,6 +61,7 @@ actix-rt.workspace = true command-error.workspace = true et-test-helpers.workspace = true object_store.workspace = true +reqwest.workspace = true temp-env = { workspace = true, features = ["async_closure"] } tempfile.workspace = true diff --git a/services/storage/tests/s3_backend.rs b/services/storage/tests/s3_backend.rs index ebb9b864..c80b440f 100644 --- a/services/storage/tests/s3_backend.rs +++ b/services/storage/tests/s3_backend.rs @@ -24,6 +24,7 @@ use std::collections::BTreeMap; use std::net::{Ipv4Addr, SocketAddr}; use std::path::Path; use std::process::{Command, Stdio}; +use std::time::Duration; use actix_web::http::StatusCode; use actix_web::{App, test, web}; @@ -78,6 +79,8 @@ fn start_rustfs(volume: &Path, port: u16, log_path: &Path) -> ChildGuard { let stdout = log_file.file().try_clone().unwrap(); let stderr = log_file.file().try_clone().unwrap(); let child = Command::new("rustfs") + // The readiness probe is required by this fixture, regardless of the parent's RustFS settings. + .env("RUSTFS_HEALTH_ENDPOINT_ENABLE", "true") .arg("server") .arg(volume) .arg("--address") @@ -100,6 +103,40 @@ fn start_rustfs(volume: &Path, port: u16, log_path: &Path) -> ChildGuard { guard } +/// Wait for storage and IAM initialization, which can finish after the HTTP listener opens. +#[expect( + clippy::single_call_fn, + reason = "server readiness is distinct from accepting a TCP connection" +)] +async fn wait_for_ready(port: u16, log_path: &Path) { + let client = reqwest::Client::builder() + .timeout(Duration::from_secs(1)) + .build() + .unwrap(); + let endpoint = format!("http://127.0.0.1:{port}/health/ready"); + let mut last_response = String::default(); + let ready = actix_rt::time::timeout(Duration::from_secs(30), async { + loop { + match client.get(&endpoint).send().await { + Ok(response) if response.status().is_success() => break, + Ok(response) => { + let status = response.status(); + let body = response.text().await.unwrap_or_else(|error| error.to_string()); + last_response = format!("{status}: {body}"); + } + Err(error) => last_response = error.to_string(), + } + actix_rt::time::sleep(Duration::from_millis(50)).await; + } + }) + .await; + assert!( + ready.is_ok(), + "rustfs storage did not become ready: {last_response}\n--- rustfs output ---\n{}", + fs_err::read_to_string(log_path).unwrap_or_default() + ); +} + /// The `AWS_*` pairs `object_store`'s S3 builder consumes, pointing it at the local rustfs. /// /// `AWS_ALLOW_HTTP` is required because the endpoint is plain HTTP; addressing stays path-style, which is @@ -149,10 +186,11 @@ async fn round_trips_put_and_get_through_the_s3_backend() { let log_path = log_dir.path().join("rustfs.log"); let port = reserve_port(); let mut rustfs = start_rustfs(volume.path(), port, &log_path); + wait_for_ready(port, &log_path).await; let config = StorageConfig::new(format!("s3://{BUCKET}")); - let (put_status, get_status, body) = temp_env::async_with_vars(aws_env(port), async { + let (put_status, put_body, get_status, body) = temp_env::async_with_vars(aws_env(port), async { let app = test::init_service( App::new() .app_data(web::Data::new(registry_with_agent(AGENT))) @@ -165,7 +203,9 @@ async fn round_trips_put_and_get_through_the_s3_backend() { .peer_addr(SocketAddr::new(Ipv4Addr::LOCALHOST.into(), 40123)) .set_payload(BODY) .to_request(); - let put_status = test::call_service(&app, put).await.status(); + let put_response = test::call_service(&app, put).await; + let put_status = put_response.status(); + let put_body = test::read_body(put_response).await; let get = test::TestRequest::get() .uri(&format!("/storage/{AGENT}/{FILENAME}")) @@ -174,7 +214,7 @@ async fn round_trips_put_and_get_through_the_s3_backend() { let get_status = get_resp.status(); let body = test::read_body(get_resp).await; - (put_status, get_status, body) + (put_status, put_body, get_status, body) }) .await; @@ -185,7 +225,8 @@ async fn round_trips_put_and_get_through_the_s3_backend() { assert_eq!( put_status, StatusCode::OK, - "PUT through the S3 backend should succeed\n--- rustfs output ---\n{}", + "PUT through the S3 backend should succeed\n--- PUT response ---\n{}\n--- rustfs output ---\n{}", + String::from_utf8_lossy(&put_body), rustfs_log() ); assert_eq!( diff --git a/utilities/cli/src/deployment_types/k3s_recovery.rs b/utilities/cli/src/deployment_types/k3s_recovery.rs index cdc6b625..a53321ea 100644 --- a/utilities/cli/src/deployment_types/k3s_recovery.rs +++ b/utilities/cli/src/deployment_types/k3s_recovery.rs @@ -85,21 +85,31 @@ fn role(settings: &RecoverySettings, namespace: &str) -> Role { } else { vec!["get"] }; - Role { - metadata: plain_meta(namespace, NAME), - rules: Some(vec![PolicyRule { + let mut rules = vec![PolicyRule { + api_groups: Some(vec!["apps".into()]), + resources: Some(vec!["deployments".into()]), + resource_names: Some(settings.workloads.iter().map(|item| item.deployment.clone()).collect()), + verbs: verbs.into_iter().map(str::to_owned).collect(), + ..PolicyRule::default() + }]; + let dependencies: Vec<_> = settings + .resilience + .dependency_names() + .into_iter() + .filter(|name| !settings.workloads.iter().any(|item| &item.deployment == name)) + .collect(); + if !dependencies.is_empty() { + rules.push(PolicyRule { api_groups: Some(vec!["apps".into()]), resources: Some(vec!["deployments".into()]), - resource_names: Some( - settings - .workloads - .iter() - .map(|workload| workload.deployment.clone()) - .collect(), - ), - verbs: verbs.into_iter().map(str::to_owned).collect(), + resource_names: Some(dependencies), + verbs: vec!["get".into()], ..PolicyRule::default() - }]), + }); + } + Role { + metadata: plain_meta(namespace, NAME), + rules: Some(rules), } } @@ -220,7 +230,7 @@ fn health_probes(config: &et_recovery_supervisor::config::Config) -> Result<(Pro }; let startup_seconds = et_recovery_supervisor::health::silence_limit(config) .as_secs() - .saturating_add(30); + .saturating_add(config.resilience.connect_timeout_seconds); let failure_threshold = match i32::try_from(startup_seconds) { Ok(seconds) => seconds, Err(error) => { diff --git a/utilities/cli/src/recovery.rs b/utilities/cli/src/recovery.rs index 50cc054b..ed9fcf62 100644 --- a/utilities/cli/src/recovery.rs +++ b/utilities/cli/src/recovery.rs @@ -84,6 +84,9 @@ pub struct RecoverySettings { pub policy: Policy, /// Existing, continuous native-Python deployments and their application evidence identities. pub workloads: Vec, + /// Optional safeguards for transport, dependencies, coordinated actions and sustained recovery. + #[serde(default)] + pub resilience: et_recovery_supervisor::resilience::Settings, /// Limits for the supervisor's own container. #[serde(default, skip_serializing_if = "Limits::is_unset")] pub limits: Limits, @@ -102,6 +105,7 @@ impl RecoverySettings { config.mode = self.mode; config.poll_seconds = self.poll_seconds; config.command_timeout_seconds = self.command_timeout_seconds; + config.resilience.clone_from(&self.resilience); config.validate()?; Ok(config) } diff --git a/utilities/cli/tests/recovery.rs b/utilities/cli/tests/recovery.rs index e38c6a5d..432e837b 100644 --- a/utilities/cli/tests/recovery.rs +++ b/utilities/cli/tests/recovery.rs @@ -125,6 +125,30 @@ fn generates_valid_supervisor_settings_and_scoped_in_cluster_credentials() { ); } +/// Supporting dependencies receive read-only permissions independently of workload restart permissions. +#[test] +fn dependency_observation_permissions_do_not_authorize_dependency_restarts() { + let mut input = scenario(); + input["recovery"]["mode"] = json!("controller-assisted"); + input["recovery"]["resilience"] = json!({ + "dependencies": {"example-worker": ["ws-server"]}, + "max_concurrent_recoveries": 1_u32, + "stability_seconds": 10_u64, + }); + let (_root, objects) = generate(&input); + let role = object(&objects, "Role", "recovery-supervisor"); + assert_eq!(role["rules"][0]["verbs"], json!(["get", "patch"])); + assert_eq!(role["rules"][1]["resourceNames"], json!(["ws-server"])); + assert_eq!(role["rules"][1]["verbs"], json!(["get"])); + let settings: EnvConfig = + serde_env::from_iter(environment(object(&objects, "Deployment", "recovery-supervisor"))).unwrap(); + assert_eq!( + settings.recovery.resilience.dependencies["example-worker"], + vec!["ws-server"] + ); + assert_eq!(settings.recovery.resilience.max_concurrent_recoveries, 1); +} + /// Preserve recovery state and prevent overlapping supervisor or checkpoint-writer replacements. #[test] fn budgets_persist_and_replacements_do_not_overlap_during_rollouts() { diff --git a/utilities/int-gen/src/openapi.rs b/utilities/int-gen/src/openapi.rs index e3a80a80..202ee2d9 100644 --- a/utilities/int-gen/src/openapi.rs +++ b/utilities/int-gen/src/openapi.rs @@ -209,19 +209,10 @@ fn inject_wasm_baseurl_fallback(body: &str) -> String { body.replacen(needle, replacement, 1) } -/// Give the generated client's `ClientHooks::exec` an exponential-backoff retry loop (native targets only). +/// Route native request execution through the hand-maintained retry policy without duplicating it in generation. /// -/// progenitor's per-call flow is `pre()` -> `exec()` -> `post()`, and `exec`'s default just calls -/// `self.client().execute(request)` once. We override it so every REST call (module discovery, asset fetch, per-agent -/// storage) tolerates a transient failure -- in particular a ws-server that isn't up yet at startup -- by retrying -/// with backoff. Only requests that `try_clone()` (no streaming body) are retried; any other request runs once. -/// -/// NOTE: we hand-inject this only because reqwest's own retry support (`ClientBuilder::retries`, shipped in -/// 0.12.23) has **no backoff yet** -- its `tower::retry::Policy::retry` returns `std::future::Ready<()>` (retries -/// fire immediately) and the upstream `backoff` field is commented out behind a `// TODO? backoff futures...`. -/// When reqwest ships backoff, delete this hook and configure `.retries()` on the `reqwest::Client` instead (see -/// ). The hook is `#[cfg(not(wasm32))]` because `tokio::time::sleep` -/// + `SystemTime` don't work under `wasm32-unknown-unknown`; WASM consumers keep the default `exec`. +/// The policy covers replayable idempotent requests, transient HTTP responses and transport errors. Browser WASM +/// retains progenitor's default execution because the native policy uses Tokio time. #[expect( clippy::single_call_fn, reason = "post-process step kept separate for readability; the named function documents intent" @@ -230,44 +221,16 @@ fn inject_retry_exec(body: &str) -> String { // progenitor emits an empty `impl ClientHooks` that takes the trait defaults; we swap it for one carrying a custom // `exec`. let needle = "impl ClientHooks<()> for &Client {}"; - let replacement = r#"impl ClientHooks<()> for &Client { - // Injected by `utilities/int-gen` (inject_retry_exec): retry request execution with exponential backoff. reqwest's - // native retry has no backoff yet -- remove this and use `ClientBuilder::retries` once it does. + let replacement = r#"#[cfg(not(target_arch = "wasm32"))] +mod retry; +impl ClientHooks<()> for &Client { #[cfg(not(target_arch = "wasm32"))] async fn exec( &self, request: ::reqwest::Request, _info: &OperationInfo, ) -> ::reqwest::Result<::reqwest::Response> { - use ::retry_policies::policies::ExponentialBackoff; - use ::retry_policies::{RetryDecision, RetryPolicy as _}; - let policy = ExponentialBackoff::builder() - .retry_bounds( - ::core::time::Duration::from_millis(250), - ::core::time::Duration::from_secs(5), - ) - .build_with_total_retry_duration(::core::time::Duration::from_secs(30)); - let started = ::std::time::SystemTime::now(); - let mut n_past_retries: u32 = 0; - loop { - // Retry only when the request can be replayed (no streaming body). - let Some(attempt) = request.try_clone() else { - return self.client().execute(request).await; - }; - match self.client().execute(attempt).await { - Ok(response) => return Ok(response), - Err(err) => match policy.should_retry(started, n_past_retries) { - RetryDecision::Retry { execute_after } => { - let wait = execute_after - .duration_since(::std::time::SystemTime::now()) - .unwrap_or_default(); - ::tokio::time::sleep(wait).await; - n_past_retries = n_past_retries.saturating_add(1); - } - RetryDecision::DoNotRetry => return Err(err), - }, - } - } + crate::retry::execute(self.client(), request).await } }"#; assert!( diff --git a/verification/local/output/pyo3-math1-recovery/k3s.yaml b/verification/local/output/pyo3-math1-recovery/k3s.yaml index 5e1485da..6bc5c414 100644 --- a/verification/local/output/pyo3-math1-recovery/k3s.yaml +++ b/verification/local/output/pyo3-math1-recovery/k3s.yaml @@ -405,6 +405,8 @@ spec: value: "180" - name: RECOVERY_POLL_SECONDS value: "2" + - name: RECOVERY_RESILIENCE + value: '{"action_interval_seconds":0,"backoff_max_seconds":300,"backoff_multiplier":1,"connect_timeout_seconds":30,"dependencies":{},"jitter_seconds":0,"max_concurrent_recoveries":0,"reconnect":true,"stability_seconds":0}' - name: RECOVERY_STATE_FILE value: /app/state/recovery.json - name: RECOVERY_WORKLOADS