[1/n] [reconfigurator-planning] move determination of add/update zones into a separate method (#8920)

I'd like to determine condition 4 of whether to perform updates (all
deployment units are at known versions) by looking at the blueprint
after noop conversions have been applied. We could either try and guess
what would happen by looking at the noop info, or just move this
determination to after noop updates have been applied.

Also make the planning report store the reasons zone adds and updates
are blocked, and print them as part of the planning report.

There are no behavior changes in this PR.

Author: sunshowers

dev-tools/reconfigurator-cli/tests/output/cmds-mupdate-update-flow-stdout

@@ -505,9 +505,12 @@ planning report for blueprint a5a8f242-ffa5-473c-8efd-2acf2dc0b736:
 chicken switches:
     add zones with mupdate override:   false
 
-* waiting on MUPdate overrides
-* MUPdate overrides exist
-* zone updates waiting on MUPdate overrides
+* zone adds waiting on blockers
+* zone adds and updates are blocked:
+  - current target release generation (2) is lower than minimum required by blueprint (3)
+  - sleds have remove mupdate override set in blueprint: 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6, d81c6a84-79b8-4958-ae41-ea46c9b19763
+  - sleds have mupdate override errors: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c
+* zone updates waiting on zone add blockers
 
 
 
@@ -880,9 +883,11 @@ planning report for blueprint 626487fa-7139-45ec-8416-902271fc730b:
 chicken switches:
     add zones with mupdate override:   false
 
-* waiting on MUPdate overrides
-* MUPdate overrides exist
-* zone updates waiting on MUPdate overrides
+* zone adds waiting on blockers
+* zone adds and updates are blocked:
+  - sleds have remove mupdate override set in blueprint: 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6, d81c6a84-79b8-4958-ae41-ea46c9b19763
+  - sleds have mupdate override errors: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c
+* zone updates waiting on zone add blockers
 
 
 > blueprint-diff latest
@@ -1104,9 +1109,12 @@ chicken switches:
     add zones with mupdate override:   false
 
 * noop converting 6/6 install-dataset zones to artifact store on sled 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6
-* waiting on MUPdate overrides
-* MUPdate overrides exist
-* zone updates waiting on MUPdate overrides
+* zone adds waiting on blockers
+* zone adds and updates are blocked:
+  - current target release generation (3) is lower than minimum required by blueprint (4)
+  - sleds have remove mupdate override set in blueprint: d81c6a84-79b8-4958-ae41-ea46c9b19763
+  - sleds have mupdate override errors: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c
+* zone updates waiting on zone add blockers
 
 
 
@@ -1393,9 +1401,11 @@ chicken switches:
     add zones with mupdate override:   false
 
 * skipping noop zone image source check on sled 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6: all 6 zones are already from artifacts
-* waiting on MUPdate overrides
-* MUPdate overrides exist
-* zone updates waiting on MUPdate overrides
+* zone adds waiting on blockers
+* zone adds and updates are blocked:
+  - current target release generation (3) is lower than minimum required by blueprint (4)
+  - sleds have mupdate override errors: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c
+* zone updates waiting on zone add blockers
 
 
 > blueprint-show latest
@@ -1568,9 +1578,11 @@ chicken switches:
     add zones with mupdate override:   false
 
 * skipping noop zone image source check on sled 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6: all 6 zones are already from artifacts
-* waiting on MUPdate overrides
-* MUPdate overrides exist
-* zone updates waiting on MUPdate overrides
+* zone adds waiting on blockers
+* zone adds and updates are blocked:
+  - current target release generation (3) is lower than minimum required by blueprint (4)
+  - sleds have mupdate override errors: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c
+* zone updates waiting on zone add blockers
 
 
 
@@ -1811,9 +1823,10 @@ chicken switches:
 
 * skipping noop zone image source check on sled 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6: all 6 zones are already from artifacts
 * noop converting 6/6 install-dataset zones to artifact store on sled d81c6a84-79b8-4958-ae41-ea46c9b19763
-* waiting on MUPdate overrides
-* MUPdate overrides exist
-* zone updates waiting on MUPdate overrides
+* zone adds waiting on blockers
+* zone adds and updates are blocked:
+  - sleds have mupdate override errors: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c
+* zone updates waiting on zone add blockers
 
 
 > blueprint-show latest
@@ -1987,9 +2000,10 @@ chicken switches:
 
 * skipping noop zone image source check on sled 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6: all 6 zones are already from artifacts
 * noop converting 6/6 install-dataset zones to artifact store on sled d81c6a84-79b8-4958-ae41-ea46c9b19763
