Revert "[RSS] Initial blueprint: only set address for Crucible datasets (#7310)"

This reverts commit 0603f0b.

Author: davepacheco

Cargo.lock

@@ -112,7 +112,9 @@ impl DatasetName {
         &self.pool_name
     }
 
-    pub fn kind(&self) -> &DatasetKind {
+    // TODO(https://github.com/oxidecomputer/omicron/issues/7115): Rename
+    // this to "kind?
+    pub fn dataset(&self) -> &DatasetKind {
         &self.kind
     }
 

common/src/disk.rs

@@ -19,7 +19,6 @@ use omicron_uuid_kinds::SledUuid;
 use omicron_uuid_kinds::ZpoolUuid;
 use std::collections::BTreeSet;
 use std::net::IpAddr;
-use std::net::SocketAddrV6;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Note {
@@ -170,17 +169,6 @@ pub enum SledKind {
     OrphanedDataset { dataset: BlueprintDatasetConfig },
     /// A dataset claims to be on a zpool that does not exist.
     DatasetOnNonexistentZpool { dataset: BlueprintDatasetConfig },
-    /// A Crucible dataset does not have its `address` set to its corresponding
-    /// Crucible zone.
-    CrucibleDatasetWithIncorrectAddress {
-        dataset: BlueprintDatasetConfig,
-        expected_address: SocketAddrV6,
-    },
-    /// A non-Crucible dataset has an address.
-    NonCrucibleDatasetWithAddress {
-        dataset: BlueprintDatasetConfig,
-        address: SocketAddrV6,
-    },
 }
 
 impl fmt::Display for SledKind {
@@ -364,24 +352,6 @@ impl fmt::Display for SledKind {
                     dataset.kind, dataset.id, dataset.pool
                 )
             }
-            SledKind::CrucibleDatasetWithIncorrectAddress {
-                dataset,
-                expected_address,
-            } => {
-                write!(
-                    f,
-                    "Crucible dataset {} has bad address {:?} (expected {})",
-                    dataset.id, dataset.address, expected_address,
-                )
-            }
-            SledKind::NonCrucibleDatasetWithAddress { dataset, address } => {
-                write!(
-                    f,
-                    "non-Crucible dataset ({:?} {}) has an address: {} \
-                     (only Crucible datasets should have addresses)",
-                    dataset.kind, dataset.id, address,
-                )
-            }
         }
     }
 }

nexus/reconfigurator/blippy/src/blippy.rs

@@ -6,12 +6,10 @@ use crate::blippy::Blippy;
 use crate::blippy::Severity;
 use crate::blippy::SledKind;
 use nexus_sled_agent_shared::inventory::ZoneKind;
-use nexus_types::deployment::blueprint_zone_type;
 use nexus_types::deployment::BlueprintDatasetConfig;
 use nexus_types::deployment::BlueprintDatasetFilter;
 use nexus_types::deployment::BlueprintZoneConfig;
 use nexus_types::deployment::BlueprintZoneFilter;
-use nexus_types::deployment::BlueprintZoneType;
 use nexus_types::deployment::OmicronZoneExternalIp;
 use omicron_common::address::DnsSubnet;
 use omicron_common::address::Ipv6Subnet;
@@ -415,10 +413,6 @@ fn check_datasets(blippy: &mut Blippy<'_>) {
         }
     }
 
-    // In a check below, we want to look up Crucible zones by zpool; build that
-    // map as we perform the next set of checks.
-    let mut crucible_zone_by_zpool = BTreeMap::new();
-
     // There should be a dataset for every dataset referenced by a running zone
     // (filesystem or durable).
     for (sled_id, zone_config) in blippy
@@ -437,7 +431,7 @@ fn check_datasets(blippy: &mut Blippy<'_>) {
             Some(dataset) => {
                 match sled_datasets
                     .get(&dataset.pool().id())
-                    .and_then(|by_zpool| by_zpool.get(dataset.kind()))
+                    .and_then(|by_zpool| by_zpool.get(dataset.dataset()))
                 {
                     Some(dataset) => {
                         expected_datasets.insert(dataset.id);
@@ -477,21 +471,6 @@ fn check_datasets(blippy: &mut Blippy<'_>) {
                     );
                 }
             }
-
-            if dataset.kind == DatasetKind::Crucible {
-                match &zone_config.zone_type {
-                    BlueprintZoneType::Crucible(crucible_zone_config) => {
-                        crucible_zone_by_zpool.insert(
-                            dataset.dataset.pool_name.id(),
-                            crucible_zone_config,
-                        );
-                    }
-                    _ => unreachable!(
-                        "zone_type.durable_dataset() returned Crucible for \
-                         non-Crucible zone type"
-                    ),
-                }
-            }
         }
     }
 
