From 25a6504435aba4b38f4fdbc6d61cd2da81654837 Mon Sep 17 00:00:00 2001 From: Travis Sturzl Date: Fri, 21 May 2021 17:54:52 -0600 Subject: [PATCH 01/13] add memory cgroup controller --- .gitignore | 4 + Cargo.toml | 2 +- src/cgroups/controller_type.rs | 2 + src/cgroups/memory.rs | 306 +++++++++++++++++++++++++++++++++ src/cgroups/mod.rs | 1 + 5 files changed, 314 insertions(+), 1 deletion(-) create mode 100644 src/cgroups/memory.rs diff --git a/.gitignore b/.gitignore index 58b204af4..9039ea945 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,6 @@ /target .vagrant/ + +tags +tags.lock +tags.temp diff --git a/Cargo.toml b/Cargo.toml index 6a9b7e598..eb8f2e502 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,4 +19,4 @@ mio = { version = "0.7", features = ["os-ext", "os-poll"] } chrono = "0.4" once_cell = "1.6.0" futures = { version = "0.3", features = ["thread-pool"] } -regex = "1.5" \ No newline at end of file +regex = "1.5" diff --git a/src/cgroups/controller_type.rs b/src/cgroups/controller_type.rs index 744356b9b..d16f1a9ea 100644 --- a/src/cgroups/controller_type.rs +++ b/src/cgroups/controller_type.rs @@ -4,6 +4,7 @@ pub enum ControllerType { Devices, HugeTlb, Pids, + Memory, } impl ToString for ControllerType { @@ -12,6 +13,7 @@ impl ToString for ControllerType { Self::Devices => "devices".into(), Self::HugeTlb => "hugetlb".into(), Self::Pids => "pids".into(), + Self::Memory => "memory".into(), } } } diff --git a/src/cgroups/memory.rs b/src/cgroups/memory.rs new file mode 100644 index 000000000..8e766eca5 --- /dev/null +++ b/src/cgroups/memory.rs @@ -0,0 +1,306 @@ +use std::io::{prelude::*, Write}; +use std::{ + fs::{create_dir_all, OpenOptions}, + path::Path, +}; + +use anyhow::{Result, *}; +use nix::{errno::Errno, unistd::Pid}; + +use crate::{ + cgroups::Controller, + spec::{LinuxMemory, LinuxResources}, +}; + +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"; +const CGROUP_MEMORY_MAX_USAGE: &str = "memory.max_usage_in_bytes"; +const CGROUP_MEMORY_SWAPPINESS: &str = "memory.swappiness"; +const CGROUP_MEMORY_RESERVATION: &str = "memory.soft_limit_in_bytes"; + +const CGROUP_KERNEL_MEMORY_LIMIT: &str = "memory.kmem.limit_in_bytes"; +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<()> { + create_dir_all(&cgroup_root)?; + let memory = linux_resources.memory.as_ref().unwrap(); + let reservation = memory.reservation.unwrap_or(0); + + Self::set_memory_and_swap(&memory, cgroup_root)?; + + if reservation != 0 { + Self::set_i64(reservation, &cgroup_root.join(CGROUP_MEMORY_RESERVATION))?; + } + + if let Some(swappiness) = memory.swappiness { + if swappiness <= 100 { + Self::set_u64(swappiness, &cgroup_root.join(CGROUP_MEMORY_SWAPPINESS))?; + } else { + // invalid swappiness value + return Err(anyhow!( + "Invalid swappiness value: {}. Valid range is 0-100", + swappiness + )); + } + } + + // NOTE: Seems as though kernel and kernelTCP are both deprecated + // neither are implemented by runc. Tests pass without this, but + // kept in per the spec. + if let Some(kmem) = memory.kernel { + Self::set_i64(kmem, &cgroup_root.join(CGROUP_KERNEL_MEMORY_LIMIT))?; + } + if let Some(tcp_mem) = memory.kernel_tcp { + Self::set_i64(tcp_mem, &cgroup_root.join(CGROUP_KERNEL_TCP_MEMORY_LIMIT))?; + } + + OpenOptions::new() + .create(false) + .write(true) + .truncate(false) + .open(cgroup_root.join("cgroup.procs"))? + .write_all(pid.to_string().as_bytes())?; + + Ok(()) + } +} + +impl Memory { + 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)?; + + let val = contents.parse::()?; + Ok(val) + } + + 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)?; + + let val = contents.parse::()?; + Ok(val) + } + + 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)?; + + let val = contents.parse::()?; + Ok(val) + } + + fn set_i64(val: i64, path: &Path) -> std::io::Result<()> { + OpenOptions::new() + .create(false) + .write(true) + .truncate(true) + .open(path)? + .write_all(val.to_string().as_bytes())?; + Ok(()) + } + + fn set_u64(val: u64, path: &Path) -> std::io::Result<()> { + OpenOptions::new() + .create(false) + .write(true) + .truncate(true) + .open(path)? + .write_all(val.to_string().as_bytes())?; + Ok(()) + } + + fn set_memory(val: i64, cgroup_root: &Path) -> Result<()> { + let path = cgroup_root.join(CGROUP_MEMORY_LIMIT); + + match Self::set_i64(val, &path) { + Ok(_) => Ok(()), + Err(e) => { + // we need to look into the raw OS error for an EBUSY status + if let Some(code) = e.raw_os_error() { + // the nix crate has a handy enum for these, lets use that + let errno = Errno::from_i32(code); + // if the error is EBUSY + if let Errno::EBUSY = errno { + let usage = Self::get_memory_usage(cgroup_root)?; + let max_usage = Self::get_memory_max_usage(cgroup_root)?; + Err(anyhow!( + "unable to set memory limit to {} (current usage: {}, peak usage: {})", + val, + usage, + max_usage, + )) + } else { + Err(anyhow!(e)) + } + } else { + Err(anyhow!(e)) + } + } + } + } + + fn set_swap(val: i64, cgroup_root: &Path) -> Result<()> { + if val == 0 { + return Ok(()); + } + + let path = cgroup_root.join(CGROUP_MEMORY_SWAP_LIMIT); + + Self::set_i64(val, &path)?; + + Ok(()) + } + + fn set_memory_and_swap(resource: &LinuxMemory, cgroup_root: &Path) -> Result<()> { + let limit = resource.limit.unwrap_or(0); + let mut swap = resource.swap.unwrap_or(0); + + if limit == -1 && swap == 0 { + if cgroup_root.join(CGROUP_MEMORY_SWAP_LIMIT).exists() { + swap = -1; + } + } + + // 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 in the kernel's validation + // see: + // https://github.com/opencontainers/runc/blob/master/libcontainer/cgroups/fs/memory.go#L89 + if limit != 0 && swap != 0 { + let current_limit = Self::get_memory_limit(cgroup_root)?; + + if swap == -1 || current_limit < swap { + Self::set_swap(swap, cgroup_root)?; + Self::set_memory(limit, cgroup_root)?; + return Ok(()); + } + } + + Self::set_memory(limit, cgroup_root)?; + Self::set_swap(swap, cgroup_root)?; + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::spec::LinuxMemory; + + fn set_fixture(temp_dir: &std::path::Path, filename: &str, val: &str) -> Result<()> { + std::fs::OpenOptions::new() + .create(true) + .write(true) + .truncate(true) + .open(temp_dir.join(filename))? + .write_all(val.as_bytes())?; + + Ok(()) + } + + fn create_temp_dir(test_name: &str) -> Result { + std::fs::create_dir_all(std::env::temp_dir().join(test_name))?; + Ok(std::env::temp_dir().join(test_name)) + } + + #[test] + fn test_set_memory() { + let limit = 1024; + let tmp = create_temp_dir("test_set_memory").expect("create temp directory for test"); + set_fixture(&tmp, CGROUP_MEMORY_USAGE, "0").expect("Set fixure for memory usage"); + set_fixture(&tmp, CGROUP_MEMORY_MAX_USAGE, "0").expect("Set fixure for max memory usage"); + set_fixture(&tmp, CGROUP_MEMORY_LIMIT, "0").expect("Set fixure for memory limit"); + Memory::set_memory(limit, &tmp).expect("Set memory limit"); + let content = + std::fs::read_to_string(tmp.join(CGROUP_MEMORY_LIMIT)).expect("Read to string"); + assert_eq!(limit.to_string(), content) + } + + #[test] + fn test_set_swap() { + let limit = 512; + let tmp = create_temp_dir("test_set_swap").expect("create temp directory for test"); + set_fixture(&tmp, CGROUP_MEMORY_SWAP_LIMIT, "0").expect("Set fixure for swap limit"); + Memory::set_swap(limit, &tmp).expect("Set swap limit"); + let content = + std::fs::read_to_string(tmp.join(CGROUP_MEMORY_SWAP_LIMIT)).expect("Read to string"); + assert_eq!(limit.to_string(), content) + } + + #[test] + fn test_set_memory_and_swap() { + let tmp = + create_temp_dir("test_set_memory_and_swap").expect("create temp directory for test"); + set_fixture(&tmp, CGROUP_MEMORY_USAGE, "0").expect("Set fixure for memory usage"); + set_fixture(&tmp, CGROUP_MEMORY_MAX_USAGE, "0").expect("Set fixure for max memory usage"); + set_fixture(&tmp, CGROUP_MEMORY_LIMIT, "0").expect("Set fixure for memory limit"); + set_fixture(&tmp, CGROUP_MEMORY_SWAP_LIMIT, "0").expect("Set fixure for swap limit"); + + // test unlimited memory with no set swap + { + let limit = -1; + let linux_memory = &LinuxMemory { + limit: Some(limit), + swap: None, // Some(0) gives the same outcome + reservation: None, + kernel: None, + kernel_tcp: None, + swappiness: None, + }; + Memory::set_memory_and_swap(linux_memory, &tmp).expect("Set memory and swap"); + + let limit_content = + std::fs::read_to_string(tmp.join(CGROUP_MEMORY_LIMIT)).expect("Read to string"); + assert_eq!(limit.to_string(), limit_content); + + let swap_content = std::fs::read_to_string(tmp.join(CGROUP_MEMORY_SWAP_LIMIT)) + .expect("Read to string"); + // swap should be set to -1 also + assert_eq!(limit.to_string(), swap_content); + } + + // test setting swap and memory to arbitrary values + { + let limit = 1024 * 1024 * 1024; + let swap = 1024; + let linux_memory = &LinuxMemory { + limit: Some(limit), + swap: Some(swap), + reservation: None, + kernel: None, + kernel_tcp: None, + swappiness: None, + }; + Memory::set_memory_and_swap(linux_memory, &tmp).expect("Set memory and swap"); + + let limit_content = + std::fs::read_to_string(tmp.join(CGROUP_MEMORY_LIMIT)).expect("Read to string"); + assert_eq!(limit.to_string(), limit_content); + + let swap_content = std::fs::read_to_string(tmp.join(CGROUP_MEMORY_SWAP_LIMIT)) + .expect("Read to string"); + assert_eq!(swap.to_string(), swap_content); + } + } +} diff --git a/src/cgroups/mod.rs b/src/cgroups/mod.rs index f6c2acdbd..ff6635365 100644 --- a/src/cgroups/mod.rs +++ b/src/cgroups/mod.rs @@ -3,6 +3,7 @@ mod controller_type; mod devices; mod hugetlb; mod manager; +mod memory; mod pids; pub use controller::Controller; pub use controller_type::ControllerType; From 6f2225c2a659d8ca24d4103b0d87dc8548d18df3 Mon Sep 17 00:00:00 2001 From: Travis Sturzl Date: Fri, 21 May 2021 20:59:16 -0600 Subject: [PATCH 02/13] hmmm --- src/cgroups/memory.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/cgroups/memory.rs b/src/cgroups/memory.rs index 8e766eca5..5f11540ea 100644 --- a/src/cgroups/memory.rs +++ b/src/cgroups/memory.rs @@ -26,6 +26,10 @@ pub struct Memory {} impl Controller for Memory { fn apply(linux_resources: &LinuxResources, cgroup_root: &Path, pid: Pid) -> Result<()> { + log::info!( + "Memory controller path: {}", + cgroup_root.to_str().unwrap_or("") + ); create_dir_all(&cgroup_root)?; let memory = linux_resources.memory.as_ref().unwrap(); let reservation = memory.reservation.unwrap_or(0); @@ -187,17 +191,18 @@ impl Memory { // see: // https://github.com/opencontainers/runc/blob/master/libcontainer/cgroups/fs/memory.go#L89 if limit != 0 && swap != 0 { - let current_limit = Self::get_memory_limit(cgroup_root)?; + let current_limit = + Self::get_memory_limit(cgroup_root).expect("Should load current memory limit"); if swap == -1 || current_limit < swap { - Self::set_swap(swap, cgroup_root)?; - Self::set_memory(limit, cgroup_root)?; + Self::set_swap(swap, cgroup_root).expect("should set swap"); + Self::set_memory(limit, cgroup_root).expect("should set mem"); return Ok(()); } } - Self::set_memory(limit, cgroup_root)?; - Self::set_swap(swap, cgroup_root)?; + Self::set_memory(limit, cgroup_root).expect("should set mem"); + Self::set_swap(swap, cgroup_root).expect("should set swap"); Ok(()) } From 488276456472429f747000944b37dc3280fbcec4 Mon Sep 17 00:00:00 2001 From: Travis Sturzl Date: Fri, 21 May 2021 22:27:51 -0600 Subject: [PATCH 03/13] working --- src/cgroups/memory.rs | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/cgroups/memory.rs b/src/cgroups/memory.rs index 5f11540ea..5879507f8 100644 --- a/src/cgroups/memory.rs +++ b/src/cgroups/memory.rs @@ -31,6 +31,7 @@ impl Controller for Memory { cgroup_root.to_str().unwrap_or("") ); create_dir_all(&cgroup_root)?; + let memory = linux_resources.memory.as_ref().unwrap(); let reservation = memory.reservation.unwrap_or(0); @@ -83,6 +84,12 @@ impl Memory { .open(path)? .read_to_string(&mut contents)?; + contents = contents.trim().to_string(); + + if contents == "max" { + return Ok(u64::MAX); + } + let val = contents.parse::()?; Ok(val) } @@ -96,11 +103,17 @@ impl Memory { .open(path)? .read_to_string(&mut contents)?; + contents = contents.trim().to_string(); + + if contents == "max" { + return Ok(u64::MAX); + } + let val = contents.parse::()?; 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() @@ -109,7 +122,13 @@ impl Memory { .open(path)? .read_to_string(&mut contents)?; - let val = contents.parse::()?; + contents = contents.trim().to_string(); + + if contents == "max" { + return Ok(u64::MAX); + } + + let val = contents.parse::()?; Ok(val) } @@ -176,7 +195,7 @@ impl Memory { } fn set_memory_and_swap(resource: &LinuxMemory, cgroup_root: &Path) -> Result<()> { - let limit = resource.limit.unwrap_or(0); + let limit = resource.limit.unwrap_or(-1); let mut swap = resource.swap.unwrap_or(0); if limit == -1 && swap == 0 { @@ -194,7 +213,7 @@ impl Memory { let current_limit = Self::get_memory_limit(cgroup_root).expect("Should load current memory limit"); - if swap == -1 || current_limit < swap { + if swap == -1 || current_limit < swap as u64 { Self::set_swap(swap, cgroup_root).expect("should set swap"); Self::set_memory(limit, cgroup_root).expect("should set mem"); return Ok(()); From 8459e3edafd728b38100b4f5de84e9c8a376c462 Mon Sep 17 00:00:00 2001 From: Travis Sturzl Date: Fri, 21 May 2021 22:34:17 -0600 Subject: [PATCH 04/13] fix typo --- src/cgroups/memory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cgroups/memory.rs b/src/cgroups/memory.rs index 5879507f8..ad871b156 100644 --- a/src/cgroups/memory.rs +++ b/src/cgroups/memory.rs @@ -206,7 +206,7 @@ impl Memory { // 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 in the kernel's validation + // values don't fit the kernel's validation // see: // https://github.com/opencontainers/runc/blob/master/libcontainer/cgroups/fs/memory.go#L89 if limit != 0 && swap != 0 { From f05acfdc97e77a2e6457905ca01675a03e736e01 Mon Sep 17 00:00:00 2001 From: Travis Sturzl Date: Fri, 21 May 2021 22:41:09 -0600 Subject: [PATCH 05/13] add OOM control --- src/cgroups/memory.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/cgroups/memory.rs b/src/cgroups/memory.rs index ad871b156..259c3c5d8 100644 --- a/src/cgroups/memory.rs +++ b/src/cgroups/memory.rs @@ -18,6 +18,7 @@ const CGROUP_MEMORY_USAGE: &str = "memory.usage_in_bytes"; const CGROUP_MEMORY_MAX_USAGE: &str = "memory.max_usage_in_bytes"; const CGROUP_MEMORY_SWAPPINESS: &str = "memory.swappiness"; const CGROUP_MEMORY_RESERVATION: &str = "memory.soft_limit_in_bytes"; +const CGROUP_MEMORY_OOM_CONTROL: &str = "memory.oom_control"; const CGROUP_KERNEL_MEMORY_LIMIT: &str = "memory.kmem.limit_in_bytes"; const CGROUP_KERNEL_TCP_MEMORY_LIMIT: &str = "memory.kmem.tcp.limit_in_bytes"; @@ -41,6 +42,10 @@ impl Controller for Memory { Self::set_i64(reservation, &cgroup_root.join(CGROUP_MEMORY_RESERVATION))?; } + if linux_resources.disable_oom_killer { + Self::set_u64(1, &cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL))?; + } + if let Some(swappiness) = memory.swappiness { if swappiness <= 100 { Self::set_u64(swappiness, &cgroup_root.join(CGROUP_MEMORY_SWAPPINESS))?; From 6d8fbae0b932133c7dc39bbc5e156b98b48d571c Mon Sep 17 00:00:00 2001 From: Travis Sturzl Date: Fri, 21 May 2021 22:55:32 -0600 Subject: [PATCH 06/13] fix tests --- src/cgroups/memory.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/cgroups/memory.rs b/src/cgroups/memory.rs index 259c3c5d8..8bfdc2349 100644 --- a/src/cgroups/memory.rs +++ b/src/cgroups/memory.rs @@ -118,7 +118,7 @@ impl Memory { 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() @@ -130,10 +130,10 @@ impl Memory { contents = contents.trim().to_string(); if contents == "max" { - return Ok(u64::MAX); + return Ok(i64::MAX); } - let val = contents.parse::()?; + let val = contents.parse::()?; Ok(val) } @@ -215,18 +215,17 @@ impl Memory { // see: // https://github.com/opencontainers/runc/blob/master/libcontainer/cgroups/fs/memory.go#L89 if limit != 0 && swap != 0 { - let current_limit = - Self::get_memory_limit(cgroup_root).expect("Should load current memory limit"); + let current_limit = Self::get_memory_limit(cgroup_root)?; - if swap == -1 || current_limit < swap as u64 { - Self::set_swap(swap, cgroup_root).expect("should set swap"); - Self::set_memory(limit, cgroup_root).expect("should set mem"); + if swap == -1 || current_limit < swap { + Self::set_swap(swap, cgroup_root)?; + Self::set_memory(limit, cgroup_root)?; return Ok(()); } } - Self::set_memory(limit, cgroup_root).expect("should set mem"); - Self::set_swap(swap, cgroup_root).expect("should set swap"); + Self::set_memory(limit, cgroup_root)?; + Self::set_swap(swap, cgroup_root)?; Ok(()) } From b246640ce6437dd84218186a261e7f9a04f0d8e9 Mon Sep 17 00:00:00 2001 From: Travis Sturzl Date: Fri, 21 May 2021 23:22:33 -0600 Subject: [PATCH 07/13] flip oom control --- src/cgroups/memory.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cgroups/memory.rs b/src/cgroups/memory.rs index 8bfdc2349..b33ab01f6 100644 --- a/src/cgroups/memory.rs +++ b/src/cgroups/memory.rs @@ -43,6 +43,8 @@ impl Controller for Memory { } if linux_resources.disable_oom_killer { + Self::set_u64(0, &cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL))?; + } else { Self::set_u64(1, &cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL))?; } From cecbf3a7d106128192cdfbd51aa4619a403bb7f3 Mon Sep 17 00:00:00 2001 From: Travis Sturzl Date: Fri, 21 May 2021 23:48:00 -0600 Subject: [PATCH 08/13] ignore missing memory configuration --- src/cgroups/memory.rs | 72 +++++++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/src/cgroups/memory.rs b/src/cgroups/memory.rs index b33ab01f6..479a120eb 100644 --- a/src/cgroups/memory.rs +++ b/src/cgroups/memory.rs @@ -33,50 +33,50 @@ impl Controller for Memory { ); create_dir_all(&cgroup_root)?; - let memory = linux_resources.memory.as_ref().unwrap(); - let reservation = memory.reservation.unwrap_or(0); + if let Some(memory) = &linux_resources.memory { + let reservation = memory.reservation.unwrap_or(0); - Self::set_memory_and_swap(&memory, cgroup_root)?; + Self::set_memory_and_swap(&memory, cgroup_root)?; - if reservation != 0 { - Self::set_i64(reservation, &cgroup_root.join(CGROUP_MEMORY_RESERVATION))?; - } - - if linux_resources.disable_oom_killer { - Self::set_u64(0, &cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL))?; - } else { - Self::set_u64(1, &cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL))?; - } + if reservation != 0 { + Self::set_i64(reservation, &cgroup_root.join(CGROUP_MEMORY_RESERVATION))?; + } - if let Some(swappiness) = memory.swappiness { - if swappiness <= 100 { - Self::set_u64(swappiness, &cgroup_root.join(CGROUP_MEMORY_SWAPPINESS))?; + if linux_resources.disable_oom_killer { + Self::set_u64(0, &cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL))?; } else { - // invalid swappiness value - return Err(anyhow!( - "Invalid swappiness value: {}. Valid range is 0-100", - swappiness - )); + Self::set_u64(1, &cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL))?; } - } - // NOTE: Seems as though kernel and kernelTCP are both deprecated - // neither are implemented by runc. Tests pass without this, but - // kept in per the spec. - if let Some(kmem) = memory.kernel { - Self::set_i64(kmem, &cgroup_root.join(CGROUP_KERNEL_MEMORY_LIMIT))?; - } - if let Some(tcp_mem) = memory.kernel_tcp { - Self::set_i64(tcp_mem, &cgroup_root.join(CGROUP_KERNEL_TCP_MEMORY_LIMIT))?; - } + if let Some(swappiness) = memory.swappiness { + if swappiness <= 100 { + Self::set_u64(swappiness, &cgroup_root.join(CGROUP_MEMORY_SWAPPINESS))?; + } else { + // invalid swappiness value + return Err(anyhow!( + "Invalid swappiness value: {}. Valid range is 0-100", + swappiness + )); + } + } - OpenOptions::new() - .create(false) - .write(true) - .truncate(false) - .open(cgroup_root.join("cgroup.procs"))? - .write_all(pid.to_string().as_bytes())?; + // NOTE: Seems as though kernel and kernelTCP are both deprecated + // neither are implemented by runc. Tests pass without this, but + // kept in per the spec. + if let Some(kmem) = memory.kernel { + Self::set_i64(kmem, &cgroup_root.join(CGROUP_KERNEL_MEMORY_LIMIT))?; + } + if let Some(tcp_mem) = memory.kernel_tcp { + Self::set_i64(tcp_mem, &cgroup_root.join(CGROUP_KERNEL_TCP_MEMORY_LIMIT))?; + } + OpenOptions::new() + .create(false) + .write(true) + .truncate(false) + .open(cgroup_root.join("cgroup.procs"))? + .write_all(pid.to_string().as_bytes())?; + } Ok(()) } } From bb15e3752ffed2ce1e750ee4e4086d982a727004 Mon Sep 17 00:00:00 2001 From: Travis Sturzl Date: Sun, 23 May 2021 20:15:59 -0600 Subject: [PATCH 09/13] Adopt the new subsystem work --- src/cgroups/manager.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/cgroups/manager.rs b/src/cgroups/manager.rs index 38cb29fda..b971ef7fa 100644 --- a/src/cgroups/manager.rs +++ b/src/cgroups/manager.rs @@ -7,13 +7,15 @@ use procfs::process::Process; use crate::{cgroups::ControllerType, spec::LinuxResources, utils::PathBufExt}; -use super::{devices::Devices, hugetlb::Hugetlb, pids::Pids, Controller}; +use super::{devices::Devices, hugetlb::Hugetlb, memory::Memory, pids::Pids, Controller}; const CONTROLLERS: &[ControllerType] = &[ ControllerType::Devices, ControllerType::HugeTlb, + ControllerType::Memory, ControllerType::Pids, ]; + pub struct Manager { subsystems: HashMap, } @@ -36,6 +38,7 @@ impl Manager { match subsys.0.as_str() { "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)?, _ => continue, } From c1da9bdb840be9007c484618802b743c9961f1e0 Mon Sep 17 00:00:00 2001 From: Travis Sturzl Date: Sun, 23 May 2021 20:18:14 -0600 Subject: [PATCH 10/13] add the integration test for memory subsystem back --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 81d25a77f..66f7fb365 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -45,7 +45,7 @@ jobs: expect_err_num=8 act_err_num=0 cd $(go env GOPATH)/src/github.com/opencontainers/runtime-tools - 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") + 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") for case in "${test_cases[@]}"; do title="Running $case" if [ 0 -ne $(sudo RUNTIME=$GITHUB_WORKSPACE/target/x86_64-unknown-linux-gnu/debug/youki ./validation/$case | grep "not ok" | wc -l) ]; then From 6d5157b0011c3d6d3f2785c49c05128eeb99bb75 Mon Sep 17 00:00:00 2001 From: Travis Sturzl Date: Sun, 23 May 2021 20:21:01 -0600 Subject: [PATCH 11/13] we only need the first result --- src/cgroups/manager.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/cgroups/manager.rs b/src/cgroups/manager.rs index b971ef7fa..1215ec0d8 100644 --- a/src/cgroups/manager.rs +++ b/src/cgroups/manager.rs @@ -62,17 +62,13 @@ impl Manager { let mount = Process::myself()? .mountinfo()? .into_iter() - .filter(|m| m.fs_type == "cgroup" && m.mount_point.ends_with(subsystem)) - .collect::>() - .pop() + .find(|m| m.fs_type == "cgroup" && m.mount_point.ends_with(subsystem)) .unwrap(); let cgroup = Process::myself()? .cgroups()? .into_iter() - .filter(|c| c.controllers.contains(&subsystem.to_owned())) - .collect::>() - .pop() + .find(|c| c.controllers.contains(&subsystem.to_owned())) .unwrap(); let p = if cgroup_path.to_string_lossy().into_owned().is_empty() { From 54978c21cd8bc27018642da93a56beb98f0ee2d1 Mon Sep 17 00:00:00 2001 From: Travis Sturzl Date: Sun, 23 May 2021 22:15:20 -0600 Subject: [PATCH 12/13] general cleanup --- src/cgroups/memory.rs | 94 ++++++++++++++++++++++++------------------- 1 file changed, 52 insertions(+), 42 deletions(-) diff --git a/src/cgroups/memory.rs b/src/cgroups/memory.rs index 479a120eb..3dedba7aa 100644 --- a/src/cgroups/memory.rs +++ b/src/cgroups/memory.rs @@ -36,21 +36,21 @@ impl Controller for Memory { if let Some(memory) = &linux_resources.memory { let reservation = memory.reservation.unwrap_or(0); - Self::set_memory_and_swap(&memory, cgroup_root)?; + Self::apply(&memory, cgroup_root)?; if reservation != 0 { - Self::set_i64(reservation, &cgroup_root.join(CGROUP_MEMORY_RESERVATION))?; + Self::set(reservation, &cgroup_root.join(CGROUP_MEMORY_RESERVATION))?; } if linux_resources.disable_oom_killer { - Self::set_u64(0, &cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL))?; + Self::set(0, &cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL))?; } else { - Self::set_u64(1, &cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL))?; + Self::set(1, &cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL))?; } if let Some(swappiness) = memory.swappiness { if swappiness <= 100 { - Self::set_u64(swappiness, &cgroup_root.join(CGROUP_MEMORY_SWAPPINESS))?; + Self::set(swappiness, &cgroup_root.join(CGROUP_MEMORY_SWAPPINESS))?; } else { // invalid swappiness value return Err(anyhow!( @@ -64,10 +64,10 @@ 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_i64(kmem, &cgroup_root.join(CGROUP_KERNEL_MEMORY_LIMIT))?; + Self::set(kmem, &cgroup_root.join(CGROUP_KERNEL_MEMORY_LIMIT))?; } if let Some(tcp_mem) = memory.kernel_tcp { - Self::set_i64(tcp_mem, &cgroup_root.join(CGROUP_KERNEL_TCP_MEMORY_LIMIT))?; + Self::set(tcp_mem, &cgroup_root.join(CGROUP_KERNEL_TCP_MEMORY_LIMIT))?; } OpenOptions::new() @@ -139,17 +139,7 @@ impl Memory { Ok(val) } - fn set_i64(val: i64, path: &Path) -> std::io::Result<()> { - OpenOptions::new() - .create(false) - .write(true) - .truncate(true) - .open(path)? - .write_all(val.to_string().as_bytes())?; - Ok(()) - } - - fn set_u64(val: u64, path: &Path) -> std::io::Result<()> { + fn set(val: T, path: &Path) -> std::io::Result<()> { OpenOptions::new() .create(false) .write(true) @@ -162,7 +152,7 @@ impl Memory { fn set_memory(val: i64, cgroup_root: &Path) -> Result<()> { let path = cgroup_root.join(CGROUP_MEMORY_LIMIT); - match Self::set_i64(val, &path) { + match Self::set(val, &path) { Ok(_) => Ok(()), Err(e) => { // we need to look into the raw OS error for an EBUSY status @@ -196,39 +186,59 @@ impl Memory { let path = cgroup_root.join(CGROUP_MEMORY_SWAP_LIMIT); - Self::set_i64(val, &path)?; + Self::set(val, &path)?; Ok(()) } - fn set_memory_and_swap(resource: &LinuxMemory, cgroup_root: &Path) -> Result<()> { - let limit = resource.limit.unwrap_or(-1); - let mut swap = resource.swap.unwrap_or(0); - - if limit == -1 && swap == 0 { - if cgroup_root.join(CGROUP_MEMORY_SWAP_LIMIT).exists() { - swap = -1; - } - } - + fn set_memory_and_swap( + limit: i64, + swap: i64, + is_updated: bool, + cgroup_root: &Path, + ) -> Result<()> { // 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 // see: - // https://github.com/opencontainers/runc/blob/master/libcontainer/cgroups/fs/memory.go#L89 - if limit != 0 && swap != 0 { - let current_limit = Self::get_memory_limit(cgroup_root)?; - - if swap == -1 || current_limit < swap { - Self::set_swap(swap, cgroup_root)?; - Self::set_memory(limit, cgroup_root)?; - return Ok(()); - } + // https://github.com/opencontainers/runc/blob/3f6594675675d4e88901c782462f56497260b1d2/libcontainer/cgroups/fs/memory.go#L89 + if is_updated { + Self::set_swap(swap, cgroup_root)?; + Self::set_memory(limit, cgroup_root)?; } - Self::set_memory(limit, cgroup_root)?; Self::set_swap(swap, cgroup_root)?; + Ok(()) + } + fn apply(resource: &LinuxMemory, cgroup_root: &Path) -> Result<()> { + match resource.limit { + Some(limit) => { + let current_limit = Self::get_memory_limit(cgroup_root)?; + match resource.swap { + Some(swap) => { + let is_updated = swap == -1 || current_limit < swap; + Self::set_memory_and_swap(limit, swap, is_updated, cgroup_root)?; + } + None => { + if limit == -1 { + Self::set_memory_and_swap(limit, -1, true, cgroup_root)?; + } else { + let is_updated = current_limit < 0; + Self::set_memory_and_swap(limit, 0, is_updated, cgroup_root)?; + } + } + } + } + None => match resource.swap { + Some(swap) => { + Self::set_memory_and_swap(0, swap, false, cgroup_root)?; + } + None => { + Self::set_memory_and_swap(0, 0, false, cgroup_root)?; + } + }, + } Ok(()) } } @@ -298,7 +308,7 @@ mod tests { kernel_tcp: None, swappiness: None, }; - Memory::set_memory_and_swap(linux_memory, &tmp).expect("Set memory and swap"); + Memory::apply(linux_memory, &tmp).expect("Set memory and swap"); let limit_content = std::fs::read_to_string(tmp.join(CGROUP_MEMORY_LIMIT)).expect("Read to string"); @@ -322,7 +332,7 @@ mod tests { kernel_tcp: None, swappiness: None, }; - Memory::set_memory_and_swap(linux_memory, &tmp).expect("Set memory and swap"); + Memory::apply(linux_memory, &tmp).expect("Set memory and swap"); let limit_content = std::fs::read_to_string(tmp.join(CGROUP_MEMORY_LIMIT)).expect("Read to string"); From b808f30c85b814bc109e7056a85c8aa0c8f139ab Mon Sep 17 00:00:00 2001 From: Travis Sturzl Date: Sun, 23 May 2021 22:27:27 -0600 Subject: [PATCH 13/13] cleanup errno check --- src/cgroups/memory.rs | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/src/cgroups/memory.rs b/src/cgroups/memory.rs index 3dedba7aa..8cc5805e3 100644 --- a/src/cgroups/memory.rs +++ b/src/cgroups/memory.rs @@ -156,24 +156,21 @@ impl Memory { Ok(_) => Ok(()), Err(e) => { // we need to look into the raw OS error for an EBUSY status - if let Some(code) = e.raw_os_error() { - // the nix crate has a handy enum for these, lets use that - let errno = Errno::from_i32(code); - // if the error is EBUSY - if let Errno::EBUSY = errno { - let usage = Self::get_memory_usage(cgroup_root)?; - let max_usage = Self::get_memory_max_usage(cgroup_root)?; - Err(anyhow!( - "unable to set memory limit to {} (current usage: {}, peak usage: {})", - val, - usage, - max_usage, - )) - } else { - Err(anyhow!(e)) - } - } else { - Err(anyhow!(e)) + match e.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)?; + Err(anyhow!( + "unable to set memory limit to {} (current usage: {}, peak usage: {})", + val, + usage, + max_usage, + )) + } + _ => Err(anyhow!(e)), + }, + None => Err(anyhow!(e)), } } }