Skip to content

Commit

Permalink
[Reconfigurator] Introduce PlanningInput that includes external net…
Browse files Browse the repository at this point in the history
…working information from CRDB (#5344)

The primary change in this PR is that the blueprint planner now wants a
`PlanningInput` (which contains a `Policy`) instead of a `Policy`. The
bulk of the diff is adding new `DataStore` methods (and tests for them)
that fetch all currently-allocated external IPs and NICs for services.

Some incidental changes that came along for the ride that I hope are not
controversial, but could be backed out if they are:

* `nexus_db_model::NetworkInterface::slot` is now a `SqlU8` instead of
an `i16`. I didn't have to change the queries here, so I think they're
still converting this to an `i16`, which is probably okay? I could make
a pass on them if needed though.
* I added an `omicron_uuid_kinds::ServiceKind` and started using it in
this PR. I did not attempt to make a pass through all service UUIDs to
start using this; I think this can be done incrementally?

Other notes:

* I'm not sure about the name `PlanningInput`. It feels vague; isn't
every argument to the planner a kind of "planning input"? But I'm not
sure what else to call "`Policy` plus extra CRDB state".
* This does not change execution at all. It's possible when I get to
that there will need to be some changes here, but I think this is
probably close enough that it can be reviewed, and any changes will be
small and can be rolled into the execution work.
  • Loading branch information
jgallagher authored Apr 1, 2024
1 parent e961b0b commit f2ea800
Show file tree
Hide file tree
Showing 21 changed files with 793 additions and 168 deletions.
4 changes: 4 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions dev-tools/omdb/src/bin/omdb/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2120,7 +2120,7 @@ async fn cmd_db_network_list_vnics(
struct NicRow {
ip: IpNetwork,
mac: MacAddr,
slot: i16,
slot: u8,
primary: bool,
kind: &'static str,
subnet: String,
Expand Down Expand Up @@ -2241,7 +2241,7 @@ async fn cmd_db_network_list_vnics(
let row = NicRow {
ip: nic.ip,
mac: *nic.mac,
slot: nic.slot,
slot: *nic.slot,
primary: nic.primary,
kind,
subnet,
Expand Down
1 change: 1 addition & 0 deletions dev-tools/reconfigurator-cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ nexus-reconfigurator-planning.workspace = true
nexus-reconfigurator-execution.workspace = true
nexus-types.workspace = true
omicron-common.workspace = true
omicron-uuid-kinds.workspace = true
# See omicron-rpaths for more about the "pq-sys" dependency.
pq-sys = "*"
reedline.workspace = true
Expand Down
67 changes: 62 additions & 5 deletions dev-tools/reconfigurator-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,22 @@ use nexus_reconfigurator_planning::planner::Planner;
use nexus_reconfigurator_planning::system::{
SledBuilder, SledHwInventory, SystemDescription,
};
use nexus_types::deployment::ExternalIp;
use nexus_types::deployment::PlanningInput;
use nexus_types::deployment::ServiceNetworkInterface;
use nexus_types::deployment::{Blueprint, UnstableReconfiguratorState};
use nexus_types::internal_api::params::DnsConfigParams;
use nexus_types::inventory::Collection;
use nexus_types::inventory::OmicronZonesConfig;
use nexus_types::inventory::SledRole;
use omicron_common::api::external::Generation;
use omicron_common::api::external::Name;
use omicron_uuid_kinds::{GenericUuid, OmicronZoneKind, TypedUuid};
use reedline::{Reedline, Signal};
use std::cell::RefCell;
use std::collections::BTreeMap;
use std::io::BufRead;
use std::net::IpAddr;
use swrite::{swriteln, SWrite};
use tabled::Tabled;
use uuid::Uuid;
Expand All @@ -50,6 +56,14 @@ struct ReconfiguratorSim {
/// blueprints created by the user
blueprints: IndexMap<Uuid, Blueprint>,

/// external IPs allocated to services
///
/// In the real system, external IPs have IDs, but those IDs only live in
/// CRDB - they're not part of the zone config sent from Reconfigurator to
/// sled-agent. This mimics the minimal bit of the CRDB `external_ip` table
/// we need.
external_ips: RefCell<IndexMap<IpAddr, Uuid>>,

/// internal DNS configurations
internal_dns: BTreeMap<Generation, DnsConfigParams>,
/// external DNS configurations
Expand Down Expand Up @@ -92,6 +106,49 @@ impl ReconfiguratorSim {
let _ = entry.or_insert(blueprint);
Ok(())
}

fn planning_input(
&self,
parent_blueprint: &Blueprint,
) -> anyhow::Result<PlanningInput> {
let policy = self.system.to_policy().context("generating policy")?;
let service_external_ips = parent_blueprint
.all_omicron_zones()
.filter_map(|(_, zone)| {
let Ok(Some(ip)) = zone.zone_type.external_ip() else {
return None;
};
let service_id =
TypedUuid::<OmicronZoneKind>::from_untyped_uuid(zone.id);
let external_ip = ExternalIp {
id: *self
.external_ips
.borrow_mut()
.entry(ip)
.or_insert_with(Uuid::new_v4),
ip: ip.into(),
};
Some((service_id, external_ip))
})
.collect();
let service_nics = parent_blueprint
.all_omicron_zones()
.filter_map(|(_, zone)| {
let nic = zone.zone_type.service_vnic()?;
let service_id =
TypedUuid::<OmicronZoneKind>::from_untyped_uuid(zone.id);
let nic = ServiceNetworkInterface {
id: nic.id,
mac: nic.mac,
ip: nic.ip.into(),
slot: nic.slot,
primary: nic.primary,
};
Some((service_id, nic))
})
.collect();
Ok(PlanningInput { policy, service_external_ips, service_nics })
}
}

/// interactive REPL for exploring the planner
Expand All @@ -115,6 +172,7 @@ fn main() -> anyhow::Result<()> {
system: SystemDescription::new(),
collections: IndexMap::new(),
blueprints: IndexMap::new(),
external_ips: RefCell::new(IndexMap::new()),
internal_dns: BTreeMap::new(),
external_dns: BTreeMap::new(),
log,
Expand Down Expand Up @@ -655,9 +713,8 @@ fn cmd_blueprint_plan(
.collections
.get(&collection_id)
.ok_or_else(|| anyhow!("no such collection: {}", collection_id))?;
let policy = sim.system.to_policy().context("generating policy")?;
let creator = "reconfigurator-sim";

let planning_input = sim.planning_input(parent_blueprint)?;
let planner = Planner::new_based_on(
sim.log.clone(),
parent_blueprint,
Expand Down Expand Up @@ -688,7 +745,7 @@ fn cmd_blueprint_plan(
// matter, either. We'll just pick the parent blueprint's.
parent_blueprint.internal_dns_version,
parent_blueprint.external_dns_version,
&policy,
&planning_input,
creator,
collection,
)
Expand All @@ -709,13 +766,13 @@ fn cmd_blueprint_edit(
let blueprint_id = args.blueprint_id;
let blueprint = sim.blueprint_lookup(blueprint_id)?;
let creator = args.creator.as_deref().unwrap_or("reconfigurator-cli");
let policy = sim.system.to_policy().context("assembling policy")?;
let planning_input = sim.planning_input(blueprint)?;
let mut builder = BlueprintBuilder::new_based_on(
&sim.log,
&blueprint,
blueprint.internal_dns_version,
blueprint.external_dns_version,
&policy,
&planning_input,
creator,
)
.context("creating blueprint builder")?;
Expand Down
6 changes: 6 additions & 0 deletions nexus/db-model/src/external_ip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,12 @@ pub struct ExternalIp {
pub is_probe: bool,
}

impl From<ExternalIp> for nexus_types::deployment::ExternalIp {
fn from(ext_ip: ExternalIp) -> Self {
Self { id: ext_ip.id, ip: ext_ip.ip }
}
}

/// A view type constructed from `ExternalIp` used to represent Floating IP
/// objects in user-facing APIs.
///
Expand Down
25 changes: 20 additions & 5 deletions nexus/db-model/src/network_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::schema::instance_network_interface;
use crate::schema::network_interface;
use crate::schema::service_network_interface;
use crate::Name;
use crate::SqlU8;
use chrono::DateTime;
use chrono::Utc;
use db_macros::Resource;
Expand Down Expand Up @@ -59,7 +60,7 @@ pub struct NetworkInterface {
// If neither is specified, auto-assign one of each?
pub ip: ipnetwork::IpNetwork,

pub slot: i16,
pub slot: SqlU8,
#[diesel(column_name = is_primary)]
pub primary: bool,
}
Expand Down Expand Up @@ -91,10 +92,10 @@ impl NetworkInterface {
name: self.name().clone(),
ip: self.ip.ip(),
mac: self.mac.into(),
subnet: subnet,
subnet,
vni: external::Vni::try_from(0).unwrap(),
primary: self.primary,
slot: self.slot.try_into().unwrap(),
slot: *self.slot,
}
}
}
Expand All @@ -117,7 +118,7 @@ pub struct InstanceNetworkInterface {
pub mac: MacAddr,
pub ip: ipnetwork::IpNetwork,

pub slot: i16,
pub slot: SqlU8,
#[diesel(column_name = is_primary)]
pub primary: bool,
}
Expand All @@ -140,11 +141,25 @@ pub struct ServiceNetworkInterface {
pub mac: MacAddr,
pub ip: ipnetwork::IpNetwork,

pub slot: i16,
pub slot: SqlU8,
#[diesel(column_name = is_primary)]
pub primary: bool,
}

impl From<ServiceNetworkInterface>
for nexus_types::deployment::ServiceNetworkInterface
{
fn from(nic: ServiceNetworkInterface) -> Self {
Self {
id: nic.id(),
mac: *nic.mac,
ip: nic.ip,
slot: *nic.slot,
primary: nic.primary,
}
}
}

impl NetworkInterface {
/// Treat this `NetworkInterface` as an `InstanceNetworkInterface`.
///
Expand Down
Loading

0 comments on commit f2ea800

Please sign in to comment.