Skip to content

[fm] Add disk diagnoser with typed fact tables #10541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
smklein merged 24 commits into from
Jun 18, 2026
Merged

[fm] Add disk diagnoser with typed fact tables #10541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
6e094f4
[fm] Add disk diagnoser with typed fact tables
smklein Jun 2, 2026
bfedaa5
better errors
smklein Jun 3, 2026
1b5106a
clearer constraints, a non-null column for physical_id
smklein Jun 3, 2026
ae1ec41
merge
smklein Jun 10, 2026
4b665e7
PR feedback: growable fact CHECK constraint, filter deleted disks, to…
smklein Jun 10, 2026
272b368
[fm] diagnosis engines read their Input from the SitrepBuilder
smklein Jun 10, 2026
14c2a06
drop unused lookup_fm_fact_physical_disk_for_case index
smklein Jun 10, 2026
d2a6767
[fm] disk diagnoser: index parent cases by disk
smklein Jun 10, 2026
82ef44f
simplify comments
smklein Jun 11, 2026
6f2e4a5
[fm] disk diagnoser: close uninterpretable cases; deflake omdb test
smklein Jun 11, 2026
cebfc8c
merge
smklein Jun 11, 2026
83c73ad
[fm] derive fact row kind from payload via exhaustive match
smklein Jun 12, 2026
546fc5d
[fm] tidy disk diagnoser per review feedback
smklein Jun 15, 2026
2718755
[fm] split Fact into FactMetadata + payload
smklein Jun 15, 2026
0ada45d
[fm] disk diagnoser: clearer names for parsing a case
smklein Jun 15, 2026
39e4228
[fm] disk diagnoser: build case_by_disk in a single pass
smklein Jun 15, 2026
4b6e928
[fm] disk diagnoser: separate case-closing from fact reconciliation
smklein Jun 15, 2026
e731c5d
[fm] disk diagnoser tests: name the disk-fact triple, dedup fact-id l…
smklein Jun 16, 2026
f9479fc
[fm] address review feedback from eliza on #10541
smklein Jun 16, 2026
221dcc2
merge
smklein Jun 16, 2026
3c5545d
merge
smklein Jun 16, 2026
0604e5e
[fm] omdb test: redact racy fm_sitrep_gc orphan counts
smklein Jun 16, 2026
8e68736
[fm] address PR review feedback
smklein Jun 17, 2026
c4ff887
[fm] use DuplicateItem error to read kept case, avoiding a second lookup
smklein Jun 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 36 additions & 22 deletions dev-tools/omdb/tests/successes.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,19 +700,22 @@ task: "fm_analysis"
configured period: every <REDACTED_DURATION>m
last completed activation: <REDACTED ITERATIONS>, triggered by <TRIGGERED_BY_REDACTED>
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
parent sitrep ID: None
parent sitrep ID: Some(..........<REDACTED_UUID>........... (sitrep))
current inventory collection ID: Some(..........<REDACTED_UUID>........... (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(..........<REDACTED_UUID>........... (sitrep)))

preparation report:
parent sitrep: <none>
parent sitrep: ..........<REDACTED_UUID>...........
inventory collection: ..........<REDACTED_UUID>...........
<FM_INPUT_INV_COMPARISON_REDACTED>
no new ereports since the parent sitrep
no cases copied forward

no in-service control plane disks

analysis report:
sitrep ID: ..........<REDACTED_UUID>...........
no cases changed in this analysis step
Expand All @@ -722,28 +725,28 @@ task: "fm_rendezvous"
configured period: every <REDACTED_DURATION>m
last completed activation: <REDACTED ITERATIONS>, triggered by <TRIGGERED_BY_REDACTED>
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
(i) no FM situation report loaded, so rendezvous was not performed
current sitrep: ..........<REDACTED_UUID>...........
creating requested alerts:
(i) note: this operation was not executed
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>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 <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>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 <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>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

Expand All @@ -752,15 +755,17 @@ task: "fm_sitrep_gc"
last completed activation: <REDACTED ITERATIONS>, triggered by <TRIGGERED_BY_REDACTED>
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
batch size: 1000
orphaned sitreps deleted: 0
orphaned sitreps deleted: <ORPHANED_SITREPS_DELETED_REDACTED>
batches: 1
orphaned fm_alert_request rows deleted: 0
batches: 1
orphaned fm_case rows deleted: 0
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: <ORPHANED_FMSITREPANALYSISREPORT_ROWS_DELETED_REDACTED>

@hawkw hawkw Jun 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol FMSITREPANALYSISREPORT

batches: 1
orphaned fm_support_bundle_request rows deleted: 0
batches: 1
Expand All @@ -775,7 +780,9 @@ task: "fm_sitrep_loader"
configured period: every <REDACTED_DURATION>s
last completed activation: <REDACTED ITERATIONS>, triggered by <TRIGGERED_BY_REDACTED>
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
no FM situation report available to load
loaded latest FM situation report as of <REDACTED_TIMESTAMP>:
sitrep ..........<REDACTED_UUID>........... (sitrep) (v1)
made current at: <REDACTED_TIMESTAMP>

task: "instance_reincarnation"
configured period: every <REDACTED_DURATION>m
Expand Down Expand Up @@ -1391,19 +1398,22 @@ task: "fm_analysis"
configured period: every <REDACTED_DURATION>m
last completed activation: <REDACTED ITERATIONS>, triggered by <TRIGGERED_BY_REDACTED>
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
parent sitrep ID: None
parent sitrep ID: Some(..........<REDACTED_UUID>........... (sitrep))
current inventory collection ID: Some(..........<REDACTED_UUID>........... (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(..........<REDACTED_UUID>........... (sitrep)))

preparation report:
parent sitrep: <none>
parent sitrep: ..........<REDACTED_UUID>...........
inventory collection: ..........<REDACTED_UUID>...........
<FM_INPUT_INV_COMPARISON_REDACTED>
no new ereports since the parent sitrep
no cases copied forward

no in-service control plane disks

analysis report:
sitrep ID: ..........<REDACTED_UUID>...........
no cases changed in this analysis step
Expand All @@ -1413,28 +1423,28 @@ task: "fm_rendezvous"
configured period: every <REDACTED_DURATION>m
last completed activation: <REDACTED ITERATIONS>, triggered by <TRIGGERED_BY_REDACTED>
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
(i) no FM situation report loaded, so rendezvous was not performed
current sitrep: ..........<REDACTED_UUID>...........
creating requested alerts:
(i) note: this operation was not executed
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>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 <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>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 <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>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

Expand All @@ -1443,15 +1453,17 @@ task: "fm_sitrep_gc"
last completed activation: <REDACTED ITERATIONS>, triggered by <TRIGGERED_BY_REDACTED>
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
batch size: 1000
orphaned sitreps deleted: 0
orphaned sitreps deleted: <ORPHANED_SITREPS_DELETED_REDACTED>
batches: 1
orphaned fm_alert_request rows deleted: 0
batches: 1
orphaned fm_case rows deleted: 0
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: <ORPHANED_FMSITREPANALYSISREPORT_ROWS_DELETED_REDACTED>
batches: 1
orphaned fm_support_bundle_request rows deleted: 0
batches: 1
Expand All @@ -1466,7 +1478,9 @@ task: "fm_sitrep_loader"
configured period: every <REDACTED_DURATION>s
last completed activation: <REDACTED ITERATIONS>, triggered by <TRIGGERED_BY_REDACTED>
started at <REDACTED_TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
no FM situation report available to load
loaded latest FM situation report as of <REDACTED_TIMESTAMP>:
sitrep ..........<REDACTED_UUID>........... (sitrep) (v1)
made current at: <REDACTED_TIMESTAMP>

task: "instance_reincarnation"
configured period: every <REDACTED_DURATION>m
Expand Down
44 changes: 44 additions & 0 deletions dev-tools/omdb/tests/test_all_output.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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".
Comment on lines +191 to +205

@hawkw hawkw Jun 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rocks, thank you!

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]] = &[
Expand Down Expand Up @@ -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]+\))",

