Repository navigation
[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.
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
Merged
Changes from 13 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 bfedaa5
better errors
smklein 1b5106a
clearer constraints, a non-null column for physical_id
smklein ae1ec41
merge
smklein 4b665e7
PR feedback: growable fact CHECK constraint, filter deleted disks, to…
smklein 272b368
[fm] diagnosis engines read their Input from the SitrepBuilder
smklein 14c2a06
drop unused lookup_fm_fact_physical_disk_for_case index
smklein d2a6767
[fm] disk diagnoser: index parent cases by disk
smklein 82ef44f
simplify comments
smklein 6f2e4a5
[fm] disk diagnoser: close uninterpretable cases; deflake omdb test
smklein cebfc8c
merge
smklein 83c73ad
[fm] derive fact row kind from payload via exhaustive match
smklein 546fc5d
[fm] tidy disk diagnoser per review feedback
smklein 2718755
[fm] split Fact into FactMetadata + payload
smklein 0ada45d
[fm] disk diagnoser: clearer names for parsing a case
smklein 39e4228
[fm] disk diagnoser: build case_by_disk in a single pass
smklein 4b6e928
[fm] disk diagnoser: separate case-closing from fact reconciliation
smklein e731c5d
[fm] disk diagnoser tests: name the disk-fact triple, dedup fact-id l…
smklein f9479fc
[fm] address review feedback from eliza on #10541
smklein 221dcc2
merge
smklein 3c5545d
merge
smklein 0604e5e
[fm] omdb test: redact racy fm_sitrep_gc orphan counts
smklein 8e68736
[fm] address PR review feedback
smklein c4ff887
[fm] use DuplicateItem error to read kept case, avoiding a second lookup
smklein File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,15 @@ 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]+\))", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 `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 | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This rocks, thank you!