Skip to content

Commit bd1f13b

Browse files
authored
Specify external DNS IPs in PlanningInput instead of deriving them from the parent blueprint (#9291)
This is built on top of #9250, and is related to both #9238 and #8949: * Trims down some of the `BlueprintResourceAllocator` internals * Adds an explicit `ExternalIpPolicy` type; today it holds a couple of IP pools and a set of external DNS IPs, but it gives us a cleaner place to integrate all the IP pool work that @bnaecker is doing with Reconfigurator Since we don't actually have external DNS IPs in a policy anywhere, this adds `DataStore::external_dns_external_ips_specified_by_rack_setup()` (which is implemented as "load the current target blueprint and scan it for external DNS zones"; i.e., exactly what the builder was doing before). We should eventually delete this method once it's possible for an operator to reconfigure the external DNS IPs, but this lets the planning input build up the policy as though we already had this information available, more or less. Other than that new method, _almost_ all the changes in this PR are to tests; the non-test changes are pretty minimal.
1 parent 4e046a8 commit bd1f13b

File tree

17 files changed

+825
-323
lines changed

17 files changed

+825
-323
lines changed

common/src/address.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,16 @@ impl Ipv4Range {
628628
let end_num = u32::from(self.last);
629629
end_num - start_num + 1
630630
}
631+
632+
/// Returns `true` if `self` has any IPs in common with `other`; false
633+
/// otherwise.
634+
pub fn overlaps(&self, other: &Ipv4Range) -> bool {
635+
// We're disjoint if we either end before other or begin after it; any
636+
// other combination means we have some IP(s) in common.
637+
let is_disjoint = self.last_address() < other.first_address()
638+
|| self.first_address() > other.last_address();
639+
!is_disjoint
640+
}
631641
}
632642

633643
impl From<Ipv4Addr> for Ipv4Range {
@@ -701,6 +711,16 @@ impl Ipv6Range {
701711
let end_num = u128::from(self.last);
702712
end_num - start_num + 1
703713
}
714+
715+
/// Returns `true` if `self` has any IPs in common with `other`; false
716+
/// otherwise.
717+
pub fn overlaps(&self, other: &Ipv6Range) -> bool {
718+
// We're disjoint if we either end before other or begin after it; any
719+
// other combination means we have some IP(s) in common.
720+
let is_disjoint = self.last_address() < other.first_address()
721+
|| self.first_address() > other.last_address();
722+
!is_disjoint
723+
}
704724
}
705725

