Skip to content

Commit

Permalink
Remove datasets from RSS -> Nexus handoff
Browse files Browse the repository at this point in the history 
Fixes #7318 - two sets can't be out of sync if we delete one of them!
  • Loading branch information
@jgallagher
jgallagher committed Jan 7, 2025
1 parent 0603f0b commit e733234
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 232 deletions.
22 changes: 13 additions & 9 deletions nexus/src/app/rack.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ use omicron_common::api::external::NameOrId;
use omicron_common::api::external::ResourceType;
use omicron_common::api::internal::shared::ExternalPortDiscovery;
use omicron_common::api::internal::shared::LldpAdminStatus;
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::SledUuid;
use oxnet::IpNet;
use sled_agent_client::types::AddSledRequest;
Expand Down Expand Up @@ -140,15 +141,18 @@ impl super::Nexus {
.collect();

let datasets: Vec<_> = request
.datasets
.into_iter()
.map(|dataset| {
db::model::Dataset::new(
dataset.dataset_id,
dataset.zpool_id,
dataset.request.address,
dataset.request.kind,
)
.blueprint
.blueprint_datasets
.values()
.flat_map(|config| {
config.datasets.values().map(|dataset| {
db::model::Dataset::new(
dataset.id,
dataset.pool.id().into_untyped_uuid(),
dataset.address,
dataset.kind.clone(),
)
})
})
.collect();

Expand Down
2 changes: 0 additions & 2 deletions nexus/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,6 @@ impl nexus_test_interface::NexusServer for Server {
nexus_types::internal_api::params::PhysicalDiskPutRequest,
>,
zpools: Vec<nexus_types::internal_api::params::ZpoolPutRequest>,
datasets: Vec<nexus_types::internal_api::params::DatasetCreateRequest>,
internal_dns_zone_config: nexus_types::internal_api::params::DnsConfigParams,
external_dns_zone_name: &str,
recovery_silo: nexus_sled_agent_shared::recovery_silo::RecoverySiloConfig,
Expand Down Expand Up @@ -303,7 +302,6 @@ impl nexus_test_interface::NexusServer for Server {
blueprint,
physical_disks,
zpools,
datasets,
internal_services_ip_pool_ranges,
certs,
internal_dns_zone_config,
Expand Down
1 change: 0 additions & 1 deletion nexus/test-interface/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ pub trait NexusServer: Send + Sync + 'static {
blueprint: Blueprint,
physical_disks: Vec<PhysicalDiskPutRequest>,
zpools: Vec<nexus_types::internal_api::params::ZpoolPutRequest>,
datasets: Vec<nexus_types::internal_api::params::DatasetCreateRequest>,
internal_dns_config: nexus_types::internal_api::params::DnsConfigParams,
external_dns_zone_name: &str,
recovery_silo: nexus_sled_agent_shared::recovery_silo::RecoverySiloConfig,
Expand Down
74 changes: 3 additions & 71 deletions nexus/test-utils/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ use nexus_types::deployment::CockroachDbPreserveDowngrade;
use nexus_types::deployment::OmicronZoneExternalFloatingAddr;
use nexus_types::deployment::OmicronZoneExternalFloatingIp;
use nexus_types::external_api::views::SledState;
use nexus_types::internal_api::params::DatasetCreateRequest;
use nexus_types::internal_api::params::DatasetPutRequest;
use nexus_types::internal_api::params::DnsConfigParams;
use omicron_common::address::DNS_OPTE_IPV4_SUBNET;
use omicron_common::address::NEXUS_OPTE_IPV4_SUBNET;
Expand Down Expand Up @@ -216,15 +214,13 @@ pub async fn test_setup<N: NexusServer>(
}

struct RackInitRequestBuilder {
datasets: Vec<nexus_types::internal_api::params::DatasetCreateRequest>,
internal_dns_config: DnsConfigBuilder,
mac_addrs: Box<dyn Iterator<Item = MacAddr> + Send>,
}

impl RackInitRequestBuilder {
fn new() -> Self {
Self {
datasets: vec![],
internal_dns_config: DnsConfigBuilder::new(),
mac_addrs: Box::new(MacAddr::iter_system()),
}
Expand All @@ -245,49 +241,13 @@ impl RackInitRequestBuilder {
.expect("Failed to set up DNS for {kind}");
}

// Keeps track of:
// - The "DatasetPutRequest" (for handoff to Nexus)
// - The internal DNS configuration for this service
fn add_dataset(
&mut self,
zone_id: OmicronZoneUuid,
zpool_id: ZpoolUuid,
dataset_id: DatasetUuid,
address: SocketAddrV6,
kind: DatasetKind,
service_name: ServiceName,
) {
self.datasets.push(DatasetCreateRequest {
zpool_id: zpool_id.into_untyped_uuid(),
dataset_id,
request: DatasetPutRequest { address: Some(address), kind },
});
let zone = self
.internal_dns_config
.host_zone(zone_id, *address.ip())
.expect("Failed to set up DNS for {kind}");
self.internal_dns_config
.service_backend_zone(service_name, &zone, address.port())
.expect("Failed to set up DNS for {kind}");
}

// Special handling of ClickHouse, which has multiple SRV records for its
// single zone.
fn add_clickhouse_dataset(
fn add_clickhouse_to_dns(
&mut self,
zone_id: OmicronZoneUuid,
zpool_id: ZpoolUuid,
dataset_id: DatasetUuid,
address: SocketAddrV6,
) {
self.datasets.push(DatasetCreateRequest {
zpool_id: zpool_id.into_untyped_uuid(),
dataset_id,
request: DatasetPutRequest {
address: Some(address),
kind: DatasetKind::Clickhouse,
},
});
self.internal_dns_config
.host_zone_clickhouse(zone_id, ServiceName::Clickhouse, address)
.expect("Failed to setup ClickHouse DNS");
Expand Down Expand Up @@ -449,14 +409,10 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> {

let zone_id = OmicronZoneUuid::new_v4();
let zpool_id = ZpoolUuid::new_v4();
let dataset_id = DatasetUuid::new_v4();
eprintln!("DB address: {}", address);
self.rack_init_builder.add_dataset(
self.rack_init_builder.add_service_to_dns(
zone_id,
zpool_id,
dataset_id,
address,
DatasetKind::Cockroach,
ServiceName::Cockroach,
);
let pool_name = illumos_utils::zpool::ZpoolName::new_external(zpool_id)
Expand Down Expand Up @@ -490,16 +446,10 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> {

let zone_id = OmicronZoneUuid::new_v4();
let zpool_id = ZpoolUuid::new_v4();
let dataset_id = DatasetUuid::new_v4();
let http_address = clickhouse.http_address();
let http_port = http_address.port();
let native_address = clickhouse.native_address();
self.rack_init_builder.add_clickhouse_dataset(
zone_id,
zpool_id,
dataset_id,
http_address,
);
self.rack_init_builder.add_clickhouse_to_dns(zone_id, http_address);
self.clickhouse = Some(clickhouse);

// NOTE: We could pass this port information via DNS, rather than
Expand Down Expand Up @@ -934,24 +884,6 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> {
.expect("Must launch internal nexus first"),
self.config,
blueprint,
// NOTE: We should probably hand off
// "self.rack_init_builder.datasets" here, but Nexus won't be happy
// if we pass it right now:
//
// - When we "call .add_dataset(...)", we need to keep track of
// which zpool the dataset is coming from. For these synthetic
// environments, we make this value up.
// - When we tell Nexus about datasets, we need to provide the
// parent zpool UUID, which must be known to Nexus's database.
// - The sled agent we're creating to run alongside this test DOES
// create synthetic zpools on boot, but
// (a) They're not the same zpools we're making up when we start
// Clickhouse / CRDB (we're basically making distinct calls to
// Uuid::new_v4()).
// (b) These sled-agent-created zpools are registered with Nexus
// asynchronously, and we're not making any effort (currently) to
// wait for them to be known to Nexus.
vec![],
vec![],
vec![],
dns_config,
Expand Down
10 changes: 0 additions & 10 deletions nexus/types/src/internal_api/params.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use omicron_common::api::internal::shared::DatasetKind;
use omicron_common::api::internal::shared::ExternalPortDiscovery;
use omicron_common::api::internal::shared::RackNetworkConfig;
use omicron_common::api::internal::shared::SourceNatConfig;
use omicron_uuid_kinds::DatasetUuid;
use omicron_uuid_kinds::PhysicalDiskUuid;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -156,13 +155,6 @@ impl fmt::Display for ServiceKind {
}
}

#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)]
pub struct DatasetCreateRequest {
pub zpool_id: Uuid,
pub dataset_id: DatasetUuid,
pub request: DatasetPutRequest,
}

#[derive(Debug, Clone, Deserialize, JsonSchema)]
pub struct RackInitializationRequest {
/// Blueprint describing services initialized by RSS.
Expand All @@ -174,8 +166,6 @@ pub struct RackInitializationRequest {
/// Zpools created within the physical disks created by the control plane.
pub zpools: Vec<ZpoolPutRequest>,

/// Datasets on the rack which have been provisioned by RSS.
pub datasets: Vec<DatasetCreateRequest>,
/// Ranges of the service IP pool which may be used for internal services,
/// such as Nexus.
pub internal_services_ip_pool_ranges: Vec<IpRange>,
Expand Down
50 changes: 0 additions & 50 deletions openapi/nexus-internal.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3160,52 +3160,10 @@
"start_time"
]
},
"DatasetCreateRequest": {
"type": "object",
"properties": {
"dataset_id": {
"$ref": "#/components/schemas/TypedUuidForDatasetKind"
},
"request": {
"$ref": "#/components/schemas/DatasetPutRequest"
},
"zpool_id": {
"type": "string",
"format": "uuid"
}
},
"required": [
"dataset_id",
"request",
"zpool_id"
]
},
"DatasetKind": {
"description": "The kind of dataset. See the `DatasetKind` enum in omicron-common for possible values.",
"type": "string"
},
"DatasetPutRequest": {
"description": "Describes a dataset within a pool.",
"type": "object",
"properties": {
"address": {
"nullable": true,
"description": "Address on which a service is responding to requests for the dataset.",
"type": "string"
},
"kind": {
"description": "Type of dataset being inserted.",
"allOf": [
{
"$ref": "#/components/schemas/DatasetKind"
}
]
}
},
"required": [
"kind"
]
},
"DemoSaga": {
"description": "Identifies an instance of the demo saga",
"type": "object",
Expand Down Expand Up @@ -4956,13 +4914,6 @@
"$ref": "#/components/schemas/Certificate"
}
},
"datasets": {
"description": "Datasets on the rack which have been provisioned by RSS.",
"type": "array",
"items": {
"$ref": "#/components/schemas/DatasetCreateRequest"
}
},
"external_dns_zone_name": {
"description": "delegated DNS name for external DNS",
"type": "string"
Expand Down Expand Up @@ -5025,7 +4976,6 @@
"allowed_source_ips",
"blueprint",
"certs",
"datasets",
"external_dns_zone_name",
"external_port_count",
"internal_dns_zone_config",
Expand Down
59 changes: 0 additions & 59 deletions sled-agent/src/rack_setup/service.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ use omicron_common::ledger::{self, Ledger, Ledgerable};
use omicron_ddm_admin_client::{Client as DdmAdminClient, DdmError};
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::SledUuid;
use omicron_uuid_kinds::ZpoolUuid;
use serde::{Deserialize, Serialize};
use sled_agent_client::{
types as SledAgentTypes, Client as SledAgentClient, Error as SledAgentError,
Expand Down Expand Up @@ -815,63 +814,6 @@ impl ServiceInner {
self.log.new(o!("component" => "NexusClient")),
);

// Convert all the information we have about datasets into a format
// which can be stored in CockroachDB once they make their way to Nexus.
//
// TODO(https://github.com/oxidecomputer/omicron/issues/6998): Remove me!
// This is somewhat redundant with the information we're supplying in
// the initial blueprint, and could be de-duplicated if we can fully
// migrate to a world where "datasets exist in the blueprint, but not
// in CockroachDB".
let mut datasets = HashMap::<
(ZpoolUuid, DatasetKind),
NexusTypes::DatasetCreateRequest,
>::new();
for sled_config in service_plan.services.values() {
// Add all datasets, as they appear in our plan.
//
// Note that we assume all datasets have a "None" address
// field -- the coupling of datasets with addresses is linked
// to usage by specific zones.
for dataset in sled_config.datasets.datasets.values() {
let duplicate = datasets.insert(
(dataset.name.pool().id(), dataset.name.kind().clone()),
NexusTypes::DatasetCreateRequest {
zpool_id: dataset.name.pool().id().into_untyped_uuid(),
dataset_id: dataset.id,
request: NexusTypes::DatasetPutRequest {
address: None,
kind: dataset.name.kind().clone(),
},
},
);

if let Some(dataset) = duplicate {
panic!("Inserting multiple datasets of the same kind on a single zpool: {dataset:?}");
}
}

// If any datasets came from a zone, patch their addresses.
//
// It would be better if "datasets" was keyed off of a DatasetUuid,
// but the SeldConfig storing zones does not know about
// DatasetUuids.
//
// Instead, we use the (Zpool, DatasetKind) tuple as a unique
// identifier for the dataset.
for zone in &sled_config.zones {
if let Some(dataset) = zone.zone_type.durable_dataset() {
let Some(entry) = datasets.get_mut(&(
dataset.dataset.pool_name.id(),
dataset.kind.clone(),
)) else {
panic!("zone's durable dataset not found");
};
entry.request.address = Some(dataset.address.to_string());
}
}
}
let datasets: Vec<_> = datasets.into_values().collect();
let internal_services_ip_pool_ranges = config
.internal_services_ip_pool_ranges
.clone()
Expand Down Expand Up @@ -1058,7 +1000,6 @@ impl ServiceInner {
blueprint,
physical_disks,
zpools,
datasets,
internal_services_ip_pool_ranges,
certs: config.external_certificates.clone(),
internal_dns_zone_config: service_plan.dns_config.clone(),
Expand Down
Loading

0 comments on commit e733234

Please sign in to comment.