-* waiting on MUPdate overrides
-* MUPdate overrides exist
-* zone updates waiting on MUPdate overrides
+* zone adds waiting on blockers
+* zone adds and updates are blocked:
+  - sleds have mupdate override errors: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c
+* zone updates waiting on zone add blockers
 
 
 
@@ -2962,9 +2976,11 @@ chicken switches:
 
 * skipping noop zone image source check on sled c3bc4c6d-fdde-4fc4-8493-89d2a1e5ee6b: all 0 zones are already from artifacts
 * skipping noop zone image source check on sled d81c6a84-79b8-4958-ae41-ea46c9b19763: all 6 zones are already from artifacts
-* waiting on MUPdate overrides
-* MUPdate overrides exist
-* zone updates waiting on MUPdate overrides
+* zone adds waiting on blockers
+* zone adds and updates are blocked:
+  - current target release generation (4) is lower than minimum required by blueprint (5)
+  - sleds have remove mupdate override set in blueprint: 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6
+* zone updates waiting on zone add blockers
 
 
 > blueprint-diff latest
@@ -3191,11 +3207,13 @@ generated blueprint 27e755bc-dc10-4647-853c-f89bb3a15a2c based on parent bluepri
 planning report for blueprint 27e755bc-dc10-4647-853c-f89bb3a15a2c:
 * skipping noop zone image source check on sled c3bc4c6d-fdde-4fc4-8493-89d2a1e5ee6b: all 0 zones are already from artifacts
 * skipping noop zone image source check on sled d81c6a84-79b8-4958-ae41-ea46c9b19763: all 6 zones are already from artifacts
-* MUPdate overrides exist
-* adding zones despite MUPdate override, as specified by the `add_zones_with_mupdate_override` chicken switch
+* zone adds and updates are blocked:
+  - current target release generation (4) is lower than minimum required by blueprint (5)
+  - sleds have remove mupdate override set in blueprint: 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6
+* adding zones despite being blocked, as specified by the `add_zones_with_mupdate_override` chicken switch
 * discretionary zone placement waiting for NTP zones on sleds: c3bc4c6d-fdde-4fc4-8493-89d2a1e5ee6b
 * missing NTP zone on sled c3bc4c6d-fdde-4fc4-8493-89d2a1e5ee6b
-* zone updates waiting on MUPdate overrides
+* zone updates waiting on zone add blockers
 
 
 > blueprint-diff latest

dev-tools/reconfigurator-cli/tests/output/cmds-noop-image-source-stdout

@@ -183,9 +183,10 @@ chicken switches:
 
 * noop converting 6/6 install-dataset zones to artifact store on sled 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6
 * noop converting 5/6 install-dataset zones to artifact store on sled aff6c093-197d-42c5-ad80-9f10ba051a34
-* waiting on MUPdate overrides
-* MUPdate overrides exist
-* zone updates waiting on MUPdate overrides
+* zone adds waiting on blockers
+* zone adds and updates are blocked:
+  - sleds have remove mupdate override set in blueprint: d81c6a84-79b8-4958-ae41-ea46c9b19763
+* zone updates waiting on zone add blockers
 
 
 
@@ -549,9 +550,10 @@ chicken switches:
 
 * skipping noop zone image source check on sled 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6: all 6 zones are already from artifacts
 * noop converting 2/2 install-dataset zones to artifact store on sled e96e226f-4ed9-4c01-91b9-69a9cd076c9e
-* waiting on MUPdate overrides
-* MUPdate overrides exist
-* zone updates waiting on MUPdate overrides
+* zone adds waiting on blockers
+* zone adds and updates are blocked:
+  - sleds have remove mupdate override set in blueprint: d81c6a84-79b8-4958-ae41-ea46c9b19763
+* zone updates waiting on zone add blockers
 
 
 

nexus/reconfigurator/planning/src/planner.rs

