Adds network interface attach/detach

[bnaecker]

• Add helpers to check if an address provided by a user can be
  requested, i.e., is in the subnet and is not a reserved address.
• Add a query used to insert a network interface into the database,
  handling both IP allocation and validation of any provided instance
  (i.e., that the instance spans exactly one VPC). This subsumes the
  previous IP allocation query.
• Removes the instance_create_network_interface method, moving the
  instance create saga to use the new
  vpc_subnet_create_network_interface method, with an instance
  provided in the construction request.
• Adds parameters for setting up network interfaces (possibly multiple)
  in the instance create request.
• Adds support for specifying zero or more network interfaces at
  instance creation. These must be new interfaces, support for attaching
  to existing interfaces is not yet implemented.

[bnaecker]

An apology is in order for the size of this PR :) I didn't quite do my homework, and intended to update Omicron to support: creating interfaces for an instance, at the time the instance is created; creating an interface, then creating an instance attached to that previously-created interface; and creating an interface, and attaching it to a running instance.

That was overly ambitious, and couldn't possibly work, since hot-plug of interfaces isn't supported by either the sled agent or Propolis at this point. So this code updated some of Nexus to support all of those cases, but then reined in the scope to just the first. One can now create zero or more network interfaces for an instance, at the time the instance is created. The other paths will come in follow-up work, probably deferred for a while.

[smklein]

First pass - looks solid, and dang those are some fancy CTEs

[bnaecker]

@smklein @davepacheco This is ready for another review.

I had previously convinced myself that it should be possible to create a network interface that is not attached to any particular instance. This is similar to how disks work, but it really doesn't make much sense for interfaces. A network interface is better thought of as a binding of an IP address to an instance. We want to add support for reserving particular IPs, not assigned to an instance, but these will be managed as a separate resource and table.

The latest commit here removes the optional/nullable instance_id from a network interface object. It also moves the endpoints (and internal methods) for creating/deleting network interfaces under an instances URL, to be explicit that we create an interface for a particular instance.

[davepacheco]

Nice! I still need to take a look at subnet_allocation.rs but I wanted to post this first.

[davepacheco]

I wonder if any of this belongs in nexus/src/db/error.rs, and whether you can reuse any of the functions there. Mostly there's just some details about the structure of these errors that seems tricky and it'd be nice to not duplicate it. But I don't immediately see how to do that without refactoring those functions further.

[bnaecker]

Yeah, I'm happy to move this around.

I am also a bit leery of this, to be honest. These error messages are extremely coupled to the database, and it's very easy to get them wrong or forget to update them as we move indexes around. I've already done that once in the process of this PR: I moved a unique index on (vpc_id, name) to one on (instance_id, name), and forgot to update the constraint name. I really wish there were a better way to do this, but maybe it just comes with the territory of trying to catch lots of different errors from the database in one query.

I think one way to refactor usefully might be to check that the database failed a request because it violated a given constraint. For example, we could define an enum with the error variants we want to catch, along with the name of the constraint (or other error condition) that should be derived from. Then we can have a from_pool function which is basically this match expression, just looking for those specific strings and translating to the enum variant.

I have a bug-fix PR lined up where I do this whole song-and-dance again. I'll look at it through this lens, to see if a trait or other shared code would work.

[davepacheco]

I moved a unique index on (vpc_id, name) to one on (instance_id, name), and forgot to update the constraint name. I really wish there were a better way to do this, but maybe it just comes with the territory of trying to catch lots of different errors from the database in one query.

It'd be neat if Diesel knew about constraints the way it knows about tables, columns, etc.

[bnaecker]

I'm going to hold off moving this around for this PR, but I'll prototype how to make this easier in the bug-fix PR I'm also working on.

[bnaecker]

I've experimented with this for a bit. It can be made to work, but I'm really not sure it's worth it. Specifically, I don't think it reduces the complexity -- the solution I have in mind is definitely more opaque to me than the original. It does allow us to package the diesel::result::DatabaseErrorKind, string we want to check for, and the "handler" for building our error type from the database error and the candidate record. But it's pretty complicated. I'm happy to show you offline, or post it here, whatever works. I'll mull it over some more tomorrow, and maybe add it to omicron_nexus::db::error if it seems useful.

[bnaecker]

Just a note, there are now some conflicts with main. I'll be rebasing (and squashing) all this manually, after the reviews are completed.