Adds network interface attach/detach

README.adoc

@@ -645,6 +645,11 @@ documents being checked in.
 In general, changes any service API **require the following set of build steps**:
 
 * Make changes to the service API
+* Build the package for the modified service alone. This can be done by changing
+  directories there, or `cargo build -p <package>`. This is step is important,
+  to avoid the circular dependency at this point. One needs to update this one
+  OpenAPI document, without rebuilding the other components that depend on a
+  now-outdated spec.
 * Update the OpenAPI document by running the relevant test with overwrite set:
   `EXPECTORATE=overwrite cargo test test_nexus_openapi_internal` (changing the
   test name as necessary)

common/src/api/external/mod.rs

@@ -1774,20 +1774,22 @@ pub struct NetworkInterface {
     #[serde(flatten)]
     pub identity: IdentityMetadata,
 
-    /** The Instance to which the interface belongs. */
+    /// The Instance to which the interface belongs.
     pub instance_id: Uuid,
 
-    /** The VPC to which the interface belongs. */
+    /// The VPC to which the interface belongs.
     pub vpc_id: Uuid,
 
-    /** The subnet to which the interface belongs. */
+    /// The subnet to which the interface belongs.
     pub subnet_id: Uuid,
 
-    /** The MAC address assigned to this interface. */
+    /// The MAC address assigned to this interface.
     pub mac: MacAddr,
 
-    /** The IP address assigned to this interface. */
+    /// The IP address assigned to this interface.
     pub ip: IpAddr,
+    // TODO-correctness: We need to split this into an optional V4 and optional
+    // V6 address, at least one of which must be specified.
 }
 
 #[cfg(test)]

common/src/sql/dbinit.sql

@@ -465,8 +465,13 @@ CREATE TABLE omicron.public.network_interface (
     time_modified TIMESTAMPTZ NOT NULL,
     /* Indicates that the object has been deleted */
     time_deleted TIMESTAMPTZ,
-    /* FK into Instance table. */
+
+    /* FK into Instance table.
+     * Note that interfaces are always attached to a particular instance.
+     * IP addresses may be reserved, but this is a different resource.
+     */
     instance_id UUID NOT NULL,
+
     /* FK into VPC table */
     vpc_id UUID NOT NULL,
     /* FK into VPCSubnet table. */
@@ -483,12 +488,6 @@ CREATE TABLE omicron.public.network_interface (
  * as moving IPs between NICs on different instances, etc.
  */
 
-CREATE UNIQUE INDEX ON omicron.public.network_interface (
-    vpc_id,
-    name
-) WHERE
-    time_deleted IS NULL;
-
 /* Ensure we do not assign the same address twice within a subnet */
 CREATE UNIQUE INDEX ON omicron.public.network_interface (
     subnet_id,
@@ -505,6 +504,18 @@ CREATE UNIQUE INDEX ON omicron.public.network_interface (
 ) WHERE
     time_deleted IS NULL;
 
+/*
+ * Index used to verify that an Instance's networking is contained
+ * within a single VPC.
+ */
+CREATE UNIQUE INDEX ON omicron.public.network_interface (
+    instance_id,
+    name
+)
+STORING (vpc_id)
+WHERE
+    time_deleted IS NULL;
+
 CREATE TYPE omicron.public.vpc_router_kind AS ENUM (
     'system',
     'custom'

nexus/src/db/datastore.rs

@@ -79,8 +79,9 @@ use crate::db::{
     },
     pagination::paginated,
     pagination::paginated_multicolumn,
-    subnet_allocation::AllocateIpQuery,
     subnet_allocation::FilterConflictingVpcSubnetRangesQuery,
+    subnet_allocation::InsertNetworkInterfaceQuery,
+    subnet_allocation::NetworkInterfaceError,
     subnet_allocation::SubnetError,
     update_and_check::{UpdateAndCheck, UpdateStatus},
 };
@@ -1698,109 +1699,153 @@ impl DataStore {
     /*
      * Network interfaces
      */
-
     pub async fn instance_create_network_interface(
         &self,
         interface: IncompleteNetworkInterface,
-    ) -> CreateResult<NetworkInterface> {
+    ) -> Result<NetworkInterface, NetworkInterfaceError> {
         use db::schema::network_interface::dsl;
+        let query = InsertNetworkInterfaceQuery {
+            interface: interface.clone(),
+            now: Utc::now(),
+        };
+        diesel::insert_into(dsl::network_interface)
+            .values(query)
+            .returning(NetworkInterface::as_returning())
+            .get_result_async(self.pool())
+            .await
+            .map_err(|e| NetworkInterfaceError::from_pool(e, &interface))
+    }
 
-        // TODO: Longer term, it would be nice to decouple the IP allocation
-        // (and MAC allocation) from the NetworkInterface table, so that
-        // retrying from parallel inserts doesn't need to happen here.
-
-        let name = interface.identity.name.clone();
-        match interface.ip {
-            // Attempt an insert with a requested IP address
-            Some(ip) => {
-                interface.subnet.contains(ip)?;
-                let row = NetworkInterface {
-                    identity: interface.identity,
-                    instance_id: interface.instance_id,
-                    vpc_id: interface.vpc_id,
-                    subnet_id: interface.subnet.id(),
-                    mac: interface.mac,
-                    ip: ip.into(),
-                };
-                diesel::insert_into(dsl::network_interface)
-                    .values(row)
-                    .returning(NetworkInterface::as_returning())
-                    .get_result_async(self.pool())
-                    .await
-                    .map_err(|e| {
-                        public_error_from_diesel_pool(
-                            e,
-                            ErrorHandler::Conflict(
-                                ResourceType::NetworkInterface,
-                                name.as_str(),
-                            ),
-                        )
-                    })
-            }
-            // Insert and allocate an IP address
-            None => {
-                let allocation_query = AllocateIpQuery {
-                    block: ipnetwork::IpNetwork::V4(
-                        interface.subnet.ipv4_block.0 .0,
+    /// Delete all network interfaces attached to the given instance.
+    // NOTE: This is mostly useful in the context of sagas, but might be helpful
+    // in other situations, such as moving an instance between VPC Subnets.
+    pub async fn instance_delete_all_network_interfaces(
+        &self,
+        instance_id: &Uuid,
+    ) -> DeleteResult {
+        use db::schema::network_interface::dsl;
+        let now = Utc::now();
+        diesel::update(dsl::network_interface)
+            .filter(dsl::instance_id.eq(*instance_id))
+            .filter(dsl::time_deleted.is_null())
+            .set(dsl::time_deleted.eq(now))
+            .execute_async(self.pool())
+            .await
+            .map_err(|e| {
+                public_error_from_diesel_pool(
+                    e,
+                    ErrorHandler::NotFoundByLookup(
+                        ResourceType::Instance,
+                        LookupType::ById(*instance_id),
                     ),
-                    interface,
-                    now: Utc::now(),
-                };
-                diesel::insert_into(dsl::network_interface)
-                    .values(allocation_query)
-                    .returning(NetworkInterface::as_returning())
-                    .get_result_async(self.pool())
-                    .await
-                    .map_err(|e| {
-                        if let PoolError::Connection(ConnectionError::Query(
-                            diesel::result::Error::NotFound,
-                        )) = e
-                        {
-                            Error::InvalidRequest {
-                                message: "no available IP addresses"
-                                    .to_string(),
-                            }
-                        } else {
-                            public_error_from_diesel_pool(
-                                e,
-                                ErrorHandler::Conflict(
-                                    ResourceType::NetworkInterface,
-                                    name.as_str(),
-                                ),
-                            )
-                        }
-                    })
-            }
-        }
+                )
+            })?;
+        Ok(())
     }
 
     pub async fn instance_delete_network_interface(
         &self,
-        network_interface_id: &Uuid,
+        interface_id: &Uuid,
     ) -> DeleteResult {
         use db::schema::network_interface::dsl;
+        let now = Utc::now();
+        let result = diesel::update(dsl::network_interface)
+            .filter(dsl::id.eq(*interface_id))
+            .filter(dsl::time_deleted.is_null())
+            .set((dsl::time_deleted.eq(now),))
+            .check_if_exists::<db::model::NetworkInterface>(*interface_id)
+            .execute_and_check(self.pool())
+            .await
+            .map_err(|e| {
+                public_error_from_diesel_pool(
+                    e,
+                    ErrorHandler::NotFoundByLookup(
+                        ResourceType::NetworkInterface,
+                        LookupType::ById(*interface_id),
+                    ),
+                )
+            })?;
+        match result.status {
+            UpdateStatus::Updated => Ok(()),
+            UpdateStatus::NotUpdatedButExists => {
+                let interface = &result.found;
+                if interface.time_deleted().is_some() {
+                    // Already deleted
+                    Ok(())
+                } else {
+                    Err(Error::internal_error(&format!(
+                        "failed to delete network interface: {}",
+                        interface_id
+                    )))
+                }
+            }
+        }
+    }
+
+    pub async fn subnet_lookup_network_interface(
+        &self,
+        subnet_id: &Uuid,
+        interface_name: &Name,
+    ) -> LookupResult<db::model::NetworkInterface> {
+        use db::schema::network_interface::dsl;
+
+        dsl::network_interface
+            .filter(dsl::subnet_id.eq(*subnet_id))
+            .filter(dsl::time_deleted.is_null())
+            .filter(dsl::name.eq(interface_name.clone()))
+            .select(db::model::NetworkInterface::as_select())
+            .get_result_async(self.pool())
+            .await
+            .map_err(|e| {
+                public_error_from_diesel_pool(
+                    e,
+                    ErrorHandler::NotFoundByLookup(
+                        ResourceType::NetworkInterface,
+                        LookupType::ByName(interface_name.to_string()),
+                    ),
+                )
+            })
+    }
 
-        // TODO-correctness: Do not allow deleting interfaces on running
-        // instances until we support hotplug
+    /// List network interfaces associated with a given instance.
+    pub async fn instance_list_network_interfaces(
+        &self,
+        instance_id: &Uuid,
+        pagparams: &DataPageParams<'_, Name>,
+    ) -> ListResultVec<NetworkInterface> {
+        use db::schema::network_interface::dsl;
+        paginated(dsl::network_interface, dsl::name, &pagparams)
+            .filter(dsl::time_deleted.is_null())
+            .filter(dsl::instance_id.eq(*instance_id))
+            .select(NetworkInterface::as_select())
+            .load_async::<NetworkInterface>(self.pool())
+            .await
+            .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server))
+    }
 
-        let now = Utc::now();
-        diesel::update(dsl::network_interface)
+    /// Get a network interface by name attached to an instance
+    pub async fn instance_lookup_network_interface(
+        &self,
+        instance_id: &Uuid,
+        interface_name: &Name,
+    ) -> LookupResult<NetworkInterface> {
+        use db::schema::network_interface::dsl;
+        dsl::network_interface
+            .filter(dsl::instance_id.eq(*instance_id))
+            .filter(dsl::name.eq(interface_name.clone()))
             .filter(dsl::time_deleted.is_null())
-            .filter(dsl::id.eq(*network_interface_id))
-            .set(dsl::time_deleted.eq(now))
-            .returning(NetworkInterface::as_returning())
+            .select(NetworkInterface::as_select())
             .get_result_async(self.pool())
             .await
             .map_err(|e| {
                 public_error_from_diesel_pool(
                     e,
                     ErrorHandler::NotFoundByLookup(
                         ResourceType::NetworkInterface,
-                        LookupType::ById(*network_interface_id),
+                        LookupType::ByName(interface_name.to_string()),
                     ),
                 )
-            })?;
-        Ok(())
+            })
     }
 
     // Create a record for a new Oximeter instance