From 359286a87d6cc170eb09e87d57f8bc3325085d47 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 28 Aug 2020 09:30:03 -0500 Subject: [PATCH 1/9] agent/rustjail: Add anyhow dependency anyhow provides `anyhow::Error`, a trait object based error type for easy idiomatic error handling in Rust applications. Signed-off-by: Julio Montes --- src/agent/rustjail/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/agent/rustjail/Cargo.toml b/src/agent/rustjail/Cargo.toml index db07963c0d..9bd7023203 100644 --- a/src/agent/rustjail/Cargo.toml +++ b/src/agent/rustjail/Cargo.toml @@ -24,3 +24,4 @@ scan_fmt = "0.2" regex = "1.1" path-absolutize = "1.2.0" dirs = "3.0.1" +anyhow = "1.0.32" From 6a4c9b14f23c687737b6637bb9303799011e32ca Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 28 Aug 2020 09:32:20 -0500 Subject: [PATCH 2/9] agent/rustjail/cgroups: Use anyhow for error handling Return `anyhow::Result` from all the functions in this directory. Add function `io_error_kind_eq` to compare an `anyhow::Error` with an `io::Error`, this function downcast the `anyhow::Error`. Signed-off-by: Julio Montes --- src/agent/rustjail/src/cgroups/fs/mod.rs | 77 ++++++++++-------------- src/agent/rustjail/src/cgroups/mod.rs | 18 +++--- 2 files changed, 41 insertions(+), 54 deletions(-) diff --git a/src/agent/rustjail/src/cgroups/fs/mod.rs b/src/agent/rustjail/src/cgroups/fs/mod.rs index e279ea2003..70657398a8 100644 --- a/src/agent/rustjail/src/cgroups/fs/mod.rs +++ b/src/agent/rustjail/src/cgroups/fs/mod.rs @@ -5,7 +5,7 @@ use crate::cgroups::FreezerState; use crate::cgroups::Manager as CgroupManager; use crate::container::DEFAULT_DEVICES; -use crate::errors::*; +use anyhow::{anyhow, Context, Error, Result}; use lazy_static; use libc::{self, pid_t}; use nix::errno::Errno; @@ -167,6 +167,14 @@ lazy_static! { }; } +pub fn io_error_kind_eq(e: &Error, k: std::io::ErrorKind) -> bool { + if let Some(ref re) = e.downcast_ref::() { + return re.kind() == k; + } + + return false; +} + pub const KB: u128 = 1000; pub const MB: u128 = 1000 * KB; pub const GB: u128 = 1000 * MB; @@ -313,7 +321,8 @@ where { let p = format!("{}/{}", dir, file); fs::write(p.as_str(), v.to_string().as_bytes()) - .chain_err(|| format!("couldn't write to file: {:?}", p))?; + .context(format!("couldn't write to file: {:?}", p))?; + Ok(()) } @@ -327,7 +336,7 @@ fn copy_parent(dir: &str, file: &str) -> Result<()> { let parent = if let Some(index) = dir.rfind('/') { &dir[..index] } else { - return Err(ErrorKind::ErrorCode("cannot copy file from parent".to_string()).into()); + return Err(anyhow!("cannot copy file from parent")); }; match read_file(parent, file) { @@ -340,12 +349,9 @@ fn copy_parent(dir: &str, file: &str) -> Result<()> { return copy_parent(dir, file); } } - Err(Error(ErrorKind::Io(e), _)) => { - if e.kind() == std::io::ErrorKind::NotFound { - copy_parent(parent, file)?; - return copy_parent(dir, file); - } - return Err(ErrorKind::Io(e).into()); + Err(ref e) if io_error_kind_eq(e, std::io::ErrorKind::NotFound) => { + copy_parent(parent, file)?; + return copy_parent(dir, file); } Err(e) => return Err(e.into()), } @@ -362,14 +368,8 @@ fn write_nonzero(dir: &str, file: &str, v: i128) -> Result<()> { fn try_write_nonzero(dir: &str, file: &str, v: i128) -> Result<()> { match write_nonzero(dir, file, v) { Ok(_) => Ok(()), - Err(Error(ErrorKind::Io(e), _)) => { - if e.kind() == std::io::ErrorKind::PermissionDenied { - return Ok(()); - } else { - return Err(ErrorKind::Io(e).into()); - } - } - e => e, + Err(ref e) if io_error_kind_eq(e, std::io::ErrorKind::PermissionDenied) => Ok(()), + Err(e) => Err(anyhow!(e)), } } @@ -394,7 +394,7 @@ fn try_write_file(dir: &str, file: &str, v: T) -> Result<()> { info!(sl!(), "{}", err.desc()); - return Err(e); + return Err(anyhow!(e)); } Ok(_) => {} @@ -668,17 +668,14 @@ where T: ToString, { match write_file(dir, file, v) { - Err(Error(ErrorKind::Io(e), _)) => { - if e.kind() != std::io::ErrorKind::PermissionDenied - && e.kind() != std::io::ErrorKind::Other - { - return Err(ErrorKind::Io(e).into()); - } - + Err(ref e) + if io_error_kind_eq(e, std::io::ErrorKind::PermissionDenied) + || io_error_kind_eq(e, std::io::ErrorKind::Other) => + { return Ok(()); } - Err(e) => return Err(e.into()), + Err(e) => return Err(anyhow!(e)), Ok(_) => return Ok(()), } @@ -727,17 +724,11 @@ impl Subsystem for Memory { fn get_exist_memory_data(dir: &str, sub: &str) -> Result> { let res = match get_memory_data(dir, sub) { - Err(Error(ErrorKind::Io(e), _)) => { - if e.kind() == std::io::ErrorKind::NotFound { - None - } else { - return Err(ErrorKind::Io(e).into()); - } - } + Err(ref e) if io_error_kind_eq(e, std::io::ErrorKind::NotFound) => None, Ok(r) => Some(r), - Err(e) => return Err(e.into()), + Err(e) => return Err(anyhow!(e)), }; Ok(res) @@ -872,19 +863,15 @@ fn rate(d: &LinuxThrottleDevice) -> String { fn write_blkio_device(dir: &str, file: &str, v: T) -> Result<()> { match write_file(dir, file, v) { - Err(Error(ErrorKind::Io(e), _)) => { + Err(ref e) if io_error_kind_eq(e, std::io::ErrorKind::Other) => { // only ignore ENODEV - if e.kind() == std::io::ErrorKind::Other { - let raw = std::io::Error::last_os_error().raw_os_error().unwrap(); - if Errno::from_i32(raw) == Errno::ENODEV { - return Ok(()); - } + let raw = std::io::Error::last_os_error().raw_os_error().unwrap(); + if Errno::from_i32(raw) == Errno::ENODEV { + return Ok(()); } - - return Err(ErrorKind::Io(e).into()); } - Err(e) => return Err(e.into()), + Err(e) => return Err(anyhow!(e)), Ok(_) => {} } @@ -1381,7 +1368,7 @@ impl CgroupManager for Manager { let m = if self.paths.get("devices").is_some() { get_procs(self.paths.get("devices").unwrap())? } else { - return Err(ErrorKind::ErrorCode("no devices cgroup".to_string()).into()); + return Err(anyhow!("no devices cgroup".to_string())); }; Ok(m) @@ -1391,7 +1378,7 @@ impl CgroupManager for Manager { let m = if self.paths.get("devices").is_some() { get_all_procs(self.paths.get("devices").unwrap())? } else { - return Err(ErrorKind::ErrorCode("no devices cgroup".to_string()).into()); + return Err(anyhow!("no devices cgroup")); }; Ok(m) diff --git a/src/agent/rustjail/src/cgroups/mod.rs b/src/agent/rustjail/src/cgroups/mod.rs index 0dbec74eb0..8165650752 100644 --- a/src/agent/rustjail/src/cgroups/mod.rs +++ b/src/agent/rustjail/src/cgroups/mod.rs @@ -3,8 +3,8 @@ // SPDX-License-Identifier: Apache-2.0 // -use crate::errors::*; // use crate::configs::{FreezerState, Config}; +use anyhow::{anyhow, Result}; use oci::LinuxResources; use protocols::agent::CgroupStats; use std::collections::HashMap; @@ -16,34 +16,34 @@ pub type FreezerState = &'static str; pub trait Manager { fn apply(&self, _pid: i32) -> Result<()> { - Err(ErrorKind::ErrorCode("not supported!".to_string()).into()) + Err(anyhow!("not supported!".to_string())) } fn get_pids(&self) -> Result> { - Err(ErrorKind::ErrorCode("not supported!".to_string()).into()) + Err(anyhow!("not supported!")) } fn get_all_pids(&self) -> Result> { - Err(ErrorKind::ErrorCode("not supported!".to_string()).into()) + Err(anyhow!("not supported!")) } fn get_stats(&self) -> Result { - Err(ErrorKind::ErrorCode("not supported!".to_string()).into()) + Err(anyhow!("not supported!")) } fn freeze(&self, _state: FreezerState) -> Result<()> { - Err(ErrorKind::ErrorCode("not supported!".to_string()).into()) + Err(anyhow!("not supported!")) } fn destroy(&mut self) -> Result<()> { - Err(ErrorKind::ErrorCode("not supported!".to_string()).into()) + Err(anyhow!("not supported!")) } fn get_paths(&self) -> Result> { - Err(ErrorKind::ErrorCode("not supported!".to_string()).into()) + Err(anyhow!("not supported!")) } fn set(&self, _container: &LinuxResources, _update: bool) -> Result<()> { - Err(ErrorKind::ErrorCode("not supported!".to_string()).into()) + Err(anyhow!("not supported!")) } } From 2e3e2ce114ae48109be137dfe541cdc22f9d8760 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 28 Aug 2020 10:36:26 -0500 Subject: [PATCH 3/9] agent/rustjail/capabilities: Use anyhow for error handling Use `.to_string` to wrap up `caps::errors::Error`s since they are not thread safe, otherwise `cargo build` will fail with the following error: ``` doesn't satisfy `caps::errors::Error: std::marker::Sync` ``` Signed-off-by: Julio Montes --- src/agent/rustjail/src/capabilities.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/agent/rustjail/src/capabilities.rs b/src/agent/rustjail/src/capabilities.rs index 6a2f1201ab..c250993f09 100644 --- a/src/agent/rustjail/src/capabilities.rs +++ b/src/agent/rustjail/src/capabilities.rs @@ -8,9 +8,9 @@ use lazy_static; -use crate::errors::*; use crate::log_child; use crate::sync::write_count; +use anyhow::{anyhow, Result}; use caps::{self, CapSet, Capability, CapsHashSet}; use oci::LinuxCapabilities; use std::collections::HashMap; @@ -96,7 +96,7 @@ fn to_capshashset(cfd_log: RawFd, caps: &[String]) -> CapsHashSet { } pub fn reset_effective() -> Result<()> { - caps::set(None, CapSet::Effective, caps::all())?; + caps::set(None, CapSet::Effective, caps::all()).map_err(|e| anyhow!(e.to_string()))?; Ok(()) } @@ -104,24 +104,27 @@ pub fn drop_priviledges(cfd_log: RawFd, caps: &LinuxCapabilities) -> Result<()> let all = caps::all(); for c in all.difference(&to_capshashset(cfd_log, caps.bounding.as_ref())) { - caps::drop(None, CapSet::Bounding, *c)?; + caps::drop(None, CapSet::Bounding, *c).map_err(|e| anyhow!(e.to_string()))?; } caps::set( None, CapSet::Effective, to_capshashset(cfd_log, caps.effective.as_ref()), - )?; + ) + .map_err(|e| anyhow!(e.to_string()))?; caps::set( None, CapSet::Permitted, to_capshashset(cfd_log, caps.permitted.as_ref()), - )?; + ) + .map_err(|e| anyhow!(e.to_string()))?; caps::set( None, CapSet::Inheritable, to_capshashset(cfd_log, caps.inheritable.as_ref()), - )?; + ) + .map_err(|e| anyhow!(e.to_string()))?; if let Err(_) = caps::set( None, From c192446a595daf144dd6cd428851b9b0e7657fd7 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 28 Aug 2020 11:16:34 -0500 Subject: [PATCH 4/9] agent/rustjail: Use anyhow for error handling Convert all Errors and Results to `anyhow::Error` and `anyhow::Result` respectively Signed-off-by: Julio Montes --- src/agent/rustjail/src/configs/mod.rs | 1 - src/agent/rustjail/src/container.rs | 80 ++++++++++++--------------- src/agent/rustjail/src/mount.rs | 33 +++++------ src/agent/rustjail/src/sync.rs | 43 ++++++-------- src/agent/rustjail/src/validator.rs | 47 ++++++++-------- 5 files changed, 88 insertions(+), 116 deletions(-) diff --git a/src/agent/rustjail/src/configs/mod.rs b/src/agent/rustjail/src/configs/mod.rs index 2ee991a917..dea1682d26 100644 --- a/src/agent/rustjail/src/configs/mod.rs +++ b/src/agent/rustjail/src/configs/mod.rs @@ -10,7 +10,6 @@ use serde_json; use protocols::oci::State as OCIState; -use crate::errors::*; use std::collections::HashMap; use std::fmt; use std::path::PathBuf; diff --git a/src/agent/rustjail/src/container.rs b/src/agent/rustjail/src/container.rs index ceb8d2c0aa..9b25199e71 100644 --- a/src/agent/rustjail/src/container.rs +++ b/src/agent/rustjail/src/container.rs @@ -14,6 +14,7 @@ use std::os::unix::io::RawFd; use std::path::{Path, PathBuf}; use std::time::SystemTime; // use crate::sync::Cond; +use anyhow::{anyhow, Context, Result}; use libc::pid_t; use oci::{LinuxDevice, LinuxIDMapping}; use std::clone::Clone; @@ -24,7 +25,6 @@ use std::process::{Child, Command}; use crate::cgroups::Manager as CgroupManager; use crate::process::Process; // use crate::intelrdt::Manager as RdtManager; -use crate::errors::*; use crate::log_child; use crate::specconv::CreateOpts; use crate::sync::*; @@ -44,7 +44,6 @@ use nix::sched::{self, CloneFlags}; use nix::sys::signal::{self, Signal}; use nix::sys::stat::{self, Mode}; use nix::unistd::{self, ForkResult, Gid, Pid, Uid}; -use nix::Error; use libc; use protobuf::SingularPtrField; @@ -294,11 +293,10 @@ impl Container for LinuxContainer { fn pause(&mut self) -> Result<()> { let status = self.status(); if status != Status::RUNNING && status != Status::CREATED { - return Err(ErrorKind::ErrorCode(format!( + return Err(anyhow!( "failed to pause container: current status is: {:?}", status - )) - .into()); + )); } if self.cgroup_manager.is_some() { @@ -310,17 +308,13 @@ impl Container for LinuxContainer { self.status.transition(Status::PAUSED); return Ok(()); } - Err(ErrorKind::ErrorCode(String::from("failed to get container's cgroup manager")).into()) + Err(anyhow!("failed to get container's cgroup manager")) } fn resume(&mut self) -> Result<()> { let status = self.status(); if status != Status::PAUSED { - return Err(ErrorKind::ErrorCode(format!( - "container status is: {:?}, not paused", - status - )) - .into()); + return Err(anyhow!("container status is: {:?}, not paused", status)); } if self.cgroup_manager.is_some() { @@ -332,7 +326,7 @@ impl Container for LinuxContainer { self.status.transition(Status::RUNNING); return Ok(()); } - Err(ErrorKind::ErrorCode(String::from("failed to get container's cgroup manager")).into()) + Err(anyhow!("failed to get container's cgroup manager")) } } @@ -383,11 +377,11 @@ fn do_init_child(cwfd: RawFd) -> Result<()> { let p = if spec.process.is_some() { spec.process.as_ref().unwrap() } else { - return Err(ErrorKind::ErrorCode("didn't find process in Spec".to_string()).into()); + return Err(anyhow!("didn't find process in Spec")); }; if spec.linux.is_none() { - return Err(ErrorKind::ErrorCode("no linux config".to_string()).into()); + return Err(anyhow!("no linux config")); } let linux = spec.linux.as_ref().unwrap(); @@ -401,7 +395,7 @@ fn do_init_child(cwfd: RawFd) -> Result<()> { for ns in &nses { let s = NAMESPACES.get(&ns.r#type.as_str()); if s.is_none() { - return Err(ErrorKind::ErrorCode("invalid ns type".to_string()).into()); + return Err(anyhow!("invalid ns type")); } let s = s.unwrap(); @@ -569,7 +563,7 @@ fn do_init_child(cwfd: RawFd) -> Result<()> { // NoNewPeiviledges, Drop capabilities if oci_process.no_new_privileges { if let Err(_) = prctl::set_no_new_privileges(true) { - return Err(ErrorKind::ErrorCode("cannot set no new privileges".to_string()).into()); + return Err(anyhow!("cannot set no new privileges")); } } @@ -621,9 +615,7 @@ fn do_init_child(cwfd: RawFd) -> Result<()> { match find_file(exec_file) { Some(_) => (), None => { - return Err( - ErrorKind::ErrorCode(format!("the file {} is not exist", &args[0])).into(), - ); + return Err(anyhow!("the file {} is not exist", &args[0])); } } } @@ -655,7 +647,7 @@ fn do_init_child(cwfd: RawFd) -> Result<()> { do_exec(&args); - Err(ErrorKind::ErrorCode("fail to create container".to_string()).into()) + Err(anyhow!("fail to create container")) } impl BaseContainer for LinuxContainer { @@ -668,7 +660,7 @@ impl BaseContainer for LinuxContainer { } fn state(&self) -> Result { - Err(ErrorKind::ErrorCode(String::from("not suppoerted")).into()) + Err(anyhow!("not suppoerted")) } fn oci_state(&self) -> Result { @@ -708,7 +700,7 @@ impl BaseContainer for LinuxContainer { } } - Err(ErrorKind::ErrorCode(format!("invalid eid {}", eid)).into()) + Err(anyhow!("invalid eid {}", eid)) } fn stats(&self) -> Result { @@ -747,7 +739,7 @@ impl BaseContainer for LinuxContainer { let mut fifofd: RawFd = -1; if p.init { if let Ok(_) = stat::stat(fifo_file.as_str()) { - return Err(ErrorKind::ErrorCode("exec fifo exists".to_string()).into()); + return Err(anyhow!("exec fifo exists")); } unistd::mkfifo(fifo_file.as_str(), Mode::from_bits(0o622).unwrap())?; // defer!(fs::remove_file(&fifo_file)?); @@ -763,18 +755,18 @@ impl BaseContainer for LinuxContainer { fscgroup::init_static(); if self.config.spec.is_none() { - return Err(ErrorKind::ErrorCode("no spec".to_string()).into()); + return Err(anyhow!("no spec")); } let spec = self.config.spec.as_ref().unwrap(); if spec.linux.is_none() { - return Err(ErrorKind::ErrorCode("no linux config".to_string()).into()); + return Err(anyhow!("no linux config")); } let linux = spec.linux.as_ref().unwrap(); let st = self.oci_state()?; - let (pfd_log, cfd_log) = unistd::pipe().chain_err(|| "failed to create pipe")?; + let (pfd_log, cfd_log) = unistd::pipe().context("failed to create pipe")?; fcntl::fcntl(pfd_log, FcntlArg::F_SETFD(FdFlag::FD_CLOEXEC)); let child_logger = logger.new(o!("action" => "child process log")); @@ -802,8 +794,8 @@ impl BaseContainer for LinuxContainer { }); info!(logger, "exec fifo opened!"); - let (prfd, cwfd) = unistd::pipe().chain_err(|| "failed to create pipe")?; - let (crfd, pwfd) = unistd::pipe().chain_err(|| "failed to create pipe")?; + let (prfd, cwfd) = unistd::pipe().context("failed to create pipe")?; + let (crfd, pwfd) = unistd::pipe().context("failed to create pipe")?; fcntl::fcntl(prfd, FcntlArg::F_SETFD(FdFlag::FD_CLOEXEC)); fcntl::fcntl(pwfd, FcntlArg::F_SETFD(FdFlag::FD_CLOEXEC)); @@ -858,7 +850,7 @@ impl BaseContainer for LinuxContainer { if pidns.is_some() { sched::setns(pidns.unwrap(), CloneFlags::CLONE_NEWPID) - .chain_err(|| "failed to join pidns")?; + .context("failed to join pidns")?; unistd::close(pidns.unwrap())?; } else { sched::unshare(CloneFlags::CLONE_NEWPID)?; @@ -923,7 +915,7 @@ impl BaseContainer for LinuxContainer { // create the pipes for notify process exited let (exit_pipe_r, exit_pipe_w) = unistd::pipe2(OFlag::O_CLOEXEC) - .chain_err(|| "failed to create pipe") + .context("failed to create pipe") .map_err(|e| { signal::kill(Pid::from_raw(child.id() as i32), Some(Signal::SIGKILL)); e @@ -1056,11 +1048,7 @@ fn do_exec(args: &[String]) -> Result<()> { fn update_namespaces(logger: &Logger, spec: &mut Spec, init_pid: RawFd) -> Result<()> { let linux = match spec.linux.as_mut() { - None => { - return Err( - ErrorKind::ErrorCode("Spec didn't container linux field".to_string()).into(), - ) - } + None => return Err(anyhow!("Spec didn't container linux field")), Some(l) => l, }; @@ -1107,7 +1095,7 @@ fn get_pid_namespace(logger: &Logger, linux: &Linux) -> Result> { } } - Err(ErrorKind::ErrorCode("cannot find the pid ns".to_string()).into()) + Err(anyhow!("cannot find the pid ns")) } fn is_userns_enabled(linux: &Linux) -> bool { @@ -1271,7 +1259,7 @@ fn write_mappings(logger: &Logger, path: &str, maps: &[LinuxIDMapping]) -> Resul fn setid(uid: Uid, gid: Gid) -> Result<()> { // set uid/gid if let Err(e) = prctl::set_keep_capabilities(true) { - bail!(format!("set keep capabilities returned {}", e)); + bail!(anyhow!(e).context("set keep capabilities returned")); }; { unistd::setresgid(gid, gid, gid)?; @@ -1285,7 +1273,7 @@ fn setid(uid: Uid, gid: Gid) -> Result<()> { } if let Err(e) = prctl::set_keep_capabilities(false) { - bail!(format!("set keep capabilities returned {}", e)); + bail!(anyhow!(e).context("set keep capabilities returned")); }; Ok(()) } @@ -1306,10 +1294,10 @@ impl LinuxContainer { if let Err(e) = fs::create_dir_all(root.as_str()) { if e.kind() == std::io::ErrorKind::AlreadyExists { - return Err(e).chain_err(|| format!("container {} already exists", id.as_str())); + return Err(e).context(format!("container {} already exists", id.as_str())); } - return Err(e).chain_err(|| format!("fail to create container directory {}", root)); + return Err(e).context(format!("fail to create container directory {}", root)); } unistd::chown( @@ -1317,7 +1305,7 @@ impl LinuxContainer { Some(unistd::getuid()), Some(unistd::getgid()), ) - .chain_err(|| format!("cannot change onwer of container {} root", id))?; + .context(format!("cannot change onwer of container {} root", id))?; if config.spec.is_none() { return Err(nix::Error::Sys(Errno::EINVAL).into()); @@ -1359,7 +1347,7 @@ impl LinuxContainer { } fn load>(_id: T, _base: T) -> Result { - Err(ErrorKind::ErrorCode("not supported".to_string()).into()) + Err(anyhow!("not supported")) } /* fn new_parent_process(&self, p: &Process) -> Result> { @@ -1500,7 +1488,7 @@ fn execute_hook(logger: &Logger, h: &Hook, st: &OCIState) -> Result<()> { let binary = PathBuf::from(h.path.as_str()); let path = binary.canonicalize()?; if !path.exists() { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } let args = h.args.clone(); @@ -1531,11 +1519,11 @@ fn execute_hook(logger: &Logger, h: &Hook, st: &OCIState) -> Result<()> { if status != 0 { if status == -libc::ETIMEDOUT { - return Err(ErrorKind::Nix(Error::from_errno(Errno::ETIMEDOUT)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::ETIMEDOUT))); } else if status == -libc::EPIPE { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EPIPE)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EPIPE))); } else { - return Err(ErrorKind::Nix(Error::from_errno(Errno::UnknownErrno)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::UnknownErrno))); } } diff --git a/src/agent/rustjail/src/mount.rs b/src/agent/rustjail/src/mount.rs index 69f3560b13..ae0e7c5be6 100644 --- a/src/agent/rustjail/src/mount.rs +++ b/src/agent/rustjail/src/mount.rs @@ -3,6 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // +use anyhow::{anyhow, Context, Error, Result}; use libc::uid_t; use nix::errno::Errno; use nix::fcntl::{self, OFlag}; @@ -23,7 +24,6 @@ use std::fs::File; use std::io::{BufRead, BufReader}; use crate::container::DEFAULT_DEVICES; -use crate::errors::*; use crate::sync::write_count; use lazy_static; use std::string::ToString; @@ -139,7 +139,7 @@ pub fn init_rootfs( for m in &spec.mounts { let (mut flags, data) = parse_mount(&m); if !m.destination.starts_with("/") || m.destination.contains("..") { - return Err(ErrorKind::Nix(nix::Error::Sys(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::Sys(Errno::EINVAL))); } if m.r#type == "cgroup" { mount_cgroups(cfd_log, &m, rootfs, flags, &data, cpath, mounts)?; @@ -289,7 +289,7 @@ pub fn pivot_rootfs(path: &P) -> Result<( // Change to the new root so that the pivot_root actually acts on it. unistd::fchdir(newroot)?; - unistd::pivot_root(".", ".").chain_err(|| format!("failed to pivot_root on {:?}", path))?; + unistd::pivot_root(".", ".").context(format!("failed to pivot_root on {:?}", path))?; // Currently our "." is oldroot (according to the current kernel code). // However, purely for safety, we will fchdir(oldroot) since there isn't @@ -311,7 +311,7 @@ pub fn pivot_rootfs(path: &P) -> Result<( )?; // Preform the unmount. MNT_DETACH allows us to unmount /proc/self/cwd. - mount::umount2(".", MntFlags::MNT_DETACH).chain_err(|| "failed to do umount2")?; + mount::umount2(".", MntFlags::MNT_DETACH).context("failed to do umount2")?; // Switch back to our shiny new root. unistd::chdir("/")?; @@ -395,7 +395,7 @@ fn parse_mount_table() -> Result> { infos.push(info); } else { - return Err(ErrorKind::ErrorCode("failed to parse mount info file".to_string()).into()); + return Err(anyhow!("failed to parse mount info file".to_string())); } } @@ -408,20 +408,17 @@ pub fn ms_move_root(rootfs: &str) -> Result { let root_path = Path::new(rootfs); let abs_root_buf = root_path.absolutize()?; - let abs_root = abs_root_buf.to_str().ok_or::( - ErrorKind::ErrorCode(format!("failed to parse {} to absolute path", rootfs)).into(), - )?; + let abs_root = abs_root_buf + .to_str() + .ok_or::(anyhow!("failed to parse {} to absolute path", rootfs))?; for info in mount_infos.iter() { let mount_point = Path::new(&info.mount_point); let abs_mount_buf = mount_point.absolutize()?; - let abs_mount_point = abs_mount_buf.to_str().ok_or::( - ErrorKind::ErrorCode(format!( - "failed to parse {} to absolute path", - info.mount_point - )) - .into(), - )?; + let abs_mount_point = abs_mount_buf.to_str().ok_or::(anyhow!( + "failed to parse {} to absolute path", + info.mount_point + ))?; let abs_mount_point_string = String::from(abs_mount_point); // Umount every syfs and proc file systems, except those under the container rootfs @@ -443,7 +440,7 @@ pub fn ms_move_root(rootfs: &str) -> Result { Ok(_) => (), Err(e) => { if e.ne(&nix::Error::from(Errno::EINVAL)) && e.ne(&nix::Error::from(Errno::EPERM)) { - return Err(ErrorKind::ErrorCode(e.to_string()).into()); + return Err(anyhow!(e)); } // If we have not privileges for umounting (e.g. rootless), then @@ -630,7 +627,7 @@ fn create_devices(devices: &[LinuxDevice], bind: bool) -> Result<()> { for dev in devices { if !dev.path.starts_with("/dev") || dev.path.contains("..") { let msg = format!("{} is not a valid device path", dev.path); - bail!(ErrorKind::ErrorCode(msg)); + bail!(anyhow!(msg)); } op(dev)?; } @@ -661,7 +658,7 @@ lazy_static! { fn mknod_dev(dev: &LinuxDevice) -> Result<()> { let f = match LINUXDEVICETYPE.get(dev.r#type.as_str()) { Some(v) => v, - None => return Err(ErrorKind::ErrorCode("invalid spec".to_string()).into()), + None => return Err(anyhow!("invalid spec".to_string())), }; stat::mknod( diff --git a/src/agent/rustjail/src/sync.rs b/src/agent/rustjail/src/sync.rs index b17277f87b..20bf7b4706 100644 --- a/src/agent/rustjail/src/sync.rs +++ b/src/agent/rustjail/src/sync.rs @@ -3,13 +3,13 @@ // SPDX-License-Identifier: Apache-2.0 // -use crate::errors::*; use nix::errno::Errno; use nix::unistd; -use nix::Error; use std::mem; use std::os::unix::io::RawFd; +use anyhow::{anyhow, Result}; + pub const SYNC_SUCCESS: i32 = 1; pub const SYNC_FAILED: i32 = 2; pub const SYNC_DATA: i32 = 3; @@ -40,7 +40,7 @@ pub fn write_count(fd: RawFd, buf: &[u8], count: usize) -> Result { } Err(e) => { - if e != Error::from_errno(Errno::EINTR) { + if e != nix::Error::from_errno(Errno::EINTR) { return Err(e.into()); } } @@ -64,7 +64,7 @@ fn read_count(fd: RawFd, count: usize) -> Result> { } Err(e) => { - if e != Error::from_errno(Errno::EINTR) { + if e != nix::Error::from_errno(Errno::EINTR) { return Err(e.into()); } } @@ -77,13 +77,12 @@ fn read_count(fd: RawFd, count: usize) -> Result> { pub fn read_sync(fd: RawFd) -> Result> { let buf = read_count(fd, MSG_SIZE)?; if buf.len() != MSG_SIZE { - return Err(ErrorKind::ErrorCode(format!( + return Err(anyhow!( "process: {} failed to receive sync message from peer: got msg length: {}, expected: {}", std::process::id(), buf.len(), MSG_SIZE - )) - .into()); + )); } let buf_array: [u8; MSG_SIZE] = [buf[0], buf[1], buf[2], buf[3]]; let msg: i32 = i32::from_be_bytes(buf_array); @@ -111,19 +110,17 @@ pub fn read_sync(fd: RawFd) -> Result> { } let error_str = match std::str::from_utf8(&error_buf) { - Ok(v) => v, + Ok(v) => String::from(v), Err(e) => { - return Err(ErrorKind::ErrorCode(format!( - "receive error message from child process failed: {:?}", - e - )) - .into()) + return Err( + anyhow!(e).context("receive error message from child process failed") + ); } }; - return Err(ErrorKind::ErrorCode(String::from(error_str)).into()); + return Err(anyhow!(error_str)); } - _ => return Err(ErrorKind::ErrorCode("error in receive sync message".to_string()).into()), + _ => return Err(anyhow!("error in receive sync message")), } } @@ -132,7 +129,7 @@ pub fn write_sync(fd: RawFd, msg_type: i32, data_str: &str) -> Result<()> { let count = write_count(fd, &buf, MSG_SIZE)?; if count != MSG_SIZE { - return Err(ErrorKind::ErrorCode("error in send sync message".to_string()).into()); + return Err(anyhow!("error in send sync message")); } match msg_type { @@ -140,9 +137,7 @@ pub fn write_sync(fd: RawFd, msg_type: i32, data_str: &str) -> Result<()> { Ok(_count) => unistd::close(fd)?, Err(e) => { unistd::close(fd)?; - return Err( - ErrorKind::ErrorCode("error in send message to process".to_string()).into(), - ); + return Err(anyhow!(e).context("error in send message to process")); } }, SYNC_DATA => { @@ -151,10 +146,7 @@ pub fn write_sync(fd: RawFd, msg_type: i32, data_str: &str) -> Result<()> { Ok(_count) => (), Err(e) => { unistd::close(fd)?; - return Err(ErrorKind::ErrorCode( - "error in send message to process".to_string(), - ) - .into()); + return Err(anyhow!(e).context("error in send message to process")); } } @@ -162,10 +154,7 @@ pub fn write_sync(fd: RawFd, msg_type: i32, data_str: &str) -> Result<()> { Ok(_count) => (), Err(e) => { unistd::close(fd)?; - return Err(ErrorKind::ErrorCode( - "error in send message to process".to_string(), - ) - .into()); + return Err(anyhow!(e).context("error in send message to process")); } } } diff --git a/src/agent/rustjail/src/validator.rs b/src/agent/rustjail/src/validator.rs index 3e06398d89..14ffef1bd8 100644 --- a/src/agent/rustjail/src/validator.rs +++ b/src/agent/rustjail/src/validator.rs @@ -4,10 +4,9 @@ // use crate::container::Config; -use crate::errors::*; +use anyhow::{anyhow, Result}; use lazy_static; use nix::errno::Errno; -use nix::Error; use oci::{LinuxIDMapping, LinuxNamespace, Spec}; use protobuf::RepeatedField; use std::collections::HashMap; @@ -30,14 +29,14 @@ fn get_namespace_path(nses: &Vec, key: &str) -> Result { } } - Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()) + Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))) } fn rootfs(root: &str) -> Result<()> { let path = PathBuf::from(root); // not absolute path or not exists if !path.exists() || !path.is_absolute() { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } // symbolic link? ..? @@ -65,7 +64,7 @@ fn rootfs(root: &str) -> Result<()> { let canon = path.canonicalize()?; if cleaned != canon { // There is symbolic in path - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } Ok(()) @@ -81,11 +80,11 @@ fn hostname(oci: &Spec) -> Result<()> { } if oci.linux.is_none() { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } let linux = oci.linux.as_ref().unwrap(); if !contain_namespace(&linux.namespaces, "uts") { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } Ok(()) @@ -98,7 +97,7 @@ fn security(oci: &Spec) -> Result<()> { } if !contain_namespace(&linux.namespaces, "mount") { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } // don't care about selinux at present @@ -113,7 +112,7 @@ fn idmapping(maps: &Vec) -> Result<()> { } } - Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()) + Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))) } fn usernamespace(oci: &Spec) -> Result<()> { @@ -121,7 +120,7 @@ fn usernamespace(oci: &Spec) -> Result<()> { if contain_namespace(&linux.namespaces, "user") { let user_ns = PathBuf::from("/proc/self/ns/user"); if !user_ns.exists() { - return Err(ErrorKind::ErrorCode("user namespace not supported!".to_string()).into()); + return Err(anyhow!("user namespace not supported!")); } // check if idmappings is correct, at least I saw idmaps // with zero size was passed to agent @@ -130,7 +129,7 @@ fn usernamespace(oci: &Spec) -> Result<()> { } else { // no user namespace but idmap if linux.uid_mappings.len() != 0 || linux.gid_mappings.len() != 0 { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } } @@ -142,7 +141,7 @@ fn cgroupnamespace(oci: &Spec) -> Result<()> { if contain_namespace(&linux.namespaces, "cgroup") { let path = PathBuf::from("/proc/self/ns/cgroup"); if !path.exists() { - return Err(ErrorKind::ErrorCode("cgroup unsupported!".to_string()).into()); + return Err(anyhow!("cgroup unsupported!")); } } Ok(()) @@ -176,7 +175,7 @@ fn check_host_ns(path: &str) -> Result<()> { } let real_cpath = cpath.read_link()?; if real_cpath == real_hpath { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } Ok(()) @@ -189,13 +188,13 @@ fn sysctl(oci: &Spec) -> Result<()> { if contain_namespace(&linux.namespaces, "ipc") { continue; } else { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } } if key.starts_with("net.") { if !contain_namespace(&linux.namespaces, "network") { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } let net = get_namespace_path(&linux.namespaces, "network")?; @@ -212,11 +211,11 @@ fn sysctl(oci: &Spec) -> Result<()> { } if key == "kernel.hostname" { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } } - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } Ok(()) } @@ -224,11 +223,11 @@ fn sysctl(oci: &Spec) -> Result<()> { fn rootless_euid_mapping(oci: &Spec) -> Result<()> { let linux = oci.linux.as_ref().unwrap(); if !contain_namespace(&linux.namespaces, "user") { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } if linux.gid_mappings.len() == 0 || linux.gid_mappings.len() == 0 { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } Ok(()) @@ -252,20 +251,20 @@ fn rootless_euid_mount(oci: &Spec) -> Result<()> { let fields: Vec<&str> = opt.split('=').collect(); if fields.len() != 2 { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } let id = fields[1].trim().parse::()?; if opt.starts_with("uid=") { if !has_idmapping(&linux.uid_mappings, id) { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } } if opt.starts_with("gid=") { if !has_idmapping(&linux.gid_mappings, id) { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } } } @@ -285,11 +284,11 @@ pub fn validate(conf: &Config) -> Result<()> { let oci = conf.spec.as_ref().unwrap(); if oci.linux.is_none() { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } if oci.root.is_none() { - return Err(ErrorKind::Nix(Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } let root = oci.root.as_ref().unwrap().path.as_str(); From 33759af548c55d847d69498b49ac13087a5c5c0b Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 28 Aug 2020 13:51:38 -0500 Subject: [PATCH 5/9] agent: Add anyhow dependency anyhow provides `anyhow::Error`, a trait object based error type for easy idiomatic error handling in Rust applications Signed-off-by: Julio Montes --- src/agent/Cargo.lock | 8 ++++++++ src/agent/Cargo.toml | 1 + 2 files changed, 9 insertions(+) diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index a81f4f3ab5..044f39437e 100644 --- a/src/agent/Cargo.lock +++ b/src/agent/Cargo.lock @@ -24,6 +24,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "anyhow" +version = "1.0.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6b602bfe940d21c130f3895acd65221e8a61270debe89d628b9cb4e3ccb8569b" + [[package]] name = "arc-swap" version = "0.4.6" @@ -262,6 +268,7 @@ checksum = "b8b7a7c0c47db5545ed3fef7468ee7bb5b74691498139e4b3f6a20685dc6dd8e" name = "kata-agent" version = "0.1.0" dependencies = [ + "anyhow", "error-chain", "lazy_static", "libc", @@ -652,6 +659,7 @@ checksum = "4c691c0e608126e00913e33f0ccf3727d5fc84573623b8d65b2df340b5201783" name = "rustjail" version = "0.1.0" dependencies = [ + "anyhow", "caps", "dirs", "error-chain", diff --git a/src/agent/Cargo.toml b/src/agent/Cargo.toml index 862564eeb5..e3529161ad 100644 --- a/src/agent/Cargo.toml +++ b/src/agent/Cargo.toml @@ -32,6 +32,7 @@ slog-scope = "4.1.2" tempfile = "3.1.0" prometheus = { version = "0.9.0", features = ["process"] } procfs = "0.7.9" +anyhow = "1.0.32" [workspace] members = [ From fbb79739c9767c76c1a23ad1f0973aa197805054 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 28 Aug 2020 13:47:48 -0500 Subject: [PATCH 6/9] agent: Use anyhow for error handling Don't use `rustjail::errors` for error handling, since it's not thread safe and there are better alternatives like `anyhow`. `anyhow` attaches context to help the person troubleshooting the error understand where things went wrong, for example: Current error messages: ``` No such file or directory (os error 2) ``` With `anyhow`: ``` Error: Failed to read config.json Caused by: No such file or directory (os error 2) ``` fixes #641 Signed-off-by: Julio Montes --- src/agent/src/config.rs | 35 +++++++------------ src/agent/src/device.rs | 63 +++++++++++++--------------------- src/agent/src/main.rs | 45 +++++++++--------------- src/agent/src/metrics.rs | 2 +- src/agent/src/mount.rs | 51 ++++++++++++--------------- src/agent/src/random.rs | 2 +- src/agent/src/rpc.rs | 74 ++++++++++++++++------------------------ src/agent/src/sandbox.rs | 31 +++++------------ 8 files changed, 116 insertions(+), 187 deletions(-) diff --git a/src/agent/src/config.rs b/src/agent/src/config.rs index 2a16caded8..10c151eeaa 100644 --- a/src/agent/src/config.rs +++ b/src/agent/src/config.rs @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: Apache-2.0 // -use rustjail::errors::*; +use anyhow::{anyhow, Result}; use std::env; use std::fs; use std::time; @@ -108,7 +108,7 @@ impl agentConfig { fn get_vsock_port(p: &str) -> Result { let fields: Vec<&str> = p.split("=").collect(); if fields.len() != 2 { - return Err(ErrorKind::ErrorCode("invalid port parameter".to_string()).into()); + return Err(anyhow!("invalid port parameter")); } Ok(fields[1].parse::()?) @@ -134,7 +134,7 @@ fn logrus_to_slog_level(logrus_level: &str) -> Result { "trace" => slog::Level::Trace, _ => { - return Err(ErrorKind::ErrorCode(String::from("invalid log level")).into()); + return Err(anyhow!("invalid log level")); } }; @@ -145,11 +145,11 @@ fn get_log_level(param: &str) -> Result { let fields: Vec<&str> = param.split("=").collect(); if fields.len() != 2 { - return Err(ErrorKind::ErrorCode(String::from("invalid log level parameter")).into()); + return Err(anyhow!("invalid log level parameter")); } if fields[0] != LOG_LEVEL_OPTION { - Err(ErrorKind::ErrorCode(String::from("invalid log level key name")).into()) + Err(anyhow!("invalid log level key name")) } else { Ok(logrus_to_slog_level(fields[1])?) } @@ -159,17 +159,17 @@ fn get_hotplug_timeout(param: &str) -> Result { let fields: Vec<&str> = param.split("=").collect(); if fields.len() != 2 { - return Err(ErrorKind::ErrorCode(String::from("invalid hotplug timeout parameter")).into()); + return Err(anyhow!("invalid hotplug timeout parameter")); } let key = fields[0]; if key != HOTPLUG_TIMOUT_OPTION { - return Err(ErrorKind::ErrorCode(String::from("invalid hotplug timeout key name")).into()); + return Err(anyhow!("invalid hotplug timeout key name")); } let value = fields[1].parse::(); if value.is_err() { - return Err(ErrorKind::ErrorCode(String::from("unable to parse hotplug timeout")).into()); + return Err(anyhow!("unable to parse hotplug timeout")); } Ok(time::Duration::from_secs(value.unwrap())) @@ -179,31 +179,22 @@ fn get_container_pipe_size(param: &str) -> Result { let fields: Vec<&str> = param.split("=").collect(); if fields.len() != 2 { - return Err( - ErrorKind::ErrorCode(String::from("invalid container pipe size parameter")).into(), - ); + return Err(anyhow!("invalid container pipe size parameter")); } let key = fields[0]; if key != CONTAINER_PIPE_SIZE_OPTION { - return Err( - ErrorKind::ErrorCode(String::from("invalid container pipe size key name")).into(), - ); + return Err(anyhow!("invalid container pipe size key name")); } let res = fields[1].parse::(); if res.is_err() { - return Err( - ErrorKind::ErrorCode(String::from("unable to parse container pipe size")).into(), - ); + return Err(anyhow!("unable to parse container pipe size")); } let value = res.unwrap(); if value < 0 { - return Err(ErrorKind::ErrorCode(String::from( - "container pipe size should not be negative", - )) - .into()); + return Err(anyhow!("container pipe size should not be negative")); } Ok(value) @@ -232,7 +223,7 @@ mod tests { // helper function to make errors less crazy-long fn make_err(desc: &str) -> Error { - ErrorKind::ErrorCode(desc.to_string()).into() + anyhow!(desc) } // Parameters: diff --git a/src/agent/src/device.rs b/src/agent/src/device.rs index 423fc9abbb..5dbb166153 100644 --- a/src/agent/src/device.rs +++ b/src/agent/src/device.rs @@ -15,9 +15,9 @@ use crate::linux_abi::*; use crate::mount::{DRIVERBLKTYPE, DRIVERMMIOBLKTYPE, DRIVERNVDIMMTYPE, DRIVERSCSITYPE}; use crate::sandbox::Sandbox; use crate::{AGENT_CONFIG, GLOBAL_DEVICE_WATCHER}; +use anyhow::{anyhow, Result}; use oci::{LinuxDeviceCgroup, LinuxResources, Spec}; use protocols::agent::Device; -use rustjail::errors::*; // Convenience macro to obtain the scope logger macro_rules! sl { @@ -61,11 +61,10 @@ fn get_pci_device_address(pci_id: &str) -> Result { let tokens: Vec<&str> = pci_id.split("/").collect(); if tokens.len() != 2 { - return Err(ErrorKind::ErrorCode(format!( + return Err(anyhow!( "PCI Identifier for device should be of format [bridgeAddr/deviceAddr], got {}", pci_id - )) - .into()); + )); } let bridge_id = tokens[0]; @@ -85,11 +84,11 @@ fn get_pci_device_address(pci_id: &str) -> Result { let bus_num = files_slice.len(); if bus_num != 1 { - return Err(ErrorKind::ErrorCode(format!( + return Err(anyhow!( "Expected an entry for bus in {}, got {} entries instead", - bridge_bus_path, bus_num - )) - .into()); + bridge_bus_path, + bus_num + )); } let bus = files_slice[0].file_name().unwrap().to_str().unwrap(); @@ -135,11 +134,11 @@ fn get_device_name(sandbox: &Arc>, dev_addr: &str) -> Result name, Err(_) => { GLOBAL_DEVICE_WATCHER.lock().unwrap().remove_entry(dev_addr); - return Err(ErrorKind::ErrorCode(format!( + return Err(anyhow!( "Timeout reached after {:?} waiting for device {}", - hotplug_timeout, dev_addr - )) - .into()); + hotplug_timeout, + dev_addr + )); } }; @@ -164,11 +163,10 @@ pub fn get_pci_device_name(sandbox: &Arc>, pci_id: &str) -> Resul fn scan_scsi_bus(scsi_addr: &str) -> Result<()> { let tokens: Vec<&str> = scsi_addr.split(":").collect(); if tokens.len() != 2 { - return Err(ErrorKind::Msg(format!( + return Err(anyhow!( "Unexpected format for SCSI Address: {}, expect SCSIID:LUA", scsi_addr - )) - .into()); + )); } // Scan scsi host passing in the channel, SCSI id and LUN. @@ -203,24 +201,19 @@ fn update_spec_device_list(device: &Device, spec: &mut Spec) -> Result<()> { // If no container_path is provided, we won't be able to match and // update the device in the OCI spec device list. This is an error. if device.container_path == "" { - return Err(ErrorKind::Msg(format!( + return Err(anyhow!( "container_path cannot empty for device {:?}", device - )) - .into()); + )); } let linux = match spec.linux.as_mut() { - None => { - return Err( - ErrorKind::ErrorCode("Spec didn't container linux field".to_string()).into(), - ) - } + None => return Err(anyhow!("Spec didn't container linux field")), Some(l) => l, }; if !Path::new(&device.vm_path).exists() { - return Err(ErrorKind::Msg(format!("vm_path:{} doesn't exist", device.vm_path)).into()); + return Err(anyhow!("vm_path:{} doesn't exist", device.vm_path)); } let meta = fs::metadata(&device.vm_path)?; @@ -283,7 +276,7 @@ fn virtiommio_blk_device_handler( _sandbox: &Arc>, ) -> Result<()> { if device.vm_path == "" { - return Err(ErrorKind::Msg("Invalid path for virtio mmio blk device".to_string()).into()); + return Err(anyhow!("Invalid path for virtio mmio blk device")); } update_spec_device_list(device, spec) @@ -325,7 +318,7 @@ fn virtio_nvdimm_device_handler( _sandbox: &Arc>, ) -> Result<()> { if device.vm_path == "" { - return Err(ErrorKind::Msg("Invalid path for nvdimm device".to_string()).into()); + return Err(anyhow!("Invalid path for nvdimm device")); } update_spec_device_list(device, spec) @@ -349,23 +342,19 @@ fn add_device(device: &Device, spec: &mut Spec, sandbox: &Arc>) - device.id, device.field_type, device.vm_path, device.container_path, device.options); if device.field_type == "" { - return Err(ErrorKind::Msg(format!("invalid type for device {:?}", device)).into()); + return Err(anyhow!("invalid type for device {:?}", device)); } if device.id == "" && device.vm_path == "" { - return Err( - ErrorKind::Msg(format!("invalid ID and VM path for device {:?}", device)).into(), - ); + return Err(anyhow!("invalid ID and VM path for device {:?}", device)); } if device.container_path == "" { - return Err( - ErrorKind::Msg(format!("invalid container path for device {:?}", device)).into(), - ); + return Err(anyhow!("invalid container path for device {:?}", device)); } match DEVICEHANDLERLIST.get(device.field_type.as_str()) { - None => Err(ErrorKind::Msg(format!("Unknown device type {}", device.field_type)).into()), + None => Err(anyhow!("Unknown device type {}", device.field_type)), Some(dev_handler) => dev_handler(device, spec, sandbox), } } @@ -380,11 +369,7 @@ pub fn update_device_cgroup(spec: &mut Spec) -> Result<()> { let minor = stat::minor(rdev) as i64; let linux = match spec.linux.as_mut() { - None => { - return Err( - ErrorKind::ErrorCode("Spec didn't container linux field".to_string()).into(), - ) - } + None => return Err(anyhow!("Spec didn't container linux field")), Some(l) => l, }; diff --git a/src/agent/src/main.rs b/src/agent/src/main.rs index ab50d147a7..3a4c44514d 100644 --- a/src/agent/src/main.rs +++ b/src/agent/src/main.rs @@ -29,13 +29,13 @@ extern crate slog; extern crate netlink; use crate::netlink::{RtnlHandle, NETLINK_ROUTE}; +use anyhow::{anyhow, Context, Result}; use nix::fcntl::{self, OFlag}; use nix::sys::socket::{self, AddressFamily, SockAddr, SockFlag, SockType}; use nix::sys::wait::{self, WaitStatus}; use nix::unistd; use nix::unistd::dup; use prctl::set_child_subreaper; -use rustjail::errors::*; use signal_hook::{iterator::Signals, SIGCHLD}; use std::collections::HashMap; use std::env; @@ -208,26 +208,21 @@ fn start_sandbox(logger: &Logger, config: &agentConfig, init_mode: bool) -> Resu let builder = thread::Builder::new(); - let handle = builder - .spawn(move || { - let shells = shells.lock().unwrap(); - let result = setup_debug_console(shells.to_vec(), debug_console_vport); - if result.is_err() { - // Report error, but don't fail - warn!(thread_logger, "failed to setup debug console"; + let handle = builder.spawn(move || { + let shells = shells.lock().unwrap(); + let result = setup_debug_console(shells.to_vec(), debug_console_vport); + if result.is_err() { + // Report error, but don't fail + warn!(thread_logger, "failed to setup debug console"; "error" => format!("{}", result.unwrap_err())); - } - }) - .map_err(|e| format!("{:?}", e))?; + } + })?; shell_handle = Some(handle); } // Initialize unique sandbox structure. - let mut s = Sandbox::new(&logger).map_err(|e| { - error!(logger, "Failed to create sandbox with error: {:?}", e); - e - })?; + let mut s = Sandbox::new(&logger).context("Failed to create sandbox")?; if init_mode { let mut rtnl = RtnlHandle::new(NETLINK_ROUTE, 0).unwrap(); @@ -249,12 +244,12 @@ fn start_sandbox(logger: &Logger, config: &agentConfig, init_mode: bool) -> Resu let _ = server.start().unwrap(); - let _ = rx.recv().map_err(|e| format!("{:?}", e)); + let _ = rx.recv()?; server.shutdown(); if let Some(handle) = shell_handle { - handle.join().map_err(|e| format!("{:?}", e))?; + handle.join().map_err(|e| anyhow!("{:?}", e))?; } Ok(()) @@ -265,12 +260,8 @@ use nix::sys::wait::WaitPidFlag; fn setup_signal_handler(logger: &Logger, sandbox: Arc>) -> Result<()> { let logger = logger.new(o!("subsystem" => "signals")); - set_child_subreaper(true).map_err(|err| { - format!( - "failed to setup agent as a child subreaper, failed with {}", - err - ) - })?; + set_child_subreaper(true) + .map_err(|err| anyhow!(err).context("failed to setup agent as a child subreaper"))?; let signals = Signals::new(&[SIGCHLD])?; @@ -381,7 +372,7 @@ fn sethostname(hostname: &OsStr) -> Result<()> { unsafe { libc::sethostname(hostname.as_bytes().as_ptr() as *const libc::c_char, size) }; if result != 0 { - Err(ErrorKind::ErrorCode("failed to set hostname".to_string()).into()) + Err(anyhow!("failed to set hostname")) } else { Ok(()) } @@ -420,9 +411,7 @@ fn setup_debug_console(shells: Vec, port: u32) -> Result<()> { } if shell == "" { - return Err( - ErrorKind::ErrorCode("no shell found to launch debug console".to_string()).into(), - ); + return Err(anyhow!("no shell found to launch debug console")); } let f: RawFd = if port > 0 { @@ -452,7 +441,7 @@ fn setup_debug_console(shells: Vec, port: u32) -> Result<()> { let mut cmd = match cmd { Ok(c) => c, - Err(_) => return Err(ErrorKind::ErrorCode("failed to spawn shell".to_string()).into()), + Err(_) => return Err(anyhow!("failed to spawn shell")), }; cmd.wait()?; diff --git a/src/agent/src/metrics.rs b/src/agent/src/metrics.rs index 733c4f5426..20809aceb4 100644 --- a/src/agent/src/metrics.rs +++ b/src/agent/src/metrics.rs @@ -7,8 +7,8 @@ extern crate procfs; use prometheus::{Encoder, Gauge, GaugeVec, IntCounter, TextEncoder}; +use anyhow::Result; use protocols; -use rustjail::errors::*; const NAMESPACE_KATA_AGENT: &str = "kata_agent"; const NAMESPACE_KATA_GUEST: &str = "kata_guest"; diff --git a/src/agent/src/mount.rs b/src/agent/src/mount.rs index 9198884745..7ef6d745b3 100644 --- a/src/agent/src/mount.rs +++ b/src/agent/src/mount.rs @@ -3,7 +3,6 @@ // SPDX-License-Identifier: Apache-2.0 // -use rustjail::errors::*; use std::collections::HashMap; use std::ffi::CString; use std::fs; @@ -26,6 +25,7 @@ use crate::device::{get_pci_device_name, get_scsi_device_name, online_device}; use crate::linux_abi::*; use crate::protocols::agent::Storage; use crate::Sandbox; +use anyhow::{anyhow, Context, Result}; use slog::Logger; pub const DRIVER9PTYPE: &str = "9p"; @@ -191,11 +191,11 @@ impl<'a> BareMount<'a> { let cstr_fs_type: CString; if self.source.len() == 0 { - return Err(ErrorKind::ErrorCode("need mount source".to_string()).into()); + return Err(anyhow!("need mount source")); } if self.destination.len() == 0 { - return Err(ErrorKind::ErrorCode("need mount destination".to_string()).into()); + return Err(anyhow!("need mount destination")); } cstr_source = CString::new(self.source)?; @@ -205,7 +205,7 @@ impl<'a> BareMount<'a> { dest = cstr_dest.as_ptr(); if self.fs_type.len() == 0 { - return Err(ErrorKind::ErrorCode("need mount FS type".to_string()).into()); + return Err(anyhow!("need mount FS type")); } cstr_fs_type = CString::new(self.fs_type)?; @@ -227,13 +227,12 @@ impl<'a> BareMount<'a> { let rc = unsafe { mount(source, dest, fs_type, self.flags.bits(), options) }; if rc < 0 { - return Err(ErrorKind::ErrorCode(format!( + return Err(anyhow!( "failed to mount {:?} to {:?}, with error: {}", self.source, self.destination, io::Error::last_os_error() - )) - .into()); + )); } Ok(()) } @@ -274,8 +273,10 @@ fn local_storage_handler( return Ok("".to_string()); } - fs::create_dir_all(&storage.mount_point) - .chain_err(|| format!("failed to create dir all {:?}", &storage.mount_point))?; + fs::create_dir_all(&storage.mount_point).context(format!( + "failed to create dir all {:?}", + &storage.mount_point + ))?; let opts_vec: Vec = storage.options.to_vec(); @@ -332,11 +333,11 @@ fn virtio_blk_storage_handler( // use the virt path provided in Storage Source if storage.source.starts_with("/dev") { let metadata = fs::metadata(&storage.source) - .chain_err(|| format!("get metadata on file {:?}", &storage.source))?; + .context(format!("get metadata on file {:?}", &storage.source))?; let mode = metadata.permissions().mode(); if mode & libc::S_IFBLK == 0 { - return Err(ErrorKind::ErrorCode(format!("Invalid device {}", &storage.source)).into()); + return Err(anyhow!("Invalid device {}", &storage.source)); } } else { let dev_path = get_pci_device_name(&sandbox, &storage.source)?; @@ -376,7 +377,7 @@ fn mount_storage(logger: &Logger, storage: &Storage) -> Result<()> { DRIVER9PTYPE | DRIVERVIRTIOFSTYPE => { let dest_path = Path::new(storage.mount_point.as_str()); if !dest_path.exists() { - fs::create_dir_all(dest_path).chain_err(|| "Create mount destination failed")?; + fs::create_dir_all(dest_path).context("Create mount destination failed")?; } } _ => { @@ -450,11 +451,10 @@ pub fn add_storages( let handler = match STORAGEHANDLERLIST.get(&handler_name.as_str()) { None => { - return Err(ErrorKind::ErrorCode(format!( + return Err(anyhow!( "Failed to find the storage handler {}", storage.driver.to_owned() - )) - .into()); + )); } Some(f) => f, }; @@ -480,7 +480,7 @@ fn mount_to_rootfs(logger: &Logger, m: &INIT_MOUNT) -> Result<()> { let bare_mount = BareMount::new(m.src, m.dest, m.fstype, flags, options.as_str(), logger); - fs::create_dir_all(Path::new(m.dest)).chain_err(|| "could not create directory")?; + fs::create_dir_all(Path::new(m.dest)).context("could not create directory")?; if let Err(err) = bare_mount.mount() { if m.src != "dev" { @@ -514,7 +514,7 @@ pub fn get_mount_fs_type(mount_point: &str) -> Result { // any error ecountered. pub fn get_mount_fs_type_from_file(mount_file: &str, mount_point: &str) -> Result { if mount_point == "" { - return Err(ErrorKind::ErrorCode(format!("Invalid mount point {}", mount_point)).into()); + return Err(anyhow!("Invalid mount point {}", mount_point)); } let file = File::open(mount_file)?; @@ -536,11 +536,10 @@ pub fn get_mount_fs_type_from_file(mount_file: &str, mount_point: &str) -> Resul } } - Err(ErrorKind::ErrorCode(format!( + Err(anyhow!( "failed to find FS type for mount point {}", mount_point )) - .into()) } pub fn get_cgroup_mounts(logger: &Logger, cg_path: &str) -> Result> { @@ -635,7 +634,7 @@ pub fn cgroups_mount(logger: &Logger) -> Result<()> { pub fn remove_mounts(mounts: &Vec) -> Result<()> { for m in mounts.iter() { - mount::umount(m.as_str()).chain_err(|| format!("failed to umount {:?}", m))?; + mount::umount(m.as_str()).context(format!("failed to umount {:?}", m))?; } Ok(()) } @@ -647,21 +646,15 @@ fn ensure_destination_exists(destination: &str, fs_type: &str) -> Result<()> { if !d.exists() { let dir = match d.parent() { Some(d) => d, - None => { - return Err(ErrorKind::ErrorCode(format!( - "mount destination {} doesn't exist", - destination - )) - .into()) - } + None => return Err(anyhow!("mount destination {} doesn't exist", destination)), }; if !dir.exists() { - fs::create_dir_all(dir).chain_err(|| format!("create dir all failed on {:?}", dir))?; + fs::create_dir_all(dir).context(format!("create dir all failed on {:?}", dir))?; } } if fs_type != "bind" || d.is_dir() { - fs::create_dir_all(d).chain_err(|| format!("create dir all failed on {:?}", d))?; + fs::create_dir_all(d).context(format!("create dir all failed on {:?}", d))?; } else { fs::OpenOptions::new().create(true).open(d)?; } diff --git a/src/agent/src/random.rs b/src/agent/src/random.rs index 23446d3212..6d28a49a0c 100644 --- a/src/agent/src/random.rs +++ b/src/agent/src/random.rs @@ -3,11 +3,11 @@ // SPDX-License-Identifier: Apache-2.0 // +use anyhow::Result; use libc; use nix::errno::Errno; use nix::fcntl::{self, OFlag}; use nix::sys::stat::Mode; -use rustjail::errors::*; use std::fs; pub const RNGDEV: &str = "/dev/random"; diff --git a/src/agent/src/rpc.rs b/src/agent/src/rpc.rs index 0cb695fa02..befc5557ba 100644 --- a/src/agent/src/rpc.rs +++ b/src/agent/src/rpc.rs @@ -7,6 +7,7 @@ use std::path::Path; use std::sync::{Arc, Mutex}; use ttrpc; +use anyhow::{anyhow, Context, Result}; use oci::{LinuxNamespace, Root, Spec}; use protobuf::{RepeatedField, SingularPtrField}; use protocols::agent::{ @@ -21,7 +22,6 @@ use protocols::health::{ use protocols::types::Interface; use rustjail; use rustjail::container::{BaseContainer, Container, LinuxContainer}; -use rustjail::errors::*; use rustjail::process::Process; use rustjail::specconv::CreateOpts; @@ -90,9 +90,7 @@ impl agentService { Some(spec) => rustjail::grpc_to_oci(spec), None => { error!(sl!(), "no oci spec in the create container request!"); - return Err( - ErrorKind::Nix(nix::Error::from_errno(nix::errno::Errno::EINVAL)).into(), - ); + return Err(anyhow!(nix::Error::from_errno(nix::errno::Errno::EINVAL))); } }; @@ -101,7 +99,7 @@ impl agentService { // re-scan PCI bus // looking for hidden devices - rescan_pci_bus().chain_err(|| "Could not rescan PCI bus")?; + rescan_pci_bus().context("Could not rescan PCI bus")?; // Some devices need some extra processing (the ones invoked with // --device for instance), and that's what this call is doing. It @@ -163,7 +161,7 @@ impl agentService { tp } else { info!(sl!(), "no process configurations!"); - return Err(ErrorKind::Nix(nix::Error::from_errno(nix::errno::Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(nix::errno::Errno::EINVAL))); }; ctr.start(p)?; @@ -184,7 +182,7 @@ impl agentService { let ctr: &mut LinuxContainer = match s.get_container(cid.as_str()) { Some(cr) => cr, None => { - return Err(ErrorKind::Nix(nix::Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } }; @@ -203,7 +201,7 @@ impl agentService { let ctr: &mut LinuxContainer = match sandbox.get_container(cid.as_str()) { Some(cr) => cr, None => { - return Err(ErrorKind::Nix(nix::Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } }; @@ -252,13 +250,13 @@ impl agentService { }); if let Err(_) = rx.recv_timeout(Duration::from_secs(req.timeout as u64)) { - return Err(ErrorKind::Nix(nix::Error::from_errno(nix::errno::Errno::ETIME)).into()); + return Err(anyhow!(nix::Error::from_errno(nix::errno::Errno::ETIME))); } if let Err(_) = handle.join() { - return Err( - ErrorKind::Nix(nix::Error::from_errno(nix::errno::Errno::UnknownErrno)).into(), - ); + return Err(anyhow!(nix::Error::from_errno( + nix::errno::Errno::UnknownErrno + ))); } let s = self.sandbox.clone(); @@ -301,7 +299,7 @@ impl agentService { let process = if req.process.is_some() { req.process.as_ref().unwrap() } else { - return Err(ErrorKind::Nix(nix::Error::from_errno(nix::errno::Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(nix::errno::Errno::EINVAL))); }; let pipe_size = AGENT_CONFIG.read().unwrap().container_pipe_size; @@ -311,9 +309,7 @@ impl agentService { let ctr = match sandbox.get_container(cid.as_str()) { Some(v) => v, None => { - return Err( - ErrorKind::Nix(nix::Error::from_errno(nix::errno::Errno::EINVAL)).into(), - ); + return Err(anyhow!(nix::Error::from_errno(nix::errno::Errno::EINVAL))); } }; @@ -396,7 +392,7 @@ impl agentService { let ctr: &mut LinuxContainer = match sandbox.get_container(cid.as_str()) { Some(cr) => cr, None => { - return Err(ErrorKind::Nix(nix::Error::from_errno(Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(Errno::EINVAL))); } }; @@ -472,9 +468,7 @@ impl agentService { Err(e) => match e { nix::Error::Sys(nix::errno::Errno::EAGAIN) => l = 0, _ => { - return Err( - ErrorKind::Nix(nix::Error::from_errno(nix::errno::Errno::EIO)).into(), - ); + return Err(anyhow!(nix::Error::from_errno(nix::errno::Errno::EIO))); } }, } @@ -513,7 +507,7 @@ impl agentService { } if fd == -1 { - return Err(ErrorKind::Nix(nix::Error::from_errno(nix::errno::Errno::EINVAL)).into()); + return Err(anyhow!(nix::Error::from_errno(nix::errno::Errno::EINVAL))); } let vector = read_stream(fd, req.len as usize)?; @@ -1338,7 +1332,7 @@ fn get_memory_info(block_size: bool, hotplug: bool) -> Result<(u64, bool)> { Ok(v) => { if v.len() == 0 { info!(sl!(), "string in empty???"); - return Err(ErrorKind::ErrorCode("Invalid block size".to_string()).into()); + return Err(anyhow!("Invalid block size")); } size = v.trim().parse::()?; @@ -1346,7 +1340,7 @@ fn get_memory_info(block_size: bool, hotplug: bool) -> Result<(u64, bool)> { Err(e) => { info!(sl!(), "memory block size error: {:?}", e.kind()); if e.kind() != std::io::ErrorKind::NotFound { - return Err(ErrorKind::Io(e).into()); + return Err(anyhow!(e)); } } } @@ -1364,9 +1358,9 @@ fn get_memory_info(block_size: bool, hotplug: bool) -> Result<(u64, bool)> { match e { nix::Error::Sys(errno) => match errno { Errno::ENOENT => plug = false, - _ => return Err(ErrorKind::Nix(e).into()), + _ => return Err(anyhow!(e)), }, - _ => return Err(ErrorKind::Nix(e).into()), + _ => return Err(anyhow!(e)), } } } @@ -1407,15 +1401,15 @@ fn read_stream(fd: RawFd, l: usize) -> Result> { // was closed, instead it would return a 0 reading length, please // see https://github.com/rust-lang/rfcs/blob/master/text/0517-io-os-reform.md#errors if len == 0 { - return Err(ErrorKind::ErrorCode("read meet eof".to_string()).into()); + return Err(anyhow!("read meet eof")); } } Err(e) => match e { nix::Error::Sys(errno) => match errno { Errno::EAGAIN => v.resize(0, 0), - _ => return Err(ErrorKind::Nix(nix::Error::Sys(errno)).into()), + _ => return Err(anyhow!(nix::Error::Sys(errno))), }, - _ => return Err(ErrorKind::ErrorCode("read error".to_string()).into()), + _ => return Err(anyhow!("read error")), }, } @@ -1430,15 +1424,13 @@ fn find_process<'a>( ) -> Result<&'a mut Process> { let ctr = match sandbox.get_container(cid) { Some(v) => v, - None => return Err(ErrorKind::ErrorCode(String::from("Invalid container id")).into()), + None => return Err(anyhow!("Invalid container id")), }; if init || eid == "" { let p = match ctr.processes.get_mut(&ctr.init_process_pid) { Some(v) => v, - None => { - return Err(ErrorKind::ErrorCode(String::from("cannot find init process!")).into()) - } + None => return Err(anyhow!("cannot find init process!")), }; return Ok(p); @@ -1446,7 +1438,7 @@ fn find_process<'a>( let p = match ctr.get_process(eid) { Ok(v) => v, - Err(_) => return Err(ErrorKind::ErrorCode("Invalid exec id".to_string()).into()), + Err(_) => return Err(anyhow!("Invalid exec id")), }; Ok(p) @@ -1496,11 +1488,7 @@ fn update_container_namespaces( sandbox_pidns: bool, ) -> Result<()> { let linux = match spec.linux.as_mut() { - None => { - return Err( - ErrorKind::ErrorCode("Spec didn't container linux field".to_string()).into(), - ) - } + None => return Err(anyhow!("Spec didn't container linux field")), Some(l) => l, }; @@ -1704,7 +1692,7 @@ fn setup_bundle(cid: &str, spec: &mut Spec) -> Result { ); let _ = spec.save(config_path.to_str().unwrap()); - let olddir = unistd::getcwd().chain_err(|| "cannot getcwd")?; + let olddir = unistd::getcwd().context("cannot getcwd")?; unistd::chdir(bundle_path.to_str().unwrap())?; Ok(olddir) @@ -1712,7 +1700,7 @@ fn setup_bundle(cid: &str, spec: &mut Spec) -> Result { fn load_kernel_module(module: &protocols::agent::KernelModule) -> Result<()> { if module.name == "" { - return Err(ErrorKind::ErrorCode("Kernel module name is empty".to_string()).into()); + return Err(anyhow!("Kernel module name is empty")); } info!( @@ -1744,11 +1732,9 @@ fn load_kernel_module(module: &protocols::agent::KernelModule) -> Result<()> { "load_kernel_module return code: {} stdout:{} stderr:{}", code, std_out, std_err ); - return Err(ErrorKind::ErrorCode(msg).into()); - } - None => { - return Err(ErrorKind::ErrorCode("Process terminated by signal".to_string()).into()) + return Err(anyhow!(msg)); } + None => return Err(anyhow!("Process terminated by signal")), } } diff --git a/src/agent/src/sandbox.rs b/src/agent/src/sandbox.rs index 1bec2bb12f..964c1d6861 100644 --- a/src/agent/src/sandbox.rs +++ b/src/agent/src/sandbox.rs @@ -9,6 +9,7 @@ use crate::mount::{get_mount_fs_type, remove_mounts, TYPEROOTFS}; use crate::namespace::Namespace; use crate::namespace::NSTYPEPID; use crate::network::Network; +use anyhow::{anyhow, Context, Result}; use libc::pid_t; use netlink::{RtnlHandle, NETLINK_ROUTE}; use oci::{Hook, Hooks}; @@ -17,7 +18,6 @@ use regex::Regex; use rustjail::cgroups; use rustjail::container::BaseContainer; use rustjail::container::LinuxContainer; -use rustjail::errors::*; use rustjail::process::Process; use slog::Logger; use std::collections::HashMap; @@ -105,13 +105,7 @@ impl Sandbox { // acquiring a lock on sandbox. pub fn unset_sandbox_storage(&mut self, path: &str) -> Result { match self.storages.get_mut(path) { - None => { - return Err(ErrorKind::ErrorCode(format!( - "Sandbox storage with path {} not found", - path - )) - .into()) - } + None => return Err(anyhow!("Sandbox storage with path {} not found", path)), Some(count) => { *count -= 1; if *count < 1 { @@ -131,7 +125,7 @@ impl Sandbox { pub fn remove_sandbox_storage(&self, path: &str) -> Result<()> { let mounts = vec![path.to_string()]; remove_mounts(&mounts)?; - fs::remove_dir_all(path).chain_err(|| format!("failed to remove dir {:?}", path))?; + fs::remove_dir_all(path).context(format!("failed to remove dir {:?}", path))?; Ok(()) } @@ -168,11 +162,7 @@ impl Sandbox { self.shared_ipcns = match Namespace::new(&self.logger).as_ipc().setup() { Ok(ns) => ns, Err(err) => { - return Err(ErrorKind::ErrorCode(format!( - "Failed to setup persistent IPC namespace with error: {}", - err - )) - .into()) + return Err(anyhow!(err).context("Failed to setup persistent IPC namespace")); } }; @@ -183,11 +173,7 @@ impl Sandbox { { Ok(ns) => ns, Err(err) => { - return Err(ErrorKind::ErrorCode(format!( - "Failed to setup persistent UTS namespace with error: {}", - err - )) - .into()) + return Err(anyhow!(err).context("Failed to setup persistent UTS namespace")); } }; Ok(true) @@ -206,10 +192,9 @@ impl Sandbox { if self.sandbox_pidns.is_none() && self.containers.len() == 0 { let init_pid = c.init_process_pid; if init_pid == -1 { - return Err(ErrorKind::ErrorCode(String::from( - "Failed to setup pid namespace: init container pid is -1", - )) - .into()); + return Err(anyhow!( + "Failed to setup pid namespace: init container pid is -1" + )); } let mut pid_ns = Namespace::new(&self.logger).as_pid(); From 46d7b9b8dc4ee1ef76678d2d7d203ec9e784ad1d Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 28 Aug 2020 13:55:57 -0500 Subject: [PATCH 7/9] agent/rustjail: remove rustjail::errors `anyhow` replaces `rustjail::errors`, hence it's not longer needed Signed-off-by: Julio Montes --- src/agent/rustjail/src/container.rs | 2 +- src/agent/rustjail/src/errors.rs | 34 ----------------------------- src/agent/rustjail/src/lib.rs | 2 -- src/agent/rustjail/src/mount.rs | 2 +- 4 files changed, 2 insertions(+), 38 deletions(-) delete mode 100644 src/agent/rustjail/src/errors.rs diff --git a/src/agent/rustjail/src/container.rs b/src/agent/rustjail/src/container.rs index 9b25199e71..af0b67f231 100644 --- a/src/agent/rustjail/src/container.rs +++ b/src/agent/rustjail/src/container.rs @@ -14,7 +14,7 @@ use std::os::unix::io::RawFd; use std::path::{Path, PathBuf}; use std::time::SystemTime; // use crate::sync::Cond; -use anyhow::{anyhow, Context, Result}; +use anyhow::{anyhow, bail, Context, Result}; use libc::pid_t; use oci::{LinuxDevice, LinuxIDMapping}; use std::clone::Clone; diff --git a/src/agent/rustjail/src/errors.rs b/src/agent/rustjail/src/errors.rs deleted file mode 100644 index 8478a50b6d..0000000000 --- a/src/agent/rustjail/src/errors.rs +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright (c) 2019 Ant Financial -// -// SPDX-License-Identifier: Apache-2.0 -// - -// define errors here - -error_chain! { - types { - Error, ErrorKind, ResultExt, Result; - } - // foreign error conv to chain error - foreign_links { - Io(std::io::Error); - Nix(nix::Error); - Ffi(std::ffi::NulError); - Caps(caps::errors::Error); - Serde(serde_json::Error); - FromUTF8(std::string::FromUtf8Error); - Parse(std::num::ParseIntError); - Scanfmt(scan_fmt::parse::ScanError); - Ip(std::net::AddrParseError); - Regex(regex::Error); - EnvVar(std::env::VarError); - UTF8(std::str::Utf8Error); - } - // define new errors - errors { - ErrorCode(t: String) { - description("Error Code") - display("Error Code: '{}'", t) - } - } -} diff --git a/src/agent/rustjail/src/lib.rs b/src/agent/rustjail/src/lib.rs index b3866d988d..c77da3a309 100644 --- a/src/agent/rustjail/src/lib.rs +++ b/src/agent/rustjail/src/lib.rs @@ -13,7 +13,6 @@ #![allow(non_upper_case_globals)] // #![allow(unused_comparisons)] #[macro_use] -extern crate error_chain; extern crate serde; extern crate serde_json; #[macro_use] @@ -45,7 +44,6 @@ macro_rules! sl { pub mod capabilities; pub mod cgroups; pub mod container; -pub mod errors; pub mod mount; pub mod process; pub mod specconv; diff --git a/src/agent/rustjail/src/mount.rs b/src/agent/rustjail/src/mount.rs index ae0e7c5be6..460ba2c56b 100644 --- a/src/agent/rustjail/src/mount.rs +++ b/src/agent/rustjail/src/mount.rs @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // -use anyhow::{anyhow, Context, Error, Result}; +use anyhow::{anyhow, bail, Context, Error, Result}; use libc::uid_t; use nix::errno::Errno; use nix::fcntl::{self, OFlag}; From 6c96d666673dcb6f803c33bae405553b9ff5f071 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 28 Aug 2020 13:57:26 -0500 Subject: [PATCH 8/9] agent: update Cargo toml and lock `rustjail::erros` was removed in a previous commit, hence some external crates like `error_chain` are no longger required, update Cargo.toml and Cargo.lock to reflect these changes. Signed-off-by: Julio Montes --- src/agent/Cargo.lock | 43 ----------------------------------- src/agent/Cargo.toml | 1 - src/agent/rustjail/Cargo.toml | 1 - 3 files changed, 45 deletions(-) diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index 044f39437e..b8569e237b 100644 --- a/src/agent/Cargo.lock +++ b/src/agent/Cargo.lock @@ -1,14 +1,5 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -[[package]] -name = "addr2line" -version = "0.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a49806b9dadc843c61e7c97e72490ad7f7220ae249012fbda9ad0609457c0543" -dependencies = [ - "gimli", -] - [[package]] name = "adler32" version = "1.0.4" @@ -54,19 +45,6 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8aac770f1885fd7e387acedd76065302551364496e46b3dd00860b2f8359b9d" -[[package]] -name = "backtrace" -version = "0.3.48" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0df2f85c8a2abbe3b7d7e748052fdd9b76a0458fdeb16ad4223f5eca78c7c130" -dependencies = [ - "addr2line", - "cfg-if", - "libc", - "object", - "rustc-demangle", -] - [[package]] name = "base64" version = "0.11.0" @@ -213,7 +191,6 @@ version = "0.12.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d371106cc88ffdfb1eabd7111e432da544f16f3e2d7bf1dfe8bf575f1df045cd" dependencies = [ - "backtrace", "version_check", ] @@ -246,12 +223,6 @@ dependencies = [ "wasi", ] -[[package]] -name = "gimli" -version = "0.21.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bcc8e0c9bce37868955864dbecd2b1ab2bdf967e6f28066d65aaac620444b65c" - [[package]] name = "hex" version = "0.4.2" @@ -269,7 +240,6 @@ name = "kata-agent" version = "0.1.0" dependencies = [ "anyhow", - "error-chain", "lazy_static", "libc", "logging", @@ -412,12 +382,6 @@ dependencies = [ "autocfg", ] -[[package]] -name = "object" -version = "0.19.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9cbca9424c482ee628fa549d9c812e2cd22f1180b9222c9200fdfa6eb31aecb2" - [[package]] name = "oci" version = "0.1.0" @@ -649,12 +613,6 @@ dependencies = [ "crossbeam-utils", ] -[[package]] -name = "rustc-demangle" -version = "0.1.16" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4c691c0e608126e00913e33f0ccf3727d5fc84573623b8d65b2df340b5201783" - [[package]] name = "rustjail" version = "0.1.0" @@ -662,7 +620,6 @@ dependencies = [ "anyhow", "caps", "dirs", - "error-chain", "lazy_static", "libc", "nix 0.17.0", diff --git a/src/agent/Cargo.toml b/src/agent/Cargo.toml index e3529161ad..7b4ef1f1eb 100644 --- a/src/agent/Cargo.toml +++ b/src/agent/Cargo.toml @@ -11,7 +11,6 @@ rustjail = { path = "rustjail" } protocols = { path = "protocols" } netlink = { path = "netlink", features = ["with-log", "with-agent-handler"] } lazy_static = "1.3.0" -error-chain = "0.12.1" ttrpc = { git = "https://github.com/containerd/ttrpc-rust.git", branch="0.3.0" } protobuf = "=2.14.0" libc = "0.2.58" diff --git a/src/agent/rustjail/Cargo.toml b/src/agent/rustjail/Cargo.toml index 9bd7023203..c8481efa6b 100644 --- a/src/agent/rustjail/Cargo.toml +++ b/src/agent/rustjail/Cargo.toml @@ -5,7 +5,6 @@ authors = ["The Kata Containers community "] edition = "2018" [dependencies] -error-chain = "0.12.1" serde = "1.0.91" serde_json = "1.0.39" serde_derive = "1.0.91" From 8b07bc2c805660b143558adf7821b48391667307 Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Fri, 28 Aug 2020 14:44:43 -0500 Subject: [PATCH 9/9] agent: fix unit tests - remove rustjail::errors Fix unit tests and use `anyhow::Error`. Signed-off-by: Julio Montes --- src/agent/src/config.rs | 3 ++- src/agent/src/main.rs | 4 ++-- src/agent/src/sandbox.rs | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/agent/src/config.rs b/src/agent/src/config.rs index 10c151eeaa..1b854fe743 100644 --- a/src/agent/src/config.rs +++ b/src/agent/src/config.rs @@ -203,6 +203,7 @@ fn get_container_pipe_size(param: &str) -> Result { #[cfg(test)] mod tests { use super::*; + use anyhow::Error; use std::fs::File; use std::io::Write; use std::time; @@ -223,7 +224,7 @@ mod tests { // helper function to make errors less crazy-long fn make_err(desc: &str) -> Error { - anyhow!(desc) + anyhow!(desc.to_string()) } // Parameters: diff --git a/src/agent/src/main.rs b/src/agent/src/main.rs index 3a4c44514d..01fd71f7b7 100644 --- a/src/agent/src/main.rs +++ b/src/agent/src/main.rs @@ -470,7 +470,7 @@ mod tests { assert!(result.is_err()); assert_eq!( result.unwrap_err().to_string(), - "Error Code: 'no shell found to launch debug console'" + "no shell found to launch debug console" ); } @@ -496,7 +496,7 @@ mod tests { assert!(result.is_err()); assert_eq!( result.unwrap_err().to_string(), - "Error Code: 'no shell found to launch debug console'" + "no shell found to launch debug console" ); } } diff --git a/src/agent/src/sandbox.rs b/src/agent/src/sandbox.rs index 964c1d6861..c34c3cdd74 100644 --- a/src/agent/src/sandbox.rs +++ b/src/agent/src/sandbox.rs @@ -350,6 +350,7 @@ mod tests { //use rustjail::Error; use super::Sandbox; use crate::{mount::BareMount, skip_if_not_root}; + use anyhow::Error; use nix::mount::MsFlags; use oci::{Linux, Root, Spec}; use rustjail::container::LinuxContainer; @@ -359,7 +360,7 @@ mod tests { use std::os::unix::fs::PermissionsExt; use tempfile::Builder; - fn bind_mount(src: &str, dst: &str, logger: &Logger) -> Result<(), rustjail::errors::Error> { + fn bind_mount(src: &str, dst: &str, logger: &Logger) -> Result<(), Error> { let baremount = BareMount::new(src, dst, "bind", MsFlags::MS_BIND, "", &logger); baremount.mount() }