Drop the service table
#5287
Drop the service table
#5287
Conversation
|
We discussed this in |
|
This is caught up with |
There was a problem hiding this comment.
Great!
In #4947 I wrote:
Given that, I think we can just drop the services table altogether. There is only one thing that uses it as far as I know, and that's DNS propagation.
Boy did that turn out to be wrong. Sorry!
| @@ -873,20 +873,6 @@ table! { | |||
| } | |||
| } | |||
|
|
|||
| table! { | |||
nexus/src/app/silo.rs
Outdated
| @@ -100,7 +100,14 @@ impl super::Nexus { | |||
| .blueprint_target_get_current_full(opctx) | |||
| .await | |||
| .internal_context("loading target blueprint")?; | |||
| let target = target_blueprint.as_ref().map(|(_, blueprint)| blueprint); | |||
| let target = target_blueprint | |||
There was a problem hiding this comment.
Maybe we want to have blueprint_target_get_current_full() return this 500 itself now that we expect all systems to have a target blueprint.
There was a problem hiding this comment.
This had more knock-on effects than I expected, but I think basically all positive: 78f9429
#5287 got rid of `omdb db services ...`, so I've found myself wanting to see the current target blueprint. omdb can do that, but it required listing the blueprints, visually finding the target, then calling `nexus blueprints show <ID>`. With this PR, we can accept the string `current-target` instead, and I figured that might also be useful in diffs (e.g., `nexus blueprints diff current-target <SOME_ID>` to see changes from the current target to some other blueprint).
I still need to test this on madrid and I do not want to merge it before we cut the next release, but I believe this is ready for review.
Related changes / fallout also included in this PR:
omdb db services ...subcommands are all gone. I believe this functionality is covered byomdb's inspection of blueprints instead.SledResourceKind::{Dataset,Service,Reserved}variants that were unused. This isn't required, strictly speaking, butSledResourceKind::Servicewas intended to reference theservicetable, so I thought it was clearer to drop these for now (we can add them back when we get to the point of using them).There are two major pieces of functionality that now require systems to have a current target blueprint set:
DataStore::nexus_external_addresses()now takes an explicitBlueprintinstead of anOption<Blueprint>. Its callers (silo creation and reconfigurator DNS updates) fail if they cannot read the current target blueprint.DataStore::vpc_resolve_to_sleds()now implicitly requires a target blueprint to be set, if and only if the VPC being queried involves control plane services. (In practice today, that means the VPC ID is exactlySERVICES_VPC_ID, although in the future we could have multiple service-related VPCs.) I didn't want to make this method take an explicit blueprint, because I think its most frequent callers are specifying instance VPCs, which do not need to access the blueprint.These two together mean that deploying this change to a system without a target blueprint will result in (a) the inability to create silos or update external DNS via reconfigurator and (b) services (including Nexus) will not get the OPTE firewall rules they need to allow incoming traffic. All newly-deployed systems have a (disabled) blueprint set as of #5244, but we'll need to perform the necessary support operation to bring already-deployed systems in line.
Closes #4947.