diff --git a/dev-tools/omdb/tests/successes.out b/dev-tools/omdb/tests/successes.out index 11bbb64a8ab..ee1f67b7393 100644 --- a/dev-tools/omdb/tests/successes.out +++ b/dev-tools/omdb/tests/successes.out @@ -700,19 +700,22 @@ task: "fm_analysis" configured period: every m last completed activation: , triggered by started at (s ago) and ran for ms - parent sitrep ID: None + parent sitrep ID: Some(..................... (sitrep)) current inventory collection ID: Some(..................... (collection)) ereport classes consumed: (none) FAULT MANAGEMENT ANALYSIS SUMMARY ================================= -/!\ analysis failed: FM analysis is not yet implemented + no changes from the current situation report (Some(..................... (sitrep))) preparation report: - parent sitrep: + parent sitrep: ..................... inventory collection: ..................... + no new ereports since the parent sitrep no cases copied forward + no in-service control plane disks + analysis report: sitrep ID: ..................... no cases changed in this analysis step @@ -722,28 +725,28 @@ task: "fm_rendezvous" configured period: every m last completed activation: , triggered by started at (s ago) and ran for ms -(i) no FM situation report loaded, so rendezvous was not performed + current sitrep: ..................... creating requested alerts: -(i) note: this operation was not executed + started at (s ago) and ran for ms alerts requested: 0 requested in this sitrep: 0 created in this activation: 0 already created: 0 errors: 0 creating requested support bundles: -(i) note: this operation was not executed + started at (s ago) and ran for ms support bundles requested: 0 requested in this sitrep: 0 created in this activation: 0 already created: 0 errors: 0 marking ereports as seen: -(i) note: this operation was not executed + started at (s ago) and ran for ms total ereports in sitrep: 0 not marked when the sitrep was loaded: 0 marked seen by this activation: 0 already marked seen: 0 - batch size: 0 + batch size: 1000 batches: 0 errors: 0 @@ -752,7 +755,7 @@ task: "fm_sitrep_gc" last completed activation: , triggered by started at (s ago) and ran for ms batch size: 1000 - orphaned sitreps deleted: 0 + orphaned sitreps deleted: batches: 1 orphaned fm_alert_request rows deleted: 0 batches: 1 @@ -760,7 +763,9 @@ task: "fm_sitrep_gc" batches: 1 orphaned fm_ereport_in_case rows deleted: 0 batches: 1 - orphaned fm_sitrep_analysis_report rows deleted: 0 + orphaned fm_fact_physical_disk rows deleted: 0 + batches: 1 + orphaned fm_sitrep_analysis_report rows deleted: batches: 1 orphaned fm_support_bundle_request rows deleted: 0 batches: 1 @@ -775,7 +780,9 @@ task: "fm_sitrep_loader" configured period: every s last completed activation: , triggered by started at (s ago) and ran for ms - no FM situation report available to load + loaded latest FM situation report as of : + sitrep ..................... (sitrep) (v1) + made current at: task: "instance_reincarnation" configured period: every m @@ -1391,19 +1398,22 @@ task: "fm_analysis" configured period: every m last completed activation: , triggered by started at (s ago) and ran for ms - parent sitrep ID: None + parent sitrep ID: Some(..................... (sitrep)) current inventory collection ID: Some(..................... (collection)) ereport classes consumed: (none) FAULT MANAGEMENT ANALYSIS SUMMARY ================================= -/!\ analysis failed: FM analysis is not yet implemented + no changes from the current situation report (Some(..................... (sitrep))) preparation report: - parent sitrep: + parent sitrep: ..................... inventory collection: ..................... + no new ereports since the parent sitrep no cases copied forward + no in-service control plane disks + analysis report: sitrep ID: ..................... no cases changed in this analysis step @@ -1413,28 +1423,28 @@ task: "fm_rendezvous" configured period: every m last completed activation: , triggered by started at (s ago) and ran for ms -(i) no FM situation report loaded, so rendezvous was not performed + current sitrep: ..................... creating requested alerts: -(i) note: this operation was not executed + started at (s ago) and ran for ms alerts requested: 0 requested in this sitrep: 0 created in this activation: 0 already created: 0 errors: 0 creating requested support bundles: -(i) note: this operation was not executed + started at (s ago) and ran for ms support bundles requested: 0 requested in this sitrep: 0 created in this activation: 0 already created: 0 errors: 0 marking ereports as seen: -(i) note: this operation was not executed + started at (s ago) and ran for ms total ereports in sitrep: 0 not marked when the sitrep was loaded: 0 marked seen by this activation: 0 already marked seen: 0 - batch size: 0 + batch size: 1000 batches: 0 errors: 0 @@ -1443,7 +1453,7 @@ task: "fm_sitrep_gc" last completed activation: , triggered by started at (s ago) and ran for ms batch size: 1000 - orphaned sitreps deleted: 0 + orphaned sitreps deleted: batches: 1 orphaned fm_alert_request rows deleted: 0 batches: 1 @@ -1451,7 +1461,9 @@ task: "fm_sitrep_gc" batches: 1 orphaned fm_ereport_in_case rows deleted: 0 batches: 1 - orphaned fm_sitrep_analysis_report rows deleted: 0 + orphaned fm_fact_physical_disk rows deleted: 0 + batches: 1 + orphaned fm_sitrep_analysis_report rows deleted: batches: 1 orphaned fm_support_bundle_request rows deleted: 0 batches: 1 @@ -1466,7 +1478,9 @@ task: "fm_sitrep_loader" configured period: every s last completed activation: , triggered by started at (s ago) and ran for ms - no FM situation report available to load + loaded latest FM situation report as of : + sitrep ..................... (sitrep) (v1) + made current at: task: "instance_reincarnation" configured period: every m diff --git a/dev-tools/omdb/tests/test_all_output.rs b/dev-tools/omdb/tests/test_all_output.rs index 815c4e757b5..777a9c6047f 100644 --- a/dev-tools/omdb/tests/test_all_output.rs +++ b/dev-tools/omdb/tests/test_all_output.rs @@ -11,6 +11,7 @@ use dropshot::Method; use expectorate::assert_contents; use gateway_client::ClientInfo as _; use http::StatusCode; +use nexus_test_utils::background::activate_background_task; use nexus_test_utils::wait_for_producer; use nexus_test_utils::{OXIMETER_UUID, PRODUCER_UUID}; use nexus_test_utils_macros::nexus_test; @@ -187,6 +188,27 @@ async fn test_omdb_success_cases() { .wait_for_at_least_one_inventory_collection(Duration::from_secs(60)) .await; + // Drive the FM task pipeline to its steady state, so that the omdb + // snapshot of each task's last completed activation is deterministic + // rather than racing the tasks' watch-channel triggers: + // + // 1. `fm_analysis` commits the first sitrep (unless its natural cadence + // already has). This run reports "committed new sitrep". + // 2. `fm_sitrep_loader` loads that sitrep and publishes it on the sitrep + // watch channel. + // 3. `fm_analysis` re-runs with the loaded sitrep as its parent and + // reports "no changes" -- the steady-state output asserted below. + // (No later activation ever commits another sitrep here: this + // environment has no in-service control plane disks and no + // consumable ereports, so every post-load analysis is a no-op.) + // 4. `fm_rendezvous` runs against the loaded sitrep, so its status shows + // the executed operations rather than "no FM situation report loaded". + let lockstep_client = &cptestctx.lockstep_client; + activate_background_task(lockstep_client, "fm_analysis").await; + activate_background_task(lockstep_client, "fm_sitrep_loader").await; + activate_background_task(lockstep_client, "fm_analysis").await; + activate_background_task(lockstep_client, "fm_rendezvous").await; + let mut output = String::new(); let invocations: &[&[&str]] = &[ @@ -358,6 +380,28 @@ async fn test_omdb_success_cases() { .field("triggered by", r"[\w ]+") .section(&["task: \"tuf_artifact_replication\"", "request ringbuf:"]); + // The `fm_analysis` task's input report includes a line comparing the + // current inventory collection against the parent sitrep's collection, + // which can be either "same" or "different" depending on whether a new + // inventory was collected between sitreps. Collapse both forms. + redactor.variable_regex( + "fm_input_inv_comparison", + r" --> (same collection as parent sitrep|different from parent sitrep \(collection [-a-f0-9]+\))", + ); + + // The `fm_sitrep_gc` task's orphan counts are racy. When two `fm_analysis` + // activations overlap (e.g. the boot-time activation and the explicit drive + // above), both can insert a first sitrep before either is made current; the + // loser's sitrep and its stashed analysis report are inserted but orphaned + // (see `DataStore::fm_sitrep_insert`'s `ParentNotCurrent` path), and a + // later GC pass deletes them. Whether that race happened before this + // snapshot is timing-dependent, so redact both counts. Other child tables + // stay zero: with no faults, orphaned sitreps carry no cases, facts, + // ereports, or bundles. + redactor + .field("orphaned sitreps deleted:", r"\d+") + .field("orphaned fm_sitrep_analysis_report rows deleted:", r"\d+"); + // The `sp_ereport_ingester` task's output depends on how many simulated // sled agents ahppen to register with Nexus before its first execution. // These redactions work around the issue described in diff --git a/nexus/db-model/src/fm.rs b/nexus/db-model/src/fm.rs index cea76f3299a..b3013d9ab25 100644 --- a/nexus/db-model/src/fm.rs +++ b/nexus/db-model/src/fm.rs @@ -26,6 +26,8 @@ mod case; pub use case::*; mod diagnosis_engine; pub use diagnosis_engine::*; +mod fact_physical_disk; +pub use fact_physical_disk::*; mod support_bundle_request; pub use support_bundle_request::*; mod sitrep_analysis_report; diff --git a/nexus/db-model/src/fm/case.rs b/nexus/db-model/src/fm/case.rs index 04776f99539..d2e34cf06ed 100644 --- a/nexus/db-model/src/fm/case.rs +++ b/nexus/db-model/src/fm/case.rs @@ -64,6 +64,7 @@ impl CaseMetadata { alerts_requested: _, support_bundles_requested: _, ereports: _, + facts: _, } = case; Self { sitrep_id: sitrep_id.into(), diff --git a/nexus/db-model/src/fm/diagnosis_engine.rs b/nexus/db-model/src/fm/diagnosis_engine.rs index 7d354142bbb..a99ceeac1ad 100644 --- a/nexus/db-model/src/fm/diagnosis_engine.rs +++ b/nexus/db-model/src/fm/diagnosis_engine.rs @@ -24,6 +24,7 @@ impl_enum_type!( pub enum DiagnosisEngine; PowerShelf => b"power_shelf" + PhysicalDisk => b"physical_disk" ); @@ -31,6 +32,9 @@ impl From for fm::DiagnosisEngineKind { fn from(de: DiagnosisEngine) -> Self { match de { DiagnosisEngine::PowerShelf => fm::DiagnosisEngineKind::PowerShelf, + DiagnosisEngine::PhysicalDisk => { + fm::DiagnosisEngineKind::PhysicalDisk + } } } } @@ -39,6 +43,9 @@ impl From for DiagnosisEngine { fn from(fm_de: fm::DiagnosisEngineKind) -> Self { match fm_de { fm::DiagnosisEngineKind::PowerShelf => DiagnosisEngine::PowerShelf, + fm::DiagnosisEngineKind::PhysicalDisk => { + DiagnosisEngine::PhysicalDisk + } } } } diff --git a/nexus/db-model/src/fm/fact_physical_disk.rs b/nexus/db-model/src/fm/fact_physical_disk.rs new file mode 100644 index 00000000000..2c1518f0077 --- /dev/null +++ b/nexus/db-model/src/fm/fact_physical_disk.rs @@ -0,0 +1,164 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Database representation of the physical-disk diagnosis engine's facts. +//! +//! Each physical-disk fact is stored as typed columns in the +//! `fm_fact_physical_disk` table. The `kind` discriminant selects which payload +//! columns are populated. + +use crate::DbTypedUuid; +use crate::impl_enum_type; +use crate::inventory::InvZpoolHealth; +use chrono::{DateTime, Utc}; +use nexus_db_schema::schema::fm_fact_physical_disk; +use nexus_types::fm; +use nexus_types::fm::case::FactMetadata; +use nexus_types::fm::{DiskFact, FactPayload, ZpoolUnhealthyFactPayload}; +use omicron_common::api::external::Error; +use omicron_uuid_kinds::{ + CaseKind, CollectionKind, FactKind, PhysicalDiskKind, SitrepKind, ZpoolKind, +}; + +impl_enum_type!( + FmFactPhysicalDiskKindEnum: + + #[derive(Clone, Copy, Debug, AsExpression, FromSqlRow, PartialEq, Eq)] + pub enum FmFactPhysicalDiskKind; + + ZpoolUnhealthy => b"zpool_unhealthy" +); + +/// Diesel row for the `fm_fact_physical_disk` table. +/// +/// The payload columns are populated according to `kind`: a column is `Some` +/// if it belongs to that `kind`'s payload, and `None` otherwise. +#[derive(Queryable, Insertable, Clone, Debug, Selectable)] +#[diesel(table_name = fm_fact_physical_disk)] +pub struct FmFactPhysicalDisk { + pub id: DbTypedUuid, + /// The sitrep to which this fact belongs. + /// + /// This will change as the fact is carried forward from one sitrep to the + /// next. + pub sitrep_id: DbTypedUuid, + pub case_id: DbTypedUuid, + /// Sitrep in which this fact was first added. + /// + /// Preserved unchanged when the fact is carried forward; debug-only. + pub created_sitrep_id: DbTypedUuid, + pub comment: String, + + /// The physical disk this fact is about. Common to every `kind`. + pub physical_disk_id: DbTypedUuid, + pub kind: FmFactPhysicalDiskKind, + + // Columns for the `ZpoolUnhealthy` kind. + pub zpool_id: Option>, + pub last_seen_health: Option, + pub observed_in_inv: Option>, + pub time_observed: Option>, +} + +impl FmFactPhysicalDisk { + /// Build a row from a fact's shared metadata (`metadata`) and its + /// physical-disk payload (`disk_fact`). + pub fn from_sitrep( + sitrep_id: impl Into>, + case_id: impl Into>, + metadata: &FactMetadata, + disk_fact: &DiskFact, + ) -> Self { + // Destructure exhaustively: a new `FactMetadata` field will fail to + // compile here until it is mapped to a column. + let FactMetadata { id, created_sitrep_id, comment } = metadata; + let mut row = Self { + id: (*id).into(), + sitrep_id: sitrep_id.into(), + case_id: case_id.into(), + created_sitrep_id: (*created_sitrep_id).into(), + comment: comment.clone(), + physical_disk_id: disk_fact.physical_disk_id().into(), + kind: db_kind(disk_fact), + zpool_id: None, + last_seen_health: None, + observed_in_inv: None, + time_observed: None, + }; + // Each arm populates the columns belonging to its `kind` and leaves + // every other payload column `None`. A column missed here is caught + // at insert time by the table's per-kind CHECK constraint. + match disk_fact { + DiskFact::ZpoolUnhealthy(p) => { + row.zpool_id = Some(p.zpool_id.into()); + row.last_seen_health = Some(p.last_seen_health.into()); + row.observed_in_inv = Some(p.observed_in_inv.into()); + row.time_observed = Some(p.time_observed); + } + } + row + } + + /// Reconstruct an in-memory fact from a row. + /// + /// A NULL in a column the CHECK constraint requires for this `kind` + /// yields an internal error rather than a panic. + pub fn into_fact(self) -> Result { + let kind = self.kind; + let payload = match kind { + FmFactPhysicalDiskKind::ZpoolUnhealthy => { + FactPayload::PhysicalDisk(DiskFact::ZpoolUnhealthy( + ZpoolUnhealthyFactPayload { + physical_disk_id: self.physical_disk_id.into(), + zpool_id: self + .zpool_id + .ok_or_else(|| missing_column(kind, "zpool_id"))? + .into(), + last_seen_health: self + .last_seen_health + .ok_or_else(|| { + missing_column(kind, "last_seen_health") + })? + .into(), + observed_in_inv: self + .observed_in_inv + .ok_or_else(|| { + missing_column(kind, "observed_in_inv") + })? + .into(), + time_observed: self.time_observed.ok_or_else(|| { + missing_column(kind, "time_observed") + })?, + }, + )) + } + }; + Ok(fm::case::Fact { + metadata: FactMetadata { + id: self.id.into(), + created_sitrep_id: self.created_sitrep_id.into(), + comment: self.comment, + }, + payload, + }) + } +} + +/// The `kind` discriminant for a fact's payload. Exhaustive by construction: +/// adding a `DiskFact` variant will not compile until it is mapped here, so +/// `from_sitrep` can never write a row whose `kind` was defaulted rather than +/// derived from the payload. +fn db_kind(disk_fact: &DiskFact) -> FmFactPhysicalDiskKind { + match disk_fact { + DiskFact::ZpoolUnhealthy(_) => FmFactPhysicalDiskKind::ZpoolUnhealthy, + } +} + +fn missing_column(kind: FmFactPhysicalDiskKind, column: &str) -> Error { + Error::internal_error(&format!( + "fm_fact_physical_disk row of kind {kind:?} has a NULL {column}, \ + violating the CHECK constraint requiring it to be non-NULL for \ + this kind" + )) +} diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 088dc2aff2a..8300fff6ddb 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.rs @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock}; /// /// This must be updated when you change the database schema. Refer to /// schema/crdb/README.adoc in the root of this repository for details. -pub const SCHEMA_VERSION: Version = Version::new(268, 0, 0); +pub const SCHEMA_VERSION: Version = Version::new(269, 0, 0); /// List of all past database schema versions, in *reverse* order /// @@ -28,6 +28,7 @@ pub static KNOWN_VERSIONS: LazyLock> = LazyLock::new(|| { // | leaving the first copy as an example for the next person. // v // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"), + KnownVersion::new(269, "fm-disk-de-and-facts"), KnownVersion::new(268, "fm-sitrep-analysis-report"), KnownVersion::new(267, "add-disruption-policy"), KnownVersion::new(266, "alert-version"), diff --git a/nexus/db-queries/src/db/datastore/fm.rs b/nexus/db-queries/src/db/datastore/fm.rs index 4be12118474..8b754d3625d 100644 --- a/nexus/db-queries/src/db/datastore/fm.rs +++ b/nexus/db-queries/src/db/datastore/fm.rs @@ -34,6 +34,7 @@ use nexus_db_schema::schema::ereport::dsl as ereport_dsl; use nexus_db_schema::schema::fm_alert_request::dsl as alert_req_dsl; use nexus_db_schema::schema::fm_case::dsl as case_dsl; use nexus_db_schema::schema::fm_ereport_in_case::dsl as case_ereport_dsl; +use nexus_db_schema::schema::fm_fact_physical_disk::dsl as fact_pd_dsl; use nexus_db_schema::schema::fm_sitrep::dsl as sitrep_dsl; use nexus_db_schema::schema::fm_sitrep_analysis_report::dsl as analysis_report_dsl; use nexus_db_schema::schema::fm_sitrep_history::dsl as history_dsl; @@ -49,6 +50,7 @@ use omicron_uuid_kinds::AlertUuid; use omicron_uuid_kinds::CaseEreportKind; use omicron_uuid_kinds::CaseKind; use omicron_uuid_kinds::CaseUuid; +use omicron_uuid_kinds::FactKind; use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::SitrepUuid; use omicron_uuid_kinds::SupportBundleKind; @@ -123,6 +125,7 @@ sitrep_child_tables! { SupportBundleRequestDataSelectionEreports => { table: "fm_support_bundle_request_data_selection_ereports" }, SupportBundleRequest => { table: "fm_support_bundle_request" }, Case => { table: "fm_case" }, + FmFactPhysicalDisk => { table: "fm_fact_physical_disk" }, AnalysisReport => { table: "fm_sitrep_analysis_report" }, } @@ -369,6 +372,8 @@ impl DataStore { let mut support_bundle_requests = self.support_bundle_requests_read_on_conn(id, conn).await?; + let mut case_facts = self.fm_facts_read_on_conn(id, conn).await?; + // Next, load the case metadata entries and marry them to the sets of // ereports, alert requests, and support bundle requests for those // cases that we loaded in the previous steps. @@ -411,6 +416,7 @@ impl DataStore { alert_requests.remove(&id).unwrap_or_default(); let support_bundles_requested = support_bundle_requests.remove(&id).unwrap_or_default(); + let facts = case_facts.remove(&id).unwrap_or_default(); fm::Case { id, metadata: fm::case::Metadata { @@ -422,6 +428,7 @@ impl DataStore { alerts_requested, ereports, support_bundles_requested, + facts, } })); } @@ -500,6 +507,69 @@ impl DataStore { Ok(by_case) } + /// Fetch all case facts belonging to cases in the given sitrep, grouped + /// by `case_id`. + /// + /// Each diagnosis engine stores its facts in its own typed table, read by + /// its own paginated loop here and unioned into the same `by_case` map. A + /// case belongs to exactly one engine, so keys never collide across tables. + async fn fm_facts_read_on_conn( + &self, + id: SitrepUuid, + conn: &async_bb8_diesel::Connection, + ) -> Result>, Error> { + let mut by_case = + HashMap::>::new(); + + // NOTE: Each per-DE set of facts belongs to a distinct table. + // This would be a reasonable spot to insert parallel_task_set + // and do some concurrent queries. + // + // (... gonna punt on that until we actually have a couple different + // tables to query from). + + // --- physical-disk diagnosis engine facts --- + let mut paginator: Paginator> = + Paginator::new(SQL_BATCH_SIZE, PaginationOrder::Descending); + while let Some(p) = paginator.next() { + let batch = paginated( + fact_pd_dsl::fm_fact_physical_disk, + fact_pd_dsl::id, + &p.current_pagparams(), + ) + .filter(fact_pd_dsl::sitrep_id.eq(id.into_untyped_uuid())) + .select(model::fm::FmFactPhysicalDisk::as_select()) + .load_async(conn) + .await + .map_err(|e| { + public_error_from_diesel(e, ErrorHandler::Server) + .internal_context("failed to load physical-disk case facts") + })?; + + paginator = p.found_batch(&batch, &|f| f.id); + for row in batch { + let case_id: CaseUuid = row.case_id.into(); + let fact = row.into_fact()?; + let id = fact.metadata.id; + by_case + .entry(case_id) + .or_default() + .insert_unique(fact) + .map_err(|_| { + let internal_message = format!( + "encountered multiple case facts for case \ + {case_id} with the same fact UUID {id}. this \ + should really not be possible, as the fact \ + UUID is a primary key!", + ); + Error::InternalError { internal_message } + })?; + } + } + + Ok(by_case) + } + /// Fetch all support bundle requests belonging to cases in the given /// sitrep, including their child data selection tables. async fn support_bundle_requests_read_on_conn( @@ -749,6 +819,7 @@ impl DataStore { let mut support_bundles_requested = Vec::new(); let mut bundle_data_selections_requested = Vec::new(); let mut case_ereports = Vec::new(); + let mut physical_disk_facts = Vec::new(); for case in sitrep.cases { let case_id = case.id; cases.push(model::fm::CaseMetadata::from_sitrep(sitrep_id, &case)); @@ -772,6 +843,21 @@ impl DataStore { ); bundle_data_selections_requested.push((req_id, data_selection)); } + for fact in case.facts.iter() { + // Dispatch each fact to its diagnosis engine's typed table. + match &fact.payload { + fm::FactPayload::PhysicalDisk(disk_fact) => { + physical_disk_facts.push( + model::fm::FmFactPhysicalDisk::from_sitrep( + sitrep_id, + case_id, + &fact.metadata, + disk_fact, + ), + ); + } + } + } } if !case_ereports.is_empty() { @@ -830,6 +916,19 @@ impl DataStore { ) .await?; + if !physical_disk_facts.is_empty() { + diesel::insert_into(fact_pd_dsl::fm_fact_physical_disk) + .values(physical_disk_facts) + .execute_async(&*conn) + .await + .map_err(|e| { + public_error_from_diesel(e, ErrorHandler::Server) + .internal_context( + "failed to insert physical-disk case facts", + ) + })?; + } + if !cases.is_empty() { diesel::insert_into(case_dsl::fm_case) .values(cases) @@ -1165,24 +1264,8 @@ impl DataStore { .map(|id| id.into_untyped_uuid()) .collect::>(); - struct SitrepDeleteResult { - sitreps_deleted: usize, - case_ereports_deleted: usize, - alert_requests_deleted: usize, - support_bundle_requests_deleted: usize, - cases_deleted: usize, - analysis_reports_deleted: usize, - } - let err = OptionalError::new(); - let SitrepDeleteResult { - sitreps_deleted, - case_ereports_deleted, - alert_requests_deleted, - support_bundle_requests_deleted, - cases_deleted, - analysis_reports_deleted, - } = self + let (sitreps_deleted, child_rows_deleted) = self // Sitrep deletion is transactional to prevent a sitrep from being // left in a partially-deleted state should the Nexus instance // attempting the delete operation die suddenly. @@ -1203,47 +1286,33 @@ impl DataStore { } } - // Delete case ereport assignments - let case_ereports_deleted = diesel::delete( - case_ereport_dsl::fm_ereport_in_case.filter( - case_ereport_dsl::sitrep_id.eq_any(ids.clone()), - ), - ) - .execute_async(&conn) - .await?; - - // Delete case alert requests. - let alert_requests_deleted = diesel::delete( - alert_req_dsl::fm_alert_request.filter(alert_req_dsl::sitrep_id.eq_any(ids.clone())) - ) - .execute_async(&conn) - .await?; - - // Delete support bundle request child data selection rows, - // then the requests themselves. - let support_bundle_requests_deleted = - Self::support_bundle_requests_delete_on_conn(&conn, ids.clone()) - .await?; - - // Delete case metadata records. - let cases_deleted = diesel::delete( - case_dsl::fm_case - .filter(case_dsl::sitrep_id.eq_any(ids.clone())), - ) - .execute_async(&conn) - .await?; - - // Delete sitrep analysis report records. - let analysis_reports_deleted = diesel::delete( - analysis_report_dsl::fm_sitrep_analysis_report.filter( - analysis_report_dsl::sitrep_id - .eq_any(ids.clone()), - ), - ) - .execute_async(&conn) - .await?; + // Delete every child row by sitrep_id. Driving this off + // SitrepChildTable::ALL keeps it in lockstep with the + // orphan GC: a child table registered in the + // `sitrep_child_tables!` macro is deleted here + // automatically. There are no foreign keys between these + // tables, so deletion order doesn't matter. + let mut child_rows_deleted = + BTreeMap::::new(); + for &table in SitrepChildTable::ALL { + let mut builder = QueryBuilder::new(); + builder.sql("DELETE FROM omicron.public."); + builder.sql(table.table_name()); + builder.sql(" WHERE "); + builder.sql(table.sitrep_id_column()); + builder.sql(" = ANY("); + builder + .param() + .bind::, _>( + ids.clone(), + ); + builder.sql(")"); + let deleted = + builder.query::<()>().execute_async(&conn).await?; + child_rows_deleted.insert(table, deleted); + } - // Delete sitrep metadata records. + // Delete sitrep metadata records last. let sitreps_deleted = diesel::delete( sitrep_dsl::fm_sitrep .filter(sitrep_dsl::id.eq_any(ids.clone())), @@ -1251,14 +1320,7 @@ impl DataStore { .execute_async(&conn) .await?; - Ok(SitrepDeleteResult { - sitreps_deleted, - cases_deleted, - alert_requests_deleted, - support_bundle_requests_deleted, - case_ereports_deleted, - analysis_reports_deleted, - }) + Ok((sitreps_deleted, child_rows_deleted)) } }) .await @@ -1272,11 +1334,7 @@ impl DataStore { "deleted {sitreps_deleted} of {} sitreps", ids.len(); "ids" => ?ids, "sitreps_deleted" => sitreps_deleted, - "cases_deleted" => cases_deleted, - "case_ereports_deleted" => case_ereports_deleted, - "alert_requests_deleted" => alert_requests_deleted, - "support_bundle_requests_deleted" => support_bundle_requests_deleted, - "analysis_reports_deleted" => analysis_reports_deleted, + "child_rows_deleted" => ?child_rows_deleted, ); Ok(sitreps_deleted) @@ -1521,43 +1579,6 @@ impl DataStore { builder.query() } - /// Delete child data selection rows for support bundle requests, then the - /// support bundle request rows themselves. - #[cfg(test)] - async fn support_bundle_requests_delete_on_conn( - conn: &async_bb8_diesel::Connection, - sitrep_ids: Vec, - ) -> Result { - use nexus_db_schema::schema::fm_support_bundle_request_data_selection_ereports::dsl as ereports_dsl; - use nexus_db_schema::schema::fm_support_bundle_request_data_selection_flags::dsl as flags_dsl; - use nexus_db_schema::schema::fm_support_bundle_request_data_selection_host_info::dsl as host_info_dsl; - - diesel::delete( - flags_dsl::fm_support_bundle_request_data_selection_flags - .filter(flags_dsl::sitrep_id.eq_any(sitrep_ids.clone())), - ) - .execute_async(conn) - .await?; - diesel::delete( - host_info_dsl::fm_support_bundle_request_data_selection_host_info - .filter(host_info_dsl::sitrep_id.eq_any(sitrep_ids.clone())), - ) - .execute_async(conn) - .await?; - diesel::delete( - ereports_dsl::fm_support_bundle_request_data_selection_ereports - .filter(ereports_dsl::sitrep_id.eq_any(sitrep_ids.clone())), - ) - .execute_async(conn) - .await?; - diesel::delete( - support_bundle_req_dsl::fm_support_bundle_request - .filter(support_bundle_req_dsl::sitrep_id.eq_any(sitrep_ids)), - ) - .execute_async(conn) - .await - } - pub async fn fm_sitrep_version_list( &self, opctx: &OpContext, @@ -1616,6 +1637,7 @@ mod tests { use nexus_types::fm::ereport::{EreportData, Reporter}; use omicron_test_utils::dev; use omicron_uuid_kinds::CollectionUuid; + use omicron_uuid_kinds::FactUuid; use omicron_uuid_kinds::OmicronZoneUuid; use omicron_uuid_kinds::SupportBundleUuid; use std::collections::BTreeMap; @@ -2097,6 +2119,7 @@ mod tests { ereports, alerts_requested, support_bundles_requested, + facts, } = case; let case_id = id; let Some(expected) = this.cases.get(&case_id) else { @@ -2128,6 +2151,7 @@ mod tests { &expected.metadata.de, de, "while checking case {case_id}" ); + assert_eq!(&expected.facts, facts, "while checking case {case_id}"); // Now, check that all the ereports are present in both cases. assert_eq!(ereports.len(), expected.ereports.len()); @@ -2303,17 +2327,45 @@ mod tests { .unwrap(); } + let mut facts = iddqd::IdOrdMap::new(); + facts + .insert_unique(fm::case::Fact { + metadata: fm::case::FactMetadata { + id: FactUuid::new_v4(), + created_sitrep_id: sitrep_id, + comment: "a representative fact for case 1".to_string(), + }, + payload: fm::FactPayload::PhysicalDisk( + fm::DiskFact::ZpoolUnhealthy( + fm::ZpoolUnhealthyFactPayload { + physical_disk_id: + omicron_uuid_kinds::PhysicalDiskUuid::new_v4( + ), + zpool_id: + omicron_uuid_kinds::ZpoolUuid::new_v4(), + last_seen_health: + nexus_types::inventory::ZpoolHealth::Degraded, + observed_in_inv: CollectionUuid::new_v4(), + time_observed: omicron_common::now_db_precision( + ), + }, + ), + ), + }) + .unwrap(); + fm::Case { id: omicron_uuid_kinds::CaseUuid::new_v4(), metadata: fm::case::Metadata { created_sitrep_id: sitrep_id, closed_sitrep_id: None, - de: fm::DiagnosisEngineKind::PowerShelf, + de: fm::DiagnosisEngineKind::PhysicalDisk, comment: "my cool case".to_string(), }, ereports, alerts_requested, support_bundles_requested, + facts, } }; @@ -2351,6 +2403,7 @@ mod tests { ereports, alerts_requested, support_bundles_requested: iddqd::IdOrdMap::new(), + facts: iddqd::IdOrdMap::new(), } }; @@ -2400,6 +2453,7 @@ mod tests { new_ereport_ids: Default::default(), open_cases: Default::default(), closed_cases_copied_forward: Default::default(), + in_service_disks: Default::default(), }; let analysis_report = AnalysisReport { sitrep_id: sitrep.id(), @@ -2544,6 +2598,7 @@ mod tests { ereports: iddqd::IdOrdMap::new(), alerts_requested: iddqd::IdOrdMap::new(), support_bundles_requested, + facts: iddqd::IdOrdMap::new(), }; let mut cases = iddqd::IdOrdMap::new(); @@ -3011,6 +3066,7 @@ mod tests { new_ereport_ids: Default::default(), open_cases: Default::default(), closed_cases_copied_forward: Default::default(), + in_service_disks: Default::default(), }; let analysis_report = AnalysisReport { sitrep_id: ghost_sitrep_id, diff --git a/nexus/db-schema/src/enums.rs b/nexus/db-schema/src/enums.rs index 637db54c753..1a364032e74 100644 --- a/nexus/db-schema/src/enums.rs +++ b/nexus/db-schema/src/enums.rs @@ -58,6 +58,7 @@ define_enums! { DownstairsClientStoppedReasonEnum => "downstairs_client_stopped_reason_type", EreporterTypeEnum => "ereporter_type", FailureDomainEnum => "failure_domain", + FmFactPhysicalDiskKindEnum => "fm_fact_physical_disk_kind", HwM2SlotEnum => "hw_m2_slot", HwPowerStateEnum => "hw_power_state", HwRotSlotEnum => "hw_rot_slot", diff --git a/nexus/db-schema/src/schema.rs b/nexus/db-schema/src/schema.rs index 0fe801308c7..301044fb779 100644 --- a/nexus/db-schema/src/schema.rs +++ b/nexus/db-schema/src/schema.rs @@ -3265,6 +3265,22 @@ table! { } } +table! { + fm_fact_physical_disk (sitrep_id, id) { + id -> Uuid, + sitrep_id -> Uuid, + case_id -> Uuid, + created_sitrep_id -> Uuid, + comment -> Text, + physical_disk_id -> Uuid, + kind -> crate::enums::FmFactPhysicalDiskKindEnum, + zpool_id -> Nullable, + last_seen_health -> Nullable, + observed_in_inv -> Nullable, + time_observed -> Nullable, + } +} + table! { fm_ereport_in_case (sitrep_id, id) { id -> Uuid, @@ -3280,6 +3296,8 @@ table! { allow_tables_to_appear_in_same_query!(fm_ereport_in_case, ereport); allow_tables_to_appear_in_same_query!(fm_sitrep, fm_case); +allow_tables_to_appear_in_same_query!(fm_sitrep, fm_fact_physical_disk); +allow_tables_to_appear_in_same_query!(fm_case, fm_fact_physical_disk); table! { fm_alert_request (sitrep_id, id) { diff --git a/nexus/fm/src/analysis_input.rs b/nexus/fm/src/analysis_input.rs index 50749cca262..f90cd7446ff 100644 --- a/nexus/fm/src/analysis_input.rs +++ b/nexus/fm/src/analysis_input.rs @@ -8,6 +8,7 @@ use chrono::{DateTime, Utc}; use iddqd::IdOrdMap; use nexus_types::fm::analysis_reports::ClosedCaseReport; use nexus_types::fm::{self, Sitrep, SitrepVersion}; +use nexus_types::in_service_disk::InServiceDisk; use nexus_types::inventory; use omicron_uuid_kinds::CollectionUuid; use std::collections::BTreeMap; @@ -31,6 +32,7 @@ pub use nexus_types::fm::analysis_reports::InputReport as Report; /// is constructed, the inputs are immutable and cannot be modified. To /// construct a new `Input` as part of a preparaation phase, use /// [`Input::builder`]. +#[derive(Debug)] pub struct Input { parent_sitrep: Option>, inv: Arc, @@ -39,6 +41,7 @@ pub struct Input { new_ereports: IdOrdMap, open_cases: IdOrdMap, closed_cases_copied_forward: IdOrdMap, + in_service_disks: Arc>, } impl Input { @@ -65,11 +68,18 @@ impl Input { &self.closed_cases_copied_forward } + /// All control-plane-managed disks (`physical_disk.disk_policy = + /// in_service` in the DB), indexed by `physical_disk_id`. + pub fn in_service_disks(&self) -> &IdOrdMap { + &self.in_service_disks + } + /// Returns a [`Builder`] for constructing a new `Input` from the provided - /// `parent_sitrep` and inventory collection. + /// `parent_sitrep`, inventory collection, and in-service disks. pub fn builder( parent_sitrep: Option>, inv: Arc, + in_service_disks: Arc>, ) -> Result { // Before preparing analysis inputs, check that the proposed input // inventory collection is at least as new as the parent sitrep's @@ -94,6 +104,7 @@ impl Input { Ok(Builder { parent_sitrep, inv, + in_service_disks, new_ereports: IdOrdMap::default(), unmarked_seen_ereports: BTreeSet::default(), }) @@ -117,6 +128,7 @@ pub enum InvalidInputs { pub struct Builder { parent_sitrep: Option>, inv: Arc, + in_service_disks: Arc>, /// Ereports which are new and should be input to analysis in the next /// sitrep. new_ereports: IdOrdMap, @@ -184,6 +196,11 @@ impl Builder { .collect(), open_cases: BTreeMap::new(), closed_cases_copied_forward: BTreeMap::new(), + in_service_disks: self + .in_service_disks + .iter() + .map(|d| d.physical_disk_id) + .collect(), }; // Determine which cases must be copied forwards into the next sitrep. @@ -234,6 +251,7 @@ impl Builder { new_ereports: self.new_ereports, open_cases, closed_cases_copied_forward, + in_service_disks: self.in_service_disks, }; (input, report) @@ -347,6 +365,7 @@ mod tests { .collect(), alerts_requested: Default::default(), support_bundles_requested: Default::default(), + facts: Default::default(), } }; let open_case2 = { @@ -367,6 +386,7 @@ mod tests { .collect(), alerts_requested: Default::default(), support_bundles_requested: Default::default(), + facts: Default::default(), } }; let closed_case_with_unmarked = { @@ -394,6 +414,7 @@ mod tests { .collect(), alerts_requested: Default::default(), support_bundles_requested: Default::default(), + facts: Default::default(), } }; let closed_case_without_unmarked = { @@ -415,6 +436,7 @@ mod tests { .collect(), alerts_requested: Default::default(), support_bundles_requested: Default::default(), + facts: Default::default(), } }; @@ -464,8 +486,12 @@ mod tests { // Build analysis input let (input, report) = { - let mut builder = Input::builder(Some(parent_sitrep), inv) - .expect("collection start time check should always pass"); + let mut builder = Input::builder( + Some(parent_sitrep), + inv, + Arc::new(IdOrdMap::new()), + ) + .expect("collection start time check should always pass"); // Pass in four ereports: // - two that are in the open cases of the parent sitrep // - one that is in the (to-be-copied-forward) closed case diff --git a/nexus/fm/src/builder.rs b/nexus/fm/src/builder.rs index d528b69c0a4..2037fbe5ea8 100644 --- a/nexus/fm/src/builder.rs +++ b/nexus/fm/src/builder.rs @@ -24,6 +24,9 @@ pub struct SitrepBuilder<'a> { pub parent_sitrep: Option<&'a fm::Sitrep>, pub sitrep_id: SitrepUuid, pub cases: case::AllCases, + /// The analysis input this builder was constructed from; `cases` is + /// seeded from its open cases. + input: &'a analysis_input::Input, closed_cases_copied_forward: &'a IdOrdMap, comment: String, } @@ -66,11 +69,20 @@ impl<'a> SitrepBuilder<'a> { inventory, parent_sitrep, comment: String::new(), + input: inputs, closed_cases_copied_forward, cases, } } + /// The analysis input this builder was constructed from. + /// + /// The returned reference borrows the input (lifetime `'a`), not the + /// builder, so callers may hold it while mutating the builder. + pub fn input(&self) -> &'a analysis_input::Input { + self.input + } + pub fn comment(&self) -> &str { &self.comment } diff --git a/nexus/fm/src/builder/case.rs b/nexus/fm/src/builder/case.rs index b85b10f3067..f7389877341 100644 --- a/nexus/fm/src/builder/case.rs +++ b/nexus/fm/src/builder/case.rs @@ -10,6 +10,7 @@ use nexus_types::alert::AlertPayload; use nexus_types::fm; use nexus_types::support_bundle::BundleDataSelection; use omicron_uuid_kinds::CaseUuid; +use omicron_uuid_kinds::FactUuid; use omicron_uuid_kinds::SitrepUuid; use std::sync::Arc; @@ -76,6 +77,7 @@ impl AllCases { ereports: Default::default(), alerts_requested: Default::default(), support_bundles_requested: Default::default(), + facts: Default::default(), }; let mut builder = CaseBuilder::new(&self.log, sitrep_id, case, case_rng); @@ -226,6 +228,78 @@ impl CaseBuilder { self.report_log.entry("case closed").comment(comment); } + /// Replace this case's free-form comment string. + pub fn set_comment(&mut self, comment: impl ToString) { + self.case.metadata.comment = comment.to_string(); + } + + /// Emit a new fact under this case. + /// + /// Returns the newly generated fact UUID. + pub fn add_fact( + &mut self, + payload: impl Into, + comment: impl ToString, + ) -> FactUuid { + let id = loop { + let id = self.rng.next_fact(); + if !self.case.facts.contains_key(&id) { + break id; + } + }; + let payload = payload.into(); + let comment = comment.to_string(); + slog::info!( + &self.log, + "added a fact"; + "fact_id" => %id, + "payload" => ?payload, + "comment" => %comment, + ); + self.report_log + .entry("added fact") + .kv("fact_id", id) + .kv("payload", &payload) + .comment(comment.clone()); + let fact = fm::case::Fact { + metadata: fm::case::FactMetadata { + id, + created_sitrep_id: self.sitrep_id, + comment, + }, + payload, + }; + self.case.facts.insert_unique(fact).expect("UUID should be unused"); + id + } + + /// Remove a fact from this case. The fact will not be carried forward + /// into the next sitrep. `comment` records why it was removed. + pub fn remove_fact(&mut self, id: FactUuid, comment: impl ToString) { + let comment = comment.to_string(); + if let Some(fact) = self.case.facts.remove(&id) { + slog::info!( + &self.log, + "removed a fact"; + "fact_id" => %id, + "payload" => ?fact.payload, + "comment" => %comment, + ); + self.report_log + .entry("removed fact") + .kv("fact_id", id) + .kv("payload", &fact.payload) + .comment(comment); + } else { + slog::warn!( + &self.log, + "tried to remove a fact that does not exist"; + "fact_id" => %id, + "comment" => %comment, + ); + } + } + pub fn add_ereport( &mut self, report: &Arc, diff --git a/nexus/fm/src/builder/rng.rs b/nexus/fm/src/builder/rng.rs index 5490ef67ae9..86b6a1a7da9 100644 --- a/nexus/fm/src/builder/rng.rs +++ b/nexus/fm/src/builder/rng.rs @@ -14,6 +14,8 @@ use omicron_uuid_kinds::CaseEreportKind; use omicron_uuid_kinds::CaseEreportUuid; use omicron_uuid_kinds::CaseKind; use omicron_uuid_kinds::CaseUuid; +use omicron_uuid_kinds::FactKind; +use omicron_uuid_kinds::FactUuid; use omicron_uuid_kinds::SitrepUuid; use omicron_uuid_kinds::SupportBundleKind; use omicron_uuid_kinds::SupportBundleUuid; @@ -64,6 +66,7 @@ pub(super) struct CaseBuilderRng { ereport_assignment_rng: TypedUuidRng, alert_rng: TypedUuidRng, support_bundle_rng: TypedUuidRng, + fact_rng: TypedUuidRng, } impl CaseBuilderRng { @@ -85,7 +88,11 @@ impl CaseBuilderRng { &mut sitrep.parent, (case_id, "support-bundle"), ); - Self { alert_rng, ereport_assignment_rng, support_bundle_rng } + let fact_rng = TypedUuidRng::from_parent_rng( + &mut sitrep.parent, + (case_id, "case-fact"), + ); + Self { alert_rng, ereport_assignment_rng, support_bundle_rng, fact_rng } } pub(super) fn next_alert(&mut self) -> AlertUuid { @@ -99,4 +106,8 @@ impl CaseBuilderRng { pub(super) fn next_support_bundle(&mut self) -> SupportBundleUuid { self.support_bundle_rng.next() } + + pub(super) fn next_fact(&mut self) -> FactUuid { + self.fact_rng.next() + } } diff --git a/nexus/fm/src/diagnosis.rs b/nexus/fm/src/diagnosis/mod.rs similarity index 66% rename from nexus/fm/src/diagnosis.rs rename to nexus/fm/src/diagnosis/mod.rs index 96ea491f1b1..30e210779e3 100644 --- a/nexus/fm/src/diagnosis.rs +++ b/nexus/fm/src/diagnosis/mod.rs @@ -2,22 +2,22 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. +//! Fault management diagnosis engines. +//! +//! Each submodule defines one diagnosis engine (DE). `analyze` dispatches to +//! each engine in turn; engines are deterministic and idempotent per RFD 603. + use crate::SitrepBuilder; -use crate::analysis_input::Input; -pub fn analyze( - _input: &Input, - _builder: &mut SitrepBuilder<'_>, -) -> anyhow::Result<()> { - anyhow::bail!("FM analysis is not yet implemented") +mod physical_disk; + +pub fn analyze(builder: &mut SitrepBuilder<'_>) -> anyhow::Result<()> { + physical_disk::analyze(builder)?; + Ok(()) } -/// Ereport classes that the diagnosis engine currently understands. -/// Preparation only surfaces ereports whose class is in this set — there is -/// no value in loading ereports FM analysis cannot consume. -/// -/// Empty until [`analyze`] gains real handling. Grow this alongside FM -/// analysis as new classes gain support. +/// Ereport classes that any diagnosis engine in this build of Nexus knows +/// how to consume. The background task uses this to filter loaded ereports. /// /// **NULL-class ereports are intentionally excluded by the loader's SQL /// filter** (`class = ANY(...)` never matches NULL). If FM analysis ever diff --git a/nexus/fm/src/diagnosis/physical_disk.rs b/nexus/fm/src/diagnosis/physical_disk.rs new file mode 100644 index 00000000000..202752f70f7 --- /dev/null +++ b/nexus/fm/src/diagnosis/physical_disk.rs @@ -0,0 +1,1068 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Disk diagnosis engine. + +use crate::SitrepBuilder; +use iddqd::{BiHashItem, BiHashMap, IdOrdItem, IdOrdMap, bi_upcast, id_upcast}; +use nexus_types::fm; +use nexus_types::fm::DiagnosisEngineKind; +use nexus_types::fm::{DiskFact, ZpoolUnhealthyFactPayload}; +use nexus_types::inventory::ZpoolHealth; +use omicron_uuid_kinds::{CaseUuid, FactUuid, PhysicalDiskUuid, ZpoolUuid}; +use std::collections::BTreeMap; + +/// A [`DiskFact::ZpoolUnhealthy`] payload paired with the `FactUuid` it +/// lives under. +#[derive(Clone, Copy, Debug)] +struct ZpoolUnhealthyFact { + fact_id: FactUuid, + payload: ZpoolUnhealthyFactPayload, +} + +impl IdOrdItem for ZpoolUnhealthyFact { + type Key<'a> = FactUuid; + fn key(&self) -> Self::Key<'_> { + self.fact_id + } + id_upcast!(); +} + +/// One in-service disk paired with the current observed health of its zpool. +/// `zpool_health` is `None` when the disk's zpool was not seen in the current +/// inventory (e.g., sled down, lossy collection). +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +struct DiskHealthSnapshot { + physical_disk_id: PhysicalDiskUuid, + zpool_id: ZpoolUuid, + zpool_health: Option, +} + +impl IdOrdItem for DiskHealthSnapshot { + type Key<'a> = PhysicalDiskUuid; + fn key(&self) -> Self::Key<'_> { + self.physical_disk_id + } + id_upcast!(); +} + +/// A parent-forwarded Disk case, parsed into the form this engine acts on. +/// Each Disk case is about a single physical disk; every fact on the case +/// must reference that disk. +struct ParsedDiskCase { + /// The case this was parsed from. + case_id: CaseUuid, + /// The physical disk this case is about. + physical_disk_id: PhysicalDiskUuid, + /// All `ZpoolUnhealthy` facts on this case. Normally one; pathological + /// cases may have multiple. Regardless, the diagnosis engine keeps all of + /// them. + unhealthy_facts: IdOrdMap, +} + +/// A `ParsedDiskCase` is indexed by both the case it came from and the disk it +/// concerns. Keying on `physical_disk_id` makes "at most one case per disk" an +/// invariant of the map itself: inserting a second case for a disk already +/// present fails (see [`analyze`]). +impl BiHashItem for ParsedDiskCase { + type K1<'a> = CaseUuid; + type K2<'a> = PhysicalDiskUuid; + fn key1(&self) -> Self::K1<'_> { + self.case_id + } + fn key2(&self) -> Self::K2<'_> { + self.physical_disk_id + } + bi_upcast!(); +} + +/// Why a parent-forwarded Disk case could not be interpreted. +/// +/// Uninterpretable cases are closed by [`analyze`]: an open case this engine +/// cannot process would otherwise be carried forward into every future +/// sitrep with no path to closure. +#[derive(Debug, Eq, PartialEq, thiserror::Error)] +enum UninterpretableCase { + #[error( + "fact {fact_id} does not belong to the physical-disk diagnosis engine" + )] + ForeignFactPayload { fact_id: FactUuid }, + #[error( + "facts reference different physical disks ({expected} and {found}, \ + 1 expected)" + )] + DisagreeingDisks { expected: PhysicalDiskUuid, found: PhysicalDiskUuid }, + #[error("case has no facts, so the disk it concerns cannot be determined")] + NoFacts, +} + +/// Parse one parent-forwarded Disk case into a [`ParsedDiskCase`], or explain +/// why it cannot be interpreted. +fn parse_case(case: &fm::Case) -> Result { + let mut unhealthy_facts: IdOrdMap = IdOrdMap::new(); + let mut case_disk_id: Option = None; + for fact in case.facts.iter() { + // Every fact on a physical-disk case must carry a physical-disk + // payload; a foreign payload is a data-model violation. + let Some(disk_fact) = fact.payload.as_physical_disk() else { + return Err(UninterpretableCase::ForeignFactPayload { + fact_id: fact.metadata.id, + }); + }; + match disk_fact { + DiskFact::ZpoolUnhealthy(payload) => { + let payload = *payload; + let disk_id = + *case_disk_id.get_or_insert(payload.physical_disk_id); + if disk_id != payload.physical_disk_id { + return Err(UninterpretableCase::DisagreeingDisks { + expected: disk_id, + found: payload.physical_disk_id, + }); + } + unhealthy_facts + .insert_unique(ZpoolUnhealthyFact { + fact_id: fact.metadata.id, + payload, + }) + .expect("fact ids are unique within a case"); + } + } + } + let Some(physical_disk_id) = case_disk_id else { + return Err(UninterpretableCase::NoFacts); + }; + Ok(ParsedDiskCase { case_id: case.id, physical_disk_id, unhealthy_facts }) +} + +pub(super) fn analyze(builder: &mut SitrepBuilder<'_>) -> anyhow::Result<()> { + let input = builder.input(); + let inv_collection_id = input.inventory().id; + let inv_time_done = input.inventory().time_done; + + // Index every zpool we observed in this inventory, so we can distinguish + // "saw it, it's Online" from "didn't see it at all" when looking up by + // an in-service disk's zpool below. + let observed_zpool_health: BTreeMap = input + .inventory() + .sled_agents + .iter() + .flat_map(|sa| sa.zpools.iter()) + .map(|z| (z.id, z.health)) + .collect(); + + // The current health snapshot for every in-service disk, keyed by + // physical_disk_id. Absence from this index is a positive signal that + // the control plane has moved on from the disk (expungement / + // decommissioning); see the analysis task's input preparation. + let in_service_health: IdOrdMap = input + .in_service_disks() + .iter() + .map(|d| DiskHealthSnapshot { + physical_disk_id: d.physical_disk_id, + zpool_id: d.zpool_id, + zpool_health: observed_zpool_health.get(&d.zpool_id).copied(), + }) + .collect(); + + // Index the Disk cases from the parent sitrep, keyed by both the case id + // and the physical disk the case concerns. + // + // Keying on the disk makes "at most one case per disk" an invariant of the + // map: `insert_unique` rejects a second case for a disk already present. + // A disk with two parent cases is pathological; we keep the first one + // inserted and close the rest as duplicates. + let mut cases = BiHashMap::::new(); + for case in input + .open_cases() + .iter() + .filter(|c| c.metadata.de == DiagnosisEngineKind::PhysicalDisk) + { + let parsed_case = match parse_case(case) { + Ok(parsed_case) => parsed_case, + Err(reason) => { + // Close the cases we couldn't interpret, so they don't ride + // along as open-but-unprocessable in every future sitrep. + builder + .cases + .case_mut(&case.id) + .expect("case_id came from builder's open cases") + .close(format!("cannot interpret case: {reason}")); + continue; + } + }; + + if let Err(dup) = cases.insert_unique(parsed_case) { + // A case for this disk is already present; keep it and close this + // one. The disk id is the only key that can collide here, since + // case ids are unique across `open_cases()`, so there is exactly + // one duplicate. + let kept = dup.duplicates()[0]; + builder + .cases + .case_mut(&case.id) + .expect("case_id came from builder's open cases") + .close(format!( + "duplicate of case {} for disk {}", + kept.case_id, kept.physical_disk_id, + )); + } + } + + // Close the surviving parent case for any disk that has recovered or left + // service. A still-faulty disk's facts are reconciled in the next loop; a + // disk in service but absent from this inventory is left alone (absence is + // NOT a recovery signal: the sled could be powered off, or the collection + // could be lossy). + for parsed_case in cases.iter() { + let mut case_mut = builder + .cases + .case_mut(&parsed_case.case_id) + .expect("case_id came from builder's open cases"); + match in_service_health.get(&parsed_case.physical_disk_id) { + None => { + case_mut.close(format!( + "disk {} no longer in service", + parsed_case.physical_disk_id, + )); + } + Some(snap) if snap.zpool_health == Some(ZpoolHealth::Online) => { + case_mut + .close(format!("zpool {} back to Online", snap.zpool_id)); + } + // Faulty or absent from inventory: leave open; the next loop + // reconciles the faulty ones and leaves the absent ones untouched. + Some(_) => {} + } + } + + // For each currently-faulty in-service disk, ensure its case carries + // exactly one fact matching the current observation: reuse the + // parent-forwarded case if any (dropping any stale facts), otherwise open + // a fresh case. + for disk in in_service_health.iter() { + // Only modify "facts" for disk cases where the zpool health is known, + // and it's not "Online" already. + // + // For all other cases, the zpool is healthy, so we don't have + // a case anymore. + let Some(current_health) = disk.zpool_health else { + continue; + }; + if current_health == ZpoolHealth::Online { + continue; + } + + let mut case_mut = match cases.get2(&disk.physical_disk_id) { + Some(parsed_case) => { + let mut case_mut = builder + .cases + .case_mut(&parsed_case.case_id) + .expect("case_id came from builder's open cases"); + let mut has_match = false; + // Although we currently expect only one unhealthy fact for + // each case, the schema does allow for more than one. + // We iterate over all facts of these type to invalidate + // all the facts which are out-of-date. + for fact in parsed_case.unhealthy_facts.iter() { + if fact.payload.zpool_id == disk.zpool_id + && fact.payload.last_seen_health == current_health + { + // An accurate fact is already present; keep it. + has_match = true; + } else { + // Stale observation; drop it. + case_mut.remove_fact( + fact.fact_id, + "stale zpool observation, superseded by current \ + inventory", + ); + } + } + if has_match { + continue; + } + case_mut + } + None => builder.cases.open_case(DiagnosisEngineKind::PhysicalDisk), + }; + + case_mut.add_fact( + DiskFact::ZpoolUnhealthy(ZpoolUnhealthyFactPayload { + physical_disk_id: disk.physical_disk_id, + zpool_id: disk.zpool_id, + last_seen_health: current_health, + observed_in_inv: inv_collection_id, + time_observed: inv_time_done, + }), + format!("zpool {} health={current_health}", disk.zpool_id), + ); + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::analysis_input::Input; + use crate::builder::{SitrepBuilder, SitrepBuilderRng}; + use crate::test_util::FmTest; + use chrono::Utc; + use iddqd::IdOrdMap; + use nexus_types::external_api::physical_disk::PhysicalDiskKind; + use nexus_types::fm::{self, Sitrep, SitrepVersion}; + use nexus_types::in_service_disk::InServiceDisk; + use nexus_types::inventory; + use omicron_test_utils::dev; + use omicron_uuid_kinds::{ + OmicronZoneUuid, PhysicalDiskUuid, SitrepUuid, SledUuid, + }; + use std::sync::Arc; + + /// Make an in-service disk set from a list of zpool IDs. Each zpool gets + /// its own fresh `PhysicalDiskUuid` and dummy identity fields + /// (vendor/serial/model); tests in this module only care about the zpool + /// dimension. + fn mk_in_service( + zpool_ids: impl IntoIterator, + ) -> IdOrdMap { + zpool_ids + .into_iter() + .map(|zpool_id| InServiceDisk { + physical_disk_id: PhysicalDiskUuid::new_v4(), + zpool_id, + sled_id: SledUuid::new_v4(), + vendor: "test-vendor".to_string(), + serial: format!("test-serial-{zpool_id}"), + model: "test-model".to_string(), + variant: PhysicalDiskKind::U2, + }) + .collect() + } + + /// Find the `physical_disk_id` for the given `zpool_id` in the + /// in-service set, or fabricate a fresh one if not present (e.g., when + /// simulating an expunged disk whose case should still reference some + /// stable PhysicalDiskUuid). + fn disk_id_for( + in_service: &IdOrdMap, + zpool_id: ZpoolUuid, + ) -> PhysicalDiskUuid { + in_service + .iter() + .find(|d| d.zpool_id == zpool_id) + .map(|d| d.physical_disk_id) + .unwrap_or_else(PhysicalDiskUuid::new_v4) + } + + /// Make a synthetic test scenario from the example system: returns a + /// `LogContext` (the caller must `cleanup_successful()` it), the + /// example collection, and every zpool ID in that collection. + fn setup( + test_name: &'static str, + ) -> (dev::LogContext, inventory::Collection, Vec) { + let (fm_test, logctx) = FmTest::new_with_logctx(test_name); + // Build the example system once to get a Collection with zpools. + let (example, _bp) = fm_test.system_builder.build(); + let zpool_ids: Vec = example + .collection + .sled_agents + .iter() + .flat_map(|sa| sa.zpools.iter().map(|z| z.id)) + .collect(); + assert!( + !zpool_ids.is_empty(), + "example system should have at least one zpool" + ); + (logctx, example.collection, zpool_ids) + } + + /// Set the zpool with `zpool_id` to `health`, panicking if not found. + fn set_health( + collection: &mut inventory::Collection, + zpool_id: ZpoolUuid, + health: ZpoolHealth, + ) { + for mut sa in collection.sled_agents.iter_mut() { + for z in sa.zpools.iter_mut() { + if z.id == zpool_id { + z.health = health; + return; + } + } + } + panic!("zpool {zpool_id} not found in collection"); + } + + /// Build an `Input` from a collection, an optional parent sitrep, and a + /// pre-built set of in-service disks. + fn build_input( + collection: inventory::Collection, + parent_sitrep: Option, + in_service: IdOrdMap, + ) -> Input { + let parent = parent_sitrep.map(|s| { + Arc::new(( + SitrepVersion { + id: s.id(), + version: 0, + time_made_current: Utc::now(), + }, + s, + )) + }); + let builder = + Input::builder(parent, Arc::new(collection), Arc::new(in_service)) + .expect("input builder should accept fresh inventory"); + let (input, _report) = builder.build(); + input + } + + /// Run `analyze` over an input and return the resulting Sitrep along + /// with the analysis report. + fn run_analyze( + log: &slog::Logger, + input: &Input, + ) -> (Sitrep, fm::analysis_reports::AnalysisReport) { + let mut builder = SitrepBuilder::new_with_rng( + log, + input, + SitrepBuilderRng::from_seed("disk-analyze"), + ); + analyze(&mut builder).expect("analyze ok"); + builder.build(OmicronZoneUuid::new_v4(), Utc::now()) + } + + /// Make a `ZpoolUnhealthy` (Degraded) fact for the given disk and zpool. + fn make_degraded_fact( + parent_sitrep_id: SitrepUuid, + inv_collection_id: omicron_uuid_kinds::CollectionUuid, + physical_disk_id: PhysicalDiskUuid, + zpool_id: ZpoolUuid, + ) -> fm::case::Fact { + fm::case::Fact { + metadata: fm::case::FactMetadata { + id: omicron_uuid_kinds::FactUuid::new_v4(), + created_sitrep_id: parent_sitrep_id, + comment: format!("zpool {zpool_id} degraded"), + }, + payload: DiskFact::ZpoolUnhealthy(ZpoolUnhealthyFactPayload { + physical_disk_id, + zpool_id, + last_seen_health: ZpoolHealth::Degraded, + observed_in_inv: inv_collection_id, + time_observed: Utc::now(), + }) + .into(), + } + } + + /// Make an open `PhysicalDisk` case carrying the given facts. + fn make_disk_case( + case_id: omicron_uuid_kinds::CaseUuid, + parent_sitrep_id: SitrepUuid, + facts: impl IntoIterator, + ) -> fm::Case { + let mut fact_map = iddqd::IdOrdMap::new(); + for fact in facts { + fact_map.insert_unique(fact).unwrap(); + } + fm::Case { + id: case_id, + metadata: fm::case::Metadata { + created_sitrep_id: parent_sitrep_id, + closed_sitrep_id: None, + de: DiagnosisEngineKind::PhysicalDisk, + comment: "a disk case".to_string(), + }, + ereports: Default::default(), + alerts_requested: Default::default(), + support_bundles_requested: Default::default(), + facts: fact_map, + } + } + + /// Make a parent sitrep containing the given cases. + fn make_parent_sitrep( + parent_sitrep_id: SitrepUuid, + inv_collection_id: omicron_uuid_kinds::CollectionUuid, + cases: impl IntoIterator, + ) -> Sitrep { + let mut case_map = iddqd::IdOrdMap::new(); + for case in cases { + case_map.insert_unique(case).unwrap(); + } + Sitrep { + metadata: fm::SitrepMetadata { + id: parent_sitrep_id, + inv_collection_id, + creator_id: OmicronZoneUuid::new_v4(), + parent_sitrep_id: None, + time_created: Utc::now(), + next_inv_min_time_started: Utc::now(), + comment: String::new(), + }, + cases: case_map, + ereports_by_id: Default::default(), + } + } + + fn make_parent_with_disk_case( + parent_sitrep_id: SitrepUuid, + inv_collection_id: omicron_uuid_kinds::CollectionUuid, + physical_disk_id: PhysicalDiskUuid, + zpool_id: ZpoolUuid, + ) -> Sitrep { + let fact = make_degraded_fact( + parent_sitrep_id, + inv_collection_id, + physical_disk_id, + zpool_id, + ); + let case = make_disk_case( + omicron_uuid_kinds::CaseUuid::new_v4(), + parent_sitrep_id, + [fact], + ); + make_parent_sitrep(parent_sitrep_id, inv_collection_id, [case]) + } + + /// A physical-disk fact found in a sitrep, paired with the case it lives + /// on and its decoded [`DiskFact`] payload. + #[derive(Debug)] + struct DiskFactRef<'a> { + case: &'a fm::Case, + fact: &'a fm::case::Fact, + disk_fact: DiskFact, + } + + /// Collect every physical-disk fact in a sitrep, with its case and decoded + /// payload. Optionally filtered to open cases only. + fn disk_facts(sitrep: &Sitrep, open_only: bool) -> Vec> { + sitrep + .cases + .iter() + .filter(|c| c.metadata.de == DiagnosisEngineKind::PhysicalDisk) + .filter(|c| !open_only || c.is_open()) + .flat_map(|c| { + c.facts.iter().filter_map(move |f| { + f.payload.as_physical_disk().map(|d| DiskFactRef { + case: c, + fact: f, + disk_fact: d.clone(), + }) + }) + }) + .collect() + } + + /// The fact UUID of the one physical-disk fact on the one physical-disk + /// case in `sitrep`. Panics unless there is exactly one. + fn sole_disk_fact_id(sitrep: &Sitrep) -> omicron_uuid_kinds::FactUuid { + let facts = disk_facts(sitrep, false); + assert_eq!(facts.len(), 1, "expected exactly one physical-disk fact"); + facts[0].fact.metadata.id + } + + #[test] + fn opens_on_degraded_in_service() { + let (logctx, mut collection, zpools) = + setup("disk_open_degraded_in_service"); + let target = zpools[0]; + set_health(&mut collection, target, ZpoolHealth::Degraded); + let in_service = mk_in_service(zpools.iter().copied()); + let expected_disk_id = disk_id_for(&in_service, target); + let input = build_input(collection, None, in_service); + + let (sitrep, _report) = run_analyze(&logctx.log, &input); + let facts = disk_facts(&sitrep, true); + assert_eq!(facts.len(), 1); + match &facts[0].disk_fact { + DiskFact::ZpoolUnhealthy(ZpoolUnhealthyFactPayload { + physical_disk_id, + zpool_id, + last_seen_health, + .. + }) => { + assert_eq!(*physical_disk_id, expected_disk_id); + assert_eq!(*zpool_id, target); + assert_eq!(*last_seen_health, ZpoolHealth::Degraded); + } + } + logctx.cleanup_successful(); + } + + #[test] + fn skips_degraded_when_expunged() { + let (logctx, mut collection, zpools) = setup("disk_skip_expunged"); + let target = zpools[0]; + set_health(&mut collection, target, ZpoolHealth::Faulted); + // target is *not* in the in-service set. + let in_service = mk_in_service(zpools.iter().copied().skip(1)); + let input = build_input(collection, None, in_service); + + let (sitrep, _report) = run_analyze(&logctx.log, &input); + let cases = disk_facts(&sitrep, false); + assert!( + cases.is_empty(), + "no Disk cases should be opened for expunged zpool, got: {:?}", + cases + ); + logctx.cleanup_successful(); + } + + #[test] + fn idempotent_when_case_already_open() { + let (logctx, mut collection, zpools) = setup("disk_idempotent"); + let target = zpools[0]; + set_health(&mut collection, target, ZpoolHealth::Degraded); + let in_service = mk_in_service(zpools.iter().copied()); + let target_disk_id = disk_id_for(&in_service, target); + let parent_id = SitrepUuid::new_v4(); + let parent = make_parent_with_disk_case( + parent_id, + collection.id, + target_disk_id, + target, + ); + + let input = build_input(collection, Some(parent), in_service); + let (sitrep, _report) = run_analyze(&logctx.log, &input); + let open_cases = disk_facts(&sitrep, true); + assert_eq!(open_cases.len(), 1); + match &open_cases[0].disk_fact { + DiskFact::ZpoolUnhealthy(ZpoolUnhealthyFactPayload { + zpool_id, + .. + }) => { + assert_eq!(*zpool_id, target); + } + } + logctx.cleanup_successful(); + } + + #[test] + fn closes_on_recovery() { + let (logctx, collection, zpools) = setup("disk_close_on_recovery"); + let target = zpools[0]; + // The example system reports zpools as Online by default. + let in_service = mk_in_service(zpools.iter().copied()); + let target_disk_id = disk_id_for(&in_service, target); + let parent_id = SitrepUuid::new_v4(); + let parent = make_parent_with_disk_case( + parent_id, + collection.id, + target_disk_id, + target, + ); + + let input = build_input(collection, Some(parent), in_service); + let (sitrep, report) = run_analyze(&logctx.log, &input); + let all = disk_facts(&sitrep, false); + assert_eq!(all.len(), 1); + assert!( + !all[0].case.is_open(), + "case should be closed when zpool returns to Online", + ); + let report_str = format!("{}", report.display_multiline(0)); + assert!( + report_str.contains("back to Online"), + "close comment should call out the recovery cause, got: \ + {report_str}", + ); + logctx.cleanup_successful(); + } + + #[test] + fn closes_on_expungement() { + let (logctx, mut collection, zpools) = + setup("disk_close_on_expungement"); + let target = zpools[0]; + set_health(&mut collection, target, ZpoolHealth::Degraded); + // Target is NOT in-service in this sitrep (just expunged), so + // disk_id_for fabricates a stable PhysicalDiskUuid for it. + let in_service = mk_in_service(zpools.iter().copied().skip(1)); + let target_disk_id = disk_id_for(&in_service, target); + let parent_id = SitrepUuid::new_v4(); + let parent = make_parent_with_disk_case( + parent_id, + collection.id, + target_disk_id, + target, + ); + + let input = build_input(collection, Some(parent), in_service); + let (sitrep, report) = run_analyze(&logctx.log, &input); + let all = disk_facts(&sitrep, false); + assert_eq!(all.len(), 1); + assert!( + !all[0].case.is_open(), + "case should be closed when zpool's disk is expunged", + ); + let report_str = format!("{}", report.display_multiline(0)); + assert!( + report_str.contains("no longer in service"), + "close comment should call out the expungement cause, got: \ + {report_str}", + ); + logctx.cleanup_successful(); + } + + #[test] + fn keeps_open_on_absence_from_inventory() { + // A zpool the case is about does NOT appear in the inventory at all + // (sled powered off, lossy collection, etc.). The case should stay + // open: absence is not a recovery signal. + let (logctx, collection, zpools) = setup("disk_keep_open_on_absence"); + let phantom = ZpoolUuid::new_v4(); + assert!(!zpools.contains(&phantom)); + let in_service = mk_in_service(zpools.iter().copied().chain([phantom])); + let phantom_disk_id = disk_id_for(&in_service, phantom); + let parent_id = SitrepUuid::new_v4(); + let parent = make_parent_with_disk_case( + parent_id, + collection.id, + phantom_disk_id, + phantom, + ); + + let input = build_input(collection, Some(parent), in_service); + let (sitrep, _report) = run_analyze(&logctx.log, &input); + let all = disk_facts(&sitrep, false); + assert_eq!(all.len(), 1); + assert!( + all[0].case.is_open(), + "case should remain open when its zpool is absent from the \ + current inventory collection (sled could be down or inventory \ + is lossy)", + ); + logctx.cleanup_successful(); + } + + /// A parent Disk case with zero facts has no derivable disk ID; the + /// engine closes it as uninterpretable rather than carrying an + /// unprocessable open case forward into every future sitrep. + #[test] + fn empty_case_is_closed() { + let (logctx, collection, _zpools) = setup("disk_empty_case_closed"); + let in_service = mk_in_service(std::iter::empty()); + + let parent_sitrep_id = SitrepUuid::new_v4(); + let empty_case_id = omicron_uuid_kinds::CaseUuid::new_v4(); + let empty_case = make_disk_case(empty_case_id, parent_sitrep_id, []); + let parent = + make_parent_sitrep(parent_sitrep_id, collection.id, [empty_case]); + + let input = build_input(collection, Some(parent), in_service); + let (sitrep, report) = run_analyze(&logctx.log, &input); + + let case = sitrep + .cases + .iter() + .find(|c| c.id == empty_case_id) + .expect("empty case should still be in the output sitrep"); + assert!(!case.is_open(), "uninterpretable empty case should be closed",); + let report_str = format!("{}", report.display_multiline(0)); + assert!( + report_str.contains("cannot interpret case"), + "close comment should say the case was uninterpretable, got: \ + {report_str}", + ); + logctx.cleanup_successful(); + } + + /// Two open parent cases about the same disk: the engine keeps and + /// maintains the first one it encounters and closes the other as a + /// duplicate. Because `open_cases()` is ordered by case id, the survivor is + /// deterministically the lowest-ID case. (A half-maintained duplicate would + /// otherwise decay into an uninterpretable empty case.) + #[test] + fn duplicate_case_is_closed() { + let (logctx, mut collection, zpools) = setup("disk_duplicate_closed"); + let target = zpools[0]; + set_health(&mut collection, target, ZpoolHealth::Degraded); + let in_service = mk_in_service(zpools.iter().copied()); + let target_disk_id = disk_id_for(&in_service, target); + let parent_id = SitrepUuid::new_v4(); + + let id_a = omicron_uuid_kinds::CaseUuid::new_v4(); + let id_b = omicron_uuid_kinds::CaseUuid::new_v4(); + let (kept_id, dup_id) = + if id_a < id_b { (id_a, id_b) } else { (id_b, id_a) }; + let kept_fact = make_degraded_fact( + parent_id, + collection.id, + target_disk_id, + target, + ); + let kept_fact_id = kept_fact.metadata.id; + let dup_fact = make_degraded_fact( + parent_id, + collection.id, + target_disk_id, + target, + ); + let parent = make_parent_sitrep( + parent_id, + collection.id, + [ + make_disk_case(kept_id, parent_id, [kept_fact]), + make_disk_case(dup_id, parent_id, [dup_fact]), + ], + ); + + let input = build_input(collection, Some(parent), in_service); + let (sitrep, report) = run_analyze(&logctx.log, &input); + + let kept = sitrep.cases.get(&kept_id).expect("kept case present"); + let dup = sitrep.cases.get(&dup_id).expect("duplicate case present"); + assert!(kept.is_open(), "lowest-ID case should remain open"); + assert!(!dup.is_open(), "duplicate case should be closed"); + // The kept case's fact is still accurate (Degraded), so it should + // carry forward unchanged. + assert!( + kept.facts.contains_key(&kept_fact_id), + "kept case should retain its fact", + ); + // No third case should have been opened for this disk. + let disk_case_count = sitrep + .cases + .iter() + .filter(|c| c.metadata.de == DiagnosisEngineKind::PhysicalDisk) + .count(); + assert_eq!(disk_case_count, 2); + let report_str = format!("{}", report.display_multiline(0)); + assert!( + report_str.contains("duplicate of case"), + "close comment should call out the duplicate, got: {report_str}", + ); + logctx.cleanup_successful(); + } + + /// A case whose facts disagree about which disk they concern is closed + /// as uninterpretable. Because fault detection is independent of case + /// bookkeeping, the still-unhealthy disk gets a fresh, well-formed case + /// in the same pass. + #[test] + fn uninterpretable_case_is_replaced() { + let (logctx, mut collection, zpools) = + setup("disk_uninterpretable_replaced"); + let target = zpools[0]; + set_health(&mut collection, target, ZpoolHealth::Degraded); + let in_service = mk_in_service(zpools.iter().copied()); + let target_disk_id = disk_id_for(&in_service, target); + let parent_id = SitrepUuid::new_v4(); + + // One fact about the target disk, one about an unrelated disk: the + // case is self-contradictory. + let corrupt_case_id = omicron_uuid_kinds::CaseUuid::new_v4(); + let corrupt_case = make_disk_case( + corrupt_case_id, + parent_id, + [ + make_degraded_fact( + parent_id, + collection.id, + target_disk_id, + target, + ), + make_degraded_fact( + parent_id, + collection.id, + PhysicalDiskUuid::new_v4(), + ZpoolUuid::new_v4(), + ), + ], + ); + let parent = + make_parent_sitrep(parent_id, collection.id, [corrupt_case]); + + let input = build_input(collection, Some(parent), in_service); + let (sitrep, report) = run_analyze(&logctx.log, &input); + + let corrupt = sitrep + .cases + .get(&corrupt_case_id) + .expect("corrupt case should still be in the output sitrep"); + assert!( + !corrupt.is_open(), + "case with disagreeing facts should be closed", + ); + // The target disk is still Degraded and in service, so a fresh case + // should have been opened for it. + let open = disk_facts(&sitrep, true); + assert_eq!( + open.len(), + 1, + "expected exactly one open Disk fact (on the replacement case)", + ); + assert_ne!(open[0].case.id, corrupt_case_id); + match &open[0].disk_fact { + DiskFact::ZpoolUnhealthy(ZpoolUnhealthyFactPayload { + physical_disk_id, + zpool_id, + last_seen_health, + .. + }) => { + assert_eq!(*physical_disk_id, target_disk_id); + assert_eq!(*zpool_id, target); + assert_eq!(*last_seen_health, ZpoolHealth::Degraded); + } + } + let report_str = format!("{}", report.display_multiline(0)); + assert!( + report_str.contains("cannot interpret case"), + "close comment should say the case was uninterpretable, got: \ + {report_str}", + ); + logctx.cleanup_successful(); + } + + /// When the parent sitrep's fact content matches the diagnosis engine's + /// current observation, the fact carries forward with the same UUID, + /// with no remove-and-readd churn. + #[test] + fn fact_uuid_stable_when_observation_unchanged() { + let (logctx, mut collection, zpools) = setup("disk_fact_uuid_stable"); + let target = zpools[0]; + set_health(&mut collection, target, ZpoolHealth::Degraded); + let in_service = mk_in_service(zpools.iter().copied()); + let target_disk_id = disk_id_for(&in_service, target); + let parent_id = SitrepUuid::new_v4(); + let parent = make_parent_with_disk_case( + parent_id, + collection.id, + target_disk_id, + target, + ); + // Capture the parent's fact UUID for the target zpool. + let parent_fact_id = sole_disk_fact_id(&parent); + + let input = build_input(collection, Some(parent), in_service); + let (sitrep, _report) = run_analyze(&logctx.log, &input); + let open = disk_facts(&sitrep, true); + assert_eq!(open.len(), 1, "expected exactly one open Disk fact"); + assert_eq!( + open[0].fact.metadata.id, parent_fact_id, + "fact UUID should be stable across sitreps when the \ + observation hasn't changed", + ); + match &open[0].disk_fact { + DiskFact::ZpoolUnhealthy(ZpoolUnhealthyFactPayload { + zpool_id, + last_seen_health, + .. + }) => { + assert_eq!(*zpool_id, target); + assert_eq!(*last_seen_health, ZpoolHealth::Degraded); + } + } + logctx.cleanup_successful(); + } + + /// When the parent's fact recorded a different `last_seen_health` than + /// what we observe now, the diagnosis engine removes the stale fact and emits + /// a fresh one (new UUID). The case stays open because the zpool is + /// still unhealthy, just with a different value. + #[test] + fn fact_uuid_rotates_when_observation_changes() { + let (logctx, mut collection, zpools) = setup("disk_fact_uuid_rotates"); + let target = zpools[0]; + // Parent recorded Degraded; current inventory shows Faulted. + set_health(&mut collection, target, ZpoolHealth::Faulted); + let in_service = mk_in_service(zpools.iter().copied()); + let target_disk_id = disk_id_for(&in_service, target); + let parent_id = SitrepUuid::new_v4(); + let parent = make_parent_with_disk_case( + parent_id, + collection.id, + target_disk_id, + target, + ); + let parent_fact_id = sole_disk_fact_id(&parent); + + let input = build_input(collection, Some(parent), in_service); + let (sitrep, _report) = run_analyze(&logctx.log, &input); + let open = disk_facts(&sitrep, true); + assert_eq!( + open.len(), + 1, + "expected exactly one open Disk fact (the refreshed one)", + ); + assert_ne!( + open[0].fact.metadata.id, parent_fact_id, + "fact UUID should rotate because last_seen_health changed", + ); + match &open[0].disk_fact { + DiskFact::ZpoolUnhealthy(ZpoolUnhealthyFactPayload { + zpool_id, + last_seen_health, + .. + }) => { + assert_eq!(*zpool_id, target); + assert_eq!(*last_seen_health, ZpoolHealth::Faulted); + } + } + // The case itself should still be the same one that was carried + // forward; only the fact rotated. + assert!(open[0].case.is_open()); + logctx.cleanup_successful(); + } + + /// When the parent's fact references a different zpool than the one + /// currently backing the disk (the zpool was destroyed and recreated), + /// the fact rotates even though the observed health is unchanged. + /// Today's adoption flow mints a new physical disk UUID alongside a new + /// zpool, so this can't happen via normal control plane operation; this + /// pins the engine's behavior if that ever changes. + #[test] + fn fact_uuid_rotates_when_zpool_replaced() { + let (logctx, mut collection, zpools) = + setup("disk_fact_uuid_rotates_zpool_replaced"); + let target = zpools[0]; + set_health(&mut collection, target, ZpoolHealth::Degraded); + let in_service = mk_in_service(zpools.iter().copied()); + let target_disk_id = disk_id_for(&in_service, target); + // The parent's fact records the same disk and the same health + // (Degraded), but a zpool that no longer exists. + let old_zpool_id = ZpoolUuid::new_v4(); + let parent_id = SitrepUuid::new_v4(); + let parent = make_parent_with_disk_case( + parent_id, + collection.id, + target_disk_id, + old_zpool_id, + ); + let parent_fact_id = sole_disk_fact_id(&parent); + + let input = build_input(collection, Some(parent), in_service); + let (sitrep, _report) = run_analyze(&logctx.log, &input); + let open = disk_facts(&sitrep, true); + assert_eq!( + open.len(), + 1, + "expected exactly one open Disk fact (the refreshed one)", + ); + assert_ne!( + open[0].fact.metadata.id, parent_fact_id, + "fact UUID should rotate because the disk's zpool changed", + ); + match &open[0].disk_fact { + DiskFact::ZpoolUnhealthy(ZpoolUnhealthyFactPayload { + zpool_id, + last_seen_health, + .. + }) => { + assert_eq!( + *zpool_id, target, + "refreshed fact should reference the current zpool", + ); + assert_eq!(*last_seen_health, ZpoolHealth::Degraded); + } + } + assert!(open[0].case.is_open()); + logctx.cleanup_successful(); + } +} diff --git a/nexus/src/app/background/tasks/fm_analysis.rs b/nexus/src/app/background/tasks/fm_analysis.rs index a4eb99123cf..20f8fa6d175 100644 --- a/nexus/src/app/background/tasks/fm_analysis.rs +++ b/nexus/src/app/background/tasks/fm_analysis.rs @@ -9,11 +9,15 @@ use anyhow::Context; use chrono::Utc; use fm::analysis_input::InvalidInputs; use futures::future::BoxFuture; +use iddqd::IdOrdMap; +use nexus_db_model::PhysicalDiskPolicy; use nexus_db_queries::context::OpContext; use nexus_db_queries::db::DataStore; use nexus_db_queries::db::datastore; +use nexus_db_queries::db::identity::Asset; use nexus_db_queries::db::pagination::Paginator; use nexus_fm as fm; +use nexus_types::in_service_disk::InServiceDisk; use nexus_types::internal_api::background::FmAnalysisStatus; use nexus_types::internal_api::background::fm_analysis as status; use nexus_types::inventory; @@ -230,9 +234,16 @@ impl FmAnalysis { (fm::analysis_input::Input, status::PreparationStatus), PreparationError, > { - let mut builder = - fm::analysis_input::Input::builder(parent_sitrep, inv)?; let mut warnings = Vec::new(); + + let in_service_disks = + Arc::new(self.load_in_service_disks(opctx, &mut warnings).await?); + + let mut builder = fm::analysis_input::Input::builder( + parent_sitrep, + inv, + in_service_disks, + )?; self.load_new_ereports(opctx, &mut builder, &mut warnings) .await .context("failed to load new ereports")?; @@ -241,6 +252,67 @@ impl FmAnalysis { Ok((input, status::PreparationStatus { warnings, report })) } + /// Load all in-service control plane disks, projected down to FM's + /// [`InServiceDisk`] view. + async fn load_in_service_disks( + &self, + opctx: &OpContext, + warnings: &mut Vec, + ) -> anyhow::Result> { + // Load all external (U.2) zpools and project them down to FM's + // `InServiceDisk` view, filtering on `disk_policy = in_service` and a + // live (non-soft-deleted) physical_disk row. M.2 disks are not + // represented as control plane disks today, so the U.2-only filter + // on the underlying query matches reality. + // + // See `nexus_types::in_service_disk` for why FM reads the executed + // DB view rather than the target blueprint. + let zpools_and_disks = self + .datastore + .zpool_list_all_external_batched(opctx) + .await + .context("failed to load in-service control plane disks")?; + let mut in_service_disks = IdOrdMap::new(); + for (zpool, disk) in zpools_and_disks { + if disk.time_deleted().is_some() + || disk.disk_policy != PhysicalDiskPolicy::InService + { + continue; + } + let physical_disk_id = disk.id(); + let zpool_id = zpool.id(); + if in_service_disks + .insert_unique(InServiceDisk { + physical_disk_id, + zpool_id, + sled_id: disk.sled_id.into(), + vendor: disk.vendor, + serial: disk.serial, + model: disk.model, + variant: disk.variant.into(), + }) + .is_err() + { + // One live zpool per disk is a code-maintained invariant, + // not a schema constraint. Tolerate a violation rather than + // panicking the analysis task: keep the first zpool seen + // for the disk. + slog::warn!( + &opctx.log, + "multiple live zpools reference the same physical disk"; + "physical_disk_id" => %physical_disk_id, + "zpool_id" => %zpool_id, + ); + warnings.push(format!( + "multiple live zpools reference physical disk \ + {physical_disk_id} (kept first seen, ignored zpool \ + {zpool_id})" + )); + } + } + Ok(in_service_disks) + } + async fn load_new_ereports( &mut self, opctx: &OpContext, @@ -303,7 +375,7 @@ impl FmAnalysis { ) -> status::AnalysisStatus { let start_time = Utc::now(); let mut sitrep_builder = fm::SitrepBuilder::new(&opctx.log, &inputs); - let result = fm::diagnosis::analyze(&inputs, &mut sitrep_builder); + let result = fm::diagnosis::analyze(&mut sitrep_builder); let end_time = Utc::now(); let (sitrep, report) = sitrep_builder.build(self.nexus_id, end_time); diff --git a/nexus/src/app/background/tasks/fm_rendezvous.rs b/nexus/src/app/background/tasks/fm_rendezvous.rs index b099b3534cc..bbe39e562d2 100644 --- a/nexus/src/app/background/tasks/fm_rendezvous.rs +++ b/nexus/src/app/background/tasks/fm_rendezvous.rs @@ -552,6 +552,7 @@ mod tests { alerts_requested: iddqd::IdOrdMap::new(), ereports: iddqd::IdOrdMap::new(), support_bundles_requested: iddqd::IdOrdMap::new(), + facts: iddqd::IdOrdMap::new(), }; case1 .alerts_requested @@ -632,6 +633,7 @@ mod tests { alerts_requested: iddqd::IdOrdMap::new(), ereports: iddqd::IdOrdMap::new(), support_bundles_requested: iddqd::IdOrdMap::new(), + facts: iddqd::IdOrdMap::new(), }; case2 .alerts_requested @@ -936,6 +938,7 @@ mod tests { ereports, alerts_requested: iddqd::IdOrdMap::new(), support_bundles_requested: iddqd::IdOrdMap::new(), + facts: iddqd::IdOrdMap::new(), } }; @@ -1148,6 +1151,7 @@ mod tests { ereports, alerts_requested: iddqd::IdOrdMap::new(), support_bundles_requested: iddqd::IdOrdMap::new(), + facts: iddqd::IdOrdMap::new(), } }; @@ -1257,6 +1261,7 @@ mod tests { ereports, alerts_requested: iddqd::IdOrdMap::new(), support_bundles_requested: iddqd::IdOrdMap::new(), + facts: iddqd::IdOrdMap::new(), } }; @@ -1452,6 +1457,7 @@ mod tests { alerts_requested: iddqd::IdOrdMap::new(), ereports: iddqd::IdOrdMap::new(), support_bundles_requested: iddqd::IdOrdMap::new(), + facts: iddqd::IdOrdMap::new(), }; case1 .support_bundles_requested @@ -1628,6 +1634,7 @@ mod tests { alerts_requested: iddqd::IdOrdMap::new(), ereports: iddqd::IdOrdMap::new(), support_bundles_requested: iddqd::IdOrdMap::new(), + facts: iddqd::IdOrdMap::new(), }; case.support_bundles_requested .insert_unique(fm::case::SupportBundleRequest { diff --git a/nexus/types/output/analysis_input_report_empty.out b/nexus/types/output/analysis_input_report_empty.out index ca90107b7c2..491434e16f6 100644 --- a/nexus/types/output/analysis_input_report_empty.out +++ b/nexus/types/output/analysis_input_report_empty.out @@ -2,3 +2,5 @@ parent sitrep: inventory collection: bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb no new ereports since the parent sitrep no cases copied forward + +no in-service control plane disks diff --git a/nexus/types/output/analysis_input_report_same_inv.out b/nexus/types/output/analysis_input_report_same_inv.out index edc44f45362..9cd8ff958d6 100644 --- a/nexus/types/output/analysis_input_report_same_inv.out +++ b/nexus/types/output/analysis_input_report_same_inv.out @@ -3,3 +3,5 @@ inventory collection: bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb --> same collection as parent sitrep no new ereports since the parent sitrep no cases copied forward + +no in-service control plane disks diff --git a/nexus/types/output/analysis_input_report_with_cases.out b/nexus/types/output/analysis_input_report_with_cases.out index 1410e083863..66b4ea38b50 100644 --- a/nexus/types/output/analysis_input_report_with_cases.out +++ b/nexus/types/output/analysis_input_report_with_cases.out @@ -20,3 +20,7 @@ cases (2 total): closed in sitrep: 55555555-5555-5555-5555-555555555555 copied forwards because these ereports haven't been marked seen yet: * ereport dddddddd-dddd-dddd-dddd-dddddddddddd:2 + +in-service control plane disks (2 total): + * disk 11111111-1111-1111-1111-111111111111 + * disk 22222222-2222-2222-2222-222222222222 diff --git a/nexus/types/src/fm.rs b/nexus/types/src/fm.rs index 14bce765847..779c0cf6a9c 100644 --- a/nexus/types/src/fm.rs +++ b/nexus/types/src/fm.rs @@ -12,6 +12,8 @@ pub mod ereport; pub use ereport::{Ereport, EreportId}; pub mod case; pub use case::Case; +pub mod fact; +pub use fact::{DiskFact, FactPayload, ZpoolUnhealthyFactPayload}; pub mod json_display; use case::AlertRequest; @@ -223,4 +225,5 @@ pub struct SitrepVersion { #[strum(serialize_all = "snake_case")] pub enum DiagnosisEngineKind { PowerShelf, + PhysicalDisk, } diff --git a/nexus/types/src/fm/analysis_reports.rs b/nexus/types/src/fm/analysis_reports.rs index 5986c78d662..d5e2929a754 100644 --- a/nexus/types/src/fm/analysis_reports.rs +++ b/nexus/types/src/fm/analysis_reports.rs @@ -9,7 +9,9 @@ use super::case; use super::ereport::EreportId; use super::json_display::fmt_json_value; use iddqd::IdOrdMap; -use omicron_uuid_kinds::{CaseUuid, CollectionUuid, SitrepUuid}; +use omicron_uuid_kinds::{ + CaseUuid, CollectionUuid, PhysicalDiskUuid, SitrepUuid, +}; use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; use std::collections::BTreeSet; @@ -226,6 +228,9 @@ pub struct InputReport { /// Cases which have closed, but which have been copied forwards as they /// contain ereports which have not yet been marked seen. pub closed_cases_copied_forward: BTreeMap, + /// All control-plane-managed physical disks visible to the diagnosis + /// engines for this analysis pass. + pub in_service_disks: BTreeSet, } #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] @@ -256,6 +261,7 @@ impl fmt::Display for InputReportMultilineDisplay<'_> { new_ereport_ids, open_cases, closed_cases_copied_forward, + in_service_disks, }, indent, } = self; @@ -354,6 +360,21 @@ impl fmt::Display for InputReportMultilineDisplay<'_> { writeln!(f, "{:indent$}no cases copied forward", "")?; } + if in_service_disks.is_empty() { + writeln!(f, "\n{:indent$}no in-service control plane disks", "")?; + } else { + writeln!( + f, + "\n{:indent$}in-service control plane disks ({} total):", + "", + in_service_disks.len() + )?; + let indent = indent + 2; + for disk_id in in_service_disks { + writeln!(f, "{:indent$}* disk {disk_id}", "")?; + } + } + Ok(()) } } @@ -365,7 +386,8 @@ mod tests { use super::*; use ereport_types::{Ena, EreportId}; use omicron_uuid_kinds::{ - CaseUuid, CollectionUuid, EreporterRestartUuid, SitrepUuid, + CaseUuid, CollectionUuid, EreporterRestartUuid, PhysicalDiskUuid, + SitrepUuid, }; use std::str::FromStr; @@ -431,6 +453,16 @@ mod tests { }, ); + let mut in_service_disks = BTreeSet::new(); + in_service_disks.insert( + PhysicalDiskUuid::from_str("11111111-1111-1111-1111-111111111111") + .unwrap(), + ); + in_service_disks.insert( + PhysicalDiskUuid::from_str("22222222-2222-2222-2222-222222222222") + .unwrap(), + ); + InputReport { parent_sitrep_id: Some(parent_sitrep_id), parent_inv_id: Some(parent_inv_id), @@ -438,6 +470,7 @@ mod tests { new_ereport_ids, open_cases, closed_cases_copied_forward, + in_service_disks, } } @@ -453,6 +486,7 @@ mod tests { new_ereport_ids: BTreeSet::new(), open_cases: BTreeMap::new(), closed_cases_copied_forward: BTreeMap::new(), + in_service_disks: BTreeSet::new(), } } @@ -471,6 +505,7 @@ mod tests { new_ereport_ids: BTreeSet::new(), open_cases: BTreeMap::new(), closed_cases_copied_forward: BTreeMap::new(), + in_service_disks: BTreeSet::new(), } } diff --git a/nexus/types/src/fm/case.rs b/nexus/types/src/fm/case.rs index 275c00a25d7..b0717b954a0 100644 --- a/nexus/types/src/fm/case.rs +++ b/nexus/types/src/fm/case.rs @@ -2,14 +2,17 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. +use super::json_display::fmt_json_value; use crate::alert::AlertClass; use crate::fm::DiagnosisEngineKind; use crate::fm::Ereport; use crate::fm::EreportId; +use crate::fm::FactPayload; use crate::support_bundle::BundleDataSelection; use iddqd::{IdOrdItem, IdOrdMap}; use omicron_uuid_kinds::{ - AlertUuid, CaseEreportUuid, CaseUuid, SitrepUuid, SupportBundleUuid, + AlertUuid, CaseEreportUuid, CaseUuid, FactUuid, SitrepUuid, + SupportBundleUuid, }; use serde::{Deserialize, Serialize}; use std::fmt; @@ -24,6 +27,9 @@ pub struct Case { pub ereports: IdOrdMap, pub alerts_requested: IdOrdMap, pub support_bundles_requested: IdOrdMap, + /// Diagnosis-engine-derived facts attached to this case. See + /// [`Fact`] for semantics. + pub facts: IdOrdMap, } impl Case { @@ -159,6 +165,94 @@ impl CaseEreport { } } +/// A diagnosis-engine-derived fact attached to a [`Case`]. +/// +/// Facts are **immutable**: to "update" a fact, the diagnosis engine +/// removes the old one and adds a fresh one. As long as a fact's content +/// matches the engine's current view, the same fact is carried forward +/// across sitreps unchanged. +/// +/// The `payload` is a fully-typed [`FactPayload`] whose variant is owned by +/// the case's diagnosis engine (see [`Metadata::de`]). +#[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)] +pub struct Fact { + #[serde(flatten)] + pub metadata: FactMetadata, + pub payload: FactPayload, +} + +/// The diagnosis-engine-agnostic part of a [`Fact`]: everything that is not +/// the typed [`payload`](Fact::payload). Every diagnosis engine's facts share +/// these fields. +#[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)] +pub struct FactMetadata { + pub id: FactUuid, + /// The sitrep in which this fact was first added. Preserved + /// unchanged when the fact is carried forward into a child sitrep. + /// Debug-only. + pub created_sitrep_id: SitrepUuid, + pub comment: String, +} + +impl IdOrdItem for Fact { + type Key<'a> = &'a FactUuid; + fn key(&self) -> Self::Key<'_> { + &self.metadata.id + } + iddqd::id_upcast!(); +} + +impl Fact { + pub fn display_multiline( + &self, + indent: usize, + sitrep_id: Option, + ) -> impl fmt::Display + '_ { + struct DisplayFact<'a> { + fact: &'a Fact, + indent: usize, + sitrep_id: Option, + } + + impl fmt::Display for DisplayFact<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + const BULLET: &str = "* "; + + let &Self { + fact: + Fact { + metadata: + FactMetadata { id, created_sitrep_id, comment }, + payload, + }, + indent, + sitrep_id, + } = self; + let this_sitrep = |s| { + if Some(s) == sitrep_id { " <-- this sitrep" } else { "" } + }; + + writeln!(f, "{BULLET:>indent$}fact {id}")?; + if !comment.is_empty() { + writeln!(f, "{:>indent$}// {comment}", "")?; + } + writeln!( + f, + "{:>indent$}added in: {created_sitrep_id}{}", + "", + this_sitrep(*created_sitrep_id), + )?; + let payload = serde_json::to_value(payload) + .unwrap_or(serde_json::Value::Null); + fmt_json_value(f, "payload", &payload, indent)?; + writeln!(f) + } + } + + DisplayFact { fact: self, indent, sitrep_id } + } +} + #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] pub struct AlertRequest { pub id: AlertUuid, @@ -218,6 +312,7 @@ impl fmt::Display for DisplayCase<'_> { ereports, alerts_requested, support_bundles_requested, + facts, }, indent, sitrep_id, @@ -287,6 +382,16 @@ impl fmt::Display for DisplayCase<'_> { } } + if !facts.is_empty() { + writeln!(f, "\n{:>indent$}facts:", "")?; + writeln!(f, "{:>indent$}------", "")?; + + let indent = indent + 2; + for fact in facts.iter() { + fact.display_multiline(indent, sitrep_id).fmt(f)?; + } + } + if !alerts_requested.is_empty() { writeln!(f, "\n{:>indent$}alerts requested:", "")?; writeln!(f, "{:>indent$}-----------------", "")?; @@ -379,12 +484,14 @@ mod tests { use super::*; use crate::fm::DiagnosisEngineKind; use crate::fm::ereport::EreportFilters; - use crate::inventory::SpType; + use crate::fm::{DiskFact, ZpoolUnhealthyFactPayload}; + use crate::inventory::{SpType, ZpoolHealth}; use crate::support_bundle::BundleDataSelection; use ereport_types::{Ena, EreportId}; use omicron_uuid_kinds::{ - AlertUuid, CaseUuid, EreporterRestartUuid, OmicronZoneUuid, SitrepUuid, - SupportBundleUuid, + AlertUuid, CaseUuid, CollectionUuid, EreporterRestartUuid, FactUuid, + OmicronZoneUuid, PhysicalDiskUuid, SitrepUuid, SupportBundleUuid, + ZpoolUuid, }; use std::str::FromStr; use std::sync::Arc; @@ -522,6 +629,38 @@ mod tests { }) .unwrap(); + let mut facts = IdOrdMap::new(); + facts + .insert_unique(Fact { + metadata: FactMetadata { + id: FactUuid::from_str( + "f00f00f0-0f00-4f00-8f00-f00f00f00f00", + ) + .unwrap(), + created_sitrep_id, + comment: "made-up fact for display test".to_string(), + }, + payload: FactPayload::PhysicalDisk(DiskFact::ZpoolUnhealthy( + ZpoolUnhealthyFactPayload { + physical_disk_id: PhysicalDiskUuid::from_str( + "d15d15d1-5d15-4d15-8d15-d15d15d15d15", + ) + .unwrap(), + zpool_id: ZpoolUuid::from_str( + "200100f0-0100-4f00-8f00-f00f00f00f00", + ) + .unwrap(), + last_seen_health: ZpoolHealth::Degraded, + observed_in_inv: CollectionUuid::from_str( + "c0110011-c011-4011-8011-c0110011c011", + ) + .unwrap(), + time_observed: chrono::DateTime::::MIN_UTC, + }, + )), + }) + .unwrap(); + // Create the case let case = Case { id: case_id, @@ -535,6 +674,7 @@ mod tests { ereports, alerts_requested, support_bundles_requested, + facts, }; eprintln!("example case display:"); diff --git a/nexus/types/src/fm/fact.rs b/nexus/types/src/fm/fact.rs new file mode 100644 index 00000000000..490ee7446f1 --- /dev/null +++ b/nexus/types/src/fm/fact.rs @@ -0,0 +1,88 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Typed payloads of diagnosis-engine-derived [`Fact`]s. +//! +//! Each diagnosis engine owns one variant of [`FactPayload`]. A [`Case`] +//! belongs to exactly one diagnosis engine (see [`Metadata::de`]), so every +//! fact on a case carries that engine's variant. +//! +//! [`Fact`]: super::case::Fact +//! [`Case`]: super::case::Case +//! [`Metadata::de`]: super::case::Metadata::de + +use crate::inventory::ZpoolHealth; +use chrono::{DateTime, Utc}; +use omicron_uuid_kinds::{CollectionUuid, PhysicalDiskUuid, ZpoolUuid}; +use serde::{Deserialize, Serialize}; + +/// The typed payload of a [`Fact`](super::case::Fact). +/// +/// One variant per diagnosis engine. The variant a fact carries always +/// matches its case's [`de`](super::case::Metadata::de); other engines and +/// shared FM code must not interpret another engine's variant. +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(tag = "engine", rename_all = "snake_case")] +pub enum FactPayload { + /// A fact owned by the physical-disk diagnosis engine. + PhysicalDisk(DiskFact), +} + +impl From for FactPayload { + fn from(fact: DiskFact) -> Self { + FactPayload::PhysicalDisk(fact) + } +} + +impl FactPayload { + /// The physical-disk payload, or `None` if this fact belongs to a + /// different diagnosis engine. + pub fn as_physical_disk(&self) -> Option<&DiskFact> { + #[allow(unreachable_patterns)] + match self { + FactPayload::PhysicalDisk(fact) => Some(fact), + _ => None, + } + } +} + +/// Per-fact state for the physical-disk diagnosis engine. +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(tag = "kind", rename_all = "snake_case")] +pub enum DiskFact { + /// The zpool's most recently observed health is non-`Online`. + ZpoolUnhealthy(ZpoolUnhealthyFactPayload), +} + +impl DiskFact { + /// The physical disk this fact is about. Common to every kind of disk + /// fact: a Disk case is keyed by its physical disk, and every fact on + /// the case agrees on this value. + pub fn physical_disk_id(&self) -> PhysicalDiskUuid { + match self { + DiskFact::ZpoolUnhealthy(p) => p.physical_disk_id, + } + } +} + +/// Payload of a [`DiskFact::ZpoolUnhealthy`] fact. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] +pub struct ZpoolUnhealthyFactPayload { + /// The physical disk this fact (and its parent case) is about. + /// Every fact on a Disk case must agree on this value. + pub physical_disk_id: PhysicalDiskUuid, + /// The zpool whose health was observed. Kept for provenance: the + /// case is keyed by `physical_disk_id`, but knowing the exact zpool + /// makes the fact self-describing when read in isolation. + pub zpool_id: ZpoolUuid, + pub last_seen_health: ZpoolHealth, + /// Inventory collection that produced this observation. Recorded for + /// provenance only: the diagnosis engine never looks this collection + /// back up (it may well have been GC'd by the time anyone reads the + /// fact). If multiple `ZpoolUnhealthy` facts ever end up on the same + /// case, this lets a human reader see which inventory each came from. + pub observed_in_inv: CollectionUuid, + /// `time_done` of `observed_in_inv`. + pub time_observed: DateTime, +} diff --git a/nexus/types/src/in_service_disk.rs b/nexus/types/src/in_service_disk.rs new file mode 100644 index 00000000000..b3ce6c2a597 --- /dev/null +++ b/nexus/types/src/in_service_disk.rs @@ -0,0 +1,41 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! "Currently in-service control-plane disks": the executed view from +//! the `physical_disk` and `zpool` DB tables. +//! +//! This is distinct from the planned view in `BlueprintPhysicalDiskConfig`: +//! a disk is in this set only after the control plane has actually committed +//! to managing it (`physical_disk.disk_policy = 'in_service'`), not while a +//! planner is merely proposing to expunge or adopt it. Consumers that need +//! the *committed* view of which disks are part of the rack (fault +//! management diagnosers in particular) should read this rather than the +//! target blueprint. + +use crate::external_api::physical_disk::PhysicalDiskKind; +use iddqd::{IdOrdItem, id_upcast}; +use omicron_uuid_kinds::PhysicalDiskUuid; +use omicron_uuid_kinds::SledUuid; +use omicron_uuid_kinds::ZpoolUuid; + +/// One control-plane-managed physical disk, joined with its zpool and the +/// stable identity facts the DB already knows about it. +#[derive(Clone, Debug, PartialEq)] +pub struct InServiceDisk { + pub physical_disk_id: PhysicalDiskUuid, + pub zpool_id: ZpoolUuid, + pub sled_id: SledUuid, + pub vendor: String, + pub serial: String, + pub model: String, + pub variant: PhysicalDiskKind, +} + +impl IdOrdItem for InServiceDisk { + type Key<'a> = PhysicalDiskUuid; + fn key(&self) -> Self::Key<'_> { + self.physical_disk_id + } + id_upcast!(); +} diff --git a/nexus/types/src/inventory.rs b/nexus/types/src/inventory.rs index 9fccee9ef5f..2370cdd3ce1 100644 --- a/nexus/types/src/inventory.rs +++ b/nexus/types/src/inventory.rs @@ -45,7 +45,7 @@ use sled_agent_types_versions::latest::inventory::SingleMeasurementInventory; use sled_agent_types_versions::latest::inventory::SledCpuFamily; use sled_agent_types_versions::latest::inventory::SledRole; use sled_agent_types_versions::latest::inventory::SvcsEnabledNotOnlineResult; -use sled_agent_types_versions::latest::inventory::ZpoolHealth; +pub use sled_agent_types_versions::latest::inventory::ZpoolHealth; use sled_hardware_types::BaseboardId; use std::collections::BTreeMap; use std::collections::BTreeSet; diff --git a/nexus/types/src/lib.rs b/nexus/types/src/lib.rs index 36e945f58eb..9a5eeb77c06 100644 --- a/nexus/types/src/lib.rs +++ b/nexus/types/src/lib.rs @@ -35,6 +35,7 @@ pub mod deployment; pub mod external_api; pub mod fm; pub mod identity; +pub mod in_service_disk; pub mod instance; pub mod internal_api; pub mod inventory; diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 010a23c5cce..2e21cae22db 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -7730,7 +7730,8 @@ CREATE TABLE IF NOT EXISTS omicron.public.fm_sitrep_analysis_report ( CREATE TYPE IF NOT EXISTS omicron.public.diagnosis_engine AS ENUM ( - 'power_shelf' + 'power_shelf', + 'physical_disk' ); CREATE TABLE IF NOT EXISTS omicron.public.fm_case ( @@ -7757,6 +7758,59 @@ CREATE INDEX IF NOT EXISTS lookup_fm_cases_for_sitrep ON omicron.public.fm_case (sitrep_id); +-- Per-engine "facts" attached to a case. Each diagnosis engine persists its +-- facts in its own table (one table per engine), with a fact's content +-- represented as typed columns. The `fm_fact_physical_disk` table below holds +-- the physical-disk engine's facts. +CREATE TYPE IF NOT EXISTS omicron.public.fm_fact_physical_disk_kind AS ENUM ( + 'zpool_unhealthy' +); + +CREATE TABLE IF NOT EXISTS omicron.public.fm_fact_physical_disk ( + -- Stable UUID for this fact across sitreps. + id UUID NOT NULL, + -- Sitrep this row belongs to. + sitrep_id UUID NOT NULL, + -- UUID of the case this fact attaches to. + case_id UUID NOT NULL, + -- UUID of the sitrep in which this fact was first added. Preserved + -- unchanged when the fact is carried forward into a child sitrep, so + -- this can be used to tell at a glance how long a fact has been + -- attached to its case. Debug-only. + created_sitrep_id UUID NOT NULL, + -- Free-form, debug-only comment. + comment TEXT NOT NULL, + + -- The physical disk this fact is about. Common to every kind of + -- physical-disk fact (the case is keyed by it), so it is always present + -- regardless of `kind`. + physical_disk_id UUID NOT NULL, + + -- Which physical-disk fact this row represents. The columns below are + -- populated according to this discriminant (see the CHECK constraint). + kind omicron.public.fm_fact_physical_disk_kind NOT NULL, + + -- Columns for a 'zpool_unhealthy' fact. NULL for any other kind. + zpool_id UUID, + last_seen_health omicron.public.inv_zpool_health, + observed_in_inv UUID, + time_observed TIMESTAMPTZ, + + PRIMARY KEY (sitrep_id, id), + + -- Each variant validates that the columns it expects are present. + -- Future variants should add their own constraint like this one, + -- leaving existing constraints untouched. + CONSTRAINT zpool_unhealthy_columns_present CHECK ( + kind != 'zpool_unhealthy' OR ( + zpool_id IS NOT NULL + AND last_seen_health IS NOT NULL + AND observed_in_inv IS NOT NULL + AND time_observed IS NOT NULL + ) + ) +); + CREATE TABLE IF NOT EXISTS omicron.public.fm_ereport_in_case ( -- ID of this association. When an ereport is assigned to a case, that -- association is assigned a UUID. These are used primarily to aid in @@ -8742,7 +8796,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '268.0.0', NULL) + (TRUE, NOW(), NOW(), '269.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/schema/crdb/fm-disk-de-and-facts/up1.sql b/schema/crdb/fm-disk-de-and-facts/up1.sql new file mode 100644 index 00000000000..602e91c8175 --- /dev/null +++ b/schema/crdb/fm-disk-de-and-facts/up1.sql @@ -0,0 +1 @@ +ALTER TYPE omicron.public.diagnosis_engine ADD VALUE IF NOT EXISTS 'physical_disk'; diff --git a/schema/crdb/fm-disk-de-and-facts/up2.sql b/schema/crdb/fm-disk-de-and-facts/up2.sql new file mode 100644 index 00000000000..3aad0937d4c --- /dev/null +++ b/schema/crdb/fm-disk-de-and-facts/up2.sql @@ -0,0 +1,3 @@ +CREATE TYPE IF NOT EXISTS omicron.public.fm_fact_physical_disk_kind AS ENUM ( + 'zpool_unhealthy' +); diff --git a/schema/crdb/fm-disk-de-and-facts/up3.sql b/schema/crdb/fm-disk-de-and-facts/up3.sql new file mode 100644 index 00000000000..51692acc720 --- /dev/null +++ b/schema/crdb/fm-disk-de-and-facts/up3.sql @@ -0,0 +1,44 @@ +CREATE TABLE IF NOT EXISTS omicron.public.fm_fact_physical_disk ( + -- Stable UUID for this fact across sitreps. + id UUID NOT NULL, + -- Sitrep this row belongs to. + sitrep_id UUID NOT NULL, + -- UUID of the case this fact attaches to. + case_id UUID NOT NULL, + -- UUID of the sitrep in which this fact was first added. Preserved + -- unchanged when the fact is carried forward into a child sitrep, so + -- this can be used to tell at a glance how long a fact has been + -- attached to its case. Debug-only. + created_sitrep_id UUID NOT NULL, + -- Free-form, debug-only comment. + comment TEXT NOT NULL, + + -- The physical disk this fact is about. Common to every kind of + -- physical-disk fact (the case is keyed by it), so it is always present + -- regardless of `kind`. + physical_disk_id UUID NOT NULL, + + -- Which physical-disk fact this row represents. The columns below are + -- populated according to this discriminant (see the CHECK constraint). + kind omicron.public.fm_fact_physical_disk_kind NOT NULL, + + -- Columns for a 'zpool_unhealthy' fact. NULL for any other kind. + zpool_id UUID, + last_seen_health omicron.public.inv_zpool_health, + observed_in_inv UUID, + time_observed TIMESTAMPTZ, + + PRIMARY KEY (sitrep_id, id), + + -- Each variant validates that the columns it expects are present. + -- Future variants should add their own constraint like this one, + -- leaving existing constraints untouched. + CONSTRAINT zpool_unhealthy_columns_present CHECK ( + kind != 'zpool_unhealthy' OR ( + zpool_id IS NOT NULL + AND last_seen_health IS NOT NULL + AND observed_in_inv IS NOT NULL + AND time_observed IS NOT NULL + ) + ) +); diff --git a/test-utils/src/dev/test_cmds.rs b/test-utils/src/dev/test_cmds.rs index f254c06726e..60919f4311f 100644 --- a/test-utils/src/dev/test_cmds.rs +++ b/test-utils/src/dev/test_cmds.rs @@ -244,6 +244,19 @@ impl<'a> Redactor<'a> { self } + /// Replace any text matching `pattern` with ``. + /// + /// More flexible than [`Self::field`], which requires a fixed + /// `namevalue` shape. Use this when the entire variable substring + /// is itself a regex (e.g., the substring may take one of several + /// alternative forms across runs). + pub fn variable_regex(&mut self, name: &str, pattern: &str) -> &mut Self { + let re = regex::Regex::new(pattern).unwrap(); + let replacement = format!("<{}_REDACTED>", name.to_uppercase()); + self.extra_regex.push((re, replacement)); + self + } + /// Redact an entire indented section. /// /// This can be used if the shape of a section might change from run to run. diff --git a/uuid-kinds/src/lib.rs b/uuid-kinds/src/lib.rs index 4687e2d5710..304e122201a 100644 --- a/uuid-kinds/src/lib.rs +++ b/uuid-kinds/src/lib.rs @@ -60,6 +60,7 @@ impl_typed_uuid_kinds! { ExternalIp = {}, ExternalSubnet = {}, ExternalZpool = {}, + Fact = {}, FmdHostCase = {}, FmdResource = {}, Instance = {},