diff --git a/Cargo.lock b/Cargo.lock index 5949144ab5..8e1d3ca90c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -464,9 +464,9 @@ dependencies = [ [[package]] name = "bhyve_api" version = "0.0.0" -source = "git+https://github.com/oxidecomputer/propolis?rev=6d7ed9a033babc054db9eff5b59dee978d2b0d76#6d7ed9a033babc054db9eff5b59dee978d2b0d76" +source = "git+https://github.com/oxidecomputer/propolis?rev=50cb28f586083fdb990e401bc6146e7dac9b2753#50cb28f586083fdb990e401bc6146e7dac9b2753" dependencies = [ - "bhyve_api_sys 0.0.0 (git+https://github.com/oxidecomputer/propolis?rev=6d7ed9a033babc054db9eff5b59dee978d2b0d76)", + "bhyve_api_sys 0.0.0 (git+https://github.com/oxidecomputer/propolis?rev=50cb28f586083fdb990e401bc6146e7dac9b2753)", "libc", "strum", ] @@ -484,7 +484,7 @@ dependencies = [ [[package]] name = "bhyve_api_sys" version = "0.0.0" -source = "git+https://github.com/oxidecomputer/propolis?rev=6d7ed9a033babc054db9eff5b59dee978d2b0d76#6d7ed9a033babc054db9eff5b59dee978d2b0d76" +source = "git+https://github.com/oxidecomputer/propolis?rev=50cb28f586083fdb990e401bc6146e7dac9b2753#50cb28f586083fdb990e401bc6146e7dac9b2753" dependencies = [ "libc", "strum", @@ -3471,7 +3471,7 @@ version = "0.1.0" dependencies = [ "anyhow", "async-trait", - "bhyve_api 0.0.0 (git+https://github.com/oxidecomputer/propolis?rev=6d7ed9a033babc054db9eff5b59dee978d2b0d76)", + "bhyve_api 0.0.0 (git+https://github.com/oxidecomputer/propolis?rev=50cb28f586083fdb990e401bc6146e7dac9b2753)", "byteorder", "camino", "camino-tempfile", @@ -5526,7 +5526,7 @@ dependencies = [ "pq-sys", "pretty_assertions", "progenitor-client", - "propolis-client 0.1.0 (git+https://github.com/oxidecomputer/propolis?rev=6d7ed9a033babc054db9eff5b59dee978d2b0d76)", + "propolis-client 0.1.0 (git+https://github.com/oxidecomputer/propolis?rev=50cb28f586083fdb990e401bc6146e7dac9b2753)", "rand 0.8.5", "rcgen", "ref-cast", @@ -5776,7 +5776,7 @@ dependencies = [ "oximeter-producer", "oxnet", "pretty_assertions", - "propolis-client 0.1.0 (git+https://github.com/oxidecomputer/propolis?rev=6d7ed9a033babc054db9eff5b59dee978d2b0d76)", + "propolis-client 0.1.0 (git+https://github.com/oxidecomputer/propolis?rev=50cb28f586083fdb990e401bc6146e7dac9b2753)", "propolis-mock-server", "rand 0.8.5", "rcgen", @@ -7207,7 +7207,7 @@ dependencies = [ [[package]] name = "propolis-client" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/propolis?rev=6d7ed9a033babc054db9eff5b59dee978d2b0d76#6d7ed9a033babc054db9eff5b59dee978d2b0d76" +source = "git+https://github.com/oxidecomputer/propolis?rev=50cb28f586083fdb990e401bc6146e7dac9b2753#50cb28f586083fdb990e401bc6146e7dac9b2753" dependencies = [ "async-trait", "base64 0.21.7", @@ -7221,7 +7221,7 @@ dependencies = [ "slog", "thiserror", "tokio", - "tokio-tungstenite 0.20.1", + "tokio-tungstenite 0.21.0", "uuid", ] @@ -7249,7 +7249,7 @@ dependencies = [ [[package]] name = "propolis-mock-server" version = "0.0.0" -source = "git+https://github.com/oxidecomputer/propolis?rev=6d7ed9a033babc054db9eff5b59dee978d2b0d76#6d7ed9a033babc054db9eff5b59dee978d2b0d76" +source = "git+https://github.com/oxidecomputer/propolis?rev=50cb28f586083fdb990e401bc6146e7dac9b2753#50cb28f586083fdb990e401bc6146e7dac9b2753" dependencies = [ "anyhow", "atty", @@ -7259,7 +7259,7 @@ dependencies = [ "futures", "hyper 0.14.28", "progenitor", - "propolis_types 0.0.0 (git+https://github.com/oxidecomputer/propolis?rev=6d7ed9a033babc054db9eff5b59dee978d2b0d76)", + "propolis_types 0.0.0 (git+https://github.com/oxidecomputer/propolis?rev=50cb28f586083fdb990e401bc6146e7dac9b2753)", "rand 0.8.5", "reqwest", "schemars", @@ -7272,7 +7272,7 @@ dependencies = [ "slog-term", "thiserror", "tokio", - "tokio-tungstenite 0.20.1", + "tokio-tungstenite 0.21.0", "uuid", ] @@ -7291,7 +7291,7 @@ dependencies = [ [[package]] name = "propolis_types" version = "0.0.0" -source = "git+https://github.com/oxidecomputer/propolis?rev=6d7ed9a033babc054db9eff5b59dee978d2b0d76#6d7ed9a033babc054db9eff5b59dee978d2b0d76" +source = "git+https://github.com/oxidecomputer/propolis?rev=50cb28f586083fdb990e401bc6146e7dac9b2753#50cb28f586083fdb990e401bc6146e7dac9b2753" dependencies = [ "schemars", "serde", diff --git a/Cargo.toml b/Cargo.toml index d461e0585a..69269a1de4 100644 --- a/Cargo.toml +++ b/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" diff --git a/package-manifest.toml b/package-manifest.toml index 8e27588be3..27a5397bf1 100644 --- a/package-manifest.toml +++ b/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//propolis-server.sha256.txt -source.sha256 = "f8f41b47bc00811fefe2ba75e0f6f8ab77765776c04021e0b31f09c3b21108a9" +source.sha256 = "864e74222d3e617f1bd7b7ba8d0e5cc18134dca121fc4339369620d1419c5bb0" output.type = "zone" [package.mg-ddm-gz] diff --git a/sled-agent/src/common/instance.rs b/sled-agent/src/common/instance.rs index 02623455f4..ed0aceff82 100644 --- a/sled-agent/src/common/instance.rs +++ b/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(), } } diff --git a/sled-agent/src/sim/instance.rs b/sled-agent/src/sim/instance.rs index 2ac8618399..06739df8fa 100644 --- a/sled-agent/src/sim/instance.rs +++ b/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,