BlueprintBuilder: remove SledEditor::baseboard_id() (#9405)

`SledEditor` was storing each sled's baseboard ID, but:

* it's only available via `PlanningInput`, and putting it directly into
the blueprint is difficult for mostly-irrelevant reasons
* there was only one caller of `SledEditor::baseboard_id()`:
`sled_ensure_mupdate_override()` (to match up sled ID <-> baseboard ID)
* `sled_ensure_mupdate_override()` itself only has one caller, the
planner, which already knows the baseboard ID of the sled it's ensuring

So this PR removes `SledEditor::baseboard_id()` entirely, and has the
planner pass the baseboard ID in. (On the update watercooler today we
also mentioned the possibility of a separate
`BlueprintBuilder::clear_pending_mgs_updates(baseboard_id)` method, but
that's tricky to implement because some nontrivial logic for "should we
clear pending MGS updates" is nested inside `SledEditor` itself.)

Author: jgallagher

nexus/reconfigurator/planning/src/blueprint_builder/builder.rs

@@ -631,7 +631,6 @@ impl<'a> BlueprintBuilder<'a> {
                             )
                         })?;
                     SledEditor::for_existing_active(
-                        Arc::new(details.baseboard_id.clone()),
                         details.resources.subnet,
                         sled_cfg.clone(),
                     )
