From 7ce194e7494664bbfeca9e39c5e77c75b037adea Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 5 Feb 2024 14:44:14 -0800 Subject: [PATCH] fix checks --- .../blueprint-execution/src/omicron_zones.rs | 78 ++++++++++--------- nexus/db-queries/src/db/datastore/dns.rs | 12 +-- nexus/deployment/src/planner.rs | 2 +- .../src/app/background/blueprint_execution.rs | 45 ++++++----- nexus/types/src/inventory.rs | 4 +- 5 files changed, 72 insertions(+), 69 deletions(-) diff --git a/nexus/blueprint-execution/src/omicron_zones.rs b/nexus/blueprint-execution/src/omicron_zones.rs index 415aeacc3f6..1d5c4444b10 100644 --- a/nexus/blueprint-execution/src/omicron_zones.rs +++ b/nexus/blueprint-execution/src/omicron_zones.rs @@ -169,38 +169,38 @@ mod test { .await .expect("failed to deploy no zones"); - // The particular dataset doesn't matter for this test. - // We re-use the same one to not obfuscate things - let dataset = OmicronZoneDataset { - pool_name: format!("oxp_{}", Uuid::new_v4()).parse().unwrap(), - }; - - let generation = Generation::new(); - // Zones are updated in a particular order, but each request contains // the full set of zones that must be running. // See `rack_setup::service::ServiceInner::run` for more details. - let mut zones = OmicronZonesConfig { - generation, - zones: vec![OmicronZoneConfig { - id: Uuid::new_v4(), - underlay_address: "::1".parse().unwrap(), - zone_type: OmicronZoneType::InternalDns { - dataset, - dns_address: "oh-hello-internal-dns".into(), - gz_address: "::1".parse().unwrap(), - gz_address_index: 0, - http_address: "some-ipv6-address".into(), - }, - }], - }; + fn make_zones() -> OmicronZonesConfig { + OmicronZonesConfig { + generation: Generation::new(), + zones: vec![OmicronZoneConfig { + id: Uuid::new_v4(), + underlay_address: "::1".parse().unwrap(), + zone_type: OmicronZoneType::InternalDns { + dataset: OmicronZoneDataset { + pool_name: format!("oxp_{}", Uuid::new_v4()) + .parse() + .unwrap(), + }, + dns_address: "oh-hello-internal-dns".into(), + gz_address: "::1".parse().unwrap(), + gz_address_index: 0, + http_address: "some-ipv6-address".into(), + }, + }], + } + } // Create a blueprint with only the `InternalDns` zone for both servers // We reuse the same `OmicronZonesConfig` because the details don't // matter for this test. + let mut zones1 = make_zones(); + let mut zones2 = make_zones(); let blueprint = Arc::new(create_blueprint(BTreeMap::from([ - (sled_id1, zones.clone()), - (sled_id2, zones.clone()), + (sled_id1, zones1.clone()), + (sled_id2, zones2.clone()), ]))); // Set expectations for the initial requests sent to the fake @@ -274,21 +274,25 @@ mod test { s2.verify_and_clear(); // Add an `InternalNtp` zone for our next update - zones.generation = generation.next(); - zones.zones.push(OmicronZoneConfig { - id: Uuid::new_v4(), - underlay_address: "::1".parse().unwrap(), - zone_type: OmicronZoneType::InternalNtp { - address: "::1".into(), - dns_servers: vec!["::1".parse().unwrap()], - domain: None, - ntp_servers: vec!["some-ntp-server-addr".into()], - }, - }); + fn append_zone(zones: &mut OmicronZonesConfig) { + zones.generation = zones.generation.next(); + zones.zones.push(OmicronZoneConfig { + id: Uuid::new_v4(), + underlay_address: "::1".parse().unwrap(), + zone_type: OmicronZoneType::InternalNtp { + address: "::1".into(), + dns_servers: vec!["::1".parse().unwrap()], + domain: None, + ntp_servers: vec!["some-ntp-server-addr".into()], + }, + }); + } + append_zone(&mut zones1); + append_zone(&mut zones2); let blueprint = Arc::new(create_blueprint(BTreeMap::from([ - (sled_id1, zones.clone()), - (sled_id2, zones.clone()), + (sled_id1, zones1), + (sled_id2, zones2), ]))); // Set our new expectations diff --git a/nexus/db-queries/src/db/datastore/dns.rs b/nexus/db-queries/src/db/datastore/dns.rs index 78fdf2d8acf..2d59fcf7e5a 100644 --- a/nexus/db-queries/src/db/datastore/dns.rs +++ b/nexus/db-queries/src/db/datastore/dns.rs @@ -462,7 +462,8 @@ impl DataStore { } // This must only be used inside a transaction. Otherwise, it may make - // invalid changes to the database state. Use `dns_update()` instead. + // invalid changes to the database state. Use one of the `dns_update_*()` + // functions instead. // // The caller should already have checked (in the same transaction) that // their version number is the correct next version. @@ -574,10 +575,11 @@ impl DataStore { /// and add the name back with different records. /// /// You use this object to build up a _description_ of the changes to the DNS -/// zone's configuration. Then you call [`DataStore::dns_update()`] to apply -/// these changes transactionally to the database. The changes are then -/// propagated asynchronously to the DNS servers. No changes are made (to -/// either the database or the DNS servers) while you modify this object. +/// zone's configuration. Then you call [`DataStore::dns_update_incremental()`] +/// or [`DataStore::dns_update_from_version()`] to apply these changes +/// transactionally to the database. The changes are then propagated +/// asynchronously to the DNS servers. No changes are made (to either the +/// database or the DNS servers) while you modify this object. /// /// This object changes all of the zones associated with a particular DNS group /// because the assumption right now is that they're equivalent. (In practice, diff --git a/nexus/deployment/src/planner.rs b/nexus/deployment/src/planner.rs index d1f82000154..7ac900fd1f7 100644 --- a/nexus/deployment/src/planner.rs +++ b/nexus/deployment/src/planner.rs @@ -11,8 +11,8 @@ use crate::blueprint_builder::Ensure; use crate::blueprint_builder::Error; use nexus_types::deployment::Blueprint; use nexus_types::deployment::Policy; -use slog::{info, Logger}; use omicron_common::api::external::Generation; +use slog::{info, Logger}; pub struct Planner<'a> { log: Logger, diff --git a/nexus/src/app/background/blueprint_execution.rs b/nexus/src/app/background/blueprint_execution.rs index 4caa89d53cf..d340f1d38b1 100644 --- a/nexus/src/app/background/blueprint_execution.rs +++ b/nexus/src/app/background/blueprint_execution.rs @@ -212,31 +212,30 @@ mod test { // Create a non-empty blueprint describing two servers and verify that // the task correctly winds up making requests to both of them and - // reporting success. We reuse the same `OmicronZonesConfig` in - // constructing the blueprint because the details don't matter for this - // test. - let zones = OmicronZonesConfig { - generation: Generation::new(), - zones: vec![OmicronZoneConfig { - id: Uuid::new_v4(), - underlay_address: "::1".parse().unwrap(), - zone_type: OmicronZoneType::InternalDns { - dataset: OmicronZoneDataset { - pool_name: format!("oxp_{}", Uuid::new_v4()) - .parse() - .unwrap(), + // reporting success. + fn make_zones() -> OmicronZonesConfig { + OmicronZonesConfig { + generation: Generation::new(), + zones: vec![OmicronZoneConfig { + id: Uuid::new_v4(), + underlay_address: "::1".parse().unwrap(), + zone_type: OmicronZoneType::InternalDns { + dataset: OmicronZoneDataset { + pool_name: format!("oxp_{}", Uuid::new_v4()) + .parse() + .unwrap(), + }, + dns_address: "oh-hello-internal-dns".into(), + gz_address: "::1".parse().unwrap(), + gz_address_index: 0, + http_address: "some-ipv6-address".into(), }, - dns_address: "oh-hello-internal-dns".into(), - gz_address: "::1".parse().unwrap(), - gz_address_index: 0, - http_address: "some-ipv6-address".into(), - }, - }], - }; - + }], + } + } let mut blueprint = create_blueprint(BTreeMap::from([ - (sled_id1, zones.clone()), - (sled_id2, zones.clone()), + (sled_id1, make_zones()), + (sled_id2, make_zones()), ])); blueprint_tx.send(Some(Arc::new(blueprint.clone()))).unwrap(); diff --git a/nexus/types/src/inventory.rs b/nexus/types/src/inventory.rs index 89bb6519de0..760d3918f4d 100644 --- a/nexus/types/src/inventory.rs +++ b/nexus/types/src/inventory.rs @@ -139,9 +139,7 @@ impl Collection { pub fn scrimlets(&self) -> impl Iterator + '_ { self.sled_agents .iter() - .filter(|(_, inventory)| { - inventory.sled_role == SledRole::Scrimlet - }) + .filter(|(_, inventory)| inventory.sled_role == SledRole::Scrimlet) .map(|(sled_id, _)| *sled_id) } }