Skip to content

[inventory] Add svcs in maintenance to DB #9589

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

Open
karencfv wants to merge 13 commits into
base: main
Choose a base branch
from
Open

[inventory] Add svcs in maintenance to DB #9589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
428a607
[inventory] Add svcs in maintenance to DB
karencfv Jan 5, 2026
64852ea
insert rows
karencfv Jan 6, 2026
52f7849
display works
karencfv Jan 6, 2026
3b5e779
clean up and disable svcs in sim
karencfv Jan 6, 2026
c643201
expectorate
karencfv Jan 6, 2026
9b1ac94
fmt
karencfv Jan 6, 2026
aa071e0
fix display bug
karencfv Jan 7, 2026
e3f0a1f
Clean up
karencfv Jan 8, 2026
4b80284
Ability to remove rows from collection
karencfv Jan 8, 2026
ed3ed7f
Disbale health monitor on simulated system
karencfv Jan 8, 2026
4489b98
fix tests
karencfv Jan 8, 2026
16af1d0
merge main
karencfv Jan 8, 2026
335832e
fmt
karencfv Jan 8, 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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions dev-tools/reconfigurator-cli/tests/output/cmds-example-stdout
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1608,6 +1608,9 @@ LEDGERED SLED CONFIG
(measurement set is empty)
reconciler task status: idle (finished at <REDACTED_TIMESTAMP> after running for <REDACTED_DURATION>s)

HEALTH MONITOR
no data on SMF services in maintenance has been collected

sled 32d8d836-4d8a-4e54-8fa9-f31d79c42646 (role = Gimlet, serial serial2)
found at: <REDACTED_TIMESTAMP> from fake sled agent
address: [fd00:1122:3344:103::1]:12345
Expand Down Expand Up @@ -1744,6 +1747,9 @@ LEDGERED SLED CONFIG
(measurement set is empty)
reconciler task status: idle (finished at <REDACTED_TIMESTAMP> after running for <REDACTED_DURATION>s)

HEALTH MONITOR
no data on SMF services in maintenance has been collected

sled 89d02b1b-478c-401a-8e28-7a26f74fa41b (role = Gimlet, serial serial0)
found at: <REDACTED_TIMESTAMP> from fake sled agent
address: [fd00:1122:3344:101::1]:12345
Expand Down Expand Up @@ -1973,6 +1979,9 @@ LEDGERED SLED CONFIG
(measurement set is empty)
reconciler task status: idle (finished at <REDACTED_TIMESTAMP> after running for <REDACTED_DURATION>s)

HEALTH MONITOR
no data on SMF services in maintenance has been collected

KEEPER MEMBERSHIP
no membership retrieved

Expand Down
9 changes: 9 additions & 0 deletions dev-tools/reconfigurator-cli/tests/output/cmds-mupdate-update-flow-stdout
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,9 @@ LEDGERED SLED CONFIG
(measurement set is empty)
reconciler task status: idle (finished at <REDACTED_TIMESTAMP> after running for <REDACTED_DURATION>s)

HEALTH MONITOR
no data on SMF services in maintenance has been collected

sled 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6 (role = Gimlet, serial serial0)
found at: <REDACTED_TIMESTAMP> from fake sled agent
address: [fd00:1122:3344:101::1]:12345
Expand Down Expand Up @@ -450,6 +453,9 @@ LEDGERED SLED CONFIG
(measurement set is empty)
reconciler task status: idle (finished at <REDACTED_TIMESTAMP> after running for <REDACTED_DURATION>s)

HEALTH MONITOR
no data on SMF services in maintenance has been collected

sled d81c6a84-79b8-4958-ae41-ea46c9b19763 (role = Gimlet, serial serial2)
found at: <REDACTED_TIMESTAMP> from fake sled agent
address: [fd00:1122:3344:103::1]:12345
Expand Down Expand Up @@ -566,6 +572,9 @@ LEDGERED SLED CONFIG
(measurement set is empty)
reconciler task status: idle (finished at <REDACTED_TIMESTAMP> after running for <REDACTED_DURATION>s)

HEALTH MONITOR
no data on SMF services in maintenance has been collected

KEEPER MEMBERSHIP
no membership retrieved

Expand Down
9 changes: 9 additions & 0 deletions dev-tools/reconfigurator-cli/tests/output/cmds-nexus-generation-autobump-stdout
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,9 @@ LEDGERED SLED CONFIG
(measurement set is empty)
reconciler task status: idle (finished at <REDACTED_TIMESTAMP> after running for <REDACTED_DURATION>s)

