From 86ce6d8139f196007a18a1326d162a0e8884f103 Mon Sep 17 00:00:00 2001 From: Furisto <24721048+Furisto@users.noreply.github.com> Date: Thu, 3 Jun 2021 22:21:51 +0200 Subject: [PATCH 1/8] Add cgroup v1 cpu controller --- src/cgroups/common.rs | 15 ++++++- src/cgroups/v1/controller_type.rs | 2 + src/cgroups/v1/cpu.rs | 66 +++++++++++++++++++++++++++++++ src/cgroups/v1/manager.rs | 12 ++++-- src/cgroups/v1/mod.rs | 1 + 5 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 src/cgroups/v1/cpu.rs diff --git a/src/cgroups/common.rs b/src/cgroups/common.rs index 77ae5abe0..41160edc2 100644 --- a/src/cgroups/common.rs +++ b/src/cgroups/common.rs @@ -14,6 +14,7 @@ use procfs::process::Process; use crate::cgroups::v1; use crate::cgroups::v2; +pub const CGROUP_PROCS: &str = "cgroup.procs"; pub const DEFAULT_CGROUP_ROOT: &str = "/sys/fs/cgroup"; pub trait CgroupManager { @@ -50,6 +51,18 @@ pub fn write_cgroup_file>(path: P, data: &str) -> Result<()> { Ok(()) } +#[inline] +pub fn write_cgroup_file_, T: ToString>(path: P, data: T) -> Result<()> { + fs::OpenOptions::new() + .create(false) + .write(true) + .truncate(false) + .open(path)? + .write_all(data.to_string().as_bytes())?; + + Ok(()) +} + pub fn create_cgroup_manager>(cgroup_path: P) -> Result> { let cgroup_mount = Process::myself()? .mountinfo()? @@ -84,7 +97,7 @@ pub fn create_cgroup_manager>(cgroup_path: P) -> Result Ok(Box::new(v1::manager::Manager::new(cgroup_path.into())?)), - } + } } _ => bail!("could not find cgroup filesystem"), } diff --git a/src/cgroups/v1/controller_type.rs b/src/cgroups/v1/controller_type.rs index 933c593b1..96fa7161c 100644 --- a/src/cgroups/v1/controller_type.rs +++ b/src/cgroups/v1/controller_type.rs @@ -1,6 +1,7 @@ use std::string::ToString; pub enum ControllerType { + Cpu, Devices, HugeTlb, Pids, @@ -13,6 +14,7 @@ pub enum ControllerType { impl ToString for ControllerType { fn to_string(&self) -> String { match self { + Self::Cpu => "cpu".into(), Self::Devices => "devices".into(), Self::HugeTlb => "hugetlb".into(), Self::Pids => "pids".into(), diff --git a/src/cgroups/v1/cpu.rs b/src/cgroups/v1/cpu.rs new file mode 100644 index 000000000..412142e34 --- /dev/null +++ b/src/cgroups/v1/cpu.rs @@ -0,0 +1,66 @@ +use std::{fs, path::Path}; + +use anyhow::Result; +use nix::unistd::Pid; +use oci_spec::{LinuxCpu, LinuxResources}; + +use crate::cgroups::common::{self, CGROUP_PROCS}; + +use super::Controller; + +const CGROUP_CPU_SHARES: &str = "cpu.shares"; +const CGROUP_CPU_QUOTA: &str = "cpu.cfs_quota_us"; +const CGROUP_CPU_PERIOD: &str = "cpu.cfs_period_us"; +const CGROUP_CPU_RT_RUNTIME: &str = "cpu.rt_runtime_us"; +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<()> { + 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(()) + } +} + +impl Cpu { + fn apply(root_path: &Path, cpu: &LinuxCpu) -> Result<()> { + if let Some(cpu_shares) = cpu.shares { + if cpu_shares != 0 { + common::write_cgroup_file_(root_path.join(CGROUP_CPU_SHARES), cpu_shares)?; + } + } + + if let Some(cpu_period) = cpu.period { + if cpu_period != 0 { + common::write_cgroup_file_(root_path.join(CGROUP_CPU_PERIOD), cpu_period)?; + } + } + + if let Some(cpu_quota) = cpu.quota { + if cpu_quota != 0 { + common::write_cgroup_file_(root_path.join(CGROUP_CPU_QUOTA), cpu_quota)?; + } + } + + if let Some(rt_runtime) = cpu.realtime_runtime { + if rt_runtime != 0 { + common::write_cgroup_file_(root_path.join(CGROUP_CPU_RT_RUNTIME), rt_runtime)?; + } + } + + if let Some(rt_period) = cpu.realtime_period { + if rt_period != 0 { + common::write_cgroup_file_(root_path.join(CGROUP_CPU_RT_PERIOD), rt_period)?; + } + } + + Ok(()) + } +} diff --git a/src/cgroups/v1/manager.rs b/src/cgroups/v1/manager.rs index 24949a957..28824a56d 100644 --- a/src/cgroups/v1/manager.rs +++ b/src/cgroups/v1/manager.rs @@ -7,15 +7,16 @@ use nix::unistd::Pid; use procfs::process::Process; use super::{ - blkio::Blkio, devices::Devices, hugetlb::Hugetlb, memory::Memory, + blkio::Blkio, cpu::Cpu, devices::Devices, hugetlb::Hugetlb, memory::Memory, network_classifier::NetworkClassifier, network_priority::NetworkPriority, pids::Pids, - Controller, + Controller, ControllerType, }; -use crate::{cgroups::common::CgroupManager, cgroups::v1::ControllerType, utils::PathBufExt}; +use crate::{cgroups::common::CgroupManager, utils::PathBufExt}; use oci_spec::LinuxResources; const CONTROLLERS: &[ControllerType] = &[ + ControllerType::Cpu, ControllerType::Devices, ControllerType::HugeTlb, ControllerType::Memory, @@ -56,6 +57,10 @@ impl Manager { return m.mount_point.ends_with("net_cls,net_prio") || m.mount_point.ends_with("net_prio,net_cls"); } + + if subsystem == "cpu" { + return m.mount_point.ends_with("cpu,cpuacct"); + } } m.mount_point.ends_with(subsystem) }) @@ -83,6 +88,7 @@ impl CgroupManager for Manager { fn apply(&self, linux_resources: &LinuxResources, pid: Pid) -> Result<()> { for subsys in &self.subsystems { match subsys.0.as_str() { + "cpu" => Cpu::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)?, diff --git a/src/cgroups/v1/mod.rs b/src/cgroups/v1/mod.rs index c2b57df56..bd94050c2 100644 --- a/src/cgroups/v1/mod.rs +++ b/src/cgroups/v1/mod.rs @@ -1,6 +1,7 @@ mod blkio; mod controller; mod controller_type; +mod cpu; mod devices; mod hugetlb; pub mod manager; From 997449dcc4708ad42ce498d2dae0b79e6590c9c3 Mon Sep 17 00:00:00 2001 From: Furisto <24721048+Furisto@users.noreply.github.com> Date: Thu, 3 Jun 2021 23:10:24 +0200 Subject: [PATCH 2/8] Add cgroup v1 cpuset controller --- src/cgroups/v1/controller_type.rs | 2 ++ src/cgroups/v1/cpuset.rs | 42 +++++++++++++++++++++++++++++++ src/cgroups/v1/manager.rs | 6 +++-- src/cgroups/v1/mod.rs | 1 + 4 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 src/cgroups/v1/cpuset.rs diff --git a/src/cgroups/v1/controller_type.rs b/src/cgroups/v1/controller_type.rs index 96fa7161c..735a60a17 100644 --- a/src/cgroups/v1/controller_type.rs +++ b/src/cgroups/v1/controller_type.rs @@ -2,6 +2,7 @@ use std::string::ToString; pub enum ControllerType { Cpu, + CpuSet, Devices, HugeTlb, Pids, @@ -15,6 +16,7 @@ impl ToString for ControllerType { fn to_string(&self) -> String { match self { Self::Cpu => "cpu".into(), + Self::CpuSet => "cpuset".into(), Self::Devices => "devices".into(), Self::HugeTlb => "hugetlb".into(), Self::Pids => "pids".into(), diff --git a/src/cgroups/v1/cpuset.rs b/src/cgroups/v1/cpuset.rs new file mode 100644 index 000000000..3ab2d640f --- /dev/null +++ b/src/cgroups/v1/cpuset.rs @@ -0,0 +1,42 @@ +use std::{fs, path::Path}; + +use anyhow::Result; +use nix::unistd::Pid; +use oci_spec::{LinuxCpu, LinuxResources}; + +use crate::cgroups::common::{self, CGROUP_PROCS}; + +use super::Controller; + +const CGROUP_CPUSET_CPUS: &str = "cpuset.cpus"; +const CGROUP_CPUSET_MEMS: &str = "cpuset.mems"; + +pub struct CpuSet {} + +impl Controller for CpuSet { + fn apply(linux_resources: &LinuxResources, cgroup_root: &Path, pid: Pid) -> Result<()> { + log::debug!("Apply CpuSet cgroup config"); + fs::create_dir_all(cgroup_root)?; + + if let Some(cpuset) = &linux_resources.cpu { + Self::apply(cgroup_root, cpuset)?; + } + + common::write_cgroup_file_(cgroup_root.join(CGROUP_PROCS), pid)?; + Ok(()) + } +} + +impl CpuSet { + fn apply(root_path: &Path, cpuset: &LinuxCpu) -> Result<()> { + if let Some(cpus) = &cpuset.cpus { + common::write_cgroup_file(root_path.join(CGROUP_CPUSET_CPUS), cpus)?; + } + + if let Some(mems) = &cpuset.mems { + common::write_cgroup_file(root_path.join(CGROUP_CPUSET_MEMS), mems)?; + } + + Ok(()) + } +} diff --git a/src/cgroups/v1/manager.rs b/src/cgroups/v1/manager.rs index 28824a56d..fd8c922e3 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::{ - blkio::Blkio, cpu::Cpu, devices::Devices, hugetlb::Hugetlb, memory::Memory, + blkio::Blkio, cpu::Cpu, cpuset::CpuSet, devices::Devices, hugetlb::Hugetlb, memory::Memory, network_classifier::NetworkClassifier, network_priority::NetworkPriority, pids::Pids, Controller, ControllerType, }; @@ -17,6 +17,7 @@ use oci_spec::LinuxResources; const CONTROLLERS: &[ControllerType] = &[ ControllerType::Cpu, + ControllerType::CpuSet, ControllerType::Devices, ControllerType::HugeTlb, ControllerType::Memory, @@ -89,6 +90,7 @@ impl CgroupManager for Manager { for subsys in &self.subsystems { match subsys.0.as_str() { "cpu" => Cpu::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)?, @@ -96,7 +98,7 @@ impl CgroupManager for Manager { "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)?, - _ => continue, + _ => unreachable!("every subsystem should have an associated controller"), } } diff --git a/src/cgroups/v1/mod.rs b/src/cgroups/v1/mod.rs index bd94050c2..d3eb050b7 100644 --- a/src/cgroups/v1/mod.rs +++ b/src/cgroups/v1/mod.rs @@ -2,6 +2,7 @@ mod blkio; mod controller; mod controller_type; mod cpu; +mod cpuset; mod devices; mod hugetlb; pub mod manager; From acdc65eefc5b3253674c1895f0e5321783e6acf8 Mon Sep 17 00:00:00 2001 From: Furisto <24721048+Furisto@users.noreply.github.com> Date: Thu, 3 Jun 2021 23:57:56 +0200 Subject: [PATCH 3/8] Unit tests for cgroup v1 cpu controller --- src/cgroups/v1/cpu.rs | 89 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/src/cgroups/v1/cpu.rs b/src/cgroups/v1/cpu.rs index 412142e34..20a2c53bf 100644 --- a/src/cgroups/v1/cpu.rs +++ b/src/cgroups/v1/cpu.rs @@ -64,3 +64,92 @@ impl Cpu { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::cgroups::test::{set_fixture, setup, LinuxCpuBuilder}; + use std::fs; + + #[test] + fn test_set_shares() { + // arrange + let (tmp, shares) = setup("test_set_shares", CGROUP_CPU_SHARES); + let _ = set_fixture(&tmp, CGROUP_CPU_SHARES, "") + .unwrap_or_else(|_| panic!("set test fixture for {}", CGROUP_CPU_SHARES)); + let cpu = LinuxCpuBuilder::new().with_shares(2048).build(); + + // act + Cpu::apply(&tmp, &cpu).expect("apply cpu"); + + // assert + let content = fs::read_to_string(shares) + .unwrap_or_else(|_| panic!("read {} file content", CGROUP_CPU_SHARES)); + assert_eq!(content, 2048.to_string()); + } + + #[test] + fn test_set_quota() { + // arrange + const QUOTA: i64 = 200000; + let (tmp, max) = setup("test_set_quota", CGROUP_CPU_QUOTA); + let cpu = LinuxCpuBuilder::new().with_quota(QUOTA).build(); + + // act + Cpu::apply(&tmp, &cpu).expect("apply cpu"); + + // assert + let content = fs::read_to_string(max) + .unwrap_or_else(|_| panic!("read {} file content", CGROUP_CPU_QUOTA)); + assert_eq!(content, QUOTA.to_string()); + } + + #[test] + fn test_set_period() { + // arrange + const PERIOD: u64 = 100000; + let (tmp, max) = setup("test_set_period", CGROUP_CPU_PERIOD); + let cpu = LinuxCpuBuilder::new().with_period(PERIOD).build(); + + // act + Cpu::apply(&tmp, &cpu).expect("apply cpu"); + + // assert + let content = fs::read_to_string(max) + .unwrap_or_else(|_| panic!("read {} file content", CGROUP_CPU_PERIOD)); + assert_eq!(content, PERIOD.to_string()); + } + + #[test] + fn test_set_rt_runtime() { + // arrange + const RUNTIME: i64 = 100000; + let (tmp, max) = setup("test_set_rt_runtime", CGROUP_CPU_RT_RUNTIME); + let cpu = LinuxCpuBuilder::new().with_realtime_runtime(RUNTIME).build(); + + // act + Cpu::apply(&tmp, &cpu).expect("apply cpu"); + + // assert + let content = fs::read_to_string(max) + .unwrap_or_else(|_| panic!("read {} file content", CGROUP_CPU_RT_RUNTIME)); + assert_eq!(content, RUNTIME.to_string()); + } + + #[test] + fn test_set_rt_period() { + // arrange + const PERIOD: u64 = 100000; + let (tmp, max) = setup("test_set_rt_period", CGROUP_CPU_RT_PERIOD); + let cpu = LinuxCpuBuilder::new().with_realtime_period(PERIOD).build(); + + // act + Cpu::apply(&tmp, &cpu).expect("apply cpu"); + + // assert + let content = fs::read_to_string(max) + .unwrap_or_else(|_| panic!("read {} file content", CGROUP_CPU_RT_PERIOD)); + assert_eq!(content, PERIOD.to_string()); + } + +} From 4af3fdf8316ba52998d693f7db9d87ec9bafd0b4 Mon Sep 17 00:00:00 2001 From: Furisto <24721048+Furisto@users.noreply.github.com> Date: Thu, 3 Jun 2021 23:58:41 +0200 Subject: [PATCH 4/8] Unit tests for cgroup v1 cpuset controller --- src/cgroups/v1/cpuset.rs | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/cgroups/v1/cpuset.rs b/src/cgroups/v1/cpuset.rs index 3ab2d640f..c45b05e34 100644 --- a/src/cgroups/v1/cpuset.rs +++ b/src/cgroups/v1/cpuset.rs @@ -40,3 +40,41 @@ impl CpuSet { Ok(()) } } + +#[cfg(test)] +mod tests { + use std::fs; + + use super::*; + use crate::cgroups::test::{setup, LinuxCpuBuilder}; + + #[test] + fn test_set_cpus() { + // arrange + let (tmp, cpus) = setup("test_set_cpus", CGROUP_CPUSET_CPUS); + let cpuset = LinuxCpuBuilder::new().with_cpus("1-3".to_owned()).build(); + + // act + CpuSet::apply(&tmp, &cpuset).expect("apply cpuset"); + + // assert + let content = fs::read_to_string(&cpus) + .unwrap_or_else(|_| panic!("read {} file content", CGROUP_CPUSET_CPUS)); + assert_eq!(content, "1-3"); + } + + #[test] + fn test_set_mems() { + // arrange + let (tmp, mems) = setup("test_set_mems", CGROUP_CPUSET_MEMS); + let cpuset = LinuxCpuBuilder::new().with_mems("1-3".to_owned()).build(); + + // act + CpuSet::apply(&tmp, &cpuset).expect("apply cpuset"); + + // assert + let content = fs::read_to_string(&mems) + .unwrap_or_else(|_| panic!("read {} file content", CGROUP_CPUSET_MEMS)); + assert_eq!(content, "1-3"); + } +} From 7a09809fe8ac7fddbcc57c5bf40d02b4828c4f6e Mon Sep 17 00:00:00 2001 From: Furisto <24721048+Furisto@users.noreply.github.com> Date: Fri, 4 Jun 2021 15:34:07 +0200 Subject: [PATCH 5/8] Cut down on boilerplate --- src/cgroups/common.rs | 9 ++-- src/cgroups/v1/blkio.rs | 15 ++++--- src/cgroups/v1/cpu.rs | 63 ++++++++++++++-------------- src/cgroups/v1/cpuset.rs | 6 +-- src/cgroups/v1/devices.rs | 6 +-- src/cgroups/v1/hugetlb.rs | 9 ++-- src/cgroups/v1/memory.rs | 33 +++++++++------ src/cgroups/v1/network_classifier.rs | 5 ++- src/cgroups/v1/network_priority.rs | 5 ++- src/cgroups/v1/pids.rs | 9 ++-- src/cgroups/v2/cpu.rs | 4 +- src/cgroups/v2/cpuset.rs | 4 +- src/cgroups/v2/manager.rs | 7 ++-- 13 files changed, 98 insertions(+), 77 deletions(-) diff --git a/src/cgroups/common.rs b/src/cgroups/common.rs index 41160edc2..11eb83c36 100644 --- a/src/cgroups/common.rs +++ b/src/cgroups/common.rs @@ -40,7 +40,7 @@ impl Display for Cgroup { } #[inline] -pub fn write_cgroup_file>(path: P, data: &str) -> Result<()> { +pub fn write_cgroup_file_str>(path: P, data: &str) -> Result<()> { fs::OpenOptions::new() .create(false) .write(true) @@ -52,7 +52,7 @@ pub fn write_cgroup_file>(path: P, data: &str) -> Result<()> { } #[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<()> { fs::OpenOptions::new() .create(false) .write(true) @@ -96,7 +96,10 @@ pub fn create_cgroup_manager>(cgroup_path: P) -> Result Ok(Box::new(v1::manager::Manager::new(cgroup_path.into())?)), + _ => { + log::info!("cgroup manager V1 will be used"); + Ok(Box::new(v1::manager::Manager::new(cgroup_path.into())?)) + } } } _ => bail!("could not find cgroup filesystem"), diff --git a/src/cgroups/v1/blkio.rs b/src/cgroups/v1/blkio.rs index 18a9f2291..2801eb2b7 100644 --- a/src/cgroups/v1/blkio.rs +++ b/src/cgroups/v1/blkio.rs @@ -3,7 +3,10 @@ use std::{ path::Path, }; -use crate::cgroups::{common, v1::Controller}; +use crate::cgroups::{ + common::{self, CGROUP_PROCS}, + v1::Controller, +}; use oci_spec::{LinuxBlockIo, LinuxResources}; const CGROUP_BLKIO_THROTTLE_READ_BPS: &str = "blkio.throttle.read_bps_device"; @@ -26,7 +29,7 @@ impl Controller for Blkio { Self::apply(cgroup_root, blkio)?; } - common::write_cgroup_file(&cgroup_root.join("cgroup.procs"), &pid.to_string())?; + common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } @@ -34,28 +37,28 @@ impl Controller for Blkio { impl Blkio { fn apply(root_path: &Path, blkio: &LinuxBlockIo) -> anyhow::Result<()> { for trbd in &blkio.blkio_throttle_read_bps_device { - common::write_cgroup_file( + common::write_cgroup_file_str( &root_path.join(CGROUP_BLKIO_THROTTLE_READ_BPS), &format!("{}:{} {}", trbd.major, trbd.minor, trbd.rate), )?; } for twbd in &blkio.blkio_throttle_write_bps_device { - common::write_cgroup_file( + common::write_cgroup_file_str( &root_path.join(CGROUP_BLKIO_THROTTLE_WRITE_BPS), &format!("{}:{} {}", twbd.major, twbd.minor, twbd.rate), )?; } for trid in &blkio.blkio_throttle_read_iops_device { - common::write_cgroup_file( + common::write_cgroup_file_str( &root_path.join(CGROUP_BLKIO_THROTTLE_READ_IOPS), &format!("{}:{} {}", trid.major, trid.minor, trid.rate), )?; } for twid in &blkio.blkio_throttle_write_iops_device { - common::write_cgroup_file( + common::write_cgroup_file_str( &root_path.join(CGROUP_BLKIO_THROTTLE_WRITE_IOPS), &format!("{}:{} {}", twid.major, twid.minor, twid.rate), )?; diff --git a/src/cgroups/v1/cpu.rs b/src/cgroups/v1/cpu.rs index 20a2c53bf..50a7c7eb5 100644 --- a/src/cgroups/v1/cpu.rs +++ b/src/cgroups/v1/cpu.rs @@ -24,7 +24,7 @@ impl Controller for Cpu { Self::apply(cgroup_root, cpu)?; } - common::write_cgroup_file_(cgroup_root.join(CGROUP_PROCS), pid)?; + common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } @@ -33,31 +33,31 @@ impl Cpu { fn apply(root_path: &Path, cpu: &LinuxCpu) -> Result<()> { if let Some(cpu_shares) = cpu.shares { if cpu_shares != 0 { - common::write_cgroup_file_(root_path.join(CGROUP_CPU_SHARES), cpu_shares)?; + common::write_cgroup_file(root_path.join(CGROUP_CPU_SHARES), cpu_shares)?; } } if let Some(cpu_period) = cpu.period { if cpu_period != 0 { - common::write_cgroup_file_(root_path.join(CGROUP_CPU_PERIOD), cpu_period)?; + common::write_cgroup_file(root_path.join(CGROUP_CPU_PERIOD), cpu_period)?; } } if let Some(cpu_quota) = cpu.quota { if cpu_quota != 0 { - common::write_cgroup_file_(root_path.join(CGROUP_CPU_QUOTA), cpu_quota)?; + common::write_cgroup_file(root_path.join(CGROUP_CPU_QUOTA), cpu_quota)?; } } if let Some(rt_runtime) = cpu.realtime_runtime { if rt_runtime != 0 { - common::write_cgroup_file_(root_path.join(CGROUP_CPU_RT_RUNTIME), rt_runtime)?; + common::write_cgroup_file(root_path.join(CGROUP_CPU_RT_RUNTIME), rt_runtime)?; } } if let Some(rt_period) = cpu.realtime_period { if rt_period != 0 { - common::write_cgroup_file_(root_path.join(CGROUP_CPU_RT_PERIOD), rt_period)?; + common::write_cgroup_file(root_path.join(CGROUP_CPU_RT_PERIOD), rt_period)?; } } @@ -122,34 +122,35 @@ mod tests { #[test] fn test_set_rt_runtime() { - // arrange - const RUNTIME: i64 = 100000; - let (tmp, max) = setup("test_set_rt_runtime", CGROUP_CPU_RT_RUNTIME); - let cpu = LinuxCpuBuilder::new().with_realtime_runtime(RUNTIME).build(); - - // act - Cpu::apply(&tmp, &cpu).expect("apply cpu"); - - // assert - let content = fs::read_to_string(max) - .unwrap_or_else(|_| panic!("read {} file content", CGROUP_CPU_RT_RUNTIME)); - assert_eq!(content, RUNTIME.to_string()); + // arrange + const RUNTIME: i64 = 100000; + let (tmp, max) = setup("test_set_rt_runtime", CGROUP_CPU_RT_RUNTIME); + let cpu = LinuxCpuBuilder::new() + .with_realtime_runtime(RUNTIME) + .build(); + + // act + Cpu::apply(&tmp, &cpu).expect("apply cpu"); + + // assert + let content = fs::read_to_string(max) + .unwrap_or_else(|_| panic!("read {} file content", CGROUP_CPU_RT_RUNTIME)); + assert_eq!(content, RUNTIME.to_string()); } #[test] fn test_set_rt_period() { - // arrange - const PERIOD: u64 = 100000; - let (tmp, max) = setup("test_set_rt_period", CGROUP_CPU_RT_PERIOD); - let cpu = LinuxCpuBuilder::new().with_realtime_period(PERIOD).build(); - - // act - Cpu::apply(&tmp, &cpu).expect("apply cpu"); - - // assert - let content = fs::read_to_string(max) - .unwrap_or_else(|_| panic!("read {} file content", CGROUP_CPU_RT_PERIOD)); - assert_eq!(content, PERIOD.to_string()); - } + // arrange + const PERIOD: u64 = 100000; + let (tmp, max) = setup("test_set_rt_period", CGROUP_CPU_RT_PERIOD); + let cpu = LinuxCpuBuilder::new().with_realtime_period(PERIOD).build(); + + // act + Cpu::apply(&tmp, &cpu).expect("apply cpu"); + // assert + let content = fs::read_to_string(max) + .unwrap_or_else(|_| panic!("read {} file content", CGROUP_CPU_RT_PERIOD)); + assert_eq!(content, PERIOD.to_string()); + } } diff --git a/src/cgroups/v1/cpuset.rs b/src/cgroups/v1/cpuset.rs index c45b05e34..eb265ffe1 100644 --- a/src/cgroups/v1/cpuset.rs +++ b/src/cgroups/v1/cpuset.rs @@ -22,7 +22,7 @@ impl Controller for CpuSet { Self::apply(cgroup_root, cpuset)?; } - common::write_cgroup_file_(cgroup_root.join(CGROUP_PROCS), pid)?; + common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } @@ -30,11 +30,11 @@ impl Controller for CpuSet { impl CpuSet { fn apply(root_path: &Path, cpuset: &LinuxCpu) -> Result<()> { if let Some(cpus) = &cpuset.cpus { - common::write_cgroup_file(root_path.join(CGROUP_CPUSET_CPUS), cpus)?; + common::write_cgroup_file_str(root_path.join(CGROUP_CPUSET_CPUS), cpus)?; } if let Some(mems) = &cpuset.mems { - common::write_cgroup_file(root_path.join(CGROUP_CPUSET_MEMS), mems)?; + common::write_cgroup_file_str(root_path.join(CGROUP_CPUSET_MEMS), mems)?; } Ok(()) diff --git a/src/cgroups/v1/devices.rs b/src/cgroups/v1/devices.rs index 321941e80..fe1635e74 100644 --- a/src/cgroups/v1/devices.rs +++ b/src/cgroups/v1/devices.rs @@ -3,7 +3,7 @@ use std::{fs::create_dir_all, path::Path}; use anyhow::Result; use nix::unistd::Pid; -use crate::cgroups::common; +use crate::cgroups::common::{self, CGROUP_PROCS}; use crate::{cgroups::v1::Controller, rootfs::default_devices}; use oci_spec::{LinuxDeviceCgroup, LinuxDeviceType, LinuxResources}; @@ -27,7 +27,7 @@ impl Controller for Devices { Self::apply_device(&d, &cgroup_root)?; } - common::write_cgroup_file(cgroup_root.join("cgroup.procs"), &pid.to_string())?; + common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } @@ -40,7 +40,7 @@ impl Devices { cgroup_root.join("devices.deny") }; - common::write_cgroup_file(path, &device.to_string())?; + common::write_cgroup_file_str(path, &device.to_string())?; Ok(()) } diff --git a/src/cgroups/v1/hugetlb.rs b/src/cgroups/v1/hugetlb.rs index e17f940e3..fe88958d0 100644 --- a/src/cgroups/v1/hugetlb.rs +++ b/src/cgroups/v1/hugetlb.rs @@ -3,7 +3,10 @@ use std::{fs, path::Path}; use anyhow::anyhow; use regex::Regex; -use crate::cgroups::{common, v1::Controller}; +use crate::cgroups::{ + common::{self, CGROUP_PROCS}, + v1::Controller, +}; use oci_spec::{LinuxHugepageLimit, LinuxResources}; pub struct Hugetlb {} @@ -21,7 +24,7 @@ impl Controller for Hugetlb { Self::apply(cgroup_root, hugetlb)? } - common::write_cgroup_file(cgroup_root.join("cgroup.procs"), &pid.to_string())?; + common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } @@ -42,7 +45,7 @@ impl Hugetlb { common::write_cgroup_file( &root_path.join(format!("hugetlb.{}.limit_in_bytes", hugetlb.page_size)), - &hugetlb.limit.to_string(), + &hugetlb.limit, )?; Ok(()) } diff --git a/src/cgroups/v1/memory.rs b/src/cgroups/v1/memory.rs index 20151c685..9a29d4747 100644 --- a/src/cgroups/v1/memory.rs +++ b/src/cgroups/v1/memory.rs @@ -7,7 +7,7 @@ use std::{ use anyhow::{Result, *}; use nix::{errno::Errno, unistd::Pid}; -use crate::cgroups::common; +use crate::cgroups::common::{self, CGROUP_PROCS}; use crate::cgroups::v1::Controller; use oci_spec::{LinuxMemory, LinuxResources}; @@ -35,18 +35,24 @@ impl Controller for Memory { Self::apply(&memory, cgroup_root)?; if reservation != 0 { - Self::set(reservation, &cgroup_root.join(CGROUP_MEMORY_RESERVATION))?; + common::write_cgroup_file( + cgroup_root.join(CGROUP_MEMORY_RESERVATION), + reservation, + )?; } if linux_resources.disable_oom_killer { - Self::set(0, &cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL))?; + common::write_cgroup_file(cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL), 0)?; } else { - Self::set(1, &cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL))?; + common::write_cgroup_file(cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL), 1)?; } if let Some(swappiness) = memory.swappiness { if swappiness <= 100 { - Self::set(swappiness, &cgroup_root.join(CGROUP_MEMORY_SWAPPINESS))?; + common::write_cgroup_file( + cgroup_root.join(CGROUP_MEMORY_SWAPPINESS), + swappiness, + )?; } else { // invalid swappiness value return Err(anyhow!( @@ -60,14 +66,17 @@ impl Controller for Memory { // neither are implemented by runc. Tests pass without this, but // kept in per the spec. if let Some(kmem) = memory.kernel { - Self::set(kmem, &cgroup_root.join(CGROUP_KERNEL_MEMORY_LIMIT))?; + common::write_cgroup_file(cgroup_root.join(CGROUP_KERNEL_MEMORY_LIMIT), kmem)?; } if let Some(tcp_mem) = memory.kernel_tcp { - Self::set(tcp_mem, &cgroup_root.join(CGROUP_KERNEL_TCP_MEMORY_LIMIT))?; + common::write_cgroup_file( + cgroup_root.join(CGROUP_KERNEL_TCP_MEMORY_LIMIT), + tcp_mem, + )?; } } - common::write_cgroup_file(cgroup_root.join("cgroup.procs"), &pid.to_string())?; + common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } @@ -170,14 +179,12 @@ impl Memory { } } - fn set_swap(val: i64, cgroup_root: &Path) -> Result<()> { - if val == 0 { + fn set_swap(swap: i64, cgroup_root: &Path) -> Result<()> { + if swap == 0 { return Ok(()); } - let path = cgroup_root.join(CGROUP_MEMORY_SWAP_LIMIT); - Self::set(val, &path)?; - + common::write_cgroup_file(cgroup_root.join(CGROUP_MEMORY_SWAP_LIMIT), swap)?; Ok(()) } diff --git a/src/cgroups/v1/network_classifier.rs b/src/cgroups/v1/network_classifier.rs index b953ef68f..0ecd9c873 100644 --- a/src/cgroups/v1/network_classifier.rs +++ b/src/cgroups/v1/network_classifier.rs @@ -4,6 +4,7 @@ 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}; @@ -18,7 +19,7 @@ impl Controller for NetworkClassifier { Self::apply(cgroup_root, network)?; } - common::write_cgroup_file(cgroup_root.join("cgroup.procs"), &pid.to_string())?; + common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } @@ -26,7 +27,7 @@ impl Controller for NetworkClassifier { impl NetworkClassifier { fn apply(root_path: &Path, network: &LinuxNetwork) -> Result<()> { if let Some(class_id) = network.class_id { - common::write_cgroup_file(&root_path.join("net_cls.classid"), &class_id.to_string())?; + common::write_cgroup_file(root_path.join("net_cls.classid"), class_id)?; } Ok(()) diff --git a/src/cgroups/v1/network_priority.rs b/src/cgroups/v1/network_priority.rs index fcc7b800c..d12c66fe5 100644 --- a/src/cgroups/v1/network_priority.rs +++ b/src/cgroups/v1/network_priority.rs @@ -4,6 +4,7 @@ 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}; @@ -18,7 +19,7 @@ impl Controller for NetworkPriority { Self::apply(cgroup_root, network)?; } - common::write_cgroup_file(cgroup_root.join("cgroup.procs"), &pid.to_string())?; + common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } @@ -26,7 +27,7 @@ impl Controller for NetworkPriority { impl NetworkPriority { fn apply(root_path: &Path, network: &LinuxNetwork) -> Result<()> { let priorities: String = network.priorities.iter().map(|p| p.to_string()).collect(); - common::write_cgroup_file(&root_path.join("net_prio.ifpriomap"), &priorities.trim())?; + common::write_cgroup_file_str(root_path.join("net_prio.ifpriomap"), &priorities.trim())?; Ok(()) } diff --git a/src/cgroups/v1/pids.rs b/src/cgroups/v1/pids.rs index 1b18c6758..bb2f3af45 100644 --- a/src/cgroups/v1/pids.rs +++ b/src/cgroups/v1/pids.rs @@ -5,7 +5,10 @@ use std::{ use anyhow::Result; -use crate::cgroups::{common, v1::Controller}; +use crate::cgroups::{ + common::{self, CGROUP_PROCS}, + v1::Controller, +}; use oci_spec::{LinuxPids, LinuxResources}; pub struct Pids {} @@ -23,7 +26,7 @@ impl Controller for Pids { Self::apply(cgroup_root, pids)?; } - common::write_cgroup_file(cgroup_root.join("cgroup.procs"), &pid.to_string())?; + common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; Ok(()) } } @@ -36,7 +39,7 @@ impl Pids { "max".to_string() }; - common::write_cgroup_file(&root_path.join("pids.max"), &limit)?; + common::write_cgroup_file_str(&root_path.join("pids.max"), &limit)?; Ok(()) } } diff --git a/src/cgroups/v2/cpu.rs b/src/cgroups/v2/cpu.rs index 2976acec3..b81fba855 100644 --- a/src/cgroups/v2/cpu.rs +++ b/src/cgroups/v2/cpu.rs @@ -33,7 +33,7 @@ impl Cpu { shares = Self::convert_shares_to_cgroup2(shares); if shares != 0 { // will result in Erno 34 (numerical result out of range) otherwise - common::write_cgroup_file(&path.join(CGROUP_CPU_WEIGHT), &shares.to_string())?; + common::write_cgroup_file(path.join(CGROUP_CPU_WEIGHT), shares)?; } } @@ -57,7 +57,7 @@ impl Cpu { // 250000 250000 -> 1 CPU worth of runtime every 250ms // 10000 50000 -> 20% of one CPU every 50ms let max = quota_string + " " + &period_string; - common::write_cgroup_file(&path.join(CGROUP_CPU_MAX), &max)?; + common::write_cgroup_file_str(path.join(CGROUP_CPU_MAX), &max)?; Ok(()) } diff --git a/src/cgroups/v2/cpuset.rs b/src/cgroups/v2/cpuset.rs index 47539097c..b477080b2 100644 --- a/src/cgroups/v2/cpuset.rs +++ b/src/cgroups/v2/cpuset.rs @@ -24,11 +24,11 @@ impl Controller for CpuSet { impl CpuSet { fn apply(path: &Path, cpuset: &LinuxCpu) -> Result<()> { if let Some(cpus) = &cpuset.cpus { - common::write_cgroup_file(&path.join(CGROUP_CPUSET_CPUS), cpus)?; + common::write_cgroup_file_str(path.join(CGROUP_CPUSET_CPUS), cpus)?; } if let Some(mems) = &cpuset.mems { - common::write_cgroup_file(&path.join(CGROUP_CPUSET_MEMS), mems)?; + common::write_cgroup_file_str(path.join(CGROUP_CPUSET_MEMS), mems)?; } Ok(()) diff --git a/src/cgroups/v2/manager.rs b/src/cgroups/v2/manager.rs index 8c7c473e5..1413d36dd 100644 --- a/src/cgroups/v2/manager.rs +++ b/src/cgroups/v2/manager.rs @@ -13,13 +13,12 @@ use super::{cpu::Cpu, cpuset::CpuSet, hugetlb::HugeTlb, io::Io, memory::Memory, use crate::{ cgroups::v2::controller::Controller, cgroups::{ - common::{self, CgroupManager}, + common::{self, CgroupManager, CGROUP_PROCS}, v2::controller_type::ControllerType, }, utils::PathBufExt, }; -const CGROUP_PROCS: &str = "cgroup.procs"; const CGROUP_CONTROLLERS: &str = "cgroup.controllers"; const CGROUP_SUBTREE_CONTROL: &str = "cgroup.subtree_control"; @@ -66,7 +65,7 @@ impl Manager { // if this were set, writing to the cgroups.procs file will fail with Erno 16 (device or resource busy) if components.peek().is_some() { for controller in &controllers { - common::write_cgroup_file( + common::write_cgroup_file_str( ¤t_path.join(CGROUP_SUBTREE_CONTROL), controller, )?; @@ -74,7 +73,7 @@ impl Manager { } } - common::write_cgroup_file(&full_path.join(CGROUP_PROCS), &pid.to_string())?; + common::write_cgroup_file(&full_path.join(CGROUP_PROCS), pid)?; Ok(full_path) } From 9b981700e548608e9223d91679eb098bba271535 Mon Sep 17 00:00:00 2001 From: Furisto <24721048+Furisto@users.noreply.github.com> Date: Fri, 4 Jun 2021 16:58:46 +0200 Subject: [PATCH 6/8] Activate cpu integration test --- integration_test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration_test.sh b/integration_test.sh index 3a4f82c3e..7fc9a1628 100755 --- a/integration_test.sh +++ b/integration_test.sh @@ -3,7 +3,7 @@ root=$(pwd) cd integration_test/src/github.com/opencontainers/runtime-tools GOPATH=$root/integration_test make runtimetest validation-executables -test_cases=("default/default.t" "linux_cgroups_devices/linux_cgroups_devices.t" "linux_cgroups_hugetlb/linux_cgroups_hugetlb.t" "linux_cgroups_pids/linux_cgroups_pids.t" "linux_cgroups_memory/linux_cgroups_memory.t" "linux_cgroups_network/linux_cgroups_network.t") +test_cases=("default/default.t" "linux_cgroups_devices/linux_cgroups_devices.t" "linux_cgroups_hugetlb/linux_cgroups_hugetlb.t" "linux_cgroups_pids/linux_cgroups_pids.t" "linux_cgroups_memory/linux_cgroups_memory.t" "linux_cgroups_network/linux_cgroups_network.t" "linux_cgroups_cpus/linux_cgroups_cpus.t") for case in "${test_cases[@]}"; do echo "Running $case" if [ 0 -ne $(sudo RUST_BACKTRACE=1 YOUKI_LOG_LEVEL=debug RUNTIME=$root/target/x86_64-unknown-linux-gnu/debug/youki $root/integration_test/src/github.com/opencontainers/runtime-tools/validation/$case | grep "not ok" | wc -l) ]; then From 0809ca1e1008d693d225a140b33376d6e53689ef Mon Sep 17 00:00:00 2001 From: Furisto <24721048+Furisto@users.noreply.github.com> Date: Fri, 4 Jun 2021 19:52:25 +0200 Subject: [PATCH 7/8] Fix error while moving task into cgroup 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. --- src/cgroups/common.rs | 11 ++++++++- src/cgroups/v1/cpuset.rs | 48 +++++++++++++++++++++++++++++++--------- 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/cgroups/common.rs b/src/cgroups/common.rs index 11eb83c36..a05ae04fa 100644 --- a/src/cgroups/common.rs +++ b/src/cgroups/common.rs @@ -6,7 +6,7 @@ use std::{ path::{Path, PathBuf}, }; -use anyhow::{bail, Result}; +use anyhow::{anyhow, bail, Result}; use nix::unistd::Pid; use oci_spec::LinuxResources; use procfs::process::Process; @@ -63,6 +63,15 @@ pub fn write_cgroup_file, T: ToString>(path: P, data: T) -> Resul Ok(()) } +pub fn get_cgroupv1_mount_path(subsystem: &str) -> Result { + Process::myself()? + .mountinfo()? + .into_iter() + .find(|m| m.fs_type == "cgroup" && m.mount_point.ends_with(subsystem)) + .map(|m| m.mount_point) + .ok_or_else(|| anyhow!("could not find mountpoint for {}", subsystem)) +} + pub fn create_cgroup_manager>(cgroup_path: P) -> Result> { let cgroup_mount = Process::myself()? .mountinfo()? diff --git a/src/cgroups/v1/cpuset.rs b/src/cgroups/v1/cpuset.rs index eb265ffe1..2d7342dd9 100644 --- a/src/cgroups/v1/cpuset.rs +++ b/src/cgroups/v1/cpuset.rs @@ -1,12 +1,12 @@ use std::{fs, path::Path}; -use anyhow::Result; +use anyhow::{bail, Result}; use nix::unistd::Pid; use oci_spec::{LinuxCpu, LinuxResources}; use crate::cgroups::common::{self, CGROUP_PROCS}; -use super::Controller; +use super::{Controller, ControllerType}; const CGROUP_CPUSET_CPUS: &str = "cpuset.cpus"; const CGROUP_CPUSET_MEMS: &str = "cpuset.mems"; @@ -14,27 +14,55 @@ const CGROUP_CPUSET_MEMS: &str = "cpuset.mems"; pub struct CpuSet {} impl Controller for CpuSet { - fn apply(linux_resources: &LinuxResources, cgroup_root: &Path, pid: Pid) -> Result<()> { + fn apply(linux_resources: &LinuxResources, cgroup_path: &Path, pid: Pid) -> Result<()> { log::debug!("Apply CpuSet cgroup config"); - fs::create_dir_all(cgroup_root)?; + fs::create_dir_all(cgroup_path)?; if let Some(cpuset) = &linux_resources.cpu { - Self::apply(cgroup_root, cpuset)?; + Self::apply(cgroup_path, cpuset)?; } - common::write_cgroup_file(cgroup_root.join(CGROUP_PROCS), pid)?; + 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(()) } } impl CpuSet { - fn apply(root_path: &Path, cpuset: &LinuxCpu) -> Result<()> { + fn apply(cgroup_path: &Path, cpuset: &LinuxCpu) -> Result<()> { if let Some(cpus) = &cpuset.cpus { - common::write_cgroup_file_str(root_path.join(CGROUP_CPUSET_CPUS), cpus)?; - } + common::write_cgroup_file_str(cgroup_path.join(CGROUP_CPUSET_CPUS), cpus)?; + } if let Some(mems) = &cpuset.mems { - common::write_cgroup_file_str(root_path.join(CGROUP_CPUSET_MEMS), mems)?; + common::write_cgroup_file_str(cgroup_path.join(CGROUP_CPUSET_MEMS), mems)?; + } + + Ok(()) + } + + // 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<()> { + let mut current = common::get_cgroupv1_mount_path(&ControllerType::CpuSet.to_string())?; + let relative_cgroup_path = cgroup_path.strip_prefix(¤t)?; + + for component in relative_cgroup_path.components() { + let parent_value = fs::read_to_string(current.join(interface_file))?; + if parent_value.trim().is_empty() { + bail!("cpuset parent value is empty") + } + + current.push(component); + let child_path = current.join(interface_file); + let child_value = fs::read_to_string(&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() { + common::write_cgroup_file_str(&child_path, &parent_value)?; + } } Ok(()) From b2d90d32de301fc4ae5b90a7daa4bc896e7d8cef Mon Sep 17 00:00:00 2001 From: Furisto <24721048+Furisto@users.noreply.github.com> Date: Sat, 5 Jun 2021 00:20:02 +0200 Subject: [PATCH 8/8] Improve reliability of cgroup removal --- src/cgroups/v1/manager.rs | 15 +++++++++++++-- src/utils.rs | 20 ++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/cgroups/v1/manager.rs b/src/cgroups/v1/manager.rs index fd8c922e3..86dd8268e 100644 --- a/src/cgroups/v1/manager.rs +++ b/src/cgroups/v1/manager.rs @@ -1,5 +1,6 @@ +use std::fs; +use std::path::Path; use std::{collections::HashMap, path::PathBuf}; -use std::{fs::remove_dir, path::Path}; use anyhow::Result; use nix::unistd::Pid; @@ -12,6 +13,8 @@ use super::{ Controller, ControllerType, }; +use crate::cgroups::common::CGROUP_PROCS; +use crate::utils; use crate::{cgroups::common::CgroupManager, utils::PathBufExt}; use oci_spec::LinuxResources; @@ -109,7 +112,15 @@ impl CgroupManager for Manager { for cgroup_path in &self.subsystems { if cgroup_path.1.exists() { log::debug!("remove cgroup {:?}", cgroup_path.1); - remove_dir(&cgroup_path.1)?; + let procs_path = cgroup_path.1.join(CGROUP_PROCS); + let procs = fs::read_to_string(&procs_path)?; + + for line in procs.lines() { + let pid: i32 = line.parse()?; + let _ = nix::sys::signal::kill(Pid::from_raw(pid), nix::sys::signal::SIGKILL); + } + + utils::delete_with_retry(cgroup_path.1)?; } } diff --git a/src/utils.rs b/src/utils.rs index 46590483d..06113e907 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,5 +1,7 @@ use std::ffi::CString; +use std::fs; use std::path::{Path, PathBuf}; +use std::time::Duration; use anyhow::{bail, Result}; use nix::{env::clearenv, errno::Errno, unistd}; @@ -72,6 +74,24 @@ pub fn get_cgroup_path(cgroups_path: &Option, container_id: &str) -> Pa } } +pub fn delete_with_retry>(path: P) -> Result<()> { + let mut attempts = 0; + let mut delay = Duration::from_millis(10); + let path = path.as_ref(); + + while attempts < 5 { + if fs::remove_dir(path).is_ok() { + return Ok(()); + } + + std::thread::sleep(delay); + attempts += attempts; + delay *= attempts; + } + + bail!("could not delete {:?}", path) +} + #[cfg(test)] mod tests { use super::*;