sled-agent: Move InternalDisks and friends from sled-agent-config-reconciler to sled-agent-types

sled-agent/config-reconciler/Cargo.toml

@@ -50,6 +50,7 @@ proptest.workspace = true
 schemars.workspace = true
 scopeguard.workspace = true
 serde_json.workspace = true
+sled-agent-types = { workspace = true, features = ["testing"] }
 sled-storage = { workspace = true, features = ["testing"] }
 test-strategy.workspace = true
 xshell.workspace = true

sled-agent/config-reconciler/src/handle.rs

@@ -13,6 +13,10 @@ use nexus_sled_agent_shared::inventory::InventoryZpool;
 use nexus_sled_agent_shared::inventory::OmicronSledConfig;
 use omicron_common::disk::DatasetName;
 use sled_agent_api::ArtifactConfig;
+use sled_agent_types::internal_disks::InternalDisksReceiver;
+use sled_agent_types::raw_disks;
+use sled_agent_types::raw_disks::RawDisksReceiver;
+use sled_agent_types::raw_disks::RawDisksSender;
 use sled_storage::config::MountConfig;
 use sled_storage::disk::Disk;
 use sled_storage::manager::NestedDatasetConfig;
@@ -51,12 +55,8 @@ use crate::TimeSyncStatus;
 use crate::dataset_serialization_task::DatasetTaskHandle;
 use crate::dataset_serialization_task::NestedDatasetMountError;
 use crate::dump_setup_task;
-use crate::internal_disks::InternalDisksReceiver;
 use crate::ledger::CurrentSledConfig;
 use crate::ledger::LedgerTaskHandle;
-use crate::raw_disks;
-use crate::raw_disks::RawDisksReceiver;
-use crate::raw_disks::RawDisksSender;
 use crate::reconciler_task;
 use crate::reconciler_task::CurrentlyManagedZpools;
 use crate::reconciler_task::CurrentlyManagedZpoolsReceiver;

sled-agent/config-reconciler/src/lib.rs

@@ -15,16 +15,17 @@
 //! will spawn several tokio tasks that run for the duration of the sled-agent
 //! process:
 //!
-//! * A task for managing internal disks (in the `internal_disks` module of this
-//!   crate). This task takes raw disks as input over a watch channel, and emits
-//!   managed internal disks as output on another watch channel. Sled-agent
-//!   components that care about internal disks can get access to this output
-//!   channel via [`ConfigReconcilerHandle::internal_disks_rx()`]. On Gimlet and
-//!   Cosmo, "internal" disks have the M.2 form factor; much existing code will
-//!   refer to them as `m2` disks, but the form factor may change in future
-//!   sleds. The term "internal disks" is for "the disks that are not easily
-//!   swappable and can be managed before trust quorum has unlocked"; we use
-//!   them to store ledgers, etc.
+//! * A task for managing internal disks (implemented in the `sled-agent-types`
+//!   crate to avoid cyclic crate dependencies). This task takes raw disks as
+//!   input over a watch channel, and emits managed internal disks as output on
+//!   another watch channel. Sled-agent components that care about internal
+//!   disks can get access to this output channel via
+//!   [`ConfigReconcilerHandle::internal_disks_rx()`]. On Gimlet and Cosmo,
+//!   "internal" disks have the M.2 form factor; much existing code will refer
+//!   to them as `m2` disks, but the form factor may change in future sleds. The
+//!   term "internal disks" is for "the disks that are not easily swappable and
+//!   can be managed before trust quorum has unlocked"; we use them to store
+//!   ledgers, etc.
 //! * A task for serializing ZFS operations on datasets (in the
 //!   `dataset_serialization_task` module of this crate). This task takes
 //!   requests over an `mpsc` channel. This channel is not exposed directly;
@@ -53,9 +54,7 @@ mod dataset_serialization_task;
 mod disks_common;
 mod dump_setup_task;
 mod handle;
-mod internal_disks;
 mod ledger;
-mod raw_disks;
 mod reconciler_task;
 mod sled_agent_facilities;
 
@@ -74,16 +73,19 @@ pub use handle::ConfigReconcilerSpawnToken;
 pub use handle::InventoryError;
 pub use handle::ReconcilerInventory;
 pub use handle::TimeSyncConfig;
-pub use internal_disks::InternalDisks;
-pub use internal_disks::InternalDisksReceiver;
-pub use internal_disks::InternalDisksWithBootDisk;
 pub use ledger::LedgerArtifactConfigError;
 pub use ledger::LedgerNewConfigError;
 pub use ledger::LedgerTaskError;
-pub use raw_disks::RawDisksSender;
 pub use reconciler_task::CurrentlyManagedZpools;
 pub use reconciler_task::CurrentlyManagedZpoolsReceiver;
 pub use reconciler_task::TimeSyncError;
 pub use reconciler_task::TimeSyncStatus;
 pub use sled_agent_facilities::SledAgentArtifactStore;
 pub use sled_agent_facilities::SledAgentFacilities;
