diff --git a/Cargo.lock b/Cargo.lock index 0ba71a834..988ca2d0c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1938,6 +1938,7 @@ dependencies = [ "serde", "serde_json", "serial_test", + "thiserror", ] [[package]] diff --git a/crates/libcgroups/Cargo.toml b/crates/libcgroups/Cargo.toml index 3f91762dc..ef33a10cd 100644 --- a/crates/libcgroups/Cargo.toml +++ b/crates/libcgroups/Cargo.toml @@ -23,7 +23,6 @@ cgroupsv2_devices = ["rbpf", "libbpf-sys", "errno", "libc"] nix = "0.26.2" procfs = "0.15.1" log = "0.4" -anyhow = "1.0" oci-spec = { version = "^0.6.0", features = ["runtime"] } dbus = { version = "0.9.7", optional = true } fixedbitset = "0.4.2" @@ -32,8 +31,10 @@ rbpf = {version = "0.1.0", optional = true } libbpf-sys = { version = "1.1.1", optional = true } errno = { version = "0.3.1", optional = true } libc = { version = "0.2.142", optional = true } +thiserror = "1.0.40" [dev-dependencies] +anyhow = "1.0" oci-spec = { version = "^0.6.0", features = ["proptests", "runtime"] } quickcheck = "1" mockall = { version = "0.11.4", features = [] } diff --git a/crates/libcgroups/src/common.rs b/crates/libcgroups/src/common.rs index b64beb6e4..68b17de04 100644 --- a/crates/libcgroups/src/common.rs +++ b/crates/libcgroups/src/common.rs @@ -2,11 +2,10 @@ use std::{ fmt::{Debug, Display}, fs::{self, File}, io::{BufRead, BufReader, Write}, - path::{Path, PathBuf}, + path::{Path, PathBuf, StripPrefixError}, time::Duration, }; -use anyhow::{bail, Context, Result}; use nix::{ sys::statfs::{statfs, CGROUP2_SUPER_MAGIC, TMPFS_MAGIC}, unistd::Pid, @@ -30,23 +29,93 @@ pub const CGROUP_PROCS: &str = "cgroup.procs"; pub const DEFAULT_CGROUP_ROOT: &str = "/sys/fs/cgroup"; pub trait CgroupManager { + type Error; + /// Adds a task specified by its pid to the cgroup - fn add_task(&self, pid: Pid) -> Result<()>; + fn add_task(&self, pid: Pid) -> Result<(), Self::Error>; /// Applies resource restrictions to the cgroup - fn apply(&self, controller_opt: &ControllerOpt) -> Result<()>; + fn apply(&self, controller_opt: &ControllerOpt) -> Result<(), Self::Error>; /// Removes the cgroup - fn remove(&self) -> Result<()>; + fn remove(&self) -> Result<(), Self::Error>; /// Sets the freezer cgroup to the specified state - fn freeze(&self, state: FreezerState) -> Result<()>; + fn freeze(&self, state: FreezerState) -> Result<(), Self::Error>; /// Retrieve statistics for the cgroup - fn stats(&self) -> Result; + fn stats(&self) -> Result; /// Gets the PIDs inside the cgroup - fn get_all_pids(&self) -> Result>; + fn get_all_pids(&self) -> Result, Self::Error>; +} + +#[derive(thiserror::Error, Debug)] +pub enum AnyManagerError { + #[error(transparent)] + Systemd(#[from] systemd::manager::SystemdManagerError), + #[error(transparent)] + V1(#[from] v1::manager::V1ManagerError), + #[error(transparent)] + V2(#[from] v2::manager::V2ManagerError), +} + +pub enum AnyCgroupManager { + Systemd(systemd::manager::Manager), + V1(v1::manager::Manager), + V2(v2::manager::Manager), +} + +impl CgroupManager for AnyCgroupManager { + type Error = AnyManagerError; + + fn add_task(&self, pid: Pid) -> Result<(), Self::Error> { + match self { + AnyCgroupManager::Systemd(m) => Ok(m.add_task(pid)?), + AnyCgroupManager::V1(m) => Ok(m.add_task(pid)?), + AnyCgroupManager::V2(m) => Ok(m.add_task(pid)?), + } + } + + fn apply(&self, controller_opt: &ControllerOpt) -> Result<(), Self::Error> { + match self { + AnyCgroupManager::Systemd(m) => Ok(m.apply(controller_opt)?), + AnyCgroupManager::V1(m) => Ok(m.apply(controller_opt)?), + AnyCgroupManager::V2(m) => Ok(m.apply(controller_opt)?), + } + } + + fn remove(&self) -> Result<(), Self::Error> { + match self { + AnyCgroupManager::Systemd(m) => Ok(m.remove()?), + AnyCgroupManager::V1(m) => Ok(m.remove()?), + AnyCgroupManager::V2(m) => Ok(m.remove()?), + } + } + + fn freeze(&self, state: FreezerState) -> Result<(), Self::Error> { + match self { + AnyCgroupManager::Systemd(m) => Ok(m.freeze(state)?), + AnyCgroupManager::V1(m) => Ok(m.freeze(state)?), + AnyCgroupManager::V2(m) => Ok(m.freeze(state)?), + } + } + + fn stats(&self) -> Result { + match self { + AnyCgroupManager::Systemd(m) => Ok(m.stats()?), + AnyCgroupManager::V1(m) => Ok(m.stats()?), + AnyCgroupManager::V2(m) => Ok(m.stats()?), + } + } + + fn get_all_pids(&self) -> Result, Self::Error> { + match self { + AnyCgroupManager::Systemd(m) => Ok(m.get_all_pids()?), + AnyCgroupManager::V1(m) => Ok(m.get_all_pids()?), + AnyCgroupManager::V2(m) => Ok(m.get_all_pids()?), + } + } } #[derive(Debug)] @@ -92,39 +161,103 @@ pub struct ControllerOpt<'a> { pub freezer_state: Option, } +#[derive(thiserror::Error, Debug)] +pub enum WrappedIoError { + #[error("failed to open {path}: {err}")] + Open { err: std::io::Error, path: PathBuf }, + #[error("failed to write {data} to {path}: {err}")] + Write { + err: std::io::Error, + path: PathBuf, + data: String, + }, + #[error("failed to read {path}: {err}")] + Read { err: std::io::Error, path: PathBuf }, + #[error("failed to create dir {path}: {err}")] + CreateDir { err: std::io::Error, path: PathBuf }, + #[error("at {path}: {err}")] + Other { err: std::io::Error, path: PathBuf }, +} + +impl WrappedIoError { + pub fn inner(&self) -> &std::io::Error { + match self { + WrappedIoError::Open { err, .. } => err, + WrappedIoError::Write { err, .. } => err, + WrappedIoError::Read { err, .. } => err, + WrappedIoError::CreateDir { err, .. } => err, + WrappedIoError::Other { err, .. } => err, + } + } +} + #[inline] -pub fn write_cgroup_file_str>(path: P, data: &str) -> Result<()> { +pub fn write_cgroup_file_str>(path: P, data: &str) -> Result<(), WrappedIoError> { + let path = path.as_ref(); + fs::OpenOptions::new() .create(false) .write(true) .truncate(false) - .open(path.as_ref()) - .with_context(|| format!("failed to open {:?}", path.as_ref()))? + .open(path) + .map_err(|err| WrappedIoError::Open { + err, + path: path.to_path_buf(), + })? .write_all(data.as_bytes()) - .with_context(|| format!("failed to write {} to {:?}", data, path.as_ref()))?; + .map_err(|err| WrappedIoError::Write { + err, + path: path.to_path_buf(), + data: data.into(), + })?; Ok(()) } #[inline] -pub fn write_cgroup_file, T: ToString>(path: P, data: T) -> Result<()> { +pub fn write_cgroup_file, T: ToString>( + path: P, + data: T, +) -> Result<(), WrappedIoError> { + let path = path.as_ref(); let data = data.to_string(); + fs::OpenOptions::new() .create(false) .write(true) .truncate(false) - .open(path.as_ref()) - .with_context(|| format!("failed to open {:?}", path.as_ref()))? + .open(path) + .map_err(|err| WrappedIoError::Open { + err, + path: path.to_path_buf(), + })? .write_all(data.as_bytes()) - .with_context(|| format!("failed to write {} to {:?}", data, path.as_ref()))?; + .map_err(|err| WrappedIoError::Write { + err, + path: path.to_path_buf(), + data, + })?; Ok(()) } #[inline] -pub fn read_cgroup_file>(path: P) -> Result { +pub fn read_cgroup_file>(path: P) -> Result { let path = path.as_ref(); - fs::read_to_string(path).with_context(|| format!("failed to open {path:?}")) + fs::read_to_string(path).map_err(|err| WrappedIoError::Read { + err, + path: path.to_path_buf(), + }) +} + +#[derive(thiserror::Error, Debug)] +pub enum GetCgroupSetupError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("non default cgroup root not supported")] + NonDefault, + #[error("failed to detect cgroup setup")] + FailedToDetect, } /// Determines the cgroup setup of the system. Systems typically have one of @@ -135,7 +268,7 @@ pub fn read_cgroup_file>(path: P) -> Result { /// an additional unified hierarchy which doesn't have any /// controllers attached. Resource control can purely be achieved /// through the cgroup v1 hierarchy, not through the cgroup v2 hierarchy. -pub fn get_cgroup_setup() -> Result { +pub fn get_cgroup_setup() -> Result { let default_root = Path::new(DEFAULT_CGROUP_ROOT); match default_root.exists() { true => { @@ -143,12 +276,9 @@ pub fn get_cgroup_setup() -> Result { // If the filesystem is tmpfs instead the system is either in legacy or // hybrid mode. If a cgroup2 filesystem has been mounted under the "unified" // folder we are in hybrid mode, otherwise we are in legacy mode. - let stat = statfs(default_root).with_context(|| { - format!( - "failed to stat default cgroup root {}", - &default_root.display() - ) - })?; + let stat = statfs(default_root) + .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err)) + .wrap_other(default_root)?; if stat.filesystem_type() == CGROUP2_SUPER_MAGIC { return Ok(CgroupSetup::Unified); } @@ -157,7 +287,8 @@ pub fn get_cgroup_setup() -> Result { let unified = Path::new("/sys/fs/cgroup/unified"); if Path::new(unified).exists() { let stat = statfs(unified) - .with_context(|| format!("failed to stat {}", unified.display()))?; + .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err)) + .wrap_other(unified)?; if stat.filesystem_type() == CGROUP2_SUPER_MAGIC { return Ok(CgroupSetup::Hybrid); } @@ -166,36 +297,60 @@ pub fn get_cgroup_setup() -> Result { return Ok(CgroupSetup::Legacy); } } - false => bail!("non default cgroup root not supported"), + false => return Err(GetCgroupSetupError::NonDefault), } - bail!("failed to detect cgroup setup"); + Err(GetCgroupSetupError::FailedToDetect) +} + +#[derive(thiserror::Error, Debug)] +pub enum CreateCgroupSetupError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("non default cgroup root not supported")] + NonDefault, + #[error("failed to detect cgroup setup")] + FailedToDetect, + #[error("v1 error: {0}")] + V1(#[from] v1::manager::V1ManagerError), + #[error("v2 error: {0}")] + V2(#[from] v2::manager::V2ManagerError), + #[error("systemd error: {0}")] + Systemd(#[from] systemd::manager::SystemdManagerError), } pub fn create_cgroup_manager>( cgroup_path: P, systemd_cgroup: bool, container_name: &str, -) -> Result> { - let cgroup_setup = get_cgroup_setup()?; +) -> Result { + let cgroup_setup = get_cgroup_setup().map_err(|err| match err { + GetCgroupSetupError::WrappedIo(err) => CreateCgroupSetupError::WrappedIo(err), + GetCgroupSetupError::NonDefault => CreateCgroupSetupError::NonDefault, + GetCgroupSetupError::FailedToDetect => CreateCgroupSetupError::FailedToDetect, + })?; let cgroup_path = cgroup_path.into(); match cgroup_setup { - CgroupSetup::Legacy | CgroupSetup::Hybrid => create_v1_cgroup_manager(cgroup_path), + CgroupSetup::Legacy | CgroupSetup::Hybrid => { + Ok(create_v1_cgroup_manager(cgroup_path)?.any()) + } CgroupSetup::Unified => { // ref https://github.com/opencontainers/runtime-spec/blob/main/config-linux.md#cgroups-path if cgroup_path.is_absolute() || !systemd_cgroup { - return create_v2_cgroup_manager(cgroup_path); + return Ok(create_v2_cgroup_manager(cgroup_path)?.any()); } - create_systemd_cgroup_manager(cgroup_path, container_name) + Ok(create_systemd_cgroup_manager(cgroup_path, container_name)?.any()) } } } #[cfg(feature = "v1")] -fn create_v1_cgroup_manager(cgroup_path: PathBuf) -> Result> { +fn create_v1_cgroup_manager( + cgroup_path: PathBuf, +) -> Result { log::info!("cgroup manager V1 will be used"); - Ok(Box::new(v1::manager::Manager::new(cgroup_path)?)) + v1::manager::Manager::new(cgroup_path) } #[cfg(not(feature = "v1"))] @@ -204,12 +359,11 @@ fn create_v1_cgroup_manager(_cgroup_path: PathBuf) -> Result Result> { +fn create_v2_cgroup_manager( + cgroup_path: PathBuf, +) -> Result { log::info!("cgroup manager V2 will be used"); - Ok(Box::new(v2::manager::Manager::new( - DEFAULT_CGROUP_ROOT.into(), - cgroup_path, - )?)) + v2::manager::Manager::new(DEFAULT_CGROUP_ROOT.into(), cgroup_path) } #[cfg(not(feature = "v2"))] @@ -221,9 +375,9 @@ fn create_v2_cgroup_manager(_cgroup_path: PathBuf) -> Result Result> { +) -> Result { if !systemd::booted() { - bail!( + panic!( "systemd cgroup flag passed, but systemd support for managing cgroups is not available" ); } @@ -234,12 +388,12 @@ fn create_systemd_cgroup_manager( "systemd cgroup manager with system bus {} will be used", use_system ); - Ok(Box::new(systemd::manager::Manager::new( + systemd::manager::Manager::new( DEFAULT_CGROUP_ROOT.into(), cgroup_path, container_name.into(), use_system, - )?)) + ) } #[cfg(not(feature = "systemd"))] @@ -250,29 +404,34 @@ fn create_systemd_cgroup_manager( bail!("systemd cgroup feature is required, but was not enabled during compile time"); } -pub fn get_all_pids(path: &Path) -> Result> { +pub fn get_all_pids(path: &Path) -> Result, WrappedIoError> { log::debug!("scan pids in folder: {:?}", path); let mut result = vec![]; walk_dir(path, &mut |p| { let file_path = p.join(CGROUP_PROCS); if file_path.exists() { - let file = File::open(file_path)?; + let file = File::open(&file_path).wrap_open(&file_path)?; for line in BufReader::new(file).lines().flatten() { - result.push(Pid::from_raw(line.parse::()?)) + result.push(Pid::from_raw( + line.parse::() + .map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err)) + .wrap_other(&file_path)?, + )) } } - Ok(()) + Ok::<(), WrappedIoError>(()) })?; Ok(result) } -fn walk_dir(path: &Path, c: &mut F) -> Result<()> +fn walk_dir(path: &Path, c: &mut F) -> Result<(), E> where - F: FnMut(&Path) -> Result<()>, + F: FnMut(&Path) -> Result<(), E>, + E: From, { c(path)?; - for entry in fs::read_dir(path)? { - let entry = entry?; + for entry in fs::read_dir(path).wrap_read(path)? { + let entry = entry.wrap_open(path)?; let path = entry.path(); if path.is_dir() { @@ -283,11 +442,20 @@ where } pub(crate) trait PathBufExt { - fn join_safely>(&self, path: P) -> Result; + fn join_safely>(&self, path: P) -> Result; +} + +#[derive(thiserror::Error, Debug)] +pub enum JoinSafelyError { + #[error("failed to strip prefix from {path}: {err}")] + StripPrefix { + err: StripPrefixError, + path: PathBuf, + }, } impl PathBufExt for PathBuf { - fn join_safely>(&self, path: P) -> Result { + fn join_safely>(&self, path: P) -> Result { let path = path.as_ref(); if path.is_relative() { return Ok(self.join(path)); @@ -295,7 +463,10 @@ impl PathBufExt for PathBuf { let stripped = path .strip_prefix("/") - .with_context(|| format!("failed to strip prefix from {}", path.display()))?; + .map_err(|err| JoinSafelyError::StripPrefix { + err, + path: path.to_path_buf(), + })?; Ok(self.join(stripped)) } } @@ -411,7 +582,7 @@ pub(crate) fn delete_with_retry, L: Into>>( path: P, retries: u32, limit_backoff: L, -) -> Result<()> { +) -> Result<(), WrappedIoError> { let mut attempts = 0; let mut delay = Duration::from_millis(10); let path = path.as_ref(); @@ -430,5 +601,93 @@ pub(crate) fn delete_with_retry, L: Into>>( } } - bail!("could not delete {:?}", path) + Err(std::io::Error::new( + std::io::ErrorKind::TimedOut, + "could not delete".to_string(), + )) + .wrap_other(path)? +} + +pub(crate) trait WrapIoResult { + type Target; + + fn wrap_create_dir>(self, path: P) -> Result; + fn wrap_read>(self, path: P) -> Result; + fn wrap_open>(self, path: P) -> Result; + fn wrap_write, D: Into>( + self, + path: P, + data: D, + ) -> Result; + fn wrap_other>(self, path: P) -> Result; +} + +impl WrapIoResult for Result { + type Target = T; + + fn wrap_create_dir>(self, path: P) -> Result { + self.map_err(|err| WrappedIoError::CreateDir { + err, + path: path.into(), + }) + } + + fn wrap_read>(self, path: P) -> Result { + self.map_err(|err| WrappedIoError::Read { + err, + path: path.into(), + }) + } + + fn wrap_open>(self, path: P) -> Result { + self.map_err(|err| WrappedIoError::Open { + err, + path: path.into(), + }) + } + + fn wrap_write, D: Into>( + self, + path: P, + data: D, + ) -> Result { + self.map_err(|err| WrappedIoError::Write { + err, + path: path.into(), + data: data.into(), + }) + } + + fn wrap_other>(self, path: P) -> Result { + self.map_err(|err| WrappedIoError::Other { + err, + path: path.into(), + }) + } +} + +#[derive(Debug)] +pub enum EitherError { + Left(L), + Right(R), +} + +impl Display for EitherError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + EitherError::Left(left) => ::fmt(left, f), + EitherError::Right(right) => ::fmt(right, f), + } + } +} + +impl std::error::Error for EitherError {} + +#[derive(Debug)] +pub struct MustBePowerOfTwo; + +impl Display for MustBePowerOfTwo { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("page size must be in the format of 2^(integer)") + } } diff --git a/crates/libcgroups/src/stats.rs b/crates/libcgroups/src/stats.rs index 075f5314e..fc9ba53e3 100644 --- a/crates/libcgroups/src/stats.rs +++ b/crates/libcgroups/src/stats.rs @@ -1,13 +1,21 @@ -use anyhow::{bail, Context, Result}; use serde::Serialize; -use std::{collections::HashMap, fmt::Display, fs, path::Path}; +use std::{ + collections::HashMap, + fmt::Display, + fs, + num::ParseIntError, + path::{Path, PathBuf}, +}; + +use crate::common::{WrapIoResult, WrappedIoError}; use super::common; -pub trait StatsProvider { +pub(crate) trait StatsProvider { + type Error; type Stats; - fn stats(cgroup_path: &Path) -> Result; + fn stats(cgroup_path: &Path) -> Result; } /// Reports the statistics for a cgroup @@ -188,8 +196,18 @@ pub struct PSIData { pub avg300: f64, } +#[derive(thiserror::Error, Debug)] +pub enum SupportedPageSizesError { + #[error("io error: {0}")] + Io(#[from] std::io::Error), + #[error("failed to parse value {value}: {err}")] + Parse { value: String, err: ParseIntError }, + #[error("failed to determine page size from {dir_name}")] + Failed { dir_name: String }, +} + /// Reports which hugepage sizes are supported by the system -pub fn supported_page_sizes() -> Result> { +pub fn supported_page_sizes() -> Result, SupportedPageSizesError> { let mut sizes = Vec::new(); for hugetlb_entry in fs::read_dir("/sys/kernel/mm/hugepages")? { let hugetlb_entry = hugetlb_entry?; @@ -206,12 +224,15 @@ pub fn supported_page_sizes() -> Result> { Ok(sizes) } -fn extract_page_size(dir_name: &str) -> Result { +fn extract_page_size(dir_name: &str) -> Result { if let Some(size) = dir_name .strip_prefix("hugepages-") .and_then(|name_stripped| name_stripped.strip_suffix("kB")) { - let size: u64 = parse_value(size)?; + let size: u64 = size.parse().map_err(|err| SupportedPageSizesError::Parse { + value: size.into(), + err, + })?; let size_moniker = if size >= (1 << 20) { (size >> 20).to_string() + "GB" @@ -224,7 +245,9 @@ fn extract_page_size(dir_name: &str) -> Result { return Ok(size_moniker); } - bail!("failed to determine page size from {}", dir_name); + Err(SupportedPageSizesError::Failed { + dir_name: dir_name.into(), + }) } /// Parses this string slice into an u64 @@ -235,10 +258,8 @@ fn extract_page_size(dir_name: &str) -> Result { /// let value = parse_value("32").unwrap(); /// assert_eq!(value, 32); /// ``` -pub fn parse_value(value: &str) -> Result { - value - .parse() - .with_context(|| format!("failed to parse {value}")) +pub fn parse_value(value: &str) -> Result { + value.parse() } /// Parses a single valued file to an u64 @@ -250,58 +271,82 @@ pub fn parse_value(value: &str) -> Result { /// let value = parse_single_value(&Path::new("memory.current")).unwrap(); /// assert_eq!(value, 32); /// ``` -pub fn parse_single_value(file_path: &Path) -> Result { +pub fn parse_single_value(file_path: &Path) -> Result { let value = common::read_cgroup_file(file_path)?; let value = value.trim(); if value == "max" { return Ok(u64::MAX); } - value.parse().with_context(|| { - format!( - "failed to parse value {} from {}", - value, - file_path.display() - ) - }) + value + .parse() + .map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err)) + .wrap_other(file_path) +} + +#[derive(thiserror::Error, Debug)] +pub enum ParseFlatKeyedDataError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("flat keyed data at {path} contains entries that do not conform to 'key value'")] + DoesNotConform { path: PathBuf }, + #[error("failed to parse value {value} from {path}")] + FailedToParse { + value: String, + path: PathBuf, + err: ParseIntError, + }, } /// Parses a file that is structured according to the flat keyed format -pub fn parse_flat_keyed_data(file_path: &Path) -> Result> { +pub(crate) fn parse_flat_keyed_data( + file_path: &Path, +) -> Result, ParseFlatKeyedDataError> { let mut stats = HashMap::new(); let keyed_data = common::read_cgroup_file(file_path)?; for entry in keyed_data.lines() { let entry_fields: Vec<&str> = entry.split_ascii_whitespace().collect(); if entry_fields.len() != 2 { - bail!( - "flat keyed data at {} contains entries that do not conform to 'key value'", - &file_path.display() - ); + return Err(ParseFlatKeyedDataError::DoesNotConform { + path: file_path.to_path_buf(), + }); } stats.insert( entry_fields[0].to_owned(), - entry_fields[1].parse().with_context(|| { - format!( - "failed to parse value {} from {}", - entry_fields[0], - file_path.display() - ) - })?, + entry_fields[1] + .parse() + .map_err(|err| ParseFlatKeyedDataError::FailedToParse { + value: entry_fields[0].into(), + path: file_path.to_path_buf(), + err, + })?, ); } Ok(stats) } +#[derive(thiserror::Error, Debug)] +pub enum ParseNestedKeyedDataError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("nested keyed data at {path} contains entries that do not conform to key format")] + DoesNotConform { path: PathBuf }, +} + /// Parses a file that is structed according to the nested keyed format -pub fn parse_nested_keyed_data(file_path: &Path) -> Result>> { +pub fn parse_nested_keyed_data( + file_path: &Path, +) -> Result>, ParseNestedKeyedDataError> { let mut stats: HashMap> = HashMap::new(); let keyed_data = common::read_cgroup_file(file_path)?; for entry in keyed_data.lines() { let entry_fields: Vec<&str> = entry.split_ascii_whitespace().collect(); if entry_fields.len() < 2 || !entry_fields[1..].iter().all(|p| p.contains('=')) { - bail!("nested key data at {} contains entries that do not conform to the nested key format", file_path.display()); + return Err(ParseNestedKeyedDataError::DoesNotConform { + path: file_path.to_path_buf(), + }); } stats.insert( @@ -317,50 +362,76 @@ pub fn parse_nested_keyed_data(file_path: &Path) -> Result Result<(u64, u64)> { +#[derive(thiserror::Error, Debug)] +pub enum ParseDeviceNumberError { + #[error("failed to parse device number from {device}: expected 2 parts, found {numbers}")] + TooManyNumbers { device: String, numbers: usize }, + #[error("failed to parse device number from {device}: {err}")] + MalformedNumber { device: String, err: ParseIntError }, +} + +pub(crate) fn parse_device_number(device: &str) -> Result<(u64, u64), ParseDeviceNumberError> { let numbers: Vec<&str> = device.split_terminator(':').collect(); if numbers.len() != 2 { - bail!("failed to parse device number {}", device); + return Err(ParseDeviceNumberError::TooManyNumbers { + device: device.into(), + numbers: numbers.len(), + }); } - Ok((numbers[0].parse()?, numbers[1].parse()?)) + Ok(( + numbers[0] + .parse() + .map_err(|err| ParseDeviceNumberError::MalformedNumber { + device: device.into(), + err, + })?, + numbers[1] + .parse() + .map_err(|err| ParseDeviceNumberError::MalformedNumber { + device: device.into(), + err, + })?, + )) +} + +#[derive(thiserror::Error, Debug)] +pub enum PidStatsError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("failed to parse current pids: {0}")] + ParseCurrent(ParseIntError), + #[error("failed to parse pids limit: {0}")] + ParseLimit(ParseIntError), } /// Returns cgroup pid statistics -pub fn pid_stats(cgroup_path: &Path) -> Result { +pub fn pid_stats(cgroup_path: &Path) -> Result { let mut stats = PidStats::default(); let current = common::read_cgroup_file(cgroup_path.join("pids.current"))?; stats.current = current .trim() .parse() - .context("failed to parse current pids")?; + .map_err(PidStatsError::ParseCurrent)?; let limit = common::read_cgroup_file(cgroup_path.join("pids.max")).map(|l| l.trim().to_owned())?; if limit != "max" { - stats.limit = limit.parse().context("failed to parse pids limit")?; + stats.limit = limit.parse().map_err(PidStatsError::ParseLimit)?; } Ok(stats) } -pub fn psi_stats(psi_file: &Path) -> Result { +pub fn psi_stats(psi_file: &Path) -> Result { let mut stats = PSIStats::default(); let psi = common::read_cgroup_file(psi_file)?; for line in psi.lines() { match &line[0..4] { - "some" => stats.some = parse_psi(&line[4..])?, - "full" => stats.full = parse_psi(&line[4..])?, + "some" => stats.some = parse_psi(&line[4..], psi_file)?, + "full" => stats.full = parse_psi(&line[4..], psi_file)?, _ => continue, } } @@ -368,7 +439,9 @@ pub fn psi_stats(psi_file: &Path) -> Result { Ok(stats) } -fn parse_psi(stat_line: &str) -> Result { +fn parse_psi(stat_line: &str, path: &Path) -> Result { + use std::io::{Error, ErrorKind}; + let mut psi_data = PSIData::default(); for kv in stat_line.split_ascii_whitespace() { @@ -376,17 +449,20 @@ fn parse_psi(stat_line: &str) -> Result { Some(("avg10", v)) => { psi_data.avg10 = v .parse() - .with_context(|| format!("invalid psi value {v}"))? + .map_err(|err| Error::new(ErrorKind::InvalidData, err)) + .wrap_other(path)? } Some(("avg60", v)) => { psi_data.avg60 = v .parse() - .with_context(|| format!("invalid psi value {v}"))? + .map_err(|err| Error::new(ErrorKind::InvalidData, err)) + .wrap_other(path)? } Some(("avg300", v)) => { psi_data.avg300 = v .parse() - .with_context(|| format!("invalid psi value {v}"))? + .map_err(|err| Error::new(ErrorKind::InvalidData, err)) + .wrap_other(path)? } _ => continue, } diff --git a/crates/libcgroups/src/systemd/controller.rs b/crates/libcgroups/src/systemd/controller.rs index 5e2075a63..2e93003c5 100644 --- a/crates/libcgroups/src/systemd/controller.rs +++ b/crates/libcgroups/src/systemd/controller.rs @@ -1,14 +1,15 @@ use std::collections::HashMap; -use anyhow::Result; use dbus::arg::RefArg; use crate::common::ControllerOpt; -pub(crate) trait Controller { +pub(super) trait Controller { + type Error; + fn apply( options: &ControllerOpt, systemd_version: u32, properties: &mut HashMap<&str, Box>, - ) -> Result<()>; + ) -> Result<(), Self::Error>; } diff --git a/crates/libcgroups/src/systemd/cpu.rs b/crates/libcgroups/src/systemd/cpu.rs index 2bf3d6e02..870aca535 100644 --- a/crates/libcgroups/src/systemd/cpu.rs +++ b/crates/libcgroups/src/systemd/cpu.rs @@ -1,6 +1,5 @@ use std::collections::HashMap; -use anyhow::{bail, Context, Result}; use dbus::arg::RefArg; use oci_spec::runtime::LinuxCpu; @@ -12,18 +11,25 @@ pub const CPU_QUOTA: &str = "CPUQuotaPerSecUSec"; pub const CPU_PERIOD: &str = "CPUQuotaPeriodUSec"; const MICROSECS_PER_SEC: u64 = 1_000_000; +#[derive(thiserror::Error, Debug)] +pub enum SystemdCpuError { + #[error("realtime is not supported on systemd v2 yet")] + RealtimeSystemd, +} + pub(crate) struct Cpu {} impl Controller for Cpu { + type Error = SystemdCpuError; + fn apply( options: &ControllerOpt, _: u32, properties: &mut HashMap<&str, Box>, - ) -> Result<()> { + ) -> Result<(), Self::Error> { if let Some(cpu) = options.resources.cpu() { log::debug!("Applying cpu resource restrictions"); - return Self::apply(cpu, properties) - .context("could not apply cpu resource restrictions"); + Self::apply(cpu, properties)?; } Ok(()) @@ -31,9 +37,12 @@ impl Controller for Cpu { } impl Cpu { - fn apply(cpu: &LinuxCpu, properties: &mut HashMap<&str, Box>) -> Result<()> { + fn apply( + cpu: &LinuxCpu, + properties: &mut HashMap<&str, Box>, + ) -> Result<(), SystemdCpuError> { if Self::is_realtime_requested(cpu) { - bail!("realtime is not supported on systemd v2 yet"); + return Err(SystemdCpuError::RealtimeSystemd); } if let Some(mut shares) = cpu.shares() { @@ -82,6 +91,7 @@ pub fn convert_shares_to_cgroup2(shares: u64) -> u64 { #[cfg(test)] mod tests { + use anyhow::{Context, Result}; use dbus::arg::ArgType; use oci_spec::runtime::LinuxCpuBuilder; @@ -97,7 +107,7 @@ mod tests { let mut properties: HashMap<&str, Box> = HashMap::new(); // act - Cpu::apply(&cpu, &mut properties).context("apply cpu")?; + Cpu::apply(&cpu, &mut properties)?; // assert assert!(properties.contains_key(CPU_WEIGHT)); @@ -119,7 +129,7 @@ mod tests { let mut properties: HashMap<&str, Box> = HashMap::new(); // act - Cpu::apply(&cpu, &mut properties).context("apply cpu")?; + Cpu::apply(&cpu, &mut properties)?; // assert assert!(properties.contains_key(CPU_QUOTA)); @@ -143,7 +153,7 @@ mod tests { let mut properties: HashMap<&str, Box> = HashMap::new(); // act - Cpu::apply(&cpu, &mut properties).context("apply cpu")?; + Cpu::apply(&cpu, &mut properties)?; // assert assert!(properties.contains_key(CPU_PERIOD)); diff --git a/crates/libcgroups/src/systemd/cpuset.rs b/crates/libcgroups/src/systemd/cpuset.rs index e85fb7d6d..ba36f6014 100644 --- a/crates/libcgroups/src/systemd/cpuset.rs +++ b/crates/libcgroups/src/systemd/cpuset.rs @@ -1,6 +1,5 @@ use std::collections::HashMap; -use anyhow::{bail, Context, Result}; use dbus::arg::RefArg; use fixedbitset::FixedBitSet; use oci_spec::runtime::LinuxCpu; @@ -12,18 +11,29 @@ use super::controller::Controller; pub const ALLOWED_CPUS: &str = "AllowedCPUs"; pub const ALLOWED_NODES: &str = "AllowedMemoryNodes"; +#[derive(thiserror::Error, Debug)] +pub enum SystemdCpuSetError { + #[error("setting cpuset restrictions requires systemd version greather than 243")] + OldSystemd, + #[error("could not create bitmask for cpus: {0}")] + CpusBitmask(BitmaskError), + #[error("could not create bitmask for memory nodes: {0}")] + MemoryNodesBitmask(BitmaskError), +} + pub struct CpuSet {} impl Controller for CpuSet { + type Error = SystemdCpuSetError; + fn apply( options: &ControllerOpt, systemd_version: u32, properties: &mut HashMap<&str, Box>, - ) -> Result<()> { + ) -> Result<(), Self::Error> { if let Some(cpu) = options.resources.cpu() { log::debug!("Applying cpuset resource restrictions"); - return Self::apply(cpu, systemd_version, properties) - .context("could not apply cpuset resource restrictions"); + return Self::apply(cpu, systemd_version, properties); } Ok(()) @@ -35,19 +45,18 @@ impl CpuSet { cpu: &LinuxCpu, systemd_version: u32, properties: &mut HashMap<&str, Box>, - ) -> Result<()> { + ) -> Result<(), SystemdCpuSetError> { if systemd_version <= 243 { - bail!("setting cpuset restrictions requires systemd version greather than 243"); + return Err(SystemdCpuSetError::OldSystemd); } if let Some(cpus) = cpu.cpus() { - let cpu_mask = to_bitmask(cpus).context("could not create bitmask for cpus")?; + let cpu_mask = to_bitmask(cpus).map_err(SystemdCpuSetError::CpusBitmask)?; properties.insert(ALLOWED_CPUS, Box::new(cpu_mask)); } if let Some(mems) = cpu.mems() { - let mems_mask = - to_bitmask(mems).context("could not create bitmask for memory nodes")?; + let mems_mask = to_bitmask(mems).map_err(SystemdCpuSetError::MemoryNodesBitmask)?; properties.insert(ALLOWED_NODES, Box::new(mems_mask)); } @@ -55,7 +64,18 @@ impl CpuSet { } } -pub fn to_bitmask(range: &str) -> Result> { +#[derive(thiserror::Error, Debug)] +pub enum BitmaskError { + #[error("invalid index {index}: {err}")] + InvalidIndex { + err: std::num::ParseIntError, + index: String, + }, + #[error("invalid cpu range {0}")] + InvalidRange(String), +} + +pub fn to_bitmask(range: &str) -> Result, BitmaskError> { let mut bitset = FixedBitSet::with_capacity(8); for cpu_set in range.split_terminator(',') { @@ -66,16 +86,25 @@ pub fn to_bitmask(range: &str) -> Result> { let cpus: Vec<&str> = cpu_set.split('-').map(|s| s.trim()).collect(); if cpus.len() == 1 { - let cpu_index: usize = cpus[0].parse()?; + let cpu_index: usize = cpus[0].parse().map_err(|err| BitmaskError::InvalidIndex { + err, + index: cpus[0].into(), + })?; if cpu_index >= bitset.len() { bitset.grow(bitset.len() + 8); } bitset.set(cpu_index, true); } else { - let start_index = cpus[0].parse()?; - let end_index = cpus[1].parse()?; + let start_index = cpus[0].parse().map_err(|err| BitmaskError::InvalidIndex { + err, + index: cpus[0].into(), + })?; + let end_index = cpus[1].parse().map_err(|err| BitmaskError::InvalidIndex { + err, + index: cpus[1].into(), + })?; if start_index > end_index { - bail!("invalid cpu range {}", cpu_set); + return Err(BitmaskError::InvalidRange(cpu_set.into())); } if end_index >= bitset.len() { @@ -98,6 +127,7 @@ pub fn to_bitmask(range: &str) -> Result> { #[cfg(test)] mod tests { + use anyhow::{Context, Result}; use dbus::arg::{ArgType, RefArg}; use oci_spec::runtime::LinuxCpuBuilder; diff --git a/crates/libcgroups/src/systemd/dbus/client.rs b/crates/libcgroups/src/systemd/dbus/client.rs index 2de3606b8..13bc4723f 100644 --- a/crates/libcgroups/src/systemd/dbus/client.rs +++ b/crates/libcgroups/src/systemd/dbus/client.rs @@ -1,11 +1,29 @@ use crate::systemd::dbus::systemd_api::OrgFreedesktopSystemd1Manager; -use anyhow::{Context, Result}; use dbus::arg::{RefArg, Variant}; use dbus::blocking::{Connection, Proxy}; use std::collections::HashMap; +use std::num::ParseIntError; use std::path::PathBuf; use std::time::Duration; +#[derive(thiserror::Error, Debug)] +pub enum SystemdClientError { + #[error("dbus error: {0}")] + DBus(#[from] dbus::Error), + #[error("failed to start transient unit {unit_name}, parent is {parent}: {err}")] + FailedTransient { + err: dbus::Error, + unit_name: String, + parent: String, + }, + #[error("failed to stop unit {unit_name}: {err}")] + FailedStop { err: dbus::Error, unit_name: String }, + #[error("failed to set properties for unit {unit_name}: {err}")] + FailedProperties { err: dbus::Error, unit_name: String }, + #[error("could not parse systemd version: {0}")] + SystemdVersion(ParseIntError), +} + pub trait SystemdClient { fn is_system(&self) -> bool; @@ -17,19 +35,19 @@ pub trait SystemdClient { pid: u32, parent: &str, unit_name: &str, - ) -> Result<()>; + ) -> Result<(), SystemdClientError>; - fn stop_transient_unit(&self, unit_name: &str) -> Result<()>; + fn stop_transient_unit(&self, unit_name: &str) -> Result<(), SystemdClientError>; fn set_unit_properties( &self, unit_name: &str, properties: &HashMap<&str, Box>, - ) -> Result<()>; + ) -> Result<(), SystemdClientError>; - fn systemd_version(&self) -> Result; + fn systemd_version(&self) -> Result; - fn control_cgroup_root(&self) -> Result; + fn control_cgroup_root(&self) -> Result; } /// Client is a wrapper providing higher level API and abatraction around dbus. @@ -41,13 +59,13 @@ pub struct Client { impl Client { /// Uses the system bus to communicate with systemd - pub fn new_system() -> Result { + pub fn new_system() -> Result { let conn = Connection::new_system()?; Ok(Client { conn, system: true }) } /// Uses the session bus to communicate with systemd - pub fn new_session() -> Result { + pub fn new_session() -> Result { let conn = Connection::new_session()?; Ok(Client { conn, @@ -83,7 +101,7 @@ impl SystemdClient for Client { pid: u32, parent: &str, unit_name: &str, - ) -> Result<()> { + ) -> Result<(), SystemdClientError> { // To view and introspect the methods under the 'org.freedesktop.systemd1' destination // and object path under it use the following command: // `gdbus introspect --system --dest org.freedesktop.systemd1 --object-path /org/freedesktop/systemd1` @@ -122,18 +140,23 @@ impl SystemdClient for Client { log::debug!("Starting transient unit: {:?}", properties); proxy .start_transient_unit(unit_name, "replace", properties, vec![]) - .with_context(|| { - format!("failed to start transient unit {unit_name}, parent is {parent}") + .map_err(|err| SystemdClientError::FailedTransient { + err, + unit_name: unit_name.into(), + parent: parent.into(), })?; Ok(()) } - fn stop_transient_unit(&self, unit_name: &str) -> Result<()> { + fn stop_transient_unit(&self, unit_name: &str) -> Result<(), SystemdClientError> { let proxy = self.create_proxy(); proxy .stop_unit(unit_name, "replace") - .with_context(|| format!("failed to stop unit {unit_name}"))?; + .map_err(|err| SystemdClientError::FailedStop { + err, + unit_name: unit_name.into(), + })?; Ok(()) } @@ -141,7 +164,7 @@ impl SystemdClient for Client { &self, unit_name: &str, properties: &HashMap<&str, Box>, - ) -> Result<()> { + ) -> Result<(), SystemdClientError> { let proxy = self.create_proxy(); let props = properties @@ -151,33 +174,32 @@ impl SystemdClient for Client { proxy .set_unit_properties(unit_name, true, props) - .with_context(|| format!("failed to set properties for unit {unit_name:?}"))?; + .map_err(|err| SystemdClientError::FailedProperties { + err, + unit_name: unit_name.into(), + })?; Ok(()) } - fn systemd_version(&self) -> Result { + fn systemd_version(&self) -> Result { let proxy = self.create_proxy(); let version = proxy - .version() - .context("dbus request failed")? + .version()? .chars() .skip_while(|c| c.is_alphabetic()) .take_while(|c| c.is_numeric()) .collect::() .parse::() - .context("could not parse systemd version")?; + .map_err(SystemdClientError::SystemdVersion)?; Ok(version) } - fn control_cgroup_root(&self) -> Result { + fn control_cgroup_root(&self) -> Result { let proxy = self.create_proxy(); - let cgroup_root = proxy - .control_group() - .context("failed to get systemd control group")?; - PathBuf::try_from(&cgroup_root) - .with_context(|| format!("parse systemd control cgroup {cgroup_root} into path")) + let cgroup_root = proxy.control_group()?; + Ok(PathBuf::from(&cgroup_root)) } } diff --git a/crates/libcgroups/src/systemd/manager.rs b/crates/libcgroups/src/systemd/manager.rs index 1bd47c720..b2d082dd3 100644 --- a/crates/libcgroups/src/systemd/manager.rs +++ b/crates/libcgroups/src/systemd/manager.rs @@ -1,11 +1,11 @@ use std::{ collections::HashMap, + convert::Infallible, fmt::{Debug, Display}, fs::{self}, path::Component::RootDir, }; -use anyhow::{anyhow, bail, Context, Result}; use dbus::arg::RefArg; use nix::{unistd::Pid, NixPath}; use std::path::{Path, PathBuf}; @@ -15,13 +15,17 @@ use super::{ controller_type::{ControllerType, CONTROLLER_TYPES}, cpu::Cpu, cpuset::CpuSet, - dbus::client::{Client, SystemdClient}, + dbus::client::{Client, SystemdClient, SystemdClientError}, memory::Memory, pids::Pids, }; use crate::{ - common::{self, CgroupManager, ControllerOpt, FreezerState, PathBufExt}, + common::{ + self, AnyCgroupManager, CgroupManager, ControllerOpt, FreezerState, JoinSafelyError, + PathBufExt, WrapIoResult, WrappedIoError, + }, systemd::unified::Unified, + v2::manager::V2ManagerError, }; use crate::{stats::Stats, v2::manager::Manager as FsManager}; @@ -61,19 +65,29 @@ struct CgroupsPath { name: String, } +#[derive(thiserror::Error, Debug)] +pub enum CgroupsPathError { + #[error("no cgroups path has been provided")] + NoPath, + #[error("cgroups path does not contain valid utf8")] + InvalidUtf8(PathBuf), + #[error("cgroups path is malformed: {0}")] + MalformedPath(PathBuf), +} + impl TryFrom<&Path> for CgroupsPath { - type Error = anyhow::Error; + type Error = CgroupsPathError; fn try_from(cgroups_path: &Path) -> Result { // if cgroups_path was provided it should be of the form [slice]:[prefix]:[name], // for example: "system.slice:docker:1234". if cgroups_path.len() == 0 { - bail!("no cgroups path has been provided"); + return Err(CgroupsPathError::NoPath); } let parts = cgroups_path .to_str() - .ok_or_else(|| anyhow!("failed to parse cgroups path {:?}", cgroups_path))? + .ok_or_else(|| CgroupsPathError::InvalidUtf8(cgroups_path.to_path_buf()))? .split(':') .collect::>(); @@ -88,7 +102,7 @@ impl TryFrom<&Path> for CgroupsPath { prefix: parts[1].to_owned(), name: parts[2].to_owned(), }, - _ => bail!("cgroup path {:?} is invalid", cgroups_path), + _ => return Err(CgroupsPathError::MalformedPath(cgroups_path.to_path_buf())), }; Ok(destructured_path) @@ -126,27 +140,56 @@ impl Debug for Manager { } } +#[derive(thiserror::Error, Debug)] +pub enum SystemdManagerError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("failed to destructure cgroups path: {0}")] + CgroupsPath(#[from] CgroupsPathError), + #[error("dbus error: {0}")] + DBus(#[from] dbus::Error), + #[error("invalid slice name: {0}")] + InvalidSliceName(String), + #[error(transparent)] + SystemdClient(#[from] SystemdClientError), + #[error("failed to join safely: {0}")] + JoinSafely(#[from] JoinSafelyError), + #[error("file not found: {0}")] + FileNotFound(PathBuf), + #[error("bad delegation boundary {boundary} for cgroups path {cgroup}")] + BadDelegationBoundary { boundary: PathBuf, cgroup: PathBuf }, + #[error("in v2 manager: {0}")] + V2Manager(#[from] V2ManagerError), + + #[error("in cpu controller: {0}")] + Cpu(#[from] super::cpu::SystemdCpuError), + #[error("in cpuset controller: {0}")] + CpuSet(#[from] super::cpuset::SystemdCpuSetError), + #[error("in memory controller: {0}")] + Memory(#[from] super::memory::SystemdMemoryError), + #[error("in pids controller: {0}")] + Pids(Infallible), + #[error("in pids unified controller: {0}")] + Unified(#[from] super::unified::SystemdUnifiedError), +} + impl Manager { pub fn new( root_path: PathBuf, cgroups_path: PathBuf, container_name: String, use_system: bool, - ) -> Result { - let mut destructured_path = cgroups_path - .as_path() - .try_into() - .with_context(|| format!("failed to destructure cgroups path {cgroups_path:?}"))?; + ) -> Result { + let mut destructured_path = cgroups_path.as_path().try_into()?; ensure_parent_unit(&mut destructured_path, use_system); let client = match use_system { - true => Client::new_system().context("failed to create system dbus client")?, - false => Client::new_session().context("failed to create session dbus client")?, + true => Client::new_system()?, + false => Client::new_session()?, }; let (cgroups_path, delegation_boundary) = - Self::construct_cgroups_path(&destructured_path, &client) - .context("failed to construct cgroups path")?; + Self::construct_cgroups_path(&destructured_path, &client)?; let full_path = root_path.join_safely(&cgroups_path)?; let fs_manager = FsManager::new(root_path.clone(), cgroups_path.clone())?; @@ -179,7 +222,7 @@ impl Manager { fn construct_cgroups_path( cgroups_path: &CgroupsPath, client: &dyn SystemdClient, - ) -> Result<(PathBuf, PathBuf)> { + ) -> Result<(PathBuf, PathBuf), SystemdManagerError> { // if the user provided a '.slice' (as in a branch of a tree) // we need to convert it to a filesystem path. @@ -187,24 +230,20 @@ impl Manager { let systemd_root = client.control_cgroup_root()?; let unit_name = Self::get_unit_name(cgroups_path); - let cgroups_path = systemd_root - .join_safely(&parent) - .with_context(|| format!("failed to join {systemd_root:?} with {parent:?}"))? - .join_safely(&unit_name) - .with_context(|| format!("failed to join {parent:?} with {unit_name:?}"))?; + let cgroups_path = systemd_root.join_safely(parent)?.join_safely(unit_name)?; Ok((cgroups_path, systemd_root)) } // systemd represents slice hierarchy using `-`, so we need to follow suit when // generating the path of slice. For example, 'test-a-b.slice' becomes // '/test.slice/test-a.slice/test-a-b.slice'. - fn expand_slice(slice: &str) -> Result { + fn expand_slice(slice: &str) -> Result { let suffix = ".slice"; if slice.len() <= suffix.len() || !slice.ends_with(suffix) { - bail!("invalid slice name: {}", slice); + return Err(SystemdManagerError::InvalidSliceName(slice.into())); } if slice.contains('/') { - bail!("invalid slice name: {}", slice); + return Err(SystemdManagerError::InvalidSliceName(slice.into())); } let mut path = "".to_owned(); let mut prefix = "".to_owned(); @@ -215,7 +254,7 @@ impl Manager { } for component in slice_name.split('-') { if component.is_empty() { - bail!("invalid slice name: {}", slice); + return Err(SystemdManagerError::InvalidSliceName(slice.into())); } // Append the component to the path and to the prefix. path = format!("{path}/{prefix}{component}{suffix}"); @@ -226,7 +265,7 @@ impl Manager { /// ensures that each level in the downward path from the delegation boundary down to /// the scope or slice of the transient unit has all available controllers enabled - fn ensure_controllers_attached(&self) -> Result<()> { + fn ensure_controllers_attached(&self) -> Result<(), SystemdManagerError> { let full_boundary_path = self.root_path.join_safely(&self.delegation_boundary)?; let controllers: Vec = self @@ -240,7 +279,11 @@ impl Manager { let mut current_path = full_boundary_path; let mut components = self .cgroups_path - .strip_prefix(&self.delegation_boundary)? + .strip_prefix(&self.delegation_boundary) + .map_err(|_| SystemdManagerError::BadDelegationBoundary { + boundary: self.delegation_boundary.clone(), + cgroup: self.cgroups_path.clone(), + })? .components() .filter(|c| c.ne(&RootDir)) .peekable(); @@ -270,17 +313,17 @@ impl Manager { fn get_available_controllers>( &self, cgroups_path: P, - ) -> Result> { + ) -> Result, SystemdManagerError> { let controllers_path = self.root_path.join(cgroups_path).join(CGROUP_CONTROLLERS); if !controllers_path.exists() { - bail!( - "cannot get available controllers. {:?} does not exist", - controllers_path - ) + return Err(SystemdManagerError::FileNotFound(controllers_path)); } let mut controllers = Vec::new(); - for controller in fs::read_to_string(controllers_path)?.split_whitespace() { + for controller in fs::read_to_string(&controllers_path) + .wrap_read(controllers_path)? + .split_whitespace() + { match controller { "cpu" => controllers.push(ControllerType::Cpu), "memory" => controllers.push(ControllerType::Memory), @@ -292,62 +335,59 @@ impl Manager { Ok(controllers) } - fn write_controllers(path: &Path, controllers: &[String]) -> Result<()> { + fn write_controllers(path: &Path, controllers: &[String]) -> Result<(), SystemdManagerError> { for controller in controllers { common::write_cgroup_file_str(path.join(CGROUP_SUBTREE_CONTROL), controller)?; } Ok(()) } + + pub fn any(self) -> AnyCgroupManager { + AnyCgroupManager::Systemd(self) + } } impl CgroupManager for Manager { - fn add_task(&self, pid: Pid) -> Result<()> { + type Error = SystemdManagerError; + + fn add_task(&self, pid: Pid) -> Result<(), Self::Error> { // Dont attach any pid to the cgroup if -1 is specified as a pid if pid.as_raw() == -1 { return Ok(()); } log::debug!("Starting {:?}", self.unit_name); - self.client - .start_transient_unit( - &self.container_name, - pid.as_raw() as u32, - &self.destructured_path.parent, - &self.unit_name, - ) - .with_context(|| { - format!( - "failed to create unit {} for container {}", - self.unit_name, self.container_name - ) - })?; + self.client.start_transient_unit( + &self.container_name, + pid.as_raw() as u32, + &self.destructured_path.parent, + &self.unit_name, + )?; Ok(()) } - fn apply(&self, controller_opt: &ControllerOpt) -> Result<()> { + fn apply(&self, controller_opt: &ControllerOpt) -> Result<(), Self::Error> { let mut properties: HashMap<&str, Box> = HashMap::new(); - let systemd_version = self - .client - .systemd_version() - .context("could not retrieve systemd version")?; + let systemd_version = self.client.systemd_version()?; for controller in CONTROLLER_TYPES { match controller { ControllerType::Cpu => { - Cpu::apply(controller_opt, systemd_version, &mut properties)? + Cpu::apply(controller_opt, systemd_version, &mut properties)?; } ControllerType::CpuSet => { - CpuSet::apply(controller_opt, systemd_version, &mut properties)? + CpuSet::apply(controller_opt, systemd_version, &mut properties)?; } ControllerType::Pids => { - Pids::apply(controller_opt, systemd_version, &mut properties)? + Pids::apply(controller_opt, systemd_version, &mut properties) + .map_err(SystemdManagerError::Pids)?; } ControllerType::Memory => { - Memory::apply(controller_opt, systemd_version, &mut properties)? + Memory::apply(controller_opt, systemd_version, &mut properties)?; } _ => {} }; @@ -357,45 +397,41 @@ impl CgroupManager for Manager { log::debug!("{:?}", properties); if !properties.is_empty() { - self.ensure_controllers_attached() - .context("failed to attach controllers")?; + self.ensure_controllers_attached()?; self.client - .set_unit_properties(&self.unit_name, &properties) - .context("could not apply resource restrictions")?; + .set_unit_properties(&self.unit_name, &properties)?; } Ok(()) } - fn remove(&self) -> Result<()> { + fn remove(&self) -> Result<(), Self::Error> { log::debug!("remove {}", self.unit_name); if self.client.transient_unit_exists(&self.unit_name) { - self.client - .stop_transient_unit(&self.unit_name) - .with_context(|| { - format!("could not remove control group {}", self.destructured_path) - })?; + self.client.stop_transient_unit(&self.unit_name)?; } Ok(()) } - fn freeze(&self, state: FreezerState) -> Result<()> { - self.fs_manager.freeze(state) + fn freeze(&self, state: FreezerState) -> Result<(), Self::Error> { + Ok(self.fs_manager.freeze(state)?) } - fn stats(&self) -> Result { - self.fs_manager.stats() + fn stats(&self) -> Result { + Ok(self.fs_manager.stats()?) } - fn get_all_pids(&self) -> Result> { - common::get_all_pids(&self.full_path) + fn get_all_pids(&self) -> Result, Self::Error> { + Ok(common::get_all_pids(&self.full_path)?) } } #[cfg(test)] mod tests { + use anyhow::{Context, Result}; + use crate::systemd::dbus::client::SystemdClient; use super::*; @@ -417,11 +453,11 @@ mod tests { _pid: u32, _parent: &str, _unit_name: &str, - ) -> Result<()> { + ) -> Result<(), SystemdClientError> { Ok(()) } - fn stop_transient_unit(&self, _unit_name: &str) -> Result<()> { + fn stop_transient_unit(&self, _unit_name: &str) -> Result<(), SystemdClientError> { Ok(()) } @@ -429,15 +465,15 @@ mod tests { &self, _unit_name: &str, _properties: &HashMap<&str, Box>, - ) -> Result<()> { + ) -> Result<(), SystemdClientError> { Ok(()) } - fn systemd_version(&self) -> Result { + fn systemd_version(&self) -> Result { Ok(245) } - fn control_cgroup_root(&self) -> Result { + fn control_cgroup_root(&self) -> Result { Ok(PathBuf::from("/")) } } diff --git a/crates/libcgroups/src/systemd/memory.rs b/crates/libcgroups/src/systemd/memory.rs index ed8e7f1f3..b9290b517 100644 --- a/crates/libcgroups/src/systemd/memory.rs +++ b/crates/libcgroups/src/systemd/memory.rs @@ -1,6 +1,5 @@ use std::collections::HashMap; -use anyhow::{bail, Context, Result}; use dbus::arg::RefArg; use oci_spec::runtime::LinuxMemory; @@ -14,18 +13,29 @@ pub const MEMORY_HIGH: &str = "MemoryHigh"; pub const MEMORY_MAX: &str = "MemoryMax"; pub const MEMORY_SWAP: &str = "MemorySwapMax"; +#[derive(thiserror::Error, Debug)] +pub enum SystemdMemoryError { + #[error("invalid memory reservation value: {0}")] + ReservationValue(i64), + #[error("invalid memory limit value: {0}")] + MemoryLimit(i64), + #[error("cgroup v2 swap value cannot be calculated from swap of {swap} and limit of {limit}")] + SwapValue { swap: i64, limit: String }, +} + pub struct Memory {} impl Controller for Memory { + type Error = SystemdMemoryError; + fn apply( options: &ControllerOpt, _: u32, properties: &mut HashMap<&str, Box>, - ) -> Result<()> { + ) -> Result<(), Self::Error> { if let Some(memory) = options.resources.memory() { log::debug!("applying memory resource restrictions"); - return Self::apply(memory, properties) - .context("could not apply memory resource restrictions"); + return Self::apply(memory, properties); } Ok(()) @@ -33,7 +43,10 @@ impl Controller for Memory { } impl Memory { - fn apply(memory: &LinuxMemory, properties: &mut HashMap<&str, Box>) -> Result<()> { + fn apply( + memory: &LinuxMemory, + properties: &mut HashMap<&str, Box>, + ) -> Result<(), SystemdMemoryError> { if let Some(reservation) = memory.reservation() { match reservation { 1..=i64::MAX => { @@ -42,7 +55,7 @@ impl Memory { -1 => { properties.insert(MEMORY_LOW, Box::new(u64::MAX)); } - _ => bail!("invalid memory reservation value: {}", reservation), + _ => return Err(SystemdMemoryError::ReservationValue(reservation)), } } @@ -54,12 +67,11 @@ impl Memory { -1 => { properties.insert(MEMORY_MAX, Box::new(u64::MAX)); } - _ => bail!("invalid memory limit value: {}", limit), + _ => return Err(SystemdMemoryError::MemoryLimit(limit)), } } - Self::apply_swap(memory.swap(), memory.limit(), properties) - .context("could not apply swap")?; + Self::apply_swap(memory.swap(), memory.limit(), properties)?; Ok(()) } @@ -72,7 +84,7 @@ impl Memory { swap: Option, limit: Option, properties: &mut HashMap<&str, Box>, - ) -> Result<()> { + ) -> Result<(), SystemdMemoryError> { let value: Box = match (limit, swap) { // memory is unlimited and swap not specified -> assume swap unlimited (Some(-1), None) => Box::new(u64::MAX), @@ -84,11 +96,13 @@ impl Memory { // if swap is greater than zero and memory limit is unspecified swap cannot be // calculated. If memory limit is zero the container would have only swap. If // memory is unlimited it would be bigger than swap. - (_, Some(0)) | (None | Some(0) | Some(-1), Some(1..=i64::MAX)) => bail!( - "cgroup v2 swap value cannot be calculated from swap of {} and limit of {}", - swap.unwrap(), - limit.map_or("none".to_owned(), |v| v.to_string()) - ), + (_, Some(0)) | (None | Some(0) | Some(-1), Some(1..=i64::MAX)) => { + return Err(SystemdMemoryError::SwapValue { + swap: swap.unwrap(), + limit: limit.map_or("none".to_owned(), |v| v.to_string()), + }) + } + (Some(l), Some(s)) if l < s => Box::new((s - l) as u64), _ => return Ok(()), }; @@ -100,6 +114,7 @@ impl Memory { #[cfg(test)] mod tests { + use anyhow::{Context, Result}; use dbus::arg::ArgType; use oci_spec::runtime::LinuxMemoryBuilder; diff --git a/crates/libcgroups/src/systemd/pids.rs b/crates/libcgroups/src/systemd/pids.rs index e29cf2a23..c91402034 100644 --- a/crates/libcgroups/src/systemd/pids.rs +++ b/crates/libcgroups/src/systemd/pids.rs @@ -1,6 +1,5 @@ -use std::collections::HashMap; +use std::{collections::HashMap, convert::Infallible}; -use anyhow::{Context, Result}; use dbus::arg::RefArg; use oci_spec::runtime::LinuxPids; @@ -13,14 +12,16 @@ pub const TASKS_MAX: &str = "TasksMax"; pub struct Pids {} impl Controller for Pids { + type Error = Infallible; + fn apply( options: &ControllerOpt, _: u32, properties: &mut HashMap<&str, Box>, - ) -> Result<()> { + ) -> Result<(), Self::Error> { if let Some(pids) = options.resources.pids() { log::debug!("Applying pids resource restrictions"); - return Self::apply(pids, properties).context(""); + Self::apply(pids, properties); } Ok(()) @@ -28,7 +29,7 @@ impl Controller for Pids { } impl Pids { - fn apply(pids: &LinuxPids, properties: &mut HashMap<&str, Box>) -> Result<()> { + fn apply(pids: &LinuxPids, properties: &mut HashMap<&str, Box>) { let limit = if pids.limit() > 0 { pids.limit() as u64 } else { @@ -36,13 +37,13 @@ impl Pids { }; properties.insert(TASKS_MAX, Box::new(limit)); - Ok(()) } } #[cfg(test)] mod tests { use super::*; + use anyhow::{anyhow, Context, Result}; use dbus::arg::ArgType; use oci_spec::runtime::{LinuxPidsBuilder, LinuxResources, LinuxResourcesBuilder}; @@ -65,7 +66,9 @@ mod tests { .build()?; let (options, mut properties) = setup(&resources); - ::apply(&options, 245, &mut properties).context("apply pids")?; + ::apply(&options, 245, &mut properties) + .map_err(|err| anyhow!(err)) + .context("apply pids")?; assert_eq!(properties.len(), 1); assert!(properties.contains_key(TASKS_MAX)); @@ -84,7 +87,9 @@ mod tests { .build()?; let (options, mut properties) = setup(&resources); - ::apply(&options, 245, &mut properties).context("apply pids")?; + ::apply(&options, 245, &mut properties) + .map_err(|err| anyhow!(err)) + .context("apply pids")?; assert_eq!(properties.len(), 1); assert!(properties.contains_key(TASKS_MAX)); @@ -103,7 +108,9 @@ mod tests { .build()?; let (options, mut properties) = setup(&resources); - ::apply(&options, 245, &mut properties).context("apply pids")?; + ::apply(&options, 245, &mut properties) + .map_err(|err| anyhow!(err)) + .context("apply pids")?; assert_eq!(properties.len(), 1); assert!(properties.contains_key(TASKS_MAX)); diff --git a/crates/libcgroups/src/systemd/unified.rs b/crates/libcgroups/src/systemd/unified.rs index 870ff61bf..c65f2f3ea 100644 --- a/crates/libcgroups/src/systemd/unified.rs +++ b/crates/libcgroups/src/systemd/unified.rs @@ -1,27 +1,51 @@ -use anyhow::{bail, Context, Result}; use dbus::arg::RefArg; -use std::collections::HashMap; +use std::{collections::HashMap, num::ParseIntError}; use super::{ controller::Controller, cpu::{self, convert_shares_to_cgroup2}, - cpuset::{self, to_bitmask}, + cpuset::{self, to_bitmask, BitmaskError}, memory, pids, }; use crate::common::ControllerOpt; +#[derive(thiserror::Error, Debug)] +pub enum SystemdUnifiedError { + #[error("failed to parse cpu weight {value}: {err}")] + CpuWeight { err: ParseIntError, value: String }, + #[error("invalid format for cpu.max: {0}")] + CpuMax(String), + #[error("failed to to parse cpu quota {value}: {err}")] + CpuQuota { err: ParseIntError, value: String }, + #[error("failed to to parse cpu period {value}: {err}")] + CpuPeriod { err: ParseIntError, value: String }, + #[error("setting {0} requires systemd version greater than 243")] + OldSystemd(String), + #[error("invalid value for cpuset.cpus {0}")] + CpuSetCpu(BitmaskError), + #[error("failed to parse {name} {value}: {err}")] + Memory { + err: ParseIntError, + name: String, + value: String, + }, + #[error("failed to to parse pids.max {value}: {err}")] + PidsMax { err: ParseIntError, value: String }, +} + pub struct Unified {} impl Controller for Unified { + type Error = SystemdUnifiedError; + fn apply( options: &ControllerOpt, systemd_version: u32, properties: &mut HashMap<&str, Box>, - ) -> Result<()> { + ) -> Result<(), Self::Error> { if let Some(unified) = options.resources.unified() { log::debug!("applying unified resource restrictions"); - Self::apply(unified, systemd_version, properties) - .context("failed to apply unified resource restrictions")?; + Self::apply(unified, systemd_version, properties)?; } Ok(()) @@ -33,43 +57,50 @@ impl Unified { unified: &HashMap, systemd_version: u32, properties: &mut HashMap<&str, Box>, - ) -> Result<()> { + ) -> Result<(), SystemdUnifiedError> { for (key, value) in unified { match key.as_str() { "cpu.weight" => { - let shares = value - .parse::() - .with_context(|| format!("failed to parse cpu weight: {value}"))?; + let shares = + value + .parse::() + .map_err(|err| SystemdUnifiedError::CpuWeight { + err, + value: value.into(), + })?; properties.insert(cpu::CPU_WEIGHT, Box::new(convert_shares_to_cgroup2(shares))); } "cpu.max" => { let parts: Vec<&str> = value.split_whitespace().collect(); if parts.is_empty() || parts.len() > 2 { - bail!("invalid format for cpu.max: {}", value); + return Err(SystemdUnifiedError::CpuMax(value.into())); } - let quota = parts[0] - .parse::() - .with_context(|| format!("failed to parse cpu quota: {}", parts[0]))?; + let quota = + parts[0] + .parse::() + .map_err(|err| SystemdUnifiedError::CpuQuota { + err, + value: parts[0].into(), + })?; properties.insert(cpu::CPU_QUOTA, Box::new(quota)); if parts.len() == 2 { - let period = parts[1].parse::().with_context(|| { - format!("failed to to parse cpu period: {}", parts[1]) + let period = parts[1].parse::().map_err(|err| { + SystemdUnifiedError::CpuPeriod { + err, + value: parts[1].into(), + } })?; properties.insert(cpu::CPU_PERIOD, Box::new(period)); } } cpuset @ ("cpuset.cpus" | "cpuset.mems") => { if systemd_version <= 243 { - bail!( - "setting {} requires systemd version greater than 243", - cpuset - ); + return Err(SystemdUnifiedError::OldSystemd(cpuset.into())); } - let bitmask = to_bitmask(value) - .with_context(|| format!("invalid value for cpuset.cpus: {value}"))?; + let bitmask = to_bitmask(value).map_err(SystemdUnifiedError::CpuSetCpu)?; let systemd_cpuset = match cpuset { "cpuset.cpus" => cpuset::ALLOWED_CPUS, @@ -80,9 +111,14 @@ impl Unified { properties.insert(systemd_cpuset, Box::new(bitmask)); } memory @ ("memory.min" | "memory.low" | "memory.high" | "memory.max") => { - let value = value - .parse::() - .with_context(|| format!("failed to parse {memory}: {value}"))?; + let value = + value + .parse::() + .map_err(|err| SystemdUnifiedError::Memory { + err, + name: memory.into(), + value: value.into(), + })?; let systemd_memory = match memory { "memory.min" => memory::MEMORY_MIN, "memory.low" => memory::MEMORY_LOW, @@ -93,7 +129,12 @@ impl Unified { properties.insert(systemd_memory, Box::new(value)); } "pids.max" => { - let pids = value.trim().parse::()?; + let pids = value.trim().parse::().map_err(|err| { + SystemdUnifiedError::PidsMax { + err, + value: value.into(), + } + })?; properties.insert(pids::TASKS_MAX, Box::new(pids as u64)); } @@ -107,6 +148,7 @@ impl Unified { #[cfg(test)] mod tests { + use anyhow::{bail, Context, Result}; use dbus::arg::ArgType; use super::*; diff --git a/crates/libcgroups/src/test_manager.rs b/crates/libcgroups/src/test_manager.rs index 220584297..e24a956b5 100644 --- a/crates/libcgroups/src/test_manager.rs +++ b/crates/libcgroups/src/test_manager.rs @@ -1,6 +1,5 @@ -use std::cell::RefCell; +use std::{cell::RefCell, convert::Infallible}; -use anyhow::Result; use nix::unistd::Pid; use crate::{ @@ -24,30 +23,32 @@ impl Default for TestManager { } impl CgroupManager for TestManager { - fn add_task(&self, pid: Pid) -> Result<()> { + type Error = Infallible; + + fn add_task(&self, pid: Pid) -> Result<(), Infallible> { self.add_task_args.borrow_mut().push(pid); Ok(()) } // NOTE: The argument cannot be stored due to lifetime. - fn apply(&self, _controller_opt: &ControllerOpt) -> Result<()> { + fn apply(&self, _controller_opt: &ControllerOpt) -> Result<(), Infallible> { *self.apply_called.borrow_mut() = true; Ok(()) } - fn remove(&self) -> Result<()> { + fn remove(&self) -> Result<(), Infallible> { unimplemented!() } - fn freeze(&self, _state: FreezerState) -> Result<()> { + fn freeze(&self, _state: FreezerState) -> Result<(), Infallible> { unimplemented!() } - fn stats(&self) -> anyhow::Result { + fn stats(&self) -> Result { unimplemented!() } - fn get_all_pids(&self) -> Result> { + fn get_all_pids(&self) -> Result, Infallible> { unimplemented!() } } diff --git a/crates/libcgroups/src/v1/blkio.rs b/crates/libcgroups/src/v1/blkio.rs index 522bf7cba..ec078e8ba 100644 --- a/crates/libcgroups/src/v1/blkio.rs +++ b/crates/libcgroups/src/v1/blkio.rs @@ -1,14 +1,17 @@ -use std::path::Path; +use std::{ + num::ParseIntError, + path::{Path, PathBuf}, +}; use crate::{ - common::{self, ControllerOpt}, - stats::{self, BlkioDeviceStat, BlkioStats, StatsProvider}, - v1::Controller, + common::{self, ControllerOpt, WrappedIoError}, + stats::{self, BlkioDeviceStat, BlkioStats, ParseDeviceNumberError, StatsProvider}, }; -use anyhow::{Context, Result}; use oci_spec::runtime::LinuxBlockIo; +use super::controller::Controller; + // Throttling/upper limit policy // --------------------------------------- // Upper limit on the number of read operations a device can perform specified in bytes @@ -76,9 +79,10 @@ const BLKIO_MERGED: &str = "blkio.io_merged_recursive"; pub struct Blkio {} impl Controller for Blkio { + type Error = WrappedIoError; type Resource = LinuxBlockIo; - fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()> { + fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<(), Self::Error> { log::debug!("Apply blkio cgroup config"); if let Some(blkio) = Self::needs_to_handle(controller_opt) { @@ -93,10 +97,25 @@ impl Controller for Blkio { } } +#[derive(thiserror::Error, Debug)] +pub enum V1BlkioStatsError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("failed to parse device value {value} in {path}: {err}")] + FailedParseValue { + value: String, + path: PathBuf, + err: ParseIntError, + }, + #[error("failed to parse device number: {0}")] + FailedParseNumber(#[from] ParseDeviceNumberError), +} + impl StatsProvider for Blkio { + type Error = V1BlkioStatsError; type Stats = BlkioStats; - fn stats(cgroup_path: &Path) -> Result { + fn stats(cgroup_path: &Path) -> Result { if cgroup_path.join(BLKIO_WEIGHT).exists() { return Self::get_weight_division_policy_stats(cgroup_path); } @@ -106,7 +125,7 @@ impl StatsProvider for Blkio { } impl Blkio { - fn apply(root_path: &Path, blkio: &LinuxBlockIo) -> Result<()> { + fn apply(root_path: &Path, blkio: &LinuxBlockIo) -> Result<(), WrappedIoError> { if let Some(blkio_weight) = blkio.weight() { // be aligned with what runc does // See also: https://github.com/opencontainers/runc/blob/81044ad7c902f3fc153cb8ffadaf4da62855193f/libcontainer/cgroups/fs/blkio.go#L28-L33 @@ -159,7 +178,7 @@ impl Blkio { Ok(()) } - fn get_throttling_policy_stats(cgroup_path: &Path) -> Result { + fn get_throttling_policy_stats(cgroup_path: &Path) -> Result { let stats = BlkioStats { service_bytes: Self::parse_blkio_file( &cgroup_path.join(BLKIO_THROTTLE_IO_SERVICE_BYTES), @@ -171,7 +190,9 @@ impl Blkio { Ok(stats) } - fn get_weight_division_policy_stats(cgroup_path: &Path) -> Result { + fn get_weight_division_policy_stats( + cgroup_path: &Path, + ) -> Result { let stats = BlkioStats { time: Self::parse_blkio_file(&cgroup_path.join(BLKIO_TIME))?, sectors: Self::parse_blkio_file(&cgroup_path.join(BLKIO_SECTORS))?, @@ -187,7 +208,7 @@ impl Blkio { Ok(stats) } - fn parse_blkio_file(blkio_file: &Path) -> Result> { + fn parse_blkio_file(blkio_file: &Path) -> Result, V1BlkioStatsError> { let content = common::read_cgroup_file(blkio_file)?; let mut stats = Vec::new(); for entry in content.lines() { @@ -203,21 +224,21 @@ impl Blkio { None }; let value = if entry_fields.len() == 3 { - entry_fields[2].parse().with_context(|| { - format!( - "failed to parse device value {} in {}", - entry_fields[2], - blkio_file.display() - ) - })? + entry_fields[2] + .parse() + .map_err(|err| V1BlkioStatsError::FailedParseValue { + value: entry_fields[2].into(), + path: blkio_file.to_path_buf(), + err, + })? } else { - entry_fields[1].parse().with_context(|| { - format!( - "failed to parse device value {} in {}", - entry_fields[1], - blkio_file.display() - ) - })? + entry_fields[1] + .parse() + .map_err(|err| V1BlkioStatsError::FailedParseValue { + value: entry_fields[1].into(), + path: blkio_file.to_path_buf(), + err, + })? }; let stat = BlkioDeviceStat { @@ -241,7 +262,6 @@ mod tests { use super::*; use crate::test::{create_temp_dir, set_fixture, setup}; - use anyhow::Result; use oci_spec::runtime::{LinuxBlockIoBuilder, LinuxThrottleDeviceBuilder}; #[test] @@ -344,7 +364,7 @@ mod tests { } #[test] - fn test_stat_throttling_policy() -> Result<()> { + fn test_stat_throttling_policy() -> Result<(), Box> { let tmp = create_temp_dir("test_stat_throttling_policy").expect("create test directory"); let content = &[ "8:0 Read 20", diff --git a/crates/libcgroups/src/v1/controller.rs b/crates/libcgroups/src/v1/controller.rs index 3966fb86b..86cd5a17c 100644 --- a/crates/libcgroups/src/v1/controller.rs +++ b/crates/libcgroups/src/v1/controller.rs @@ -1,22 +1,22 @@ use std::{fs, path::Path}; -use anyhow::Result; use nix::unistd::Pid; -use crate::common::{self, ControllerOpt, CGROUP_PROCS}; +use crate::common::{self, ControllerOpt, WrapIoResult, WrappedIoError, CGROUP_PROCS}; -pub trait Controller { +pub(super) trait Controller { + type Error: From; type Resource; /// Adds a new task specified by its pid to the cgroup - fn add_task(pid: Pid, cgroup_path: &Path) -> Result<()> { - fs::create_dir_all(cgroup_path)?; + fn add_task(pid: Pid, cgroup_path: &Path) -> Result<(), Self::Error> { + fs::create_dir_all(cgroup_path).wrap_create_dir(cgroup_path)?; common::write_cgroup_file(cgroup_path.join(CGROUP_PROCS), pid)?; Ok(()) } /// Applies resource restrictions to the cgroup - fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()>; + fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<(), Self::Error>; /// Checks if the controller needs to handle this request fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource>; diff --git a/crates/libcgroups/src/v1/controller_type.rs b/crates/libcgroups/src/v1/controller_type.rs index 334b2643c..dcc3f66e8 100644 --- a/crates/libcgroups/src/v1/controller_type.rs +++ b/crates/libcgroups/src/v1/controller_type.rs @@ -1,6 +1,6 @@ use std::fmt::Display; -#[derive(Hash, PartialEq, Eq, Debug, Clone)] +#[derive(Hash, PartialEq, Eq, Debug, Clone, Copy)] pub enum ControllerType { Cpu, CpuAcct, diff --git a/crates/libcgroups/src/v1/cpu.rs b/crates/libcgroups/src/v1/cpu.rs index a60a9ab20..ac02f4163 100644 --- a/crates/libcgroups/src/v1/cpu.rs +++ b/crates/libcgroups/src/v1/cpu.rs @@ -1,14 +1,13 @@ -use std::path::Path; +use std::path::{Path, PathBuf}; -use anyhow::{bail, Context, Result}; use oci_spec::runtime::LinuxCpu; use crate::{ - common::{self, ControllerOpt}, - stats::{CpuThrottling, StatsProvider}, + common::{self, ControllerOpt, WrappedIoError}, + stats::{parse_flat_keyed_data, CpuThrottling, ParseFlatKeyedDataError, StatsProvider}, }; -use super::Controller; +use super::controller::Controller; const CGROUP_CPU_SHARES: &str = "cpu.shares"; const CGROUP_CPU_QUOTA: &str = "cpu.cfs_quota_us"; @@ -22,13 +21,14 @@ const CGROUP_CPU_IDLE: &str = "cpu.idle"; pub struct Cpu {} impl Controller for Cpu { + type Error = WrappedIoError; type Resource = LinuxCpu; - fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()> { + fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<(), Self::Error> { log::debug!("Apply Cpu cgroup config"); if let Some(cpu) = Self::needs_to_handle(controller_opt) { - Self::apply(cgroup_root, cpu).context("failed to apply cpu resource restrictions")?; + Self::apply(cgroup_root, cpu)?; } Ok(()) @@ -51,53 +51,46 @@ impl Controller for Cpu { } } +#[derive(thiserror::Error, Debug)] +pub enum V1CpuStatsError { + #[error("error parsing data: {0}")] + ParseData(#[from] ParseFlatKeyedDataError), + #[error("missing field {field} from {path}")] + MissingField { field: &'static str, path: PathBuf }, +} + impl StatsProvider for Cpu { + type Error = V1CpuStatsError; type Stats = CpuThrottling; - fn stats(cgroup_path: &Path) -> Result { + fn stats(cgroup_path: &Path) -> Result { let mut stats = CpuThrottling::default(); let stat_path = cgroup_path.join(CGROUP_CPU_STAT); - let stat_content = common::read_cgroup_file(&stat_path)?; - - let parts: Vec<&str> = stat_content.split_ascii_whitespace().collect(); - if parts.len() < 6 { - bail!( - "{} contains less than the expected number of entries", - stat_path.display() - ); - } - - if parts[0] != "nr_periods" { - bail!( - "{} does not contain the number of elapsed periods", - stat_path.display() - ); - } - - if parts[2] != "nr_throttled" { - bail!( - "{} does not contain the number of throttled periods", - stat_path.display() - ); - } - if parts[4] != "throttled_time" { - bail!( - "{} does not contain the total time tasks have spent throttled", - stat_path.display() - ); + let stat_table = parse_flat_keyed_data(&stat_path)?; + + macro_rules! get { + ($name: expr => $field: ident) => { + stats.$field = + *stat_table + .get($name) + .ok_or_else(|| V1CpuStatsError::MissingField { + field: $name, + path: stat_path.clone(), + })?; + }; } - stats.periods = parts[1].parse().context("failed to parse nr_periods")?; - stats.throttled_periods = parts[3].parse().context("failed to parse nr_throttled")?; - stats.throttled_time = parts[5].parse().context("failed to parse throttled time")?; + get!("nr_periods" => periods); + get!("nr_throttled" => throttled_periods); + get!("throttled_time" => throttled_time); Ok(stats) } } impl Cpu { - fn apply(root_path: &Path, cpu: &LinuxCpu) -> Result<()> { + fn apply(root_path: &Path, cpu: &LinuxCpu) -> Result<(), WrappedIoError> { if let Some(cpu_shares) = cpu.shares() { if cpu_shares != 0 { common::write_cgroup_file(root_path.join(CGROUP_CPU_SHARES), cpu_shares)?; diff --git a/crates/libcgroups/src/v1/cpuacct.rs b/crates/libcgroups/src/v1/cpuacct.rs index 2efb9e534..58626fb6b 100644 --- a/crates/libcgroups/src/v1/cpuacct.rs +++ b/crates/libcgroups/src/v1/cpuacct.rs @@ -1,13 +1,14 @@ -use std::path::Path; - -use anyhow::{bail, Context, Result}; +use std::{ + num::ParseIntError, + path::{Path, PathBuf}, +}; use crate::{ - common::{self, ControllerOpt}, - stats::{CpuUsage, StatsProvider}, + common::{self, ControllerOpt, WrappedIoError}, + stats::{parse_flat_keyed_data, CpuUsage, ParseFlatKeyedDataError, StatsProvider}, }; -use super::Controller; +use super::controller::Controller; // Contains user mode and kernel mode cpu consumption const CGROUP_CPUACCT_STAT: &str = "cpuacct.stat"; @@ -21,9 +22,10 @@ const CGROUP_CPUACCT_PERCPU: &str = "cpuacct.usage_percpu"; pub struct CpuAcct {} impl Controller for CpuAcct { + type Error = WrappedIoError; type Resource = (); - fn apply(_controller_opt: &ControllerOpt, _cgroup_path: &Path) -> Result<()> { + fn apply(_controller_opt: &ControllerOpt, _cgroup_path: &Path) -> Result<(), Self::Error> { Ok(()) } @@ -32,10 +34,31 @@ impl Controller for CpuAcct { } } +#[derive(thiserror::Error, Debug)] +pub enum V1CpuAcctStatsError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("error parsing data: {0}")] + ParseData(#[from] ParseFlatKeyedDataError), + #[error("missing field {field} from {path}")] + MissingField { field: &'static str, path: PathBuf }, + #[error("failed to parse total cpu usage: {0}")] + ParseTotalCpu(ParseIntError), + #[error("failed to parse per core {mode} mode cpu usage in {path}: {err}")] + FailedToParseField { + mode: &'static str, + path: PathBuf, + err: ParseIntError, + }, + #[error("failed to parse per core cpu usage: {0}")] + ParsePerCore(ParseIntError), +} + impl StatsProvider for CpuAcct { + type Error = V1CpuAcctStatsError; type Stats = CpuUsage; - fn stats(cgroup_path: &Path) -> Result { + fn stats(cgroup_path: &Path) -> Result { let mut stats = CpuUsage::default(); Self::get_total_cpu_usage(cgroup_path, &mut stats)?; Self::get_per_core_usage(cgroup_path, &mut stats)?; @@ -45,54 +68,43 @@ impl StatsProvider for CpuAcct { } impl CpuAcct { - fn get_total_cpu_usage(cgroup_path: &Path, stats: &mut CpuUsage) -> Result<()> { + fn get_total_cpu_usage( + cgroup_path: &Path, + stats: &mut CpuUsage, + ) -> Result<(), V1CpuAcctStatsError> { let stat_file_path = cgroup_path.join(CGROUP_CPUACCT_STAT); - let stat_file_content = common::read_cgroup_file(&stat_file_path)?; - - // the first two entries of the file should look like this - // user 746908 - // system 213896 - let parts: Vec<&str> = stat_file_content.split_whitespace().collect(); - - if parts.len() < 4 { - bail!( - "{} contains less than the expected number of entries", - stat_file_path.display() - ); - } - - if !parts[0].eq("user") { - bail!( - "{} does not contain user mode cpu usage", - stat_file_path.display() - ); - } - - if !parts[2].eq("system") { - bail!( - "{} does not contain kernel mode cpu usage", - stat_file_path.display() - ); + let stat_table = parse_flat_keyed_data(&stat_file_path)?; + + macro_rules! get { + ($name: expr => $field: ident) => { + stats.$field = + *stat_table + .get($name) + .ok_or_else(|| V1CpuAcctStatsError::MissingField { + field: $name, + path: stat_file_path.clone(), + })?; + }; } - stats.usage_user = parts[1] - .parse() - .context("failed to parse user mode cpu usage")?; - stats.usage_kernel = parts[3] - .parse() - .context("failed to parse kernel mode cpu usage")?; + get!("user" => usage_user); + get!("system" => usage_kernel); let total = common::read_cgroup_file(cgroup_path.join(CGROUP_CPUACCT_USAGE))?; stats.usage_total = total .trim() .parse() - .context("failed to parse total cpu usage")?; + .map_err(V1CpuAcctStatsError::ParseTotalCpu)?; Ok(()) } - fn get_per_core_usage(cgroup_path: &Path, stats: &mut CpuUsage) -> Result<()> { - let all_content = common::read_cgroup_file(cgroup_path.join(CGROUP_CPUACCT_USAGE_ALL))?; + fn get_per_core_usage( + cgroup_path: &Path, + stats: &mut CpuUsage, + ) -> Result<(), V1CpuAcctStatsError> { + let path = cgroup_path.join(CGROUP_CPUACCT_USAGE_ALL); + let all_content = common::read_cgroup_file(&path)?; // first line is header, skip it for entry in all_content.lines().skip(1) { let entry_parts: Vec<&str> = entry.split_ascii_whitespace().collect(); @@ -100,16 +112,24 @@ impl CpuAcct { continue; } - stats.per_core_usage_user.push( - entry_parts[1] - .parse() - .context("failed to parse per core user mode cpu usage")?, - ); - stats.per_core_usage_kernel.push( - entry_parts[2] - .parse() - .context("failed to parse per core kernel mode cpu usage")?, - ); + stats + .per_core_usage_user + .push(entry_parts[1].parse().map_err(|err| { + V1CpuAcctStatsError::FailedToParseField { + mode: "user", + path: path.clone(), + err, + } + })?); + stats + .per_core_usage_kernel + .push(entry_parts[2].parse().map_err(|err| { + V1CpuAcctStatsError::FailedToParseField { + mode: "kernel", + path: path.clone(), + err, + } + })?); } let percpu_content = common::read_cgroup_file(cgroup_path.join(CGROUP_CPUACCT_PERCPU))?; @@ -117,7 +137,7 @@ impl CpuAcct { .split_ascii_whitespace() .map(|v| v.parse()) .collect::, _>>() - .context("failed to parse per core cpu usage")?; + .map_err(V1CpuAcctStatsError::ParsePerCore)?; Ok(()) } diff --git a/crates/libcgroups/src/v1/cpuset.rs b/crates/libcgroups/src/v1/cpuset.rs index 92af2aa8b..9cb69b376 100644 --- a/crates/libcgroups/src/v1/cpuset.rs +++ b/crates/libcgroups/src/v1/cpuset.rs @@ -1,24 +1,46 @@ -use std::{fs, path::Path}; +use std::{ + fs, + path::{Path, PathBuf, StripPrefixError}, +}; -use anyhow::{bail, Context, Result}; use nix::unistd; use oci_spec::runtime::LinuxCpu; use unistd::Pid; -use crate::common::{self, ControllerOpt, CGROUP_PROCS}; +use crate::common::{self, ControllerOpt, WrapIoResult, WrappedIoError, CGROUP_PROCS}; -use super::{util, Controller, ControllerType}; +use super::{ + controller::Controller, + util::{self, V1MountPointError}, + ControllerType, +}; const CGROUP_CPUSET_CPUS: &str = "cpuset.cpus"; const CGROUP_CPUSET_MEMS: &str = "cpuset.mems"; +#[derive(thiserror::Error, Debug)] +pub enum V1CpuSetControllerError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("bad cgroup path {path}: {err}")] + BadCgroupPath { + err: StripPrefixError, + path: PathBuf, + }, + #[error("cpuset parent value is empty")] + EmptyParent, + #[error("mount point error: {0}")] + MountPoint(#[from] V1MountPointError), +} + pub struct CpuSet {} impl Controller for CpuSet { + type Error = V1CpuSetControllerError; type Resource = LinuxCpu; - fn add_task(pid: Pid, cgroup_path: &Path) -> Result<()> { - fs::create_dir_all(cgroup_path)?; + fn add_task(pid: Pid, cgroup_path: &Path) -> Result<(), Self::Error> { + fs::create_dir_all(cgroup_path).wrap_create_dir(cgroup_path)?; Self::ensure_not_empty(cgroup_path, CGROUP_CPUSET_CPUS)?; Self::ensure_not_empty(cgroup_path, CGROUP_CPUSET_MEMS)?; @@ -27,12 +49,11 @@ impl Controller for CpuSet { Ok(()) } - fn apply(controller_opt: &ControllerOpt, cgroup_path: &Path) -> Result<()> { + fn apply(controller_opt: &ControllerOpt, cgroup_path: &Path) -> Result<(), Self::Error> { log::debug!("Apply CpuSet cgroup config"); if let Some(cpuset) = Self::needs_to_handle(controller_opt) { - Self::apply(cgroup_path, cpuset) - .context("failed to apply cpuset resource restrictions")?; + Self::apply(cgroup_path, cpuset)?; } Ok(()) @@ -50,7 +71,7 @@ impl Controller for CpuSet { } impl CpuSet { - fn apply(cgroup_path: &Path, cpuset: &LinuxCpu) -> Result<()> { + fn apply(cgroup_path: &Path, cpuset: &LinuxCpu) -> Result<(), V1CpuSetControllerError> { if let Some(cpus) = &cpuset.cpus() { common::write_cgroup_file_str(cgroup_path.join(CGROUP_CPUSET_CPUS), cpus)?; } @@ -64,19 +85,28 @@ impl CpuSet { // if a task is moved into the cgroup and a value has not been set for cpus and mems // Errno 28 (no space left on device) will be returned. Therefore we set the value from the parent if required. - fn ensure_not_empty(cgroup_path: &Path, interface_file: &str) -> Result<()> { + fn ensure_not_empty( + cgroup_path: &Path, + interface_file: &str, + ) -> Result<(), V1CpuSetControllerError> { let mut current = util::get_subsystem_mount_point(&ControllerType::CpuSet)?; - let relative_cgroup_path = cgroup_path.strip_prefix(¤t)?; + let relative_cgroup_path = cgroup_path.strip_prefix(¤t).map_err(|err| { + V1CpuSetControllerError::BadCgroupPath { + err, + path: cgroup_path.to_path_buf(), + } + })?; for component in relative_cgroup_path.components() { - let parent_value = fs::read_to_string(current.join(interface_file))?; + let parent_value = + fs::read_to_string(current.join(interface_file)).wrap_read(cgroup_path)?; if parent_value.trim().is_empty() { - bail!("cpuset parent value is empty") + return Err(V1CpuSetControllerError::EmptyParent); } current.push(component); let child_path = current.join(interface_file); - let child_value = fs::read_to_string(&child_path)?; + let child_value = fs::read_to_string(&child_path).wrap_read(&child_path)?; // the file can contain a newline character. Need to trim it away, // otherwise it is not considered empty and value will not be written if child_value.trim().is_empty() { diff --git a/crates/libcgroups/src/v1/devices.rs b/crates/libcgroups/src/v1/devices.rs index 2d58992b8..77b246090 100644 --- a/crates/libcgroups/src/v1/devices.rs +++ b/crates/libcgroups/src/v1/devices.rs @@ -1,17 +1,16 @@ use std::path::Path; -use anyhow::Result; - use super::controller::Controller; -use crate::common::{self, default_allow_devices, default_devices, ControllerOpt}; +use crate::common::{self, default_allow_devices, default_devices, ControllerOpt, WrappedIoError}; use oci_spec::runtime::LinuxDeviceCgroup; pub struct Devices {} impl Controller for Devices { + type Error = WrappedIoError; type Resource = (); - fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()> { + fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<(), Self::Error> { log::debug!("Apply Devices cgroup config"); if let Some(devices) = controller_opt.resources.devices().as_ref() { @@ -39,7 +38,7 @@ impl Controller for Devices { } impl Devices { - fn apply_device(device: &LinuxDeviceCgroup, cgroup_root: &Path) -> Result<()> { + fn apply_device(device: &LinuxDeviceCgroup, cgroup_root: &Path) -> Result<(), WrappedIoError> { let path = if device.allow() { cgroup_root.join("devices.allow") } else { diff --git a/crates/libcgroups/src/v1/freezer.rs b/crates/libcgroups/src/v1/freezer.rs index cda198c51..15bcaca94 100644 --- a/crates/libcgroups/src/v1/freezer.rs +++ b/crates/libcgroups/src/v1/freezer.rs @@ -1,32 +1,38 @@ -use std::io::prelude::*; -use std::{ - fs::{create_dir_all, OpenOptions}, - path::Path, - thread, time, -}; +use std::io::Read; +use std::{fs::OpenOptions, path::Path, thread, time}; -use anyhow::{Result, *}; - -use super::Controller; -use crate::common; +use crate::common::{self, WrapIoResult, WrappedIoError}; use crate::common::{ControllerOpt, FreezerState}; +use super::controller::Controller; + const CGROUP_FREEZER_STATE: &str = "freezer.state"; const FREEZER_STATE_THAWED: &str = "THAWED"; const FREEZER_STATE_FROZEN: &str = "FROZEN"; const FREEZER_STATE_FREEZING: &str = "FREEZING"; +#[derive(thiserror::Error, Debug)] +pub enum V1FreezerControllerError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("unexpected state {state} while freezing")] + UnexpectedState { state: String }, + #[error("unable to freeze")] + UnableToFreeze, +} + pub struct Freezer {} impl Controller for Freezer { + type Error = V1FreezerControllerError; type Resource = FreezerState; - fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()> { + fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<(), Self::Error> { log::debug!("Apply Freezer cgroup config"); - create_dir_all(cgroup_root)?; + std::fs::create_dir_all(cgroup_root).wrap_create_dir(cgroup_root)?; if let Some(freezer_state) = Self::needs_to_handle(controller_opt) { - Self::apply(freezer_state, cgroup_root).context("failed to appyl freezer")?; + Self::apply(freezer_state, cgroup_root)?; } Ok(()) @@ -38,7 +44,10 @@ impl Controller for Freezer { } impl Freezer { - fn apply(freezer_state: &FreezerState, cgroup_root: &Path) -> Result<()> { + fn apply( + freezer_state: &FreezerState, + cgroup_root: &Path, + ) -> Result<(), V1FreezerControllerError> { match freezer_state { FreezerState::Undefined => {} FreezerState::Thawed => { @@ -48,7 +57,7 @@ impl Freezer { )?; } FreezerState::Frozen => { - let r = || -> Result<()> { + let r = || -> Result<(), V1FreezerControllerError> { // We should do our best to retry if FREEZING is seen until it becomes FROZEN. // Add sleep between retries occasionally helped when system is extremely slow. // see: @@ -84,11 +93,11 @@ impl Freezer { } _ => { // should not reach here. - bail!("unexpected state {} while freezing", r.trim()); + return Err(V1FreezerControllerError::UnexpectedState { state: r }); } } } - bail!("unbale to freeze"); + Err(V1FreezerControllerError::UnableToFreeze) }(); if r.is_err() { @@ -105,14 +114,16 @@ impl Freezer { Ok(()) } - fn read_freezer_state(cgroup_root: &Path) -> Result { + fn read_freezer_state(cgroup_root: &Path) -> Result { let path = cgroup_root.join(CGROUP_FREEZER_STATE); let mut content = String::new(); OpenOptions::new() .create(false) .read(true) - .open(path)? - .read_to_string(&mut content)?; + .open(path) + .wrap_open(cgroup_root)? + .read_to_string(&mut content) + .wrap_read(cgroup_root)?; Ok(content) } } diff --git a/crates/libcgroups/src/v1/hugetlb.rs b/crates/libcgroups/src/v1/hugetlb.rs index 752eb8aa9..7ce06b189 100644 --- a/crates/libcgroups/src/v1/hugetlb.rs +++ b/crates/libcgroups/src/v1/hugetlb.rs @@ -1,27 +1,40 @@ -use std::{collections::HashMap, path::Path}; - -use anyhow::{bail, Context, Result}; +use std::{collections::HashMap, num::ParseIntError, path::Path}; use crate::{ - common::{self, ControllerOpt}, - stats::{supported_page_sizes, HugeTlbStats, StatsProvider}, + common::{self, ControllerOpt, EitherError, MustBePowerOfTwo, WrappedIoError}, + stats::{supported_page_sizes, HugeTlbStats, StatsProvider, SupportedPageSizesError}, }; -use super::Controller; use oci_spec::runtime::LinuxHugepageLimit; +use super::controller::Controller; + +#[derive(thiserror::Error, Debug)] +pub enum V1HugeTlbControllerError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("malformed page size {page_size}: {err}")] + MalformedPageSize { + page_size: String, + err: EitherError, + }, +} + pub struct HugeTlb {} impl Controller for HugeTlb { + type Error = V1HugeTlbControllerError; type Resource = Vec; - fn apply(controller_opt: &ControllerOpt, cgroup_root: &std::path::Path) -> Result<()> { + fn apply( + controller_opt: &ControllerOpt, + cgroup_root: &std::path::Path, + ) -> Result<(), Self::Error> { log::debug!("Apply Hugetlb cgroup config"); if let Some(hugepage_limits) = Self::needs_to_handle(controller_opt) { for hugetlb in hugepage_limits { - Self::apply(cgroup_root, hugetlb) - .context("failed to apply hugetlb resource restrictions")? + Self::apply(cgroup_root, hugetlb)? } } @@ -39,10 +52,21 @@ impl Controller for HugeTlb { } } +#[derive(thiserror::Error, Debug)] +pub enum V1HugeTlbStatsError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("error getting supported page sizes: {0}")] + SupportedPageSizes(#[from] SupportedPageSizesError), + #[error("error parsing value: {0}")] + Parse(#[from] ParseIntError), +} + impl StatsProvider for HugeTlb { + type Error = V1HugeTlbStatsError; type Stats = HashMap; - fn stats(cgroup_path: &Path) -> Result { + fn stats(cgroup_path: &Path) -> Result { let page_sizes = supported_page_sizes()?; let mut hugetlb_stats = HashMap::with_capacity(page_sizes.len()); @@ -56,15 +80,29 @@ impl StatsProvider for HugeTlb { } impl HugeTlb { - fn apply(root_path: &Path, hugetlb: &LinuxHugepageLimit) -> Result<()> { - let page_size: String = hugetlb + fn apply( + root_path: &Path, + hugetlb: &LinuxHugepageLimit, + ) -> Result<(), V1HugeTlbControllerError> { + let raw_page_size: String = hugetlb .page_size() .chars() .take_while(|c| c.is_ascii_digit()) .collect(); - let page_size: u64 = page_size.parse()?; + let page_size: u64 = match raw_page_size.parse() { + Ok(page_size) => page_size, + Err(err) => { + return Err(V1HugeTlbControllerError::MalformedPageSize { + page_size: raw_page_size, + err: EitherError::Left(err), + }) + } + }; if !Self::is_power_of_two(page_size) { - bail!("page size must be in the format of 2^(integer)"); + return Err(V1HugeTlbControllerError::MalformedPageSize { + page_size: raw_page_size, + err: EitherError::Right(MustBePowerOfTwo), + }); } common::write_cgroup_file( @@ -78,7 +116,10 @@ impl HugeTlb { (number != 0) && (number & (number.saturating_sub(1))) == 0 } - fn stats_for_page_size(cgroup_path: &Path, page_size: &str) -> Result { + fn stats_for_page_size( + cgroup_path: &Path, + page_size: &str, + ) -> Result { let mut stats = HugeTlbStats::default(); let usage_file = format!("hugetlb.{page_size}.usage_in_bytes"); diff --git a/crates/libcgroups/src/v1/manager.rs b/crates/libcgroups/src/v1/manager.rs index 43ee7b524..71d29615f 100644 --- a/crates/libcgroups/src/v1/manager.rs +++ b/crates/libcgroups/src/v1/manager.rs @@ -3,34 +3,89 @@ use std::path::Path; use std::time::Duration; use std::{collections::HashMap, path::PathBuf}; -use anyhow::bail; -use anyhow::Result; use nix::unistd::Pid; use procfs::process::Process; +use procfs::ProcError; -use super::ControllerType as CtrlType; +use super::blkio::V1BlkioStatsError; +use super::cpu::V1CpuStatsError; +use super::cpuacct::V1CpuAcctStatsError; +use super::cpuset::V1CpuSetControllerError; +use super::freezer::V1FreezerControllerError; +use super::hugetlb::{V1HugeTlbControllerError, V1HugeTlbStatsError}; +use super::memory::{V1MemoryControllerError, V1MemoryStatsError}; +use super::util::V1MountPointError; use super::{ - blkio::Blkio, controller_type::CONTROLLERS, cpu::Cpu, cpuacct::CpuAcct, cpuset::CpuSet, - devices::Devices, freezer::Freezer, hugetlb::HugeTlb, memory::Memory, + blkio::Blkio, controller::Controller, controller_type::CONTROLLERS, cpu::Cpu, cpuacct::CpuAcct, + cpuset::CpuSet, devices::Devices, freezer::Freezer, hugetlb::HugeTlb, memory::Memory, network_classifier::NetworkClassifier, network_priority::NetworkPriority, - perf_event::PerfEvent, pids::Pids, util, Controller, + perf_event::PerfEvent, pids::Pids, util, ControllerType as CtrlType, }; -use crate::common::{self, CgroupManager, ControllerOpt, FreezerState, PathBufExt, CGROUP_PROCS}; -use crate::stats::{Stats, StatsProvider}; +use crate::common::{ + self, AnyCgroupManager, CgroupManager, ControllerOpt, FreezerState, JoinSafelyError, + PathBufExt, WrapIoResult, WrappedIoError, CGROUP_PROCS, +}; +use crate::stats::{PidStatsError, Stats, StatsProvider}; +use crate::v1::ControllerType; pub struct Manager { subsystems: HashMap, } +#[derive(thiserror::Error, Debug)] +pub enum V1ManagerError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("mount point error: {0}")] + MountPoint(#[from] V1MountPointError), + #[error("proc error: {0}")] + Proc(#[from] ProcError), + #[error("while joining paths: {0}")] + JoinSafely(#[from] JoinSafelyError), + #[error("cgroup {0} is required to fulfill the request, but is not supported by this system")] + CGroupRequired(ControllerType), + #[error("subsystem does not exist")] + SubsystemDoesNotExist, + + #[error(transparent)] + BlkioController(WrappedIoError), + #[error(transparent)] + CpuController(WrappedIoError), + #[error(transparent)] + CpuAcctController(WrappedIoError), + #[error(transparent)] + CpuSetController(#[from] V1CpuSetControllerError), + #[error(transparent)] + FreezerController(#[from] V1FreezerControllerError), + #[error(transparent)] + HugeTlbController(#[from] V1HugeTlbControllerError), + #[error(transparent)] + MemoryController(#[from] V1MemoryControllerError), + #[error(transparent)] + PidsController(WrappedIoError), + + #[error(transparent)] + BlkioStats(#[from] V1BlkioStatsError), + #[error(transparent)] + CpuStats(#[from] V1CpuStatsError), + #[error(transparent)] + CpuAcctStats(#[from] V1CpuAcctStatsError), + #[error(transparent)] + PidsStats(PidStatsError), + #[error(transparent)] + HugeTlbStats(#[from] V1HugeTlbStatsError), + #[error(transparent)] + MemoryStats(#[from] V1MemoryStatsError), +} impl Manager { /// Constructs a new cgroup manager with cgroups_path being relative to the root of the subsystem - pub fn new(cgroup_path: PathBuf) -> Result { + pub fn new(cgroup_path: PathBuf) -> Result { let mut subsystems = HashMap::::new(); for subsystem in CONTROLLERS { if let Ok(subsystem_path) = Self::get_subsystem_path(&cgroup_path, subsystem) { - subsystems.insert(subsystem.clone(), subsystem_path); + subsystems.insert(*subsystem, subsystem_path); } else { log::warn!("cgroup {} not supported on this system", subsystem); } @@ -39,7 +94,10 @@ impl Manager { Ok(Manager { subsystems }) } - fn get_subsystem_path(cgroup_path: &Path, subsystem: &CtrlType) -> Result { + fn get_subsystem_path( + cgroup_path: &Path, + subsystem: &CtrlType, + ) -> Result { log::debug!("Get path for subsystem: {}", subsystem); let mount_point = util::get_subsystem_mount_point(subsystem)?; @@ -61,7 +119,7 @@ impl Manager { fn get_required_controllers( &self, controller_opt: &ControllerOpt, - ) -> Result> { + ) -> Result, V1ManagerError> { let mut required_controllers = HashMap::new(); for controller in CONTROLLERS { @@ -88,25 +146,31 @@ impl Manager { if let Some(subsystem_path) = self.subsystems.get(controller) { required_controllers.insert(controller, subsystem_path); } else { - bail!("cgroup {} is required to fulfill the request, but is not supported by this system", controller); + return Err(V1ManagerError::CGroupRequired(*controller)); } } } Ok(required_controllers) } + + pub fn any(self) -> AnyCgroupManager { + AnyCgroupManager::V1(self) + } } impl CgroupManager for Manager { - fn get_all_pids(&self) -> Result> { + type Error = V1ManagerError; + + fn get_all_pids(&self) -> Result, Self::Error> { let devices = self.subsystems.get(&CtrlType::Devices); if let Some(p) = devices { - common::get_all_pids(p) + Ok(common::get_all_pids(p)?) } else { - bail!("subsystem does not exist") + Err(V1ManagerError::SubsystemDoesNotExist) } } - fn add_task(&self, pid: Pid) -> Result<()> { + fn add_task(&self, pid: Pid) -> Result<(), Self::Error> { for subsys in &self.subsystems { match subsys.0 { CtrlType::Cpu => Cpu::add_task(pid, subsys.1)?, @@ -117,7 +181,9 @@ impl CgroupManager for Manager { CtrlType::Memory => Memory::add_task(pid, subsys.1)?, CtrlType::Pids => Pids::add_task(pid, subsys.1)?, CtrlType::PerfEvent => PerfEvent::add_task(pid, subsys.1)?, - CtrlType::Blkio => Blkio::add_task(pid, subsys.1)?, + CtrlType::Blkio => { + Blkio::add_task(pid, subsys.1).map_err(V1ManagerError::BlkioController)? + } CtrlType::NetworkPriority => NetworkPriority::add_task(pid, subsys.1)?, CtrlType::NetworkClassifier => NetworkClassifier::add_task(pid, subsys.1)?, CtrlType::Freezer => Freezer::add_task(pid, subsys.1)?, @@ -127,7 +193,7 @@ impl CgroupManager for Manager { Ok(()) } - fn apply(&self, controller_opt: &ControllerOpt) -> Result<()> { + fn apply(&self, controller_opt: &ControllerOpt) -> Result<(), Self::Error> { for subsys in self.get_required_controllers(controller_opt)? { match subsys.0 { CtrlType::Cpu => Cpu::apply(controller_opt, subsys.1)?, @@ -138,7 +204,8 @@ impl CgroupManager for Manager { CtrlType::Memory => Memory::apply(controller_opt, subsys.1)?, CtrlType::Pids => Pids::apply(controller_opt, subsys.1)?, CtrlType::PerfEvent => PerfEvent::apply(controller_opt, subsys.1)?, - CtrlType::Blkio => Blkio::apply(controller_opt, subsys.1)?, + CtrlType::Blkio => Blkio::apply(controller_opt, subsys.1) + .map_err(V1ManagerError::BlkioController)?, CtrlType::NetworkPriority => NetworkPriority::apply(controller_opt, subsys.1)?, CtrlType::NetworkClassifier => NetworkClassifier::apply(controller_opt, subsys.1)?, CtrlType::Freezer => Freezer::apply(controller_opt, subsys.1)?, @@ -148,15 +215,18 @@ impl CgroupManager for Manager { Ok(()) } - fn remove(&self) -> Result<()> { + fn remove(&self) -> Result<(), Self::Error> { for cgroup_path in &self.subsystems { if cgroup_path.1.exists() { log::debug!("remove cgroup {:?}", cgroup_path.1); let procs_path = cgroup_path.1.join(CGROUP_PROCS); - let procs = fs::read_to_string(procs_path)?; + let procs = fs::read_to_string(&procs_path).wrap_read(&procs_path)?; for line in procs.lines() { - let pid: i32 = line.parse()?; + let pid: i32 = line + .parse() + .map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err)) + .wrap_other(&procs_path)?; let _ = nix::sys::signal::kill(Pid::from_raw(pid), nix::sys::signal::SIGKILL); } @@ -167,27 +237,29 @@ impl CgroupManager for Manager { Ok(()) } - fn freeze(&self, state: FreezerState) -> Result<()> { + fn freeze(&self, state: FreezerState) -> Result<(), Self::Error> { let controller_opt = ControllerOpt { resources: &Default::default(), freezer_state: Some(state), oom_score_adj: None, disable_oom_killer: false, }; - Freezer::apply( + Ok(Freezer::apply( &controller_opt, self.subsystems.get(&CtrlType::Freezer).unwrap(), - ) + )?) } - fn stats(&self) -> Result { + fn stats(&self) -> Result { let mut stats = Stats::default(); for subsystem in &self.subsystems { match subsystem.0 { CtrlType::Cpu => stats.cpu.throttling = Cpu::stats(subsystem.1)?, CtrlType::CpuAcct => stats.cpu.usage = CpuAcct::stats(subsystem.1)?, - CtrlType::Pids => stats.pids = Pids::stats(subsystem.1)?, + CtrlType::Pids => { + stats.pids = Pids::stats(subsystem.1).map_err(V1ManagerError::PidsStats)? + } CtrlType::HugeTlb => stats.hugetlb = HugeTlb::stats(subsystem.1)?, CtrlType::Blkio => stats.blkio = Blkio::stats(subsystem.1)?, CtrlType::Memory => stats.memory = Memory::stats(subsystem.1)?, diff --git a/crates/libcgroups/src/v1/memory.rs b/crates/libcgroups/src/v1/memory.rs index 75f0d762c..e1c5fd396 100644 --- a/crates/libcgroups/src/v1/memory.rs +++ b/crates/libcgroups/src/v1/memory.rs @@ -1,16 +1,21 @@ use std::collections::HashMap; +use std::fmt::Display; use std::io::{prelude::*, Write}; +use std::num::ParseIntError; +use std::path::PathBuf; use std::{fs::OpenOptions, path::Path}; -use anyhow::{anyhow, bail, Result}; use nix::errno::Errno; -use super::Controller; -use crate::common::{self, ControllerOpt}; -use crate::stats::{self, parse_single_value, MemoryData, MemoryStats, StatsProvider}; +use crate::common::{self, ControllerOpt, WrapIoResult, WrappedIoError}; +use crate::stats::{ + self, parse_single_value, MemoryData, MemoryStats, ParseFlatKeyedDataError, StatsProvider, +}; use oci_spec::runtime::LinuxMemory; +use super::controller::Controller; + const CGROUP_MEMORY_SWAP_LIMIT: &str = "memory.memsw.limit_in_bytes"; const CGROUP_MEMORY_LIMIT: &str = "memory.limit_in_bytes"; const CGROUP_MEMORY_USAGE: &str = "memory.usage_in_bytes"; @@ -43,12 +48,56 @@ const MEMORY_LIMIT_IN_BYTES: &str = ".limit_in_bytes"; // Number of times memory usage hit limits const MEMORY_FAIL_COUNT: &str = ".failcnt"; +#[derive(Debug)] +pub enum MalformedThing { + Limit, + Usage, + MaxUsage, +} + +impl Display for MalformedThing { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + MalformedThing::Limit => f.write_str("memory limit"), + MalformedThing::Usage => f.write_str("memory usage"), + MalformedThing::MaxUsage => f.write_str("memory max usage"), + } + } +} + +#[derive(thiserror::Error, Debug)] +pub enum V1MemoryControllerError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("invalid swappiness value: {supplied}. valid range is 0-100")] + SwappinessOutOfRange { supplied: u64 }, + #[error("read malformed {thing} {limit} from {path}: {err}")] + MalformedValue { + thing: MalformedThing, + limit: String, + path: PathBuf, + err: ParseIntError, + }, + #[error( + "unable to set memory limit to {target} (current usage: {current}, peak usage: {peak})" + )] + UnableToSet { + target: i64, + current: u64, + peak: u64, + }, +} + pub struct Memory {} impl Controller for Memory { + type Error = V1MemoryControllerError; type Resource = LinuxMemory; - fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()> { + fn apply( + controller_opt: &ControllerOpt, + cgroup_root: &Path, + ) -> Result<(), V1MemoryControllerError> { log::debug!("Apply Memory cgroup config"); if let Some(memory) = &controller_opt.resources.memory() { @@ -77,10 +126,9 @@ impl Controller for Memory { )?; } else { // invalid swappiness value - return Err(anyhow!( - "Invalid swappiness value: {}. Valid range is 0-100", - swappiness - )); + return Err(V1MemoryControllerError::SwappinessOutOfRange { + supplied: swappiness, + }); } } @@ -105,11 +153,19 @@ impl Controller for Memory { controller_opt.resources.memory().as_ref() } } +#[derive(thiserror::Error, Debug)] +pub enum V1MemoryStatsError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("error parsing stat data: {0}")] + Parse(#[from] ParseFlatKeyedDataError), +} impl StatsProvider for Memory { + type Error = V1MemoryStatsError; type Stats = MemoryStats; - fn stats(cgroup_path: &Path) -> Result { + fn stats(cgroup_path: &Path) -> Result { let memory = Self::get_memory_data(cgroup_path, MEMORY_PREFIX)?; let memswap = Self::get_memory_data(cgroup_path, MEMORY_AND_SWAP_PREFIX)?; let kernel = Self::get_memory_data(cgroup_path, MEMORY_KERNEL_PREFIX)?; @@ -131,7 +187,10 @@ impl StatsProvider for Memory { } impl Memory { - fn get_memory_data(cgroup_path: &Path, file_prefix: &str) -> Result { + fn get_memory_data( + cgroup_path: &Path, + file_prefix: &str, + ) -> Result { let memory_data = MemoryData { usage: parse_single_value( &cgroup_path.join(format!("{file_prefix}{MEMORY_USAGE_IN_BYTES}")), @@ -150,7 +209,7 @@ impl Memory { Ok(memory_data) } - fn hierarchy_enabled(cgroup_path: &Path) -> Result { + fn hierarchy_enabled(cgroup_path: &Path) -> Result { let hierarchy_path = cgroup_path.join(MEMORY_USE_HIERARCHY); let hierarchy = common::read_cgroup_file(hierarchy_path)?; let enabled = matches!(hierarchy.trim(), "1"); @@ -158,18 +217,20 @@ impl Memory { Ok(enabled) } - fn get_stat_data(cgroup_path: &Path) -> Result> { + fn get_stat_data(cgroup_path: &Path) -> Result, ParseFlatKeyedDataError> { stats::parse_flat_keyed_data(&cgroup_path.join(MEMORY_STAT)) } - fn get_memory_usage(cgroup_root: &Path) -> Result { + fn get_memory_usage(cgroup_root: &Path) -> Result { let path = cgroup_root.join(CGROUP_MEMORY_USAGE); let mut contents = String::new(); OpenOptions::new() .create(false) .read(true) - .open(path)? - .read_to_string(&mut contents)?; + .open(&path) + .wrap_open(&path)? + .read_to_string(&mut contents) + .wrap_read(&path)?; contents = contents.trim().to_string(); @@ -177,18 +238,28 @@ impl Memory { return Ok(u64::MAX); } - let val = contents.parse::()?; + let val = + contents + .parse::() + .map_err(|err| V1MemoryControllerError::MalformedValue { + thing: MalformedThing::Usage, + limit: contents, + path, + err, + })?; Ok(val) } - fn get_memory_max_usage(cgroup_root: &Path) -> Result { + fn get_memory_max_usage(cgroup_root: &Path) -> Result { let path = cgroup_root.join(CGROUP_MEMORY_MAX_USAGE); let mut contents = String::new(); OpenOptions::new() .create(false) .read(true) - .open(path)? - .read_to_string(&mut contents)?; + .open(&path) + .wrap_open(&path)? + .read_to_string(&mut contents) + .wrap_read(&path)?; contents = contents.trim().to_string(); @@ -196,18 +267,28 @@ impl Memory { return Ok(u64::MAX); } - let val = contents.parse::()?; + let val = + contents + .parse::() + .map_err(|err| V1MemoryControllerError::MalformedValue { + thing: MalformedThing::MaxUsage, + limit: contents, + path, + err, + })?; Ok(val) } - fn get_memory_limit(cgroup_root: &Path) -> Result { + fn get_memory_limit(cgroup_root: &Path) -> Result { let path = cgroup_root.join(CGROUP_MEMORY_LIMIT); let mut contents = String::new(); OpenOptions::new() .create(false) .read(true) - .open(path)? - .read_to_string(&mut contents)?; + .open(&path) + .wrap_open(&path)? + .read_to_string(&mut contents) + .wrap_read(&path)?; contents = contents.trim().to_string(); @@ -215,21 +296,32 @@ impl Memory { return Ok(i64::MAX); } - let val = contents.parse::()?; + let val = + contents + .parse::() + .map_err(|err| V1MemoryControllerError::MalformedValue { + thing: MalformedThing::Limit, + limit: contents, + path, + err, + })?; Ok(val) } - fn set(val: T, path: &Path) -> std::io::Result<()> { + fn set(val: T, path: &Path) -> Result<(), WrappedIoError> { + let data = val.to_string(); OpenOptions::new() .create(false) .write(true) .truncate(true) - .open(path)? - .write_all(val.to_string().as_bytes())?; + .open(path) + .wrap_open(path)? + .write_all(data.as_bytes()) + .wrap_write(path, data)?; Ok(()) } - fn set_memory(val: i64, cgroup_root: &Path) -> Result<()> { + fn set_memory(val: i64, cgroup_root: &Path) -> Result<(), V1MemoryControllerError> { if val == 0 { return Ok(()); } @@ -239,27 +331,26 @@ impl Memory { Ok(_) => Ok(()), Err(e) => { // we need to look into the raw OS error for an EBUSY status - match e.raw_os_error() { + match e.inner().raw_os_error() { Some(code) => match Errno::from_i32(code) { Errno::EBUSY => { let usage = Self::get_memory_usage(cgroup_root)?; let max_usage = Self::get_memory_max_usage(cgroup_root)?; - bail!( - "unable to set memory limit to {} (current usage: {}, peak usage: {})", - val, - usage, - max_usage, - ) + Err(V1MemoryControllerError::UnableToSet { + target: val, + current: usage, + peak: max_usage, + }) } - _ => bail!(e), + _ => Err(e)?, }, - None => bail!(e), + None => Err(e)?, } } } } - fn set_swap(swap: i64, cgroup_root: &Path) -> Result<()> { + fn set_swap(swap: i64, cgroup_root: &Path) -> Result<(), V1MemoryControllerError> { if swap == 0 { return Ok(()); } @@ -273,7 +364,7 @@ impl Memory { swap: i64, is_updated: bool, cgroup_root: &Path, - ) -> Result<()> { + ) -> Result<(), V1MemoryControllerError> { // According to runc we need to change the write sequence of // limit and swap so it won't fail, because the new and old // values don't fit the kernel's validation @@ -288,7 +379,7 @@ impl Memory { Ok(()) } - fn apply(resource: &LinuxMemory, cgroup_root: &Path) -> Result<()> { + fn apply(resource: &LinuxMemory, cgroup_root: &Path) -> Result<(), V1MemoryControllerError> { match resource.limit() { Some(limit) => { let current_limit = Self::get_memory_limit(cgroup_root)?; diff --git a/crates/libcgroups/src/v1/mod.rs b/crates/libcgroups/src/v1/mod.rs index e80e9a01e..59405acd2 100644 --- a/crates/libcgroups/src/v1/mod.rs +++ b/crates/libcgroups/src/v1/mod.rs @@ -14,6 +14,5 @@ mod network_priority; pub mod perf_event; mod pids; pub mod util; -pub use controller::Controller; pub use controller_type::ControllerType; pub use manager::Manager; diff --git a/crates/libcgroups/src/v1/network_classifier.rs b/crates/libcgroups/src/v1/network_classifier.rs index 6c27d2abd..a0760c885 100644 --- a/crates/libcgroups/src/v1/network_classifier.rs +++ b/crates/libcgroups/src/v1/network_classifier.rs @@ -1,22 +1,21 @@ use std::path::Path; -use anyhow::{Context, Result}; - -use super::Controller; -use crate::common::{self, ControllerOpt}; +use crate::common::{self, ControllerOpt, WrappedIoError}; use oci_spec::runtime::LinuxNetwork; +use super::controller::Controller; + pub struct NetworkClassifier {} impl Controller for NetworkClassifier { + type Error = WrappedIoError; type Resource = LinuxNetwork; - fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()> { + fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<(), Self::Error> { log::debug!("Apply NetworkClassifier cgroup config"); if let Some(network) = Self::needs_to_handle(controller_opt) { - Self::apply(cgroup_root, network) - .context("failed to apply network classifier resource restrictions")?; + Self::apply(cgroup_root, network)?; } Ok(()) @@ -28,7 +27,7 @@ impl Controller for NetworkClassifier { } impl NetworkClassifier { - fn apply(root_path: &Path, network: &LinuxNetwork) -> Result<()> { + fn apply(root_path: &Path, network: &LinuxNetwork) -> Result<(), WrappedIoError> { if let Some(class_id) = network.class_id() { common::write_cgroup_file(root_path.join("net_cls.classid"), class_id)?; } diff --git a/crates/libcgroups/src/v1/network_priority.rs b/crates/libcgroups/src/v1/network_priority.rs index c55533378..cc922a88d 100644 --- a/crates/libcgroups/src/v1/network_priority.rs +++ b/crates/libcgroups/src/v1/network_priority.rs @@ -1,22 +1,21 @@ use std::path::Path; -use anyhow::{Context, Result}; - -use super::Controller; -use crate::common::{self, ControllerOpt}; +use crate::common::{self, ControllerOpt, WrappedIoError}; use oci_spec::runtime::LinuxNetwork; +use super::controller::Controller; + pub struct NetworkPriority {} impl Controller for NetworkPriority { + type Error = WrappedIoError; type Resource = LinuxNetwork; - fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()> { + fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<(), Self::Error> { log::debug!("Apply NetworkPriority cgroup config"); if let Some(network) = Self::needs_to_handle(controller_opt) { - Self::apply(cgroup_root, network) - .context("failed to apply network priority resource restrictions")?; + Self::apply(cgroup_root, network)?; } Ok(()) @@ -28,7 +27,7 @@ impl Controller for NetworkPriority { } impl NetworkPriority { - fn apply(root_path: &Path, network: &LinuxNetwork) -> Result<()> { + fn apply(root_path: &Path, network: &LinuxNetwork) -> Result<(), WrappedIoError> { if let Some(ni_priorities) = network.priorities() { let priorities: String = ni_priorities.iter().map(|p| p.to_string()).collect(); common::write_cgroup_file_str(root_path.join("net_prio.ifpriomap"), priorities.trim())?; diff --git a/crates/libcgroups/src/v1/perf_event.rs b/crates/libcgroups/src/v1/perf_event.rs index 5f6c3877d..a795cd252 100644 --- a/crates/libcgroups/src/v1/perf_event.rs +++ b/crates/libcgroups/src/v1/perf_event.rs @@ -1,14 +1,15 @@ -use super::Controller; -use crate::common::ControllerOpt; -use anyhow::Result; +use crate::common::{ControllerOpt, WrappedIoError}; use std::path::Path; +use super::controller::Controller; + pub struct PerfEvent {} impl Controller for PerfEvent { + type Error = WrappedIoError; type Resource = (); - fn apply(_controller_opt: &ControllerOpt, _cgroup_root: &Path) -> Result<()> { + fn apply(_controller_opt: &ControllerOpt, _cgroup_root: &Path) -> Result<(), Self::Error> { Ok(()) } //no need to handle any case diff --git a/crates/libcgroups/src/v1/pids.rs b/crates/libcgroups/src/v1/pids.rs index cd7ffb18c..5ee4325c9 100644 --- a/crates/libcgroups/src/v1/pids.rs +++ b/crates/libcgroups/src/v1/pids.rs @@ -1,27 +1,27 @@ use std::path::Path; -use anyhow::{Context, Result}; - -use super::Controller; use crate::{ - common::{self, ControllerOpt}, - stats::{self, PidStats, StatsProvider}, + common::{self, ControllerOpt, WrappedIoError}, + stats::{self, PidStats, PidStatsError, StatsProvider}, }; use oci_spec::runtime::LinuxPids; +use super::controller::Controller; + // Contains the maximum allowed number of active pids const CGROUP_PIDS_MAX: &str = "pids.max"; pub struct Pids {} impl Controller for Pids { + type Error = WrappedIoError; type Resource = LinuxPids; - fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()> { + fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<(), Self::Error> { log::debug!("Apply pids cgroup config"); if let Some(pids) = &controller_opt.resources.pids() { - Self::apply(cgroup_root, pids).context("failed to apply pids resource restrictions")?; + Self::apply(cgroup_root, pids)?; } Ok(()) @@ -33,15 +33,16 @@ impl Controller for Pids { } impl StatsProvider for Pids { + type Error = PidStatsError; type Stats = PidStats; - fn stats(cgroup_path: &Path) -> Result { + fn stats(cgroup_path: &Path) -> Result { stats::pid_stats(cgroup_path) } } impl Pids { - fn apply(root_path: &Path, pids: &LinuxPids) -> Result<()> { + fn apply(root_path: &Path, pids: &LinuxPids) -> Result<(), WrappedIoError> { let limit = if pids.limit() > 0 { pids.limit().to_string() } else { diff --git a/crates/libcgroups/src/v1/util.rs b/crates/libcgroups/src/v1/util.rs index acc7bb3f1..a33a109f4 100644 --- a/crates/libcgroups/src/v1/util.rs +++ b/crates/libcgroups/src/v1/util.rs @@ -1,16 +1,26 @@ use std::{collections::HashMap, path::PathBuf}; -use anyhow::{anyhow, Context, Result}; -use procfs::process::Process; +use procfs::{process::Process, ProcError}; use super::{controller_type::CONTROLLERS, ControllerType}; +#[derive(thiserror::Error, Debug)] +pub enum V1MountPointError { + #[error("failed to read process info from /proc/self: {0}")] + ReadSelf(ProcError), + #[error("failed to get mountinfo: {0}")] + MountInfo(ProcError), + #[error("could not find mountpoint for {subsystem}")] + NotFound { subsystem: ControllerType }, +} + /// List all cgroup v1 subsystem mount points on the system. This can include unsupported /// subsystems, comounted controllers and named hierarchies. -pub fn list_subsystem_mount_points() -> Result> { - Ok(Process::myself()? +pub fn list_subsystem_mount_points() -> Result, V1MountPointError> { + Ok(Process::myself() + .map_err(V1MountPointError::ReadSelf)? .mountinfo() - .context("failed to get mountinfo")? + .map_err(V1MountPointError::MountInfo)? .into_iter() .filter(|m| m.fs_type == "cgroup") .map(|m| m.mount_point) @@ -18,7 +28,8 @@ pub fn list_subsystem_mount_points() -> Result> { } /// List the mount points of all currently supported cgroup subsystems. -pub fn list_supported_mount_points() -> Result> { +pub fn list_supported_mount_points() -> Result, V1MountPointError> +{ let mut mount_paths = HashMap::with_capacity(CONTROLLERS.len()); for controller in CONTROLLERS { @@ -30,38 +41,41 @@ pub fn list_supported_mount_points() -> Result> Ok(mount_paths) } -pub fn get_subsystem_mount_point(subsystem: &ControllerType) -> Result { - let subsystem = subsystem.to_string(); - Process::myself()? +pub fn get_subsystem_mount_point(subsystem: &ControllerType) -> Result { + let subsystem_name = subsystem.to_string(); + Process::myself() + .map_err(V1MountPointError::ReadSelf)? .mountinfo() - .context("failed to get mountinfo")? + .map_err(V1MountPointError::MountInfo)? .into_iter() .find(|m| { if m.fs_type == "cgroup" { // Some systems mount net_prio and net_cls in the same directory // other systems mount them in their own diretories. This // should handle both cases. - if subsystem == "net_cls" { + if subsystem_name == "net_cls" { return m.mount_point.ends_with("net_cls,net_prio") || m.mount_point.ends_with("net_prio,net_cls") || m.mount_point.ends_with("net_cls"); - } else if subsystem == "net_prio" { + } else if subsystem_name == "net_prio" { return m.mount_point.ends_with("net_cls,net_prio") || m.mount_point.ends_with("net_prio,net_cls") || m.mount_point.ends_with("net_prio"); } - if subsystem == "cpu" { + if subsystem_name == "cpu" { return m.mount_point.ends_with("cpu,cpuacct") || m.mount_point.ends_with("cpu"); } - if subsystem == "cpuacct" { + if subsystem_name == "cpuacct" { return m.mount_point.ends_with("cpu,cpuacct") || m.mount_point.ends_with("cpuacct"); } } - m.mount_point.ends_with(&subsystem) + m.mount_point.ends_with(&subsystem_name) }) .map(|m| m.mount_point) - .ok_or_else(|| anyhow!("could not find mountpoint for {}", subsystem)) + .ok_or(V1MountPointError::NotFound { + subsystem: *subsystem, + }) } diff --git a/crates/libcgroups/src/v2/controller.rs b/crates/libcgroups/src/v2/controller.rs index 7287c1eb3..e88b45887 100644 --- a/crates/libcgroups/src/v2/controller.rs +++ b/crates/libcgroups/src/v2/controller.rs @@ -1,8 +1,9 @@ -use anyhow::Result; use std::path::Path; use crate::common::ControllerOpt; -pub trait Controller { - fn apply(controller_opt: &ControllerOpt, cgroup_path: &Path) -> Result<()>; +pub(super) trait Controller { + type Error; + + fn apply(controller_opt: &ControllerOpt, cgroup_path: &Path) -> Result<(), Self::Error>; } diff --git a/crates/libcgroups/src/v2/cpu.rs b/crates/libcgroups/src/v2/cpu.rs index 1314ea6de..8b4fc5e98 100644 --- a/crates/libcgroups/src/v2/cpu.rs +++ b/crates/libcgroups/src/v2/cpu.rs @@ -1,8 +1,11 @@ -use anyhow::{bail, Context, Result}; -use std::{borrow::Cow, path::Path}; +use std::{ + borrow::Cow, + num::ParseIntError, + path::{Path, PathBuf}, +}; use crate::{ - common::{self, ControllerOpt}, + common::{self, ControllerOpt, WrappedIoError}, stats::{self, CpuStats, StatsProvider}, }; @@ -20,32 +23,64 @@ const MAX_CPU_WEIGHT: u64 = 10000; const CPU_STAT: &str = "cpu.stat"; const CPU_PSI: &str = "cpu.pressure"; +#[derive(thiserror::Error, Debug)] +pub enum V2CpuControllerError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("realtime is not supported on v2 yet")] + RealtimeV2, +} + pub struct Cpu {} impl Controller for Cpu { - fn apply(controller_opt: &ControllerOpt, path: &Path) -> Result<()> { + type Error = V2CpuControllerError; + + fn apply(controller_opt: &ControllerOpt, path: &Path) -> Result<(), Self::Error> { if let Some(cpu) = &controller_opt.resources.cpu() { - Self::apply(path, cpu).context("failed to apply cpu resource restrictions")?; + Self::apply(path, cpu)?; } Ok(()) } } +#[derive(thiserror::Error, Debug)] +pub enum V2CpuStatsError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("failed parsing value {value} for field {field} in {path}: {err}")] + ParseField { + value: String, + field: String, + path: PathBuf, + err: ParseIntError, + }, +} + impl StatsProvider for Cpu { + type Error = V2CpuStatsError; type Stats = CpuStats; - fn stats(cgroup_path: &Path) -> Result { + fn stats(cgroup_path: &Path) -> Result { let mut stats = CpuStats::default(); + let stats_path = cgroup_path.join(CPU_STAT); - let stat_content = common::read_cgroup_file(cgroup_path.join(CPU_STAT))?; + let stat_content = common::read_cgroup_file(&stats_path)?; for entry in stat_content.lines() { let parts: Vec<&str> = entry.split_ascii_whitespace().collect(); if parts.len() != 2 { continue; } - let value = parts[1].parse()?; + let value = parts[1] + .parse() + .map_err(|err| V2CpuStatsError::ParseField { + value: parts[1].into(), + field: parts[0].into(), + path: stats_path.clone(), + err, + })?; match parts[0] { "usage_usec" => stats.usage.usage_total = value, "user_usec" => stats.usage.usage_user = value, @@ -54,16 +89,15 @@ impl StatsProvider for Cpu { } } - stats.psi = - stats::psi_stats(&cgroup_path.join(CPU_PSI)).context("could not read cpu psi")?; + stats.psi = stats::psi_stats(&cgroup_path.join(CPU_PSI))?; Ok(stats) } } impl Cpu { - fn apply(path: &Path, cpu: &LinuxCpu) -> Result<()> { + fn apply(path: &Path, cpu: &LinuxCpu) -> Result<(), V2CpuControllerError> { if Self::is_realtime_requested(cpu) { - bail!("realtime is not supported on cgroup v2 yet"); + return Err(V2CpuControllerError::RealtimeV2); } if let Some(mut shares) = cpu.shares() { @@ -126,7 +160,10 @@ impl Cpu { false } - fn create_period_only_value(cpu_max_file: &Path, period: u64) -> Result>> { + fn create_period_only_value( + cpu_max_file: &Path, + period: u64, + ) -> Result>, V2CpuControllerError> { let old_cpu_max = common::read_cgroup_file(cpu_max_file)?; if let Some(old_quota) = old_cpu_max.split_whitespace().next() { return Ok(Some(format!("{old_quota} {period}").into())); diff --git a/crates/libcgroups/src/v2/cpuset.rs b/crates/libcgroups/src/v2/cpuset.rs index 924d9fccb..dfbce6e37 100644 --- a/crates/libcgroups/src/v2/cpuset.rs +++ b/crates/libcgroups/src/v2/cpuset.rs @@ -1,7 +1,6 @@ -use anyhow::{Context, Result}; use std::path::Path; -use crate::common::{self, ControllerOpt}; +use crate::common::{self, ControllerOpt, WrappedIoError}; use oci_spec::runtime::LinuxCpu; use super::controller::Controller; @@ -12,10 +11,11 @@ const CGROUP_CPUSET_MEMS: &str = "cpuset.mems"; pub struct CpuSet {} impl Controller for CpuSet { - fn apply(controller_opt: &ControllerOpt, cgroup_path: &Path) -> Result<()> { + type Error = WrappedIoError; + + fn apply(controller_opt: &ControllerOpt, cgroup_path: &Path) -> Result<(), Self::Error> { if let Some(cpuset) = &controller_opt.resources.cpu() { - Self::apply(cgroup_path, cpuset) - .context("failed to apply cpuset resource restrictions")?; + Self::apply(cgroup_path, cpuset)?; } Ok(()) @@ -23,7 +23,7 @@ impl Controller for CpuSet { } impl CpuSet { - fn apply(path: &Path, cpuset: &LinuxCpu) -> Result<()> { + fn apply(path: &Path, cpuset: &LinuxCpu) -> Result<(), WrappedIoError> { if let Some(cpus) = &cpuset.cpus() { common::write_cgroup_file_str(path.join(CGROUP_CPUSET_CPUS), cpus)?; } diff --git a/crates/libcgroups/src/v2/devices/bpf.rs b/crates/libcgroups/src/v2/devices/bpf.rs index d523afd5f..21ae1f652 100644 --- a/crates/libcgroups/src/v2/devices/bpf.rs +++ b/crates/libcgroups/src/v2/devices/bpf.rs @@ -4,10 +4,17 @@ pub struct ProgramInfo { pub fd: i32, } +#[derive(thiserror::Error, Debug)] +pub enum BpfError { + #[error(transparent)] + Errno(#[from] errno::Errno), + #[error("Failed to increase rlimit")] + FailedToIncreaseRLimit, +} + #[cfg_attr(test, automock)] pub mod prog { use super::ProgramInfo; - use anyhow::{bail, Result}; use std::os::unix::io::RawFd; use std::ptr; @@ -31,7 +38,7 @@ pub mod prog { bpf_prog_attach, bpf_prog_detach2, bpf_prog_get_fd_by_id, bpf_prog_load, bpf_prog_query, }; - pub fn load(license: &str, insns: &[u8]) -> Result { + pub fn load(license: &str, insns: &[u8]) -> Result { let insns_cnt = insns.len() / std::mem::size_of::(); let insns = insns as *const _ as *const bpf_insn; let opts = libbpf_sys::bpf_prog_load_opts { @@ -59,7 +66,7 @@ pub mod prog { } /// Given a fd for a cgroup, collect the programs associated with it - pub fn query(cgroup_fd: RawFd) -> Result> { + pub fn query(cgroup_fd: RawFd) -> Result, super::BpfError> { let mut prog_ids: Vec = vec![0_u32; 64]; let mut attach_flags = 0_u32; for _ in 0..10 { @@ -110,7 +117,7 @@ pub mod prog { Ok(prog_fds) } - pub fn detach2(prog_fd: RawFd, cgroup_fd: RawFd) -> Result<()> { + pub fn detach2(prog_fd: RawFd, cgroup_fd: RawFd) -> Result<(), super::BpfError> { #[allow(unused_unsafe)] let ret = unsafe { bpf_prog_detach2(prog_fd, cgroup_fd, BPF_CGROUP_DEVICE) }; if ret != 0 { @@ -119,7 +126,7 @@ pub mod prog { Ok(()) } - pub fn attach(prog_fd: RawFd, cgroup_fd: RawFd) -> Result<()> { + pub fn attach(prog_fd: RawFd, cgroup_fd: RawFd) -> Result<(), super::BpfError> { #[allow(unused_unsafe)] let ret = unsafe { bpf_prog_attach(prog_fd, cgroup_fd, BPF_CGROUP_DEVICE, BPF_F_ALLOW_MULTI) }; @@ -130,7 +137,7 @@ pub mod prog { Ok(()) } - pub fn bump_memlock_rlimit() -> Result<()> { + pub fn bump_memlock_rlimit() -> Result<(), super::BpfError> { let rlimit = rlimit { rlim_cur: 128 << 20, rlim_max: 128 << 20, @@ -138,7 +145,7 @@ pub mod prog { #[allow(unused_unsafe)] if unsafe { setrlimit(RLIMIT_MEMLOCK, &rlimit) } != 0 { - bail!("Failed to increase rlimit"); + return Err(super::BpfError::FailedToIncreaseRLimit); } Ok(()) diff --git a/crates/libcgroups/src/v2/devices/controller.rs b/crates/libcgroups/src/v2/devices/controller.rs index f3592a883..8c324417f 100644 --- a/crates/libcgroups/src/v2/devices/controller.rs +++ b/crates/libcgroups/src/v2/devices/controller.rs @@ -1,8 +1,8 @@ use std::os::unix::io::AsRawFd; use std::path::Path; -use anyhow::Result; - +use super::bpf::BpfError; +use super::program::ProgramError; use super::*; use nix::fcntl::OFlag; use nix::sys::stat::Mode; @@ -21,8 +21,23 @@ const LICENSE: &str = "Apache"; pub struct Devices {} +#[derive(thiserror::Error, Debug)] +pub enum DevicesControllerError { + #[error("bpf error: {0}")] + Bpf(#[from] BpfError), + #[error("nix error: {0}")] + Nix(#[from] nix::Error), + #[error("program error: {0}")] + Program(#[from] ProgramError), +} + impl Controller for Devices { - fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()> { + type Error = DevicesControllerError; + + fn apply( + controller_opt: &ControllerOpt, + cgroup_root: &Path, + ) -> Result<(), DevicesControllerError> { #[cfg(not(feature = "cgroupsv2_devices"))] return Ok(()); @@ -35,7 +50,7 @@ impl Devices { pub fn apply_devices( cgroup_root: &Path, linux_devices: &Option>, - ) -> Result<()> { + ) -> Result<(), DevicesControllerError> { log::debug!("Apply Devices cgroup config"); // FIXME: should we start as "deny all"? @@ -45,7 +60,7 @@ impl Devices { if let Some(devices) = linux_devices { for d in devices { log::debug!("apply user defined rule: {:?}", d); - emulator.add_rule(d)?; + emulator.add_rule(d); } } @@ -56,7 +71,7 @@ impl Devices { .concat() { log::debug!("apply default rule: {:?}", d); - emulator.add_rule(&d)?; + emulator.add_rule(&d); } let prog = program::Program::from_rules(&emulator.rules, emulator.default_allow)?; diff --git a/crates/libcgroups/src/v2/devices/emulator.rs b/crates/libcgroups/src/v2/devices/emulator.rs index 16757126c..065f78c05 100644 --- a/crates/libcgroups/src/v2/devices/emulator.rs +++ b/crates/libcgroups/src/v2/devices/emulator.rs @@ -1,4 +1,3 @@ -use anyhow::Result; use oci_spec::runtime::{LinuxDeviceCgroup, LinuxDeviceType}; // For cgroup v1 compatibility, runc implements a device emulator to caculate the final rules given @@ -28,29 +27,27 @@ impl Emulator { } } - pub fn add_rules(&mut self, rules: &[LinuxDeviceCgroup]) -> Result<()> { + pub fn add_rules(&mut self, rules: &[LinuxDeviceCgroup]) { for rule in rules { - self.add_rule(rule)?; + self.add_rule(rule); } - Ok(()) } - pub fn add_rule(&mut self, rule: &LinuxDeviceCgroup) -> Result<()> { + pub fn add_rule(&mut self, rule: &LinuxDeviceCgroup) { // special case, switch to blacklist or whitelist and clear all existing rules // NOTE: we ignore other fields when type='a', this is same as cgroup v1 and runc if rule.typ().unwrap_or_default() == LinuxDeviceType::A { self.default_allow = rule.allow(); self.rules.clear(); - return Ok(()); + return; } // empty access match nothing, just discard this rule if rule.access().is_none() { - return Ok(()); + return; } self.rules.push(rule.clone()); - Ok(()) } } @@ -79,7 +76,7 @@ mod tests { .unwrap(); // act - emulator.add_rule(&cgroup).expect("add type A rule"); + emulator.add_rule(&cgroup); // assert assert_eq!(emulator.rules.len(), 0); @@ -93,7 +90,7 @@ mod tests { let cgroup = LinuxDeviceCgroupBuilder::default().build().unwrap(); // act - emulator.add_rule(&cgroup).expect("add empty rule"); + emulator.add_rule(&cgroup); // assert assert_eq!(emulator.rules.len(), 0); @@ -112,7 +109,7 @@ mod tests { .unwrap(); // act - emulator.add_rule(&cgroup).expect("add permission rule"); + emulator.add_rule(&cgroup); // assert let top_rule = emulator.rules.first().unwrap(); diff --git a/crates/libcgroups/src/v2/devices/program.rs b/crates/libcgroups/src/v2/devices/program.rs index 06e435d0f..c091ac8a4 100644 --- a/crates/libcgroups/src/v2/devices/program.rs +++ b/crates/libcgroups/src/v2/devices/program.rs @@ -1,4 +1,3 @@ -use anyhow::{bail, Result}; use oci_spec::runtime::*; use rbpf::disassembler::disassemble; @@ -9,8 +8,23 @@ pub struct Program { prog: BpfCode, } +#[derive(thiserror::Error, Debug)] +pub enum ProgramError { + #[error("io error: {0}")] + Io(#[from] std::io::Error), + #[error("invalid access: {0}")] + InvalidAccess(char), + #[error("{0} device not supported")] + DeviceNotSupported(&'static str), + #[error("wildcard device type should be removed when cleaning rules")] + WildcardDevice, +} + impl Program { - pub fn from_rules(rules: &[LinuxDeviceCgroup], default_allow: bool) -> Result { + pub fn from_rules( + rules: &[LinuxDeviceCgroup], + default_allow: bool, + ) -> Result { let mut prog = Program { prog: BpfCode::new(), }; @@ -89,7 +103,7 @@ impl Program { .push(); } - fn add_rule(&mut self, rule: &LinuxDeviceCgroup) -> Result<()> { + fn add_rule(&mut self, rule: &LinuxDeviceCgroup) -> Result<(), ProgramError> { let dev_type = bpf_dev_type(rule.typ().unwrap_or_default())?; let access = bpf_access(rule.access().clone().unwrap_or_default())?; let has_access = access @@ -188,7 +202,7 @@ impl Program { major: u32, minor: u32, access: String, - ) -> Result { + ) -> Result { let mut mem = bpf_cgroup_dev_ctx(typ, major, minor, access)?; let vm = rbpf::EbpfVmRaw::new(Some(self.prog.into_bytes()))?; let result = vm.execute_program(&mut mem[..])?; @@ -196,27 +210,25 @@ impl Program { } } -fn bpf_dev_type(typ: LinuxDeviceType) -> Result { +fn bpf_dev_type(typ: LinuxDeviceType) -> Result { let dev_type: u32 = match typ { LinuxDeviceType::C => libbpf_sys::BPF_DEVCG_DEV_CHAR, - LinuxDeviceType::U => bail!("unbuffered char device not supported"), + LinuxDeviceType::U => return Err(ProgramError::DeviceNotSupported("unbuffered char")), LinuxDeviceType::B => libbpf_sys::BPF_DEVCG_DEV_BLOCK, - LinuxDeviceType::P => bail!("pipe device not supported"), - LinuxDeviceType::A => { - bail!("wildcard device type should be removed when cleaning rules") - } + LinuxDeviceType::P => return Err(ProgramError::DeviceNotSupported("pipe device")), + LinuxDeviceType::A => return Err(ProgramError::WildcardDevice), }; Ok(dev_type) } -fn bpf_access(access: String) -> Result { +fn bpf_access(access: String) -> Result { let mut v = 0_u32; for c in access.chars() { let cur_access = match c { 'r' => libbpf_sys::BPF_DEVCG_ACC_READ, 'w' => libbpf_sys::BPF_DEVCG_ACC_WRITE, 'm' => libbpf_sys::BPF_DEVCG_ACC_MKNOD, - _ => bail!("invalid access: {}", c), + _ => return Err(ProgramError::InvalidAccess(c)), }; v |= cur_access; } @@ -228,7 +240,7 @@ fn bpf_cgroup_dev_ctx( major: u32, minor: u32, access: String, -) -> Result> { +) -> Result, ProgramError> { let mut mem = Vec::with_capacity(12); let mut type_access = 0_u32; @@ -248,15 +260,16 @@ fn bpf_cgroup_dev_ctx( #[cfg(test)] mod tests { use super::*; + use anyhow::Result; use oci_spec::runtime::LinuxDeviceCgroupBuilder; fn build_bpf_program(rules: &Option>) -> Result { let mut em = crate::v2::devices::emulator::Emulator::with_default_allow(false); if let Some(rules) = rules { - em.add_rules(rules)?; + em.add_rules(rules); } - Program::from_rules(&em.rules, em.default_allow) + Ok(Program::from_rules(&em.rules, em.default_allow)?) } #[test] diff --git a/crates/libcgroups/src/v2/freezer.rs b/crates/libcgroups/src/v2/freezer.rs index 6c5b97ba8..0428568e1 100644 --- a/crates/libcgroups/src/v2/freezer.rs +++ b/crates/libcgroups/src/v2/freezer.rs @@ -1,25 +1,46 @@ -use anyhow::{bail, Context, Result}; use std::{ fs::OpenOptions, io::{BufRead, BufReader, Read, Seek, Write}, path::Path, - str, thread, + str::{self, Utf8Error}, + thread, time::Duration, }; -use crate::common::{ControllerOpt, FreezerState}; +use crate::common::{ControllerOpt, FreezerState, WrapIoResult, WrappedIoError}; use super::controller::Controller; const CGROUP_FREEZE: &str = "cgroup.freeze"; const CGROUP_EVENTS: &str = "cgroup.events"; +#[derive(thiserror::Error, Debug)] +pub enum V2FreezerError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("freezer not supported: {0}")] + NotSupported(WrappedIoError), + #[error("expected \"cgroup.freeze\" to be in state {expected:?} but was in {actual:?}")] + ExepectedToBe { + expected: FreezerState, + actual: FreezerState, + }, + #[error("unexpected \"cgroup.freeze\" state: {state}")] + UnknownState { state: String }, + #[error("timeout of {0} ms reached waiting for the cgroup to freeze")] + Timeout(u128), + #[error("invalid utf8: {0}")] + InvalidUtf8(#[from] Utf8Error), +} + pub struct Freezer {} impl Controller for Freezer { - fn apply(controller_opt: &ControllerOpt, cgroup_path: &Path) -> Result<()> { + type Error = V2FreezerError; + + fn apply(controller_opt: &ControllerOpt, cgroup_path: &Path) -> Result<(), Self::Error> { if let Some(freezer_state) = controller_opt.freezer_state { - Self::apply(freezer_state, cgroup_path).context("failed to apply freezer")?; + Self::apply(freezer_state, cgroup_path)?; } Ok(()) @@ -27,62 +48,70 @@ impl Controller for Freezer { } impl Freezer { - fn apply(freezer_state: FreezerState, path: &Path) -> Result<()> { + fn apply(freezer_state: FreezerState, path: &Path) -> Result<(), V2FreezerError> { let state_str = match freezer_state { FreezerState::Undefined => return Ok(()), FreezerState::Frozen => "1", FreezerState::Thawed => "0", }; - match OpenOptions::new() - .create(false) - .write(true) - .open(path.join(CGROUP_FREEZE)) - { - Err(e) => { - if let FreezerState::Frozen = freezer_state { - bail!("freezer not supported {}", e); + let target = path.join(CGROUP_FREEZE); + match OpenOptions::new().create(false).write(true).open(&target) { + Err(err) => { + if freezer_state == FreezerState::Frozen { + return Err(V2FreezerError::NotSupported(WrappedIoError::Open { + err, + path: target, + })); } return Ok(()); } - Ok(mut file) => file.write_all(state_str.as_bytes())?, + Ok(mut file) => file + .write_all(state_str.as_bytes()) + .wrap_write(target, state_str)?, }; // confirm that the cgroup did actually change states. let actual_state = Self::read_freezer_state(path)?; if !actual_state.eq(&freezer_state) { - bail!( - "expected \"cgroup.freeze\" to be in state {:?} but was in {:?}", - freezer_state, - actual_state - ); + return Err(V2FreezerError::ExepectedToBe { + expected: freezer_state, + actual: actual_state, + }); } Ok(()) } - fn read_freezer_state(path: &Path) -> Result { + fn read_freezer_state(path: &Path) -> Result { + let target = path.join(CGROUP_FREEZE); let mut buf = [0; 1]; OpenOptions::new() .create(false) .read(true) - .open(path.join(CGROUP_FREEZE))? - .read_exact(&mut buf)?; + .open(&target) + .wrap_open(&target)? + .read_exact(&mut buf) + .wrap_read(&target)?; let state = str::from_utf8(&buf)?; match state { "0" => Ok(FreezerState::Thawed), "1" => Self::wait_frozen(path), - _ => bail!("unknown \"cgroup.freeze\" state: {}", state), + _ => Err(V2FreezerError::UnknownState { + state: state.into(), + }), } } // wait_frozen polls cgroup.events until it sees "frozen 1" in it. - fn wait_frozen(path: &Path) -> Result { + fn wait_frozen(path: &Path) -> Result { + let path = path.join(CGROUP_EVENTS); let f = OpenOptions::new() .create(false) .read(true) - .open(path.join(CGROUP_EVENTS))?; + .open(&path) + .wrap_open(&path)?; let mut f = BufReader::new(f); let wait_time = Duration::from_millis(10); @@ -92,13 +121,10 @@ impl Freezer { loop { if iter == max_iter { - bail!( - "timeout of {} ms reached waiting for the cgroup to freeze", - wait_time.as_millis() * max_iter - ); + return Err(V2FreezerError::Timeout(wait_time.as_millis() * max_iter)); } line.clear(); - let num_bytes = f.read_line(&mut line)?; + let num_bytes = f.read_line(&mut line).wrap_read(&path)?; if num_bytes == 0 { break; } @@ -111,7 +137,7 @@ impl Freezer { } iter += 1; thread::sleep(wait_time); - f.rewind()?; + f.rewind().wrap_other(&path)?; } } diff --git a/crates/libcgroups/src/v2/hugetlb.rs b/crates/libcgroups/src/v2/hugetlb.rs index fd75a59f2..576543114 100644 --- a/crates/libcgroups/src/v2/hugetlb.rs +++ b/crates/libcgroups/src/v2/hugetlb.rs @@ -1,33 +1,65 @@ -use anyhow::{bail, Context, Result}; -use std::{collections::HashMap, path::Path}; +use std::{ + collections::HashMap, + num::ParseIntError, + path::{Path, PathBuf}, +}; use super::controller::Controller; use crate::{ - common::{self, ControllerOpt}, - stats::{parse_single_value, supported_page_sizes, HugeTlbStats, StatsProvider}, + common::{self, ControllerOpt, EitherError, MustBePowerOfTwo, WrappedIoError}, + stats::{ + parse_single_value, supported_page_sizes, HugeTlbStats, StatsProvider, + SupportedPageSizesError, + }, }; use oci_spec::runtime::LinuxHugepageLimit; +#[derive(thiserror::Error, Debug)] +pub enum V2HugeTlbControllerError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("malformed page size {page_size}: {err}")] + MalformedPageSize { + page_size: String, + err: EitherError, + }, +} + pub struct HugeTlb {} impl Controller for HugeTlb { - fn apply(controller_opt: &ControllerOpt, cgroup_root: &std::path::Path) -> Result<()> { + type Error = V2HugeTlbControllerError; + + fn apply( + controller_opt: &ControllerOpt, + cgroup_root: &std::path::Path, + ) -> Result<(), Self::Error> { log::debug!("Apply hugetlb cgroup v2 config"); if let Some(hugepage_limits) = controller_opt.resources.hugepage_limits() { for hugetlb in hugepage_limits { - Self::apply(cgroup_root, hugetlb) - .context("failed to apply hugetlb resource restrictions")? + Self::apply(cgroup_root, hugetlb)? } } Ok(()) } } +#[derive(thiserror::Error, Debug)] +pub enum V2HugeTlbStatsError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("getting supported huge page sizes: {0}")] + SupportedPageSizes(#[from] SupportedPageSizesError), + #[error("failed to parse max value for {path}: {err}")] + ParseMax { path: PathBuf, err: ParseIntError }, +} + impl StatsProvider for HugeTlb { + type Error = V2HugeTlbStatsError; type Stats = HashMap; - fn stats(cgroup_path: &Path) -> Result { + fn stats(cgroup_path: &Path) -> Result { let page_sizes = supported_page_sizes()?; let mut hugetlb_stats = HashMap::with_capacity(page_sizes.len()); @@ -43,15 +75,29 @@ impl StatsProvider for HugeTlb { } impl HugeTlb { - fn apply(root_path: &Path, hugetlb: &LinuxHugepageLimit) -> Result<()> { - let page_size: String = hugetlb + fn apply( + root_path: &Path, + hugetlb: &LinuxHugepageLimit, + ) -> Result<(), V2HugeTlbControllerError> { + let page_size_raw: String = hugetlb .page_size() .chars() .take_while(|c| c.is_ascii_digit()) .collect(); - let page_size: u64 = page_size.parse()?; + let page_size: u64 = match page_size_raw.parse() { + Ok(page_size) => page_size, + Err(err) => { + return Err(V2HugeTlbControllerError::MalformedPageSize { + page_size: page_size_raw, + err: EitherError::Left(err), + }) + } + }; if !Self::is_power_of_two(page_size) { - bail!("page size must be in the format of 2^(integer)"); + return Err(V2HugeTlbControllerError::MalformedPageSize { + page_size: page_size_raw, + err: EitherError::Right(MustBePowerOfTwo), + }); } common::write_cgroup_file( @@ -65,15 +111,22 @@ impl HugeTlb { (number != 0) && (number & (number.saturating_sub(1))) == 0 } - fn stats_for_page_size(cgroup_path: &Path, page_size: &str) -> Result { + fn stats_for_page_size( + cgroup_path: &Path, + page_size: &str, + ) -> Result { let events_file = format!("hugetlb.{page_size}.events"); - let events = common::read_cgroup_file(cgroup_path.join(&events_file))?; + let path = cgroup_path.join(events_file); + let events = common::read_cgroup_file(&path)?; let fail_count: u64 = events .lines() .find(|l| l.starts_with("max")) .map(|l| l[3..].trim().parse()) .transpose() - .with_context(|| format!("failed to parse max value for {events_file}"))? + .map_err(|err| V2HugeTlbStatsError::ParseMax { + path: path.clone(), + err, + })? .unwrap_or_default(); Ok(HugeTlbStats { diff --git a/crates/libcgroups/src/v2/io.rs b/crates/libcgroups/src/v2/io.rs index 167572d79..5a00b916b 100644 --- a/crates/libcgroups/src/v2/io.rs +++ b/crates/libcgroups/src/v2/io.rs @@ -1,10 +1,14 @@ -use std::path::{Path, PathBuf}; - -use anyhow::{bail, Context, Result}; +use std::{ + num::ParseIntError, + path::{Path, PathBuf}, +}; use crate::{ - common::{self, ControllerOpt}, - stats::{self, psi_stats, BlkioDeviceStat, BlkioStats, StatsProvider}, + common::{self, ControllerOpt, WrappedIoError}, + stats::{ + self, psi_stats, BlkioDeviceStat, BlkioStats, ParseDeviceNumberError, + ParseNestedKeyedDataError, StatsProvider, + }, }; use super::controller::Controller; @@ -15,28 +19,51 @@ const CGROUP_IO_WEIGHT: &str = "io.weight"; const CGROUP_IO_STAT: &str = "io.stat"; const CGROUP_IO_PSI: &str = "io.pressure"; +#[derive(thiserror::Error, Debug)] +pub enum V2IoControllerError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("cannot set leaf_weight with cgroupv2")] + LeafWeight, +} + pub struct Io {} impl Controller for Io { - fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()> { + type Error = V2IoControllerError; + + fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<(), Self::Error> { log::debug!("Apply io cgroup v2 config"); if let Some(io) = &controller_opt.resources.block_io() { - Self::apply(cgroup_root, io).context("failed to apply io resource restrictions")?; + Self::apply(cgroup_root, io)?; } Ok(()) } } +#[derive(thiserror::Error, Debug)] +pub enum V2IoStatsError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("while parsing stat table: {0}")] + ParseNestedKeyedData(#[from] ParseNestedKeyedDataError), + #[error("while parsing device number: {0}")] + ParseDeviceNumber(#[from] ParseDeviceNumberError), + #[error("while parsing table value: {0}")] + ParseInt(#[from] ParseIntError), +} + impl StatsProvider for Io { + type Error = V2IoStatsError; type Stats = BlkioStats; - fn stats(cgroup_path: &Path) -> Result { + fn stats(cgroup_path: &Path) -> Result { let keyed_data = stats::parse_nested_keyed_data(&cgroup_path.join(CGROUP_IO_STAT))?; let mut service_bytes = Vec::with_capacity(keyed_data.len()); let mut serviced = Vec::with_capacity(keyed_data.len()); for entry in keyed_data { let (major, minor) = stats::parse_device_number(&entry.0)?; - for value in &entry.1 { + for value in entry.1 { if value.starts_with("rbytes") { service_bytes.push(BlkioDeviceStat { major, @@ -72,7 +99,7 @@ impl StatsProvider for Io { let stats = BlkioStats { service_bytes, serviced, - psi: psi_stats(&cgroup_path.join(CGROUP_IO_PSI)).context("could not read io psi")?, + psi: psi_stats(&cgroup_path.join(CGROUP_IO_PSI))?, ..Default::default() }; @@ -97,7 +124,7 @@ impl Io { } // linux kernel doc: https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html#io - fn apply(root_path: &Path, blkio: &LinuxBlockIo) -> Result<()> { + fn apply(root_path: &Path, blkio: &LinuxBlockIo) -> Result<(), V2IoControllerError> { if let Some(weight_device) = blkio.weight_device() { for wd in weight_device { common::write_cgroup_file( @@ -108,7 +135,7 @@ impl Io { } if let Some(leaf_weight) = blkio.leaf_weight() { if leaf_weight > 0 { - bail!("cannot set leaf_weight with cgroupv2"); + return Err(V2IoControllerError::LeafWeight); } } if let Some(io_weight) = blkio.weight() { diff --git a/crates/libcgroups/src/v2/manager.rs b/crates/libcgroups/src/v2/manager.rs index e4c654669..812ba676c 100644 --- a/crates/libcgroups/src/v2/manager.rs +++ b/crates/libcgroups/src/v2/manager.rs @@ -5,8 +5,6 @@ use std::{ time::Duration, }; -use anyhow::{Context, Result}; - use nix::unistd::Pid; #[cfg(feature = "cgroupsv2_devices")] @@ -16,23 +14,67 @@ use super::{ controller_type::{ ControllerType, PseudoControllerType, CONTROLLER_TYPES, PSEUDO_CONTROLLER_TYPES, }, - cpu::Cpu, + cpu::{Cpu, V2CpuControllerError, V2CpuStatsError}, cpuset::CpuSet, - freezer::Freezer, - hugetlb::HugeTlb, - io::Io, - memory::Memory, + freezer::{Freezer, V2FreezerError}, + hugetlb::{HugeTlb, V2HugeTlbControllerError, V2HugeTlbStatsError}, + io::{Io, V2IoControllerError, V2IoStatsError}, + memory::{Memory, V2MemoryControllerError, V2MemoryStatsError}, pids::Pids, - unified::Unified, - util::{self, CGROUP_SUBTREE_CONTROL}, + unified::{Unified, V2UnifiedError}, + util::{self, V2UtilError, CGROUP_SUBTREE_CONTROL}, }; use crate::{ - common::{self, CgroupManager, ControllerOpt, FreezerState, PathBufExt, CGROUP_PROCS}, - stats::{Stats, StatsProvider}, + common::{ + self, AnyCgroupManager, CgroupManager, ControllerOpt, FreezerState, JoinSafelyError, + PathBufExt, WrapIoResult, WrappedIoError, CGROUP_PROCS, + }, + stats::{PidStatsError, Stats, StatsProvider}, }; pub const CGROUP_KILL: &str = "cgroup.kill"; +#[derive(thiserror::Error, Debug)] +pub enum V2ManagerError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("while joining paths: {0}")] + JoinSafely(#[from] JoinSafelyError), + #[error(transparent)] + Util(#[from] V2UtilError), + + #[error(transparent)] + CpuController(#[from] V2CpuControllerError), + #[error(transparent)] + CpuSetController(WrappedIoError), + #[error(transparent)] + HugeTlbController(#[from] V2HugeTlbControllerError), + #[error(transparent)] + IoController(#[from] V2IoControllerError), + #[error(transparent)] + MemoryController(#[from] V2MemoryControllerError), + #[error(transparent)] + PidsController(WrappedIoError), + #[error(transparent)] + UnifiedController(#[from] V2UnifiedError), + #[error(transparent)] + FreezerController(#[from] V2FreezerError), + #[cfg(feature = "cgroupsv2_devices")] + #[error(transparent)] + DevicesController(#[from] super::devices::controller::DevicesControllerError), + + #[error(transparent)] + CpuStats(#[from] V2CpuStatsError), + #[error(transparent)] + HugeTlbStats(#[from] V2HugeTlbStatsError), + #[error(transparent)] + PidsStats(PidStatsError), + #[error(transparent)] + MemoryStats(#[from] V2MemoryStatsError), + #[error(transparent)] + IoStats(#[from] V2IoStatsError), +} + pub struct Manager { root_path: PathBuf, cgroup_path: PathBuf, @@ -42,7 +84,7 @@ pub struct Manager { impl Manager { /// Constructs a new cgroup manager with root path being the mount point /// of a cgroup v2 fs and cgroup path being a relative path from the root - pub fn new(root_path: PathBuf, cgroup_path: PathBuf) -> Result { + pub fn new(root_path: PathBuf, cgroup_path: PathBuf) -> Result { let full_path = root_path.join_safely(&cgroup_path)?; Ok(Self { @@ -52,7 +94,7 @@ impl Manager { }) } - fn create_unified_cgroup(&self, pid: Pid) -> Result<()> { + fn create_unified_cgroup(&self, pid: Pid) -> Result<(), V2ManagerError> { let controllers: Vec = util::get_available_controllers(&self.root_path)? .iter() .map(|c| format!("{}{}", "+", c)) @@ -69,8 +111,11 @@ impl Manager { while let Some(component) = components.next() { current_path = current_path.join(component); if !current_path.exists() { - fs::create_dir(¤t_path)?; - fs::metadata(¤t_path)?.permissions().set_mode(0o755); + fs::create_dir(¤t_path).wrap_create_dir(¤t_path)?; + fs::metadata(¤t_path) + .wrap_other(¤t_path)? + .permissions() + .set_mode(0o755); } // last component cannot have subtree_control enabled due to internal process constraint @@ -84,22 +129,28 @@ impl Manager { Ok(()) } - fn write_controllers(path: &Path, controllers: &[String]) -> Result<()> { + fn write_controllers(path: &Path, controllers: &[String]) -> Result<(), WrappedIoError> { for controller in controllers { common::write_cgroup_file_str(path.join(CGROUP_SUBTREE_CONTROL), controller)?; } Ok(()) } + + pub fn any(self) -> AnyCgroupManager { + AnyCgroupManager::V2(self) + } } impl CgroupManager for Manager { - fn add_task(&self, pid: Pid) -> Result<()> { + type Error = V2ManagerError; + + fn add_task(&self, pid: Pid) -> Result<(), Self::Error> { self.create_unified_cgroup(pid)?; Ok(()) } - fn apply(&self, controller_opt: &ControllerOpt) -> Result<()> { + fn apply(&self, controller_opt: &ControllerOpt) -> Result<(), Self::Error> { for controller in CONTROLLER_TYPES { match controller { ControllerType::Cpu => Cpu::apply(controller_opt, &self.full_path)?, @@ -127,18 +178,21 @@ impl CgroupManager for Manager { Ok(()) } - fn remove(&self) -> Result<()> { + fn remove(&self) -> Result<(), Self::Error> { if self.full_path.exists() { log::debug!("remove cgroup {:?}", self.full_path); let kill_file = self.full_path.join(CGROUP_KILL); if kill_file.exists() { - fs::write(kill_file, "1").context("failed to kill cgroup")?; + fs::write(&kill_file, "1").wrap_write(&kill_file, "1")?; } else { let procs_path = self.full_path.join(CGROUP_PROCS); - let procs = fs::read_to_string(procs_path)?; + let procs = fs::read_to_string(&procs_path).wrap_read(&procs_path)?; for line in procs.lines() { - let pid: i32 = line.parse()?; + let pid: i32 = line + .parse() + .map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err)) + .wrap_other(&procs_path)?; let _ = nix::sys::signal::kill(Pid::from_raw(pid), nix::sys::signal::SIGKILL); } } @@ -149,24 +203,26 @@ impl CgroupManager for Manager { Ok(()) } - fn freeze(&self, state: FreezerState) -> Result<()> { + fn freeze(&self, state: FreezerState) -> Result<(), Self::Error> { let controller_opt = ControllerOpt { resources: &Default::default(), freezer_state: Some(state), oom_score_adj: None, disable_oom_killer: false, }; - Freezer::apply(&controller_opt, &self.full_path) + Ok(Freezer::apply(&controller_opt, &self.full_path)?) } - fn stats(&self) -> Result { + fn stats(&self) -> Result { let mut stats = Stats::default(); for subsystem in CONTROLLER_TYPES { match subsystem { ControllerType::Cpu => stats.cpu = Cpu::stats(&self.full_path)?, ControllerType::HugeTlb => stats.hugetlb = HugeTlb::stats(&self.full_path)?, - ControllerType::Pids => stats.pids = Pids::stats(&self.full_path)?, + ControllerType::Pids => { + stats.pids = Pids::stats(&self.full_path).map_err(V2ManagerError::PidsStats)? + } ControllerType::Memory => stats.memory = Memory::stats(&self.full_path)?, ControllerType::Io => stats.blkio = Io::stats(&self.full_path)?, _ => continue, @@ -176,7 +232,7 @@ impl CgroupManager for Manager { Ok(stats) } - fn get_all_pids(&self) -> Result> { - common::get_all_pids(&self.full_path) + fn get_all_pids(&self) -> Result, Self::Error> { + Ok(common::get_all_pids(&self.full_path)?) } } diff --git a/crates/libcgroups/src/v2/memory.rs b/crates/libcgroups/src/v2/memory.rs index b3ea65fef..9b915eff7 100644 --- a/crates/libcgroups/src/v2/memory.rs +++ b/crates/libcgroups/src/v2/memory.rs @@ -1,11 +1,10 @@ -use anyhow::{bail, Context, Result}; use std::path::Path; use oci_spec::runtime::LinuxMemory; use crate::{ - common::{self, ControllerOpt}, - stats::{self, MemoryData, MemoryStats, StatsProvider}, + common::{self, ControllerOpt, WrappedIoError}, + stats::{self, MemoryData, MemoryStats, ParseFlatKeyedDataError, StatsProvider}, }; use super::controller::Controller; @@ -16,30 +15,54 @@ const CGROUP_MEMORY_LOW: &str = "memory.low"; const MEMORY_STAT: &str = "memory.stat"; const MEMORY_PSI: &str = "memory.pressure"; +#[derive(thiserror::Error, Debug)] +pub enum V2MemoryControllerError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("invalid memory value {0}")] + MemoryValue(i64), + #[error("invalid swap value {0}")] + SwapValue(i64), + #[error("swap memory ({swap}) should be bigger than memory limit ({limit})")] + SwapTooSmall { swap: i64, limit: i64 }, + #[error("unable to set swap limit without memory limit")] + SwapWithoutLimit, + #[error("invalid memory reservation value: {0}")] + MemoryReservation(i64), +} + pub struct Memory {} impl Controller for Memory { - fn apply(controller_opt: &ControllerOpt, cgroup_path: &Path) -> Result<()> { + type Error = V2MemoryControllerError; + + fn apply(controller_opt: &ControllerOpt, cgroup_path: &Path) -> Result<(), Self::Error> { if let Some(memory) = &controller_opt.resources.memory() { - Self::apply(cgroup_path, memory) - .context("failed to apply memory resource restrictions")?; + Self::apply(cgroup_path, memory)?; } Ok(()) } } +#[derive(thiserror::Error, Debug)] +pub enum V2MemoryStatsError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("while parsing stat table: {0}")] + ParseNestedKeyedData(#[from] ParseFlatKeyedDataError), +} impl StatsProvider for Memory { + type Error = V2MemoryStatsError; type Stats = MemoryStats; - fn stats(cgroup_path: &Path) -> Result { + fn stats(cgroup_path: &Path) -> Result { let stats = MemoryStats { memory: Self::get_memory_data(cgroup_path, "memory", "oom")?, memswap: Self::get_memory_data(cgroup_path, "memory.swap", "fail")?, hierarchy: true, stats: stats::parse_flat_keyed_data(&cgroup_path.join(MEMORY_STAT))?, - psi: stats::psi_stats(&cgroup_path.join(MEMORY_PSI)) - .context("could not read memory psi")?, + psi: stats::psi_stats(&cgroup_path.join(MEMORY_PSI))?, ..Default::default() }; @@ -52,7 +75,7 @@ impl Memory { cgroup_path: &Path, file_prefix: &str, fail_event: &str, - ) -> Result { + ) -> Result { let usage = stats::parse_single_value(&cgroup_path.join(format!("{}.{}", file_prefix, "current")))?; let limit = @@ -75,17 +98,17 @@ impl Memory { }) } - fn set>(path: P, val: i64) -> Result<()> { + fn set>(path: P, val: i64) -> Result<(), WrappedIoError> { if val == 0 { Ok(()) } else if val == -1 { - common::write_cgroup_file_str(path, "max") + Ok(common::write_cgroup_file_str(path, "max")?) } else { - common::write_cgroup_file(path, val) + Ok(common::write_cgroup_file(path, val)?) } } - fn apply(path: &Path, memory: &LinuxMemory) -> Result<()> { + fn apply(path: &Path, memory: &LinuxMemory) -> Result<(), V2MemoryControllerError> { // if nothing is set just exit right away if memory.reservation().is_none() && memory.limit().is_none() && memory.swap().is_none() { return Ok(()); @@ -93,11 +116,11 @@ impl Memory { match memory.limit() { Some(limit) if limit < -1 => { - bail!("invalid memory value: {}", limit); + return Err(V2MemoryControllerError::MemoryValue(limit)); } Some(limit) => match memory.swap() { Some(swap) if swap < -1 => { - bail!("invalid swap value: {}", swap); + return Err(V2MemoryControllerError::SwapValue(swap)); } Some(swap) => { // -1 means max @@ -105,11 +128,7 @@ impl Memory { Memory::set(path.join(CGROUP_MEMORY_SWAP), swap)?; } else { if swap < limit { - bail!( - "swap memory ({}) should be bigger than memory limit ({})", - swap, - limit - ); + return Err(V2MemoryControllerError::SwapTooSmall { swap, limit }); } // In cgroup v1 swap is memory+swap, but in cgroup v2 swap is @@ -129,14 +148,14 @@ impl Memory { }, None => { if memory.swap().is_some() { - bail!("unable to set swap limit without memory limit"); + return Err(V2MemoryControllerError::SwapWithoutLimit); } } }; if let Some(reservation) = memory.reservation() { if reservation < -1 { - bail!("invalid memory reservation value: {}", reservation); + return Err(V2MemoryControllerError::MemoryReservation(reservation)); } Memory::set(path.join(CGROUP_MEMORY_LOW), reservation)?; } diff --git a/crates/libcgroups/src/v2/pids.rs b/crates/libcgroups/src/v2/pids.rs index 38d90df3b..766332869 100644 --- a/crates/libcgroups/src/v2/pids.rs +++ b/crates/libcgroups/src/v2/pids.rs @@ -1,10 +1,8 @@ use std::path::Path; -use anyhow::{Context, Result}; - use crate::{ - common::{self, ControllerOpt}, - stats::{self, PidStats, StatsProvider}, + common::{self, ControllerOpt, WrappedIoError}, + stats::{self, PidStats, PidStatsError, StatsProvider}, }; use super::controller::Controller; @@ -13,25 +11,31 @@ use oci_spec::runtime::LinuxPids; pub struct Pids {} impl Controller for Pids { - fn apply(controller_opt: &ControllerOpt, cgroup_root: &std::path::Path) -> Result<()> { + type Error = WrappedIoError; + + fn apply( + controller_opt: &ControllerOpt, + cgroup_root: &std::path::Path, + ) -> Result<(), Self::Error> { log::debug!("Apply pids cgroup v2 config"); if let Some(pids) = &controller_opt.resources.pids() { - Self::apply(cgroup_root, pids).context("failed to apply pids resource restrictions")?; + Self::apply(cgroup_root, pids)?; } Ok(()) } } impl StatsProvider for Pids { + type Error = PidStatsError; type Stats = PidStats; - fn stats(cgroup_path: &Path) -> Result { + fn stats(cgroup_path: &Path) -> Result { stats::pid_stats(cgroup_path) } } impl Pids { - fn apply(root_path: &Path, pids: &LinuxPids) -> Result<()> { + fn apply(root_path: &Path, pids: &LinuxPids) -> Result<(), WrappedIoError> { let limit = if pids.limit() > 0 { pids.limit().to_string() } else { diff --git a/crates/libcgroups/src/v2/unified.rs b/crates/libcgroups/src/v2/unified.rs index 0caecf155..8b6af1d1b 100644 --- a/crates/libcgroups/src/v2/unified.rs +++ b/crates/libcgroups/src/v2/unified.rs @@ -1,9 +1,18 @@ use std::{collections::HashMap, path::Path}; -use anyhow::{Context, Result}; - use super::controller_type::ControllerType; -use crate::common::{self, ControllerOpt}; +use crate::common::{self, ControllerOpt, WrappedIoError}; + +#[derive(thiserror::Error, Debug)] +pub enum V2UnifiedError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("subsystem {subsystem} is not available: {err}")] + SubsystemNotAvailable { + subsystem: String, + err: WrappedIoError, + }, +} pub struct Unified {} @@ -12,10 +21,9 @@ impl Unified { controller_opt: &ControllerOpt, cgroup_path: &Path, controllers: Vec, - ) -> Result<()> { + ) -> Result<(), V2UnifiedError> { if let Some(unified) = &controller_opt.resources.unified() { - Self::apply_impl(unified, cgroup_path, &controllers) - .context("failed to apply unified resource restrictions")?; + Self::apply_impl(unified, cgroup_path, &controllers)?; } Ok(()) @@ -25,29 +33,20 @@ impl Unified { unified: &HashMap, cgroup_path: &Path, controllers: &[ControllerType], - ) -> Result<()> { - { - log::debug!("Apply unified cgroup config"); - for (cgroup_file, value) in unified { - common::write_cgroup_file_str(cgroup_path.join(cgroup_file), value).map_err( - |e| { - let (subsystem, _) = cgroup_file - .split_once('.') - .with_context(|| { - format!("failed to split {} with {}", cgroup_file, ".") - }) - .unwrap(); - let context = if !controllers.iter().any(|c| c.to_string() == subsystem) { - format!( - "failed to set {cgroup_file} to {value}: subsystem {subsystem} is not available" - ) - } else { - format!("failed to set {cgroup_file} to {value}: {e}") - }; - - e.context(context) - }, - )?; + ) -> Result<(), V2UnifiedError> { + log::debug!("Apply unified cgroup config"); + for (cgroup_file, value) in unified { + if let Err(err) = common::write_cgroup_file_str(cgroup_path.join(cgroup_file), value) { + let (subsystem, _) = cgroup_file.split_once('.').unwrap_or((cgroup_file, "")); + + if controllers.iter().any(|c| c.to_string() == subsystem) { + Err(err)?; + } else { + return Err(V2UnifiedError::SubsystemNotAvailable { + subsystem: subsystem.into(), + err, + }); + } } } diff --git a/crates/libcgroups/src/v2/util.rs b/crates/libcgroups/src/v2/util.rs index 9bed000ce..c90473395 100644 --- a/crates/libcgroups/src/v2/util.rs +++ b/crates/libcgroups/src/v2/util.rs @@ -1,32 +1,42 @@ use std::path::{Path, PathBuf}; -use anyhow::{anyhow, bail, Result}; -use procfs::process::Process; +use procfs::{process::Process, ProcError}; -use crate::common; +use crate::common::{self, WrappedIoError}; use super::controller_type::ControllerType; pub const CGROUP_CONTROLLERS: &str = "cgroup.controllers"; pub const CGROUP_SUBTREE_CONTROL: &str = "cgroup.subtree_control"; -pub fn get_unified_mount_point() -> Result { +#[derive(thiserror::Error, Debug)] +pub enum V2UtilError { + #[error("io error: {0}")] + WrappedIo(#[from] WrappedIoError), + #[error("proc error: {0}")] + Proc(#[from] ProcError), + #[error("could not find mountpoint for unified")] + CouldNotFind, + #[error("cannot get available controllers. {0} does not exist")] + DoesNotExist(PathBuf), +} + +pub fn get_unified_mount_point() -> Result { Process::myself()? .mountinfo()? .into_iter() .find(|m| m.fs_type == "cgroup2") .map(|m| m.mount_point) - .ok_or_else(|| anyhow!("could not find mountpoint for unified")) + .ok_or(V2UtilError::CouldNotFind) } -pub fn get_available_controllers>(root_path: P) -> Result> { +pub fn get_available_controllers>( + root_path: P, +) -> Result, V2UtilError> { let root_path = root_path.as_ref(); let controllers_path = root_path.join(CGROUP_CONTROLLERS); if !controllers_path.exists() { - bail!( - "cannot get available controllers. {:?} does not exist", - controllers_path - ) + return Err(V2UtilError::DoesNotExist(controllers_path)); } let mut controllers = Vec::new(); diff --git a/crates/libcontainer/src/container/builder_impl.rs b/crates/libcontainer/src/container/builder_impl.rs index 8745960ef..b8d3e38bd 100644 --- a/crates/libcontainer/src/container/builder_impl.rs +++ b/crates/libcontainer/src/container/builder_impl.rs @@ -13,6 +13,7 @@ use crate::{ workload::ExecutorManager, }; use anyhow::{bail, Context, Result}; +use libcgroups::common::CgroupManager; use nix::unistd::Pid; use oci_spec::runtime::Spec; use std::{fs, io::Write, os::unix::prelude::RawFd, path::PathBuf}; @@ -75,7 +76,7 @@ impl<'a> ContainerBuilderImpl<'a> { self.rootless.is_some(), ); let cmanager = libcgroups::common::create_cgroup_manager( - &cgroups_path, + cgroups_path, self.use_systemd || self.rootless.is_some(), &self.container_id, )?; @@ -168,7 +169,7 @@ impl<'a> ContainerBuilderImpl<'a> { self.rootless.is_some(), ); let cmanager = libcgroups::common::create_cgroup_manager( - &cgroups_path, + cgroups_path, self.use_systemd || self.rootless.is_some(), &self.container_id, )?; diff --git a/crates/libcontainer/src/container/container_delete.rs b/crates/libcontainer/src/container/container_delete.rs index 63a125286..cf1519c2e 100644 --- a/crates/libcontainer/src/container/container_delete.rs +++ b/crates/libcontainer/src/container/container_delete.rs @@ -3,7 +3,7 @@ use crate::config::YoukiConfig; use crate::hooks; use crate::process::intel_rdt::delete_resctrl_subdirectory; use anyhow::{bail, Context, Result}; -use libcgroups; +use libcgroups::{self, common::CgroupManager}; use nix::sys::signal; use std::fs; diff --git a/crates/libcontainer/src/container/container_events.rs b/crates/libcontainer/src/container/container_events.rs index dd0f4db0d..fd90788c5 100644 --- a/crates/libcontainer/src/container/container_events.rs +++ b/crates/libcontainer/src/container/container_events.rs @@ -2,6 +2,7 @@ use std::{thread, time::Duration}; use super::{Container, ContainerStatus}; use anyhow::{bail, Context, Result}; +use libcgroups::common::CgroupManager; impl Container { /// Displays container events diff --git a/crates/libcontainer/src/container/container_kill.rs b/crates/libcontainer/src/container/container_kill.rs index 8893a61df..425950f14 100644 --- a/crates/libcontainer/src/container/container_kill.rs +++ b/crates/libcontainer/src/container/container_kill.rs @@ -1,7 +1,7 @@ use super::{Container, ContainerStatus}; use crate::signal::Signal; use anyhow::{bail, Context, Result}; -use libcgroups::common::{create_cgroup_manager, get_cgroup_setup}; +use libcgroups::common::{create_cgroup_manager, get_cgroup_setup, CgroupManager}; use nix::sys::signal::{self}; impl Container { @@ -80,7 +80,7 @@ impl Container { let use_systemd = self .systemd() .context("container state does not contain cgroup manager")?; - let cmanger = create_cgroup_manager(&cgroups_path, use_systemd, self.id())?; + let cmanger = create_cgroup_manager(cgroups_path, use_systemd, self.id())?; cmanger.freeze(libcgroups::common::FreezerState::Thawed)?; } libcgroups::common::CgroupSetup::Unified => {} @@ -95,7 +95,7 @@ impl Container { let use_systemd = self .systemd() .context("container state does not contain cgroup manager")?; - let cmanger = create_cgroup_manager(&cgroups_path, use_systemd, self.id())?; + let cmanger = create_cgroup_manager(cgroups_path, use_systemd, self.id())?; let ret = cmanger.freeze(libcgroups::common::FreezerState::Frozen); if ret.is_err() { log::warn!( diff --git a/crates/libcontainer/src/container/container_pause.rs b/crates/libcontainer/src/container/container_pause.rs index 443d14d5e..951bc890a 100644 --- a/crates/libcontainer/src/container/container_pause.rs +++ b/crates/libcontainer/src/container/container_pause.rs @@ -1,6 +1,6 @@ use super::{Container, ContainerStatus}; use anyhow::{bail, Context, Result}; -use libcgroups::common::FreezerState; +use libcgroups::common::{CgroupManager, FreezerState}; impl Container { /// Suspends all processes within the container diff --git a/crates/libcontainer/src/container/container_resume.rs b/crates/libcontainer/src/container/container_resume.rs index e98071c59..bde02e91c 100644 --- a/crates/libcontainer/src/container/container_resume.rs +++ b/crates/libcontainer/src/container/container_resume.rs @@ -1,7 +1,7 @@ use super::{Container, ContainerStatus}; use anyhow::{bail, Context, Result}; -use libcgroups::common::FreezerState; +use libcgroups::common::{CgroupManager, FreezerState}; impl Container { /// Resumes all processes within the container diff --git a/crates/libcontainer/src/process/args.rs b/crates/libcontainer/src/process/args.rs index 98ddb8543..f54a691b5 100644 --- a/crates/libcontainer/src/process/args.rs +++ b/crates/libcontainer/src/process/args.rs @@ -1,4 +1,4 @@ -use libcgroups::common::CgroupManager; +use libcgroups::common::AnyCgroupManager; use oci_spec::runtime::Spec; use std::os::unix::prelude::RawFd; use std::path::PathBuf; @@ -33,7 +33,7 @@ pub struct ContainerArgs<'a> { /// Options for rootless containers pub rootless: &'a Option>, /// Cgroup Manager - pub cgroup_manager: Box, + pub cgroup_manager: AnyCgroupManager, /// If the container is to be run in detached mode pub detached: bool, /// Manage the functions that actually run on the container diff --git a/crates/libcontainer/src/process/container_intermediate_process.rs b/crates/libcontainer/src/process/container_intermediate_process.rs index b30a8d6fb..290a786c1 100644 --- a/crates/libcontainer/src/process/container_intermediate_process.rs +++ b/crates/libcontainer/src/process/container_intermediate_process.rs @@ -34,7 +34,7 @@ pub fn container_intermediate_process( // the cgroup of the process will form the root of the cgroup hierarchy in // the cgroup namespace. apply_cgroups( - args.cgroup_manager.as_ref(), + &args.cgroup_manager, linux.resources().as_ref(), matches!(args.container_type, ContainerType::InitContainer), ) @@ -137,7 +137,10 @@ pub fn container_intermediate_process( Ok(pid) } -fn apply_cgroups( +fn apply_cgroups< + C: CgroupManager + ?Sized, + E: std::error::Error + Send + Sync + 'static, +>( cmanager: &C, resources: Option<&LinuxResources>, init: bool, diff --git a/crates/youki/src/commands/mod.rs b/crates/youki/src/commands/mod.rs index 33bff6aa1..4e606fb4a 100644 --- a/crates/youki/src/commands/mod.rs +++ b/crates/youki/src/commands/mod.rs @@ -4,7 +4,7 @@ use std::{ path::{Path, PathBuf}, }; -use libcgroups::common::CgroupManager; +use libcgroups::common::AnyCgroupManager; use libcontainer::container::Container; pub mod checkpoint; @@ -56,12 +56,16 @@ fn container_exists>(root_path: P, container_id: &str) -> Result< fn create_cgroup_manager>( root_path: P, container_id: &str, -) -> Result> { +) -> Result { let container = load_container(root_path, container_id)?; let cgroups_path = container.spec()?.cgroup_path; let systemd_cgroup = container .systemd() .context("could not determine cgroup manager")?; - libcgroups::common::create_cgroup_manager(cgroups_path, systemd_cgroup, container.id()) + Ok(libcgroups::common::create_cgroup_manager( + cgroups_path, + systemd_cgroup, + container.id(), + )?) } diff --git a/crates/youki/src/commands/ps.rs b/crates/youki/src/commands/ps.rs index 789639dfc..b51456c4a 100644 --- a/crates/youki/src/commands/ps.rs +++ b/crates/youki/src/commands/ps.rs @@ -1,5 +1,6 @@ use crate::commands::create_cgroup_manager; use anyhow::{bail, Result}; +use libcgroups::common::CgroupManager; use liboci_cli::Ps; use std::{path::PathBuf, process::Command}; diff --git a/crates/youki/src/commands/update.rs b/crates/youki/src/commands/update.rs index d9e683b49..15016c6ad 100644 --- a/crates/youki/src/commands/update.rs +++ b/crates/youki/src/commands/update.rs @@ -4,6 +4,7 @@ use std::path::PathBuf; use crate::commands::create_cgroup_manager; use anyhow::Result; +use libcgroups::common::CgroupManager; use libcgroups::{self, common::ControllerOpt}; use liboci_cli::Update; use oci_spec::runtime::{LinuxPidsBuilder, LinuxResources, LinuxResourcesBuilder};