HEALTH MONITOR
no data on SMF services in maintenance has been collected

sled 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6 (role = Gimlet, serial serial0)
found at: <REDACTED_TIMESTAMP> from fake sled agent
address: [fd00:1122:3344:101::1]:12345
Expand Down Expand Up @@ -886,6 +889,9 @@ LEDGERED SLED CONFIG
(measurement set is empty)
reconciler task status: idle (finished at <REDACTED_TIMESTAMP> after running for <REDACTED_DURATION>s)

HEALTH MONITOR
no data on SMF services in maintenance has been collected

sled d81c6a84-79b8-4958-ae41-ea46c9b19763 (role = Gimlet, serial serial2)
found at: <REDACTED_TIMESTAMP> from fake sled agent
address: [fd00:1122:3344:103::1]:12345
Expand Down Expand Up @@ -1061,6 +1067,9 @@ LEDGERED SLED CONFIG
(measurement set is empty)
reconciler task status: idle (finished at <REDACTED_TIMESTAMP> after running for <REDACTED_DURATION>s)

HEALTH MONITOR
no data on SMF services in maintenance has been collected

KEEPER MEMBERSHIP
no membership retrieved

Expand Down
9 changes: 9 additions & 0 deletions dev-tools/reconfigurator-cli/tests/output/cmds-target-release-stdout
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,9 @@ LEDGERED SLED CONFIG
(measurement set is empty)
reconciler task status: idle (finished at <REDACTED_TIMESTAMP> after running for <REDACTED_DURATION>s)

HEALTH MONITOR
no data on SMF services in maintenance has been collected

sled 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6 (role = Gimlet, serial serial0)
found at: <REDACTED_TIMESTAMP> from fake sled agent
address: [fd00:1122:3344:101::1]:12345
Expand Down Expand Up @@ -873,6 +876,9 @@ LEDGERED SLED CONFIG
(measurement set is empty)
reconciler task status: idle (finished at <REDACTED_TIMESTAMP> after running for <REDACTED_DURATION>s)

HEALTH MONITOR
no data on SMF services in maintenance has been collected

sled d81c6a84-79b8-4958-ae41-ea46c9b19763 (role = Gimlet, serial serial2)
found at: <REDACTED_TIMESTAMP> from fake sled agent
address: [fd00:1122:3344:103::1]:12345
Expand Down Expand Up @@ -1048,6 +1054,9 @@ LEDGERED SLED CONFIG
(measurement set is empty)
reconciler task status: idle (finished at <REDACTED_TIMESTAMP> after running for <REDACTED_DURATION>s)

HEALTH MONITOR
no data on SMF services in maintenance has been collected

KEEPER MEMBERSHIP
no membership retrieved

Expand Down
9 changes: 9 additions & 0 deletions dev-tools/reconfigurator-cli/tests/output/cmds-unsafe-zone-mgs-stdout
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,9 @@ LEDGERED SLED CONFIG
(measurement set is empty)
reconciler task status: idle (finished at <REDACTED_TIMESTAMP> after running for <REDACTED_DURATION>s)

HEALTH MONITOR
no data on SMF services in maintenance has been collected

sled 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6 (role = Gimlet, serial serial0)
found at: <REDACTED_TIMESTAMP> from fake sled agent
address: [fd00:1122:3344:101::1]:12345
Expand Down Expand Up @@ -857,6 +860,9 @@ LEDGERED SLED CONFIG
(measurement set is empty)
reconciler task status: idle (finished at <REDACTED_TIMESTAMP> after running for <REDACTED_DURATION>s)

HEALTH MONITOR
no data on SMF services in maintenance has been collected

sled d81c6a84-79b8-4958-ae41-ea46c9b19763 (role = Gimlet, serial serial2)
found at: <REDACTED_TIMESTAMP> from fake sled agent
address: [fd00:1122:3344:103::1]:12345
Expand Down Expand Up @@ -1032,6 +1038,9 @@ LEDGERED SLED CONFIG
(measurement set is empty)
reconciler task status: idle (finished at <REDACTED_TIMESTAMP> after running for <REDACTED_DURATION>s)

HEALTH MONITOR
no data on SMF services in maintenance has been collected

KEEPER MEMBERSHIP
no membership retrieved

