diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index dbe17243..c0f2e034 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -1625,13 +1625,11 @@ async fn main() -> Result<()> { let hives = load_hives(); // Before serving, because a hive whose role does not exist cannot log in, // and one whose policy does not exist logs in able to read nothing — - // either way it cannot collect what this daemon writes for it. Policy - // first: the role names the policy, so writing it the other way round - // leaves a window where the role points at nothing. Same "log and carry - // on" shape as every connect above. + // either way it cannot collect what this daemon writes for it. A store + // that is not up yet is retried in the background instead of being + // abandoned, since the two of them boot together. let hive_names: Vec = hives.iter().map(|h| h.name.clone()).collect(); - read_policy::ensure_hive_policies(&hive_names).await; - read_policy::ensure_hive_roles(&hive_names).await; + read_policy::ensure_hive_access(hive_names).await; let state = AppState { hives: Arc::new(hives), diff --git a/swarm-controller/src/read_policy.rs b/swarm-controller/src/read_policy.rs index 70769cbb..af437575 100644 --- a/swarm-controller/src/read_policy.rs +++ b/swarm-controller/src/read_policy.rs @@ -6,44 +6,167 @@ //! without the policy the token it gets may read nothing — either way the read //! half of a delivery answers 403. //! -//! Both are startup passes rather than hooks. The policy document is the same -//! for every hive and depends on nothing ([`swarm_secret_client::policy`] -//! explains why it is that wide), and the hive list is loaded once because a -//! config change means a redeploy. Per-hive failures are reported and skipped — -//! one hive that cannot be provisioned should not take down a daemon that -//! serves everything else, and the next start retries it. +//! A pass runs at startup, and repeats until it reaches the store. The two +//! start together, so the store being the slower one is the ordinary case +//! rather than an edge one, and a pass that gave up there provisions nobody +//! until something restarts this daemon. Per-hive failures are different: they +//! are reported and skipped — one hive that cannot be provisioned should not +//! take down a daemon that serves everything else, and a store that answers is +//! answering the same way next time, so the next start is the retry for those. +//! +//! The policy document is the same for every hive and depends on nothing +//! ([`swarm_secret_client::policy`] explains why it is that wide), and the hive +//! list is loaded once because a config change means a redeploy. + +use std::{future::Future, time::Duration}; use anyhow::{Context, Result}; -use swarm_secret_client::{SecretStore, client::DEFAULT_CERT_MOUNT, policy}; +use swarm_secret_client::{Error, SecretStore, client::DEFAULT_CERT_MOUNT, policy}; /// File holding the authority hives are issued from, as /// `swarm-controller.nix` names it. pub const ENV_HIVE_CLIENT_CA: &str = "SWARM_CONTROLLER_HIVE_CLIENT_CA_FILE"; -/// Give every hive in `hives` the read policy its certificate will carry. +/// How long to wait before re-attempting a pass that never reached the store. /// -/// Does nothing when this deployment has no store to reach, which is the -/// shape a controller without a secret store runs in. -pub async fn ensure_hive_policies(hives: &[String]) { - if hives.is_empty() { - return; - } - let store = match crate::store::connect().await { - Ok(store) => store, - Err(e) => { - tracing::info!(reason = %e, "no secret store reachable; hive read policies are not managed here"); - return; - } - }; - for hive in hives { - if let Err(e) = write_one(&store, hive).await { - tracing::warn!(hive, error = %format!("{e:#}"), "writing this hive's read policy failed"); - } +/// `nix/host-modules/swarm-bao.nix`'s bootstrap unit retries the other side of +/// this same race on the same cadence: the two halves of one race should not +/// disagree about how long a wait is worth. +const RETRY_INTERVAL: Duration = Duration::from_secs(30); + +/// How many times [`RETRY_INTERVAL`] is waited before giving up. A store that +/// has not appeared within [`RETRY_WINDOW`] is absent rather than slow, and a +/// daemon logging into the void forever tells an operator nothing. +const RETRY_ATTEMPTS: u32 = 2_880; + +/// What [`RETRY_ATTEMPTS`] at [`RETRY_INTERVAL`] come to. A named third +/// constant because it is the claim the other two make together, and either +/// one of them can be edited alone. +const RETRY_WINDOW: Duration = Duration::from_hours(24); + +/// A pass that never got as far as the store, carrying what to say about it. +/// +/// Returned rather than logged where it happens: the first attempt deserves a +/// warning and the two-hundredth is noise, and only the caller knows which one +/// this is. +struct Unreachable(String); + +/// Give every hive in `hives` the access its certificate needs. +/// +/// Returns once the first pass is done, so a store that is already up leaves +/// nothing deferred. When that pass cannot reach the store, it repeats in the +/// background for up to [`RETRY_WINDOW`]. +pub async fn ensure_hive_access(hives: Vec) { + if let Err(Unreachable(reason)) = provision(&hives).await { + tracing::warn!( + reason, + retry_in = ?RETRY_INTERVAL, + "hive provisioning could not reach the swarm secret store; retrying in the background" + ); + spawn_retry(hives); } } +/// Repeat [`provision`] until it reaches the store, or [`RETRY_WINDOW`] passes. +fn spawn_retry(hives: Vec) { + tokio::spawn(async move { + let reached = retry_until_ok(RETRY_INTERVAL, RETRY_ATTEMPTS, move || { + let hives = hives.clone(); + async move { + let outcome = provision(&hives).await; + if let Err(Unreachable(reason)) = &outcome { + tracing::debug!(reason, "hive provisioning: store still unreachable"); + } + outcome + } + }) + .await; + if let Some(attempt) = reached { + tracing::info!(attempt, "hive provisioning reached the store"); + } else { + tracing::warn!( + attempts = RETRY_ATTEMPTS, + window = ?RETRY_WINDOW, + "giving up on hive provisioning; no hive can log in to the store until this daemon is restarted" + ); + } + }); +} + +/// Run `attempt` every `interval` until it succeeds, at most `attempts` times. +/// Answers with the attempt that succeeded, or `None` if none did. +/// +/// The cadence and the bound are parameters rather than the constants above so +/// that a test can drive this loop without waiting out a real day. +async fn retry_until_ok(interval: Duration, attempts: u32, mut attempt: F) -> Option +where + F: FnMut() -> Fut, + Fut: Future>, +{ + let mut ticker = tokio::time::interval(interval); + // `interval`'s first tick is immediate, and that pass is the one the caller + // already ran before deciding to retry at all. + ticker.tick().await; + for n in 1..=attempts { + ticker.tick().await; + if attempt().await.is_ok() { + return Some(n); + } + } + None +} + +/// One pass: log in once, write every hive's policy, then every hive's role. +/// +/// Policy first, because the role names the policy — the other order leaves a +/// window in which the role points at nothing. +/// +/// # Errors +/// [`Unreachable`] for the states a later attempt can resolve: a store that is +/// not up, and an authority file that is not placed yet. A deployment that is +/// configured for neither is `Ok` — there is nothing to wait for. +async fn provision(hives: &[String]) -> Result<(), Unreachable> { + if hives.is_empty() { + return Ok(()); + } + let store = match crate::store::connect().await { + Ok(store) => store, + // An unset `BAO_*` variable is a deployment that runs no store, not + // one whose store is late: asking again changes nothing. + Err(Error::MissingEnv(var)) => { + tracing::info!( + var, + "no secret store configured; hive access is not managed here" + ); + return Ok(()); + } + Err(e) => return Err(Unreachable(e.to_string())), + }; + for hive in hives { + if let Err(e) = write_policy_for(&store, hive).await { + tracing::warn!(hive, error = %format!("{e:#}"), "writing this hive's read policy failed"); + } + } + + let Some(path) = std::env::var_os(ENV_HIVE_CLIENT_CA) else { + tracing::info!("no hive certificate authority configured; hive roles are not managed here"); + return Ok(()); + }; + let path = path.to_string_lossy().into_owned(); + // Retryable rather than fatal: the unit that places this file and this + // daemon start together, so "not there yet" is a moment, not a verdict. + let ca = std::fs::read_to_string(&path) + .map_err(|e| Unreachable(format!("reading the hive authority {path}: {e}")))?; + for hive in hives { + if let Err(e) = write_role(&store, hive, &ca).await { + tracing::warn!(hive, error = %format!("{e:#}"), "creating this hive's store role failed"); + } + } + Ok(()) +} + /// One hive's policy: its own name, the shared document. -async fn write_one(store: &SecretStore, hive: &str) -> Result<()> { +async fn write_policy_for(store: &SecretStore, hive: &str) -> Result<()> { let name = policy::hive_object_name(hive)?; store .write_policy(&name, &policy::render()) @@ -53,38 +176,6 @@ async fn write_one(store: &SecretStore, hive: &str) -> Result<()> { Ok(()) } -/// Create each hive's cert-auth role, so the certificate it already holds logs -/// in and carries the policy written above. -/// -/// Does nothing when no authority is configured, which is the deployment that -/// has not onboarded a hive yet. -pub async fn ensure_hive_roles(hives: &[String]) { - let Some(path) = std::env::var_os(ENV_HIVE_CLIENT_CA) else { - tracing::info!("no hive certificate authority configured; hive roles are not managed here"); - return; - }; - let path = path.to_string_lossy().into_owned(); - let ca = match std::fs::read_to_string(&path) { - Ok(ca) => ca, - Err(e) => { - tracing::warn!(path, error = %e, var = ENV_HIVE_CLIENT_CA, "reading the hive authority failed; hive roles are not created"); - return; - } - }; - let store = match crate::store::connect().await { - Ok(store) => store, - Err(e) => { - tracing::warn!(error = %e, "connecting to the swarm secret store failed; hive roles are not created"); - return; - } - }; - for hive in hives { - if let Err(e) = write_role(&store, hive, &ca).await { - tracing::warn!(hive, error = %format!("{e:#}"), "creating this hive's store role failed"); - } - } -} - /// The role for one hive: its own certificate's subject, its own policy. async fn write_role(store: &SecretStore, hive: &str, ca: &str) -> Result<()> { let name = policy::hive_object_name(hive)?; @@ -97,3 +188,48 @@ async fn write_role(store: &SecretStore, hive: &str, ca: &str) -> Result<()> { tracing::info!(hive, role = %name, "hive store role in place"); Ok(()) } + +#[cfg(test)] +mod tests { + use std::{cell::Cell, time::Duration}; + + use super::{RETRY_ATTEMPTS, RETRY_INTERVAL, RETRY_WINDOW, Unreachable, retry_until_ok}; + + #[test] + fn the_retry_bound_is_the_window_it_claims() { + assert_eq!(RETRY_INTERVAL * RETRY_ATTEMPTS, RETRY_WINDOW); + } + + /// The property this issue is about: a pass that fails because the store is + /// not up yet happens again. + #[tokio::test] + async fn a_pass_is_repeated_until_it_reaches_the_store() { + let calls = Cell::new(0_u32); + let reached = retry_until_ok(Duration::from_millis(1), 10, || { + calls.set(calls.get() + 1); + let still_down = calls.get() < 3; + async move { + if still_down { + Err(Unreachable("store not up".to_owned())) + } else { + Ok(()) + } + } + }) + .await; + assert_eq!(reached, Some(3)); + assert_eq!(calls.get(), 3, "the loop stops at the first success"); + } + + #[tokio::test] + async fn the_loop_is_bounded_rather_than_endless() { + let calls = Cell::new(0_u32); + let reached = retry_until_ok(Duration::from_millis(1), 5, || { + calls.set(calls.get() + 1); + async { Err(Unreachable("store not up".to_owned())) } + }) + .await; + assert_eq!(reached, None, "a store that never appears is given up on"); + assert_eq!(calls.get(), 5, "one attempt per interval, and no more"); + } +}