+
+// Re-export types that have moved to sled-agent-types purely for crate
+// dependency graph reasons.
+pub use sled_agent_types::internal_disks::InternalDisks;
+pub use sled_agent_types::internal_disks::InternalDisksReceiver;
+pub use sled_agent_types::internal_disks::InternalDisksWithBootDisk;
+pub use sled_agent_types::raw_disks::RawDisksSender;

sled-agent/config-reconciler/src/reconciler_task.rs

@@ -21,6 +21,7 @@ use omicron_common::disk::DatasetKind;
 use omicron_uuid_kinds::DatasetUuid;
 use omicron_uuid_kinds::OmicronZoneUuid;
 use omicron_uuid_kinds::PhysicalDiskUuid;
+use sled_agent_types::raw_disks::RawDisksReceiver;
 use sled_storage::config::MountConfig;
 use sled_storage::dataset::U2_DEBUG_DATASET;
 use sled_storage::dataset::ZONE_DATASET;
@@ -39,7 +40,6 @@ use tokio::sync::watch;
 use crate::TimeSyncConfig;
 use crate::dataset_serialization_task::DatasetTaskHandle;
 use crate::ledger::CurrentSledConfig;
-use crate::raw_disks::RawDisksReceiver;
 use crate::sled_agent_facilities::SledAgentFacilities;
 
 mod datasets;

sled-agent/config-reconciler/src/reconciler_task/external_disks.rs

@@ -10,6 +10,7 @@
 use futures::future;
 use id_map::IdMap;
 use id_map::IdMappable;
+use iddqd::IdOrdMap;
 use illumos_utils::zpool::Zpool;
 use illumos_utils::zpool::ZpoolName;
 use key_manager::StorageKeyRequester;
@@ -38,7 +39,6 @@ use tokio::sync::watch;
 
 use crate::disks_common::MaybeUpdatedDisk;
 use crate::disks_common::update_properties_from_raw_disk;
-use crate::raw_disks::RawDiskWithId;
 
 /// Set of currently managed zpools.
 ///
