Blueprint PlanningInput: Fewer Options (#8470)

This gets rid of two `Option<_>`s in the planning input:

* `TufRepoPolicy::description` changes from `Option<TufRepoDescription>`
to non-optional `TargetReleaseDescription`. The latter is isomorphic to
`Option`, but spells the `None` case as
`TargetReleaseDescription::Initial`.
* `PlanningInput::old_repo` is now a non-optional `TufRepoPolicy`. In
the case where `tuf_repo` is "the initial policy" (i.e.,
`TargetReleaseDescription::Initial`, `old_repo` is _also_ set to the
initial policy. In all other cases, it's "the previous policy".

Author: jgallagher

dev-tools/reconfigurator-cli/src/lib.rs

@@ -23,7 +23,6 @@ use nexus_reconfigurator_planning::system::{SledBuilder, SystemDescription};
 use nexus_reconfigurator_simulation::SimStateBuilder;
 use nexus_reconfigurator_simulation::Simulator;
 use nexus_reconfigurator_simulation::{BlueprintId, SimState};
-use nexus_types::deployment::OmicronZoneNic;
 use nexus_types::deployment::PlanningInput;
 use nexus_types::deployment::SledFilter;
 use nexus_types::deployment::execution;
@@ -35,6 +34,7 @@ use nexus_types::deployment::{
     BlueprintZoneImageSource, PendingMgsUpdateDetails,
 };
 use nexus_types::deployment::{BlueprintZoneImageVersion, PendingMgsUpdate};
+use nexus_types::deployment::{OmicronZoneNic, TargetReleaseDescription};
 use nexus_types::external_api::views::SledPolicy;
 use nexus_types::external_api::views::SledProvisionPolicy;
 use omicron_common::address::REPO_DEPOT_PORT;
@@ -1673,7 +1673,7 @@ fn cmd_show(sim: &mut ReconfiguratorSim) -> anyhow::Result<Option<String>> {
 
     let target_release = state.system().description().target_release();
     match target_release.description() {
-        Some(tuf_desc) => {
+        TargetReleaseDescription::TufRepo(tuf_desc) => {
             swriteln!(
                 s,
                 "target release (generation {}): {} ({})",
@@ -1692,7 +1692,7 @@ fn cmd_show(sim: &mut ReconfiguratorSim) -> anyhow::Result<Option<String>> {
                 );
             }
         }
-        None => {
+        TargetReleaseDescription::Initial => {
             swriteln!(
                 s,
                 "target release (generation {}): unset",
@@ -1759,10 +1759,9 @@ fn cmd_set(
             })?;
             let description = artifacts_with_plan.description().clone();
             drop(artifacts_with_plan);
-            state
-                .system_mut()
-                .description_mut()
-                .set_target_release(Some(description));
+            state.system_mut().description_mut().set_target_release(
+                TargetReleaseDescription::TufRepo(description),
+            );
             format!("set target release based on {}", filename)
         }
     };

dev-tools/reconfigurator-cli/tests/input/target-release.txt

@@ -61,8 +61,12 @@ blueprint-plan df06bb57-ad42-4431-9206-abff322896c7 b1bda47d-2c19-4fba-96e3-d9df
 blueprint-diff df06bb57-ad42-4431-9206-abff322896c7 7f976e0d-d2a5-4eeb-9e82-c82bc2824aba
 
 # Finish updating the last sled and do one more planning run.
-# There should be nothing left to do.
+# This should update one control plane zone.
 sled-update-sp d81c6a84-79b8-4958-ae41-ea46c9b19763 --active 1.0.0
 inventory-generate
 blueprint-plan 7f976e0d-d2a5-4eeb-9e82-c82bc2824aba a71f7a73-35a6-45e8-acbe-f1c5925eed69
 blueprint-diff 7f976e0d-d2a5-4eeb-9e82-c82bc2824aba 9034c710-3e57-45f3-99e5-4316145e87ac
+
+# We should continue walking through the update. We need to build out a
+# reconfigurator-cli subcommand to simulate updated zone image sources (just
+# like we have sled-update-sp for simulated SP updates).

dev-tools/reconfigurator-cli/tests/output/target-release-stdout

@@ -1095,7 +1095,7 @@ impl ZoneKind {
             ZoneKind::BoundaryNtp => "ntp",
             ZoneKind::Clickhouse => "clickhouse",
             ZoneKind::ClickhouseKeeper => "clickhouse_keeper",
-            ZoneKind::ClickhouseServer => "clickhouse",
+            ZoneKind::ClickhouseServer => "clickhouse_server",
             ZoneKind::CockroachDb => "cockroachdb",
             ZoneKind::Crucible => "crucible-zone",
             ZoneKind::CruciblePantry => "crucible-pantry-zone",

nexus-sled-agent-shared/src/inventory.rs

@@ -1497,7 +1497,7 @@ mod test {
                 clickhouse_policy: None,
                 oximeter_read_policy: OximeterReadPolicy::new(1),
                 tuf_repo: TufRepoPolicy::initial(),
-                old_repo: None,
+                old_repo: TufRepoPolicy::initial(),
                 log,
             }
             .build()

nexus/reconfigurator/execution/src/dns.rs

@@ -45,6 +45,8 @@ use nexus_types::deployment::PendingMgsUpdates;
 use nexus_types::deployment::PlanningInput;
 use nexus_types::deployment::SledFilter;
 use nexus_types::deployment::SledResources;
+use nexus_types::deployment::TargetReleaseDescription;
+use nexus_types::deployment::TufRepoContentsError;
 use nexus_types::deployment::ZpoolFilter;
 use nexus_types::deployment::ZpoolName;
 use nexus_types::deployment::blueprint_zone_type;
@@ -56,7 +58,6 @@ use omicron_common::address::DNS_PORT;
 use omicron_common::address::NTP_PORT;
 use omicron_common::address::ReservedRackSubnet;
 use omicron_common::api::external::Generation;
-use omicron_common::api::external::TufRepoDescription;
 use omicron_common::api::external::Vni;
 use omicron_common::api::internal::shared::NetworkInterface;
 use omicron_common::api::internal::shared::NetworkInterfaceKind;
@@ -135,6 +136,8 @@ pub enum Error {
         expected: Generation,
         actual: Generation,
     },
+    #[error(transparent)]
+    TufRepoContentsError(#[from] TufRepoContentsError),
 }
 
 /// Describes the result of an idempotent "ensure" operation
@@ -1179,7 +1182,7 @@ impl<'a> BlueprintBuilder<'a> {
                 gz_address: dns_subnet.gz_address(),
                 gz_address_index,
             });
-        let image_source = self.zone_image_source(zone_type.kind());
+        let image_source = self.zone_image_source(zone_type.kind())?;
 
         let zone = BlueprintZoneConfig {
             disposition: BlueprintZoneDisposition::InService,
@@ -1232,7 +1235,7 @@ impl<'a> BlueprintBuilder<'a> {
                 dns_address,
                 nic,
             });
-        let image_source = self.zone_image_source(zone_type.kind());
+        let image_source = self.zone_image_source(zone_type.kind())?;
 
         let zone = BlueprintZoneConfig {
             disposition: BlueprintZoneDisposition::InService,
@@ -1272,7 +1275,7 @@ impl<'a> BlueprintBuilder<'a> {
             });
         let filesystem_pool =
             self.sled_select_zpool(sled_id, zone_type.kind())?;
-        let image_source = self.zone_image_source(zone_type.kind());
+        let image_source = self.zone_image_source(zone_type.kind())?;
 
         let zone = BlueprintZoneConfig {
             disposition: BlueprintZoneDisposition::InService,
@@ -1425,7 +1428,7 @@ impl<'a> BlueprintBuilder<'a> {
         });
         let filesystem_pool =
             self.sled_select_zpool(sled_id, zone_type.kind())?;
-        let image_source = self.zone_image_source(zone_type.kind());
+        let image_source = self.zone_image_source(zone_type.kind())?;
 
         let zone = BlueprintZoneConfig {
             disposition: BlueprintZoneDisposition::InService,
@@ -1451,7 +1454,7 @@ impl<'a> BlueprintBuilder<'a> {
             });
         let filesystem_pool =
             self.sled_select_zpool(sled_id, zone_type.kind())?;
-        let image_source = self.zone_image_source(zone_type.kind());
+        let image_source = self.zone_image_source(zone_type.kind())?;
 
         let zone = BlueprintZoneConfig {
             disposition: BlueprintZoneDisposition::InService,
@@ -1476,7 +1479,7 @@ impl<'a> BlueprintBuilder<'a> {
         );
         let filesystem_pool =
             self.sled_select_zpool(sled_id, zone_type.kind())?;
-        let image_source = self.zone_image_source(zone_type.kind());
+        let image_source = self.zone_image_source(zone_type.kind())?;
 
         let zone = BlueprintZoneConfig {
             disposition: BlueprintZoneDisposition::InService,
@@ -1511,7 +1514,7 @@ impl<'a> BlueprintBuilder<'a> {
                 dataset: OmicronZoneDataset { pool_name },
             });
         let filesystem_pool = pool_name;
-        let image_source = self.zone_image_source(zone_type.kind());
+        let image_source = self.zone_image_source(zone_type.kind())?;
 
         let zone = BlueprintZoneConfig {
             disposition: BlueprintZoneDisposition::InService,
@@ -1538,7 +1541,7 @@ impl<'a> BlueprintBuilder<'a> {
                 address,
                 dataset: OmicronZoneDataset { pool_name },
             });
-        let image_source = self.zone_image_source(zone_type.kind());
+        let image_source = self.zone_image_source(zone_type.kind())?;
 
         let zone = BlueprintZoneConfig {
             disposition: BlueprintZoneDisposition::InService,
@@ -1567,7 +1570,7 @@ impl<'a> BlueprintBuilder<'a> {
             },
         );
         let filesystem_pool = pool_name;
-        let image_source = self.zone_image_source(zone_type.kind());
+        let image_source = self.zone_image_source(zone_type.kind())?;
 
         let zone = BlueprintZoneConfig {
             disposition: BlueprintZoneDisposition::InService,
@@ -1596,7 +1599,7 @@ impl<'a> BlueprintBuilder<'a> {
             },
         );
         let filesystem_pool = pool_name;
-        let image_source = self.zone_image_source(zone_type.kind());
+        let image_source = self.zone_image_source(zone_type.kind())?;
 
         let zone = BlueprintZoneConfig {
             disposition: BlueprintZoneDisposition::InService,
@@ -1723,7 +1726,7 @@ impl<'a> BlueprintBuilder<'a> {
             });
         let filesystem_pool =
             self.sled_select_zpool(sled_id, zone_type.kind())?;
-        let image_source = self.zone_image_source(zone_type.kind());
+        let image_source = self.zone_image_source(zone_type.kind())?;
 
         self.sled_add_zone(
             sled_id,
@@ -1981,39 +1984,22 @@ impl<'a> BlueprintBuilder<'a> {
         self.pending_mgs_updates.remove(baseboard_id);
     }
 
-    fn zone_image_artifact(
-        repo: Option<&TufRepoDescription>,
-        zone_kind: ZoneKind,
-    ) -> BlueprintZoneImageSource {
-        repo.and_then(|repo| {
-            repo.artifacts
-                .iter()
-                .find(|artifact| {
-                    zone_kind.is_control_plane_zone_artifact(&artifact.id)
-                })
-                .map(BlueprintZoneImageSource::from_available_artifact)
-        })
-        .unwrap_or(BlueprintZoneImageSource::InstallDataset)
-    }
-
     /// Try to find an artifact in either the current or previous release repo
     /// that contains an image for a zone of the given kind; see RFD 565 §9.
     /// Defaults to the install dataset.
     pub(crate) fn zone_image_source(
         &self,
         zone_kind: ZoneKind,
-    ) -> BlueprintZoneImageSource {
+    ) -> Result<BlueprintZoneImageSource, TufRepoContentsError> {
         let new_repo = self.input.tuf_repo().description();
-        let old_repo =
-            self.input.old_repo().and_then(|repo| repo.description());
-        Self::zone_image_artifact(
-            if self.zone_is_ready_for_update(zone_kind, new_repo) {
-                new_repo
-            } else {
-                old_repo
-            },
-            zone_kind,
-        )
+        let old_repo = self.input.old_repo().description();
+        let repo_choice = if self.zone_is_ready_for_update(zone_kind, new_repo)
+        {
+            new_repo
+        } else {
+            old_repo
+        };
+        repo_choice.zone_image_source(zone_kind)
     }
 
     /// Return `true` iff a zone of the given kind is ready to be updated;
@@ -2022,7 +2008,7 @@ impl<'a> BlueprintBuilder<'a> {
     fn zone_is_ready_for_update(
         &self,
         zone_kind: ZoneKind,
-        new_repo: Option<&TufRepoDescription>,
+        new_repo: &TargetReleaseDescription,
     ) -> bool {
         match zone_kind {
             ZoneKind::Nexus => {
@@ -2035,8 +2021,15 @@ impl<'a> BlueprintBuilder<'a> {
                     )
                     .filter(|z| z.zone_type.kind() != ZoneKind::Nexus)
                     .all(|z| {
-                        z.image_source
-                            == Self::zone_image_artifact(new_repo, z.kind())
+                        // This comparison ignores any TUF repo contents errors
+                        // from `zone_image_source`. This means we'll never be
+                        // able to update Nexus if we can't tell if _other_ zone
+                        // types aren't updated, which seems correct.
+                        Some(&z.image_source)
+                            == new_repo
+                                .zone_image_source(z.kind())
+                                .ok()
+                                .as_ref()
                     })
                 })
             }