Unrolling saga actions for external IPs and NICs (#1474)

* Unrolling saga actions for external IPs and NICs

Previously, for NICs and external IPs, the saga action that actually
created the records and the corresponding undo were not together in a
saga node. The undo was associated with a preceding action that created
UUIDs for the records. That was because each action may have created
multiple records, where the count was not known when we create the saga
template, and so the forward action performed multiple fallible steps.
To unwind it completely, including reverting the creation of N
resources, in the case where the N + 1th failed, we put the undo in the
preceding step. That worked, but was certainly confusing and bad for
maintenance, since the action and its undo were in different nodes. It
also resulted in a ton of extra complexity in some of the downstream
queries, which now had to be aware of the fact that we might replay a
partially-played saga node. This commit undoes all that.

- Unroll the instance external IP address query
- Unroll the network interface creation query
- Actually delete NICs when we delete the instance, and add test for
  that
- Simplify network interface query. An additional CTE detecting the
  partial-saga replay case is no longer needed, since we don't run the
  saga that way.

* Review feedback

- Create UUIDs in separate saga actions for NICs and external IPs
- Remove remaining cruft from NIC-creation query that was required to
  handle the previous saga implementation.

Author: bnaecker

nexus/src/app/instance.rs

@@ -5,6 +5,8 @@
 //! Virtual Machine Instances
 
 use super::MAX_DISKS_PER_INSTANCE;
+use super::MAX_EXTERNAL_IPS_PER_INSTANCE;
+use super::MAX_NICS_PER_INSTANCE;
 use crate::app::sagas;
 use crate::authn;
 use crate::authz;
@@ -59,6 +61,37 @@ impl super::Nexus {
                 MAX_DISKS_PER_INSTANCE
             )));
         }
+        if params.external_ips.len() > MAX_EXTERNAL_IPS_PER_INSTANCE {
+            return Err(Error::invalid_request(&format!(
+                "An instance may not have more than {} external IP addresses",
+                MAX_EXTERNAL_IPS_PER_INSTANCE,
+            )));
+        }
+        if let params::InstanceNetworkInterfaceAttachment::Create(ref ifaces) =
+            params.network_interfaces
+        {
+            if ifaces.len() > MAX_NICS_PER_INSTANCE {
+                return Err(Error::invalid_request(&format!(
+                    "An instance may not have more than {} network interfaces",
+                    MAX_NICS_PER_INSTANCE,
+                )));
+            }
+            // Check that all VPC names are the same.
+            //
+            // This isn't strictly necessary, as the queries to create these
+            // interfaces would fail in the saga, but it's easier to handle here.
+            if ifaces
+                .iter()
+                .map(|iface| &iface.vpc_name)
+                .collect::<std::collections::BTreeSet<_>>()
+                .len()
+                != 1
+            {
+                return Err(Error::invalid_request(
+                    "All interfaces must be in the same VPC",
+                ));
+            }
+        }
 
         // Reject instances where the memory is not at least
         // MIN_MEMORY_SIZE_BYTES
@@ -217,6 +250,9 @@ impl super::Nexus {
         self.db_datastore
             .project_delete_instance(opctx, &authz_instance)
             .await?;
+        self.db_datastore
+            .instance_delete_all_network_interfaces(opctx, &authz_instance)
+            .await?;
         // Ignore the count of addresses deleted
         self.db_datastore
             .deallocate_instance_external_ip_by_instance_id(
@@ -502,12 +538,12 @@ impl super::Nexus {
             .partition(|ip| ip.kind == IpKind::SNat);
 
         // Sanity checks on the number and kind of each IP address.
-        if external_ips.len() > crate::app::MAX_EPHEMERAL_IPS_PER_INSTANCE {
+        if external_ips.len() > MAX_EXTERNAL_IPS_PER_INSTANCE {
             return Err(Error::internal_error(
                 format!(
                     "Expected the number of external IPs to be limited to \
-                {}, but found {}",
-                    crate::app::MAX_EPHEMERAL_IPS_PER_INSTANCE,
+                    {}, but found {}",
+                    MAX_EXTERNAL_IPS_PER_INSTANCE,
                     external_ips.len(),
                 )
                 .as_str(),

nexus/src/app/mod.rs

@@ -52,10 +52,10 @@ mod sagas;
 
 pub(crate) const MAX_DISKS_PER_INSTANCE: u32 = 8;
 
-pub(crate) const MAX_NICS_PER_INSTANCE: u32 = 8;
+pub(crate) const MAX_NICS_PER_INSTANCE: usize = 8;
 
-// TODO-completness: Support multiple Ephemeral IPs
-pub(crate) const MAX_EPHEMERAL_IPS_PER_INSTANCE: usize = 1;
+// TODO-completness: Support multiple external IPs
+pub(crate) const MAX_EXTERNAL_IPS_PER_INSTANCE: usize = 1;
 
 /// Manages an Oxide fleet -- the heart of the control plane
 pub struct Nexus {