api: one ensure API to rule them all (#813)

Remove the existing instance ensure APIs and collapse them into a single
API that takes a set of instance properties and a mechanism for creating
the instance, which can be either creating a new VM from a spec or
initializing as a live migration target. In the latter case, the client
can also supply a list of instance spec components that the client
wishes to override the components in the source's spec. This gives
Omicron a way to replace backend information (Crucible generation
number, host VNIC names) that changes over live migration.

Remove all of the instance ensure types that supported the old ensure
interface. One notable consequence of this is that the `DiskRequest`
type also caused propolis-server's versions of Crucible's
`VolumeConstructionRequest` and `CrucibleOpts` types to appear in the
generated client. Omicron code tends to pull these types in from
propolis-client (often transitively through sled-agent-client). This is
a useful way of making sure Omicron components serialize/deserialize
VCRs the same way Propolis does, so preserve it by re-exporting these
types from propolis-client.

Remove the parts of the mock server that validated disk, NIC, and
cloud-init volume requests. The Omicron tests that use the mock server
primarily check that its serial console interface works as expected and
don't heavily exercise these checks. (Omicron CI's end-to-end test will
fail if sled-agent mishandles the Propolis ensure API.)

Finally, remove a few other unused types from the propolis_api_types
crate.

Tests: cargo test, PHD, driving ad hoc VMs with propolis-cli.

bin/mock-server/src/lib/api_types.rs

@@ -22,7 +22,6 @@ impl TryFrom<types::PciPath> for propolis_types::PciPath {
             .map_err(|e| e.to_string())
     }
 }
-pub use propolis_types::PciPath;
 
 // Duplicate the parameter types for the endpoints related to the serial console
 

bin/mock-server/src/lib/lib.rs

@@ -4,28 +4,23 @@
 
 //! Implementation of a mock Propolis server
 
-use std::io::{Error as IoError, ErrorKind};
 use std::sync::Arc;
 
-use base64::Engine;
 use dropshot::{
     channel, endpoint, ApiDescription, HttpError, HttpResponseCreated,
     HttpResponseOk, HttpResponseUpdatedNoContent, Query, RequestContext,
     TypedBody, WebsocketConnection,
 };
 use futures::SinkExt;
-use slog::{error, info, o, Logger};
+use slog::{error, o, Logger};
 use thiserror::Error;
 use tokio::sync::{watch, Mutex};
 use tokio_tungstenite::tungstenite::protocol::{Role, WebSocketConfig};
 use tokio_tungstenite::tungstenite::Message;
 use tokio_tungstenite::WebSocketStream;
 
 mod api_types;
-mod copied;
-
-use crate::copied::{slot_to_pci_path, SlotType};
-use api_types::types as api;
+use api_types::types::{self as api, InstanceEnsureRequest};
 
 #[derive(Debug, Eq, PartialEq, Error)]
 pub enum Error {
@@ -141,12 +136,7 @@ async fn instance_ensure(
 ) -> Result<HttpResponseCreated<api::InstanceEnsureResponse>, HttpError> {
     let server_context = rqctx.context();
     let request = request.into_inner();
-    let (properties, nics, disks, cloud_init_bytes) = (
-        request.properties,
-        request.nics,
-        request.disks,
-        request.cloud_init_bytes,
-    );
+    let InstanceEnsureRequest { properties, .. } = request;
 
     // Handle an already-initialized instance
     let mut instance = server_context.instance.lock().await;
@@ -160,58 +150,6 @@ async fn instance_ensure(
             migrate: None,
         }));
     }
-
-    // Perform some basic validation of the requested properties
-    for nic in &nics {
-        info!(server_context.log, "Creating NIC: {:#?}", nic);
-        slot_to_pci_path(nic.slot, SlotType::Nic).map_err(|e| {
-            let err = IoError::new(
-                ErrorKind::InvalidData,
-                format!("Cannot parse vnic PCI: {}", e),
-            );
-            HttpError::for_internal_error(format!(
-                "Cannot build instance: {}",
-                err
-            ))
-        })?;
-    }
-
-    for disk in &disks {
-        info!(server_context.log, "Creating Disk: {:#?}", disk);
-        slot_to_pci_path(disk.slot, SlotType::Disk).map_err(|e| {
-            let err = IoError::new(
-                ErrorKind::InvalidData,
-                format!("Cannot parse disk PCI: {}", e),
-            );
-            HttpError::for_internal_error(format!(
-                "Cannot build instance: {}",
-                err
-            ))
-        })?;
-        info!(server_context.log, "Disk {} created successfully", disk.name);
-    }
-
-    if let Some(cloud_init_bytes) = &cloud_init_bytes {
-        info!(server_context.log, "Creating cloud-init disk");
-        slot_to_pci_path(api::Slot(0), SlotType::CloudInit).map_err(|e| {
-            let err = IoError::new(ErrorKind::InvalidData, e.to_string());
-            HttpError::for_internal_error(format!(
-                "Cannot build instance: {}",
-                err
-            ))
-        })?;
-        base64::engine::general_purpose::STANDARD
-            .decode(cloud_init_bytes)
-            .map_err(|e| {
-                let err = IoError::new(ErrorKind::InvalidInput, e.to_string());
-                HttpError::for_internal_error(format!(
-                    "Cannot build instance: {}",
-                    err
-                ))
-            })?;
-        info!(server_context.log, "cloud-init disk created");
-    }
-
     *instance = Some(InstanceContext::new(properties, &server_context.log));
     Ok(HttpResponseCreated(api::InstanceEnsureResponse { migrate: None }))
 }