706726
impl From<Ipv6Addr> for Ipv6Range {

dev-tools/reconfigurator-cli/tests/output/cmds-stdout

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -871,7 +871,7 @@ result:
871871
loaded sleds: 04ef3330-c682-4a08-8def-fcc4bef31bcd, 90c1102a-b9f5-4d88-92a2-60d54a2d98cc, dde1c0e2-b10d-4621-b420-f179f7a7a00a
872872
loaded collections: 6e066695-94bc-4250-bd63-fd799c166cc1
873873
loaded blueprints: (none)
874-
loaded service IP pool ranges: [V4(Ipv4Range { first: 192.0.2.2, last: 192.0.2.20 })]
874+
loaded external IP policy: ExternalIpPolicy { service_pool_ipv4_ranges: [Ipv4Range { first: 192.0.2.2, last: 192.0.2.20 }], service_pool_ipv6_ranges: [], external_dns_ips: {} }
875875
loaded internal DNS generations: (none)
876876
loaded external DNS generations: (none)
877877
config:

dev-tools/reconfigurator-cli/tests/output/cmds-target-release-stdout

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1038,7 +1038,7 @@ result:
10381038
loaded sleds: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c, 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6, d81c6a84-79b8-4958-ae41-ea46c9b19763
10391039
loaded collections: f45ba181-4b56-42cc-a762-874d90184a43, eb0796d5-ab8a-4f7b-a884-b4aeacb8ab51, 61f451b3-2121-4ed6-91c7-a550054f6c21
10401040
loaded blueprints: 184f10b3-61cb-41ef-9b93-3489b2bac559, dbcbd3d6-41ff-48ae-ac0b-1becc9b2fd21, 8da82a8e-bf97-4fbd-8ddd-9f6462732cf1, 58d5e830-0884-47d8-a7cd-b2b3751adeb4
1041-
loaded service IP pool ranges: [V4(Ipv4Range { first: 192.0.2.2, last: 192.0.2.20 })]
1041+
loaded external IP policy: ExternalIpPolicy { service_pool_ipv4_ranges: [Ipv4Range { first: 192.0.2.2, last: 192.0.2.20 }, Ipv4Range { first: 198.51.100.1, last: 198.51.100.30 }], service_pool_ipv6_ranges: [], external_dns_ips: {198.51.100.1, 198.51.100.2, 198.51.100.3} }
10421042
loaded internal DNS generations: (none)
10431043
loaded external DNS generations: (none)
10441044
config:

nexus/db-queries/src/db/datastore/deployment/external_networking.rs

Lines changed: 202 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ use nexus_db_model::IpConfig;
1616
use nexus_db_model::IpPool;
1717
use nexus_sled_agent_shared::inventory::ZoneKind;
1818
use nexus_types::deployment::BlueprintZoneConfig;
19+
use nexus_types::deployment::BlueprintZoneDisposition;
20+
use nexus_types::deployment::BlueprintZoneType;
1921
use nexus_types::deployment::OmicronZoneExternalIp;
2022
use omicron_common::api::external::Error;
2123
use omicron_common::api::external::IdentityMetadataCreateParams;
@@ -30,8 +32,57 @@ use slog::error;
3032
use slog::info;
3133
use slog::warn;
3234
use slog_error_chain::InlineErrorChain;
35+
use std::collections::BTreeSet;
36+
use std::net::IpAddr;
3337

3438
impl DataStore {
39+
/// Return the set of external IPs configured for our external DNS servers
40+
/// when the rack was set up.
41+
///
42+
/// We should have explicit storage for the external IPs on which we run
43+
/// external DNS that an operator can update. Today, we do not: whatever
44+
/// external DNS IPs are provided at rack setup time are the IPs we use
45+
/// forever. (Fixing this is tracked by
46+
/// <https://github.com/oxidecomputer/omicron/issues/8255>.)
47+
pub async fn external_dns_external_ips_specified_by_rack_setup(
48+
&self,
49+
opctx: &OpContext,
50+
) -> Result<BTreeSet<IpAddr>, Error> {
51+
// We can _implicitly_ determine the set of external DNS IPs provided
52+
// during rack setup by examining the current target blueprint and
53+
// looking at the IPs of all of its external DNS zones. We _must_
54+
// include expunged zones as well as in-service zones: during an update,
55+
// we'll create a blueprint that expunges an external DNS zone, waits
56+
// for it to go away, then wants to reassign that zone's external IP to
57+
// a new external DNS zone. But because we are scanning expunged zones,
58+
// we also have to allow for duplicates - this isn't an error and is
59+
// expected if we've performed more than one update, at least until we
60+
// start pruning old expunged zones out of the blueprint (tracked by
61+
// https://github.com/oxidecomputer/omicron/issues/5552).
62+
//
63+
// Because we can't (yet) change external DNS IPs, we don't have to
64+
// worry about the current blueprint changing between when we read it
65+
// and when we calculate the set of external DNS IPs: the set will be
66+
// identical for all blueprints back to the original one created by RSS.
67+
//
68+
// We don't really need to load the entire blueprint here, but it's easy
69+
// and ideally this code will be deleted in relatively short order.
70+
let (_target, blueprint) =
71+
self.blueprint_target_get_current_full(opctx).await?;
72+
73+
let external_dns_ips = blueprint
74+
.all_omicron_zones(BlueprintZoneDisposition::any)
75+
.filter_map(|(_sled_id, z)| match &z.zone_type {
76+
BlueprintZoneType::ExternalDns(external_dns) => {
77+
Some(external_dns.dns_address.addr.ip())
78+
}
79+
_ => None,
80+
})
81+
.collect();
82+
83+
Ok(external_dns_ips)
84+
}
85+
3586
pub(super) async fn ensure_zone_external_networking_allocated_on_connection(
3687
&self,
3788
conn: &async_bb8_diesel::Connection<DbConnection>,
@@ -461,9 +512,10 @@ mod tests {
461512
use chrono::Utc;
462513
use nexus_config::NUM_INITIAL_RESERVED_IP_ADDRESSES;
463514
use nexus_db_model::SqlU16;
515+
use nexus_reconfigurator_planning::blueprint_builder::BlueprintBuilder;
464516
use nexus_sled_agent_shared::inventory::OmicronZoneDataset;
517+
use nexus_types::deployment::BlueprintTarget;
465518
use nexus_types::deployment::BlueprintZoneConfig;
466-
use nexus_types::deployment::BlueprintZoneDisposition;
467519
use nexus_types::deployment::BlueprintZoneImageSource;
468520
use nexus_types::deployment::BlueprintZoneType;
469521
use nexus_types::deployment::OmicronZoneExternalFloatingAddr;
@@ -483,10 +535,12 @@ mod tests {
483535
use omicron_common::api::external::Vni;
484536
use omicron_common::zpool_name::ZpoolName;
485537
use omicron_test_utils::dev;
538+
use omicron_uuid_kinds::BlueprintUuid;
486539
use omicron_uuid_kinds::ExternalIpUuid;
540+
use omicron_uuid_kinds::ExternalZpoolUuid;
541+
use omicron_uuid_kinds::SledUuid;
487542
use omicron_uuid_kinds::ZpoolUuid;
488543
use oxnet::IpNet;
489-
use std::collections::BTreeSet;
490544
use std::net::IpAddr;
491545
use std::net::SocketAddr;
492546
use uuid::Uuid;
@@ -1292,4 +1346,150 @@ mod tests {
12921346
db.terminate().await;
12931347
logctx.cleanup_successful();
12941348
}
1349+
1350+
#[tokio::test]
1351+
async fn test_external_dns_external_ips_specified_by_rack_setup() {
1352+
const TEST_NAME: &str =
1353+
"test_external_dns_external_ips_specified_by_rack_setup";
1354+
1355+
// Helper closures to reduce boilerplate below.
1356+
let make_bp_target = |blueprint_id| BlueprintTarget {
1357+
target_id: blueprint_id,
1358+
enabled: false,
1359+
time_made_target: Utc::now(),
1360+
};
1361+
let mut opte_ip_iter = DNS_OPTE_IPV4_SUBNET.addr_iter();
1362+
let mut mac_iter = MacAddr::iter_system();
1363+
let mut make_external_dns_zone = |ip, disposition| {
1364+
let zone_id = OmicronZoneUuid::new_v4();
1365+
let pool = ZpoolName::External(ExternalZpoolUuid::new_v4());
1366+
BlueprintZoneConfig {
1367+
disposition,
1368+
id: zone_id,
1369+
filesystem_pool: pool,
1370+
zone_type: BlueprintZoneType::ExternalDns(
1371+
blueprint_zone_type::ExternalDns {
1372+
dataset: OmicronZoneDataset { pool_name: pool },
1373+
http_address: "[::1]:0".parse().unwrap(),
1374+
dns_address: OmicronZoneExternalFloatingAddr {
1375+
id: ExternalIpUuid::new_v4(),
1376+
addr: SocketAddr::new(ip, 0),
1377+
},
1378+
nic: NetworkInterface {
1379+
id: Uuid::new_v4(),
1380+
kind: NetworkInterfaceKind::Service {
1381+
id: zone_id.into_untyped_uuid(),
1382+
},
1383+
name: "test-external-dns".parse().unwrap(),
1384+
ip: opte_ip_iter.next().unwrap().into(),
1385+
mac: mac_iter.next().unwrap(),
1386+
subnet: IpNet::from(*DNS_OPTE_IPV4_SUBNET),
1387+
vni: Vni::SERVICES_VNI,
1388+
primary: true,
1389+
slot: 0,
1390+
transit_ips: vec![],
1391+
},
1392+
},
1393+
),
1394+
image_source: BlueprintZoneImageSource::InstallDataset,
1395+
}
1396+
};
1397+
1398+
// Set up.
1399+
let logctx = dev::test_setup_log(TEST_NAME);
1400+
let db = TestDatabase::new_with_datastore(&logctx.log).await;
1401+
let (opctx, datastore) = (db.opctx(), db.datastore());
1402+
1403+
// Create a blueprint with one sled and no zones. Insert it and make it
1404+
// the target.
1405+
let sled_id = SledUuid::new_v4();
1406+
let bp0 = BlueprintBuilder::build_empty_with_sleds(
1407+
std::iter::once(sled_id),
1408+
TEST_NAME,
1409+
);
1410+
datastore.blueprint_insert(opctx, &bp0).await.expect("inserted bp0");
1411+
datastore
1412+
.blueprint_target_set_current(opctx, make_bp_target(bp0.id))
1413+
.await
1414+
.expect("made bp0 the target");
1415+
1416+
// No external DNS zones => no external DNS IPs.
1417+
let external_dns_ips = datastore
1418+
.external_dns_external_ips_specified_by_rack_setup(opctx)
1419+
.await
1420+
.expect("got external DNS IPs");
1421+
assert_eq!(external_dns_ips, BTreeSet::new());
1422+
1423+
// Create a blueprint with three in-service external DNS zones. We
1424+
// should get their IPs back.
1425+
let expected_ips = ["192.168.1.1", "192.168.1.2", "192.168.1.3"]
1426+
.into_iter()
1427+
.map(|ip| ip.parse::<IpAddr>().unwrap())
1428+
.collect::<BTreeSet<_>>();
1429+
let mut bp1 = bp0.clone();
1430+
bp1.id = BlueprintUuid::new_v4();
1431+
bp1.parent_blueprint_id = Some(bp0.id);
1432+
for &ip in &expected_ips {
1433+
bp1.sleds.get_mut(&sled_id).unwrap().zones.insert(
1434+
make_external_dns_zone(ip, BlueprintZoneDisposition::InService),
1435+
);
1436+
}
1437+
1438+
// Insert bp1 and make it the target. Confirm we get back the expected
1439+
// external DNS IPs.
1440+
datastore.blueprint_insert(opctx, &bp1).await.expect("inserted bp1");
1441+
datastore
1442+
.blueprint_target_set_current(opctx, make_bp_target(bp1.id))
1443+
.await
1444+
.expect("made bp1 the target");
1445+
let external_dns_ips = datastore
1446+
.external_dns_external_ips_specified_by_rack_setup(opctx)
1447+
.await
1448+
.expect("got external DNS IPs");
1449+
assert_eq!(external_dns_ips, expected_ips);
1450+
1451+
// Create a third blueprint with multiple expunged external DNS zones
1452+
// covering a couple additional IPs. Those should also be returned.
1453+
let extra_ips = ["192.168.1.4", "192.168.1.5"]
1454+
.into_iter()
1455+
.map(|ip| ip.parse::<IpAddr>().unwrap())
1456+
.collect::<BTreeSet<_>>();
1457+
assert_eq!(expected_ips.intersection(&extra_ips).count(), 0);
1458+
1459+
let mut bp2 = bp1.clone();
1460+
bp2.id = BlueprintUuid::new_v4();
1461+
bp2.parent_blueprint_id = Some(bp1.id);
1462+
for &ip in &extra_ips {
1463+
for i in 0..4 {
1464+
bp2.sleds.get_mut(&sled_id).unwrap().zones.insert(
1465+
make_external_dns_zone(
1466+
ip,
1467+
BlueprintZoneDisposition::Expunged {
1468+
as_of_generation: Generation::new(),
1469+
ready_for_cleanup: i % 2 == 0,
1470+
},
1471+
),
1472+
);
1473+
}
1474+
}
1475+
1476+
// Insert bp1 and make it the target. Confirm we get back the expected
1477+
// external DNS IPs.
1478+
datastore.blueprint_insert(opctx, &bp2).await.expect("inserted bp2");
1479+
datastore
1480+
.blueprint_target_set_current(opctx, make_bp_target(bp2.id))
1481+
.await
1482+
.expect("made bp2 the target");
1483+
let external_dns_ips = datastore
1484+
.external_dns_external_ips_specified_by_rack_setup(opctx)
1485+
.await
1486+
.expect("got external DNS IPs");
1487+
let expected_ips =
1488+
expected_ips.union(&extra_ips).copied().collect::<BTreeSet<_>>();
1489+
assert_eq!(external_dns_ips, expected_ips);
1490+
1491+
// Clean up.
1492+
db.terminate().await;
1493+
logctx.cleanup_successful();
1494+
}
12951495
}

0 commit comments

Comments
 (0)