diff options
author | Lars Wirzenius <liw@liw.fi> | 2021-03-25 09:57:21 +0200 |
---|---|---|
committer | Lars Wirzenius <liw@liw.fi> | 2021-03-25 11:34:01 +0200 |
commit | 23138e5bdb8ca751834009d99d85b37e5e4ae5ae (patch) | |
tree | 27e9b77e04a877a3efdb7b9795f5977060b3488a /src | |
parent | 6497e12ae4df4c7cb98992f5cf1948b3eebc486d (diff) | |
download | vmadm-23138e5bdb8ca751834009d99d85b37e5e4ae5ae.tar.gz |
feat: give more useful and specific error messages
Diffstat (limited to 'src')
-rw-r--r-- | src/cloudinit.rs | 33 | ||||
-rw-r--r-- | src/config.rs | 9 | ||||
-rw-r--r-- | src/errors.rs | 4 | ||||
-rw-r--r-- | src/image.rs | 17 | ||||
-rw-r--r-- | src/install.rs | 9 | ||||
-rw-r--r-- | src/libvirt.rs | 100 | ||||
-rw-r--r-- | src/spec.rs | 11 | ||||
-rw-r--r-- | src/sshkeys.rs | 49 |
8 files changed, 174 insertions, 58 deletions
diff --git a/src/cloudinit.rs b/src/cloudinit.rs index 169b13f..e6d2587 100644 --- a/src/cloudinit.rs +++ b/src/cloudinit.rs @@ -13,7 +13,7 @@ use shell_words::quote; use std::default::Default; use std::fs::write; // use std::os::unix::fs::PermissionsExt; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; use tempfile::tempdir; @@ -119,8 +119,8 @@ pub enum CloudInitError { KeyError(#[from] KeyError), /// Something went wrong doing I/O. - #[error(transparent)] - IoError(#[from] std::io::Error), + #[error("could not write to {0}: {1}")] + WriteError(PathBuf, #[source] std::io::Error), /// Something went wrong parsing a string as UTF8. #[error(transparent)] @@ -129,6 +129,14 @@ pub enum CloudInitError { /// Something went wrong parsing or serializing to YAML. #[error(transparent)] SerdeError(#[from] serde_yaml::Error), + + /// Could not create a temporary directory. + #[error("couldn't create a temporary directory: {0}")] + TempDirError(#[source] std::io::Error), + + /// Could not execute command. + #[error("couldn't execute {0}: {1}")] + CommandError(String, #[source] std::io::Error), } #[derive(Clone, Debug, Serialize)] @@ -304,11 +312,13 @@ impl CloudInitConfig { pub fn create_dir(&self, path: &Path) -> Result<(), CloudInitError> { let metadata = path.join("meta-data"); debug!("writing metadata to {}", metadata.display()); - write(&metadata, self.metadata()?)?; + write(&metadata, self.metadata()?) + .map_err(|err| CloudInitError::WriteError(metadata, err))?; let userdata = path.join("user-data"); debug!("writing userdata to {}", userdata.display()); - write(&userdata, self.userdata()?)?; + write(&userdata, self.userdata()?) + .map_err(|err| CloudInitError::WriteError(userdata, err))?; // let scriptsdir = path.join("scripts/per-once"); // let script = scriptsdir.join("vmadm-configure-sshd"); @@ -328,10 +338,13 @@ impl CloudInitConfig { /// The image will be attached to the VM when it starts. /// cloud-init finds it via the volume ID (file system label). pub fn create_iso(&self, filename: &Path) -> Result<(), CloudInitError> { - let dir = tempdir()?; + let dir = match tempdir() { + Ok(path) => path, + Err(err) => return Err(CloudInitError::TempDirError(err)), + }; self.create_dir(dir.path())?; - let r = Command::new("genisoimage") + let r = match Command::new("genisoimage") .arg("-quiet") .arg("-volid") .arg("cidata") @@ -340,7 +353,11 @@ impl CloudInitConfig { .arg("-output") .arg(filename) .arg(dir.path()) - .output()?; + .output() + { + Ok(r) => r, + Err(err) => return Err(CloudInitError::CommandError("genisoimage".to_string(), err)), + }; if !r.status.success() { let stderr = String::from_utf8(r.stderr)?; return Err(CloudInitError::IsoFailed(stderr)); diff --git a/src/config.rs b/src/config.rs index 88c186b..1866971 100644 --- a/src/config.rs +++ b/src/config.rs @@ -41,9 +41,9 @@ pub struct Configuration { /// Errors from this module. #[derive(Debug, thiserror::Error)] pub enum ConfigurationError { - /// I/O error. - #[error(transparent)] - IoError(#[from] std::io::Error), + /// Error reading configuration file. + #[error("couldn't read configuration file {0}")] + ReadError(PathBuf, #[source] std::io::Error), /// YAML parsing error. #[error(transparent)] @@ -55,7 +55,8 @@ impl Configuration { pub fn from_file(filename: &Path) -> Result<Self, ConfigurationError> { if filename.exists() { debug!("reading configuration file {}", filename.display()); - let config = fs::read(filename)?; + let config = fs::read(filename) + .map_err(|err| ConfigurationError::ReadError(filename.to_path_buf(), err))?; let config: Configuration = serde_yaml::from_slice(&config)?; debug!("config: {:#?}", config); Ok(config) diff --git a/src/errors.rs b/src/errors.rs deleted file mode 100644 index ba7875a..0000000 --- a/src/errors.rs +++ /dev/null @@ -1,4 +0,0 @@ -use thiserror::Error; - -#[derive(Debug, Error)] -pub enum VmadminError {} diff --git a/src/image.rs b/src/image.rs index 62f80f7..7b7c2ce 100644 --- a/src/image.rs +++ b/src/image.rs @@ -12,9 +12,13 @@ pub enum ImageError { #[error("qemu-img resize failed: {0}")] ResizeError(String), - /// I/O error. - #[error(transparent)] - IoError(#[from] std::io::Error), + /// Base image copy error. + #[error("could not copy base image {0} to {1}")] + BaseImageCopy(PathBuf, PathBuf, #[source] std::io::Error), + + /// Could not execute command. + #[error("couldn't execute {0}: {1}")] + CommandError(String, #[source] std::io::Error), /// Error parsing a string as UTF8. #[error(transparent)] @@ -30,7 +34,9 @@ pub struct VirtualMachineImage { impl VirtualMachineImage { /// Create new image from a base image. pub fn new_from_base(base_image: &Path, image: &Path) -> Result<Self, ImageError> { - copy(base_image, image)?; + copy(base_image, image).map_err(|err| { + ImageError::BaseImageCopy(base_image.to_path_buf(), image.to_path_buf(), err) + })?; Ok(Self { filename: image.to_path_buf(), }) @@ -47,7 +53,8 @@ impl VirtualMachineImage { .arg("resize") .arg(self.filename()) .arg(format!("{}", new_size)) - .output()?; + .output() + .map_err(|err| ImageError::CommandError("qemu-img resize".to_string(), err))?; if !r.status.success() { let stderr = String::from_utf8(r.stderr)?; return Err(ImageError::ResizeError(stderr)); diff --git a/src/install.rs b/src/install.rs index dce0fea..e74e6ba 100644 --- a/src/install.rs +++ b/src/install.rs @@ -17,9 +17,9 @@ pub enum VirtInstallError { #[error("virt-install failed: {0}")] VirtInstallFailed(String), - /// I/O error. - #[error(transparent)] - IoError(#[from] std::io::Error), + /// Failed to run virt-install. + #[error("couldn't run virt-install")] + Run(#[source] std::io::Error), /// Error parsing a string as UTF8. #[error(transparent)] @@ -115,7 +115,8 @@ pub fn virt_install(args: &VirtInstallArgs, iso: &Path) -> Result<PathBuf, VirtI .arg("--graphics=spice") .arg("--noautoconsole") .arg("--quiet") - .output()?; + .output() + .map_err(|err| VirtInstallError::Run(err))?; if !r.status.success() { let stderr = String::from_utf8(r.stderr)?; return Err(VirtInstallError::VirtInstallFailed(stderr)); diff --git a/src/libvirt.rs b/src/libvirt.rs index 2162f3c..e65ea1e 100644 --- a/src/libvirt.rs +++ b/src/libvirt.rs @@ -13,9 +13,45 @@ use virt::domain::{ /// Errors from this module. #[derive(Debug, thiserror::Error)] pub enum VirtError { - /// Error creating virtual machine. - #[error(transparent)] - VirtError(#[from] virt::error::Error), + /// Error connecting to libvirtd. + #[error("couldn't connect to the libvirt daemon")] + Connect(#[source] virt::error::Error), + + /// Error listing domains. + #[error("couldn't list all domains")] + Domains(#[source] virt::error::Error), + + /// Error listing domains. + #[error("couldn't get name of domain")] + GetName(#[source] virt::error::Error), + + /// Error checking if domain is active. + #[error("couldn't check is domain {0} is active")] + IsActive(String, #[source] virt::error::Error), + + /// Error getting domain's XML description. + #[error("couldn't get domain's XML description: {0}")] + GetXml(String, #[source] virt::error::Error), + + /// Error detaching cloud-init ISO from domain + #[error("couldn't detach cloud-init ISO file from domain {0}")] + DetachIso(String, #[source] virt::error::Error), + + /// Error detaching drive from domain + #[error("couldn't create domain {0}")] + Create(String, #[source] virt::error::Error), + + /// Error shutting down domain + #[error("couldn't shut down domain {0}")] + Shutdown(String, #[source] virt::error::Error), + + /// Error undefining domain + #[error("couldn't undefine domain {0}")] + Undefine(String, #[source] virt::error::Error), + + /// Error undefining domain + #[error("couldn't set domain {0} to be autostarted")] + Autostart(String, #[source] virt::error::Error), /// Error doing I/O. #[error(transparent)] @@ -30,18 +66,21 @@ pub struct Libvirt { impl Libvirt { pub fn connect(url: &str) -> Result<Self, VirtError> { debug!("connecting to libvirtd {}", url); - let conn = Connect::open(url)?; + let conn = Connect::open(url).map_err(|err| VirtError::Connect(err))?; Ok(Self { conn }) } fn get_domains(&self) -> Result<Vec<Domain>, VirtError> { debug!("listing all domains"); - Ok(self.conn.list_all_domains(0)?) + Ok(self + .conn + .list_all_domains(0) + .map_err(|err| VirtError::Domains(err))?) } fn get_domain(&self, name: &str) -> Result<Option<Domain>, VirtError> { for domain in self.get_domains()? { - if domain.get_name()? == name { + if get_name(&domain)? == name { return Ok(Some(domain)); } } @@ -51,14 +90,14 @@ impl Libvirt { pub fn names(&self) -> Result<Vec<String>, VirtError> { let mut ret = vec![]; for domain in self.get_domains()? { - ret.push(domain.get_name()?); + ret.push(get_name(&domain)?); } Ok(ret) } pub fn is_active(&self, name: &str) -> Result<bool, VirtError> { if let Some(domain) = self.get_domain(name)? { - Ok(domain.is_active()?) + Ok(is_active(&domain, name)?) } else { Ok(false) } @@ -76,12 +115,14 @@ impl Libvirt { pub fn detach_cloud_init_iso(&self, name: &str) -> Result<(), VirtError> { if let Some(domain) = self.get_domain(name)? { debug!("detaching cloud-init ISO from {}", name); - let xml = domain.get_xml_desc(0)?; + let xml = get_xml(&domain, name)?; let disk = find_iso_xml(&xml); let flags = VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_AFFECT_LIVE; if disk.len() > 0 { - domain.detach_device_flags(&disk, flags)?; + domain + .detach_device_flags(&disk, flags) + .map_err(|err| VirtError::DetachIso(name.to_string(), err))?; } } Ok(()) @@ -89,9 +130,11 @@ impl Libvirt { pub fn trigger_start(&self, name: &str) -> Result<(), VirtError> { if let Some(domain) = self.get_domain(name)? { - if !domain.is_active()? { + if !is_active(&domain, name)? { debug!("starting {}", name); - domain.create()?; + domain + .create() + .map_err(|err| VirtError::Create(name.to_string(), err))?; } } Ok(()) @@ -108,8 +151,10 @@ impl Libvirt { pub fn trigger_shutdown(&self, name: &str) -> Result<(), VirtError> { if let Some(domain) = self.get_domain(name)? { debug!("asking {} to shut down", name); - if domain.is_active()? { - domain.shutdown()?; + if is_active(&domain, name)? { + domain + .shutdown() + .map_err(|err| VirtError::Shutdown(name.to_string(), err))?; } } Ok(()) @@ -129,7 +174,9 @@ impl Libvirt { self.shutdown(name)?; debug!("undefine {}", name); - domain.undefine()?; + domain + .undefine() + .map_err(|err| VirtError::Undefine(name.to_string(), err))?; debug!("removing image file {}", image.display()); std::fs::remove_file(image)?; @@ -140,12 +187,33 @@ impl Libvirt { pub fn set_autostart(&self, name: &str, autostart: bool) -> Result<(), VirtError> { if let Some(domain) = self.get_domain(name)? { - domain.set_autostart(autostart)?; + domain + .set_autostart(autostart) + .map_err(|err| VirtError::Autostart(name.to_string(), err))?; } Ok(()) } } +fn get_name(domain: &Domain) -> Result<String, VirtError> { + let name = domain.get_name().map_err(|err| VirtError::GetName(err))?; + Ok(name) +} + +fn is_active(domain: &Domain, name: &str) -> Result<bool, VirtError> { + let is = domain + .is_active() + .map_err(|err| VirtError::IsActive(name.to_string(), err))?; + Ok(is) +} + +fn get_xml(domain: &Domain, name: &str) -> Result<String, VirtError> { + let is = domain + .get_xml_desc(0) + .map_err(|err| VirtError::GetXml(name.to_string(), err))?; + Ok(is) +} + fn wait_until_inactive(domain: &Domain, name: &str) { debug!("waiting for domain {} to become inactive", name); let briefly = Duration::from_millis(1000); diff --git a/src/spec.rs b/src/spec.rs index b9828e9..a1d1c2f 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -216,14 +216,14 @@ pub enum SpecificationError { #[error("No SSH authorized keys specified for {0} and no default configured")] NoAuthorizedKeys(String), + /// Error reading specification file. + #[error("Couldn't read specification file {0}")] + Read(PathBuf, #[source] std::io::Error), + /// Error reading SSH public key. #[error("Failed to read SSH public key file {0}")] SshKeyRead(PathBuf, #[source] std::io::Error), - /// I/O error. - #[error(transparent)] - IoError(#[from] std::io::Error), - /// Error parsing string as UTF8. #[error(transparent)] FromUtf8Error(#[from] std::string::FromUtf8Error), @@ -247,7 +247,8 @@ impl Specification { filename: &Path, ) -> Result<Vec<Specification>, SpecificationError> { debug!("reading specification from {}", filename.display()); - let spec = fs::read(filename)?; + let spec = fs::read(filename) + .map_err(|err| SpecificationError::Read(filename.to_path_buf(), err))?; let input: HashMap<String, OneVmInputSpecification> = serde_yaml::from_slice(&spec)?; debug!("specification as read from file: {:#?}", input); diff --git a/src/sshkeys.rs b/src/sshkeys.rs index 1425cb3..349cabe 100644 --- a/src/sshkeys.rs +++ b/src/sshkeys.rs @@ -3,7 +3,7 @@ use std::fs::{read, File, Permissions}; use std::io::Write; use std::os::unix::fs::PermissionsExt; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; use tempfile::tempdir; @@ -18,9 +18,29 @@ pub enum KeyError { #[error("ssh-keygen failed to certify a key: {0}")] CertError(String), - /// I/O error. - #[error(transparent)] - IoError(#[from] std::io::Error), + /// Error creating a temporary directory. + #[error("Couldn't create temporary directory")] + TempDir(#[source] std::io::Error), + + /// Error running ssh-keygen. + #[error("Couldn't run ssh-keygen")] + Run(#[source] std::io::Error), + + /// Error reading a file. + #[error("Couldn't read file {0}")] + Read(PathBuf, #[source] std::io::Error), + + /// Error writing a file. + #[error("Couldn't write file {0}")] + Write(PathBuf, #[source] std::io::Error), + + /// Error creating a file. + #[error("Couldn't create file {0}")] + Create(PathBuf, #[source] std::io::Error), + + /// Error setting file permissions. + #[error("Couldn't set permissions for file {0}")] + SetPerm(PathBuf, #[source] std::io::Error), /// Error parsing a string as UTF8. #[error(transparent)] @@ -90,7 +110,7 @@ impl KeyPair { /// Generate a new key pair of the desired kind. pub fn generate(kind: KeyKind) -> Result<Self, KeyError> { - let dirname = tempdir()?; + let dirname = tempdir().map_err(|err| KeyError::TempDir(err))?; let private_key = dirname.path().join("key"); let output = Command::new("ssh-keygen") .arg("-f") @@ -101,7 +121,8 @@ impl KeyPair { .arg(format!("{}", kind.bits())) .arg("-N") .arg("") - .output()?; + .output() + .map_err(|err| KeyError::Run(err))?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr).into_owned(); @@ -128,7 +149,7 @@ impl KeyPair { } fn read_string(filename: &Path) -> Result<String, KeyError> { - let bytes = read(filename)?; + let bytes = read(filename).map_err(|err| KeyError::Read(filename.to_path_buf(), err))?; Ok(String::from_utf8(bytes)?) } @@ -157,7 +178,7 @@ impl CaKey { /// /// Return as a string. pub fn certify_host(&self, host_key: &KeyPair, hostname: &str) -> Result<String, KeyError> { - let dirname = tempdir()?; + let dirname = tempdir().map_err(|err| KeyError::TempDir(err))?; let ca_key = dirname.path().join("ca"); let host_key_pub = dirname.path().join("host.pub"); let cert = dirname.path().join("host-cert.pub"); @@ -174,7 +195,8 @@ impl CaKey { .arg("-I") .arg(format!("host key for {}", hostname)) .arg(&host_key_pub) - .output()?; + .output() + .map_err(|err| KeyError::Run(err))?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr).into_owned(); @@ -186,10 +208,13 @@ impl CaKey { } fn write_string(filename: &Path, s: &str) -> Result<(), KeyError> { - let mut file = File::create(filename)?; + let mut file = + File::create(filename).map_err(|err| KeyError::Create(filename.to_path_buf(), err))?; let ro_user = Permissions::from_mode(0o600); - file.set_permissions(ro_user)?; - file.write_all(s.as_bytes())?; + file.set_permissions(ro_user) + .map_err(|err| KeyError::SetPerm(filename.to_path_buf(), err))?; + file.write_all(s.as_bytes()) + .map_err(|err| KeyError::Write(filename.to_path_buf(), err))?; Ok(()) } |