From bd75858d152b3caddc6f0c9810d4ec4fd734e726 Mon Sep 17 00:00:00 2001 From: Dan Rosen Date: Wed, 18 Mar 2026 13:32:35 -0400 Subject: [PATCH] nexus-types: add SupportBundleRequest type, Display impls, and Case field Add the SupportBundleRequest type to fm::case and the support_bundles_requested field to Case. Add Display impls for BundleData, BundleDataSelection, SledSelection, and EreportFilters so that case formatting can show the full data selection for each requested support bundle. Add Serialize/Deserialize derives to the support bundle selection types (BundleDataSelection, BundleData, BundleDataCategory, SledSelection, EreportFilters) since SupportBundleRequest contains Option. Add proptest Arbitrary impls for BundleDataSelection and EreportFilters (in test_utils modules) and a proptest-based serde round-trip test for BundleDataSelection. Add test_case_display coverage for support bundle requests, exercising both data_selection: None and Some with parameterized categories. --- nexus/db-model/src/fm/case.rs | 1 + nexus/db-queries/src/db/datastore/fm.rs | 4 + nexus/fm/src/builder/case.rs | 1 + .../src/app/background/tasks/fm_rendezvous.rs | 2 + nexus/types/src/fm/case.rs | 100 +++++++++++++- nexus/types/src/fm/ereport.rs | 102 +++++++++++++- nexus/types/src/support_bundle.rs | 125 +++++++++++++++++- 7 files changed, 329 insertions(+), 6 deletions(-) diff --git a/nexus/db-model/src/fm/case.rs b/nexus/db-model/src/fm/case.rs index efe5d4e5487..1922df26a30 100644 --- a/nexus/db-model/src/fm/case.rs +++ b/nexus/db-model/src/fm/case.rs @@ -58,6 +58,7 @@ impl CaseMetadata { de, comment, alerts_requested: _, + support_bundles_requested: _, ereports: _, } = case; Self { diff --git a/nexus/db-queries/src/db/datastore/fm.rs b/nexus/db-queries/src/db/datastore/fm.rs index 81072530b5c..f3334621deb 100644 --- a/nexus/db-queries/src/db/datastore/fm.rs +++ b/nexus/db-queries/src/db/datastore/fm.rs @@ -385,6 +385,7 @@ impl DataStore { comment, ereports, alerts_requested, + support_bundles_requested: iddqd::IdOrdMap::new(), } })); } @@ -1552,6 +1553,7 @@ mod tests { de, ereports, alerts_requested, + support_bundles_requested: _, } = case; let case_id = id; let Some(expected) = this.cases.get(&case_id) else { @@ -1770,6 +1772,7 @@ mod tests { de: fm::DiagnosisEngineKind::PowerShelf, ereports, alerts_requested, + support_bundles_requested: iddqd::IdOrdMap::new(), comment: "my cool case".to_string(), } }; @@ -1802,6 +1805,7 @@ mod tests { de: fm::DiagnosisEngineKind::PowerShelf, ereports, alerts_requested, + support_bundles_requested: iddqd::IdOrdMap::new(), comment: "break in case of emergency".to_string(), } }; diff --git a/nexus/fm/src/builder/case.rs b/nexus/fm/src/builder/case.rs index 29d3cc3aa4d..f5b6a2d1419 100644 --- a/nexus/fm/src/builder/case.rs +++ b/nexus/fm/src/builder/case.rs @@ -67,6 +67,7 @@ impl AllCases { comment: String::new(), ereports: Default::default(), alerts_requested: Default::default(), + support_bundles_requested: Default::default(), }; entry.insert(CaseBuilder::new( &self.log, sitrep_id, case, case_rng, diff --git a/nexus/src/app/background/tasks/fm_rendezvous.rs b/nexus/src/app/background/tasks/fm_rendezvous.rs index 4d15465b84c..9e893d83e42 100644 --- a/nexus/src/app/background/tasks/fm_rendezvous.rs +++ b/nexus/src/app/background/tasks/fm_rendezvous.rs @@ -230,6 +230,7 @@ mod tests { de: fm::DiagnosisEngineKind::PowerShelf, alerts_requested: iddqd::IdOrdMap::new(), ereports: iddqd::IdOrdMap::new(), + support_bundles_requested: iddqd::IdOrdMap::new(), comment: "my great case".to_string(), }; case1 @@ -305,6 +306,7 @@ mod tests { de: fm::DiagnosisEngineKind::PowerShelf, alerts_requested: iddqd::IdOrdMap::new(), ereports: iddqd::IdOrdMap::new(), + support_bundles_requested: iddqd::IdOrdMap::new(), comment: "my other great case".to_string(), }; case2 diff --git a/nexus/types/src/fm/case.rs b/nexus/types/src/fm/case.rs index afcefd3dd93..991ce788ba1 100644 --- a/nexus/types/src/fm/case.rs +++ b/nexus/types/src/fm/case.rs @@ -5,8 +5,11 @@ use crate::alert::AlertClass; use crate::fm::DiagnosisEngineKind; use crate::fm::Ereport; +use crate::support_bundle::BundleDataSelection; use iddqd::{IdOrdItem, IdOrdMap}; -use omicron_uuid_kinds::{AlertUuid, CaseEreportUuid, CaseUuid, SitrepUuid}; +use omicron_uuid_kinds::{ + AlertUuid, CaseEreportUuid, CaseUuid, SitrepUuid, SupportBundleUuid, +}; use serde::{Deserialize, Serialize}; use std::fmt; use std::sync::Arc; @@ -21,6 +24,7 @@ pub struct Case { pub ereports: IdOrdMap, pub alerts_requested: IdOrdMap, + pub support_bundles_requested: IdOrdMap, pub comment: String, } @@ -88,6 +92,22 @@ impl iddqd::IdOrdItem for AlertRequest { iddqd::id_upcast!(); } +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] +pub struct SupportBundleRequest { + pub id: SupportBundleUuid, + pub requested_sitrep_id: SitrepUuid, + pub data_selection: Option, +} + +impl iddqd::IdOrdItem for SupportBundleRequest { + type Key<'a> = &'a SupportBundleUuid; + fn key(&self) -> Self::Key<'_> { + &self.id + } + + iddqd::id_upcast!(); +} + struct DisplayCase<'a> { case: &'a Case, indent: usize, @@ -120,6 +140,7 @@ impl fmt::Display for DisplayCase<'_> { ereports, comment, alerts_requested, + support_bundles_requested, }, indent, sitrep_id, @@ -234,6 +255,44 @@ impl fmt::Display for DisplayCase<'_> { } } + if !support_bundles_requested.is_empty() { + writeln!(f, "\n{:>indent$}support bundles requested:", "")?; + writeln!(f, "{:>indent$}-------------------------", "")?; + + let indent = indent + 2; + for SupportBundleRequest { + id, + requested_sitrep_id, + data_selection, + } in support_bundles_requested.iter() + { + const REQUESTED_IN: &str = "requested in:"; + const DATA: &str = "data:"; + const WIDTH: usize = const_max_len(&[REQUESTED_IN, DATA]); + + writeln!(f, "{BULLET:>indent$}bundle {id}",)?; + writeln!( + f, + "{:>indent$}{REQUESTED_IN: { + writeln!( + f, + "{:>indent$}{DATA: { + writeln!(f, "{:>indent$}{DATA}", "")?; + writeln!(f, "{}\n", selection.display(indent + 2))?; + } + } + } + } + writeln!(f)?; Ok(()) @@ -244,11 +303,17 @@ impl fmt::Display for DisplayCase<'_> { mod tests { use super::*; use crate::fm::DiagnosisEngineKind; + use crate::fm::ereport::EreportFilters; use crate::inventory::SpType; + use crate::support_bundle::{ + BundleData, BundleDataSelection, SledSelection, + }; use ereport_types::{Ena, EreportId}; use omicron_uuid_kinds::{ AlertUuid, CaseUuid, EreporterRestartUuid, OmicronZoneUuid, SitrepUuid, + SupportBundleUuid, }; + use std::collections::HashSet; use std::str::FromStr; use std::sync::Arc; @@ -276,6 +341,12 @@ mod tests { let alert2_id = AlertUuid::from_str("8a6f88ef-c436-44a9-b4cb-cae91d7306c9") .unwrap(); + let bundle1_id = + SupportBundleUuid::from_str("d1a2b3c4-e5f6-7890-abcd-ef1234567890") + .unwrap(); + let bundle2_id = + SupportBundleUuid::from_str("a9b8c7d6-e5f4-3210-fedc-ba0987654321") + .unwrap(); // Create some ereports let mut ereports = IdOrdMap::new(); @@ -349,6 +420,32 @@ mod tests { }) .unwrap(); + let mut bundle1_data = BundleDataSelection::new(); + bundle1_data.insert(BundleData::Reconfigurator); + bundle1_data.insert(BundleData::SpDumps); + bundle1_data + .insert(BundleData::HostInfo(HashSet::from([SledSelection::All]))); + bundle1_data.insert(BundleData::Ereports(EreportFilters { + only_classes: vec!["hw.pwr.*".to_string()], + ..Default::default() + })); + + let mut support_bundles_requested = IdOrdMap::new(); + support_bundles_requested + .insert_unique(SupportBundleRequest { + id: bundle1_id, + requested_sitrep_id: created_sitrep_id, + data_selection: Some(bundle1_data), + }) + .unwrap(); + support_bundles_requested + .insert_unique(SupportBundleRequest { + id: bundle2_id, + requested_sitrep_id: closed_sitrep_id, + data_selection: None, + }) + .unwrap(); + // Create the case let case = Case { id: case_id, @@ -357,6 +454,7 @@ mod tests { de: DiagnosisEngineKind::PowerShelf, ereports, alerts_requested, + support_bundles_requested, comment: "Power shelf rectifier added and removed here :-)" .to_string(), }; diff --git a/nexus/types/src/fm/ereport.rs b/nexus/types/src/fm/ereport.rs index 0e78743f549..dd8ebc725f0 100644 --- a/nexus/types/src/fm/ereport.rs +++ b/nexus/types/src/fm/ereport.rs @@ -217,7 +217,7 @@ fn get_sp_metadata_string( } /// A set of filters for fetching ereports. -#[derive(Clone, Debug, Default, Eq, PartialEq)] +#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)] pub struct EreportFilters { /// If present, include only ereports that were collected at the specified /// timestamp or later. @@ -240,6 +240,69 @@ pub struct EreportFilters { pub only_classes: Vec, } +/// Displayer for pretty-printing [`EreportFilters`]. +#[must_use = "this struct does nothing unless displayed"] +pub struct DisplayEreportFilters<'a> { + filters: &'a EreportFilters, +} + +impl EreportFilters { + pub fn display(&self) -> DisplayEreportFilters<'_> { + DisplayEreportFilters { filters: self } + } +} + +impl fmt::Display for DisplayEreportFilters<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + use itertools::Itertools; + + let filters = self.filters; + + // Writes a semicolon-separated part to the formatter, tracking whether + // we've written anything yet. + let mut empty = true; + let mut fmt_part = + |f: &mut fmt::Formatter, args: fmt::Arguments| -> fmt::Result { + if !empty { + write!(f, "; ")?; + } + empty = false; + f.write_fmt(args) + }; + + if let Some(start) = &filters.start_time { + fmt_part(f, format_args!("start: {start}"))?; + } + if let Some(end) = &filters.end_time { + fmt_part(f, format_args!("end: {end}"))?; + } + if !filters.only_serials.is_empty() { + fmt_part( + f, + format_args!( + "serials: {}", + filters.only_serials.iter().format(", ") + ), + )?; + } + if !filters.only_classes.is_empty() { + fmt_part( + f, + format_args!( + "classes: {}", + filters.only_classes.iter().format(", ") + ), + )?; + } + + // If no filters are set, display "none" rather than empty output. + if empty { + write!(f, "none")?; + } + Ok(()) + } +} + impl EreportFilters { pub fn check_time_range(&self) -> Result<(), Error> { if let (Some(start), Some(end)) = (self.start_time, self.end_time) { @@ -253,3 +316,40 @@ impl EreportFilters { Ok(()) } } + +#[cfg(test)] +pub(crate) mod test_utils { + use super::*; + use proptest::prelude::*; + + fn arb_datetime() -> impl Strategy> { + // Generate timestamps in a reasonable range (2020-2030). + (1577836800i64..1893456000i64) + .prop_map(|secs| DateTime::from_timestamp(secs, 0).unwrap()) + } + + impl Arbitrary for EreportFilters { + type Parameters = (); + type Strategy = BoxedStrategy; + + fn arbitrary_with(_: Self::Parameters) -> Self::Strategy { + ( + prop::option::of(arb_datetime()), + prop::option::of(arb_datetime()), + prop::collection::vec(".*", 0..=3), + prop::collection::vec(".*", 0..=3), + ) + .prop_map( + |(start_time, end_time, only_serials, only_classes)| { + EreportFilters { + start_time, + end_time, + only_serials, + only_classes, + } + }, + ) + .boxed() + } + } +} diff --git a/nexus/types/src/support_bundle.rs b/nexus/types/src/support_bundle.rs index ed53e426bb9..71282c069f5 100644 --- a/nexus/types/src/support_bundle.rs +++ b/nexus/types/src/support_bundle.rs @@ -8,12 +8,16 @@ //! They are shared between the support bundle collector and FM case types. use crate::fm::ereport::EreportFilters; +use itertools::Itertools; use omicron_uuid_kinds::SledUuid; +use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::collections::HashSet; +use std::fmt; /// Describes the category of support bundle data. -#[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)] +#[derive(Debug, Clone, Copy, Hash, Eq, PartialEq, Serialize, Deserialize)] +#[cfg_attr(test, derive(test_strategy::Arbitrary))] pub enum BundleDataCategory { /// Collects reconfigurator state (some of the latest blueprints, /// information about the target blueprint). @@ -35,7 +39,8 @@ pub enum BundleDataCategory { /// For categories without additional parameters, the variant is a unit variant. /// For categories that can be filtered or configured, the variant contains /// that configuration data. -#[derive(Debug, Clone, Eq, PartialEq)] +#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)] +#[cfg_attr(test, derive(test_strategy::Arbitrary))] pub enum BundleData { Reconfigurator, HostInfo(HashSet), @@ -56,12 +61,44 @@ impl BundleData { } } +/// Displayer for pretty-printing [`BundleData`]. +#[must_use = "this struct does nothing unless displayed"] +pub struct DisplayBundleData<'a> { + data: &'a BundleData, +} + +impl BundleData { + pub fn display(&self) -> DisplayBundleData<'_> { + DisplayBundleData { data: self } + } +} + +impl fmt::Display for DisplayBundleData<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self.data { + BundleData::Reconfigurator => write!(f, "reconfigurator"), + BundleData::HostInfo(sleds) => { + write!( + f, + "host_info({})", + sleds.iter().format_with(", ", |s, f| f(&s.display())) + ) + } + BundleData::SledCubbyInfo => write!(f, "sled_cubby_info"), + BundleData::SpDumps => write!(f, "sp_dumps"), + BundleData::Ereports(filters) => { + write!(f, "ereports({})", filters.display()) + } + } + } +} + /// A collection of bundle data specifications. /// /// This wrapper ensures that categories and data always match - you can't /// insert (BundleDataCategory::Reconfigurator, BundleData::SpDumps) /// because each BundleData determines its own category. -#[derive(Debug, Clone, Eq, PartialEq)] +#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)] pub struct BundleDataSelection { data: HashMap, } @@ -94,6 +131,32 @@ impl BundleDataSelection { } } +/// Displayer for pretty-printing [`BundleDataSelection`]. +#[must_use = "this struct does nothing unless displayed"] +pub struct DisplayBundleDataSelection<'a> { + selection: &'a BundleDataSelection, + indent: usize, +} + +impl BundleDataSelection { + pub fn display(&self, indent: usize) -> DisplayBundleDataSelection<'_> { + DisplayBundleDataSelection { selection: self, indent } + } +} + +impl fmt::Display for DisplayBundleDataSelection<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let indent = self.indent; + for (i, item) in self.selection.data.values().enumerate() { + if i > 0 { + writeln!(f)?; + } + write!(f, "{:>indent$}- {}", "", item.display())?; + } + Ok(()) + } +} + impl FromIterator for BundleDataSelection { fn from_iter>(iter: T) -> Self { let mut selection = Self::new(); @@ -128,8 +191,62 @@ impl Default for BundleDataSelection { /// /// Multiple values of this enum are joined together into a HashSet. /// Therefore "SledSelection::All" overrides specific sleds. -#[derive(Debug, Clone, Hash, Eq, PartialEq)] +#[derive(Debug, Clone, Hash, Eq, PartialEq, Serialize, Deserialize)] +#[cfg_attr(test, derive(test_strategy::Arbitrary))] pub enum SledSelection { All, Specific(SledUuid), } + +/// Displayer for pretty-printing [`SledSelection`]. +#[must_use = "this struct does nothing unless displayed"] +pub struct DisplaySledSelection<'a> { + selection: &'a SledSelection, +} + +impl SledSelection { + pub fn display(&self) -> DisplaySledSelection<'_> { + DisplaySledSelection { selection: self } + } +} + +impl fmt::Display for DisplaySledSelection<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self.selection { + SledSelection::All => write!(f, "all"), + SledSelection::Specific(id) => write!(f, "{id}"), + } + } +} + +#[cfg(test)] +pub(crate) mod test_utils { + use super::*; + use proptest::prelude::*; + + impl Arbitrary for BundleDataSelection { + type Parameters = (); + type Strategy = BoxedStrategy; + + fn arbitrary_with(_: Self::Parameters) -> Self::Strategy { + prop::collection::vec(any::(), 0..=5) + .prop_map(|data| data.into_iter().collect()) + .boxed() + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use proptest::prelude::*; + use test_strategy::proptest; + + #[proptest] + fn bundle_data_selection_serde_round_trip(selection: BundleDataSelection) { + let json = serde_json::to_string(&selection).unwrap(); + let deserialized: BundleDataSelection = + serde_json::from_str(&json).unwrap(); + prop_assert_eq!(selection, deserialized); + } +}