From b43aae070b8bf851c4c21ce4fce59be58ece4a7b Mon Sep 17 00:00:00 2001 From: Furisto <24721048+Furisto@users.noreply.github.com> Date: Sat, 26 Jun 2021 01:26:24 +0200 Subject: [PATCH 1/3] Seperate adding tasks and applying resource restrictions --- src/cgroups/common.rs | 3 +- src/cgroups/v1/blkio.rs | 25 ++---- src/cgroups/v1/controller.rs | 12 ++- src/cgroups/v1/controller_type.rs | 1 + src/cgroups/v1/cpu.rs | 10 +-- src/cgroups/v1/cpuacct.rs | 22 +++--- src/cgroups/v1/cpuset.rs | 14 +++- src/cgroups/v1/devices.rs | 9 +-- src/cgroups/v1/freezer.rs | 43 +++++----- src/cgroups/v1/hugetlb.rs | 23 ++---- src/cgroups/v1/manager.rs | 62 ++++++++++----- src/cgroups/v1/memory.rs | 23 ++---- src/cgroups/v1/network_classifier.rs | 8 +- src/cgroups/v1/network_priority.rs | 8 +- src/cgroups/v1/pids.rs | 13 +-- src/cgroups/v2/manager.rs | 48 ++++++------ src/cgroups/v2/systemd_manager.rs | 113 +++++++++++++-------------- src/process/fork.rs | 3 +- 18 files changed, 214 insertions(+), 226 deletions(-) diff --git a/src/cgroups/common.rs b/src/cgroups/common.rs index 26f626805..84a6dc148 100644 --- a/src/cgroups/common.rs +++ b/src/cgroups/common.rs @@ -19,7 +19,8 @@ pub const CGROUP_PROCS: &str = "cgroup.procs"; pub const DEFAULT_CGROUP_ROOT: &str = "/sys/fs/cgroup"; pub trait CgroupManager { - fn apply(&self, linux_resources: &LinuxResources, pid: Pid) -> Result<()>; + fn add_task(&self, pid: Pid) -> Result<()>; + fn apply(&self, linux_resources: &LinuxResources) -> Result<()>; fn remove(&self) -> Result<()>; } diff --git a/src/cgroups/v1/blkio.rs b/src/cgroups/v1/blkio.rs index 2801eb2b7..d80dc36af 100644 --- a/src/cgroups/v1/blkio.rs +++ b/src/cgroups/v1/blkio.rs @@ -1,12 +1,7 @@ -use std::{ - fs::{self}, - path::Path, -}; - -use crate::cgroups::{ - common::{self, CGROUP_PROCS}, - v1::Controller, -}; +use std::path::Path; + +use crate::cgroups::{common, v1::Controller}; +use anyhow::Result; use oci_spec::{LinuxBlockIo, LinuxResources}; const CGROUP_BLKIO_THROTTLE_READ_BPS: &str = "blkio.throttle.read_bps_device"; @@ -17,25 +12,19 @@ const CGROUP_BLKIO_THROTTLE_WRITE_IOPS: &str = "blkio.throttle.write_iops_device pub struct Blkio {} impl Controller for Blkio { - fn apply( - linux_resources: &LinuxResources, - cgroup_root: &Path, - pid: nix::unistd::Pid, - ) -> anyhow::Result<()> { + fn apply(linux_resources: &LinuxResources, cgroup_root: &Path) -> Result<()> { log::debug!("Apply blkio cgroup config"); - fs::create_dir_all(cgroup_root)?; if let Some(blkio) = &linux_resources.block_io { Self::apply(cgroup_root, blkio)?; } - common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } impl Blkio { - fn apply(root_path: &Path, blkio: &LinuxBlockIo) -> anyhow::Result<()> { + fn apply(root_path: &Path, blkio: &LinuxBlockIo) -> Result<()> { for trbd in &blkio.blkio_throttle_read_bps_device { common::write_cgroup_file_str( &root_path.join(CGROUP_BLKIO_THROTTLE_READ_BPS), @@ -70,6 +59,8 @@ impl Blkio { #[cfg(test)] mod tests { + use std::fs; + use super::*; use crate::cgroups::test::setup; use oci_spec::{LinuxBlockIo, LinuxThrottleDevice}; diff --git a/src/cgroups/v1/controller.rs b/src/cgroups/v1/controller.rs index 84e0b3cc2..408d357da 100644 --- a/src/cgroups/v1/controller.rs +++ b/src/cgroups/v1/controller.rs @@ -1,10 +1,18 @@ -use std::path::Path; +use std::{fs, path::Path}; use anyhow::Result; use nix::unistd::Pid; use oci_spec::LinuxResources; +use crate::cgroups::common::{self, CGROUP_PROCS}; + pub trait Controller { - fn apply(linux_resources: &LinuxResources, cgroup_root: &Path, pid: Pid) -> Result<()>; + fn add_task(pid: Pid, cgroup_path: &Path) -> Result<()> { + fs::create_dir_all(cgroup_path)?; + common::write_cgroup_file(cgroup_path.join(CGROUP_PROCS), pid)?; + Ok(()) + } + + fn apply(linux_resources: &LinuxResources, cgroup_root: &Path) -> Result<()>; } diff --git a/src/cgroups/v1/controller_type.rs b/src/cgroups/v1/controller_type.rs index 449c1bf41..6fbf0f37f 100644 --- a/src/cgroups/v1/controller_type.rs +++ b/src/cgroups/v1/controller_type.rs @@ -1,5 +1,6 @@ use std::fmt::Display; +#[derive(Hash, PartialEq, Eq, Debug, Clone)] pub enum ControllerType { Cpu, CpuAcct, diff --git a/src/cgroups/v1/cpu.rs b/src/cgroups/v1/cpu.rs index 50a7c7eb5..856d90a0b 100644 --- a/src/cgroups/v1/cpu.rs +++ b/src/cgroups/v1/cpu.rs @@ -1,10 +1,9 @@ -use std::{fs, path::Path}; +use std::path::Path; use anyhow::Result; -use nix::unistd::Pid; use oci_spec::{LinuxCpu, LinuxResources}; -use crate::cgroups::common::{self, CGROUP_PROCS}; +use crate::cgroups::common; use super::Controller; @@ -17,14 +16,13 @@ const CGROUP_CPU_RT_PERIOD: &str = "cpu.rt_period_us"; pub struct Cpu {} impl Controller for Cpu { - fn apply(linux_resources: &LinuxResources, cgroup_root: &Path, pid: Pid) -> Result<()> { + fn apply(linux_resources: &LinuxResources, cgroup_root: &Path) -> Result<()> { log::debug!("Apply Cpu cgroup config"); - fs::create_dir_all(cgroup_root)?; + if let Some(cpu) = &linux_resources.cpu { Self::apply(cgroup_root, cpu)?; } - common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } diff --git a/src/cgroups/v1/cpuacct.rs b/src/cgroups/v1/cpuacct.rs index 947599cb2..889f905d0 100644 --- a/src/cgroups/v1/cpuacct.rs +++ b/src/cgroups/v1/cpuacct.rs @@ -1,37 +1,33 @@ -use std::{fs, path::Path}; +use std::path::Path; use anyhow::Result; -use nix::unistd::Pid; use oci_spec::LinuxResources; -use crate::cgroups::common::{self, CGROUP_PROCS}; - use super::Controller; pub struct CpuAcct {} impl Controller for CpuAcct { - fn apply(_linux_resources: &LinuxResources, cgroup_path: &Path, pid: Pid) -> Result<()> { - log::debug!("Apply cpuacct cgroup config"); - fs::create_dir_all(cgroup_path)?; - - common::write_cgroup_file(cgroup_path.join(CGROUP_PROCS), pid)?; + fn apply(_linux_resources: &LinuxResources, _cgroup_path: &Path) -> Result<()> { Ok(()) } } #[cfg(test)] mod tests { + use std::fs; + + use nix::unistd::Pid; + use super::*; - use crate::cgroups::test::setup; + use crate::cgroups::{common::CGROUP_PROCS, test::setup}; #[test] - fn test_apply() { + fn test_add_task() { let (tmp, procs) = setup("test_cpuacct_apply", CGROUP_PROCS); - let resource = LinuxResources::default(); let pid = Pid::from_raw(1000); - CpuAcct::apply(&resource, &tmp, pid).expect("apply cpuacct"); + CpuAcct::add_task(pid, &tmp).expect("apply cpuacct"); let content = fs::read_to_string(&procs) .unwrap_or_else(|_| panic!("read {} file content", CGROUP_PROCS)); diff --git a/src/cgroups/v1/cpuset.rs b/src/cgroups/v1/cpuset.rs index c08833c59..e232120c3 100644 --- a/src/cgroups/v1/cpuset.rs +++ b/src/cgroups/v1/cpuset.rs @@ -1,8 +1,9 @@ use std::{fs, path::Path}; use anyhow::{bail, Result}; -use nix::unistd::Pid; +use nix::unistd; use oci_spec::{LinuxCpu, LinuxResources}; +use unistd::Pid; use crate::cgroups::common::{self, CGROUP_PROCS}; @@ -14,18 +15,23 @@ const CGROUP_CPUSET_MEMS: &str = "cpuset.mems"; pub struct CpuSet {} impl Controller for CpuSet { - fn apply(linux_resources: &LinuxResources, cgroup_path: &Path, pid: Pid) -> Result<()> { - log::debug!("Apply CpuSet cgroup config"); + fn add_task(pid: Pid, cgroup_path: &Path) -> Result<()> { fs::create_dir_all(cgroup_path)?; Self::ensure_not_empty(cgroup_path, CGROUP_CPUSET_CPUS)?; Self::ensure_not_empty(cgroup_path, CGROUP_CPUSET_MEMS)?; + common::write_cgroup_file(cgroup_path.join(CGROUP_PROCS), pid)?; + Ok(()) + } + + fn apply(linux_resources: &LinuxResources, cgroup_path: &Path) -> Result<()> { + log::debug!("Apply CpuSet cgroup config"); + if let Some(cpuset) = &linux_resources.cpu { Self::apply(cgroup_path, cpuset)?; } - common::write_cgroup_file(cgroup_path.join(CGROUP_PROCS), pid)?; Ok(()) } } diff --git a/src/cgroups/v1/devices.rs b/src/cgroups/v1/devices.rs index 2599ed06a..1f71bfcda 100644 --- a/src/cgroups/v1/devices.rs +++ b/src/cgroups/v1/devices.rs @@ -1,18 +1,16 @@ -use std::{fs::create_dir_all, path::Path}; +use std::path::Path; use anyhow::Result; -use nix::unistd::Pid; -use crate::cgroups::common::{self, CGROUP_PROCS}; +use crate::cgroups::common; use crate::{cgroups::v1::Controller, rootfs::default_devices}; use oci_spec::{LinuxDeviceCgroup, LinuxDeviceType, LinuxResources}; pub struct Devices {} impl Controller for Devices { - fn apply(linux_resources: &LinuxResources, cgroup_root: &Path, pid: Pid) -> Result<()> { + fn apply(linux_resources: &LinuxResources, cgroup_root: &Path) -> Result<()> { log::debug!("Apply Devices cgroup config"); - create_dir_all(&cgroup_root)?; for d in &linux_resources.devices { Self::apply_device(d, cgroup_root)?; @@ -27,7 +25,6 @@ impl Controller for Devices { Self::apply_device(&d, &cgroup_root)?; } - common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } diff --git a/src/cgroups/v1/freezer.rs b/src/cgroups/v1/freezer.rs index 5ea995e93..27806ca0c 100644 --- a/src/cgroups/v1/freezer.rs +++ b/src/cgroups/v1/freezer.rs @@ -6,9 +6,8 @@ use std::{ }; use anyhow::{Result, *}; -use nix::unistd::Pid; -use crate::cgroups::common::{self, CGROUP_PROCS}; +use crate::cgroups::common; use crate::cgroups::v1::Controller; use oci_spec::{FreezerState, LinuxResources}; @@ -20,7 +19,7 @@ const FREEZER_STATE_FREEZING: &str = "FREEZING"; pub struct Freezer {} impl Controller for Freezer { - fn apply(linux_resources: &LinuxResources, cgroup_root: &Path, pid: Pid) -> Result<()> { + fn apply(linux_resources: &LinuxResources, cgroup_root: &Path) -> Result<()> { log::debug!("Apply Freezer cgroup config"); create_dir_all(&cgroup_root)?; @@ -28,7 +27,6 @@ impl Controller for Freezer { Self::apply(freezer_state, cgroup_root)?; } - common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } @@ -116,8 +114,10 @@ impl Freezer { #[cfg(test)] mod tests { use super::*; + use crate::cgroups::common::CGROUP_PROCS; use crate::cgroups::test::set_fixture; use crate::utils::create_temp_dir; + use nix::unistd::Pid; use oci_spec::FreezerState; #[test] @@ -160,10 +160,9 @@ mod tests { } #[test] - fn test_apply() { - let tmp = - create_temp_dir("test_set_freezer_state").expect("create temp directory for test"); - set_fixture(&tmp, CGROUP_FREEZER_STATE, "").expect("Set fixure for freezer state"); + fn test_add_and_apply() { + let tmp = create_temp_dir("test_add_task").expect("create temp directory for test"); + set_fixture(&tmp, CGROUP_FREEZER_STATE, "").expect("set fixure for freezer state"); set_fixture(&tmp, CGROUP_PROCS, "").expect("set fixture for proc file"); // set Thawed state. @@ -182,13 +181,13 @@ mod tests { }; let pid = Pid::from_raw(1000); - let _ = - ::apply(&linux_resources, &tmp, pid).expect("freezer apply"); + Freezer::add_task(pid, &tmp).expect("freezer add task"); + ::apply(&linux_resources, &tmp).expect("freezer apply"); let state_content = - std::fs::read_to_string(tmp.join(CGROUP_FREEZER_STATE)).expect("Read to string"); + std::fs::read_to_string(tmp.join(CGROUP_FREEZER_STATE)).expect("read to string"); assert_eq!(FREEZER_STATE_THAWED, state_content); let pid_content = - std::fs::read_to_string(tmp.join(CGROUP_PROCS)).expect("Read to string"); + std::fs::read_to_string(tmp.join(CGROUP_PROCS)).expect("read to string"); assert_eq!(pid_content, "1000"); } @@ -208,13 +207,13 @@ mod tests { }; let pid = Pid::from_raw(1001); - let _ = - ::apply(&linux_resources, &tmp, pid).expect("freezer apply"); + Freezer::add_task(pid, &tmp).expect("freezer add task"); + ::apply(&linux_resources, &tmp).expect("freezer apply"); let state_content = - std::fs::read_to_string(tmp.join(CGROUP_FREEZER_STATE)).expect("Read to string"); + std::fs::read_to_string(tmp.join(CGROUP_FREEZER_STATE)).expect("read to string"); assert_eq!(FREEZER_STATE_FROZEN, state_content); let pid_content = - std::fs::read_to_string(tmp.join(CGROUP_PROCS)).expect("Read to string"); + std::fs::read_to_string(tmp.join(CGROUP_PROCS)).expect("read to string"); assert_eq!(pid_content, "1001"); } @@ -233,16 +232,16 @@ mod tests { freezer: Some(FreezerState::Undefined), }; - let old_state_content = - std::fs::read_to_string(tmp.join(CGROUP_FREEZER_STATE)).expect("Read to string"); let pid = Pid::from_raw(1002); - let _ = - ::apply(&linux_resources, &tmp, pid).expect("freezer apply"); + let old_state_content = + std::fs::read_to_string(tmp.join(CGROUP_FREEZER_STATE)).expect("read to string"); + Freezer::add_task(pid, &tmp).expect("freezer add task"); + ::apply(&linux_resources, &tmp).expect("freezer apply"); let state_content = - std::fs::read_to_string(tmp.join(CGROUP_FREEZER_STATE)).expect("Read to string"); + std::fs::read_to_string(tmp.join(CGROUP_FREEZER_STATE)).expect("read to string"); assert_eq!(old_state_content, state_content); let pid_content = - std::fs::read_to_string(tmp.join(CGROUP_PROCS)).expect("Read to string"); + std::fs::read_to_string(tmp.join(CGROUP_PROCS)).expect("read to string"); assert_eq!(pid_content, "1002"); } } diff --git a/src/cgroups/v1/hugetlb.rs b/src/cgroups/v1/hugetlb.rs index ad1a2604b..1ee473f79 100644 --- a/src/cgroups/v1/hugetlb.rs +++ b/src/cgroups/v1/hugetlb.rs @@ -1,36 +1,27 @@ -use std::{fs, path::Path}; +use std::path::Path; -use anyhow::bail; +use anyhow::{bail, Result}; use regex::Regex; -use crate::cgroups::{ - common::{self, CGROUP_PROCS}, - v1::Controller, -}; +use crate::cgroups::{common, v1::Controller}; use oci_spec::{LinuxHugepageLimit, LinuxResources}; pub struct Hugetlb {} impl Controller for Hugetlb { - fn apply( - linux_resources: &LinuxResources, - cgroup_root: &std::path::Path, - pid: nix::unistd::Pid, - ) -> anyhow::Result<()> { + fn apply(linux_resources: &LinuxResources, cgroup_root: &std::path::Path) -> Result<()> { log::debug!("Apply Hugetlb cgroup config"); - fs::create_dir_all(cgroup_root)?; for hugetlb in &linux_resources.hugepage_limits { Self::apply(cgroup_root, hugetlb)? } - common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } impl Hugetlb { - fn apply(root_path: &Path, hugetlb: &LinuxHugepageLimit) -> anyhow::Result<()> { + fn apply(root_path: &Path, hugetlb: &LinuxHugepageLimit) -> Result<()> { let re = Regex::new(r"(?P[0-9]+)[KMG]B")?; let caps = re.captures(&hugetlb.page_size); match caps { @@ -44,8 +35,8 @@ impl Hugetlb { } common::write_cgroup_file( - &root_path.join(format!("hugetlb.{}.limit_in_bytes", hugetlb.page_size)), - &hugetlb.limit, + root_path.join(format!("hugetlb.{}.limit_in_bytes", hugetlb.page_size)), + hugetlb.limit, )?; Ok(()) } diff --git a/src/cgroups/v1/manager.rs b/src/cgroups/v1/manager.rs index e7814d372..2398b97b3 100644 --- a/src/cgroups/v1/manager.rs +++ b/src/cgroups/v1/manager.rs @@ -7,6 +7,7 @@ use nix::unistd::Pid; use procfs::process::Process; +use super::ControllerType; use super::{ blkio::Blkio, controller_type::CONTROLLERS, cpu::Cpu, cpuacct::CpuAcct, cpuset::CpuSet, devices::Devices, freezer::Freezer, hugetlb::Hugetlb, memory::Memory, @@ -19,16 +20,16 @@ use crate::utils; use crate::{cgroups::common::CgroupManager, utils::PathBufExt}; use oci_spec::LinuxResources; pub struct Manager { - subsystems: HashMap, + subsystems: HashMap, } impl Manager { pub fn new(cgroup_path: PathBuf) -> Result { - let mut subsystems = HashMap::::new(); - for subsystem in CONTROLLERS.iter().map(|c| c.to_string()) { + let mut subsystems = HashMap::::new(); + for subsystem in CONTROLLERS { subsystems.insert( - subsystem.to_owned(), - Self::get_subsystem_path(&cgroup_path, &subsystem)?, + subsystem.clone(), + Self::get_subsystem_path(&cgroup_path, &subsystem.to_string())?, ); } @@ -58,21 +59,44 @@ impl Manager { } impl CgroupManager for Manager { - fn apply(&self, linux_resources: &LinuxResources, pid: Pid) -> Result<()> { + fn add_task(&self, pid: Pid) -> Result<()> { for subsys in &self.subsystems { - match subsys.0.as_str() { - "cpu" => Cpu::apply(linux_resources, &subsys.1, pid)?, - "cpuacct" => CpuAcct::apply(linux_resources, &subsys.1, pid)?, - "cpuset" => CpuSet::apply(linux_resources, &subsys.1, pid)?, - "devices" => Devices::apply(linux_resources, &subsys.1, pid)?, - "hugetlb" => Hugetlb::apply(linux_resources, &subsys.1, pid)?, - "memory" => Memory::apply(linux_resources, &subsys.1, pid)?, - "pids" => Pids::apply(linux_resources, &subsys.1, pid)?, - "blkio" => Blkio::apply(linux_resources, &subsys.1, pid)?, - "net_prio" => NetworkPriority::apply(linux_resources, &subsys.1, pid)?, - "net_cls" => NetworkClassifier::apply(linux_resources, &subsys.1, pid)?, - "freezer" => Freezer::apply(linux_resources, &subsys.1, pid)?, - _ => unreachable!("every subsystem should have an associated controller"), + match subsys.0 { + ControllerType::Cpu => Cpu::add_task(pid, subsys.1)?, + ControllerType::CpuAcct => CpuAcct::add_task(pid, subsys.1)?, + ControllerType::CpuSet => CpuSet::add_task(pid, subsys.1)?, + ControllerType::Devices => Devices::add_task(pid, subsys.1)?, + ControllerType::HugeTlb => Hugetlb::add_task(pid, subsys.1)?, + ControllerType::Memory => Memory::add_task(pid, subsys.1)?, + ControllerType::Pids => Pids::add_task(pid, subsys.1)?, + ControllerType::Blkio => Blkio::add_task(pid, subsys.1)?, + ControllerType::NetworkPriority => NetworkPriority::add_task(pid, subsys.1)?, + ControllerType::NetworkClassifier => NetworkClassifier::add_task(pid, subsys.1)?, + _ => continue, + } + } + + Ok(()) + } + + fn apply(&self, linux_resources: &LinuxResources) -> Result<()> { + for subsys in &self.subsystems { + match subsys.0 { + ControllerType::Cpu => Cpu::apply(linux_resources, &subsys.1)?, + ControllerType::CpuAcct => CpuAcct::apply(linux_resources, &subsys.1)?, + ControllerType::CpuSet => CpuSet::apply(linux_resources, &subsys.1)?, + ControllerType::Devices => Devices::apply(linux_resources, &subsys.1)?, + ControllerType::HugeTlb => Hugetlb::apply(linux_resources, &subsys.1)?, + ControllerType::Memory => Memory::apply(linux_resources, &subsys.1)?, + ControllerType::Pids => Pids::apply(linux_resources, &subsys.1)?, + ControllerType::Blkio => Blkio::apply(linux_resources, &subsys.1)?, + ControllerType::NetworkPriority => { + NetworkPriority::apply(linux_resources, &subsys.1)? + } + ControllerType::NetworkClassifier => { + NetworkClassifier::apply(linux_resources, &subsys.1)? + } + ControllerType::Freezer => Freezer::apply(linux_resources, &subsys.1)?, } } diff --git a/src/cgroups/v1/memory.rs b/src/cgroups/v1/memory.rs index f00a7f450..aa09e58e7 100644 --- a/src/cgroups/v1/memory.rs +++ b/src/cgroups/v1/memory.rs @@ -1,13 +1,10 @@ use std::io::{prelude::*, Write}; -use std::{ - fs::{create_dir_all, OpenOptions}, - path::Path, -}; +use std::{fs::OpenOptions, path::Path}; use anyhow::{Result, *}; -use nix::{errno::Errno, unistd::Pid}; +use nix::errno::Errno; -use crate::cgroups::common::{self, CGROUP_PROCS}; +use crate::cgroups::common::{self}; use crate::cgroups::v1::Controller; use oci_spec::{LinuxMemory, LinuxResources}; @@ -25,9 +22,8 @@ const CGROUP_KERNEL_TCP_MEMORY_LIMIT: &str = "memory.kmem.tcp.limit_in_bytes"; pub struct Memory {} impl Controller for Memory { - fn apply(linux_resources: &LinuxResources, cgroup_root: &Path, pid: Pid) -> Result<()> { + fn apply(linux_resources: &LinuxResources, cgroup_root: &Path) -> Result<()> { log::debug!("Apply Memory cgroup config"); - create_dir_all(&cgroup_root)?; if let Some(memory) = &linux_resources.memory { let reservation = memory.reservation.unwrap_or(0); @@ -76,7 +72,6 @@ impl Controller for Memory { } } - common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } @@ -239,6 +234,7 @@ impl Memory { #[cfg(test)] mod tests { use super::*; + use crate::cgroups::common::CGROUP_PROCS; use crate::cgroups::test::set_fixture; use crate::utils::create_temp_dir; use oci_spec::LinuxMemory; @@ -368,8 +364,7 @@ mod tests { freezer: None, }; - let pid = Pid::from_raw(pid_int); - let result = ::apply(&linux_resources, &tmp, pid); + let result = ::apply(&linux_resources, &tmp); if result.is_err() { if let Some(swappiness) = memory_limits.swappiness { @@ -455,10 +450,6 @@ mod tests { } }; - // check procs file - let procs_content = std::fs::read_to_string(tmp.join(CGROUP_PROCS)).expect("read procs file"); - let procs_check = procs_content == pid.to_string(); - // useful for debugging println!("reservation_check: {:?}", reservation_check); println!("kernel_check: {:?}", kernel_check); @@ -467,7 +458,7 @@ mod tests { println!("limit_swap_check: {:?}", limit_swap_check); // combine all the checks - reservation_check && kernel_check && kernel_tcp_check && swappiness_check && limit_swap_check && procs_check + reservation_check && kernel_check && kernel_tcp_check && swappiness_check && limit_swap_check } } } diff --git a/src/cgroups/v1/network_classifier.rs b/src/cgroups/v1/network_classifier.rs index 88da25653..fed3ae2e7 100644 --- a/src/cgroups/v1/network_classifier.rs +++ b/src/cgroups/v1/network_classifier.rs @@ -1,25 +1,21 @@ -use std::{fs::create_dir_all, path::Path}; +use std::path::Path; use anyhow::Result; -use nix::unistd::Pid; use crate::cgroups::common; -use crate::cgroups::common::CGROUP_PROCS; use crate::cgroups::v1::Controller; use oci_spec::{LinuxNetwork, LinuxResources}; pub struct NetworkClassifier {} impl Controller for NetworkClassifier { - fn apply(linux_resources: &LinuxResources, cgroup_root: &Path, pid: Pid) -> Result<()> { + fn apply(linux_resources: &LinuxResources, cgroup_root: &Path) -> Result<()> { log::debug!("Apply NetworkClassifier cgroup config"); - create_dir_all(&cgroup_root)?; if let Some(network) = linux_resources.network.as_ref() { Self::apply(cgroup_root, network)?; } - common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } diff --git a/src/cgroups/v1/network_priority.rs b/src/cgroups/v1/network_priority.rs index 05f6c3bf6..6a25a017c 100644 --- a/src/cgroups/v1/network_priority.rs +++ b/src/cgroups/v1/network_priority.rs @@ -1,25 +1,21 @@ -use std::{fs::create_dir_all, path::Path}; +use std::path::Path; use anyhow::Result; -use nix::unistd::Pid; use crate::cgroups::common; -use crate::cgroups::common::CGROUP_PROCS; use crate::cgroups::v1::Controller; use oci_spec::{LinuxNetwork, LinuxResources}; pub struct NetworkPriority {} impl Controller for NetworkPriority { - fn apply(linux_resources: &LinuxResources, cgroup_root: &Path, pid: Pid) -> Result<()> { + fn apply(linux_resources: &LinuxResources, cgroup_root: &Path) -> Result<()> { log::debug!("Apply NetworkPriority cgroup config"); - create_dir_all(&cgroup_root)?; if let Some(network) = linux_resources.network.as_ref() { Self::apply(cgroup_root, network)?; } - common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } diff --git a/src/cgroups/v1/pids.rs b/src/cgroups/v1/pids.rs index e41153db1..09f905cce 100644 --- a/src/cgroups/v1/pids.rs +++ b/src/cgroups/v1/pids.rs @@ -1,14 +1,8 @@ -use std::{ - fs::{self}, - path::Path, -}; +use std::path::Path; use anyhow::Result; -use crate::cgroups::{ - common::{self, CGROUP_PROCS}, - v1::Controller, -}; +use crate::cgroups::{common, v1::Controller}; use oci_spec::{LinuxPids, LinuxResources}; pub struct Pids {} @@ -17,16 +11,13 @@ impl Controller for Pids { fn apply( linux_resources: &LinuxResources, cgroup_root: &std::path::Path, - pid: nix::unistd::Pid, ) -> anyhow::Result<()> { log::debug!("Apply pids cgroup config"); - fs::create_dir_all(cgroup_root)?; if let Some(pids) = &linux_resources.pids { Self::apply(cgroup_root, pids)?; } - common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } diff --git a/src/cgroups/v2/manager.rs b/src/cgroups/v2/manager.rs index 77a04c618..1da2aae4f 100644 --- a/src/cgroups/v2/manager.rs +++ b/src/cgroups/v2/manager.rs @@ -34,28 +34,31 @@ const CONTROLLER_TYPES: &[ControllerType] = &[ pub struct Manager { root_path: PathBuf, cgroup_path: PathBuf, + full_path: PathBuf, } impl Manager { pub fn new(root_path: PathBuf, cgroup_path: PathBuf) -> Result { + let full_path = root_path.join_absolute_path(&cgroup_path)?; + Ok(Self { root_path, cgroup_path, + full_path, }) } - fn create_unified_cgroup(&self, cgroup_path: &Path, pid: Pid) -> Result { - let full_path = self.root_path.join_absolute_path(cgroup_path)?; + fn create_unified_cgroup(&self, pid: Pid) -> Result<()> { let controllers: Vec = self - .get_available_controllers(&self.root_path)? - .into_iter() + .get_available_controllers()? + .iter() .map(|c| format!("{}{}", "+", c.to_string())) .collect(); Self::write_controllers(&self.root_path, &controllers)?; let mut current_path = self.root_path.clone(); - let mut components = cgroup_path.components().skip(1).peekable(); + let mut components = self.cgroup_path.components().skip(1).peekable(); while let Some(component) = components.next() { current_path = current_path.join(component); if !current_path.exists() { @@ -70,15 +73,12 @@ impl Manager { } } - common::write_cgroup_file(&full_path.join(CGROUP_PROCS), pid)?; - Ok(full_path) + common::write_cgroup_file(&self.full_path.join(CGROUP_PROCS), pid)?; + Ok(()) } - fn get_available_controllers>( - &self, - cgroup_path: P, - ) -> Result> { - let controllers_path = self.root_path.join(cgroup_path).join(CGROUP_CONTROLLERS); + fn get_available_controllers(&self) -> Result> { + let controllers_path = self.root_path.join(CGROUP_CONTROLLERS); if !controllers_path.exists() { bail!( "cannot get available controllers. {:?} does not exist", @@ -112,17 +112,20 @@ impl Manager { } impl CgroupManager for Manager { - fn apply(&self, linux_resources: &LinuxResources, pid: Pid) -> Result<()> { - let full_cgroup_path = self.create_unified_cgroup(&self.cgroup_path, pid)?; + fn add_task(&self, pid: Pid) -> Result<()> { + self.create_unified_cgroup(pid)?; + Ok(()) + } + fn apply(&self, linux_resources: &LinuxResources) -> Result<()> { for controller in CONTROLLER_TYPES { match controller { - ControllerType::Cpu => Cpu::apply(linux_resources, &full_cgroup_path)?, - ControllerType::CpuSet => CpuSet::apply(linux_resources, &full_cgroup_path)?, - ControllerType::HugeTlb => HugeTlb::apply(linux_resources, &&full_cgroup_path)?, - ControllerType::Io => Io::apply(linux_resources, &&full_cgroup_path)?, - ControllerType::Memory => Memory::apply(linux_resources, &full_cgroup_path)?, - ControllerType::Pids => Pids::apply(linux_resources, &&full_cgroup_path)?, + ControllerType::Cpu => Cpu::apply(linux_resources, &self.full_path)?, + ControllerType::CpuSet => CpuSet::apply(linux_resources, &self.full_path)?, + ControllerType::HugeTlb => HugeTlb::apply(linux_resources, &self.full_path)?, + ControllerType::Io => Io::apply(linux_resources, &self.full_path)?, + ControllerType::Memory => Memory::apply(linux_resources, &self.full_path)?, + ControllerType::Pids => Pids::apply(linux_resources, &self.full_path)?, } } @@ -130,9 +133,8 @@ impl CgroupManager for Manager { } fn remove(&self) -> Result<()> { - let full_path = self.root_path.join_absolute_path(&self.cgroup_path)?; - log::debug!("remove cgroup {:?}", full_path); - fs::remove_dir_all(full_path)?; + log::debug!("remove cgroup {:?}", self.full_path); + fs::remove_dir_all(&self.full_path)?; Ok(()) } diff --git a/src/cgroups/v2/systemd_manager.rs b/src/cgroups/v2/systemd_manager.rs index a21b65bf6..726ed1a7e 100644 --- a/src/cgroups/v2/systemd_manager.rs +++ b/src/cgroups/v2/systemd_manager.rs @@ -30,7 +30,8 @@ const CONTROLLER_TYPES: &[ControllerType] = &[ /// SystemDCGroupManager is a driver for managing cgroups via systemd. pub struct SystemDCGroupManager { root_path: PathBuf, - cgroups_path: CgroupsPath, + cgroups_path: PathBuf, + full_path: PathBuf, } /// Represents the systemd cgroups path: @@ -45,6 +46,19 @@ struct CgroupsPath { impl SystemDCGroupManager { pub fn new(root_path: PathBuf, cgroups_path: PathBuf) -> Result { + // TODO: create the systemd unit using a dbus client. + let cgroups_path = Self::new_cgroups_path(cgroups_path)?; + let cgroups_path = Self::get_cgroups_path(cgroups_path)?; + let full_path = root_path.join_absolute_path(&cgroups_path)?; + + Ok(SystemDCGroupManager { + root_path, + cgroups_path, + full_path, + }) + } + + fn new_cgroups_path(cgroups_path: PathBuf) -> Result { // cgroups path may never be empty as it is defaulted to `/youki` // see 'get_cgroup_path' under utils.rs. // if cgroups_path was provided it should be of the form [slice]:[scope_prefix]:[name], @@ -69,35 +83,27 @@ impl SystemDCGroupManager { name = parts[2]; } - // TODO: create the systemd unit using a dbus client. - - Ok(SystemDCGroupManager { - root_path, - cgroups_path: CgroupsPath { - parent: parent.to_owned(), - scope: scope.to_owned(), - name: name.to_owned(), - }, + Ok(CgroupsPath { + parent: parent.to_owned(), + scope: scope.to_owned(), + name: name.to_owned(), }) } /// get_unit_name returns the unit (scope) name from the path provided by the user /// for example: foo:docker:bar returns in '/docker-bar.scope' - fn get_unit_name(&self) -> String { + fn get_unit_name(cgroups_path: CgroupsPath) -> String { // By default we create a scope unless specified explicitly. - if !self.cgroups_path.name.ends_with(".slice") { - return format!( - "{}-{}.scope", - self.cgroups_path.scope, self.cgroups_path.name - ); + if !cgroups_path.name.ends_with(".slice") { + return format!("{}-{}.scope", cgroups_path.scope, cgroups_path.name); } - self.cgroups_path.name.clone() + cgroups_path.name.clone() } // 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(&self, 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); @@ -125,15 +131,15 @@ impl SystemDCGroupManager { // get_cgroups_path generates a cgroups path from the one provided by the user via cgroupsPath. // an example of the final path: "/machine.slice/docker-foo.scope" - fn get_cgroups_path(&self) -> Result { + fn get_cgroups_path(cgroups_path: CgroupsPath) -> Result { // the root slice is under 'machine.slice'. let mut slice = Path::new("/machine.slice").to_path_buf(); // if the user provided a '.slice' (as in a branch of a tree) // we need to "unpack it". - if !self.cgroups_path.parent.is_empty() { - slice = self.expand_slice(&self.cgroups_path.parent)?; + if !cgroups_path.parent.is_empty() { + slice = Self::expand_slice(&cgroups_path.parent)?; } - let unit_name = self.get_unit_name(); + let unit_name = Self::get_unit_name(cgroups_path); let cgroups_path = slice.join(unit_name); Ok(cgroups_path) } @@ -141,9 +147,7 @@ impl SystemDCGroupManager { /// create_unified_cgroup verifies sure that *each level* in the downward path from the root cgroup /// down to the cgroup_path provided by the user is a valid cgroup hierarchy, /// containing the attached controllers and that it contains the container pid. - fn create_unified_cgroup(&self, pid: Pid) -> Result { - let cgroups_path = self.get_cgroups_path()?; - let full_path = self.root_path.join_absolute_path(&cgroups_path)?; + fn create_unified_cgroup(&self, pid: Pid) -> Result<()> { let controllers: Vec = self .get_available_controllers(&self.root_path)? .into_iter() @@ -154,7 +158,7 @@ impl SystemDCGroupManager { Self::write_controllers(&self.root_path, &controllers)?; let mut current_path = self.root_path.clone(); - let mut components = cgroups_path.components().skip(1).peekable(); + let mut components = self.cgroups_path.components().skip(1).peekable(); // Verify that *each level* in the downward path from the root cgroup // down to the cgroup_path provided by the user is a valid cgroup hierarchy. // containing the attached controllers. @@ -172,8 +176,8 @@ impl SystemDCGroupManager { } } - common::write_cgroup_file(full_path.join(CGROUP_PROCS), &pid)?; - Ok(full_path) + common::write_cgroup_file(self.full_path.join(CGROUP_PROCS), &pid)?; + Ok(()) } fn get_available_controllers>( @@ -212,21 +216,25 @@ impl SystemDCGroupManager { } impl CgroupManager for SystemDCGroupManager { - fn apply(&self, linux_resources: &LinuxResources, pid: Pid) -> Result<()> { + fn add_task(&self, pid: Pid) -> Result<()> { // Dont attach any pid to the cgroup if -1 is specified as a pid if pid.as_raw() == -1 { return Ok(()); } - let full_cgroup_path = self.create_unified_cgroup(pid)?; + self.create_unified_cgroup(pid)?; + Ok(()) + } + + fn apply(&self, linux_resources: &LinuxResources) -> Result<()> { for controller in CONTROLLER_TYPES { match controller { - ControllerType::Cpu => Cpu::apply(linux_resources, &full_cgroup_path)?, - ControllerType::CpuSet => CpuSet::apply(linux_resources, &full_cgroup_path)?, - ControllerType::HugeTlb => HugeTlb::apply(linux_resources, &&full_cgroup_path)?, - ControllerType::Io => Io::apply(linux_resources, &&full_cgroup_path)?, - ControllerType::Memory => Memory::apply(linux_resources, &full_cgroup_path)?, - ControllerType::Pids => Pids::apply(linux_resources, &&full_cgroup_path)?, + ControllerType::Cpu => Cpu::apply(linux_resources, &self.full_path)?, + ControllerType::CpuSet => CpuSet::apply(linux_resources, &self.full_path)?, + ControllerType::HugeTlb => HugeTlb::apply(linux_resources, &self.full_path)?, + ControllerType::Io => Io::apply(linux_resources, &self.full_path)?, + ControllerType::Memory => Memory::apply(linux_resources, &self.full_path)?, + ControllerType::Pids => Pids::apply(linux_resources, &self.full_path)?, } } @@ -244,13 +252,8 @@ mod tests { #[test] fn expand_slice_works() -> Result<()> { - let manager = SystemDCGroupManager::new( - PathBuf::from("/sys/fs/cgroup"), - PathBuf::from("test-a-b.slice:docker:foo"), - )?; - assert_eq!( - manager.expand_slice("test-a-b.slice")?, + SystemDCGroupManager::expand_slice("test-a-b.slice")?, PathBuf::from("/test.slice/test-a.slice/test-a-b.slice"), ); @@ -259,13 +262,12 @@ mod tests { #[test] fn get_cgroups_path_works_with_a_complex_slice() -> Result<()> { - let manager = SystemDCGroupManager::new( - PathBuf::from("/sys/fs/cgroup"), - PathBuf::from("test-a-b.slice:docker:foo"), - )?; + let cgroups_path = + SystemDCGroupManager::new_cgroups_path(PathBuf::from("test-a-b.slice:docker:foo")) + .expect(""); assert_eq!( - manager.get_cgroups_path()?, + SystemDCGroupManager::get_cgroups_path(cgroups_path)?, PathBuf::from("/test.slice/test-a.slice/test-a-b.slice/docker-foo.scope"), ); @@ -274,13 +276,12 @@ mod tests { #[test] fn get_cgroups_path_works_with_a_simple_slice() -> Result<()> { - let manager = SystemDCGroupManager::new( - PathBuf::from("/sys/fs/cgroup"), - PathBuf::from("machine.slice:libpod:foo"), - )?; + let cgroups_path = + SystemDCGroupManager::new_cgroups_path(PathBuf::from("machine.slice:libpod:foo")) + .expect(""); assert_eq!( - manager.get_cgroups_path()?, + SystemDCGroupManager::get_cgroups_path(cgroups_path)?, PathBuf::from("/machine.slice/libpod-foo.scope"), ); @@ -289,13 +290,11 @@ mod tests { #[test] fn get_cgroups_path_works_with_scope() -> Result<()> { - let manager = SystemDCGroupManager::new( - PathBuf::from("/sys/fs/cgroup"), - PathBuf::from(":docker:foo"), - )?; + let cgroups_path = + SystemDCGroupManager::new_cgroups_path(PathBuf::from(":docker:foo")).expect(""); assert_eq!( - manager.get_cgroups_path()?, + SystemDCGroupManager::get_cgroups_path(cgroups_path)?, PathBuf::from("/machine.slice/docker-foo.scope"), ); diff --git a/src/process/fork.rs b/src/process/fork.rs index 211d59915..4fe23ff76 100644 --- a/src/process/fork.rs +++ b/src/process/fork.rs @@ -69,7 +69,8 @@ pub fn fork_first>( let init_pid = parent.wait_for_child_ready(child)?; log::debug!("init pid is {:?}", init_pid); if rootless.is_none() && linux.resources.is_some() { - cmanager.apply(&linux.resources.as_ref().unwrap(), Pid::from_raw(init_pid))?; + cmanager.add_task(Pid::from_raw(init_pid))?; + cmanager.apply(&linux.resources.as_ref().unwrap())?; } // update status and pid of the container process From 8f872d74caaca6b48b2de84108b4d08175acd30c Mon Sep 17 00:00:00 2001 From: Furisto <24721048+Furisto@users.noreply.github.com> Date: Sat, 26 Jun 2021 10:58:31 +0200 Subject: [PATCH 2/3] Shorten names --- src/cgroups/v1/manager.rs | 50 +++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/src/cgroups/v1/manager.rs b/src/cgroups/v1/manager.rs index 2398b97b3..a79773027 100644 --- a/src/cgroups/v1/manager.rs +++ b/src/cgroups/v1/manager.rs @@ -7,7 +7,7 @@ use nix::unistd::Pid; use procfs::process::Process; -use super::ControllerType; +use super::ControllerType as CtrlType; use super::{ blkio::Blkio, controller_type::CONTROLLERS, cpu::Cpu, cpuacct::CpuAcct, cpuset::CpuSet, devices::Devices, freezer::Freezer, hugetlb::Hugetlb, memory::Memory, @@ -20,12 +20,12 @@ use crate::utils; use crate::{cgroups::common::CgroupManager, utils::PathBufExt}; use oci_spec::LinuxResources; pub struct Manager { - subsystems: HashMap, + subsystems: HashMap, } impl Manager { pub fn new(cgroup_path: PathBuf) -> Result { - let mut subsystems = HashMap::::new(); + let mut subsystems = HashMap::::new(); for subsystem in CONTROLLERS { subsystems.insert( subsystem.clone(), @@ -62,16 +62,16 @@ impl CgroupManager for Manager { fn add_task(&self, pid: Pid) -> Result<()> { for subsys in &self.subsystems { match subsys.0 { - ControllerType::Cpu => Cpu::add_task(pid, subsys.1)?, - ControllerType::CpuAcct => CpuAcct::add_task(pid, subsys.1)?, - ControllerType::CpuSet => CpuSet::add_task(pid, subsys.1)?, - ControllerType::Devices => Devices::add_task(pid, subsys.1)?, - ControllerType::HugeTlb => Hugetlb::add_task(pid, subsys.1)?, - ControllerType::Memory => Memory::add_task(pid, subsys.1)?, - ControllerType::Pids => Pids::add_task(pid, subsys.1)?, - ControllerType::Blkio => Blkio::add_task(pid, subsys.1)?, - ControllerType::NetworkPriority => NetworkPriority::add_task(pid, subsys.1)?, - ControllerType::NetworkClassifier => NetworkClassifier::add_task(pid, subsys.1)?, + CtrlType::Cpu => Cpu::add_task(pid, subsys.1)?, + CtrlType::CpuAcct => CpuAcct::add_task(pid, subsys.1)?, + CtrlType::CpuSet => CpuSet::add_task(pid, subsys.1)?, + CtrlType::Devices => Devices::add_task(pid, subsys.1)?, + CtrlType::HugeTlb => Hugetlb::add_task(pid, subsys.1)?, + CtrlType::Memory => Memory::add_task(pid, subsys.1)?, + CtrlType::Pids => Pids::add_task(pid, subsys.1)?, + CtrlType::Blkio => Blkio::add_task(pid, subsys.1)?, + CtrlType::NetworkPriority => NetworkPriority::add_task(pid, subsys.1)?, + CtrlType::NetworkClassifier => NetworkClassifier::add_task(pid, subsys.1)?, _ => continue, } } @@ -82,21 +82,19 @@ impl CgroupManager for Manager { fn apply(&self, linux_resources: &LinuxResources) -> Result<()> { for subsys in &self.subsystems { match subsys.0 { - ControllerType::Cpu => Cpu::apply(linux_resources, &subsys.1)?, - ControllerType::CpuAcct => CpuAcct::apply(linux_resources, &subsys.1)?, - ControllerType::CpuSet => CpuSet::apply(linux_resources, &subsys.1)?, - ControllerType::Devices => Devices::apply(linux_resources, &subsys.1)?, - ControllerType::HugeTlb => Hugetlb::apply(linux_resources, &subsys.1)?, - ControllerType::Memory => Memory::apply(linux_resources, &subsys.1)?, - ControllerType::Pids => Pids::apply(linux_resources, &subsys.1)?, - ControllerType::Blkio => Blkio::apply(linux_resources, &subsys.1)?, - ControllerType::NetworkPriority => { - NetworkPriority::apply(linux_resources, &subsys.1)? - } - ControllerType::NetworkClassifier => { + CtrlType::Cpu => Cpu::apply(linux_resources, &subsys.1)?, + CtrlType::CpuAcct => CpuAcct::apply(linux_resources, &subsys.1)?, + CtrlType::CpuSet => CpuSet::apply(linux_resources, &subsys.1)?, + CtrlType::Devices => Devices::apply(linux_resources, &subsys.1)?, + CtrlType::HugeTlb => Hugetlb::apply(linux_resources, &subsys.1)?, + CtrlType::Memory => Memory::apply(linux_resources, &subsys.1)?, + CtrlType::Pids => Pids::apply(linux_resources, &subsys.1)?, + CtrlType::Blkio => Blkio::apply(linux_resources, &subsys.1)?, + CtrlType::NetworkPriority => NetworkPriority::apply(linux_resources, &subsys.1)?, + CtrlType::NetworkClassifier => { NetworkClassifier::apply(linux_resources, &subsys.1)? } - ControllerType::Freezer => Freezer::apply(linux_resources, &subsys.1)?, + CtrlType::Freezer => Freezer::apply(linux_resources, &subsys.1)?, } } From fcfff8bf4594e7aae48695c01c9fb7bdfdf5c527 Mon Sep 17 00:00:00 2001 From: Furisto <24721048+Furisto@users.noreply.github.com> Date: Sat, 26 Jun 2021 11:03:54 +0200 Subject: [PATCH 3/3] Address review comments - Add comments for functions - Use better naming in systemd cgroup manager --- src/cgroups/common.rs | 3 +++ src/cgroups/v1/manager.rs | 1 + src/cgroups/v2/manager.rs | 2 ++ src/cgroups/v2/systemd_manager.rs | 25 ++++++++++++------------- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/cgroups/common.rs b/src/cgroups/common.rs index 84a6dc148..20fd04101 100644 --- a/src/cgroups/common.rs +++ b/src/cgroups/common.rs @@ -19,8 +19,11 @@ pub const CGROUP_PROCS: &str = "cgroup.procs"; pub const DEFAULT_CGROUP_ROOT: &str = "/sys/fs/cgroup"; pub trait CgroupManager { + /// Adds a task specified by its pid to the cgroup fn add_task(&self, pid: Pid) -> Result<()>; + /// Applies resource restrictions to the cgroup fn apply(&self, linux_resources: &LinuxResources) -> Result<()>; + /// Removes the cgroup fn remove(&self) -> Result<()>; } diff --git a/src/cgroups/v1/manager.rs b/src/cgroups/v1/manager.rs index a79773027..3266f3bf1 100644 --- a/src/cgroups/v1/manager.rs +++ b/src/cgroups/v1/manager.rs @@ -24,6 +24,7 @@ pub struct Manager { } 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 { let mut subsystems = HashMap::::new(); for subsystem in CONTROLLERS { diff --git a/src/cgroups/v2/manager.rs b/src/cgroups/v2/manager.rs index 1da2aae4f..5a605e294 100644 --- a/src/cgroups/v2/manager.rs +++ b/src/cgroups/v2/manager.rs @@ -38,6 +38,8 @@ 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 { let full_path = root_path.join_absolute_path(&cgroup_path)?; diff --git a/src/cgroups/v2/systemd_manager.rs b/src/cgroups/v2/systemd_manager.rs index 726ed1a7e..375fc1a7e 100644 --- a/src/cgroups/v2/systemd_manager.rs +++ b/src/cgroups/v2/systemd_manager.rs @@ -47,8 +47,8 @@ struct CgroupsPath { impl SystemDCGroupManager { pub fn new(root_path: PathBuf, cgroups_path: PathBuf) -> Result { // TODO: create the systemd unit using a dbus client. - let cgroups_path = Self::new_cgroups_path(cgroups_path)?; - let cgroups_path = Self::get_cgroups_path(cgroups_path)?; + let destructured_path = Self::destructure_cgroups_path(cgroups_path)?; + let cgroups_path = Self::construct_cgroups_path(destructured_path)?; let full_path = root_path.join_absolute_path(&cgroups_path)?; Ok(SystemDCGroupManager { @@ -58,7 +58,7 @@ impl SystemDCGroupManager { }) } - fn new_cgroups_path(cgroups_path: PathBuf) -> Result { + fn destructure_cgroups_path(cgroups_path: PathBuf) -> Result { // cgroups path may never be empty as it is defaulted to `/youki` // see 'get_cgroup_path' under utils.rs. // if cgroups_path was provided it should be of the form [slice]:[scope_prefix]:[name], @@ -97,7 +97,7 @@ impl SystemDCGroupManager { if !cgroups_path.name.ends_with(".slice") { return format!("{}-{}.scope", cgroups_path.scope, cgroups_path.name); } - cgroups_path.name.clone() + cgroups_path.name } // systemd represents slice hierarchy using `-`, so we need to follow suit when @@ -131,7 +131,7 @@ impl SystemDCGroupManager { // get_cgroups_path generates a cgroups path from the one provided by the user via cgroupsPath. // an example of the final path: "/machine.slice/docker-foo.scope" - fn get_cgroups_path(cgroups_path: CgroupsPath) -> Result { + fn construct_cgroups_path(cgroups_path: CgroupsPath) -> Result { // the root slice is under 'machine.slice'. let mut slice = Path::new("/machine.slice").to_path_buf(); // if the user provided a '.slice' (as in a branch of a tree) @@ -176,8 +176,7 @@ impl SystemDCGroupManager { } } - common::write_cgroup_file(self.full_path.join(CGROUP_PROCS), &pid)?; - Ok(()) + common::write_cgroup_file(self.full_path.join(CGROUP_PROCS), pid) } fn get_available_controllers>( @@ -263,11 +262,11 @@ mod tests { #[test] fn get_cgroups_path_works_with_a_complex_slice() -> Result<()> { let cgroups_path = - SystemDCGroupManager::new_cgroups_path(PathBuf::from("test-a-b.slice:docker:foo")) + SystemDCGroupManager::destructure_cgroups_path(PathBuf::from("test-a-b.slice:docker:foo")) .expect(""); assert_eq!( - SystemDCGroupManager::get_cgroups_path(cgroups_path)?, + SystemDCGroupManager::construct_cgroups_path(cgroups_path)?, PathBuf::from("/test.slice/test-a.slice/test-a-b.slice/docker-foo.scope"), ); @@ -277,11 +276,11 @@ mod tests { #[test] fn get_cgroups_path_works_with_a_simple_slice() -> Result<()> { let cgroups_path = - SystemDCGroupManager::new_cgroups_path(PathBuf::from("machine.slice:libpod:foo")) + SystemDCGroupManager::destructure_cgroups_path(PathBuf::from("machine.slice:libpod:foo")) .expect(""); assert_eq!( - SystemDCGroupManager::get_cgroups_path(cgroups_path)?, + SystemDCGroupManager::construct_cgroups_path(cgroups_path)?, PathBuf::from("/machine.slice/libpod-foo.scope"), ); @@ -291,10 +290,10 @@ mod tests { #[test] fn get_cgroups_path_works_with_scope() -> Result<()> { let cgroups_path = - SystemDCGroupManager::new_cgroups_path(PathBuf::from(":docker:foo")).expect(""); + SystemDCGroupManager::destructure_cgroups_path(PathBuf::from(":docker:foo")).expect(""); assert_eq!( - SystemDCGroupManager::get_cgroups_path(cgroups_path)?, + SystemDCGroupManager::construct_cgroups_path(cgroups_path)?, PathBuf::from("/machine.slice/docker-foo.scope"), );