[spr] changes to main this commit is based on

Created using spr 1.3.6-beta.1

[skip ci]

Author: sunshowers

nexus/db-model/src/target_release.rs

@@ -64,11 +64,13 @@ impl TargetRelease {
     pub fn into_external(
         &self,
         release_source: views::TargetReleaseSource,
+        mupdate_override: bool,
     ) -> views::TargetRelease {
         views::TargetRelease {
             generation: (&self.generation.0).into(),
             time_requested: self.time_requested,
             release_source,
+            mupdate_override,
         }
     }
 }

nexus/db-queries/src/db/datastore/deployment.rs

@@ -1560,6 +1560,23 @@ impl DataStore {
         Self::blueprint_current_target_only(&conn).await.map_err(|e| e.into())
     }
 
+    /// Get the minimum generation for the current target blueprint, if one exists
+    pub async fn blueprint_target_get_current_min_gen(
+        &self,
+        opctx: &OpContext,
+    ) -> Result<Generation, Error> {
+        opctx.authorize(authz::Action::Read, &authz::BLUEPRINT_CONFIG).await?;
+        let conn = self.pool_connection_authorized(opctx).await?;
+        let target = Self::blueprint_current_target_only(&conn).await?;
+
+        let authz_blueprint = authz_blueprint_from_id(target.target_id);
+        Self::blueprint_get_minimum_generation_connection(
+            &authz_blueprint,
+            &conn,
+        )
+        .await
+    }
+
     // Helper to fetch the current blueprint target (without fetching the entire
     // blueprint for that target).
     //
@@ -1587,6 +1604,28 @@ impl DataStore {
 
         Ok(current_target.into())
     }
+
+    // Helper to fetch the minimum generation for a blueprint ID (without
+    // fetching the entire blueprint for that ID.)
+    async fn blueprint_get_minimum_generation_connection(
+        authz: &authz::Blueprint,
+        conn: &async_bb8_diesel::Connection<DbConnection>,
+    ) -> Result<Generation, Error> {
+        use nexus_db_schema::schema::blueprint::dsl;
+
+        let id = authz.id();
+        let db_blueprint = dsl::blueprint
+            .filter(dsl::id.eq(id))
+            .select(DbBlueprint::as_select())
+            .first_async::<DbBlueprint>(conn)
+            .await
+            .optional()
+            .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?;
+        let db_blueprint = db_blueprint.ok_or_else(|| {
+            Error::not_found_by_id(ResourceType::Blueprint, &id)
+        })?;
+        Ok(db_blueprint.target_release_minimum_generation.0)
+    }
 }
 
 // Helper to create an `authz::Blueprint` for a specific blueprint ID

nexus/db-queries/src/db/datastore/target_release.rs

@@ -125,7 +125,15 @@ impl DataStore {
                 }
             }
         };
-        Ok(target_release.into_external(release_source))
+        // We choose to fetch the blueprint directly from the database rather
+        // than relying on the cached blueprint in Nexus because our APIs try to
+        // be strongly consistent. This shows up/will show up as a warning in
+        // the UI, and we don't want the warning to flicker in and out of
+        // existence based on which Nexus is getting hit.
+        let min_gen = self.blueprint_target_get_current_min_gen(opctx).await?;
+        // The semantics of min_gen mean we use a > sign here, not >=.
+        let mupdate_override = min_gen > target_release.generation.0;
+        Ok(target_release.into_external(release_source, mupdate_override))
     }
 }
 
