Skip to content

[fm] add SMF service diagnosis engine for services in maintenance - #11712

Open
smklein wants to merge 4 commits into
mainfrom
fm-smf-service-de
Open

smklein wants to merge 4 commits into
mainfrom
fm-smf-service-de

Conversation

@smklein

@smklein smklein commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

Progress towards #11675. Diagnosis engines can't create Active Problems yet, so this adds a diagnosis engine that opens and closes fault management cases for SMF services in maintenance. Exposing these as Active Problems is left for later work.

What this does

  • Adds the set of in-service sleds to the FM analysis input. It is read from the sled table, the same way in-service disks are.
  • Moves is_propolis_zone from nexus/src/app/update.rs into nexus_types::inventory, so the update status check and FM share it.
  • Adds the SmfService diagnosis engine, its fm_fact_smf_service table, and the migration that creates them.

Behaviour

There is one case per SMF service instance, identified by its sled, zone, and FMRI. Only the maintenance state is tracked. Offline, degraded, and unrecognized states are deliberately ignored, as are services in Propolis zones.

For each sled, based on its SMF status in the latest inventory:

Sled state Effect on cases
Not in service Close its cases
Missing from inventory, SvcsCmdError, or DataUnavailable No change: don't open, don't close
Clean listing Open cases for services in maintenance; close cases for services no longer listed in maintenance
Listing with unparseable svcs lines (non-empty errors) Open as usual, but don't close any case, and log a sitrep warning. A line that fails to parse could be hiding a service that is still in maintenance.

Differences from contact_support

The update system's contact_support flag currently reports the same condition. This engine intentionally differs from it in two ways:

  • It doesn't treat SvcsCmdError or DataUnavailable as a problem. Being unable to query service health could become its own fault later.
  • It only considers services in maintenance. We can totally change this. Just starting conservatively, since an open case which becomes an active problem should avoid false positives.

Testing

  • Unit tests for the diagnosis engine cover:
    • opening a case;
    • keeping a case unchanged while the service stays in maintenance;
    • closing a case on recovery, and when the sled leaves service (whether or not the sled is still in inventory);
    • leaving cases alone when the sled's status is unknown;
    • partial listings, including that they only affect their own sled;
    • ignoring other states and Propolis zones;
    • closing duplicate and uninterpretable cases.
  • Added SMF service facts to the existing sitrep round-trip test in the datastore.
  • Ran the schema migration tests locally.

Comment thread nexus/src/app/update.rs
//
// This function should not be used for anything other than filtering propolis
// zones out of the `enabled_smf_services_not_online_by_sled` field
fn is_propolis_zone(zone: &str) -> bool {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This got moved to nexus/types/src/inventory.rs so we can share the implementation (it's simple, but I don't want this to be copy-pasted)

@smklein
smklein marked this pull request as ready for review November 5, 2026 22:04
pub sled_id: DbTypedUuid<SledKind>,
pub zone_name: String,
pub fmri: String,
pub kind: FmFactSmfServiceKind,

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.

perhaps nitpicky of me but: kind: FmFactSmfServiceKind feels like it's saying it's the kind of service, and it took me a moment to realize this was the fact kind. But I realize it's a conventional name, so... 🤷‍♀️

Comment on lines +674 to +697
// --- SMF service diagnosis engine facts ---
let mut paginator: Paginator<DbTypedUuid<FactKind>> =
Paginator::new(SQL_BATCH_SIZE, PaginationOrder::Descending);
while let Some(p) = paginator.next() {
let batch = paginated(
fact_smf_dsl::fm_fact_smf_service,
fact_smf_dsl::id,
&p.current_pagparams(),
)
.filter(fact_smf_dsl::sitrep_id.eq(id.into_untyped_uuid()))
.select(model::fm::FmFactSmfService::as_select())
.load_async(conn)
.await
.map_err(|e| {
public_error_from_diesel(e, ErrorHandler::Server)
.internal_context("failed to load SMF service case facts")
})?;

paginator = p.found_batch(&batch, &|f| f.id);
for row in batch {
let case_id: CaseUuid = row.case_id.into();
insert_fact_for_case(&mut by_case, case_id, row.into_fact())?;
}
}

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.

how many more of these do you think we're gonna have to write before we make a macro that does this?

Comment on lines +212 to +215
}) if !in_maintenance.contains(&(
service.zone_name.clone(),
service.fmri.clone(),
)) =>

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.

i really thought that BiHashMap worked with tuples of borrowed keys here, but maybe it doesn't? In any case, it's a lil sad we clone this here...

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants