diff --git a/Cargo.lock b/Cargo.lock index 5048914c74..1aec1ccc89 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -693,7 +693,7 @@ dependencies = [ [[package]] name = "bhyve_api" version = "0.0.0" -source = "git+https://github.com/oxidecomputer/propolis?rev=220a6f367c18f2452dbc4fa9086f3fe73b961739#220a6f367c18f2452dbc4fa9086f3fe73b961739" +source = "git+https://github.com/oxidecomputer/propolis?rev=d4529fd8247386b422b78e1203315d5baea5ea8b#d4529fd8247386b422b78e1203315d5baea5ea8b" dependencies = [ "bhyve_api_sys", "libc", @@ -703,7 +703,7 @@ dependencies = [ [[package]] name = "bhyve_api_sys" version = "0.0.0" -source = "git+https://github.com/oxidecomputer/propolis?rev=220a6f367c18f2452dbc4fa9086f3fe73b961739#220a6f367c18f2452dbc4fa9086f3fe73b961739" +source = "git+https://github.com/oxidecomputer/propolis?rev=d4529fd8247386b422b78e1203315d5baea5ea8b#d4529fd8247386b422b78e1203315d5baea5ea8b" dependencies = [ "libc", "strum", @@ -7023,7 +7023,7 @@ dependencies = [ "pq-sys", "pretty_assertions", "progenitor-client", - "propolis-client 0.1.0 (git+https://github.com/oxidecomputer/propolis?rev=220a6f367c18f2452dbc4fa9086f3fe73b961739)", + "propolis-client 0.1.0 (git+https://github.com/oxidecomputer/propolis?rev=d4529fd8247386b422b78e1203315d5baea5ea8b)", "qorb", "rand", "rcgen", @@ -7289,7 +7289,7 @@ dependencies = [ "oximeter-producer", "oxnet", "pretty_assertions", - "propolis-client 0.1.0 (git+https://github.com/oxidecomputer/propolis?rev=220a6f367c18f2452dbc4fa9086f3fe73b961739)", + "propolis-client 0.1.0 (git+https://github.com/oxidecomputer/propolis?rev=d4529fd8247386b422b78e1203315d5baea5ea8b)", "propolis-mock-server", "propolis_api_types", "rand", @@ -9004,12 +9004,14 @@ dependencies = [ [[package]] name = "propolis-client" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/propolis?rev=220a6f367c18f2452dbc4fa9086f3fe73b961739#220a6f367c18f2452dbc4fa9086f3fe73b961739" +source = "git+https://github.com/oxidecomputer/propolis?rev=d4529fd8247386b422b78e1203315d5baea5ea8b#d4529fd8247386b422b78e1203315d5baea5ea8b" dependencies = [ "async-trait", "base64 0.21.7", + "crucible-client-types", "futures", "progenitor", + "propolis_api_types", "rand", "reqwest", "schemars", @@ -9046,7 +9048,7 @@ dependencies = [ [[package]] name = "propolis-mock-server" version = "0.0.0" -source = "git+https://github.com/oxidecomputer/propolis?rev=220a6f367c18f2452dbc4fa9086f3fe73b961739#220a6f367c18f2452dbc4fa9086f3fe73b961739" +source = "git+https://github.com/oxidecomputer/propolis?rev=d4529fd8247386b422b78e1203315d5baea5ea8b#d4529fd8247386b422b78e1203315d5baea5ea8b" dependencies = [ "anyhow", "atty", @@ -9088,12 +9090,13 @@ dependencies = [ [[package]] name = "propolis_api_types" version = "0.0.0" -source = "git+https://github.com/oxidecomputer/propolis?rev=220a6f367c18f2452dbc4fa9086f3fe73b961739#220a6f367c18f2452dbc4fa9086f3fe73b961739" +source = "git+https://github.com/oxidecomputer/propolis?rev=d4529fd8247386b422b78e1203315d5baea5ea8b#d4529fd8247386b422b78e1203315d5baea5ea8b" dependencies = [ "crucible-client-types", "propolis_types", "schemars", "serde", + "serde_with", "thiserror 1.0.69", "uuid", ] @@ -9101,7 +9104,7 @@ dependencies = [ [[package]] name = "propolis_types" version = "0.0.0" -source = "git+https://github.com/oxidecomputer/propolis?rev=220a6f367c18f2452dbc4fa9086f3fe73b961739#220a6f367c18f2452dbc4fa9086f3fe73b961739" +source = "git+https://github.com/oxidecomputer/propolis?rev=d4529fd8247386b422b78e1203315d5baea5ea8b#d4529fd8247386b422b78e1203315d5baea5ea8b" dependencies = [ "schemars", "serde", @@ -10739,6 +10742,7 @@ dependencies = [ "omicron-workspace-hack", "oxnet", "progenitor", + "propolis-client 0.1.0 (git+https://github.com/oxidecomputer/propolis?rev=d4529fd8247386b422b78e1203315d5baea5ea8b)", "regress 0.9.1", "reqwest", "schemars", @@ -10764,7 +10768,7 @@ dependencies = [ "omicron-uuid-kinds", "omicron-workspace-hack", "oxnet", - "propolis-client 0.1.0 (git+https://github.com/oxidecomputer/propolis?rev=220a6f367c18f2452dbc4fa9086f3fe73b961739)", + "propolis-client 0.1.0 (git+https://github.com/oxidecomputer/propolis?rev=d4529fd8247386b422b78e1203315d5baea5ea8b)", "rcgen", "schemars", "serde", diff --git a/Cargo.toml b/Cargo.toml index 3d29f61cf9..8906ab4d70 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -547,10 +547,10 @@ prettyplease = { version = "0.2.25", features = ["verbatim"] } proc-macro2 = "1.0" progenitor = "0.8.0" progenitor-client = "0.8.0" -bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "220a6f367c18f2452dbc4fa9086f3fe73b961739" } -propolis_api_types = { git = "https://github.com/oxidecomputer/propolis", rev = "220a6f367c18f2452dbc4fa9086f3fe73b961739" } -propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "220a6f367c18f2452dbc4fa9086f3fe73b961739" } -propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "220a6f367c18f2452dbc4fa9086f3fe73b961739" } +bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "d4529fd8247386b422b78e1203315d5baea5ea8b" } +propolis_api_types = { git = "https://github.com/oxidecomputer/propolis", rev = "d4529fd8247386b422b78e1203315d5baea5ea8b" } +propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "d4529fd8247386b422b78e1203315d5baea5ea8b" } +propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "d4529fd8247386b422b78e1203315d5baea5ea8b" } proptest = "1.5.0" qorb = "0.2.1" quote = "1.0" diff --git a/clients/sled-agent-client/Cargo.toml b/clients/sled-agent-client/Cargo.toml index 770588d6b4..e6c77fe24a 100644 --- a/clients/sled-agent-client/Cargo.toml +++ b/clients/sled-agent-client/Cargo.toml @@ -17,6 +17,7 @@ omicron-uuid-kinds.workspace = true omicron-workspace-hack.workspace = true oxnet.workspace = true progenitor.workspace = true +propolis-client.workspace = true regress.workspace = true reqwest = { workspace = true, features = [ "json", "rustls-tls", "stream" ] } schemars.workspace = true diff --git a/clients/sled-agent-client/src/lib.rs b/clients/sled-agent-client/src/lib.rs index f2eb957650..6a9c721587 100644 --- a/clients/sled-agent-client/src/lib.rs +++ b/clients/sled-agent-client/src/lib.rs @@ -12,6 +12,8 @@ use serde::Serialize; use std::convert::TryFrom; use uuid::Uuid; +pub use propolis_client::{CrucibleOpts, VolumeConstructionRequest}; + progenitor::generate_api!( spec = "../../openapi/sled-agent.json", derives = [schemars::JsonSchema, PartialEq], diff --git a/dev-tools/ls-apis/tests/api_dependencies.out b/dev-tools/ls-apis/tests/api_dependencies.out index 8e061f2906..d2b672e62c 100644 --- a/dev-tools/ls-apis/tests/api_dependencies.out +++ b/dev-tools/ls-apis/tests/api_dependencies.out @@ -67,8 +67,8 @@ Oximeter (client: oximeter-client) consumed by: omicron-nexus (omicron/nexus) via 2 paths Propolis (client: propolis-client) - consumed by: omicron-nexus (omicron/nexus) via 2 paths - consumed by: omicron-sled-agent (omicron/sled-agent) via 1 path + consumed by: omicron-nexus (omicron/nexus) via 3 paths + consumed by: omicron-sled-agent (omicron/sled-agent) via 2 paths Crucible Repair (client: repair-client) consumed by: crucible-downstairs (crucible/downstairs) via 1 path diff --git a/dev-tools/omdb/src/bin/omdb/db.rs b/dev-tools/omdb/src/bin/omdb/db.rs index e501e650b1..629e465212 100644 --- a/dev-tools/omdb/src/bin/omdb/db.rs +++ b/dev-tools/omdb/src/bin/omdb/db.rs @@ -135,7 +135,7 @@ use omicron_uuid_kinds::InstanceUuid; use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::PropolisUuid; use omicron_uuid_kinds::SledUuid; -use sled_agent_client::types::VolumeConstructionRequest; +use sled_agent_client::VolumeConstructionRequest; use std::borrow::Cow; use std::cmp::Ordering; use std::collections::BTreeMap; diff --git a/nexus/db-queries/src/db/datastore/region_replacement.rs b/nexus/db-queries/src/db/datastore/region_replacement.rs index 8aad7f2cfd..e4a7d78224 100644 --- a/nexus/db-queries/src/db/datastore/region_replacement.rs +++ b/nexus/db-queries/src/db/datastore/region_replacement.rs @@ -924,7 +924,7 @@ mod test { use crate::db::pub_test_utils::TestDatabase; use omicron_test_utils::dev; - use sled_agent_client::types::VolumeConstructionRequest; + use sled_agent_client::VolumeConstructionRequest; #[tokio::test] async fn test_one_replacement_per_volume() { diff --git a/nexus/db-queries/src/db/datastore/region_snapshot_replacement.rs b/nexus/db-queries/src/db/datastore/region_snapshot_replacement.rs index 90b014c582..5568822e07 100644 --- a/nexus/db-queries/src/db/datastore/region_snapshot_replacement.rs +++ b/nexus/db-queries/src/db/datastore/region_snapshot_replacement.rs @@ -1371,7 +1371,7 @@ mod test { use crate::db::pub_test_utils::TestDatabase; use omicron_test_utils::dev; use omicron_uuid_kinds::DatasetUuid; - use sled_agent_client::types::VolumeConstructionRequest; + use sled_agent_client::VolumeConstructionRequest; #[tokio::test] async fn test_one_replacement_per_volume() { diff --git a/nexus/db-queries/src/db/datastore/volume.rs b/nexus/db-queries/src/db/datastore/volume.rs index 505533da50..e55c374073 100644 --- a/nexus/db-queries/src/db/datastore/volume.rs +++ b/nexus/db-queries/src/db/datastore/volume.rs @@ -58,7 +58,7 @@ use omicron_uuid_kinds::UpstairsRepairKind; use serde::Deserialize; use serde::Deserializer; use serde::Serialize; -use sled_agent_client::types::VolumeConstructionRequest; +use sled_agent_client::VolumeConstructionRequest; use std::collections::HashSet; use std::collections::VecDeque; use std::net::AddrParseError; @@ -1118,7 +1118,7 @@ impl DataStore { ) -> CreateResult { let volume = self.volume_checkout(volume_id, reason).await?; - let vcr: sled_agent_client::types::VolumeConstructionRequest = + let vcr: sled_agent_client::VolumeConstructionRequest = serde_json::from_str(volume.data())?; let randomized_vcr = serde_json::to_string( @@ -2578,10 +2578,15 @@ fn region_in_vcr( VolumeConstructionRequest::Region { opts, .. } => { for target in &opts.target { - let parsed_target: SocketAddrV6 = target.parse()?; - if parsed_target == *region { - region_found = true; - break; + match target { + SocketAddr::V6(t) if *t == *region => { + region_found = true; + break; + } + SocketAddr::V6(_) => {} + SocketAddr::V4(_) => { + bail!("region target contains an IPv4 address"); + } } } } @@ -2643,9 +2648,16 @@ fn read_only_target_in_vcr( } for target in &opts.target { - let parsed_target: SocketAddrV6 = target.parse()?; - if parsed_target == *read_only_target && opts.read_only { - return Ok(true); + match target { + SocketAddr::V6(t) + if *t == *read_only_target && opts.read_only => + { + return Ok(true); + } + SocketAddr::V6(_) => {} + SocketAddr::V4(_) => { + bail!("region target contains an IPv4 address"); + } } } } @@ -3177,9 +3189,9 @@ impl DataStore { blocks_per_extent: 1, extent_count: 1, gen: 1, - opts: sled_agent_client::types::CrucibleOpts { + opts: sled_agent_client::CrucibleOpts { id: volume_to_delete_id.0, - target: vec![existing.0.to_string()], + target: vec![existing.0.into()], lossy: false, flush_timeout: None, key: None, @@ -3442,7 +3454,9 @@ pub fn read_only_resources_associated_with_volume( VolumeConstructionRequest::Region { opts, .. } => { for target in &opts.target { if opts.read_only { - crucible_targets.read_only_targets.push(target.clone()); + crucible_targets + .read_only_targets + .push(target.to_string()); } } } @@ -3481,7 +3495,7 @@ pub fn read_write_resources_associated_with_volume( VolumeConstructionRequest::Region { opts, .. } => { if !opts.read_only { for target in &opts.target { - targets.push(target.clone()); + targets.push(target.to_string()); } } } @@ -3597,10 +3611,11 @@ fn replace_region_in_vcr( VolumeConstructionRequest::Region { opts, gen, .. } => { for target in &mut opts.target { - let parsed_target: SocketAddrV6 = target.parse()?; - if parsed_target == old_region { - *target = new_region.to_string(); - old_region_found = true; + if let SocketAddr::V6(target) = target { + if *target == old_region { + *target = new_region; + old_region_found = true; + } } } @@ -3681,10 +3696,11 @@ fn replace_read_only_target_in_vcr( } for target in &mut opts.target { - let parsed_target: SocketAddrV6 = target.parse()?; - if parsed_target == old_target.0 && opts.read_only { - *target = new_target.0.to_string(); - replacements += 1; + if let SocketAddr::V6(target) = target { + if *target == old_target.0 && opts.read_only { + *target = new_target.0; + replacements += 1; + } } } } @@ -3727,9 +3743,10 @@ fn find_matching_rw_regions_in_volume( VolumeConstructionRequest::Region { opts, .. } => { if !opts.read_only { for target in &opts.target { - let parsed_target: SocketAddrV6 = target.parse()?; - if parsed_target.ip() == ip { - matched_targets.push(parsed_target); + if let SocketAddr::V6(target) = target { + if target.ip() == ip { + matched_targets.push(*target); + } } } } @@ -4006,7 +4023,7 @@ mod tests { use nexus_types::external_api::params::DiskSource; use omicron_common::api::external::ByteCount; use omicron_test_utils::dev; - use sled_agent_client::types::CrucibleOpts; + use sled_agent_client::CrucibleOpts; // Assert that Nexus will not fail to deserialize an old version of // CrucibleResources that was serialized before schema update 6.0.0. @@ -4145,7 +4162,7 @@ mod tests { .await .unwrap(); - let mut region_addresses: Vec = + let mut region_addresses: Vec = Vec::with_capacity(datasets_and_regions.len()); for (i, (_, region)) in datasets_and_regions.iter().enumerate() { @@ -4162,7 +4179,7 @@ mod tests { let address: SocketAddrV6 = datastore.region_addr(region.id()).await.unwrap().unwrap(); - region_addresses.push(address.to_string()); + region_addresses.push(address); } // Manually create a replacement region at the first dataset @@ -4209,9 +4226,9 @@ mod tests { id: volume_id, target: vec![ // target to replace - region_addresses[0].clone(), - region_addresses[1].clone(), - region_addresses[2].clone(), + region_addresses[0].into(), + region_addresses[1].into(), + region_addresses[2].into(), ], lossy: false, flush_timeout: None, @@ -4238,7 +4255,7 @@ mod tests { db::datastore::VolumeReplacementParams { volume_id, region_id: datasets_and_regions[0].1.id(), - region_addr: region_addresses[0].parse().unwrap(), + region_addr: region_addresses[0], }, /* replacement */ db::datastore::VolumeReplacementParams { @@ -4271,9 +4288,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - replacement_region_addr.to_string(), // replaced - region_addresses[1].clone(), - region_addresses[2].clone(), + replacement_region_addr.into(), // replaced + region_addresses[1].into(), + region_addresses[2].into(), ], lossy: false, flush_timeout: None, @@ -4302,7 +4319,7 @@ mod tests { db::datastore::VolumeReplacementParams { volume_id: volume_to_delete_id, region_id: datasets_and_regions[0].1.id(), - region_addr: region_addresses[0].parse().unwrap(), + region_addr: region_addresses[0], }, ) .await @@ -4329,9 +4346,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - region_addresses[0].clone(), // back to what it was - region_addresses[1].clone(), - region_addresses[2].clone(), + region_addresses[0].into(), // back to what it was + region_addresses[1].into(), + region_addresses[2].into(), ], lossy: false, flush_timeout: None, @@ -4383,7 +4400,7 @@ mod tests { .await .unwrap(); - let mut region_addresses: Vec = + let mut region_addresses: Vec = Vec::with_capacity(datasets_and_regions.len()); for (i, (_, region)) in datasets_and_regions.iter().enumerate() { @@ -4400,7 +4417,7 @@ mod tests { let address: SocketAddrV6 = datastore.region_addr(region.id()).await.unwrap().unwrap(); - region_addresses.push(address.to_string()); + region_addresses.push(address); } // Manually create a replacement region at the first dataset @@ -4435,28 +4452,31 @@ mod tests { // need to add region snapshot objects to satisfy volume create // transaction's search for resources - let address_1 = String::from("[fd00:1122:3344:104::1]:400"); - let address_2 = String::from("[fd00:1122:3344:105::1]:401"); - let address_3 = String::from("[fd00:1122:3344:106::1]:402"); + let address_1: SocketAddrV6 = + "[fd00:1122:3344:104::1]:400".parse().unwrap(); + let address_2: SocketAddrV6 = + "[fd00:1122:3344:105::1]:401".parse().unwrap(); + let address_3: SocketAddrV6 = + "[fd00:1122:3344:106::1]:402".parse().unwrap(); let region_snapshots = [ RegionSnapshot::new( DatasetUuid::new_v4(), Uuid::new_v4(), Uuid::new_v4(), - address_1.clone(), + address_1.to_string(), ), RegionSnapshot::new( DatasetUuid::new_v4(), Uuid::new_v4(), Uuid::new_v4(), - address_2.clone(), + address_2.to_string(), ), RegionSnapshot::new( DatasetUuid::new_v4(), Uuid::new_v4(), Uuid::new_v4(), - address_3.clone(), + address_3.to_string(), ), ]; @@ -4493,9 +4513,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - region_addresses[0].clone(), - region_addresses[1].clone(), - region_addresses[2].clone(), + region_addresses[0].into(), + region_addresses[1].into(), + region_addresses[2].into(), ], lossy: false, flush_timeout: None, @@ -4517,9 +4537,9 @@ mod tests { id: rop_id, target: vec![ // target to replace - address_1.clone(), - address_2.clone(), - address_3.clone(), + address_1.into(), + address_2.into(), + address_3.into(), ], lossy: false, flush_timeout: None, @@ -4587,7 +4607,7 @@ mod tests { let volume_replace_snapshot_result = datastore .volume_replace_snapshot( VolumeWithTarget(volume_id), - ExistingTarget(address_1.parse().unwrap()), + ExistingTarget(address_1), ReplacementTarget(replacement_region_addr), VolumeToDelete(volume_to_delete_id), ) @@ -4616,9 +4636,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - region_addresses[0].clone(), - region_addresses[1].clone(), - region_addresses[2].clone(), + region_addresses[0].into(), + region_addresses[1].into(), + region_addresses[2].into(), ], lossy: false, flush_timeout: None, @@ -4640,9 +4660,9 @@ mod tests { id: rop_id, target: vec![ // target replaced - replacement_region_addr.to_string(), - address_2.clone(), - address_3.clone(), + replacement_region_addr.into(), + address_2.into(), + address_3.into(), ], lossy: false, flush_timeout: None, @@ -4682,7 +4702,7 @@ mod tests { id: volume_to_delete_id, target: vec![ // replaced target stashed here - address_1.clone(), + address_1.into(), ], lossy: false, flush_timeout: None, @@ -4745,7 +4765,7 @@ mod tests { .volume_replace_snapshot( VolumeWithTarget(volume_id), ExistingTarget(replacement_region_addr), - ReplacementTarget(address_1.parse().unwrap()), + ReplacementTarget(address_1), VolumeToDelete(volume_to_delete_id), ) .await @@ -4772,9 +4792,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - region_addresses[0].clone(), - region_addresses[1].clone(), - region_addresses[2].clone(), + region_addresses[0].into(), + region_addresses[1].into(), + region_addresses[2].into(), ], lossy: false, flush_timeout: None, @@ -4796,7 +4816,9 @@ mod tests { id: rop_id, target: vec![ // back to what it was - address_1, address_2, address_3, + address_1.into(), + address_2.into(), + address_3.into(), ], lossy: false, flush_timeout: None, @@ -4836,7 +4858,7 @@ mod tests { id: volume_to_delete_id, target: vec![ // replacement stashed here - replacement_region_addr.to_string(), + replacement_region_addr.into(), ], lossy: false, flush_timeout: None, @@ -4899,16 +4921,19 @@ mod tests { // need to add region snapshot objects to satisfy volume create // transaction's search for resources - let address_1 = String::from("[fd00:1122:3344:104::1]:400"); - let address_2 = String::from("[fd00:1122:3344:105::1]:401"); - let address_3 = String::from("[fd00:1122:3344:106::1]:402"); + let address_1: SocketAddrV6 = + "[fd00:1122:3344:104::1]:400".parse().unwrap(); + let address_2: SocketAddrV6 = + "[fd00:1122:3344:105::1]:401".parse().unwrap(); + let address_3: SocketAddrV6 = + "[fd00:1122:3344:106::1]:402".parse().unwrap(); datastore .region_snapshot_create(RegionSnapshot::new( DatasetUuid::new_v4(), Uuid::new_v4(), Uuid::new_v4(), - address_1.clone(), + address_1.to_string(), )) .await .unwrap(); @@ -4917,7 +4942,7 @@ mod tests { DatasetUuid::new_v4(), Uuid::new_v4(), Uuid::new_v4(), - address_2.clone(), + address_2.to_string(), )) .await .unwrap(); @@ -4926,7 +4951,7 @@ mod tests { DatasetUuid::new_v4(), Uuid::new_v4(), Uuid::new_v4(), - address_3.clone(), + address_3.to_string(), )) .await .unwrap(); @@ -4949,9 +4974,9 @@ mod tests { opts: CrucibleOpts { id: Uuid::new_v4(), target: vec![ - address_1.clone(), - address_2, - address_3, + address_1.into(), + address_2.into(), + address_3.into(), ], lossy: false, flush_timeout: None, @@ -4971,10 +4996,7 @@ mod tests { .unwrap(); let volumes = datastore - .find_volumes_referencing_socket_addr( - &opctx, - address_1.parse().unwrap(), - ) + .find_volumes_referencing_socket_addr(&opctx, address_1.into()) .await .unwrap(); @@ -5014,9 +5036,9 @@ mod tests { opts: CrucibleOpts { id: Uuid::new_v4(), target: vec![ - String::from("[fd00:1122:3344:104::1]:400"), - String::from("[fd00:1122:3344:105::1]:401"), - String::from("[fd00:1122:3344:106::1]:402"), + "[fd00:1122:3344:104::1]:400".parse().unwrap(), + "[fd00:1122:3344:105::1]:401".parse().unwrap(), + "[fd00:1122:3344:106::1]:402".parse().unwrap(), ], lossy: false, flush_timeout: None, @@ -5050,9 +5072,9 @@ mod tests { opts: CrucibleOpts { id: Uuid::new_v4(), target: vec![ - String::from("[fd00:1122:3344:104::1]:400"), - String::from("[fd00:1122:3344:105::1]:401"), - String::from("[fd00:1122:3344:106::1]:402"), + "[fd00:1122:3344:104::1]:400".parse().unwrap(), + "[fd00:1122:3344:105::1]:401".parse().unwrap(), + "[fd00:1122:3344:106::1]:402".parse().unwrap(), ], lossy: false, flush_timeout: None, @@ -5089,9 +5111,9 @@ mod tests { opts: CrucibleOpts { id: Uuid::new_v4(), target: vec![ - String::from("[fd00:1122:3344:104::1]:400"), - String::from("[fd00:1122:3344:105::1]:401"), - String::from("[fd00:1122:3344:106::1]:402"), + "[fd00:1122:3344:104::1]:400".parse().unwrap(), + "[fd00:1122:3344:105::1]:401".parse().unwrap(), + "[fd00:1122:3344:106::1]:402".parse().unwrap(), ], lossy: false, flush_timeout: None, @@ -5133,9 +5155,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - String::from("[fd00:1122:3344:104::1]:400"), - String::from("[fd00:1122:3344:105::1]:401"), - String::from("[fd00:1122:3344:106::1]:402"), + "[fd00:1122:3344:104::1]:400".parse().unwrap(), + "[fd00:1122:3344:105::1]:401".parse().unwrap(), + "[fd00:1122:3344:106::1]:402".parse().unwrap(), ], lossy: false, flush_timeout: None, @@ -5175,9 +5197,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - String::from("[fd00:1122:3344:104::1]:400"), - new_target.0.to_string(), - String::from("[fd00:1122:3344:106::1]:402"), + "[fd00:1122:3344:104::1]:400".parse().unwrap(), + new_target.0.into(), + "[fd00:1122:3344:106::1]:402".parse().unwrap(), ], lossy: false, flush_timeout: None, @@ -5210,9 +5232,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - String::from("[fd55:1122:3344:204::1]:1000"), - String::from("[fd55:1122:3344:205::1]:1001"), - String::from("[fd55:1122:3344:206::1]:1002"), + "[fd55:1122:3344:204::1]:1000".parse().unwrap(), + "[fd55:1122:3344:205::1]:1001".parse().unwrap(), + "[fd55:1122:3344:206::1]:1002".parse().unwrap(), ], lossy: false, flush_timeout: None, @@ -5233,9 +5255,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - String::from("[fd33:1122:3344:304::1]:2000"), - String::from("[fd33:1122:3344:305::1]:2001"), - String::from("[fd33:1122:3344:306::1]:2002"), + "[fd33:1122:3344:304::1]:2000".parse().unwrap(), + "[fd33:1122:3344:305::1]:2001".parse().unwrap(), + "[fd33:1122:3344:306::1]:2002".parse().unwrap(), ], lossy: false, flush_timeout: None, @@ -5258,9 +5280,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - String::from("[fd00:1122:3344:104::1]:400"), - String::from("[fd00:1122:3344:105::1]:401"), - String::from("[fd00:1122:3344:106::1]:402"), + "[fd00:1122:3344:104::1]:400".parse().unwrap(), + "[fd00:1122:3344:105::1]:401".parse().unwrap(), + "[fd00:1122:3344:106::1]:402".parse().unwrap(), ], lossy: false, flush_timeout: None, @@ -5301,9 +5323,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - String::from("[fd55:1122:3344:204::1]:1000"), - String::from("[fd55:1122:3344:205::1]:1001"), - String::from("[fd55:1122:3344:206::1]:1002"), + "[fd55:1122:3344:204::1]:1000".parse().unwrap(), + "[fd55:1122:3344:205::1]:1001".parse().unwrap(), + "[fd55:1122:3344:206::1]:1002".parse().unwrap(), ], lossy: false, flush_timeout: None, @@ -5324,13 +5346,13 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - String::from( - "[fd33:1122:3344:304::1]:2000" - ), - String::from( - "[fd33:1122:3344:305::1]:2001" - ), - new_target.0.to_string(), + "[fd33:1122:3344:304::1]:2000" + .parse() + .unwrap(), + "[fd33:1122:3344:305::1]:2001" + .parse() + .unwrap(), + new_target.0.into(), ], lossy: false, flush_timeout: None, @@ -5353,9 +5375,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - String::from("[fd00:1122:3344:104::1]:400"), - String::from("[fd00:1122:3344:105::1]:401"), - String::from("[fd00:1122:3344:106::1]:402"), + "[fd00:1122:3344:104::1]:400".parse().unwrap(), + "[fd00:1122:3344:105::1]:401".parse().unwrap(), + "[fd00:1122:3344:106::1]:402".parse().unwrap(), ], lossy: false, flush_timeout: None, @@ -5383,9 +5405,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - String::from("[fd33:1122:3344:304::1]:2000"), - String::from("[fd33:1122:3344:305::1]:2001"), - String::from("[fd33:1122:3344:306::1]:2002"), + "[fd33:1122:3344:304::1]:2000".parse().unwrap(), + "[fd33:1122:3344:305::1]:2001".parse().unwrap(), + "[fd33:1122:3344:306::1]:2002".parse().unwrap(), ], lossy: false, flush_timeout: None, @@ -5412,9 +5434,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - String::from("[fd55:1122:3344:204::1]:1000"), - String::from("[fd55:1122:3344:205::1]:1001"), - String::from("[fd55:1122:3344:206::1]:1002"), + "[fd55:1122:3344:204::1]:1000".parse().unwrap(), + "[fd55:1122:3344:205::1]:1001".parse().unwrap(), + "[fd55:1122:3344:206::1]:1002".parse().unwrap(), ], lossy: false, flush_timeout: None, @@ -5450,9 +5472,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - new_target.0.to_string(), - String::from("[fd33:1122:3344:305::1]:2001"), - String::from("[fd33:1122:3344:306::1]:2002"), + new_target.0.into(), + "[fd33:1122:3344:305::1]:2001".parse().unwrap(), + "[fd33:1122:3344:306::1]:2002".parse().unwrap(), ], lossy: false, flush_timeout: None, @@ -5481,9 +5503,9 @@ mod tests { opts: CrucibleOpts { id: volume_id, target: vec![ - String::from("[fd55:1122:3344:204::1]:1000"), - String::from("[fd55:1122:3344:205::1]:1001"), - String::from("[fd55:1122:3344:206::1]:1002"), + "[fd55:1122:3344:204::1]:1000".parse().unwrap(), + "[fd55:1122:3344:205::1]:1001".parse().unwrap(), + "[fd55:1122:3344:206::1]:1002".parse().unwrap(), ], lossy: false, flush_timeout: None, diff --git a/nexus/db-queries/src/db/datastore/volume_repair.rs b/nexus/db-queries/src/db/datastore/volume_repair.rs index 598d9d77a2..976c7f756a 100644 --- a/nexus/db-queries/src/db/datastore/volume_repair.rs +++ b/nexus/db-queries/src/db/datastore/volume_repair.rs @@ -169,7 +169,7 @@ mod test { use crate::db::pub_test_utils::TestDatabase; use omicron_test_utils::dev; - use sled_agent_client::types::VolumeConstructionRequest; + use sled_agent_client::VolumeConstructionRequest; #[tokio::test] async fn volume_lock_conflict_error_returned() { diff --git a/nexus/src/app/background/tasks/region_replacement.rs b/nexus/src/app/background/tasks/region_replacement.rs index caa262bc8c..acadf3dee3 100644 --- a/nexus/src/app/background/tasks/region_replacement.rs +++ b/nexus/src/app/background/tasks/region_replacement.rs @@ -306,8 +306,8 @@ mod test { use nexus_db_model::RegionReplacement; use nexus_db_model::Volume; use nexus_test_utils_macros::nexus_test; - use sled_agent_client::types::CrucibleOpts; - use sled_agent_client::types::VolumeConstructionRequest; + use sled_agent_client::CrucibleOpts; + use sled_agent_client::VolumeConstructionRequest; use uuid::Uuid; type ControlPlaneTestContext = diff --git a/nexus/src/app/background/tasks/region_replacement_driver.rs b/nexus/src/app/background/tasks/region_replacement_driver.rs index 6cc28f9dfd..c08cd2edfc 100644 --- a/nexus/src/app/background/tasks/region_replacement_driver.rs +++ b/nexus/src/app/background/tasks/region_replacement_driver.rs @@ -258,7 +258,7 @@ mod test { use omicron_uuid_kinds::UpstairsKind; use omicron_uuid_kinds::UpstairsRepairKind; use omicron_uuid_kinds::UpstairsSessionKind; - use sled_agent_client::types::VolumeConstructionRequest; + use sled_agent_client::VolumeConstructionRequest; use uuid::Uuid; type ControlPlaneTestContext = diff --git a/nexus/src/app/background/tasks/region_snapshot_replacement_finish.rs b/nexus/src/app/background/tasks/region_snapshot_replacement_finish.rs index 61a84c579d..a4351e0573 100644 --- a/nexus/src/app/background/tasks/region_snapshot_replacement_finish.rs +++ b/nexus/src/app/background/tasks/region_snapshot_replacement_finish.rs @@ -198,7 +198,7 @@ mod test { use nexus_db_queries::db::datastore::region_snapshot_replacement; use nexus_test_utils_macros::nexus_test; use omicron_uuid_kinds::DatasetUuid; - use sled_agent_client::types::VolumeConstructionRequest; + use sled_agent_client::VolumeConstructionRequest; use uuid::Uuid; type ControlPlaneTestContext = diff --git a/nexus/src/app/background/tasks/region_snapshot_replacement_garbage_collect.rs b/nexus/src/app/background/tasks/region_snapshot_replacement_garbage_collect.rs index 57bbf3741c..88999efeb9 100644 --- a/nexus/src/app/background/tasks/region_snapshot_replacement_garbage_collect.rs +++ b/nexus/src/app/background/tasks/region_snapshot_replacement_garbage_collect.rs @@ -155,7 +155,7 @@ mod test { use nexus_db_model::RegionSnapshotReplacementState; use nexus_test_utils_macros::nexus_test; use omicron_uuid_kinds::DatasetUuid; - use sled_agent_client::types::VolumeConstructionRequest; + use sled_agent_client::VolumeConstructionRequest; use uuid::Uuid; type ControlPlaneTestContext = diff --git a/nexus/src/app/background/tasks/region_snapshot_replacement_start.rs b/nexus/src/app/background/tasks/region_snapshot_replacement_start.rs index f2b82a3943..412a437250 100644 --- a/nexus/src/app/background/tasks/region_snapshot_replacement_start.rs +++ b/nexus/src/app/background/tasks/region_snapshot_replacement_start.rs @@ -355,8 +355,8 @@ mod test { use omicron_common::api::external; use omicron_uuid_kinds::DatasetUuid; use omicron_uuid_kinds::GenericUuid; - use sled_agent_client::types::CrucibleOpts; - use sled_agent_client::types::VolumeConstructionRequest; + use sled_agent_client::CrucibleOpts; + use sled_agent_client::VolumeConstructionRequest; use std::collections::BTreeMap; use uuid::Uuid; @@ -701,10 +701,7 @@ mod test { gen: 1, opts: CrucibleOpts { id: Uuid::new_v4(), - target: vec![ - // the region snapshot - String::from("[::1]:12345"), - ], + target: vec!["[::1]:12345".parse().unwrap()], lossy: false, flush_timeout: None, key: None, diff --git a/nexus/src/app/background/tasks/region_snapshot_replacement_step.rs b/nexus/src/app/background/tasks/region_snapshot_replacement_step.rs index f481126312..c8010dd90d 100644 --- a/nexus/src/app/background/tasks/region_snapshot_replacement_step.rs +++ b/nexus/src/app/background/tasks/region_snapshot_replacement_step.rs @@ -566,8 +566,9 @@ mod test { use nexus_db_model::Volume; use nexus_test_utils_macros::nexus_test; use omicron_uuid_kinds::DatasetUuid; - use sled_agent_client::types::CrucibleOpts; - use sled_agent_client::types::VolumeConstructionRequest; + use sled_agent_client::CrucibleOpts; + use sled_agent_client::VolumeConstructionRequest; + use std::net::SocketAddrV6; use uuid::Uuid; type ControlPlaneTestContext = @@ -575,7 +576,7 @@ mod test { async fn add_fake_volume_for_snapshot_addr( datastore: &DataStore, - snapshot_addr: String, + snapshot_addr: SocketAddrV6, ) -> Uuid { let new_volume_id = Uuid::new_v4(); @@ -587,7 +588,7 @@ mod test { DatasetUuid::new_v4(), Uuid::new_v4(), Uuid::new_v4(), - snapshot_addr.clone(), + snapshot_addr.to_string(), )) .await .unwrap(); @@ -604,7 +605,7 @@ mod test { gen: 0, opts: CrucibleOpts { id: Uuid::new_v4(), - target: vec![snapshot_addr], + target: vec![snapshot_addr.into()], lossy: false, flush_timeout: None, key: None, @@ -656,13 +657,14 @@ mod test { let dataset_id = DatasetUuid::new_v4(); let region_id = Uuid::new_v4(); let snapshot_id = Uuid::new_v4(); - let snapshot_addr = String::from("[fd00:1122:3344::101]:9876"); + let snapshot_addr: SocketAddrV6 = + "[fd00:1122:3344::101]:9876".parse().unwrap(); let fake_region_snapshot = RegionSnapshot::new( dataset_id, region_id, snapshot_id, - snapshot_addr.clone(), + snapshot_addr.to_string(), ); datastore.region_snapshot_create(fake_region_snapshot).await.unwrap(); @@ -746,28 +748,22 @@ mod test { // Add some fake volumes that reference the region snapshot being // replaced - let new_volume_1_id = add_fake_volume_for_snapshot_addr( - &datastore, - snapshot_addr.clone(), - ) - .await; - let new_volume_2_id = add_fake_volume_for_snapshot_addr( - &datastore, - snapshot_addr.clone(), - ) - .await; + let new_volume_1_id = + add_fake_volume_for_snapshot_addr(&datastore, snapshot_addr).await; + let new_volume_2_id = + add_fake_volume_for_snapshot_addr(&datastore, snapshot_addr).await; // Add some fake volumes that do not let other_volume_1_id = add_fake_volume_for_snapshot_addr( &datastore, - String::from("[fd00:1122:3344::101]:1000"), + "[fd00:1122:3344::101]:1000".parse().unwrap(), ) .await; let other_volume_2_id = add_fake_volume_for_snapshot_addr( &datastore, - String::from("[fd12:5544:3344::912]:3901"), + "[fd12:5544:3344::912]:3901".parse().unwrap(), ) .await; diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index a3fa722d36..d46da647d5 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -155,7 +155,7 @@ impl super::Nexus { let image_id = Uuid::new_v4(); let volume_construction_request = - sled_agent_client::types::VolumeConstructionRequest::File { + sled_agent_client::VolumeConstructionRequest::File { id: image_id, block_size, path: "/opt/oxide/propolis-server/blob/alpine.iso" diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index e916974332..109c0d8521 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -63,8 +63,7 @@ use propolis_client::support::WebSocketStream; use sagas::instance_common::ExternalIpAttach; use sagas::instance_start; use sagas::instance_update; -use sled_agent_client::types::BootOrderEntry; -use sled_agent_client::types::BootSettings; +use sled_agent_client::types::InstanceBootSettings; use sled_agent_client::types::InstanceMigrationTargetParams; use sled_agent_client::types::InstanceProperties; use sled_agent_client::types::VmmPutStateBody; @@ -1053,8 +1052,6 @@ impl super::Nexus { ) .await?; - let mut boot_disk_name = None; - let mut disk_reqs = vec![]; for disk in &disks { // Disks that are attached to an instance should always have a slot @@ -1089,58 +1086,22 @@ impl super::Nexus { ) .await?; - // Propolis wants the name of the boot disk rather than ID, because we send names - // rather than IDs in the disk requsts as assembled below. - if db_instance.boot_disk_id == Some(disk.id()) { - boot_disk_name = Some(disk.name().to_string()); - } - - disk_reqs.push(sled_agent_client::types::DiskRequest { + disk_reqs.push(sled_agent_client::types::InstanceDisk { + disk_id: disk.id(), name: disk.name().to_string(), - slot: sled_agent_client::types::Slot(slot.0), + slot: slot.0, read_only: false, - device: "nvme".to_string(), - volume_construction_request: serde_json::from_str( - &volume.data(), - ) - .map_err(Error::from)?, + vcr_json: volume.data().to_owned(), }); } - let boot_settings = if let Some(boot_disk_name) = boot_disk_name { - Some(BootSettings { - order: vec![BootOrderEntry { name: boot_disk_name }], - }) - } else { - if let Some(instance_boot_disk_id) = - db_instance.boot_disk_id.as_ref() - { - // This should never occur: when setting the boot disk we ensure it is - // attached, and when detaching a disk we ensure it is not the boot - // disk. If this error is seen, the instance somehow had a boot disk - // that was not attached anyway. - // - // When Propolis accepts an ID rather than name, and we don't need to - // look up a name when assembling the Propolis request, we might as well - // remove this check; we can just pass the ID and rely on Propolis' own - // check that the boot disk is attached. - if boot_disk_name.is_none() { - error!(self.log, "instance boot disk is not attached"; - "boot_disk_id" => ?instance_boot_disk_id, - "instance id" => %db_instance.id()); - - return Err(InstanceStateChangeError::Other( - Error::internal_error(&format!( - "instance {} has boot disk {:?} but it is not attached", - db_instance.id(), - db_instance.boot_disk_id.as_ref(), - )), - )); - } - } - - None - }; + // The routines that maintain an instance's boot options are supposed to + // guarantee that the boot disk ID, if present, is the ID of an attached + // disk. If this invariant isn't upheld, Propolis will catch the failure + // when it processes its received VM configuration. + let boot_settings = db_instance + .boot_disk_id + .map(|id| InstanceBootSettings { order: vec![id] }); let nics = self .db_datastore diff --git a/nexus/src/app/sagas/disk_create.rs b/nexus/src/app/sagas/disk_create.rs index 51f6d29de1..5dcb3a0616 100644 --- a/nexus/src/app/sagas/disk_create.rs +++ b/nexus/src/app/sagas/disk_create.rs @@ -20,10 +20,10 @@ use omicron_common::api::external::Error; use rand::{rngs::StdRng, RngCore, SeedableRng}; use serde::Deserialize; use serde::Serialize; -use sled_agent_client::types::{CrucibleOpts, VolumeConstructionRequest}; -use std::collections::VecDeque; +use sled_agent_client::{CrucibleOpts, VolumeConstructionRequest}; use std::convert::TryFrom; use std::net::SocketAddrV6; +use std::{collections::VecDeque, net::SocketAddr}; use steno::ActionError; use steno::Node; use uuid::Uuid; @@ -506,7 +506,7 @@ async fn sdc_regions_ensure( )), ) }) - .map(|addr| addr.to_string()) + .map(SocketAddr::V6) }) .collect::, ActionError>>()?, diff --git a/nexus/src/app/sagas/instance_migrate.rs b/nexus/src/app/sagas/instance_migrate.rs index 2ddbe5b05c..644f0c42e3 100644 --- a/nexus/src/app/sagas/instance_migrate.rs +++ b/nexus/src/app/sagas/instance_migrate.rs @@ -539,7 +539,6 @@ async fn sim_instance_migrate( InstanceStateChangeRequest::Migrate( InstanceMigrationTargetParams { src_propolis_addr: src_vmm_addr.to_string(), - src_propolis_id, }, ), ) diff --git a/nexus/src/app/sagas/region_replacement_drive.rs b/nexus/src/app/sagas/region_replacement_drive.rs index 81f9a9a7eb..30cb76f7fb 100644 --- a/nexus/src/app/sagas/region_replacement_drive.rs +++ b/nexus/src/app/sagas/region_replacement_drive.rs @@ -148,7 +148,6 @@ use crate::app::{authn, authz, db}; use chrono::DateTime; use chrono::Utc; use nexus_db_model::VmmState; -use nexus_types::identity::Resource; use omicron_common::api::external::Error; use propolis_client::types::ReplaceResult; use serde::Deserialize; @@ -1502,7 +1501,6 @@ async fn execute_propolis_drive_action( .instance_issue_crucible_vcr_request() .id(disk.id()) .body(propolis_client::types::InstanceVcrReplace { - name: disk.name().to_string(), vcr_json: disk_new_volume_vcr, }) .send() diff --git a/nexus/src/app/sagas/region_replacement_finish.rs b/nexus/src/app/sagas/region_replacement_finish.rs index 2212e6fdf3..fcbf40db84 100644 --- a/nexus/src/app/sagas/region_replacement_finish.rs +++ b/nexus/src/app/sagas/region_replacement_finish.rs @@ -219,8 +219,8 @@ pub(crate) mod test { use nexus_db_queries::context::OpContext; use nexus_test_utils_macros::nexus_test; use omicron_uuid_kinds::DatasetUuid; - use sled_agent_client::types::CrucibleOpts; - use sled_agent_client::types::VolumeConstructionRequest; + use sled_agent_client::CrucibleOpts; + use sled_agent_client::VolumeConstructionRequest; use uuid::Uuid; type ControlPlaneTestContext = diff --git a/nexus/src/app/sagas/region_replacement_start.rs b/nexus/src/app/sagas/region_replacement_start.rs index aa9e83c037..bbd68d3aac 100644 --- a/nexus/src/app/sagas/region_replacement_start.rs +++ b/nexus/src/app/sagas/region_replacement_start.rs @@ -58,8 +58,8 @@ use nexus_db_queries::db::datastore::REGION_REDUNDANCY_THRESHOLD; use omicron_common::api::external::Error; use serde::Deserialize; use serde::Serialize; -use sled_agent_client::types::CrucibleOpts; -use sled_agent_client::types::VolumeConstructionRequest; +use sled_agent_client::CrucibleOpts; +use sled_agent_client::VolumeConstructionRequest; use std::net::SocketAddrV6; use steno::ActionError; use steno::Node; @@ -688,7 +688,7 @@ async fn srrs_create_fake_volume( gen: 0, opts: CrucibleOpts { id: new_volume_id, - target: vec![old_region_address.to_string()], + target: vec![old_region_address.into()], lossy: false, flush_timeout: None, key: None, @@ -793,7 +793,7 @@ pub(crate) mod test { use nexus_types::identity::Asset; use omicron_common::api::internal::shared::DatasetKind; use omicron_uuid_kinds::DatasetUuid; - use sled_agent_client::types::VolumeConstructionRequest; + use sled_agent_client::VolumeConstructionRequest; use uuid::Uuid; type ControlPlaneTestContext = diff --git a/nexus/src/app/sagas/region_snapshot_replacement_garbage_collect.rs b/nexus/src/app/sagas/region_snapshot_replacement_garbage_collect.rs index 675b2b0cb3..89c3ec2364 100644 --- a/nexus/src/app/sagas/region_snapshot_replacement_garbage_collect.rs +++ b/nexus/src/app/sagas/region_snapshot_replacement_garbage_collect.rs @@ -219,8 +219,8 @@ pub(crate) mod test { use nexus_db_queries::context::OpContext; use nexus_test_utils_macros::nexus_test; use omicron_uuid_kinds::DatasetUuid; - use sled_agent_client::types::CrucibleOpts; - use sled_agent_client::types::VolumeConstructionRequest; + use sled_agent_client::CrucibleOpts; + use sled_agent_client::VolumeConstructionRequest; use uuid::Uuid; type ControlPlaneTestContext = diff --git a/nexus/src/app/sagas/region_snapshot_replacement_start.rs b/nexus/src/app/sagas/region_snapshot_replacement_start.rs index b9ed75c288..4919919c99 100644 --- a/nexus/src/app/sagas/region_snapshot_replacement_start.rs +++ b/nexus/src/app/sagas/region_snapshot_replacement_start.rs @@ -68,8 +68,9 @@ use omicron_common::api::external::Error; use omicron_uuid_kinds::DatasetUuid; use serde::Deserialize; use serde::Serialize; -use sled_agent_client::types::CrucibleOpts; -use sled_agent_client::types::VolumeConstructionRequest; +use sled_agent_client::CrucibleOpts; +use sled_agent_client::VolumeConstructionRequest; +use std::net::SocketAddr; use std::net::SocketAddrV6; use steno::ActionError; use steno::Node; @@ -731,12 +732,12 @@ async fn rsrss_new_region_volume_create( ))); }; - let new_region_address = SocketAddrV6::new( + let new_region_address = SocketAddr::V6(SocketAddrV6::new( *new_dataset_address.ip(), ensured_region.port_number, 0, 0, - ); + )); // Create a volume to inflate the reference count of the newly created // read-only region. If this is not done it's possible that a user could @@ -753,7 +754,7 @@ async fn rsrss_new_region_volume_create( gen: 0, opts: CrucibleOpts { id: new_region_volume_id, - target: vec![new_region_address.to_string()], + target: vec![new_region_address], lossy: false, flush_timeout: None, key: None, @@ -1149,7 +1150,7 @@ pub(crate) mod test { use nexus_types::external_api::views; use nexus_types::identity::Asset; use omicron_uuid_kinds::GenericUuid; - use sled_agent_client::types::VolumeConstructionRequest; + use sled_agent_client::VolumeConstructionRequest; type ControlPlaneTestContext = nexus_test_utils::ControlPlaneTestContext; diff --git a/nexus/src/app/sagas/region_snapshot_replacement_step.rs b/nexus/src/app/sagas/region_snapshot_replacement_step.rs index a236fcf62c..fd34e80712 100644 --- a/nexus/src/app/sagas/region_snapshot_replacement_step.rs +++ b/nexus/src/app/sagas/region_snapshot_replacement_step.rs @@ -56,13 +56,12 @@ use crate::app::db::lookup::LookupPath; use crate::app::sagas::declare_saga_actions; use crate::app::{authn, authz, db}; use nexus_db_model::VmmState; -use nexus_types::identity::Resource; use omicron_common::api::external::Error; use propolis_client::types::ReplaceResult; use serde::Deserialize; use serde::Serialize; -use sled_agent_client::types::CrucibleOpts; -use sled_agent_client::types::VolumeConstructionRequest; +use sled_agent_client::CrucibleOpts; +use sled_agent_client::VolumeConstructionRequest; use std::net::SocketAddrV6; use steno::ActionError; use steno::Node; @@ -555,11 +554,13 @@ async fn rsrss_notify_upstairs( "vmm id" => ?vmm.id, ); + // N.B. The ID passed to this request must match the disk backend ID that + // sled agent supplies to Propolis when it creates the instance. Currently, + // sled agent uses the disk ID as the backend ID. let result = client .instance_issue_crucible_vcr_request() .id(disk.id()) .body(propolis_client::types::InstanceVcrReplace { - name: disk.name().to_string(), vcr_json: new_volume_vcr, }) .send() diff --git a/nexus/src/app/sagas/region_snapshot_replacement_step_garbage_collect.rs b/nexus/src/app/sagas/region_snapshot_replacement_step_garbage_collect.rs index 15c6a39651..f67a1ee31f 100644 --- a/nexus/src/app/sagas/region_snapshot_replacement_step_garbage_collect.rs +++ b/nexus/src/app/sagas/region_snapshot_replacement_step_garbage_collect.rs @@ -131,8 +131,8 @@ pub(crate) mod test { use nexus_db_queries::context::OpContext; use nexus_db_queries::db::datastore::region_snapshot_replacement; use nexus_test_utils_macros::nexus_test; - use sled_agent_client::types::CrucibleOpts; - use sled_agent_client::types::VolumeConstructionRequest; + use sled_agent_client::CrucibleOpts; + use sled_agent_client::VolumeConstructionRequest; use uuid::Uuid; type ControlPlaneTestContext = diff --git a/nexus/src/app/sagas/snapshot_create.rs b/nexus/src/app/sagas/snapshot_create.rs index 4dd958c50b..cceec7d70e 100644 --- a/nexus/src/app/sagas/snapshot_create.rs +++ b/nexus/src/app/sagas/snapshot_create.rs @@ -112,18 +112,21 @@ use omicron_uuid_kinds::{GenericUuid, PropolisUuid, SledUuid}; use rand::{rngs::StdRng, RngCore, SeedableRng}; use serde::Deserialize; use serde::Serialize; -use sled_agent_client::types::CrucibleOpts; use sled_agent_client::types::VmmIssueDiskSnapshotRequestBody; -use sled_agent_client::types::VolumeConstructionRequest; +use sled_agent_client::CrucibleOpts; +use sled_agent_client::VolumeConstructionRequest; use slog::info; use slog_error_chain::InlineErrorChain; use std::collections::BTreeMap; use std::collections::VecDeque; +use std::net::SocketAddr; use std::net::SocketAddrV6; use steno::ActionError; use steno::Node; use uuid::Uuid; +type ReplaceSocketsMap = BTreeMap; + // snapshot create saga: input parameters #[derive(Debug, Deserialize, Serialize)] @@ -539,7 +542,7 @@ async fn ssc_regions_ensure( )), ) }) - .map(|addr| addr.to_string()) + .map(SocketAddr::V6) }) .collect::, ActionError>>()?, @@ -1383,7 +1386,7 @@ async fn ssc_detach_disk_from_pantry( async fn ssc_start_running_snapshot( sagactx: NexusActionContext, -) -> Result, ActionError> { +) -> Result { let log = sagactx.user_data().log(); let osagactx = sagactx.user_data(); let params = sagactx.saga_params::()?; @@ -1409,7 +1412,7 @@ async fn ssc_start_running_snapshot( .await .map_err(ActionError::action_failed)?; - let mut map: BTreeMap = BTreeMap::new(); + let mut map: ReplaceSocketsMap = BTreeMap::new(); for (dataset, region) in datasets_and_regions { let Some(dataset_addr) = dataset.address() else { @@ -1438,28 +1441,22 @@ async fn ssc_start_running_snapshot( ); // Map from the region to the snapshot - let region_addr = format!( - "{}", - SocketAddrV6::new( - *dataset_addr.ip(), - crucible_region.port_number, - 0, - 0 - ) + let region_addr = SocketAddrV6::new( + *dataset_addr.ip(), + crucible_region.port_number, + 0, + 0, ); - let snapshot_addr = format!( - "{}", - SocketAddrV6::new( - *dataset_addr.ip(), - crucible_running_snapshot.port_number, - 0, - 0 - ) + let snapshot_addr = SocketAddrV6::new( + *dataset_addr.ip(), + crucible_running_snapshot.port_number, + 0, + 0, ); info!(log, "map {} to {}", region_addr, snapshot_addr); - map.insert(region_addr, snapshot_addr.clone()); + map.insert(region_addr, snapshot_addr); // Once snapshot has been validated, and running snapshot has been // started, add an entry in the region_snapshot table to correspond to @@ -1470,7 +1467,7 @@ async fn ssc_start_running_snapshot( dataset_id: dataset.id().into(), region_id: region.id(), snapshot_id, - snapshot_addr, + snapshot_addr: snapshot_addr.to_string(), volume_references: 0, // to be filled later deleting: false, }) @@ -1570,7 +1567,7 @@ async fn ssc_create_volume_record( // read-only crucible agent downstairs (corresponding to this snapshot) // launched through this saga. let replace_sockets_map = - sagactx.lookup::>("replace_sockets_map")?; + sagactx.lookup::("replace_sockets_map")?; let snapshot_volume_construction_request: VolumeConstructionRequest = create_snapshot_from_disk( &disk_volume_construction_request, @@ -1694,7 +1691,7 @@ async fn ssc_release_volume_lock( /// VolumeConstructionRequest and modifying it accordingly. fn create_snapshot_from_disk( disk: &VolumeConstructionRequest, - socket_map: &BTreeMap, + socket_map: &ReplaceSocketsMap, ) -> anyhow::Result { // When copying a disk's VolumeConstructionRequest to turn it into a // snapshot: @@ -1756,9 +1753,19 @@ fn create_snapshot_from_disk( if work.socket_modification_required { for target in &mut opts.target { - target.clone_from(socket_map.get(target).ok_or_else( - || anyhow!("target {} not found in map!", target), - )?); + let target = match target { + SocketAddr::V6(v6) => v6, + SocketAddr::V4(_) => { + anyhow::bail!( + "unexpected IPv4 address in VCR: {:?}", + work.vcr_part + ) + } + }; + + *target = *socket_map.get(target).ok_or_else(|| { + anyhow!("target {} not found in map!", target) + })?; } } } @@ -1799,7 +1806,7 @@ mod test { use omicron_common::api::external::InstanceCpuCount; use omicron_common::api::external::Name; use omicron_common::api::external::NameOrId; - use sled_agent_client::types::CrucibleOpts; + use sled_agent_client::CrucibleOpts; use sled_agent_client::TestInterfaces as SledAgentTestInterfaces; use std::str::FromStr; @@ -1834,9 +1841,9 @@ mod test { lossy: false, read_only: true, target: vec![ - "[fd00:1122:3344:101::8]:19001".into(), - "[fd00:1122:3344:101::7]:19001".into(), - "[fd00:1122:3344:101::6]:19001".into(), + "[fd00:1122:3344:101::8]:19001".parse().unwrap(), + "[fd00:1122:3344:101::7]:19001".parse().unwrap(), + "[fd00:1122:3344:101::6]:19001".parse().unwrap(), ], cert_pem: None, key_pem: None, @@ -1860,34 +1867,34 @@ mod test { lossy: false, read_only: false, target: vec![ - "[fd00:1122:3344:101::8]:19002".into(), - "[fd00:1122:3344:101::7]:19002".into(), - "[fd00:1122:3344:101::6]:19002".into(), + "[fd00:1122:3344:101::8]:19002".parse().unwrap(), + "[fd00:1122:3344:101::7]:19002".parse().unwrap(), + "[fd00:1122:3344:101::6]:19002".parse().unwrap(), ], cert_pem: None, key_pem: None, root_cert_pem: None, flush_timeout: None, - control: Some("127.0.0.1:12345".into()), + control: Some("127.0.0.1:12345".parse().unwrap()), } }, ], }; - let mut replace_sockets: BTreeMap = BTreeMap::new(); + let mut replace_sockets = ReplaceSocketsMap::new(); // Replacements for top level Region only replace_sockets.insert( - "[fd00:1122:3344:101::6]:19002".into(), - "[XXXX:1122:3344:101::6]:9000".into(), + "[fd00:1122:3344:101::6]:19002".parse().unwrap(), + "[fd01:1122:3344:101::6]:9000".parse().unwrap(), ); replace_sockets.insert( - "[fd00:1122:3344:101::7]:19002".into(), - "[XXXX:1122:3344:101::7]:9000".into(), + "[fd00:1122:3344:101::7]:19002".parse().unwrap(), + "[fd01:1122:3344:101::7]:9000".parse().unwrap(), ); replace_sockets.insert( - "[fd00:1122:3344:101::8]:19002".into(), - "[XXXX:1122:3344:101::8]:9000".into(), + "[fd00:1122:3344:101::8]:19002".parse().unwrap(), + "[fd01:1122:3344:101::8]:9000".parse().unwrap(), ); let snapshot = diff --git a/nexus/src/app/sagas/volume_delete.rs b/nexus/src/app/sagas/volume_delete.rs index a8ded4e33c..89d8306265 100644 --- a/nexus/src/app/sagas/volume_delete.rs +++ b/nexus/src/app/sagas/volume_delete.rs @@ -336,7 +336,7 @@ async fn svd_delete_crucible_snapshot_records( /// It's insufficient to rely on the struct of CrucibleResources to clean up /// that is returned as part of svd_decrease_crucible_resource_count. Imagine a /// disk that is composed of three regions (a subset of -/// [`sled_agent_client::types::VolumeConstructionRequest`] is shown here): +/// [`sled_agent_client::VolumeConstructionRequest`] is shown here): /// /// ```json /// { diff --git a/nexus/src/app/sagas/volume_remove_rop.rs b/nexus/src/app/sagas/volume_remove_rop.rs index b614495615..0f81356365 100644 --- a/nexus/src/app/sagas/volume_remove_rop.rs +++ b/nexus/src/app/sagas/volume_remove_rop.rs @@ -10,7 +10,7 @@ use nexus_db_queries::db; use omicron_common::api::external::Error; use serde::Deserialize; use serde::Serialize; -use sled_agent_client::types::VolumeConstructionRequest; +use sled_agent_client::VolumeConstructionRequest; use steno::{ActionError, Node}; use uuid::Uuid; diff --git a/nexus/tests/integration_tests/volume_management.rs b/nexus/tests/integration_tests/volume_management.rs index f0eb294e58..b059aae12b 100644 --- a/nexus/tests/integration_tests/volume_management.rs +++ b/nexus/tests/integration_tests/volume_management.rs @@ -55,6 +55,7 @@ use omicron_common::api::external::Name; use omicron_common::api::internal; use omicron_test_utils::dev::poll::wait_for_condition; use omicron_test_utils::dev::poll::CondCheckError; +use omicron_uuid_kinds::DatasetUuid; use omicron_uuid_kinds::DownstairsKind; use omicron_uuid_kinds::DownstairsRegionKind; use omicron_uuid_kinds::GenericUuid; @@ -64,9 +65,9 @@ use omicron_uuid_kinds::UpstairsRepairKind; use omicron_uuid_kinds::UpstairsSessionKind; use rand::prelude::SliceRandom; use rand::{rngs::StdRng, SeedableRng}; -use sled_agent_client::types::{CrucibleOpts, VolumeConstructionRequest}; +use sled_agent_client::{CrucibleOpts, VolumeConstructionRequest}; use std::collections::HashSet; -use std::net::SocketAddrV6; +use std::net::{SocketAddr, SocketAddrV6}; use std::sync::Arc; use uuid::Uuid; @@ -2206,44 +2207,44 @@ async fn test_keep_your_targets_straight(cptestctx: &ControlPlaneTestContext) { // insert those here manually. // (dataset_id, region_id, snapshot_id, snapshot_addr) - let region_snapshots = vec![ + let region_snapshots: Vec<(DatasetUuid, Uuid, Uuid, SocketAddr)> = vec![ // first snapshot-create ( zpool0.datasets[0].id, Uuid::new_v4(), Uuid::new_v4(), - String::from("[fd00:1122:3344:101::7]:19016"), + "[fd00:1122:3344:101::7]:19016".parse().unwrap(), ), ( zpool1.datasets[0].id, Uuid::new_v4(), Uuid::new_v4(), - String::from("[fd00:1122:3344:102::7]:19016"), + "[fd00:1122:3344:102::7]:19016".parse().unwrap(), ), ( zpool2.datasets[0].id, Uuid::new_v4(), Uuid::new_v4(), - String::from("[fd00:1122:3344:103::7]:19016"), + "[fd00:1122:3344:103::7]:19016".parse().unwrap(), ), // second snapshot-create ( zpool0.datasets[0].id, Uuid::new_v4(), Uuid::new_v4(), - String::from("[fd00:1122:3344:101::7]:19016"), // duplicate! + "[fd00:1122:3344:101::7]:19016".parse().unwrap(), ), ( zpool3.datasets[0].id, Uuid::new_v4(), Uuid::new_v4(), - String::from("[fd00:1122:3344:104::7]:19016"), + "[fd00:1122:3344:104::7]:19016".parse().unwrap(), ), ( zpool2.datasets[0].id, Uuid::new_v4(), Uuid::new_v4(), - String::from("[fd00:1122:3344:103::7]:19017"), + "[fd00:1122:3344:103::7]:19017".parse().unwrap(), ), ]; @@ -2258,7 +2259,7 @@ async fn test_keep_your_targets_straight(cptestctx: &ControlPlaneTestContext) { dataset_id: (*dataset_id).into(), region_id: *region_id, snapshot_id: *snapshot_id, - snapshot_addr: snapshot_addr.clone(), + snapshot_addr: snapshot_addr.to_string(), volume_references: 0, deleting: false, }) @@ -2283,9 +2284,9 @@ async fn test_keep_your_targets_straight(cptestctx: &ControlPlaneTestContext) { opts: CrucibleOpts { id: Uuid::new_v4(), target: vec![ - region_snapshots[0].3.clone(), - region_snapshots[1].3.clone(), - region_snapshots[2].3.clone(), + region_snapshots[0].3, + region_snapshots[1].3, + region_snapshots[2].3, ], lossy: false, flush_timeout: None, @@ -2379,7 +2380,7 @@ async fn test_keep_your_targets_straight(cptestctx: &ControlPlaneTestContext) { dataset_id: (*dataset_id).into(), region_id: *region_id, snapshot_id: *snapshot_id, - snapshot_addr: snapshot_addr.clone(), + snapshot_addr: snapshot_addr.to_string(), volume_references: 0, deleting: false, }) @@ -2404,9 +2405,9 @@ async fn test_keep_your_targets_straight(cptestctx: &ControlPlaneTestContext) { opts: CrucibleOpts { id: Uuid::new_v4(), target: vec![ - region_snapshots[3].3.clone(), - region_snapshots[4].3.clone(), - region_snapshots[5].3.clone(), + region_snapshots[3].3, + region_snapshots[4].3, + region_snapshots[5].3, ], lossy: false, flush_timeout: None, @@ -3659,7 +3660,7 @@ impl TestReadOnlyRegionReferenceUsage { gen: 1, opts: CrucibleOpts { id: Uuid::new_v4(), - target: vec![self.region_address.to_string()], + target: vec![self.region_address.into()], lossy: false, flush_timeout: None, key: None, @@ -3789,7 +3790,7 @@ impl TestReadOnlyRegionReferenceUsage { gen: 1, opts: CrucibleOpts { id: Uuid::new_v4(), - target: vec![self.region_address.to_string()], + target: vec![self.region_address.into()], lossy: false, flush_timeout: None, key: None, @@ -3822,7 +3823,7 @@ impl TestReadOnlyRegionReferenceUsage { gen: 1, opts: CrucibleOpts { id: Uuid::new_v4(), - target: vec![self.region_address.to_string()], + target: vec![self.region_address.into()], lossy: false, flush_timeout: None, key: None, @@ -3857,7 +3858,7 @@ impl TestReadOnlyRegionReferenceUsage { gen: 1, opts: CrucibleOpts { id: Uuid::new_v4(), - target: vec![self.region_address.to_string()], + target: vec![self.region_address.into()], lossy: false, flush_timeout: None, key: None, @@ -5377,30 +5378,30 @@ async fn test_migrate_to_ref_count_with_records_region_snapshot_deleting( let zpool3 = iter.next().expect("Expected four zpools"); // (dataset_id, region_id, snapshot_id, snapshot_addr) - let region_snapshots = vec![ + let region_snapshots: Vec<(DatasetUuid, Uuid, Uuid, SocketAddr)> = vec![ ( zpool0.datasets[0].id, Uuid::new_v4(), Uuid::new_v4(), - String::from("[fd00:1122:3344:101::7]:19016"), + "[fd00:1122:3344:101::7]:19016".parse().unwrap(), ), ( zpool1.datasets[0].id, Uuid::new_v4(), Uuid::new_v4(), - String::from("[fd00:1122:3344:102::7]:19016"), + "[fd00:1122:3344:102::7]:19016".parse().unwrap(), ), ( zpool2.datasets[0].id, Uuid::new_v4(), Uuid::new_v4(), - String::from("[fd00:1122:3344:103::7]:19016"), + "[fd00:1122:3344:103::7]:19016".parse().unwrap(), ), ( zpool3.datasets[0].id, Uuid::new_v4(), Uuid::new_v4(), - String::from("[fd00:1122:3344:104::7]:19016"), + "[fd00:1122:3344:104::7]:19016".parse().unwrap(), ), ]; @@ -5413,7 +5414,7 @@ async fn test_migrate_to_ref_count_with_records_region_snapshot_deleting( dataset_id: to_db_typed_uuid(*dataset_id), region_id: *region_id, snapshot_id: *snapshot_id, - snapshot_addr: snapshot_addr.clone(), + snapshot_addr: snapshot_addr.to_string(), volume_references: 0, deleting: false, }) @@ -5441,9 +5442,9 @@ async fn test_migrate_to_ref_count_with_records_region_snapshot_deleting( opts: CrucibleOpts { id: Uuid::new_v4(), target: vec![ - region_snapshots[0].3.clone(), - region_snapshots[1].3.clone(), - region_snapshots[2].3.clone(), + region_snapshots[0].3, + region_snapshots[1].3, + region_snapshots[2].3, ], lossy: false, flush_timeout: None, @@ -5479,9 +5480,9 @@ async fn test_migrate_to_ref_count_with_records_region_snapshot_deleting( opts: CrucibleOpts { id: Uuid::new_v4(), target: vec![ - region_snapshots[1].3.clone(), - region_snapshots[2].3.clone(), - region_snapshots[3].3.clone(), + region_snapshots[1].3, + region_snapshots[2].3, + region_snapshots[3].3, ], lossy: false, flush_timeout: None, @@ -5519,7 +5520,10 @@ async fn test_migrate_to_ref_count_with_records_region_snapshot_deleting( ); assert_eq!(region_snapshot_to_delete.region_id, region_snapshots[0].1); assert_eq!(region_snapshot_to_delete.snapshot_id, region_snapshots[0].2); - assert_eq!(region_snapshot_to_delete.snapshot_addr, region_snapshots[0].3); + assert_eq!( + region_snapshot_to_delete.snapshot_addr.parse::().unwrap(), + region_snapshots[0].3 + ); assert_eq!(region_snapshot_to_delete.volume_references, 0); assert_eq!(region_snapshot_to_delete.deleting, true); @@ -6050,9 +6054,9 @@ async fn test_no_zombie_read_only_regions(cptestctx: &ControlPlaneTestContext) { opts: CrucibleOpts { id: Uuid::new_v4(), target: vec![ - region_addrs[0].to_string(), - region_addrs[1].to_string(), - region_addrs[2].to_string(), + region_addrs[0].into(), + region_addrs[1].into(), + region_addrs[2].into(), ], lossy: false, flush_timeout: None, @@ -6236,9 +6240,9 @@ async fn test_no_zombie_read_write_regions( opts: CrucibleOpts { id: Uuid::new_v4(), target: vec![ - region_addrs[0].to_string(), - region_addrs[1].to_string(), - region_addrs[2].to_string(), + region_addrs[0].into(), + region_addrs[1].into(), + region_addrs[2].into(), ], lossy: false, flush_timeout: None, diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index c7d2b36de4..81d9211104 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -2570,36 +2570,6 @@ } ] }, - "BootOrderEntry": { - "description": "An entry in the boot order stored in a [`BootSettings`] component.\n\n
JSON schema\n\n```json { \"description\": \"An entry in the boot order stored in a [`BootSettings`] component.\", \"type\": \"object\", \"required\": [ \"name\" ], \"properties\": { \"name\": { \"description\": \"The name of another component in the spec that Propolis should try to boot from.\\n\\nCurrently, only disk device components are supported.\", \"type\": \"string\" } } } ```
", - "type": "object", - "properties": { - "name": { - "description": "The name of another component in the spec that Propolis should try to boot from.\n\nCurrently, only disk device components are supported.", - "type": "string" - } - }, - "required": [ - "name" - ] - }, - "BootSettings": { - "description": "Settings supplied to the guest's firmware image that specify the order in which it should consider its options when selecting a device to try to boot from.\n\n
JSON schema\n\n```json { \"description\": \"Settings supplied to the guest's firmware image that specify the order in which it should consider its options when selecting a device to try to boot from.\", \"type\": \"object\", \"required\": [ \"order\" ], \"properties\": { \"order\": { \"description\": \"An ordered list of components to attempt to boot from.\", \"type\": \"array\", \"items\": { \"$ref\": \"#/components/schemas/BootOrderEntry\" } } }, \"additionalProperties\": false } ```
", - "type": "object", - "properties": { - "order": { - "description": "An ordered list of components to attempt to boot from.", - "type": "array", - "items": { - "$ref": "#/components/schemas/BootOrderEntry" - } - } - }, - "required": [ - "order" - ], - "additionalProperties": false - }, "BootstoreStatus": { "type": "object", "properties": { @@ -2891,59 +2861,6 @@ } ] }, - "CrucibleOpts": { - "description": "CrucibleOpts\n\n
JSON schema\n\n```json { \"type\": \"object\", \"required\": [ \"id\", \"lossy\", \"read_only\", \"target\" ], \"properties\": { \"cert_pem\": { \"type\": [ \"string\", \"null\" ] }, \"control\": { \"type\": [ \"string\", \"null\" ] }, \"flush_timeout\": { \"type\": [ \"number\", \"null\" ], \"format\": \"float\" }, \"id\": { \"type\": \"string\", \"format\": \"uuid\" }, \"key\": { \"type\": [ \"string\", \"null\" ] }, \"key_pem\": { \"type\": [ \"string\", \"null\" ] }, \"lossy\": { \"type\": \"boolean\" }, \"read_only\": { \"type\": \"boolean\" }, \"root_cert_pem\": { \"type\": [ \"string\", \"null\" ] }, \"target\": { \"type\": \"array\", \"items\": { \"type\": \"string\" } } } } ```
", - "type": "object", - "properties": { - "cert_pem": { - "nullable": true, - "type": "string" - }, - "control": { - "nullable": true, - "type": "string" - }, - "flush_timeout": { - "nullable": true, - "type": "number", - "format": "float" - }, - "id": { - "type": "string", - "format": "uuid" - }, - "key": { - "nullable": true, - "type": "string" - }, - "key_pem": { - "nullable": true, - "type": "string" - }, - "lossy": { - "type": "boolean" - }, - "read_only": { - "type": "boolean" - }, - "root_cert_pem": { - "nullable": true, - "type": "string" - }, - "target": { - "type": "array", - "items": { - "type": "string" - } - } - }, - "required": [ - "id", - "lossy", - "read_only", - "target" - ] - }, "DatasetConfig": { "description": "Configuration information necessary to request a single dataset.\n\nThese datasets are tracked directly by Nexus.", "type": "object", @@ -3250,34 +3167,6 @@ "identity" ] }, - "DiskRequest": { - "description": "DiskRequest\n\n
JSON schema\n\n```json { \"type\": \"object\", \"required\": [ \"device\", \"name\", \"read_only\", \"slot\", \"volume_construction_request\" ], \"properties\": { \"device\": { \"type\": \"string\" }, \"name\": { \"type\": \"string\" }, \"read_only\": { \"type\": \"boolean\" }, \"slot\": { \"$ref\": \"#/components/schemas/Slot\" }, \"volume_construction_request\": { \"$ref\": \"#/components/schemas/VolumeConstructionRequest\" } } } ```
", - "type": "object", - "properties": { - "device": { - "type": "string" - }, - "name": { - "type": "string" - }, - "read_only": { - "type": "boolean" - }, - "slot": { - "$ref": "#/components/schemas/Slot" - }, - "volume_construction_request": { - "$ref": "#/components/schemas/VolumeConstructionRequest" - } - }, - "required": [ - "device", - "name", - "read_only", - "slot", - "volume_construction_request" - ] - }, "DiskRuntimeState": { "description": "Runtime state of the Disk, which includes its attach state and some minimal metadata", "type": "object", @@ -3866,12 +3755,65 @@ } ] }, + "InstanceBootSettings": { + "description": "Configures how an instance is told to try to boot.", + "type": "object", + "properties": { + "order": { + "description": "Propolis should tell guest firmware to try to boot from devices in this order.", + "type": "array", + "items": { + "type": "string", + "format": "uuid" + } + } + }, + "required": [ + "order" + ] + }, "InstanceCpuCount": { "description": "The number of CPUs in an Instance", "type": "integer", "format": "uint16", "minimum": 0 }, + "InstanceDisk": { + "description": "A request to attach a disk to an instance.", + "type": "object", + "properties": { + "disk_id": { + "description": "The disk's UUID.", + "type": "string", + "format": "uuid" + }, + "name": { + "description": "The disk's name, used to generate the serial number for the virtual disk exposed to the guest.", + "type": "string" + }, + "read_only": { + "description": "True if the disk is read-only.", + "type": "boolean" + }, + "slot": { + "description": "The logical slot number assigned to the disk in its database record.", + "type": "integer", + "format": "uint8", + "minimum": 0 + }, + "vcr_json": { + "description": "A JSON representation of the Crucible volume construction request for this attachment.", + "type": "string" + } + }, + "required": [ + "disk_id", + "name", + "read_only", + "slot", + "vcr_json" + ] + }, "InstanceEnsureBody": { "description": "The body of a request to ensure that a instance and VMM are known to a sled agent.", "type": "object", @@ -3978,7 +3920,7 @@ "nullable": true, "allOf": [ { - "$ref": "#/components/schemas/BootSettings" + "$ref": "#/components/schemas/InstanceBootSettings" } ] }, @@ -3992,7 +3934,7 @@ "disks": { "type": "array", "items": { - "$ref": "#/components/schemas/DiskRequest" + "$ref": "#/components/schemas/InstanceDisk" } }, "ephemeral_ip": { @@ -4062,16 +4004,10 @@ "src_propolis_addr": { "description": "The address of the Propolis server that will serve as the migration source.", "type": "string" - }, - "src_propolis_id": { - "description": "The Propolis ID of the migration source.", - "type": "string", - "format": "uuid" } }, "required": [ - "src_propolis_addr", - "src_propolis_id" + "src_propolis_addr" ] }, "InstanceProperties": { @@ -5682,12 +5618,6 @@ "vmm_state" ] }, - "Slot": { - "description": "A stable index which is translated by Propolis into a PCI BDF, visible to the guest.\n\n
JSON schema\n\n```json { \"description\": \"A stable index which is translated by Propolis into a PCI BDF, visible to the guest.\", \"type\": \"integer\", \"format\": \"uint8\", \"minimum\": 0.0 } ```
", - "type": "integer", - "format": "uint8", - "minimum": 0 - }, "SourceNatConfig": { "description": "An IP address and port range used for source NAT, i.e., making outbound network connections from guests or services.", "type": "object", @@ -6269,151 +6199,6 @@ "format": "uint32", "minimum": 0 }, - "VolumeConstructionRequest": { - "description": "VolumeConstructionRequest\n\n
JSON schema\n\n```json { \"oneOf\": [ { \"type\": \"object\", \"required\": [ \"block_size\", \"id\", \"sub_volumes\", \"type\" ], \"properties\": { \"block_size\": { \"type\": \"integer\", \"format\": \"uint64\", \"minimum\": 0.0 }, \"id\": { \"type\": \"string\", \"format\": \"uuid\" }, \"read_only_parent\": { \"allOf\": [ { \"$ref\": \"#/components/schemas/VolumeConstructionRequest\" } ] }, \"sub_volumes\": { \"type\": \"array\", \"items\": { \"$ref\": \"#/components/schemas/VolumeConstructionRequest\" } }, \"type\": { \"type\": \"string\", \"enum\": [ \"volume\" ] } } }, { \"type\": \"object\", \"required\": [ \"block_size\", \"id\", \"type\", \"url\" ], \"properties\": { \"block_size\": { \"type\": \"integer\", \"format\": \"uint64\", \"minimum\": 0.0 }, \"id\": { \"type\": \"string\", \"format\": \"uuid\" }, \"type\": { \"type\": \"string\", \"enum\": [ \"url\" ] }, \"url\": { \"type\": \"string\" } } }, { \"type\": \"object\", \"required\": [ \"block_size\", \"blocks_per_extent\", \"extent_count\", \"gen\", \"opts\", \"type\" ], \"properties\": { \"block_size\": { \"type\": \"integer\", \"format\": \"uint64\", \"minimum\": 0.0 }, \"blocks_per_extent\": { \"type\": \"integer\", \"format\": \"uint64\", \"minimum\": 0.0 }, \"extent_count\": { \"type\": \"integer\", \"format\": \"uint32\", \"minimum\": 0.0 }, \"gen\": { \"type\": \"integer\", \"format\": \"uint64\", \"minimum\": 0.0 }, \"opts\": { \"$ref\": \"#/components/schemas/CrucibleOpts\" }, \"type\": { \"type\": \"string\", \"enum\": [ \"region\" ] } } }, { \"type\": \"object\", \"required\": [ \"block_size\", \"id\", \"path\", \"type\" ], \"properties\": { \"block_size\": { \"type\": \"integer\", \"format\": \"uint64\", \"minimum\": 0.0 }, \"id\": { \"type\": \"string\", \"format\": \"uuid\" }, \"path\": { \"type\": \"string\" }, \"type\": { \"type\": \"string\", \"enum\": [ \"file\" ] } } } ] } ```
", - "oneOf": [ - { - "type": "object", - "properties": { - "block_size": { - "type": "integer", - "format": "uint64", - "minimum": 0 - }, - "id": { - "type": "string", - "format": "uuid" - }, - "read_only_parent": { - "nullable": true, - "allOf": [ - { - "$ref": "#/components/schemas/VolumeConstructionRequest" - } - ] - }, - "sub_volumes": { - "type": "array", - "items": { - "$ref": "#/components/schemas/VolumeConstructionRequest" - } - }, - "type": { - "type": "string", - "enum": [ - "volume" - ] - } - }, - "required": [ - "block_size", - "id", - "sub_volumes", - "type" - ] - }, - { - "type": "object", - "properties": { - "block_size": { - "type": "integer", - "format": "uint64", - "minimum": 0 - }, - "id": { - "type": "string", - "format": "uuid" - }, - "type": { - "type": "string", - "enum": [ - "url" - ] - }, - "url": { - "type": "string" - } - }, - "required": [ - "block_size", - "id", - "type", - "url" - ] - }, - { - "type": "object", - "properties": { - "block_size": { - "type": "integer", - "format": "uint64", - "minimum": 0 - }, - "blocks_per_extent": { - "type": "integer", - "format": "uint64", - "minimum": 0 - }, - "extent_count": { - "type": "integer", - "format": "uint32", - "minimum": 0 - }, - "gen": { - "type": "integer", - "format": "uint64", - "minimum": 0 - }, - "opts": { - "$ref": "#/components/schemas/CrucibleOpts" - }, - "type": { - "type": "string", - "enum": [ - "region" - ] - } - }, - "required": [ - "block_size", - "blocks_per_extent", - "extent_count", - "gen", - "opts", - "type" - ] - }, - { - "type": "object", - "properties": { - "block_size": { - "type": "integer", - "format": "uint64", - "minimum": 0 - }, - "id": { - "type": "string", - "format": "uuid" - }, - "path": { - "type": "string" - }, - "type": { - "type": "string", - "enum": [ - "file" - ] - } - }, - "required": [ - "block_size", - "id", - "path", - "type" - ] - } - ] - }, "VpcFirewallRuleAction": { "type": "string", "enum": [ diff --git a/package-manifest.toml b/package-manifest.toml index b28ac7d59f..8140ce43af 100644 --- a/package-manifest.toml +++ b/package-manifest.toml @@ -624,10 +624,10 @@ service_name = "propolis-server" only_for_targets.image = "standard" source.type = "prebuilt" source.repo = "propolis" -source.commit = "220a6f367c18f2452dbc4fa9086f3fe73b961739" +source.commit = "d4529fd8247386b422b78e1203315d5baea5ea8b" # The SHA256 digest is automatically posted to: # https://buildomat.eng.oxide.computer/public/file/oxidecomputer/propolis/image//propolis-server.sha256.txt -source.sha256 = "964bf262677496118f8cea95c257d0a57c76ddca70733217b0666657b53bd6e6" +source.sha256 = "3e5995281e2b222fbfa3537fcc846e0706361db5ab57de6656811871bcc04cc3" output.type = "zone" [package.mg-ddm-gz] diff --git a/sled-agent/src/instance.rs b/sled-agent/src/instance.rs index 21832fde23..8b327ddd9a 100644 --- a/sled-agent/src/instance.rs +++ b/sled-agent/src/instance.rs @@ -443,8 +443,8 @@ struct InstanceRunner { dhcp_config: DhcpCfg, // Disk related properties - requested_disks: Vec, - boot_settings: Option, + requested_disks: Vec, + boot_settings: Option, cloud_init_bytes: Option>, // Internal State management @@ -847,84 +847,365 @@ impl InstanceRunner { res.map(|_| ()) } - /// Sends an instance ensure request to this instance's Propolis. + /// Sends an instance ensure request to this instance's Propolis, + /// constructing its configuration from the fields in `self` that describe + /// the instance's virtual hardware configuration. async fn propolis_ensure_inner( &self, client: &PropolisClient, running_zone: &RunningZone, migrate: Option, ) -> Result<(), Error> { - let nics = running_zone + // A bit of history helps to explain the workings of the rest of this + // function. + // + // In the past, the Propolis API accepted an InstanceEnsureRequest + // struct that described a VM's hardware configuration at more or less + // the level of specificity used in Nexus's database. Callers passed + // this struct irrespective of whether they were starting a brand new VM + // migrating from an existing VM in some other Propolis. It was + // Propolis's job to convert any insufficiently-specific parameters + // passed in its API into the concrete details needed to set up VM + // components (e.g., converting device slot numbers into concrete PCI + // bus/device/function numbers). + // + // The Propolis VM creation API used below differs from this scheme in + // two important ways: + // + // 1. Callers are responsible for filling in all the details of a VM's + // configuration (e.g., choosing PCI BDFs for PCI devices). + // 2. During a live migration, the migration target inherits most of its + // configuration directly from the migration source. Propolis only + // allows clients to specify new configurations for the specific set + // of components that it expects to be reconfigured over a migration. + // These are described further below. + // + // This scheme aims to + // + // 1. prevent bugs where an instance can't migrate because the control + // plane has specified conflicting configurations to the source and + // target, and + // 2. maximize the updateability of VM configurations by allowing the + // control plane, which (at least in Nexus) is relatively easy to + // update, to make the rules about how Nexus instance configurations + // are realized in Propolis VMs. + // + // See propolis#804 for even more context on this API's design. + // + // A "virtual platform" is a set of rules that describe how to realize a + // Propolis VM configuration from a control plane instance description. + // The logic below encodes the "Oxide MVP" virtual platform that + // Propolis implicitly implemented in its legacy instance-ensure API. In + // the future there will be additional virtual platforms that may encode + // different rules and configuration options. + // + // TODO(#615): Eventually this logic should move to a robust virtual + // platform library in Nexus. + + use propolis_client::{ + types::{ + BlobStorageBackend, Board, BootOrderEntry, BootSettings, + Chipset, ComponentV0, CrucibleStorageBackend, + InstanceInitializationMethod, NvmeDisk, QemuPvpanic, + ReplacementComponent, SerialPort, SerialPortNumber, VirtioDisk, + VirtioNetworkBackend, VirtioNic, + }, + PciPath, SpecKey, + }; + + // Define some local helper structs for unpacking hardware descriptions + // into the types Propolis wants to see in its specifications. + struct DiskComponents { + id: Uuid, + device: NvmeDisk, + backend: CrucibleStorageBackend, + } + + struct NicComponents { + id: Uuid, + device: VirtioNic, + backend: VirtioNetworkBackend, + } + + // Assemble the list of NVMe disks associated with this instance. + let disks: Vec = self + .requested_disks + .iter() + .map(|disk| -> Result { + // One of the details Propolis asks clients to fill in is the + // serial number for each NVMe disk. It's important that this + // number remain stable because it can be written to an + // instance's nonvolatile EFI variables, specifically its boot + // order variables, which can be undercut if a serial number + // changes. + // + // The old version of the Propolis API generated serial numbers + // by taking the control plane disk name and padding it with + // zero bytes. Match this behavior here. + // + // Note that this scheme violates version 1.2.1 of the NVMe + // specification: section 1.5 says that string fields like this + // one should be left-justified and padded with spaces, not + // zeroes. Future versions of this logic may switch to this + // behavior. + let serial_number = + propolis_client::support::nvme_serial_from_str( + &disk.name, 0, + ); + + Ok(DiskComponents { + id: disk.disk_id, + device: NvmeDisk { + backend_id: SpecKey::Uuid(disk.disk_id), + // The old Propolis API added 16 to disk slot numbers to + // get their PCI device numbers. + pci_path: PciPath::new(0, disk.slot + 0x10, 0)?, + serial_number, + }, + backend: CrucibleStorageBackend { + readonly: disk.read_only, + request_json: disk.vcr_json.0.clone(), + }, + }) + }) + .collect::, _>>()?; + + // Next, assemble the list of guest NICs. + let nics: Vec = running_zone .opte_ports() - .map(|port| { - self.requested_nics + .map(|port| -> Result { + let nic = self + .requested_nics .iter() // We expect to match NIC slots to OPTE port slots. // Error out if we can't find a NIC for a port. - .position(|nic| nic.slot == port.slot()) + .find(|nic| nic.slot == port.slot()) .ok_or(Error::Opte( illumos_utils::opte::Error::NoNicforPort( port.name().into(), port.slot().into(), ), + ))?; + + Ok(NicComponents { + id: nic.id, + device: VirtioNic { + backend_id: SpecKey::Uuid(nic.id), + interface_id: nic.id, + // The old Propolis API added 8 to NIC slot numbers to + // get their PCI device numbers. + pci_path: PciPath::new(0, nic.slot + 8, 0)?, + }, + backend: VirtioNetworkBackend { + vnic_name: port.name().to_string(), + }, + }) + }) + .collect::, _>>()?; + + // When a VM migrates, the migration target inherits most of its + // configuration directly from the migration source. The exceptions are + // cases where the target VM needs new parameters in order to interface + // with services on its sled or in the rest of the rack. These include + // + // - Crucible disks: the target needs to connect to its downstairs + // instances with new generation numbers supplied from Nexus + // - OPTE ports: the target needs to bind its VNICs to the correct + // devices for its new host; those devices may have different names + // than their counterparts on the source host + // + // If this is a request to migrate, construct a list of source component + // specifications that this caller intends to replace. Otherwise, + // construct a complete instance specification for a new VM. + let request = if let Some(params) = migrate { + // TODO(#6073): The migration ID is sent in the VMM registration + // request and isn't part of the migration target params (despite + // being known when the migration-start request is sent). If it were + // sent here it would no longer be necessary to read the ID back + // from the saved VMM/instance state. + // + // In practice, Nexus should (by the construction of the instance + // migration saga) always initialize a migration-destination VMM + // with a valid migration ID. But since that invariant is + // technically part of a different component, return an error here + // instead of unwrapping if it's violated. + let migration_id = self + .state + .migration_in() + .ok_or_else(|| { + Error::Migration(anyhow::anyhow!( + "migration requested but no migration ID was \ + supplied when this VMM was registered" )) - .map(|pos| { - let nic = &self.requested_nics[pos]; - propolis_client::types::NetworkInterfaceRequest { - interface_id: nic.id, - name: port.name().to_string(), - slot: propolis_client::types::Slot(port.slot()), - } + })? + .migration_id; + + // Add the new Crucible backends to the list of source instance spec + // elements that should be replaced in the target's spec. + let mut elements_to_replace: std::collections::HashMap<_, _> = + disks + .into_iter() + .map(|disk| { + ( + // N.B. This ID must match the one supplied when the + // instance was started. + disk.id.to_string(), + ReplacementComponent::CrucibleStorageBackend( + disk.backend, + ), + ) }) - }) - .collect::, _>>()?; - - let migrate = match migrate { - Some(params) => { - let migration_id = self.state - .migration_in() - // TODO(eliza): This is a bit of an unfortunate dance: the - // initial instance-ensure-registered request is what sends - // the migration ID, but it's the subsequent - // instance-ensure-state request (which we're handling here) - // that includes migration the source VMM's UUID and IP - // address. Because the API currently splits the migration - // IDs between the instance-ensure-registered and - // instance-ensure-state requests, we have to stash the - // migration ID in an `Option` and `expect()` it here, - // panicking if we get an instance-ensure-state request with - // a source Propolis ID if the instance wasn't registered - // with a migration in ID. - // - // This is kind of a shame. Eventually, we should consider - // reworking the API ensure-state request contains the - // migration ID, and we don't have to unwrap here. See: - // https://github.com/oxidecomputer/omicron/issues/6073 - .expect("if we have migration target params, we should also have a migration in") - .migration_id; - Some(propolis_client::types::InstanceMigrateInitiateRequest { - src_addr: params.src_propolis_addr.to_string(), - src_uuid: params.src_propolis_id, + .collect(); + + // Add new OPTE backend configuration to the replacement list. + elements_to_replace.extend(nics.into_iter().map(|nic| { + ( + // N.B. This ID must match the one supplied when the + // instance was started. + nic.id.to_string(), + ReplacementComponent::VirtioNetworkBackend(nic.backend), + ) + })); + + propolis_client::types::InstanceEnsureRequest { + properties: self.properties.clone(), + init: InstanceInitializationMethod::MigrationTarget { migration_id, - }) + replace_components: elements_to_replace, + src_addr: params.src_propolis_addr.to_string(), + }, } - None => None, - }; + } else { + // This is not a request to migrate, so construct a brand new spec + // to send to Propolis. + // + // Spec components must all have unique names. This routine ensures + // that names are unique as follows: + // + // 1. Backend components corresponding to specific control plane + // objects (e.g. Crucible disks, network interfaces) are + // identified by their control plane record IDs, which are UUIDs. + // (If these UUIDs collide, Nexus has many other problems.) + // 2. Devices attached to those backends are identified by a string + // that includes the backend UUID; see `id_to_device_name` below. + // 3. Other components that are implicitly added to all VMs are + // assigned unique component names within this function. + // + // Because *Nexus object names* (which *can* alias) are never used + // directly to name spec components, there should never be a + // conflict, so this helper asserts that all keys in the component + // map are unique. + fn add_component( + spec: &mut propolis_client::types::InstanceSpecV0, + key: String, + component: ComponentV0, + ) { + assert!(spec.components.insert(key, component).is_none()); + } + + fn id_to_device_name(id: &Uuid) -> String { + format!("{id}:device") + } + + let mut spec = propolis_client::types::InstanceSpecV0 { + board: Board { + chipset: Chipset::default(), + cpus: self.vcpus, + memory_mb: self.memory_mib, + cpuid: None, + }, + components: Default::default(), + }; + + for (name, num) in [ + ("com1", SerialPortNumber::Com1), + ("com2", SerialPortNumber::Com2), + ("com3", SerialPortNumber::Com3), + ("com4", SerialPortNumber::Com4), + ] { + add_component( + &mut spec, + name.to_string(), + ComponentV0::SerialPort(SerialPort { num }), + ); + } + + for DiskComponents { id, device, backend } in disks.into_iter() { + add_component( + &mut spec, + id_to_device_name(&id), + ComponentV0::NvmeDisk(device), + ); - let request = propolis_client::types::InstanceEnsureRequest { - properties: self.properties.clone(), - vcpus: self.vcpus, - memory: self.memory_mib, - nics, - disks: self - .requested_disks - .iter() - .cloned() - .map(Into::into) - .collect(), - boot_settings: self.boot_settings.clone(), - migrate, - cloud_init_bytes: self.cloud_init_bytes.clone().map(|x| x.0), + add_component( + &mut spec, + id.to_string(), + ComponentV0::CrucibleStorageBackend(backend), + ); + } + + for NicComponents { id, device, backend } in nics.into_iter() { + add_component( + &mut spec, + id_to_device_name(&id), + ComponentV0::VirtioNic(device), + ); + add_component( + &mut spec, + id.to_string(), + ComponentV0::VirtioNetworkBackend(backend), + ); + } + + add_component( + &mut spec, + "pvpanic".to_owned(), + ComponentV0::QemuPvpanic(QemuPvpanic { enable_isa: true }), + ); + + if let Some(settings) = &self.boot_settings { + // The boot order contains a list of disk IDs. Propolis matches + // boot order entries based on device component names, so pass + // the ID through `id_to_device_name` when generating the + // Propolis boot order. + let settings = ComponentV0::BootSettings(BootSettings { + order: settings + .order + .iter() + .map(|id| BootOrderEntry { + id: SpecKey::Name(id_to_device_name(&id)), + }) + .collect(), + }); + + add_component(&mut spec, "boot_settings".to_string(), settings); + } + + if let Some(cloud_init) = &self.cloud_init_bytes { + let device_name = "cloud-init-dev"; + let backend_name = "cloud-init-backend"; + + // The old Propolis API (and sled-agent's arguments to it) + // always attached cloud-init drives at BDF 0.24.0. + let device = ComponentV0::VirtioDisk(VirtioDisk { + backend_id: SpecKey::Name(backend_name.to_string()), + pci_path: PciPath::new(0, 0x18, 0).unwrap(), + }); + + let backend = + ComponentV0::BlobStorageBackend(BlobStorageBackend { + base64: cloud_init.0.clone(), + readonly: true, + }); + + add_component(&mut spec, device_name.to_string(), device); + add_component(&mut spec, backend_name.to_string(), backend); + } + + propolis_client::types::InstanceEnsureRequest { + properties: self.properties.clone(), + init: InstanceInitializationMethod::Spec { spec }, + } }; debug!(self.log, "Sending ensure request to propolis: {:?}", request); diff --git a/sled-agent/src/sim/http_entrypoints_pantry.rs b/sled-agent/src/sim/http_entrypoints_pantry.rs index c98c7db665..e879cea70f 100644 --- a/sled-agent/src/sim/http_entrypoints_pantry.rs +++ b/sled-agent/src/sim/http_entrypoints_pantry.rs @@ -9,7 +9,7 @@ use dropshot::{ HttpResponseDeleted, HttpResponseOk, HttpResponseUpdatedNoContent, Path as TypedPath, RequestContext, TypedBody, }; -use propolis_client::types::VolumeConstructionRequest; +use propolis_client::VolumeConstructionRequest; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::sync::Arc; diff --git a/sled-agent/src/sim/sled_agent.rs b/sled-agent/src/sim/sled_agent.rs index 1f099fc036..0653b52508 100644 --- a/sled-agent/src/sim/sled_agent.rs +++ b/sled-agent/src/sim/sled_agent.rs @@ -45,7 +45,11 @@ use omicron_uuid_kinds::{ }; use oxnet::Ipv6Net; use propolis_client::{ - types::VolumeConstructionRequest, Client as PropolisClient, + types::{ + Board, Chipset, ComponentV0, InstanceInitializationMethod, + InstanceSpecV0, SerialPort, SerialPortNumber, + }, + Client as PropolisClient, VolumeConstructionRequest, }; use sled_agent_api::SupportBundleMetadata; use sled_agent_api::SupportBundleState; @@ -125,7 +129,7 @@ fn extract_targets_from_volume_construction_request( VolumeConstructionRequest::Region { opts, .. } => { for target in &opts.target { - res.push(SocketAddr::from_str(&target)?); + res.push(*target); } } @@ -301,10 +305,7 @@ impl SledAgent { // Ensure that any disks that are in this request are attached to // this instance. - let id = match disk.volume_construction_request { - VolumeConstructionRequest::Volume { id, .. } => id, - _ => panic!("Unexpected construction type"), - }; + let id = disk.disk_id; self.disks .sim_ensure( &id, @@ -353,15 +354,30 @@ impl SledAgent { description: "sled-agent-sim created instance".to_string(), metadata, }; + let body = propolis_client::types::InstanceEnsureRequest { properties, - memory: hardware.properties.memory.to_whole_mebibytes(), - vcpus: hardware.properties.ncpus.0 as u8, - nics: vec![], - disks: vec![], - boot_settings: None, - migrate: None, - cloud_init_bytes: None, + init: InstanceInitializationMethod::Spec { + spec: InstanceSpecV0 { + board: Board { + cpus: hardware.properties.ncpus.0 as u8, + chipset: Chipset::default(), + memory_mb: hardware + .properties + .memory + .to_whole_mebibytes(), + cpuid: None, + }, + components: [( + "com1".to_string(), + ComponentV0::SerialPort(SerialPort { + num: SerialPortNumber::Com1, + }), + )] + .into_iter() + .collect(), + }, + }, }; // Try to create the instance client.instance_ensure().body(body).send().await.map_err( @@ -397,7 +413,7 @@ impl SledAgent { .await?; for disk_request in &hardware.disks { - let vcr = &disk_request.volume_construction_request; + let vcr = serde_json::from_str(&disk_request.vcr_json.0)?; self.map_disk_ids_to_region_ids(&vcr).await?; } diff --git a/sled-agent/src/sim/storage.rs b/sled-agent/src/sim/storage.rs index 2299ba9db9..c706c05b14 100644 --- a/sled-agent/src/sim/storage.rs +++ b/sled-agent/src/sim/storage.rs @@ -36,7 +36,7 @@ use omicron_uuid_kinds::PhysicalDiskUuid; use omicron_uuid_kinds::PropolisUuid; use omicron_uuid_kinds::SupportBundleUuid; use omicron_uuid_kinds::ZpoolUuid; -use propolis_client::types::VolumeConstructionRequest; +use propolis_client::VolumeConstructionRequest; use serde::Serialize; use sled_agent_api::SupportBundleMetadata; use sled_agent_api::SupportBundleState; diff --git a/sled-agent/types/src/instance.rs b/sled-agent/types/src/instance.rs index 39726030b0..3ff73925e4 100644 --- a/sled-agent/types/src/instance.rs +++ b/sled-agent/types/src/instance.rs @@ -47,6 +47,39 @@ pub struct InstanceEnsureBody { pub metadata: InstanceMetadata, } +/// A request to attach a disk to an instance. +#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] +pub struct InstanceDisk { + /// The disk's UUID. + pub disk_id: Uuid, + /// The logical slot number assigned to the disk in its database record. + pub slot: u8, + /// True if the disk is read-only. + pub read_only: bool, + /// A JSON representation of the Crucible volume construction request for + /// this attachment. + // + // This is marked as `NoDebug` because the VCR contains the volume's + // encryption keys. + pub vcr_json: NoDebug, + + /// The disk's name, used to generate the serial number for the virtual disk + /// exposed to the guest. + // + // TODO(#7153): Making this depend on the disk name means that a disk's ID + // may change if it is renamed or if a snapshot of it is used to create a + // new disk. + pub name: String, +} + +/// Configures how an instance is told to try to boot. +#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] +pub struct InstanceBootSettings { + /// Propolis should tell guest firmware to try to boot from devices in this + /// order. + pub order: Vec, +} + /// Describes the instance hardware. #[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] pub struct InstanceHardware { @@ -59,9 +92,8 @@ pub struct InstanceHardware { pub floating_ips: Vec, pub firewall_rules: Vec, pub dhcp_config: DhcpConfig, - // TODO: replace `propolis_client::*` with locally-modeled request type - pub disks: Vec, - pub boot_settings: Option, + pub disks: Vec, + pub boot_settings: Option, pub cloud_init_bytes: Option>, } @@ -152,9 +184,6 @@ pub struct VmmUnregisterResponse { /// migration. #[derive(Copy, Clone, Debug, Deserialize, Serialize, JsonSchema)] pub struct InstanceMigrationTargetParams { - /// The Propolis ID of the migration source. - pub src_propolis_id: Uuid, - /// The address of the Propolis server that will serve as the migration /// source. pub src_propolis_addr: SocketAddr,