Skip to content

Add the CRDB HTTP address to DNS, blueprint #8351

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

Open
wants to merge 8 commits into
base: crdb-parse-http-addr
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions common/src/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ pub const REPO_DEPOT_PORT: u16 = 12348;

pub const COCKROACH_PORT: u16 = 32221;
pub const COCKROACH_ADMIN_PORT: u16 = 32222;
pub const COCKROACH_HTTP_PORT: u16 = 32223;
pub const CRUCIBLE_PORT: u16 = 32345;
pub const CLICKHOUSE_HTTP_PORT: u16 = 8123;
pub const CLICKHOUSE_INTERSERVER_PORT: u16 = 9009;
Expand Down
36 changes: 36 additions & 0 deletions internal-dns/types/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,38 @@ impl DnsConfigBuilder {
)
}

/// Higher-level shorthand for adding a Cockroach zone with several
/// services.
///
/// # Errors
///
/// This fails if the provided `addresses` aren't using the same IP address.
/// It also fails if the services have already been added to the
/// configuration.
pub fn host_zone_cockroach(
&mut self,
zone_id: OmicronZoneUuid,
listen_address: SocketAddrV6,
http_address: SocketAddrV6,
) -> anyhow::Result<()> {
anyhow::ensure!(
listen_address.ip() == http_address.ip(),
"The IP address of the listen and HTTP address must match",
);
let zone = self.host_zone(zone_id, *listen_address.ip())?;
self.service_backend_zone(
ServiceName::Cockroach,
&zone,
listen_address.port(),
)?;
self.service_backend_zone(
ServiceName::CockroachHttp,
&zone,
http_address.port(),
)?;
Ok(())
}

