-
Notifications
You must be signed in to change notification settings - Fork 62
BlueprintSledConfig: add sled subnet
#9416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…omoting internal NTP
| Diffable, | ||
| )] | ||
| #[schemars(rename = "Ipv6Subnet")] | ||
| pub struct Ipv6Subnet<const N: u8> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, https://docs.rs/oxnet has Ipv6Net but doesn't use a const generic. Nothing to change here but an interesting inconsistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a newtype around oxnet::Ipv6Net, actually! All it does is add the const generic.
| nic: NetworkInterface { | ||
| id: uuid::Uuid::new_v4(), | ||
| kind: NetworkInterfaceKind::Service { | ||
| id: zone_id.into_untyped_uuid(), | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ew, we should use typed UUIDs here :(
| .slot_b | ||
| .artifact_hash() | ||
| .map(ArtifactHash), | ||
| subnet: Ipv6Network::from(sled.subnet).into(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sighhh ideally we'd use oxnet here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC this is relying on diesel knowing this type can be converted to/from sql_types::Inet: https://github.com/diesel-rs/diesel/blob/b7337c66d2b522348fa4e0fb11c44e561444bb4c/diesel/src/pg/types/network_address.rs#L33-L37
To use oxnet we'd need to add diesel support to it, I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think so. Runs into annoying orphan rule issues.
Sled subnets are chosen when the sled is added to the cluster (either by RSS or by the "add sled" process), and do not change for the lifetime of the sled's membership. This PR adds that subnet to
BlueprintSledConfig, removing one of the remaining bits whereBlueprintBuilder::new_based_on()needs information other than the parent blueprint.This PR is staged on top of #9393.