[reconfigurator-planning] clear remove-mupdate-override from blueprint even if no target release is set (#9082)

Currently:

* if a target release is set, we go ahead and clear the
remove-mupdate-override instruction from blueprints, regardless of
whether artifacts match
* if no target release is set, we don't do that

This behavior is inconsistent. We shouldn't gate the mupdate override
part of the state machine on a target release not being set.

Author: sunshowers

dev-tools/reconfigurator-cli/tests/input/cmds-mupdate-override-without-target-release.txt

@@ -0,0 +1,24 @@
+# Test clearing the mupdate override if a target release isn't set.
+load-example --nsleds 1 --ndisks-per-sled 3
+
+# Create a TUF repository from a fake manifest. We don't set this as the
+# target release; rather, we just MUPdate to it (also setting the mupdate
+# override).
+tuf-assemble ../../update-common/manifests/fake.toml
+sled-update-install-dataset serial0 --from-repo repo-1.0.0.zip
+sled-set serial0 mupdate-override 6123eac1-ec5b-42ba-b73f-9845105a9971
+
+# Generate a new inventory and plan against that. This diff will have the
+# "will remove mupdate override" message within it.
+inventory-generate
+blueprint-plan latest latest
+blueprint-diff latest
+
+# Now apply the remove-mupdate-override instruction to the sled.
+sled-set serial0 mupdate-override unset
+
+# Plan again. This diff should show that the remove-mupdate-override instruction
+# was removed from the blueprint.
+inventory-generate
+blueprint-plan latest latest
+blueprint-diff latest

dev-tools/reconfigurator-cli/tests/output/cmds-add-zones-with-mupdate-override-stdout

@@ -194,7 +194,7 @@ to:   blueprint 8da82a8e-bf97-4fbd-8ddd-9f6462732cf1
 
 internal DNS:
   DNS zone: "control-plane.oxide.internal" (unchanged)
-    unchanged names: 51 (records: 65)
+    unchanged names: 52 (records: 68)
 
 external DNS:
   DNS zone: "oxide.example" (unchanged)
@@ -255,7 +255,7 @@ to:   blueprint 58d5e830-0884-47d8-a7cd-b2b3751adeb4
 
 internal DNS:
   DNS zone: "control-plane.oxide.internal" (unchanged)
-    unchanged names: 51 (records: 65)
+    unchanged names: 52 (records: 68)
 
 external DNS:
   DNS zone: "oxide.example" (unchanged)
@@ -386,6 +386,14 @@ internal DNS:
 * DNS zone: "control-plane.oxide.internal": 
 +   name: 571a8adf-f0b8-458c-8e6c-5a71e82af7ae.host          (records: 1)
 +       AAAA fd00:1122:3344:103::28
+*   name: _nexus-lockstep._tcp                               (records: 3 -> 4)
+-       SRV  port 12232 0c71b3b2-6ceb-4e8f-b020-b08675e83038.host.control-plane.oxide.internal
+-       SRV  port 12232 3eeb8d49-eb1a-43f8-bb64-c2338421c2c6.host.control-plane.oxide.internal
+-       SRV  port 12232 466a9f29-62bf-4e63-924a-b9efdb86afec.host.control-plane.oxide.internal
++       SRV  port 12232 0c71b3b2-6ceb-4e8f-b020-b08675e83038.host.control-plane.oxide.internal
++       SRV  port 12232 3eeb8d49-eb1a-43f8-bb64-c2338421c2c6.host.control-plane.oxide.internal
++       SRV  port 12232 466a9f29-62bf-4e63-924a-b9efdb86afec.host.control-plane.oxide.internal
++       SRV  port 12232 571a8adf-f0b8-458c-8e6c-5a71e82af7ae.host.control-plane.oxide.internal
 *   name: _nexus._tcp                                        (records: 3 -> 4)
 -       SRV  port 12221 0c71b3b2-6ceb-4e8f-b020-b08675e83038.host.control-plane.oxide.internal
 -       SRV  port 12221 3eeb8d49-eb1a-43f8-bb64-c2338421c2c6.host.control-plane.oxide.internal

dev-tools/reconfigurator-cli/tests/output/cmds-mupdate-override-without-target-release-stderr

@@ -15,7 +15,6 @@ use crate::blueprint_editor::NoAvailableDnsSubnets;
 use crate::blueprint_editor::SledEditError;
 use crate::blueprint_editor::SledEditor;
 use crate::mgs_updates::PendingHostPhase2Changes;
-use crate::planner::NoopConvertGlobalIneligibleReason;
 use crate::planner::NoopConvertInfo;
 use crate::planner::NoopConvertSledIneligibleReason;
 use crate::planner::ZoneExpungeReason;
@@ -2739,22 +2738,13 @@ impl IdOrdItem for EnsureMupdateOverrideUpdatedZone {
 /// though inventory no longer has the sled.
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub(crate) enum BpMupdateOverrideNotClearedReason {
-    /// There is a global reason noop conversions are not possible.
-    NoopGlobalIneligible(NoopConvertGlobalIneligibleReason),
-
     /// There is a sled-specific reason noop conversions are not possible.
     NoopSledIneligible(NoopConvertSledIneligibleReason),
 }
 
 impl fmt::Display for BpMupdateOverrideNotClearedReason {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            BpMupdateOverrideNotClearedReason::NoopGlobalIneligible(reason) => {
-                write!(
-                    f,
-                    "no sleds can be noop-converted to Artifact: {reason}",
-                )
-            }
             BpMupdateOverrideNotClearedReason::NoopSledIneligible(reason) => {
                 write!(
                     f,

dev-tools/reconfigurator-cli/tests/output/cmds-mupdate-override-without-target-release-stdout

@@ -9,6 +9,7 @@ use crate::blueprint_builder::EditedSledScalarEdits;
 use crate::blueprint_builder::EnsureMupdateOverrideAction;
 use crate::blueprint_builder::EnsureMupdateOverrideUpdatedZone;
 use crate::blueprint_builder::SledEditCounts;
+use crate::planner::NoopConvertGlobalIneligibleReason;
 use crate::planner::NoopConvertSledEligible;
 use crate::planner::NoopConvertSledIneligibleReason;
 use crate::planner::NoopConvertSledInfoMut;
@@ -980,10 +981,15 @@ impl ActiveSledEditor {
                             })
                         }
                     },
-                    NoopConvertSledInfoMut::GlobalIneligible(reason) => {
-                        Ok(EnsureMupdateOverrideAction::BpOverrideNotCleared {
-                            bp_override,
-                            reason: NoopGlobalIneligible(reason.clone()),
+                    NoopConvertSledInfoMut::GlobalIneligible(
+                        NoopConvertGlobalIneligibleReason::NoTargetRelease,
+                    ) => {
+                        // It's fine to clear the override when there's no
+                        // target release, even though noop conversions aren't
+                        // possible.
+                        self.set_remove_mupdate_override(None);
+                        Ok(EnsureMupdateOverrideAction::BpClearOverride {
+                            prev_bp_override: bp_override,
                         })
                     }
                 }

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

@@ -5222,6 +5222,10 @@
               "$ref": "#/components/schemas/PlanningAddSufficientZonesExist"
             }
           },
+          "target_release_generation_is_one": {
+            "description": "Set to true if the target release generation is 1, which would allow zones to be added.",
+            "type": "boolean"
+          },
           "waiting_on": {
             "nullable": true,
             "description": "What are we waiting on to start zone additions?",
@@ -5243,7 +5247,8 @@
           "sleds_waiting_for_ntp_zone",
           "sleds_without_ntp_zones_in_inventory",
           "sleds_without_zpools_for_ntp_zones",
-          "sufficient_zones_exist"
+          "sufficient_zones_exist",
+          "target_release_generation_is_one"
         ]
       },
       "PlanningAddSufficientZonesExist": {