Update propolis mock-server and client deps

With the propolis mock-server split out from the "real" propolis-server, and the handmade types cleaned out of the propolis-client library, we can now free ourselves of the somewhat circular dependency situation.
oxidecomputer · Nov 13, 2023 · 64ffe41 · 64ffe41
1 parent 6bbaa80
commit 64ffe41
Show file tree

Hide file tree

Showing 14 changed files with 221 additions and 940 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -168,7 +168,6 @@ criterion = { version = "0.5.1", features = [ "async_tokio" ] }
 crossbeam = "0.8"
 crossterm = { version = "0.27.0", features = ["event-stream"] }
 crucible-agent-client = { git = "https://github.com/oxidecomputer/crucible", rev = "da534e73380f3cc53ca0de073e1ea862ae32109b" }
-crucible-client-types = { git = "https://github.com/oxidecomputer/crucible", rev = "da534e73380f3cc53ca0de073e1ea862ae32109b" }
 crucible-pantry-client = { git = "https://github.com/oxidecomputer/crucible", rev = "da534e73380f3cc53ca0de073e1ea862ae32109b" }
 crucible-smf = { git = "https://github.com/oxidecomputer/crucible", rev = "da534e73380f3cc53ca0de073e1ea862ae32109b" }
 curve25519-dalek = "4"
@@ -289,9 +288,9 @@ pretty-hex = "0.3.0"
 proc-macro2 = "1.0"
 progenitor = { git = "https://github.com/oxidecomputer/progenitor", branch = "main" }
 progenitor-client = { git = "https://github.com/oxidecomputer/progenitor", branch = "main" }
-bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "4019eb10fc2f4ba9bf210d0461dc6292b68309c2" }
-propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "4019eb10fc2f4ba9bf210d0461dc6292b68309c2", features = [ "generated-migration" ] }
-propolis-server = { git = "https://github.com/oxidecomputer/propolis", rev = "4019eb10fc2f4ba9bf210d0461dc6292b68309c2", default-features = false, features = ["mock-only"] }
+bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "5ed82315541271e2734746a9ca79e39f35c12283" }
+propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "5ed82315541271e2734746a9ca79e39f35c12283" }
+propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "5ed82315541271e2734746a9ca79e39f35c12283" }
 proptest = "1.3.1"
 quote = "1.0"
 rand = "0.8.5"
@@ -544,9 +543,9 @@ opt-level = 3
 #steno = { path = "../steno" }
 #[patch."https://github.com/oxidecomputer/propolis"]
 #propolis-client = { path = "../propolis/lib/propolis-client" }
+#propolis-mock-server = { path = "../propolis/bin/mock-server" }
 #[patch."https://github.com/oxidecomputer/crucible"]
 #crucible-agent-client = { path = "../crucible/agent-client" }
-#crucible-client-types = { path = "../crucible/crucible-client-types" }
 #crucible-pantry-client = { path = "../crucible/pantry-client" }
 #crucible-smf = { path = "../crucible/smf" }
 #[patch.crates-io]

diff --git a/sled-agent/Cargo.toml b/sled-agent/Cargo.toml
@@ -19,7 +19,6 @@ cfg-if.workspace = true
 chrono.workspace = true
 clap.workspace = true
 # Only used by the simulated sled agent.
-crucible-client-types.workspace = true
 crucible-agent-client.workspace = true
 ddm-admin-client.workspace = true
 derive_more.workspace = true
@@ -47,8 +46,8 @@ once_cell.workspace = true
 oximeter.workspace = true
 oximeter-instruments.workspace = true
 oximeter-producer.workspace = true
-propolis-client = { workspace = true, features = [ "generated-migration" ] }
-propolis-server.workspace = true # Only used by the simulated sled agent
+propolis-client.workspace = true
+propolis-mock-server.workspace = true # Only used by the simulated sled agent
 rand = { workspace = true, features = ["getrandom"] }
 reqwest = { workspace = true, features = ["rustls-tls", "stream"] }
 schemars = { workspace = true, features = [ "chrono", "uuid1" ] }

diff --git a/sled-agent/src/common/disk.rs b/sled-agent/src/common/disk.rs
@@ -9,7 +9,7 @@ use chrono::Utc;
 use omicron_common::api::external::DiskState;
 use omicron_common::api::external::Error;
 use omicron_common::api::internal::nexus::DiskRuntimeState;
-use propolis_client::api::DiskAttachmentState as PropolisDiskState;
+use propolis_client::types::DiskAttachmentState as PropolisDiskState;
 use uuid::Uuid;
 
 /// Action to be taken on behalf of state transition.

