diff --git a/sled-agent/config-reconciler/src/ledger.rs b/sled-agent/config-reconciler/src/ledger.rs index f94f8a23f5e..b00db0458d2 100644 --- a/sled-agent/config-reconciler/src/ledger.rs +++ b/sled-agent/config-reconciler/src/ledger.rs @@ -29,9 +29,9 @@ use tufaceous_artifact::ArtifactHash; use crate::InternalDisksReceiver; use crate::SledAgentArtifactStore; -use ledgered_sled_config_versioning::read_ledgered_sled_config; +use ledgered_config_versioning::read_ledgered_sled_config; -mod ledgered_sled_config_versioning; +pub(crate) mod ledgered_config_versioning; const CONFIG_LEDGER_FILENAME: &str = "omicron-sled-config.json"; diff --git a/sled-agent/config-reconciler/src/ledger/ledgered_sled_config_versioning.rs b/sled-agent/config-reconciler/src/ledger/ledgered_config_versioning.rs similarity index 79% rename from sled-agent/config-reconciler/src/ledger/ledgered_sled_config_versioning.rs rename to sled-agent/config-reconciler/src/ledger/ledgered_config_versioning.rs index 66f3e2821be..b80217f9b7d 100644 --- a/sled-agent/config-reconciler/src/ledger/ledgered_sled_config_versioning.rs +++ b/sled-agent/config-reconciler/src/ledger/ledgered_config_versioning.rs @@ -2,14 +2,16 @@ // 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/. -//! Module for converting older formats of the sled configuration files. +//! Module for converting older formats of Sled Agent's ledgered config files. use camino::Utf8PathBuf; use omicron_ledger::Ledger; use omicron_ledger::Ledgerable; use serde::Deserialize; use serde::Serialize; +use sled_agent_types::artifact::ArtifactConfig; use sled_agent_types::inventory::OmicronSledConfig; +use sled_agent_types_versions::v1; use sled_agent_types_versions::v4; use sled_agent_types_versions::v10; use sled_agent_types_versions::v11; @@ -23,12 +25,12 @@ use slog::warn; use slog_error_chain::InlineErrorChain; use std::error::Error as StdError; -/// Trait describing an ordered sequence of `OmicronSledConfig` versions, each +/// Trait describing an ordered sequence of versions of a ledgered config, each /// of which can be converted from its previous version. /// -/// When adding a new [`OmicronSledConfig`] version, add your new version to the -/// `version_conversion_chain!()` invocation below. Use the fully-versioned name -/// (e.g., `vN::inventory::OmicronSledConfig`), not the [`OmicronSledConfig`] +/// When adding a new version of a ledgered config, add your new version to the +/// relevant `version_conversion_chain!()` invocation below. Use the +/// fully-versioned name (e.g., `vN::inventory::OmicronSledConfig`), not the /// alias from `latest`. /// /// Also update the unit tests at the bottom of this file to cover your new @@ -43,14 +45,14 @@ trait VersionConversionChain: Ledgerable { // recursing. const IS_TERMINAL: bool = false; - /// The previous [`OmicronSledConfig`] version, which must be convertible - /// into this version. + /// The previous version of this config, which must be convertible into + /// this version. type Previous: VersionConversionChain + TryInto; } macro_rules! version_conversion_chain { // base case - ($current:path, $previous:path) => { + ($current:path, $previous:path $(,)?) => { impl VersionConversionChain for $current { const DESCRIPTION: &str = stringify!($current); type Previous = $previous; @@ -64,9 +66,9 @@ macro_rules! version_conversion_chain { }; } -// This list is ordered from newest to oldest; this is the order in which we'll -// attempt to parse the ledgered config. Add new versions to the top of the -// list. +// These lists are ordered from newest to oldest; this is the order in which +// we'll attempt to parse the ledgered config. Add new versions to the top of +// the relevant list. version_conversion_chain!( v51::inventory::OmicronSledConfig, v50::inventory::OmicronSledConfig, @@ -78,6 +80,11 @@ version_conversion_chain!( VersionConversionChainTerminal, ); +version_conversion_chain!( + v1::artifact::ArtifactConfig, + VersionConversionChainTerminal, +); + /// Read the ledgered [`OmicronSledConfig`], converting from older versions if /// needed. /// @@ -97,25 +104,48 @@ pub(super) async fn read_ledgered_sled_config( log: &Logger, paths: Vec, ) -> Option { + read_ledgered_config(log, paths).await +} + +/// Read the ledgered [`ArtifactConfig`], converting from older versions if +/// needed. +/// +/// # Panics +/// +/// As with `read_ledgered_sled_config`, this panics if we can read a config +/// of some known older version but cannot convert it to the latest version. +pub async fn read_ledgered_artifact_config( + log: &Logger, + paths: Vec, +) -> Option { + read_ledgered_config(log, paths).await +} + +async fn read_ledgered_config( + log: &Logger, + paths: Vec, +) -> Option +where + T: VersionConversionChain + Clone, +{ // Attempt to read the ledger as the current version; if this succeeds, // we're done. - if let Some(config) = Ledger::new(log, paths.clone()).await { - info!(log, "Ledger of sled config exists"); + if let Some(config) = Ledger::::new(log, paths.clone()).await { + info!(log, "Ledger of config exists"; "version" => T::DESCRIPTION); return Some(config.into_inner()); } // Try to read the config as the previous version; if we have an older // version on disk, this will recurse until we get to it, but then convert // it up through our previous version before returning. - let prev_version = try_ledgered_config_versions_chain::< - ::Previous, - >(log, paths.clone()) - .await?; + let prev_version = + try_ledgered_config_versions_chain::(log, paths.clone()) + .await?; let current_version = prev_version.try_into().unwrap_or_else(|e| { panic!( "failed to convert {} to the current version: {}", - ::DESCRIPTION, + T::DESCRIPTION, InlineErrorChain::new(&e) ); }); @@ -199,12 +229,12 @@ where } } -async fn write_converted_ledger( +async fn write_converted_ledger( log: &Logger, paths: Vec, - sled_config: OmicronSledConfig, -) -> OmicronSledConfig { - let mut config_ledger = Ledger::new_with(log, paths.clone(), sled_config); + config: T, +) -> T { + let mut config_ledger = Ledger::new_with(log, paths.clone(), config); match config_ledger.commit().await { Ok(()) => (), @@ -215,7 +245,7 @@ async fn write_converted_ledger( // next time we run. warn!( log, - "Failed to write new sled config converted from \ + "Failed to write new config converted from \ from older version"; InlineErrorChain::new(&err), ); @@ -261,6 +291,16 @@ impl TryFrom } } +impl TryFrom for v1::artifact::ArtifactConfig { + type Error = std::io::Error; + + fn try_from( + _: VersionConversionChainTerminal, + ) -> Result { + unreachable!("terminal type is uninhabitable") + } +} + #[cfg(test)] pub(super) mod tests { use super::*; @@ -269,22 +309,25 @@ pub(super) mod tests { use omicron_test_utils::dev; // v4 config collected from a test system. - const V4_CONFIG_PATH: &str = "test-data/v4-sled-config.json"; + const V4_SLED_CONFIG_PATH: &str = "test-data/v4-sled-config.json"; // paths for expectorate checks - const EXPECTORATE_V10_CONFIG_PATH: &str = + const EXPECTORATE_V10_SLED_CONFIG_PATH: &str = "expectorate/v10-sled-config.json"; - const EXPECTORATE_V11_CONFIG_PATH: &str = + const EXPECTORATE_V11_SLED_CONFIG_PATH: &str = "expectorate/v11-sled-config.json"; - const EXPECTORATE_V14_CONFIG_PATH: &str = + const EXPECTORATE_V14_SLED_CONFIG_PATH: &str = "expectorate/v14-sled-config.json"; - const EXPECTORATE_V49_CONFIG_PATH: &str = + const EXPECTORATE_V49_SLED_CONFIG_PATH: &str = "expectorate/v49-sled-config.json"; - const EXPECTORATE_V50_CONFIG_PATH: &str = + const EXPECTORATE_V50_SLED_CONFIG_PATH: &str = "expectorate/v50-sled-config.json"; - const EXPECTORATE_V51_CONFIG_PATH: &str = + const EXPECTORATE_V51_SLED_CONFIG_PATH: &str = "expectorate/v51-sled-config.json"; + // v1 artifact config collected from a test system. + const V1_ARTIFACT_CONFIG_PATH: &str = "test-data/v1-artifact-config.json"; + // This is solely an expectorate test to guarantee: // // * the conversions for various versions function (at least starting from @@ -297,7 +340,7 @@ pub(super) mod tests { let v4 = Ledger::::new( log, - vec![V4_CONFIG_PATH.into()], + vec![V4_SLED_CONFIG_PATH.into()], ) .await .expect("read v4 from test-data") @@ -317,27 +360,27 @@ pub(super) mod tests { let v51 = v51::inventory::OmicronSledConfig::from(v50.clone()); expectorate::assert_contents( - EXPECTORATE_V10_CONFIG_PATH, + EXPECTORATE_V10_SLED_CONFIG_PATH, &serde_json::to_string_pretty(&v10).unwrap(), ); expectorate::assert_contents( - EXPECTORATE_V11_CONFIG_PATH, + EXPECTORATE_V11_SLED_CONFIG_PATH, &serde_json::to_string_pretty(&v11).unwrap(), ); expectorate::assert_contents( - EXPECTORATE_V14_CONFIG_PATH, + EXPECTORATE_V14_SLED_CONFIG_PATH, &serde_json::to_string_pretty(&v14).unwrap(), ); expectorate::assert_contents( - EXPECTORATE_V49_CONFIG_PATH, + EXPECTORATE_V49_SLED_CONFIG_PATH, &serde_json::to_string_pretty(&v49).unwrap(), ); expectorate::assert_contents( - EXPECTORATE_V50_CONFIG_PATH, + EXPECTORATE_V50_SLED_CONFIG_PATH, &serde_json::to_string_pretty(&v50).unwrap(), ); expectorate::assert_contents( - EXPECTORATE_V51_CONFIG_PATH, + EXPECTORATE_V51_SLED_CONFIG_PATH, &serde_json::to_string_pretty(&v51).unwrap(), ); logctx.cleanup_successful(); @@ -358,16 +401,16 @@ pub(super) mod tests { let counts = check_ledger_reads::( log, - EXPECTORATE_V51_CONFIG_PATH, - Some(V4_CONFIG_PATH), + EXPECTORATE_V51_SLED_CONFIG_PATH, + Some(V4_SLED_CONFIG_PATH), &[ - V4_CONFIG_PATH, - EXPECTORATE_V10_CONFIG_PATH, - EXPECTORATE_V11_CONFIG_PATH, - EXPECTORATE_V14_CONFIG_PATH, - EXPECTORATE_V49_CONFIG_PATH, - EXPECTORATE_V50_CONFIG_PATH, - EXPECTORATE_V51_CONFIG_PATH, + V4_SLED_CONFIG_PATH, + EXPECTORATE_V10_SLED_CONFIG_PATH, + EXPECTORATE_V11_SLED_CONFIG_PATH, + EXPECTORATE_V14_SLED_CONFIG_PATH, + EXPECTORATE_V49_SLED_CONFIG_PATH, + EXPECTORATE_V50_SLED_CONFIG_PATH, + EXPECTORATE_V51_SLED_CONFIG_PATH, ], |paths| read_ledgered_sled_config(log, paths), ) @@ -389,6 +432,44 @@ pub(super) mod tests { logctx.cleanup_successful(); } + #[tokio::test] + async fn read_artifact_config_converts_from_older_versions() { + let logctx = dev::test_setup_log( + "read_artifact_config_converts_from_older_versions", + ); + let log = &logctx.log; + + // Use an explicit type so that adding a new artifact config version + // breaks compilation here. Bump the version and add the new version's + // path to the array of ledger paths below. + type LatestConfig = v1::artifact::ArtifactConfig; + + let counts = check_ledger_reads::( + log, + V1_ARTIFACT_CONFIG_PATH, + None, + &[V1_ARTIFACT_CONFIG_PATH], + |paths| read_ledgered_artifact_config(log, paths), + ) + .await; + + // For now, the v1 wire format parses correctly as the latest config + // version. Once we make an incompatible change, switch this to + // `converted > 0`. + assert_eq!( + counts.converted, 0, + "all artifact config versions share a wire format, so nothing \ + should have needed conversion" + ); + assert!( + counts.unchanged > 0, + "no fixture parsed as the latest version; \ + the no-conversion path is untested" + ); + + logctx.cleanup_successful(); + } + /// How many fixtures took each branch of [`check_ledger_reads`]. struct LedgerReadCounts { /// Fixtures that did not parse as the latest version, and so had to be diff --git a/sled-agent/config-reconciler/src/lib.rs b/sled-agent/config-reconciler/src/lib.rs index 9bb0c0435dc..8cf173a035e 100644 --- a/sled-agent/config-reconciler/src/lib.rs +++ b/sled-agent/config-reconciler/src/lib.rs @@ -77,6 +77,7 @@ pub use internal_disks::InternalDisksWithBootDisk; pub use ledger::LedgerArtifactConfigError; pub use ledger::LedgerNewConfigError; pub use ledger::LedgerTaskError; +pub use ledger::ledgered_config_versioning::read_ledgered_artifact_config; pub use mupdate_override::ResolverStatusExt; pub use raw_disks::RawDisksSender; pub use reconciler_task::CurrentlyManagedZpools; diff --git a/sled-agent/config-reconciler/test-data/v1-artifact-config.json b/sled-agent/config-reconciler/test-data/v1-artifact-config.json new file mode 100644 index 00000000000..12df56e851a --- /dev/null +++ b/sled-agent/config-reconciler/test-data/v1-artifact-config.json @@ -0,0 +1,8 @@ +{ + "generation": 7, + "artifacts": [ + "0d5f5c7b95c30f0a9d0eff0e2a99c8dfa53a1de4d3ba0c67ec5c39e5cf7d0e21", + "55e5c1d2b0b60ca1bd0e50a7cbbc22b0aa6b0d4f1c8d9c2a4b3ee9d6d2f0a1c3", + "988cab6ea184b7912350e5af151bd18152ee2702bad7dc0b977d414eb8062e27" + ] +} diff --git a/sled-agent/src/artifact_store.rs b/sled-agent/src/artifact_store.rs index 507acaff2d3..0b09165967d 100644 --- a/sled-agent/src/artifact_store.rs +++ b/sled-agent/src/artifact_store.rs @@ -41,6 +41,7 @@ use sha2::{Digest, Sha256}; use sled_agent_config_reconciler::ConfigReconcilerHandle; use sled_agent_config_reconciler::InternalDisksReceiver; use sled_agent_config_reconciler::SledAgentArtifactStore; +use sled_agent_config_reconciler::read_ledgered_artifact_config; use sled_agent_types::artifact::ArtifactConfig; use sled_agent_types::artifact::{ArtifactListResponse, ArtifactPutResponse}; use slog::{Logger, error, info}; @@ -135,9 +136,8 @@ impl ArtifactStore { } } - let config = Ledger::new(&log, ledger_paths.clone()) - .await - .map(Ledger::into_inner); + let config = + read_ledgered_artifact_config(&log, ledger_paths.clone()).await; let (config_tx, config) = watch::channel(config); // Somewhat arbitrary bound size, large enough that we should never hit it. let (ledger_tx, ledger_rx) = mpsc::channel(256);