pick up Propolis 50cb28f5

Cargo.toml

@@ -394,9 +394,9 @@ prettyplease = { version = "0.2.20", features = ["verbatim"] }
 proc-macro2 = "1.0"
 progenitor = { git = "https://github.com/oxidecomputer/progenitor", branch = "main" }
 progenitor-client = { git = "https://github.com/oxidecomputer/progenitor", branch = "main" }
-bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "6d7ed9a033babc054db9eff5b59dee978d2b0d76" }
-propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "6d7ed9a033babc054db9eff5b59dee978d2b0d76" }
-propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "6d7ed9a033babc054db9eff5b59dee978d2b0d76" }
+bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "50cb28f586083fdb990e401bc6146e7dac9b2753" }
+propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "50cb28f586083fdb990e401bc6146e7dac9b2753" }
+propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "50cb28f586083fdb990e401bc6146e7dac9b2753" }
 proptest = "1.4.0"
 quote = "1.0"
 rand = "0.8.5"

package-manifest.toml

@@ -532,10 +532,10 @@ service_name = "propolis-server"
 only_for_targets.image = "standard"
 source.type = "prebuilt"
 source.repo = "propolis"
-source.commit = "6d7ed9a033babc054db9eff5b59dee978d2b0d76"
+source.commit = "50cb28f586083fdb990e401bc6146e7dac9b2753"
 # The SHA256 digest is automatically posted to:
 # https://buildomat.eng.oxide.computer/public/file/oxidecomputer/propolis/image/<commit>/propolis-server.sha256.txt
-source.sha256 = "f8f41b47bc00811fefe2ba75e0f6f8ab77765776c04021e0b31f09c3b21108a9"
+source.sha256 = "864e74222d3e617f1bd7b7ba8d0e5cc18134dca121fc4339369620d1419c5bb0"
 output.type = "zone"
 
 [package.mg-ddm-gz]

sled-agent/src/common/instance.rs

@@ -117,50 +117,57 @@ impl ObservedPropolisState {
         instance_runtime: &InstanceRuntimeState,
         propolis_state: &InstanceStateMonitorResponse,
     ) -> Self {
-        let migration_status =
-            match (instance_runtime.migration_id, &propolis_state.migration) {
-                // If the runtime state and Propolis state agree that there's
-                // a migration in progress, and they agree on its ID, the
-                // Propolis migration state determines the migration status.
-                (Some(this_id), Some(propolis_migration))
-                    if this_id == propolis_migration.migration_id =>
-                {
-                    match propolis_migration.state {
-                        PropolisMigrationState::Finish => {
-                            ObservedMigrationStatus::Succeeded
-                        }
-                        PropolisMigrationState::Error => {
-                            ObservedMigrationStatus::Failed
-                        }
-                        _ => ObservedMigrationStatus::InProgress,
-                    }
-                }
-
-                // If both sides have a migration ID, but the IDs don't match,
-                // assume the instance's migration ID is newer. This can happen
-                // if Propolis was initialized via migration in and has not yet
-                // been told to migrate out.
-                (Some(_), Some(_)) => ObservedMigrationStatus::Pending,
-
-                // If only Propolis has a migration ID, assume it was from a
-                // prior migration in and report that no migration is in
-                // progress. This could be improved with propolis#508.
-                (None, Some(_)) => ObservedMigrationStatus::NoMigration,
-
-                // A migration source's migration IDs get set before its
-                // Propolis actually gets asked to migrate, so it's possible for
-                // the runtime state to contain an ID while the Propolis has
-                // none, in which case the migration is pending.
-                (Some(_), None) => ObservedMigrationStatus::Pending,
-
-                // If neither side has a migration ID, then there's clearly no
-                // migration.
-                (None, None) => ObservedMigrationStatus::NoMigration,
+        // If there's no migration currently registered with this sled, report
+        // the current state and that no migration is currently in progress,
+        // even if Propolis has some migration data to share. (This case arises
+        // when Propolis returns state from a previous migration that sled agent
+        // has already retired.)
+        //
+        // N.B. This needs to be read from the instance runtime state and not
+        //      the migration runtime state to ensure that, once a migration in
+        //      completes, the "completed" observation is reported to
+        //      `InstanceStates::apply_propolis_observation` exactly once.
+        //      Otherwise that routine will try to apply the "inbound migration
+        //      complete" instance state transition twice.
+        let Some(migration_id) = instance_runtime.migration_id else {
+            return Self {
+                vmm_state: PropolisInstanceState(propolis_state.state),
+                migration_status: ObservedMigrationStatus::NoMigration,
+                time: Utc::now(),
             };
+        };
+
+        // Sled agent believes a live migration may be in progress. See if
+        // either of the Propolis migrations corresponds to it.
+        let propolis_migration = match (
+            &propolis_state.migration.migration_in,
+            &propolis_state.migration.migration_out,
+        ) {
+            (Some(inbound), _) if inbound.id == migration_id => inbound,
+            (_, Some(outbound)) if outbound.id == migration_id => outbound,
+            _ => {
+                // Sled agent believes this instance should be migrating, but
+                // Propolis isn't reporting a matching migration yet, so assume
+                // the migration is still pending.
+                return Self {
+                    vmm_state: PropolisInstanceState(propolis_state.state),
+                    migration_status: ObservedMigrationStatus::Pending,
+                    time: Utc::now(),
+                };
+            }
+        };
 
         Self {
             vmm_state: PropolisInstanceState(propolis_state.state),
-            migration_status,
+            migration_status: match propolis_migration.state {
+                PropolisMigrationState::Finish => {
+                    ObservedMigrationStatus::Succeeded
+                }
+                PropolisMigrationState::Error => {
+                    ObservedMigrationStatus::Failed
+                }
+                _ => ObservedMigrationStatus::InProgress,
+            },
             time: Utc::now(),
         }
     }

sled-agent/src/sim/instance.rs

@@ -19,7 +19,8 @@ use omicron_common::api::internal::nexus::{
     InstanceRuntimeState, MigrationRole, SledInstanceState, VmmState,
 };
 use propolis_client::types::{
-    InstanceMigrateStatusResponse as PropolisMigrateStatus,
+    InstanceMigrateStatusResponse as PropolisMigrateResponse,
+    InstanceMigrationStatus as PropolisMigrationStatus,
     InstanceState as PropolisInstanceState, InstanceStateMonitorResponse,
 };
 use std::collections::VecDeque;
@@ -32,7 +33,7 @@ use crate::common::instance::{Action as InstanceAction, InstanceStates};
 #[derive(Clone, Debug)]
 enum MonitorChange {
     PropolisState(PropolisInstanceState),
-    MigrateStatus(PropolisMigrateStatus),
+    MigrateStatus(PropolisMigrateResponse),
 }
 
 /// A simulation of an Instance created by the external Oxide API.
@@ -70,10 +71,10 @@ impl SimInstanceInner {
         self.queue.push_back(MonitorChange::PropolisState(propolis_state));
     }
 
-    /// Pushes a Propolis migration status to the state change queue.
-    fn queue_migration_status(
+    /// Pushes a Propolis migration update to the state change queue.
+    fn queue_migration_update(
         &mut self,
-        migrate_status: PropolisMigrateStatus,
+        migrate_status: PropolisMigrateResponse,
     ) {
         self.queue.push_back(MonitorChange::MigrateStatus(migrate_status))
     }
@@ -92,22 +93,42 @@ impl SimInstanceInner {
                     self
                 )
             });