diff --git a/sled-agent/src/common/instance.rs b/sled-agent/src/common/instance.rs
@@ -10,8 +10,9 @@ use omicron_common::api::external::InstanceState as ApiInstanceState;
 use omicron_common::api::internal::nexus::{
     InstanceRuntimeState, SledInstanceState, VmmRuntimeState,
 };
-use propolis_client::api::{
+use propolis_client::types::{
     InstanceState as PropolisApiState, InstanceStateMonitorResponse,
+    MigrationState,
 };
 use uuid::Uuid;
 
@@ -36,7 +37,7 @@ impl From<PropolisApiState> for PropolisInstanceState {
 
 impl From<PropolisInstanceState> for ApiInstanceState {
     fn from(value: PropolisInstanceState) -> Self {
-        use propolis_client::api::InstanceState as State;
+        use propolis_client::types::InstanceState as State;
         match value.0 {
             // Nexus uses the VMM state as the externally-visible instance state
             // when an instance has an active VMM. A Propolis that is "creating"
@@ -119,7 +120,6 @@ impl ObservedPropolisState {
                 (Some(this_id), Some(propolis_migration))
                     if this_id == propolis_migration.migration_id =>
                 {
-                    use propolis_client::api::MigrationState;
                     match propolis_migration.state {
                         MigrationState::Finish => {
                             ObservedMigrationStatus::Succeeded
@@ -510,7 +510,7 @@ mod test {
     use chrono::Utc;
     use omicron_common::api::external::Generation;
     use omicron_common::api::internal::nexus::InstanceRuntimeState;
-    use propolis_client::api::InstanceState as Observed;
+    use propolis_client::types::InstanceState as Observed;
     use uuid::Uuid;
 
     fn make_instance() -> InstanceStates {

diff --git a/sled-agent/src/instance.rs b/sled-agent/src/instance.rs
@@ -190,7 +190,7 @@ struct InstanceInner {
     log: Logger,
 
     // Properties visible to Propolis
-    properties: propolis_client::api::InstanceProperties,
+    properties: propolis_client::types::InstanceProperties,
 
     // The ID of the Propolis server (and zone) running this instance
     propolis_id: Uuid,
@@ -214,7 +214,7 @@ struct InstanceInner {
 
     // Disk related properties
     // TODO: replace `propolis_client::handmade::*` with properly-modeled local types
-    requested_disks: Vec<propolis_client::handmade::api::DiskRequest>,
+    requested_disks: Vec<propolis_client::types::DiskRequest>,
     cloud_init_bytes: Option<String>,
 
     // Internal State management
@@ -379,7 +379,7 @@ impl InstanceInner {
     /// Sends an instance state PUT request to this instance's Propolis.
     async fn propolis_state_put(
         &self,
-        request: propolis_client::api::InstanceStateRequested,
+        request: propolis_client::types::InstanceStateRequested,
     ) -> Result<(), Error> {
         let res = self
             .running_state
@@ -409,11 +409,11 @@ impl InstanceInner {
     ) -> Result<(), Error> {
         let nics = running_zone
             .opte_ports()
-            .map(|port| propolis_client::api::NetworkInterfaceRequest {
+            .map(|port| propolis_client::types::NetworkInterfaceRequest {
                 // TODO-correctness: Remove `.vnic()` call when we use the port
                 // directly.
                 name: port.vnic_name().to_string(),
-                slot: propolis_client::api::Slot(port.slot()),
+                slot: propolis_client::types::Slot(port.slot()),
             })
             .collect();
 
@@ -423,7 +423,7 @@ impl InstanceInner {
                     self.state.instance().migration_id.ok_or_else(|| {
                         Error::Migration(anyhow!("Missing Migration UUID"))
                     })?;
-                Some(propolis_client::api::InstanceMigrateInitiateRequest {
+                Some(propolis_client::types::InstanceMigrateInitiateRequest {
                     src_addr: params.src_propolis_addr.to_string(),
                     src_uuid: params.src_propolis_id,
                     migration_id,
@@ -432,7 +432,7 @@ impl InstanceInner {
             None => None,
         };
 
-        let request = propolis_client::api::InstanceEnsureRequest {
+        let request = propolis_client::types::InstanceEnsureRequest {
             properties: self.properties.clone(),
             nics,
             disks: self
@@ -648,7 +648,7 @@ impl Instance {
         let instance = InstanceInner {
             log: log.new(o!("instance_id" => id.to_string())),
             // NOTE: Mostly lies.
-            properties: propolis_client::api::InstanceProperties {
+            properties: propolis_client::types::InstanceProperties {
                 id,
                 name: hardware.properties.hostname.clone(),
                 description: "Test description".to_string(),
@@ -789,7 +789,7 @@ impl Instance {
         &self,
         state: crate::params::InstanceStateRequested,
     ) -> Result<SledInstanceState, Error> {
-        use propolis_client::api::InstanceStateRequested as PropolisRequest;
+        use propolis_client::types::InstanceStateRequested as PropolisRequest;
         let mut inner = self.inner.lock().await;
         let (propolis_state, next_published) = match state {
             InstanceStateRequested::MigrationTarget(migration_params) => {
@@ -1035,7 +1035,9 @@ impl Instance {
             // known to Propolis.
             let response = client
                 .instance_state_monitor()
-                .body(propolis_client::api::InstanceStateMonitorRequest { gen })
+                .body(propolis_client::types::InstanceStateMonitorRequest {
+                    gen,
+                })
                 .send()
                 .await?
                 .into_inner();

diff --git a/sled-agent/src/params.rs b/sled-agent/src/params.rs
@@ -70,8 +70,8 @@ pub struct InstanceHardware {
     pub external_ips: Vec<IpAddr>,
     pub firewall_rules: Vec<VpcFirewallRule>,
     pub dhcp_config: DhcpConfig,
-    // TODO: replace `propolis_client::handmade::*` with locally-modeled request type
-    pub disks: Vec<propolis_client::handmade::api::DiskRequest>,
+    // TODO: replace `propolis_client::*` with locally-modeled request type
+    pub disks: Vec<propolis_client::types::DiskRequest>,
     pub cloud_init_bytes: Option<String>,
 }
 

diff --git a/sled-agent/src/sim/disk.rs b/sled-agent/src/sim/disk.rs
@@ -19,7 +19,7 @@ use omicron_common::api::internal::nexus::DiskRuntimeState;
 use omicron_common::api::internal::nexus::ProducerEndpoint;
 use oximeter_producer::LogConfig;
 use oximeter_producer::Server as ProducerServer;
-use propolis_client::api::DiskAttachmentState as PropolisDiskState;
+use propolis_client::types::DiskAttachmentState as PropolisDiskState;
 use std::net::{Ipv6Addr, SocketAddr};
 use std::sync::Arc;
 use std::time::Duration;

diff --git a/sled-agent/src/sim/http_entrypoints_pantry.rs b/sled-agent/src/sim/http_entrypoints_pantry.rs
@@ -4,11 +4,11 @@
 
 //! HTTP entrypoint functions for simulating the crucible pantry API.
 
-use crucible_client_types::VolumeConstructionRequest;
 use dropshot::{
     endpoint, ApiDescription, HttpError, HttpResponseDeleted, HttpResponseOk,
     HttpResponseUpdatedNoContent, Path as TypedPath, RequestContext, TypedBody,
 };
+use propolis_client::types::VolumeConstructionRequest;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use std::sync::Arc;

diff --git a/sled-agent/src/sim/instance.rs b/sled-agent/src/sim/instance.rs
@@ -19,9 +19,10 @@ use omicron_common::api::external::ResourceType;
 use omicron_common::api::internal::nexus::{
     InstanceRuntimeState, SledInstanceState,
 };
-use propolis_client::api::InstanceMigrateStatusResponse as PropolisMigrateStatus;
-use propolis_client::api::InstanceState as PropolisInstanceState;
-use propolis_client::api::InstanceStateMonitorResponse;
+use propolis_client::types::{
+    InstanceMigrateStatusResponse as PropolisMigrateStatus,
+    InstanceState as PropolisInstanceState, InstanceStateMonitorResponse,
+};
 use std::collections::VecDeque;
 use std::sync::Arc;
 use std::sync::Mutex;
@@ -131,11 +132,11 @@ impl SimInstanceInner {
                     });
                 self.queue_migration_status(PropolisMigrateStatus {
                     migration_id,
-                    state: propolis_client::api::MigrationState::Sync,
+                    state: propolis_client::types::MigrationState::Sync,
                 });
                 self.queue_migration_status(PropolisMigrateStatus {
                     migration_id,
-                    state: propolis_client::api::MigrationState::Finish,
+                    state: propolis_client::types::MigrationState::Finish,
                 });
                 self.queue_propolis_state(PropolisInstanceState::Running);
             }

diff --git a/sled-agent/src/sim/sled_agent.rs b/sled-agent/src/sim/sled_agent.rs
@@ -35,15 +35,16 @@ use uuid::Uuid;
 use std::collections::HashMap;
 use std::str::FromStr;
 
-use crucible_client_types::VolumeConstructionRequest;
 use dropshot::HttpServer;
 use illumos_utils::opte::params::{
     DeleteVirtualNetworkInterfaceHost, SetVirtualNetworkInterfaceHost,
 };
 use nexus_client::types::PhysicalDiskKind;
 use omicron_common::address::PROPOLIS_PORT;
-use propolis_client::Client as PropolisClient;
-use propolis_server::mock_server::Context as PropolisContext;
+use propolis_client::{
+    types::VolumeConstructionRequest, Client as PropolisClient,
+};
+use propolis_mock_server::Context as PropolisContext;
 
 /// Simulates management of the control plane on a sled
 ///
@@ -70,13 +71,14 @@ pub struct SledAgent {
 }
 
 fn extract_targets_from_volume_construction_request(
-    vec: &mut Vec<SocketAddr>,
     vcr: &VolumeConstructionRequest,
-) {
+) -> Result<Vec<SocketAddr>, std::net::AddrParseError> {
     // A snapshot is simply a flush with an extra parameter, and flushes are
     // only sent to sub volumes, not the read only parent. Flushes are only
     // processed by regions, so extract each region that would be affected by a
     // flush.
+
+    let mut res = vec![];
     match vcr {
         VolumeConstructionRequest::Volume {
             id: _,
@@ -85,9 +87,9 @@ fn extract_targets_from_volume_construction_request(
             read_only_parent: _,
         } => {
             for sub_volume in sub_volumes.iter() {
-                extract_targets_from_volume_construction_request(
-                    vec, sub_volume,
-                );
+                res.extend(extract_targets_from_volume_construction_request(
+                    sub_volume,
+                )?);
             }
         }
 
@@ -103,14 +105,15 @@ fn extract_targets_from_volume_construction_request(
             gen: _,
         } => {
             for target in &opts.target {
-                vec.push(*target);
+                res.push(SocketAddr::from_str(target)?);
             }
         }
 
         VolumeConstructionRequest::File { .. } => {
             // noop
         }
     }
+    Ok(res)
 }
 
 impl SledAgent {
@@ -171,23 +174,19 @@ impl SledAgent {
         volume_construction_request: &VolumeConstructionRequest,
     ) -> Result<(), Error> {
         let disk_id = match volume_construction_request {
-            VolumeConstructionRequest::Volume {
-                id,
-                block_size: _,
-                sub_volumes: _,
-                read_only_parent: _,
-            } => id,
+            VolumeConstructionRequest::Volume { id, .. } => id,
 
             _ => {
                 panic!("root of volume construction request not a volume!");
             }
         };
 
-        let mut targets = Vec::new();
-        extract_targets_from_volume_construction_request(
-            &mut targets,
+        let targets = extract_targets_from_volume_construction_request(
             &volume_construction_request,
-        );
+        )
+        .map_err(|e| {
+            Error::invalid_request(&format!("bad socketaddr: {e:?}"))
+        })?;
 
         let mut region_ids = Vec::new();
 
@@ -640,11 +639,10 @@ impl SledAgent {
             ..Default::default()
         };
         let propolis_log = log.new(o!("component" => "propolis-server-mock"));
-        let private =
-            Arc::new(PropolisContext::new(Default::default(), propolis_log));
+        let private = Arc::new(PropolisContext::new(propolis_log));
         info!(log, "Starting mock propolis-server...");
         let dropshot_log = log.new(o!("component" => "dropshot"));
-        let mock_api = propolis_server::mock_server::api();
+        let mock_api = propolis_mock_server::api();
 
         let srv = dropshot::HttpServerStarter::new(
             &dropshot_config,

diff --git a/sled-agent/src/sim/storage.rs b/sled-agent/src/sim/storage.rs
@@ -16,13 +16,13 @@ use chrono::prelude::*;
 use crucible_agent_client::types::{
     CreateRegion, Region, RegionId, RunningSnapshot, Snapshot, State,
 };
-use crucible_client_types::VolumeConstructionRequest;
 use dropshot::HandlerTaskMode;
 use dropshot::HttpError;
 use futures::lock::Mutex;
 use nexus_client::types::{
     ByteCount, PhysicalDiskKind, PhysicalDiskPutRequest, ZpoolPutRequest,
 };
+use propolis_client::types::VolumeConstructionRequest;
 use slog::Logger;
 use std::collections::HashMap;
 use std::collections::HashSet;

diff --git a/tools/update_crucible.sh b/tools/update_crucible.sh
@@ -21,7 +21,6 @@ PACKAGES=(
 
 CRATES=(
   "crucible-agent-client"
-  "crucible-client-types"
   "crucible-pantry-client"
   "crucible-smf"
 )