From 90c3e186bd6e749e45a5a907e81517f10d5583ee Mon Sep 17 00:00:00 2001 From: Rain Date: Thu, 29 May 2025 06:58:23 +0000 Subject: [PATCH 1/3] [spr] changes to main this commit is based on Created using spr 1.3.6-beta.1 [skip ci] --- Cargo.lock | 64 +- Cargo.toml | 8 +- common/Cargo.toml | 1 + common/src/update/mupdate_override.rs | 38 +- dev-tools/omdb/tests/successes.out | 27 +- dev-tools/reconfigurator-cli/src/lib.rs | 58 ++ .../cmds-set-remove-mupdate-override.txt | 5 + .../tests/output/cmds-example-stdout | 22 +- .../output/cmds-expunge-newly-added-stdout | 33 +- .../tests/output/cmds-set-mgs-updates-stdout | 115 +-- .../cmds-set-remove-mupdate-override-stdout | 267 ++++++- .../tests/output/cmds-set-zone-images-stdout | 73 +- illumos-utils/src/running_zone.rs | 2 +- illumos-utils/src/zfs.rs | 2 +- installinator/Cargo.toml | 1 + installinator/src/write.rs | 62 +- internal-dns/resolver/src/resolver.rs | 2 +- nexus/db-model/src/deployment.rs | 6 + nexus/db-model/src/schema_versions.rs | 3 +- .../db-queries/src/db/datastore/deployment.rs | 3 + nexus/db-queries/src/db/datastore/rack.rs | 5 + nexus/db-schema/src/schema.rs | 2 + .../tests/integration/blueprint_edit.rs | 2 +- nexus/reconfigurator/execution/src/dns.rs | 1 + .../planning/src/blueprint_builder/builder.rs | 33 + .../example_builder_zone_counts_blueprint.txt | 11 +- .../output/planner_basic_add_sled_2_3.txt | 5 +- .../output/planner_basic_add_sled_3_5.txt | 5 +- ...dataset_settings_modified_in_place_1_2.txt | 5 +- .../planner_decommissions_sleds_1_2.txt | 5 +- .../planner_decommissions_sleds_bp2.txt | 11 +- .../planner_deploy_all_keeper_nodes_1_2.txt | 5 +- .../planner_deploy_all_keeper_nodes_3_4.txt | 5 +- .../planner_deploy_all_keeper_nodes_4_5.txt | 5 +- .../planner_deploy_all_keeper_nodes_5_6.txt | 5 +- ...lanner_expunge_clickhouse_clusters_3_4.txt | 5 +- ...lanner_expunge_clickhouse_clusters_5_6.txt | 5 +- ...ouse_zones_after_policy_is_changed_3_4.txt | 5 +- .../output/planner_nonprovisionable_1_2.txt | 5 +- .../output/planner_nonprovisionable_2_2a.txt | 5 +- .../output/planner_nonprovisionable_bp2.txt | 11 +- .../output/zone_image_source_change_1.txt | 5 +- .../background/tasks/blueprint_execution.rs | 1 + .../app/background/tasks/blueprint_load.rs | 1 + nexus/test-utils/src/lib.rs | 1 + nexus/types/src/deployment.rs | 30 +- nexus/types/src/deployment/blueprint_diff.rs | 21 +- .../types/src/deployment/blueprint_display.rs | 2 + openapi/nexus-internal.json | 18 + .../bp-add-target-release-min-gen/up1.sql | 2 + .../bp-add-target-release-min-gen/up2.sql | 2 + schema/crdb/dbinit.sql | 20 +- sled-agent/src/rack_setup/service.rs | 1 + sled-agent/zone-images/Cargo.toml | 5 +- sled-agent/zone-images/src/lib.rs | 2 +- .../zone-images/src/mupdate_override.rs | 653 ++++++++++++++++-- sled-agent/zone-images/src/source_resolver.rs | 17 +- test-utils/src/dev/clickhouse.rs | 2 +- test-utils/src/dev/db.rs | 4 +- test-utils/src/dev/dendrite.rs | 2 +- test-utils/src/dev/maghemite.rs | 2 +- wicketd/Cargo.toml | 1 + wicketd/tests/integration_tests/updates.rs | 63 +- workspace-hack/Cargo.toml | 2 + 64 files changed, 1499 insertions(+), 291 deletions(-) create mode 100644 schema/crdb/bp-add-target-release-min-gen/up1.sql create mode 100644 schema/crdb/bp-add-target-release-min-gen/up2.sql diff --git a/Cargo.lock b/Cargo.lock index 0008adc5abb..9db389329a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -76,9 +76,9 @@ dependencies = [ [[package]] name = "allocator-api2" -version = "0.2.18" +version = "0.2.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5c6cb57a04249c6480766f7f7cef5467412af1490f8d1e243141daddada3264f" +checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" [[package]] name = "android-tzdata" @@ -1058,9 +1058,9 @@ dependencies = [ [[package]] name = "camino-tempfile" -version = "1.3.0" +version = "1.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4f37f0bd2bfeb019b359945a89e5a3541ac105195da0b30219a0a1b84d681dda" +checksum = "64308c4c82a5c38679945ddf88738dc1483dcc563bbb5780755ae9f8497d2b20" dependencies = [ "camino", "tempfile", @@ -1068,9 +1068,9 @@ dependencies = [ [[package]] name = "camino-tempfile-ext" -version = "0.3.0" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f7b3ab964bd19840be2c70b7795188187a8c1c90006a310b1500bf67b35a80d3" +checksum = "97daccb5e4621ca2bbd3e8716a24f11721afed7fc82c9008d30c6fed3390183a" dependencies = [ "anstream", "anstyle", @@ -3046,12 +3046,12 @@ dependencies = [ [[package]] name = "errno" -version = "0.3.9" +version = "0.3.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "534c5cf6194dfab3db3242765c03bbe257cf92f22b38f6bc0c58d59108a820ba" +checksum = "cea14ef9355e3beab063703aa9dab15afd25f0667c341310c1e5274bb1d0da18" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -3242,9 +3242,9 @@ checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" [[package]] name = "foldhash" -version = "0.1.3" +version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f81ec6369c545a7d40e4589b5597581fa1c441fe1cce96dd1de43159910a36a2" +checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" [[package]] name = "foreign-types" @@ -4554,14 +4554,16 @@ dependencies = [ [[package]] name = "iddqd" -version = "0.3.0" +version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8129594fa2dd41828ac3a1f3080037c24b7588fff5422b0dba0ace331b95a7b2" +checksum = "7c3407dc49fe79cf7281b48d3a251cf8c11ff960a652e32cbef8848216c4b3c8" dependencies = [ + "allocator-api2", "daft", "debug-ignore", "derive-where", "equivalent", + "foldhash", "hashbrown 0.15.3", "ref-cast", "rustc-hash 2.1.1", @@ -4814,6 +4816,7 @@ dependencies = [ "hex", "hex-literal", "http", + "iddqd", "illumos-utils", "installinator-client", "installinator-common", @@ -5507,6 +5510,12 @@ version = "0.4.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "78b3ae25bc7c8c38cec158d1f2757ee79e9b3740fbc7ccf0e59e4b08d793fa89" +[[package]] +name = "linux-raw-sys" +version = "0.9.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd945864f07fe9f5371a27ad7b52a172b4b499999f1d97574c9fa68373937e12" + [[package]] name = "litemap" version = "0.7.3" @@ -7141,6 +7150,7 @@ dependencies = [ "hex", "http", "id-map", + "iddqd", "ipnetwork", "libc", "macaddr", @@ -7954,6 +7964,7 @@ dependencies = [ "elliptic-curve", "ff", "flate2", + "foldhash", "fs-err 3.1.0", "futures", "futures-channel", @@ -10835,6 +10846,19 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rustix" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c71e83d6afe7ff64890ec6b71d6a69bb8a610ab78ce364b3352876bb4c801266" +dependencies = [ + "bitflags 2.9.0", + "errno", + "libc", + "linux-raw-sys 0.9.4", + "windows-sys 0.59.0", +] + [[package]] name = "rustls" version = "0.21.12" @@ -11714,18 +11738,21 @@ dependencies = [ "camino", "camino-tempfile-ext", "dropshot", - "id-map", + "iddqd", "illumos-utils", "nexus-sled-agent-shared", "omicron-common", "omicron-uuid-kinds", "omicron-workspace-hack", "pretty_assertions", + "rayon", "serde_json", + "sha2", "sled-storage", "slog", "slog-error-chain", "thiserror 1.0.69", + "tufaceous-artifact", ] [[package]] @@ -12589,14 +12616,14 @@ checksum = "42a4d50cdb458045afc8131fd91b64904da29548bcb63c7236e0844936c13078" [[package]] name = "tempfile" -version = "3.13.0" +version = "3.20.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f0f2c9fc62d0beef6951ccffd757e241266a2c833136efbe35af6cd2567dca5b" +checksum = "e8a64e3985349f2441a1a9ef0b853f869006c3855f2cda6862a94d26ebb9d6a1" dependencies = [ - "cfg-if", "fastrand", + "getrandom 0.3.1", "once_cell", - "rustix 0.38.37", + "rustix 1.0.7", "windows-sys 0.59.0", ] @@ -14424,6 +14451,7 @@ dependencies = [ "serde", "serde_json", "sha2", + "sled-agent-zone-images", "sled-hardware-types", "slog", "slog-dtrace", diff --git a/Cargo.toml b/Cargo.toml index 271ee64d830..82d2e4dc5ac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -363,8 +363,8 @@ buf-list = { version = "1.0.3", features = ["tokio1"] } byteorder = "1.5.0" bytes = "1.10.1" camino = { version = "1.1", features = ["serde1"] } -camino-tempfile = "1.3.0" -camino-tempfile-ext = { version = "0.3.0", features = ["assert-color"] } +camino-tempfile = "1.4.1" +camino-tempfile-ext = { version = "0.3.1", features = ["assert-color"] } cancel-safe-futures = "0.1.5" cargo_metadata = "0.19.2" chacha20poly1305 = "0.10.1" @@ -473,7 +473,7 @@ hyper = "1.6.0" hyper-util = "0.1.11" hyper-rustls = "0.26.0" hyper-staticfile = "0.10.1" -iddqd = { version = "0.3.0", features = ["daft", "serde"] } +iddqd = { version = "0.3.3", features = ["daft", "serde"] } id-map = { path = "id-map" } illumos-utils = { path = "illumos-utils" } iana-time-zone = "0.1.63" @@ -708,7 +708,7 @@ libsw = { version = "3.4.0", features = ["tokio"] } syn = { version = "2.0" } tabled = "0.15.0" tar = "0.4" -tempfile = "3.10" +tempfile = "3.20" term = "0.7" termios = "0.3" termtree = "0.5.1" diff --git a/common/Cargo.toml b/common/Cargo.toml index 2e1cb217743..c2fc8115dd1 100644 --- a/common/Cargo.toml +++ b/common/Cargo.toml @@ -25,6 +25,7 @@ dropshot.workspace = true futures.workspace = true hex.workspace = true http.workspace = true +iddqd.workspace = true id-map.workspace = true ipnetwork.workspace = true lldp_protocol.workspace = true diff --git a/common/src/update/mupdate_override.rs b/common/src/update/mupdate_override.rs index e657320e0c7..2e641eb1791 100644 --- a/common/src/update/mupdate_override.rs +++ b/common/src/update/mupdate_override.rs @@ -6,9 +6,10 @@ use std::collections::BTreeSet; +use iddqd::{IdOrdItem, IdOrdMap, id_upcast}; use omicron_uuid_kinds::MupdateOverrideUuid; use serde::{Deserialize, Serialize}; -use tufaceous_artifact::ArtifactHashId; +use tufaceous_artifact::{ArtifactHash, ArtifactHashId}; /// MUPdate override information, typically serialized as JSON (RFD 556). /// @@ -20,10 +21,45 @@ pub struct MupdateOverrideInfo { pub mupdate_uuid: MupdateOverrideUuid, /// Artifact hashes written out to the install dataset. + /// + /// Currently includes the host phase 2 and composite control plane + /// artifacts. Information about individual zones is included in + /// [`Self::zones`]. pub hash_ids: BTreeSet, + + /// Control plane zone file names and hashes. + pub zones: IdOrdMap, } impl MupdateOverrideInfo { /// The name of the file on the install dataset. pub const FILE_NAME: &'static str = "mupdate-override.json"; } + +/// Control plane zone information written out to the install dataset. +/// +/// Part of [`MupdateOverrideInfo`]. +#[derive( + Clone, Debug, Eq, Ord, PartialEq, PartialOrd, Deserialize, Serialize, +)] +pub struct MupdateOverrideZone { + /// The file name. + pub file_name: String, + + /// The file size. + pub file_size: u64, + + /// The hash of the file. + pub hash: ArtifactHash, +} + +impl IdOrdItem for MupdateOverrideZone { + type Key<'a> = &'a str; + + #[inline] + fn key(&self) -> Self::Key<'_> { + &self.file_name + } + + id_upcast!(); +} diff --git a/dev-tools/omdb/tests/successes.out b/dev-tools/omdb/tests/successes.out index eef7d76fcc5..e4b48f5a947 100644 --- a/dev-tools/omdb/tests/successes.out +++ b/dev-tools/omdb/tests/successes.out @@ -1469,11 +1469,12 @@ parent: read from:: SingleNode METADATA: - created by::::::::::: nexus-test-utils - created at::::::::::: - comment:::::::::::::: initial test blueprint - internal DNS version: 1 - external DNS version: 2 + created by::::::::::::: nexus-test-utils + created at::::::::::::: + comment:::::::::::::::: initial test blueprint + internal DNS version::: 1 + external DNS version::: 2 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 0 @@ -1570,11 +1571,12 @@ parent: read from:: SingleNode METADATA: - created by::::::::::: nexus-test-utils - created at::::::::::: - comment:::::::::::::: initial test blueprint - internal DNS version: 1 - external DNS version: 2 + created by::::::::::::: nexus-test-utils + created at::::::::::::: + comment:::::::::::::::: initial test blueprint + internal DNS version::: 1 + external DNS version::: 2 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 0 @@ -1656,8 +1658,9 @@ to: blueprint ............. cluster.preserve_downgrade_option: (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 2 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 2 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/dev-tools/reconfigurator-cli/src/lib.rs b/dev-tools/reconfigurator-cli/src/lib.rs index 294a2d6df5a..a9e86cb860b 100644 --- a/dev-tools/reconfigurator-cli/src/lib.rs +++ b/dev-tools/reconfigurator-cli/src/lib.rs @@ -53,6 +53,7 @@ use std::borrow::Cow; use std::collections::BTreeMap; use std::fmt::{self, Write}; use std::io::IsTerminal; +use std::num::ParseIntError; use std::str::FromStr; use swrite::{SWrite, swriteln}; use tabled::Tabled; @@ -439,6 +440,16 @@ enum BlueprintEditCommands { /// the UUID to set the field to, or "unset" value: MupdateOverrideUuidOpt, }, + /// set the minimum generation for which target releases are accepted + /// + /// At the moment, this just sets the field to the given value. In the + /// future, we'll likely want to set this based on the current target + /// release generation. + #[clap(visible_alias = "set-target-release-min-gen")] + SetTargetReleaseMinimumGeneration { + /// the minimum target release generation + generation: Generation, + }, /// expunge a zone ExpungeZone { zone_id: OmicronZoneUuid }, /// configure an SP update @@ -559,6 +570,42 @@ impl From for Option { } } +/// Clap field for an optional generation. +/// +/// This structure is similar to `Option`, but is specified separately to: +/// +/// 1. Disable clap's magic around `Option`. +/// 2. Provide a custom parser. +/// +/// There are other ways to do both 1 and 2 (e.g. specify the type as +/// `std::option::Option`), but when combined they're uglier than this. +#[derive(Clone, Copy, Debug)] +enum GenerationOpt { + Unset, + Set(Generation), +} + +impl FromStr for GenerationOpt { + type Err = ParseIntError; + + fn from_str(s: &str) -> Result { + if s == "unset" || s == "none" { + Ok(Self::Unset) + } else { + Ok(Self::Set(s.parse::()?)) + } + } +} + +impl From for Option { + fn from(value: GenerationOpt) -> Self { + match value { + GenerationOpt::Unset => None, + GenerationOpt::Set(generation) => Some(generation), + } + } +} + #[derive(Clone, Debug, Subcommand)] enum SpUpdateComponent { /// update the SP itself @@ -1104,6 +1151,17 @@ fn cmd_blueprint_edit( } } } + BlueprintEditCommands::SetTargetReleaseMinimumGeneration { + generation, + } => { + builder + .set_target_release_minimum_generation( + blueprint.target_release_minimum_generation, + generation, + ) + .context("failed to set target release minimum generation")?; + format!("set target release minimum generation to {generation}") + } BlueprintEditCommands::SetZoneImage { zone_id, image_source } => { let sled_id = sled_with_zone(&builder, &zone_id)?; let source = BlueprintZoneImageSource::from(image_source); diff --git a/dev-tools/reconfigurator-cli/tests/input/cmds-set-remove-mupdate-override.txt b/dev-tools/reconfigurator-cli/tests/input/cmds-set-remove-mupdate-override.txt index d3ce43b1051..ba23a335401 100644 --- a/dev-tools/reconfigurator-cli/tests/input/cmds-set-remove-mupdate-override.txt +++ b/dev-tools/reconfigurator-cli/tests/input/cmds-set-remove-mupdate-override.txt @@ -39,3 +39,8 @@ blueprint-edit latest debug force-sled-generation-bump d81c6a84-79b8-4958-ae41-e blueprint-edit latest set-remove-mupdate-override 00320471-945d-413c-85e7-03e091a70b3c ffffffff-ffff-ffff-ffff-ffffffffffff blueprint-diff 7f976e0d-d2a5-4eeb-9e82-c82bc2824aba latest + +# Set the target release minimum generation for the blueprint. +blueprint-edit latest set-target-release-min-gen 2 +blueprint-show latest +blueprint-diff afb09faf-a586-4483-9289-04d4f1d8ba23 latest diff --git a/dev-tools/reconfigurator-cli/tests/output/cmds-example-stdout b/dev-tools/reconfigurator-cli/tests/output/cmds-example-stdout index 1f92791db7c..a9b0c4c4c2c 100644 --- a/dev-tools/reconfigurator-cli/tests/output/cmds-example-stdout +++ b/dev-tools/reconfigurator-cli/tests/output/cmds-example-stdout @@ -355,11 +355,12 @@ parent: 02697f74-b14a-4418-90f0-c28b2a3a6aa9 read from:: SingleNode METADATA: - created by::::::::::: test suite - created at::::::::::: - comment:::::::::::::: (none) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: test suite + created at::::::::::::: + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 0 @@ -451,11 +452,12 @@ parent: 02697f74-b14a-4418-90f0-c28b2a3a6aa9 read from:: SingleNode METADATA: - created by::::::::::: test suite - created at::::::::::: - comment:::::::::::::: (none) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: test suite + created at::::::::::::: + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 0 diff --git a/dev-tools/reconfigurator-cli/tests/output/cmds-expunge-newly-added-stdout b/dev-tools/reconfigurator-cli/tests/output/cmds-expunge-newly-added-stdout index 8895083a72b..670e341ff04 100644 --- a/dev-tools/reconfigurator-cli/tests/output/cmds-expunge-newly-added-stdout +++ b/dev-tools/reconfigurator-cli/tests/output/cmds-expunge-newly-added-stdout @@ -301,11 +301,12 @@ parent: 06c88262-f435-410e-ba98-101bed41ec27 read from:: SingleNode METADATA: - created by::::::::::: test suite - created at::::::::::: - comment:::::::::::::: (none) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: test suite + created at::::::::::::: + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 0 @@ -608,11 +609,12 @@ parent: 3f00b694-1b16-4aaa-8f78-e6b3a527b434 read from:: SingleNode METADATA: - created by::::::::::: reconfigurator-cli - created at::::::::::: - comment:::::::::::::: (none) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: reconfigurator-cli + created at::::::::::::: + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 0 @@ -929,11 +931,12 @@ parent: 366b0b68-d80e-4bc1-abd3-dc69837847e0 read from:: SingleNode METADATA: - created by::::::::::: reconfigurator-sim - created at::::::::::: - comment:::::::::::::: (none) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: reconfigurator-sim + created at::::::::::::: + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 0 diff --git a/dev-tools/reconfigurator-cli/tests/output/cmds-set-mgs-updates-stdout b/dev-tools/reconfigurator-cli/tests/output/cmds-set-mgs-updates-stdout index 2601536d2f8..334769e3bd5 100644 --- a/dev-tools/reconfigurator-cli/tests/output/cmds-set-mgs-updates-stdout +++ b/dev-tools/reconfigurator-cli/tests/output/cmds-set-mgs-updates-stdout @@ -175,11 +175,12 @@ parent: 6ccc786b-17f1-4562-958f-5a7d9a5a15fd read from:: SingleNode METADATA: - created by::::::::::: test suite - created at::::::::::: - comment:::::::::::::: (none) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: test suite + created at::::::::::::: + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 0 @@ -358,11 +359,12 @@ parent: ad97e762-7bf1-45a6-a98f-60afb7e491c0 read from:: SingleNode METADATA: - created by::::::::::: reconfigurator-cli - created at::::::::::: - comment:::::::::::::: (none) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: reconfigurator-cli + created at::::::::::::: + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 1 Pending MGS-managed updates (all baseboards): @@ -537,8 +539,9 @@ to: blueprint cca24b71-09b5-4042-9185-b33e9f2ebba0 cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) @@ -836,8 +839,9 @@ to: blueprint ad97e762-7bf1-45a6-a98f-60afb7e491c0 cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) @@ -1145,11 +1149,12 @@ parent: cca24b71-09b5-4042-9185-b33e9f2ebba0 read from:: SingleNode METADATA: - created by::::::::::: reconfigurator-cli - created at::::::::::: - comment:::::::::::::: (none) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: reconfigurator-cli + created at::::::::::::: + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 1 Pending MGS-managed updates (all baseboards): @@ -1324,8 +1329,9 @@ to: blueprint 5bf974f3-81f9-455b-b24e-3099f765664c cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) @@ -1624,8 +1630,9 @@ to: blueprint cca24b71-09b5-4042-9185-b33e9f2ebba0 cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) @@ -1935,11 +1942,12 @@ parent: 5bf974f3-81f9-455b-b24e-3099f765664c read from:: SingleNode METADATA: - created by::::::::::: reconfigurator-cli - created at::::::::::: - comment:::::::::::::: (none) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: reconfigurator-cli + created at::::::::::::: + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 2 Pending MGS-managed updates (all baseboards): @@ -2115,8 +2123,9 @@ to: blueprint 1b837a27-3be1-4fcb-8499-a921c839e1d0 cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) @@ -2423,11 +2432,12 @@ parent: 1b837a27-3be1-4fcb-8499-a921c839e1d0 read from:: SingleNode METADATA: - created by::::::::::: reconfigurator-cli - created at::::::::::: - comment:::::::::::::: (none) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: reconfigurator-cli + created at::::::::::::: + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 1 Pending MGS-managed updates (all baseboards): @@ -2602,8 +2612,9 @@ to: blueprint 3682a71b-c6ca-4b7e-8f84-16df80c85960 cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) @@ -2745,15 +2756,27 @@ edit contents of a blueprint directly Usage: blueprint-edit [OPTIONS] Commands: - add-nexus add a Nexus instance to a particular sled - add-cockroach add a CockroachDB instance to a particular sled - set-zone-image set the image source for a zone - set-remove-mupdate-override set the remove_mupdate_override field for a sled - expunge-zone expunge a zone - set-sp-update configure an SP update - delete-sp-update delete a configured SP update - debug debug commands that bypass normal checks - help Print this message or the help of the given subcommand(s) + add-nexus + add a Nexus instance to a particular sled + add-cockroach + add a CockroachDB instance to a particular sled + set-zone-image + set the image source for a zone + set-remove-mupdate-override + set the remove_mupdate_override field for a sled + set-target-release-minimum-generation + set the minimum generation for which target releases are accepted [aliases: + set-target-release-min-gen] + expunge-zone + expunge a zone + set-sp-update + configure an SP update + delete-sp-update + delete a configured SP update + debug + debug commands that bypass normal checks + help + Print this message or the help of the given subcommand(s) Arguments: id of the blueprint to edit, "latest", or "target" diff --git a/dev-tools/reconfigurator-cli/tests/output/cmds-set-remove-mupdate-override-stdout b/dev-tools/reconfigurator-cli/tests/output/cmds-set-remove-mupdate-override-stdout index 0975375624b..e62a155f933 100644 --- a/dev-tools/reconfigurator-cli/tests/output/cmds-set-remove-mupdate-override-stdout +++ b/dev-tools/reconfigurator-cli/tests/output/cmds-set-remove-mupdate-override-stdout @@ -202,11 +202,12 @@ parent: df06bb57-ad42-4431-9206-abff322896c7 read from:: SingleNode METADATA: - created by::::::::::: reconfigurator-cli - created at::::::::::: - comment:::::::::::::: (none) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: reconfigurator-cli + created at::::::::::::: + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 0 @@ -252,7 +253,7 @@ to: blueprint afb09faf-a586-4483-9289-04d4f1d8ba23 sled 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c (active, config generation 1): sled aff6c093-197d-42c5-ad80-9f10ba051a34 (active, config generation 2): - will remove mupdate override: 00000000-0000-0000-0000-000000000000 + will remove mupdate override: 00000000-0000-0000-0000-000000000000 (unchanged) REMOVED SLEDS: @@ -283,8 +284,258 @@ to: blueprint afb09faf-a586-4483-9289-04d4f1d8ba23 cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) + + OXIMETER SETTINGS: + generation: 1 (unchanged) + read from:: SingleNode (unchanged) + + +internal DNS: + DNS zone: "control-plane.oxide.internal" (unchanged) + name: 00320471-945d-413c-85e7-03e091a70b3c.sled (records: 1) + AAAA fd00:1122:3344:108::1 + name: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c.sled (records: 1) + AAAA fd00:1122:3344:102::1 + name: 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6.sled (records: 1) + AAAA fd00:1122:3344:101::1 + name: 9a867dc9-d505-427f-9eff-cdb1d4d9bd73.sled (records: 1) + AAAA fd00:1122:3344:106::1 + name: _repo-depot._tcp (records: 7) + SRV port 12348 00320471-945d-413c-85e7-03e091a70b3c.sled.control-plane.oxide.internal + SRV port 12348 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c.sled.control-plane.oxide.internal + SRV port 12348 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6.sled.control-plane.oxide.internal + SRV port 12348 9a867dc9-d505-427f-9eff-cdb1d4d9bd73.sled.control-plane.oxide.internal + SRV port 12348 aff6c093-197d-42c5-ad80-9f10ba051a34.sled.control-plane.oxide.internal + SRV port 12348 b82ede02-399c-48c6-a1de-411df4fa49a7.sled.control-plane.oxide.internal + SRV port 12348 d81c6a84-79b8-4958-ae41-ea46c9b19763.sled.control-plane.oxide.internal + name: aff6c093-197d-42c5-ad80-9f10ba051a34.sled (records: 1) + AAAA fd00:1122:3344:104::1 + name: b82ede02-399c-48c6-a1de-411df4fa49a7.sled (records: 1) + AAAA fd00:1122:3344:105::1 + name: d81c6a84-79b8-4958-ae41-ea46c9b19763.sled (records: 1) + AAAA fd00:1122:3344:103::1 + +external DNS: + DNS zone: "oxide.example" (unchanged) + name: example-silo.sys (records: 0) + + + + +> # Set the target release minimum generation for the blueprint. +> blueprint-edit latest set-target-release-min-gen 2 +blueprint ce365dff-2cdb-4f35-a186-b15e20e1e700 created from latest blueprint (afb09faf-a586-4483-9289-04d4f1d8ba23): set target release minimum generation to 2 + +> blueprint-show latest +blueprint ce365dff-2cdb-4f35-a186-b15e20e1e700 +parent: afb09faf-a586-4483-9289-04d4f1d8ba23 + + sled: 00320471-945d-413c-85e7-03e091a70b3c (active, config generation 2) + will remove mupdate override: ffffffff-ffff-ffff-ffff-ffffffffffff + + physical disks: + ------------------------------------- + vendor model serial disposition + ------------------------------------- + + + datasets: + --------------------------------------------------------------------------- + dataset name dataset id disposition quota reservation compression + --------------------------------------------------------------------------- + + + omicron zones: + -------------------------------------------------------------- + zone type zone id image source disposition underlay IP + -------------------------------------------------------------- + + + + sled: 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c (active, config generation 1) + + physical disks: + ------------------------------------- + vendor model serial disposition + ------------------------------------- + + + datasets: + --------------------------------------------------------------------------- + dataset name dataset id disposition quota reservation compression + --------------------------------------------------------------------------- + + + omicron zones: + -------------------------------------------------------------- + zone type zone id image source disposition underlay IP + -------------------------------------------------------------- + + + + sled: 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6 (active, config generation 2) + will remove mupdate override: ffffffff-ffff-ffff-ffff-ffffffffffff + + physical disks: + ------------------------------------- + vendor model serial disposition + ------------------------------------- + + + datasets: + --------------------------------------------------------------------------- + dataset name dataset id disposition quota reservation compression + --------------------------------------------------------------------------- + + + omicron zones: + -------------------------------------------------------------- + zone type zone id image source disposition underlay IP + -------------------------------------------------------------- + + + + sled: 9a867dc9-d505-427f-9eff-cdb1d4d9bd73 (active, config generation 3) + + physical disks: + ------------------------------------- + vendor model serial disposition + ------------------------------------- + + + datasets: + --------------------------------------------------------------------------- + dataset name dataset id disposition quota reservation compression + --------------------------------------------------------------------------- + + + omicron zones: + -------------------------------------------------------------- + zone type zone id image source disposition underlay IP + -------------------------------------------------------------- + + + + sled: aff6c093-197d-42c5-ad80-9f10ba051a34 (active, config generation 2) + will remove mupdate override: 00000000-0000-0000-0000-000000000000 + + physical disks: + ------------------------------------- + vendor model serial disposition + ------------------------------------- + + + datasets: + --------------------------------------------------------------------------- + dataset name dataset id disposition quota reservation compression + --------------------------------------------------------------------------- + + + omicron zones: + -------------------------------------------------------------- + zone type zone id image source disposition underlay IP + -------------------------------------------------------------- + + + + sled: b82ede02-399c-48c6-a1de-411df4fa49a7 (active, config generation 3) + will remove mupdate override: ffffffff-ffff-ffff-ffff-ffffffffffff + + physical disks: + ------------------------------------- + vendor model serial disposition + ------------------------------------- + + + datasets: + --------------------------------------------------------------------------- + dataset name dataset id disposition quota reservation compression + --------------------------------------------------------------------------- + + + omicron zones: + -------------------------------------------------------------- + zone type zone id image source disposition underlay IP + -------------------------------------------------------------- + + + + sled: d81c6a84-79b8-4958-ae41-ea46c9b19763 (active, config generation 3) + will remove mupdate override: 00000000-0000-0000-0000-000000000000 + + physical disks: + ------------------------------------- + vendor model serial disposition + ------------------------------------- + + + datasets: + --------------------------------------------------------------------------- + dataset name dataset id disposition quota reservation compression + --------------------------------------------------------------------------- + + + omicron zones: + -------------------------------------------------------------- + zone type zone id image source disposition underlay IP + -------------------------------------------------------------- + + + COCKROACHDB SETTINGS: + state fingerprint::::::::::::::::: (none) + cluster.preserve_downgrade_option: (do not modify) + + OXIMETER SETTINGS: + generation: 1 + read from:: SingleNode + + METADATA: + created by::::::::::::: reconfigurator-cli + created at::::::::::::: + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 2 + + PENDING MGS-MANAGED UPDATES: 0 + + +> blueprint-diff afb09faf-a586-4483-9289-04d4f1d8ba23 latest +from: blueprint afb09faf-a586-4483-9289-04d4f1d8ba23 +to: blueprint ce365dff-2cdb-4f35-a186-b15e20e1e700 + + UNCHANGED SLEDS: + + sled 00320471-945d-413c-85e7-03e091a70b3c (active, config generation 2): + will remove mupdate override: ffffffff-ffff-ffff-ffff-ffffffffffff (unchanged) + + sled 2b8f0cb3-0295-4b3c-bc58-4fe88b57112c (active, config generation 1): + + sled 98e6b7c2-2efa-41ca-b20a-0a4d61102fe6 (active, config generation 2): + will remove mupdate override: ffffffff-ffff-ffff-ffff-ffffffffffff (unchanged) + + sled 9a867dc9-d505-427f-9eff-cdb1d4d9bd73 (active, config generation 3): + + sled aff6c093-197d-42c5-ad80-9f10ba051a34 (active, config generation 2): + will remove mupdate override: 00000000-0000-0000-0000-000000000000 (unchanged) + + sled b82ede02-399c-48c6-a1de-411df4fa49a7 (active, config generation 3): + will remove mupdate override: ffffffff-ffff-ffff-ffff-ffffffffffff (unchanged) + + sled d81c6a84-79b8-4958-ae41-ea46c9b19763 (active, config generation 3): + will remove mupdate override: 00000000-0000-0000-0000-000000000000 (unchanged) + + COCKROACHDB SETTINGS: + state fingerprint::::::::::::::::: (none) (unchanged) + cluster.preserve_downgrade_option: (do not modify) (unchanged) + + METADATA: + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) +* target release min gen: 1 -> 2 OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/dev-tools/reconfigurator-cli/tests/output/cmds-set-zone-images-stdout b/dev-tools/reconfigurator-cli/tests/output/cmds-set-zone-images-stdout index d62c17123a9..e10d2fb7183 100644 --- a/dev-tools/reconfigurator-cli/tests/output/cmds-set-zone-images-stdout +++ b/dev-tools/reconfigurator-cli/tests/output/cmds-set-zone-images-stdout @@ -93,11 +93,12 @@ parent: 1b013011-2062-4b48-b544-a32b23bce83a read from:: SingleNode METADATA: - created by::::::::::: test suite - created at::::::::::: - comment:::::::::::::: (none) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: test suite + created at::::::::::::: + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 0 @@ -199,11 +200,12 @@ parent: 9766ca20-38d4-4380-b005-e7c43c797e7c read from:: SingleNode METADATA: - created by::::::::::: reconfigurator-cli - created at::::::::::: - comment:::::::::::::: (none) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: reconfigurator-cli + created at::::::::::::: + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 0 @@ -294,8 +296,9 @@ to: blueprint f714e6ea-e85a-4d7d-93c2-a018744fe176 cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) @@ -480,11 +483,12 @@ parent: bb128f06-a2e1-44c1-8874-4f789d0ff896 read from:: SingleNode METADATA: - created by::::::::::: reconfigurator-cli - created at::::::::::: - comment:::::::::::::: (none) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: reconfigurator-cli + created at::::::::::::: + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 0 @@ -575,8 +579,9 @@ to: blueprint d9c572a1-a68c-4945-b1ec-5389bd588fe9 cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) @@ -673,15 +678,27 @@ edit contents of a blueprint directly Usage: blueprint-edit [OPTIONS] Commands: - add-nexus add a Nexus instance to a particular sled - add-cockroach add a CockroachDB instance to a particular sled - set-zone-image set the image source for a zone - set-remove-mupdate-override set the remove_mupdate_override field for a sled - expunge-zone expunge a zone - set-sp-update configure an SP update - delete-sp-update delete a configured SP update - debug debug commands that bypass normal checks - help Print this message or the help of the given subcommand(s) + add-nexus + add a Nexus instance to a particular sled + add-cockroach + add a CockroachDB instance to a particular sled + set-zone-image + set the image source for a zone + set-remove-mupdate-override + set the remove_mupdate_override field for a sled + set-target-release-minimum-generation + set the minimum generation for which target releases are accepted [aliases: + set-target-release-min-gen] + expunge-zone + expunge a zone + set-sp-update + configure an SP update + delete-sp-update + delete a configured SP update + debug + debug commands that bypass normal checks + help + Print this message or the help of the given subcommand(s) Arguments: id of the blueprint to edit, "latest", or "target" diff --git a/illumos-utils/src/running_zone.rs b/illumos-utils/src/running_zone.rs index 0ebf078b3f5..182910a9354 100644 --- a/illumos-utils/src/running_zone.rs +++ b/illumos-utils/src/running_zone.rs @@ -1016,7 +1016,7 @@ impl ZoneBuilderFactory { ) -> Self { let temp_dir = match temp_dir { Some(dir) => Utf8PathBuf::from(dir), - None => Utf8TempDir::new().unwrap().into_path(), + None => Utf8TempDir::new().unwrap().keep(), }; Self { fake_cfg: Some(FakeZoneBuilderConfig { diff --git a/illumos-utils/src/zfs.rs b/illumos-utils/src/zfs.rs index 1795f5846bc..f28c3372280 100644 --- a/illumos-utils/src/zfs.rs +++ b/illumos-utils/src/zfs.rs @@ -704,7 +704,7 @@ fn ensure_mountpoint_empty(path: &Utf8Path) -> Result<(), MountpointError> { let prefix = format!("{MOUNTPOINT_TRANSFER_PREFIX}{file}-"); let destination_dir = Utf8TempDir::with_prefix_in(prefix, parent) .map_err(|err| MountpointError::CreateTransferDirectory(err))? - .into_path(); + .keep(); let entries = path.read_dir_utf8().map_err(|err| MountpointError::Readdir(err))?; diff --git a/installinator/Cargo.toml b/installinator/Cargo.toml index 68bf11b584d..5d524fd5826 100644 --- a/installinator/Cargo.toml +++ b/installinator/Cargo.toml @@ -21,6 +21,7 @@ display-error-chain.workspace = true futures.workspace = true hex.workspace = true http.workspace = true +iddqd.workspace = true illumos-utils.workspace = true installinator-client.workspace = true installinator-common.workspace = true diff --git a/installinator/src/write.rs b/installinator/src/write.rs index 84b88a5cb8a..e6a0860d608 100644 --- a/installinator/src/write.rs +++ b/installinator/src/write.rs @@ -12,21 +12,26 @@ use std::{ use anyhow::{Context, Result, anyhow, ensure}; use async_trait::async_trait; use buf_list::BufList; -use bytes::Buf; +use bytes::{Buf, Bytes}; use camino::{Utf8Path, Utf8PathBuf}; +use iddqd::IdOrdMap; use illumos_utils::zpool::{Zpool, ZpoolName}; use installinator_common::{ ControlPlaneZonesSpec, ControlPlaneZonesStepId, RawDiskWriter, StepContext, StepProgress, StepResult, StepSuccess, UpdateEngine, WriteComponent, WriteError, WriteOutput, WriteSpec, WriteStepId, }; -use omicron_common::{disk::M2Slot, update::MupdateOverrideInfo}; +use omicron_common::{ + disk::M2Slot, + update::{MupdateOverrideInfo, MupdateOverrideZone}, +}; use omicron_uuid_kinds::MupdateOverrideUuid; use sha2::{Digest, Sha256}; use slog::{Logger, info, warn}; use tokio::{ fs::File, io::{AsyncWrite, AsyncWriteExt}, + task::JoinSet, }; use tufaceous_artifact::{ArtifactHash, ArtifactHashId}; use tufaceous_lib::ControlPlaneZoneImages; @@ -56,7 +61,8 @@ struct ArtifactDestination { impl ArtifactDestination { fn from_directory(dir: &Utf8Path) -> Result { - let control_plane_dir = dir.join("zones"); + // The install dataset goes into a directory called "install". + let control_plane_dir = dir.join("install"); std::fs::create_dir_all(&control_plane_dir) .with_context(|| format!("error creating directories at {dir}"))?; @@ -678,7 +684,7 @@ impl ControlPlaneZoneWriteContext<'_> { async move |cx| { let transport = transport.into_value(cx.token()).await; let mupdate_json = - self.mupdate_override_artifact(mupdate_uuid); + self.mupdate_override_artifact(mupdate_uuid).await; let out_path = self .output_directory @@ -759,23 +765,63 @@ impl ControlPlaneZoneWriteContext<'_> { .register(); } - fn mupdate_override_artifact( + async fn mupdate_override_artifact( &self, mupdate_uuid: MupdateOverrideUuid, ) -> BufList { - // Might be worth writing out individual hash IDs for each zone in the - // future. let hash_ids = [self.host_phase_2_id.clone(), self.control_plane_id.clone()] .into_iter() .collect(); - let mupdate_override = MupdateOverrideInfo { mupdate_uuid, hash_ids }; + let zones = compute_zone_hashes(&self.zones).await; + + let mupdate_override = + MupdateOverrideInfo { mupdate_uuid, hash_ids, zones }; let json_bytes = serde_json::to_vec(&mupdate_override) .expect("this serialization is infallible"); BufList::from(json_bytes) } } +/// Computes the zone hash IDs. +/// +/// Hash computation is done in parallel on blocking tasks. +/// +/// # Panics +/// +/// Panics if the runtime shuts down causing a task abort, or a task panics. +async fn compute_zone_hashes( + images: &ControlPlaneZoneImages, +) -> IdOrdMap { + let mut tasks = JoinSet::new(); + for (file_name, data) in &images.zones { + let file_name = file_name.clone(); + // data is a Bytes so is cheap to clone. + let data: Bytes = data.clone(); + // Compute hashes in parallel. + tasks.spawn_blocking(move || { + let mut hasher = Sha256::new(); + hasher.update(&data); + let hash = hasher.finalize(); + MupdateOverrideZone { + file_name, + file_size: u64::try_from(data.len()).unwrap(), + hash: ArtifactHash(hash.into()), + } + }); + } + + let mut output = IdOrdMap::new(); + while let Some(res) = tasks.join_next().await { + // Propagate panics across tasks—this is the standard pattern we follow + // in installinator. + output + .insert_unique(res.expect("task panicked")) + .expect("filenames are unique"); + } + output +} + fn remove_contents_of(path: &Utf8Path) -> io::Result<()> { use std::fs; diff --git a/internal-dns/resolver/src/resolver.rs b/internal-dns/resolver/src/resolver.rs index 016632c47e2..8c550ee435f 100644 --- a/internal-dns/resolver/src/resolver.rs +++ b/internal-dns/resolver/src/resolver.rs @@ -528,7 +528,7 @@ mod test { if !self.successful { // If we didn't explicitly succeed, then we want to keep the // temporary directory around. - let _ = self.storage_path.take().unwrap().into_path(); + let _ = self.storage_path.take().unwrap().keep(); } } } diff --git a/nexus/db-model/src/deployment.rs b/nexus/db-model/src/deployment.rs index b427ba6cc20..b89a2b09de6 100644 --- a/nexus/db-model/src/deployment.rs +++ b/nexus/db-model/src/deployment.rs @@ -65,6 +65,7 @@ pub struct Blueprint { pub time_created: DateTime, pub creator: String, pub comment: String, + pub target_release_minimum_generation: Generation, } impl From<&'_ nexus_types::deployment::Blueprint> for Blueprint { @@ -81,6 +82,9 @@ impl From<&'_ nexus_types::deployment::Blueprint> for Blueprint { time_created: bp.time_created, creator: bp.creator.clone(), comment: bp.comment.clone(), + target_release_minimum_generation: Generation( + bp.target_release_minimum_generation, + ), } } } @@ -92,6 +96,8 @@ impl From for nexus_types::deployment::BlueprintMetadata { parent_blueprint_id: value.parent_blueprint_id.map(From::from), internal_dns_version: *value.internal_dns_version, external_dns_version: *value.external_dns_version, + target_release_minimum_generation: *value + .target_release_minimum_generation, cockroachdb_fingerprint: value.cockroachdb_fingerprint, cockroachdb_setting_preserve_downgrade: CockroachDbPreserveDowngrade::from_optional_string( diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 3a787f4e141..00abc282a60 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.rs @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock}; /// /// This must be updated when you change the database schema. Refer to /// schema/crdb/README.adoc in the root of this repository for details. -pub const SCHEMA_VERSION: Version = Version::new(144, 0, 0); +pub const SCHEMA_VERSION: Version = Version::new(145, 0, 0); /// List of all past database schema versions, in *reverse* order /// @@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock> = LazyLock::new(|| { // | leaving the first copy as an example for the next person. // v // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"), + KnownVersion::new(145, "bp-add-target-release-min-gen"), KnownVersion::new(144, "inventory-omicron-sled-config"), KnownVersion::new(143, "alerts-renamening"), KnownVersion::new(142, "bp-add-remove-mupdate-override"), diff --git a/nexus/db-queries/src/db/datastore/deployment.rs b/nexus/db-queries/src/db/datastore/deployment.rs index 1df74355b8f..9e2a6d12834 100644 --- a/nexus/db-queries/src/db/datastore/deployment.rs +++ b/nexus/db-queries/src/db/datastore/deployment.rs @@ -442,6 +442,7 @@ impl DataStore { parent_blueprint_id, internal_dns_version, external_dns_version, + target_release_minimum_generation, cockroachdb_fingerprint, cockroachdb_setting_preserve_downgrade, time_created, @@ -467,6 +468,7 @@ impl DataStore { blueprint.parent_blueprint_id.map(From::from), *blueprint.internal_dns_version, *blueprint.external_dns_version, + *blueprint.target_release_minimum_generation, blueprint.cockroachdb_fingerprint, blueprint.cockroachdb_setting_preserve_downgrade, blueprint.time_created, @@ -949,6 +951,7 @@ impl DataStore { parent_blueprint_id, internal_dns_version, external_dns_version, + target_release_minimum_generation, cockroachdb_fingerprint, cockroachdb_setting_preserve_downgrade, clickhouse_cluster_config, diff --git a/nexus/db-queries/src/db/datastore/rack.rs b/nexus/db-queries/src/db/datastore/rack.rs index 27c68cfdcef..a121c51e043 100644 --- a/nexus/db-queries/src/db/datastore/rack.rs +++ b/nexus/db-queries/src/db/datastore/rack.rs @@ -1061,6 +1061,7 @@ mod test { parent_blueprint_id: None, internal_dns_version: *Generation::new(), external_dns_version: *Generation::new(), + target_release_minimum_generation: *Generation::new(), cockroachdb_fingerprint: String::new(), clickhouse_cluster_config: None, oximeter_read_version: *Generation::new(), @@ -1550,6 +1551,7 @@ mod test { parent_blueprint_id: None, internal_dns_version: *Generation::new(), external_dns_version: *Generation::new(), + target_release_minimum_generation: *Generation::new(), cockroachdb_fingerprint: String::new(), clickhouse_cluster_config: None, oximeter_read_version: *Generation::new(), @@ -1810,6 +1812,7 @@ mod test { parent_blueprint_id: None, internal_dns_version: *Generation::new(), external_dns_version: *Generation::new(), + target_release_minimum_generation: *Generation::new(), cockroachdb_fingerprint: String::new(), clickhouse_cluster_config: None, oximeter_read_version: *Generation::new(), @@ -2019,6 +2022,7 @@ mod test { parent_blueprint_id: None, internal_dns_version: *Generation::new(), external_dns_version: *Generation::new(), + target_release_minimum_generation: *Generation::new(), cockroachdb_fingerprint: String::new(), clickhouse_cluster_config: None, oximeter_read_version: *Generation::new(), @@ -2158,6 +2162,7 @@ mod test { parent_blueprint_id: None, internal_dns_version: *Generation::new(), external_dns_version: *Generation::new(), + target_release_minimum_generation: *Generation::new(), cockroachdb_fingerprint: String::new(), clickhouse_cluster_config: None, oximeter_read_version: *Generation::new(), diff --git a/nexus/db-schema/src/schema.rs b/nexus/db-schema/src/schema.rs index 9875d3bc395..356f3132bab 100644 --- a/nexus/db-schema/src/schema.rs +++ b/nexus/db-schema/src/schema.rs @@ -1791,6 +1791,8 @@ table! { cockroachdb_fingerprint -> Text, cockroachdb_setting_preserve_downgrade -> Nullable, + + target_release_minimum_generation -> Int8, } } diff --git a/nexus/reconfigurator/cli-integration-tests/tests/integration/blueprint_edit.rs b/nexus/reconfigurator/cli-integration-tests/tests/integration/blueprint_edit.rs index cb02e01f48b..fbfed8169eb 100644 --- a/nexus/reconfigurator/cli-integration-tests/tests/integration/blueprint_edit.rs +++ b/nexus/reconfigurator/cli-integration-tests/tests/integration/blueprint_edit.rs @@ -72,7 +72,7 @@ async fn test_blueprint_edit(cptestctx: &ControlPlaneTestContext) { let tmpdir = camino_tempfile::tempdir().expect("failed to create tmpdir"); // Save the path and prevent the temporary directory from being cleaned up // automatically. We want to be preserve the contents if this test fails. - let tmpdir_path = tmpdir.into_path(); + let tmpdir_path = tmpdir.keep(); let saved_state1_path = tmpdir_path.join("reconfigurator-state1.json"); let saved_state2_path = tmpdir_path.join("reconfigurator-state2.json"); let script1_path = tmpdir_path.join("cmds1"); diff --git a/nexus/reconfigurator/execution/src/dns.rs b/nexus/reconfigurator/execution/src/dns.rs index 7d3b8187a2a..e62ba48e9e6 100644 --- a/nexus/reconfigurator/execution/src/dns.rs +++ b/nexus/reconfigurator/execution/src/dns.rs @@ -700,6 +700,7 @@ mod test { parent_blueprint_id: None, internal_dns_version: initial_dns_generation, external_dns_version: Generation::new(), + target_release_minimum_generation: Generation::new(), cockroachdb_fingerprint: String::new(), clickhouse_cluster_config: None, oximeter_read_version: Generation::new(), diff --git a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs index 7822d0cfed8..473ce571bd3 100644 --- a/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs +++ b/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs @@ -124,6 +124,14 @@ pub enum Error { AllocateExternalNetworking(#[from] ExternalNetworkingError), #[error("can only have {INTERNAL_DNS_REDUNDANCY} internal DNS servers")] PolicySpecifiesTooManyInternalDnsServers, + #[error( + "mismatch while setting target_release_minimum_generation, \ + expected current value is {expected} but actual value is {actual}" + )] + TargetReleaseMinimumGenerationMismatch { + expected: Generation, + actual: Generation, + }, } /// Describes the result of an idempotent "ensure" operation @@ -404,6 +412,7 @@ pub struct BlueprintBuilder<'a> { // corresponding fields in `Blueprint`. sled_editors: BTreeMap, cockroachdb_setting_preserve_downgrade: CockroachDbPreserveDowngrade, + target_release_minimum_generation: Generation, creator: String, operations: Vec, @@ -465,6 +474,7 @@ impl<'a> BlueprintBuilder<'a> { parent_blueprint_id: None, internal_dns_version: Generation::new(), external_dns_version: Generation::new(), + target_release_minimum_generation: Generation::new(), cockroachdb_fingerprint: String::new(), cockroachdb_setting_preserve_downgrade: CockroachDbPreserveDowngrade::DoNotModify, @@ -541,6 +551,8 @@ impl<'a> BlueprintBuilder<'a> { cockroachdb_setting_preserve_downgrade: parent_blueprint .cockroachdb_setting_preserve_downgrade, pending_mgs_updates: parent_blueprint.pending_mgs_updates.clone(), + target_release_minimum_generation: parent_blueprint + .target_release_minimum_generation, creator: creator.to_owned(), operations: Vec::new(), comments: Vec::new(), @@ -709,6 +721,8 @@ impl<'a> BlueprintBuilder<'a> { parent_blueprint_id: Some(self.parent_blueprint.id), internal_dns_version: self.input.internal_dns_version(), external_dns_version: self.input.external_dns_version(), + target_release_minimum_generation: self + .target_release_minimum_generation, cockroachdb_fingerprint: self .input .cockroachdb_settings() @@ -716,6 +730,7 @@ impl<'a> BlueprintBuilder<'a> { .clone(), cockroachdb_setting_preserve_downgrade: self .cockroachdb_setting_preserve_downgrade, + clickhouse_cluster_config, oximeter_read_version: oximeter_read_version.into(), oximeter_read_mode, @@ -1880,6 +1895,24 @@ impl<'a> BlueprintBuilder<'a> { .len() } + /// Given the current value of `target_release_minimum_generation`, set the + /// new value for this blueprint. + pub fn set_target_release_minimum_generation( + &mut self, + current: Generation, + target_release_minimum_generation: Generation, + ) -> Result<(), Error> { + if self.target_release_minimum_generation != current { + return Err(Error::TargetReleaseMinimumGenerationMismatch { + expected: current, + actual: self.target_release_minimum_generation, + }); + } + self.target_release_minimum_generation = + target_release_minimum_generation; + Ok(()) + } + /// Allow a test to manually add an external DNS address, which could /// ordinarily only come from RSS. /// diff --git a/nexus/reconfigurator/planning/tests/output/example_builder_zone_counts_blueprint.txt b/nexus/reconfigurator/planning/tests/output/example_builder_zone_counts_blueprint.txt index d6ae5b36df1..931ba7a7208 100644 --- a/nexus/reconfigurator/planning/tests/output/example_builder_zone_counts_blueprint.txt +++ b/nexus/reconfigurator/planning/tests/output/example_builder_zone_counts_blueprint.txt @@ -485,10 +485,11 @@ parent: e35b2fdd-354d-48d9-acb5-703b2c269a54 read from:: SingleNode METADATA: - created by::::::::::: test suite - created at::::::::::: 1970-01-01T00:00:00.000Z - comment:::::::::::::: (none) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: test suite + created at::::::::::::: 1970-01-01T00:00:00.000Z + comment:::::::::::::::: (none) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 0 diff --git a/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_2_3.txt b/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_2_3.txt index 90141f22aca..b61012a9cb5 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_2_3.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_2_3.txt @@ -332,8 +332,9 @@ to: blueprint 4171ad05-89dd-474b-846b-b007e4346366 cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_3_5.txt b/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_3_5.txt index f07fefe1136..6cad4b16c80 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_3_5.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_basic_add_sled_3_5.txt @@ -362,8 +362,9 @@ to: blueprint f432fcd5-1284-4058-8b4a-9286a3de6163 cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_dataset_settings_modified_in_place_1_2.txt b/nexus/reconfigurator/planning/tests/output/planner_dataset_settings_modified_in_place_1_2.txt index b94ef7fed7c..e7f0cedcbfd 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_dataset_settings_modified_in_place_1_2.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_dataset_settings_modified_in_place_1_2.txt @@ -111,8 +111,9 @@ to: blueprint fe13be30-94c2-4fa6-aad5-ae3c5028f6bb cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_1_2.txt b/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_1_2.txt index b47cfd4b326..ebee2b5d256 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_1_2.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_1_2.txt @@ -360,8 +360,9 @@ to: blueprint 1ac2d88f-27dd-4506-8585-6b2be832528e cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_bp2.txt b/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_bp2.txt index 75d2082e848..e9205152ba6 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_bp2.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_decommissions_sleds_bp2.txt @@ -292,10 +292,11 @@ parent: 516e80a3-b362-4fac-bd3c-4559717120dd read from:: SingleNode METADATA: - created by::::::::::: test_blueprint2 - created at::::::::::: 1970-01-01T00:00:00.000Z - comment:::::::::::::: sled a1b477db-b629-48eb-911d-1ccdafca75b9 expunged (expunged 10 disks, 47 datasets, 15 zones) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: test_blueprint2 + created at::::::::::::: 1970-01-01T00:00:00.000Z + comment:::::::::::::::: sled a1b477db-b629-48eb-911d-1ccdafca75b9 expunged (expunged 10 disks, 47 datasets, 15 zones) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 0 diff --git a/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_1_2.txt b/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_1_2.txt index cca404c11e5..340de055190 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_1_2.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_1_2.txt @@ -293,8 +293,9 @@ to: blueprint 31ef2071-2ec9-49d9-8827-fd83b17a0e3d cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_3_4.txt b/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_3_4.txt index d7d89d9c1e4..cc775c85ed0 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_3_4.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_3_4.txt @@ -293,8 +293,9 @@ to: blueprint 92fa943c-7dd4-48c3-9447-c9d0665744b6 cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_4_5.txt b/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_4_5.txt index 8670259cf21..5792aa9e1a1 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_4_5.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_4_5.txt @@ -301,8 +301,9 @@ to: blueprint 2886dab5-61a2-46b4-87af-bc7aeb44cccb cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_5_6.txt b/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_5_6.txt index 60e8d63eb6e..3a4dd93b303 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_5_6.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_deploy_all_keeper_nodes_5_6.txt @@ -299,8 +299,9 @@ to: blueprint cb39be9d-5476-44fa-9edf-9938376219ef cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_expunge_clickhouse_clusters_3_4.txt b/nexus/reconfigurator/planning/tests/output/planner_expunge_clickhouse_clusters_3_4.txt index bad6af55c5b..f3756df48d4 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_expunge_clickhouse_clusters_3_4.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_expunge_clickhouse_clusters_3_4.txt @@ -381,8 +381,9 @@ to: blueprint 74f2e7fd-687e-4c9e-b5d8-e474a5bb8e7c cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_expunge_clickhouse_clusters_5_6.txt b/nexus/reconfigurator/planning/tests/output/planner_expunge_clickhouse_clusters_5_6.txt index 8c4d907e67e..0669b9aa713 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_expunge_clickhouse_clusters_5_6.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_expunge_clickhouse_clusters_5_6.txt @@ -306,8 +306,9 @@ to: blueprint df68d4d4-5af4-4b56-95bb-1654a6957d4f cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_expunge_clickhouse_zones_after_policy_is_changed_3_4.txt b/nexus/reconfigurator/planning/tests/output/planner_expunge_clickhouse_zones_after_policy_is_changed_3_4.txt index f291512e2e1..8853a1133be 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_expunge_clickhouse_zones_after_policy_is_changed_3_4.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_expunge_clickhouse_zones_after_policy_is_changed_3_4.txt @@ -311,8 +311,9 @@ to: blueprint d895ef50-9978-454c-bdfb-b8dbe2c9a918 cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_1_2.txt b/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_1_2.txt index bc28aba9643..f3c6389a96f 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_1_2.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_1_2.txt @@ -529,8 +529,9 @@ to: blueprint 9f71f5d3-a272-4382-9154-6ea2e171a6c6 cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_2_2a.txt b/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_2_2a.txt index 5714f0cb2bd..404d249786a 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_2_2a.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_2_2a.txt @@ -523,8 +523,9 @@ mismatched zone type: after: InternalNtp( cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) -* external DNS version: 1 -> 2 + internal DNS version::: 1 (unchanged) +* external DNS version::: 1 -> 2 + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_bp2.txt b/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_bp2.txt index 19643dc4daa..c07415c2b70 100644 --- a/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_bp2.txt +++ b/nexus/reconfigurator/planning/tests/output/planner_nonprovisionable_bp2.txt @@ -464,10 +464,11 @@ parent: 4d4e6c38-cd95-4c4e-8f45-6af4d686964b read from:: SingleNode METADATA: - created by::::::::::: test_blueprint2 - created at::::::::::: 1970-01-01T00:00:00.000Z - comment:::::::::::::: sled 48d95fef-bc9f-4f50-9a53-1e075836291d expunged (expunged 10 disks, 45 datasets, 14 zones) - internal DNS version: 1 - external DNS version: 1 + created by::::::::::::: test_blueprint2 + created at::::::::::::: 1970-01-01T00:00:00.000Z + comment:::::::::::::::: sled 48d95fef-bc9f-4f50-9a53-1e075836291d expunged (expunged 10 disks, 45 datasets, 14 zones) + internal DNS version::: 1 + external DNS version::: 1 + target release min gen: 1 PENDING MGS-MANAGED UPDATES: 0 diff --git a/nexus/reconfigurator/planning/tests/output/zone_image_source_change_1.txt b/nexus/reconfigurator/planning/tests/output/zone_image_source_change_1.txt index 029ffcb291c..33702b2ec03 100644 --- a/nexus/reconfigurator/planning/tests/output/zone_image_source_change_1.txt +++ b/nexus/reconfigurator/planning/tests/output/zone_image_source_change_1.txt @@ -111,8 +111,9 @@ to: blueprint 665dc34a-dbf2-4d13-9ceb-9542d434ab0e cluster.preserve_downgrade_option: (do not modify) (unchanged) METADATA: - internal DNS version: 1 (unchanged) - external DNS version: 1 (unchanged) + internal DNS version::: 1 (unchanged) + external DNS version::: 1 (unchanged) + target release min gen: 1 (unchanged) OXIMETER SETTINGS: generation: 1 (unchanged) diff --git a/nexus/src/app/background/tasks/blueprint_execution.rs b/nexus/src/app/background/tasks/blueprint_execution.rs index 13afc57c80c..028dfc2d865 100644 --- a/nexus/src/app/background/tasks/blueprint_execution.rs +++ b/nexus/src/app/background/tasks/blueprint_execution.rs @@ -265,6 +265,7 @@ mod test { parent_blueprint_id: Some(current_target.target_id), internal_dns_version: dns_version, external_dns_version: dns_version, + target_release_minimum_generation: Generation::new(), cockroachdb_fingerprint: String::new(), clickhouse_cluster_config: None, oximeter_read_version: Generation::new(), diff --git a/nexus/src/app/background/tasks/blueprint_load.rs b/nexus/src/app/background/tasks/blueprint_load.rs index f3a6d4ff709..7f6f0aa5ba4 100644 --- a/nexus/src/app/background/tasks/blueprint_load.rs +++ b/nexus/src/app/background/tasks/blueprint_load.rs @@ -224,6 +224,7 @@ mod test { parent_blueprint_id: Some(parent_blueprint_id), internal_dns_version: Generation::new(), external_dns_version: Generation::new(), + target_release_minimum_generation: Generation::new(), cockroachdb_fingerprint: String::new(), clickhouse_cluster_config: None, oximeter_read_version: Generation::new(), diff --git a/nexus/test-utils/src/lib.rs b/nexus/test-utils/src/lib.rs index 62a0c807b90..d3257fc86d9 100644 --- a/nexus/test-utils/src/lib.rs +++ b/nexus/test-utils/src/lib.rs @@ -945,6 +945,7 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { parent_blueprint_id: None, internal_dns_version: dns_config.generation, external_dns_version: Generation::new(), + target_release_minimum_generation: Generation::new(), cockroachdb_fingerprint: String::new(), cockroachdb_setting_preserve_downgrade: CockroachDbPreserveDowngrade::DoNotModify, diff --git a/nexus/types/src/deployment.rs b/nexus/types/src/deployment.rs index 0145a06d0bb..a462f67cdef 100644 --- a/nexus/types/src/deployment.rs +++ b/nexus/types/src/deployment.rs @@ -174,6 +174,21 @@ pub struct Blueprint { // See blueprint execution for more on this. pub external_dns_version: Generation, + /// The minimum release generation to accept for target release + /// configuration. Target release configuration with a generation less than + /// this number will be ignored. + /// + /// For example, let's say that the current target release generation is 5. + /// Then, when reconfigurator detects a MUPdate: + /// + /// * the target release is ignored in favor of the install dataset + /// * this field is set to 6 + /// + /// Once the target release generation is updated to 6 or higher, + /// Reconfigurator knows that it is back in charge of driving the system to + /// the target release. + pub target_release_minimum_generation: Generation, + /// CockroachDB state fingerprint when this blueprint was created // See `nexus/db-queries/src/db/datastore/cockroachdb_settings.rs` for more // on this. @@ -215,6 +230,8 @@ impl Blueprint { parent_blueprint_id: self.parent_blueprint_id, internal_dns_version: self.internal_dns_version, external_dns_version: self.external_dns_version, + target_release_minimum_generation: self + .target_release_minimum_generation, cockroachdb_fingerprint: self.cockroachdb_fingerprint.clone(), cockroachdb_setting_preserve_downgrade: Some( self.cockroachdb_setting_preserve_downgrade, @@ -492,6 +509,12 @@ impl BlueprintDisplay<'_> { EXTERNAL_DNS_VERSION, self.blueprint.external_dns_version.to_string(), ), + ( + TARGET_RELEASE_MIN_GEN, + self.blueprint + .target_release_minimum_generation + .to_string(), + ), ], ) } @@ -531,8 +554,9 @@ impl fmt::Display for BlueprintDisplay<'_> { // Handled by `make_oximeter_table`, called below. oximeter_read_version: _, oximeter_read_mode: _, - // These five fields are handled by `make_metadata_table()`, called + // These six fields are handled by `make_metadata_table()`, called // below. + target_release_minimum_generation: _, internal_dns_version: _, external_dns_version: _, time_created: _, @@ -1664,6 +1688,10 @@ pub struct BlueprintMetadata { pub internal_dns_version: Generation, /// external DNS version when this blueprint was created pub external_dns_version: Generation, + /// The minimum generation for the target release. + /// + /// See [`Blueprint::target_release_minimum_generation`]. + pub target_release_minimum_generation: Generation, /// CockroachDB state fingerprint when this blueprint was created pub cockroachdb_fingerprint: String, /// Whether to set `cluster.preserve_downgrade_option` and what to set it to diff --git a/nexus/types/src/deployment/blueprint_diff.rs b/nexus/types/src/deployment/blueprint_diff.rs index 9a58076412b..22f4bf1b7a8 100644 --- a/nexus/types/src/deployment/blueprint_diff.rs +++ b/nexus/types/src/deployment/blueprint_diff.rs @@ -62,6 +62,7 @@ impl<'a> BlueprintDiffSummary<'a> { sleds, pending_mgs_updates, clickhouse_cluster_config, + target_release_minimum_generation, // Metadata fields for which changes don't reflect semantic // changes from one blueprint to the next. id: _, @@ -102,6 +103,13 @@ impl<'a> BlueprintDiffSummary<'a> { return true; } + // Did the target release minimum generation change? + if target_release_minimum_generation.before + != target_release_minimum_generation.after + { + return true; + } + // All fields checked or ignored; if we get here, there are no // meaningful changes. false @@ -1762,6 +1770,10 @@ impl<'diff> BlueprintDiffDisplay<'diff> { vec![ diff_row!(internal_dns_version, INTERNAL_DNS_VERSION), diff_row!(external_dns_version, EXTERNAL_DNS_VERSION), + diff_row!( + target_release_minimum_generation, + TARGET_RELEASE_MIN_GEN + ), ], ), ] @@ -1906,7 +1918,14 @@ impl fmt::Display for BlueprintDiffDisplay<'_> { let mut rows = Vec::new(); if let Some(id) = sled.remove_mupdate_override { - rows.push((WILL_REMOVE_MUPDATE_OVERRIDE, id.to_string())); + // For unchanged sleds, the tense of "will remove mupdate + // override" can be a bit confusing because it doesn't + // indicate that the value of this field has not changed. + // Add an "(unchanged)" at the end. + rows.push(( + WILL_REMOVE_MUPDATE_OVERRIDE, + format!("{id} {UNCHANGED_PARENS}"), + )); } let list = KvList::new_unchanged(None, rows); writeln!(f, "{list}")?; diff --git a/nexus/types/src/deployment/blueprint_display.rs b/nexus/types/src/deployment/blueprint_display.rs index 6209406c11e..599d1691647 100644 --- a/nexus/types/src/deployment/blueprint_display.rs +++ b/nexus/types/src/deployment/blueprint_display.rs @@ -42,6 +42,8 @@ pub mod constants { pub const CREATED_AT: &str = "created at"; pub const INTERNAL_DNS_VERSION: &str = "internal DNS version"; pub const EXTERNAL_DNS_VERSION: &str = "external DNS version"; + // Keep this a bit short to not make the key column too wide. + pub const TARGET_RELEASE_MIN_GEN: &str = "target release min gen"; pub const COMMENT: &str = "comment"; pub const UNCHANGED_PARENS: &str = "(unchanged)"; diff --git a/openapi/nexus-internal.json b/openapi/nexus-internal.json index 7e6a4e7acdc..25b3ac4acf2 100644 --- a/openapi/nexus-internal.json +++ b/openapi/nexus-internal.json @@ -2405,6 +2405,14 @@ "$ref": "#/components/schemas/BlueprintSledConfig" } }, + "target_release_minimum_generation": { + "description": "The minimum release generation to accept for target release configuration. Target release configuration with a generation less than this number will be ignored.\n\nFor example, let's say that the current target release generation is 5. Then, when reconfigurator detects a MUPdate:\n\n* the target release is ignored in favor of the install dataset * this field is set to 6\n\nOnce the target release generation is updated to 6 or higher, Reconfigurator knows that it is back in charge of driving the system to the target release.", + "allOf": [ + { + "$ref": "#/components/schemas/Generation" + } + ] + }, "time_created": { "description": "when this blueprint was generated (for debugging)", "type": "string", @@ -2423,6 +2431,7 @@ "oximeter_read_version", "pending_mgs_updates", "sleds", + "target_release_minimum_generation", "time_created" ] }, @@ -2551,6 +2560,14 @@ } ] }, + "target_release_minimum_generation": { + "description": "The minimum generation for the target release.\n\nSee [`Blueprint::target_release_minimum_generation`].", + "allOf": [ + { + "$ref": "#/components/schemas/Generation" + } + ] + }, "time_created": { "description": "when this blueprint was generated (for debugging)", "type": "string", @@ -2564,6 +2581,7 @@ "external_dns_version", "id", "internal_dns_version", + "target_release_minimum_generation", "time_created" ] }, diff --git a/schema/crdb/bp-add-target-release-min-gen/up1.sql b/schema/crdb/bp-add-target-release-min-gen/up1.sql new file mode 100644 index 00000000000..7c96a5d95d8 --- /dev/null +++ b/schema/crdb/bp-add-target-release-min-gen/up1.sql @@ -0,0 +1,2 @@ +ALTER TABLE omicron.public.blueprint + ADD COLUMN IF NOT EXISTS target_release_minimum_generation INT8 NOT NULL DEFAULT 1; diff --git a/schema/crdb/bp-add-target-release-min-gen/up2.sql b/schema/crdb/bp-add-target-release-min-gen/up2.sql new file mode 100644 index 00000000000..39fd663598d --- /dev/null +++ b/schema/crdb/bp-add-target-release-min-gen/up2.sql @@ -0,0 +1,2 @@ +ALTER TABLE omicron.public.blueprint + ALTER COLUMN target_release_minimum_generation DROP DEFAULT; diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 257db58c75f..e6bcb9bd199 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -4074,7 +4074,23 @@ CREATE TABLE IF NOT EXISTS omicron.public.blueprint ( -- represented by the presence of the default value in that field. -- -- `cluster.preserve_downgrade_option` - cockroachdb_setting_preserve_downgrade TEXT + cockroachdb_setting_preserve_downgrade TEXT, + + -- The smallest value of the target_release table's generation field that's + -- accepted by the blueprint. + -- + -- For example, let's say that the current target release generation is 5. + -- Then, when reconfigurator detects a MUPdate: + -- + -- * the target release is ignored in favor of the install dataset + -- * this field is set to 6 + -- + -- Once the target release generation is updated to 6 or higher, + -- Reconfigurator knows that it is back in charge of driving the system to + -- the target release. + -- + -- This is set to 1 by default in application code. + target_release_minimum_generation INT8 NOT NULL ); -- table describing both the current and historical target blueprints of the @@ -5671,7 +5687,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '144.0.0', NULL) + (TRUE, NOW(), NOW(), '145.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/sled-agent/src/rack_setup/service.rs b/sled-agent/src/rack_setup/service.rs index ad1778501bb..1fa6fb80e62 100644 --- a/sled-agent/src/rack_setup/service.rs +++ b/sled-agent/src/rack_setup/service.rs @@ -1541,6 +1541,7 @@ pub(crate) fn build_initial_blueprint_from_sled_configs( // generation of 1. Nexus will bump this up when it updates external DNS // (including creating the recovery silo). external_dns_version: Generation::new(), + target_release_minimum_generation: Generation::new(), // Nexus will fill in the CockroachDB values during initialization. cockroachdb_fingerprint: String::new(), cockroachdb_setting_preserve_downgrade: diff --git a/sled-agent/zone-images/Cargo.toml b/sled-agent/zone-images/Cargo.toml index 5c388a19dd2..ba0a5247496 100644 --- a/sled-agent/zone-images/Cargo.toml +++ b/sled-agent/zone-images/Cargo.toml @@ -10,16 +10,19 @@ workspace = true [dependencies] anyhow.workspace = true camino.workspace = true -id-map.workspace = true +iddqd.workspace = true illumos-utils.workspace = true nexus-sled-agent-shared.workspace = true omicron-common.workspace = true omicron-workspace-hack.workspace = true +rayon.workspace = true serde_json.workspace = true +sha2.workspace = true sled-storage.workspace = true slog.workspace = true slog-error-chain.workspace = true thiserror.workspace = true +tufaceous-artifact.workspace = true [dev-dependencies] camino-tempfile-ext.workspace = true diff --git a/sled-agent/zone-images/src/lib.rs b/sled-agent/zone-images/src/lib.rs index a6956e0956c..a35f20d6cdd 100644 --- a/sled-agent/zone-images/src/lib.rs +++ b/sled-agent/zone-images/src/lib.rs @@ -10,5 +10,5 @@ mod mupdate_override; mod source_resolver; -pub(crate) use mupdate_override::*; +pub use mupdate_override::*; pub use source_resolver::*; diff --git a/sled-agent/zone-images/src/mupdate_override.rs b/sled-agent/zone-images/src/mupdate_override.rs index 14ec5fd4c72..501cf8a36a8 100644 --- a/sled-agent/zone-images/src/mupdate_override.rs +++ b/sled-agent/zone-images/src/mupdate_override.rs @@ -6,18 +6,27 @@ //! //! For more about commingling MUPdate and update, see RFD 556. +use std::fmt; use std::fs; +use std::fs::File; use std::fs::FileType; use std::io; +use std::io::Read; use std::sync::Arc; use crate::ZoneImageZpools; use camino::Utf8Path; use camino::Utf8PathBuf; -use id_map::IdMap; -use id_map::IdMappable; +use iddqd::IdOrdItem; +use iddqd::IdOrdMap; +use iddqd::id_upcast; use illumos_utils::zpool::ZpoolName; use omicron_common::update::MupdateOverrideInfo; +use omicron_common::update::MupdateOverrideZone; +use rayon::iter::ParallelBridge; +use rayon::iter::ParallelIterator; +use sha2::Digest; +use sha2::Sha256; use sled_storage::dataset::INSTALL_DATASET; use slog::debug; use slog::error; @@ -26,14 +35,34 @@ use slog::o; use slog::warn; use slog_error_chain::InlineErrorChain; use thiserror::Error; +use tufaceous_artifact::ArtifactHash; + +/// Describes the current state of mupdate overrides. +#[derive(Clone, Debug)] +pub struct MupdateOverrideStatus { + /// The boot zpool. + pub boot_zpool: ZpoolName, + + /// The boot disk path. + pub boot_disk_path: Utf8PathBuf, + + /// Status of the boot disk. + pub boot_disk_override: + Result, MupdateOverrideReadError>, + + /// Status of the non-boot disks. This results in warnings. + pub non_boot_disk_overrides: IdOrdMap, +} #[derive(Debug)] pub(crate) struct AllMupdateOverrides { + // This is internal-only, and currently a duplicate of MupdateOverrideStatus + // in case we need to store more information here in the future. boot_zpool: ZpoolName, boot_disk_path: Utf8PathBuf, boot_disk_override: Result, MupdateOverrideReadError>, - non_boot_disk_overrides: IdMap, + non_boot_disk_overrides: IdOrdMap, } impl AllMupdateOverrides { @@ -74,7 +103,7 @@ impl AllMupdateOverrides { .all_m2_zpools .iter() .filter(|&zpool_name| zpool_name != boot_zpool); - let non_boot_disks_overrides = non_boot_zpools + let non_boot_disk_overrides = non_boot_zpools .map(|zpool_name| { let dataset = zpool_name.dataset_mountpoint(zpools.root, INSTALL_DATASET); @@ -95,13 +124,22 @@ impl AllMupdateOverrides { boot_zpool: *boot_zpool, boot_disk_path, boot_disk_override: boot_disk_res, - non_boot_disk_overrides: non_boot_disks_overrides, + non_boot_disk_overrides, }; ret.log_results(&log); ret } + pub(crate) fn status(&self) -> MupdateOverrideStatus { + MupdateOverrideStatus { + boot_zpool: self.boot_zpool, + boot_disk_path: self.boot_disk_path.clone(), + boot_disk_override: self.boot_disk_override.clone(), + non_boot_disk_overrides: self.non_boot_disk_overrides.clone(), + } + } + fn log_results(&self, log: &slog::Logger) { let log = log.new(o!( "boot_zpool" => self.boot_zpool.to_string(), @@ -198,7 +236,21 @@ fn read_mupdate_override( contents: data, } })?; - Some(data) + let artifacts = + MupdateOverrideArtifactsResult::new(dataset_dir, data); + if artifacts.is_valid() { + // If there are errors, return them as appropriate. + Some(artifacts.info) + } else { + // At least one artifact was invalid: return an error. + // + // XXX: Should we be more fine-grained than this, handle + // errors on a per-artifact basis? Seems excessive. + return Err(MupdateOverrideReadError::ArtifactRead { + dataset_dir: dataset_dir.to_owned(), + artifacts, + }); + } } Err(error) => { if error.kind() == std::io::ErrorKind::NotFound { @@ -225,15 +277,15 @@ fn read_mupdate_override( } #[derive(Clone, Debug, PartialEq)] -struct MupdateOverrideNonBootInfo { +pub struct MupdateOverrideNonBootInfo { /// The name of the zpool. - zpool_name: ZpoolName, + pub zpool_name: ZpoolName, /// The path that was read from. - path: Utf8PathBuf, + pub path: Utf8PathBuf, /// The result of performing the read operation. - result: MupdateOverrideNonBootResult, + pub result: MupdateOverrideNonBootResult, } impl MupdateOverrideNonBootInfo { @@ -272,16 +324,18 @@ impl MupdateOverrideNonBootInfo { } } -impl IdMappable for MupdateOverrideNonBootInfo { - type Id = ZpoolName; +impl IdOrdItem for MupdateOverrideNonBootInfo { + type Key<'a> = ZpoolName; - fn id(&self) -> Self::Id { + fn key(&self) -> Self::Key<'_> { self.zpool_name } + + id_upcast!(); } #[derive(Clone, Debug, PartialEq)] -enum MupdateOverrideNonBootResult { +pub enum MupdateOverrideNonBootResult { /// The override is present and matches the value on the boot disk. MatchesPresent, @@ -335,7 +389,7 @@ impl MupdateOverrideNonBootResult { } #[derive(Clone, Debug, PartialEq, Eq)] -enum MupdateOverrideNonBootMismatch { +pub enum MupdateOverrideNonBootMismatch { /// The override is present on the boot disk but absent on the other disk. BootPresentOtherAbsent, @@ -399,8 +453,187 @@ impl MupdateOverrideNonBootMismatch { } } -#[derive(Clone, Debug, Error, PartialEq)] -enum MupdateOverrideReadError { +/// The result of reading artifacts from an install dataset. +/// +/// This may or may not be valid, depending on the status of the artifacts. See +/// [`Self::is_valid`]. +#[derive(Clone, Debug, PartialEq)] +pub struct MupdateOverrideArtifactsResult { + pub info: MupdateOverrideInfo, + pub data: IdOrdMap, +} + +impl MupdateOverrideArtifactsResult { + /// Makes a new `MupdateOverrideArtifacts` by reading artifacts from the + /// given directory. + fn new(dir: &Utf8Path, info: MupdateOverrideInfo) -> Self { + let artifacts: Vec<_> = info + .zones + .iter() + // Parallelize artifact reading to speed it up. + .par_bridge() + .map(|zone| { + let artifact_path = dir.join(&zone.file_name); + let status = validate_one(&artifact_path, &zone); + + MupdateOverrideArtifactResult { + file_name: zone.file_name.clone(), + path: artifact_path, + expected_size: zone.file_size, + expected_hash: zone.hash, + status, + } + }) + .collect(); + + Self { info, data: artifacts.into_iter().collect() } + } + + /// Returns true if all artifacts are valid. + pub fn is_valid(&self) -> bool { + self.data.iter().all(|artifact| artifact.is_valid()) + } + + /// Returns a displayable representation of the artifacts. + pub fn display(&self) -> MupdateOverrideArtifactsDisplay<'_> { + MupdateOverrideArtifactsDisplay { artifacts: &self.data } + } +} + +pub struct MupdateOverrideArtifactsDisplay<'a> { + artifacts: &'a IdOrdMap, +} + +impl fmt::Display for MupdateOverrideArtifactsDisplay<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + for artifact in self.artifacts { + match &artifact.status { + ArtifactReadResult::Valid => { + writeln!( + f, + " {}: ok ({} bytes, {})", + artifact.file_name, + artifact.expected_size, + artifact.expected_hash + )?; + } + ArtifactReadResult::Mismatch { actual_size, actual_hash } => { + writeln!( + f, + " {}: mismatch (expected {} bytes, {}; \ + found {} bytes, {})", + artifact.file_name, + artifact.expected_size, + artifact.expected_hash, + actual_size, + actual_hash + )?; + } + ArtifactReadResult::Error(error) => { + writeln!(f, " {}: error ({})", artifact.file_name, error)?; + } + } + } + + Ok(()) + } +} + +#[derive(Clone, Debug, PartialEq)] +pub struct MupdateOverrideArtifactResult { + /// The filename. + pub file_name: String, + + /// The full path to the file. + pub path: Utf8PathBuf, + + /// The expected size. + pub expected_size: u64, + + /// The expected hash. + pub expected_hash: ArtifactHash, + + /// The status on disk. + pub status: ArtifactReadResult, +} + +impl MupdateOverrideArtifactResult { + fn is_valid(&self) -> bool { + matches!(self.status, ArtifactReadResult::Valid) + } +} + +impl IdOrdItem for MupdateOverrideArtifactResult { + type Key<'a> = &'a str; + + fn key(&self) -> Self::Key<'_> { + &self.file_name + } + + id_upcast!(); +} + +#[derive(Clone, Debug, PartialEq)] +pub enum ArtifactReadResult { + /// The artifact was read successfully and matches. + Valid, + + /// The artifact was read successfully but does not match. + Mismatch { + /// The actual file size. + actual_size: u64, + + /// The actual hash. + actual_hash: ArtifactHash, + }, + + /// An error occurred while reading the artifact. + Error(ArcIoError), +} + +fn validate_one( + artifact_path: &Utf8Path, + zone: &MupdateOverrideZone, +) -> ArtifactReadResult { + let mut f = match File::open(artifact_path) { + Ok(f) => f, + Err(error) => { + return ArtifactReadResult::Error(ArcIoError::new(error)); + } + }; + + match compute_size_and_hash(&mut f) { + Ok((actual_size, actual_hash)) => { + if zone.file_size == actual_size && zone.hash == actual_hash { + ArtifactReadResult::Valid + } else { + ArtifactReadResult::Mismatch { actual_size, actual_hash } + } + } + Err(error) => ArtifactReadResult::Error(ArcIoError::new(error)), + } +} + +fn compute_size_and_hash( + f: &mut File, +) -> Result<(u64, ArtifactHash), io::Error> { + let mut hasher = Sha256::new(); + // Zone artifacts are pretty big, so we read them in chunks. + let mut buffer = [0u8; 8192]; + let mut total_bytes_read = 0; + loop { + let bytes_read = f.read(&mut buffer)?; + if bytes_read == 0 { + break; + } + hasher.update(&buffer[..bytes_read]); + total_bytes_read += bytes_read; + } + Ok((total_bytes_read as u64, ArtifactHash(hasher.finalize().into()))) +} + +#[derive(Clone, Debug, PartialEq, Error)] +pub enum MupdateOverrideReadError { #[error( "error retrieving metadata for install dataset directory \ `{dataset_dir}`" @@ -434,12 +667,18 @@ enum MupdateOverrideReadError { #[source] error: ArcSerdeJsonError, }, + + #[error("error reading artifacts in `{dataset_dir}:\n{}`", artifacts.display())] + ArtifactRead { + dataset_dir: Utf8PathBuf, + artifacts: MupdateOverrideArtifactsResult, + }, } /// An `io::Error` wrapper that implements `Clone` and `PartialEq`. #[derive(Clone, Debug, Error)] #[error(transparent)] -struct ArcIoError(Arc); +pub struct ArcIoError(Arc); impl ArcIoError { fn new(error: io::Error) -> Self { @@ -458,7 +697,7 @@ impl PartialEq for ArcIoError { /// A `serde_json::Error` that implements `Clone` and `PartialEq`. #[derive(Clone, Debug, Error)] #[error(transparent)] -struct ArcSerdeJsonError(Arc); +pub struct ArcSerdeJsonError(Arc); impl ArcSerdeJsonError { fn new(error: serde_json::Error) -> Self { @@ -479,6 +718,9 @@ impl PartialEq for ArcSerdeJsonError { #[cfg(test)] mod tests { use super::*; + use camino_tempfile_ext::fixture::ChildPath; + use camino_tempfile_ext::fixture::FixtureError; + use camino_tempfile_ext::fixture::FixtureKind; use camino_tempfile_ext::prelude::*; use dropshot::ConfigLogging; use dropshot::ConfigLoggingLevel; @@ -531,11 +773,6 @@ mod tests { static NON_BOOT_3_PATHS: LazyLock = LazyLock::new(|| OverridePaths::for_uuid(NON_BOOT_3_UUID)); - static OVERRIDE_UUID: MupdateOverrideUuid = - MupdateOverrideUuid::from_u128(0x70b965c2_fc95_4843_a34d_a2c7246788a8); - static OVERRIDE_2_UUID: MupdateOverrideUuid = - MupdateOverrideUuid::from_u128(0x20588f8f_c680_4101_afc7_820226d03ada); - /// Boot disk present / no other disks. (This produces a warning, but is /// otherwise okay.) #[test] @@ -544,12 +781,10 @@ mod tests { "mupdate_override_read_other_absent", &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug }, ); - let override_info = override_info(); let dir = Utf8TempDir::new().unwrap(); - - dir.child(&BOOT_PATHS.override_json) - .write_str(&serde_json::to_string(&override_info).unwrap()) - .unwrap(); + let cx = WriteInstallDatasetContext::new_basic(); + let info = + cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); let zpools = ZoneImageZpools { root: dir.path(), @@ -559,9 +794,9 @@ mod tests { AllMupdateOverrides::read_all(&logctx.log, &zpools, &BOOT_ZPOOL); assert_eq!( overrides.boot_disk_override.as_ref().unwrap().as_ref(), - Some(&override_info) + Some(&info) ); - assert_eq!(overrides.non_boot_disk_overrides, IdMap::new()); + assert_eq!(overrides.non_boot_disk_overrides, IdOrdMap::new()); logctx.cleanup_successful(); } @@ -573,15 +808,13 @@ mod tests { "mupdate_override_read_both_present", &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug }, ); - let override_info = override_info(); let dir = Utf8TempDir::new().unwrap(); - - dir.child(&BOOT_PATHS.override_json) - .write_str(&serde_json::to_string(&override_info).unwrap()) - .unwrap(); - dir.child(&NON_BOOT_PATHS.override_json) - .write_str(&serde_json::to_string(&override_info).unwrap()) - .unwrap(); + let cx = WriteInstallDatasetContext::new_basic(); + let info = + cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); + let info2 = + cx.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); + assert_eq!(info, info2, "the same contents must have been written out"); let zpools = ZoneImageZpools { root: dir.path(), @@ -592,7 +825,7 @@ mod tests { AllMupdateOverrides::read_all(&logctx.log, &zpools, &BOOT_ZPOOL); assert_eq!( overrides.boot_disk_override.as_ref().unwrap().as_ref(), - Some(&override_info) + Some(&info) ); assert_eq!( overrides.non_boot_disk_overrides, @@ -615,6 +848,7 @@ mod tests { "mupdate_override_read_both_absent", &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug }, ); + let dir = Utf8TempDir::new().unwrap(); // Create the directories but not the override JSONs within them. @@ -653,12 +887,11 @@ mod tests { "mupdate_override_read_boot_present_other_absent", &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug }, ); - let override_info = override_info(); let dir = Utf8TempDir::new().unwrap(); + let cx = WriteInstallDatasetContext::new_basic(); + let info = + cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); - dir.child(&BOOT_PATHS.override_json) - .write_str(&serde_json::to_string(&override_info).unwrap()) - .unwrap(); // Create the directory, but not the override JSON within it. dir.child(&NON_BOOT_PATHS.install_dataset).create_dir_all().unwrap(); @@ -671,7 +904,7 @@ mod tests { AllMupdateOverrides::read_all(&logctx.log, &zpools, &BOOT_ZPOOL); assert_eq!( overrides.boot_disk_override.as_ref().unwrap().as_ref(), - Some(&override_info) + Some(&info) ); assert_eq!( overrides.non_boot_disk_overrides, @@ -696,15 +929,14 @@ mod tests { "mupdate_override_read_boot_absent_other_present", &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug }, ); - let override_info = override_info(); let dir = Utf8TempDir::new().unwrap(); + let cx = WriteInstallDatasetContext::new_basic(); // Create the directory, but not the override JSON within it. dir.child(&BOOT_PATHS.install_dataset).create_dir_all().unwrap(); - dir.child(&NON_BOOT_PATHS.override_json) - .write_str(&serde_json::to_string(&override_info).unwrap()) - .unwrap(); + let info = + cx.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); let zpools = ZoneImageZpools { root: dir.path(), @@ -723,7 +955,7 @@ mod tests { path: dir.path().join(&NON_BOOT_PATHS.override_json), result: MupdateOverrideNonBootResult::Mismatch( MupdateOverrideNonBootMismatch::BootAbsentOtherPresent { - non_boot_disk_info: override_info.clone() + non_boot_disk_info: info.clone() }, ), }] @@ -741,16 +973,17 @@ mod tests { "mupdate_override_read_different_values", &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug }, ); - let override_info = override_info(); - let override_info_2 = override_info_2(); + let dir = Utf8TempDir::new().unwrap(); - dir.child(&BOOT_PATHS.override_json) - .write_str(&serde_json::to_string(&override_info).unwrap()) - .expect("failed to write override json"); - dir.child(&NON_BOOT_PATHS.override_json) - .write_str(&serde_json::to_string(&override_info_2).unwrap()) - .expect("failed to write override json"); + // Make two different contexts. Each will have a different mupdate_uuid + // so will not match. + let cx = WriteInstallDatasetContext::new_basic(); + let info = + cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); + let cx2 = WriteInstallDatasetContext::new_basic(); + let info2 = + cx2.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); let zpools = ZoneImageZpools { root: dir.path(), @@ -760,7 +993,7 @@ mod tests { AllMupdateOverrides::read_all(&logctx.log, &zpools, &BOOT_ZPOOL); assert_eq!( overrides.boot_disk_override.as_ref().unwrap().as_ref(), - Some(&override_info), + Some(&info), ); assert_eq!( overrides.non_boot_disk_overrides, @@ -769,7 +1002,7 @@ mod tests { path: dir.path().join(&NON_BOOT_PATHS.override_json), result: MupdateOverrideNonBootResult::Mismatch( MupdateOverrideNonBootMismatch::ValueMismatch { - non_boot_disk_info: override_info_2, + non_boot_disk_info: info2, } ), }] @@ -878,15 +1111,14 @@ mod tests { "mupdate_override_read_boot_read_error", &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug }, ); - let override_info = override_info(); let dir = Utf8TempDir::new().unwrap(); + let cx = WriteInstallDatasetContext::new_basic(); // Create an empty file: this won't deserialize correctly. dir.child(&BOOT_PATHS.override_json).touch().unwrap(); // File with the correct contents. - dir.child(&NON_BOOT_PATHS.override_json) - .write_str(&serde_json::to_string(&override_info).unwrap()) - .unwrap(); + let info = + cx.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); // File that's absent. dir.child(&NON_BOOT_2_PATHS.install_dataset).create_dir_all().unwrap(); // Read error (empty file). @@ -915,7 +1147,7 @@ mod tests { path: dir.path().join(&NON_BOOT_PATHS.override_json), result: MupdateOverrideNonBootResult::Mismatch( MupdateOverrideNonBootMismatch::BootDiskReadError { - non_boot_disk_info: Some(override_info), + non_boot_disk_info: Some(info), }, ), }, @@ -947,18 +1179,293 @@ mod tests { logctx.cleanup_successful(); } - fn override_info() -> MupdateOverrideInfo { - MupdateOverrideInfo { - mupdate_uuid: OVERRIDE_UUID, - hash_ids: BTreeSet::new(), + /// Error case: zones don't match expected ones on boot disk. + #[test] + fn read_boot_disk_zone_mismatch() { + let logctx = LogContext::new( + "mupdate_override_read_boot_disk_zone_mismatch", + &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug }, + ); + let dir = Utf8TempDir::new().unwrap(); + let cx = WriteInstallDatasetContext::new_basic(); + let mut invalid_cx = cx.clone(); + invalid_cx.make_error_cases(); + + invalid_cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); + let valid_info = + cx.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); + + let zpools = ZoneImageZpools { + root: dir.path(), + all_m2_zpools: vec![BOOT_ZPOOL, NON_BOOT_ZPOOL], + }; + + let overrides = + AllMupdateOverrides::read_all(&logctx.log, &zpools, &BOOT_ZPOOL); + assert_eq!( + overrides.boot_disk_override.as_ref().unwrap_err(), + &MupdateOverrideReadError::ArtifactRead { + dataset_dir: dir + .child(&BOOT_PATHS.install_dataset) + .as_path() + .to_path_buf(), + artifacts: invalid_cx.expected_result( + dir.child(&BOOT_PATHS.install_dataset).as_path() + ), + } + ); + + assert_eq!( + overrides.non_boot_disk_overrides, + [MupdateOverrideNonBootInfo { + zpool_name: NON_BOOT_ZPOOL, + path: dir.path().join(&NON_BOOT_PATHS.override_json), + result: MupdateOverrideNonBootResult::Mismatch( + MupdateOverrideNonBootMismatch::BootDiskReadError { + non_boot_disk_info: Some(valid_info), + } + ) + }] + .into_iter() + .collect(), + ); + + logctx.cleanup_successful(); + } + + /// Warning case: zones don't match expected ones on non-boot disk. + #[test] + fn read_non_boot_disk_zone_mismatch() { + let logctx = LogContext::new( + "mupdate_override_read_non_boot_disk_zone_mismatch", + &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug }, + ); + let dir = Utf8TempDir::new().unwrap(); + let cx = WriteInstallDatasetContext::new_basic(); + let mut invalid_cx = cx.clone(); + invalid_cx.make_error_cases(); + + let info = + cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); + invalid_cx + .write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)) + .unwrap(); + + let zpools = ZoneImageZpools { + root: dir.path(), + all_m2_zpools: vec![BOOT_ZPOOL, NON_BOOT_ZPOOL], + }; + + let overrides = + AllMupdateOverrides::read_all(&logctx.log, &zpools, &BOOT_ZPOOL); + // The boot disk is valid. + assert_eq!( + overrides.boot_disk_override.as_ref().unwrap().as_ref(), + Some(&info) + ); + + // The non-boot disk has an error. + assert_eq!( + overrides.non_boot_disk_overrides, + [MupdateOverrideNonBootInfo { + zpool_name: NON_BOOT_ZPOOL, + path: dir.path().join(&NON_BOOT_PATHS.override_json), + result: MupdateOverrideNonBootResult::ReadError( + MupdateOverrideReadError::ArtifactRead { + dataset_dir: dir + .path() + .join(&NON_BOOT_PATHS.install_dataset), + artifacts: invalid_cx.expected_result( + &dir.path().join(&NON_BOOT_PATHS.install_dataset) + ), + }, + ), + }] + .into_iter() + .collect(), + ); + + logctx.cleanup_successful(); + } + + /// Context for writing out fake zones to install dataset directories. + /// + /// The tests in this module ensure that the override JSON's list of zones + /// matches the zone files on disk. + #[derive(Clone, Debug)] + struct WriteInstallDatasetContext { + zones: IdOrdMap, + mupdate_uuid: MupdateOverrideUuid, + } + + impl WriteInstallDatasetContext { + /// Initializes a new context with a couple of zones and no known + /// errors. + fn new_basic() -> Self { + Self { + zones: [ + ZoneContents::new("zone1.tar.gz", b"zone1"), + ZoneContents::new("zone2.tar.gz", b"zone2"), + ZoneContents::new("zone3.tar.gz", b"zone3"), + ZoneContents::new("zone4.tar.gz", b"zone4"), + ZoneContents::new("zone5.tar.gz", b"zone5"), + ] + .into_iter() + .collect(), + mupdate_uuid: MupdateOverrideUuid::new_v4(), + } } + + /// Makes a number of error cases for testing. + fn make_error_cases(&mut self) { + // zone1.tar.gz is valid. + // For zone2.tar.gz, change the size. + self.zones.get_mut("zone2.tar.gz").unwrap().json_size = 1024; + // For zone3.tar.gz, change the hash. + self.zones.get_mut("zone3.tar.gz").unwrap().json_hash = + ArtifactHash([0; 32]); + // Don't write out zone4 but include it in the JSON. + self.zones.get_mut("zone4.tar.gz").unwrap().write_to_disk = false; + // Write out zone5 but don't include it in the JSON. + self.zones.get_mut("zone5.tar.gz").unwrap().include_in_json = false; + } + + fn override_info(&self) -> MupdateOverrideInfo { + MupdateOverrideInfo { + mupdate_uuid: self.mupdate_uuid, + // The hash IDs are not used for validation, so leave this + // empty. + hash_ids: BTreeSet::new(), + zones: self + .zones + .iter() + .filter_map(|zone| { + zone.include_in_json.then(|| MupdateOverrideZone { + file_name: zone.file_name.clone(), + file_size: zone.json_size, + hash: zone.json_hash, + }) + }) + .collect(), + } + } + + /// Returns the expected result of the override, taking into account + /// mismatches, etc. + fn expected_result( + &self, + dir: &Utf8Path, + ) -> MupdateOverrideArtifactsResult { + let info = self.override_info(); + let data = self + .zones + .iter() + .filter_map(|zone| { + // Currently, zone files not present in the JSON aren't + // reported at all. + // + // XXX: should they be? + zone.include_in_json.then(|| zone.expected_result(dir)) + }) + .collect(); + MupdateOverrideArtifactsResult { info, data } + } + + /// Writes the context to a directory, returning the JSON that was + /// written out. + fn write_to( + &self, + dir: &ChildPath, + ) -> Result { + for zone in &self.zones { + if zone.write_to_disk { + dir.child(&zone.file_name).write_binary(&zone.contents)?; + } + } + + let info = self.override_info(); + let json = serde_json::to_string(&info).map_err(|e| { + FixtureError::new(FixtureKind::WriteFile).with_source(e) + })?; + // No need to create intermediate directories with + // camino-tempfile-ext. + dir.child(MupdateOverrideInfo::FILE_NAME).write_str(&json)?; + + Ok(info) + } + } + + #[derive(Clone, Debug)] + struct ZoneContents { + file_name: String, + contents: Vec, + // json_size and json_hash are stored separately, so tests can tweak + // them before writing out the override info. + json_size: u64, + json_hash: ArtifactHash, + write_to_disk: bool, + include_in_json: bool, } - fn override_info_2() -> MupdateOverrideInfo { - MupdateOverrideInfo { - mupdate_uuid: OVERRIDE_2_UUID, - hash_ids: BTreeSet::new(), + impl ZoneContents { + fn new(file_name: &str, contents: &[u8]) -> Self { + let size = contents.len() as u64; + let hash = compute_hash(contents); + Self { + file_name: file_name.to_string(), + contents: contents.to_vec(), + json_size: size, + json_hash: hash, + write_to_disk: true, + include_in_json: true, + } } + + fn expected_result( + &self, + dir: &Utf8Path, + ) -> MupdateOverrideArtifactResult { + let status = if !self.write_to_disk { + // Missing from the disk + ArtifactReadResult::Error(ArcIoError::new(io::Error::new( + io::ErrorKind::NotFound, + "file not found", + ))) + } else { + let actual_size = self.contents.len() as u64; + let actual_hash = compute_hash(&self.contents); + if self.json_size != actual_size + || self.json_hash != actual_hash + { + ArtifactReadResult::Mismatch { actual_size, actual_hash } + } else { + ArtifactReadResult::Valid + } + }; + + MupdateOverrideArtifactResult { + file_name: self.file_name.clone(), + path: dir.join(&self.file_name), + expected_size: self.json_size, + expected_hash: self.json_hash, + status, + } + } + } + + impl IdOrdItem for ZoneContents { + type Key<'a> = &'a str; + + fn key(&self) -> Self::Key<'_> { + &self.file_name + } + + id_upcast!(); + } + + fn compute_hash(contents: &[u8]) -> ArtifactHash { + let hash = Sha256::digest(contents); + ArtifactHash(hash.into()) } fn dataset_missing_error(dir_path: &Utf8Path) -> MupdateOverrideReadError { diff --git a/sled-agent/zone-images/src/source_resolver.rs b/sled-agent/zone-images/src/source_resolver.rs index 576e1676ad0..3b31c7a6a14 100644 --- a/sled-agent/zone-images/src/source_resolver.rs +++ b/sled-agent/zone-images/src/source_resolver.rs @@ -5,6 +5,7 @@ //! Zone image lookup. use crate::AllMupdateOverrides; +use crate::MupdateOverrideStatus; use camino::Utf8Path; use camino::Utf8PathBuf; use illumos_utils::zpool::ZpoolName; @@ -70,6 +71,12 @@ impl ZoneImageSourceResolver { self.inner.lock().unwrap().override_image_directory(path); } + /// Returns current information about resolver status and health. + pub fn status(&self) -> ResolverStatus { + let inner = self.inner.lock().unwrap(); + ResolverStatus { mupdate_override: inner.mupdate_overrides.status() } + } + /// Returns a [`ZoneImageFileSource`] consisting of the file name, plus a /// list of potential paths to search, for a zone image. pub fn file_source_for( @@ -83,6 +90,13 @@ impl ZoneImageSourceResolver { } } +/// Current status of the zone image resolver. +#[derive(Clone, Debug)] +pub struct ResolverStatus { + /// The mupdate override status. + pub mupdate_override: MupdateOverrideStatus, +} + #[derive(Debug)] struct ResolverInner { #[expect(unused)] @@ -90,9 +104,6 @@ struct ResolverInner { image_directory_override: Option, // Store all collected information for mupdate overrides -- we're going to // need to report this via inventory. - // - // This isn't actually used yet. - #[expect(unused)] mupdate_overrides: AllMupdateOverrides, } diff --git a/test-utils/src/dev/clickhouse.rs b/test-utils/src/dev/clickhouse.rs index 3770a6aa3d3..f06cfec526f 100644 --- a/test-utils/src/dev/clickhouse.rs +++ b/test-utils/src/dev/clickhouse.rs @@ -825,7 +825,7 @@ impl ClickHouseDataDir { ]; // Persist this temporary directory since we're going to be doing the // cleanup ourselves. - let dir = self.dir.into_path(); + let dir = self.dir.keep(); let mut error_paths = BTreeMap::new(); // contents_first = true ensures that we delete inner files before diff --git a/test-utils/src/dev/db.rs b/test-utils/src/dev/db.rs index 6249570c630..90116c05c86 100644 --- a/test-utils/src/dev/db.rs +++ b/test-utils/src/dev/db.rs @@ -439,7 +439,7 @@ impl CockroachStarter { // the user can debug if they want. We'll skip cleanup of the // temporary directory for the same reason and also so that // CockroachDB doesn't trip over its files being gone. - let _preserve_directory = self.temp_dir.into_path(); + let _preserve_directory = self.temp_dir.keep(); Err(match poll_error { poll::Error::PermanentError(e) => e, @@ -683,7 +683,7 @@ impl Drop for CockroachInstance { #[allow(unused_must_use)] if let Some(temp_dir) = self.temp_dir.take() { // Do NOT clean up the temporary directory in this case. - let path = temp_dir.into_path(); + let path = temp_dir.keep(); eprintln!( "WARN: temporary directory leaked: {path:?}\n\ \tIf you would like to access the database for debugging, run the following:\n\n\ diff --git a/test-utils/src/dev/dendrite.rs b/test-utils/src/dev/dendrite.rs index 6840480cfc3..5dc30dffedf 100644 --- a/test-utils/src/dev/dendrite.rs +++ b/test-utils/src/dev/dendrite.rs @@ -65,7 +65,7 @@ impl DendriteInstance { let child = Some(child); - let temp_dir = temp_dir.into_path(); + let temp_dir = temp_dir.keep(); if port == 0 { port = discover_port( temp_dir.join("dendrite_stdout").display().to_string(), diff --git a/test-utils/src/dev/maghemite.rs b/test-utils/src/dev/maghemite.rs index a633751bca8..4c2d85df3ee 100644 --- a/test-utils/src/dev/maghemite.rs +++ b/test-utils/src/dev/maghemite.rs @@ -65,7 +65,7 @@ impl MgdInstance { let child = Some(child); - let temp_dir = temp_dir.into_path(); + let temp_dir = temp_dir.keep(); if port == 0 { port = discover_port( temp_dir.join("mgd_stdout").display().to_string(), diff --git a/wicketd/Cargo.toml b/wicketd/Cargo.toml index 3cb85f2e2f2..3097a1bd6c4 100644 --- a/wicketd/Cargo.toml +++ b/wicketd/Cargo.toml @@ -89,6 +89,7 @@ openapi-lint.workspace = true openapiv3.workspace = true rand.workspace = true serde_json.workspace = true +sled-agent-zone-images.workspace = true subprocess.workspace = true tar.workspace = true tokio = { workspace = true, features = ["test-util"] } diff --git a/wicketd/tests/integration_tests/updates.rs b/wicketd/tests/integration_tests/updates.rs index 16bf3927f7b..a03d6095a67 100644 --- a/wicketd/tests/integration_tests/updates.rs +++ b/wicketd/tests/integration_tests/updates.rs @@ -11,9 +11,14 @@ use camino_tempfile::Utf8TempDir; use clap::Parser; use gateway_messages::SpPort; use gateway_test_utils::setup as gateway_setup; +use illumos_utils::zpool::ZpoolName; use installinator::HOST_PHASE_2_FILE_NAME; use maplit::btreeset; use omicron_common::update::MupdateOverrideInfo; +use omicron_uuid_kinds::ZpoolUuid; +use sled_agent_zone_images::{ + MupdateOverrideNonBootResult, ZoneImageSourceResolver, ZoneImageZpools, +}; use tokio::sync::oneshot; use tufaceous_artifact::{ArtifactHashId, ArtifactKind, KnownArtifactKind}; use update_engine::NestedError; @@ -359,9 +364,13 @@ async fn test_installinator_fetch() { } }); + // Simulate a couple of zpools. + let zpool1_uuid = ZpoolUuid::new_v4(); + let zpool2_uuid = ZpoolUuid::new_v4(); + let a_path = temp_dir.path().join("pool/int").join(zpool1_uuid.to_string()); + let b_path = temp_dir.path().join("pool/int").join(zpool2_uuid.to_string()); + let update_id_str = update_id.to_string(); - let a_path = temp_dir.path().join("installinator-out-a"); - let b_path = temp_dir.path().join("installinator-out-b"); let args = installinator::InstallinatorApp::try_parse_from([ "installinator", "install", @@ -399,7 +408,7 @@ async fn test_installinator_fetch() { // The control plane zone names here are defined in `fake.toml` which we // load above. for file_name in - [HOST_PHASE_2_FILE_NAME, "zones/zone1.tar.gz", "zones/zone2.tar.gz"] + [HOST_PHASE_2_FILE_NAME, "install/zone1.tar.gz", "install/zone2.tar.gz"] { let a_path = a_path.join(file_name); assert!(a_path.is_file(), "{a_path} was written out"); @@ -411,9 +420,9 @@ async fn test_installinator_fetch() { // Ensure that the MUPdate override files were written correctly. // // In the mode where we specify a destination directory to write to, - // the install dataset translates to "/zones". + // the install dataset translates to "/install". let b_override_path = - a_path.join("zones").join(MupdateOverrideInfo::FILE_NAME); + a_path.join("install").join(MupdateOverrideInfo::FILE_NAME); assert!(b_override_path.is_file(), "{b_override_path} was written out"); // Ensure that the MUPdate override file can be parsed. @@ -432,7 +441,7 @@ async fn test_installinator_fetch() { // Ensure that the B path also had the same file written out. let b_override_path = - b_path.join("zones").join(MupdateOverrideInfo::FILE_NAME); + b_path.join("install").join(MupdateOverrideInfo::FILE_NAME); assert!(b_override_path.is_file(), "{b_override_path} was written out"); // Ensure that the MUPdate override file can be parsed. @@ -447,6 +456,48 @@ async fn test_installinator_fetch() { "mupdate override info matches across A and B drives", ); + // Check that the zone1 and zone2 images are present in the zone set. (The + // names come from fake-non-semver.toml, under + // [artifact.control-plane.source]). + assert!( + a_override_info.zones.contains_key("zone1.tar.gz"), + "zone1 is present in the zone set" + ); + assert!( + a_override_info.zones.contains_key("zone2.tar.gz"), + "zone2 is present in the zone set" + ); + + // Run sled-agent-zone-images against these paths, and ensure that the + // mupdate override is correctly picked up. Pick zpool1 arbitrarily as the boot zpool. + let boot_zpool = ZpoolName::new_internal(zpool1_uuid); + let non_boot_zpool = ZpoolName::new_internal(zpool2_uuid); + let zpools = ZoneImageZpools { + root: temp_dir.path(), + all_m2_zpools: vec![boot_zpool, non_boot_zpool], + }; + let image_resolver = + ZoneImageSourceResolver::new(&log, &zpools, &boot_zpool); + + // Ensure that the resolver picks up the mupdate override. + let override_status = image_resolver.status().mupdate_override; + eprintln!("override_status: {:#?}", override_status); + + let info = override_status + .boot_disk_override + .expect("mupdate override successful") + .expect("mupdate override present"); + assert_eq!(info, a_override_info, "info matches a_override_info"); + + let non_boot_status = override_status + .non_boot_disk_overrides + .get(&non_boot_zpool) + .expect("non-boot disk status should be present"); + assert_eq!( + non_boot_status.result, + MupdateOverrideNonBootResult::MatchesPresent, + ); + recv_handle.await.expect("recv_handle succeeded"); wicketd_testctx.teardown().await; diff --git a/workspace-hack/Cargo.toml b/workspace-hack/Cargo.toml index d3672358f1d..3296a2fd693 100644 --- a/workspace-hack/Cargo.toml +++ b/workspace-hack/Cargo.toml @@ -49,6 +49,7 @@ either = { version = "1.14.0" } elliptic-curve = { version = "0.13.8", features = ["ecdh", "hazmat", "pem", "std"] } ff = { version = "0.13.0", default-features = false, features = ["alloc"] } flate2 = { version = "1.1.1" } +foldhash = { version = "0.1.5" } fs-err = { version = "3.1.0", default-features = false, features = ["tokio"] } futures = { version = "0.3.31" } futures-channel = { version = "0.3.31", features = ["sink"] } @@ -173,6 +174,7 @@ either = { version = "1.14.0" } elliptic-curve = { version = "0.13.8", features = ["ecdh", "hazmat", "pem", "std"] } ff = { version = "0.13.0", default-features = false, features = ["alloc"] } flate2 = { version = "1.1.1" } +foldhash = { version = "0.1.5" } fs-err = { version = "3.1.0", default-features = false, features = ["tokio"] } futures = { version = "0.3.31" } futures-channel = { version = "0.3.31", features = ["sink"] } From ae0cd87d6e9d9522a63e6b1e62d335de824cb2e2 Mon Sep 17 00:00:00 2001 From: Rain Date: Thu, 29 May 2025 07:11:10 +0000 Subject: [PATCH 2/3] use RAMDISK_IMAGE_PATH Created using spr 1.3.6-beta.1 --- sled-agent/zone-images/src/source_resolver.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sled-agent/zone-images/src/source_resolver.rs b/sled-agent/zone-images/src/source_resolver.rs index 92dba495477..65d34400a4d 100644 --- a/sled-agent/zone-images/src/source_resolver.rs +++ b/sled-agent/zone-images/src/source_resolver.rs @@ -6,6 +6,7 @@ use crate::AllMupdateOverrides; use crate::MupdateOverrideStatus; +use crate::RAMDISK_IMAGE_PATH; use crate::install_dataset_file_name; use camino::Utf8Path; use camino::Utf8PathBuf; @@ -138,7 +139,7 @@ impl ResolverInner { OmicronZoneImageSource::InstallDataset => { // Look for the image in the ramdisk first let mut zone_image_paths = - vec![Utf8PathBuf::from("/opt/oxide")]; + vec![Utf8PathBuf::from(RAMDISK_IMAGE_PATH)]; // Inject an image path if requested by a test. if let Some(path) = &self.image_directory_override { zone_image_paths.push(path.clone()); From 134801fb8be6d9402a0b162201ee81d22b36edf6 Mon Sep 17 00:00:00 2001 From: Rain Date: Fri, 6 Jun 2025 16:24:32 +0000 Subject: [PATCH 3/3] [spr] changes introduced through rebase Created using spr 1.3.6-beta.1 [skip ci] --- Cargo.lock | 16 +- Cargo.toml | 2 +- clients/gateway-client/Cargo.toml | 1 + clients/gateway-client/src/lib.rs | 5 +- clients/installinator-client/Cargo.toml | 2 +- clients/installinator-client/src/lib.rs | 1 + common/src/update/mod.rs | 2 + common/src/update/mupdate_override.rs | 37 +- common/src/update/zone_manifest.rs | 52 + gateway-cli/Cargo.toml | 1 + gateway-cli/src/main.rs | 3 +- gateway-types/Cargo.toml | 1 + gateway-types/src/update.rs | 3 +- installinator-api/Cargo.toml | 1 + installinator-api/src/lib.rs | 6 +- installinator-common/src/progress.rs | 3 + installinator/Cargo.toml | 1 - installinator/src/artifact.rs | 6 +- installinator/src/dispatch.rs | 1 + installinator/src/mock_peers.rs | 12 +- installinator/src/reporter.rs | 16 +- installinator/src/write.rs | 88 +- ipcc/Cargo.toml | 2 +- ipcc/src/lib.rs | 9 +- openapi/gateway.json | 7 +- openapi/installinator.json | 7 +- sled-agent/zone-images/Cargo.toml | 1 + sled-agent/zone-images/src/errors.rs | 46 + .../src/install_dataset_metadata.rs | 287 ++++ sled-agent/zone-images/src/lib.rs | 8 + .../zone-images/src/mupdate_override.rs | 1217 +++-------------- sled-agent/zone-images/src/source_resolver.rs | 22 +- sled-agent/zone-images/src/test_utils.rs | 292 ++++ sled-agent/zone-images/src/zone_manifest.rs | 827 +++++++++++ uuid-kinds/src/lib.rs | 1 + wicketd/src/installinator_progress.rs | 20 +- wicketd/src/update_tracker.rs | 5 +- wicketd/tests/integration_tests/updates.rs | 74 +- 38 files changed, 1963 insertions(+), 1122 deletions(-) create mode 100644 common/src/update/zone_manifest.rs create mode 100644 sled-agent/zone-images/src/errors.rs create mode 100644 sled-agent/zone-images/src/install_dataset_metadata.rs create mode 100644 sled-agent/zone-images/src/test_utils.rs create mode 100644 sled-agent/zone-images/src/zone_manifest.rs diff --git a/Cargo.lock b/Cargo.lock index 9db389329a7..4cbf18a6b38 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3480,6 +3480,7 @@ dependencies = [ "gateway-client", "gateway-messages", "omicron-common", + "omicron-uuid-kinds", "omicron-workspace-hack", "reqwest", "serde", @@ -3503,6 +3504,7 @@ dependencies = [ "daft", "gateway-messages", "gateway-types", + "omicron-uuid-kinds", "omicron-workspace-hack", "progenitor 0.10.0", "rand 0.8.5", @@ -3606,6 +3608,7 @@ dependencies = [ "hex", "ipcc", "omicron-common", + "omicron-uuid-kinds", "omicron-workspace-hack", "schemars", "serde", @@ -4554,14 +4557,12 @@ dependencies = [ [[package]] name = "iddqd" -version = "0.3.3" +version = "0.3.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7c3407dc49fe79cf7281b48d3a251cf8c11ff960a652e32cbef8848216c4b3c8" +checksum = "fb5b72a689cecdaca7349c7fd30715e3f494945e60c480de56a7089c814fa622" dependencies = [ "allocator-api2", "daft", - "debug-ignore", - "derive-where", "equivalent", "foldhash", "hashbrown 0.15.3", @@ -4849,7 +4850,6 @@ dependencies = [ "tufaceous-artifact", "tufaceous-lib", "update-engine", - "uuid", ] [[package]] @@ -4861,6 +4861,7 @@ dependencies = [ "hyper", "installinator-common", "omicron-common", + "omicron-uuid-kinds", "omicron-workspace-hack", "schemars", "serde", @@ -4875,6 +4876,7 @@ version = "0.1.0" dependencies = [ "installinator-common", "omicron-common", + "omicron-uuid-kinds", "omicron-workspace-hack", "progenitor 0.10.0", "regress", @@ -4884,7 +4886,6 @@ dependencies = [ "serde_json", "slog", "update-engine", - "uuid", ] [[package]] @@ -5002,13 +5003,13 @@ dependencies = [ "ciborium", "libipcc", "omicron-common", + "omicron-uuid-kinds", "omicron-workspace-hack", "proptest", "serde", "test-strategy", "thiserror 1.0.69", "tufaceous-artifact", - "uuid", ] [[package]] @@ -11746,6 +11747,7 @@ dependencies = [ "omicron-workspace-hack", "pretty_assertions", "rayon", + "serde", "serde_json", "sha2", "sled-storage", diff --git a/Cargo.toml b/Cargo.toml index 82d2e4dc5ac..fd04ad4ea23 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -473,7 +473,7 @@ hyper = "1.6.0" hyper-util = "0.1.11" hyper-rustls = "0.26.0" hyper-staticfile = "0.10.1" -iddqd = { version = "0.3.3", features = ["daft", "serde"] } +iddqd = { version = "0.3.5", features = ["daft", "serde"] } id-map = { path = "id-map" } illumos-utils = { path = "illumos-utils" } iana-time-zone = "0.1.63" diff --git a/clients/gateway-client/Cargo.toml b/clients/gateway-client/Cargo.toml index cd4f352086a..6e74b903a57 100644 --- a/clients/gateway-client/Cargo.toml +++ b/clients/gateway-client/Cargo.toml @@ -13,6 +13,7 @@ chrono.workspace = true daft.workspace = true gateway-messages.workspace = true gateway-types.workspace = true +omicron-uuid-kinds.workspace = true progenitor.workspace = true rand.workspace = true reqwest = { workspace = true, features = ["rustls-tls", "stream"] } diff --git a/clients/gateway-client/src/lib.rs b/clients/gateway-client/src/lib.rs index 6b2d2e4fd46..df7b88c13b4 100644 --- a/clients/gateway-client/src/lib.rs +++ b/clients/gateway-client/src/lib.rs @@ -74,7 +74,10 @@ progenitor::generate_api!( SpUpdateStatus = { derives = [PartialEq, Hash, Eq] }, UpdatePreparationProgress = { derives = [PartialEq, Hash, Eq] }, }, - replace = { RotSlot = gateway_types::rot::RotSlot }, + replace = { + RotSlot = gateway_types::rot::RotSlot, + TypedUuidForMupdateKind = omicron_uuid_kinds::MupdateUuid, + }, ); // Override the impl of Ord for SpIdentifier because the default one orders the diff --git a/clients/installinator-client/Cargo.toml b/clients/installinator-client/Cargo.toml index ba869d79bd6..b12ad1b5fa8 100644 --- a/clients/installinator-client/Cargo.toml +++ b/clients/installinator-client/Cargo.toml @@ -10,6 +10,7 @@ workspace = true [dependencies] installinator-common.workspace = true omicron-common.workspace = true +omicron-uuid-kinds.workspace = true progenitor.workspace = true regress.workspace = true reqwest = { workspace = true, features = ["rustls-tls", "stream"] } @@ -18,5 +19,4 @@ serde.workspace = true serde_json.workspace = true slog.workspace = true update-engine.workspace = true -uuid.workspace = true omicron-workspace-hack.workspace = true diff --git a/clients/installinator-client/src/lib.rs b/clients/installinator-client/src/lib.rs index 05e9327396d..366677ae6fe 100644 --- a/clients/installinator-client/src/lib.rs +++ b/clients/installinator-client/src/lib.rs @@ -27,6 +27,7 @@ progenitor::generate_api!( ProgressEventForInstallinatorSpec = installinator_common::ProgressEvent, StepEventForGenericSpec = installinator_common::StepEvent, StepEventForInstallinatorSpec = installinator_common::StepEvent, + TypedUuidForMupdateKind = omicron_uuid_kinds::MupdateUuid, } ); diff --git a/common/src/update/mod.rs b/common/src/update/mod.rs index 12f91feb987..55291be242c 100644 --- a/common/src/update/mod.rs +++ b/common/src/update/mod.rs @@ -4,6 +4,8 @@ mod artifact_id; mod mupdate_override; +mod zone_manifest; pub use artifact_id::*; pub use mupdate_override::*; +pub use zone_manifest::*; diff --git a/common/src/update/mupdate_override.rs b/common/src/update/mupdate_override.rs index 2e641eb1791..bf3b6434072 100644 --- a/common/src/update/mupdate_override.rs +++ b/common/src/update/mupdate_override.rs @@ -6,10 +6,9 @@ use std::collections::BTreeSet; -use iddqd::{IdOrdItem, IdOrdMap, id_upcast}; use omicron_uuid_kinds::MupdateOverrideUuid; use serde::{Deserialize, Serialize}; -use tufaceous_artifact::{ArtifactHash, ArtifactHashId}; +use tufaceous_artifact::ArtifactHashId; /// MUPdate override information, typically serialized as JSON (RFD 556). /// @@ -23,43 +22,11 @@ pub struct MupdateOverrideInfo { /// Artifact hashes written out to the install dataset. /// /// Currently includes the host phase 2 and composite control plane - /// artifacts. Information about individual zones is included in - /// [`Self::zones`]. + /// artifacts. pub hash_ids: BTreeSet, - - /// Control plane zone file names and hashes. - pub zones: IdOrdMap, } impl MupdateOverrideInfo { /// The name of the file on the install dataset. pub const FILE_NAME: &'static str = "mupdate-override.json"; } - -/// Control plane zone information written out to the install dataset. -/// -/// Part of [`MupdateOverrideInfo`]. -#[derive( - Clone, Debug, Eq, Ord, PartialEq, PartialOrd, Deserialize, Serialize, -)] -pub struct MupdateOverrideZone { - /// The file name. - pub file_name: String, - - /// The file size. - pub file_size: u64, - - /// The hash of the file. - pub hash: ArtifactHash, -} - -impl IdOrdItem for MupdateOverrideZone { - type Key<'a> = &'a str; - - #[inline] - fn key(&self) -> Self::Key<'_> { - &self.file_name - } - - id_upcast!(); -} diff --git a/common/src/update/zone_manifest.rs b/common/src/update/zone_manifest.rs new file mode 100644 index 00000000000..c1ee4422e68 --- /dev/null +++ b/common/src/update/zone_manifest.rs @@ -0,0 +1,52 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use iddqd::{IdOrdItem, IdOrdMap, id_upcast}; +use omicron_uuid_kinds::MupdateUuid; +use serde::{Deserialize, Serialize}; +use tufaceous_artifact::ArtifactHash; + +/// Describes the set of Omicron zones written out into an install dataset. +#[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)] +pub struct OmicronZoneManifest { + /// The UUID of the mupdate which created this manifest. Intended primarily + /// for checking equality. + pub mupdate_id: MupdateUuid, + + /// Omicron zone file names and hashes. + pub zones: IdOrdMap, +} + +impl OmicronZoneManifest { + /// The name of the file. + pub const FILE_NAME: &str = "zones.json"; +} + +/// Information about an Omicron zone file written out to the install dataset. +/// +/// Part of [`OmicronZoneManifest`]. +#[derive( + Clone, Debug, Eq, Ord, PartialEq, PartialOrd, Deserialize, Serialize, +)] +pub struct OmicronZoneFileMetadata { + /// The file name. + pub file_name: String, + + /// The file size. + pub file_size: u64, + + /// The hash of the file. + pub hash: ArtifactHash, +} + +impl IdOrdItem for OmicronZoneFileMetadata { + type Key<'a> = &'a str; + + #[inline] + fn key(&self) -> Self::Key<'_> { + &self.file_name + } + + id_upcast!(); +} diff --git a/gateway-cli/Cargo.toml b/gateway-cli/Cargo.toml index cb0e47c82a0..048a8a29951 100644 --- a/gateway-cli/Cargo.toml +++ b/gateway-cli/Cargo.toml @@ -12,6 +12,7 @@ anyhow.workspace = true clap.workspace = true futures.workspace = true omicron-common.workspace = true +omicron-uuid-kinds.workspace = true reqwest.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/gateway-cli/src/main.rs b/gateway-cli/src/main.rs index 9267a0a8bc1..ae15cfc9988 100644 --- a/gateway-cli/src/main.rs +++ b/gateway-cli/src/main.rs @@ -19,6 +19,7 @@ use gateway_client::types::SpComponentFirmwareSlot; use gateway_client::types::SpIdentifier; use gateway_client::types::SpUpdateStatus; use gateway_client::types::UpdateAbortBody; +use omicron_uuid_kinds::MupdateUuid; use serde::Serialize; use slog::Drain; use slog::Level; @@ -148,7 +149,7 @@ enum Command { conflicts_with = "clear", required_unless_present = "clear" )] - update_id: Option, + update_id: Option, /// Hash of the host OS image. #[clap( long, diff --git a/gateway-types/Cargo.toml b/gateway-types/Cargo.toml index af50c7443cf..406f51c36e8 100644 --- a/gateway-types/Cargo.toml +++ b/gateway-types/Cargo.toml @@ -16,6 +16,7 @@ gateway-messages.workspace = true hex.workspace = true ipcc.workspace = true omicron-common.workspace = true +omicron-uuid-kinds.workspace = true omicron-workspace-hack.workspace = true schemars.workspace = true serde.workspace = true diff --git a/gateway-types/src/update.rs b/gateway-types/src/update.rs index c47065e9493..c3167628493 100644 --- a/gateway-types/src/update.rs +++ b/gateway-types/src/update.rs @@ -5,6 +5,7 @@ use std::time::Duration; use gateway_messages::UpdateStatus; +use omicron_uuid_kinds::MupdateUuid; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use tufaceous_artifact::ArtifactHash; @@ -95,7 +96,7 @@ impl From )] #[serde(rename_all = "snake_case")] pub struct InstallinatorImageId { - pub update_id: Uuid, + pub update_id: MupdateUuid, pub host_phase_2: ArtifactHash, pub control_plane: ArtifactHash, } diff --git a/installinator-api/Cargo.toml b/installinator-api/Cargo.toml index c35e9903f76..f5ef114d986 100644 --- a/installinator-api/Cargo.toml +++ b/installinator-api/Cargo.toml @@ -13,6 +13,7 @@ dropshot.workspace = true hyper.workspace = true installinator-common.workspace = true omicron-common.workspace = true +omicron-uuid-kinds.workspace = true omicron-workspace-hack.workspace = true schemars.workspace = true serde.workspace = true diff --git a/installinator-api/src/lib.rs b/installinator-api/src/lib.rs index 6a336d87510..cdde1c96a37 100644 --- a/installinator-api/src/lib.rs +++ b/installinator-api/src/lib.rs @@ -16,17 +16,17 @@ use dropshot::{ }; use hyper::header; use installinator_common::EventReport; +use omicron_uuid_kinds::MupdateUuid; use schemars::JsonSchema; use serde::Deserialize; use tufaceous_artifact::ArtifactHashId; -use uuid::Uuid; const PROGRESS_REPORT_MAX_BYTES: usize = 4 * 1024 * 1024; #[derive(Debug, Deserialize, JsonSchema)] pub struct ReportQuery { /// A unique identifier for the update. - pub update_id: Uuid, + pub update_id: MupdateUuid, } #[dropshot::api_description] @@ -95,7 +95,7 @@ impl EventReportStatus { /// Intended to be called by `report_progress` implementations. pub fn to_http_result( self, - update_id: Uuid, + update_id: MupdateUuid, ) -> Result { match self { EventReportStatus::Processed => Ok(HttpResponseUpdatedNoContent()), diff --git a/installinator-common/src/progress.rs b/installinator-common/src/progress.rs index 0c311580c63..3a3b9c316ae 100644 --- a/installinator-common/src/progress.rs +++ b/installinator-common/src/progress.rs @@ -297,6 +297,9 @@ pub enum ControlPlaneZonesStepId { /// Writing the MUPdate override file. MupdateOverride, + /// Writing the zone manifest. + ZoneManifest, + /// Syncing writes to disk. Fsync, diff --git a/installinator/Cargo.toml b/installinator/Cargo.toml index 5d524fd5826..053bda63b93 100644 --- a/installinator/Cargo.toml +++ b/installinator/Cargo.toml @@ -48,7 +48,6 @@ tokio = { workspace = true, features = ["full"] } tufaceous-artifact.workspace = true tufaceous-lib.workspace = true update-engine.workspace = true -uuid.workspace = true omicron-workspace-hack.workspace = true [dev-dependencies] diff --git a/installinator/src/artifact.rs b/installinator/src/artifact.rs index 30b70c4d010..695e395c1bb 100644 --- a/installinator/src/artifact.rs +++ b/installinator/src/artifact.rs @@ -10,9 +10,9 @@ use futures::StreamExt; use installinator_client::ClientError; use installinator_common::EventReport; use ipcc::{InstallinatorImageId, Ipcc}; +use omicron_uuid_kinds::MupdateUuid; use tokio::sync::mpsc; use tufaceous_artifact::{ArtifactHash, ArtifactHashId}; -use uuid::Uuid; use crate::{errors::HttpError, fetch::FetchReceiver}; @@ -27,7 +27,7 @@ pub(crate) struct ArtifactIdOpts { conflicts_with = "from_ipcc", required_unless_present = "from_ipcc" )] - update_id: Option, + update_id: Option, #[clap( long, @@ -134,7 +134,7 @@ impl ArtifactClient { pub(crate) async fn report_progress( &self, - update_id: Uuid, + update_id: MupdateUuid, report: EventReport, ) -> Result<(), ClientError> { self.client diff --git a/installinator/src/dispatch.rs b/installinator/src/dispatch.rs index e9bf8a9c031..f3504d94c65 100644 --- a/installinator/src/dispatch.rs +++ b/installinator/src/dispatch.rs @@ -351,6 +351,7 @@ impl InstallOpts { control_plane_zones.into_value(cx.token()).await; let mut writer = ArtifactWriter::new( + image_id.update_id, &host_2_phase_id_2, &host_phase_2_artifact.artifact, &control_plane_id_2, diff --git a/installinator/src/mock_peers.rs b/installinator/src/mock_peers.rs index 9d6c06c14e1..1416d9cc5b0 100644 --- a/installinator/src/mock_peers.rs +++ b/installinator/src/mock_peers.rs @@ -19,13 +19,13 @@ use async_trait::async_trait; use bytes::Bytes; use installinator_client::{ClientError, ResponseValue}; use installinator_common::EventReport; +use omicron_uuid_kinds::MupdateUuid; use proptest::{collection::vec_deque, prelude::*}; use reqwest::StatusCode; use test_strategy::Arbitrary; use tokio::sync::mpsc; use tufaceous_artifact::ArtifactHashId; use update_engine::events::StepEventIsTerminal; -use uuid::Uuid; use crate::{ errors::{DiscoverPeersError, HttpError}, @@ -458,7 +458,7 @@ impl ResponseAction_ { /// In the future, this will be combined with `MockPeers` so we can model. #[derive(Debug)] struct MockProgressBackend { - update_id: Uuid, + update_id: MupdateUuid, // Use an unbounded sender to avoid async code in handle_valid_peer_event. report_sender: mpsc::UnboundedSender, behaviors: Mutex, @@ -481,7 +481,7 @@ impl MockProgressBackend { )); fn new( - update_id: Uuid, + update_id: MupdateUuid, report_sender: mpsc::UnboundedSender, behaviors: ReportBehaviors, ) -> Self { @@ -574,7 +574,7 @@ impl ReportProgressImpl for MockProgressBackend { async fn report_progress_impl( &self, peer: PeerAddress, - update_id: Uuid, + update_id: MupdateUuid, report: EventReport, ) -> Result<(), ClientError> { assert_eq!(update_id, self.update_id, "update ID matches"); @@ -712,8 +712,8 @@ mod tests { universe: MockPeersUniverse, #[strategy((0..2000u64).prop_map(Duration::from_millis))] timeout: Duration, - #[strategy(any::<[u8; 16]>().prop_map(Uuid::from_bytes))] - update_id: Uuid, + #[strategy(any::<[u8; 16]>().prop_map(MupdateUuid::from_bytes))] + update_id: MupdateUuid, valid_peer_behaviors: ReportBehaviors, ) { with_test_runtime(async move { diff --git a/installinator/src/reporter.rs b/installinator/src/reporter.rs index 12445d75bc6..f3e0cd76151 100644 --- a/installinator/src/reporter.rs +++ b/installinator/src/reporter.rs @@ -18,13 +18,13 @@ use display_error_chain::DisplayErrorChain; use http::StatusCode; use installinator_client::ClientError; use installinator_common::{Event, EventBuffer, EventReport}; +use omicron_uuid_kinds::MupdateUuid; use tokio::{ sync::{mpsc, watch}, task::JoinHandle, time, }; use update_engine::AsError; -use uuid::Uuid; use crate::{ artifact::ArtifactClient, @@ -56,7 +56,7 @@ const DISCOVER_INTERVAL: Duration = Duration::from_secs(5); #[derive(Debug)] pub(crate) struct ProgressReporter { log: slog::Logger, - update_id: Uuid, + update_id: MupdateUuid, report_backend: ReportProgressBackend, // Receives updates about progress and completion. event_receiver: mpsc::Receiver, @@ -69,7 +69,7 @@ pub(crate) struct ProgressReporter { impl ProgressReporter { pub(crate) fn new( log: &slog::Logger, - update_id: Uuid, + update_id: MupdateUuid, report_backend: ReportProgressBackend, ) -> (Self, mpsc::Sender) { let (event_sender, event_receiver) = update_engine::channel(); @@ -377,7 +377,7 @@ struct ReportTask { impl ReportTask { fn new( peer: PeerAddress, - update_id: Uuid, + update_id: MupdateUuid, report_rx: watch::Receiver>, backend: ReportProgressBackend, ) -> ReportTask { @@ -416,7 +416,7 @@ impl ReportTask { async fn report_task_loop( log: &slog::Logger, peer: PeerAddress, - update_id: Uuid, + update_id: MupdateUuid, mut report_rx: watch::Receiver>, backend: ReportProgressBackend, ) { @@ -523,7 +523,7 @@ impl ReportProgressBackend { pub(crate) async fn send_report_to_peer( &self, peer: PeerAddress, - update_id: Uuid, + update_id: MupdateUuid, report: EventReport, ) -> Result { let log = self.log.new(slog::o!("peer" => peer.to_string())); @@ -586,7 +586,7 @@ pub(crate) trait ReportProgressImpl: fmt::Debug + Send + Sync { async fn report_progress_impl( &self, peer: PeerAddress, - update_id: Uuid, + update_id: MupdateUuid, report: EventReport, ) -> Result<(), ClientError>; } @@ -618,7 +618,7 @@ impl ReportProgressImpl for HttpProgressBackend { async fn report_progress_impl( &self, peer: PeerAddress, - update_id: Uuid, + update_id: MupdateUuid, report: EventReport, ) -> Result<(), ClientError> { let artifact_client = ArtifactClient::new(peer.address(), &self.log); diff --git a/installinator/src/write.rs b/installinator/src/write.rs index e6a0860d608..87a6acb8482 100644 --- a/installinator/src/write.rs +++ b/installinator/src/write.rs @@ -23,9 +23,11 @@ use installinator_common::{ }; use omicron_common::{ disk::M2Slot, - update::{MupdateOverrideInfo, MupdateOverrideZone}, + update::{ + MupdateOverrideInfo, OmicronZoneFileMetadata, OmicronZoneManifest, + }, }; -use omicron_uuid_kinds::MupdateOverrideUuid; +use omicron_uuid_kinds::{MupdateOverrideUuid, MupdateUuid}; use sha2::{Digest, Sha256}; use slog::{Logger, info, warn}; use tokio::{ @@ -206,6 +208,7 @@ pub(crate) struct ArtifactWriter<'a> { impl<'a> ArtifactWriter<'a> { pub(crate) fn new( + mupdate_id: MupdateUuid, host_phase_2_id: &'a ArtifactHashId, host_phase_2_data: &'a BufList, control_plane_id: &'a ArtifactHashId, @@ -222,7 +225,7 @@ impl<'a> ArtifactWriter<'a> { // At the moment, there's no reason to have it be passed in via Wicket // or some other means, though there are conceivably reasons to do so in // the future. - let mupdate_uuid = MupdateOverrideUuid::new_v4(); + let mupdate_override_uuid = MupdateOverrideUuid::new_v4(); Self { drives, @@ -233,7 +236,8 @@ impl<'a> ArtifactWriter<'a> { host_phase_2_data, control_plane_id, control_plane_zones, - mupdate_uuid, + mupdate_id, + mupdate_override_uuid, }, } } @@ -539,7 +543,8 @@ struct ArtifactsToWrite<'a> { host_phase_2_data: &'a BufList, control_plane_id: &'a ArtifactHashId, control_plane_zones: &'a ControlPlaneZoneImages, - mupdate_uuid: MupdateOverrideUuid, + mupdate_id: MupdateUuid, + mupdate_override_uuid: MupdateOverrideUuid, } impl ArtifactsToWrite<'_> { @@ -587,7 +592,8 @@ impl ArtifactsToWrite<'_> { zones: self.control_plane_zones, host_phase_2_id: self.host_phase_2_id, control_plane_id: self.control_plane_id, - mupdate_uuid: self.mupdate_uuid, + mupdate_id: self.mupdate_id, + mupdate_override_uuid: self.mupdate_override_uuid, }; cx.with_nested_engine(|engine| { inner_cx.register_steps( @@ -623,7 +629,8 @@ struct ControlPlaneZoneWriteContext<'a> { zones: &'a ControlPlaneZoneImages, host_phase_2_id: &'a ArtifactHashId, control_plane_id: &'a ArtifactHashId, - mupdate_uuid: MupdateOverrideUuid, + mupdate_id: MupdateUuid, + mupdate_override_uuid: MupdateOverrideUuid, } impl ControlPlaneZoneWriteContext<'_> { @@ -636,7 +643,7 @@ impl ControlPlaneZoneWriteContext<'_> { use update_engine::StepHandle; let slot = self.slot; - let mupdate_uuid = self.mupdate_uuid; + let mupdate_override_uuid = self.mupdate_override_uuid; // If we're on a gimlet, remove any files in the control plane // destination directory. @@ -684,7 +691,7 @@ impl ControlPlaneZoneWriteContext<'_> { async move |cx| { let transport = transport.into_value(cx.token()).await; let mupdate_json = - self.mupdate_override_artifact(mupdate_uuid).await; + self.mupdate_override_artifact(mupdate_override_uuid); let out_path = self .output_directory @@ -702,7 +709,42 @@ impl ControlPlaneZoneWriteContext<'_> { StepSuccess::new(transport) .with_message(format!( - "{out_path} written with UUID: {mupdate_uuid}", + "{out_path} written with mupdate override UUID: \ + {mupdate_override_uuid}", + )) + .into() + }, + ) + .register(); + + transport = engine + .new_step( + WriteComponent::ControlPlane, + ControlPlaneZonesStepId::ZoneManifest, + "Writing zone manifest", + async move |cx| { + let transport = transport.into_value(cx.token()).await; + let zone_manifest_json = + self.omicron_zone_manifest_artifact().await; + + let out_path = self + .output_directory + .join(OmicronZoneManifest::FILE_NAME); + + write_artifact_impl( + WriteComponent::ControlPlane, + slot, + zone_manifest_json, + &out_path, + transport, + &cx, + ) + .await?; + + StepSuccess::new(transport) + .with_message(format!( + "{out_path} written with mupdate UUID: {}", + self.mupdate_id, )) .into() }, @@ -765,22 +807,33 @@ impl ControlPlaneZoneWriteContext<'_> { .register(); } - async fn mupdate_override_artifact( + fn mupdate_override_artifact( &self, - mupdate_uuid: MupdateOverrideUuid, + mupdate_override_uuid: MupdateOverrideUuid, ) -> BufList { let hash_ids = [self.host_phase_2_id.clone(), self.control_plane_id.clone()] .into_iter() .collect(); - let zones = compute_zone_hashes(&self.zones).await; - let mupdate_override = - MupdateOverrideInfo { mupdate_uuid, hash_ids, zones }; + let mupdate_override = MupdateOverrideInfo { + mupdate_uuid: mupdate_override_uuid, + hash_ids, + }; let json_bytes = serde_json::to_vec(&mupdate_override) .expect("this serialization is infallible"); BufList::from(json_bytes) } + + async fn omicron_zone_manifest_artifact(&self) -> BufList { + let zones = compute_zone_hashes(&self.zones).await; + + let omicron_zone_manifest = + OmicronZoneManifest { mupdate_id: self.mupdate_id, zones }; + let json_bytes = serde_json::to_vec(&omicron_zone_manifest) + .expect("this serialization is infallible"); + BufList::from(json_bytes) + } } /// Computes the zone hash IDs. @@ -792,7 +845,7 @@ impl ControlPlaneZoneWriteContext<'_> { /// Panics if the runtime shuts down causing a task abort, or a task panics. async fn compute_zone_hashes( images: &ControlPlaneZoneImages, -) -> IdOrdMap { +) -> IdOrdMap { let mut tasks = JoinSet::new(); for (file_name, data) in &images.zones { let file_name = file_name.clone(); @@ -803,7 +856,7 @@ async fn compute_zone_hashes( let mut hasher = Sha256::new(); hasher.update(&data); let hash = hasher.finalize(); - MupdateOverrideZone { + OmicronZoneFileMetadata { file_name, file_size: u64::try_from(data.len()).unwrap(), hash: ArtifactHash(hash.into()), @@ -1232,6 +1285,7 @@ mod tests { }; let mut writer = ArtifactWriter::new( + MupdateUuid::new_v4(), &host_id, &artifact_host, &control_plane_id, diff --git a/ipcc/Cargo.toml b/ipcc/Cargo.toml index 3f2a374cb2b..8008aa5c4fb 100644 --- a/ipcc/Cargo.toml +++ b/ipcc/Cargo.toml @@ -10,10 +10,10 @@ workspace = true [dependencies] ciborium.workspace = true omicron-common.workspace = true +omicron-uuid-kinds.workspace = true serde.workspace = true tufaceous-artifact.workspace = true thiserror.workspace = true -uuid.workspace = true omicron-workspace-hack.workspace = true libipcc.workspace = true diff --git a/ipcc/src/lib.rs b/ipcc/src/lib.rs index 1d29e3162b3..d5cd93aa445 100644 --- a/ipcc/src/lib.rs +++ b/ipcc/src/lib.rs @@ -10,11 +10,11 @@ //! (through MGS) or set from userland via libipcc. use libipcc::{IpccError, IpccHandle}; +use omicron_uuid_kinds::MupdateUuid; use serde::Deserialize; use serde::Serialize; use thiserror::Error; use tufaceous_artifact::ArtifactHash; -use uuid::Uuid; #[cfg(test)] use proptest::arbitrary::any; @@ -46,8 +46,8 @@ pub struct InstallinatorImageId { /// Installinator can send progress and completion messages to all peers it /// finds, and any peer that cares about progress can use this ID to match /// the progress message with a running recovery. - #[cfg_attr(test, strategy(any::<[u8; 16]>().prop_map(Uuid::from_bytes)))] - pub update_id: Uuid, + #[cfg_attr(test, strategy(any::<[u8; 16]>().prop_map(MupdateUuid::from_bytes)))] + pub update_id: MupdateUuid, /// SHA-256 hash of the host phase 2 image to fetch. pub host_phase_2: ArtifactHash, /// SHA-256 hash of the control plane image to fetch. @@ -178,6 +178,7 @@ impl Ipcc { #[cfg(test)] mod tests { use super::*; + use omicron_uuid_kinds::MupdateUuid; use test_strategy::proptest; #[proptest] @@ -297,7 +298,7 @@ mod tests { ]; let expected = InstallinatorImageId { - update_id: Uuid::from_bytes([ + update_id: MupdateUuid::from_bytes([ 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, ]), host_phase_2: ArtifactHash([ diff --git a/openapi/gateway.json b/openapi/gateway.json index 4e5f68a2af9..b103bad4101 100644 --- a/openapi/gateway.json +++ b/openapi/gateway.json @@ -1933,8 +1933,7 @@ "format": "hex string (32 bytes)" }, "update_id": { - "type": "string", - "format": "uuid" + "$ref": "#/components/schemas/TypedUuidForMupdateKind" } }, "required": [ @@ -3719,6 +3718,10 @@ "type": "string", "format": "uuid" }, + "TypedUuidForMupdateKind": { + "type": "string", + "format": "uuid" + }, "UpdateAbortBody": { "type": "object", "properties": { diff --git a/openapi/installinator.json b/openapi/installinator.json index 6419760fbd7..3dd7cecee78 100644 --- a/openapi/installinator.json +++ b/openapi/installinator.json @@ -65,8 +65,7 @@ "description": "A unique identifier for the update.", "required": true, "schema": { - "type": "string", - "format": "uuid" + "$ref": "#/components/schemas/TypedUuidForMupdateKind" } } ], @@ -2310,6 +2309,10 @@ "slots_attempted", "slots_written" ] + }, + "TypedUuidForMupdateKind": { + "type": "string", + "format": "uuid" } }, "responses": { diff --git a/sled-agent/zone-images/Cargo.toml b/sled-agent/zone-images/Cargo.toml index ba0a5247496..b7dca7f87f2 100644 --- a/sled-agent/zone-images/Cargo.toml +++ b/sled-agent/zone-images/Cargo.toml @@ -16,6 +16,7 @@ nexus-sled-agent-shared.workspace = true omicron-common.workspace = true omicron-workspace-hack.workspace = true rayon.workspace = true +serde.workspace = true serde_json.workspace = true sha2.workspace = true sled-storage.workspace = true diff --git a/sled-agent/zone-images/src/errors.rs b/sled-agent/zone-images/src/errors.rs new file mode 100644 index 00000000000..3ef2a94b348 --- /dev/null +++ b/sled-agent/zone-images/src/errors.rs @@ -0,0 +1,46 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use std::{io, sync::Arc}; +use thiserror::Error; + +/// An `io::Error` wrapper that implements `Clone` and `PartialEq`. +#[derive(Clone, Debug, Error)] +#[error(transparent)] +pub struct ArcIoError(pub Arc); + +impl ArcIoError { + pub(crate) fn new(error: io::Error) -> Self { + Self(Arc::new(error)) + } +} + +/// Testing aid. +impl PartialEq for ArcIoError { + fn eq(&self, other: &Self) -> bool { + // Simply comparing io::ErrorKind is good enough for tests. + self.0.kind() == other.0.kind() + } +} + +/// A `serde_json::Error` that implements `Clone` and `PartialEq`. +#[derive(Clone, Debug, Error)] +#[error(transparent)] +pub struct ArcSerdeJsonError(pub Arc); + +impl ArcSerdeJsonError { + pub(crate) fn new(error: serde_json::Error) -> Self { + Self(Arc::new(error)) + } +} + +/// Testing aid. +impl PartialEq for ArcSerdeJsonError { + fn eq(&self, other: &Self) -> bool { + // Simply comparing line/column/category is good enough for tests. + self.0.line() == other.0.line() + && self.0.column() == other.0.column() + && self.0.classify() == other.0.classify() + } +} diff --git a/sled-agent/zone-images/src/install_dataset_metadata.rs b/sled-agent/zone-images/src/install_dataset_metadata.rs new file mode 100644 index 00000000000..79022224999 --- /dev/null +++ b/sled-agent/zone-images/src/install_dataset_metadata.rs @@ -0,0 +1,287 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Generic code to read a metadata file from an install dataset. + +use camino::{Utf8Path, Utf8PathBuf}; +use iddqd::{IdOrdItem, IdOrdMap, id_upcast}; +use illumos_utils::zpool::ZpoolName; +use serde::de::DeserializeOwned; +use sled_storage::dataset::INSTALL_DATASET; +use std::fs::{self, FileType}; +use thiserror::Error; + +use crate::{ArcIoError, ArcSerdeJsonError, ZoneImageZpools}; + +#[derive(Debug)] +pub(crate) struct AllInstallMetadataFiles { + pub(crate) boot_zpool: ZpoolName, + pub(crate) boot_dataset_dir: Utf8PathBuf, + pub(crate) boot_disk_path: Utf8PathBuf, + pub(crate) boot_disk_metadata: Result, InstallMetadataReadError>, + pub(crate) non_boot_disk_metadata: IdOrdMap>, +} + +impl AllInstallMetadataFiles +where + T: DeserializeOwned + PartialEq, +{ + /// Attempt to find manifest files. + /// + /// For now we treat the boot disk as authoritative, since install-dataset + /// artifacts are always served from the boot disk. There is a class of + /// issues here related to transient failures on one of the M.2s that we're + /// acknowledging but not tackling for now. + /// + /// In general, this API follows an interpreter pattern: first read all the + /// results and put them in a map, then make decisions based on them in a + /// separate step. This enables better testing and ensures that changes to + /// behavior are described in the type system. + pub(crate) fn read_all( + _log: &slog::Logger, + metadata_file_name: &str, + zpools: &ZoneImageZpools<'_>, + boot_zpool: &ZpoolName, + ) -> Self { + let boot_dataset_dir = + boot_zpool.dataset_mountpoint(zpools.root, INSTALL_DATASET); + + // Read the file from the boot disk. + let boot_disk_path = boot_dataset_dir.join(metadata_file_name); + let boot_disk_metadata = + read_install_metadata_file::(&boot_dataset_dir, &boot_disk_path); + + // Read the file from all other disks. We attempt to make sure they match up + // and will log a warning if they don't, though (until we have a better + // story on transient failures) it's not fatal. + let non_boot_zpools = zpools + .all_m2_zpools + .iter() + .filter(|&zpool_name| zpool_name != boot_zpool); + let non_boot_disk_overrides = non_boot_zpools + .map(|zpool_name| { + let dataset_dir = + zpool_name.dataset_mountpoint(zpools.root, INSTALL_DATASET); + + let path = dataset_dir.join(metadata_file_name); + let res = read_install_metadata_file(&dataset_dir, &path); + let result = + InstallMetadataNonBootResult::new(res, &boot_disk_metadata); + + InstallMetadataNonBootInfo { + zpool_name: *zpool_name, + dataset_dir, + path, + result, + } + }) + .collect(); + + Self { + boot_zpool: *boot_zpool, + boot_dataset_dir, + boot_disk_path, + boot_disk_metadata, + non_boot_disk_metadata: non_boot_disk_overrides, + } + } +} + +pub(crate) fn read_install_metadata_file( + dataset_dir: &Utf8Path, + metadata_path: &Utf8Path, +) -> Result, InstallMetadataReadError> +where + T: DeserializeOwned, +{ + // First check that the dataset directory exists. This distinguishes the + // two cases: + // + // 1. The install dataset is missing (an error). + // 2. The install dataset is present, but the metadata file is missing + // (expected). + // + // It would be nice if we could use openat-style APIs to read the file + // from the opened directory, but: + // + // * those don't exist in rust std + // * it's not crucial -- we don't expect TOCTTOU races much in this code + // path, and we're not generally resilient to them anyway. + // + // We use symlink_metadata (lstat) rather than metadata (stat) because + // there really shouldn't be any symlinks involved. + let dir_metadata = fs::symlink_metadata(dataset_dir).map_err(|error| { + InstallMetadataReadError::DatasetDirMetadata { + dataset_dir: dataset_dir.to_owned(), + error: ArcIoError::new(error), + } + })?; + if !dir_metadata.is_dir() { + return Err(InstallMetadataReadError::DatasetNotDirectory { + dataset_dir: dataset_dir.to_owned(), + file_type: dir_metadata.file_type(), + }); + } + + let metadata = match fs::read_to_string(metadata_path) { + Ok(data) => { + let metadata = + serde_json::from_str::(&data).map_err(|error| { + InstallMetadataReadError::Deserialize { + path: metadata_path.to_owned(), + error: ArcSerdeJsonError::new(error), + contents: data, + } + })?; + Some(metadata) + } + Err(error) => { + if error.kind() == std::io::ErrorKind::NotFound { + None + } else { + return Err(InstallMetadataReadError::Read { + path: metadata_path.to_owned(), + error: ArcIoError::new(error), + }); + } + } + }; + + Ok(metadata) +} + +#[derive(Clone, Debug, PartialEq)] +pub struct InstallMetadataNonBootInfo { + /// The name of the zpool. + pub zpool_name: ZpoolName, + + /// The dataset directory. + pub dataset_dir: Utf8PathBuf, + + /// The path that was read from. + pub path: Utf8PathBuf, + + /// The result of performing the read operation. + pub result: InstallMetadataNonBootResult, +} + +impl IdOrdItem for InstallMetadataNonBootInfo { + type Key<'a> + = ZpoolName + where + T: 'a; + + fn key(&self) -> Self::Key<'_> { + self.zpool_name + } + + id_upcast!(); +} + +#[derive(Clone, Debug, PartialEq)] +pub enum InstallMetadataNonBootResult { + /// The file is present and matches the value on the boot disk. + MatchesPresent(T), + + /// The file is absent and is also absent on the boot disk. + MatchesAbsent, + + /// A mismatch between the boot disk and the other disk was detected. + Mismatch(InstallMetadataNonBootMismatch), + + /// An error occurred while reading the mupdate override info on this disk. + ReadError(InstallMetadataReadError), +} + +impl InstallMetadataNonBootResult { + pub(crate) fn new( + res: Result, InstallMetadataReadError>, + boot_disk_res: &Result, InstallMetadataReadError>, + ) -> Self { + match (res, boot_disk_res) { + (Ok(Some(non_boot_disk_info)), Ok(Some(boot_disk_info))) => { + if boot_disk_info == &non_boot_disk_info { + Self::MatchesPresent(non_boot_disk_info) + } else { + Self::Mismatch( + InstallMetadataNonBootMismatch::ValueMismatch { + non_boot_disk_info, + }, + ) + } + } + (Ok(Some(non_boot_disk_info)), Ok(None)) => Self::Mismatch( + InstallMetadataNonBootMismatch::BootAbsentOtherPresent { + non_boot_disk_info, + }, + ), + (Ok(None), Ok(Some(_))) => Self::Mismatch( + InstallMetadataNonBootMismatch::BootPresentOtherAbsent, + ), + (Ok(None), Ok(None)) => Self::MatchesAbsent, + (Ok(non_boot_disk_info), Err(_)) => Self::Mismatch( + InstallMetadataNonBootMismatch::BootDiskReadError { + non_boot_disk_info, + }, + ), + (Err(error), _) => Self::ReadError(error), + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum InstallMetadataNonBootMismatch { + /// The file is present on the boot disk but absent on the other disk. + BootPresentOtherAbsent, + + /// The file is absent on the boot disk but present on the other disk. + BootAbsentOtherPresent { + /// The information found on the other disk. + non_boot_disk_info: T, + }, + + /// The file's contents differ between the boot disk and the other disk. + ValueMismatch { non_boot_disk_info: T }, + + /// There was a read error on the boot disk, so we were unable to verify + /// consistency. + BootDiskReadError { + /// The value as found on this disk. This value is logged but not used. + non_boot_disk_info: Option, + }, +} + +#[derive(Clone, Debug, PartialEq, Error)] +pub enum InstallMetadataReadError { + #[error( + "error retrieving metadata for install dataset directory \ + `{dataset_dir}`" + )] + DatasetDirMetadata { + dataset_dir: Utf8PathBuf, + #[source] + error: ArcIoError, + }, + + #[error( + "expected install dataset `{dataset_dir}` to be a directory, \ + found {file_type:?}" + )] + DatasetNotDirectory { dataset_dir: Utf8PathBuf, file_type: FileType }, + + #[error("error reading metadata file from `{path}`")] + Read { + path: Utf8PathBuf, + #[source] + error: ArcIoError, + }, + + #[error("error deserializing `{path}`, contents: {contents:?}")] + Deserialize { + path: Utf8PathBuf, + contents: String, + #[source] + error: ArcSerdeJsonError, + }, +} diff --git a/sled-agent/zone-images/src/lib.rs b/sled-agent/zone-images/src/lib.rs index a35f20d6cdd..a4414db79bf 100644 --- a/sled-agent/zone-images/src/lib.rs +++ b/sled-agent/zone-images/src/lib.rs @@ -7,8 +7,16 @@ //! This contains a subset of zone image code at the moment: you're encouraged //! to move more code into this crate as appropriate. +mod errors; +mod install_dataset_metadata; mod mupdate_override; mod source_resolver; +#[cfg(test)] +mod test_utils; +mod zone_manifest; +pub use errors::*; +pub use install_dataset_metadata::*; pub use mupdate_override::*; pub use source_resolver::*; +pub use zone_manifest::*; diff --git a/sled-agent/zone-images/src/mupdate_override.rs b/sled-agent/zone-images/src/mupdate_override.rs index 501cf8a36a8..956af882069 100644 --- a/sled-agent/zone-images/src/mupdate_override.rs +++ b/sled-agent/zone-images/src/mupdate_override.rs @@ -6,58 +6,42 @@ //! //! For more about commingling MUPdate and update, see RFD 556. -use std::fmt; -use std::fs; -use std::fs::File; -use std::fs::FileType; -use std::io; -use std::io::Read; -use std::sync::Arc; - +use crate::AllInstallMetadataFiles; +use crate::InstallMetadataNonBootInfo; +use crate::InstallMetadataNonBootMismatch; +use crate::InstallMetadataNonBootResult; +use crate::InstallMetadataReadError; use crate::ZoneImageZpools; -use camino::Utf8Path; use camino::Utf8PathBuf; use iddqd::IdOrdItem; use iddqd::IdOrdMap; use iddqd::id_upcast; use illumos_utils::zpool::ZpoolName; use omicron_common::update::MupdateOverrideInfo; -use omicron_common::update::MupdateOverrideZone; -use rayon::iter::ParallelBridge; -use rayon::iter::ParallelIterator; -use sha2::Digest; -use sha2::Sha256; -use sled_storage::dataset::INSTALL_DATASET; -use slog::debug; use slog::error; use slog::info; use slog::o; use slog::warn; use slog_error_chain::InlineErrorChain; use thiserror::Error; -use tufaceous_artifact::ArtifactHash; /// Describes the current state of mupdate overrides. #[derive(Clone, Debug)] pub struct MupdateOverrideStatus { - /// The boot zpool. - pub boot_zpool: ZpoolName, - - /// The boot disk path. + /// The path to the mupdate override JSON on the boot disk. pub boot_disk_path: Utf8PathBuf, /// Status of the boot disk. pub boot_disk_override: Result, MupdateOverrideReadError>, - /// Status of the non-boot disks. This results in warnings. + /// Status of the non-boot disks. This results in warnings in case of a + /// mismatch. pub non_boot_disk_overrides: IdOrdMap, } #[derive(Debug)] pub(crate) struct AllMupdateOverrides { - // This is internal-only, and currently a duplicate of MupdateOverrideStatus - // in case we need to store more information here in the future. boot_zpool: ZpoolName, boot_disk_path: Utf8PathBuf, boot_disk_override: @@ -69,61 +53,32 @@ impl AllMupdateOverrides { /// Attempt to find MUPdate override files. If present, this file will cause /// install-dataset artifacts to be used even if the image source is Artifact. /// - /// For now we treat the boot disk as authoritative, since install-dataset - /// artifacts are always served from the boot disk. There is a class of issues - /// here related to transient failures on one of the M.2s that we're - /// acknowledging but not tackling for now. - /// - /// In general, this API follows an interpreter pattern: first read all the - /// results and put them in a map, then make decisions based on them in a - /// separate step. This enables better testing and ensures that changes to - /// behavior are described in the type system. - /// /// For more about commingling MUPdate and update, see RFD 556. - /// - /// TODO: This is somewhat complex error handling logic that's similar to, - /// but different from, `Ledgerable` (for example, it only does an equality - /// check, not an ordering check, and it always considers the boot disk to - /// be authoritative). Consider extracting this out into something generic. pub(crate) fn read_all( log: &slog::Logger, zpools: &ZoneImageZpools<'_>, boot_zpool: &ZpoolName, ) -> Self { - let dataset = - boot_zpool.dataset_mountpoint(zpools.root, INSTALL_DATASET); - - let (boot_disk_path, boot_disk_res) = - read_mupdate_override(log, &dataset); - - // Now read the file from all other disks. We attempt to make sure they - // match up and will log a warning if they don't, though (until we have - // a better story on transient failures) it's not fatal. - let non_boot_zpools = zpools - .all_m2_zpools - .iter() - .filter(|&zpool_name| zpool_name != boot_zpool); - let non_boot_disk_overrides = non_boot_zpools - .map(|zpool_name| { - let dataset = - zpool_name.dataset_mountpoint(zpools.root, INSTALL_DATASET); - - let (path, res) = read_mupdate_override(log, &dataset); - MupdateOverrideNonBootInfo { - zpool_name: *zpool_name, - path, - result: MupdateOverrideNonBootResult::new( - res, - &boot_disk_res, - ), - } - }) + let files = AllInstallMetadataFiles::::read_all( + log, + MupdateOverrideInfo::FILE_NAME, + zpools, + boot_zpool, + ); + + let boot_disk_override = files + .boot_disk_metadata + .map_err(MupdateOverrideReadError::InstallMetadata); + let non_boot_disk_overrides = files + .non_boot_disk_metadata + .into_iter() + .map(MupdateOverrideNonBootInfo::new) .collect(); let ret = Self { - boot_zpool: *boot_zpool, - boot_disk_path, - boot_disk_override: boot_disk_res, + boot_zpool: files.boot_zpool, + boot_disk_path: files.boot_disk_path, + boot_disk_override, non_boot_disk_overrides, }; @@ -133,7 +88,6 @@ impl AllMupdateOverrides { pub(crate) fn status(&self) -> MupdateOverrideStatus { MupdateOverrideStatus { - boot_zpool: self.boot_zpool, boot_disk_path: self.boot_disk_path.clone(), boot_disk_override: self.boot_disk_override.clone(), non_boot_disk_overrides: self.non_boot_disk_overrides.clone(), @@ -142,6 +96,7 @@ impl AllMupdateOverrides { fn log_results(&self, log: &slog::Logger) { let log = log.new(o!( + "component" => "mupdate_override", "boot_zpool" => self.boot_zpool.to_string(), "boot_disk_path" => self.boot_disk_path.to_string(), )); @@ -184,142 +139,88 @@ impl AllMupdateOverrides { } } -fn read_mupdate_override( - log: &slog::Logger, - dataset_dir: &Utf8Path, -) -> (Utf8PathBuf, Result, MupdateOverrideReadError>) -{ - let override_path = dataset_dir.join(MupdateOverrideInfo::FILE_NAME); - - fn inner( - log: &slog::Logger, - dataset_dir: &Utf8Path, - override_path: &Utf8Path, - ) -> Result, MupdateOverrideReadError> { - // First check that the dataset directory exists. This distinguishes the - // two cases: - // - // 1. The install dataset is missing (an error). - // 2. The install dataset is present, but the override file is missing - // (expected). - // - // It would be nice if we could use openat-style APIs to read the file - // from the opened directory, but: - // - // * those don't exist in rust std - // * it's not crucial -- we don't expect TOCTTOU races much in this code - // path, and we're not generally resilient to them anyway. - // - // We use symlink_metadata (lstat) rather than metadata (stat) because - // there really shouldn't be any symlinks involved. - let dir_metadata = - fs::symlink_metadata(dataset_dir).map_err(|error| { - MupdateOverrideReadError::DatasetDirMetadata { - dataset_dir: dataset_dir.to_owned(), - error: ArcIoError::new(error), - } - })?; - if !dir_metadata.is_dir() { - return Err(MupdateOverrideReadError::DatasetNotDirectory { - dataset_dir: dataset_dir.to_owned(), - file_type: dir_metadata.file_type(), - }); - } - - let mupdate_override = match std::fs::read_to_string(&override_path) { - Ok(data) => { - let data = serde_json::from_str::(&data) - .map_err(|error| { - MupdateOverrideReadError::Deserialize { - path: override_path.to_owned(), - error: ArcSerdeJsonError::new(error), - contents: data, - } - })?; - let artifacts = - MupdateOverrideArtifactsResult::new(dataset_dir, data); - if artifacts.is_valid() { - // If there are errors, return them as appropriate. - Some(artifacts.info) - } else { - // At least one artifact was invalid: return an error. - // - // XXX: Should we be more fine-grained than this, handle - // errors on a per-artifact basis? Seems excessive. - return Err(MupdateOverrideReadError::ArtifactRead { - dataset_dir: dataset_dir.to_owned(), - artifacts, - }); - } - } - Err(error) => { - if error.kind() == std::io::ErrorKind::NotFound { - debug!( - log, - "mupdate override file not found, treating as absent"; - "path" => %override_path - ); - None - } else { - return Err(MupdateOverrideReadError::Read { - path: override_path.to_owned(), - error: ArcIoError::new(error), - }); - } - } - }; - - Ok(mupdate_override) - } - - let res = inner(log, dataset_dir, &override_path); - (override_path, res) -} - +/// Describes the result of reading a mupdate override file from a non-boot disk. #[derive(Clone, Debug, PartialEq)] pub struct MupdateOverrideNonBootInfo { - /// The name of the zpool. + /// The zpool name. pub zpool_name: ZpoolName, - /// The path that was read from. + /// The path to the mupdate override file. pub path: Utf8PathBuf, - /// The result of performing the read operation. + /// The result of reading the mupdate override file. pub result: MupdateOverrideNonBootResult, } impl MupdateOverrideNonBootInfo { + pub fn new(info: InstallMetadataNonBootInfo) -> Self { + let result = match info.result { + InstallMetadataNonBootResult::MatchesPresent(_) => { + MupdateOverrideNonBootResult::MatchesPresent + } + InstallMetadataNonBootResult::MatchesAbsent => { + MupdateOverrideNonBootResult::MatchesAbsent + } + InstallMetadataNonBootResult::Mismatch(mismatch) => { + let mupdate_mismatch = match mismatch { + InstallMetadataNonBootMismatch::BootPresentOtherAbsent => { + MupdateOverrideNonBootMismatch::BootPresentOtherAbsent + } + InstallMetadataNonBootMismatch::BootAbsentOtherPresent { non_boot_disk_info } => { + MupdateOverrideNonBootMismatch::BootAbsentOtherPresent { non_boot_disk_info } + } + InstallMetadataNonBootMismatch::ValueMismatch { non_boot_disk_info } => { + MupdateOverrideNonBootMismatch::ValueMismatch { non_boot_disk_info } + } + InstallMetadataNonBootMismatch::BootDiskReadError { non_boot_disk_info } => { + MupdateOverrideNonBootMismatch::BootDiskReadError { non_boot_disk_info } + } + }; + MupdateOverrideNonBootResult::Mismatch(mupdate_mismatch) + } + InstallMetadataNonBootResult::ReadError(error) => { + MupdateOverrideNonBootResult::ReadError( + MupdateOverrideReadError::InstallMetadata(error), + ) + } + }; + + Self { zpool_name: info.zpool_name, path: info.path, result } + } + fn log_result(&self, log: &slog::Logger) { let log = log.new(o!( - "non_boot_zpool_name" => self.zpool_name.to_string(), - "non_boot_path" => self.path.to_string(), + "zpool_name" => self.zpool_name.to_string(), + "path" => self.path.to_string(), )); match &self.result { - MupdateOverrideNonBootResult::MatchesAbsent => { + MupdateOverrideNonBootResult::MatchesPresent => { info!( log, - "mupdate override absent on this non-boot \ - disk, matches absence on boot disk", + "mupdate override for non-boot disk matches boot disk (present)" ); } - MupdateOverrideNonBootResult::MatchesPresent => { + MupdateOverrideNonBootResult::MatchesAbsent => { info!( log, - "mupdate override present on this non-boot \ - disk, matches presence on boot disk", + "mupdate override for non-boot disk matches boot disk (absent)" + ); + } + MupdateOverrideNonBootResult::Mismatch(mismatch) => { + warn!( + log, + "mupdate override for non-boot disk does not match boot disk"; + "mismatch" => ?mismatch, ); } MupdateOverrideNonBootResult::ReadError(error) => { warn!( log, - "failed to read mupdate override from other disk"; + "error reading mupdate override for non-boot disk"; "error" => InlineErrorChain::new(error), ); } - MupdateOverrideNonBootResult::Mismatch(mismatch) => { - mismatch.log_to(&log) - } } } } @@ -334,444 +235,67 @@ impl IdOrdItem for MupdateOverrideNonBootInfo { id_upcast!(); } +/// The result of reading a mupdate override file from a non-boot disk. #[derive(Clone, Debug, PartialEq)] pub enum MupdateOverrideNonBootResult { - /// The override is present and matches the value on the boot disk. + /// The non-boot disk matches the boot disk (both present). MatchesPresent, - /// The override is absent and is also absent on the boot disk. + /// The non-boot disk matches the boot disk (both absent). MatchesAbsent, - /// A mismatch between the boot disk and the other disk was detected. + /// The non-boot disk does not match the boot disk. Mismatch(MupdateOverrideNonBootMismatch), - /// An error occurred while reading the mupdate override info on this disk. + /// There was an error reading the mupdate override file from the non-boot disk. ReadError(MupdateOverrideReadError), } -impl MupdateOverrideNonBootResult { - fn new( - res: Result, MupdateOverrideReadError>, - boot_disk_res: &Result< - Option, - MupdateOverrideReadError, - >, - ) -> Self { - match (res, boot_disk_res) { - (Ok(Some(non_boot_disk_info)), Ok(Some(boot_disk_info))) => { - if boot_disk_info == &non_boot_disk_info { - Self::MatchesPresent - } else { - Self::Mismatch( - MupdateOverrideNonBootMismatch::ValueMismatch { - non_boot_disk_info, - }, - ) - } - } - (Ok(Some(non_boot_disk_info)), Ok(None)) => Self::Mismatch( - MupdateOverrideNonBootMismatch::BootAbsentOtherPresent { - non_boot_disk_info, - }, - ), - (Ok(None), Ok(Some(_))) => Self::Mismatch( - MupdateOverrideNonBootMismatch::BootPresentOtherAbsent, - ), - (Ok(None), Ok(None)) => Self::MatchesAbsent, - (Ok(non_boot_disk_info), Err(_)) => Self::Mismatch( - MupdateOverrideNonBootMismatch::BootDiskReadError { - non_boot_disk_info, - }, - ), - (Err(error), _) => Self::ReadError(error), - } - } -} - -#[derive(Clone, Debug, PartialEq, Eq)] +/// Describes a mismatch between the boot disk and a non-boot disk. +#[derive(Clone, Debug, PartialEq)] pub enum MupdateOverrideNonBootMismatch { - /// The override is present on the boot disk but absent on the other disk. + /// The boot disk is present but the non-boot disk is absent. BootPresentOtherAbsent, - /// The override is absent on the boot disk but present on the other disk. - BootAbsentOtherPresent { - /// The information found on the other disk. - non_boot_disk_info: MupdateOverrideInfo, - }, + /// The boot disk is absent but the non-boot disk is present. + BootAbsentOtherPresent { non_boot_disk_info: MupdateOverrideInfo }, - /// The override value differs between the boot disk and the other disk. + /// Both disks are present but have different values. ValueMismatch { non_boot_disk_info: MupdateOverrideInfo }, - /// There was a read error on the boot disk, so we were unable to verify - /// consistency. - BootDiskReadError { - /// The value as found on this disk. This value is logged but not used. - non_boot_disk_info: Option, - }, + /// There was an error reading the boot disk. + BootDiskReadError { non_boot_disk_info: Option }, } -impl MupdateOverrideNonBootMismatch { - // This function assumes that `log` has already been provided context about - // the zpool name and path. - fn log_to(&self, log: &slog::Logger) { - match self { - Self::BootPresentOtherAbsent => { - warn!( - log, - "mupdate override absent on this non-boot disk but \ - present on boot disk, treating file on boot disk as \ - authoritative" - ) - } - Self::BootAbsentOtherPresent { non_boot_disk_info } => { - warn!( - log, - "mupdate override present on this non-boot disk but \ - absent on boot disk, treating the absence on boot disk \ - as authoritative"; - "non_boot_disk_info" => ?non_boot_disk_info, - ) - } - Self::ValueMismatch { non_boot_disk_info } => { - warn!( - log, - "mupdate override value present on both this non-boot \ - disk and the boot disk, but different across disks, \ - treating boot disk as authoritative"; - "non_boot_disk_info" => ?non_boot_disk_info, - ) - } - Self::BootDiskReadError { non_boot_disk_info } => { - warn!( - log, - "mupdate override read error on boot disk, unable \ - to verify consistency across disks"; - "non_boot_disk_info" => ?non_boot_disk_info, - ) - } - } - } -} - -/// The result of reading artifacts from an install dataset. -/// -/// This may or may not be valid, depending on the status of the artifacts. See -/// [`Self::is_valid`]. -#[derive(Clone, Debug, PartialEq)] -pub struct MupdateOverrideArtifactsResult { - pub info: MupdateOverrideInfo, - pub data: IdOrdMap, -} - -impl MupdateOverrideArtifactsResult { - /// Makes a new `MupdateOverrideArtifacts` by reading artifacts from the - /// given directory. - fn new(dir: &Utf8Path, info: MupdateOverrideInfo) -> Self { - let artifacts: Vec<_> = info - .zones - .iter() - // Parallelize artifact reading to speed it up. - .par_bridge() - .map(|zone| { - let artifact_path = dir.join(&zone.file_name); - let status = validate_one(&artifact_path, &zone); - - MupdateOverrideArtifactResult { - file_name: zone.file_name.clone(), - path: artifact_path, - expected_size: zone.file_size, - expected_hash: zone.hash, - status, - } - }) - .collect(); - - Self { info, data: artifacts.into_iter().collect() } - } - - /// Returns true if all artifacts are valid. - pub fn is_valid(&self) -> bool { - self.data.iter().all(|artifact| artifact.is_valid()) - } - - /// Returns a displayable representation of the artifacts. - pub fn display(&self) -> MupdateOverrideArtifactsDisplay<'_> { - MupdateOverrideArtifactsDisplay { artifacts: &self.data } - } -} - -pub struct MupdateOverrideArtifactsDisplay<'a> { - artifacts: &'a IdOrdMap, -} - -impl fmt::Display for MupdateOverrideArtifactsDisplay<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - for artifact in self.artifacts { - match &artifact.status { - ArtifactReadResult::Valid => { - writeln!( - f, - " {}: ok ({} bytes, {})", - artifact.file_name, - artifact.expected_size, - artifact.expected_hash - )?; - } - ArtifactReadResult::Mismatch { actual_size, actual_hash } => { - writeln!( - f, - " {}: mismatch (expected {} bytes, {}; \ - found {} bytes, {})", - artifact.file_name, - artifact.expected_size, - artifact.expected_hash, - actual_size, - actual_hash - )?; - } - ArtifactReadResult::Error(error) => { - writeln!(f, " {}: error ({})", artifact.file_name, error)?; - } - } - } - - Ok(()) - } -} - -#[derive(Clone, Debug, PartialEq)] -pub struct MupdateOverrideArtifactResult { - /// The filename. - pub file_name: String, - - /// The full path to the file. - pub path: Utf8PathBuf, - - /// The expected size. - pub expected_size: u64, - - /// The expected hash. - pub expected_hash: ArtifactHash, - - /// The status on disk. - pub status: ArtifactReadResult, -} - -impl MupdateOverrideArtifactResult { - fn is_valid(&self) -> bool { - matches!(self.status, ArtifactReadResult::Valid) - } -} - -impl IdOrdItem for MupdateOverrideArtifactResult { - type Key<'a> = &'a str; - - fn key(&self) -> Self::Key<'_> { - &self.file_name - } - - id_upcast!(); -} - -#[derive(Clone, Debug, PartialEq)] -pub enum ArtifactReadResult { - /// The artifact was read successfully and matches. - Valid, - - /// The artifact was read successfully but does not match. - Mismatch { - /// The actual file size. - actual_size: u64, - - /// The actual hash. - actual_hash: ArtifactHash, - }, - - /// An error occurred while reading the artifact. - Error(ArcIoError), -} - -fn validate_one( - artifact_path: &Utf8Path, - zone: &MupdateOverrideZone, -) -> ArtifactReadResult { - let mut f = match File::open(artifact_path) { - Ok(f) => f, - Err(error) => { - return ArtifactReadResult::Error(ArcIoError::new(error)); - } - }; - - match compute_size_and_hash(&mut f) { - Ok((actual_size, actual_hash)) => { - if zone.file_size == actual_size && zone.hash == actual_hash { - ArtifactReadResult::Valid - } else { - ArtifactReadResult::Mismatch { actual_size, actual_hash } - } - } - Err(error) => ArtifactReadResult::Error(ArcIoError::new(error)), - } -} - -fn compute_size_and_hash( - f: &mut File, -) -> Result<(u64, ArtifactHash), io::Error> { - let mut hasher = Sha256::new(); - // Zone artifacts are pretty big, so we read them in chunks. - let mut buffer = [0u8; 8192]; - let mut total_bytes_read = 0; - loop { - let bytes_read = f.read(&mut buffer)?; - if bytes_read == 0 { - break; - } - hasher.update(&buffer[..bytes_read]); - total_bytes_read += bytes_read; - } - Ok((total_bytes_read as u64, ArtifactHash(hasher.finalize().into()))) -} - -#[derive(Clone, Debug, PartialEq, Error)] +#[derive(Clone, Debug, Error, PartialEq)] pub enum MupdateOverrideReadError { - #[error( - "error retrieving metadata for install dataset directory \ - `{dataset_dir}`" - )] - DatasetDirMetadata { - dataset_dir: Utf8PathBuf, - #[source] - error: ArcIoError, - }, - - #[error( - "expected install dataset `{dataset_dir}` to be a directory, \ - found {file_type:?}" - )] - DatasetNotDirectory { dataset_dir: Utf8PathBuf, file_type: FileType }, - - #[error("error reading mupdate override from `{path}`")] - Read { - path: Utf8PathBuf, - #[source] - error: ArcIoError, - }, - - #[error( - "error deserializing `{path}` into MupdateOverrideInfo, \ - contents: {contents:?}" - )] - Deserialize { - path: Utf8PathBuf, - contents: String, - #[source] - error: ArcSerdeJsonError, - }, - - #[error("error reading artifacts in `{dataset_dir}:\n{}`", artifacts.display())] - ArtifactRead { - dataset_dir: Utf8PathBuf, - artifacts: MupdateOverrideArtifactsResult, - }, -} - -/// An `io::Error` wrapper that implements `Clone` and `PartialEq`. -#[derive(Clone, Debug, Error)] -#[error(transparent)] -pub struct ArcIoError(Arc); - -impl ArcIoError { - fn new(error: io::Error) -> Self { - Self(Arc::new(error)) - } -} - -/// Testing aid. -impl PartialEq for ArcIoError { - fn eq(&self, other: &Self) -> bool { - // Simply comparing io::ErrorKind is good enough for tests. - self.0.kind() == other.0.kind() - } -} - -/// A `serde_json::Error` that implements `Clone` and `PartialEq`. -#[derive(Clone, Debug, Error)] -#[error(transparent)] -pub struct ArcSerdeJsonError(Arc); - -impl ArcSerdeJsonError { - fn new(error: serde_json::Error) -> Self { - Self(Arc::new(error)) - } -} - -/// Testing aid. -impl PartialEq for ArcSerdeJsonError { - fn eq(&self, other: &Self) -> bool { - // Simply comparing line/column/category is good enough for tests. - self.0.line() == other.0.line() - && self.0.column() == other.0.column() - && self.0.classify() == other.0.classify() - } + #[error("install metadata read error")] + InstallMetadata(#[from] InstallMetadataReadError), } #[cfg(test)] mod tests { use super::*; - use camino_tempfile_ext::fixture::ChildPath; - use camino_tempfile_ext::fixture::FixtureError; - use camino_tempfile_ext::fixture::FixtureKind; + + use crate::test_utils::BOOT_PATHS; + use crate::test_utils::BOOT_ZPOOL; + use crate::test_utils::NON_BOOT_2_PATHS; + use crate::test_utils::NON_BOOT_2_ZPOOL; + use crate::test_utils::NON_BOOT_3_PATHS; + use crate::test_utils::NON_BOOT_3_ZPOOL; + use crate::test_utils::NON_BOOT_PATHS; + use crate::test_utils::NON_BOOT_ZPOOL; + use crate::test_utils::WriteInstallDatasetContext; + use crate::test_utils::dataset_missing_error; + use crate::test_utils::dataset_not_dir_error; + use crate::test_utils::deserialize_error; + use camino_tempfile_ext::prelude::*; use dropshot::ConfigLogging; use dropshot::ConfigLoggingLevel; use dropshot::test_util::LogContext; - use omicron_uuid_kinds::MupdateOverrideUuid; - use omicron_uuid_kinds::ZpoolUuid; + use iddqd::id_ord_map; use pretty_assertions::assert_eq; - use std::collections::BTreeSet; - use std::io; - use std::sync::LazyLock; - - struct OverridePaths { - install_dataset: Utf8PathBuf, - override_json: Utf8PathBuf, - } - - impl OverridePaths { - fn for_uuid(uuid: ZpoolUuid) -> Self { - let install_dataset = - Utf8PathBuf::from(format!("pool/int/{uuid}/install")); - let mupdate_override_json = - install_dataset.join("mupdate-override.json"); - Self { install_dataset, override_json: mupdate_override_json } - } - } - - const BOOT_UUID: ZpoolUuid = - ZpoolUuid::from_u128(0xd3e7205d_4efe_493b_ac5e_9175584907cd); - const BOOT_ZPOOL: ZpoolName = ZpoolName::new_internal(BOOT_UUID); - static BOOT_PATHS: LazyLock = - LazyLock::new(|| OverridePaths::for_uuid(BOOT_UUID)); - - const NON_BOOT_UUID: ZpoolUuid = - ZpoolUuid::from_u128(0x4854189f_b290_47cd_b076_374d0e1748ec); - const NON_BOOT_ZPOOL: ZpoolName = ZpoolName::new_internal(NON_BOOT_UUID); - static NON_BOOT_PATHS: LazyLock = - LazyLock::new(|| OverridePaths::for_uuid(NON_BOOT_UUID)); - - const NON_BOOT_2_UUID: ZpoolUuid = - ZpoolUuid::from_u128(0x72201e1e_9fee_4231_81cd_4e2d514cb632); - const NON_BOOT_2_ZPOOL: ZpoolName = - ZpoolName::new_internal(NON_BOOT_2_UUID); - static NON_BOOT_2_PATHS: LazyLock = - LazyLock::new(|| OverridePaths::for_uuid(NON_BOOT_2_UUID)); - - const NON_BOOT_3_UUID: ZpoolUuid = - ZpoolUuid::from_u128(0xd0d04947_93c5_40fd_97ab_4648b8cc28d6); - const NON_BOOT_3_ZPOOL: ZpoolName = - ZpoolName::new_internal(NON_BOOT_3_UUID); - static NON_BOOT_3_PATHS: LazyLock = - LazyLock::new(|| OverridePaths::for_uuid(NON_BOOT_3_UUID)); /// Boot disk present / no other disks. (This produces a warning, but is /// otherwise okay.) @@ -783,8 +307,8 @@ mod tests { ); let dir = Utf8TempDir::new().unwrap(); let cx = WriteInstallDatasetContext::new_basic(); - let info = - cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); + let info = cx.override_info(); + cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); let zpools = ZoneImageZpools { root: dir.path(), @@ -810,11 +334,9 @@ mod tests { ); let dir = Utf8TempDir::new().unwrap(); let cx = WriteInstallDatasetContext::new_basic(); - let info = - cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); - let info2 = - cx.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); - assert_eq!(info, info2, "the same contents must have been written out"); + let info = cx.override_info(); + cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); + cx.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); let zpools = ZoneImageZpools { root: dir.path(), @@ -829,13 +351,13 @@ mod tests { ); assert_eq!( overrides.non_boot_disk_overrides, - [MupdateOverrideNonBootInfo { - zpool_name: NON_BOOT_ZPOOL, - path: dir.path().join(&NON_BOOT_PATHS.override_json), - result: MupdateOverrideNonBootResult::MatchesPresent, - }] - .into_iter() - .collect(), + id_ord_map! { + MupdateOverrideNonBootInfo { + zpool_name: NON_BOOT_ZPOOL, + path: dir.path().join(&NON_BOOT_PATHS.mupdate_override_json), + result: MupdateOverrideNonBootResult::MatchesPresent, + } + }, ); logctx.cleanup_successful(); @@ -868,13 +390,13 @@ mod tests { ); assert_eq!( overrides.non_boot_disk_overrides, - [MupdateOverrideNonBootInfo { - zpool_name: NON_BOOT_ZPOOL, - path: dir.path().join(&NON_BOOT_PATHS.override_json), - result: MupdateOverrideNonBootResult::MatchesAbsent, - }] - .into_iter() - .collect(), + id_ord_map! { + MupdateOverrideNonBootInfo { + zpool_name: NON_BOOT_ZPOOL, + path: dir.path().join(&NON_BOOT_PATHS.mupdate_override_json), + result: MupdateOverrideNonBootResult::MatchesAbsent, + } + }, ); logctx.cleanup_successful(); @@ -889,8 +411,8 @@ mod tests { ); let dir = Utf8TempDir::new().unwrap(); let cx = WriteInstallDatasetContext::new_basic(); - let info = - cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); + let info = cx.override_info(); + cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); // Create the directory, but not the override JSON within it. dir.child(&NON_BOOT_PATHS.install_dataset).create_dir_all().unwrap(); @@ -908,15 +430,15 @@ mod tests { ); assert_eq!( overrides.non_boot_disk_overrides, - [MupdateOverrideNonBootInfo { - zpool_name: NON_BOOT_ZPOOL, - path: dir.path().join(&NON_BOOT_PATHS.override_json), - result: MupdateOverrideNonBootResult::Mismatch( - MupdateOverrideNonBootMismatch::BootPresentOtherAbsent, - ), - }] - .into_iter() - .collect(), + id_ord_map! { + MupdateOverrideNonBootInfo { + zpool_name: NON_BOOT_ZPOOL, + path: dir.path().join(&NON_BOOT_PATHS.mupdate_override_json), + result: MupdateOverrideNonBootResult::Mismatch( + MupdateOverrideNonBootMismatch::BootPresentOtherAbsent, + ), + } + }, ); logctx.cleanup_successful(); @@ -931,12 +453,12 @@ mod tests { ); let dir = Utf8TempDir::new().unwrap(); let cx = WriteInstallDatasetContext::new_basic(); + let info = cx.override_info(); // Create the directory, but not the override JSON within it. dir.child(&BOOT_PATHS.install_dataset).create_dir_all().unwrap(); - let info = - cx.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); + cx.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); let zpools = ZoneImageZpools { root: dir.path(), @@ -950,17 +472,17 @@ mod tests { ); assert_eq!( overrides.non_boot_disk_overrides, - [MupdateOverrideNonBootInfo { - zpool_name: NON_BOOT_ZPOOL, - path: dir.path().join(&NON_BOOT_PATHS.override_json), - result: MupdateOverrideNonBootResult::Mismatch( - MupdateOverrideNonBootMismatch::BootAbsentOtherPresent { - non_boot_disk_info: info.clone() - }, - ), - }] - .into_iter() - .collect(), + id_ord_map! { + MupdateOverrideNonBootInfo { + zpool_name: NON_BOOT_ZPOOL, + path: dir.path().join(&NON_BOOT_PATHS.mupdate_override_json), + result: MupdateOverrideNonBootResult::Mismatch( + MupdateOverrideNonBootMismatch::BootAbsentOtherPresent { + non_boot_disk_info: info.clone() + }, + ), + } + }, ); logctx.cleanup_successful(); @@ -979,11 +501,11 @@ mod tests { // Make two different contexts. Each will have a different mupdate_uuid // so will not match. let cx = WriteInstallDatasetContext::new_basic(); - let info = - cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); + let info = cx.override_info(); + cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); let cx2 = WriteInstallDatasetContext::new_basic(); - let info2 = - cx2.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); + let info2 = cx2.override_info(); + cx2.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); let zpools = ZoneImageZpools { root: dir.path(), @@ -997,17 +519,17 @@ mod tests { ); assert_eq!( overrides.non_boot_disk_overrides, - [MupdateOverrideNonBootInfo { - zpool_name: NON_BOOT_ZPOOL, - path: dir.path().join(&NON_BOOT_PATHS.override_json), - result: MupdateOverrideNonBootResult::Mismatch( - MupdateOverrideNonBootMismatch::ValueMismatch { - non_boot_disk_info: info2, - } - ), - }] - .into_iter() - .collect(), + id_ord_map! { + MupdateOverrideNonBootInfo { + zpool_name: NON_BOOT_ZPOOL, + path: dir.path().join(&NON_BOOT_PATHS.mupdate_override_json), + result: MupdateOverrideNonBootResult::Mismatch( + MupdateOverrideNonBootMismatch::ValueMismatch { + non_boot_disk_info: info2, + } + ), + } + }, ); logctx.cleanup_successful(); @@ -1042,20 +564,22 @@ mod tests { &dataset_missing_error( &dir.path().join(&BOOT_PATHS.install_dataset) ) + .into() ); assert_eq!( overrides.non_boot_disk_overrides, - [MupdateOverrideNonBootInfo { - zpool_name: NON_BOOT_ZPOOL, - path: dir.path().join(&NON_BOOT_PATHS.override_json), - result: MupdateOverrideNonBootResult::ReadError( - dataset_missing_error( - &dir.path().join(&NON_BOOT_PATHS.install_dataset) - ), - ) - }] - .into_iter() - .collect(), + id_ord_map! { + MupdateOverrideNonBootInfo { + zpool_name: NON_BOOT_ZPOOL, + path: dir.path().join(&NON_BOOT_PATHS.mupdate_override_json), + result: MupdateOverrideNonBootResult::ReadError( + dataset_missing_error( + &dir.path().join(&NON_BOOT_PATHS.install_dataset) + ) + .into(), + ) + } + }, ); logctx.cleanup_successful(); @@ -1085,20 +609,22 @@ mod tests { &dataset_not_dir_error( &dir.path().join(&BOOT_PATHS.install_dataset) ) + .into() ); assert_eq!( overrides.non_boot_disk_overrides, - [MupdateOverrideNonBootInfo { - zpool_name: NON_BOOT_ZPOOL, - path: dir.path().join(&NON_BOOT_PATHS.override_json), - result: MupdateOverrideNonBootResult::ReadError( - dataset_not_dir_error( - &dir.path().join(&NON_BOOT_PATHS.install_dataset), + id_ord_map! { + MupdateOverrideNonBootInfo { + zpool_name: NON_BOOT_ZPOOL, + path: dir.path().join(&NON_BOOT_PATHS.mupdate_override_json), + result: MupdateOverrideNonBootResult::ReadError( + dataset_not_dir_error( + &dir.path().join(&NON_BOOT_PATHS.install_dataset), + ) + .into(), ), - ), - }] - .into_iter() - .collect(), + } + }, ); logctx.cleanup_successful(); @@ -1113,16 +639,16 @@ mod tests { ); let dir = Utf8TempDir::new().unwrap(); let cx = WriteInstallDatasetContext::new_basic(); + let info = cx.override_info(); // Create an empty file: this won't deserialize correctly. - dir.child(&BOOT_PATHS.override_json).touch().unwrap(); + dir.child(&BOOT_PATHS.mupdate_override_json).touch().unwrap(); // File with the correct contents. - let info = - cx.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); + cx.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); // File that's absent. dir.child(&NON_BOOT_2_PATHS.install_dataset).create_dir_all().unwrap(); // Read error (empty file). - dir.child(&NON_BOOT_3_PATHS.override_json).touch().unwrap(); + dir.child(&NON_BOOT_3_PATHS.mupdate_override_json).touch().unwrap(); let zpools = ZoneImageZpools { root: dir.path(), @@ -1137,14 +663,19 @@ mod tests { AllMupdateOverrides::read_all(&logctx.log, &zpools, &BOOT_ZPOOL); assert_eq!( overrides.boot_disk_override.as_ref().unwrap_err(), - &deserialize_error(dir.path(), &BOOT_PATHS.override_json, "",), + &deserialize_error( + dir.path(), + &BOOT_PATHS.mupdate_override_json, + "" + ) + .into(), ); assert_eq!( overrides.non_boot_disk_overrides, - [ + id_ord_map! { MupdateOverrideNonBootInfo { zpool_name: NON_BOOT_ZPOOL, - path: dir.path().join(&NON_BOOT_PATHS.override_json), + path: dir.path().join(&NON_BOOT_PATHS.mupdate_override_json), result: MupdateOverrideNonBootResult::Mismatch( MupdateOverrideNonBootMismatch::BootDiskReadError { non_boot_disk_info: Some(info), @@ -1153,7 +684,7 @@ mod tests { }, MupdateOverrideNonBootInfo { zpool_name: NON_BOOT_2_ZPOOL, - path: dir.path().join(&NON_BOOT_2_PATHS.override_json), + path: dir.path().join(&NON_BOOT_2_PATHS.mupdate_override_json), result: MupdateOverrideNonBootResult::Mismatch( MupdateOverrideNonBootMismatch::BootDiskReadError { non_boot_disk_info: None, @@ -1162,345 +693,19 @@ mod tests { }, MupdateOverrideNonBootInfo { zpool_name: NON_BOOT_3_ZPOOL, - path: dir.path().join(&NON_BOOT_3_PATHS.override_json), + path: dir.path().join(&NON_BOOT_3_PATHS.mupdate_override_json), result: MupdateOverrideNonBootResult::ReadError( deserialize_error( dir.path(), - &NON_BOOT_3_PATHS.override_json, + &NON_BOOT_3_PATHS.mupdate_override_json, "", - ), + ) + .into(), ), }, - ] - .into_iter() - .collect(), + }, ); logctx.cleanup_successful(); } - - /// Error case: zones don't match expected ones on boot disk. - #[test] - fn read_boot_disk_zone_mismatch() { - let logctx = LogContext::new( - "mupdate_override_read_boot_disk_zone_mismatch", - &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug }, - ); - let dir = Utf8TempDir::new().unwrap(); - let cx = WriteInstallDatasetContext::new_basic(); - let mut invalid_cx = cx.clone(); - invalid_cx.make_error_cases(); - - invalid_cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); - let valid_info = - cx.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); - - let zpools = ZoneImageZpools { - root: dir.path(), - all_m2_zpools: vec![BOOT_ZPOOL, NON_BOOT_ZPOOL], - }; - - let overrides = - AllMupdateOverrides::read_all(&logctx.log, &zpools, &BOOT_ZPOOL); - assert_eq!( - overrides.boot_disk_override.as_ref().unwrap_err(), - &MupdateOverrideReadError::ArtifactRead { - dataset_dir: dir - .child(&BOOT_PATHS.install_dataset) - .as_path() - .to_path_buf(), - artifacts: invalid_cx.expected_result( - dir.child(&BOOT_PATHS.install_dataset).as_path() - ), - } - ); - - assert_eq!( - overrides.non_boot_disk_overrides, - [MupdateOverrideNonBootInfo { - zpool_name: NON_BOOT_ZPOOL, - path: dir.path().join(&NON_BOOT_PATHS.override_json), - result: MupdateOverrideNonBootResult::Mismatch( - MupdateOverrideNonBootMismatch::BootDiskReadError { - non_boot_disk_info: Some(valid_info), - } - ) - }] - .into_iter() - .collect(), - ); - - logctx.cleanup_successful(); - } - - /// Warning case: zones don't match expected ones on non-boot disk. - #[test] - fn read_non_boot_disk_zone_mismatch() { - let logctx = LogContext::new( - "mupdate_override_read_non_boot_disk_zone_mismatch", - &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug }, - ); - let dir = Utf8TempDir::new().unwrap(); - let cx = WriteInstallDatasetContext::new_basic(); - let mut invalid_cx = cx.clone(); - invalid_cx.make_error_cases(); - - let info = - cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); - invalid_cx - .write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)) - .unwrap(); - - let zpools = ZoneImageZpools { - root: dir.path(), - all_m2_zpools: vec![BOOT_ZPOOL, NON_BOOT_ZPOOL], - }; - - let overrides = - AllMupdateOverrides::read_all(&logctx.log, &zpools, &BOOT_ZPOOL); - // The boot disk is valid. - assert_eq!( - overrides.boot_disk_override.as_ref().unwrap().as_ref(), - Some(&info) - ); - - // The non-boot disk has an error. - assert_eq!( - overrides.non_boot_disk_overrides, - [MupdateOverrideNonBootInfo { - zpool_name: NON_BOOT_ZPOOL, - path: dir.path().join(&NON_BOOT_PATHS.override_json), - result: MupdateOverrideNonBootResult::ReadError( - MupdateOverrideReadError::ArtifactRead { - dataset_dir: dir - .path() - .join(&NON_BOOT_PATHS.install_dataset), - artifacts: invalid_cx.expected_result( - &dir.path().join(&NON_BOOT_PATHS.install_dataset) - ), - }, - ), - }] - .into_iter() - .collect(), - ); - - logctx.cleanup_successful(); - } - - /// Context for writing out fake zones to install dataset directories. - /// - /// The tests in this module ensure that the override JSON's list of zones - /// matches the zone files on disk. - #[derive(Clone, Debug)] - struct WriteInstallDatasetContext { - zones: IdOrdMap, - mupdate_uuid: MupdateOverrideUuid, - } - - impl WriteInstallDatasetContext { - /// Initializes a new context with a couple of zones and no known - /// errors. - fn new_basic() -> Self { - Self { - zones: [ - ZoneContents::new("zone1.tar.gz", b"zone1"), - ZoneContents::new("zone2.tar.gz", b"zone2"), - ZoneContents::new("zone3.tar.gz", b"zone3"), - ZoneContents::new("zone4.tar.gz", b"zone4"), - ZoneContents::new("zone5.tar.gz", b"zone5"), - ] - .into_iter() - .collect(), - mupdate_uuid: MupdateOverrideUuid::new_v4(), - } - } - - /// Makes a number of error cases for testing. - fn make_error_cases(&mut self) { - // zone1.tar.gz is valid. - // For zone2.tar.gz, change the size. - self.zones.get_mut("zone2.tar.gz").unwrap().json_size = 1024; - // For zone3.tar.gz, change the hash. - self.zones.get_mut("zone3.tar.gz").unwrap().json_hash = - ArtifactHash([0; 32]); - // Don't write out zone4 but include it in the JSON. - self.zones.get_mut("zone4.tar.gz").unwrap().write_to_disk = false; - // Write out zone5 but don't include it in the JSON. - self.zones.get_mut("zone5.tar.gz").unwrap().include_in_json = false; - } - - fn override_info(&self) -> MupdateOverrideInfo { - MupdateOverrideInfo { - mupdate_uuid: self.mupdate_uuid, - // The hash IDs are not used for validation, so leave this - // empty. - hash_ids: BTreeSet::new(), - zones: self - .zones - .iter() - .filter_map(|zone| { - zone.include_in_json.then(|| MupdateOverrideZone { - file_name: zone.file_name.clone(), - file_size: zone.json_size, - hash: zone.json_hash, - }) - }) - .collect(), - } - } - - /// Returns the expected result of the override, taking into account - /// mismatches, etc. - fn expected_result( - &self, - dir: &Utf8Path, - ) -> MupdateOverrideArtifactsResult { - let info = self.override_info(); - let data = self - .zones - .iter() - .filter_map(|zone| { - // Currently, zone files not present in the JSON aren't - // reported at all. - // - // XXX: should they be? - zone.include_in_json.then(|| zone.expected_result(dir)) - }) - .collect(); - MupdateOverrideArtifactsResult { info, data } - } - - /// Writes the context to a directory, returning the JSON that was - /// written out. - fn write_to( - &self, - dir: &ChildPath, - ) -> Result { - for zone in &self.zones { - if zone.write_to_disk { - dir.child(&zone.file_name).write_binary(&zone.contents)?; - } - } - - let info = self.override_info(); - let json = serde_json::to_string(&info).map_err(|e| { - FixtureError::new(FixtureKind::WriteFile).with_source(e) - })?; - // No need to create intermediate directories with - // camino-tempfile-ext. - dir.child(MupdateOverrideInfo::FILE_NAME).write_str(&json)?; - - Ok(info) - } - } - - #[derive(Clone, Debug)] - struct ZoneContents { - file_name: String, - contents: Vec, - // json_size and json_hash are stored separately, so tests can tweak - // them before writing out the override info. - json_size: u64, - json_hash: ArtifactHash, - write_to_disk: bool, - include_in_json: bool, - } - - impl ZoneContents { - fn new(file_name: &str, contents: &[u8]) -> Self { - let size = contents.len() as u64; - let hash = compute_hash(contents); - Self { - file_name: file_name.to_string(), - contents: contents.to_vec(), - json_size: size, - json_hash: hash, - write_to_disk: true, - include_in_json: true, - } - } - - fn expected_result( - &self, - dir: &Utf8Path, - ) -> MupdateOverrideArtifactResult { - let status = if !self.write_to_disk { - // Missing from the disk - ArtifactReadResult::Error(ArcIoError::new(io::Error::new( - io::ErrorKind::NotFound, - "file not found", - ))) - } else { - let actual_size = self.contents.len() as u64; - let actual_hash = compute_hash(&self.contents); - if self.json_size != actual_size - || self.json_hash != actual_hash - { - ArtifactReadResult::Mismatch { actual_size, actual_hash } - } else { - ArtifactReadResult::Valid - } - }; - - MupdateOverrideArtifactResult { - file_name: self.file_name.clone(), - path: dir.join(&self.file_name), - expected_size: self.json_size, - expected_hash: self.json_hash, - status, - } - } - } - - impl IdOrdItem for ZoneContents { - type Key<'a> = &'a str; - - fn key(&self) -> Self::Key<'_> { - &self.file_name - } - - id_upcast!(); - } - - fn compute_hash(contents: &[u8]) -> ArtifactHash { - let hash = Sha256::digest(contents); - ArtifactHash(hash.into()) - } - - fn dataset_missing_error(dir_path: &Utf8Path) -> MupdateOverrideReadError { - MupdateOverrideReadError::DatasetDirMetadata { - dataset_dir: dir_path.to_owned(), - error: ArcIoError(Arc::new(io::Error::from( - io::ErrorKind::NotFound, - ))), - } - } - - fn dataset_not_dir_error(dir_path: &Utf8Path) -> MupdateOverrideReadError { - // A `FileType` must unfortunately be retrieved from disk -- can't - // create a new one in-memory. We assume that `dir.path()` passed in - // actually has the described error condition. - MupdateOverrideReadError::DatasetNotDirectory { - dataset_dir: dir_path.to_owned(), - file_type: fs::symlink_metadata(dir_path) - .expect("lstat on dir.path() succeeded") - .file_type(), - } - } - - fn deserialize_error( - dir_path: &Utf8Path, - json_path: &Utf8Path, - contents: &str, - ) -> MupdateOverrideReadError { - MupdateOverrideReadError::Deserialize { - path: dir_path.join(json_path), - contents: contents.to_owned(), - error: ArcSerdeJsonError(Arc::new( - serde_json::from_str::(contents) - .unwrap_err(), - )), - } - } } diff --git a/sled-agent/zone-images/src/source_resolver.rs b/sled-agent/zone-images/src/source_resolver.rs index 3b31c7a6a14..b2f7f59e325 100644 --- a/sled-agent/zone-images/src/source_resolver.rs +++ b/sled-agent/zone-images/src/source_resolver.rs @@ -5,7 +5,9 @@ //! Zone image lookup. use crate::AllMupdateOverrides; +use crate::AllZoneManifests; use crate::MupdateOverrideStatus; +use crate::ZoneManifestStatus; use camino::Utf8Path; use camino::Utf8PathBuf; use illumos_utils::zpool::ZpoolName; @@ -74,7 +76,10 @@ impl ZoneImageSourceResolver { /// Returns current information about resolver status and health. pub fn status(&self) -> ResolverStatus { let inner = self.inner.lock().unwrap(); - ResolverStatus { mupdate_override: inner.mupdate_overrides.status() } + let zone_manifest = inner.zone_manifests.status(); + let mupdate_override = inner.mupdate_overrides.status(); + + ResolverStatus { mupdate_override, zone_manifest } } /// Returns a [`ZoneImageFileSource`] consisting of the file name, plus a @@ -93,6 +98,9 @@ impl ZoneImageSourceResolver { /// Current status of the zone image resolver. #[derive(Clone, Debug)] pub struct ResolverStatus { + /// The zone manifest status. + pub zone_manifest: ZoneManifestStatus, + /// The mupdate override status. pub mupdate_override: MupdateOverrideStatus, } @@ -102,6 +110,9 @@ struct ResolverInner { #[expect(unused)] log: slog::Logger, image_directory_override: Option, + // Store all collected information for zones -- we're going to need to + // report this via inventory. + zone_manifests: AllZoneManifests, // Store all collected information for mupdate overrides -- we're going to // need to report this via inventory. mupdate_overrides: AllMupdateOverrides, @@ -115,10 +126,17 @@ impl ResolverInner { ) -> Self { let log = log.new(o!("component" => "ZoneImageSourceResolver")); + let zone_manifests = + AllZoneManifests::read_all(&log, zpools, boot_zpool); let mupdate_overrides = AllMupdateOverrides::read_all(&log, zpools, boot_zpool); - Self { log, image_directory_override: None, mupdate_overrides } + Self { + log, + image_directory_override: None, + zone_manifests, + mupdate_overrides, + } } fn override_image_directory( diff --git a/sled-agent/zone-images/src/test_utils.rs b/sled-agent/zone-images/src/test_utils.rs new file mode 100644 index 00000000000..3f5c8ba6555 --- /dev/null +++ b/sled-agent/zone-images/src/test_utils.rs @@ -0,0 +1,292 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use std::{collections::BTreeSet, fs, io, sync::LazyLock}; + +use camino::{Utf8Path, Utf8PathBuf}; +use camino_tempfile_ext::{ + fixture::{ChildPath, FixtureError, FixtureKind}, + prelude::*, +}; +use iddqd::{IdOrdItem, IdOrdMap, id_upcast}; +use illumos_utils::zpool::ZpoolName; +use omicron_common::update::{ + MupdateOverrideInfo, OmicronZoneFileMetadata, OmicronZoneManifest, +}; +use omicron_uuid_kinds::{MupdateOverrideUuid, MupdateUuid, ZpoolUuid}; +use sha2::{Digest, Sha256}; +use tufaceous_artifact::ArtifactHash; + +use crate::{ + ArcIoError, ArcSerdeJsonError, ArtifactReadResult, + InstallMetadataReadError, ZoneManifestArtifactResult, + ZoneManifestArtifactsResult, +}; + +pub(crate) struct OverridePaths { + pub(crate) install_dataset: Utf8PathBuf, + pub(crate) zones_json: Utf8PathBuf, + pub(crate) mupdate_override_json: Utf8PathBuf, +} + +impl OverridePaths { + fn for_uuid(uuid: ZpoolUuid) -> Self { + let install_dataset = + Utf8PathBuf::from(format!("pool/int/{uuid}/install")); + let zones_json = install_dataset.join(OmicronZoneManifest::FILE_NAME); + let mupdate_override_json = + install_dataset.join(MupdateOverrideInfo::FILE_NAME); + Self { install_dataset, zones_json, mupdate_override_json } + } +} + +pub(crate) const BOOT_UUID: ZpoolUuid = + ZpoolUuid::from_u128(0xd3e7205d_4efe_493b_ac5e_9175584907cd); +pub(crate) const BOOT_ZPOOL: ZpoolName = ZpoolName::new_internal(BOOT_UUID); +pub(crate) static BOOT_PATHS: LazyLock = + LazyLock::new(|| OverridePaths::for_uuid(BOOT_UUID)); + +pub(crate) const NON_BOOT_UUID: ZpoolUuid = + ZpoolUuid::from_u128(0x4854189f_b290_47cd_b076_374d0e1748ec); +pub(crate) const NON_BOOT_ZPOOL: ZpoolName = + ZpoolName::new_internal(NON_BOOT_UUID); +pub(crate) static NON_BOOT_PATHS: LazyLock = + LazyLock::new(|| OverridePaths::for_uuid(NON_BOOT_UUID)); + +pub(crate) const NON_BOOT_2_UUID: ZpoolUuid = + ZpoolUuid::from_u128(0x72201e1e_9fee_4231_81cd_4e2d514cb632); +pub(crate) const NON_BOOT_2_ZPOOL: ZpoolName = + ZpoolName::new_internal(NON_BOOT_2_UUID); +pub(crate) static NON_BOOT_2_PATHS: LazyLock = + LazyLock::new(|| OverridePaths::for_uuid(NON_BOOT_2_UUID)); + +pub(crate) const NON_BOOT_3_UUID: ZpoolUuid = + ZpoolUuid::from_u128(0xd0d04947_93c5_40fd_97ab_4648b8cc28d6); +pub(crate) const NON_BOOT_3_ZPOOL: ZpoolName = + ZpoolName::new_internal(NON_BOOT_3_UUID); +pub(crate) static NON_BOOT_3_PATHS: LazyLock = + LazyLock::new(|| OverridePaths::for_uuid(NON_BOOT_3_UUID)); + +/// Context for writing out fake zones to install dataset directories. +/// +/// The tests in this module ensure that the override JSON's list of zones +/// matches the zone files on disk. +#[derive(Clone, Debug)] +pub(crate) struct WriteInstallDatasetContext { + pub(crate) zones: IdOrdMap, + pub(crate) mupdate_id: MupdateUuid, + pub(crate) mupdate_override_uuid: MupdateOverrideUuid, +} + +impl WriteInstallDatasetContext { + /// Initializes a new context with a couple of zones and no known + /// errors. + pub(crate) fn new_basic() -> Self { + Self { + zones: [ + ZoneContents::new("zone1.tar.gz", b"zone1"), + ZoneContents::new("zone2.tar.gz", b"zone2"), + ZoneContents::new("zone3.tar.gz", b"zone3"), + ZoneContents::new("zone4.tar.gz", b"zone4"), + ZoneContents::new("zone5.tar.gz", b"zone5"), + ] + .into_iter() + .collect(), + mupdate_id: MupdateUuid::new_v4(), + mupdate_override_uuid: MupdateOverrideUuid::new_v4(), + } + } + + /// Makes a number of error cases for testing. + pub(crate) fn make_error_cases(&mut self) { + // zone1.tar.gz is valid. + // For zone2.tar.gz, change the size. + self.zones.get_mut("zone2.tar.gz").unwrap().json_size = 1024; + // For zone3.tar.gz, change the hash. + self.zones.get_mut("zone3.tar.gz").unwrap().json_hash = + ArtifactHash([0; 32]); + // Don't write out zone4 but include it in the JSON. + self.zones.get_mut("zone4.tar.gz").unwrap().write_to_disk = false; + // Write out zone5 but don't include it in the JSON. + self.zones.get_mut("zone5.tar.gz").unwrap().include_in_json = false; + } + + pub(crate) fn override_info(&self) -> MupdateOverrideInfo { + MupdateOverrideInfo { + mupdate_uuid: self.mupdate_override_uuid, + // The hash IDs are not used for validation, so leave this + // empty. + hash_ids: BTreeSet::new(), + } + } + + pub(crate) fn zone_manifest(&self) -> OmicronZoneManifest { + OmicronZoneManifest { + mupdate_id: self.mupdate_id, + zones: self + .zones + .iter() + .filter_map(|zone| { + zone.include_in_json.then(|| OmicronZoneFileMetadata { + file_name: zone.file_name.clone(), + file_size: zone.json_size, + hash: zone.json_hash, + }) + }) + .collect(), + } + } + + /// Returns the expected result of writing the zone manifest, taking into + /// account mismatches, etc. + pub(crate) fn expected_result( + &self, + dir: &Utf8Path, + ) -> ZoneManifestArtifactsResult { + let manifest = self.zone_manifest(); + let data = self + .zones + .iter() + .filter_map(|zone| { + // Currently, zone files not present in the JSON aren't + // reported at all. + // + // XXX: should they be? + zone.include_in_json.then(|| zone.expected_result(dir)) + }) + .collect(); + ZoneManifestArtifactsResult { manifest, data } + } + + /// Writes the context to a directory, returning the JSON that was + /// written out. + pub(crate) fn write_to(&self, dir: &ChildPath) -> Result<(), FixtureError> { + for zone in &self.zones { + if zone.write_to_disk { + dir.child(&zone.file_name).write_binary(&zone.contents)?; + } + } + + let manifest = self.zone_manifest(); + let json = serde_json::to_string(&manifest).map_err(|e| { + FixtureError::new(FixtureKind::WriteFile).with_source(e) + })?; + // No need to create intermediate directories with + // camino-tempfile-ext. + dir.child(OmicronZoneManifest::FILE_NAME).write_str(&json)?; + + let info = self.override_info(); + let json = serde_json::to_string(&info).map_err(|e| { + FixtureError::new(FixtureKind::WriteFile).with_source(e) + })?; + dir.child(MupdateOverrideInfo::FILE_NAME).write_str(&json)?; + + Ok(()) + } +} + +#[derive(Clone, Debug)] +pub(crate) struct ZoneContents { + file_name: String, + contents: Vec, + // json_size and json_hash are stored separately, so tests can tweak + // them before writing out the override info. + json_size: u64, + json_hash: ArtifactHash, + write_to_disk: bool, + include_in_json: bool, +} + +impl ZoneContents { + fn new(file_name: &str, contents: &[u8]) -> Self { + let size = contents.len() as u64; + let hash = compute_hash(contents); + Self { + file_name: file_name.to_string(), + contents: contents.to_vec(), + json_size: size, + json_hash: hash, + write_to_disk: true, + include_in_json: true, + } + } + + fn expected_result(&self, dir: &Utf8Path) -> ZoneManifestArtifactResult { + let status = if !self.write_to_disk { + // Missing from the disk + ArtifactReadResult::Error(ArcIoError::new(io::Error::new( + io::ErrorKind::NotFound, + "file not found", + ))) + } else { + let actual_size = self.contents.len() as u64; + let actual_hash = compute_hash(&self.contents); + if self.json_size != actual_size || self.json_hash != actual_hash { + ArtifactReadResult::Mismatch { actual_size, actual_hash } + } else { + ArtifactReadResult::Valid + } + }; + + ZoneManifestArtifactResult { + file_name: self.file_name.clone(), + path: dir.join(&self.file_name), + expected_size: self.json_size, + expected_hash: self.json_hash, + status, + } + } +} + +impl IdOrdItem for ZoneContents { + type Key<'a> = &'a str; + + fn key(&self) -> Self::Key<'_> { + &self.file_name + } + + id_upcast!(); +} + +fn compute_hash(contents: &[u8]) -> ArtifactHash { + let hash = Sha256::digest(contents); + ArtifactHash(hash.into()) +} + +pub(crate) fn dataset_missing_error( + dir_path: &Utf8Path, +) -> InstallMetadataReadError { + InstallMetadataReadError::DatasetDirMetadata { + dataset_dir: dir_path.to_owned(), + error: ArcIoError::new(io::Error::from(io::ErrorKind::NotFound)), + } +} + +pub(crate) fn dataset_not_dir_error( + dir_path: &Utf8Path, +) -> InstallMetadataReadError { + // A `FileType` must unfortunately be retrieved from disk -- can't + // create a new one in-memory. We assume that `dir.path()` passed in + // actually has the described error condition. + InstallMetadataReadError::DatasetNotDirectory { + dataset_dir: dir_path.to_owned(), + file_type: fs::symlink_metadata(dir_path) + .expect("lstat on dir.path() succeeded") + .file_type(), + } +} + +pub(crate) fn deserialize_error( + dir_path: &Utf8Path, + json_path: &Utf8Path, + contents: &str, +) -> InstallMetadataReadError { + InstallMetadataReadError::Deserialize { + path: dir_path.join(json_path), + contents: contents.to_owned(), + error: ArcSerdeJsonError::new( + serde_json::from_str::(contents).unwrap_err(), + ), + } +} diff --git a/sled-agent/zone-images/src/zone_manifest.rs b/sled-agent/zone-images/src/zone_manifest.rs new file mode 100644 index 00000000000..4efb11ca1cf --- /dev/null +++ b/sled-agent/zone-images/src/zone_manifest.rs @@ -0,0 +1,827 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use camino::{Utf8Path, Utf8PathBuf}; +use iddqd::{IdOrdItem, IdOrdMap, id_upcast}; +use illumos_utils::zpool::ZpoolName; +use omicron_common::update::{OmicronZoneFileMetadata, OmicronZoneManifest}; +use rayon::iter::{ParallelBridge, ParallelIterator}; +use sha2::{Digest, Sha256}; +use slog::{error, info, o, warn}; +use slog_error_chain::InlineErrorChain; +use std::{ + fmt, + fs::File, + io::{self, Read}, +}; +use thiserror::Error; +use tufaceous_artifact::ArtifactHash; + +use crate::{ + AllInstallMetadataFiles, ArcIoError, InstallMetadataNonBootInfo, + InstallMetadataNonBootMismatch, InstallMetadataNonBootResult, + InstallMetadataReadError, ZoneImageZpools, +}; + +/// Describes the current state of zone manifests. +#[derive(Clone, Debug)] +pub struct ZoneManifestStatus { + /// The path to the zone manifest JSON on the boot disk. + pub boot_disk_path: Utf8PathBuf, + + /// Status of the boot disk. + pub boot_disk_result: + Result, + + /// Status of the non-boot disks. This results in warnings in case of a + /// mismatch. + pub non_boot_disk_metadata: IdOrdMap, +} + +#[derive(Debug)] +pub(crate) struct AllZoneManifests { + boot_zpool: ZpoolName, + boot_disk_path: Utf8PathBuf, + boot_disk_result: + Result, + non_boot_disk_metadata: IdOrdMap, +} + +impl AllZoneManifests { + /// Attempt to find zone manifests. + pub(crate) fn read_all( + log: &slog::Logger, + zpools: &ZoneImageZpools<'_>, + boot_zpool: &ZpoolName, + ) -> Self { + // First read all the files. + let files = AllInstallMetadataFiles::read_all( + log, + OmicronZoneManifest::FILE_NAME, + zpools, + boot_zpool, + ); + + // Validate files on the boot disk. + let boot_disk_result = match files.boot_disk_metadata { + Ok(Some(manifest)) => Ok(ZoneManifestArtifactsResult::new( + &files.boot_dataset_dir, + manifest, + )), + Ok(None) => { + // The file is missing -- this is an error. + Err(ZoneManifestReadError::NotFound( + files.boot_disk_path.clone(), + )) + } + Err(error) => Err(ZoneManifestReadError::InstallMetadata(error)), + }; + + // Validate files on non-boot disks (non-fatal, will produce warnings if + // errors or mismatches are encountered). + let non_boot_disk_metadata = files + .non_boot_disk_metadata + .into_iter() + .map(|info| ZoneManifestNonBootInfo::new(info)) + .collect::>(); + + let ret = Self { + boot_zpool: files.boot_zpool, + boot_disk_path: files.boot_disk_path, + boot_disk_result, + non_boot_disk_metadata, + }; + + ret.log_results(&log); + ret + } + + pub(crate) fn status(&self) -> ZoneManifestStatus { + ZoneManifestStatus { + boot_disk_path: self.boot_disk_path.clone(), + boot_disk_result: self.boot_disk_result.clone(), + non_boot_disk_metadata: self.non_boot_disk_metadata.clone(), + } + } + + fn log_results(&self, log: &slog::Logger) { + let log = log.new(o!( + "component" => "zone_manifest", + "boot_zpool" => self.boot_zpool.to_string(), + "boot_disk_path" => self.boot_disk_path.to_string(), + )); + + match &self.boot_disk_result { + Ok(result) => { + if result.is_valid() { + info!( + log, + "found zone manifest for boot disk"; + "data" => %result.display(), + ); + } else { + error!( + log, + "zone manifest for boot disk is invalid, \ + will not bring up zones that mismatch"; + "data" => %result.display(), + ); + } + } + Err(error) => { + // This error most likely requires operator intervention -- if + // it happens, we'll continue to bring sled-agent up but reject + // all Omicron zone image lookups. + error!( + log, + "error reading zone manifest for boot disk, \ + will not bring up Omicron zones"; + "error" => InlineErrorChain::new(error), + ); + } + } + + if self.non_boot_disk_metadata.is_empty() { + warn!( + log, + "no non-boot zpools found, unable to verify consistency -- \ + this may be a hardware issue with the non-boot M.2" + ); + } + + for info in &self.non_boot_disk_metadata { + info.log_to(&log); + } + } +} + +#[derive(Clone, Debug, PartialEq)] +pub struct ZoneManifestNonBootInfo { + /// The name of the zpool. + pub zpool_name: ZpoolName, + + /// The dataset directory. + pub dataset_dir: Utf8PathBuf, + + /// The zone manifest path. + pub path: Utf8PathBuf, + + /// The result of performing the read operation. + pub result: ZoneManifestNonBootResult, +} + +impl ZoneManifestNonBootInfo { + pub(crate) fn new( + info: InstallMetadataNonBootInfo, + ) -> Self { + let result = ZoneManifestNonBootResult::new( + &info.dataset_dir, + &info.path, + info.result, + ); + Self { + zpool_name: info.zpool_name, + dataset_dir: info.dataset_dir, + path: info.path, + result, + } + } + + pub(crate) fn log_to(&self, log: &slog::Logger) { + let log = log.new(o!( + "non_boot_zpool" => self.zpool_name.to_string(), + "non_boot_path" => self.path.to_string(), + )); + self.result.log_to(&log); + } +} + +impl IdOrdItem for ZoneManifestNonBootInfo { + type Key<'a> = ZpoolName; + + fn key(&self) -> Self::Key<'_> { + self.zpool_name + } + + id_upcast!(); +} + +#[derive(Clone, Debug, PartialEq)] +pub enum ZoneManifestNonBootResult { + /// The manifest is present and matches the value on the boot disk. + /// Information about individual zone hashes matching is stored in the + /// `ZoneManifestArtifactsResult`. + Matches(ZoneManifestArtifactsResult), + + /// A mismatch between the boot disk and the other disk was detected. + Mismatch(ZoneManifestNonBootMismatch), + + /// An error occurred while reading the zone manifest on this disk. + ReadError(ZoneManifestReadError), +} + +impl ZoneManifestNonBootResult { + fn new( + dataset_dir: &Utf8Path, + path: &Utf8Path, + result: InstallMetadataNonBootResult, + ) -> Self { + match result { + InstallMetadataNonBootResult::MatchesPresent( + non_boot_disk_metadata, + ) => Self::Matches(ZoneManifestArtifactsResult::new( + dataset_dir, + non_boot_disk_metadata, + )), + InstallMetadataNonBootResult::MatchesAbsent => { + // Error case. + Self::ReadError(ZoneManifestReadError::NotFound( + path.to_owned(), + )) + } + InstallMetadataNonBootResult::Mismatch(mismatch) => match mismatch { + InstallMetadataNonBootMismatch::BootPresentOtherAbsent => { + // Error case. + Self::ReadError(ZoneManifestReadError::NotFound( + path.to_owned(), + )) + } + InstallMetadataNonBootMismatch::BootAbsentOtherPresent { + non_boot_disk_info, + } => Self::Mismatch( + ZoneManifestNonBootMismatch::BootAbsentOtherPresent { + non_boot_disk_result: ZoneManifestArtifactsResult::new( + dataset_dir, + non_boot_disk_info, + ), + }, + ), + InstallMetadataNonBootMismatch::ValueMismatch { + non_boot_disk_info, + } => { + Self::Mismatch(ZoneManifestNonBootMismatch::ValueMismatch { + non_boot_disk_result: ZoneManifestArtifactsResult::new( + dataset_dir, + non_boot_disk_info, + ), + }) + } + InstallMetadataNonBootMismatch::BootDiskReadError { + non_boot_disk_info: Some(info), + } => Self::Mismatch( + ZoneManifestNonBootMismatch::BootDiskReadError { + non_boot_disk_result: ZoneManifestArtifactsResult::new( + dataset_dir, + info, + ), + }, + ), + InstallMetadataNonBootMismatch::BootDiskReadError { + non_boot_disk_info: None, + } => Self::ReadError(ZoneManifestReadError::NotFound( + path.to_owned(), + )), + }, + InstallMetadataNonBootResult::ReadError(error) => { + Self::ReadError(error.into()) + } + } + } + + /// Returns true if the status is valid. + /// + /// The only valid status is `Self::Matches` along with the result inside + /// being valid. + pub fn is_valid(&self) -> bool { + match self { + Self::Matches(result) => result.is_valid(), + Self::Mismatch(_) | Self::ReadError(_) => false, + } + } + + fn log_to(&self, log: &slog::Logger) { + match self { + Self::Matches(result) => { + if result.is_valid() { + info!( + log, + "found zone manifest for non-boot disk"; + "data" => %result.display(), + ); + } else { + slog::warn!( + log, + "zone manifest for non-boot disk is invalid"; + "data" => %result.display(), + ); + } + } + Self::Mismatch(mismatch) => match mismatch { + ZoneManifestNonBootMismatch::BootAbsentOtherPresent { + non_boot_disk_result, + } => { + slog::warn!( + log, + "zone manifest absent on boot disk but present on non-boot disk"; + "data" => %non_boot_disk_result.display(), + ); + } + ZoneManifestNonBootMismatch::ValueMismatch { + non_boot_disk_result, + } => { + slog::warn!( + log, + "zone manifest contents differ between boot disk and non-boot disk"; + "data" => %non_boot_disk_result.display(), + ); + } + ZoneManifestNonBootMismatch::BootDiskReadError { + non_boot_disk_result, + } => { + slog::warn!( + log, + "unable to verify zone manifest consistency between \ + boot disk and non-boot disk due to boot disk read error"; + "data" => %non_boot_disk_result.display(), + ); + } + }, + Self::ReadError(error) => { + slog::warn!( + log, + "error reading zone manifest on non-boot disk"; + "error" => InlineErrorChain::new(error), + ); + } + } + } +} + +#[derive(Clone, Debug, PartialEq)] +pub enum ZoneManifestNonBootMismatch { + /// The file is absent on the boot disk but present on the other disk. + BootAbsentOtherPresent { + /// The result of reading the file on the other disk. + non_boot_disk_result: ZoneManifestArtifactsResult, + }, + + /// The file's contents differ between the boot disk and the other disk. + ValueMismatch { non_boot_disk_result: ZoneManifestArtifactsResult }, + + /// There was a read error on the boot disk, so we were unable to verify + /// consistency. + BootDiskReadError { + /// The value as found on this disk. This value is logged but not used. + non_boot_disk_result: ZoneManifestArtifactsResult, + }, +} + +/// The result of reading artifacts from an install dataset. +/// +/// This may or may not be valid, depending on the status of the artifacts. See +/// [`Self::is_valid`]. +#[derive(Clone, Debug, PartialEq)] +pub struct ZoneManifestArtifactsResult { + pub manifest: OmicronZoneManifest, + pub data: IdOrdMap, +} + +impl ZoneManifestArtifactsResult { + /// Makes a new `ZoneManifestArtifactsResult` by reading artifacts from the + /// given directory. + fn new(dir: &Utf8Path, manifest: OmicronZoneManifest) -> Self { + let artifacts: Vec<_> = manifest + .zones + .iter() + // Parallelize artifact reading to speed it up. + .par_bridge() + .map(|zone| { + let artifact_path = dir.join(&zone.file_name); + let status = validate_one(&artifact_path, &zone); + + ZoneManifestArtifactResult { + file_name: zone.file_name.clone(), + path: artifact_path, + expected_size: zone.file_size, + expected_hash: zone.hash, + status, + } + }) + .collect(); + + Self { manifest, data: artifacts.into_iter().collect() } + } + + /// Returns true if all artifacts are valid. + pub fn is_valid(&self) -> bool { + self.data.iter().all(|artifact| artifact.is_valid()) + } + + /// Returns a displayable representation of the artifacts. + pub fn display(&self) -> ZoneManifestArtifactsDisplay<'_> { + ZoneManifestArtifactsDisplay { artifacts: &self.data } + } +} + +pub struct ZoneManifestArtifactsDisplay<'a> { + artifacts: &'a IdOrdMap, +} + +impl fmt::Display for ZoneManifestArtifactsDisplay<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + for artifact in self.artifacts { + match &artifact.status { + ArtifactReadResult::Valid => { + writeln!( + f, + " {}: ok ({} bytes, {})", + artifact.file_name, + artifact.expected_size, + artifact.expected_hash + )?; + } + ArtifactReadResult::Mismatch { actual_size, actual_hash } => { + writeln!( + f, + " {}: mismatch (expected {} bytes, {}; \ + found {} bytes, {})", + artifact.file_name, + artifact.expected_size, + artifact.expected_hash, + actual_size, + actual_hash + )?; + } + ArtifactReadResult::Error(error) => { + writeln!( + f, + " {}: error ({})", + artifact.file_name, + InlineErrorChain::new(error) + )?; + } + } + } + + Ok(()) + } +} + +#[derive(Clone, Debug, PartialEq)] +pub struct ZoneManifestArtifactResult { + /// The filename. + pub file_name: String, + + /// The full path to the file. + pub path: Utf8PathBuf, + + /// The expected size. + pub expected_size: u64, + + /// The expected hash. + pub expected_hash: ArtifactHash, + + /// The status on disk. + pub status: ArtifactReadResult, +} + +impl ZoneManifestArtifactResult { + fn is_valid(&self) -> bool { + matches!(self.status, ArtifactReadResult::Valid) + } +} + +impl IdOrdItem for ZoneManifestArtifactResult { + type Key<'a> = &'a str; + + fn key(&self) -> Self::Key<'_> { + &self.file_name + } + + id_upcast!(); +} + +#[derive(Clone, Debug, Error, PartialEq)] +pub enum ZoneManifestReadError { + #[error("error reading install metadata")] + InstallMetadata(#[from] InstallMetadataReadError), + #[error("zone manifest not found at `{0}`")] + NotFound(Utf8PathBuf), +} + +fn validate_one( + artifact_path: &Utf8Path, + zone: &OmicronZoneFileMetadata, +) -> ArtifactReadResult { + let mut f = match File::open(artifact_path) { + Ok(f) => f, + Err(error) => { + return ArtifactReadResult::Error(ArcIoError::new(error)); + } + }; + + match compute_size_and_hash(&mut f) { + Ok((actual_size, actual_hash)) => { + if zone.file_size == actual_size && zone.hash == actual_hash { + ArtifactReadResult::Valid + } else { + ArtifactReadResult::Mismatch { actual_size, actual_hash } + } + } + Err(error) => ArtifactReadResult::Error(ArcIoError::new(error)), + } +} + +fn compute_size_and_hash( + f: &mut File, +) -> Result<(u64, ArtifactHash), io::Error> { + let mut hasher = Sha256::new(); + // Zone artifacts are pretty big, so we read them in chunks. + let mut buffer = [0u8; 8192]; + let mut total_bytes_read = 0; + loop { + let bytes_read = f.read(&mut buffer)?; + if bytes_read == 0 { + break; + } + hasher.update(&buffer[..bytes_read]); + total_bytes_read += bytes_read; + } + Ok((total_bytes_read as u64, ArtifactHash(hasher.finalize().into()))) +} + +#[derive(Clone, Debug, PartialEq)] +pub enum ArtifactReadResult { + /// The artifact was read successfully and matches. + Valid, + + /// The artifact was read successfully but does not match. + Mismatch { + /// The actual file size. + actual_size: u64, + + /// The actual hash. + actual_hash: ArtifactHash, + }, + + /// An error occurred while reading the artifact. + Error(ArcIoError), +} + +#[cfg(test)] +mod tests { + use super::*; + + use crate::test_utils::{ + BOOT_PATHS, BOOT_ZPOOL, NON_BOOT_2_PATHS, NON_BOOT_2_ZPOOL, + NON_BOOT_3_PATHS, NON_BOOT_3_ZPOOL, NON_BOOT_PATHS, NON_BOOT_ZPOOL, + WriteInstallDatasetContext, deserialize_error, + }; + + use camino_tempfile_ext::prelude::*; + use dropshot::{ConfigLogging, ConfigLoggingLevel, test_util::LogContext}; + use iddqd::id_ord_map; + use pretty_assertions::assert_eq; + + // Much of the logic in this module is shared with mupdate_override.rs, and + // tested there. + + /// Success case: zone manifest JSON present on boot and non-boot disk and matches. + #[test] + fn read_success() { + let logctx = LogContext::new( + "zone_manifest_read_success", + &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug }, + ); + let dir = Utf8TempDir::new().unwrap(); + let cx = WriteInstallDatasetContext::new_basic(); + + // Write the valid manifest to both boot and non-boot disks. + cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); + cx.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); + + let zpools = ZoneImageZpools { + root: dir.path(), + all_m2_zpools: vec![BOOT_ZPOOL, NON_BOOT_ZPOOL], + }; + + let manifests = + AllZoneManifests::read_all(&logctx.log, &zpools, &BOOT_ZPOOL); + + // Boot disk should be valid. + assert_eq!( + manifests.boot_disk_result.as_ref().unwrap(), + &cx.expected_result(&dir.path().join(&BOOT_PATHS.install_dataset)) + ); + + // Non-boot disk should match boot disk. + assert_eq!( + manifests.non_boot_disk_metadata, + id_ord_map! { + ZoneManifestNonBootInfo { + zpool_name: NON_BOOT_ZPOOL, + dataset_dir: dir.path().join(&NON_BOOT_PATHS.install_dataset), + path: dir.path().join(&NON_BOOT_PATHS.zones_json), + result: ZoneManifestNonBootResult::Matches( + cx.expected_result( + &dir.path().join(&NON_BOOT_PATHS.install_dataset) + ) + ) + } + } + ); + + logctx.cleanup_successful(); + } + + /// Error case: zone manifest JSON missing from boot disk. + #[test] + fn read_boot_disk_missing() { + let logctx = LogContext::new( + "zone_manifest_read_boot_disk_missing", + &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug }, + ); + let dir = Utf8TempDir::new().unwrap(); + let cx = WriteInstallDatasetContext::new_basic(); + + // Write the valid manifest to the non-boot disk. + cx.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); + // Create the install dataset directory, but not the manifest, on the + // boot disk. + dir.child(&BOOT_PATHS.install_dataset).create_dir_all().unwrap(); + + let zpools = ZoneImageZpools { + root: dir.path(), + all_m2_zpools: vec![BOOT_ZPOOL, NON_BOOT_ZPOOL], + }; + + let manifests = + AllZoneManifests::read_all(&logctx.log, &zpools, &BOOT_ZPOOL); + assert_eq!( + manifests.boot_disk_result.as_ref().unwrap_err(), + &ZoneManifestReadError::NotFound( + dir.path().join(&BOOT_PATHS.zones_json) + ), + ); + + assert_eq!( + manifests.non_boot_disk_metadata, + id_ord_map! { + ZoneManifestNonBootInfo { + zpool_name: NON_BOOT_ZPOOL, + dataset_dir: dir.path().join(&NON_BOOT_PATHS.install_dataset), + path: dir.path().join(&NON_BOOT_PATHS.zones_json), + result: ZoneManifestNonBootResult::Mismatch( + ZoneManifestNonBootMismatch::BootAbsentOtherPresent { + non_boot_disk_result: cx.expected_result( + &dir.path().join(&NON_BOOT_PATHS.install_dataset) + ), + } + ) + } + } + ); + } + + /// Error case: zones don't match expected ones on boot disk. + #[test] + fn read_boot_disk_zone_mismatch() { + let logctx = LogContext::new( + "zone_manifest_read_boot_disk_zone_mismatch", + &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug }, + ); + let dir = Utf8TempDir::new().unwrap(); + let cx = WriteInstallDatasetContext::new_basic(); + let mut invalid_cx = cx.clone(); + invalid_cx.make_error_cases(); + + // Write the valid manifest to the non-boot disk. + cx.write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)).unwrap(); + // Write the invalid manifest to the boot disk. + invalid_cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); + + let zpools = ZoneImageZpools { + root: dir.path(), + all_m2_zpools: vec![BOOT_ZPOOL, NON_BOOT_ZPOOL], + }; + + let manifests = + AllZoneManifests::read_all(&logctx.log, &zpools, &BOOT_ZPOOL); + assert_eq!( + manifests.boot_disk_result.as_ref().unwrap(), + &invalid_cx + .expected_result(&dir.path().join(&BOOT_PATHS.install_dataset)), + ); + + assert_eq!( + manifests.non_boot_disk_metadata, + id_ord_map! { + ZoneManifestNonBootInfo { + zpool_name: NON_BOOT_ZPOOL, + dataset_dir: dir.path().join(&NON_BOOT_PATHS.install_dataset), + path: dir.path().join(&NON_BOOT_PATHS.zones_json), + result: ZoneManifestNonBootResult::Mismatch( + // The boot disk was read successfully but the zones + // didn't match what was on disk. We could treat this as + // either a ValueMismatch or a BootDiskReadError -- + // currently, we treat it as a ValueMismatch for + // convenience. + ZoneManifestNonBootMismatch::ValueMismatch { + non_boot_disk_result: cx.expected_result( + &dir.path().join(&NON_BOOT_PATHS.install_dataset) + ), + } + ) + } + }, + ); + + logctx.cleanup_successful(); + } + + /// Warning case: zones don't match expected ones on non-boot + /// disk/error/absent. + #[test] + fn read_non_boot_disk_zone_mismatch() { + let logctx = LogContext::new( + "zone_manifest_read_non_boot_disk_zone_mismatch", + &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug }, + ); + let dir = Utf8TempDir::new().unwrap(); + let cx = WriteInstallDatasetContext::new_basic(); + let mut invalid_cx = cx.clone(); + invalid_cx.make_error_cases(); + + cx.write_to(&dir.child(&BOOT_PATHS.install_dataset)).unwrap(); + invalid_cx + .write_to(&dir.child(&NON_BOOT_PATHS.install_dataset)) + .unwrap(); + // Zone manifest file that's absent. + dir.child(&NON_BOOT_2_PATHS.install_dataset).create_dir_all().unwrap(); + // Read error (empty file). + dir.child(&NON_BOOT_3_PATHS.zones_json).touch().unwrap(); + + let zpools = ZoneImageZpools { + root: dir.path(), + all_m2_zpools: vec![ + BOOT_ZPOOL, + NON_BOOT_ZPOOL, + NON_BOOT_2_ZPOOL, + NON_BOOT_3_ZPOOL, + ], + }; + + let manifests = + AllZoneManifests::read_all(&logctx.log, &zpools, &BOOT_ZPOOL); + // The boot disk is valid. + assert_eq!( + manifests.boot_disk_result.as_ref().unwrap(), + &cx.expected_result(&dir.path().join(&BOOT_PATHS.install_dataset)) + ); + + // The non-boot disks have various error cases. + assert_eq!( + manifests.non_boot_disk_metadata, + id_ord_map! { + ZoneManifestNonBootInfo { + zpool_name: NON_BOOT_ZPOOL, + dataset_dir: dir.path().join(&NON_BOOT_PATHS.install_dataset), + path: dir.path().join(&NON_BOOT_PATHS.zones_json), + result: ZoneManifestNonBootResult::Mismatch( + ZoneManifestNonBootMismatch::ValueMismatch { + non_boot_disk_result: invalid_cx.expected_result( + dir.child(&NON_BOOT_PATHS.install_dataset).as_path() + ), + } + ) + }, + ZoneManifestNonBootInfo { + zpool_name: NON_BOOT_2_ZPOOL, + dataset_dir: dir.path().join(&NON_BOOT_2_PATHS.install_dataset), + path: dir.path().join(&NON_BOOT_2_PATHS.zones_json), + result: ZoneManifestNonBootResult::ReadError( + ZoneManifestReadError::NotFound( + dir.path().join(&NON_BOOT_2_PATHS.zones_json) + ), + ) + }, + ZoneManifestNonBootInfo { + zpool_name: NON_BOOT_3_ZPOOL, + dataset_dir: dir.path().join(&NON_BOOT_3_PATHS.install_dataset), + path: dir.path().join(&NON_BOOT_3_PATHS.zones_json), + result: ZoneManifestNonBootResult::ReadError( + deserialize_error( + dir.path(), + &NON_BOOT_3_PATHS.zones_json, + "", + ) + .into() + ) + } + }, + ); + + logctx.cleanup_successful(); + } +} diff --git a/uuid-kinds/src/lib.rs b/uuid-kinds/src/lib.rs index 1d10e9642ec..25d24432d71 100644 --- a/uuid-kinds/src/lib.rs +++ b/uuid-kinds/src/lib.rs @@ -65,6 +65,7 @@ impl_typed_uuid_kind! { ExternalIp => "external_ip", Instance => "instance", LoopbackAddress => "loopback_address", + Mupdate => "mupdate", MupdateOverride => "mupdate_override", // `OmicronSledConfig`s do not themselves contain IDs, but we generate IDs // for them when they're serialized to the database during inventory diff --git a/wicketd/src/installinator_progress.rs b/wicketd/src/installinator_progress.rs index 8f8465652e4..51160140869 100644 --- a/wicketd/src/installinator_progress.rs +++ b/wicketd/src/installinator_progress.rs @@ -13,9 +13,9 @@ use std::{ }; use installinator_api::EventReportStatus; +use omicron_uuid_kinds::MupdateUuid; use tokio::sync::{oneshot, watch}; use update_engine::events::StepEventIsTerminal; -use uuid::Uuid; /// Creates the artifact server and update tracker's interfaces to the /// installinator progress tracker. @@ -45,13 +45,13 @@ pub(crate) struct IprArtifactServer { log: slog::Logger, // Note: this is a std::sync::Mutex because it isn't held past an await // point. Tokio mutexes have cancel-safety issues. - running_updates: Arc>>, + running_updates: Arc>>, } impl IprArtifactServer { pub(crate) fn report_progress( &self, - update_id: Uuid, + update_id: MupdateUuid, report: installinator_common::EventReport, ) -> EventReportStatus { let mut running_updates = self.running_updates.lock().unwrap(); @@ -120,7 +120,7 @@ impl IprArtifactServer { #[must_use] pub struct IprUpdateTracker { log: slog::Logger, - running_updates: Arc>>, + running_updates: Arc>>, } impl IprUpdateTracker { @@ -131,7 +131,7 @@ impl IprUpdateTracker { /// /// Exposed for testing. #[doc(hidden)] - pub fn register(&self, update_id: Uuid) -> IprStartReceiver { + pub fn register(&self, update_id: MupdateUuid) -> IprStartReceiver { slog::debug!(self.log, "registering new update id"; "update_id" => %update_id); let (start_sender, start_receiver) = oneshot::channel(); @@ -142,7 +142,10 @@ impl IprUpdateTracker { /// Returns the status of a running update, or None if the update ID hasn't /// been registered. - pub fn update_state(&self, update_id: Uuid) -> Option { + pub fn update_state( + &self, + update_id: MupdateUuid, + ) -> Option { let running_updates = self.running_updates.lock().unwrap(); running_updates.get(&update_id).map(|x| x.to_state()) } @@ -302,6 +305,7 @@ mod tests { use omicron_test_utils::dev::test_setup_log; use schemars::JsonSchema; use update_engine::ExecutionId; + use uuid::Uuid; use super::*; @@ -311,11 +315,11 @@ mod tests { let (ipr_artifact, ipr_update_tracker) = new(&logctx.log); - let update_id = Uuid::new_v4(); + let update_id = MupdateUuid::new_v4(); assert_eq!( ipr_artifact.report_progress( - Uuid::new_v4(), + MupdateUuid::new_v4(), installinator_common::EventReport::default() ), EventReportStatus::UnrecognizedUpdateId, diff --git a/wicketd/src/update_tracker.rs b/wicketd/src/update_tracker.rs index d94e179b9ef..80b10bd8b3b 100644 --- a/wicketd/src/update_tracker.rs +++ b/wicketd/src/update_tracker.rs @@ -37,6 +37,7 @@ use installinator_common::InstallinatorCompletionMetadata; use installinator_common::InstallinatorSpec; use installinator_common::WriteOutput; use omicron_common::disk::M2Slot; +use omicron_uuid_kinds::MupdateUuid; use semver::Version; use slog::Logger; use slog::error; @@ -496,7 +497,7 @@ impl SpawnUpdateDriver for RealSpawnUpdateDriver<'_> { // Generate an ID for this update; the update tracker will send it to the // sled as part of the InstallinatorImageId, and installinator will send it // back to our artifact server with its progress reports. - let update_id = Uuid::new_v4(); + let update_id = MupdateUuid::new_v4(); let event_buffer = Arc::new(StdMutex::new(EventBuffer::new(16))); let ipr_start_receiver = @@ -1748,7 +1749,7 @@ fn simulate_result( } struct UpdateContext { - update_id: Uuid, + update_id: MupdateUuid, sp: SpIdentifier, mgs_client: gateway_client::Client, upload_trampoline_phase_2_to_mgs: diff --git a/wicketd/tests/integration_tests/updates.rs b/wicketd/tests/integration_tests/updates.rs index a03d6095a67..b5081a5f80f 100644 --- a/wicketd/tests/integration_tests/updates.rs +++ b/wicketd/tests/integration_tests/updates.rs @@ -14,15 +14,14 @@ use gateway_test_utils::setup as gateway_setup; use illumos_utils::zpool::ZpoolName; use installinator::HOST_PHASE_2_FILE_NAME; use maplit::btreeset; -use omicron_common::update::MupdateOverrideInfo; -use omicron_uuid_kinds::ZpoolUuid; +use omicron_common::update::{MupdateOverrideInfo, OmicronZoneManifest}; +use omicron_uuid_kinds::{MupdateUuid, ZpoolUuid}; use sled_agent_zone_images::{ MupdateOverrideNonBootResult, ZoneImageSourceResolver, ZoneImageZpools, }; use tokio::sync::oneshot; use tufaceous_artifact::{ArtifactHashId, ArtifactKind, KnownArtifactKind}; use update_engine::NestedError; -use uuid::Uuid; use wicket::OutputKind; use wicket_common::{ inventory::{SpIdentifier, SpType}, @@ -350,7 +349,7 @@ async fn test_installinator_fetch() { // Create a new update ID and register it. This is required to ensure the // installinator reaches completion. - let update_id = Uuid::new_v4(); + let update_id = MupdateUuid::new_v4(); let start_receiver = wicketd_testctx.server.ipr_update_tracker.register(update_id); @@ -456,18 +455,46 @@ async fn test_installinator_fetch() { "mupdate override info matches across A and B drives", ); + // Ensure that the zone manifest can be parsed. + let a_manifest_path = + a_path.join("install").join(OmicronZoneManifest::FILE_NAME); + let a_manifest_bytes = std::fs::read(a_manifest_path) + .expect("zone manifest file successfully read"); + let a_manifest = + serde_json::from_slice::(&a_manifest_bytes) + .expect("zone manifest file successfully deserialized"); + + // Check that the mupdate ID matches. + assert_eq!(a_manifest.mupdate_id, update_id, "mupdate ID matches"); + // Check that the zone1 and zone2 images are present in the zone set. (The // names come from fake-non-semver.toml, under // [artifact.control-plane.source]). assert!( - a_override_info.zones.contains_key("zone1.tar.gz"), + a_manifest.zones.contains_key("zone1.tar.gz"), "zone1 is present in the zone set" ); assert!( - a_override_info.zones.contains_key("zone2.tar.gz"), + a_manifest.zones.contains_key("zone2.tar.gz"), "zone2 is present in the zone set" ); + // Ensure that the B path also had the same file written out. + let b_manifest_path = + b_path.join("install").join(OmicronZoneManifest::FILE_NAME); + assert!(b_manifest_path.is_file(), "{b_manifest_path} was written out"); + // Ensure that the zone manifest can be parsed. + let b_override_bytes = std::fs::read(b_manifest_path) + .expect("zone manifest file successfully read"); + let b_manifest = + serde_json::from_slice::(&b_override_bytes) + .expect("zone manifest file successfully deserialized"); + + assert_eq!( + a_manifest, b_manifest, + "zone manifests match across A and B drives" + ); + // Run sled-agent-zone-images against these paths, and ensure that the // mupdate override is correctly picked up. Pick zpool1 arbitrarily as the boot zpool. let boot_zpool = ZpoolName::new_internal(zpool1_uuid); @@ -479,15 +506,41 @@ async fn test_installinator_fetch() { let image_resolver = ZoneImageSourceResolver::new(&log, &zpools, &boot_zpool); - // Ensure that the resolver picks up the mupdate override. - let override_status = image_resolver.status().mupdate_override; - eprintln!("override_status: {:#?}", override_status); + // Ensure that the resolver picks up the zone manifest and mupdate override. + let status = image_resolver.status(); + eprintln!("status: {:#?}", status); + + // Zone manifest: + let zone_manifest_status = status.zone_manifest; + let result = zone_manifest_status + .boot_disk_result + .expect("zone manifest successful"); + assert!(result.is_valid(), "zone manifest: boot disk result is valid"); + assert_eq!( + result.manifest, a_manifest, + "zone manifest: manifest matches a_manifest" + ); + + let non_boot_result = zone_manifest_status + .non_boot_disk_metadata + .get(&non_boot_zpool) + .expect("non-boot disk result should be present"); + assert!( + non_boot_result.result.is_valid(), + "zone manifest: non-boot disk result is valid" + ); + + // Mupdate override: + let override_status = status.mupdate_override; let info = override_status .boot_disk_override .expect("mupdate override successful") .expect("mupdate override present"); - assert_eq!(info, a_override_info, "info matches a_override_info"); + assert_eq!( + info, a_override_info, + "mupdate override: info matches a_override_info" + ); let non_boot_status = override_status .non_boot_disk_overrides @@ -496,6 +549,7 @@ async fn test_installinator_fetch() { assert_eq!( non_boot_status.result, MupdateOverrideNonBootResult::MatchesPresent, + "mupdate override: non-boot disk status matches present", ); recv_handle.await.expect("recv_handle succeeded");