Add VmmState::SagaUnwound

nexus/db-model/src/schema_versions.rs

@@ -17,7 +17,7 @@ use std::collections::BTreeMap;
 ///
 /// This must be updated when you change the database schema.  Refer to
 /// schema/crdb/README.adoc in the root of this repository for details.
-pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(70, 0, 0);
+pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(71, 0, 0);
 
 /// List of all past database schema versions, in *reverse* order
 ///
@@ -29,6 +29,7 @@ static KNOWN_VERSIONS: Lazy<Vec<KnownVersion>> = Lazy::new(|| {
         // |  leaving the first copy as an example for the next person.
         // v
         // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
+        KnownVersion::new(71, "add-saga-unwound-vmm-state"),
         KnownVersion::new(70, "separate-instance-and-vmm-states"),
         KnownVersion::new(69, "expose-stage0"),
         KnownVersion::new(68, "filter-v2p-mapping-by-instance-state"),

nexus/db-model/src/vmm_state.rs

@@ -24,6 +24,7 @@ impl_enum_type!(
     Migrating => b"migrating"
     Failed => b"failed"
     Destroyed => b"destroyed"
+    SagaUnwound => b"saga_unwound"
 );
 
 impl VmmState {
@@ -37,6 +38,7 @@ impl VmmState {
             VmmState::Migrating => "migrating",
             VmmState::Failed => "failed",
             VmmState::Destroyed => "destroyed",
+            VmmState::SagaUnwound => "saga_unwound",
         }
     }
 }
@@ -58,7 +60,7 @@ impl From<VmmState> for omicron_common::api::internal::nexus::VmmState {
             VmmState::Rebooting => Output::Rebooting,
             VmmState::Migrating => Output::Migrating,
             VmmState::Failed => Output::Failed,
-            VmmState::Destroyed => Output::Destroyed,
+            VmmState::Destroyed | VmmState::SagaUnwound => Output::Destroyed,
         }
     }
 }
@@ -74,7 +76,7 @@ impl From<VmmState> for sled_agent_client::types::VmmState {
             VmmState::Rebooting => Output::Rebooting,
             VmmState::Migrating => Output::Migrating,
             VmmState::Failed => Output::Failed,
-            VmmState::Destroyed => Output::Destroyed,
+            VmmState::Destroyed | VmmState::SagaUnwound => Output::Destroyed,
         }
     }
 }
@@ -104,7 +106,12 @@ impl From<VmmState> for omicron_common::api::external::InstanceState {
             VmmState::Starting => Output::Starting,
             VmmState::Running => Output::Running,
             VmmState::Stopping => Output::Stopping,
-            VmmState::Stopped => Output::Stopped,
+            // `SagaUnwound` should map to `Stopped` so that an `instance_view`
+            // API call that produces an instance with an unwound VMM will appear to
+            // be `Stopped`. This is because instances with unwound VMMs can
+            // be started by a subsequent instance-start saga, just like
+            // instances whose internal state actually is `Stopped`.
+            VmmState::Stopped | VmmState::SagaUnwound => Output::Stopped,
             VmmState::Rebooting => Output::Rebooting,
             VmmState::Migrating => Output::Migrating,
             VmmState::Failed => Output::Failed,

nexus/src/app/instance.rs

@@ -1664,8 +1664,8 @@ impl super::Nexus {
                         vmm.runtime.state,
                     )))
                 }
-                DbVmmState::Destroyed => Err(Error::invalid_request(
-                    "cannot connect to serial console of destroyed instance",
+                DbVmmState::Destroyed | DbVmmState::SagaUnwound => Err(Error::invalid_request(
+                    "cannot connect to serial console of instance in state \"Stopped\"",
                 )),
             }
         } else {

nexus/src/app/sagas/instance_common.rs

@@ -111,19 +111,22 @@ pub async fn create_and_insert_vmm_record(
     Ok(vmm)
 }
 
-/// Given a previously-inserted VMM record, set its state to Destroyed and then
-/// delete it.
+/// Given a previously-inserted VMM record, set its state to `SagaUnwound` and
+/// then delete it.
 ///
 /// This function succeeds idempotently if called with the same parameters,
 /// provided that the VMM record was not changed by some other actor after the
 /// calling saga inserted it.
-pub async fn destroy_vmm_record(
+///
+/// This function is intended to be called when a saga which created a VMM
+/// record unwinds.
+pub async fn unwind_vmm_record(
     datastore: &DataStore,
     opctx: &OpContext,
     prev_record: &db::model::Vmm,
 ) -> Result<(), anyhow::Error> {
     let new_runtime = db::model::VmmRuntimeState {
-        state: db::model::VmmState::Destroyed,
+        state: db::model::VmmState::SagaUnwound,
         time_state_updated: Utc::now(),
         gen: prev_record.runtime.gen.next().into(),
     };
@@ -252,7 +255,7 @@ pub async fn instance_ip_get_instance_state(
     // - starting: see below.
     match (found_instance_state, found_vmm_state) {
         // If there's no VMM, the instance is definitely not on any sled.
-        (InstanceState::NoVmm, _) => {
+        (InstanceState::NoVmm, _) | (_, Some(VmmState::SagaUnwound)) => {
             sled_id = None;
         }
 

nexus/src/app/sagas/instance_migrate.rs

@@ -235,7 +235,7 @@ async fn sim_destroy_vmm_record(
     info!(osagactx.log(), "destroying vmm record for migration unwind";
           "propolis_id" => %vmm.id);
 
-    super::instance_common::destroy_vmm_record(
+    super::instance_common::unwind_vmm_record(
         osagactx.datastore(),
         &opctx,
         &vmm,

nexus/src/app/sagas/instance_start.rs

@@ -200,7 +200,7 @@ async fn sis_destroy_vmm_record(
     );
 
     let vmm = sagactx.lookup::<db::model::Vmm>("vmm_record")?;
-    super::instance_common::destroy_vmm_record(
+    super::instance_common::unwind_vmm_record(
         osagactx.datastore(),
         &opctx,
         &vmm,

schema/crdb/add-saga-unwound-vmm-state/up.sql

@@ -0,0 +1,2 @@
+ALTER TYPE omicron.public.vmm_state
+    ADD VALUE IF NOT EXISTS 'saga_unwound' AFTER 'destroyed';

schema/crdb/dbinit.sql

@@ -982,7 +982,8 @@ CREATE TYPE IF NOT EXISTS omicron.public.vmm_state AS ENUM (
     'rebooting',
     'migrating',
     'failed',
-    'destroyed'
+    'destroyed',
+    'saga_unwound'
 );
 
 /*
@@ -4075,7 +4076,7 @@ INSERT INTO omicron.public.db_metadata (
     version,
     target_version
 ) VALUES
-    (TRUE, NOW(), NOW(), '70.0.0', NULL)
+    (TRUE, NOW(), NOW(), '71.0.0', NULL)
 ON CONFLICT DO NOTHING;
 
 COMMIT;