Expand Down
13 changes: 2 additions & 11 deletions illumos-utils/src/svcs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use serde::Deserialize;
use serde::Serialize;
use slog::Logger;
use slog::{error, info};
use std::fmt::Display;
#[cfg(target_os = "illumos")]
use tokio::process::Command;

Expand Down Expand Up @@ -199,8 +198,8 @@ impl From<String> for SvcState {
#[serde(rename_all = "snake_case")]
/// Information about an SMF service that is enabled but not running
pub struct SvcInMaintenance {
fmri: String,
zone: String,
pub fmri: String,
pub zone: String,
}

impl SvcInMaintenance {
Expand All @@ -210,14 +209,6 @@ impl SvcInMaintenance {
}
}

impl Display for SvcInMaintenance {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let SvcInMaintenance { fmri, zone } = self;

writeln!(f, "FMRI: {} zone: {}", fmri, zone)
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
1 change: 1 addition & 0 deletions nexus/db-model/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ derive-where.workspace = true
diesel = { workspace = true, features = ["postgres", "r2d2", "chrono", "serde_json", "network-address", "uuid"] }
hex.workspace = true
iddqd.workspace = true
illumos-utils.workspace = true
ipnetwork.workspace = true
itertools.workspace = true
macaddr.workspace = true
Expand Down
55 changes: 53 additions & 2 deletions nexus/db-model/src/inventory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::Generation;
use crate::PhysicalDiskKind;
use crate::omicron_zone_config::{self, OmicronZoneNic};
use crate::sled_cpu_family::SledCpuFamily;
use crate::to_db_typed_uuid;
use crate::typed_uuid::DbTypedUuid;
use crate::{
ByteCount, MacAddr, Name, ServiceKind, SqlU8, SqlU16, SqlU32,
Expand All @@ -27,14 +28,16 @@ use diesel::pg::Pg;
use diesel::serialize::ToSql;
use diesel::{serialize, sql_types};
use iddqd::IdOrdMap;
use illumos_utils::svcs::SvcInMaintenance;
use ipnetwork::IpNetwork;
use nexus_db_schema::schema::inv_zone_manifest_non_boot;
use nexus_db_schema::schema::inv_zone_manifest_zone;
use nexus_db_schema::schema::{
hw_baseboard_id, inv_caboose, inv_clickhouse_keeper_membership,
inv_cockroachdb_status, inv_collection, inv_collection_error, inv_dataset,
inv_host_phase_1_active_slot, inv_host_phase_1_flash_hash,
inv_internal_dns, inv_last_reconciliation_dataset_result,
inv_health_monitor_svc_in_maintenance, inv_host_phase_1_active_slot,
inv_host_phase_1_flash_hash, inv_internal_dns,
inv_last_reconciliation_dataset_result,
inv_last_reconciliation_disk_result, inv_last_reconciliation_measurements,
inv_last_reconciliation_orphaned_dataset,
inv_last_reconciliation_zone_result, inv_measurement_manifest_non_boot,
Expand Down Expand Up @@ -63,6 +66,7 @@ use omicron_common::update::OmicronInstallManifestSource;
use omicron_common::zpool_name::ZpoolName;
use omicron_uuid_kinds::DatasetKind;
use omicron_uuid_kinds::DatasetUuid;
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::InternalZpoolKind;
use omicron_uuid_kinds::MupdateKind;
use omicron_uuid_kinds::MupdateOverrideKind;
Expand All @@ -72,6 +76,8 @@ use omicron_uuid_kinds::OmicronSledConfigUuid;
use omicron_uuid_kinds::PhysicalDiskUuid;
use omicron_uuid_kinds::SledKind;
use omicron_uuid_kinds::SledUuid;
use omicron_uuid_kinds::SvcInMaintenanceKind;
use omicron_uuid_kinds::SvcInMaintenanceUuid;
use omicron_uuid_kinds::ZpoolKind;
use omicron_uuid_kinds::{CollectionKind, OmicronZoneKind};
use omicron_uuid_kinds::{CollectionUuid, OmicronZoneUuid};
Expand Down Expand Up @@ -1016,6 +1022,51 @@ impl_enum_type!(
Idle => b"idle"
);

#[derive(Queryable, Clone, Debug, Selectable, Insertable)]
#[diesel(table_name = inv_health_monitor_svc_in_maintenance)]
pub struct InvSvcInMaintenance {
pub inv_collection_id: DbTypedUuid<CollectionKind>,
pub sled_id: DbTypedUuid<SledKind>,
pub id: DbTypedUuid<SvcInMaintenanceKind>,
pub fmri: Option<String>,
pub zone: Option<String>,
pub error_messages: Vec<String>,
pub svcs_cmd_error: Option<String>,
pub time_of_status: Option<DateTime<Utc>>,
}
Comment on lines +1025 to +1036
Copy link
Contributor Author

@karencfv karencfv Jan 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I went with flattening the SvcsInMaintenanceResult struct instead of creating two tables and joining them. It seemed like unnecessary complexity. This means that all of the rows from each collection will have the same time_of_status, error_messages, and svcs_cmd_error, but it doesn't really feel like a big deal.

pub struct SvcsInMaintenanceResult {
    pub services: Vec<SvcInMaintenance>,
    pub errors: Vec<String>,
    pub time_of_status: Option<DateTime<Utc>>,
}
pub struct SvcInMaintenance {
    pub fmri: String,
    pub zone: String,
}

Open to suggestions if there's a better way to go on about this!

Copy link
Contributor

@jgallagher jgallagher Jan 13, 2026

Choose a reason for hiding this comment

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

I don't love this because it seems to mix "Rust Vec becomes array of TEXT" and "Rust Vec becomes multiple rows" within the same type. (If we have multiple services entries we'll have multiple rows, but if we have multiple errors entries we'll only have one row.) I think we should pick one or the other; personally I shy away from SQL array because I've found them kinda difficult to work with, so I probably would have made this three tables:

  • a toplevel table that always has exactly one row per collection; this would record either time_of_status or svcs_cmd_error (with a CHECK constraint that we have one or the other, IIUC?).
  • a table for services
  • a table for errors

But I'm not sure if I'm overrotating on bad experiences with SQL arrays. @davepacheco might have an opinion here?

I guess the current model also introduces a possible invalid state it'd be nice to avoid: what if we have multiple rows from one collection that ought to have the same time_of_status / errors values, but don't? Not super likely to be a problem in practice, but sometimes we've been burned by adding this kind of structure and then later realizing it's actually kinda painful to deal with.

Copy link
Collaborator

@davepacheco davepacheco Jan 13, 2026

Choose a reason for hiding this comment

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

I also avoid arrays in SQL, also possibly for not-good reasons. I usually use a separate table like you've said here.

Copy link
Contributor Author

@karencfv karencfv Jan 13, 2026

Choose a reason for hiding this comment

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

Spoke about this at the update watercooler. Will update the PR with 3 tables


impl InvSvcInMaintenance {
pub fn new(
inv_collection_id: CollectionUuid,
sled_id: SledUuid,
svc: Option<SvcInMaintenance>,
svc_errors: Vec<String>,
svcs_cmd_error: Option<String>,
time_of_status: Option<DateTime<Utc>>,
) -> Self {
let (fmri, zone) = match svc {
Some(svc) => (Some(svc.fmri), Some(svc.zone)),
None => (None, None),
};

// This ID is only used as a primary key, it's fine to generate it here.
let id = to_db_typed_uuid(SvcInMaintenanceUuid::from_untyped_uuid(
Uuid::new_v4(),
));

Self {
inv_collection_id: inv_collection_id.into(),
sled_id: sled_id.into(),
id,
fmri,
zone,
error_messages: svc_errors,
svcs_cmd_error,
time_of_status,
}
}
}

/// See [`sled_agent_types::inventory::ConfigReconcilerInventory`].
#[derive(Queryable, Clone, Debug, Selectable, Insertable)]
#[diesel(table_name = inv_sled_config_reconciler)]
Expand Down
3 changes: 2 additions & 1 deletion nexus/db-model/src/schema_versions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock};
///
/// This must be updated when you change the database schema. Refer to
/// schema/crdb/README.adoc in the root of this repository for details.
pub const SCHEMA_VERSION: Version = Version::new(219, 0, 0);
pub const SCHEMA_VERSION: Version = Version::new(220, 0, 0);

/// List of all past database schema versions, in *reverse* order
///
Expand All @@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock<Vec<KnownVersion>> = LazyLock::new(|| {
// | leaving the first copy as an example for the next person.
// v
// KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
KnownVersion::new(220, "health-monitor-svcs-in-maintenance"),
KnownVersion::new(219, "blueprint-sled-last-used-ip"),
KnownVersion::new(218, "measurements"),
KnownVersion::new(217, "multiple-default-ip-pools-per-silo"),
Expand Down
Loading
Loading