/// Higher-level shorthand for adding an internal DNS zone, including
/// records for both its HTTP and DNS interfaces.
///
Expand Down Expand Up @@ -741,6 +773,10 @@ mod test {
"_clickhouse-server._tcp",
);
assert_eq!(ServiceName::Cockroach.dns_name(), "_cockroach._tcp",);
assert_eq!(
ServiceName::CockroachHttp.dns_name(),
"_cockroach-http._tcp",
);
assert_eq!(ServiceName::InternalDns.dns_name(), "_nameservice._tcp",);
assert_eq!(ServiceName::Nexus.dns_name(), "_nexus._tcp",);
assert_eq!(ServiceName::Oximeter.dns_name(), "_oximeter._tcp",);
Expand Down
5 changes: 5 additions & 0 deletions internal-dns/types/src/names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ pub enum ServiceName {
ClickhouseKeeper,
/// The HTTP interface to a replicated ClickHouse server.
ClickhouseServer,
/// The listen address of CockroachDB, for accessing the postgres interface
Cockroach,
/// The http address of CockroachDB, for accessing metrics
CockroachHttp,
InternalDns,
ExternalDns,
Nexus,
Expand Down Expand Up @@ -90,6 +93,7 @@ impl ServiceName {
ServiceName::ClickhouseKeeper => "clickhouse-keeper",
ServiceName::ClickhouseServer => "clickhouse-server",
ServiceName::Cockroach => "cockroach",
ServiceName::CockroachHttp => "cockroach-http",
ServiceName::ExternalDns => "external-dns",
ServiceName::InternalDns => "nameservice",
ServiceName::Nexus => "nexus",
Expand Down Expand Up @@ -123,6 +127,7 @@ impl ServiceName {
| ServiceName::ClickhouseKeeper
| ServiceName::ClickhouseServer
| ServiceName::Cockroach
| ServiceName::CockroachHttp
| ServiceName::InternalDns
| ServiceName::ExternalDns
| ServiceName::Nexus
Expand Down
46 changes: 34 additions & 12 deletions nexus/db-model/src/deployment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ use omicron_uuid_kinds::{
GenericUuid, MupdateOverrideKind, OmicronZoneKind, OmicronZoneUuid,
PhysicalDiskKind, SledKind, SledUuid, ZpoolKind, ZpoolUuid,
};
use std::net::{IpAddr, SocketAddrV6};
use std::net::{IpAddr, SocketAddr, SocketAddrV6};
use uuid::Uuid;

/// See [`nexus_types::deployment::Blueprint`].
Expand Down Expand Up @@ -578,11 +578,22 @@ impl BpOmicronZone {
bp_omicron_zone.set_zpool_name(dataset);
}
BlueprintZoneType::CockroachDb(
blueprint_zone_type::CockroachDb { address, dataset },
blueprint_zone_type::CockroachDb {
address,
http_address,
dataset,
},
) => {
// Set the common fields
bp_omicron_zone.set_primary_service_ip_and_port(address);
bp_omicron_zone.set_zpool_name(dataset);

// Set the zone specific fields
bp_omicron_zone.second_service_ip = Some(IpNetwork::from(
std::net::IpAddr::V6(*http_address.ip()),
));
bp_omicron_zone.second_service_port =
Some(SqlU16::from(http_address.port()));
}
BlueprintZoneType::Crucible(blueprint_zone_type::Crucible {
address,
Expand Down Expand Up @@ -734,8 +745,8 @@ impl BpOmicronZone {
let external_ip_id =
Self::external_ip_to_blueprint_zone_type(self.external_ip_id);

let dns_address =
omicron_zone_config::secondary_ip_and_port_to_dns_address(
let second_service_address =
omicron_zone_config::secondary_ip_and_port_to_address(
self.second_service_ip,
self.second_service_port,
);
Expand Down Expand Up @@ -802,12 +813,23 @@ impl BpOmicronZone {
},
),

ZoneType::CockroachDb => BlueprintZoneType::CockroachDb(
blueprint_zone_type::CockroachDb {
address: primary_address,
dataset: dataset?,
},
),
ZoneType::CockroachDb => {
let second_service_address =
second_service_address.and_then(|addr| match addr {
SocketAddr::V6(addr) => Ok(addr),
_ => Err(anyhow!(
"Unexpected IPv4 address for CRDB HTTP service"
)),
});

BlueprintZoneType::CockroachDb(
blueprint_zone_type::CockroachDb {
address: primary_address,
http_address: second_service_address?,
dataset: dataset?,
},
)
}
ZoneType::Crucible => {
BlueprintZoneType::Crucible(blueprint_zone_type::Crucible {
address: primary_address,
Expand All @@ -825,7 +847,7 @@ impl BpOmicronZone {
http_address: primary_address,
dns_address: OmicronZoneExternalFloatingAddr {
id: external_ip_id?,
addr: dns_address?,
addr: second_service_address?,
},
nic: nic?,
},
Expand All @@ -835,7 +857,7 @@ impl BpOmicronZone {
dataset: dataset?,
http_address: primary_address,
dns_address: omicron_zone_config::to_internal_dns_address(
dns_address?,
second_service_address?,
)?,
gz_address: self
.dns_gz_address
Expand Down
9 changes: 4 additions & 5 deletions nexus/db-model/src/inventory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1955,11 +1955,10 @@ impl InvOmicronSledConfigZone {
nic_row.map(Into::into),
)?;

let dns_address =
omicron_zone_config::secondary_ip_and_port_to_dns_address(
self.second_service_ip,
self.second_service_port,
);
let dns_address = omicron_zone_config::secondary_ip_and_port_to_address(
self.second_service_ip,
self.second_service_port,
);

let ntp_dns_servers =
omicron_zone_config::ntp_dns_servers_to_omicron_internal(
Expand Down
4 changes: 2 additions & 2 deletions nexus/db-model/src/omicron_zone_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ pub fn dataset_zpool_name_to_omicron_zone_dataset(
.ok_or_else(|| anyhow!("expected dataset zpool name, found none"))
}

/// Convert the secondary ip and port to a dns address
pub fn secondary_ip_and_port_to_dns_address(
/// Convert the secondary ip and port to an address
pub fn secondary_ip_and_port_to_address(
second_service_ip: Option<IpNetwork>,
second_service_port: Option<SqlU16>,
) -> anyhow::Result<SocketAddr> {
Expand Down
3 changes: 2 additions & 1 deletion nexus/db-model/src/schema_versions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock};
///
/// This must be updated when you change the database schema. Refer to
/// schema/crdb/README.adoc in the root of this repository for details.
pub const SCHEMA_VERSION: Version = Version::new(150, 0, 0);
pub const SCHEMA_VERSION: Version = Version::new(151, 0, 0);

/// List of all past database schema versions, in *reverse* order
///
Expand All @@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock<Vec<KnownVersion>> = LazyLock::new(|| {
// | leaving the first copy as an example for the next person.
// v
// KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
KnownVersion::new(151, "crdb-add-http-service"),
KnownVersion::new(150, "add-last-reconciliation-orphaned-datasets"),
KnownVersion::new(149, "bp-add-target-release-min-gen"),
KnownVersion::new(148, "clean-misplaced-m2s"),
Expand Down
5 changes: 1 addition & 4 deletions nexus/db-queries/src/db/datastore/deployment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,10 +649,7 @@ impl DataStore {
format!("zone {zone_id}: parse from database")
})
.map_err(|e| {
Error::internal_error(&format!(
"{:#}",
e.to_string()
))
Error::internal_error(&format!("{:?}", e))
})?;
sled_config.zones.insert(zone);
}
Expand Down
4 changes: 1 addition & 3 deletions nexus/db-queries/src/db/datastore/inventory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2569,9 +2569,7 @@ impl DataStore {
.with_context(|| {
format!("zone {:?}: parse from database", zone_id)
})
.map_err(|e| {
Error::internal_error(&format!("{:#}", e.to_string()))
})?;
.map_err(|e| Error::internal_error(&format!("{:?}", e)))?;
config_with_id.config.zones.insert(zone);
}

Expand Down
10 changes: 9 additions & 1 deletion nexus/reconfigurator/execution/src/dns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ mod test {
use nexus_types::internal_api::params::DnsRecord;
use nexus_types::internal_api::params::Srv;
use nexus_types::silo::silo_dns_name;
use omicron_common::address::COCKROACH_HTTP_PORT;
use omicron_common::address::IpRange;
use omicron_common::address::Ipv6Subnet;
use omicron_common::address::RACK_PREFIX;
Expand Down Expand Up @@ -518,8 +519,14 @@ mod test {
)
}
OmicronZoneType::CockroachDb { address, dataset } => {
let http_address =
SocketAddrV6::new(*address.ip(), COCKROACH_HTTP_PORT, 0, 0);
BlueprintZoneType::CockroachDb(
blueprint_zone_type::CockroachDb { address, dataset },
blueprint_zone_type::CockroachDb {
address,
http_address,
dataset,
},
)
}
OmicronZoneType::Crucible { address, dataset } => {
Expand Down Expand Up @@ -977,6 +984,7 @@ mod test {
ServiceName::Clickhouse,
ServiceName::ClickhouseNative,
ServiceName::Cockroach,
ServiceName::CockroachHttp,
ServiceName::InternalDns,
ServiceName::ExternalDns,
ServiceName::Nexus,
Expand Down
1 change: 1 addition & 0 deletions nexus/reconfigurator/execution/src/omicron_zones.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ mod test {
zone_type: BlueprintZoneType::CockroachDb(
blueprint_zone_type::CockroachDb {
address: "[::1]:0".parse().unwrap(),
http_address: "[::1]:0".parse().unwrap(),
dataset: OmicronZoneDataset {
pool_name: format!("oxp_{}", Uuid::new_v4())
.parse()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1505,9 +1505,12 @@ impl<'a> BlueprintBuilder<'a> {
self.sled_select_zpool(sled_id, ZoneKind::CockroachDb)?;
let port = omicron_common::address::COCKROACH_PORT;
let address = SocketAddrV6::new(underlay_ip, port, 0, 0);
let port = omicron_common::address::COCKROACH_HTTP_PORT;
let http_address = SocketAddrV6::new(underlay_ip, port, 0, 0);
let zone_type =
BlueprintZoneType::CockroachDb(blueprint_zone_type::CockroachDb {
address,
http_address,
dataset: OmicronZoneDataset { pool_name },
});
let filesystem_pool = pool_name;
Expand Down
9 changes: 9 additions & 0 deletions nexus/src/app/background/tasks/crdb_node_id_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ impl CockroachNodeIdCollector {
// This trait exists so we can inject addresses in our unit tests below that
// aren't required to have admin servers listening on the fixed
// `COCKROACH_ADMIN_PORT`.
//
// TODO: Is this as necessary, now that the blueprint can identify information
// about the CockroachDB HTTP address? Seems like it might be removable now.
trait CockroachAdminFromBlueprint {
fn cockroach_admin_addrs<'a>(
&'a self,
Expand Down Expand Up @@ -277,6 +280,12 @@ mod tests {
zone_type: BlueprintZoneType::CockroachDb(
blueprint_zone_type::CockroachDb {
address: addr,
http_address: SocketAddrV6::new(
*addr.ip(),
COCKROACH_ADMIN_PORT,
0,
0,
),
dataset: OmicronZoneDataset {
pool_name: format!("oxp_{}", zpool_id)
.parse()
Expand Down
13 changes: 8 additions & 5 deletions nexus/test-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,11 +515,13 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> {
let zone_id = OmicronZoneUuid::new_v4();
let zpool_id = ZpoolUuid::new_v4();
eprintln!("DB address: {}", address);
self.rack_init_builder.add_service_to_dns(
zone_id,
address,
ServiceName::Cockroach,
);

let http_address = database.http_addr();
self.rack_init_builder
.internal_dns_config
.host_zone_cockroach(zone_id, address, http_address)
.expect("Failed to set up CockroachDB DNS");

let pool_name = illumos_utils::zpool::ZpoolName::new_external(zpool_id)
.to_string()
.parse()
Expand All @@ -531,6 +533,7 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> {
zone_type: BlueprintZoneType::CockroachDb(
blueprint_zone_type::CockroachDb {
address,
http_address,
dataset: OmicronZoneDataset { pool_name },
},
),
Expand Down
Loading
Loading