@@ -316,7 +316,7 @@ impl ExternalDisks {
     /// set.
     pub(super) fn stop_managing_if_needed(
         &mut self,
-        raw_disks: &IdMap<RawDiskWithId>,
+        raw_disks: &IdOrdMap<RawDisk>,
         config: &IdMap<OmicronPhysicalDiskConfig>,
         log: &Logger,
     ) {
@@ -373,7 +373,7 @@ impl ExternalDisks {
     /// already managing.
     pub(super) async fn start_managing_if_needed(
         &mut self,
-        raw_disks: &IdMap<RawDiskWithId>,
+        raw_disks: &IdOrdMap<RawDisk>,
         config: &IdMap<OmicronPhysicalDiskConfig>,
         key_requester: &StorageKeyRequester,
         log: &Logger,
@@ -389,7 +389,7 @@ impl ExternalDisks {
 
     async fn start_managing_if_needed_with_disk_adopter<T: DiskAdopter>(
         &mut self,
-        raw_disks: &IdMap<RawDiskWithId>,
+        raw_disks: &IdOrdMap<RawDisk>,
         config: &IdMap<OmicronPhysicalDiskConfig>,
         log: &Logger,
         disk_adopter: &T,
@@ -705,7 +705,7 @@ mod tests {
         })
     }
 
-    fn make_raw_test_disk(variant: DiskVariant, serial: &str) -> RawDiskWithId {
+    fn make_raw_test_disk(variant: DiskVariant, serial: &str) -> RawDisk {
         RawDisk::Real(UnparsedDisk::new(
             "/test-devfs".into(),
             None,
@@ -719,7 +719,6 @@ mod tests {
             false,
             DiskFirmware::new(0, None, false, 1, vec![]),
         ))
-        .into()
     }
 
     fn with_test_runtime<Fut, T>(fut: Fut) -> T
@@ -770,7 +769,7 @@ mod tests {
         })
     }
 
-    async fn internal_disks_are_rejected_impl(raw_disks: IdMap<RawDiskWithId>) {
+    async fn internal_disks_are_rejected_impl(raw_disks: IdOrdMap<RawDisk>) {
         let logctx = dev::test_setup_log("internal_disks_are_rejected");
 
         let (currently_managed_zpools_tx, _rx) = watch::channel(Arc::default());
@@ -853,7 +852,7 @@ mod tests {
     // Report errors for any requested disks that don't exist.
     #[proptest]
     fn fail_if_disk_not_present(disks: BTreeMap<String, bool>) {
-        let mut raw_disks = IdMap::default();
+        let mut raw_disks = IdOrdMap::default();
         let mut config_disks = IdMap::default();
         let mut not_present = BTreeSet::new();
 
@@ -865,7 +864,7 @@ mod tests {
                 pool_id: ZpoolUuid::new_v4(),
             };
             if is_present {
-                raw_disks.insert(raw_disk);
+                raw_disks.insert_overwrite(raw_disk);
             } else {
                 not_present.insert(config_disk.id);
             }
@@ -879,7 +878,7 @@ mod tests {
     }
 
     async fn fail_if_disk_not_present_impl(
-        raw_disks: IdMap<RawDiskWithId>,
+        raw_disks: IdOrdMap<RawDisk>,
         config_disks: IdMap<OmicronPhysicalDiskConfig>,
         not_present: BTreeSet<PhysicalDiskUuid>,
     ) {
@@ -937,7 +936,7 @@ mod tests {
     // Stop managing disks if so requested.
     #[proptest]
     fn firmware_updates_are_propagated(disks: BTreeMap<String, bool>) {
-        let mut raw_disks = IdMap::default();
+        let mut raw_disks = IdOrdMap::default();
         let mut config_disks = IdMap::default();
         let mut should_mutate_firmware = BTreeSet::new();
 
@@ -951,7 +950,7 @@ mod tests {
             if should_mutate {
                 should_mutate_firmware.insert(raw_disk.identity().clone());
             }
-            raw_disks.insert(raw_disk);
+            raw_disks.insert_overwrite(raw_disk);
             config_disks.insert(config_disk);
         }
 
@@ -966,7 +965,7 @@ mod tests {
     }
 
     async fn firmware_updates_are_propagated_impl(
-        mut raw_disks: IdMap<RawDiskWithId>,
+        mut raw_disks: IdOrdMap<RawDisk>,
         config_disks: IdMap<OmicronPhysicalDiskConfig>,
         should_mutate_firmware: BTreeSet<DiskIdentity>,
     ) {
@@ -1011,16 +1010,13 @@ mod tests {
         // Change the firmware on some subset of disks.
         for id in should_mutate_firmware {
             let mut entry = raw_disks.get_mut(&id).unwrap();
-            let mut raw_disk = RawDisk::from(entry.clone());
-            let new_firmware = DiskFirmware::new(
-                raw_disk.firmware().active_slot().wrapping_add(1),
+            *entry.firmware_mut() = DiskFirmware::new(
+                entry.firmware().active_slot().wrapping_add(1),
                 None,
                 false,
                 1,
                 Vec::new(),
             );
-            *raw_disk.firmware_mut() = new_firmware;
-            *entry = raw_disk.into();
         }
 
         // Attempt to adopt all the config disks again; we should pick up the
@@ -1059,7 +1055,7 @@ mod tests {
     // `ExternalDiskState`.
     #[proptest]
     fn remove_disks_not_in_config(disks: BTreeMap<String, bool>) {
-        let mut raw_disks = IdMap::default();
+        let mut raw_disks = IdOrdMap::default();
         let mut config_disks = IdMap::default();
         let mut should_remove_after_adding = BTreeSet::new();
 
@@ -1073,7 +1069,7 @@ mod tests {
             if should_remove {
                 should_remove_after_adding.insert(config_disk.id);
             }
-            raw_disks.insert(raw_disk);
+            raw_disks.insert_overwrite(raw_disk);
             config_disks.insert(config_disk);
         }
 
@@ -1088,7 +1084,7 @@ mod tests {
     }
 
     async fn remove_disks_not_in_config_impl(
-        raw_disks: IdMap<RawDiskWithId>,
+        raw_disks: IdOrdMap<RawDisk>,
         mut config_disks: IdMap<OmicronPhysicalDiskConfig>,
         should_remove_after_adding: BTreeSet<PhysicalDiskUuid>,
     ) {

sled-agent/types/Cargo.toml

@@ -13,6 +13,7 @@ async-trait.workspace = true
 bootstore.workspace = true
 camino.workspace = true
 chrono.workspace = true
+futures.workspace = true
 iddqd.workspace = true
 nexus-sled-agent-shared.workspace = true
 # Note: we're trying to avoid a dependency from sled-agent-types to nexus-types
@@ -28,16 +29,26 @@ serde.workspace = true
 serde_human_bytes.workspace = true
 serde_json.workspace = true
 sha3.workspace = true
+sled-hardware.workspace = true
 sled-hardware-types.workspace = true
+sled-storage.workspace = true
 slog.workspace = true
 slog-error-chain.workspace = true
 strum.workspace = true
 thiserror.workspace = true
+tokio.workspace = true
 toml.workspace = true
 tufaceous-artifact.workspace = true
 uuid.workspace = true
 
 [dev-dependencies]
+assert_matches.workspace = true
 camino-tempfile.workspace = true
 omicron-test-utils.workspace = true
+proptest.workspace = true
 rcgen.workspace = true
+sled-storage = { workspace = true, features = ["testing"] }
+test-strategy.workspace = true
+
+[features]
+testing = []