-        self.queue_migration_status(PropolisMigrateStatus {
-            migration_id,
-            state: propolis_client::types::MigrationState::Sync,
-        });
-        self.queue_migration_status(PropolisMigrateStatus {
-            migration_id,
-            state: propolis_client::types::MigrationState::Finish,
-        });
-
-        // The state we transition to after the migration completes will depend
-        // on whether we are the source or destination.
+
         match role {
+            MigrationRole::Source => {
+                self.queue_migration_update(PropolisMigrateResponse {
+                    migration_in: None,
+                    migration_out: Some(PropolisMigrationStatus {
+                        id: migration_id,
+                        state: propolis_client::types::MigrationState::Sync,
+                    }),
+                });
+                self.queue_migration_update(PropolisMigrateResponse {
+                    migration_in: None,
+                    migration_out: Some(PropolisMigrationStatus {
+                        id: migration_id,
+                        state: propolis_client::types::MigrationState::Finish,
+                    }),
+                });
+                self.queue_graceful_stop();
+            }
             MigrationRole::Target => {
+                self.queue_migration_update(PropolisMigrateResponse {
+                    migration_in: Some(PropolisMigrationStatus {
+                        id: migration_id,
+                        state: propolis_client::types::MigrationState::Sync,
+                    }),
+                    migration_out: None,
+                });
+                self.queue_migration_update(PropolisMigrateResponse {
+                    migration_in: Some(PropolisMigrationStatus {
+                        id: migration_id,
+                        state: propolis_client::types::MigrationState::Finish,
+                    }),
+                    migration_out: None,
+                });
                 self.queue_propolis_state(PropolisInstanceState::Running)
             }
-            MigrationRole::Source => self.queue_graceful_stop(),
         }
     }
 
@@ -252,12 +273,12 @@ impl SimInstanceInner {
                     self.last_response.state = state;
                 }
                 MonitorChange::MigrateStatus(status) => {
-                    self.last_response.migration = Some(status);
+                    self.last_response.migration = status;
                 }
             }
 
             self.state.apply_propolis_observation(&ObservedPropolisState::new(
-                &self.state.instance(),
+                self.state.instance(),
                 &self.last_response,
             ))
         } else {
@@ -450,7 +471,10 @@ impl Simulatable for SimInstance {
                 last_response: InstanceStateMonitorResponse {
                     gen: 1,
                     state: PropolisInstanceState::Starting,
-                    migration: None,
+                    migration: PropolisMigrateResponse {
+                        migration_in: None,
+                        migration_out: None,
+                    },
                 },
                 queue: VecDeque::new(),
                 destroyed: false,