Skip to content

Commit 7484017

Browse files
authored
[nexus] move UuidRng code out to a common crate, make CollectionBuilder use it (#5341)
In #5270, we need determinism not just from blueprints but also collections. So move the UuidRng into a common place. As part of that, I also decided to make it its own crate and write some documentation about it, making it more generic along the way. I think this should be a pretty clean representation of what this is trying to do.
1 parent 4ca89ca commit 7484017

File tree

10 files changed

+326
-54
lines changed

10 files changed

+326
-54
lines changed

Cargo.lock

Lines changed: 13 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ members = [
7070
"test-utils",
7171
"tufaceous-lib",
7272
"tufaceous",
73+
"typed-rng",
7374
"update-common",
7475
"update-engine",
7576
"uuid-kinds",
@@ -149,6 +150,7 @@ default-members = [
149150
"test-utils",
150151
"tufaceous-lib",
151152
"tufaceous",
153+
"typed-rng",
152154
"update-common",
153155
"update-engine",
154156
"uuid-kinds",
@@ -343,6 +345,7 @@ propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev
343345
proptest = "1.4.0"
344346
quote = "1.0"
345347
rand = "0.8.5"
348+
rand_core = "0.6.4"
346349
rand_seeder = "0.2.3"
347350
ratatui = "0.26.1"
348351
rayon = "1.9"
@@ -434,6 +437,7 @@ trybuild = "1.0.89"
434437
tufaceous = { path = "tufaceous" }
435438
tufaceous-lib = { path = "tufaceous-lib" }
436439
tui-tree-widget = "0.17.0"
440+
typed-rng = { path = "typed-rng" }
437441
unicode-width = "0.1.11"
438442
update-common = { path = "update-common" }
439443
update-engine = { path = "update-engine" }

nexus/inventory/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ sled-agent-client.workspace = true
1919
slog.workspace = true
2020
strum.workspace = true
2121
thiserror.workspace = true
22+
typed-rng.workspace = true
2223
uuid.workspace = true
2324
omicron-workspace-hack.workspace = true
2425

nexus/inventory/src/builder.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,10 @@ use nexus_types::inventory::SledAgent;
2929
use nexus_types::inventory::Zpool;
3030
use std::collections::BTreeMap;
3131
use std::collections::BTreeSet;
32+
use std::hash::Hash;
3233
use std::sync::Arc;
3334
use thiserror::Error;
35+
use typed_rng::UuidRng;
3436
use uuid::Uuid;
3537

3638
/// Describes an operational error encountered during the collection process
@@ -86,6 +88,8 @@ pub struct CollectionBuilder {
8688
BTreeMap<RotPageWhich, BTreeMap<Arc<BaseboardId>, RotPageFound>>,
8789
sleds: BTreeMap<Uuid, SledAgent>,
8890
omicron_zones: BTreeMap<Uuid, OmicronZonesFound>,
91+
// We just generate one UUID for each collection.
92+
id_rng: UuidRng,
8993
}
9094

9195
impl CollectionBuilder {
@@ -111,6 +115,7 @@ impl CollectionBuilder {
111115
rot_pages_found: BTreeMap::new(),
112116
sleds: BTreeMap::new(),
113117
omicron_zones: BTreeMap::new(),
118+
id_rng: UuidRng::from_entropy(),
114119
}
115120
}
116121

@@ -123,7 +128,7 @@ impl CollectionBuilder {
123128
}
124129

125130
Collection {
126-
id: Uuid::new_v4(),
131+
id: self.id_rng.next(),
127132
errors: self.errors.into_iter().map(|e| e.to_string()).collect(),
128133
time_started: self.time_started,
129134
time_done: now_db_precision(),
@@ -140,6 +145,18 @@ impl CollectionBuilder {
140145
}
141146
}
142147

148+
/// Within tests, set a seeded RNG for deterministic results.
149+
///
150+
/// This will ensure that tests that use this builder will produce the same
151+
/// results each time they are run.
152+
pub fn set_rng_seed<H: Hash>(&mut self, seed: H) -> &mut Self {
153+
// Important to add some more bytes here, so that builders with the
154+
// same seed but different purposes don't end up with the same UUIDs.
155+
const SEED_EXTRA: &str = "collection-builder";
156+
self.id_rng.set_seed(seed, SEED_EXTRA);
157+
self
158+
}
159+
143160
/// Record service processor state `sp_state` reported by MGS
144161
///
145162
/// `sp_type` and `slot` identify which SP this was.

nexus/reconfigurator/planning/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ nexus-inventory.workspace = true
1616
nexus-types.workspace = true
1717
omicron-common.workspace = true
1818
rand.workspace = true
19-
rand_seeder.workspace = true
2019
sled-agent-client.workspace = true
2120
slog.workspace = true
2221
thiserror.workspace = true
22+
typed-rng.workspace = true
2323
uuid.workspace = true
2424

2525
omicron-workspace-hack.workspace = true

nexus/reconfigurator/planning/src/blueprint_builder.rs

Lines changed: 14 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ use omicron_common::api::external::Vni;
3838
use omicron_common::api::internal::shared::NetworkInterface;
3939
use omicron_common::api::internal::shared::NetworkInterfaceKind;
4040
use rand::rngs::StdRng;
41-
use rand::RngCore;
4241
use rand::SeedableRng;
4342
use slog::o;
4443
use slog::Logger;
@@ -50,6 +49,7 @@ use std::net::Ipv4Addr;
5049
use std::net::Ipv6Addr;
5150
use std::net::SocketAddrV6;
5251
use thiserror::Error;
52+
use typed_rng::UuidRng;
5353
use uuid::Uuid;
5454

5555
/// Errors encountered while assembling blueprints
@@ -223,7 +223,7 @@ impl<'a> BlueprintBuilder<'a> {
223223
})
224224
.collect::<Result<_, Error>>()?;
225225
Ok(Blueprint {
226-
id: rng.blueprint_rng.next_uuid(),
226+
id: rng.blueprint_rng.next(),
227227
blueprint_zones,
228228
parent_blueprint_id: None,
229229
internal_dns_version,
@@ -375,7 +375,7 @@ impl<'a> BlueprintBuilder<'a> {
375375
let blueprint_zones =
376376
self.zones.into_zones_map(self.policy.sleds.keys().copied());
377377
Blueprint {
378-
id: self.rng.blueprint_rng.next_uuid(),
378+
id: self.rng.blueprint_rng.next(),
379379
blueprint_zones,
380380
parent_blueprint_id: Some(self.parent_blueprint.id),
381381
internal_dns_version: self.internal_dns_version,
@@ -452,7 +452,7 @@ impl<'a> BlueprintBuilder<'a> {
452452
.collect();
453453

454454
let zone = OmicronZoneConfig {
455-
id: self.rng.zone_rng.next_uuid(),
455+
id: self.rng.zone_rng.next(),
456456
underlay_address: ip,
457457
zone_type: OmicronZoneType::InternalNtp {
458458
address: ntp_address.to_string(),
@@ -502,7 +502,7 @@ impl<'a> BlueprintBuilder<'a> {
502502
let port = omicron_common::address::CRUCIBLE_PORT;
503503
let address = SocketAddrV6::new(ip, port, 0, 0).to_string();
504504
let zone = OmicronZoneConfig {
505-
id: self.rng.zone_rng.next_uuid(),
505+
id: self.rng.zone_rng.next(),
506506
underlay_address: ip,
507507
zone_type: OmicronZoneType::Crucible {
508508
address,
@@ -589,7 +589,7 @@ impl<'a> BlueprintBuilder<'a> {
589589
};
590590

591591
for _ in 0..num_nexus_to_add {
592-
let nexus_id = self.rng.zone_rng.next_uuid();
592+
let nexus_id = self.rng.zone_rng.next();
593593
let external_ip = self
594594
.available_external_ips
595595
.next()
@@ -617,7 +617,7 @@ impl<'a> BlueprintBuilder<'a> {
617617
.next()
618618
.ok_or(Error::NoSystemMacAddressAvailable)?;
619619
NetworkInterface {
620-
id: self.rng.network_interface_rng.next_uuid(),
620+
id: self.rng.network_interface_rng.next(),
621621
kind: NetworkInterfaceKind::Service { id: nexus_id },
622622
name: format!("nexus-{nexus_id}").parse().unwrap(),
623623
ip,
@@ -739,14 +739,14 @@ struct BlueprintBuilderRng {
739739

740740
impl BlueprintBuilderRng {
741741
fn new() -> Self {
742-
Self::new_from_rng(StdRng::from_entropy())
742+
Self::new_from_parent(StdRng::from_entropy())
743743
}
744744

745-
fn new_from_rng(mut root_rng: StdRng) -> Self {
746-
let blueprint_rng = UuidRng::from_root_rng(&mut root_rng, "blueprint");
747-
let zone_rng = UuidRng::from_root_rng(&mut root_rng, "zone");
745+
fn new_from_parent(mut parent: StdRng) -> Self {
746+
let blueprint_rng = UuidRng::from_parent_rng(&mut parent, "blueprint");
747+
let zone_rng = UuidRng::from_parent_rng(&mut parent, "zone");
748748
let network_interface_rng =
749-
UuidRng::from_root_rng(&mut root_rng, "network_interface");
749+
UuidRng::from_parent_rng(&mut parent, "network_interface");
750750

751751
BlueprintBuilderRng { blueprint_rng, zone_rng, network_interface_rng }
752752
}
@@ -755,40 +755,7 @@ impl BlueprintBuilderRng {
755755
// Important to add some more bytes here, so that builders with the
756756
// same seed but different purposes don't end up with the same UUIDs.
757757
const SEED_EXTRA: &str = "blueprint-builder";
758-
let mut seeder = rand_seeder::Seeder::from((seed, SEED_EXTRA));
759-
*self = Self::new_from_rng(seeder.make_rng::<StdRng>());
760-
}
761-
}
762-
763-
#[derive(Debug)]
764-
pub(crate) struct UuidRng {
765-
rng: StdRng,
766-
}
767-
768-
impl UuidRng {
769-
/// Returns a new `UuidRng` generated from the root RNG.
770-
///
771-
/// `extra` is a string that should be unique to the purpose of the UUIDs.
772-
fn from_root_rng(root_rng: &mut StdRng, extra: &'static str) -> Self {
773-
let seed = root_rng.next_u64();
774-
let mut seeder = rand_seeder::Seeder::from((seed, extra));
775-
Self { rng: seeder.make_rng::<StdRng>() }
776-
}
777-
778-
/// `extra` is a string that should be unique to the purpose of the UUIDs.
779-
pub(crate) fn from_seed<H: Hash>(seed: H, extra: &'static str) -> Self {
780-
let mut seeder = rand_seeder::Seeder::from((seed, extra));
781-
Self { rng: seeder.make_rng::<StdRng>() }
782-
}
783-
784-
/// Returns a new UUIDv4 generated from the RNG.
785-
pub(crate) fn next_uuid(&mut self) -> Uuid {
786-
let mut bytes = [0; 16];
787-
self.rng.fill_bytes(&mut bytes);
788-
// Builder::from_random_bytes will turn the random bytes into a valid
789-
// UUIDv4. (Parts of the system depend on the UUID actually being valid
790-
// v4, so it's important that we don't just use `uuid::from_bytes`.)
791-
uuid::Builder::from_random_bytes(bytes).into_uuid()
758+
*self = Self::new_from_parent(typed_rng::from_seed(seed, SEED_EXTRA));
792759
}
793760
}
794761

@@ -1013,7 +980,7 @@ pub mod test {
1013980
assert_eq!(diff.sleds_changed().count(), 0);
1014981

1015982
// The next step is adding these zones to a new sled.
1016-
let new_sled_id = example.sled_rng.next_uuid();
983+
let new_sled_id = example.sled_rng.next();
1017984
let _ =
1018985
example.system.sled(SledBuilder::new().id(new_sled_id)).unwrap();
1019986
let policy = example.system.to_policy().unwrap();

nexus/reconfigurator/planning/src/example.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
//! Example blueprints
66
77
use crate::blueprint_builder::BlueprintBuilder;
8-
use crate::blueprint_builder::UuidRng;
98
use crate::system::SledBuilder;
109
use crate::system::SystemDescription;
1110
use nexus_types::deployment::Blueprint;
@@ -14,6 +13,7 @@ use nexus_types::deployment::Policy;
1413
use nexus_types::inventory::Collection;
1514
use omicron_common::api::external::Generation;
1615
use sled_agent_client::types::OmicronZonesConfig;
16+
use typed_rng::UuidRng;
1717

1818
pub struct ExampleSystem {
1919
pub system: SystemDescription,
@@ -38,8 +38,7 @@ impl ExampleSystem {
3838
) -> ExampleSystem {
3939
let mut system = SystemDescription::new();
4040
let mut sled_rng = UuidRng::from_seed(test_name, "ExampleSystem");
41-
let sled_ids: Vec<_> =
42-
(0..nsleds).map(|_| sled_rng.next_uuid()).collect();
41+
let sled_ids: Vec<_> = (0..nsleds).map(|_| sled_rng.next()).collect();
4342
for sled_id in &sled_ids {
4443
let _ = system.sled(SledBuilder::new().id(*sled_id)).unwrap();
4544
}
@@ -107,6 +106,7 @@ impl ExampleSystem {
107106
let blueprint = builder.build();
108107
let mut builder =
109108
system.to_collection_builder().expect("failed to build collection");
109+
builder.set_rng_seed((test_name, "ExampleSystem collection"));
110110

111111
for sled_id in blueprint.sleds() {
112112
let Some(zones) = blueprint.blueprint_zones.get(&sled_id) else {

nexus/reconfigurator/planning/src/planner.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ mod test {
402402
verify_blueprint(&blueprint2);
403403

404404
// Now add a new sled.
405-
let new_sled_id = example.sled_rng.next_uuid();
405+
let new_sled_id = example.sled_rng.next();
406406
let _ =
407407
example.system.sled(SledBuilder::new().id(new_sled_id)).unwrap();
408408
let policy = example.system.to_policy().unwrap();

typed-rng/Cargo.toml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
[package]
2+
name = "typed-rng"
3+
version = "0.1.0"
4+
edition = "2021"
5+
6+
[dependencies]
7+
omicron-workspace-hack.workspace = true
8+
rand.workspace = true
9+
rand_core.workspace = true
10+
rand_seeder.workspace = true
11+
uuid.workspace = true

0 commit comments

Comments
 (0)