@@ -174,40 +174,40 @@ impl<'a> Planner<'a> {
 
         let mut noop_info =
             NoopConvertInfo::new(self.input, self.inventory, &self.blueprint)?;
-        let plan_mupdate_override_res =
-            self.do_plan_mupdate_override(&mut noop_info)?;
+        let actions_by_sled = self.do_plan_mupdate_override(&mut noop_info)?;
 
         // Log noop-convert results after do_plan_mupdate_override, because this
         // step might alter noop_info.
         noop_info.log_to(&self.log);
 
         // Within `do_plan_noop_image_source`, we plan noop image sources on
         // sleds other than those currently affected by mupdate overrides. This
-        // means that we don't have to wait for the `plan_mupdate_override_res`
-        // result for that step.
+        // means that we don't have to consider anything
+        // `do_plan_mupdate_override` does for this step.
         let noop_image_source = self.do_plan_noop_image_source(noop_info)?;
 
+        let add_update_blocked_reasons =
+            self.should_plan_add_or_update(&actions_by_sled)?;
+
         // Only plan MGS-based updates updates if there are no outstanding
         // MUPdate overrides.
-        let mgs_updates = if plan_mupdate_override_res.is_empty() {
+        let mgs_updates = if add_update_blocked_reasons.is_empty() {
             self.do_plan_mgs_updates()?
         } else {
             PlanningMgsUpdatesStepReport::new(PendingMgsUpdates::new())
         };
 
         // Likewise for zone additions, unless overridden with the chicken switch.
-        let has_mupdate_override = !plan_mupdate_override_res.is_empty();
         let add_zones_with_mupdate_override =
             self.input.chicken_switches().add_zones_with_mupdate_override;
-        let mut add =
-            if !has_mupdate_override || add_zones_with_mupdate_override {
-                self.do_plan_add(&mgs_updates)?
-            } else {
-                PlanningAddStepReport::waiting_on(
-                    ZoneAddWaitingOn::MupdateOverrides,
-                )
-            };
-        add.has_mupdate_override = has_mupdate_override;
+        let mut add = if add_update_blocked_reasons.is_empty()
+            || add_zones_with_mupdate_override
+        {
+            self.do_plan_add(&mgs_updates)?
+        } else {
+            PlanningAddStepReport::waiting_on(ZoneAddWaitingOn::Blockers)
+        };
+        add.add_update_blocked_reasons = add_update_blocked_reasons;
         add.add_zones_with_mupdate_override = add_zones_with_mupdate_override;
 
         let zone_updates = if add.any_discretionary_zones_placed() {
@@ -223,10 +223,10 @@ impl<'a> Planner<'a> {
             PlanningZoneUpdatesStepReport::waiting_on(
                 ZoneUpdatesWaitingOn::PendingMgsUpdates,
             )
-        } else if !plan_mupdate_override_res.is_empty() {
-            // ... or if there are pending MUPdate overrides.
+        } else if !add.add_update_blocked_reasons.is_empty() {
+            // ... or if there are pending zone add blockers.
             PlanningZoneUpdatesStepReport::waiting_on(
-                ZoneUpdatesWaitingOn::MupdateOverrides,
+                ZoneUpdatesWaitingOn::ZoneAddBlockers,
             )
         } else {
             self.do_plan_zone_updates(&mgs_updates)?
@@ -1528,10 +1528,12 @@ impl<'a> Planner<'a> {
         Ok(report)
     }
 
+    /// Perform planning for mupdate overrides, returning a map of sleds to
+    /// actions taken.
     fn do_plan_mupdate_override(
         &mut self,
         noop_info: &mut NoopConvertInfo,
-    ) -> Result<Vec<String>, Error> {
+    ) -> Result<BTreeMap<SledUuid, EnsureMupdateOverrideAction>, Error> {
         // For each sled, compare what's in the inventory to what's in the
         // blueprint.
         let mut actions_by_sled = BTreeMap::new();
@@ -1638,7 +1640,14 @@ impl<'a> Planner<'a> {
             }
         }
 
-        // Now we need to determine whether to also perform other actions like
+        Ok(actions_by_sled)
+    }
+
+    fn should_plan_add_or_update(
+        &self,
+        actions_by_sled: &BTreeMap<SledUuid, EnsureMupdateOverrideAction>,
+    ) -> Result<Vec<String>, Error> {
+        // We need to determine whether to also perform other actions like
         // updating or adding zones. We have to be careful here:
         //
         // * We may have moved existing zones with an Artifact source to using

nexus/types/src/deployment/planning_report.rs

@@ -12,6 +12,7 @@ use super::PendingMgsUpdates;
 use super::PlannerChickenSwitches;
 
 use daft::Diffable;
+use indent_write::fmt::IndentWriter;
 use omicron_common::policy::COCKROACHDB_REDUNDANCY;
 use omicron_uuid_kinds::BlueprintUuid;
 use omicron_uuid_kinds::MupdateOverrideUuid;
@@ -26,6 +27,7 @@ use serde::Serialize;
 use std::collections::BTreeMap;
 use std::collections::BTreeSet;
 use std::fmt;
+use std::fmt::Write;
 
 /// A full blueprint planning report. Other than the blueprint ID, each
 /// field corresponds to a step in the update planner, i.e., a subroutine
@@ -531,14 +533,15 @@ pub struct PlanningAddSufficientZonesExist {
 )]
 #[serde(rename_all = "snake_case", tag = "type")]
 pub enum ZoneAddWaitingOn {
-    /// Waiting on one or more MUPdate overrides to clear.
-    MupdateOverrides,
+    /// Waiting on one or more blockers (typically MUPdate-related reasons) to
+    /// clear.
+    Blockers,
 }
 
 impl ZoneAddWaitingOn {
     pub fn as_str(&self) -> &'static str {
         match self {
-            Self::MupdateOverrides => "MUPdate overrides",
+            Self::Blockers => "blockers",
         }
     }
 }
@@ -550,10 +553,14 @@ pub struct PlanningAddStepReport {
     /// What are we waiting on to start zone additions?
     pub waiting_on: Option<ZoneAddWaitingOn>,
 
-    /// Are there any outstanding MUPdate overrides?
-    pub has_mupdate_override: bool,
+    /// Reasons why zone adds and any updates are blocked.
+    ///
+    /// This is typically a list of MUPdate-related reasons.
+    pub add_update_blocked_reasons: Vec<String>,
 
-    /// The value of the homonymous chicken switch.
+    /// The value of the homonymous chicken switch. (What this really means is
+    /// that zone adds happen despite being blocked by one or more
+    /// MUPdate-related reasons.)
     pub add_zones_with_mupdate_override: bool,
 
     pub sleds_without_ntp_zones_in_inventory: BTreeSet<SledUuid>,
@@ -580,7 +587,7 @@ impl PlanningAddStepReport {
     pub fn new() -> Self {
         Self {
             waiting_on: None,
-            has_mupdate_override: true,
+            add_update_blocked_reasons: Vec::new(),
             add_zones_with_mupdate_override: false,
             sleds_without_ntp_zones_in_inventory: BTreeSet::new(),
             sleds_without_zpools_for_ntp_zones: BTreeSet::new(),
@@ -602,6 +609,7 @@ impl PlanningAddStepReport {
 
     pub fn is_empty(&self) -> bool {
         self.waiting_on.is_none()
+            && self.add_update_blocked_reasons.is_empty()
             && self.sleds_without_ntp_zones_in_inventory.is_empty()
             && self.sleds_without_zpools_for_ntp_zones.is_empty()
             && self.sleds_waiting_for_ntp_zone.is_empty()
@@ -664,10 +672,10 @@ impl PlanningAddStepReport {
 }
 
 impl fmt::Display for PlanningAddStepReport {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, mut f: &mut fmt::Formatter) -> fmt::Result {
         let Self {
             waiting_on,
-            has_mupdate_override,
+            add_update_blocked_reasons,
             add_zones_with_mupdate_override,
             sleds_without_ntp_zones_in_inventory,
             sleds_without_zpools_for_ntp_zones,
@@ -681,17 +689,27 @@ impl fmt::Display for PlanningAddStepReport {
         } = self;
 
         if let Some(waiting_on) = waiting_on {
-            writeln!(f, "* waiting on {}", waiting_on.as_str())?;
+            writeln!(f, "* zone adds waiting on {}", waiting_on.as_str())?;
         }
 
-        if *has_mupdate_override {
-            writeln!(f, "* MUPdate overrides exist")?;
+        if !add_update_blocked_reasons.is_empty() {
+            // If zone adds are blocked on a set of reasons, zone updates are
+            // blocked on the same reason. Make that clear by saying "zone adds
+            // and updates are blocked" rather than just "zone adds are
+            // blocked".
+            writeln!(f, "* zone adds and updates are blocked:")?;
+            for reason in add_update_blocked_reasons {
+                let mut indent_writer =
+                    IndentWriter::new_skip_initial("    ", f);
+                writeln!(indent_writer, "  - {}", reason)?;
+                f = indent_writer.into_inner();
+            }
         }
 
         if *add_zones_with_mupdate_override {
             writeln!(
                 f,
-                "* adding zones despite MUPdate override, \
+                "* adding zones despite being blocked, \
                    as specified by the `add_zones_with_mupdate_override` \
                    chicken switch"
             )?;
@@ -958,8 +976,8 @@ pub enum ZoneUpdatesWaitingOn {
     /// Waiting on updates to RoT / SP / Host OS / etc.
     PendingMgsUpdates,
 
-    /// Waiting on one or more MUPdate overrides to clear.
-    MupdateOverrides,
+    /// Waiting on the same set of blockers zone adds are waiting on.
+    ZoneAddBlockers,
 }
 
 impl ZoneUpdatesWaitingOn {
@@ -969,7 +987,7 @@ impl ZoneUpdatesWaitingOn {
             Self::PendingMgsUpdates => {
                 "pending MGS updates (RoT / SP / Host OS / etc.)"
             }
-            Self::MupdateOverrides => "MUPdate overrides",
+            Self::ZoneAddBlockers => "zone add blockers",
         }
     }
 }