@@ -135,6 +143,12 @@ mod test {
     use crate::db::model::{Generation, TargetReleaseSource};
     use crate::db::pub_test_utils::TestDatabase;
     use chrono::{TimeDelta, Utc};
+    use nexus_inventory::now_db_precision;
+    use nexus_reconfigurator_planning::blueprint_builder::BlueprintBuilder;
+    use nexus_reconfigurator_planning::example::{
+        ExampleSystemBuilder, SimRngState,
+    };
+    use nexus_types::deployment::BlueprintTarget;
     use omicron_common::api::external::{
         TufArtifactMeta, TufRepoDescription, TufRepoMeta,
     };
@@ -145,7 +159,8 @@ mod test {
 
     #[tokio::test]
     async fn target_release_datastore() {
-        let logctx = dev::test_setup_log("target_release_datastore");
+        const TEST_NAME: &str = "target_release_datastore";
+        let logctx = dev::test_setup_log(TEST_NAME);
         let db = TestDatabase::new_with_datastore(&logctx.log).await;
         let (opctx, datastore) = (db.opctx(), db.datastore());
 
@@ -163,6 +178,56 @@ mod test {
         );
         assert!(initial_target_release.tuf_repo_id.is_none());
 
+        // Set up an initial blueprint and make it the target. This models real
+        // systems which always have a target blueprint.
+        let mut rng = SimRngState::from_seed(TEST_NAME);
+        let (system, mut blueprint) = ExampleSystemBuilder::new_with_rng(
+            &logctx.log,
+            rng.next_system_rng(),
+        )
+        .build();
+        assert_eq!(
+            blueprint.target_release_minimum_generation,
+            1.into(),
+            "initial blueprint should have minimum generation of 1",
+        );
+        // Treat this blueprint as the initial one for the system.
+        blueprint.parent_blueprint_id = None;
+
+        datastore
+            .blueprint_insert(&opctx, &blueprint)
+            .await
+            .expect("inserted blueprint");
+        datastore
+            .blueprint_target_set_current(
+                opctx,
+                BlueprintTarget {
+                    target_id: blueprint.id,
+                    // enabled = true or false shouldn't matter for this.
+                    enabled: true,
+                    time_made_target: now_db_precision(),
+                },
+            )
+            .await
+            .expect("set blueprint target");
+
+        // We should always be able to get a view of the target release.
+        let initial_target_release_view = datastore
+            .target_release_view(opctx, &initial_target_release)
+            .await
+            .expect("got target release");
+        eprintln!(
+            "initial target release view: {:#?}",
+            initial_target_release_view
+        );
+
+        // This target release should not have the mupdate override set, because
+        // the generation is <= the minimum generation in the target blueprint.
+        assert!(
+            !initial_target_release_view.mupdate_override,
+            "mupdate_override should be false for initial target release"
+        );
+
         // We should be able to set a new generation just like the first.
         // We allow some slack in the timestamp comparison because the
         // database only stores timestamps with μsec precision.
@@ -256,6 +321,86 @@ mod test {
         );
         assert_eq!(target_release.tuf_repo_id, Some(tuf_repo_id));
 
+        // Generate a new blueprint with a greater target release generation.
+        let mut builder = BlueprintBuilder::new_based_on(
+            &logctx.log,
+            &blueprint,
+            &system.input,
+            &system.collection,
+            TEST_NAME,
+        )
+        .expect("created blueprint builder");
+        builder.set_rng(rng.next_planner_rng());
+        builder
+            .set_target_release_minimum_generation(
+                blueprint.target_release_minimum_generation,
+                5.into(),
+            )
+            .expect("set target release minimum generation");
+        let bp2 = builder.build();
+
+        datastore
+            .blueprint_insert(&opctx, &bp2)
+            .await
+            .expect("inserted blueprint");
+        datastore
+            .blueprint_target_set_current(
+                opctx,
+                BlueprintTarget {
+                    target_id: bp2.id,
+                    // enabled = true or false shouldn't matter for this.
+                    enabled: true,
+                    time_made_target: now_db_precision(),
+                },
+            )
+            .await
+            .expect("set blueprint target");
+
+        // Fetch the target release again.
+        let target_release = datastore
+            .target_release_get_current(opctx)
+            .await
+            .expect("got target release");
+        let target_release_view_2 = datastore
+            .target_release_view(opctx, &target_release)
+            .await
+            .expect("got target release");
+
+        eprintln!("target release view 2: {target_release_view_2:#?}");
+
+        assert!(
+            target_release_view_2.mupdate_override,
+            "mupdate override is set",
+        );
+
+        // Now set the target release again -- this should cause the mupdate
+        // override to disappear.
+        let before = Utc::now();
+        let target_release = datastore
+            .target_release_insert(
+                opctx,
+                TargetRelease::new_system_version(&target_release, tuf_repo_id),
+            )
+            .await
+            .unwrap();
+        let after = Utc::now();
+
+        assert_eq!(target_release.generation, Generation(5.into()));
+        assert!(target_release.time_requested >= before);
+        assert!(target_release.time_requested <= after);
+
+        let target_release_view_3 = datastore
+            .target_release_view(opctx, &target_release)
+            .await
+            .expect("got target release");
+
+        eprintln!("target release view 3: {target_release_view_3:#?}");
+
+        assert!(
+            !target_release_view_3.mupdate_override,
+            "mupdate override is not set",
+        );
+
         // Clean up.
         db.terminate().await;
         logctx.cleanup_successful();

nexus/reconfigurator/planning/src/planner.rs

@@ -146,8 +146,8 @@ impl<'a> Planner<'a> {
 
     fn do_plan(&mut self) -> Result<(), Error> {
         self.do_plan_expunge()?;
-        self.do_plan_add()?;
         self.do_plan_decommission()?;
+        self.do_plan_add()?;
         if let UpdateStepResult::ContinueToNextStep = self.do_plan_mgs_updates()
         {
             self.do_plan_zone_updates()?;

nexus/types/src/external_api/views.rs

@@ -1488,6 +1488,17 @@ pub struct TargetRelease {
 
     /// The source of the target release.
     pub release_source: TargetReleaseSource,
+
+    /// If true, indicates that at least one sled in the system has been updated
+    /// through the recovery (MUPdate) path since the last time the target
+    /// release was set.
+    ///
+    /// In this case, the system will ignore the currently-set target release,
+    /// on the assumption that continuing an update may reintroduce or
+    /// exacerbate whatever problem caused the recovery path to be used. An
+    /// operator must set the target release again in order to resume automated
+    /// updates.
+    pub mupdate_override: bool,
 }
 
 fn expected_one_of<T: strum::VariantArray + fmt::Display>() -> String {

openapi/nexus.json

@@ -24802,6 +24802,10 @@
             "type": "integer",
             "format": "int64"
           },
+          "mupdate_override": {
+            "description": "If true, indicates that at least one sled in the system has been updated through the recovery (MUPdate) path since the last time the target release was set.\n\nIn this case, the system will ignore the currently-set target release, on the assumption that continuing an update may reintroduce or exacerbate whatever problem caused the recovery path to be used. An operator must set the target release again in order to resume automated updates.",
+            "type": "boolean"
+          },
           "release_source": {
             "description": "The source of the target release.",
             "allOf": [
@@ -24818,6 +24822,7 @@
         },
         "required": [
           "generation",
+          "mupdate_override",
           "release_source",
           "time_requested"
         ]