@@ -554,50 +533,6 @@ fn check_datasets(blippy: &mut Blippy<'_>) {
             continue;
         }
     }
-
-    // All Crucible datasets should have their address set to the address of the
-    // Crucible zone on their same zpool, and all non-Crucible datasets should
-    // not have addresses.
-    for (sled_id, dataset) in blippy
-        .blueprint()
-        .all_omicron_datasets(BlueprintDatasetFilter::InService)
-    {
-        match dataset.kind {
-            DatasetKind::Crucible => {
-                let Some(blueprint_zone_type::Crucible { address, .. }) =
-                    crucible_zone_by_zpool.get(&dataset.pool.id())
-                else {
-                    // We already checked above that all datasets have
-                    // corresponding zones, so a failure to find the zone for
-                    // this dataset would have already produced a note; just
-                    // skip it.
-                    continue;
-                };
-                if dataset.address != Some(*address) {
-                    blippy.push_sled_note(
-                        sled_id,
-                        Severity::Fatal,
-                        SledKind::CrucibleDatasetWithIncorrectAddress {
-                            dataset: dataset.clone(),
-                            expected_address: *address,
-                        },
-                    );
-                }
-            }
-            _ => {
-                if let Some(address) = dataset.address {
-                    blippy.push_sled_note(
-                        sled_id,
-                        Severity::Fatal,
-                        SledKind::NonCrucibleDatasetWithAddress {
-                            dataset: dataset.clone(),
-                            address,
-                        },
-                    );
-                }
-            }
-        }
-    }
 }
 
 #[cfg(test)]
@@ -1359,7 +1294,7 @@ mod tests {
                     == durable_zone.zone_type.durable_dataset().unwrap().kind);
             let root_dataset = root_zone.filesystem_dataset().unwrap();
             let matches_root = (dataset.pool == *root_dataset.pool())
-                && (dataset.kind == *root_dataset.kind());
+                && (dataset.kind == *root_dataset.dataset());
             !matches_durable && !matches_root
         });
 
