[update-status] Handle svcs in-transition and unrecognized states

illumos-utils/src/svcs.rs

@@ -101,7 +101,9 @@ impl SvcsResult {
                         | SvcState::Disabled
                         | SvcState::Offline
                         | SvcState::Online
-                        | SvcState::Uninitialized => {
+                        | SvcState::Uninitialized
+                        | SvcState::InTransition
+                        | SvcState::Unrecognized => {
                             let fmri = if let Some(fmri) = svc.next() {
                                 fmri.to_string()
                             } else {
@@ -166,6 +168,9 @@ impl SvcsResult {
                     SvcState::Maintenance => {
                         SvcEnabledNotOnlineState::Maintenance
                     }
+                    SvcState::Unrecognized => {
+                        SvcEnabledNotOnlineState::Unrecognized
+                    }
                     // `legacy_run` is excluded here because this state doesn't
                     // really say anything about whether a service is running or
                     // not. It just states that this is a service that isn't
@@ -176,10 +181,16 @@ impl SvcsResult {
                     // returns, so we exclude it as well.
                     // More detail in
                     // https://github.com/oxidecomputer/omicron/issues/10316
+                    //
+                    // `InTransition` (or state with '*' appended as represented
+                    // in svcs)  is excluded because it is a momentary state
+                    // while a service moves between two states, not a stable
+                    // "enabled not online" condition worth reporting.
                     SvcState::Online
                     | SvcState::Uninitialized
                     | SvcState::Disabled
-                    | SvcState::LegacyRun => return None,
+                    | SvcState::LegacyRun
+                    | SvcState::InTransition => return None,
                 };
                 Some(SvcEnabledNotOnline {
                     fmri: svc.fmri,
@@ -198,6 +209,11 @@ impl SvcsResult {
 }
 
 fn parse_svc_state(state: &str) -> Option<SvcState> {
+    // Per `man svcs`, an asterisk (*) is appended to the state of instances
+    // that are in transition from one state to another.
+    if state.ends_with('*') {
+        return Some(SvcState::InTransition);
+    }
     match state {
         "uninitialized" => Some(SvcState::Uninitialized),
         "offline" => Some(SvcState::Offline),
@@ -206,6 +222,9 @@ fn parse_svc_state(state: &str) -> Option<SvcState> {
         "maintenance" => Some(SvcState::Maintenance),
         "disabled" => Some(SvcState::Disabled),
         "legacy_run" => Some(SvcState::LegacyRun),
+        // Per `man svcs`, absent or unrecognized states are denoted by a
+        // question mark (?) character.
+        "?" => Some(SvcState::Unrecognized),
         _ => None,
     }
 }
@@ -440,6 +459,44 @@ disabled       svc:/network/tcpkey:default                      global
         );
     }
 
+    #[test]
+    fn test_svc_parse_in_transition_and_unrecognized() {
+        let output = r#"online*        svc:/milestone/sysconfig:default                 global
+?              svc:/site/fake-service:default                   global
+disabled       svc:/network/tcpkey:default                      global
+"#;
+
+        let log = log();
+        let result = SvcsResult::parse(&log, output.as_bytes());
+
+        assert_eq!(result.services.len(), 3);
+        assert_eq!(result.errors.len(), 0);
+        assert_eq!(
+            result.services[0],
+            Svc {
+                fmri: "svc:/milestone/sysconfig:default".to_string(),
+                zone: "global".to_string(),
+                state: SvcState::InTransition,
+            }
+        );
+        assert_eq!(
+            result.services[1],
+            Svc {
+                fmri: "svc:/site/fake-service:default".to_string(),
+                zone: "global".to_string(),
+                state: SvcState::Unrecognized,
+            }
+        );
+        assert_eq!(
+            result.services[2],
+            Svc {
+                fmri: "svc:/network/tcpkey:default".to_string(),
+                zone: "global".to_string(),
+                state: SvcState::Disabled,
+            }
+        );
+    }
+
     #[test]
     fn test_to_enabled_not_online() {
         let mk_svc = |i: usize, state: SvcState| Svc {
@@ -466,6 +523,8 @@ disabled       svc:/network/tcpkey:default                      global
             mk_svc(7, SvcState::Maintenance),
             mk_svc(8, SvcState::Maintenance),
             mk_svc(9, SvcState::Uninitialized),
+            mk_svc(10, SvcState::Unrecognized),
+            mk_svc(11, SvcState::InTransition),
         ];
         let result = SvcsResult {
             services,
@@ -482,6 +541,7 @@ disabled       svc:/network/tcpkey:default                      global
                 mk_e_not_o_svc(3, SvcEnabledNotOnlineState::Degraded),
                 mk_e_not_o_svc(7, SvcEnabledNotOnlineState::Maintenance),
                 mk_e_not_o_svc(8, SvcEnabledNotOnlineState::Maintenance),
+                mk_e_not_o_svc(10, SvcEnabledNotOnlineState::Unrecognized),
             ]
         );
     }

nexus/db-model/src/inventory.rs

@@ -2303,6 +2303,7 @@ impl_enum_type!(
     Offline => b"offline"
     Degraded => b"degraded"
     Maintenance => b"maintenance"
+    Unrecognized => b"unrecognized"
 );
 
 impl From<SvcEnabledNotOnlineState> for InvSvcEnabledNotOnlineState {
@@ -2317,6 +2318,9 @@ impl From<SvcEnabledNotOnlineState> for InvSvcEnabledNotOnlineState {
             SvcEnabledNotOnlineState::Maintenance => {
                 InvSvcEnabledNotOnlineState::Maintenance
             }
+            SvcEnabledNotOnlineState::Unrecognized => {
+                InvSvcEnabledNotOnlineState::Unrecognized
+            }
         }
     }
 }
@@ -2333,6 +2337,9 @@ impl From<InvSvcEnabledNotOnlineState> for SvcEnabledNotOnlineState {
             InvSvcEnabledNotOnlineState::Maintenance => {
                 SvcEnabledNotOnlineState::Maintenance
             }
+            InvSvcEnabledNotOnlineState::Unrecognized => {
+                SvcEnabledNotOnlineState::Unrecognized
+            }
         }
     }
 }

nexus/db-model/src/schema_versions.rs

@@ -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(285, 0, 0);
+pub const SCHEMA_VERSION: Version = Version::new(286, 0, 0);
 
 /// List of all past database schema versions, in *reverse* order
 ///
@@ -28,6 +28,7 @@ pub 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(286, "add-unrecognized-svc-enabled-not-online-state"),
         KnownVersion::new(285, "fm-config"),
         KnownVersion::new(284, "prune-service-nat-entries"),
         KnownVersion::new(283, "inventory-zone-nic-dual-stack"),

nexus/src/app/update.rs

@@ -1433,9 +1433,6 @@ mod test {
     }
 
     #[nexus_test(server = crate::Server)]
-    // TODO-K: Enable once https://github.com/oxidecomputer/omicron/issues/10997
-    // is worked on
-    #[ignore]
     async fn test_contact_support_services_errors_only(
         cptestctx: &ControlPlaneTestContext,
     ) {
@@ -2237,9 +2234,6 @@ mod test {
     }
 
     #[test]
-    // TODO-K: Enable once https://github.com/oxidecomputer/omicron/issues/10997
-    // is worked on
-    #[ignore]
     fn test_problems_unhealthy_services_errors_only() {
         let logctx =
             test_setup_log("test_problems_unhealthy_services_errors_only");

nexus/types/src/inventory.rs

@@ -324,11 +324,7 @@ impl Collection {
             .filter_map(|sled_agent| {
                 match &sled_agent.smf_services_enabled_not_online {
                     SvcsEnabledNotOnlineResult::SvcsEnabledNotOnline(svcs)
-                        // This should check if svcs.is_empty() which includes
-                        // parsing errors. We have a bug with this at the moment
-                        // https://github.com/oxidecomputer/omicron/issues/10997
-                        // This check should change once that issue is resolved
-                        if svcs.services.is_empty() =>
+                        if svcs.is_empty() =>
                     {
                         None
                     }

openapi/sled-agent/sled-agent-45.0.0-264d85.json.gitstub

@@ -0,0 +1 @@
+1a20a12eb13a20c58f91dfae51bb3c4836b4e069:openapi/sled-agent/sled-agent-45.0.0-264d85.json

openapi/sled-agent/sled-agent-45.0.0-264d85.json → openapi/sled-agent/sled-agent-46.0.0-1baf31.json

@@ -7,7 +7,7 @@
       "url": "https://oxide.computer",
       "email": "api@oxide.computer"
     },
-    "version": "45.0.0"
+    "version": "46.0.0"
   },
   "paths": {
     "/artifacts": {
@@ -9907,6 +9907,13 @@
             "enum": [
               "maintenance"
             ]
+          },
+          {
+            "description": "An instance whose state is absent or unrecognized. Note: as per `man svcs`, \"Absent or unrecognized states are denoted by a question mark (?) character\". So there is not an \"unrecognized\" state per se in svcs.",
+            "type": "string",
+            "enum": [
+              "unrecognized"
+            ]
           }
         ]
       },

openapi/sled-agent/sled-agent-latest.json

@@ -1 +1 @@
-sled-agent-45.0.0-264d85.json
+sled-agent-46.0.0-1baf31.json

schema/crdb/add-unrecognized-svc-enabled-not-online-state/up.sql

@@ -0,0 +1,7 @@
+ALTER TYPE
+  omicron.public.inv_svc_enabled_not_online_state
+ADD VALUE IF NOT EXISTS
+  'unrecognized'
+AFTER
+  'maintenance'
+;

schema/crdb/dbinit.sql

@@ -5318,7 +5318,8 @@ CREATE TABLE IF NOT EXISTS omicron.public.inv_internal_dns (
 CREATE TYPE IF NOT EXISTS omicron.public.inv_svc_enabled_not_online_state AS ENUM (
     'offline',
     'degraded',
-    'maintenance'
+    'maintenance',
+    'unrecognized'
 );
 
 CREATE TABLE IF NOT EXISTS omicron.public.inv_svc_enabled_not_online (
@@ -9181,7 +9182,7 @@ INSERT INTO omicron.public.db_metadata (
     version,
     target_version
 ) VALUES
-    (TRUE, NOW(), NOW(), '285.0.0', NULL)
+    (TRUE, NOW(), NOW(), '286.0.0', NULL)
 ON CONFLICT DO NOTHING;
 
 COMMIT;

sled-agent/api/src/lib.rs

@@ -22,6 +22,7 @@ use omicron_common::api::internal::{
 use sled_agent_types_versions::{
     latest, v1, v4, v6, v7, v9, v10, v11, v12, v14, v16, v17, v18, v20, v22,
     v24, v25, v26, v28, v29, v30, v31, v32, v33, v34, v37, v39, v40, v41, v42,
+    v43,
 };
 use sled_diagnostics::SledDiagnosticsQueryOutput;
 use slog_error_chain::InlineErrorChain;
@@ -38,6 +39,7 @@ api_versions!([
     // |  example for the next person.
     // v
     // (next_int, IDENT),
+    (46, MODIFY_SVC_STATE_ENUM),
     (45, REMOVE_UPLINK_ENSURE),
     (44, PROPOLIS_NVME_VWC),
     (43, INVENTORY_BASEBOARD_ID),
@@ -1130,12 +1132,26 @@ pub trait SledAgentApi {
     #[endpoint {
         method = GET,
         path = "/inventory",
-        versions = VERSION_INVENTORY_BASEBOARD_ID..,
+        versions = VERSION_MODIFY_SVC_STATE_ENUM..,
     }]
     async fn inventory(
         rqctx: RequestContext<Self::Context>,
     ) -> Result<HttpResponseOk<latest::inventory::Inventory>, HttpError>;
 
+    /// Fetch basic information about this sled
+    #[endpoint {
+        operation_id = "inventory",
+        method = GET,
+        path = "/inventory",
+        versions = VERSION_INVENTORY_BASEBOARD_ID..VERSION_MODIFY_SVC_STATE_ENUM,
+    }]
+    async fn inventory_v43(
+        rqctx: RequestContext<Self::Context>,
+    ) -> Result<HttpResponseOk<v43::inventory::Inventory>, HttpError> {
+        let HttpResponseOk(inventory) = Self::inventory(rqctx).await?;
+        inventory.try_into().map_err(HttpError::from).map(HttpResponseOk)
+    }
+
     /// Fetch basic information about this sled
     #[endpoint {
         operation_id = "inventory",
@@ -1146,7 +1162,7 @@ pub trait SledAgentApi {
     async fn inventory_v40(
         rqctx: RequestContext<Self::Context>,
     ) -> Result<HttpResponseOk<v40::inventory::Inventory>, HttpError> {
-        Self::inventory(rqctx).await.map(|HttpResponseOk(inv)| {
+        Self::inventory_v43(rqctx).await.map(|HttpResponseOk(inv)| {
             HttpResponseOk(v40::inventory::Inventory::from(inv))
         })
     }

sled-agent/types/versions/src/impls/inventory.rs

@@ -1055,6 +1055,7 @@ impl From<SvcEnabledNotOnlineState> for SvcState {
             SvcEnabledNotOnlineState::Degraded => Self::Degraded,
             SvcEnabledNotOnlineState::Maintenance => Self::Maintenance,
             SvcEnabledNotOnlineState::Offline => Self::Offline,
+            SvcEnabledNotOnlineState::Unrecognized => Self::Unrecognized,
         }
     }
 }
@@ -1069,6 +1070,8 @@ impl fmt::Display for SvcState {
             SvcState::Maintenance => "maintenance",
             SvcState::Disabled => "disabled",
             SvcState::LegacyRun => "legacy_run",
+            SvcState::InTransition => "in_transition",
+            SvcState::Unrecognized => "unrecognized",
         };
 
         write!(f, "{state}")
@@ -1081,6 +1084,7 @@ impl fmt::Display for SvcEnabledNotOnlineState {
             SvcEnabledNotOnlineState::Offline => "offline",
             SvcEnabledNotOnlineState::Degraded => "degraded",
             SvcEnabledNotOnlineState::Maintenance => "maintenance",
+            SvcEnabledNotOnlineState::Unrecognized => "unrecognized",
         };
 
         write!(f, "{state}")

sled-agent/types/versions/src/latest.rs

@@ -176,23 +176,23 @@ pub mod inventory {
     pub use crate::v24::inventory::InventoryZpool;
     pub use crate::v24::inventory::ZpoolHealth;
 
-    pub use crate::v34::inventory::Svc;
-    pub use crate::v34::inventory::SvcState;
     pub use crate::v34::inventory::SvcsError;
 
-    pub use crate::v37::inventory::SvcEnabledNotOnline;
-    pub use crate::v37::inventory::SvcEnabledNotOnlineState;
-    pub use crate::v37::inventory::SvcsEnabledNotOnline;
-    pub use crate::v37::inventory::SvcsEnabledNotOnlineResult;
-
     pub use crate::v40::inventory::FMD_MAX_CASES;
     pub use crate::v40::inventory::FMD_MAX_RESOURCES;
     pub use crate::v40::inventory::FmdHostCase;
     pub use crate::v40::inventory::FmdInventory;
     pub use crate::v40::inventory::FmdInventoryError;
     pub use crate::v40::inventory::FmdInventoryErrorKind;
     pub use crate::v40::inventory::FmdResource;
-    pub use crate::v43::inventory::Inventory;
+
+    pub use crate::v46::inventory::Inventory;
+    pub use crate::v46::inventory::Svc;
+    pub use crate::v46::inventory::SvcEnabledNotOnline;
+    pub use crate::v46::inventory::SvcEnabledNotOnlineState;
+    pub use crate::v46::inventory::SvcState;
+    pub use crate::v46::inventory::SvcsEnabledNotOnline;
+    pub use crate::v46::inventory::SvcsEnabledNotOnlineResult;
 
     pub use crate::impls::inventory::FmdHostCaseDisplay;
     pub use crate::impls::inventory::FmdInventoryDisplay;

sled-agent/types/versions/src/lib.rs

@@ -95,6 +95,8 @@ pub mod v42;
 pub mod v43;
 #[path = "propolis_nvme_vwc/mod.rs"]
 pub mod v44;
+#[path = "modify_svc_state_enum/mod.rs"]
+pub mod v46;
 #[path = "add_probe_put_endpoint/mod.rs"]
 pub mod v6;
 #[path = "multicast_support/mod.rs"]