Skip to content

Commit

Permalink
wrap this up
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewjstone committed Aug 22, 2024
1 parent e3c5ab5 commit 196868b
Show file tree
Hide file tree
Showing 6 changed files with 222 additions and 179 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions nexus/reconfigurator/execution/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ dns-service-client.workspace = true
chrono.workspace = true
futures.workspace = true
internal-dns.workspace = true
newtype-uuid.workspace = true
nexus-config.workspace = true
nexus-db-model.workspace = true
nexus-db-queries.workspace = true
Expand Down
214 changes: 213 additions & 1 deletion nexus/reconfigurator/execution/src/dns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ mod test {
use internal_dns::resolver::Resolver;
use internal_dns::ServiceName;
use internal_dns::DNS_ZONE;
use newtype_uuid::GenericUuid;
use nexus_db_model::DnsGroup;
use nexus_db_model::Silo;
use nexus_db_queries::authn;
Expand All @@ -478,6 +479,8 @@ mod test {
use nexus_reconfigurator_planning::blueprint_builder::EnsureMultiple;
use nexus_reconfigurator_planning::example::example;
use nexus_reconfigurator_preparation::PlanningInputFromDb;
use nexus_sled_agent_shared::inventory::OmicronZoneConfig;
use nexus_sled_agent_shared::inventory::OmicronZoneType;
use nexus_sled_agent_shared::inventory::ZoneKind;
use nexus_test_utils::resource_helpers::create_silo;
use nexus_test_utils::resource_helpers::DiskTestBuilder;
Expand All @@ -490,6 +493,9 @@ mod test {
use nexus_types::deployment::CockroachDbClusterVersion;
use nexus_types::deployment::CockroachDbPreserveDowngrade;
use nexus_types::deployment::CockroachDbSettings;
pub use nexus_types::deployment::OmicronZoneExternalFloatingAddr;
pub use nexus_types::deployment::OmicronZoneExternalFloatingIp;
pub use nexus_types::deployment::OmicronZoneExternalSnatIp;
use nexus_types::deployment::SledFilter;
use nexus_types::external_api::params;
use nexus_types::external_api::shared;
Expand Down Expand Up @@ -539,6 +545,212 @@ mod test {
}
}

/// **********************************************************************
/// DEPRECATION WARNING:
///
/// Remove when `deprecated_omicron_zone_config_to_blueprint_zone_config`
/// is deleted.
/// **********************************************************************
///
/// Errors from converting an [`OmicronZoneType`] into a [`BlueprintZoneType`].
#[derive(Debug, Clone)]
pub enum InvalidOmicronZoneType {
#[allow(unused)]
ExternalIpIdRequired { kind: ZoneKind },
}

/// **********************************************************************
/// DEPRECATION WARNING: Do not call this function in new code !!!
/// **********************************************************************
///
/// Convert an [`OmicronZoneConfig`] to a [`BlueprintZoneConfig`].
///
/// A `BlueprintZoneConfig` is a superset of `OmicronZoneConfig` and
/// contains auxiliary information not present in an `OmicronZoneConfig`.
/// Therefore, the only valid direction for a real system to take is a
/// lossy conversion from `BlueprintZoneConfig` to `OmicronZoneConfig`.
/// This function, however, does the opposite. We therefore have to inject
/// fake information to fill in the unknown fields in the generated
/// `OmicronZoneConfig`.
///
/// This is bad, and we should generally feel bad for doing it :). At
/// the time this was done we were backporting the blueprint system into
/// RSS while trying not to change too much code. This was a judicious
/// shortcut used right before a release for stability reasons. As the
/// number of zones managed by the reconfigurator has grown, the use
/// of this function has become more egregious, and so it was removed
/// from the production code path and into this test module. This move
/// itself is a judicious shortcut. We have a test in this module,
/// `test_blueprint_internal_dns_basic`, that is the last caller of this
/// function, and so we have moved this function into this module.
///
/// Ideally, we would get rid of this function altogether and use another
/// method for generating `BlueprintZoneConfig` structures. Unfortunately,
/// there are still a few remaining zones that need to be implemented in the
/// `BlueprintBuilder`, and some of them require custom code. Until that is
/// done, we don't have a good way of generating a test representation of
/// the real system that would properly serve this test. We could generate
/// a `BlueprintZoneConfig` by hand for each zone type in this test, on
/// top of the more modern `SystemDescription` setup, but that isn't much
/// different than what we do in this test. We'd also eventually remove it
/// for better test setup when our `BlueprintBuilder` is capable of properly
/// constructing all zone types. Instead, we do the simple thing, and reuse
/// what we alreaady have.
///
/// # Errors
///
/// If `config.zone_type` is a zone that has an external IP address (Nexus,
/// boundary NTP, external DNS), `external_ip_id` must be `Some(_)` or this
/// method will return an error.
pub fn deprecated_omicron_zone_config_to_blueprint_zone_config(
config: OmicronZoneConfig,
disposition: BlueprintZoneDisposition,
external_ip_id: Option<ExternalIpUuid>,
) -> Result<BlueprintZoneConfig, InvalidOmicronZoneType> {
let kind = config.zone_type.kind();
let zone_type = match config.zone_type {
OmicronZoneType::BoundaryNtp {
address,
dns_servers,
domain,
nic,
ntp_servers,
snat_cfg,
} => {
let external_ip_id = external_ip_id.ok_or(
InvalidOmicronZoneType::ExternalIpIdRequired { kind },
)?;
BlueprintZoneType::BoundaryNtp(
blueprint_zone_type::BoundaryNtp {
address,
ntp_servers,
dns_servers,
domain,
nic,
external_ip: OmicronZoneExternalSnatIp {
id: external_ip_id,
snat_cfg,
},
},
)
}
OmicronZoneType::Clickhouse { address, dataset } => {
BlueprintZoneType::Clickhouse(blueprint_zone_type::Clickhouse {
address,
dataset,
})
}
OmicronZoneType::ClickhouseKeeper { address, dataset } => {
BlueprintZoneType::ClickhouseKeeper(
blueprint_zone_type::ClickhouseKeeper { address, dataset },
)
}
OmicronZoneType::ClickhouseServer { address, dataset } => {
BlueprintZoneType::ClickhouseServer(
blueprint_zone_type::ClickhouseServer { address, dataset },
)
}
OmicronZoneType::CockroachDb { address, dataset } => {
BlueprintZoneType::CockroachDb(
blueprint_zone_type::CockroachDb { address, dataset },
)
}
OmicronZoneType::Crucible { address, dataset } => {
BlueprintZoneType::Crucible(blueprint_zone_type::Crucible {
address,
dataset,
})
}
OmicronZoneType::CruciblePantry { address } => {
BlueprintZoneType::CruciblePantry(
blueprint_zone_type::CruciblePantry { address },
)
}
OmicronZoneType::ExternalDns {
dataset,
dns_address,
http_address,
nic,
} => {
let external_ip_id = external_ip_id.ok_or(
InvalidOmicronZoneType::ExternalIpIdRequired { kind },
)?;
BlueprintZoneType::ExternalDns(
blueprint_zone_type::ExternalDns {
dataset,
http_address,
dns_address: OmicronZoneExternalFloatingAddr {
id: external_ip_id,
addr: dns_address,
},
nic,
},
)
}
OmicronZoneType::InternalDns {
dataset,
dns_address,
gz_address,
gz_address_index,
http_address,
} => BlueprintZoneType::InternalDns(
blueprint_zone_type::InternalDns {
dataset,
http_address,
dns_address,
gz_address,
gz_address_index,
},
),
OmicronZoneType::InternalNtp {
address,
dns_servers,
domain,
ntp_servers,
} => BlueprintZoneType::InternalNtp(
blueprint_zone_type::InternalNtp {
address,
ntp_servers,
dns_servers,
domain,
},
),
OmicronZoneType::Nexus {
external_dns_servers,
external_ip,
external_tls,
internal_address,
nic,
} => {
let external_ip_id = external_ip_id.ok_or(
InvalidOmicronZoneType::ExternalIpIdRequired { kind },
)?;
BlueprintZoneType::Nexus(blueprint_zone_type::Nexus {
internal_address,
external_ip: OmicronZoneExternalFloatingIp {
id: external_ip_id,
ip: external_ip,
},
nic,
external_tls,
external_dns_servers,
})
}
OmicronZoneType::Oximeter { address } => {
BlueprintZoneType::Oximeter(blueprint_zone_type::Oximeter {
address,
})
}
};
Ok(BlueprintZoneConfig {
disposition,
id: OmicronZoneUuid::from_untyped_uuid(config.id),
underlay_address: config.underlay_address,
filesystem_pool: config.filesystem_pool,
zone_type,
})
}

/// test blueprint_internal_dns_config(): trivial case of an empty blueprint
#[test]
fn test_blueprint_internal_dns_empty() {
Expand Down Expand Up @@ -589,7 +801,7 @@ mod test {
.zones
.into_iter()
.map(|config| -> BlueprintZoneConfig {
BlueprintZoneConfig::from_omicron_zone_config(
deprecated_omicron_zone_config_to_blueprint_zone_config(
config,
BlueprintZoneDisposition::InService,
// We don't get external IP IDs in inventory
Expand Down
Loading

0 comments on commit 196868b

Please sign in to comment.