@@ -1530,7 +1465,7 @@ mod tests {
                 if (dataset.pool == durable_dataset.dataset.pool_name
                     && dataset.kind == durable_dataset.kind)
                     || (dataset.pool == *root_dataset.pool()
-                        && dataset.kind == *root_dataset.kind())
+                        && dataset.kind == *root_dataset.dataset())
                 {
                     Some(Note {
                         severity: Severity::Fatal,
@@ -1626,108 +1561,4 @@ mod tests {
 
         logctx.cleanup_successful();
     }
-
-    #[test]
-    fn test_dataset_with_bad_address() {
-        static TEST_NAME: &str = "test_dataset_with_bad_address";
-        let logctx = test_setup_log(TEST_NAME);
-        let (_, _, mut blueprint) = example(&logctx.log, TEST_NAME);
-
-        let crucible_addr_by_zpool = blueprint
-            .all_omicron_zones(BlueprintZoneFilter::ShouldBeRunning)
-            .filter_map(|(_, z)| match z.zone_type {
-                BlueprintZoneType::Crucible(
-                    blueprint_zone_type::Crucible { address, .. },
-                ) => {
-                    let zpool_id = z
-                        .zone_type
-                        .durable_zpool()
-                        .expect("crucible zone has durable zpool")
-                        .id();
-                    Some((zpool_id, address))
-                }
-                _ => None,
-            })
-            .collect::<BTreeMap<_, _>>();
-
-        // We have three ways a dataset address can be wrong:
-        //
-        // * A Crucible dataset has no address
-        // * A Crucible dataset has an address but it doesn't match its zone
-        // * A non-Crucible dataset has an address
-        //
-        // Make all three kinds of modifications to three different datasets and
-        // ensure we see all three noted.
-        let mut cleared_crucible_addr = false;
-        let mut changed_crucible_addr = false;
-        let mut set_non_crucible_addr = false;
-        let mut expected_notes = Vec::new();
-
-        for (sled_id, datasets_config) in
-            blueprint.blueprint_datasets.iter_mut()
-        {
-            for dataset in datasets_config.datasets.values_mut() {
-                match dataset.kind {
-                    DatasetKind::Crucible => {
-                        let bad_address = if !cleared_crucible_addr {
-                            cleared_crucible_addr = true;
-                            None
-                        } else if !changed_crucible_addr {
-                            changed_crucible_addr = true;
-                            Some("[1234:5678:9abc::]:0".parse().unwrap())
-                        } else {
-                            continue;
-                        };
-
-                        dataset.address = bad_address;
-                        let expected_address = *crucible_addr_by_zpool
-                            .get(&dataset.pool.id())
-                            .expect("found crucible zone for zpool");
-                        expected_notes.push(Note {
-                            severity: Severity::Fatal,
-                            kind: Kind::Sled {
-                                sled_id: *sled_id,
-                                kind: SledKind::CrucibleDatasetWithIncorrectAddress {
-                                    dataset: dataset.clone(),
-                                    expected_address,
-                                },
-                            },
-                        });
-                    }
-                    _ => {
-                        if !set_non_crucible_addr {
-                            set_non_crucible_addr = true;
-                            let address = "[::1]:0".parse().unwrap();
-                            dataset.address = Some(address);
-                            expected_notes.push(Note {
-                            severity: Severity::Fatal,
-                            kind: Kind::Sled {
-                                sled_id: *sled_id,
-                                kind: SledKind::NonCrucibleDatasetWithAddress {
-                                    dataset: dataset.clone(),
-                                    address,
-                                },
-                            },
-                        });
-                        }
-                    }
-                }
-            }
-        }
-
-        // We should have modified 3 datasets.
-        assert_eq!(expected_notes.len(), 3);
-
-        let report =
-            Blippy::new(&blueprint).into_report(BlippyReportSortKey::Kind);
-        eprintln!("{}", report.display());
-        for note in expected_notes {
-            assert!(
-                report.notes().contains(&note),
-                "did not find expected note {note:?}"
-            );
-        }
-
-        logctx.cleanup_successful();
-    }
 }

nexus/reconfigurator/blippy/src/checks.rs

@@ -2790,7 +2790,7 @@ pub mod test {
             {
                 for dataset in dataset_configs {
                     let id = dataset.id;
-                    let kind = dataset.name.kind();
+                    let kind = dataset.name.dataset();
                     let by_kind: &mut BTreeMap<_, _> =
                         input_dataset_ids.entry(*zpool_id).or_default();
                     let prev = by_kind.insert(kind.clone(), id);

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

@@ -460,7 +460,7 @@ impl ActiveSledEditor {
         }
 
         if let Some(dataset) = config.filesystem_dataset() {
-            self.datasets.expunge(&dataset.pool().id(), dataset.kind())?;
+            self.datasets.expunge(&dataset.pool().id(), dataset.dataset())?;
         }
         if let Some(dataset) = config.zone_type.durable_dataset() {
             self.datasets

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

@@ -70,7 +70,7 @@ impl DatasetIdsBackfillFromDb {
         let iter = resources.all_datasets(ZpoolFilter::InService).flat_map(
             |(&zpool_id, configs)| {
                 configs.iter().map(move |config| {
-                    (zpool_id, config.name.kind().clone(), config.id)
+                    (zpool_id, config.name.dataset().clone(), config.id)
                 })
             },
         );
@@ -162,7 +162,7 @@ impl PartialDatasetConfig {
 
     pub fn for_transient_zone(name: DatasetName) -> Self {
         assert!(
-            matches!(name.kind(), DatasetKind::TransientZone { .. }),
+            matches!(name.dataset(), DatasetKind::TransientZone { .. }),
             "for_transient_zone called with incorrect dataset kind: {name:?}"
         );
         Self {

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

@@ -119,7 +119,6 @@ guppy.workspace = true
 hex-literal.workspace = true
 http.workspace = true
 hyper.workspace = true
-nexus-reconfigurator-blippy.workspace = true
 omicron-test-utils.workspace = true
 pretty_assertions.workspace = true
 rcgen.workspace = true

sled-agent/Cargo.toml

@@ -453,7 +453,7 @@ pub(crate) fn filter_dataset_mountpoints(
     config
         .datasets
         .into_values()
-        .filter(|dataset| *dataset.name.kind() == DatasetKind::Update)
+        .filter(|dataset| *dataset.name.dataset() == DatasetKind::Update)
         .map(|dataset| dataset.name.mountpoint(root))
 }
 

sled-agent/src/artifact_store.rs

@@ -26,7 +26,7 @@ pub struct Plan {
 }
 
 impl Plan {
-    pub fn create(
+    pub async fn create(
         log: &Logger,
         config: &Config,
         bootstrap_addrs: BTreeSet<Ipv6Addr>,