@hawkw hawkw Jun 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

urgh, I wanted to suggest we wrap this line, but it's a raw string, so that's hard. never mind.

);

// 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
Expand Down
2 changes: 2 additions & 0 deletions nexus/db-model/src/fm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions nexus/db-model/src/fm/case.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ impl CaseMetadata {
alerts_requested: _,
support_bundles_requested: _,
ereports: _,
facts: _,
} = case;
Self {
sitrep_id: sitrep_id.into(),
Expand Down
7 changes: 7 additions & 0 deletions nexus/db-model/src/fm/diagnosis_engine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,17 @@ impl_enum_type!(
pub enum DiagnosisEngine;

PowerShelf => b"power_shelf"
PhysicalDisk => b"physical_disk"

);

impl From<DiagnosisEngine> for fm::DiagnosisEngineKind {
fn from(de: DiagnosisEngine) -> Self {
match de {
DiagnosisEngine::PowerShelf => fm::DiagnosisEngineKind::PowerShelf,
DiagnosisEngine::PhysicalDisk => {
fm::DiagnosisEngineKind::PhysicalDisk
}
}
}
}
Expand All @@ -39,6 +43,9 @@ impl From<fm::DiagnosisEngineKind> for DiagnosisEngine {
fn from(fm_de: fm::DiagnosisEngineKind) -> Self {
match fm_de {
fm::DiagnosisEngineKind::PowerShelf => DiagnosisEngine::PowerShelf,
fm::DiagnosisEngineKind::PhysicalDisk => {
DiagnosisEngine::PhysicalDisk
}
}
}
}
Expand Down
Loading
Loading