Generalize network_interface table to allow for new kinds of NICs. #2767
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, all NIC records in the DB were tied to a guest instance. In enabling OPTE usage for services, it'd be nice to be able to reuse a lot of the same NetworkInterface machinery we already have without duplicating it completely. This commit adds a new `kind` column to the `network_interface` table which at the moment will be either 'instance' (NICs attached to a guest VM and exposed in the external API) or 'service' (NIC associated with an internal control plane service). The previous `instance_id` column is renamed to `parent_id` and the table to which it refers to as a FK is now dependant on the kind (either `instance` or `service`). Since a lot of the db and authz lookup macros end up relying on the specific column name, this also introduces database views for each kind which can be queried as if they were their own tables. This also allows differentiating the different kinds of NICs as necessary. CockroachDB, unlike Postgres, does not allow inserting or updating into simple view and so we also model the base table itself to execute queries that modify it. From an external perspective, this doesn't change anything (modulo some renaming from `NetworkInterface*` -> `InstanceNetworkInterface*`) in that all external APIs still only deal with instance NICs. There are some basic definitions for service NICs included but those will be more fleshed in subsequent commits.
zephraph
reviewed
Apr 5, 2023
|
This is great. I was working on implementing silo level images and doing it in a way that was much harder than what you've done here. I'm definitely interested in following this approach. |
Co-authored-by: Justin Bennett <oxide@just-be.dev>
zephraph
reviewed
Apr 5, 2023
jmpesp
approved these changes
Apr 6, 2023
zephraph
added a commit
that referenced
this pull request
Apr 19, 2023
Implements silo level images as a replacement for global images. This implementation preserves a single API endpoint `/v1/images` and a single database table to store all images. It uses database views like #2767 to map the notion of `ProjectImage` and `SiloImage` to the images table. From an authz perspective there are actually _three_ resources represented:`SiloImage`, `ProjectImage`, and `Image`. As the names suggest, `SiloImage` is the child of a silo and `ProjectImage` is the child of a project. `Image` occupies an odd space where it's currently considered the child of a Silo though it technically straddles both positions in the hierarchy. --------- Co-authored-by: David Crespo <david-crespo@users.noreply.github.com> Co-authored-by: David Crespo <david@oxidecomputer.com>
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since I wanted to split up #2419 a bit, took the opportunity to get rid of the
duplicate table I added to get going initially.
Previously, all NIC records in the DB were tied to a guest instance. In
enabling OPTE usage for services, it'd be nice to be able to reuse a lot
of the same NetworkInterface machinery we already have without
duplicating it completely. This commit adds a new
kindcolumn to thenetwork_interfacetable which at the moment will be either 'instance'(NICs attached to a guest VM and exposed in the external API) or
'service' (NIC associated with an internal control plane service). The
previous
instance_idcolumn is renamed toparent_idand the table towhich it refers to as a FK is now dependant on the kind (either
instanceorservice).Since a lot of the db and authz lookup macros end up relying on the
specific column name, this also introduces database views for each kind
which can be queried as if they were their own tables. This also allows
differentiating the different kinds of NICs as necessary.
CockroachDB, unlike Postgres, does not allow inserting or updating into
simple view and so we also model the base table itself to execute
queries that modify it.
From an external perspective, this doesn't change anything (modulo some
renaming from
NetworkInterface*->InstanceNetworkInterface*) inthat all external APIs still only deal with instance NICs.
There are some basic definitions for service NICs included but those
will be more fleshed in subsequent commits.