diff --git a/.workhorse/specs/monitoring/checks.md b/.workhorse/specs/monitoring/checks.md index f5acfd7e..97163d60 100644 --- a/.workhorse/specs/monitoring/checks.md +++ b/.workhorse/specs/monitoring/checks.md @@ -57,6 +57,7 @@ A name that encodes a parameter turns one configurable check into as many entrie Where a target has several instances of one condition, Canopy holds one state for the check, as it does for every (target, source, check). Each instance is graded through policy on its own, against its own detail, so a rule or silence written for one instance applies to only that instance. +Where it takes more than one field to say which instance this is, the detail carries those fields joined into one as well as separately, because a rule condition matches a single variable and a silence for one instance has to pin all of them. The check's effective result is then the most urgent across the instances that were not skipped, and its detail carries every instance that is not passing, each with its own result, so an operator can see which ones are in trouble without opening anything else. Its message names those instances. The check recovers when no instance is left degraded. diff --git a/.workhorse/specs/public-server/restore-replicas.md b/.workhorse/specs/public-server/restore-replicas.md index a6ad84b8..c9e363d0 100644 --- a/.workhorse/specs/public-server/restore-replicas.md +++ b/.workhorse/specs/public-server/restore-replicas.md @@ -355,8 +355,17 @@ Each replica's redaction outcome is presented alongside its restore health, so a A failed or overdue restore-health report raises a restore-verification check on the affected server, subject to the same monitoring and incident gates as any other of that server's checks. -Restore-health is tracked independently per server, type, and intent: the affected server is the check's scope, and the type and intent name it, so one replica's failed restore does not mask or merge with another's, and the snapshot is carried in the check's detail. -The check recovers when the next report for the same server, type, and intent is healthy. +A server has one restore-verification check however many replicas it has. +Each replica is an instance of that check, graded on its own and carrying its type, its intent, its declared name, and the snapshot in the check's detail (see [CHK](../monitoring/checks.md)), so a rule or silence written for one replica applies to only that replica. +The detail names the replica by its type and intent together as well as separately, since a rule matches one variable at a time and a silence for a single replica has to pin both. +The check reflects the most urgent of them, names the ones in trouble, and recovers when none is left degraded. + +A replica's state is the worse of what its latest report said and whether it has gone past its overdue bound; these are one judgement about the replica, not two competing ones. +Canopy re-derives a server's replicas on the same periodic sweep that decides overdue, from the declarations covering the server and from the reports it holds about it. +A declaration covering the server names one of its replicas whatever its consumer currently advertises: an intent that stops being advertised is a gap, and a finding standing against a replica does not go away because the consumer that reported it stopped offering to. +A replica the server has a report for is one of its replicas too, for as long as a declaration still asks for that replica somewhere in the server's group — which is what a consumer needs in order to report on it at all. +So a report about a server its declaration does not name still surfaces against that server, and a replica nothing declares any more stops being one of the server's instances and needs no separate recovery: nothing can report on it again, so a finding held against it could never recover. +A report's effect appears on the next sweep rather than the instant it lands, which is immaterial for a warning that pages nobody. A replica is also overdue — raising the same check on a periodic sweep, rather than waiting for a report that never arrives — when it has not met its intent's health expectation within the declaration's overdue bound. For an intent carrying `once`, the expectation is measured against the latest snapshot: the replica is overdue when the latest snapshot has gone unverified for longer than the bound, not merely because time has passed since an earlier snapshot was verified. @@ -364,8 +373,10 @@ For an intent without `once`, it is measured against wall-clock time since the l Overdue applies only to intents carrying `check`. A failed migration test raises a migration-test check on the affected server, under the same gates, because that server is on the upgrade path to the version that failed. -The check is named for the type and intent, as restore-verification is, and carries the target version in its detail rather than its name. +It is one check per server with its replicas as instances, as restore-verification is, and carries the target version in the detail rather than the name. A server has one candidate at a time, so there is no second version whose result the first could mask, and a name per version would spawn a catalog policy per release. +A replica whose candidate has not been tried within its overdue bound degrades the same check: untested and failed are both "this version is not known good against this deployment's data". +A recorded verdict raises the check whatever declares that replica now, and a later verdict is what supersedes it: what a version's migrations did to a deployment's data is a fact about the version, not something a declaration has to keep asking about for it to remain true. The check is a warning rather than a failure, and does not escalate. Nothing is wrong with the live server: it is running the version it always was, serving patients, and the finding is about a version it has not taken yet. @@ -373,8 +384,9 @@ Treating it as a failure would open an incident against a healthy deployment and The version's readiness is where the finding does its work. A redaction that did not fully apply raises a redaction check on the affected server, under the same gates. -The check is named for the type and intent, as restore-verification is, and carries the redaction outcome, the manifest version, and the counts of masked and skipped columns in its detail. -It recovers when the next report for the same server, type, and intent redacts fully. +It is one check per server with its redacting replicas as instances, as restore-verification is, each carrying the redaction outcome, the manifest version, and the counts of masked and skipped columns in the check's detail. +A replica is an instance of it only once it has reported a redaction outcome: a declaration that redacts but has produced no replica yet has nothing unmasked to report. +An instance recovers when the replica's next report redacts fully, and the check when none of them is left degraded. The check is a warning rather than a failure, and does not escalate. The deployment is healthy and its data is where it should be; the finding is that a replica made from that data is not as safe to hand out as it was declared to be. diff --git a/crates/canopy-mcp/src/restore.rs b/crates/canopy-mcp/src/restore.rs index 7367b689..06399b90 100644 --- a/crates/canopy-mcp/src/restore.rs +++ b/crates/canopy-mcp/src/restore.rs @@ -216,7 +216,7 @@ impl CanopyMcp { /// exact `(server, type, intent)` (from /// `BackupRestoreCheck::latest_healthy_by_key_for_group`). Does not compute /// an overdue verdict — that logic lives solely in - /// `database::restore::sweep_overdue`, which alone owns the once-vs-check + /// `database::restore::sweep_restore_checks`, which alone owns the once-vs-check /// semantics distinction. async fn restore_replica_outs( &self, diff --git a/crates/database/src/backup.rs b/crates/database/src/backup.rs index f42eac28..ef2b242c 100644 --- a/crates/database/src/backup.rs +++ b/crates/database/src/backup.rs @@ -28,7 +28,7 @@ pub async fn sweep(db: &mut AsyncPgConnection) -> Result { let rows = staleness::scan_rows(db).await?; let mut filed = staleness::sweep(db, &rows).await?; filed += reconcile::sweep(db, &rows).await?; - filed += crate::restore::sweep_overdue(db).await?; + filed += crate::restore::sweep_restore_checks(db).await?; // Not an event, but the same cadence: a plan closes once its group reports // the target, and this sweep is what notices. crate::upgrade_plans::close_met_plans(db).await?; diff --git a/crates/database/src/backup/refs.rs b/crates/database/src/backup/refs.rs index 4abef943..58f0a68c 100644 --- a/crates/database/src/backup/refs.rs +++ b/crates/database/src/backup/refs.rs @@ -104,14 +104,22 @@ pub const PREFLIGHT_ASSUME: &str = "preflight-assume"; /// exist are no longer protected from deletion. pub const PREFLIGHT_OBJECT_LOCK: &str = "preflight-object-lock"; -/// PGRO reported a failed/stale restorability check for one replica. -/// Server-scoped, `Warning`; the ref carries the `(type, intent)` dimension so -/// each replica of a server recovers independently. +/// A restore replica reported unhealthy, or has gone past its overdue bound. +/// Server-scoped, `Warning`. One check per server: each of its replicas is an +/// instance carrying its own `type`, `intent`, and replica name in the detail, +/// so an operator configures restore-verification once rather than once per +/// `(type, intent)` pair. Filed only by `crate::restore::sweep_restore_checks`. pub const RESTORE_VERIFICATION: &str = "restore-verification"; + +/// A candidate version's migrations failed against a replica of a server's +/// data, or have not been tried within the replica's overdue bound. +/// Server-scoped, `Warning`, one check per server with its replicas as +/// instances. pub const MIGRATION_TEST: &str = "migration-test"; /// The masking manifest for a redacting replica did not fully apply. -/// Server-scoped, `Warning`, does not escalate. +/// Server-scoped, `Warning`, does not escalate. One check per server with its +/// redacting replicas as instances. pub const REDACTION: &str = "redaction"; // --- shipped documentation (seeded into the catalog on first filing) --- @@ -284,23 +292,23 @@ Restore the bucket's Object-Lock configuration to GOVERNANCE mode with at least pub const RESTORE_VERIFICATION_DOC: &str = "## Description -The managed restore replica for this (server, type, intent) reported a failed or stale restorability check. +One of this server's managed restore replicas reported a failed restorability check, or has gone past its overdue bound without a healthy one. The server has one of these checks however many replicas it has: each replica is an instance of it, named in the detail by its type, its intent, and the declaration's own name. ## Results -- **warn** — the replica couldn't restore or verify the latest snapshot. +- **warn** — a replica couldn't restore, or couldn't verify the latest snapshot within its bound. ## Solve -Check the restore consumer's report detail: restore errors point at the snapshot or credentials, staleness at the consumer itself."; +Read the detail for the replicas named: restore errors point at the snapshot or credentials, staleness at the consumer itself. To handle one replica differently from the rest, write a rule or silence against its `check.replica_key` rather than the check as a whole."; pub const MIGRATION_TEST_DOC: &str = "## Description -A candidate version's schema migrations were applied to a restore replica of this server's data, and one of them failed. The server itself is unaffected: it is still running the version it was, and the finding is about a version it has not taken. +A candidate version's schema migrations were applied to a restore replica of this server's data and one of them failed, or the candidate has gone untried past the replica's overdue bound. The server itself is unaffected: it is still running the version it was, and the finding is about a version it has not taken. The version under test is in the detail rather than the check name, so a release doesn't spawn a catalog entry of its own. ## Results -- **warn**: the migrations did not complete against this deployment's data. The version carries a known issue and is held back from rollout. +- **warn**: the migrations did not complete against this deployment's data, or have not been tried against it in time. A failure carries a known issue against the version and holds it back from rollout. ## Solve @@ -308,7 +316,7 @@ Read the failing migration named in the report detail. The fix belongs to the mi pub const REDACTION_DOC: &str = "## Description -A replica of this server's data was declared to be served de-identified, and its masking manifest did not fully apply. The server itself is unaffected — this is about the copy, not the deployment. +A replica of this server's data was declared to be served de-identified, and its masking manifest did not fully apply. The server itself is unaffected — this is about the copy, not the deployment. The server has one of these checks however many redacting replicas it has, each an instance named in the detail by its type, its intent, and the declaration's own name. ## Results diff --git a/crates/database/src/backup/staleness.rs b/crates/database/src/backup/staleness.rs index bbcdea13..1d07c76a 100644 --- a/crates/database/src/backup/staleness.rs +++ b/crates/database/src/backup/staleness.rs @@ -649,6 +649,31 @@ pub(crate) async fn server_check_observed_degraded( Ok(n > 0) } +/// Every server with a currently-open, active `(canopy, ref)` issue for one of +/// these checks. +/// +/// A sweep that re-derives its checks from current state has to visit these +/// servers even when it derives nothing for them: a check whose last instance is +/// gone is recovered by being filed as passing, and a server nobody visits is a +/// check left open with nothing that could ever clear it. +pub(crate) async fn servers_with_open_checks( + db: &mut AsyncPgConnection, + checks: &[&str], +) -> Result> { + use crate::schema::issues::dsl; + let ids: Vec> = dsl::issues + .select(dsl::server_id) + .distinct() + .filter(dsl::server_id.is_not_null()) + .filter(dsl::source.eq(refs::CANOPY_SOURCE)) + .filter(dsl::ref_.eq_any(checks.to_vec())) + .filter(dsl::active.eq(true)) + .filter(dsl::resolved_at.is_null()) + .load(db) + .await?; + Ok(ids.into_iter().flatten().collect()) +} + /// Whether a group-scoped `(canopy, ref)` issue is currently open + active. pub(crate) async fn open_group_issue_active( db: &mut AsyncPgConnection, @@ -670,10 +695,10 @@ pub(crate) async fn open_group_issue_active( /// How an alert message names a server: the name an operator knows it by, /// qualified with its host when both are known, falling back to the host -/// alone and finally to the id. Shared with [`crate::backup::reconcile`] so -/// every backup alert names servers the same way. +/// alone and finally to the id. Shared across every canopy-determined check +/// so they all name servers the same way — never interpolate a bare id. // spec: BKJ#alerting -pub(super) fn server_label(server: &Server) -> String { +pub fn server_label(server: &Server) -> String { let host = server.host.as_ref().map(|h| h.0.to_string()); match (&server.name, host) { (Some(n), Some(h)) if !n.is_empty() => format!("{n} ({h})"), diff --git a/crates/database/src/migration_tests.rs b/crates/database/src/migration_tests.rs index 0834d72a..e0923f42 100644 --- a/crates/database/src/migration_tests.rs +++ b/crates/database/src/migration_tests.rs @@ -4,11 +4,12 @@ //! Candidacy here is the version axis only. Whether a server has a snapshot to //! restore and migrate is settled when a consumer's worklist is built. +use std::collections::HashMap; + use commons_errors::Result; use commons_types::{ backup::{BackupType, RestoreIntent, RunOutcome}, server::product::Product, - status::CheckResult, }; use diesel::prelude::*; use diesel_async::{AsyncPgConnection, RunQueryDsl}; @@ -17,13 +18,8 @@ use serde::{Deserialize, Serialize}; use uuid::Uuid; use crate::{ - backup::refs, - issues::{CheckFiling, Scope, file_check}, - pg_duration::PgDuration, - restore::BackupRestoreCheck, - restore::NewBackupRestoreCheck, - servers::Server, - version_known_issues::VersionKnownIssue, + backup::refs, pg_duration::PgDuration, restore::BackupRestoreCheck, + restore::NewBackupRestoreCheck, servers::Server, version_known_issues::VersionKnownIssue, versions::Version, }; @@ -178,8 +174,6 @@ impl MigrationTest { test: NewMigrationTest, ) -> Result { let server_id = report.server_id; - let r#type = report.r#type.clone(); - let intent = report.intent.clone(); let target_version_id = test.target_version_id; let failed_migration = test.failed_migration.clone(); @@ -232,8 +226,6 @@ impl MigrationTest { file_outcome( db, server_id, - &r#type, - &intent, target_version_id, failed_migration.as_deref(), ) @@ -344,74 +336,111 @@ pub async fn has_verdict( Ok(existing.is_some()) } -/// Raise or recover the server's migration-test check, and hold the target -/// version back when its migrations failed. +/// The latest recorded verdict for one replica key. +#[derive(Debug, Clone)] +pub struct KeyVerdict { + /// The version whose migrations were tried, as semver. + pub target_version: String, + /// The migration that failed, when one did. + pub failed_migration: Option, + /// The snapshot they were tried against. + pub snapshot_id: Option, + /// Whether they all applied. Read the same way [`latest_test`] reads it: a + /// report whose restore never got as far as migrating is a failure too. + pub verdict: Verdict, +} + +/// The latest verdict per `(server, type, intent)` across the fleet. +/// +/// The `migration-test` check is derived from these as well as from +/// declarations: a failed migration is a fact about a candidate version +/// measured against a deployment's data, so it stands whether or not a +/// declaration still asks for the test, and what supersedes it is a later +/// verdict (see [`crate::restore::sweep_restore_checks`]). +pub async fn latest_verdict_by_key( + db: &mut AsyncPgConnection, +) -> Result> { + use crate::schema::{backup_restore_checks as checks, migration_tests as tests, versions}; + + type Row = ( + Option, + BackupType, + RestoreIntent, + (i32, i32, i32), + Option, + Option, + RunOutcome, + ); + + let rows: Vec = tests::table + .inner_join(checks::table) + .inner_join(versions::table) + .select(( + checks::server_id, + checks::type_, + checks::intent, + (versions::major, versions::minor, versions::patch), + tests::failed_migration, + checks::snapshot_id, + checks::outcome, + )) + .filter(checks::server_id.is_not_null()) + .distinct_on((checks::server_id, checks::type_, checks::intent)) + .order_by(( + checks::server_id, + checks::type_, + checks::intent, + checks::observed_at.desc(), + checks::id.desc(), + )) + .load(db) + .await?; + + Ok(rows + .into_iter() + .filter_map( + |(server_id, r#type, intent, version, failed_migration, snapshot_id, outcome)| { + let verdict = match (outcome, &failed_migration) { + (RunOutcome::Success, None) => Verdict::Passed, + _ => Verdict::Failed, + }; + let (major, minor, patch) = version; + server_id.map(|sid| { + ( + (sid, r#type, intent), + KeyVerdict { + target_version: format!("{major}.{minor}.{patch}"), + failed_migration, + snapshot_id, + verdict, + }, + ) + }) + }, + ) + .collect()) +} + +/// Record the consequence of a migration test: a failed one raises a known +/// issue against the candidate version, which is what holds it back from +/// rollout. /// -/// A warning that does not escalate. The server is running the version it -/// always was and is serving patients; the finding is about a version it has -/// not taken, so it belongs to whoever decides whether that version ships -/// rather than to whoever is on call for outages. -// spec: RST#alerting +/// The `migration-test` check itself is filed by +/// [`crate::restore::sweep_restore_checks`], the sole filer of the restore checks: the +/// verdict recorded here and the overdue bound are two ways the same check can +/// be degraded, and only the sweep sees all of a server's replicas at once. +/// This path reads the verdict back from storage on the next pass (see +/// [`latest_verdict_by_key`]). async fn file_outcome( db: &mut AsyncPgConnection, server_id: Uuid, - r#type: &BackupType, - intent: &RestoreIntent, target_version_id: Uuid, failed_migration: Option<&str>, ) -> Result<()> { - let version = Version::get_by_id(db, target_version_id).await?; - let semver = version.as_semver(); - // Named per (type, intent) like restore-verification, so the catalog holds - // one policy rather than one per release. The version rides in the detail. - let r#ref = format!("{}:{}:{}", refs::MIGRATION_TEST, r#type, intent); - let Some(migration) = failed_migration else { - file_check( - db, - CheckFiling { - source: crate::statuses::CANOPY_SOURCE, - scope: Scope::Server(server_id), - device_id: None, - check: &r#ref, - observed: CheckResult::Passed, - title: None, - message: &format!("Migrations for {semver} applied against server {server_id}"), - detail: Some(serde_json::json!({ "target_version": semver.to_string() })), - default_ceiling: CheckResult::Warning, - default_escalates: false, - documentation: Some(refs::MIGRATION_TEST_DOC), - }, - ) - .await?; return Ok(()); }; - - file_check( - db, - CheckFiling { - source: crate::statuses::CANOPY_SOURCE, - scope: Scope::Server(server_id), - device_id: None, - check: &r#ref, - observed: CheckResult::Warning, - title: Some("migration test failed"), - message: &format!( - "Migration {migration} failed applying {semver} to a replica of server {server_id}" - ), - detail: Some(serde_json::json!({ - "target_version": semver.to_string(), - "failed_migration": migration, - "type": r#type.to_string(), - "intent": intent.to_string(), - })), - default_ceiling: CheckResult::Warning, - default_escalates: false, - documentation: Some(refs::MIGRATION_TEST_DOC), - }, - ) - .await?; - + let version = Version::get_by_id(db, target_version_id).await?; let affected = (version.major, version.minor, version.patch); if !VersionKnownIssue::unresolved_for_server(db, affected, server_id).await? { VersionKnownIssue::add( diff --git a/crates/database/src/restore.rs b/crates/database/src/restore.rs index 3967c549..8ce11d40 100644 --- a/crates/database/src/restore.rs +++ b/crates/database/src/restore.rs @@ -4,7 +4,7 @@ //! ([`RestoreConsumerCapability`]). The worklist expansion, credential issuance, //! and restore-health ingest live in the public-server and `jobs` components. -use std::collections::{HashMap, hash_map::Entry}; +use std::collections::{HashMap, HashSet, hash_map::Entry}; use commons_errors::{AppError, Result}; use commons_types::backup::{ @@ -22,7 +22,9 @@ use uuid::Uuid; use crate::backup::refs; use crate::backups::BackupRun; -use crate::issues::{CheckFiling, Scope, file_check}; +use crate::issues::{ + CheckInstance, GradedInstance, InstancedCheckFiling, Scope, file_check_instances, +}; use crate::pg_duration::PgDuration; /// An operator's declaration that a restore consumer should maintain a @@ -238,7 +240,6 @@ impl RestoreReplica { ) -> Result { use crate::schema::restore_replicas::dsl; - let existing = Self::get(db, id).await?; let name = normalize_name(update.name)?; let result = match diesel::update(dsl::restore_replicas.filter(dsl::id.eq(id))) @@ -268,36 +269,19 @@ impl RestoreReplica { Err(e) => return Err(AppError::from(e)), }; - let scope_changed = existing.group_id != result.group_id - || existing.server_id != result.server_id - || existing.r#type != result.r#type - || existing.intent != result.intent; - // Disabling drops the declaration out of the overdue sweep exactly as - // deleting it does (`sweep_overdue` filters `enabled = true`), and a - // disabled replica generates no consumer work, so `record_report` can't - // clear the alert either. Left alone, the alert and its incident stay - // open forever. - let disabled = existing.enabled && !result.enabled; - if scope_changed || disabled { - recover_old_scope_alerts( - db, - existing.group_id, - existing.server_id, - &existing.r#type, - &existing.intent, - existing.redacts, - ) - .await?; - } - + // Nothing to clear by hand when the scope moves or the declaration is + // disabled: `sweep_restore_checks` re-derives each server's restore checks + // every pass, and a key stops being derivable once neither a + // declaration nor a still-coverable report yields it, so the check + // re-files without it. That is what recomputing buys over accumulating. Ok(result) } - /// Delete a declaration and recover any active restore-verification alert - /// keyed to its `(server, type, intent)` scope — the overdue sweep only - /// walks current declarations, so an alert left behind by a deleted one - /// would otherwise never clear. Runs in a single transaction so a failure - /// partway can't leave the row deleted with its alerts unrecovered. + /// Delete a declaration. Its restore checks need no explicit recovery: the + /// next [`sweep_restore_checks`] pass re-derives each server's checks, and with the + /// declaration gone its key is derivable only while another declaration + /// still covers the `(group, type)` a report could arrive on — so a replica + /// nothing tracks any more drops out and the check re-files without it. /// /// Restore-health reports the declaration collected are retained: the FK /// from `backup_restore_checks` is `ON DELETE SET NULL`, so each report @@ -306,22 +290,14 @@ impl RestoreReplica { pub async fn delete(db: &mut AsyncPgConnection, id: Uuid) -> Result<()> { db.transaction::<_, AppError, _>(async |conn| { use crate::schema::restore_replicas::dsl; - let existing = Self::get(conn, id).await?; let n = diesel::delete(dsl::restore_replicas.filter(dsl::id.eq(id))) .execute(conn) .await?; if n == 0 { return Err(AppError::DatabaseQuery(DieselError::NotFound)); } - recover_old_scope_alerts( - conn, - existing.group_id, - existing.server_id, - &existing.r#type, - &existing.intent, - existing.redacts, - ) - .await?; + // See `update`: the next sweep re-derives the server's restore + // checks, so a deleted declaration drops out on its own. Ok(()) }) .await @@ -465,17 +441,6 @@ impl RestoreConsumerCapability { } } -/// The stable alert ref for one replica's restore-health. Per -/// `(server, type, intent)` so each replica recovers independently (one -/// intent's healthy report must not clear another's failure on the same -/// server). -fn restore_verification_ref(r#type: &BackupType, intent: &RestoreIntent) -> String { - // The check is server-scoped (the per-server dimension is the filing's - // server_id), so the name is stable per (type, intent) and shared across - // the fleet — one catalog policy, not one single-use name per server. - format!("{}:{}:{}", refs::RESTORE_VERIFICATION, r#type, intent) -} - /// Why a server a redacting declaration covers can't be redacted. /// /// Each of these withholds the server's worklist entry: a replica that @@ -539,174 +504,6 @@ pub async fn redaction_gap_for( Ok((!published).then_some((RedactionGapReason::VersionHasNoManifest, Some(shown)))) } -/// The stable check ref for one replica's redaction, keyed the same way -/// restore-verification is so the catalog holds one policy per replica shape -/// rather than one per server. -fn redaction_ref(r#type: &BackupType, intent: &RestoreIntent) -> String { - format!("{}:{}:{}", refs::REDACTION, r#type, intent) -} - -/// What a consumer's masking did to the replica a report is about. -#[derive(Debug, Clone)] -struct ReportedRedaction { - outcome: RedactionOutcome, - manifest_version: Option, - columns_masked: Option, - columns_skipped: Option, - error: Option, -} - -/// Raise or recover the server's redaction check. -/// -/// A warning that does not escalate. The deployment is healthy and its data -/// is where it should be; the finding is that a replica made from that data -/// is not as safe to hand out as it was declared to be, which belongs to -/// whoever gave out the replica rather than to whoever is on call for -/// outages. -// spec: RST#alerting -async fn file_redaction_outcome( - db: &mut AsyncPgConnection, - server_id: Uuid, - r#type: &BackupType, - intent: &RestoreIntent, - redaction: &ReportedRedaction, -) -> Result<()> { - let r#ref = redaction_ref(r#type, intent); - let detail = serde_json::json!({ - "outcome": redaction.outcome.to_string(), - "manifest_version": redaction.manifest_version, - "columns_masked": redaction.columns_masked, - "columns_skipped": redaction.columns_skipped, - "error": redaction.error, - }); - - let (observed, title, message) = match redaction.outcome { - RedactionOutcome::Complete => ( - CheckResult::Passed, - None, - format!("Replica of server {server_id} redacted: {type} / {intent}"), - ), - RedactionOutcome::Partial => { - let skipped = redaction - .columns_skipped - .map(|n| format!("{n} columns")) - .unwrap_or_else(|| "some columns".into()); - ( - CheckResult::Warning, - Some("redaction partial"), - format!( - "Replica of server {server_id} is live with {skipped} unmasked: {type} / {intent}" - ), - ) - } - RedactionOutcome::Failed => { - let why = redaction - .error - .clone() - .unwrap_or_else(|| "no reason given".into()); - ( - CheckResult::Warning, - Some("redaction failed"), - format!( - "Replica of server {server_id} is held on its previous data, unredacted: {type} / {intent}: {why}" - ), - ) - } - }; - - file_check( - db, - CheckFiling { - source: crate::statuses::CANOPY_SOURCE, - scope: Scope::Server(server_id), - device_id: None, - check: &r#ref, - observed, - title, - message: &message, - detail: Some(detail), - default_ceiling: CheckResult::Warning, - default_escalates: false, - documentation: Some(refs::REDACTION_DOC), - }, - ) - .await?; - Ok(()) -} - -/// Recover any active restore-verification alert keyed to a declaration's old -/// `(server, type, intent)`, called when the declaration stops covering that -/// scope (it was deleted, or its scope moved elsewhere). Mirrors the recovery -/// [`BackupRestoreCheck::record_report`] performs for a healthy report — -/// [`raise_group_event`] with `active: false`. If the old key is still overdue -/// under some other declaration, the next [`sweep_overdue`] pass re-raises it. -async fn recover_old_scope_alerts( - db: &mut AsyncPgConnection, - old_group_id: Uuid, - old_server_id: Option, - old_type: &BackupType, - old_intent: &RestoreIntent, - old_redacts: bool, -) -> Result<()> { - let servers = match old_server_id { - Some(sid) => vec![sid], - None => crate::servers::Server::list_live_in_group(db, old_group_id) - .await? - .into_iter() - .map(|s| s.id) - .collect(), - }; - - for sid in servers { - let r#ref = restore_verification_ref(old_type, old_intent); - file_check( - db, - CheckFiling { - source: crate::statuses::CANOPY_SOURCE, - scope: Scope::Server(sid), device_id: None, - check: &r#ref, - observed: CheckResult::Passed, - title: None, - message: &format!( - "Restore verification no longer tracked at this scope: {old_type} / {old_intent} for server {sid}" - ), - detail: None, - default_ceiling: CheckResult::Warning, - default_escalates: false, - documentation: Some(refs::RESTORE_VERIFICATION_DOC), - }, - ) - .await?; - - // Only a declaration that redacted has a redaction check to clear; - // recovering one for every declaration would seed the catalog with - // checks that were never raised. - if old_redacts { - let r#ref = redaction_ref(old_type, old_intent); - file_check( - db, - CheckFiling { - source: crate::statuses::CANOPY_SOURCE, - scope: Scope::Server(sid), - device_id: None, - check: &r#ref, - observed: CheckResult::Passed, - title: None, - message: &format!( - "Redaction no longer tracked at this scope: {old_type} / {old_intent} for server {sid}" - ), - detail: None, - default_ceiling: CheckResult::Warning, - default_escalates: false, - documentation: Some(refs::REDACTION_DOC), - }, - ) - .await?; - } - } - - Ok(()) -} /// A restore-health report submitted by a restore consumer: proof (or /// disproof) that a backup snapshot actually restores into a healthy /// database — the strongest available signal of backup health. `snapshot_id` @@ -821,99 +618,29 @@ pub struct NewBackupRestoreCheck { } impl BackupRestoreCheck { - /// Record a restore-health report and raise or recover its group-level - /// alert. A success-and-healthy report recovers the replica's - /// `restore-verification` issue; any other outcome raises it (`Error`, - /// group-level, pages regardless of `is_monitored`). + /// Record a restore-health report. + /// + /// Recording is all this does: the checks the report feeds — + /// `restore-verification` and `redaction` — are filed by [`sweep_restore_checks`], + /// which is their sole filer. A replica's state is the worse of what its + /// latest report said and whether it has gone overdue, and only the sweep + /// holds every one of a server's replicas at once; this path holds one. Two + /// filers on the same check would race and drift apart. + /// + /// The sweep runs on the same minute cadence as the reachability tick, and + /// these are non-paging warnings (see `BKJ#alerting`), so the delay between + /// a report landing and its check reflecting it does not matter. pub async fn record_report( db: &mut AsyncPgConnection, new: NewBackupRestoreCheck, ) -> Result { use crate::schema::backup_restore_checks::dsl; - - let healthy = new.outcome == RunOutcome::Success && new.replica_healthy; - let server_id = new.server_id; - let r#type = new.r#type.clone(); - let intent = new.intent.clone(); - let error = new.error.clone(); - let snapshot_id = new.snapshot_id.clone(); - let redaction = new.redaction_outcome.map(|outcome| ReportedRedaction { - outcome, - manifest_version: new.redaction_manifest_version.clone(), - columns_masked: new.redaction_columns_masked, - columns_skipped: new.redaction_columns_skipped, - error: new.redaction_error.clone(), - }); - - let check_id: i64 = diesel::insert_into(dsl::backup_restore_checks) + diesel::insert_into(dsl::backup_restore_checks) .values(new) .returning(dsl::id) .get_result(db) - .await?; - - // Restore-health is attributed per server; a report without one is - // recorded but raises no group-level incident. - if let Some(sid) = server_id { - let r#ref = restore_verification_ref(&r#type, &intent); - if healthy { - file_check( - db, - CheckFiling { - source: crate::statuses::CANOPY_SOURCE, - scope: Scope::Server(sid), - device_id: None, - check: &r#ref, - observed: CheckResult::Passed, - title: None, - message: &format!( - "Restore verification healthy: {type} / {intent} for server {sid}" - ), - detail: None, - default_ceiling: CheckResult::Warning, - default_escalates: false, - documentation: Some(refs::RESTORE_VERIFICATION_DOC), - }, - ) - .await?; - } else { - let error_detail = - error.unwrap_or_else(|| "restored database did not come up healthy".into()); - let snap = snapshot_id - .clone() - .map(|s| format!(" (snapshot {s})")) - .unwrap_or_default(); - file_check( - db, - CheckFiling { - source: crate::statuses::CANOPY_SOURCE, - scope: Scope::Server(sid), - device_id: None, - check: &r#ref, - observed: CheckResult::Failed, - title: Some("restore verification failed"), - message: &format!( - "Restore verification failed: {type} / {intent} for server {sid}{snap}: {error_detail}" - ), - detail: Some(serde_json::json!({ - "type": r#type.to_string(), - "intent": intent.to_string(), - "snapshot_id": snapshot_id, - })), - default_ceiling: CheckResult::Warning, - default_escalates: false, - documentation: Some(refs::RESTORE_VERIFICATION_DOC), - }, - ) - .await?; - } - - // Redaction is its own signal: a replica can restore healthily and - // still come up with data that was meant to be masked and isn't. - if let Some(redaction) = redaction { - file_redaction_outcome(db, sid, &r#type, &intent, &redaction).await?; - } - } - Ok(check_id) + .await + .map_err(AppError::from) } /// Recent reports for a group, newest first — the operator restore-health @@ -968,6 +695,43 @@ impl BackupRestoreCheck { .map_err(AppError::from) } + /// The latest report per `(server, type, intent)` across the fleet, whatever + /// its outcome — what the sweep needs to know each replica's *current* + /// state, as opposed to when it was last healthy. + /// + /// Fleet-wide rather than per group because the sweep derives its instances + /// from these keys as well as from declarations, so it cannot know which + /// groups to ask about until it has them. + /// + /// Carries the whole row so the redaction fields come along: a report says + /// both whether the restore came up healthy and whether its masking + /// applied, and those are two checks off one report. + pub async fn latest_by_key( + db: &mut AsyncPgConnection, + ) -> Result> { + use crate::schema::backup_restore_checks::dsl; + let rows: Vec = dsl::backup_restore_checks + .select(Self::as_select()) + .filter(dsl::server_id.is_not_null()) + .distinct_on((dsl::server_id, dsl::type_, dsl::intent)) + .order_by(( + dsl::server_id, + dsl::type_, + dsl::intent, + dsl::observed_at.desc(), + dsl::id.desc(), + )) + .load(db) + .await?; + Ok(rows + .into_iter() + .filter_map(|r| { + r.server_id + .map(|sid| ((sid, r.r#type.clone(), r.intent.clone()), r)) + }) + .collect()) + } + /// Latest *healthy* report timestamp per `(server, type, intent)` in a /// group — the freshness anchor the overdue sweep compares against. pub async fn latest_healthy_by_key_for_group( @@ -1034,27 +798,103 @@ impl BackupRestoreCheck { } } -/// Overdue restore-verification sweep: for every enabled declaration with an -/// overdue bound whose intent the consumer still advertises with the `check` -/// semantic, raise the `restore-verification` alert for any concrete -/// `(server, type, intent)` that is overdue. Overdue is measured per the intent's -/// semantics: a `once` intent is overdue when the latest snapshot has gone -/// unverified for longer than the bound; any other `check` intent is overdue -/// when it has no healthy report within the bound. Recovery is driven by the -/// next healthy report ([`BackupRestoreCheck::record_report`]), so this only -/// raises. Returns the number of overdue alerts filed. -pub async fn sweep_overdue(db: &mut AsyncPgConnection) -> Result { +/// One replica key: a `(type, intent)` pair on one server. Both dimensions are +/// open-ended strings, so the set of keys is discovered, never enumerated. +type ReplicaKey = (Uuid, BackupType, RestoreIntent); + +/// What the sweep has gathered about one replica key before it grades it. +#[derive(Default)] +struct KeyWork { + /// The operator's name for the declaration covering this key, when one does. + declared_as: Option, + /// A `migrate` declaration covers it, so restore health is not what it is + /// for — whether its candidate version applies is. + migrates: bool, + /// Whether the key's latest report is allowed to drive its checks: either a + /// declaration covers the key, or its `(group, type)` is still covered, so + /// another report could yet arrive to change the answer. + reports_count: bool, + /// Past its declaration's overdue bound. + overdue: bool, + /// That bound was measured against the latest snapshot (a `once` intent). + once: bool, + /// A `migrate` declaration's candidate has gone untried past the bound: + /// `(target version, snapshot)`. + untried: Option<(String, String)>, +} + +/// The instances one server's three restore checks are filed from. +#[derive(Default)] +struct ServerInstances { + verification: Vec, + redaction: Vec, + migration: Vec, +} + +/// The restore checks' sole filer: re-derive every server's +/// `restore-verification`, `redaction` and `migration-test` checks from what +/// Canopy currently holds, and file the ones that have something to say. +/// +/// Each of a server's replicas is one instance of each check rather than a +/// check of its own (see the Names section of the CHK spec), and an instance's +/// result is the worse of what its latest report said and whether it has gone +/// past its declaration's overdue bound: one judgement about the replica, not +/// two writers racing on one name. +/// +/// A replica key is derived from recorded facts as much as from live +/// declarations, because a finding has to survive the thing that produced it +/// going quiet. A key `(server, type, intent)` yields instances when: +/// +/// - an enabled declaration covers it on that server, whatever its consumer +/// currently advertises. A capability that stops being advertised is a gap, +/// surfaced as one; it is not grounds for a standing finding to disappear. +/// Only the overdue judgement needs the intent's semantics, so that is the +/// only thing gated on them. Or, +/// - Canopy holds a report for it and an enabled declaration still asks for that +/// replica somewhere in the server's group, so a report about a server the +/// declaration does not currently name still counts. Once no declaration asks +/// for the replica at all, nothing can report on it again +/// ([`RestoreReplica::authorizes`] is what a consumer has to satisfy to +/// report), so a finding held on it could never recover: it stops being +/// derived rather than being pinned open with no way out. Or, +/// - Canopy holds a migration verdict for it, which yields a `migration-test` +/// instance whatever declares the replica now. A failed migration is a fact +/// about a candidate version measured against this deployment's data, not a +/// deadline on a declaration, and what supersedes it is a later verdict. +/// +/// Returns the number of checks this pass left degraded. +// spec: RST#alerting +pub async fn sweep_restore_checks(db: &mut AsyncPgConnection) -> Result { use crate::schema::restore_replicas::dsl; let now = Timestamp::now(); + // Every enabled declaration, not just the ones with an overdue bound: this + // sweep is the sole filer of the restore checks, so a declaration without a + // bound still needs its latest report's health reflected. A missing bound + // means "never overdue", not "never checked". Ordered so that when two + // declarations cover one key, which of them names the instance is stable. let declarations: Vec = dsl::restore_replicas .select(RestoreReplica::as_select()) .filter(dsl::enabled.eq(true)) - .filter(dsl::overdue_after.is_not_null()) + .order_by((dsl::name, dsl::id)) .load(db) .await?; - // Per-consumer descriptors (to read semantics) and per-group health anchors. + // The recorded facts, fleet-wide and one query each. + let latest_reports = BackupRestoreCheck::latest_by_key(db).await?; + let latest_verdicts = crate::migration_tests::latest_verdict_by_key(db).await?; + + // The replicas an enabled declaration still asks for, wherever in their group + // they sit. A report is derivable into an instance while its replica is still + // declared somewhere in the group, so a report about a server the declaration + // does not currently name is not lost — but a replica nothing declares any + // more stops being derived. + let declared: HashSet<(Uuid, BackupType, RestoreIntent)> = declarations + .iter() + .map(|d| (d.group_id, d.r#type.clone(), d.intent.clone())) + .collect(); + + // Per-consumer descriptors (to read semantics) and per-group anchors. let mut capability_cache: HashMap> = HashMap::new(); let mut healthy_cache: HashMap> = @@ -1065,15 +905,11 @@ pub async fn sweep_overdue(db: &mut AsyncPgConnection) -> Result { > = HashMap::new(); let mut latest_snapshot_cache: HashMap> = HashMap::new(); - let mut filed = 0usize; - for d in declarations { - let Some(overdue_after) = d.overdue_after else { - continue; - }; + let mut work: HashMap = HashMap::new(); + let mut servers: HashMap = HashMap::new(); - // Skip declarations the consumer can't satisfy — those are gaps, not - // restore-health incidents. Only `check` intents are held to a bound. + for d in &declarations { let capabilities = match capability_cache.entry(d.consumer_device_id) { Entry::Occupied(e) => e.into_mut(), Entry::Vacant(e) => e.insert( @@ -1084,27 +920,21 @@ pub async fn sweep_overdue(db: &mut AsyncPgConnection) -> Result { .collect(), ), }; - let Some(descriptor) = capabilities.get(&d.intent) else { - continue; + let descriptor = capabilities.get(&d.intent); + let checks = descriptor.is_some_and(|desc| desc.has_semantic(semantics::CHECK)); + let once = descriptor.is_some_and(|desc| desc.has_semantic(semantics::ONCE)); + let migrates = descriptor.is_some_and(|desc| desc.has_semantic(semantics::MIGRATE)); + + let covered_servers: Vec = match d.server_id { + Some(sid) => match crate::servers::Server::get_by_id(db, sid).await.ok() { + Some(s) if s.group_id == Some(d.group_id) && s.deleted_at.is_none() => vec![s], + _ => vec![], + }, + None => crate::servers::Server::list_live_in_group(db, d.group_id).await?, }; - if !descriptor.has_semantic(semantics::CHECK) { + if covered_servers.is_empty() { continue; } - let once = descriptor.has_semantic(semantics::ONCE); - let migrates = descriptor.has_semantic(semantics::MIGRATE); - - let servers: Vec = match d.server_id { - Some(sid) => { - let s = crate::servers::Server::get_by_id(db, sid).await.ok(); - match s { - Some(s) if s.group_id == Some(d.group_id) && s.deleted_at.is_none() => { - vec![s] - } - _ => vec![], - } - } - None => crate::servers::Server::list_live_in_group(db, d.group_id).await?, - }; if let Entry::Vacant(e) = healthy_cache.entry(d.group_id) { e.insert(BackupRestoreCheck::latest_healthy_by_key_for_group(db, d.group_id).await?); @@ -1119,158 +949,620 @@ pub async fn sweep_overdue(db: &mut AsyncPgConnection) -> Result { ); } - for server in servers { + for server in covered_servers { let sid = server.id; + let key = (sid, d.r#type.clone(), d.intent.clone()); // A `migrate` intent is overdue on its own terms: the question is // whether the candidate version has been tried against the latest // snapshot, not whether the replica restored. - if migrates { - if let Some(filed_one) = sweep_migration_overdue( - db, - &d, - &server, - &latest_snapshot_cache[&d.group_id], + let untried = if migrates && checks { + untried_candidate(db, d, &server, &latest_snapshot_cache[&d.group_id], now).await? + } else { + None + }; + // Overdue is a property of the bound, so a declaration without one + // is never overdue — but its latest report's health still counts. + let overdue = match (migrates, checks, d.overdue_after) { + (false, true, Some(bound)) => is_overdue( + &key, + bound, + once, now, - overdue_after, - ) - .await? - { - filed += filed_one; - } - continue; + &healthy_cache[&d.group_id], + &verified_snapshot_cache[&d.group_id], + &latest_snapshot_cache[&d.group_id], + ), + _ => false, + }; + + servers.entry(sid).or_insert(server); + let w = work.entry(key).or_default(); + if w.declared_as.is_none() { + w.declared_as = Some(d.name.clone()); } + w.reports_count = true; + w.migrates |= migrates; + w.overdue |= overdue; + w.once |= overdue && once; + if untried.is_some() { + w.untried = untried; + } + } + } - let key = (sid, d.r#type.clone(), d.intent.clone()); - let overdue = if once { - // A `once` intent is overdue only when a snapshot exists to verify, - // it is not the last one verified, and it has stood past the bound. - match latest_snapshot_cache[&d.group_id].get(&(sid, d.r#type.clone())) { - None => false, - Some(run) => { - let verified = verified_snapshot_cache[&d.group_id].get(&key); - let already = matches!( - (verified, run.snapshot_id.as_ref()), - (Some(v), Some(s)) if v == s - ); - // Measured from the report, not `run.anchor()`: the question is - // how long this snapshot has gone unverified since it became - // available to verify, which is when it landed — not how old - // the data inside it is. - !already && now.duration_since(run.reported_at) > overdue_after.0 - } - } - } else { - match healthy_cache[&d.group_id].get(&key) { - Some(last) => now.duration_since(*last) > overdue_after.0, - None => true, - } - }; - if !overdue { - continue; + // Keys Canopy holds a report for on a server no declaration currently names, + // which stay derivable while the replica itself is still declared. + for (key, report) in &latest_reports { + let declaration = ( + report.group_id, + report.r#type.clone(), + report.intent.clone(), + ); + if work.contains_key(key) || !declared.contains(&declaration) { + continue; + } + if live_server(db, &mut servers, key.0).await?.is_none() { + continue; + } + work.entry(key.clone()).or_default().reports_count = true; + } + + // Keys with a recorded verdict, which stand whatever covers them now. + for key in latest_verdicts.keys() { + if work.contains_key(key) { + continue; + } + if live_server(db, &mut servers, key.0).await?.is_none() { + continue; + } + work.entry(key.clone()).or_default(); + } + + // Grade every key into its server's instances, in a stable order so a + // check's message and detail don't reshuffle between passes. + let mut keys: Vec = work.keys().cloned().collect(); + keys.sort_by_key(|(sid, r#type, intent)| (*sid, r#type.to_string(), intent.to_string())); + + let mut per_server: HashMap = HashMap::new(); + let mut server_order: Vec = Vec::new(); + for key in keys { + let w = &work[&key]; + let (sid, r#type, intent) = (key.0, &key.1, &key.2); + let label = match &w.declared_as { + Some(name) => format!("{name} ({type} / {intent})"), + None => format!("{type} / {intent}"), + }; + let latest = if w.reports_count { + latest_reports.get(&key) + } else { + None + }; + let instances = per_server.entry(sid).or_insert_with(|| { + server_order.push(sid); + ServerInstances::default() + }); + + // A declaration for something other than migrating is a replica whose + // restore health is expected; so is any key with a report to read. + if (w.declared_as.is_some() && !w.migrates) || latest.is_some() { + instances + .verification + .push(verification_instance(&key, &label, w, latest)); + } + if let Some(instance) = redaction_instance(&key, &label, w, latest) { + instances.redaction.push(instance); + } + if let Some(instance) = migration_instance(&key, &label, w, latest_verdicts.get(&key)) { + instances.migration.push(instance); + } + } + + // Servers whose checks are open but which derived nothing this pass: their + // last replica is gone, and a server nobody visits is a check left open with + // nothing that could ever clear it. + for sid in crate::backup::staleness::servers_with_open_checks( + db, + &[ + refs::RESTORE_VERIFICATION, + refs::REDACTION, + refs::MIGRATION_TEST, + ], + ) + .await? + { + if per_server.contains_key(&sid) || live_server(db, &mut servers, sid).await?.is_none() { + continue; + } + per_server.insert(sid, ServerInstances::default()); + server_order.push(sid); + } + + let mut degraded = 0usize; + for sid in server_order { + let found = per_server.remove(&sid).expect("entered with its server"); + let label = crate::backup::staleness::server_label( + servers.get(&sid).expect("cached when the key was derived"), + ); + degraded += file_verification(db, sid, &label, found.verification).await?; + degraded += file_redaction(db, sid, &label, found.redaction).await?; + degraded += file_migration(db, sid, &label, found.migration).await?; + } + + Ok(degraded) +} + +/// A live server by id, cached across the sweep. `None` when it is gone or +/// soft-deleted, in which case its keys are not worth deriving: nothing holds a +/// check against a server that isn't there. +async fn live_server<'a>( + db: &mut AsyncPgConnection, + cache: &'a mut HashMap, + server_id: Uuid, +) -> Result> { + if let Entry::Vacant(e) = cache.entry(server_id) { + match crate::servers::Server::get_by_id(db, server_id).await { + Ok(server) if server.deleted_at.is_none() => { + e.insert(server); } - let r#ref = restore_verification_ref(&d.r#type, &d.intent); - let message = if once { - format!( - "Latest snapshot for {} / {} on server {sid} has not been verified within its overdue bound", - d.r#type, d.intent - ) - } else { - format!( - "No healthy restore verification for {} / {} on server {sid} within its overdue bound", - d.r#type, d.intent - ) - }; - file_check( - db, - CheckFiling { - source: crate::statuses::CANOPY_SOURCE, - scope: Scope::Server(sid), - device_id: None, - check: &r#ref, - observed: CheckResult::Failed, - title: Some("restore verification overdue"), - message: &message, - detail: Some(serde_json::json!({ - "type": d.r#type.to_string(), - "intent": d.intent.to_string(), - "latest_snapshot_unverified": once, - })), - default_ceiling: CheckResult::Warning, - default_escalates: false, - documentation: Some(refs::RESTORE_VERIFICATION_DOC), - }, - ) - .await?; - filed += 1; + _ => return Ok(None), } } + Ok(cache.get(&server_id)) +} + +/// Whether one replica key has gone past its bound, per its intent's semantics. +fn is_overdue( + key: &ReplicaKey, + bound: PgDuration, + once: bool, + now: Timestamp, + healthy: &HashMap<(Uuid, BackupType, RestoreIntent), Timestamp>, + verified: &HashMap<(Uuid, BackupType, RestoreIntent), String>, + snapshots: &HashMap<(Uuid, BackupType), BackupRun>, +) -> bool { + if !once { + return match healthy.get(key) { + Some(last) => now.duration_since(*last) > bound.0, + None => true, + }; + } - Ok(filed) + // A `once` intent is overdue only when a snapshot exists to verify, it is + // not the last one verified, and it has stood past the bound. + match snapshots.get(&(key.0, key.1.clone())) { + None => false, + Some(run) => { + let already = matches!( + (verified.get(key), run.snapshot_id.as_ref()), + (Some(v), Some(s)) if v == s + ); + // Measured from the report, not `run.anchor()`: the question is how + // long this snapshot has gone unverified since it became available + // to verify, which is when it landed — not how old the data inside + // it is. + !already && now.duration_since(run.reported_at) > bound.0 + } + } } /// Whether a `migrate` declaration's server has left its candidate version -/// untested past the bound, filing the migration-test check when it has. +/// untried past the bound, and against which snapshot. /// -/// Returns the number of checks filed, or `None` when the server has nothing to -/// be overdue about: no candidate version, or no snapshot to migrate. +/// `None` when there is nothing to be overdue about: no candidate version, no +/// snapshot to migrate, a verdict already recorded for the pair, or still inside +/// the bound. A recorded verdict reaches the check through +/// [`crate::migration_tests::latest_verdict_by_key`] instead, so it is not this +/// path's business. // spec: RST#alerting -async fn sweep_migration_overdue( +async fn untried_candidate( db: &mut AsyncPgConnection, declaration: &RestoreReplica, server: &crate::servers::Server, - latest: &HashMap<(Uuid, BackupType), BackupRun>, + snapshots: &HashMap<(Uuid, BackupType), BackupRun>, now: Timestamp, - overdue_after: PgDuration, -) -> Result> { +) -> Result> { + let Some(bound) = declaration.overdue_after else { + return Ok(None); + }; let Some(version) = crate::migration_tests::candidate_for(db, server).await? else { return Ok(None); }; - let Some(run) = latest.get(&(server.id, declaration.r#type.clone())) else { + let Some(run) = snapshots.get(&(server.id, declaration.r#type.clone())) else { return Ok(None); }; let Some(snapshot_id) = run.snapshot_id.as_ref() else { return Ok(None); }; - if crate::migration_tests::has_verdict(db, server.id, snapshot_id, version.id).await? { return Ok(None); } // Measured from when the snapshot landed, which is when it became available // to migrate, not how old the data inside it is. - if now.duration_since(run.reported_at) <= overdue_after.0 { + if now.duration_since(run.reported_at) <= bound.0 { return Ok(None); } + Ok(Some(( + version.as_semver().to_string(), + snapshot_id.to_owned(), + ))) +} - let semver = version.as_semver(); - file_check( +/// The fields every instance of a restore check carries, whichever check it is: +/// what identifies the replica, for an operator reading the detail and for a +/// rule or silence written against one replica. +/// +/// `replica_key` is the two dimensions joined, because a rule condition takes +/// one variable and a silence for one replica has to pin both (see +/// [`crate::check_policies::Condition`]). +fn instance_identity(key: &ReplicaKey, declared_as: Option<&String>) -> serde_json::Value { + let (_, r#type, intent) = key; + serde_json::json!({ + "type": r#type.to_string(), + "intent": intent.to_string(), + "replica_key": format!("{type}:{intent}"), + "replica": declared_as, + }) +} + +/// Merge `extra` into an instance's identity fields. +fn instance_detail( + key: &ReplicaKey, + declared_as: Option<&String>, + extra: serde_json::Value, +) -> Option { + let mut detail = instance_identity(key, declared_as); + let (Some(object), Some(extra)) = (detail.as_object_mut(), extra.as_object()) else { + return Some(detail); + }; + for (k, v) in extra { + object.insert(k.clone(), v.clone()); + } + Some(detail) +} + +/// One replica's restore-verification instance: the worse of what its latest +/// report said and whether it has gone past its bound. +fn verification_instance( + key: &ReplicaKey, + label: &str, + work: &KeyWork, + latest: Option<&BackupRestoreCheck>, +) -> CheckInstance { + let reported_unhealthy = + latest.is_some_and(|r| r.outcome != RunOutcome::Success || !r.replica_healthy); + let observed = if reported_unhealthy || work.overdue { + CheckResult::Failed + } else { + CheckResult::Passed + }; + let why = if reported_unhealthy { + latest + .and_then(|r| r.error.clone()) + .unwrap_or_else(|| "restored database did not come up healthy".into()) + } else if work.overdue && work.once { + "latest snapshot not verified within its overdue bound".into() + } else if work.overdue { + "no healthy restore verification within its overdue bound".into() + } else { + "healthy".into() + }; + CheckInstance { + label: label.to_owned(), + observed, + detail: instance_detail( + key, + work.declared_as.as_ref(), + serde_json::json!({ + "snapshot_id": latest.and_then(|r| r.snapshot_id.clone()), + "overdue": work.overdue, + "latest_snapshot_unverified": work.overdue && work.once, + "why": why, + }), + ), + } +} + +/// The redaction instance for one replica, from its latest report. `None` when +/// the replica has not reported a redaction outcome, since nothing observed is +/// not the same as redacted — and a declaration that redacts but has never +/// reported has produced no replica to be unmasked. +fn redaction_instance( + key: &ReplicaKey, + label: &str, + work: &KeyWork, + latest: Option<&BackupRestoreCheck>, +) -> Option { + let latest = latest?; + let outcome = latest.redaction_outcome?; + let observed = match outcome { + RedactionOutcome::Complete => CheckResult::Passed, + RedactionOutcome::Partial | RedactionOutcome::Failed => CheckResult::Warning, + }; + Some(CheckInstance { + label: label.to_owned(), + observed, + detail: instance_detail( + key, + work.declared_as.as_ref(), + serde_json::json!({ + "outcome": outcome.to_string(), + "manifest_version": latest.redaction_manifest_version, + "columns_masked": latest.redaction_columns_masked, + "columns_skipped": latest.redaction_columns_skipped, + "error": latest.redaction_error, + "why": outcome.to_string(), + }), + ), + }) +} + +/// The migration-test instance for one replica: what its latest verdict said, +/// or that its candidate has gone untried past the bound. Untested and failed +/// are both "this version is not known good against this deployment's data", so +/// they are one instance and the more urgent of the two wins. +/// +/// `None` when there is neither a verdict nor a bound gone past: a replica with +/// nothing to test is not a passing test. +// spec: RST#alerting +fn migration_instance( + key: &ReplicaKey, + label: &str, + work: &KeyWork, + verdict: Option<&crate::migration_tests::KeyVerdict>, +) -> Option { + // The version named is whichever side answered: the verdict names the one it + // was reached against, the bound names the candidate it is still waiting for. + let (observed, why, target_version, snapshot, failed_migration) = match (verdict, &work.untried) + { + (Some(v), _) if v.verdict == crate::migration_tests::Verdict::Failed => ( + CheckResult::Warning, + match &v.failed_migration { + Some(migration) => { + format!("migration {migration} failed applying {}", v.target_version) + } + None => format!( + "the restore never got as far as migrating {}", + v.target_version + ), + }, + Some(v.target_version.clone()), + v.snapshot_id.clone(), + v.failed_migration.clone(), + ), + (_, Some((version, snapshot))) => ( + CheckResult::Warning, + format!("migrations for {version} not tried within the overdue bound"), + Some(version.clone()), + Some(snapshot.clone()), + None, + ), + (Some(v), None) => ( + CheckResult::Passed, + format!("migrations for {} applied", v.target_version), + Some(v.target_version.clone()), + v.snapshot_id.clone(), + None, + ), + (None, None) => return None, + }; + + Some(CheckInstance { + label: label.to_owned(), + observed, + detail: instance_detail( + key, + work.declared_as.as_ref(), + serde_json::json!({ + "target_version": target_version, + "failed_migration": failed_migration, + "snapshot_id": snapshot, + "why": why, + }), + ), + }) +} + +async fn file_verification( + db: &mut AsyncPgConnection, + server_id: Uuid, + label: &str, + instances: Vec, +) -> Result { + let total = instances.len(); + file_restore_check( db, - CheckFiling { - source: crate::statuses::CANOPY_SOURCE, - scope: Scope::Server(server.id), - device_id: None, - check: &format!( - "{}:{}:{}", - refs::MIGRATION_TEST, declaration.r#type, declaration.intent + server_id, + RestoreCheck { + r#ref: refs::RESTORE_VERIFICATION, + documentation: refs::RESTORE_VERIFICATION_DOC, + title: "restore verification failed", + gone: &format!("No restore replica of {label} is tracked any more"), + }, + instances, + &|degraded| match degraded { + [] => format!("Every restore replica of {label} is verifying healthily"), + [one] => format!( + "Restore verification failed for {label}: {} — {}", + one.label, + instance_why(one), ), - observed: CheckResult::Warning, - title: Some("migration test overdue"), - message: &format!( - "Migrations for {semver} have not been tried against server {}'s latest snapshot within its overdue bound", - server.id + many => format!( + "Restore verification failed for {} of {label}'s {total} replicas: {}", + many.len(), + instance_labels(many), ), - detail: Some(serde_json::json!({ - "target_version": semver.to_string(), - "snapshot_id": snapshot_id, - "type": declaration.r#type.to_string(), - "intent": declaration.intent.to_string(), - })), - default_ceiling: CheckResult::Warning, - default_escalates: false, - documentation: Some(refs::MIGRATION_TEST_DOC), }, ) - .await?; + .await +} + +async fn file_redaction( + db: &mut AsyncPgConnection, + server_id: Uuid, + label: &str, + instances: Vec, +) -> Result { + let total = instances.len(); + file_restore_check( + db, + server_id, + RestoreCheck { + r#ref: refs::REDACTION, + documentation: refs::REDACTION_DOC, + title: "redaction incomplete", + gone: &format!("No redacting replica of {label} is tracked any more"), + }, + instances, + &|degraded| match degraded { + [] => format!("Every redacting replica of {label} is fully masked"), + [one] => format!( + "Replica {} of {label} did not fully redact: {}", + one.label, + instance_field(one, "outcome"), + ), + many => format!( + "{} of {label}'s {total} redacting replicas did not fully redact: {}", + many.len(), + instance_labels(many), + ), + }, + ) + .await +} + +async fn file_migration( + db: &mut AsyncPgConnection, + server_id: Uuid, + label: &str, + instances: Vec, +) -> Result { + let total = instances.len(); + file_restore_check( + db, + server_id, + RestoreCheck { + r#ref: refs::MIGRATION_TEST, + documentation: refs::MIGRATION_TEST_DOC, + title: "candidate version not known good", + gone: &format!("No candidate version is under test against {label}'s data"), + }, + instances, + &|degraded| match degraded { + [] => format!("Candidate versions have been migration-tested against {label}"), + [one] => format!( + "Candidate version not clean against {label}'s data: {} — {}", + one.label, + instance_why(one), + ), + many => format!( + "Candidate versions not clean against {label}'s data for {} of {total} replicas: {}", + many.len(), + instance_labels(many), + ), + }, + ) + .await +} + +/// The fixed parts of one restore check: what it is called, the documentation it +/// ships with, its headline when degraded, and what it says once a server has no +/// instances of it left. +struct RestoreCheck<'a> { + r#ref: &'a str, + documentation: &'a str, + title: &'a str, + gone: &'a str, +} + +/// File one of a server's restore checks from its instances, and say whether it +/// came out degraded. +/// +/// Nothing is filed for a check that has nothing degraded and nothing open: a +/// server that has never had one of these findings does not need a passing row +/// and a catalog entry for it. A check that *is* open and has run out of +/// instances is recovered on its own — with no instances there is nothing left +/// to grade, so it is filed as the plain passing check it has become rather +/// than left open with nothing that could ever clear it. +async fn file_restore_check( + db: &mut AsyncPgConnection, + server_id: Uuid, + check: RestoreCheck<'_>, + instances: Vec, + message: &(dyn Fn(&[GradedInstance]) -> String + Sync), +) -> Result { + let RestoreCheck { + r#ref, + documentation, + title, + gone, + } = check; + let any_degraded = instances.iter().any(|i| i.observed != CheckResult::Passed); + if !any_degraded + && !crate::backup::staleness::open_server_issue_active(db, server_id, r#ref).await? + { + return Ok(0); + } + + let issue = if instances.is_empty() { + crate::issues::file_check( + db, + crate::issues::CheckFiling { + source: crate::statuses::CANOPY_SOURCE, + scope: Scope::Server(server_id), + device_id: None, + check: r#ref, + observed: CheckResult::Passed, + title: None, + message: gone, + detail: None, + default_ceiling: CheckResult::Warning, + default_escalates: false, + documentation: Some(documentation), + }, + ) + .await? + } else { + file_check_instances( + db, + InstancedCheckFiling { + source: crate::statuses::CANOPY_SOURCE, + scope: Scope::Server(server_id), + device_id: None, + check: r#ref, + title: Some(title), + instances, + default_ceiling: CheckResult::Warning, + default_escalates: false, + documentation: Some(documentation), + }, + message, + ) + .await? + }; + + Ok(usize::from(issue.active)) +} + +/// A degraded instance's `why`, for a single-instance message. +fn instance_why(instance: &GradedInstance) -> String { + instance_field(instance, "why") +} + +fn instance_field(instance: &GradedInstance, field: &str) -> String { + instance + .detail + .as_ref() + .and_then(|d| d.get(field)) + .and_then(|v| v.as_str()) + .unwrap_or("no detail reported") + .to_owned() +} - Ok(Some(1)) +fn instance_labels(instances: &[GradedInstance]) -> String { + instances + .iter() + .map(|i| i.label.as_str()) + .collect::>() + .join(", ") } diff --git a/crates/database/tests/it/migration_test_reports.rs b/crates/database/tests/it/migration_test_reports.rs index e8b835ea..eb050130 100644 --- a/crates/database/tests/it/migration_test_reports.rs +++ b/crates/database/tests/it/migration_test_reports.rs @@ -231,11 +231,19 @@ struct FiledCheck { escalates: bool, } +/// The server's migration-test check as it stands. +/// +/// Sweeps first: `sweep_restore_checks` is the sole filer of the restore checks, so a +/// recorded verdict reaches the check on the next pass rather than at the +/// moment it is recorded (see `BackupRestoreCheck::record_report`). async fn migration_check(conn: &mut AsyncPgConnection, server: Uuid) -> Option { + database::restore::sweep_restore_checks(conn) + .await + .expect("sweep"); sql_query( "SELECT i.observed_result AS observed, i.effective_result AS effective, i.escalates FROM issues i - WHERE i.server_id = $1 AND i.ref LIKE 'migration-test:%' AND i.active = true", + WHERE i.server_id = $1 AND i.ref = 'migration-test' AND i.active = true", ) .bind::(server) .get_result(conn) @@ -476,7 +484,7 @@ async fn an_untried_candidate_goes_overdue_and_a_tested_one_does_not() { declare_migrate(&mut conn, consumer, group, 3600).await; record_snapshot(&mut conn, consumer, group, server, "snap-old", 7200).await; - let filed = database::restore::sweep_overdue(&mut conn) + let filed = database::restore::sweep_restore_checks(&mut conn) .await .expect("sweep"); assert_eq!(filed, 1, "the candidate has gone untried past the bound"); @@ -505,7 +513,7 @@ async fn an_untried_candidate_goes_overdue_and_a_tested_one_does_not() { .expect("record pass"); assert_eq!( - database::restore::sweep_overdue(&mut conn) + database::restore::sweep_restore_checks(&mut conn) .await .expect("sweep"), 0, @@ -579,7 +587,7 @@ async fn restore_check(conn: &mut AsyncPgConnection, server: Uuid) -> Option(server) .get_result(conn) @@ -675,3 +683,54 @@ async fn a_failed_restore_leaves_the_version_unjudged() { }) .await } + +/// A verdict is a fact about a candidate version measured against a deployment's +/// data, so it surfaces on its own: the report carries no replica reference, no +/// declaration asks for the replica it came from, and there is no overdue bound +/// anywhere. Deriving the check from declarations alone lost this outright. +#[tokio::test(flavor = "multi_thread")] +async fn a_verdict_with_no_declaration_still_surfaces() { + TestDb::run(|mut conn, _url| async move { + let consumer = insert_consumer(&mut conn).await; + let group = insert_group(&mut conn).await; + let server = insert_server(&mut conn, group).await; + let target = insert_version(&mut conn, 63).await; + + let unlinked = report(consumer, group, server, RunOutcome::Success); + assert!( + unlinked.replica_id.is_none(), + "nothing links it to a replica" + ); + MigrationTest::record( + &mut conn, + unlinked, + NewMigrationTest { + target_version_id: target.id, + total_elapsed: secs(45), + failed_migration: Some("backfillNoteTypeIds".into()), + data_bytes_before: 10, + data_bytes_after: 10, + timings: vec![], + }, + ) + .await + .expect("record failure"); + + #[derive(QueryableByName)] + struct Count { + #[diesel(sql_type = sql_types::BigInt)] + count: i64, + } + let declarations: Count = sql_query("SELECT count(*) AS count FROM restore_replicas") + .get_result(&mut conn) + .await + .expect("count declarations"); + assert_eq!(declarations.count, 0, "and no declaration asks for one"); + + let filed = migration_check(&mut conn, server) + .await + .expect("the verdict alone raises the check"); + assert_eq!(filed.observed.as_deref(), Some("warning")); + }) + .await +} diff --git a/crates/database/tests/it/restore.rs b/crates/database/tests/it/restore.rs index b59fbcaf..500e3d93 100644 --- a/crates/database/tests/it/restore.rs +++ b/crates/database/tests/it/restore.rs @@ -21,13 +21,20 @@ struct Count { count: i64, } -/// Count active `restore-verification:*` check-states across a group's -/// servers. The checks are server-scoped now, so join through `servers`. +/// Count active `restore-verification` check-states across a group's servers. +/// +/// Sweeps first: `sweep_restore_checks` is the sole filer of the restore checks and +/// rebuilds each server's from its live declarations, so what a recorded report +/// or a deleted declaration did shows up on the next pass rather than at the +/// moment it happened. async fn active_restore_issues(conn: &mut AsyncPgConnection, group: Uuid) -> i64 { + database::restore::sweep_restore_checks(conn) + .await + .expect("sweep"); sql_query( "SELECT count(*) AS count FROM issues i \ JOIN servers s ON s.id = i.server_id \ - WHERE s.group_id = $1 AND i.ref LIKE 'restore-verification:%' AND i.active = true", + WHERE s.group_id = $1 AND i.ref = 'restore-verification' AND i.active = true", ) .bind::(group) .get_result::(conn) @@ -153,6 +160,26 @@ fn descriptor(intent: &str, semantics: &[&str]) -> IntentDescriptor { } } +/// Declare the replica a report is about, group-wide. The ingest only accepts a +/// report a declaration authorizes, and the sweep only derives instances for +/// declared replicas, so a report standing on its own is a state production +/// cannot reach. +async fn declare(conn: &mut AsyncPgConnection, consumer: Uuid, group: Uuid, intent: &str) -> Uuid { + RestoreReplica::create( + conn, + new_replica( + consumer, + group, + None, + RestoreIntent::from(intent), + &format!("{intent}-all"), + ), + ) + .await + .expect("declare replica") + .id +} + #[tokio::test(flavor = "multi_thread")] async fn create_list_get_roundtrip() { TestDb::run(|mut conn, _url| async move { @@ -430,8 +457,9 @@ async fn record_report_raises_then_recovers() { let consumer = insert_consumer(&mut conn).await; let group = insert_group(&mut conn, "g").await; let server = insert_server(&mut conn, group).await; + declare(&mut conn, consumer, group, "verify").await; - // A failed report raises a per-(server,type,intent) group issue. + // A failed report degrades the server's restore-verification check. BackupRestoreCheck::record_report( &mut conn, new_check( @@ -482,6 +510,7 @@ async fn record_report_files_server_scoped_with_stable_name() { let consumer = insert_consumer(&mut conn).await; let group = insert_group(&mut conn, "g").await; let server = insert_server(&mut conn, group).await; + declare(&mut conn, consumer, group, "verify").await; BackupRestoreCheck::record_report( &mut conn, @@ -496,21 +525,22 @@ async fn record_report_files_server_scoped_with_stable_name() { ) .await .expect("record failure"); + database::restore::sweep_restore_checks(&mut conn) + .await + .expect("sweep"); - // The check is server-scoped with a stable name: the server is the - // scope (issues.server_id), not baked into the check name. + // The check is server-scoped and named for the condition alone: the + // server is the scope (issues.server_id) and the replica an instance, + // neither of them baked into the check name. let rows: Vec = sql_query( "SELECT check_name, server_id, server_group_id FROM issues \ - WHERE source = 'canopy' AND ref LIKE 'restore-verification:%' AND active", + WHERE source = 'canopy' AND ref = 'restore-verification' AND active", ) .load(&mut conn) .await .expect("load"); assert_eq!(rows.len(), 1); - assert_eq!( - rows[0].check_name, - "restore-verification:tamanu-postgres:verify", - ); + assert_eq!(rows[0].check_name, "restore-verification"); assert_eq!(rows[0].server_id, Some(server)); assert_eq!(rows[0].server_group_id, None); }) @@ -523,6 +553,7 @@ async fn record_report_unhealthy_success_still_raises() { let consumer = insert_consumer(&mut conn).await; let group = insert_group(&mut conn, "g").await; let server = insert_server(&mut conn, group).await; + declare(&mut conn, consumer, group, "verify").await; // Restore succeeded but the database wasn't healthy → still a failure. BackupRestoreCheck::record_report( @@ -544,7 +575,7 @@ async fn record_report_unhealthy_success_still_raises() { } #[tokio::test(flavor = "multi_thread")] -async fn sweep_overdue_raises_for_stale_replica_but_skips_gaps() { +async fn sweep_restore_checks_raises_for_stale_replica_but_skips_gaps() { TestDb::run(|mut conn, _url| async move { let consumer = insert_consumer(&mut conn).await; let group = insert_group(&mut conn, "g").await; @@ -585,7 +616,7 @@ async fn sweep_overdue_raises_for_stale_replica_but_skips_gaps() { .await .expect("analytics decl"); - let filed = database::restore::sweep_overdue(&mut conn) + let filed = database::restore::sweep_restore_checks(&mut conn) .await .expect("sweep"); assert_eq!(filed, 1, "only the supported declaration is overdue"); @@ -646,7 +677,9 @@ async fn sweep_once_is_snapshot_driven() { // No snapshot exists yet → nothing to verify, so not overdue. assert_eq!( - database::restore::sweep_overdue(&mut conn).await.unwrap(), + database::restore::sweep_restore_checks(&mut conn) + .await + .unwrap(), 0, "no snapshot → not overdue" ); @@ -654,7 +687,9 @@ async fn sweep_once_is_snapshot_driven() { // A snapshot older than the bound, never verified → overdue. insert_old_success_run(&mut conn, consumer, group, server, "snap-1", 2).await; assert_eq!( - database::restore::sweep_overdue(&mut conn).await.unwrap(), + database::restore::sweep_restore_checks(&mut conn) + .await + .unwrap(), 1, "old unverified snapshot → overdue" ); @@ -678,7 +713,9 @@ async fn sweep_once_is_snapshot_driven() { .await .expect("record healthy"); assert_eq!( - database::restore::sweep_overdue(&mut conn).await.unwrap(), + database::restore::sweep_restore_checks(&mut conn) + .await + .unwrap(), 0, "verified latest snapshot → not overdue" ); @@ -920,7 +957,9 @@ async fn disabling_recovers_the_stale_alert() { } /// Re-enabling is not a recovery event of its own: the sweep picks the -/// declaration back up and re-raises if it is still overdue. +/// declaration back up and the replica's latest report is still the last word on +/// it, so a finding that stood before the replica was decommissioned stands +/// again after it is put back. #[tokio::test(flavor = "multi_thread")] async fn re_enabling_does_not_recover_anything() { TestDb::run(|mut conn, _url| async move { @@ -939,16 +978,6 @@ async fn re_enabling_does_not_recover_anything() { ) .await .expect("create"); - let disabled = RestoreReplica::update( - &mut conn, - r.id, - RestoreReplicaUpdate { - enabled: false, - ..update_from(&r) - }, - ) - .await - .expect("disable"); BackupRestoreCheck::record_report( &mut conn, @@ -965,6 +994,22 @@ async fn re_enabling_does_not_recover_anything() { .expect("record failure"); assert_eq!(active_restore_issues(&mut conn, group).await, 1); + let disabled = RestoreReplica::update( + &mut conn, + r.id, + RestoreReplicaUpdate { + enabled: false, + ..update_from(&r) + }, + ) + .await + .expect("disable"); + assert_eq!( + active_restore_issues(&mut conn, group).await, + 0, + "decommissioning the replica takes its finding with it" + ); + RestoreReplica::update( &mut conn, disabled.id, @@ -978,7 +1023,7 @@ async fn re_enabling_does_not_recover_anything() { assert_eq!( active_restore_issues(&mut conn, group).await, 1, - "re-enabling must not silently clear a live alert" + "re-enabling is not a recovery: the failed report still stands" ); }) .await; @@ -1019,9 +1064,8 @@ async fn delete_recovers_stale_alert_for_removed_scope() { .expect("record failure"); assert_eq!(active_restore_issues(&mut conn, group).await, 1); - // Deleting the declaration removes the only thing tracking that key. - // The sweep only walks current declarations, so without recovery the - // alert would never clear. + // Deleting the declaration removes the only thing tracking that + // replica, so the next sweep rebuilds the server's check without it. RestoreReplica::delete(&mut conn, r.id) .await .expect("delete"); @@ -1344,3 +1388,352 @@ async fn a_version_without_a_published_manifest_is_a_redaction_gap() { }) .await; } + +/// A consumer that stops advertising an intent leaves the declaration standing: +/// the operator still asked for that replica, so the failed restore it last +/// reported is still true and its finding must not quietly vanish. An intent +/// nothing advertises is a gap, surfaced as one. +#[tokio::test(flavor = "multi_thread")] +async fn a_finding_survives_its_capability_being_withdrawn() { + TestDb::run(|mut conn, _url| async move { + let consumer = insert_consumer(&mut conn).await; + let group = insert_group(&mut conn, "g").await; + let server = insert_server(&mut conn, group).await; + RestoreConsumerCapability::register( + &mut conn, + consumer, + &[descriptor("verify", &["check"])], + ) + .await + .expect("register caps"); + RestoreReplica::create( + &mut conn, + new_replica( + consumer, + group, + Some(server), + RestoreIntent::from("verify"), + "nightly", + ), + ) + .await + .expect("create"); + + BackupRestoreCheck::record_report( + &mut conn, + new_check( + consumer, + group, + server, + RestoreIntent::from("verify"), + RunOutcome::Failure, + false, + ), + ) + .await + .expect("record failure"); + assert_eq!(active_restore_issues(&mut conn, group).await, 1); + + // The consumer redeploys advertising nothing at all. + RestoreConsumerCapability::register(&mut conn, consumer, &[]) + .await + .expect("withdraw caps"); + assert_eq!( + active_restore_issues(&mut conn, group).await, + 1, + "withdrawing the capability does not clear what the replica reported" + ); + }) + .await; +} + +/// A report about a server the declaration doesn't name still surfaces. The +/// ingest authorizes a report per (group, type), so a consumer maintaining one +/// replica can report on any of the group's servers, and the finding belongs to +/// the server the report is about. +#[tokio::test(flavor = "multi_thread")] +async fn a_report_about_an_unnamed_server_still_surfaces() { + TestDb::run(|mut conn, _url| async move { + let consumer = insert_consumer(&mut conn).await; + let group = insert_group(&mut conn, "g").await; + let named = insert_server(&mut conn, group).await; + let other = insert_server(&mut conn, group).await; + let r = RestoreReplica::create( + &mut conn, + new_replica( + consumer, + group, + Some(named), + RestoreIntent::from("verify"), + "nightly", + ), + ) + .await + .expect("create"); + + BackupRestoreCheck::record_report( + &mut conn, + new_check( + consumer, + group, + other, + RestoreIntent::from("verify"), + RunOutcome::Failure, + false, + ), + ) + .await + .expect("record failure"); + assert_eq!( + active_restore_issues(&mut conn, group).await, + 1, + "the finding is held against the server the report is about" + ); + + // And it goes when nothing declares the replica any more: no consumer can + // report on it again, so a finding held on it could never recover. + RestoreReplica::delete(&mut conn, r.id) + .await + .expect("delete"); + assert_eq!(active_restore_issues(&mut conn, group).await, 0); + }) + .await; +} + +#[derive(diesel::QueryableByName)] +struct NameRow { + #[diesel(sql_type = sql_types::Text)] + name: String, +} + +/// Every canopy issue ref on a server matching a LIKE pattern. +async fn issue_refs(conn: &mut AsyncPgConnection, server_id: Uuid, pattern: &str) -> Vec { + sql_query( + "SELECT \"ref\" AS name FROM issues \ + WHERE server_id = $1 AND source = 'canopy' AND \"ref\" LIKE $2 ORDER BY \"ref\"", + ) + .bind::(server_id) + .bind::(pattern) + .load::(conn) + .await + .expect("load issue refs") + .into_iter() + .map(|r| r.name) + .collect() +} + +/// Every catalog entry matching a LIKE pattern — what an operator has to +/// configure. +async fn catalog_names(conn: &mut AsyncPgConnection, pattern: &str) -> Vec { + sql_query( + "SELECT check_name AS name FROM check_policies \ + WHERE source = 'canopy' AND check_name LIKE $1 ORDER BY check_name", + ) + .bind::(pattern) + .load::(conn) + .await + .expect("load catalog names") + .into_iter() + .map(|r| r.name) + .collect() +} + +#[derive(diesel::QueryableByName)] +struct FiledRow { + #[diesel(sql_type = sql_types::Text)] + message: String, + #[diesel(sql_type = sql_types::Nullable)] + detail: Option, +} + +async fn filed(conn: &mut AsyncPgConnection, server_id: Uuid, r#ref: &str) -> FiledRow { + sql_query( + "SELECT message, detail FROM issues \ + WHERE server_id = $1 AND source = 'canopy' AND \"ref\" = $2 AND active", + ) + .bind::(server_id) + .bind::(r#ref) + .get_result::(conn) + .await + .unwrap_or_else(|e| panic!("{ref} was filed for {server_id}: {e}")) +} + +/// A server with several replicas holds one check of each kind, with the +/// replicas as instances: the message names the ones in trouble, the detail +/// carries them with their own results, and the catalog gains one entry per +/// check rather than one per (type, intent) pair. +#[tokio::test(flavor = "multi_thread")] +async fn one_check_of_each_kind_per_server_with_the_replicas_as_instances() { + TestDb::run(|mut conn, _url| async move { + let consumer = insert_consumer(&mut conn).await; + let group = insert_group(&mut conn, "g").await; + let server = insert_server(&mut conn, group).await; + RestoreConsumerCapability::register( + &mut conn, + consumer, + &[ + descriptor("verify", &["check"]), + descriptor("analytics", &["check"]), + descriptor("dr", &["check"]), + ], + ) + .await + .expect("register caps"); + + for (intent, name, redacts) in [ + ("verify", "nightly-verify", false), + ("analytics", "analytics-copy", true), + ("dr", "dr-standby", false), + ] { + RestoreReplica::create( + &mut conn, + NewRestoreReplica { + redacts, + ..new_replica( + consumer, + group, + Some(server), + RestoreIntent::from(intent), + name, + ) + }, + ) + .await + .expect("declare replica"); + } + + // The verifying replica failed; the analytics one restored fine but came + // up with columns it should have masked; the standby is healthy. + BackupRestoreCheck::record_report( + &mut conn, + new_check( + consumer, + group, + server, + RestoreIntent::from("verify"), + RunOutcome::Failure, + false, + ), + ) + .await + .expect("record verify"); + BackupRestoreCheck::record_report( + &mut conn, + NewBackupRestoreCheck { + redaction_outcome: Some(commons_types::backup::RedactionOutcome::Partial), + redaction_columns_masked: Some(40), + redaction_columns_skipped: Some(3), + ..new_check( + consumer, + group, + server, + RestoreIntent::from("analytics"), + RunOutcome::Success, + true, + ) + }, + ) + .await + .expect("record analytics"); + BackupRestoreCheck::record_report( + &mut conn, + new_check( + consumer, + group, + server, + RestoreIntent::from("dr"), + RunOutcome::Success, + true, + ), + ) + .await + .expect("record dr"); + + // And a candidate version whose migrations do not survive the data. + let version: RowId = sql_query( + "INSERT INTO versions (major, minor, patch, status) \ + VALUES (2, 63, 0, 'published') RETURNING id", + ) + .get_result(&mut conn) + .await + .expect("insert version"); + database::migration_tests::MigrationTest::record( + &mut conn, + new_check( + consumer, + group, + server, + RestoreIntent::from("migrate"), + RunOutcome::Success, + true, + ), + database::migration_tests::NewMigrationTest { + target_version_id: version.id, + total_elapsed: PgDuration(SignedDuration::from_secs(45)), + failed_migration: Some("backfillNoteTypeIds".into()), + data_bytes_before: 10, + data_bytes_after: 10, + timings: vec![], + }, + ) + .await + .expect("record migration test"); + + database::restore::sweep_restore_checks(&mut conn) + .await + .expect("sweep"); + + for r#ref in ["restore-verification", "redaction", "migration-test"] { + assert_eq!( + issue_refs(&mut conn, server, &format!("{ref}%")).await, + vec![r#ref.to_string()], + "one {ref} check for the server, named for the condition only", + ); + assert_eq!( + catalog_names(&mut conn, &format!("{ref}%")).await, + vec![r#ref.to_string()], + "one {ref} catalog entry to configure, not one per (type, intent)", + ); + } + + // Restore verification: three replicas considered, one of them degraded, + // and the message names it rather than the healthy ones. + let verification = filed(&mut conn, server, "restore-verification").await; + let detail = verification.detail.expect("detail"); + assert_eq!(detail["total"], 3); + assert_eq!(detail["degraded"], 1); + let instances = detail["instances"].as_array().expect("instances"); + assert_eq!(instances.len(), 1); + assert_eq!(instances[0]["intent"], "verify"); + assert_eq!(instances[0]["replica"], "nightly-verify"); + assert_eq!(instances[0]["replica_key"], "tamanu-postgres:verify"); + assert!( + verification.message.contains("nightly-verify"), + "names the degraded replica: {}", + verification.message, + ); + assert!( + !verification.message.contains("dr-standby"), + "and not the healthy ones: {}", + verification.message, + ); + + // Redaction only counts the replicas that reported one, so the check is + // about the analytics copy alone. + let redaction = filed(&mut conn, server, "redaction").await; + let detail = redaction.detail.expect("detail"); + assert_eq!(detail["total"], 1); + assert_eq!(detail["instances"][0]["intent"], "analytics"); + assert_eq!(detail["instances"][0]["columns_skipped"], 3); + + // The migration finding carries the version in its detail, not its name. + let migration = filed(&mut conn, server, "migration-test").await; + let detail = migration.detail.expect("detail"); + assert_eq!(detail["instances"][0]["target_version"], "2.63.0"); + assert_eq!( + detail["instances"][0]["failed_migration"], + "backfillNoteTypeIds" + ); + }) + .await; +} diff --git a/crates/private-server/src/fns/restore_replicas.rs b/crates/private-server/src/fns/restore_replicas.rs index 87f56202..b5ecc728 100644 --- a/crates/private-server/src/fns/restore_replicas.rs +++ b/crates/private-server/src/fns/restore_replicas.rs @@ -760,9 +760,10 @@ pub async fn create( /// `2h 30m`, `20Gi`), resolved to raw seconds/bytes and validated against the /// *new* consumer+intent's advertised parameter schema; as with `create`, an /// intent the new consumer doesn't currently advertise is accepted and the -/// values pass through unvalidated, leaving the declaration with a gap. If -/// the scope changes, any active restore-verification alert for the -/// declaration's old scope is recovered. The name must be unique among the +/// values pass through unvalidated, leaving the declaration with a gap. If the +/// scope changes, the replica at the old scope stops being one Canopy derives +/// checks for, so any active restore-verification finding against it recovers on +/// the next periodic sweep. The name must be unique among the /// consumer's declarations. Requires the caller to be on the admin allow-list. /// Responds 400 if the name is blank or the overdue bound or a parameter value /// fails to parse or validate, 404 if the declaration does not exist, and 409 @@ -818,10 +819,11 @@ pub async fn update( /// Delete a restore replica declaration. /// /// Removes the declaration: the consumer stops being asked to maintain the -/// replica and loses the backup access the declaration granted. Any active -/// restore-verification alert for the declaration's scope is recovered, since -/// nothing tracks that scope any more. The restore-health reports it collected -/// are retained, detached from the deleted declaration. Requires the caller to +/// replica and loses the backup access the declaration granted. Nothing asks for +/// that replica any more, so it stops being one Canopy derives checks for and +/// any active restore-verification finding against it recovers on the next +/// periodic sweep. The restore-health reports it collected are retained, +/// detached from the deleted declaration. Requires the caller to /// be on the admin allow-list. Responds 404 if the declaration does not exist. #[utoipa::path( post, diff --git a/crates/public-server/tests/it/restore.rs b/crates/public-server/tests/it/restore.rs index cb526ef9..fab243b8 100644 --- a/crates/public-server/tests/it/restore.rs +++ b/crates/public-server/tests/it/restore.rs @@ -651,12 +651,19 @@ async fn restore_verification_records_and_raises_alert() { .await, 1, ); + + // The report is recorded on ingest; the checks it feeds are filed by + // the sweep, which is their sole filer (see BackupRestoreCheck:: + // record_report). + database::restore::sweep_restore_checks(&mut conn) + .await + .expect("sweep"); assert_eq!( count( &mut conn, "SELECT count(*) AS count FROM issues i \ JOIN servers s ON s.id = i.server_id \ - WHERE s.group_id = $1 AND i.ref LIKE 'restore-verification:%' AND i.active = true", + WHERE s.group_id = $1 AND i.ref = 'restore-verification' AND i.active = true", group, ) .await, @@ -667,7 +674,7 @@ async fn restore_verification_records_and_raises_alert() { &mut conn, "SELECT count(*) AS count FROM issues i \ JOIN servers s ON s.id = i.server_id \ - WHERE s.group_id = $1 AND i.ref LIKE 'redaction:%'", + WHERE s.group_id = $1 AND i.ref = 'redaction'", group, ) .await, @@ -724,12 +731,16 @@ async fn a_partial_redaction_warns_while_the_restore_stays_healthy() { 1, "the reported redaction is stored first-class", ); + + database::restore::sweep_restore_checks(&mut conn) + .await + .expect("sweep"); assert_eq!( count( &mut conn, "SELECT count(*) AS count FROM issues i \ JOIN servers s ON s.id = i.server_id \ - WHERE s.group_id = $1 AND i.ref LIKE 'redaction:%' AND i.active = true", + WHERE s.group_id = $1 AND i.ref = 'redaction' AND i.active = true", group, ) .await, @@ -741,7 +752,7 @@ async fn a_partial_redaction_warns_while_the_restore_stays_healthy() { &mut conn, "SELECT count(*) AS count FROM issues i \ JOIN servers s ON s.id = i.server_id \ - WHERE s.group_id = $1 AND i.ref LIKE 'restore-verification:%' AND i.active = true", + WHERE s.group_id = $1 AND i.ref = 'restore-verification' AND i.active = true", group, ) .await, @@ -785,10 +796,13 @@ async fn a_failed_redaction_warns_and_then_recovers_when_it_applies() { .json(&report("failed", Some("manifest host unreachable"))) .await .assert_status(http::StatusCode::NO_CONTENT); + database::restore::sweep_restore_checks(&mut conn) + .await + .expect("sweep"); const ACTIVE_REDACTION_CHECKS: &str = "SELECT count(*) AS count FROM issues i \ JOIN servers s ON s.id = i.server_id \ - WHERE s.group_id = $1 AND i.ref LIKE 'redaction:%' AND i.active = true"; + WHERE s.group_id = $1 AND i.ref = 'redaction' AND i.active = true"; assert_eq!( count(&mut conn, ACTIVE_REDACTION_CHECKS, group).await, 1, @@ -801,6 +815,9 @@ async fn a_failed_redaction_warns_and_then_recovers_when_it_applies() { .json(&report("complete", None)) .await .assert_status(http::StatusCode::NO_CONTENT); + database::restore::sweep_restore_checks(&mut conn) + .await + .expect("sweep"); assert_eq!( count(&mut conn, ACTIVE_REDACTION_CHECKS, group).await, 0, diff --git a/migrations/2026-08-04-041158-0000_collapse_restore_check_names/down.sql b/migrations/2026-08-04-041158-0000_collapse_restore_check_names/down.sql new file mode 100644 index 00000000..1f7c86eb --- /dev/null +++ b/migrations/2026-08-04-041158-0000_collapse_restore_check_names/down.sql @@ -0,0 +1,4 @@ +-- Irreversible: the per-(type, intent) rows that lost the collapse are deleted, +-- and which pair each survivor came from isn't recoverable from the collapsed +-- name. A silence's guard could be read back out of the rewritten rules, but the +-- ceiling it gave up cannot. Nothing to restore. diff --git a/migrations/2026-08-04-041158-0000_collapse_restore_check_names/up.sql b/migrations/2026-08-04-041158-0000_collapse_restore_check_names/up.sql new file mode 100644 index 00000000..50704f03 --- /dev/null +++ b/migrations/2026-08-04-041158-0000_collapse_restore_check_names/up.sql @@ -0,0 +1,192 @@ +-- The restore sweeps spelled the replica's type and intent into the check name: +-- restore-verification:tamanu-postgres:verify, redaction:tamanu-postgres:analytics, +-- migration-test:tamanu-postgres:migrate, and so on. Both dimensions are +-- open-ended strings, so that produced a check — and a catalog row, and a +-- listing entry — per (type, intent) pair a deployment happened to declare, for +-- one condition. A check name is a category an operator configures once; +-- anything that varies per instance belongs in the detail, where policy rules +-- read it as check.. See the Names section of the CHK spec. +-- +-- Each of these three checks is now filed once per server with its replicas as +-- instances, graded per replica and settling on the most urgent. Collapse the +-- stored state to match. This mirrors the collapse the backup checks went +-- through in 2026-08-04-022520, extended to names carrying two parameters +-- instead of one. + +-- 1. Issues. Several per-(type, intent) rows collapse onto one per +-- (target, source, ref), so pick a survivor per target and drop the rest +-- rather than letting the uniqueness constraints reject the rename. The +-- survivor is the most urgent row, then the most recently seen: that keeps +-- the row an operator is most likely already looking at, with its incident +-- membership and history. +-- +-- The survivor's per-replica detail is left as it is. It describes one +-- replica where the new shape carries every degraded instance, and the next +-- sweep overwrites it with the aggregate — within a minute, since the sweep +-- runs on the reachability tick. +CREATE TEMP TABLE restore_check_renames ON COMMIT DROP AS +SELECT + id, + base, + row_number() OVER ( + PARTITION BY source, base, server_id, server_group_id + ORDER BY + CASE effective_result + WHEN 'failed' THEN 0 + WHEN 'warning' THEN 1 + WHEN 'broken' THEN 2 + WHEN 'passed' THEN 3 + ELSE 4 + END, + last_seen DESC, + id + ) AS rank +FROM ( + SELECT i.*, split_part(i.ref, ':', 1) AS base + FROM issues i + WHERE i.source = 'canopy' + AND i.ref LIKE '%:%' + AND split_part(i.ref, ':', 1) IN ( + 'restore-verification', + 'redaction', + 'migration-test' + ) +) AS parameterised; + +-- Which incidents the departing rows were contributing to, captured before the +-- rows go so the emptied ones can be retired afterwards. +CREATE TEMP TABLE touched_restore_incidents ON COMMIT DROP AS +SELECT DISTINCT il.incident_id +FROM incident_issues il +WHERE il.left_at IS NULL + AND il.issue_id IN (SELECT id FROM restore_check_renames WHERE rank > 1); + +UPDATE incident_issues +SET left_at = now() +WHERE left_at IS NULL + AND issue_id IN (SELECT id FROM restore_check_renames WHERE rank > 1); + +DELETE FROM issues +WHERE id IN (SELECT id FROM restore_check_renames WHERE rank > 1); + +UPDATE issues AS i +SET ref = r.base, check_name = r.base +FROM restore_check_renames r +WHERE i.id = r.id AND r.rank = 1; + +-- Retire incidents the departures emptied of failing contributors. Mirrors the +-- leave path in re_evaluate_incident_membership: an incident is held open by its +-- currently-failing contributors. No Slack resolve is enqueued — the condition +-- didn't change, only how many rows describe it. +UPDATE incidents AS inc +SET closed_at = now() +WHERE inc.closed_at IS NULL + AND inc.id IN (SELECT incident_id FROM touched_restore_incidents) + AND NOT EXISTS ( + SELECT 1 + FROM incident_issues il + JOIN issues i ON i.id = il.issue_id + WHERE il.incident_id = inc.id + AND il.left_at IS NULL + AND i.effective_result = 'failed' + ); + +-- 2. Scoped policies, which is where silences live. A silence on +-- restore-verification:tamanu-postgres:verify meant "silence restore +-- verification for that one replica". Under one check name the equivalent is +-- a rule that skips the instance the silence was written for, which is what +-- per-instance grading is for. Rewrite them so operators keep the exact +-- suppression they set up, instead of it silently widening to every replica of +-- the server or being dropped. +-- +-- The rules column is an if-ladder: {"if": [condition, result, …]} (see +-- check_policies::IfLadder, and Condition's JsonLogic encoding). A Condition +-- is one operator over one variable and the DSL has no and/or, so two +-- conditions — check.type and check.intent — cannot be combined into a single +-- guard. Every instance therefore carries check.replica_key, the two +-- dimensions joined as ":": one variable, and it identifies the +-- replica exactly, so the guard matches on that rather than either dimension +-- alone. A ceiling-only silence becomes a single-branch ladder and gives up +-- its ceiling; a row that already carried rules keeps them, with the guard +-- prepended so the guard wins. +UPDATE scoped_check_policies +SET + check_name = split_part(check_name, ':', 1), + ceiling = NULL, + rules = jsonb_build_object( + 'if', + jsonb_build_array( + jsonb_build_object( + '==', + jsonb_build_array( + jsonb_build_object('var', 'check.replica_key'), + substring(check_name from position(':' in check_name) + 1) + ) + ), + COALESCE(ceiling, 'skipped') + ) || COALESCE(rules -> 'if', '[]'::jsonb) + ) +WHERE source = 'canopy' + AND check_name LIKE '%:%' + AND split_part(check_name, ':', 1) IN ( + 'restore-verification', + 'redaction', + 'migration-test' + ); + +-- 3. Catalog rows: the thing that made an operator configure one condition once +-- per (type, intent) pair. Keep the most deliberately-set row's policy for the +-- collapsed name — an operator-reviewed row beats a canopy-seeded one, and +-- among equals the most urgent ceiling wins — and drop the rest. If nothing +-- survives, the next filing re-registers the collapsed name with its shipped +-- defaults. +CREATE TEMP TABLE restore_policy_renames ON COMMIT DROP AS +SELECT + source, + check_name, + base, + row_number() OVER ( + PARTITION BY source, base + ORDER BY + CASE WHEN reviewed_by IS DISTINCT FROM 'canopy' THEN 0 ELSE 1 END, + CASE ceiling + WHEN 'failed' THEN 0 + WHEN 'warning' THEN 1 + WHEN 'broken' THEN 2 + WHEN 'passed' THEN 3 + ELSE 4 + END, + check_name + ) AS rank +FROM ( + SELECT p.*, split_part(p.check_name, ':', 1) AS base + FROM check_policies p + WHERE p.source = 'canopy' + AND p.check_name LIKE '%:%' + AND split_part(p.check_name, ':', 1) IN ( + 'restore-verification', + 'redaction', + 'migration-test' + ) +) AS parameterised; + +DELETE FROM check_policies p +USING restore_policy_renames r +WHERE p.source = r.source AND p.check_name = r.check_name AND r.rank > 1; + +-- A bare collapsed row already existing wins outright; the parameterised one +-- goes rather than colliding on the rename. +DELETE FROM check_policies p +USING restore_policy_renames r +WHERE p.source = r.source + AND p.check_name = r.check_name + AND r.rank = 1 + AND EXISTS ( + SELECT 1 FROM check_policies q + WHERE q.source = r.source AND q.check_name = r.base + ); + +UPDATE check_policies p +SET check_name = r.base +FROM restore_policy_renames r +WHERE p.source = r.source AND p.check_name = r.check_name AND r.rank = 1; diff --git a/private-web/openapi.json b/private-web/openapi.json index 55ada322..eaf3adfa 100644 --- a/private-web/openapi.json +++ b/private-web/openapi.json @@ -4808,7 +4808,7 @@ "restore_replicas" ], "summary": "Delete a restore replica declaration.", - "description": "Removes the declaration: the consumer stops being asked to maintain the\nreplica and loses the backup access the declaration granted. Any active\nrestore-verification alert for the declaration's scope is recovered, since\nnothing tracks that scope any more. The restore-health reports it collected\nare retained, detached from the deleted declaration. Requires the caller to\nbe on the admin allow-list. Responds 404 if the declaration does not exist.", + "description": "Removes the declaration: the consumer stops being asked to maintain the\nreplica and loses the backup access the declaration granted. Nothing asks for\nthat replica any more, so it stops being one Canopy derives checks for and\nany active restore-verification finding against it recovers on the next\nperiodic sweep. The restore-health reports it collected are retained,\ndetached from the deleted declaration. Requires the caller to\nbe on the admin allow-list. Responds 404 if the declaration does not exist.", "operationId": "restore_replicas_delete", "requestBody": { "content": { @@ -4888,7 +4888,7 @@ "restore_replicas" ], "summary": "Update a restore replica declaration.", - "description": "Replaces every field, including scope: the consumer, group, server,\nbackup type, and intent can be retargeted in the same call as the name,\noverdue bound, parameter values, and enabled flag. The overdue bound and\n`duration`/`bytes` parameter values accept human-unit strings (e.g.\n`2h 30m`, `20Gi`), resolved to raw seconds/bytes and validated against the\n*new* consumer+intent's advertised parameter schema; as with `create`, an\nintent the new consumer doesn't currently advertise is accepted and the\nvalues pass through unvalidated, leaving the declaration with a gap. If\nthe scope changes, any active restore-verification alert for the\ndeclaration's old scope is recovered. The name must be unique among the\nconsumer's declarations. Requires the caller to be on the admin allow-list.\nResponds 400 if the name is blank or the overdue bound or a parameter value\nfails to parse or validate, 404 if the declaration does not exist, and 409\nif the new scope or name collides with another declaration.", + "description": "Replaces every field, including scope: the consumer, group, server,\nbackup type, and intent can be retargeted in the same call as the name,\noverdue bound, parameter values, and enabled flag. The overdue bound and\n`duration`/`bytes` parameter values accept human-unit strings (e.g.\n`2h 30m`, `20Gi`), resolved to raw seconds/bytes and validated against the\n*new* consumer+intent's advertised parameter schema; as with `create`, an\nintent the new consumer doesn't currently advertise is accepted and the\nvalues pass through unvalidated, leaving the declaration with a gap. If the\nscope changes, the replica at the old scope stops being one Canopy derives\nchecks for, so any active restore-verification finding against it recovers on\nthe next periodic sweep. The name must be unique among the\nconsumer's declarations. Requires the caller to be on the admin allow-list.\nResponds 400 if the name is blank or the overdue bound or a parameter value\nfails to parse or validate, 404 if the declaration does not exist, and 409\nif the new scope or name collides with another declaration.", "operationId": "restore_replicas_update", "requestBody": { "content": { diff --git a/private-web/src/api-types.ts b/private-web/src/api-types.ts index 536d2ea0..b9f3ffc8 100644 --- a/private-web/src/api-types.ts +++ b/private-web/src/api-types.ts @@ -2443,10 +2443,11 @@ export interface paths { /** * Delete a restore replica declaration. * @description Removes the declaration: the consumer stops being asked to maintain the - * replica and loses the backup access the declaration granted. Any active - * restore-verification alert for the declaration's scope is recovered, since - * nothing tracks that scope any more. The restore-health reports it collected - * are retained, detached from the deleted declaration. Requires the caller to + * replica and loses the backup access the declaration granted. Nothing asks for + * that replica any more, so it stops being one Canopy derives checks for and + * any active restore-verification finding against it recovers on the next + * periodic sweep. The restore-health reports it collected are retained, + * detached from the deleted declaration. Requires the caller to * be on the admin allow-list. Responds 404 if the declaration does not exist. */ post: operations["restore_replicas_delete"]; @@ -2496,9 +2497,10 @@ export interface paths { * `2h 30m`, `20Gi`), resolved to raw seconds/bytes and validated against the * *new* consumer+intent's advertised parameter schema; as with `create`, an * intent the new consumer doesn't currently advertise is accepted and the - * values pass through unvalidated, leaving the declaration with a gap. If - * the scope changes, any active restore-verification alert for the - * declaration's old scope is recovered. The name must be unique among the + * values pass through unvalidated, leaving the declaration with a gap. If the + * scope changes, the replica at the old scope stops being one Canopy derives + * checks for, so any active restore-verification finding against it recovers on + * the next periodic sweep. The name must be unique among the * consumer's declarations. Requires the caller to be on the admin allow-list. * Responds 400 if the name is blank or the overdue bound or a parameter value * fails to parse or validate, 404 if the declaration does not exist, and 409