@@ -652,7 +651,6 @@ impl<'a> BlueprintBuilder<'a> {
         for (sled_id, details) in input.all_sleds(SledFilter::Commissioned) {
             if let Entry::Vacant(slot) = sled_editors.entry(sled_id) {
                 slot.insert(SledEditor::for_new_active(
-                    Arc::new(details.baseboard_id.clone()),
                     details.resources.subnet,
                 ));
             }
@@ -1299,6 +1297,7 @@ impl<'a> BlueprintBuilder<'a> {
     pub(crate) fn sled_ensure_mupdate_override(
         &mut self,
         sled_id: SledUuid,
+        baseboard_id: &BaseboardId,
         // inv_mupdate_override_info has a weird type (not Option<&T>, not &str)
         // because this is what `Result::as_ref` returns.
         inv_mupdate_override_info: Result<
@@ -1312,13 +1311,6 @@ impl<'a> BlueprintBuilder<'a> {
                 "tried to ensure mupdate override for unknown sled {sled_id}"
             ))
         })?;
-        let baseboard_id = editor.baseboard_id().ok_or_else(|| {
-            // All commissioned sleds have baseboards; this should never fail.
-            Error::Planner(anyhow!(
-                "tried to ensure mupdate override for \
-                 decommissioned sled {sled_id}"
-            ))
-        })?;
 
         // Also map the editor to the corresponding PendingMgsUpdates.
         let pending_mgs_update = self.pending_mgs_updates.entry(baseboard_id);

nexus/reconfigurator/planning/src/blueprint_editor/sled_editor.rs

@@ -36,7 +36,6 @@ use nexus_types::deployment::BlueprintZoneType;
 use nexus_types::deployment::PendingMgsUpdate;
 use nexus_types::deployment::blueprint_zone_type;
 use nexus_types::external_api::views::SledState;
-use nexus_types::inventory::BaseboardId;
 use omicron_common::address::Ipv6Subnet;
 use omicron_common::address::SLED_PREFIX;
 use omicron_common::api::external::Generation;
@@ -51,7 +50,6 @@ use scalar::ScalarEditor;
 use std::iter;
 use std::mem;
 use std::net::Ipv6Addr;
-use std::sync::Arc;
 use underlay_ip_allocator::SledUnderlayIpAllocator;
 
 mod datasets;
@@ -162,7 +160,6 @@ enum InnerSledEditor {
 
 impl SledEditor {
     pub fn for_existing_active(
-        baseboard_id: Arc<BaseboardId>,
         subnet: Ipv6Subnet<SLED_PREFIX>,
         config: BlueprintSledConfig,
     ) -> Result<Self, SledInputError> {
@@ -171,7 +168,7 @@ impl SledEditor {
             SledState::Active,
             "for_existing_active called on non-active sled"
         );
-        let inner = ActiveSledEditor::new(baseboard_id, subnet, config)?;
+        let inner = ActiveSledEditor::new(subnet, config)?;
         Ok(Self(InnerSledEditor::Active(inner)))
     }
 
@@ -191,14 +188,8 @@ impl SledEditor {
         Ok(Self(InnerSledEditor::Decommissioned(inner)))
     }
 
-    pub fn for_new_active(
-        baseboard_id: Arc<BaseboardId>,
-        subnet: Ipv6Subnet<SLED_PREFIX>,
-    ) -> Self {
-        Self(InnerSledEditor::Active(ActiveSledEditor::new_empty(
-            baseboard_id,
-            subnet,
-        )))
+    pub fn for_new_active(subnet: Ipv6Subnet<SLED_PREFIX>) -> Self {
+        Self(InnerSledEditor::Active(ActiveSledEditor::new_empty(subnet)))
     }
 
     pub fn finalize(self) -> EditedSled {
@@ -215,15 +206,6 @@ impl SledEditor {
         }
     }
 
-    /// Returns the baseboard of this sled if it is active, or `None` if it is
-    /// decommissioned.
-    pub fn baseboard_id(&self) -> Option<&Arc<BaseboardId>> {
-        match &self.0 {
-            InnerSledEditor::Active(active) => Some(&active.baseboard_id),
-            InnerSledEditor::Decommissioned(_) => None,
-        }
-    }
-
     /// Returns the subnet of this sled if it is active, or `None` if it is
     /// decommissioned.
     pub fn subnet(&self) -> Option<Ipv6Subnet<SLED_PREFIX>> {
@@ -261,10 +243,9 @@ impl SledEditor {
                 // below, we'll be left in the active state with an empty sled
                 // editor), but omicron in general is not panic safe and aborts
                 // on panic. Plus `finalize()` should never panic.
-                let mut stolen = ActiveSledEditor::new_empty(
-                    Arc::clone(&editor.baseboard_id),
-                    Ipv6Subnet::new(Ipv6Addr::LOCALHOST),
-                );
+                let mut stolen = ActiveSledEditor::new_empty(Ipv6Subnet::new(
+                    Ipv6Addr::LOCALHOST,
+                ));
                 mem::swap(editor, &mut stolen);
 
                 let mut finalized = stolen.finalize();
@@ -506,7 +487,6 @@ impl SledEditor {
 
 #[derive(Debug)]
 struct ActiveSledEditor {
-    baseboard_id: Arc<BaseboardId>,
     underlay_ip_allocator: SledUnderlayIpAllocator,
     incoming_sled_agent_generation: Generation,
     zones: ZonesEditor,
@@ -526,7 +506,6 @@ pub(crate) struct EditedSled {
 
 impl ActiveSledEditor {
     pub fn new(
-        baseboard_id: Arc<BaseboardId>,
         subnet: Ipv6Subnet<SLED_PREFIX>,
         config: BlueprintSledConfig,
     ) -> Result<Self, SledInputError> {
@@ -541,7 +520,6 @@ impl ActiveSledEditor {
             zones.zones(BlueprintZoneDisposition::any).map(|z| z.underlay_ip());
 
         Ok(Self {
-            baseboard_id,
             underlay_ip_allocator: SledUnderlayIpAllocator::new(
                 subnet, zone_ips,
             ),
@@ -557,10 +535,7 @@ impl ActiveSledEditor {
         })
     }
 
-    pub fn new_empty(
-        baseboard_id: Arc<BaseboardId>,
-        subnet: Ipv6Subnet<SLED_PREFIX>,
-    ) -> Self {
+    pub fn new_empty(subnet: Ipv6Subnet<SLED_PREFIX>) -> Self {
         // Creating the underlay IP allocator can only fail if we have a zone
         // with an IP outside the sled subnet, but we don't have any zones at
         // all, so this can't fail. Match explicitly to guard against this error
@@ -569,7 +544,6 @@ impl ActiveSledEditor {
             SledUnderlayIpAllocator::new(subnet, iter::empty());
 
         Self {
-            baseboard_id,
             underlay_ip_allocator,
             incoming_sled_agent_generation: Generation::new(),
             zones: ZonesEditor::empty(),

nexus/reconfigurator/planning/src/planner.rs

@@ -1835,7 +1835,9 @@ impl<'a> Planner<'a> {
 
         // We use the list of in-service sleds here -- we don't want to alter
         // expunged or decommissioned sleds.
-        for sled_id in self.input.all_sled_ids(SledFilter::InService) {
+        for (sled_id, sled_details) in
+            self.input.all_sleds(SledFilter::InService)
+        {
             let log = log.new(o!("sled_id" => sled_id.to_string()));
             let Some(inv_sled) = self.inventory.sled_agents.get(&sled_id)
             else {
@@ -1844,6 +1846,7 @@ impl<'a> Planner<'a> {
             };
             let action = self.blueprint.sled_ensure_mupdate_override(
                 sled_id,
+                &sled_details.baseboard_id,
                 inv_sled
                     .zone_image_resolver
                     .mupdate_override