diff --git a/nexus/src/app/background/tasks/fm_analysis.rs b/nexus/src/app/background/tasks/fm_analysis.rs index 3e0894734d4..78491c4d97d 100644 --- a/nexus/src/app/background/tasks/fm_analysis.rs +++ b/nexus/src/app/background/tasks/fm_analysis.rs @@ -263,21 +263,20 @@ impl FmAnalysis { }; } - // TODO(eliza): diff the sitrep against the parent, and return - // `Unchanged` if it's the same. - let unchanged = true; - if unchanged { - slog::info!( - &opctx.log, - "fault management analysis produced no changes from the \ - current sitrep" - ); - return status::AnalysisStatus { - start_time, - end_time, - report, - outcome: status::AnalysisOutcome::Unchanged, - }; + if let Some(parent) = inputs.parent_sitrep() { + if parent.compare_state() == sitrep { + slog::info!( + &opctx.log, + "fault management analysis produced no changes from the \ + current sitrep" + ); + return status::AnalysisStatus { + start_time, + end_time, + report, + outcome: status::AnalysisOutcome::Unchanged, + }; + } } let sitrep_id = sitrep.id(); diff --git a/nexus/types/src/fm.rs b/nexus/types/src/fm.rs index fe1c258e45b..cb78bc05f6a 100644 --- a/nexus/types/src/fm.rs +++ b/nexus/types/src/fm.rs @@ -94,6 +94,20 @@ impl Sitrep { case.support_bundles_requested.iter().map(move |req| (case, req)) }) } + + /// Returns a value which implements `PartialEq` and can be used to + /// test whether another sitrep represents the same state of the system as + /// this sitrep. + /// + /// This is distinct than comparing two `Sitrep`s directly using their + /// `PartialEq` implementation, as that comparison will test whether *all + /// fields of the sitreps* are exactly the same, while this comparison type + /// will compare equal to *any other sitrep which represents an identical + /// system state*. Two sitreps with different metadata (ID, creator ID, + /// timestamp, etc) can still represent identical states. + pub fn compare_state(&self) -> SitrepStateComparison<'_> { + SitrepStateComparison { sitrep: self } + } } /// Metadata describing a sitrep. @@ -146,6 +160,39 @@ pub struct SitrepMetadata { pub time_created: DateTime, } +pub struct SitrepStateComparison<'sitrep> { + sitrep: &'sitrep Sitrep, +} + +impl PartialEq for SitrepStateComparison<'_> { + fn eq(&self, other: &Sitrep) -> bool { + let Sitrep { + // Ignore the metadata, because two sitreps with different IDs, + // creator IDs, and timestamps can represent the same state. + metadata: _, + // Ignore the map of ereports by ID, as it's just an additional + // index of ereports that are already contained in the `cases` map. + // The state we want to compare is just whether or not the same + // ereports are assigned to the same cases, and it is unnecessary to + // also test that the by-ID indices have the same contents, since + // they always will if the two sitreps' cases contain the same + // ereports. + ereports_by_id: _, + cases, + } = self.sitrep; + + cases == &other.cases + // TO FUTURE GENERATIONS: if other top-level sitrep objects which + // represent the state of the system are added, compare them here! + } +} + +impl PartialEq for SitrepStateComparison<'_> { + fn eq(&self, other: &Self) -> bool { + self == other.sitrep + } +} + /// An entry in the sitrep version history. #[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)] pub struct SitrepVersion {