From 44147179f923867292910df2714ff5a5a2ca1f9d Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Wed, 3 Jul 2024 16:26:16 -0400 Subject: [PATCH 1/8] Add boundary NTP internal DNS name with AAAA records for chrony --- internal-dns/src/config.rs | 24 +++++++- internal-dns/src/names.rs | 7 +++ nexus/reconfigurator/execution/src/dns.rs | 68 ++++++++++++++++++++--- 3 files changed, 90 insertions(+), 9 deletions(-) diff --git a/internal-dns/src/config.rs b/internal-dns/src/config.rs index 43d6c96d2d8..df76707bdbf 100644 --- a/internal-dns/src/config.rs +++ b/internal-dns/src/config.rs @@ -60,7 +60,7 @@ //! //! This module provides types used to assemble that configuration. -use crate::names::{ServiceName, DNS_ZONE}; +use crate::names::{ServiceName, BOUNDARY_NTP_DNS_NAME, DNS_ZONE}; use anyhow::{anyhow, ensure}; use core::fmt; use dns_service_client::types::{DnsConfigParams, DnsConfigZone, DnsRecord}; @@ -407,6 +407,27 @@ impl DnsConfigBuilder { (name, vec![DnsRecord::Aaaa(sled_ip)]) }); + // Assemble the special boundary NTP name to support chrony on internal + // NTP zones. + // + // We leave this as `None` if there are no `BoundaryNtp` service zones, + // which omits it from the final set of records. + let boundary_ntp_records = self + .service_instances_zones + .get(&ServiceName::BoundaryNtp) + .map(|zone2port| { + let records = zone2port + .iter() + .map(|(zone, _port)| { + let zone_ip = self.zones.get(&zone).expect( + "service_backend_zone() ensures zones are defined", + ); + DnsRecord::Aaaa(*zone_ip) + }) + .collect::>(); + (format!("{BOUNDARY_NTP_DNS_NAME}.{DNS_ZONE}"), records) + }); + // Assemble the set of AAAA records for zones. let zone_records = self.zones.into_iter().map(|(zone, zone_ip)| { (zone.dns_name(), vec![DnsRecord::Aaaa(zone_ip)]) @@ -454,6 +475,7 @@ impl DnsConfigBuilder { let all_records = sled_records .chain(zone_records) + .chain(boundary_ntp_records) .chain(srv_records_sleds) .chain(srv_records_zones) .collect(); diff --git a/internal-dns/src/names.rs b/internal-dns/src/names.rs index 3017d3b3fc7..2dc989e056a 100644 --- a/internal-dns/src/names.rs +++ b/internal-dns/src/names.rs @@ -6,6 +6,13 @@ use omicron_uuid_kinds::{OmicronZoneUuid, SledUuid}; +/// Name for the special boundary NTP DNS name +/// +/// chrony does not support SRV records. This name resolves to AAAA records for +/// each boundary NTP zone, and then we can point internal NTP chrony instances +/// at this name for it to find the boundary NTP zones. +pub const BOUNDARY_NTP_DNS_NAME: &str = "boundary.ntp"; + /// Name for the control plane DNS zone pub const DNS_ZONE: &str = "control-plane.oxide.internal"; diff --git a/nexus/reconfigurator/execution/src/dns.rs b/nexus/reconfigurator/execution/src/dns.rs index 3504d41e4dd..faaf6ee09c4 100644 --- a/nexus/reconfigurator/execution/src/dns.rs +++ b/nexus/reconfigurator/execution/src/dns.rs @@ -457,6 +457,9 @@ mod test { use crate::overridables::Overridables; use crate::Sled; use dns_service_client::DnsDiff; + use internal_dns::config::Host; + use internal_dns::config::Zone; + use internal_dns::names::BOUNDARY_NTP_DNS_NAME; use internal_dns::resolver::Resolver; use internal_dns::ServiceName; use internal_dns::DNS_ZONE; @@ -662,7 +665,7 @@ mod test { }) .collect(); - let blueprint_dns_zone = blueprint_internal_dns_config( + let mut blueprint_dns_zone = blueprint_internal_dns_config( &blueprint, &sleds_by_id, &Default::default(), @@ -686,6 +689,10 @@ mod test { // 4. Our out-of-service zone does *not* appear in the DNS config, // neither with an AAAA record nor in an SRV record. // + // 5. The boundary NTP zones' IP addresses are mapped to AAAA records in + // the special boundary DNS name (in addition to having their normal + // zone DNS name -> AAAA record from 1). + // // Together, this tells us that we have SRV records for all services, // that those SRV records all point to at least one of the Omicron zones // for that service, and that we correctly ignored zones that were not @@ -720,6 +727,33 @@ mod test { }) .collect(); + // Prune the special boundary NTP DNS name out, collecting their IP + // addresses, and build a list of expected SRV targets to ensure these + // IPs show up both in the special boundary NTP DNS name and as their + // normal SRV records. + let boundary_ntp_ips = blueprint_dns_zone + .records + .remove(&format!("{BOUNDARY_NTP_DNS_NAME}.{DNS_ZONE}")) + .expect("missing boundary NTP DNS name") + .into_iter() + .map(|record| match record { + DnsRecord::Aaaa(ip) => ip, + _ => panic!("expected AAAA record; got {record:?}"), + }); + let mut expected_boundary_ntp_srv_targets = boundary_ntp_ips + .map(|ip| { + let Some(zone_id) = omicron_zones_by_ip.get(&ip) else { + panic!("did not find zone ID for boundary NTP IP {ip}"); + }; + let name = Host::Zone(Zone::Other(*zone_id)).fqdn(); + println!( + "Boundary NTP IP {ip} maps to expected \ + SRV record target {name}" + ); + name + }) + .collect::>(); + // Now go through all the DNS names that have AAAA records and remove // any corresponding Omicron zone. While doing this, construct a set of // the fully-qualified DNS names (i.e., with the zone name suffix @@ -814,6 +848,16 @@ mod test { ]); for (name, records) in &blueprint_dns_zone.records { + let mut this_kind = None; + let kinds_left: Vec<_> = + srv_kinds_expected.iter().copied().collect(); + for kind in kinds_left { + if kind.dns_name() == *name { + srv_kinds_expected.remove(&kind); + this_kind = Some(kind); + } + } + let srvs: Vec<_> = records .iter() .filter_map(|dns_record| match dns_record { @@ -828,19 +872,27 @@ mod test { correspond to a name that points to any Omicron zone", srv.target ); - } - - let kinds_left: Vec<_> = - srv_kinds_expected.iter().copied().collect(); - for kind in kinds_left { - if kind.dns_name() == *name { - srv_kinds_expected.remove(&kind); + if this_kind == Some(ServiceName::BoundaryNtp) { + assert!( + expected_boundary_ntp_srv_targets.contains(&srv.target), + "found boundary NTP SRV record with target {:?} \ + that does not correspond to an expected boundary \ + NTP zone", + srv.target, + ); + expected_boundary_ntp_srv_targets.remove(&srv.target); } } } println!("SRV kinds with no records found: {:?}", srv_kinds_expected); assert!(srv_kinds_expected.is_empty()); + + println!( + "Boundary NTP SRV targets not found: {:?}", + expected_boundary_ntp_srv_targets + ); + assert!(expected_boundary_ntp_srv_targets.is_empty()); } #[tokio::test] From 3c49902525ff6cfe9b879a5096de1537012d179b Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Mon, 8 Jul 2024 13:40:45 -0400 Subject: [PATCH 2/8] add --boundary-pool arg to chrony zone-setup --- sled-agent/src/services.rs | 16 ++++++++++------ smf/chrony-setup/manifest.xml | 3 ++- zone-setup/src/bin/zone-setup.rs | 19 +++++++++++++++++++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index bc4e78a6d8b..a79d5b68e7c 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -60,6 +60,8 @@ use illumos_utils::zfs::ZONE_ZFS_RAMDISK_DATASET_MOUNTPOINT; use illumos_utils::zone::AddressRequest; use illumos_utils::zpool::{PathInPool, ZpoolName}; use illumos_utils::{execute, PFEXEC}; +use internal_dns::names::BOUNDARY_NTP_DNS_NAME; +use internal_dns::names::DNS_ZONE; use internal_dns::resolver::Resolver; use itertools::Itertools; use nexus_config::{ConfigDropshotWithTls, DeploymentConfig}; @@ -1994,15 +1996,17 @@ impl ServiceManager { .add_property( "boundary", "boolean", - &is_boundary.to_string(), + is_boundary.to_string(), + ) + .add_property( + "boundary_pool", + "astring", + format!("{BOUNDARY_NTP_DNS_NAME}.{DNS_ZONE}"), ); for s in ntp_servers { - chrony_config = chrony_config.add_property( - "server", - "astring", - &s.to_string(), - ); + chrony_config = + chrony_config.add_property("server", "astring", s); } let dns_client_service; diff --git a/smf/chrony-setup/manifest.xml b/smf/chrony-setup/manifest.xml index f31f13a2ea0..706b407eb5e 100644 --- a/smf/chrony-setup/manifest.xml +++ b/smf/chrony-setup/manifest.xml @@ -12,7 +12,7 @@ + diff --git a/zone-setup/src/bin/zone-setup.rs b/zone-setup/src/bin/zone-setup.rs index f335512d833..de79aded4a6 100644 --- a/zone-setup/src/bin/zone-setup.rs +++ b/zone-setup/src/bin/zone-setup.rs @@ -104,6 +104,13 @@ struct ChronySetupArgs { /// allowed IPv6 range #[arg(short, long)] allow: Ipv6Net, + /// DNS name for the boundary NTP zone pool + #[arg( + short = 'p', + long, + value_parser = NonEmptyStringValueParser::default(), + )] + boundary_pool: String, } // The default clap parser for `serde_json::Value` is to wrap the argument in a @@ -396,6 +403,9 @@ makestep 1.0 3 leapsecmode slew maxslewrate 2708.333 +# Refresh boundary NTP servers every two minutes instead of every two weeks +refresh 120 + "; let boundary_ntp_tpl = "# @@ -447,6 +457,7 @@ maxslewrate 2708.333 boundary: is_boundary, servers, allow, + boundary_pool, } = args; let mut new_config = @@ -464,10 +475,18 @@ maxslewrate 2708.333 .expect("write to String is infallible"); } } else { + // TODO-john Replace with link to issue: remove specific boundary NTP + // servers after R10 is cut; once all racks are setting up the boundary + // NTP pool we can drop individual server lines. for s in servers { writeln!(&mut new_config, "server {s} iburst minpoll 0 maxpoll 4") .expect("write to String is infallible"); } + writeln!( + &mut new_config, + "pool {boundary_pool} iburst maxdelay 0.1 maxsources 16", + ) + .expect("write to String is infallible"); } // We read the contents from the old configuration file if it existed From fe660b4ca52679289b1693fd825729f64fe663e1 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 9 Jul 2024 10:27:27 -0400 Subject: [PATCH 3/8] SMF XML typo fix --- smf/chrony-setup/manifest.xml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/smf/chrony-setup/manifest.xml b/smf/chrony-setup/manifest.xml index 706b407eb5e..ec6f3f1221f 100644 --- a/smf/chrony-setup/manifest.xml +++ b/smf/chrony-setup/manifest.xml @@ -12,7 +12,7 @@ - From ef7340a726615124660b5fe9f64b60b406bbf0a0 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 9 Jul 2024 10:46:01 -0400 Subject: [PATCH 4/8] don't attach DNS zone too early --- internal-dns/src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal-dns/src/config.rs b/internal-dns/src/config.rs index df76707bdbf..3d033293a2b 100644 --- a/internal-dns/src/config.rs +++ b/internal-dns/src/config.rs @@ -425,7 +425,7 @@ impl DnsConfigBuilder { DnsRecord::Aaaa(*zone_ip) }) .collect::>(); - (format!("{BOUNDARY_NTP_DNS_NAME}.{DNS_ZONE}"), records) + (BOUNDARY_NTP_DNS_NAME.to_string(), records) }); // Assemble the set of AAAA records for zones. From cf33427d800bf6acd99ab37c2783318e34d3150e Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 11 Jul 2024 13:56:37 -0400 Subject: [PATCH 5/8] test fix --- nexus/reconfigurator/execution/src/dns.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/reconfigurator/execution/src/dns.rs b/nexus/reconfigurator/execution/src/dns.rs index faaf6ee09c4..690a4348b00 100644 --- a/nexus/reconfigurator/execution/src/dns.rs +++ b/nexus/reconfigurator/execution/src/dns.rs @@ -733,7 +733,7 @@ mod test { // normal SRV records. let boundary_ntp_ips = blueprint_dns_zone .records - .remove(&format!("{BOUNDARY_NTP_DNS_NAME}.{DNS_ZONE}")) + .remove(BOUNDARY_NTP_DNS_NAME) .expect("missing boundary NTP DNS name") .into_iter() .map(|record| match record { From 1098aca3eabb4bd2e745a6ae2ce2d40c3d82f27a Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 30 Jul 2024 14:08:47 -0400 Subject: [PATCH 6/8] review feedback --- internal-dns/src/names.rs | 2 +- smf/chrony-setup/manifest.xml | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/internal-dns/src/names.rs b/internal-dns/src/names.rs index 2dc989e056a..f975029d69e 100644 --- a/internal-dns/src/names.rs +++ b/internal-dns/src/names.rs @@ -11,7 +11,7 @@ use omicron_uuid_kinds::{OmicronZoneUuid, SledUuid}; /// chrony does not support SRV records. This name resolves to AAAA records for /// each boundary NTP zone, and then we can point internal NTP chrony instances /// at this name for it to find the boundary NTP zones. -pub const BOUNDARY_NTP_DNS_NAME: &str = "boundary.ntp"; +pub const BOUNDARY_NTP_DNS_NAME: &str = "boundary-ntp"; /// Name for the control plane DNS zone pub const DNS_ZONE: &str = "control-plane.oxide.internal"; diff --git a/smf/chrony-setup/manifest.xml b/smf/chrony-setup/manifest.xml index ec6f3f1221f..fca5d3f2e03 100644 --- a/smf/chrony-setup/manifest.xml +++ b/smf/chrony-setup/manifest.xml @@ -25,9 +25,19 @@ + + + + From fa58d40f8bea31e9534fa28134047d1bef11823a Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Wed, 7 Aug 2024 16:11:46 -0400 Subject: [PATCH 7/8] replace TODO with issue link --- zone-setup/src/bin/zone-setup.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/zone-setup/src/bin/zone-setup.rs b/zone-setup/src/bin/zone-setup.rs index de79aded4a6..167adf04bfc 100644 --- a/zone-setup/src/bin/zone-setup.rs +++ b/zone-setup/src/bin/zone-setup.rs @@ -475,9 +475,10 @@ maxslewrate 2708.333 .expect("write to String is infallible"); } } else { - // TODO-john Replace with link to issue: remove specific boundary NTP - // servers after R10 is cut; once all racks are setting up the boundary - // NTP pool we can drop individual server lines. + // TODO-cleanup: Remove specific boundary NTP servers after R10 is cut; + // once all racks are setting up the boundary NTP pool we can drop + // individual server lines: + // https://github.com/oxidecomputer/omicron/issues/6261 for s in servers { writeln!(&mut new_config, "server {s} iburst minpoll 0 maxpoll 4") .expect("write to String is infallible"); From 24b7562795769a6cde1de2e045f35f9c64ff968e Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 8 Aug 2024 16:06:12 -0400 Subject: [PATCH 8/8] update internal-dns unit test to include boundary NTP --- internal-dns/src/config.rs | 5 +++++ internal-dns/tests/output/internal-dns-zone.txt | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/internal-dns/src/config.rs b/internal-dns/src/config.rs index 3d033293a2b..a9ff664030e 100644 --- a/internal-dns/src/config.rs +++ b/internal-dns/src/config.rs @@ -615,6 +615,11 @@ mod test { b.service_backend_zone(ServiceName::Oximeter, &zone2, 125).unwrap(); b.service_backend_zone(ServiceName::Oximeter, &zone3, 126).unwrap(); + // Add a boundary NTP service to one of the zones; this will also + // populate the special `BOUNDARY_NTP_DNS_NAME`. + b.service_backend_zone(ServiceName::BoundaryNtp, &zone2, 127) + .unwrap(); + // A sharded service b.service_backend_sled( ServiceName::SledAgent(sled1_uuid), diff --git a/internal-dns/tests/output/internal-dns-zone.txt b/internal-dns/tests/output/internal-dns-zone.txt index e8c3f01b05c..d87805f6773 100644 --- a/internal-dns/tests/output/internal-dns-zone.txt +++ b/internal-dns/tests/output/internal-dns-zone.txt @@ -68,6 +68,17 @@ builder: "non_trivial" "data": "::1:4" } ], + "_boundary-ntp._tcp": [ + { + "type": "SRV", + "data": { + "port": 127, + "prio": 0, + "target": "001de000-c04e-4000-8000-000000000002.host.control-plane.oxide.internal", + "weight": 0 + } + } + ], "_nexus._tcp": [ { "type": "SRV", @@ -118,5 +129,11 @@ builder: "non_trivial" "weight": 0 } } + ], + "boundary-ntp": [ + { + "type": "AAAA", + "data": "::1:2" + } ] }