From 265068b4b1f726f4b6c8d1b986c4f4365cdef994 Mon Sep 17 00:00:00 2001
From: iximeow <iximeow@oxide.computer>
Date: Tue, 1 Oct 2024 18:27:47 +0000
Subject: [PATCH] Internal cleanup for boot disk/instance update

Greg and Eliza were both right in comments on #6585, but since these are
both fully internal I didn't want to add another CI round trip there :)
---
 nexus/db-model/src/instance.rs                | 11 ++++++++++-
 nexus/db-queries/src/db/datastore/instance.rs |  4 ++--
 nexus/src/app/instance.rs                     |  4 ++--
 nexus/src/app/sagas/instance_create.rs        | 11 ++++++++---
 4 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/nexus/db-model/src/instance.rs b/nexus/db-model/src/instance.rs
index 3bc9d3f993e..1a77df9276c 100644
--- a/nexus/db-model/src/instance.rs
+++ b/nexus/db-model/src/instance.rs
@@ -529,9 +529,18 @@ mod optional_time_delta {
 }
 
 /// The parts of an Instance that can be directly updated after creation.
+///
+/// The model's name `InstanceReconfigure` is in contrast to the external
+/// [`params::InstanceUpdate`]: `reconfigure` is the internal name of changing
+/// an instance's configuration in places it might conflict with the phrase
+/// "instance update". If we rename `instance update` (see
+/// https://github.com/oxidecomputer/omicron/issues/6631), then this could
+/// reasonably be renamed `InstanceUpdate` like other update models, with its
+/// user `instance_reconfigure` also renamed to `instance_update` like other
+/// update queries.
 #[derive(Clone, Debug, AsChangeset, Serialize, Deserialize)]
 #[diesel(table_name = instance, treat_none_as_null = true)]
-pub struct InstanceUpdate {
+pub struct InstanceReconfigure {
     #[diesel(column_name = boot_disk_id)]
     pub boot_disk_id: Option<Uuid>,
 }
diff --git a/nexus/db-queries/src/db/datastore/instance.rs b/nexus/db-queries/src/db/datastore/instance.rs
index e89cd8f234d..914eef19b7e 100644
--- a/nexus/db-queries/src/db/datastore/instance.rs
+++ b/nexus/db-queries/src/db/datastore/instance.rs
@@ -22,9 +22,9 @@ use crate::db::model::Generation;
 use crate::db::model::Instance;
 use crate::db::model::InstanceAutoRestart;
 use crate::db::model::InstanceAutoRestartPolicy;
+use crate::db::model::InstanceReconfigure;
 use crate::db::model::InstanceRuntimeState;
 use crate::db::model::InstanceState;
-use crate::db::model::InstanceUpdate;
 use crate::db::model::Migration;
 use crate::db::model::MigrationState;
 use crate::db::model::Name;
@@ -1009,7 +1009,7 @@ impl DataStore {
         &self,
         opctx: &OpContext,
         authz_instance: &authz::Instance,
-        update: InstanceUpdate,
+        update: InstanceReconfigure,
     ) -> Result<InstanceAndActiveVmm, Error> {
         opctx.authorize(authz::Action::Modify, authz_instance).await?;
 
diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs
index ca8c441a418..2e9b6f95d5d 100644
--- a/nexus/src/app/instance.rs
+++ b/nexus/src/app/instance.rs
@@ -19,7 +19,7 @@ use crate::external_api::params;
 use cancel_safe_futures::prelude::*;
 use futures::future::Fuse;
 use futures::{FutureExt, SinkExt, StreamExt};
-use nexus_db_model::InstanceUpdate;
+use nexus_db_model::InstanceReconfigure;
 use nexus_db_model::IpAttachState;
 use nexus_db_model::IpKind;
 use nexus_db_model::Vmm as DbVmm;
@@ -330,7 +330,7 @@ impl super::Nexus {
             None => None,
         };
 
-        let update = InstanceUpdate { boot_disk_id };
+        let update = InstanceReconfigure { boot_disk_id };
         self.datastore()
             .instance_reconfigure(opctx, &authz_instance, update)
             .await
diff --git a/nexus/src/app/sagas/instance_create.rs b/nexus/src/app/sagas/instance_create.rs
index c8680701b15..32ddd9f1e52 100644
--- a/nexus/src/app/sagas/instance_create.rs
+++ b/nexus/src/app/sagas/instance_create.rs
@@ -1077,8 +1077,9 @@ async fn sic_set_boot_disk(
         .await
         .map_err(ActionError::action_failed)?;
 
-    let initial_configuration =
-        nexus_db_model::InstanceUpdate { boot_disk_id: Some(authz_disk.id()) };
+    let initial_configuration = nexus_db_model::InstanceReconfigure {
+        boot_disk_id: Some(authz_disk.id()),
+    };
 
     datastore
         .instance_reconfigure(&opctx, &authz_instance, initial_configuration)
@@ -1109,8 +1110,12 @@ async fn sic_set_boot_disk_undo(
 
     // If there was a boot disk, clear it. If there was not a boot disk,
     // this is a no-op.
+    if params.create_params.boot_disk.is_none() {
+        return Ok(());
+    }
+
     let undo_configuration =
-        nexus_db_model::InstanceUpdate { boot_disk_id: None };
+        nexus_db_model::InstanceReconfigure { boot_disk_id: None };
 
     datastore
         .instance_reconfigure(&opctx, &authz_instance, undo_configuration)