From 97f4c852092b7adc3530434ebfc15ce3047c7f94 Mon Sep 17 00:00:00 2001 From: lif <> Date: Fri, 11 Aug 2023 19:23:08 +0000 Subject: [PATCH] update to zone 0.3 for list_blocking --- Cargo.lock | 10 +- Cargo.toml | 2 +- sled-agent/src/storage/dump_setup.rs | 193 +++++++++++++-------------- 3 files changed, 98 insertions(+), 107 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 99ce4087ed7..850bb5e2e06 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9106,7 +9106,7 @@ version = "1.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "97fee6b57c6a41524a810daee9286c02d7752c4253064d0b05472833a438f675" dependencies = [ - "cfg-if 0.1.10", + "cfg-if 1.0.0", "rand 0.8.5", "static_assertions", ] @@ -10135,9 +10135,9 @@ dependencies = [ [[package]] name = "zone" -version = "0.2.0" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b0545a42fbd7a81245726d54a0146cb4fd93882ebb6da50d60acf2e37394f198" +checksum = "a62a428a79ea2224ce8ab05d6d8a21bdd7b4b68a8dbc1230511677a56e72ef22" dependencies = [ "itertools 0.10.5", "thiserror", @@ -10147,9 +10147,9 @@ dependencies = [ [[package]] name = "zone_cfg_derive" -version = "0.2.0" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ef224b009d070d3b1adb9e375fcf8ec2f1948a412c3bbf8755c0ef4e3f91ef94" +checksum = "d5c4f01d3785e222d5aca11c9813e9c46b69abfe258756c99c9b628683626cc8" dependencies = [ "heck 0.4.1", "proc-macro-error", diff --git a/Cargo.toml b/Cargo.toml index 247a94c5613..b4cf6f942ed 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -363,7 +363,7 @@ wicket-common = { path = "wicket-common" } wicketd-client = { path = "wicketd-client" } zeroize = { version = "1.6.0", features = ["zeroize_derive", "std"] } zip = { version = "0.6.6", default-features = false, features = ["deflate","bzip2"] } -zone = { version = "0.2", default-features = false, features = ["async"] } +zone = { version = "0.3", default-features = false, features = ["async", "sync"] } [profile.dev] panic = "abort" diff --git a/sled-agent/src/storage/dump_setup.rs b/sled-agent/src/storage/dump_setup.rs index a58645b4e14..a8507ddd4b7 100644 --- a/sled-agent/src/storage/dump_setup.rs +++ b/sled-agent/src/storage/dump_setup.rs @@ -217,28 +217,6 @@ pub(self) enum ZfsGetError { Parse(#[from] std::num::ParseIntError), } -#[cfg(test)] -impl Clone for ZfsGetError { - fn clone(&self) -> Self { - match self { - ZfsGetError::IoError(_err) => unimplemented!(), - ZfsGetError::Utf8(err) => ZfsGetError::Utf8(err.clone()), - ZfsGetError::Parse(err) => ZfsGetError::Parse(err.clone()), - } - } -} - -#[cfg(test)] -#[derive(Default)] -struct Fake { - pub zpool_props: HashMap< - &'static str, - HashMap<&'static str, Result>, - >, - pub zones: Vec, -} -struct Real {} - trait Invoker { fn coreadm(&self, core_dir: &Utf8PathBuf) -> Result<(), ExecutionError>; fn dumpadm( @@ -283,71 +261,7 @@ trait Invoker { fn get_zones(&self) -> Result, ArchiveLogsError>; } -#[cfg(test)] -impl Invoker for Fake { - fn coreadm(&self, _core_dir: &Utf8PathBuf) -> Result<(), ExecutionError> { - Ok(()) - } - - fn dumpadm( - &self, - _dump_slice: &Utf8PathBuf, - _savecore_dir: Option<&Utf8PathBuf>, - ) -> Result, ExecutionError> { - Ok(None) - } - - fn zfs_get_prop( - &self, - mountpoint_or_name: impl AsRef + Sized, - property: &str, - ) -> Result { - self.zpool_props - .get(mountpoint_or_name.as_ref()) - .unwrap_or_else(|| { - panic!( - "Test did not provide fake zpool {}", - mountpoint_or_name.as_ref() - ) - }) - .get(property) - .unwrap_or_else(|| { - panic!( - "Test did not provide property {property} for fake zpool {}", - mountpoint_or_name.as_ref() - ) - }) - .clone() - } - - fn mountpoint( - &self, - zpool: &ZpoolName, - mountpoint: &'static str, - ) -> Utf8PathBuf { - Utf8PathBuf::from( - self.zpool_props - .get(zpool.to_string().as_str()) - .unwrap_or_else(|| { - panic!("Test did not provide fake zpool {}", zpool) - }) - .get("mountpoint") - .unwrap_or_else(|| { - panic!( - "Test did not provide mountpoint for fake zpool {}", - zpool - ) - }) - .clone() - .unwrap(), - ) - .join(mountpoint) - } - - fn get_zones(&self) -> Result, ArchiveLogsError> { - Ok(self.zones.clone()) - } -} +struct Real {} impl Invoker for Real { fn coreadm(&self, core_dir: &Utf8PathBuf) -> Result<(), ExecutionError> { @@ -451,18 +365,10 @@ impl Invoker for Real { } fn get_zones(&self) -> Result, ArchiveLogsError> { - // zone crate's 'deprecated' functions collide if you try to enable - // its 'sync' and 'async' features simultaneously :( - let rt = - tokio::runtime::Runtime::new().map_err(ArchiveLogsError::Tokio)?; - - rt.block_on(async { - Ok(zone::Adm::list() - .await? - .into_iter() - .filter(|z| z.global() || z.name().starts_with(ZONE_PREFIX)) - .collect::>()) - }) + Ok(zone::Adm::list_blocking()? + .into_iter() + .filter(|z| z.global() || z.name().starts_with(ZONE_PREFIX)) + .collect::>()) } } @@ -988,8 +894,6 @@ impl DumpSetupWorker { #[cfg_attr(test, allow(dead_code))] // mock doesn't construct Tokio variant #[derive(thiserror::Error, Debug)] pub enum ArchiveLogsError { - #[error("Couldn't make an async runtime to get zone info: {0}")] - Tokio(std::io::Error), #[error("I/O error: {0}")] IoError(#[from] std::io::Error), #[error("Error calling zoneadm: {0}")] @@ -1036,6 +940,93 @@ mod tests { use std::str::FromStr; use tempfile::TempDir; + impl Clone for ZfsGetError { + fn clone(&self) -> Self { + match self { + ZfsGetError::IoError(_err) => unimplemented!(), + ZfsGetError::Utf8(err) => ZfsGetError::Utf8(err.clone()), + ZfsGetError::Parse(err) => ZfsGetError::Parse(err.clone()), + } + } + } + + #[derive(Default)] + struct Fake { + pub zpool_props: HashMap< + &'static str, + HashMap<&'static str, Result>, + >, + pub zones: Vec, + } + + impl Invoker for Fake { + fn coreadm( + &self, + _core_dir: &Utf8PathBuf, + ) -> Result<(), ExecutionError> { + Ok(()) + } + + fn dumpadm( + &self, + _dump_slice: &Utf8PathBuf, + _savecore_dir: Option<&Utf8PathBuf>, + ) -> Result, ExecutionError> { + Ok(None) + } + + fn zfs_get_prop( + &self, + mountpoint_or_name: impl AsRef + Sized, + property: &str, + ) -> Result { + self.zpool_props + .get(mountpoint_or_name.as_ref()) + .unwrap_or_else(|| { + panic!( + "Test did not provide fake zpool {}", + mountpoint_or_name.as_ref() + ) + }) + .get(property) + .unwrap_or_else(|| { + panic!( + "Test did not provide property {property} for fake zpool {}", + mountpoint_or_name.as_ref() + ) + }) + .clone() + } + + fn mountpoint( + &self, + zpool: &ZpoolName, + mountpoint: &'static str, + ) -> Utf8PathBuf { + Utf8PathBuf::from( + self.zpool_props + .get(zpool.to_string().as_str()) + .unwrap_or_else(|| { + panic!("Test did not provide fake zpool {}", zpool) + }) + .get("mountpoint") + .unwrap_or_else(|| { + panic!( + "Test did not provide mountpoint for fake zpool {}", + zpool + ) + }) + .clone() + .unwrap(), + ) + .join(mountpoint) + } + + fn get_zones(&self) -> Result, ArchiveLogsError> { + Ok(self.zones.clone()) + } + } + #[test] fn test_does_not_configure_coreadm_when_no_crash_dataset_mounted() { let logctx = omicron_test_utils::dev::test_setup_log(