From 97848f1ce6525f45893b0e45eae3deb618f87cab Mon Sep 17 00:00:00 2001 From: Takashi IIGUNI Date: Mon, 27 Sep 2021 10:08:56 +0900 Subject: [PATCH] Updated cgroups oci-spec-rs to 0.5.1 or later (#303) * Updated cgroup oci-spec-rs to 0.5.1 Signed-off-by: Takashi IIGUNI --- Cargo.lock | 97 ++++++++- Cargo.toml | 10 +- cgroups/Cargo.toml | 10 +- cgroups/src/common.rs | 192 +++++++++--------- cgroups/src/test.rs | 61 ------ cgroups/src/v1/blkio.rs | 128 +++++------- cgroups/src/v1/cpu.rs | 43 ++-- cgroups/src/v1/cpuset.rs | 21 +- cgroups/src/v1/devices.rs | 70 +++---- cgroups/src/v1/freezer.rs | 51 ++--- cgroups/src/v1/hugetlb.rs | 39 ++-- cgroups/src/v1/manager.rs | 4 +- cgroups/src/v1/memory.rs | 285 ++++++++++++--------------- cgroups/src/v1/network_classifier.rs | 16 +- cgroups/src/v1/network_priority.rs | 32 +-- cgroups/src/v1/pids.rs | 24 +-- cgroups/src/v2/cpu.rs | 44 +++-- cgroups/src/v2/cpuset.rs | 19 +- cgroups/src/v2/devices/controller.rs | 2 +- cgroups/src/v2/devices/emulator.rs | 6 +- cgroups/src/v2/devices/program.rs | 84 ++++---- cgroups/src/v2/hugetlb.rs | 36 ++-- cgroups/src/v2/io.rs | 169 +++++++--------- cgroups/src/v2/memory.rs | 113 ++++------- cgroups/src/v2/pids.rs | 14 +- cgroups/src/v2/unified.rs | 81 ++++---- src/capabilities.rs | 68 ++++--- src/commands/ps.rs | 5 +- src/container/builder_impl.rs | 52 ++--- src/container/container_delete.rs | 9 +- src/container/container_events.rs | 6 +- src/container/container_pause.rs | 5 +- src/container/container_resume.rs | 5 +- src/container/container_start.rs | 8 +- src/container/init_builder.rs | 12 +- src/container/tenant_builder.rs | 243 +++++++++++++---------- src/hooks.rs | 47 ++--- src/namespaces.rs | 54 ++--- src/process/init.rs | 61 +++--- src/process/intermediate.rs | 10 +- src/rootfs.rs | 154 +++++++-------- src/rootless.rs | 34 ++-- src/seccomp/mod.rs | 50 ++--- src/syscall/linux.rs | 8 +- 44 files changed, 1241 insertions(+), 1241 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b37742cb..399dfce4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -231,6 +231,41 @@ dependencies = [ "memchr", ] +[[package]] +name = "darling" +version = "0.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5f2c43f534ea4b0b049015d00269734195e6d3f0f6635cb692251aca6f9f8b3c" +dependencies = [ + "darling_core", + "darling_macro", +] + +[[package]] +name = "darling_core" +version = "0.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e91455b86830a1c21799d94524df0845183fa55bafd9aa137b01c7d1065fa36" +dependencies = [ + "fnv", + "ident_case", + "proc-macro2", + "quote", + "strsim 0.10.0", + "syn", +] + +[[package]] +name = "darling_macro" +version = "0.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29b5acf0dea37a7f66f7b25d2c5e93fd46f8f6968b1a5d7a3e02e97768afc95a" +dependencies = [ + "darling_core", + "quote", + "syn", +] + [[package]] name = "dbus" version = "0.9.3" @@ -242,6 +277,37 @@ dependencies = [ "winapi", ] +[[package]] +name = "derive_builder" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d13202debe11181040ae9063d739fa32cfcaaebe2275fe387703460ae2365b30" +dependencies = [ + "derive_builder_macro", +] + +[[package]] +name = "derive_builder_core" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "66e616858f6187ed828df7c64a6d71720d83767a7f19740b2d1b6fe6327b36e5" +dependencies = [ + "darling", + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "derive_builder_macro" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "58a94ace95092c5acb1e97a7e846b310cfbd499652f72297da7493f618a98d73" +dependencies = [ + "derive_builder_core", + "syn", +] + [[package]] name = "env_logger" version = "0.8.4" @@ -307,6 +373,12 @@ dependencies = [ "miniz_oxide", ] +[[package]] +name = "fnv" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" + [[package]] name = "foreign-types" version = "0.5.0" @@ -446,6 +518,18 @@ dependencies = [ "wasi", ] +[[package]] +name = "getset" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24b328c01a4d71d2d8173daa93562a73ab0fe85616876f02500f53d82948c504" +dependencies = [ + "proc-macro-error", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "hashbrown" version = "0.11.2" @@ -482,6 +566,12 @@ version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9a3a5bfb195931eeb336b2a7b4d761daec841b97f947d34394601737a7bba5e4" +[[package]] +name = "ident_case" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" + [[package]] name = "indexmap" version = "1.7.0" @@ -667,10 +757,11 @@ dependencies = [ [[package]] name = "oci-spec" -version = "0.4.0" -source = "git+https://github.com/utam0k/oci-spec-rs/?tag=v0.4.0-with-bugfix#73540d3183136d0188b9c3a40f24b08295bbc92e" +version = "0.5.1" +source = "git+https://github.com/containers/oci-spec-rs?rev=5018f8e5b0355a82c08962cefa5ab07a05b930c6#5018f8e5b0355a82c08962cefa5ab07a05b930c6" dependencies = [ - "cfg-if 1.0.0", + "derive_builder", + "getset", "quickcheck", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index c1d9d00c..a3ccfde9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,7 +39,10 @@ mio = { version = "0.7.13", features = ["os-ext", "os-poll"] } chrono = { version="0.4", features = ["serde"] } once_cell = "1.6.0" futures = { version = "0.3", features = ["thread-pool"] } -oci-spec = { git="https://github.com/utam0k/oci-spec-rs/", tag = "v0.4.0-with-bugfix"} +# TODO: Fetch from crate.io instead of git when next release oci-spec-rs +# ref: https://github.com/containers/oci-spec-rs/pull/69 +# oci-spec = "0.5.1" +oci-spec = { git = "https://github.com/containers/oci-spec-rs", rev = "5018f8e5b0355a82c08962cefa5ab07a05b930c6" } cgroups = { version = "0.1.0", path = "./cgroups" } systemd = { version = "0.8", default-features = false, optional = true } dbus = "0.9.2" @@ -49,7 +52,10 @@ crossbeam-channel = "0.5" seccomp = { version = "0.1.0", path = "./seccomp" } [dev-dependencies] -oci-spec = { git = "https://github.com/utam0k/oci-spec-rs/", tag = "v0.4.0-with-bugfix", features = ["proptests"] } +# TODO: Fetch from crate.io instead of git when next release oci-spec-rs +# ref: https://github.com/containers/oci-spec-rs/pull/69 +# oci-spec = "0.5.1" +oci-spec = { git = "https://github.com/containers/oci-spec-rs", rev = "5018f8e5b0355a82c08962cefa5ab07a05b930c6", features = ["proptests"] } quickcheck = "1" serial_test = "0.5.1" diff --git a/cgroups/Cargo.toml b/cgroups/Cargo.toml index a3e3a5f0..435efb17 100644 --- a/cgroups/Cargo.toml +++ b/cgroups/Cargo.toml @@ -14,7 +14,10 @@ nix = "0.22.0" procfs = "0.10.1" log = "0.4" anyhow = "1.0" -oci-spec = { git="https://github.com/utam0k/oci-spec-rs/", tag = "v0.4.0-with-bugfix"} +# TODO: Fetch from crate.io instead of git when next release oci-spec-rs +# ref: https://github.com/containers/oci-spec-rs/pull/69 +# oci-spec = "0.5.1" +oci-spec = { git = "https://github.com/containers/oci-spec-rs", rev = "5018f8e5b0355a82c08962cefa5ab07a05b930c6" } systemd = { version = "0.8", default-features = false, optional = true } dbus = "0.9.2" serde = { version = "1.0", features = ["derive"] } @@ -24,7 +27,10 @@ errno = { version = "0.2.7", optional = true } libc = { version = "0.2.84", optional = true } [dev-dependencies] -oci-spec = { git = "https://github.com/utam0k/oci-spec-rs/", tag = "v0.4.0-with-bugfix", features = ["proptests"] } +# TODO: Fetch from crate.io instead of git when next release oci-spec-rs +# ref: https://github.com/containers/oci-spec-rs/pull/69 +# oci-spec = "0.5.1" +oci-spec = { git = "https://github.com/containers/oci-spec-rs", rev = "5018f8e5b0355a82c08962cefa5ab07a05b930c6", features = ["proptests"] } quickcheck = "1" clap = "2" serde = { version = "1.0", features = ["derive"] } diff --git a/cgroups/src/common.rs b/cgroups/src/common.rs index 334576ad..e2095de3 100644 --- a/cgroups/src/common.rs +++ b/cgroups/src/common.rs @@ -10,7 +10,10 @@ use nix::{ sys::statfs::{statfs, CGROUP2_SUPER_MAGIC, TMPFS_MAGIC}, unistd::Pid, }; -use oci_spec::runtime::{LinuxDevice, LinuxDeviceCgroup, LinuxDeviceType, LinuxResources}; +use oci_spec::runtime::{ + LinuxDevice, LinuxDeviceBuilder, LinuxDeviceCgroup, LinuxDeviceCgroupBuilder, LinuxDeviceType, + LinuxResources, +}; #[cfg(feature = "systemd_cgroups")] use systemd::daemon::booted; #[cfg(not(feature = "systemd_cgroups"))] @@ -249,109 +252,104 @@ impl PathBufExt for PathBuf { pub(crate) fn default_allow_devices() -> Vec { vec![ - LinuxDeviceCgroup { - allow: true, - typ: Some(LinuxDeviceType::C), - major: None, - minor: None, - access: "m".to_string().into(), - }, - LinuxDeviceCgroup { - allow: true, - typ: Some(LinuxDeviceType::B), - major: None, - minor: None, - access: "m".to_string().into(), - }, + LinuxDeviceCgroupBuilder::default() + .allow(true) + .typ(LinuxDeviceType::C) + .access("m") + .build() + .unwrap(), + LinuxDeviceCgroupBuilder::default() + .allow(true) + .typ(LinuxDeviceType::B) + .access("m") + .build() + .unwrap(), // /dev/console - LinuxDeviceCgroup { - allow: true, - typ: Some(LinuxDeviceType::C), - major: Some(5), - minor: Some(1), - access: "rwm".to_string().into(), - }, + LinuxDeviceCgroupBuilder::default() + .allow(true) + .typ(LinuxDeviceType::C) + .major(5) + .minor(1) + .access("rwm") + .build() + .unwrap(), // /dev/pts - LinuxDeviceCgroup { - allow: true, - typ: Some(LinuxDeviceType::C), - major: Some(136), - minor: None, - access: "rwm".to_string().into(), - }, - LinuxDeviceCgroup { - allow: true, - typ: Some(LinuxDeviceType::C), - major: Some(5), - minor: Some(2), - access: "rwm".to_string().into(), - }, + LinuxDeviceCgroupBuilder::default() + .allow(true) + .typ(LinuxDeviceType::C) + .major(136) + .access("rwm") + .build() + .unwrap(), + LinuxDeviceCgroupBuilder::default() + .allow(true) + .typ(LinuxDeviceType::C) + .major(5) + .minor(2) + .access("rwm") + .build() + .unwrap(), // tun/tap - LinuxDeviceCgroup { - allow: true, - typ: Some(LinuxDeviceType::C), - major: Some(10), - minor: Some(200), - access: "rwm".to_string().into(), - }, + LinuxDeviceCgroupBuilder::default() + .allow(true) + .typ(LinuxDeviceType::C) + .major(10) + .minor(200) + .access("rwm") + .build() + .unwrap(), ] } pub(crate) fn default_devices() -> Vec { vec![ - LinuxDevice { - path: PathBuf::from("/dev/null"), - typ: LinuxDeviceType::C, - major: 1, - minor: 3, - file_mode: Some(0o066), - uid: None, - gid: None, - }, - LinuxDevice { - path: PathBuf::from("/dev/zero"), - typ: LinuxDeviceType::C, - major: 1, - minor: 5, - file_mode: Some(0o066), - uid: None, - gid: None, - }, - LinuxDevice { - path: PathBuf::from("/dev/full"), - typ: LinuxDeviceType::C, - major: 1, - minor: 7, - file_mode: Some(0o066), - uid: None, - gid: None, - }, - LinuxDevice { - path: PathBuf::from("/dev/tty"), - typ: LinuxDeviceType::C, - major: 5, - minor: 0, - file_mode: Some(0o066), - uid: None, - gid: None, - }, - LinuxDevice { - path: PathBuf::from("/dev/urandom"), - typ: LinuxDeviceType::C, - major: 1, - minor: 9, - file_mode: Some(0o066), - uid: None, - gid: None, - }, - LinuxDevice { - path: PathBuf::from("/dev/random"), - typ: LinuxDeviceType::C, - major: 1, - minor: 8, - file_mode: Some(0o066), - uid: None, - gid: None, - }, + LinuxDeviceBuilder::default() + .path(PathBuf::from("/dev/null")) + .typ(LinuxDeviceType::C) + .major(1) + .minor(3) + .file_mode(0o066u32) + .build() + .unwrap(), + LinuxDeviceBuilder::default() + .path(PathBuf::from("/dev/zero")) + .typ(LinuxDeviceType::C) + .major(1) + .minor(5) + .file_mode(0o066u32) + .build() + .unwrap(), + LinuxDeviceBuilder::default() + .path(PathBuf::from("/dev/full")) + .typ(LinuxDeviceType::C) + .major(1) + .minor(7) + .file_mode(0o066u32) + .build() + .unwrap(), + LinuxDeviceBuilder::default() + .path(PathBuf::from("/dev/tty")) + .typ(LinuxDeviceType::C) + .major(5) + .minor(0) + .file_mode(0o066u32) + .build() + .unwrap(), + LinuxDeviceBuilder::default() + .path(PathBuf::from("/dev/urandom")) + .typ(LinuxDeviceType::C) + .major(1) + .minor(9) + .file_mode(0o066u32) + .build() + .unwrap(), + LinuxDeviceBuilder::default() + .path(PathBuf::from("/dev/random")) + .typ(LinuxDeviceType::C) + .major(1) + .minor(8) + .file_mode(0o066u32) + .build() + .unwrap(), ] } diff --git a/cgroups/src/test.rs b/cgroups/src/test.rs index 0547702a..e2c54490 100644 --- a/cgroups/src/test.rs +++ b/cgroups/src/test.rs @@ -8,8 +8,6 @@ use std::{ path::{Path, PathBuf}, }; -use oci_spec::runtime::LinuxCpu; - pub struct TempDir { path: Option, } @@ -82,62 +80,3 @@ pub fn set_fixture(temp_dir: &Path, filename: &str, val: &str) -> Result Self { - Self { - resource: LinuxCpu { - shares: None, - quota: None, - period: None, - realtime_runtime: None, - realtime_period: None, - cpus: None, - mems: None, - }, - } - } - - pub fn with_shares(mut self, shares: u64) -> Self { - self.resource.shares = Some(shares); - self - } - - pub fn with_quota(mut self, quota: i64) -> Self { - self.resource.quota = Some(quota); - self - } - - pub fn with_period(mut self, period: u64) -> Self { - self.resource.period = Some(period); - self - } - - pub fn with_realtime_runtime(mut self, runtime: i64) -> Self { - self.resource.realtime_runtime = Some(runtime); - self - } - - pub fn with_realtime_period(mut self, period: u64) -> Self { - self.resource.realtime_period = Some(period); - self - } - - pub fn with_cpus(mut self, cpus: String) -> Self { - self.resource.cpus = Some(cpus); - self - } - - pub fn with_mems(mut self, mems: String) -> Self { - self.resource.mems = Some(mems); - self - } - - pub fn build(self) -> LinuxCpu { - self.resource - } -} diff --git a/cgroups/src/v1/blkio.rs b/cgroups/src/v1/blkio.rs index 6e2e5956..4ed01d6d 100644 --- a/cgroups/src/v1/blkio.rs +++ b/cgroups/src/v1/blkio.rs @@ -85,7 +85,7 @@ impl Controller for Blkio { } fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> { - if let Some(blkio) = &controller_opt.resources.block_io { + if let Some(blkio) = &controller_opt.resources.block_io() { return Some(blkio); } @@ -107,38 +107,38 @@ impl StatsProvider for Blkio { impl Blkio { fn apply(root_path: &Path, blkio: &LinuxBlockIo) -> Result<()> { - if let Some(throttle_read_bps_device) = blkio.throttle_read_bps_device.as_ref() { + if let Some(throttle_read_bps_device) = blkio.throttle_read_bps_device().as_ref() { for trbd in throttle_read_bps_device { common::write_cgroup_file_str( &root_path.join(BLKIO_THROTTLE_READ_BPS), - &format!("{}:{} {}", trbd.major, trbd.minor, trbd.rate), + &format!("{}:{} {}", trbd.major(), trbd.minor(), trbd.rate()), )?; } } - if let Some(throttle_write_bps_device) = blkio.throttle_write_bps_device.as_ref() { + if let Some(throttle_write_bps_device) = blkio.throttle_write_bps_device().as_ref() { for twbd in throttle_write_bps_device { common::write_cgroup_file_str( &root_path.join(BLKIO_THROTTLE_WRITE_BPS), - &format!("{}:{} {}", twbd.major, twbd.minor, twbd.rate), + &format!("{}:{} {}", twbd.major(), twbd.minor(), twbd.rate()), )?; } } - if let Some(throttle_read_iops_device) = blkio.throttle_read_iops_device.as_ref() { + if let Some(throttle_read_iops_device) = blkio.throttle_read_iops_device().as_ref() { for trid in throttle_read_iops_device { common::write_cgroup_file_str( &root_path.join(BLKIO_THROTTLE_READ_IOPS), - &format!("{}:{} {}", trid.major, trid.minor, trid.rate), + &format!("{}:{} {}", trid.major(), trid.minor(), trid.rate()), )?; } } - if let Some(throttle_write_iops_device) = blkio.throttle_write_iops_device.as_ref() { + if let Some(throttle_write_iops_device) = blkio.throttle_write_iops_device().as_ref() { for twid in throttle_write_iops_device { common::write_cgroup_file_str( &root_path.join(BLKIO_THROTTLE_WRITE_IOPS), - &format!("{}:{} {}", twid.major, twid.minor, twid.rate), + &format!("{}:{} {}", twid.major(), twid.minor(), twid.rate()), )?; } } @@ -228,63 +228,21 @@ mod tests { use crate::test::{create_temp_dir, set_fixture, setup}; use anyhow::Result; - use oci_spec::runtime::{LinuxBlockIo, LinuxThrottleDevice}; - - struct BlockIoBuilder { - block_io: LinuxBlockIo, - } - - impl BlockIoBuilder { - fn new() -> Self { - let block_io = LinuxBlockIo { - weight: Some(0), - leaf_weight: Some(0), - weight_device: vec![].into(), - throttle_read_bps_device: vec![].into(), - throttle_write_bps_device: vec![].into(), - throttle_read_iops_device: vec![].into(), - throttle_write_iops_device: vec![].into(), - }; - - Self { block_io } - } - - fn with_read_bps(mut self, throttle: Vec) -> Self { - self.block_io.throttle_read_bps_device = throttle.into(); - self - } - - fn with_write_bps(mut self, throttle: Vec) -> Self { - self.block_io.throttle_write_bps_device = throttle.into(); - self - } - - fn with_read_iops(mut self, throttle: Vec) -> Self { - self.block_io.throttle_read_iops_device = throttle.into(); - self - } - - fn with_write_iops(mut self, throttle: Vec) -> Self { - self.block_io.throttle_write_iops_device = throttle.into(); - self - } - - fn build(self) -> LinuxBlockIo { - self.block_io - } - } + use oci_spec::runtime::{LinuxBlockIoBuilder, LinuxThrottleDeviceBuilder}; #[test] fn test_set_blkio_read_bps() { let (tmp, throttle) = setup("test_set_blkio_read_bps", BLKIO_THROTTLE_READ_BPS); - let blkio = BlockIoBuilder::new() - .with_read_bps(vec![LinuxThrottleDevice { - major: 8, - minor: 0, - rate: 102400, - }]) - .build(); + let blkio = LinuxBlockIoBuilder::default() + .throttle_read_bps_device(vec![LinuxThrottleDeviceBuilder::default() + .major(8) + .minor(0) + .rate(102400u64) + .build() + .unwrap()]) + .build() + .unwrap(); Blkio::apply(&tmp, &blkio).expect("apply blkio"); let content = fs::read_to_string(throttle) @@ -297,13 +255,15 @@ mod tests { fn test_set_blkio_write_bps() { let (tmp, throttle) = setup("test_set_blkio_write_bps", BLKIO_THROTTLE_WRITE_BPS); - let blkio = BlockIoBuilder::new() - .with_write_bps(vec![LinuxThrottleDevice { - major: 8, - minor: 0, - rate: 102400, - }]) - .build(); + let blkio = LinuxBlockIoBuilder::default() + .throttle_write_bps_device(vec![LinuxThrottleDeviceBuilder::default() + .major(8) + .minor(0) + .rate(102400u64) + .build() + .unwrap()]) + .build() + .unwrap(); Blkio::apply(&tmp, &blkio).expect("apply blkio"); let content = fs::read_to_string(throttle) @@ -316,13 +276,15 @@ mod tests { fn test_set_blkio_read_iops() { let (tmp, throttle) = setup("test_set_blkio_read_iops", BLKIO_THROTTLE_READ_IOPS); - let blkio = BlockIoBuilder::new() - .with_read_iops(vec![LinuxThrottleDevice { - major: 8, - minor: 0, - rate: 102400, - }]) - .build(); + let blkio = LinuxBlockIoBuilder::default() + .throttle_read_iops_device(vec![LinuxThrottleDeviceBuilder::default() + .major(8) + .minor(0) + .rate(102400u64) + .build() + .unwrap()]) + .build() + .unwrap(); Blkio::apply(&tmp, &blkio).expect("apply blkio"); let content = fs::read_to_string(throttle) @@ -335,13 +297,15 @@ mod tests { fn test_set_blkio_write_iops() { let (tmp, throttle) = setup("test_set_blkio_write_iops", BLKIO_THROTTLE_WRITE_IOPS); - let blkio = BlockIoBuilder::new() - .with_write_iops(vec![LinuxThrottleDevice { - major: 8, - minor: 0, - rate: 102400, - }]) - .build(); + let blkio = LinuxBlockIoBuilder::default() + .throttle_write_iops_device(vec![LinuxThrottleDeviceBuilder::default() + .major(8) + .minor(0) + .rate(102400u64) + .build() + .unwrap()]) + .build() + .unwrap(); Blkio::apply(&tmp, &blkio).expect("apply blkio"); let content = fs::read_to_string(throttle) diff --git a/cgroups/src/v1/cpu.rs b/cgroups/src/v1/cpu.rs index 8ffe2ab3..344c5b5c 100644 --- a/cgroups/src/v1/cpu.rs +++ b/cgroups/src/v1/cpu.rs @@ -33,12 +33,12 @@ impl Controller for Cpu { } fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> { - if let Some(cpu) = &controller_opt.resources.cpu { - if cpu.shares.is_some() - || cpu.period.is_some() - || cpu.quota.is_some() - || cpu.realtime_period.is_some() - || cpu.realtime_runtime.is_some() + if let Some(cpu) = &controller_opt.resources.cpu() { + if cpu.shares().is_some() + || cpu.period().is_some() + || cpu.quota().is_some() + || cpu.realtime_period().is_some() + || cpu.realtime_runtime().is_some() { return Some(cpu); } @@ -95,31 +95,31 @@ impl StatsProvider for Cpu { impl Cpu { fn apply(root_path: &Path, cpu: &LinuxCpu) -> Result<()> { - if let Some(cpu_shares) = cpu.shares { + 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 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 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 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 let Some(rt_period) = cpu.realtime_period() { if rt_period != 0 { common::write_cgroup_file(root_path.join(CGROUP_CPU_RT_PERIOD), rt_period)?; } @@ -132,7 +132,8 @@ impl Cpu { #[cfg(test)] mod tests { use super::*; - use crate::test::{create_temp_dir, set_fixture, setup, LinuxCpuBuilder}; + use crate::test::{create_temp_dir, set_fixture, setup}; + use oci_spec::runtime::LinuxCpuBuilder; use std::fs; #[test] @@ -141,7 +142,7 @@ mod tests { 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(); + let cpu = LinuxCpuBuilder::default().shares(2048u64).build().unwrap(); // act Cpu::apply(&tmp, &cpu).expect("apply cpu"); @@ -157,7 +158,7 @@ mod tests { // arrange const QUOTA: i64 = 200000; let (tmp, max) = setup("test_set_quota", CGROUP_CPU_QUOTA); - let cpu = LinuxCpuBuilder::new().with_quota(QUOTA).build(); + let cpu = LinuxCpuBuilder::default().quota(QUOTA).build().unwrap(); // act Cpu::apply(&tmp, &cpu).expect("apply cpu"); @@ -173,7 +174,7 @@ mod tests { // arrange const PERIOD: u64 = 100000; let (tmp, max) = setup("test_set_period", CGROUP_CPU_PERIOD); - let cpu = LinuxCpuBuilder::new().with_period(PERIOD).build(); + let cpu = LinuxCpuBuilder::default().period(PERIOD).build().unwrap(); // act Cpu::apply(&tmp, &cpu).expect("apply cpu"); @@ -189,9 +190,10 @@ mod tests { // 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(); + let cpu = LinuxCpuBuilder::default() + .realtime_runtime(RUNTIME) + .build() + .unwrap(); // act Cpu::apply(&tmp, &cpu).expect("apply cpu"); @@ -207,7 +209,10 @@ mod tests { // 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(); + let cpu = LinuxCpuBuilder::default() + .realtime_period(PERIOD) + .build() + .unwrap(); // act Cpu::apply(&tmp, &cpu).expect("apply cpu"); diff --git a/cgroups/src/v1/cpuset.rs b/cgroups/src/v1/cpuset.rs index 75e0c2bb..7345fe7e 100644 --- a/cgroups/src/v1/cpuset.rs +++ b/cgroups/src/v1/cpuset.rs @@ -39,8 +39,8 @@ impl Controller for CpuSet { } fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> { - if let Some(cpuset) = &controller_opt.resources.cpu { - if cpuset.cpus.is_some() || cpuset.mems.is_some() { + if let Some(cpuset) = &controller_opt.resources.cpu() { + if cpuset.cpus().is_some() || cpuset.mems().is_some() { return Some(cpuset); } } @@ -51,11 +51,11 @@ impl Controller for CpuSet { impl CpuSet { fn apply(cgroup_path: &Path, cpuset: &LinuxCpu) -> Result<()> { - if let Some(cpus) = &cpuset.cpus { + if let Some(cpus) = &cpuset.cpus() { common::write_cgroup_file_str(cgroup_path.join(CGROUP_CPUSET_CPUS), cpus)?; } - if let Some(mems) = &cpuset.mems { + if let Some(mems) = &cpuset.mems() { common::write_cgroup_file_str(cgroup_path.join(CGROUP_CPUSET_MEMS), mems)?; } @@ -93,13 +93,17 @@ mod tests { use std::fs; use super::*; - use crate::test::{setup, LinuxCpuBuilder}; + use crate::test::setup; + use oci_spec::runtime::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(); + let cpuset = LinuxCpuBuilder::default() + .cpus("1-3".to_owned()) + .build() + .unwrap(); // act CpuSet::apply(&tmp, &cpuset).expect("apply cpuset"); @@ -114,7 +118,10 @@ mod tests { 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(); + let cpuset = LinuxCpuBuilder::default() + .mems("1-3".to_owned()) + .build() + .unwrap(); // act CpuSet::apply(&tmp, &cpuset).expect("apply cpuset"); diff --git a/cgroups/src/v1/devices.rs b/cgroups/src/v1/devices.rs index 0f1e23cc..2d58992b 100644 --- a/cgroups/src/v1/devices.rs +++ b/cgroups/src/v1/devices.rs @@ -14,7 +14,7 @@ impl Controller for Devices { fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()> { log::debug!("Apply Devices cgroup config"); - if let Some(devices) = controller_opt.resources.devices.as_ref() { + if let Some(devices) = controller_opt.resources.devices().as_ref() { for d in devices { Self::apply_device(d, cgroup_root)?; } @@ -40,7 +40,7 @@ impl Controller for Devices { impl Devices { fn apply_device(device: &LinuxDeviceCgroup, cgroup_root: &Path) -> Result<()> { - let path = if device.allow { + let path = if device.allow() { cgroup_root.join("devices.allow") } else { cgroup_root.join("devices.deny") @@ -56,7 +56,7 @@ mod tests { use super::*; use crate::test::create_temp_dir; use crate::test::set_fixture; - use oci_spec::runtime::{LinuxDeviceCgroup, LinuxDeviceType}; + use oci_spec::runtime::{LinuxDeviceCgroupBuilder, LinuxDeviceType}; use std::fs::read_to_string; #[test] @@ -74,7 +74,7 @@ mod tests { Devices::apply_device(d, &tmp).expect("Apply default device"); println!("Device: {}", d.to_string()); - if d.allow { + if d.allow() { let allowed_content = read_to_string(tmp.join("devices.allow")).expect("read to string"); assert_eq!(allowed_content, d.to_string()); @@ -90,34 +90,34 @@ mod tests { fn test_set_mock_devices() { let tmp = create_temp_dir("test_set_mock_devices").expect("create temp directory for test"); [ - LinuxDeviceCgroup { - allow: true, - typ: Some(LinuxDeviceType::C), - major: Some(10), - minor: None, - access: "rwm".to_string().into(), - }, - LinuxDeviceCgroup { - allow: true, - typ: Some(LinuxDeviceType::A), - major: None, - minor: Some(200), - access: "rwm".to_string().into(), - }, - LinuxDeviceCgroup { - allow: false, - typ: Some(LinuxDeviceType::P), - major: Some(10), - minor: Some(200), - access: "m".to_string().into(), - }, - LinuxDeviceCgroup { - allow: false, - typ: Some(LinuxDeviceType::U), - major: None, - minor: None, - access: "rw".to_string().into(), - }, + LinuxDeviceCgroupBuilder::default() + .allow(true) + .typ(LinuxDeviceType::C) + .major(10) + .access("rwm") + .build() + .unwrap(), + LinuxDeviceCgroupBuilder::default() + .allow(true) + .typ(LinuxDeviceType::A) + .minor(200) + .access("rwm") + .build() + .unwrap(), + LinuxDeviceCgroupBuilder::default() + .allow(false) + .typ(LinuxDeviceType::P) + .major(10) + .minor(200) + .access("m") + .build() + .unwrap(), + LinuxDeviceCgroupBuilder::default() + .allow(false) + .typ(LinuxDeviceType::U) + .access("rw") + .build() + .unwrap(), ] .iter() .for_each(|d| { @@ -126,7 +126,7 @@ mod tests { Devices::apply_device(d, &tmp).expect("Apply default device"); println!("Device: {}", d.to_string()); - if d.allow { + if d.allow() { let allowed_content = read_to_string(tmp.join("devices.allow")).expect("read to string"); assert_eq!(allowed_content, d.to_string()); @@ -144,7 +144,7 @@ mod tests { set_fixture(&tmp, "devices.allow", "").expect("create allowed devices list"); set_fixture(&tmp, "devices.deny", "").expect("create denied devices list"); Devices::apply_device(&device, &tmp).expect("Apply default device"); - if device.allow { + if device.allow() { let allowed_content = read_to_string(tmp.join("devices.allow")).expect("read to string"); allowed_content == device.to_string() @@ -162,7 +162,7 @@ mod tests { set_fixture(&tmp, "devices.allow", "").expect("create allowed devices list"); set_fixture(&tmp, "devices.deny", "").expect("create denied devices list"); Devices::apply_device(device, &tmp).expect("Apply default device"); - if device.allow { + if device.allow() { let allowed_content = read_to_string(tmp.join("devices.allow")).expect("read to string"); allowed_content == device.to_string() diff --git a/cgroups/src/v1/freezer.rs b/cgroups/src/v1/freezer.rs index 2a80a2b3..d266e4c3 100644 --- a/cgroups/src/v1/freezer.rs +++ b/cgroups/src/v1/freezer.rs @@ -127,7 +127,7 @@ mod tests { use crate::common::{FreezerState, CGROUP_PROCS}; use crate::test::{create_temp_dir, set_fixture}; use nix::unistd::Pid; - use oci_spec::runtime::LinuxResources; + use oci_spec::runtime::LinuxResourcesBuilder; #[test] fn test_set_freezer_state() { @@ -176,17 +176,11 @@ mod tests { // set Thawed state. { - let linux_resources = LinuxResources { - devices: Some(vec![]), - memory: None, - cpu: None, - pids: None, - block_io: None, - hugepage_limits: Some(vec![]), - network: None, - rdma: None, - unified: None, - }; + let linux_resources = LinuxResourcesBuilder::default() + .devices(vec![]) + .hugepage_limits(vec![]) + .build() + .unwrap(); let state = FreezerState::Thawed; let controller_opt = ControllerOpt { @@ -209,18 +203,11 @@ mod tests { // set Frozen state. { - let linux_resources = LinuxResources { - devices: Some(vec![]), - memory: None, - cpu: None, - pids: None, - block_io: None, - hugepage_limits: Some(vec![]), - network: None, - rdma: None, - unified: None, - }; - + let linux_resources = LinuxResourcesBuilder::default() + .devices(vec![]) + .hugepage_limits(vec![]) + .build() + .unwrap(); let state = FreezerState::Frozen; let controller_opt = ControllerOpt { @@ -243,17 +230,11 @@ mod tests { // set Undefined state. { - let linux_resources = LinuxResources { - devices: Some(vec![]), - memory: None, - cpu: None, - pids: None, - block_io: None, - hugepage_limits: Some(vec![]), - network: None, - rdma: None, - unified: None, - }; + let linux_resources = LinuxResourcesBuilder::default() + .devices(vec![]) + .hugepage_limits(vec![]) + .build() + .unwrap(); let state = FreezerState::Undefined; diff --git a/cgroups/src/v1/hugetlb.rs b/cgroups/src/v1/hugetlb.rs index e2df4ddd..f611a52a 100644 --- a/cgroups/src/v1/hugetlb.rs +++ b/cgroups/src/v1/hugetlb.rs @@ -29,9 +29,9 @@ impl Controller for HugeTlb { } fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> { - if let Some(hugepage_limits) = controller_opt.resources.hugepage_limits.as_ref() { + if let Some(hugepage_limits) = controller_opt.resources.hugepage_limits().as_ref() { if !hugepage_limits.is_empty() { - return controller_opt.resources.hugepage_limits.as_ref(); + return controller_opt.resources.hugepage_limits().as_ref(); } } @@ -58,7 +58,7 @@ impl StatsProvider for HugeTlb { impl HugeTlb { fn apply(root_path: &Path, hugetlb: &LinuxHugepageLimit) -> Result<()> { let page_size: String = hugetlb - .page_size + .page_size() .chars() .take_while(|c| c.is_digit(10)) .collect(); @@ -68,8 +68,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(()) } @@ -101,7 +101,7 @@ impl HugeTlb { mod tests { use super::*; use crate::test::{create_temp_dir, set_fixture}; - use oci_spec::runtime::LinuxHugepageLimit; + use oci_spec::runtime::LinuxHugepageLimitBuilder; use std::fs::read_to_string; #[test] @@ -110,13 +110,15 @@ mod tests { let tmp = create_temp_dir("test_set_hugetlb").expect("create temp directory for test"); set_fixture(&tmp, page_file_name, "0").expect("Set fixture for 2 MB page size"); - let hugetlb = LinuxHugepageLimit { - page_size: "2MB".to_owned(), - limit: 16384, - }; + let hugetlb = LinuxHugepageLimitBuilder::default() + .page_size("2MB") + .limit(16384) + .build() + .unwrap(); + HugeTlb::apply(&tmp, &hugetlb).expect("apply hugetlb"); let content = read_to_string(tmp.join(page_file_name)).expect("Read hugetlb file content"); - assert_eq!(hugetlb.limit.to_string(), content); + assert_eq!(hugetlb.limit().to_string(), content); } #[test] @@ -124,10 +126,11 @@ mod tests { let tmp = create_temp_dir("test_set_hugetlb_with_invalid_page_size") .expect("create temp directory for test"); - let hugetlb = LinuxHugepageLimit { - page_size: "3MB".to_owned(), - limit: 16384, - }; + let hugetlb = LinuxHugepageLimitBuilder::default() + .page_size("3MB") + .limit(16384) + .build() + .unwrap(); let result = HugeTlb::apply(&tmp, &hugetlb); assert!( @@ -138,14 +141,14 @@ mod tests { quickcheck! { fn property_test_set_hugetlb(hugetlb: LinuxHugepageLimit) -> bool { - let page_file_name = format!("hugetlb.{:?}.limit_in_bytes", hugetlb.page_size); + let page_file_name = format!("hugetlb.{:?}.limit_in_bytes", hugetlb.page_size()); let tmp = create_temp_dir("property_test_set_hugetlb").expect("create temp directory for test"); set_fixture(&tmp, &page_file_name, "0").expect("Set fixture for page size"); let result = HugeTlb::apply(&tmp, &hugetlb); let page_size: String = hugetlb - .page_size + .page_size() .chars() .take_while(|c| c.is_digit(10)) .collect(); @@ -154,7 +157,7 @@ mod tests { if HugeTlb::is_power_of_two(page_size) && page_size != 1 { let content = read_to_string(tmp.join(page_file_name)).expect("Read hugetlb file content"); - hugetlb.limit.to_string() == content + hugetlb.limit().to_string() == content } else { result.is_err() } diff --git a/cgroups/src/v1/manager.rs b/cgroups/src/v1/manager.rs index ea3b41f2..98975cba 100644 --- a/cgroups/src/v1/manager.rs +++ b/cgroups/src/v1/manager.rs @@ -72,8 +72,8 @@ impl Manager { CtrlType::CpuSet => CpuSet::needs_to_handle(controller_opt).is_some(), CtrlType::Devices => Devices::needs_to_handle(controller_opt).is_some(), CtrlType::HugeTlb => HugeTlb::needs_to_handle(controller_opt).is_some(), - CtrlType::Memory => Memory::needs_to_handle(controller_opt).is_some(), - CtrlType::Pids => Pids::needs_to_handle(controller_opt).is_some(), + CtrlType::Memory => controller_opt.resources.memory().is_some(), // TODO: Fix Memory::need_to_handle + CtrlType::Pids => controller_opt.resources.pids().is_some(), // TODO: Fix Pids::need_to_handle CtrlType::PerfEvent => PerfEvent::needs_to_handle(controller_opt).is_some(), CtrlType::Blkio => Blkio::needs_to_handle(controller_opt).is_some(), CtrlType::NetworkPriority => { diff --git a/cgroups/src/v1/memory.rs b/cgroups/src/v1/memory.rs index 6d250955..155003f1 100644 --- a/cgroups/src/v1/memory.rs +++ b/cgroups/src/v1/memory.rs @@ -51,8 +51,8 @@ impl Controller for Memory { fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()> { log::debug!("Apply Memory cgroup config"); - if let Some(memory) = Self::needs_to_handle(controller_opt) { - let reservation = memory.reservation.unwrap_or(0); + if let Some(memory) = controller_opt.resources.memory().as_ref() { + let reservation = memory.reservation().unwrap_or(0); Self::apply(memory, cgroup_root)?; @@ -69,7 +69,7 @@ impl Controller for Memory { common::write_cgroup_file(cgroup_root.join(CGROUP_MEMORY_OOM_CONTROL), 1)?; } - if let Some(swappiness) = memory.swappiness { + if let Some(swappiness) = memory.swappiness() { if swappiness <= 100 { common::write_cgroup_file( cgroup_root.join(CGROUP_MEMORY_SWAPPINESS), @@ -87,10 +87,10 @@ impl Controller for Memory { // 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 { + if let Some(kmem) = memory.kernel() { common::write_cgroup_file(cgroup_root.join(CGROUP_KERNEL_MEMORY_LIMIT), kmem)?; } - if let Some(tcp_mem) = memory.kernel_tcp { + if let Some(tcp_mem) = memory.kernel_tcp() { common::write_cgroup_file( cgroup_root.join(CGROUP_KERNEL_TCP_MEMORY_LIMIT), tcp_mem, @@ -101,11 +101,12 @@ impl Controller for Memory { Ok(()) } - fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> { - if let Some(memory) = &controller_opt.resources.memory { - return Some(memory); - } - + fn needs_to_handle<'a>(_controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> { + // TODO: fix compile error + // error[E0515]: cannot return value referencing temporary value + // if let Some(memory) = &controller_opt.resources.memory() { + // return Some(memory); + // } None } } @@ -292,10 +293,10 @@ impl Memory { } fn apply(resource: &LinuxMemory, cgroup_root: &Path) -> Result<()> { - match resource.limit { + match resource.limit() { Some(limit) => { let current_limit = Self::get_memory_limit(cgroup_root)?; - match resource.swap { + match resource.swap() { Some(swap) => { let is_updated = swap == -1 || current_limit < swap; Self::set_memory_and_swap(limit, swap, is_updated, cgroup_root)?; @@ -310,7 +311,7 @@ impl Memory { } } } - None => match resource.swap { + 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)?, }, @@ -324,7 +325,7 @@ mod tests { use super::*; use crate::common::CGROUP_PROCS; use crate::test::{create_temp_dir, set_fixture}; - use oci_spec::runtime::{LinuxMemory, LinuxResources}; + use oci_spec::runtime::{LinuxMemoryBuilder, LinuxResourcesBuilder}; #[test] fn test_set_memory() { @@ -375,17 +376,8 @@ mod tests { // 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, - disable_oom_killer: None, - use_hierarchy: None, - }; - Memory::apply(linux_memory, &tmp).expect("Set memory and swap"); + let linux_memory = LinuxMemoryBuilder::default().limit(limit).build().unwrap(); + 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"); @@ -401,17 +393,12 @@ mod tests { { 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, - disable_oom_killer: None, - use_hierarchy: None, - }; - Memory::apply(linux_memory, &tmp).expect("Set memory and swap"); + let linux_memory = LinuxMemoryBuilder::default() + .limit(limit) + .swap(swap) + .build() + .unwrap(); + 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"); @@ -424,140 +411,130 @@ mod tests { } quickcheck! { - fn property_test_set_memory(linux_memory: LinuxMemory, disable_oom_killer: bool) -> bool { - let tmp = - create_temp_dir("property_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"); - set_fixture(&tmp, CGROUP_MEMORY_SWAP_LIMIT, "0").expect("Set fixure for swap limit"); - set_fixture(&tmp, CGROUP_MEMORY_SWAPPINESS, "0").expect("Set fixure for swappiness"); - set_fixture(&tmp, CGROUP_MEMORY_RESERVATION, "0").expect("Set fixture for memory reservation"); - set_fixture(&tmp, CGROUP_MEMORY_OOM_CONTROL, "0").expect("Set fixture for oom control"); - set_fixture(&tmp, CGROUP_KERNEL_MEMORY_LIMIT, "0").expect("Set fixture for kernel memory limit"); - set_fixture(&tmp, CGROUP_KERNEL_TCP_MEMORY_LIMIT, "0").expect("Set fixture for kernel tcp memory limit"); - set_fixture(&tmp, CGROUP_PROCS, "").expect("set fixture for proc file"); + fn property_test_set_memory(linux_memory: LinuxMemory, disable_oom_killer: bool) -> bool { + let tmp = + create_temp_dir("property_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"); + set_fixture(&tmp, CGROUP_MEMORY_SWAP_LIMIT, "0").expect("Set fixure for swap limit"); + set_fixture(&tmp, CGROUP_MEMORY_SWAPPINESS, "0").expect("Set fixure for swappiness"); + set_fixture(&tmp, CGROUP_MEMORY_RESERVATION, "0").expect("Set fixture for memory reservation"); + set_fixture(&tmp, CGROUP_MEMORY_OOM_CONTROL, "0").expect("Set fixture for oom control"); + set_fixture(&tmp, CGROUP_KERNEL_MEMORY_LIMIT, "0").expect("Set fixture for kernel memory limit"); + set_fixture(&tmp, CGROUP_KERNEL_TCP_MEMORY_LIMIT, "0").expect("Set fixture for kernel tcp memory limit"); + set_fixture(&tmp, CGROUP_PROCS, "").expect("set fixture for proc file"); - // clone to avoid use of moved value later on - let memory_limits = linux_memory; + // clone to avoid use of moved value later on + let memory_limits = linux_memory; - let linux_resources = LinuxResources { - devices: Some(vec![]), - memory: Some(linux_memory), - cpu: None, - pids: None, - block_io: None, - hugepage_limits: Some(vec![]), - network: None, - rdma: None, - unified: None, - }; + let linux_resources = LinuxResourcesBuilder::default().devices(vec![]).memory(linux_memory).hugepage_limits(vec![]).build().unwrap(); - let controller_opt = ControllerOpt { - resources: &linux_resources, - disable_oom_killer, - oom_score_adj: None, - freezer_state: None, - }; + let controller_opt = ControllerOpt { + resources: &linux_resources, + disable_oom_killer, + oom_score_adj: None, + freezer_state: None, + }; - let result = ::apply(&controller_opt, &tmp); + let result = ::apply(&controller_opt, &tmp); - if result.is_err() { - if let Some(swappiness) = memory_limits.swappiness { - // error is expected if swappiness is greater than 100 - if swappiness > 100 { - return true; - } - } else { - // useful for debugging - println!("Some unexpected error: {:?}", result.unwrap_err()); - // any other error should be considered unexpected - return false; - } - } - - // check memory reservation - let reservation_content = std::fs::read_to_string(tmp.join(CGROUP_MEMORY_RESERVATION)).expect("read memory reservation"); - let reservation_check = match memory_limits.reservation { - Some(reservation) => { - reservation_content == reservation.to_string() - } - None => reservation_content == "0", - }; - - // check kernel memory limit - let kernel_content = std::fs::read_to_string(tmp.join(CGROUP_KERNEL_MEMORY_LIMIT)).expect("read kernel memory limit"); - let kernel_check = match memory_limits.kernel { - Some(kernel) => { - kernel_content == kernel.to_string() - } - None => kernel_content == "0", - }; - - // check kernel tcp memory limit - let kernel_tcp_content = std::fs::read_to_string(tmp.join(CGROUP_KERNEL_TCP_MEMORY_LIMIT)).expect("read kernel tcp memory limit"); - let kernel_tcp_check = match memory_limits.kernel_tcp { - Some(kernel_tcp) => { - kernel_tcp_content == kernel_tcp.to_string() - } - None => kernel_tcp_content == "0", - }; - - // check swappiness - let swappiness_content = std::fs::read_to_string(tmp.join(CGROUP_MEMORY_SWAPPINESS)).expect("read swappiness"); - let swappiness_check = match memory_limits.swappiness { - Some(swappiness) if swappiness <= 100 => { - swappiness_content == swappiness.to_string() - } - None => swappiness_content == "0", - // everything else is a failure - _ => false, - }; - - // check limit and swap - let limit_content = std::fs::read_to_string(tmp.join(CGROUP_MEMORY_LIMIT)).expect("read memory limit"); - let swap_content = std::fs::read_to_string(tmp.join(CGROUP_MEMORY_SWAP_LIMIT)).expect("read swap memory limit"); - let limit_swap_check = match memory_limits.limit { - Some(limit) => { - match memory_limits.swap { - Some(swap) => { - limit_content == limit.to_string() - && swap_content == swap.to_string() + if result.is_err() { + if let Some(swappiness) = memory_limits.swappiness() { + // error is expected if swappiness is greater than 100 + if swappiness > 100 { + return true; } - None => { - if limit == -1 { + } else { + // useful for debugging + println!("Some unexpected error: {:?}", result.unwrap_err()); + // any other error should be considered unexpected + return false; + } + } + + // check memory reservation + let reservation_content = std::fs::read_to_string(tmp.join(CGROUP_MEMORY_RESERVATION)).expect("read memory reservation"); + let reservation_check = match memory_limits.reservation() { + Some(reservation) => { + reservation_content == reservation.to_string() + } + None => reservation_content == "0", + }; + + // check kernel memory limit + let kernel_content = std::fs::read_to_string(tmp.join(CGROUP_KERNEL_MEMORY_LIMIT)).expect("read kernel memory limit"); + let kernel_check = match memory_limits.kernel() { + Some(kernel) => { + kernel_content == kernel.to_string() + } + None => kernel_content == "0", + }; + + // check kernel tcp memory limit + let kernel_tcp_content = std::fs::read_to_string(tmp.join(CGROUP_KERNEL_TCP_MEMORY_LIMIT)).expect("read kernel tcp memory limit"); + let kernel_tcp_check = match memory_limits.kernel_tcp() { + Some(kernel_tcp) => { + kernel_tcp_content == kernel_tcp.to_string() + } + None => kernel_tcp_content == "0", + }; + + // check swappiness + let swappiness_content = std::fs::read_to_string(tmp.join(CGROUP_MEMORY_SWAPPINESS)).expect("read swappiness"); + let swappiness_check = match memory_limits.swappiness() { + Some(swappiness) if swappiness <= 100 => { + swappiness_content == swappiness.to_string() + } + None => swappiness_content == "0", + // everything else is a failure + _ => false, + }; + + // check limit and swap + let limit_content = std::fs::read_to_string(tmp.join(CGROUP_MEMORY_LIMIT)).expect("read memory limit"); + let swap_content = std::fs::read_to_string(tmp.join(CGROUP_MEMORY_SWAP_LIMIT)).expect("read swap memory limit"); + let limit_swap_check = match memory_limits.limit() { + Some(limit) => { + match memory_limits.swap() { + Some(swap) => { limit_content == limit.to_string() - && swap_content == "-1" - } else { - limit_content == limit.to_string() - && swap_content == "0" + && swap_content == swap.to_string() + } + None => { + if limit == -1 { + limit_content == limit.to_string() + && swap_content == "-1" + } else { + limit_content == limit.to_string() + && swap_content == "0" + } } } } - } - None => { - match memory_limits.swap { - Some(swap) => { - limit_content == "0" - && swap_content == swap.to_string() + None => { + match memory_limits.swap() { + Some(swap) => { + limit_content == "0" + && swap_content == swap.to_string() + } + None => limit_content == "0" && swap_content == "0" } - None => limit_content == "0" && swap_content == "0" } - } - }; + }; - // useful for debugging - println!("reservation_check: {:?}", reservation_check); - println!("kernel_check: {:?}", kernel_check); - println!("kernel_tcp_check: {:?}", kernel_tcp_check); - println!("swappiness_check: {:?}", swappiness_check); - println!("limit_swap_check: {:?}", limit_swap_check); + // useful for debugging + println!("reservation_check: {:?}", reservation_check); + println!("kernel_check: {:?}", kernel_check); + println!("kernel_tcp_check: {:?}", kernel_tcp_check); + println!("swappiness_check: {:?}", swappiness_check); + println!("limit_swap_check: {:?}", limit_swap_check); - // combine all the checks - reservation_check && kernel_check && kernel_tcp_check && swappiness_check && limit_swap_check - } + // combine all the checks + reservation_check && kernel_check && kernel_tcp_check && swappiness_check && limit_swap_check + } } #[test] diff --git a/cgroups/src/v1/network_classifier.rs b/cgroups/src/v1/network_classifier.rs index d60f7b9f..5c016308 100644 --- a/cgroups/src/v1/network_classifier.rs +++ b/cgroups/src/v1/network_classifier.rs @@ -23,7 +23,7 @@ impl Controller for NetworkClassifier { } fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> { - if let Some(network) = controller_opt.resources.network.as_ref() { + if let Some(network) = controller_opt.resources.network().as_ref() { return Some(network); } @@ -33,7 +33,7 @@ impl Controller for NetworkClassifier { impl NetworkClassifier { fn apply(root_path: &Path, network: &LinuxNetwork) -> Result<()> { - if let Some(class_id) = network.class_id { + if let Some(class_id) = network.class_id() { common::write_cgroup_file(root_path.join("net_cls.classid"), class_id)?; } @@ -45,6 +45,7 @@ impl NetworkClassifier { mod tests { use super::*; use crate::test::{create_temp_dir, set_fixture}; + use oci_spec::runtime::LinuxNetworkBuilder; #[test] fn test_apply_network_classifier() { @@ -52,11 +53,12 @@ mod tests { .expect("create temp directory for test"); set_fixture(&tmp, "net_cls.classid", "0").expect("set fixture for classID"); - let id = 0x100001; - let network = LinuxNetwork { - class_id: Some(id), - priorities: Some(vec![]), - }; + let id = 0x100001u32; + let network = LinuxNetworkBuilder::default() + .class_id(id) + .priorities(vec![]) + .build() + .unwrap(); NetworkClassifier::apply(&tmp, &network).expect("apply network classID"); diff --git a/cgroups/src/v1/network_priority.rs b/cgroups/src/v1/network_priority.rs index 09c20242..eee0fbe6 100644 --- a/cgroups/src/v1/network_priority.rs +++ b/cgroups/src/v1/network_priority.rs @@ -23,7 +23,7 @@ impl Controller for NetworkPriority { } fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> { - if let Some(network) = &controller_opt.resources.network { + if let Some(network) = &controller_opt.resources.network() { return Some(network); } @@ -33,7 +33,7 @@ impl Controller for NetworkPriority { impl NetworkPriority { fn apply(root_path: &Path, network: &LinuxNetwork) -> Result<()> { - if let Some(ni_priorities) = network.priorities.as_ref() { + if let Some(ni_priorities) = network.priorities().as_ref() { let priorities: String = ni_priorities.iter().map(|p| p.to_string()).collect(); common::write_cgroup_file_str(root_path.join("net_prio.ifpriomap"), priorities.trim())?; } @@ -46,7 +46,7 @@ impl NetworkPriority { mod tests { use super::*; use crate::test::{create_temp_dir, set_fixture}; - use oci_spec::runtime::LinuxInterfacePriority; + use oci_spec::runtime::{LinuxInterfacePriorityBuilder, LinuxNetworkBuilder}; #[test] fn test_apply_network_priorites() { @@ -54,20 +54,22 @@ mod tests { .expect("create temp directory for test"); set_fixture(&tmp, "net_prio.ifpriomap", "").expect("set fixture for priority map"); let priorities = vec![ - LinuxInterfacePriority { - name: "a".to_owned(), - priority: 1, - }, - LinuxInterfacePriority { - name: "b".to_owned(), - priority: 2, - }, + LinuxInterfacePriorityBuilder::default() + .name("a") + .priority(1u32) + .build() + .unwrap(), + LinuxInterfacePriorityBuilder::default() + .name("b") + .priority(2u32) + .build() + .unwrap(), ]; let priorities_string = priorities.iter().map(|p| p.to_string()).collect::(); - let network = LinuxNetwork { - class_id: None, - priorities: priorities.into(), - }; + let network = LinuxNetworkBuilder::default() + .priorities(priorities) + .build() + .unwrap(); NetworkPriority::apply(&tmp, &network).expect("apply network priorities"); diff --git a/cgroups/src/v1/pids.rs b/cgroups/src/v1/pids.rs index 82844e1e..0b8d0ac8 100644 --- a/cgroups/src/v1/pids.rs +++ b/cgroups/src/v1/pids.rs @@ -20,17 +20,19 @@ impl Controller for Pids { fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()> { log::debug!("Apply pids cgroup config"); - if let Some(pids) = &controller_opt.resources.pids { + if let Some(pids) = &controller_opt.resources.pids() { Self::apply(cgroup_root, pids).context("failed to apply pids resource restrictions")?; } Ok(()) } - fn needs_to_handle<'a>(controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> { - if let Some(pids) = &controller_opt.resources.pids { - return Some(pids); - } + fn needs_to_handle<'a>(_controller_opt: &'a ControllerOpt) -> Option<&'a Self::Resource> { + // TODO: fix compile error + // error[E0515]: cannot return value referencing temporary value + // if let Some(pids) = &controller_opt.resources.pids() { + // return Some(pids); + // } None } @@ -46,8 +48,8 @@ impl StatsProvider for Pids { impl Pids { fn apply(root_path: &Path, pids: &LinuxPids) -> Result<()> { - let limit = if pids.limit > 0 { - pids.limit.to_string() + let limit = if pids.limit() > 0 { + pids.limit().to_string() } else { "max".to_string() }; @@ -61,7 +63,7 @@ impl Pids { mod tests { use super::*; use crate::test::{create_temp_dir, set_fixture}; - use oci_spec::runtime::LinuxPids; + use oci_spec::runtime::LinuxPidsBuilder; // Contains the current number of active pids const CGROUP_PIDS_CURRENT: &str = "pids.current"; @@ -71,12 +73,12 @@ mod tests { let tmp = create_temp_dir("test_set_pids").expect("create temp directory for test"); set_fixture(&tmp, CGROUP_PIDS_MAX, "1000").expect("Set fixture for 1000 pids"); - let pids = LinuxPids { limit: 1000 }; + let pids = LinuxPidsBuilder::default().limit(1000).build().unwrap(); Pids::apply(&tmp, &pids).expect("apply pids"); let content = std::fs::read_to_string(tmp.join(CGROUP_PIDS_MAX)).expect("Read pids contents"); - assert_eq!(pids.limit.to_string(), content); + assert_eq!(pids.limit().to_string(), content); } #[test] @@ -84,7 +86,7 @@ mod tests { let tmp = create_temp_dir("test_set_pids_max").expect("create temp directory for test"); set_fixture(&tmp, CGROUP_PIDS_MAX, "0").expect("set fixture for 0 pids"); - let pids = LinuxPids { limit: 0 }; + let pids = LinuxPidsBuilder::default().limit(0).build().unwrap(); Pids::apply(&tmp, &pids).expect("apply pids"); diff --git a/cgroups/src/v2/cpu.rs b/cgroups/src/v2/cpu.rs index 277d5cfb..97288e0a 100644 --- a/cgroups/src/v2/cpu.rs +++ b/cgroups/src/v2/cpu.rs @@ -21,7 +21,7 @@ pub struct Cpu {} impl Controller for Cpu { fn apply(controller_opt: &ControllerOpt, path: &Path) -> Result<()> { - if let Some(cpu) = &controller_opt.resources.cpu { + if let Some(cpu) = &controller_opt.resources.cpu() { Self::apply(path, cpu).context("failed to apply cpu resource restrictions")?; } @@ -61,7 +61,7 @@ impl Cpu { bail!("realtime is not supported on cgroup v2 yet"); } - if let Some(mut shares) = cpu.shares { + if let Some(mut shares) = cpu.shares() { shares = Self::convert_shares_to_cgroup2(shares); if shares != 0 { // will result in Erno 34 (numerical result out of range) otherwise @@ -71,14 +71,14 @@ impl Cpu { // if quota is unrestricted set to 'max' let mut quota_string = UNRESTRICTED_QUOTA.to_owned(); - if let Some(quota) = cpu.quota { + if let Some(quota) = cpu.quota() { if quota > 0 { quota_string = quota.to_string(); } } let mut period_string: String = DEFAULT_PERIOD.to_owned(); - if let Some(period) = cpu.period { + if let Some(period) = cpu.period() { if period > 0 { period_string = period.to_string(); } @@ -103,11 +103,11 @@ impl Cpu { } fn is_realtime_requested(cpu: &LinuxCpu) -> bool { - if cpu.realtime_period.is_some() { + if cpu.realtime_period().is_some() { return true; } - if cpu.realtime_runtime.is_some() { + if cpu.realtime_runtime().is_some() { return true; } @@ -118,7 +118,8 @@ impl Cpu { #[cfg(test)] mod tests { use super::*; - use crate::test::{create_temp_dir, set_fixture, setup, LinuxCpuBuilder}; + use crate::test::{create_temp_dir, set_fixture, setup}; + use oci_spec::runtime::LinuxCpuBuilder; use std::fs; #[test] @@ -127,7 +128,7 @@ mod tests { let (tmp, weight) = setup("test_set_shares", CGROUP_CPU_WEIGHT); let _ = set_fixture(&tmp, CGROUP_CPU_MAX, "") .unwrap_or_else(|_| panic!("set test fixture for {}", CGROUP_CPU_MAX)); - let cpu = LinuxCpuBuilder::new().with_shares(22000).build(); + let cpu = LinuxCpuBuilder::default().shares(22000u64).build().unwrap(); // act Cpu::apply(&tmp, &cpu).expect("apply cpu"); @@ -143,7 +144,7 @@ mod tests { // arrange const QUOTA: i64 = 200000; let (tmp, max) = setup("test_set_positive_quota", CGROUP_CPU_MAX); - let cpu = LinuxCpuBuilder::new().with_quota(QUOTA).build(); + let cpu = LinuxCpuBuilder::default().quota(QUOTA).build().unwrap(); // act Cpu::apply(&tmp, &cpu).expect("apply cpu"); @@ -158,7 +159,7 @@ mod tests { fn test_set_zero_quota() { // arrange let (tmp, max) = setup("test_set_zero_quota", CGROUP_CPU_MAX); - let cpu = LinuxCpuBuilder::new().with_quota(0).build(); + let cpu = LinuxCpuBuilder::default().quota(0).build().unwrap(); // act Cpu::apply(&tmp, &cpu).expect("apply cpu"); @@ -177,7 +178,7 @@ mod tests { // arrange const PERIOD: u64 = 100000; let (tmp, max) = setup("test_set_positive_period", CGROUP_CPU_MAX); - let cpu = LinuxCpuBuilder::new().with_period(PERIOD).build(); + let cpu = LinuxCpuBuilder::default().period(PERIOD).build().unwrap(); // act Cpu::apply(&tmp, &cpu).expect("apply cpu"); @@ -192,7 +193,7 @@ mod tests { fn test_set_zero_period() { // arrange let (tmp, max) = setup("test_set_zero_period", CGROUP_CPU_MAX); - let cpu = LinuxCpuBuilder::new().with_period(0).build(); + let cpu = LinuxCpuBuilder::default().period(0u64).build().unwrap(); // act Cpu::apply(&tmp, &cpu).expect("apply cpu"); @@ -212,10 +213,11 @@ mod tests { const QUOTA: i64 = 200000; const PERIOD: u64 = 100000; let (tmp, max) = setup("test_set_quota_and_period", CGROUP_CPU_MAX); - let cpu = LinuxCpuBuilder::new() - .with_quota(QUOTA) - .with_period(PERIOD) - .build(); + let cpu = LinuxCpuBuilder::default() + .quota(QUOTA) + .period(PERIOD) + .build() + .unwrap(); // act Cpu::apply(&tmp, &cpu).expect("apply cpu"); @@ -231,7 +233,10 @@ mod tests { // arrange let tmp = create_temp_dir("test_realtime_runtime_not_supported") .expect("create temp directory for test"); - let cpu = LinuxCpuBuilder::new().with_realtime_runtime(5).build(); + let cpu = LinuxCpuBuilder::default() + .realtime_runtime(5) + .build() + .unwrap(); // act let result = Cpu::apply(&tmp, &cpu); @@ -248,7 +253,10 @@ mod tests { // arrange let tmp = create_temp_dir("test_realtime_period_not_supported") .expect("create temp directory for test"); - let cpu = LinuxCpuBuilder::new().with_realtime_period(5).build(); + let cpu = LinuxCpuBuilder::default() + .realtime_period(5u64) + .build() + .unwrap(); // act let result = Cpu::apply(&tmp, &cpu); diff --git a/cgroups/src/v2/cpuset.rs b/cgroups/src/v2/cpuset.rs index 80797cd6..c42023c6 100644 --- a/cgroups/src/v2/cpuset.rs +++ b/cgroups/src/v2/cpuset.rs @@ -13,7 +13,7 @@ pub struct CpuSet {} impl Controller for CpuSet { fn apply(controller_opt: &ControllerOpt, cgroup_path: &Path) -> Result<()> { - if let Some(cpuset) = &controller_opt.resources.cpu { + if let Some(cpuset) = &controller_opt.resources.cpu() { Self::apply(cgroup_path, cpuset) .context("failed to apply cpuset resource restrictions")?; } @@ -24,11 +24,11 @@ impl Controller for CpuSet { impl CpuSet { fn apply(path: &Path, cpuset: &LinuxCpu) -> Result<()> { - if let Some(cpus) = &cpuset.cpus { + if let Some(cpus) = &cpuset.cpus() { common::write_cgroup_file_str(path.join(CGROUP_CPUSET_CPUS), cpus)?; } - if let Some(mems) = &cpuset.mems { + if let Some(mems) = &cpuset.mems() { common::write_cgroup_file_str(path.join(CGROUP_CPUSET_MEMS), mems)?; } @@ -41,13 +41,17 @@ mod tests { use std::fs; use super::*; - use crate::test::{setup, LinuxCpuBuilder}; + use crate::test::setup; + use oci_spec::runtime::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(); + let cpuset = LinuxCpuBuilder::default() + .cpus("1-3".to_owned()) + .build() + .unwrap(); // act CpuSet::apply(&tmp, &cpuset).expect("apply cpuset"); @@ -62,7 +66,10 @@ mod tests { 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(); + let cpuset = LinuxCpuBuilder::default() + .mems("1-3".to_owned()) + .build() + .unwrap(); // act CpuSet::apply(&tmp, &cpuset).expect("apply cpuset"); diff --git a/cgroups/src/v2/devices/controller.rs b/cgroups/src/v2/devices/controller.rs index 306a008e..01e2d5a0 100644 --- a/cgroups/src/v2/devices/controller.rs +++ b/cgroups/src/v2/devices/controller.rs @@ -21,7 +21,7 @@ impl Controller for Devices { return Ok(()); #[cfg(feature = "cgroupsv2_devices")] - return Self::apply_devices(cgroup_root, &controller_opt.resources.devices); + return Self::apply_devices(cgroup_root, controller_opt.resources.devices()); } } diff --git a/cgroups/src/v2/devices/emulator.rs b/cgroups/src/v2/devices/emulator.rs index 1bd58ab0..62eb79ba 100644 --- a/cgroups/src/v2/devices/emulator.rs +++ b/cgroups/src/v2/devices/emulator.rs @@ -38,14 +38,14 @@ impl Emulator { pub fn add_rule(&mut self, rule: &LinuxDeviceCgroup) -> Result<()> { // special case, switch to blacklist or whitelist and clear all existing rules // NOTE: we ignore other fields when type='a', this is same as cgroup v1 and runc - if rule.typ.unwrap_or_default() == LinuxDeviceType::A { - self.default_allow = rule.allow; + if rule.typ().unwrap_or_default() == LinuxDeviceType::A { + self.default_allow = rule.allow(); self.rules.clear(); return Ok(()); } // empty access match nothing, just discard this rule - if rule.access.is_none() { + if rule.access().is_none() { return Ok(()); } diff --git a/cgroups/src/v2/devices/program.rs b/cgroups/src/v2/devices/program.rs index bc17801a..e64b872a 100644 --- a/cgroups/src/v2/devices/program.rs +++ b/cgroups/src/v2/devices/program.rs @@ -90,15 +90,15 @@ impl Program { } fn add_rule(&mut self, rule: &LinuxDeviceCgroup) -> Result<()> { - let dev_type = bpf_dev_type(rule.typ.unwrap_or_default())?; - let access = bpf_access(rule.access.clone().unwrap_or_default())?; + let dev_type = bpf_dev_type(rule.typ().unwrap_or_default())?; + let access = bpf_access(rule.access().clone().unwrap_or_default())?; let has_access = access != (libbpf_sys::BPF_DEVCG_ACC_READ | libbpf_sys::BPF_DEVCG_ACC_WRITE | libbpf_sys::BPF_DEVCG_ACC_MKNOD); - let has_major = rule.major.is_some() && rule.major.unwrap() >= 0; - let has_minor = rule.minor.is_some() && rule.minor.unwrap() >= 0; + let has_major = rule.major().is_some() && rule.major().unwrap() >= 0; + let has_minor = rule.minor().is_some() && rule.minor().unwrap() >= 0; // count of instructions of this rule let mut instruction_count = 1; // execute dev_type @@ -151,7 +151,7 @@ impl Program { self.prog .jump_conditional(Cond::NotEquals, Source::Imm) .set_dst(4) - .set_imm(rule.major.unwrap() as i32) + .set_imm(rule.major().unwrap() as i32) .set_off(next_rule_offset) .push(); } @@ -162,7 +162,7 @@ impl Program { self.prog .jump_conditional(Cond::NotEquals, Source::Imm) .set_dst(5) - .set_imm(rule.minor.unwrap() as i32) + .set_imm(rule.minor().unwrap() as i32) .set_off(next_rule_offset) .push(); } @@ -171,7 +171,7 @@ impl Program { self.prog .mov(Source::Imm, RbpfArch::X32) .set_dst(0) - .set_imm(rule.allow as i32) + .set_imm(rule.allow() as i32) .push(); self.prog.exit().push(); @@ -248,6 +248,7 @@ fn bpf_cgroup_dev_ctx( #[cfg(test)] mod tests { use super::*; + use oci_spec::runtime::LinuxDeviceCgroupBuilder; fn build_bpf_program(rules: &Option>) -> Result { let mut em = crate::v2::devices::emulator::Emulator::with_default_allow(false); @@ -260,13 +261,14 @@ mod tests { #[test] fn test_devices_allow_single() { - let rules = vec![LinuxDeviceCgroup { - allow: true, - typ: Some(LinuxDeviceType::C), - major: Some(10), - minor: Some(20), - access: "r".to_string().into(), - }]; + let rules = vec![LinuxDeviceCgroupBuilder::default() + .allow(true) + .typ(LinuxDeviceType::C) + .major(10) + .minor(20) + .access("r") + .build() + .unwrap()]; let prog = build_bpf_program(&Some(rules)).unwrap(); let ty_list = vec![ @@ -333,13 +335,11 @@ mod tests { #[test] fn test_devices_allow_all() { - let rules = vec![LinuxDeviceCgroup { - allow: true, - typ: Some(LinuxDeviceType::A), - major: None, - minor: None, - access: None, - }]; + let rules = vec![LinuxDeviceCgroupBuilder::default() + .allow(true) + .typ(LinuxDeviceType::A) + .build() + .unwrap()]; let prog = build_bpf_program(&Some(rules)).unwrap(); let ty_list = vec![ @@ -371,13 +371,13 @@ mod tests { #[test] fn test_devices_allow_wildcard() { - let rules = vec![LinuxDeviceCgroup { - allow: true, - typ: Some(LinuxDeviceType::C), - major: None, - minor: Some(20), - access: "r".to_string().into(), - }]; + let rules = vec![LinuxDeviceCgroupBuilder::default() + .allow(true) + .typ(LinuxDeviceType::C) + .minor(20) + .access("r") + .build() + .unwrap()]; let prog = build_bpf_program(&Some(rules)).unwrap(); let ty_list = vec![ @@ -414,20 +414,20 @@ mod tests { #[test] fn test_devices_allow_and_deny() { let rules = vec![ - LinuxDeviceCgroup { - allow: true, - typ: Some(LinuxDeviceType::C), - major: None, - minor: Some(20), - access: "rw".to_string().into(), - }, - LinuxDeviceCgroup { - allow: false, - typ: Some(LinuxDeviceType::C), - major: Some(10), - minor: None, - access: "r".to_string().into(), - }, + LinuxDeviceCgroupBuilder::default() + .allow(true) + .typ(LinuxDeviceType::C) + .minor(20) + .access("rw") + .build() + .unwrap(), + LinuxDeviceCgroupBuilder::default() + .allow(false) + .typ(LinuxDeviceType::C) + .major(10) + .access("r") + .build() + .unwrap(), ]; let prog = build_bpf_program(&Some(rules)).unwrap(); diff --git a/cgroups/src/v2/hugetlb.rs b/cgroups/src/v2/hugetlb.rs index 9ac0c7d1..659bb8a2 100644 --- a/cgroups/src/v2/hugetlb.rs +++ b/cgroups/src/v2/hugetlb.rs @@ -14,7 +14,7 @@ pub struct HugeTlb {} impl Controller for HugeTlb { fn apply(controller_opt: &ControllerOpt, cgroup_root: &std::path::Path) -> Result<()> { log::debug!("Apply hugetlb cgroup v2 config"); - if let Some(hugepage_limits) = controller_opt.resources.hugepage_limits.as_ref() { + if let Some(hugepage_limits) = controller_opt.resources.hugepage_limits().as_ref() { for hugetlb in hugepage_limits { Self::apply(cgroup_root, hugetlb) .context("failed to apply hugetlb resource restrictions")? @@ -45,7 +45,7 @@ impl StatsProvider for HugeTlb { impl HugeTlb { fn apply(root_path: &Path, hugetlb: &LinuxHugepageLimit) -> Result<()> { let page_size: String = hugetlb - .page_size + .page_size() .chars() .take_while(|c| c.is_digit(10)) .collect(); @@ -55,8 +55,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(()) } @@ -88,7 +88,7 @@ impl HugeTlb { mod tests { use super::*; use crate::test::{create_temp_dir, set_fixture}; - use oci_spec::runtime::LinuxHugepageLimit; + use oci_spec::runtime::LinuxHugepageLimitBuilder; use std::fs::read_to_string; #[test] @@ -97,13 +97,14 @@ mod tests { let tmp = create_temp_dir("test_set_hugetlbv2").expect("create temp directory for test"); set_fixture(&tmp, page_file_name, "0").expect("Set fixture for 2 MB page size"); - let hugetlb = LinuxHugepageLimit { - page_size: "2MB".to_owned(), - limit: 16384, - }; + let hugetlb = LinuxHugepageLimitBuilder::default() + .page_size("2MB") + .limit(16384) + .build() + .unwrap(); HugeTlb::apply(&tmp, &hugetlb).expect("apply hugetlb"); let content = read_to_string(tmp.join(page_file_name)).expect("Read hugetlb file content"); - assert_eq!(hugetlb.limit.to_string(), content); + assert_eq!(hugetlb.limit().to_string(), content); } #[test] @@ -111,10 +112,11 @@ mod tests { let tmp = create_temp_dir("test_set_hugetlbv2_with_invalid_page_size") .expect("create temp directory for test"); - let hugetlb = LinuxHugepageLimit { - page_size: "3MB".to_owned(), - limit: 16384, - }; + let hugetlb = LinuxHugepageLimitBuilder::default() + .page_size("3MB") + .limit(16384) + .build() + .unwrap(); let result = HugeTlb::apply(&tmp, &hugetlb); assert!( @@ -125,13 +127,13 @@ mod tests { quickcheck! { fn property_test_set_hugetlb(hugetlb: LinuxHugepageLimit) -> bool { - let page_file_name = format!("hugetlb.{:?}.limit_in_bytes", hugetlb.page_size); + let page_file_name = format!("hugetlb.{:?}.limit_in_bytes", hugetlb.page_size()); let tmp = create_temp_dir("property_test_set_hugetlbv2").expect("create temp directory for test"); set_fixture(&tmp, &page_file_name, "0").expect("Set fixture for page size"); let result = HugeTlb::apply(&tmp, &hugetlb); let page_size: String = hugetlb - .page_size + .page_size() .chars() .take_while(|c| c.is_digit(10)) .collect(); @@ -140,7 +142,7 @@ mod tests { if HugeTlb::is_power_of_two(page_size) && page_size != 1 { let content = read_to_string(tmp.join(page_file_name)).expect("Read hugetlb file content"); - hugetlb.limit.to_string() == content + hugetlb.limit().to_string() == content } else { result.is_err() } diff --git a/cgroups/src/v2/io.rs b/cgroups/src/v2/io.rs index c714f29d..0cc48e5e 100644 --- a/cgroups/src/v2/io.rs +++ b/cgroups/src/v2/io.rs @@ -19,7 +19,7 @@ pub struct Io {} impl Controller for Io { fn apply(controller_opt: &ControllerOpt, cgroup_root: &Path) -> Result<()> { log::debug!("Apply io cgroup v2 config"); - if let Some(io) = &controller_opt.resources.block_io { + if let Some(io) = &controller_opt.resources.block_io() { Self::apply(cgroup_root, io).context("failed to apply io resource restrictions")?; } Ok(()) @@ -85,20 +85,20 @@ impl Io { // linux kernel doc: https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html#io fn apply(root_path: &Path, blkio: &LinuxBlockIo) -> Result<()> { - if let Some(weight_device) = blkio.weight_device.as_ref() { + if let Some(weight_device) = blkio.weight_device().as_ref() { for wd in weight_device { common::write_cgroup_file( root_path.join(CGROUP_BFQ_IO_WEIGHT), - &format!("{}:{} {}", wd.major, wd.minor, wd.weight.unwrap()), + &format!("{}:{} {}", wd.major(), wd.minor(), wd.weight().unwrap()), )?; } } - if let Some(leaf_weight) = blkio.leaf_weight { + if let Some(leaf_weight) = blkio.leaf_weight() { if leaf_weight > 0 { bail!("cannot set leaf_weight with cgroupv2"); } } - if let Some(io_weight) = blkio.weight { + if let Some(io_weight) = blkio.weight() { if io_weight > 0 { common::write_cgroup_file( root_path.join(CGROUP_IO_WEIGHT), @@ -107,38 +107,38 @@ impl Io { } } - if let Some(throttle_read_bps_device) = blkio.throttle_read_bps_device.as_ref() { + if let Some(throttle_read_bps_device) = blkio.throttle_read_bps_device().as_ref() { for trbd in throttle_read_bps_device { common::write_cgroup_file( Self::io_max_path(root_path), - &format!("{}:{} rbps={}", trbd.major, trbd.minor, trbd.rate), + &format!("{}:{} rbps={}", trbd.major(), trbd.minor(), trbd.rate()), )?; } } - if let Some(throttle_write_bps_device) = blkio.throttle_write_bps_device.as_ref() { + if let Some(throttle_write_bps_device) = blkio.throttle_write_bps_device().as_ref() { for twbd in throttle_write_bps_device { common::write_cgroup_file( Self::io_max_path(root_path), - format!("{}:{} wbps={}", twbd.major, twbd.minor, twbd.rate), + format!("{}:{} wbps={}", twbd.major(), twbd.minor(), twbd.rate()), )?; } } - if let Some(throttle_read_iops_device) = blkio.throttle_read_iops_device.as_ref() { + if let Some(throttle_read_iops_device) = blkio.throttle_read_iops_device().as_ref() { for trid in throttle_read_iops_device { common::write_cgroup_file( Self::io_max_path(root_path), - format!("{}:{} riops={}", trid.major, trid.minor, trid.rate), + format!("{}:{} riops={}", trid.major(), trid.minor(), trid.rate()), )?; } } - if let Some(throttle_write_iops_device) = blkio.throttle_write_iops_device.as_ref() { + if let Some(throttle_write_iops_device) = blkio.throttle_write_iops_device().as_ref() { for twid in throttle_write_iops_device { common::write_cgroup_file( Self::io_max_path(root_path), - format!("{}:{} wiops={}", twid.major, twid.minor, twid.rate), + format!("{}:{} wiops={}", twid.major(), twid.minor(), twid.rate()), )?; } } @@ -151,70 +151,24 @@ mod test { use super::*; use crate::test::{create_temp_dir, set_fixture, setup}; - use oci_spec::runtime::{LinuxBlockIo, LinuxThrottleDevice, LinuxWeightDevice}; + use oci_spec::runtime::{ + LinuxBlockIoBuilder, LinuxThrottleDeviceBuilder, LinuxWeightDeviceBuilder, + }; use std::fs; - struct BlockIoBuilder { - block_io: LinuxBlockIo, - } - impl BlockIoBuilder { - fn new() -> Self { - let block_io = LinuxBlockIo { - weight: Some(0), - leaf_weight: Some(0), - weight_device: vec![].into(), - throttle_read_bps_device: vec![].into(), - throttle_write_bps_device: vec![].into(), - throttle_read_iops_device: vec![].into(), - throttle_write_iops_device: vec![].into(), - }; - - Self { block_io } - } - fn with_write_weight_device(mut self, throttle: Vec) -> Self { - self.block_io.weight_device = throttle.into(); - self - } - fn with_write_io_weight(mut self, iow: u16) -> Self { - self.block_io.weight = Some(iow); - self - } - - fn with_read_bps(mut self, throttle: Vec) -> Self { - self.block_io.throttle_read_bps_device = throttle.into(); - self - } - - fn with_write_bps(mut self, throttle: Vec) -> Self { - self.block_io.throttle_write_bps_device = throttle.into(); - self - } - - fn with_read_iops(mut self, throttle: Vec) -> Self { - self.block_io.throttle_read_iops_device = throttle.into(); - self - } - - fn with_write_iops(mut self, throttle: Vec) -> Self { - self.block_io.throttle_write_iops_device = throttle.into(); - self - } - - fn build(self) -> LinuxBlockIo { - self.block_io - } - } #[test] fn test_set_io_read_bps() { let (tmp, throttle) = setup("test_set_io_read_bps", "io.max"); - let blkio = BlockIoBuilder::new() - .with_read_bps(vec![LinuxThrottleDevice { - major: 8, - minor: 0, - rate: 102400, - }]) - .build(); + let blkio = LinuxBlockIoBuilder::default() + .throttle_read_bps_device(vec![LinuxThrottleDeviceBuilder::default() + .major(8) + .minor(0) + .rate(102400u64) + .build() + .unwrap()]) + .build() + .unwrap(); Io::apply(&tmp, &blkio).expect("apply blkio"); let content = fs::read_to_string(throttle).unwrap_or_else(|_| panic!("read rbps content")); @@ -226,13 +180,15 @@ mod test { fn test_set_io_write_bps() { let (tmp, throttle) = setup("test_set_io_write_bps", "io.max"); - let blkio = BlockIoBuilder::new() - .with_write_bps(vec![LinuxThrottleDevice { - major: 8, - minor: 0, - rate: 102400, - }]) - .build(); + let blkio = LinuxBlockIoBuilder::default() + .throttle_write_bps_device(vec![LinuxThrottleDeviceBuilder::default() + .major(8) + .minor(0) + .rate(102400u64) + .build() + .unwrap()]) + .build() + .unwrap(); Io::apply(&tmp, &blkio).expect("apply blkio"); let content = fs::read_to_string(throttle).unwrap_or_else(|_| panic!("read rbps content")); @@ -244,13 +200,15 @@ mod test { fn test_set_io_read_iops() { let (tmp, throttle) = setup("test_set_io_read_iops", "io.max"); - let blkio = BlockIoBuilder::new() - .with_read_iops(vec![LinuxThrottleDevice { - major: 8, - minor: 0, - rate: 102400, - }]) - .build(); + let blkio = LinuxBlockIoBuilder::default() + .throttle_read_iops_device(vec![LinuxThrottleDeviceBuilder::default() + .major(8) + .minor(0) + .rate(102400u64) + .build() + .unwrap()]) + .build() + .unwrap(); Io::apply(&tmp, &blkio).expect("apply blkio"); let content = fs::read_to_string(throttle).unwrap_or_else(|_| panic!("read riops content")); @@ -262,13 +220,15 @@ mod test { fn test_set_io_write_iops() { let (tmp, throttle) = setup("test_set_io_write_iops", "io.max"); - let blkio = BlockIoBuilder::new() - .with_write_iops(vec![LinuxThrottleDevice { - major: 8, - minor: 0, - rate: 102400, - }]) - .build(); + let blkio = LinuxBlockIoBuilder::default() + .throttle_write_iops_device(vec![LinuxThrottleDeviceBuilder::default() + .major(8) + .minor(0) + .rate(102400u64) + .build() + .unwrap()]) + .build() + .unwrap(); Io::apply(&tmp, &blkio).expect("apply blkio"); let content = fs::read_to_string(throttle).unwrap_or_else(|_| panic!("read wiops content")); @@ -279,14 +239,17 @@ mod test { #[test] fn test_set_ioweight_device() { let (tmp, throttle) = setup("test_set_io_weight_device", CGROUP_BFQ_IO_WEIGHT); - let blkio = BlockIoBuilder::new() - .with_write_weight_device(vec![LinuxWeightDevice { - major: 8, - minor: 0, - weight: Some(80), - leaf_weight: Some(0), - }]) - .build(); + let blkio = LinuxBlockIoBuilder::default() + .weight_device(vec![LinuxWeightDeviceBuilder::default() + .major(8) + .minor(0) + .weight(80u16) + .leaf_weight(0u16) + .build() + .unwrap()]) + .build() + .unwrap(); + Io::apply(&tmp, &blkio).expect("apply blkio"); let content = fs::read_to_string(throttle).unwrap_or_else(|_| panic!("read bfq_io_weight content")); @@ -297,7 +260,11 @@ mod test { #[test] fn test_set_ioweight() { let (tmp, throttle) = setup("test_set_io_weight", CGROUP_IO_WEIGHT); - let blkio = BlockIoBuilder::new().with_write_io_weight(100).build(); + let blkio = LinuxBlockIoBuilder::default() + .weight(100u16) + .build() + .unwrap(); + Io::apply(&tmp, &blkio).expect("apply blkio"); let content = fs::read_to_string(throttle).unwrap_or_else(|_| panic!("read bfq_io_weight content")); diff --git a/cgroups/src/v2/memory.rs b/cgroups/src/v2/memory.rs index 7e55f016..29c18a82 100644 --- a/cgroups/src/v2/memory.rs +++ b/cgroups/src/v2/memory.rs @@ -19,7 +19,7 @@ pub struct Memory {} impl Controller for Memory { fn apply(controller_opt: &ControllerOpt, cgroup_path: &Path) -> Result<()> { - if let Some(memory) = &controller_opt.resources.memory { + if let Some(memory) = &controller_opt.resources.memory() { Self::apply(cgroup_path, memory) .context("failed to apply memory resource restrictions")?; } @@ -84,15 +84,15 @@ impl Memory { fn apply(path: &Path, memory: &LinuxMemory) -> Result<()> { // if nothing is set just exit right away - if memory.reservation.is_none() && memory.limit.is_none() && memory.swap.is_none() { + if memory.reservation().is_none() && memory.limit().is_none() && memory.swap().is_none() { return Ok(()); } - match memory.limit { + match memory.limit() { Some(limit) if limit < -1 => { bail!("invalid memory value: {}", limit); } - Some(limit) => match memory.swap { + Some(limit) => match memory.swap() { Some(swap) if swap < -1 => { bail!("invalid swap value: {}", swap); } @@ -125,13 +125,13 @@ impl Memory { } }, None => { - if memory.swap.is_some() { + if memory.swap().is_some() { bail!("unable to set swap limit without memory limit"); } } }; - if let Some(reservation) = memory.reservation { + if let Some(reservation) = memory.reservation() { if reservation < -1 { bail!("invalid memory reservation value: {}", reservation); } @@ -146,7 +146,7 @@ impl Memory { mod tests { use super::*; use crate::test::{create_temp_dir, set_fixture}; - use oci_spec::runtime::LinuxMemory; + use oci_spec::runtime::LinuxMemoryBuilder; use std::fs::read_to_string; #[test] @@ -159,17 +159,15 @@ mod tests { let limit = 1024; let reservation = 512; let swap = 2048; - let memory_limits = &LinuxMemory { - limit: Some(limit), - reservation: Some(reservation), - swap: Some(swap), - kernel: None, - kernel_tcp: None, - swappiness: None, - disable_oom_killer: None, - use_hierarchy: None, - }; - Memory::apply(&tmp, memory_limits).expect("apply memory limits"); + + let memory_limits = LinuxMemoryBuilder::default() + .limit(limit) + .reservation(reservation) + .swap(swap) + .build() + .unwrap(); + + Memory::apply(&tmp, &memory_limits).expect("apply memory limits"); let limit_content = read_to_string(tmp.join(CGROUP_MEMORY_MAX)).expect("read memory limit"); assert_eq!(limit_content, limit.to_string()); @@ -190,17 +188,9 @@ mod tests { set_fixture(&tmp, CGROUP_MEMORY_LOW, "0").expect("set fixture for memory reservation"); set_fixture(&tmp, CGROUP_MEMORY_SWAP, "0").expect("set fixture for swap limit"); - let memory_limits = &LinuxMemory { - limit: Some(-1), - reservation: None, - swap: None, - kernel: None, - kernel_tcp: None, - swappiness: None, - disable_oom_killer: None, - use_hierarchy: None, - }; - Memory::apply(&tmp, memory_limits).expect("apply memory limits"); + let memory_limits = LinuxMemoryBuilder::default().limit(-1).build().unwrap(); + + Memory::apply(&tmp, &memory_limits).expect("apply memory limits"); let limit_content = read_to_string(tmp.join(CGROUP_MEMORY_MAX)).expect("read memory limit"); assert_eq!(limit_content, "max"); @@ -217,18 +207,9 @@ mod tests { set_fixture(&tmp, CGROUP_MEMORY_LOW, "0").expect("set fixture for memory reservation"); set_fixture(&tmp, CGROUP_MEMORY_SWAP, "0").expect("set fixture for swap limit"); - let memory_limits = &LinuxMemory { - limit: None, - swap: Some(512), - reservation: None, - kernel: None, - kernel_tcp: None, - swappiness: None, - disable_oom_killer: None, - use_hierarchy: None, - }; + let memory_limits = LinuxMemoryBuilder::default().swap(512).build().unwrap(); - let result = Memory::apply(&tmp, memory_limits); + let result = Memory::apply(&tmp, &memory_limits); assert!(result.is_err()); } @@ -240,18 +221,9 @@ mod tests { set_fixture(&tmp, CGROUP_MEMORY_LOW, "0").expect("set fixture for memory reservation"); set_fixture(&tmp, CGROUP_MEMORY_SWAP, "0").expect("set fixture for swap limit"); - let memory_limits = &LinuxMemory { - limit: Some(-2), - swap: None, - reservation: None, - kernel: None, - kernel_tcp: None, - swappiness: None, - disable_oom_killer: None, - use_hierarchy: None, - }; + let memory_limits = LinuxMemoryBuilder::default().limit(-2).build().unwrap(); - let result = Memory::apply(&tmp, memory_limits); + let result = Memory::apply(&tmp, &memory_limits); assert!(result.is_err()); } @@ -263,18 +235,13 @@ mod tests { set_fixture(&tmp, CGROUP_MEMORY_LOW, "0").expect("set fixture for memory reservation"); set_fixture(&tmp, CGROUP_MEMORY_SWAP, "0").expect("set fixture for swap limit"); - let memory_limits = &LinuxMemory { - limit: Some(512), - swap: Some(-3), - reservation: None, - kernel: None, - kernel_tcp: None, - swappiness: None, - disable_oom_killer: None, - use_hierarchy: None, - }; + let memory_limits = LinuxMemoryBuilder::default() + .limit(512) + .swap(-3) + .build() + .unwrap(); - let result = Memory::apply(&tmp, memory_limits); + let result = Memory::apply(&tmp, &memory_limits); assert!(result.is_err()); } @@ -290,27 +257,27 @@ mod tests { // we need to check for expected errors first and foremost or we'll get false negatives // later - if let Some(limit) = linux_memory.limit { + if let Some(limit) = linux_memory.limit() { if limit < -1 { return result.is_err(); } } - if let Some(swap) = linux_memory.swap { + if let Some(swap) = linux_memory.swap() { if swap < -1 { return result.is_err(); } - if linux_memory.limit.is_none() { + if linux_memory.limit().is_none() { return result.is_err(); } - if let Some(limit) = linux_memory.limit { + if let Some(limit) = linux_memory.limit() { if limit != -1 && swap != -1 && swap < limit { return result.is_err(); } } } - if let Some(reservation) = linux_memory.reservation { + if let Some(reservation) = linux_memory.reservation() { if reservation < -1 { return result.is_err(); } @@ -318,7 +285,7 @@ mod tests { // check the limit file is set as expected let limit_content = read_to_string(tmp.join(CGROUP_MEMORY_MAX)).expect("read memory limit to string"); - let limit_check = match linux_memory.limit { + let limit_check = match linux_memory.limit() { Some(limit) if limit == -1 => limit_content == "max", Some(limit) => limit_content == limit.to_string(), None => limit_content == "0", @@ -326,21 +293,21 @@ mod tests { // check the swap file is set as expected let swap_content = read_to_string(tmp.join(CGROUP_MEMORY_SWAP)).expect("read swap limit to string"); - let swap_check = match linux_memory.swap { + let swap_check = match linux_memory.swap() { Some(swap) if swap == -1 => swap_content == "max", Some(swap) => { - if let Some(limit) = linux_memory.limit { + if let Some(limit) = linux_memory.limit() { if limit == -1 { swap_content == swap.to_string() } else { - swap_content == (swap - linux_memory.limit.unwrap()).to_string() + swap_content == (swap - linux_memory.limit().unwrap()).to_string() } } else { false } } None => { - match linux_memory.limit { + match linux_memory.limit() { Some(limit) if limit == -1 => swap_content == "max", _ => swap_content == "0", } @@ -350,7 +317,7 @@ mod tests { // check the resevation file is set as expected let reservation_content = read_to_string(tmp.join(CGROUP_MEMORY_LOW)).expect("read memory reservation to string"); - let reservation_check = match linux_memory.reservation { + let reservation_check = match linux_memory.reservation() { Some(reservation) if reservation == -1 => reservation_content == "max", Some(reservation) => reservation_content == reservation.to_string(), None => reservation_content == "0", diff --git a/cgroups/src/v2/pids.rs b/cgroups/src/v2/pids.rs index 57449446..8b268acd 100644 --- a/cgroups/src/v2/pids.rs +++ b/cgroups/src/v2/pids.rs @@ -15,7 +15,7 @@ pub struct Pids {} impl Controller for Pids { fn apply(controller_opt: &ControllerOpt, cgroup_root: &std::path::Path) -> Result<()> { log::debug!("Apply pids cgroup v2 config"); - if let Some(pids) = &controller_opt.resources.pids { + if let Some(pids) = &controller_opt.resources.pids() { Self::apply(cgroup_root, pids).context("failed to apply pids resource restrictions")?; } Ok(()) @@ -32,8 +32,8 @@ impl StatsProvider for Pids { impl Pids { fn apply(root_path: &Path, pids: &LinuxPids) -> Result<()> { - let limit = if pids.limit > 0 { - pids.limit.to_string() + let limit = if pids.limit() > 0 { + pids.limit().to_string() } else { "max".to_string() }; @@ -45,7 +45,7 @@ impl Pids { mod tests { use super::*; use crate::test::{create_temp_dir, set_fixture}; - use oci_spec::runtime::LinuxPids; + use oci_spec::runtime::LinuxPidsBuilder; #[test] fn test_set_pids() { @@ -53,12 +53,12 @@ mod tests { let tmp = create_temp_dir("v2_test_set_pids").expect("create temp directory for test"); set_fixture(&tmp, pids_file_name, "1000").expect("Set fixture for 1000 pids"); - let pids = LinuxPids { limit: 1000 }; + let pids = LinuxPidsBuilder::default().limit(1000).build().unwrap(); Pids::apply(&tmp, &pids).expect("apply pids"); let content = std::fs::read_to_string(tmp.join(pids_file_name)).expect("Read pids contents"); - assert_eq!(pids.limit.to_string(), content); + assert_eq!(pids.limit().to_string(), content); } #[test] @@ -67,7 +67,7 @@ mod tests { let tmp = create_temp_dir("v2_test_set_pids_max").expect("create temp directory for test"); set_fixture(&tmp, pids_file_name, "0").expect("set fixture for 0 pids"); - let pids = LinuxPids { limit: 0 }; + let pids = LinuxPidsBuilder::default().limit(0).build().unwrap(); Pids::apply(&tmp, &pids).expect("apply pids"); diff --git a/cgroups/src/v2/unified.rs b/cgroups/src/v2/unified.rs index 65276fb3..780761c2 100644 --- a/cgroups/src/v2/unified.rs +++ b/cgroups/src/v2/unified.rs @@ -13,7 +13,7 @@ impl Unified { cgroup_path: &Path, controllers: Vec, ) -> Result<()> { - if let Some(unified) = &controller_opt.resources.unified { + if let Some(unified) = &controller_opt.resources.unified() { log::debug!("Apply unified cgroup config"); for (cgroup_file, value) in unified { common::write_cgroup_file_str(cgroup_path.join(cgroup_file), value).map_err( @@ -45,10 +45,10 @@ impl Unified { #[cfg(test)] mod tests { - use std::array::IntoIter; + use std::collections::HashMap; use std::fs; - use oci_spec::runtime::LinuxResources; + use oci_spec::runtime::LinuxResourcesBuilder; use crate::test::{create_temp_dir, set_fixture}; use crate::v2::controller_type::ControllerType; @@ -62,20 +62,21 @@ mod tests { let hugetlb_limit_path = set_fixture(&tmp, "hugetlb.1GB.limit_in_bytes", "").unwrap(); let cpu_weight_path = set_fixture(&tmp, "cpu.weight", "").unwrap(); - let resources = LinuxResources { - unified: Some( - IntoIter::new([ - ( - "hugetlb.1GB.limit_in_bytes".to_owned(), - "72348034".to_owned(), - ), - ("cpu.weight".to_owned(), "5000".to_owned()), - ]) - .collect(), - ), - ..Default::default() + let unified = { + let mut u = HashMap::new(); + u.insert( + "hugetlb.1GB.limit_in_bytes".to_owned(), + "72348034".to_owned(), + ); + u.insert("cpu.weight".to_owned(), "5000".to_owned()); + u }; + let resources = LinuxResourcesBuilder::default() + .unified(unified) + .build() + .unwrap(); + let controller_opt = ControllerOpt { resources: &resources, freezer_state: None, @@ -99,20 +100,21 @@ mod tests { let tmp = create_temp_dir("test_set_unified_failed_to_write_subsystem_not_enabled").unwrap(); - let resources = LinuxResources { - unified: Some( - IntoIter::new([ - ( - "hugetlb.1GB.limit_in_bytes".to_owned(), - "72348034".to_owned(), - ), - ("cpu.weight".to_owned(), "5000".to_owned()), - ]) - .collect(), - ), - ..Default::default() + let unified = { + let mut u = HashMap::new(); + u.insert( + "hugetlb.1GB.limit_in_bytes".to_owned(), + "72348034".to_owned(), + ); + u.insert("cpu.weight".to_owned(), "5000".to_owned()); + u }; + let resources = LinuxResourcesBuilder::default() + .unified(unified) + .build() + .unwrap(); + let controller_opt = ControllerOpt { resources: &resources, freezer_state: None, @@ -132,20 +134,21 @@ mod tests { // arrange let tmp = create_temp_dir("test_set_unified_failed_to_write_subsystem_enabled").unwrap(); - let resources = LinuxResources { - unified: Some( - IntoIter::new([ - ( - "hugetlb.1GB.limit_in_bytes".to_owned(), - "72348034".to_owned(), - ), - ("cpu.weight".to_owned(), "5000".to_owned()), - ]) - .collect(), - ), - ..Default::default() + let unified = { + let mut u = HashMap::new(); + u.insert( + "hugetlb.1GB.limit_in_bytes".to_owned(), + "72348034".to_owned(), + ); + u.insert("cpu.weight".to_owned(), "5000".to_owned()); + u }; + let resources = LinuxResourcesBuilder::default() + .unified(unified) + .build() + .unwrap(); + let controller_opt = ControllerOpt { resources: &resources, oom_score_adj: None, diff --git a/src/capabilities.rs b/src/capabilities.rs index 0ddfb1a3..d9e409f1 100644 --- a/src/capabilities.rs +++ b/src/capabilities.rs @@ -132,24 +132,24 @@ pub fn reset_effective(syscall: &S) -> Result<()> { /// Drop any extra granted capabilities, and reset to defaults which are in oci specification pub fn drop_privileges(cs: &LinuxCapabilities, syscall: &S) -> Result<()> { - log::debug!("dropping bounding capabilities to {:?}", cs.bounding); - if let Some(bounding) = cs.bounding.as_ref() { + log::debug!("dropping bounding capabilities to {:?}", cs.bounding()); + if let Some(bounding) = cs.bounding().as_ref() { syscall.set_capability(CapSet::Bounding, &to_set(bounding))?; } - if let Some(effective) = cs.effective.as_ref() { + if let Some(effective) = cs.effective().as_ref() { syscall.set_capability(CapSet::Effective, &to_set(effective))?; } - if let Some(permitted) = cs.permitted.as_ref() { + if let Some(permitted) = cs.permitted().as_ref() { syscall.set_capability(CapSet::Permitted, &to_set(permitted))?; } - if let Some(inheritable) = cs.inheritable.as_ref() { + if let Some(inheritable) = cs.inheritable().as_ref() { syscall.set_capability(CapSet::Inheritable, &to_set(inheritable))?; } - if let Some(ambient) = cs.ambient.as_ref() { + if let Some(ambient) = cs.ambient().as_ref() { // check specifically for ambient, as those might not always be available if let Err(e) = syscall.set_capability(CapSet::Ambient, &to_set(ambient)) { log::error!("failed to set ambient capabilities: {}", e); @@ -161,6 +161,9 @@ pub fn drop_privileges(cs: &LinuxCapabilities, syscall: &S) #[cfg(test)] mod tests { + use oci_spec::runtime::LinuxCapabilitiesBuilder; + use std::collections::HashSet; + use super::*; use crate::syscall::test::TestHelperSyscall; @@ -555,13 +558,14 @@ mod tests { let tests = vec![ Testcase { name: format!("all LinuxCapabilities fields with caps: {:?}", cps), - input: LinuxCapabilities { - bounding: cps.clone().into_iter().collect::().into(), - effective: cps.clone().into_iter().collect::().into(), - inheritable: cps.clone().into_iter().collect::().into(), - permitted: cps.clone().into_iter().collect::().into(), - ambient: cps.clone().into_iter().collect::().into(), - }, + input: LinuxCapabilitiesBuilder::default() + .bounding(cps.clone().into_iter().collect::()) + .effective(cps.clone().into_iter().collect::()) + .inheritable(cps.clone().into_iter().collect::()) + .permitted(cps.clone().into_iter().collect::()) + .ambient(cps.clone().into_iter().collect::()) + .build() + .unwrap(), want: vec![ (CapSet::Bounding, cps.clone()), (CapSet::Effective, cps.clone()), @@ -572,29 +576,37 @@ mod tests { }, Testcase { name: format!("partial LinuxCapabilities fields with caps: {:?}", cps), - input: LinuxCapabilities { - bounding: cps.clone().into_iter().collect::().into(), - effective: cps.clone().into_iter().collect::().into(), - inheritable: None, - permitted: cps.clone().into_iter().collect::().into(), - ambient: None, - }, + input: LinuxCapabilitiesBuilder::default() + .bounding(cps.clone().into_iter().collect::()) + .effective(cps.clone().into_iter().collect::()) + .permitted(cps.clone().into_iter().collect::()) + .build() + .unwrap(), want: vec![ (CapSet::Bounding, cps.clone()), (CapSet::Effective, cps.clone()), (CapSet::Permitted, cps.clone()), + (CapSet::Inheritable, cps.clone()), + (CapSet::Ambient, cps.clone()), ], }, Testcase { name: format!("empty LinuxCapabilities fields with caps: {:?}", cps), - input: LinuxCapabilities { - bounding: None, - effective: None, - inheritable: None, - permitted: None, - ambient: None, - }, - want: vec![], + input: LinuxCapabilitiesBuilder::default() + .bounding(HashSet::new()) + .effective(HashSet::new()) + .inheritable(HashSet::new()) + .permitted(HashSet::new()) + .ambient(HashSet::new()) + .build() + .unwrap(), + want: vec![ + (CapSet::Bounding, cps.clone()), + (CapSet::Effective, cps.clone()), + (CapSet::Permitted, cps.clone()), + (CapSet::Inheritable, cps.clone()), + (CapSet::Ambient, cps), + ], }, ]; diff --git a/src/commands/ps.rs b/src/commands/ps.rs index 79004205..a55692cf 100644 --- a/src/commands/ps.rs +++ b/src/commands/ps.rs @@ -29,7 +29,10 @@ impl Ps { let spec = oci_spec::runtime::Spec::load(config_absolute_path)?; log::debug!("spec: {:?}", spec); let cgroups_path = utils::get_cgroup_path( - &spec.linux.context("no linux in spec")?.cgroups_path, + spec.linux() + .as_ref() + .context("no linux in spec")? + .cgroups_path(), container.id(), ); let systemd_cgroup = container diff --git a/src/container/builder_impl.rs b/src/container/builder_impl.rs index 989db60a..4983319d 100644 --- a/src/container/builder_impl.rs +++ b/src/container/builder_impl.rs @@ -48,14 +48,14 @@ impl<'a> ContainerBuilderImpl<'a> { } fn run_container(&mut self) -> Result<()> { - let linux = self.spec.linux.as_ref().context("no linux in spec")?; - let cgroups_path = utils::get_cgroup_path(&linux.cgroups_path, &self.container_id); + let linux = self.spec.linux().as_ref().context("no linux in spec")?; + let cgroups_path = utils::get_cgroup_path(linux.cgroups_path(), &self.container_id); let cmanager = cgroups::common::create_cgroup_manager(&cgroups_path, self.use_systemd)?; - let process = self.spec.process.as_ref().context("No process in spec")?; + let process = self.spec.process().as_ref().context("No process in spec")?; if self.init { - if let Some(hooks) = self.spec.hooks.as_ref() { - hooks::run_hooks(hooks.create_runtime.as_ref(), self.container.as_ref())? + if let Some(hooks) = self.spec.hooks().as_ref() { + hooks::run_hooks(hooks.create_runtime().as_ref(), self.container.as_ref())? } } @@ -78,7 +78,7 @@ impl<'a> ContainerBuilderImpl<'a> { // is not writeable unless you're an privileged user (if !dumpable is // set). All children inherit their parent's oom_score_adj value on // fork(2) so this will always be propagated properly. - if let Some(oom_score_adj) = process.oom_score_adj { + if let Some(oom_score_adj) = process.oom_score_adj() { log::debug!("Set OOM score to {}", oom_score_adj); let mut f = fs::File::create("/proc/self/oom_score_adj")?; f.write_all(oom_score_adj.to_string().as_bytes())?; @@ -92,7 +92,7 @@ impl<'a> ContainerBuilderImpl<'a> { // going to be switching to a different security context. Thus setting // ourselves to be non-dumpable only breaks things (like rootless // containers), which is the recommendation from the kernel folks. - if linux.namespaces.is_some() { + if linux.namespaces().is_some() { prctl::set_dumpable(false).unwrap(); } @@ -147,8 +147,8 @@ impl<'a> ContainerBuilderImpl<'a> { let init_pid = receiver_from_intermediate.wait_for_intermediate_ready()?; log::debug!("init pid is {:?}", init_pid); - if self.rootless.is_none() && linux.resources.is_some() && self.init { - if let Some(resources) = linux.resources.as_ref() { + if self.rootless.is_none() && linux.resources().is_some() && self.init { + if let Some(resources) = linux.resources().as_ref() { apply_cgroups(resources, init_pid, cmanager.as_ref())?; } } @@ -215,7 +215,7 @@ mod tests { sched::{unshare, CloneFlags}, unistd::{self, getgid, getuid}, }; - use oci_spec::runtime::LinuxIdMapping; + use oci_spec::runtime::LinuxIdMappingBuilder; use serial_test::serial; use crate::process::channel::{intermediate_to_main, main_to_intermediate}; @@ -225,11 +225,11 @@ mod tests { #[test] #[serial] fn setup_uid_mapping_should_succeed() -> Result<()> { - let uid_mapping = LinuxIdMapping { - host_id: u32::from(getuid()), - container_id: 0, - size: 1, - }; + let uid_mapping = LinuxIdMappingBuilder::default() + .host_id(getuid()) + .container_id(0u32) + .size(1u32) + .build()?; let uid_mappings = vec![uid_mapping]; let rootless = Rootless { uid_mappings: Some(&uid_mappings), @@ -246,9 +246,9 @@ mod tests { let line = fs::read_to_string(format!("/proc/{}/uid_map", child.as_raw()))?; let line_splited = line.split_whitespace(); for (act, expect) in line_splited.zip([ - uid_mapping.container_id.to_string().as_str(), - uid_mapping.host_id.to_string().as_str(), - uid_mapping.size.to_string().as_str(), + uid_mapping.container_id().to_string().as_str(), + uid_mapping.host_id().to_string().as_str(), + uid_mapping.size().to_string().as_str(), ]) { assert_eq!(act, expect); } @@ -271,11 +271,11 @@ mod tests { #[test] #[serial] fn setup_gid_mapping_should_successed() -> Result<()> { - let gid_mapping = LinuxIdMapping { - host_id: u32::from(getgid()), - container_id: 0, - size: 1, - }; + let gid_mapping = LinuxIdMappingBuilder::default() + .host_id(getgid()) + .container_id(0u32) + .size(1u32) + .build()?; let gid_mappings = vec![gid_mapping]; let rootless = Rootless { gid_mappings: Some(&gid_mappings), @@ -291,9 +291,9 @@ mod tests { let line = fs::read_to_string(format!("/proc/{}/gid_map", child.as_raw()))?; let line_splited = line.split_whitespace(); for (act, expect) in line_splited.zip([ - gid_mapping.container_id.to_string().as_str(), - gid_mapping.host_id.to_string().as_str(), - gid_mapping.size.to_string().as_str(), + gid_mapping.container_id().to_string().as_str(), + gid_mapping.host_id().to_string().as_str(), + gid_mapping.size().to_string().as_str(), ]) { assert_eq!(act, expect); } diff --git a/src/container/container_delete.rs b/src/container/container_delete.rs index 217a483a..19c26a8c 100644 --- a/src/container/container_delete.rs +++ b/src/container/container_delete.rs @@ -48,7 +48,10 @@ impl Container { })?; let cgroups_path = utils::get_cgroup_path( - &spec.linux.context("no linux in spec")?.cgroups_path, + spec.linux() + .as_ref() + .context("no linux in spec")? + .cgroups_path(), self.id(), ); @@ -64,8 +67,8 @@ impl Container { format!("failed to remove cgroup {}", cgroups_path.display()) })?; - if let Some(hooks) = spec.hooks.as_ref() { - hooks::run_hooks(hooks.poststop.as_ref(), Some(self)) + if let Some(hooks) = spec.hooks().as_ref() { + hooks::run_hooks(hooks.poststop().as_ref(), Some(self)) .with_context(|| "failed to run post stop hooks")?; } } diff --git a/src/container/container_events.rs b/src/container/container_events.rs index 9933afc2..7b27d0f5 100644 --- a/src/container/container_events.rs +++ b/src/container/container_events.rs @@ -31,7 +31,11 @@ impl Container { } let cgroups_path = utils::get_cgroup_path( - &self.spec()?.linux.context("no linux in spec")?.cgroups_path, + self.spec()? + .linux() + .as_ref() + .context("no linux in spec")? + .cgroups_path(), self.id(), ); let use_systemd = self diff --git a/src/container/container_pause.rs b/src/container/container_pause.rs index 74b0439f..cb957841 100644 --- a/src/container/container_pause.rs +++ b/src/container/container_pause.rs @@ -36,7 +36,10 @@ impl Container { let spec = self.spec()?; let cgroups_path = utils::get_cgroup_path( - &spec.linux.context("no linux in spec")?.cgroups_path, + spec.linux() + .as_ref() + .context("no linux in spec")? + .cgroups_path(), self.id(), ); diff --git a/src/container/container_resume.rs b/src/container/container_resume.rs index 8d7570d9..8a24d40f 100644 --- a/src/container/container_resume.rs +++ b/src/container/container_resume.rs @@ -38,7 +38,10 @@ impl Container { let spec = self.spec()?; let cgroups_path = utils::get_cgroup_path( - &spec.linux.context("no linux in spec")?.cgroups_path, + spec.linux() + .as_ref() + .context("no linux in spec")? + .cgroups_path(), self.id(), ); diff --git a/src/container/container_start.rs b/src/container/container_start.rs index e1074c62..e6b90080 100644 --- a/src/container/container_start.rs +++ b/src/container/container_start.rs @@ -42,11 +42,11 @@ impl Container { let spec = self .spec() .with_context(|| format!("failed to load runtime spec for container {}", self.id()))?; - if let Some(hooks) = spec.hooks.as_ref() { + if let Some(hooks) = spec.hooks().as_ref() { // While prestart is marked as deprecated in the OCI spec, the docker and integration test still // uses it. #[allow(deprecated)] - hooks::run_hooks(hooks.prestart.as_ref(), Some(self)) + hooks::run_hooks(hooks.prestart().as_ref(), Some(self)) .with_context(|| "failed to run pre start hooks")?; } @@ -60,8 +60,8 @@ impl Container { // Run post start hooks. It runs after the container process is started. // It is called in the runtime namespace. - if let Some(hooks) = spec.hooks.as_ref() { - hooks::run_hooks(hooks.poststart.as_ref(), Some(self)) + if let Some(hooks) = spec.hooks().as_ref() { + hooks::run_hooks(hooks.poststart().as_ref(), Some(self)) .with_context(|| "failed to run post start hooks")?; } diff --git a/src/container/init_builder.rs b/src/container/init_builder.rs index b1d9fa9a..73361b57 100644 --- a/src/container/init_builder.rs +++ b/src/container/init_builder.rs @@ -46,12 +46,12 @@ impl<'a> InitContainerBuilder<'a> { let mut container = self.create_container_state(&container_dir)?; container .set_systemd(self.use_systemd) - .set_annotations(spec.annotations.clone()); + .set_annotations(spec.annotations().clone()); unistd::chdir(&*container_dir)?; let notify_path = container_dir.join(NOTIFY_FILE); // convert path of root file system of the container to absolute path - let rootfs = fs::canonicalize(&spec.root.as_ref().context("no root in spec")?.path)?; + let rootfs = fs::canonicalize(&spec.root().as_ref().context("no root in spec")?.path())?; // if socket file path is given in commandline options, // get file descriptors of console socket @@ -109,15 +109,15 @@ impl<'a> InitContainerBuilder<'a> { } fn validate_spec(spec: &Spec) -> Result<()> { - if !spec.version.starts_with("1.0") { + if !spec.version().starts_with("1.0") { bail!( "runtime spec has incompatible version '{}'. Only 1.0.X is supported", - spec.version + spec.version() ); } - if let Some(process) = &spec.process { - if let Some(profile) = &process.apparmor_profile { + if let Some(process) = &spec.process() { + if let Some(profile) = &process.apparmor_profile() { if !apparmor::is_enabled()? { bail!( "apparmor profile {} is specified in runtime spec, \ diff --git a/src/container/tenant_builder.rs b/src/container/tenant_builder.rs index 1cfff85d..2078b69d 100644 --- a/src/container/tenant_builder.rs +++ b/src/container/tenant_builder.rs @@ -2,8 +2,9 @@ use anyhow::{bail, Context, Result}; use caps::Capability; use nix::unistd; use oci_spec::runtime::{ - Capabilities as SpecCapabilities, Capability as SpecCapability, LinuxCapabilities, - LinuxNamespace, LinuxNamespaceType, Process, Spec, + Capabilities as SpecCapabilities, Capability as SpecCapability, LinuxBuilder, + LinuxCapabilities, LinuxCapabilitiesBuilder, LinuxNamespace, LinuxNamespaceBuilder, + LinuxNamespaceType, Process, ProcessBuilder, Spec, SpecBuilder, }; use procfs::process::Namespace; @@ -90,14 +91,16 @@ impl<'a> TenantContainerBuilder<'a> { pub fn build(self) -> Result<()> { let container_dir = self.lookup_container_dir()?; let container = self.load_container_state(container_dir.clone())?; - let mut spec = self.load_init_spec(&container_dir)?; - self.adapt_spec_for_tenant(&mut spec, &container)?; + + let spec = self.load_init_spec(&container_dir)?; + let spec = self.adapt_spec_for_tenant(&spec, &container)?; + log::debug!("{:#?}", spec); unistd::chdir(&*container_dir)?; let notify_path = Self::setup_notify_listener(&container_dir)?; // convert path of root file system of the container to absolute path - let rootfs = fs::canonicalize(&spec.root.as_ref().context("no root in spec")?.path)?; + let rootfs = fs::canonicalize(&spec.root().as_ref().context("no root in spec")?.path())?; // if socket file path is given in commandline options, // get file descriptors of console socket @@ -156,28 +159,72 @@ impl<'a> TenantContainerBuilder<'a> { Ok(container) } - fn adapt_spec_for_tenant(&self, spec: &mut Spec, container: &Container) -> Result<()> { - if let Some(ref process) = self.process { - self.set_process(spec, process)?; + fn adapt_spec_for_tenant(&self, spec: &Spec, container: &Container) -> Result { + let process = if let Some(ref process) = self.process { + self.set_process(process)? } else { - self.set_working_dir(spec)?; - self.set_args(spec)?; - self.set_environment(spec)?; - self.set_no_new_privileges(spec)?; - self.set_capabilities(spec)?; - } + let mut process_builder = ProcessBuilder::default(); + + process_builder = match self.set_working_dir()? { + Some(cwd) => process_builder.cwd(cwd), + None => process_builder, + }; + + process_builder = process_builder.args(self.set_args()?); + process_builder = process_builder.env(self.set_environment()?); + + process_builder = match self.set_no_new_privileges() { + Some(no_new_priv) => process_builder.no_new_privileges(no_new_priv), + None => process_builder, + }; + + process_builder = match self.set_capabilities(spec)? { + Some(caps) => process_builder.capabilities(caps), + None => process_builder, + }; + + process_builder.build()? + }; if container.pid().is_none() { bail!("Could not retrieve container init pid"); } let init_process = procfs::process::Process::new(container.pid().unwrap().as_raw())?; - self.set_namespaces(spec, init_process.namespaces()?)?; + let ns = self.set_namespaces(init_process.namespaces()?)?; + let linux = LinuxBuilder::default().namespaces(ns).build()?; - Ok(()) + let mut spec_builder = SpecBuilder::default() + .process(process) + .version(spec.version()) + .linux(linux); + + spec_builder = match spec.root() { + Some(root) => spec_builder.root(root.clone()), + None => spec_builder, + }; + spec_builder = match spec.mounts() { + Some(mounts) => spec_builder.mounts(mounts.clone()), + None => spec_builder, + }; + spec_builder = match spec.hostname() { + Some(hostname) => spec_builder.hostname(hostname.clone()), + None => spec_builder, + }; + spec_builder = match spec.hooks() { + Some(hooks) => spec_builder.hooks(hooks.clone()), + None => spec_builder, + }; + spec_builder = match spec.annotations() { + Some(annotations) => spec_builder.annotations(annotations.clone()), + None => spec_builder, + }; + + let spec = spec_builder.build()?; + Ok(spec) } - fn set_process(&self, spec: &mut Spec, process: &Path) -> Result<()> { + fn set_process(&self, process: &Path) -> Result { if !process.exists() { bail!( "Process.json file does not exist at specified path {}", @@ -186,12 +233,11 @@ impl<'a> TenantContainerBuilder<'a> { } let process = utils::open(process)?; - let process_spec: Process = serde_json::from_reader(process)?; - spec.process = Some(process_spec); - Ok(()) + let process_spec = serde_json::from_reader(process)?; + Ok(process_spec) } - fn set_working_dir(&self, spec: &mut Spec) -> Result<()> { + fn set_working_dir(&self) -> Result> { if let Some(ref cwd) = self.cwd { if cwd.is_relative() { bail!( @@ -199,51 +245,32 @@ impl<'a> TenantContainerBuilder<'a> { cwd.display() ); } - - spec.process.as_mut().context("no process in spec")?.cwd = cwd.to_path_buf(); + return Ok(Some(cwd.into())); } - - Ok(()) + Ok(None) } - fn set_args(&self, spec: &mut Spec) -> Result<()> { + fn set_args(&self) -> Result> { if self.args.is_empty() { bail!("Container command was not specified") } - spec.process.as_mut().context("no process in spec")?.args = self.args.clone().into(); - Ok(()) + Ok(self.args.clone()) } - fn set_environment(&self, spec: &mut Spec) -> Result<()> { - spec.process - .as_mut() - .context("no process in spec")? + fn set_environment(&self) -> Result> { + Ok(self .env - .as_mut() - .context("no env in process spec")? - .append( - &mut self - .env - .iter() - .map(|(k, v)| format!("{}={}", k, v)) - .collect(), - ); - - Ok(()) + .iter() + .map(|(k, v)| format!("{}={}", k, v)) + .collect()) } - fn set_no_new_privileges(&self, spec: &mut Spec) -> Result<()> { - if let Some(no_new_privs) = self.no_new_privs { - spec.process - .as_mut() - .context("no process in spec")? - .no_new_privileges = no_new_privs.into(); - } - Ok(()) + fn set_no_new_privileges(&self) -> Option { + self.no_new_privs } - fn set_capabilities(&self, spec: &mut Spec) -> Result<()> { + fn set_capabilities(&self, spec: &Spec) -> Result> { if !self.capabilities.is_empty() { let mut caps: Vec = Vec::with_capacity(self.capabilities.len()); for cap in &self.capabilities { @@ -253,70 +280,84 @@ impl<'a> TenantContainerBuilder<'a> { let caps: SpecCapabilities = caps.iter().map(|c| SpecCapability::from_cap(*c)).collect(); - if let Some(ref mut spec_caps) = spec - .process - .as_mut() + if let Some(ref spec_caps) = spec + .process() + .as_ref() .context("no process in spec")? - .capabilities + .capabilities() { - spec_caps - .ambient - .as_mut() - .context("no ambient caps in process spec")? - .extend(&caps); - spec_caps - .bounding - .as_mut() - .context("no bounding caps in process spec")? - .extend(&caps); - spec_caps - .effective - .as_mut() - .context("no effective caps in process spec")? - .extend(&caps); - spec_caps - .inheritable - .as_mut() - .context("no inheritable caps in process spec")? - .extend(&caps); - spec_caps - .permitted - .as_mut() - .context("no permitted caps in process spec")? - .extend(&caps); - } else { - spec.process - .as_mut() - .context("no process in spec")? - .capabilities = Some(LinuxCapabilities { - ambient: caps.clone().into(), - bounding: caps.clone().into(), - effective: caps.clone().into(), - inheritable: caps.clone().into(), - permitted: caps.into(), - }) + let mut capabilities_builder = LinuxCapabilitiesBuilder::default(); + capabilities_builder = match spec_caps.ambient() { + Some(ambient) => { + let ambient: SpecCapabilities = ambient.union(&caps).copied().collect(); + capabilities_builder.ambient(ambient) + } + None => capabilities_builder, + }; + capabilities_builder = match spec_caps.bounding() { + Some(bounding) => { + let bounding: SpecCapabilities = bounding.union(&caps).copied().collect(); + capabilities_builder.bounding(bounding) + } + None => capabilities_builder, + }; + capabilities_builder = match spec_caps.effective() { + Some(effective) => { + let effective: SpecCapabilities = effective.union(&caps).copied().collect(); + capabilities_builder.effective(effective) + } + None => capabilities_builder, + }; + capabilities_builder = match spec_caps.inheritable() { + Some(inheritable) => { + let inheritable: SpecCapabilities = + inheritable.union(&caps).copied().collect(); + capabilities_builder.inheritable(inheritable) + } + None => capabilities_builder, + }; + capabilities_builder = match spec_caps.permitted() { + Some(permitted) => { + let permitted: SpecCapabilities = permitted.union(&caps).copied().collect(); + capabilities_builder.permitted(permitted) + } + None => capabilities_builder, + }; + + let c = capabilities_builder.build()?; + return Ok(Some(c)); } + + return Ok(Some( + LinuxCapabilitiesBuilder::default() + .bounding(caps.clone()) + .effective(caps.clone()) + .inheritable(caps.clone()) + .permitted(caps.clone()) + .ambient(caps) + .build()?, + )); } - Ok(()) + Ok(None) } - fn set_namespaces(&self, spec: &mut Spec, init_namespaces: Vec) -> Result<()> { + fn set_namespaces(&self, init_namespaces: Vec) -> Result> { let mut tenant_namespaces = Vec::with_capacity(init_namespaces.len()); for ns_type in NAMESPACE_TYPES.iter().copied() { if let Some(init_ns) = init_namespaces.iter().find(|n| n.ns_type.eq(ns_type)) { let tenant_ns = LinuxNamespaceType::try_from(ns_type)?; - tenant_namespaces.push(LinuxNamespace { - typ: tenant_ns, - path: Some(init_ns.path.clone()), - }) + tenant_namespaces.push( + LinuxNamespaceBuilder::default() + .typ(tenant_ns) + .path(init_ns.path.clone()) + .build()?, + ) } } - let mut linux = &mut spec.linux.as_mut().context("no linux in spec")?; - linux.namespaces = tenant_namespaces.into(); - Ok(()) + Ok(tenant_namespaces) } fn should_use_systemd(&self, container: &Container) -> bool { diff --git a/src/hooks.rs b/src/hooks.rs index 8191cd25..d9c8881c 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -27,21 +27,21 @@ pub fn run_hooks(hooks: Option<&Vec>, container: Option<&Container>) -> Re if let Some(hooks) = hooks { for hook in hooks { - let mut hook_command = process::Command::new(&hook.path); + let mut hook_command = process::Command::new(&hook.path()); // Based on OCI spec, the first arguement of the args vector is the // arg0, which can be different from the path. For example, path // may be "/usr/bin/true" and arg0 is set to "true". However, rust // command differenciates arg0 from args, where rust command arg // doesn't include arg0. So we have to make the split arg0 from the // rest of args. - if let Some((arg0, args)) = hook.args.as_ref().map(|a| a.split_first()).flatten() { + if let Some((arg0, args)) = hook.args().as_ref().map(|a| a.split_first()).flatten() { log::debug!("run_hooks arg0: {:?}, args: {:?}", arg0, args); hook_command.arg0(arg0).args(args) } else { - hook_command.arg0(&hook.path.as_path().display().to_string()) + hook_command.arg0(&hook.path().as_path().display().to_string()) }; - let envs: HashMap = if let Some(env) = hook.env.as_ref() { + let envs: HashMap = if let Some(env) = hook.env().as_ref() { utils::parse_env(env) } else { HashMap::new() @@ -78,7 +78,7 @@ pub fn run_hooks(hooks: Option<&Vec>, container: Option<&Container>) -> Re } } - let res = if let Some(timeout_sec) = hook.timeout { + let res = if let Some(timeout_sec) = hook.timeout() { // Rust does not make it easy to handle executing a command and // timeout. Here we decided to wait for the command in a // different thread, so the main thread is not blocked. We use a @@ -137,8 +137,9 @@ pub fn run_hooks(hooks: Option<&Vec>, container: Option<&Container>) -> Re mod test { use super::*; use anyhow::{bail, Result}; + use oci_spec::runtime::HookBuilder; use serial_test::serial; - use std::{env, fs, path::PathBuf}; + use std::{env, fs}; fn is_command_in_path(program: &str) -> bool { if let Ok(path) = env::var("PATH") { @@ -170,12 +171,8 @@ mod test { { assert!(is_command_in_path("true"), "The true was not found."); let default_container: Container = Default::default(); - let hook = Hook { - path: PathBuf::from("true"), - args: None, - env: None, - timeout: None, - }; + + let hook = HookBuilder::default().path("true").build()?; let hooks = Some(vec![hook]); run_hooks(hooks.as_ref(), Some(&default_container)).context("Failed true")?; } @@ -187,16 +184,15 @@ mod test { ); // Use `printenv` to make sure the environment is set correctly. let default_container: Container = Default::default(); - let hook = Hook { - path: PathBuf::from("bash"), - args: Some(vec![ + let hook = HookBuilder::default() + .path("bash") + .args(vec![ String::from("bash"), String::from("-c"), String::from("printenv key > /dev/null"), - ]), - env: Some(vec![String::from("key=value")]), - timeout: None, - }; + ]) + .env(vec![String::from("key=value")]) + .build()?; let hooks = Some(vec![hook]); run_hooks(hooks.as_ref(), Some(&default_container)).context("Failed printenv test")?; } @@ -211,16 +207,15 @@ mod test { fn test_run_hook_timeout() -> Result<()> { let default_container: Container = Default::default(); // We use `tail -f /dev/null` here to simulate a hook command that hangs. - let hook = Hook { - path: PathBuf::from("tail"), - args: Some(vec![ + let hook = HookBuilder::default() + .path("tail") + .args(vec![ String::from("tail"), String::from("-f"), String::from("/dev/null"), - ]), - env: None, - timeout: Some(1), - }; + ]) + .timeout(1) + .build()?; let hooks = Some(vec![hook]); match run_hooks(hooks.as_ref(), Some(&default_container)) { Ok(_) => { diff --git a/src/namespaces.rs b/src/namespaces.rs index a0b5caf2..f7b968e3 100644 --- a/src/namespaces.rs +++ b/src/namespaces.rs @@ -37,7 +37,7 @@ impl From>> for Namespaces { let namespace_map: collections::HashMap = namespaces .unwrap_or(&vec![]) .iter() - .map(|ns| (get_clone_flag(ns.typ), ns.clone())) + .map(|ns| (get_clone_flag(ns.typ()), ns.clone())) .collect(); Namespaces { @@ -63,14 +63,14 @@ impl Namespaces { pub fn unshare_or_setns(&self, namespace: &LinuxNamespace) -> Result<()> { log::debug!("unshare or setns: {:?}", namespace); - if namespace.path.is_none() { - self.command.unshare(get_clone_flag(namespace.typ))?; + if namespace.path().is_none() { + self.command.unshare(get_clone_flag(namespace.typ()))?; } else { - let ns_path = namespace.path.as_ref().unwrap(); + let ns_path = namespace.path().as_ref().unwrap(); let fd = fcntl::open(ns_path, fcntl::OFlag::empty(), stat::Mode::empty()) .with_context(|| format!("Failed to open namespace fd: {:?}", ns_path))?; self.command - .set_ns(fd, get_clone_flag(namespace.typ)) + .set_ns(fd, get_clone_flag(namespace.typ())) .with_context(|| "Failed to set namespace")?; unistd::close(fd).with_context(|| "Failed to close namespace fd")?; } @@ -87,31 +87,33 @@ impl Namespaces { mod tests { use super::*; use crate::syscall::test::TestHelperSyscall; - use oci_spec::runtime::LinuxNamespaceType; + use oci_spec::runtime::{LinuxNamespaceBuilder, LinuxNamespaceType}; use serial_test::serial; fn gen_sample_linux_namespaces() -> Vec { vec![ - LinuxNamespace { - typ: LinuxNamespaceType::Mount, - path: Some("/dev/null".into()), - }, - LinuxNamespace { - typ: LinuxNamespaceType::Network, - path: Some("/dev/null".into()), - }, - LinuxNamespace { - typ: LinuxNamespaceType::Pid, - path: None, - }, - LinuxNamespace { - typ: LinuxNamespaceType::User, - path: None, - }, - LinuxNamespace { - typ: LinuxNamespaceType::Ipc, - path: None, - }, + LinuxNamespaceBuilder::default() + .typ(LinuxNamespaceType::Mount) + .path("/dev/null") + .build() + .unwrap(), + LinuxNamespaceBuilder::default() + .typ(LinuxNamespaceType::Network) + .path("/dev/null") + .build() + .unwrap(), + LinuxNamespaceBuilder::default() + .typ(LinuxNamespaceType::Pid) + .build() + .unwrap(), + LinuxNamespaceBuilder::default() + .typ(LinuxNamespaceType::User) + .build() + .unwrap(), + LinuxNamespaceBuilder::default() + .typ(LinuxNamespaceType::Ipc) + .build() + .unwrap(), ] } diff --git a/src/process/init.rs b/src/process/init.rs index c217139f..0b5327a7 100644 --- a/src/process/init.rs +++ b/src/process/init.rs @@ -164,13 +164,13 @@ pub fn container_init( ) -> Result<()> { let command = args.syscall; let spec = &args.spec; - let linux = spec.linux.as_ref().context("no linux in spec")?; - let proc = spec.process.as_ref().context("no process in spec")?; - let mut envs: Vec = proc.env.as_ref().unwrap_or(&vec![]).clone(); + let linux = spec.linux().as_ref().context("no linux in spec")?; + let proc = spec.process().as_ref().context("no process in spec")?; + let mut envs: Vec = proc.env().as_ref().unwrap_or(&vec![]).clone(); let rootfs = &args.rootfs; - let hooks = spec.hooks.as_ref(); + let hooks = spec.hooks().as_ref(); let container = args.container.as_ref(); - let namespaces = Namespaces::from(linux.namespaces.as_ref()); + let namespaces = Namespaces::from(linux.namespaces().as_ref()); // set up tty if specified if let Some(csocketfd) = args.console_socket { @@ -196,14 +196,14 @@ pub fn container_init( // Only set the host name if entering into a new uts namespace if let Some(uts_namespace) = namespaces.get(LinuxNamespaceType::Uts) { - if uts_namespace.path.is_none() { - if let Some(hostname) = spec.hostname.as_ref() { + if uts_namespace.path().is_none() { + if let Some(hostname) = spec.hostname().as_ref() { command.set_hostname(hostname)?; } } } - if let Some(true) = proc.no_new_privileges { + if let Some(true) = proc.no_new_privileges() { let _ = prctl::set_no_new_privileges(true); } @@ -211,7 +211,7 @@ pub fn container_init( // create_container hook needs to be called after the namespace setup, but // before pivot_root is called. This runs in the container namespaces. if let Some(hooks) = hooks { - hooks::run_hooks(hooks.create_container.as_ref(), container) + hooks::run_hooks(hooks.create_container().as_ref(), container) .context("Failed to run create container hooks")?; } @@ -237,18 +237,18 @@ pub fn container_init( rootfs::adjust_root_mount_propagation(linux) .context("Failed to set propagation type of root mount")?; - if let Some(kernel_params) = &linux.sysctl { + if let Some(kernel_params) = &linux.sysctl() { sysctl(kernel_params) .with_context(|| format!("Failed to sysctl: {:?}", kernel_params))?; } } - if let Some(profile) = &proc.apparmor_profile { + if let Some(profile) = &proc.apparmor_profile() { apparmor::apply_profile(profile) .with_context(|| format!("failed to apply apparmor profile {}", profile))?; } - if let Some(true) = spec.root.as_ref().map(|r| r.readonly.unwrap_or(false)) { + if let Some(true) = spec.root().as_ref().map(|r| r.readonly().unwrap_or(false)) { nix_mount( None::<&str>, "/", @@ -258,51 +258,54 @@ pub fn container_init( )? } - if let Some(paths) = &linux.readonly_paths { + if let Some(paths) = &linux.readonly_paths() { // mount readonly path for path in paths { readonly_path(path).context("Failed to set read only path")?; } } - if let Some(paths) = &linux.masked_paths { + if let Some(paths) = &linux.masked_paths() { // mount masked path for path in paths { - masked_path(path, &linux.mount_label).context("Failed to set masked path")?; + masked_path(path, linux.mount_label()).context("Failed to set masked path")?; } } - let cwd = format!("{}", proc.cwd.display()); + let cwd = format!("{}", proc.cwd().display()); let do_chdir = if cwd.is_empty() { false } else { // This chdir must run before setting up the user. // This may allow the user running youki to access directories // that the container user cannot access. - match unistd::chdir(&*proc.cwd) { + match unistd::chdir(&*proc.cwd()) { Ok(_) => false, Err(nix::Error::EPERM) => true, Err(e) => bail!("Failed to chdir: {}", e), } }; - set_supplementary_gids(&proc.user, &args.rootless) + set_supplementary_gids(proc.user(), &args.rootless) .context("failed to set supplementary gids")?; command - .set_id(Uid::from_raw(proc.user.uid), Gid::from_raw(proc.user.gid)) + .set_id( + Uid::from_raw(proc.user().uid()), + Gid::from_raw(proc.user().gid()), + ) .context("Failed to configure uid and gid")?; // Without no new privileges, seccomp is a privileged operation. We have to // do this before dropping capabilities. Otherwise, we should do it later, // as close to exec as possible. - if linux.seccomp.is_some() && proc.no_new_privileges.is_none() { - seccomp::initialize_seccomp(linux.seccomp.as_ref().unwrap()) + if linux.seccomp().is_some() && proc.no_new_privileges().is_none() { + seccomp::initialize_seccomp(linux.seccomp().as_ref().unwrap()) .context("Failed to execute seccomp")?; } capabilities::reset_effective(command).context("Failed to reset effective capabilities")?; - if let Some(caps) = &proc.capabilities { + if let Some(caps) = &proc.capabilities() { capabilities::drop_privileges(caps, command).context("Failed to drop capabilities")?; } @@ -351,8 +354,8 @@ pub fn container_init( // change directory to process.cwd if process.cwd is not empty if do_chdir { - unistd::chdir(&*proc.cwd) - .with_context(|| format!("Failed to chdir {}", proc.cwd.display()))?; + unistd::chdir(&*proc.cwd()) + .with_context(|| format!("Failed to chdir {}", proc.cwd().display()))?; } // Reset the process env based on oci spec. @@ -375,18 +378,18 @@ pub fn container_init( // before pivot_root is called. This runs in the container namespaces. if args.init { if let Some(hooks) = hooks { - hooks::run_hooks(hooks.start_container.as_ref(), container)? + hooks::run_hooks(hooks.start_container().as_ref(), container)? } } - if linux.seccomp.is_some() && proc.no_new_privileges.is_some() { + if linux.seccomp().is_some() && proc.no_new_privileges().is_some() { // Initialize seccomp profile right before we are ready to execute the // payload. The notify socket will still need network related syscalls. - seccomp::initialize_seccomp(linux.seccomp.as_ref().unwrap()) + seccomp::initialize_seccomp(linux.seccomp().as_ref().unwrap()) .context("Failed to execute seccomp")?; } - if let Some(args) = proc.args.as_ref() { + if let Some(args) = proc.args().as_ref() { utils::do_exec(&args[0], args)?; } else { bail!("On non-Windows, at least one process arg entry is required.") @@ -422,7 +425,7 @@ pub fn container_init( // Privileged user starting a normal container: Just add the supplementary groups. // fn set_supplementary_gids(user: &User, rootless: &Option) -> Result<()> { - if let Some(additional_gids) = &user.additional_gids { + if let Some(additional_gids) = &user.additional_gids() { if additional_gids.is_empty() { return Ok(()); } diff --git a/src/process/intermediate.rs b/src/process/intermediate.rs index e8ec071b..dcd7b154 100644 --- a/src/process/intermediate.rs +++ b/src/process/intermediate.rs @@ -13,8 +13,8 @@ pub fn container_intermediate( ) -> Result<()> { let command = &args.syscall; let spec = &args.spec; - let linux = spec.linux.as_ref().context("no linux in spec")?; - let namespaces = Namespaces::from(linux.namespaces.as_ref()); + let linux = spec.linux().as_ref().context("no linux in spec")?; + let namespaces = Namespaces::from(linux.namespaces().as_ref()); // if new user is specified in specification, this will be true and new // namespace will be created, check @@ -24,7 +24,7 @@ pub fn container_intermediate( namespaces .unshare_or_setns(user_namespace) .with_context(|| format!("Failed to enter pid namespace: {:?}", user_namespace))?; - if user_namespace.path.is_none() { + if user_namespace.path().is_none() { log::debug!("creating new user namespace"); // child needs to be dumpable, otherwise the non root parent is not // allowed to write the uid/gid maps @@ -46,8 +46,8 @@ pub fn container_intermediate( } // set limits and namespaces to the process - let proc = spec.process.as_ref().context("no process in spec")?; - if let Some(rlimits) = proc.rlimits.as_ref() { + let proc = spec.process().as_ref().context("no process in spec")?; + if let Some(rlimits) = proc.rlimits().as_ref() { for rlimit in rlimits.iter() { command.set_rlimit(rlimit).context("failed to set rlimit")?; } diff --git a/src/rootfs.rs b/src/rootfs.rs index 242fe1b0..5e21d64b 100644 --- a/src/rootfs.rs +++ b/src/rootfs.rs @@ -12,7 +12,7 @@ use nix::sys::stat::{Mode, SFlag}; use nix::unistd::{chown, close}; use nix::unistd::{Gid, Uid}; use nix::NixPath; -use oci_spec::runtime::{Linux, LinuxDevice, LinuxDeviceType, Mount, Spec}; +use oci_spec::runtime::{Linux, LinuxDevice, LinuxDeviceBuilder, LinuxDeviceType, Mount, Spec}; use procfs::process::{MountInfo, MountOptFields, Process}; use std::fs::OpenOptions; use std::fs::{canonicalize, create_dir_all, remove_file}; @@ -22,8 +22,8 @@ use std::path::{Path, PathBuf}; pub fn prepare_rootfs(spec: &Spec, rootfs: &Path, bind_devices: bool) -> Result<()> { log::debug!("Prepare rootfs: {:?}", rootfs); let mut flags = MsFlags::MS_REC; - let linux = spec.linux.as_ref().context("no linux in spec")?; - if let Some(roofs_propagation) = linux.rootfs_propagation.as_ref() { + let linux = spec.linux().as_ref().context("no linux in spec")?; + if let Some(roofs_propagation) = linux.rootfs_propagation().as_ref() { match roofs_propagation.as_str() { "shared" => flags |= MsFlags::MS_SHARED, "private" => flags |= MsFlags::MS_PRIVATE, @@ -49,15 +49,15 @@ pub fn prepare_rootfs(spec: &Spec, rootfs: &Path, bind_devices: bool) -> Result< None::<&str>, )?; - if let Some(mounts) = spec.mounts.as_ref() { + if let Some(mounts) = spec.mounts().as_ref() { for mount in mounts.iter() { log::debug!("Mount... {:?}", mount); let (flags, data) = parse_mount(mount); - let mount_label = linux.mount_label.as_ref(); - if mount.typ == Some("cgroup".to_string()) { + let mount_label = linux.mount_label().as_ref(); + if *mount.typ() == Some("cgroup".to_string()) { // skip log::warn!("A feature of cgroup is unimplemented."); - } else if mount.destination == PathBuf::from("/dev") { + } else if *mount.destination() == PathBuf::from("/dev") { mount_to_container( mount, rootfs, @@ -74,7 +74,7 @@ pub fn prepare_rootfs(spec: &Spec, rootfs: &Path, bind_devices: bool) -> Result< } setup_default_symlinks(rootfs).context("Failed to setup default symlinks")?; - if let Some(added_devices) = linux.devices.as_ref() { + if let Some(added_devices) = linux.devices().as_ref() { create_devices( rootfs, default_devices().iter().chain(added_devices), @@ -119,60 +119,54 @@ fn setup_default_symlinks(rootfs: &Path) -> Result<()> { pub fn default_devices() -> Vec { vec![ - LinuxDevice { - path: PathBuf::from("/dev/null"), - typ: LinuxDeviceType::C, - major: 1, - minor: 3, - file_mode: Some(0o066), - uid: None, - gid: None, - }, - LinuxDevice { - path: PathBuf::from("/dev/zero"), - typ: LinuxDeviceType::C, - major: 1, - minor: 5, - file_mode: Some(0o066), - uid: None, - gid: None, - }, - LinuxDevice { - path: PathBuf::from("/dev/full"), - typ: LinuxDeviceType::C, - major: 1, - minor: 7, - file_mode: Some(0o066), - uid: None, - gid: None, - }, - LinuxDevice { - path: PathBuf::from("/dev/tty"), - typ: LinuxDeviceType::C, - major: 5, - minor: 0, - file_mode: Some(0o066), - uid: None, - gid: None, - }, - LinuxDevice { - path: PathBuf::from("/dev/urandom"), - typ: LinuxDeviceType::C, - major: 1, - minor: 9, - file_mode: Some(0o066), - uid: None, - gid: None, - }, - LinuxDevice { - path: PathBuf::from("/dev/random"), - typ: LinuxDeviceType::C, - major: 1, - minor: 8, - file_mode: Some(0o066), - uid: None, - gid: None, - }, + LinuxDeviceBuilder::default() + .path(PathBuf::from("/dev/null")) + .typ(LinuxDeviceType::C) + .major(1) + .minor(3) + .file_mode(0o066u32) + .build() + .unwrap(), + LinuxDeviceBuilder::default() + .path(PathBuf::from("/dev/zero")) + .typ(LinuxDeviceType::C) + .major(1) + .minor(5) + .file_mode(0o066u32) + .build() + .unwrap(), + LinuxDeviceBuilder::default() + .path(PathBuf::from("/dev/full")) + .typ(LinuxDeviceType::C) + .major(1) + .minor(7) + .file_mode(0o066u32) + .build() + .unwrap(), + LinuxDeviceBuilder::default() + .path(PathBuf::from("/dev/tty")) + .typ(LinuxDeviceType::C) + .major(5) + .minor(0) + .file_mode(0o066u32) + .build() + .unwrap(), + LinuxDeviceBuilder::default() + .path(PathBuf::from("/dev/urandom")) + .typ(LinuxDeviceType::C) + .major(1) + .minor(9) + .file_mode(0o066u32) + .build() + .unwrap(), + LinuxDeviceBuilder::default() + .path(PathBuf::from("/dev/random")) + .typ(LinuxDeviceType::C) + .major(1) + .minor(8) + .file_mode(0o066u32) + .build() + .unwrap(), ] } @@ -184,8 +178,8 @@ where if bind { let _ = devices .map(|dev| { - if !dev.path.starts_with("/dev") { - panic!("{} is not a valid device path", dev.path.display()); + if !dev.path().starts_with("/dev") { + panic!("{} is not a valid device path", dev.path().display()); } bind_dev(rootfs, dev) @@ -194,8 +188,8 @@ where } else { devices .map(|dev| { - if !dev.path.starts_with("/dev") { - panic!("{} is not a valid device path", dev.path.display()); + if !dev.path().starts_with("/dev") { + panic!("{} is not a valid device path", dev.path().display()); } mknod_dev(rootfs, dev) @@ -208,7 +202,7 @@ where } fn bind_dev(rootfs: &Path, dev: &LinuxDevice) -> Result<()> { - let full_container_path = rootfs.join(dev.path.as_in_container()?); + let full_container_path = rootfs.join(dev.path().as_in_container()?); let fd = open( &full_container_path, @@ -217,7 +211,7 @@ fn bind_dev(rootfs: &Path, dev: &LinuxDevice) -> Result<()> { )?; close(fd)?; nix_mount( - Some(&dev.path), + Some(dev.path()), &full_container_path, Some("bind"), MsFlags::MS_BIND, @@ -244,17 +238,17 @@ fn mknod_dev(rootfs: &Path, dev: &LinuxDevice) -> Result<()> { | ((major & !0xfff) << 32)) as u64 } - let full_container_path = rootfs.join(dev.path.as_in_container()?); + let full_container_path = rootfs.join(dev.path().as_in_container()?); mknod( &full_container_path, - to_sflag(dev.typ), - Mode::from_bits_truncate(dev.file_mode.unwrap_or(0)), - makedev(dev.major, dev.minor), + to_sflag(dev.typ()), + Mode::from_bits_truncate(dev.file_mode().unwrap_or(0)), + makedev(dev.major(), dev.minor()), )?; chown( &full_container_path, - dev.uid.map(Uid::from_raw), - dev.gid.map(Gid::from_raw), + dev.uid().map(Uid::from_raw), + dev.gid().map(Gid::from_raw), )?; Ok(()) @@ -267,7 +261,7 @@ fn mount_to_container( data: &str, label: Option<&String>, ) -> Result<()> { - let typ = m.typ.as_deref(); + let typ = m.typ().as_deref(); let d = if let Some(l) = label { if typ != Some("proc") && typ != Some("sysfs") { if data.is_empty() { @@ -284,10 +278,10 @@ fn mount_to_container( let dest_for_host = format!( "{}{}", rootfs.to_string_lossy().into_owned(), - m.destination.display() + m.destination().display() ); let dest = Path::new(&dest_for_host); - let source = m.source.as_ref().context("no source in mount spec")?; + let source = m.source().as_ref().context("no source in mount spec")?; let src = if typ == Some("bind") { let src = canonicalize(source)?; let dir = if src.is_file() { @@ -313,7 +307,7 @@ fn mount_to_container( if let Err(errno) = nix_mount(Some(&*src), dest, typ, flags, Some(&*d)) { if !matches!(errno, Errno::EINVAL) { - bail!("mount of {:?} failed", m.destination); + bail!("mount of {:?} failed", m.destination()); } nix_mount(Some(&*src), dest, typ, flags, Some(data))?; } @@ -342,7 +336,7 @@ fn mount_to_container( fn parse_mount(m: &Mount) -> (MsFlags, String) { let mut flags = MsFlags::empty(); let mut data = Vec::new(); - if let Some(options) = &m.options { + if let Some(options) = &m.options() { for s in options { if let Some((is_clear, flag)) = match s.as_str() { "defaults" => Some((false, MsFlags::empty())), @@ -430,7 +424,7 @@ fn make_parent_mount_private(rootfs: &Path) -> Result<()> { /// Change propagation type of rootfs as specified in spec. pub fn adjust_root_mount_propagation(linux: &Linux) -> Result<()> { - let rootfs_propagation = linux.rootfs_propagation.as_deref(); + let rootfs_propagation = linux.rootfs_propagation().as_deref(); let flags = match rootfs_propagation { Some("shared") => Some(MsFlags::MS_SHARED), Some("unbindable") => Some(MsFlags::MS_UNBINDABLE), diff --git a/src/rootless.rs b/src/rootless.rs index aac5651f..30fd0275 100644 --- a/src/rootless.rs +++ b/src/rootless.rs @@ -24,8 +24,8 @@ pub struct Rootless<'a> { impl<'a> Rootless<'a> { pub fn new(spec: &'a Spec) -> Result>> { - let linux = spec.linux.as_ref().context("no linux in spec")?; - let namespaces = Namespaces::from(linux.namespaces.as_ref()); + let linux = spec.linux().as_ref().context("no linux in spec")?; + let namespaces = Namespaces::from(linux.namespaces().as_ref()); let user_namespace = namespaces.get(LinuxNamespaceType::User); // If conditions requires us to use rootless, we must either create a new @@ -34,7 +34,7 @@ impl<'a> Rootless<'a> { bail!("rootless container requires valid user namespace definition"); } - if user_namespace.is_some() && user_namespace.unwrap().path.is_none() { + if user_namespace.is_some() && user_namespace.unwrap().path().is_none() { log::debug!("rootless container should be created"); log::warn!( "resource constraints and multi id mapping is unimplemented for rootless containers" @@ -83,13 +83,13 @@ impl<'a> Rootless<'a> { impl<'a> From<&'a Linux> for Rootless<'a> { fn from(linux: &'a Linux) -> Self { - let namespaces = Namespaces::from(linux.namespaces.as_ref()); + let namespaces = Namespaces::from(linux.namespaces().as_ref()); let user_namespace = namespaces.get(LinuxNamespaceType::User); Self { newuidmap: None, newgidmap: None, - uid_mappings: linux.uid_mappings.as_ref(), - gid_mappings: linux.gid_mappings.as_ref(), + uid_mappings: linux.uid_mappings().as_ref(), + gid_mappings: linux.gid_mappings().as_ref(), user_namespace: user_namespace.cloned(), privileged: nix::unistd::geteuid().is_root(), } @@ -112,18 +112,18 @@ pub fn rootless_required() -> bool { /// Validates that the spec contains the required information for /// running in rootless mode fn validate(spec: &Spec) -> Result<()> { - let linux = spec.linux.as_ref().context("no linux in spec")?; - let namespaces = Namespaces::from(linux.namespaces.as_ref()); + let linux = spec.linux().as_ref().context("no linux in spec")?; + let namespaces = Namespaces::from(linux.namespaces().as_ref()); if namespaces.get(LinuxNamespaceType::User).is_none() { bail!("rootless containers require the specification of a user namespace"); } let gid_mappings = linux - .gid_mappings + .gid_mappings() .as_ref() .context("rootless containers require gidMappings in spec")?; let uid_mappings = linux - .uid_mappings + .uid_mappings() .as_ref() .context("rootless containers require uidMappings in spec")?; @@ -136,13 +136,13 @@ fn validate(spec: &Spec) -> Result<()> { } validate_mounts( - spec.mounts.as_ref().context("no mounts in spec")?, + spec.mounts().as_ref().context("no mounts in spec")?, uid_mappings, gid_mappings, )?; - if let Some(process) = &spec.process { - if let Some(additional_gids) = &process.user.additional_gids { + if let Some(process) = &spec.process() { + if let Some(additional_gids) = &process.user().additional_gids() { let privileged = nix::unistd::geteuid().is_root(); match (privileged, additional_gids.is_empty()) { @@ -174,7 +174,7 @@ fn validate_mounts( gid_mappings: &[LinuxIdMapping], ) -> Result<()> { for mount in mounts { - if let Some(options) = &mount.options { + if let Some(options) = &mount.options() { for opt in options { if opt.starts_with("uid=") && !is_id_mapped(opt[4..].parse()?, uid_mappings) { bail!("Mount {:?} specifies option {} which is not mapped inside the rootless container", mount, opt); @@ -193,13 +193,13 @@ fn validate_mounts( fn is_id_mapped(id: u32, mappings: &[LinuxIdMapping]) -> bool { mappings .iter() - .any(|m| id >= m.container_id && id <= m.container_id + m.size) + .any(|m| id >= m.container_id() && id <= m.container_id() + m.size()) } /// Looks up the location of the newuidmap and newgidmap binaries which /// are required to write multiple user/group mappings pub fn lookup_map_binaries(spec: &Linux) -> Result> { - if let Some(uid_mappings) = spec.uid_mappings.as_ref() { + if let Some(uid_mappings) = spec.uid_mappings().as_ref() { if uid_mappings.len() == 1 && uid_mappings.len() == 1 { return Ok(None); } @@ -231,7 +231,7 @@ fn write_id_mapping( ) -> Result<()> { let mappings: Vec = mappings .iter() - .map(|m| format!("{} {} {}", m.container_id, m.host_id, m.size)) + .map(|m| format!("{} {} {}", m.container_id(), m.host_id(), m.size())) .collect(); log::debug!("Write ID mapping: {:?}", mappings); if mappings.len() == 1 { diff --git a/src/seccomp/mod.rs b/src/seccomp/mod.rs index c7690534..048b1e2f 100644 --- a/src/seccomp/mod.rs +++ b/src/seccomp/mod.rs @@ -201,16 +201,16 @@ fn translate_arch(arch: &Arch) -> scmp_arch { } pub fn initialize_seccomp(seccomp: &LinuxSeccomp) -> Result<()> { - if seccomp.flags.is_some() { + if seccomp.flags().is_some() { // runc did not support this, so let's skip it for now. bail!("seccomp flags are not yet supported"); } // TODO: fix default action error number. The spec repo doesn't have it yet. - let default_action = translate_action(&seccomp.default_action, None); + let default_action = translate_action(&seccomp.default_action(), None); let mut ctx = FilterContext::default(default_action)?; - if let Some(architectures) = seccomp.architectures.as_ref() { + if let Some(architectures) = seccomp.architectures().as_ref() { for arch in architectures { let arch_token = translate_arch(arch); ctx.add_arch(arch_token as u32) @@ -233,9 +233,9 @@ pub fn initialize_seccomp(seccomp: &LinuxSeccomp) -> Result<()> { ); } - if let Some(syscalls) = seccomp.syscalls.as_ref() { + if let Some(syscalls) = seccomp.syscalls().as_ref() { for syscall in syscalls { - let action = translate_action(&syscall.action, syscall.errno_ret); + let action = translate_action(&syscall.action(), syscall.errno_ret()); if action == default_action { // When the action is the same as the default action, the rule is redundent. We can // skip this here to avoid failing when we add the rules. @@ -246,7 +246,7 @@ pub fn initialize_seccomp(seccomp: &LinuxSeccomp) -> Result<()> { continue; } - for name in &syscall.names { + for name in syscall.names().iter() { let syscall_number = match translate_syscall(name) { Ok(x) => x, Err(_) => { @@ -262,14 +262,14 @@ pub fn initialize_seccomp(seccomp: &LinuxSeccomp) -> Result<()> { // Not clear why but if there are multiple arg attached to one // syscall rule, we have to add them seperatly. add_rule will // return EINVAL. runc does the same but doesn't explain why. - match syscall.args.as_ref() { + match syscall.args().as_ref() { Some(args) => { for arg in args { let mut rule = Rule::new(action, syscall_number); - let cmp = Compare::new(arg.index as u32) - .op(translate_op(&arg.op)) - .datum_a(arg.value) - .datum_b(arg.value_two.unwrap_or(0)) + let cmp = Compare::new(arg.index() as u32) + .op(translate_op(&arg.op())) + .datum_a(arg.value()) + .datum_b(arg.value_two().unwrap_or(0)) .build() .context("Failed to build a seccomp compare rule")?; rule.add_comparator(cmp); @@ -310,7 +310,8 @@ mod tests { use anyhow::Result; use mio::unix::pipe; use nix::sys::wait; - use oci_spec::runtime::{Arch, LinuxSeccomp, LinuxSyscall}; + use oci_spec::runtime::Arch; + use oci_spec::runtime::{LinuxSeccompBuilder, LinuxSyscallBuilder}; use serial_test::serial; use std::io::Read; use std::io::Write; @@ -332,17 +333,16 @@ mod tests { // we can make sure that getcwd failed because of seccomp rule. let expect_error = libc::EAGAIN; - let seccomp_profile = LinuxSeccomp { - default_action: LinuxSeccompAction::ScmpActAllow, - architectures: Some(vec![Arch::ScmpArchNative]), - flags: None, - syscalls: Some(vec![LinuxSyscall { - names: vec![String::from("getcwd")], - action: LinuxSeccompAction::ScmpActErrno, - errno_ret: Some(expect_error as u32), - args: None, - }]), - }; + let syscall = LinuxSyscallBuilder::default() + .names(vec![String::from("getcwd")]) + .action(LinuxSeccompAction::ScmpActErrno) + .errno_ret(expect_error as u32) + .build()?; + let seccomp_profile = LinuxSeccompBuilder::default() + .default_action(LinuxSeccompAction::ScmpActAllow) + .architectures(vec![Arch::ScmpArchNative]) + .syscalls(vec![syscall]) + .build()?; // Since Rust cargo test uses a single process to execute all tests, it // is a good idea to fork a child process to test the seccomp profile, @@ -391,7 +391,7 @@ mod tests { .context("Failed to load test spec for seccomp")?; // We know linux and seccomp exist, so let's just unwrap. - let seccomp_profile = spec.linux.unwrap().seccomp.unwrap(); + let seccomp_profile = spec.linux().as_ref().unwrap().seccomp().as_ref().unwrap(); match unsafe { nix::unistd::fork()? } { nix::unistd::ForkResult::Parent { child } => { let status = wait::waitpid(child, None)?; @@ -409,7 +409,7 @@ mod tests { } nix::unistd::ForkResult::Child => { let _ = prctl::set_no_new_privileges(true); - let ret = initialize_seccomp(&seccomp_profile); + let ret = initialize_seccomp(seccomp_profile); let exit_code = if ret.is_ok() { 0 } else { -1 }; std::process::exit(exit_code); } diff --git a/src/syscall/linux.rs b/src/syscall/linux.rs index 8ff9f0bc..b5c5d33a 100644 --- a/src/syscall/linux.rs +++ b/src/syscall/linux.rs @@ -162,12 +162,12 @@ impl Syscall for LinuxSyscall { /// Sets resource limit for process fn set_rlimit(&self, rlimit: &LinuxRlimit) -> Result<()> { let rlim = &libc::rlimit { - rlim_cur: rlimit.soft, - rlim_max: rlimit.hard, + rlim_cur: rlimit.soft(), + rlim_max: rlimit.hard(), }; - let res = unsafe { libc::setrlimit(rlimit.typ as u32, rlim) }; + let res = unsafe { libc::setrlimit(rlimit.typ() as u32, rlim) }; if let Err(e) = Errno::result(res).map(drop) { - bail!("Failed to set {:?}. {:?}", rlimit.typ, e) + bail!("Failed to set {:?}. {:?}", rlimit.typ(), e) } Ok(()) }