Skip to content

Commit d811bda

Browse files
authored
[sled-agent-config-reconciler] Add separate OmicronDatasets type (#8218)
This is more consistent with how the reconciler remembers zones and disks. This is almost all just moving code around. The only nontrivial changes are: 1. In the spot where we ought to delete datasets, we at least "forget" them in-memory and log an error about leaking the ZFS dataset 2. The dataset serialization task remembers fewer details about datasets it ensured (just the names - that's all we need for the error checking it does when dealing with nested datasets) Staged on top of #8064. PR 1 of 3 working towards #8173.
1 parent 625ef9b commit d811bda

File tree

15 files changed

+1296
-992
lines changed

15 files changed

+1296
-992
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dev-tools/omdb/src/bin/omdb/db.rs

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7351,30 +7351,28 @@ fn inv_collection_print_sleds(collection: &Collection) {
73517351
"LAST RECONCILED CONFIG",
73527352
&last_reconciliation.last_reconciled_config,
73537353
);
7354-
let disk_errs = collect_config_reconciler_errors(
7355-
&last_reconciliation.external_disks,
7356-
);
7357-
let dataset_errs = collect_config_reconciler_errors(
7358-
&last_reconciliation.datasets,
7359-
);
7360-
let zone_errs = collect_config_reconciler_errors(
7361-
&last_reconciliation.zones,
7362-
);
7363-
for (label, errs) in [
7364-
("disk", disk_errs),
7365-
("dataset", dataset_errs),
7366-
("zone", zone_errs),
7367-
] {
7368-
if errs.is_empty() {
7369-
println!(" all {label}s reconciled successfully");
7370-
} else {
7371-
println!(
7372-
" {} {label} reconciliation errors:",
7373-
errs.len()
7374-
);
7375-
for err in errs {
7376-
println!(" {err}");
7377-
}
7354+
}
7355+
let disk_errs = collect_config_reconciler_errors(
7356+
&last_reconciliation.external_disks,
7357+
);
7358+
let dataset_errs =
7359+
collect_config_reconciler_errors(&last_reconciliation.datasets);
7360+
let zone_errs =
7361+
collect_config_reconciler_errors(&last_reconciliation.zones);
7362+
for (label, errs) in [
7363+
("disk", disk_errs),
7364+
("dataset", dataset_errs),
7365+
("zone", zone_errs),
7366+
] {
7367+
if errs.is_empty() {
7368+
println!(" all {label}s reconciled successfully");
7369+
} else {
7370+
println!(
7371+
" {} {label} reconciliation errors:",
7372+
errs.len()
7373+
);
7374+
for err in errs {
7375+
println!(" {err}");
73787376
}
73797377
}
73807378
}

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use chrono::{DateTime, Utc};
1212
use daft::Diffable;
1313
use id_map::IdMap;
1414
use id_map::IdMappable;
15+
use omicron_common::disk::{DatasetKind, DatasetName};
1516
use omicron_common::ledger::Ledgerable;
1617
use omicron_common::{
1718
api::{
@@ -327,6 +328,10 @@ impl OmicronZoneConfig {
327328
Some(self.id),
328329
)
329330
}
331+
332+
pub fn dataset_name(&self) -> Option<DatasetName> {
333+
self.zone_type.dataset_name()
334+
}
330335
}
331336

332337
/// Describes a persistent ZFS dataset associated with an Omicron zone
@@ -613,6 +618,41 @@ impl OmicronZoneType {
613618
| OmicronZoneType::Oximeter { .. } => None,
614619
}
615620
}
621+
622+
/// If this kind of zone has an associated dataset, return the dataset's
623+
/// name. Otherwise, return `None`.
624+
pub fn dataset_name(&self) -> Option<DatasetName> {
625+
let (dataset, dataset_kind) = match self {
626+
OmicronZoneType::BoundaryNtp { .. }
627+
| OmicronZoneType::InternalNtp { .. }
628+
| OmicronZoneType::Nexus { .. }
629+
| OmicronZoneType::Oximeter { .. }
630+
| OmicronZoneType::CruciblePantry { .. } => None,
631+
OmicronZoneType::Clickhouse { dataset, .. } => {
632+
Some((dataset, DatasetKind::Clickhouse))
633+
}
634+
OmicronZoneType::ClickhouseKeeper { dataset, .. } => {
635+
Some((dataset, DatasetKind::ClickhouseKeeper))
636+
}
637+
OmicronZoneType::ClickhouseServer { dataset, .. } => {
638+
Some((dataset, DatasetKind::ClickhouseServer))
639+
}
640+
OmicronZoneType::CockroachDb { dataset, .. } => {
641+
Some((dataset, DatasetKind::Cockroach))
642+
}
643+
OmicronZoneType::Crucible { dataset, .. } => {
644+
Some((dataset, DatasetKind::Crucible))
645+
}
646+
OmicronZoneType::ExternalDns { dataset, .. } => {
647+
Some((dataset, DatasetKind::ExternalDns))
648+
}
649+
OmicronZoneType::InternalDns { dataset, .. } => {
650+
Some((dataset, DatasetKind::InternalDns))
651+
}
652+
}?;
653+
654+
Some(DatasetName::new(dataset.pool_name, dataset_kind))
655+
}
616656
}
617657

618658
/// Like [`OmicronZoneType`], but without any associated data.

schema/all-zones-requests.json

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"$schema": "http://json-schema.org/draft-07/schema#",
33
"title": "OmicronZonesConfigLocal",
4-
"description": "Combines the Nexus-provided `OmicronZonesConfig` (which describes what Nexus wants for all of its zones) with the locally-determined configuration for these zones.",
4+
"description": "Legacy type of the ledgered zone config.",
55
"type": "object",
66
"required": [
77
"ledger_generation",
@@ -10,20 +10,10 @@
1010
],
1111
"properties": {
1212
"ledger_generation": {
13-
"description": "ledger-managed generation number\n\nThis generation is managed by the ledger facility itself. It's bumped whenever we write a new ledger. In practice, we don't currently have any reason to bump this _for a given Omicron generation_ so it's somewhat redundant. In principle, if we needed to modify the ledgered configuration due to some event that doesn't change the Omicron config (e.g., if we wanted to move the root filesystem to a different path), we could do that by bumping this generation.",
14-
"allOf": [
15-
{
16-
"$ref": "#/definitions/Generation"
17-
}
18-
]
13+
"$ref": "#/definitions/Generation"
1914
},
2015
"omicron_generation": {
21-
"description": "generation of the Omicron-provided part of the configuration\n\nThis generation number is outside of Sled Agent's control. We store exactly what we were given and use this number to decide when to fail requests to establish an outdated configuration.\n\nYou can think of this as a major version number, with `ledger_generation` being a minor version number. See `is_newer_than()`.",
22-
"allOf": [
23-
{
24-
"$ref": "#/definitions/Generation"
25-
}
26-
]
16+
"$ref": "#/definitions/Generation"
2717
},
2818
"zones": {
2919
"type": "array",
@@ -269,7 +259,6 @@
269259
}
270260
},
271261
"OmicronZoneConfigLocal": {
272-
"description": "Combines the Nexus-provided `OmicronZoneConfig` (which describes what Nexus wants for this zone) with any locally-determined configuration (like the path to the root filesystem)",
273262
"type": "object",
274263
"required": [
275264
"root",

sled-agent/config-reconciler/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ expectorate.workspace = true
4545
illumos-utils = { workspace = true, features = ["testing"] }
4646
omicron-test-utils.workspace = true
4747
proptest.workspace = true
48+
schemars.workspace = true
4849
scopeguard.workspace = true
4950
serde_json.workspace = true
5051
sled-storage = { workspace = true, features = ["testing"] }

0 commit comments

Comments
 (0)