From 143564df9a37c3c08678fd89f106c3e075e3e92f Mon Sep 17 00:00:00 2001 From: Rain Date: Thu, 28 Aug 2025 03:41:59 +0000 Subject: [PATCH 1/2] [spr] changes to main this commit is based on Created using spr 1.3.6-beta.1 [skip ci] --- nexus-sled-agent-shared/src/inventory.rs | 104 ++++++++--- nexus/reconfigurator/planning/src/planner.rs | 187 +++++++++---------- nexus/reconfigurator/planning/src/system.rs | 52 +++++- 3 files changed, 212 insertions(+), 131 deletions(-) diff --git a/nexus-sled-agent-shared/src/inventory.rs b/nexus-sled-agent-shared/src/inventory.rs index 176106c12a7..2bdeee6b202 100644 --- a/nexus-sled-agent-shared/src/inventory.rs +++ b/nexus-sled-agent-shared/src/inventory.rs @@ -223,40 +223,52 @@ impl ConfigReconcilerInventory { datasets, orphaned_datasets: IdOrdMap::new(), zones, - boot_partitions: { - BootPartitionContents { - boot_disk: Ok(M2Slot::A), - slot_a: Ok(BootPartitionDetails { - header: BootImageHeader { - flags: 0, - data_size: 1000, - image_size: 1000, - target_size: 1000, - sha256: [0; 32], - image_name: "fake from debug_assume_success()" - .to_string(), - }, - artifact_hash: ArtifactHash([0x0a; 32]), - artifact_size: 1000, - }), - slot_b: Ok(BootPartitionDetails { - header: BootImageHeader { - flags: 0, - data_size: 1000, - image_size: 1000, - target_size: 1000, - sha256: [1; 32], - image_name: "fake from debug_assume_success()" - .to_string(), - }, - artifact_hash: ArtifactHash([0x0b; 32]), - artifact_size: 1000, - }), - } - }, + boot_partitions: BootPartitionContents::debug_assume_success(), remove_mupdate_override, } } + + /// Given a sled config, update an existing reconciler result to simulate an + /// output that sled-agent could have emitted if reconciliation succeeded. + /// + /// This method should only be used by tests and dev tools; real code should + /// look at the actual `last_reconciliation` value from the parent + /// [`Inventory`]. + pub fn debug_update_assume_success(&mut self, config: OmicronSledConfig) { + let external_disks = config + .disks + .iter() + .map(|d| (d.id, ConfigReconcilerInventoryResult::Ok)) + .collect(); + let datasets = config + .datasets + .iter() + .map(|d| (d.id, ConfigReconcilerInventoryResult::Ok)) + .collect(); + let zones = config + .zones + .iter() + .map(|z| (z.id, ConfigReconcilerInventoryResult::Ok)) + .collect(); + let remove_mupdate_override = + config.remove_mupdate_override.map(|_| { + RemoveMupdateOverrideInventory { + boot_disk_result: Ok( + RemoveMupdateOverrideBootSuccessInventory::Removed, + ), + non_boot_message: "mupdate override successfully removed \ + on non-boot disks" + .to_owned(), + } + }); + + self.last_reconciled_config = config; + self.external_disks = external_disks; + self.datasets = datasets; + self.orphaned_datasets = IdOrdMap::new(); + self.zones = zones; + self.remove_mupdate_override = remove_mupdate_override; + } } #[derive(Clone, Debug, PartialEq, Eq, Deserialize, JsonSchema, Serialize)] @@ -286,6 +298,36 @@ impl BootPartitionContents { M2Slot::B => &self.slot_b, } } + + pub fn debug_assume_success() -> Self { + Self { + boot_disk: Ok(M2Slot::A), + slot_a: Ok(BootPartitionDetails { + header: BootImageHeader { + flags: 0, + data_size: 1000, + image_size: 1000, + target_size: 1000, + sha256: [0; 32], + image_name: "fake from debug_assume_success()".to_string(), + }, + artifact_hash: ArtifactHash([0x0a; 32]), + artifact_size: 1000, + }), + slot_b: Ok(BootPartitionDetails { + header: BootImageHeader { + flags: 0, + data_size: 1000, + image_size: 1000, + target_size: 1000, + sha256: [1; 32], + image_name: "fake from debug_assume_success()".to_string(), + }, + artifact_hash: ArtifactHash([0x0b; 32]), + artifact_size: 1000, + }), + } + } } #[derive(Clone, Debug, PartialEq, Eq, Deserialize, JsonSchema, Serialize)] diff --git a/nexus/reconfigurator/planning/src/planner.rs b/nexus/reconfigurator/planning/src/planner.rs index 0731ba5350e..7a545b0a005 100644 --- a/nexus/reconfigurator/planning/src/planner.rs +++ b/nexus/reconfigurator/planning/src/planner.rs @@ -2142,7 +2142,6 @@ pub(crate) mod test { use nexus_types::deployment::OmicronZoneExternalSnatIp; use nexus_types::deployment::SledDisk; use nexus_types::deployment::TargetReleaseDescription; - use nexus_types::deployment::TufRepoPolicy; use nexus_types::deployment::blueprint_zone_type; use nexus_types::deployment::blueprint_zone_type::InternalDns; use nexus_types::external_api::views::PhysicalDiskState; @@ -5662,13 +5661,7 @@ pub(crate) mod test { )) ); - // This generation is successively incremented for each TUF repo. We use - // generation 2 to represent the first generation with a TUF repo - // attached. - let target_release_generation = Generation::from_u32(2); - // Manually specify a TUF repo with fake zone images. - let mut input_builder = example.input.clone().into_builder(); let version = ArtifactVersion::new_static("1.0.0-freeform") .expect("can't parse artifact version"); let fake_hash = ArtifactHash([0; 32]); @@ -5679,22 +5672,23 @@ pub(crate) mod test { hash: fake_hash, }; let artifacts = create_artifacts_at_version(&version); - let target_release_generation = target_release_generation.next(); - input_builder.policy_mut().tuf_repo = TufRepoPolicy { - target_release_generation, - description: TargetReleaseDescription::TufRepo( - TufRepoDescription { - repo: TufRepoMeta { - hash: fake_hash, - targets_role_version: 0, - valid_until: Utc::now(), - system_version: Version::new(1, 0, 0), - file_name: String::from(""), - }, - artifacts, + let description = + TargetReleaseDescription::TufRepo(TufRepoDescription { + repo: TufRepoMeta { + hash: fake_hash, + targets_role_version: 0, + valid_until: Utc::now(), + system_version: Version::new(1, 0, 0), + file_name: String::from(""), }, - ), - }; + artifacts, + }); + example.system.set_target_release_and_old_repo(description); + + let mut input_builder = example + .system + .to_planning_input_builder() + .expect("created PlanningInputBuilder"); // Some helper predicates for the assertions below. let is_old_nexus = |zone: &BlueprintZoneConfig| -> bool { @@ -5988,7 +5982,6 @@ pub(crate) mod test { // The planner should avoid doing this update until it has confirmation // from inventory that the cluster is healthy. - let mut input_builder = example.input.clone().into_builder(); let version = ArtifactVersion::new_static("1.0.0-freeform") .expect("can't parse artifact version"); let fake_hash = ArtifactHash([0; 32]); @@ -5999,23 +5992,24 @@ pub(crate) mod test { hash: fake_hash, }; let artifacts = create_artifacts_at_version(&version); - let target_release_generation = Generation::from_u32(2); - input_builder.policy_mut().tuf_repo = TufRepoPolicy { - target_release_generation, - description: TargetReleaseDescription::TufRepo( - TufRepoDescription { - repo: TufRepoMeta { - hash: fake_hash, - targets_role_version: 0, - valid_until: Utc::now(), - system_version: Version::new(1, 0, 0), - file_name: String::from(""), - }, - artifacts, + let description = + TargetReleaseDescription::TufRepo(TufRepoDescription { + repo: TufRepoMeta { + hash: fake_hash, + targets_role_version: 0, + valid_until: Utc::now(), + system_version: Version::new(1, 0, 0), + file_name: String::from(""), }, - ), - }; - example.input = input_builder.build(); + artifacts, + }); + example.system.set_target_release_and_old_repo(description); + + example.input = example + .system + .to_planning_input_builder() + .expect("created PlanningInputBuilder") + .build(); // Manually update all zones except Cockroach // @@ -6315,10 +6309,12 @@ pub(crate) mod test { ); // Use that boundary NTP zone to promote others. - let mut input_builder = example.input.clone().into_builder(); - input_builder.policy_mut().target_boundary_ntp_zone_count = - BOUNDARY_NTP_REDUNDANCY; - example.input = input_builder.build(); + example.system.target_boundary_ntp_zone_count(BOUNDARY_NTP_REDUNDANCY); + example.input = example + .system + .to_planning_input_builder() + .expect("created PlanningInputBuilder") + .build(); let blueprint_name = "blueprint_with_boundary_ntp"; let new_blueprint = Planner::new_based_on( log.clone(), @@ -6409,7 +6405,6 @@ pub(crate) mod test { // The planner should avoid doing this update until it has confirmation // from inventory that the cluster is healthy. - let mut input_builder = example.input.clone().into_builder(); let version = ArtifactVersion::new_static("1.0.0-freeform") .expect("can't parse artifact version"); let fake_hash = ArtifactHash([0; 32]); @@ -6420,23 +6415,24 @@ pub(crate) mod test { hash: fake_hash, }; let artifacts = create_artifacts_at_version(&version); - let target_release_generation = Generation::from_u32(2); - input_builder.policy_mut().tuf_repo = TufRepoPolicy { - target_release_generation, - description: TargetReleaseDescription::TufRepo( - TufRepoDescription { - repo: TufRepoMeta { - hash: fake_hash, - targets_role_version: 0, - valid_until: Utc::now(), - system_version: Version::new(1, 0, 0), - file_name: String::from(""), - }, - artifacts, + let description = + TargetReleaseDescription::TufRepo(TufRepoDescription { + repo: TufRepoMeta { + hash: fake_hash, + targets_role_version: 0, + valid_until: Utc::now(), + system_version: Version::new(1, 0, 0), + file_name: String::from(""), }, - ), - }; - example.input = input_builder.build(); + artifacts, + }); + example.system.set_target_release_and_old_repo(description); + + example.input = example + .system + .to_planning_input_builder() + .expect("created PlanningInputBuilder") + .build(); // Manually update all zones except boundary NTP // @@ -6837,7 +6833,6 @@ pub(crate) mod test { // The planner should avoid doing this update until it has confirmation // from inventory that the Internal DNS servers are ready. - let mut input_builder = example.input.clone().into_builder(); let version = ArtifactVersion::new_static("1.0.0-freeform") .expect("can't parse artifact version"); let fake_hash = ArtifactHash([0; 32]); @@ -6848,23 +6843,25 @@ pub(crate) mod test { hash: fake_hash, }; let artifacts = create_artifacts_at_version(&version); - let target_release_generation = Generation::from_u32(2); - input_builder.policy_mut().tuf_repo = TufRepoPolicy { - target_release_generation, - description: TargetReleaseDescription::TufRepo( - TufRepoDescription { - repo: TufRepoMeta { - hash: fake_hash, - targets_role_version: 0, - valid_until: Utc::now(), - system_version: Version::new(1, 0, 0), - file_name: String::from(""), - }, - artifacts, + + let description = + TargetReleaseDescription::TufRepo(TufRepoDescription { + repo: TufRepoMeta { + hash: fake_hash, + targets_role_version: 0, + valid_until: Utc::now(), + system_version: Version::new(1, 0, 0), + file_name: String::from(""), }, - ), - }; - example.input = input_builder.build(); + artifacts, + }); + example.system.set_target_release_and_old_repo(description); + + example.input = example + .system + .to_planning_input_builder() + .expect("created PlanningInputBuilder") + .build(); // Manually update all zones except Internal DNS // @@ -7101,7 +7098,6 @@ pub(crate) mod test { // Manually specify a TUF repo with fake images for all zones. // Only the name and kind of the artifacts matter. - let mut input_builder = example.input.clone().into_builder(); let version = ArtifactVersion::new_static("2.0.0-freeform") .expect("can't parse artifact version"); let fake_hash = ArtifactHash([0; 32]); @@ -7111,25 +7107,24 @@ pub(crate) mod test { }, hash: fake_hash, }; - // We use generation 2 to represent the first generation with a TUF repo - // attached. - let target_release_generation = Generation::new().next(); - let tuf_repo = TufRepoPolicy { - target_release_generation, - description: TargetReleaseDescription::TufRepo( - TufRepoDescription { - repo: TufRepoMeta { - hash: fake_hash, - targets_role_version: 0, - valid_until: Utc::now(), - system_version: Version::new(1, 0, 0), - file_name: String::from(""), - }, - artifacts: create_artifacts_at_version(&version), + + let description = + TargetReleaseDescription::TufRepo(TufRepoDescription { + repo: TufRepoMeta { + hash: fake_hash, + targets_role_version: 0, + valid_until: Utc::now(), + system_version: Version::new(1, 0, 0), + file_name: String::from(""), }, - ), - }; - input_builder.policy_mut().tuf_repo = tuf_repo; + artifacts: create_artifacts_at_version(&version), + }); + example.system.set_target_release_and_old_repo(description); + + let input_builder = example + .system + .to_planning_input_builder() + .expect("created PlanningInputBuilder"); let input = input_builder.build(); /// Expected number of planner iterations required to converge. diff --git a/nexus/reconfigurator/planning/src/system.rs b/nexus/reconfigurator/planning/src/system.rs index d9ee2cf64e2..c9ddaea938b 100644 --- a/nexus/reconfigurator/planning/src/system.rs +++ b/nexus/reconfigurator/planning/src/system.rs @@ -249,6 +249,18 @@ impl SystemDescription { self.target_nexus_zone_count } + pub fn target_boundary_ntp_zone_count( + &mut self, + count: usize, + ) -> &mut Self { + self.target_boundary_ntp_zone_count = count; + self + } + + pub fn get_target_boundary_ntp_zone_count(&self) -> usize { + self.target_boundary_ntp_zone_count + } + pub fn target_crucible_pantry_zone_count( &mut self, count: usize, @@ -461,8 +473,17 @@ impl SystemDescription { completed_at: Utc::now(), ran_for: Duration::from_secs(5), }; - sled.inventory_sled_agent.last_reconciliation = - Some(ConfigReconcilerInventory::debug_assume_success(sled_config)); + match sled.inventory_sled_agent.last_reconciliation.as_mut() { + Some(last_reconciliation) => { + last_reconciliation.debug_update_assume_success(sled_config); + } + None => { + sled.inventory_sled_agent.last_reconciliation = + Some(ConfigReconcilerInventory::debug_assume_success( + sled_config, + )); + } + }; Ok(self) } @@ -777,7 +798,7 @@ impl SystemDescription { description, }; - self.tuf_repo = new_repo; + let _old_repo = self.set_tuf_repo_inner(new_repo); // It's tempting to consider setting old_repo to the current tuf_repo, // but that requires the invariant that old_repo is always the current @@ -785,11 +806,34 @@ impl SystemDescription { // https://github.com/oxidecomputer/omicron/issues/8056 for some // discussion. // - // We may want a more explicit operation to set the old repo, though. + // We provide a method to set the old repo explicitly with these + // assumptions in mind: `set_target_release_and_old_repo`. self } + pub fn set_target_release_and_old_repo( + &mut self, + description: TargetReleaseDescription, + ) -> &mut Self { + let new_repo = TufRepoPolicy { + target_release_generation: self + .tuf_repo + .target_release_generation + .next(), + description, + }; + + let old_repo = self.set_tuf_repo_inner(new_repo); + self.old_repo = old_repo; + + self + } + + fn set_tuf_repo_inner(&mut self, new_repo: TufRepoPolicy) -> TufRepoPolicy { + mem::replace(&mut self.tuf_repo, new_repo) + } + pub fn set_ignore_impossible_mgs_updates_since( &mut self, since: DateTime, From 4debaf85682b576398c643a0814a8f211809e208 Mon Sep 17 00:00:00 2001 From: Rain Date: Fri, 19 Sep 2025 18:14:44 +0000 Subject: [PATCH 2/2] fix style Created using spr 1.3.6-beta.1 --- nexus/reconfigurator/planning/src/example.rs | 916 +++++++++---------- 1 file changed, 458 insertions(+), 458 deletions(-) diff --git a/nexus/reconfigurator/planning/src/example.rs b/nexus/reconfigurator/planning/src/example.rs index 34fae95be38..1f0b6c788d5 100644 --- a/nexus/reconfigurator/planning/src/example.rs +++ b/nexus/reconfigurator/planning/src/example.rs @@ -253,9 +253,9 @@ impl ExampleSystemBuilder { /// Currently, this value can be anywhere between 0 and 5. (More can be /// added in the future if necessary.) pub fn nsleds(mut self, nsleds: usize) -> Self { - self.sled_settings.resize(nsleds, BuilderSledSettings::default()); - self -} + self.sled_settings.resize(nsleds, BuilderSledSettings::default()); + self + } /// Set the number of disks per sled in the example system. /// @@ -264,9 +264,9 @@ impl ExampleSystemBuilder { /// /// If [`Self::create_zones`] is set to `false`, this is ignored. pub fn ndisks_per_sled(mut self, ndisks_per_sled: u8) -> Self { - self.ndisks_per_sled = ndisks_per_sled; - self -} + self.ndisks_per_sled = ndisks_per_sled; + self + } /// Set the number of Nexus instances in the example system. /// @@ -275,9 +275,9 @@ impl ExampleSystemBuilder { /// /// If [`Self::create_zones`] is set to `false`, this is ignored. pub fn nexus_count(mut self, nexus_count: usize) -> Self { - self.nexus_count = Some(ZoneCount(nexus_count)); - self -} + self.nexus_count = Some(ZoneCount(nexus_count)); + self + } /// Set the number of internal DNS instances in the example system. /// @@ -286,19 +286,19 @@ impl ExampleSystemBuilder { /// /// If [`Self::create_zones`] is set to `false`, this is ignored. pub fn internal_dns_count( - mut self, - internal_dns_count: usize, -) -> anyhow::Result { - if internal_dns_count > INTERNAL_DNS_REDUNDANCY { - anyhow::bail!( - "internal_dns_count {} is greater than INTERNAL_DNS_REDUNDANCY {}", - internal_dns_count, - INTERNAL_DNS_REDUNDANCY, - ); + mut self, + internal_dns_count: usize, + ) -> anyhow::Result { + if internal_dns_count > INTERNAL_DNS_REDUNDANCY { + anyhow::bail!( + "internal_dns_count {} is greater than INTERNAL_DNS_REDUNDANCY {}", + internal_dns_count, + INTERNAL_DNS_REDUNDANCY, + ); + } + self.internal_dns_count = ZoneCount(internal_dns_count); + Ok(self) } - self.internal_dns_count = ZoneCount(internal_dns_count); - Ok(self) -} /// Set the number of external DNS instances in the example system. /// @@ -308,37 +308,37 @@ impl ExampleSystemBuilder { /// /// Each DNS server is assigned an address in the 10.x.x.x range. pub fn external_dns_count( - mut self, - external_dns_count: usize, -) -> anyhow::Result { - if external_dns_count > 30 { - anyhow::bail!( - "external_dns_count {} is greater than 30", - external_dns_count, - ); + mut self, + external_dns_count: usize, + ) -> anyhow::Result { + if external_dns_count > 30 { + anyhow::bail!( + "external_dns_count {} is greater than 30", + external_dns_count, + ); + } + self.external_dns_count = ZoneCount(external_dns_count); + Ok(self) } - self.external_dns_count = ZoneCount(external_dns_count); - Ok(self) -} /// Set the number of Crucible pantry instances in the example system. /// /// If [`Self::create_zones`] is set to `false`, this is ignored. pub fn crucible_pantry_count( - mut self, - crucible_pantry_count: usize, -) -> Self { - self.crucible_pantry_count = ZoneCount(crucible_pantry_count); - self -} + mut self, + crucible_pantry_count: usize, + ) -> Self { + self.crucible_pantry_count = ZoneCount(crucible_pantry_count); + self + } /// Create zones in the example system. /// /// The default is `true`. pub fn create_zones(mut self, create_zones: bool) -> Self { - self.create_zones = create_zones; - self -} + self.create_zones = create_zones; + self + } /// Create disks in the blueprint. /// @@ -347,49 +347,49 @@ impl ExampleSystemBuilder { /// If [`Self::ndisks_per_sled`] is set to 0, then this is implied: if no /// disks are created, then the blueprint won't have any disks. pub fn create_disks_in_blueprint(mut self, create: bool) -> Self { - self.create_disks_in_blueprint = create; - self -} + self.create_disks_in_blueprint = create; + self + } /// Set the policy for a sled in the example system by index. /// /// Returns an error if `index >= nsleds`. pub fn with_sled_policy( - mut self, - index: usize, - policy: SledPolicy, -) -> anyhow::Result { - let Some(settings) = self.sled_settings.get_mut(index) else { - bail!( - "sled index {} out of range (0..{})", - index, - self.sled_settings.len(), - ); - }; - settings.policy = policy; - Ok(self) -} + mut self, + index: usize, + policy: SledPolicy, + ) -> anyhow::Result { + let Some(settings) = self.sled_settings.get_mut(index) else { + bail!( + "sled index {} out of range (0..{})", + index, + self.sled_settings.len(), + ); + }; + settings.policy = policy; + Ok(self) + } /// Set the target release to an initial `0.0.1` version, and image sources to /// Artifact corresponding to the release. pub fn with_target_release_0_0_1(self) -> anyhow::Result { - // Find the 0.0.1 release relative to this crate's root directory. - let root_dir = Utf8Path::new(env!("CARGO_MANIFEST_DIR")); - let manifest_path = root_dir - .parent() - .unwrap() - .parent() - .unwrap() - .parent() - .unwrap() - .join("update-common/manifests/fake-0.0.1.toml"); - self.with_target_release_manifest( - &manifest_path, - // allow_non_semver is false because fake-0.0.1.toml doesn't contain - // non-semver artifacts. - false, - ) -} + // Find the 0.0.1 release relative to this crate's root directory. + let root_dir = Utf8Path::new(env!("CARGO_MANIFEST_DIR")); + let manifest_path = root_dir + .parent() + .unwrap() + .parent() + .unwrap() + .parent() + .unwrap() + .join("update-common/manifests/fake-0.0.1.toml"); + self.with_target_release_manifest( + &manifest_path, + // allow_non_semver is false because fake-0.0.1.toml doesn't contain + // non-semver artifacts. + false, + ) + } pub fn with_target_release_manifest( mut self, @@ -426,417 +426,417 @@ impl ExampleSystemBuilder { /// /// Return the system, and the initial blueprint that matches it. pub fn build(&self) -> (ExampleSystem, Blueprint) { - let nexus_count = self.get_nexus_zones(); - - slog::debug!( - &self.log, - "Creating example system"; - "nsleds" => self.sled_settings.len(), - "ndisks_per_sled" => self.ndisks_per_sled, - "nexus_count" => nexus_count.0, - "internal_dns_count" => self.internal_dns_count.0, - "external_dns_count" => self.external_dns_count.0, - "crucible_pantry_count" => self.crucible_pantry_count.0, - "create_zones" => self.create_zones, - "create_disks_in_blueprint" => self.create_disks_in_blueprint, - ); - - let mut rng = self.rng.clone(); - - let mut system = SystemDescription::new(); - // Update the system's target counts with the counts. (Note that - // there's no external DNS count.) - system - .target_nexus_zone_count(nexus_count.0) - .target_internal_dns_zone_count(self.internal_dns_count.0) - .target_crucible_pantry_zone_count(self.crucible_pantry_count.0); - - // Set the target release if one is available. We don't do this - // unconditionally because we don't want the target release generation - // number to be incremented if self.target_release is - // TargetReleaseDescription::Initial. - if let TargetReleaseDescription::TufRepo(_) = &self.target_release { - system.set_target_release(self.target_release.clone()); - } + let nexus_count = self.get_nexus_zones(); + + slog::debug!( + &self.log, + "Creating example system"; + "nsleds" => self.sled_settings.len(), + "ndisks_per_sled" => self.ndisks_per_sled, + "nexus_count" => nexus_count.0, + "internal_dns_count" => self.internal_dns_count.0, + "external_dns_count" => self.external_dns_count.0, + "crucible_pantry_count" => self.crucible_pantry_count.0, + "create_zones" => self.create_zones, + "create_disks_in_blueprint" => self.create_disks_in_blueprint, + ); - let sled_ids_with_settings: Vec<_> = self - .sled_settings - .iter() - .map(|settings| (rng.sled_rng.next(), settings)) - .collect(); - - let artifacts_by_kind = if let TargetReleaseDescription::TufRepo(repo) = - &self.target_release - { - // Build a map of artifact versions by kind. For the artifacts we - // care about, we currently only expect a single version, so this - // map is expected to be unique. - // - // TODO-correctness Need to choose gimlet vs cosmo here! Can we use - // update-common's UpdatePlan instead of this jank? - // https://github.com/oxidecomputer/omicron/issues/8777 - let mut artifacts_by_kind = HashMap::new(); - for artifact in &repo.artifacts { - artifacts_by_kind.insert(&artifact.id.kind, artifact); + let mut rng = self.rng.clone(); + + let mut system = SystemDescription::new(); + // Update the system's target counts with the counts. (Note that + // there's no external DNS count.) + system + .target_nexus_zone_count(nexus_count.0) + .target_internal_dns_zone_count(self.internal_dns_count.0) + .target_crucible_pantry_zone_count(self.crucible_pantry_count.0); + + // Set the target release if one is available. We don't do this + // unconditionally because we don't want the target release generation + // number to be incremented if self.target_release is + // TargetReleaseDescription::Initial. + if let TargetReleaseDescription::TufRepo(_) = &self.target_release { + system.set_target_release(self.target_release.clone()); } - Some(artifacts_by_kind) - } else { - None - }; - - for (sled_id, settings) in &sled_ids_with_settings { - let _ = system - .sled( - SledBuilder::new() - .id(*sled_id) - .npools(self.ndisks_per_sled) - .policy(settings.policy), - ) - .unwrap(); - } - let mut input_builder = system - .to_planning_input_builder() - .expect("failed to make planning input builder"); - let base_input = input_builder.clone().build(); - - // Start with an empty blueprint containing only our sleds, no zones. - let initial_blueprint = BlueprintBuilder::build_empty_with_sleds_seeded( - base_input.all_sled_ids(SledFilter::Commissioned), - "test suite", - rng.blueprint1_rng, - ); - - // Start with an empty collection - let collection = system - .to_collection_builder() - .expect("failed to build collection") - .build(); - - // Now make a blueprint and collection with some zones on each sled. - let mut builder = BlueprintBuilder::new_based_on( - &self.log, - &initial_blueprint, - &base_input, - &collection, - "test suite", - rng.blueprint2_rng, - ) - .unwrap(); - - // Add as many external IPs as is necessary for external DNS zones. We - // pick addresses in the TEST-NET-2 (RFC 5737) range. - for i in 0..self.external_dns_count.0 { - builder - .inject_untracked_external_dns_ip(IpAddr::V4(Ipv4Addr::new( - 198, - 51, - 100, - (i + 1) - .try_into() - .expect("external_dns_count is always <= 30"), - ))) - .expect( - "this shouldn't error because provided external IPs \ - are all unique", - ); - } + let sled_ids_with_settings: Vec<_> = self + .sled_settings + .iter() + .map(|settings| (rng.sled_rng.next(), settings)) + .collect(); + + let artifacts_by_kind = if let TargetReleaseDescription::TufRepo(repo) = + &self.target_release + { + // Build a map of artifact versions by kind. For the artifacts we + // care about, we currently only expect a single version, so this + // map is expected to be unique. + // + // TODO-correctness Need to choose gimlet vs cosmo here! Can we use + // update-common's UpdatePlan instead of this jank? + // https://github.com/oxidecomputer/omicron/issues/8777 + let mut artifacts_by_kind = HashMap::new(); + for artifact in &repo.artifacts { + artifacts_by_kind.insert(&artifact.id.kind, artifact); + } + Some(artifacts_by_kind) + } else { + None + }; - let discretionary_sled_count = - base_input.all_sled_ids(SledFilter::Discretionary).count(); - - // * Create disks and non-discretionary zones on all sleds. - // * Only create discretionary zones on discretionary sleds. - let mut discretionary_ix = 0; - for (sled_id, sled_details) in - base_input.all_sleds(SledFilter::Commissioned) - { - if self.create_disks_in_blueprint { - let _ = builder - .sled_add_disks(sled_id, &sled_details.resources) - .unwrap(); - } - if self.create_zones { - let _ = builder - .sled_ensure_zone_ntp( - sled_id, - self.target_release - .zone_image_source(ZoneKind::BoundaryNtp) - .expect("obtained BoundaryNtp image source"), + for (sled_id, settings) in &sled_ids_with_settings { + let _ = system + .sled( + SledBuilder::new() + .id(*sled_id) + .npools(self.ndisks_per_sled) + .policy(settings.policy), ) .unwrap(); + } - // Create discretionary zones if allowed. - if sled_details.policy.matches(SledFilter::Discretionary) { - for _ in 0..nexus_count - .on(discretionary_ix, discretionary_sled_count) - { - builder - .sled_add_zone_nexus_with_config( - sled_id, - false, - vec![], - self.target_release - .zone_image_source(ZoneKind::Nexus) - .expect("obtained Nexus image source"), - initial_blueprint.nexus_generation, - ) - .unwrap(); - } - if discretionary_ix == 0 { - builder - .sled_add_zone_clickhouse( - sled_id, - self.target_release - .zone_image_source(ZoneKind::Clickhouse) - .expect("obtained Clickhouse image source"), - ) - .unwrap(); - } - for _ in 0..self - .internal_dns_count - .on(discretionary_ix, discretionary_sled_count) - { - builder - .sled_add_zone_internal_dns( - sled_id, - self.target_release - .zone_image_source(ZoneKind::InternalDns) - .expect( - "obtained InternalDNS image source", - ), - ) - .unwrap(); - } - for _ in 0..self - .external_dns_count - .on(discretionary_ix, discretionary_sled_count) - { - builder - .sled_add_zone_external_dns( - sled_id, - self.target_release - .zone_image_source(ZoneKind::ExternalDns) - .expect( - "obtained ExternalDNS image source", - ), - ) - .unwrap(); + let mut input_builder = system + .to_planning_input_builder() + .expect("failed to make planning input builder"); + let base_input = input_builder.clone().build(); + + // Start with an empty blueprint containing only our sleds, no zones. + let initial_blueprint = BlueprintBuilder::build_empty_with_sleds_seeded( + base_input.all_sled_ids(SledFilter::Commissioned), + "test suite", + rng.blueprint1_rng, + ); + + // Start with an empty collection + let collection = system + .to_collection_builder() + .expect("failed to build collection") + .build(); + + // Now make a blueprint and collection with some zones on each sled. + let mut builder = BlueprintBuilder::new_based_on( + &self.log, + &initial_blueprint, + &base_input, + &collection, + "test suite", + rng.blueprint2_rng, + ) + .unwrap(); + + // Add as many external IPs as is necessary for external DNS zones. We + // pick addresses in the TEST-NET-2 (RFC 5737) range. + for i in 0..self.external_dns_count.0 { + builder + .inject_untracked_external_dns_ip(IpAddr::V4(Ipv4Addr::new( + 198, + 51, + 100, + (i + 1) + .try_into() + .expect("external_dns_count is always <= 30"), + ))) + .expect( + "this shouldn't error because provided external IPs \ + are all unique", + ); + } + + let discretionary_sled_count = + base_input.all_sled_ids(SledFilter::Discretionary).count(); + + // * Create disks and non-discretionary zones on all sleds. + // * Only create discretionary zones on discretionary sleds. + let mut discretionary_ix = 0; + for (sled_id, sled_details) in + base_input.all_sleds(SledFilter::Commissioned) + { + if self.create_disks_in_blueprint { + let _ = builder + .sled_add_disks(sled_id, &sled_details.resources) + .unwrap(); + } + if self.create_zones { + let _ = builder + .sled_ensure_zone_ntp( + sled_id, + self.target_release + .zone_image_source(ZoneKind::BoundaryNtp) + .expect("obtained BoundaryNtp image source"), + ) + .unwrap(); + + // Create discretionary zones if allowed. + if sled_details.policy.matches(SledFilter::Discretionary) { + for _ in 0..nexus_count + .on(discretionary_ix, discretionary_sled_count) + { + builder + .sled_add_zone_nexus_with_config( + sled_id, + false, + vec![], + self.target_release + .zone_image_source(ZoneKind::Nexus) + .expect("obtained Nexus image source"), + initial_blueprint.nexus_generation, + ) + .unwrap(); + } + if discretionary_ix == 0 { + builder + .sled_add_zone_clickhouse( + sled_id, + self.target_release + .zone_image_source(ZoneKind::Clickhouse) + .expect("obtained Clickhouse image source"), + ) + .unwrap(); + } + for _ in 0..self + .internal_dns_count + .on(discretionary_ix, discretionary_sled_count) + { + builder + .sled_add_zone_internal_dns( + sled_id, + self.target_release + .zone_image_source(ZoneKind::InternalDns) + .expect( + "obtained InternalDNS image source", + ), + ) + .unwrap(); + } + for _ in 0..self + .external_dns_count + .on(discretionary_ix, discretionary_sled_count) + { + builder + .sled_add_zone_external_dns( + sled_id, + self.target_release + .zone_image_source(ZoneKind::ExternalDns) + .expect( + "obtained ExternalDNS image source", + ), + ) + .unwrap(); + } + for _ in 0..self + .crucible_pantry_count + .on(discretionary_ix, discretionary_sled_count) + { + builder + .sled_add_zone_crucible_pantry( + sled_id, + self.target_release + .zone_image_source(ZoneKind::CruciblePantry) + .expect( + "obtained CruciblePantry image source", + ), + ) + .unwrap(); + } + discretionary_ix += 1; } - for _ in 0..self - .crucible_pantry_count - .on(discretionary_ix, discretionary_sled_count) - { - builder - .sled_add_zone_crucible_pantry( + + for pool_name in sled_details.resources.zpools.keys() { + let _ = builder + .sled_ensure_zone_crucible( sled_id, + *pool_name, self.target_release - .zone_image_source(ZoneKind::CruciblePantry) - .expect( - "obtained CruciblePantry image source", - ), + .zone_image_source(ZoneKind::Crucible) + .expect("obtained Crucible image source"), ) .unwrap(); } - discretionary_ix += 1; } + builder.sled_ensure_zone_datasets(sled_id).unwrap(); - for pool_name in sled_details.resources.zpools.keys() { - let _ = builder - .sled_ensure_zone_crucible( + if let Some(artifacts_by_kind) = &artifacts_by_kind { + // Set the host phase 2 artifact version to Artifact to avoid a + // noop conversion in the first planning run. + let host_phase_2_artifact = + artifacts_by_kind.get(&ArtifactKind::HOST_PHASE_2).unwrap(); + + builder + .sled_set_host_phase_2( sled_id, - *pool_name, - self.target_release - .zone_image_source(ZoneKind::Crucible) - .expect("obtained Crucible image source"), + BlueprintHostPhase2DesiredSlots { + slot_a: + BlueprintHostPhase2DesiredContents::Artifact { + version: + BlueprintArtifactVersion::Available { + version: host_phase_2_artifact + .id + .version + .clone(), + }, + hash: host_phase_2_artifact.hash, + }, + slot_b: + BlueprintHostPhase2DesiredContents::Artifact { + version: + BlueprintArtifactVersion::Available { + version: host_phase_2_artifact + .id + .version + .clone(), + }, + hash: host_phase_2_artifact.hash, + }, + }, ) - .unwrap(); - } + .expect("sled is present in blueprint"); + }; } - builder.sled_ensure_zone_datasets(sled_id).unwrap(); - if let Some(artifacts_by_kind) = &artifacts_by_kind { - // Set the host phase 2 artifact version to Artifact to avoid a - // noop conversion in the first planning run. - let host_phase_2_artifact = - artifacts_by_kind.get(&ArtifactKind::HOST_PHASE_2).unwrap(); - - builder - .sled_set_host_phase_2( - sled_id, - BlueprintHostPhase2DesiredSlots { - slot_a: - BlueprintHostPhase2DesiredContents::Artifact { - version: - BlueprintArtifactVersion::Available { - version: host_phase_2_artifact - .id - .version - .clone(), - }, - hash: host_phase_2_artifact.hash, - }, - slot_b: - BlueprintHostPhase2DesiredContents::Artifact { - version: - BlueprintArtifactVersion::Available { - version: host_phase_2_artifact - .id - .version - .clone(), - }, - hash: host_phase_2_artifact.hash, + let blueprint = builder.build(); + for sled_cfg in blueprint.sleds.values() { + for zone in sled_cfg.zones.iter() { + let service_id = zone.id; + if let Some((external_ip, nic)) = + zone.zone_type.external_networking() + { + input_builder + .add_omicron_zone_external_ip(service_id, external_ip) + .expect("failed to add Omicron zone external IP"); + input_builder + .add_omicron_zone_nic( + service_id, + OmicronZoneNic { + // TODO-cleanup use `TypedUuid` everywhere + id: VnicUuid::from_untyped_uuid(nic.id), + mac: nic.mac, + ip: nic.ip, + slot: nic.slot, + primary: nic.primary, }, - }, - ) - .expect("sled is present in blueprint"); - }; - } - - let blueprint = builder.build(); - for sled_cfg in blueprint.sleds.values() { - for zone in sled_cfg.zones.iter() { - let service_id = zone.id; - if let Some((external_ip, nic)) = - zone.zone_type.external_networking() - { - input_builder - .add_omicron_zone_external_ip(service_id, external_ip) - .expect("failed to add Omicron zone external IP"); - input_builder - .add_omicron_zone_nic( - service_id, - OmicronZoneNic { - // TODO-cleanup use `TypedUuid` everywhere - id: VnicUuid::from_untyped_uuid(nic.id), - mac: nic.mac, - ip: nic.ip, - slot: nic.slot, - primary: nic.primary, - }, - ) - .expect("failed to add Omicron zone NIC"); + ) + .expect("failed to add Omicron zone NIC"); + } } } - } - for (sled_id, sled_cfg) in &blueprint.sleds { - let sled_cfg = sled_cfg.clone().into_in_service_sled_config(); - system.sled_set_omicron_config(*sled_id, sled_cfg).unwrap(); - } - - // We just ensured that a handful of datasets should exist in - // the blueprint, but they don't yet exist in the SystemDescription. - // - // Go back and add them so that the blueprint is consistent with - // inventory. - for (sled_id, sled_cfg) in &blueprint.sleds { - let sled = system.get_sled_mut(*sled_id).unwrap(); - - for dataset_config in sled_cfg.datasets.iter() { - let config = dataset_config.clone().try_into().unwrap(); - sled.add_synthetic_dataset(config); + for (sled_id, sled_cfg) in &blueprint.sleds { + let sled_cfg = sled_cfg.clone().into_in_service_sled_config(); + system.sled_set_omicron_config(*sled_id, sled_cfg).unwrap(); } - } - if let Some(artifacts_by_kind) = &artifacts_by_kind { - // Set all MGS and host phase 2 versions out of the TUF repo to - // ensure that the planner is quiesced at the time the initial - // system is returned. - let sp_version = artifacts_by_kind - .get(&ArtifactKind::from(KnownArtifactKind::GimletSp)) - .unwrap() - .id - .version - .clone(); - let rot_a_version = artifacts_by_kind - .get(&ArtifactKind::GIMLET_ROT_IMAGE_A) - .unwrap() - .id - .version - .clone(); - let rot_b_version = artifacts_by_kind - .get(&ArtifactKind::GIMLET_ROT_IMAGE_B) - .unwrap() - .id - .version - .clone(); - let host_phase_1_hash = artifacts_by_kind - .get(&ArtifactKind::HOST_PHASE_1) - .unwrap() - .hash; - let host_phase_2_hash = artifacts_by_kind - .get(&ArtifactKind::HOST_PHASE_2) - .unwrap() - .hash; - - for sled_id in blueprint.sleds.keys() { - system - .sled_update_sp_versions( - *sled_id, - Some(sp_version.clone()), - Some(ExpectedVersion::Version(sp_version.clone())), - ) - .expect("sled was just added to the system"); - system - .sled_update_rot_versions( - *sled_id, - RotStateOverrides { - active_slot_override: None, - slot_a_version_override: Some( - ExpectedVersion::Version(rot_a_version.clone()), - ), - slot_b_version_override: Some( - ExpectedVersion::Version(rot_b_version.clone()), - ), - persistent_boot_preference_override: None, - pending_persistent_boot_preference_override: None, - transient_boot_preference_override: None, - }, - ) - .expect("sled was just added to the system"); + // We just ensured that a handful of datasets should exist in + // the blueprint, but they don't yet exist in the SystemDescription. + // + // Go back and add them so that the blueprint is consistent with + // inventory. + for (sled_id, sled_cfg) in &blueprint.sleds { + let sled = system.get_sled_mut(*sled_id).unwrap(); + + for dataset_config in sled_cfg.datasets.iter() { + let config = dataset_config.clone().try_into().unwrap(); + sled.add_synthetic_dataset(config); + } + } - // TODO-correctness Need to choose gimlet vs cosmo here! Need help - // from tufaceous to tell us which is which. - // https://github.com/oxidecomputer/omicron/issues/8777 - system - .sled_update_host_phase_1_artifacts( - *sled_id, - None, - Some(host_phase_1_hash), - Some(host_phase_1_hash), - ) - .expect("sled was just added to the system"); - - // We must set host phase 2 artifacts after sled_set_omicron_config - // is called, because sled_set_omicron_config resets - // last_reconciliation state and wipes away host phase 2 artifact - // hashes. - system - .sled_update_host_phase_2_artifacts( - *sled_id, - None, - Some(host_phase_2_hash), - Some(host_phase_2_hash), - ) - .expect("sled was just added to the system"); + if let Some(artifacts_by_kind) = &artifacts_by_kind { + // Set all MGS and host phase 2 versions out of the TUF repo to + // ensure that the planner is quiesced at the time the initial + // system is returned. + let sp_version = artifacts_by_kind + .get(&ArtifactKind::from(KnownArtifactKind::GimletSp)) + .unwrap() + .id + .version + .clone(); + let rot_a_version = artifacts_by_kind + .get(&ArtifactKind::GIMLET_ROT_IMAGE_A) + .unwrap() + .id + .version + .clone(); + let rot_b_version = artifacts_by_kind + .get(&ArtifactKind::GIMLET_ROT_IMAGE_B) + .unwrap() + .id + .version + .clone(); + let host_phase_1_hash = artifacts_by_kind + .get(&ArtifactKind::HOST_PHASE_1) + .unwrap() + .hash; + let host_phase_2_hash = artifacts_by_kind + .get(&ArtifactKind::HOST_PHASE_2) + .unwrap() + .hash; + + for sled_id in blueprint.sleds.keys() { + system + .sled_update_sp_versions( + *sled_id, + Some(sp_version.clone()), + Some(ExpectedVersion::Version(sp_version.clone())), + ) + .expect("sled was just added to the system"); + system + .sled_update_rot_versions( + *sled_id, + RotStateOverrides { + active_slot_override: None, + slot_a_version_override: Some( + ExpectedVersion::Version(rot_a_version.clone()), + ), + slot_b_version_override: Some( + ExpectedVersion::Version(rot_b_version.clone()), + ), + persistent_boot_preference_override: None, + pending_persistent_boot_preference_override: None, + transient_boot_preference_override: None, + }, + ) + .expect("sled was just added to the system"); + + // TODO-correctness Need to choose gimlet vs cosmo here! Need help + // from tufaceous to tell us which is which. + // https://github.com/oxidecomputer/omicron/issues/8777 + system + .sled_update_host_phase_1_artifacts( + *sled_id, + None, + Some(host_phase_1_hash), + Some(host_phase_1_hash), + ) + .expect("sled was just added to the system"); + + // We must set host phase 2 artifacts after sled_set_omicron_config + // is called, because sled_set_omicron_config resets + // last_reconciliation state and wipes away host phase 2 artifact + // hashes. + system + .sled_update_host_phase_2_artifacts( + *sled_id, + None, + Some(host_phase_2_hash), + Some(host_phase_2_hash), + ) + .expect("sled was just added to the system"); + } } - } - let mut builder = - system.to_collection_builder().expect("failed to build collection"); - builder.set_rng(rng.collection_rng); - - // The blueprint evolves separately from the system -- so it's returned - // as a separate value. - let example = ExampleSystem { - system, - input: input_builder.build(), - collection: builder.build(), - initial_blueprint, - }; - (example, blueprint) -} + let mut builder = + system.to_collection_builder().expect("failed to build collection"); + builder.set_rng(rng.collection_rng); + + // The blueprint evolves separately from the system -- so it's returned + // as a separate value. + let example = ExampleSystem { + system, + input: input_builder.build(), + collection: builder.build(), + initial_blueprint, + }; + (example, blueprint) + } } /// Per-sled state.