Skip to content

Commit e5fa9fb

Browse files
sunshowerssunshowers
authored
[sled-agent] make OmicronSledConfig use IdOrdMap (#9378)
For backwards wire protocol and ledgered data compatibility, iddqd now has an alternate serializer that uses maps rather than sequences. This means that we don't need a sled-agent-api version bump, which is neat. Also remove `RawDiskWithId` since the `DiskIdentity` can be borrowed from `RawDisk`.
1 parent 108747e commit e5fa9fb

File tree

27 files changed

+365
-330
lines changed

27 files changed

+365
-330
lines changed

‎Cargo.lock‎

Lines changed: 3 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@ hyper = "1.6.0"
501501
hyper-util = "0.1.16"
502502
hyper-rustls = "0.27.7"
503503
hyper-staticfile = "0.10.1"
504-
iddqd = { version = "0.3.13", features = ["daft", "serde", "schemars08"] }
504+
iddqd = { version = "0.3.16", features = ["daft", "serde", "schemars08"] }
505505
id-map = { path = "id-map" }
506506
illumos-utils = { path = "illumos-utils" }
507507
iana-time-zone = "0.1.63"

‎common/Cargo.toml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ futures.workspace = true
2626
hex.workspace = true
2727
http.workspace = true
2828
iddqd.workspace = true
29-
id-map.workspace = true
3029
ipnetwork.workspace = true
3130
lldp_protocol.workspace = true
3231
macaddr.workspace = true

‎common/src/disk.rs‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
use anyhow::bail;
88
use camino::{Utf8Path, Utf8PathBuf};
99
use daft::Diffable;
10-
use id_map::IdMappable;
10+
use iddqd::IdOrdItem;
11+
use iddqd::id_upcast;
1112
use omicron_uuid_kinds::DatasetUuid;
1213
use omicron_uuid_kinds::PhysicalDiskUuid;
1314
use omicron_uuid_kinds::ZpoolUuid;
@@ -44,12 +45,14 @@ pub struct OmicronPhysicalDiskConfig {
4445
pub pool_id: ZpoolUuid,
4546
}
4647

47-
impl IdMappable for OmicronPhysicalDiskConfig {
48-
type Id = PhysicalDiskUuid;
48+
impl IdOrdItem for OmicronPhysicalDiskConfig {
49+
type Key<'a> = PhysicalDiskUuid;
4950

50-
fn id(&self) -> Self::Id {
51+
fn key(&self) -> Self::Key<'_> {
5152
self.id
5253
}
54+
55+
id_upcast!();
5356
}
5457

5558
#[derive(
@@ -410,12 +413,14 @@ pub struct DatasetConfig {
410413
pub inner: SharedDatasetConfig,
411414
}
412415

413-
impl IdMappable for DatasetConfig {
414-
type Id = DatasetUuid;
416+
impl IdOrdItem for DatasetConfig {
417+
type Key<'a> = DatasetUuid;
415418

416-
fn id(&self) -> Self::Id {
419+
fn key(&self) -> Self::Key<'_> {
417420
self.id
418421
}
422+
423+
id_upcast!();
419424
}
420425

421426
#[derive(

‎nexus-sled-agent-shared/Cargo.toml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ workspace = true
1010
camino.workspace = true
1111
chrono.workspace = true
1212
daft.workspace = true
13-
id-map.workspace = true
1413
iddqd.workspace = true
1514
illumos-utils.workspace = true
1615
indent_write.workspace = true

‎nexus-sled-agent-shared/src/inventory.rs‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ use std::time::Duration;
1212
use camino::Utf8PathBuf;
1313
use chrono::{DateTime, Utc};
1414
use daft::Diffable;
15-
use id_map::IdMap;
16-
use id_map::IdMappable;
1715
use iddqd::IdOrdItem;
1816
use iddqd::IdOrdMap;
1917
use iddqd::id_upcast;
@@ -406,7 +404,7 @@ pub enum ConfigReconcilerInventoryStatus {
406404
NotYetRun,
407405
/// The reconciler task is actively running.
408406
Running {
409-
config: OmicronSledConfig,
407+
config: Box<OmicronSledConfig>,
410408
started_at: DateTime<Utc>,
411409
running_for: Duration,
412410
},
@@ -1030,9 +1028,16 @@ impl HostPhase2DesiredSlots {
10301028
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq, Eq)]
10311029
pub struct OmicronSledConfig {
10321030
pub generation: Generation,
1033-
pub disks: IdMap<OmicronPhysicalDiskConfig>,
1034-
pub datasets: IdMap<DatasetConfig>,
1035-
pub zones: IdMap<OmicronZoneConfig>,
1031+
// Serialize and deserialize disks, datasets, and zones as maps for
1032+
// backwards compatibility. Newer IdOrdMaps should not use IdOrdMapAsMap.
1033+
#[serde(
1034+
with = "iddqd::id_ord_map::IdOrdMapAsMap::<OmicronPhysicalDiskConfig>"
1035+
)]
1036+
pub disks: IdOrdMap<OmicronPhysicalDiskConfig>,
1037+
#[serde(with = "iddqd::id_ord_map::IdOrdMapAsMap::<DatasetConfig>")]
1038+
pub datasets: IdOrdMap<DatasetConfig>,
1039+
#[serde(with = "iddqd::id_ord_map::IdOrdMapAsMap::<OmicronZoneConfig>")]
1040+
pub zones: IdOrdMap<OmicronZoneConfig>,
10361041
pub remove_mupdate_override: Option<MupdateOverrideUuid>,
10371042
#[serde(default = "HostPhase2DesiredSlots::current_contents")]
10381043
pub host_phase_2: HostPhase2DesiredSlots,
@@ -1042,9 +1047,9 @@ impl Default for OmicronSledConfig {
10421047
fn default() -> Self {
10431048
Self {
10441049
generation: Generation::new(),
1045-
disks: IdMap::default(),
1046-
datasets: IdMap::default(),
1047-
zones: IdMap::default(),
1050+
disks: IdOrdMap::default(),
1051+
datasets: IdOrdMap::default(),
1052+
zones: IdOrdMap::default(),
10481053
remove_mupdate_override: None,
10491054
host_phase_2: HostPhase2DesiredSlots::current_contents(),
10501055
}
@@ -1107,12 +1112,14 @@ pub struct OmicronZoneConfig {
11071112
pub image_source: OmicronZoneImageSource,
11081113
}
11091114

1110-
impl IdMappable for OmicronZoneConfig {
1111-
type Id = OmicronZoneUuid;
1115+
impl IdOrdItem for OmicronZoneConfig {
1116+
type Key<'a> = OmicronZoneUuid;
11121117

1113-
fn id(&self) -> Self::Id {
1118+
fn key(&self) -> Self::Key<'_> {
11141119
self.id
11151120
}
1121+
1122+
id_upcast!();
11161123
}
11171124

11181125
impl OmicronZoneConfig {

‎nexus/db-model/src/inventory.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -970,7 +970,7 @@ impl InvConfigReconcilerStatus {
970970
let config = get_config(&config_id.into())
971971
.context("missing sled config we should have fetched")?;
972972
ConfigReconcilerInventoryStatus::Running {
973-
config,
973+
config: Box::new(config),
974974
started_at: self.reconciler_status_timestamp.context(
975975
"missing reconciler status timestamp \
976976
for kind 'running'",

‎nexus/db-queries/src/db/datastore/inventory.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3150,9 +3150,9 @@ impl DataStore {
31503150
remove_mupdate_override: sled_config
31513151
.remove_mupdate_override
31523152
.map(From::from),
3153-
disks: IdMap::default(),
3154-
datasets: IdMap::default(),
3155-
zones: IdMap::default(),
3153+
disks: IdOrdMap::default(),
3154+
datasets: IdOrdMap::default(),
3155+
zones: IdOrdMap::default(),
31563156
host_phase_2: sled_config.host_phase_2.into(),
31573157
},
31583158
});
@@ -3268,7 +3268,7 @@ impl DataStore {
32683268
.map_err(|e| {
32693269
Error::internal_error(&format!("{:#}", e.to_string()))
32703270
})?;
3271-
config_with_id.config.zones.insert(zone);
3271+
config_with_id.config.zones.insert_overwrite(zone);
32723272
}
32733273

32743274
bail_unless!(
@@ -3309,7 +3309,7 @@ impl DataStore {
33093309
row.id, row.sled_config_id
33103310
))
33113311
})?;
3312-
config_with_id.config.datasets.insert(
3312+
config_with_id.config.datasets.insert_overwrite(
33133313
row.try_into().map_err(|e| {
33143314
Error::internal_error(&format!("{e:#}"))
33153315
})?,
@@ -3350,7 +3350,7 @@ impl DataStore {
33503350
row.id, row.sled_config_id
33513351
))
33523352
})?;
3353-
config_with_id.config.disks.insert(row.into());
3353+
config_with_id.config.disks.insert_overwrite(row.into());
33543354
}
33553355
}
33563356
}
@@ -4302,11 +4302,11 @@ impl ConfigReconcilerRows {
43024302
// If this config exactly matches the ledgered or
43034303
// most-recently-reconciled configs, we can reuse those IDs.
43044304
// Otherwise, accumulate a new one.
4305-
let reconciler_status_sled_config = if Some(config)
4305+
let reconciler_status_sled_config = if Some(&**config)
43064306
== sled_agent.ledgered_sled_config.as_ref()
43074307
{
43084308
ledgered_sled_config
4309-
} else if Some(config)
4309+
} else if Some(&**config)
43104310
== sled_agent
43114311
.last_reconciliation
43124312
.as_ref()
@@ -5255,7 +5255,7 @@ mod test {
52555255
sa1.last_reconciliation = None;
52565256

52575257
sa2.reconciler_status = ConfigReconcilerInventoryStatus::Running {
5258-
config: sa2.ledgered_sled_config.clone().unwrap(),
5258+
config: Box::new(sa2.ledgered_sled_config.clone().unwrap()),
52595259
started_at: now_db_precision(),
52605260
running_for: Duration::from_secs(1),
52615261
};

‎nexus/inventory/src/collector.rs‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -719,7 +719,8 @@ mod test {
719719
use super::Collector;
720720
use crate::StaticSledAgentEnumerator;
721721
use gateway_messages::SpPort;
722-
use id_map::IdMap;
722+
use iddqd::IdOrdMap;
723+
use iddqd::id_ord_map;
723724
use nexus_sled_agent_shared::inventory::ConfigReconcilerInventoryStatus;
724725
use nexus_sled_agent_shared::inventory::HostPhase2DesiredSlots;
725726
use nexus_sled_agent_shared::inventory::OmicronSledConfig;
@@ -989,18 +990,18 @@ mod test {
989990
client
990991
.omicron_config_put(&OmicronSledConfig {
991992
generation: Generation::from(3),
992-
disks: IdMap::default(),
993-
datasets: IdMap::default(),
994-
zones: [OmicronZoneConfig {
995-
id: zone_id,
996-
zone_type: OmicronZoneType::Oximeter {
997-
address: zone_address,
998-
},
999-
filesystem_pool: Some(filesystem_pool),
1000-
image_source: OmicronZoneImageSource::InstallDataset,
1001-
}]
1002-
.into_iter()
1003-
.collect(),
993+
disks: IdOrdMap::default(),
994+
datasets: IdOrdMap::default(),
995+
zones: id_ord_map! {
996+
OmicronZoneConfig {
997+
id: zone_id,
998+
zone_type: OmicronZoneType::Oximeter {
999+
address: zone_address,
1000+
},
1001+
filesystem_pool: Some(filesystem_pool),
1002+
image_source: OmicronZoneImageSource::InstallDataset,
1003+
}
1004+
},
10041005
remove_mupdate_override: None,
10051006
host_phase_2: HostPhase2DesiredSlots::current_contents(),
10061007
})

‎nexus/inventory/src/examples.rs‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -501,11 +501,14 @@ pub fn representative() -> Representative {
501501
let mut zpools = Vec::new();
502502
for disk in &disks {
503503
let pool_id = zpool_id_iter.next().unwrap();
504-
sled14.disks.insert(OmicronPhysicalDiskConfig {
505-
identity: disk.identity.clone(),
506-
id: disk_id_iter.next().unwrap(),
507-
pool_id,
508-
});
504+
sled14
505+
.disks
506+
.insert_unique(OmicronPhysicalDiskConfig {
507+
identity: disk.identity.clone(),
508+
id: disk_id_iter.next().unwrap(),
509+
pool_id,
510+
})
511+
.unwrap();
509512
zpools.push(InventoryZpool {
510513
id: pool_id,
511514
total_size: ByteCount::from(4096),
@@ -524,15 +527,18 @@ pub fn representative() -> Representative {
524527
reservation: None,
525528
compression: "lz4".to_string(),
526529
}];
527-
sled14.datasets.insert(DatasetConfig {
528-
id: datasets[0].id.unwrap(),
529-
name: dataset_name,
530-
inner: SharedDatasetConfig {
531-
compression: datasets[0].compression.parse().unwrap(),
532-
quota: datasets[0].quota,
533-
reservation: datasets[0].reservation,
534-
},
535-
});
530+
sled14
531+
.datasets
532+
.insert_unique(DatasetConfig {
533+
id: datasets[0].id.unwrap(),
534+
name: dataset_name,
535+
inner: SharedDatasetConfig {
536+
compression: datasets[0].compression.parse().unwrap(),
537+
quota: datasets[0].quota,
538+
reservation: datasets[0].reservation,
539+
},
540+
})
541+
.unwrap();
536542

537543
builder
538544
.found_sled_inventory(

0 commit comments

Comments
 (0)