Move cgroup config into youki config

Signed-off-by: Aidan Hobson Sayers <aidanhs@cantab.net>

crates/libcgroups/src/common.rs

@@ -11,6 +11,7 @@ use oci_spec::runtime::LinuxResources;
 use oci_spec::runtime::{
     LinuxDevice, LinuxDeviceBuilder, LinuxDeviceCgroup, LinuxDeviceCgroupBuilder, LinuxDeviceType,
 };
+use serde::{Deserialize, Serialize};
 
 use super::stats::Stats;
 use super::{systemd, v1, v2};
@@ -326,7 +327,7 @@ pub enum CreateCgroupSetupError {
     Systemd(#[from] systemd::manager::SystemdManagerError),
 }
 
-#[derive(Clone)]
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
 pub struct CgroupConfig {
     pub cgroup_path: PathBuf,
     pub systemd_cgroup: bool,

crates/libcontainer/src/config.rs

@@ -5,8 +5,6 @@ use std::path::{Path, PathBuf};
 use oci_spec::runtime::{Hooks, Spec};
 use serde::{Deserialize, Serialize};
 
-use crate::utils;
-
 #[derive(Debug, thiserror::Error)]
 pub enum ConfigError {
     #[error("failed to save config")]
@@ -43,20 +41,14 @@ const YOUKI_CONFIG_NAME: &str = "youki_config.json";
 #[non_exhaustive]
 pub struct YoukiConfig {
     pub hooks: Option<Hooks>,
-    pub cgroup_path: PathBuf,
+    pub cgroup_config: libcgroups::common::CgroupConfig,
 }
 
 impl<'a> YoukiConfig {
-    pub fn from_spec(spec: &'a Spec, container_id: &str) -> Result<Self> {
+    pub fn from_spec(spec: &'a Spec, cgroup_config: libcgroups::common::CgroupConfig) -> Result<Self> {
         Ok(YoukiConfig {
             hooks: spec.hooks().clone(),
-            cgroup_path: utils::get_cgroup_path(
-                spec.linux()
-                    .as_ref()
-                    .ok_or(ConfigError::MissingLinux)?
-                    .cgroups_path(),
-                container_id,
-            ),
+            cgroup_config,
         })
     }
 
@@ -106,13 +98,15 @@ mod tests {
     fn test_config_from_spec() -> Result<()> {
         let container_id = "sample";
         let spec = Spec::default();
+        let cgroup_config = libcgroups::common::CgroupConfig {
+            cgroup_path: PathBuf::from(format!(":youki:{container_id}")),
+            systemd_cgroup: true,
+            container_name: container_id.to_owned(),
+        };
         let config = YoukiConfig::from_spec(&spec, container_id)?;
         assert_eq!(&config.hooks, spec.hooks());
-        dbg!(&config.cgroup_path);
-        assert_eq!(
-            config.cgroup_path,
-            PathBuf::from(format!(":youki:{container_id}"))
-        );
+        dbg!(&config.cgroup_config);
+        assert_eq!(config.cgroup_config, cgroup_config);
         Ok(())
     }
 

crates/libcontainer/src/container/builder_impl.rs

@@ -8,7 +8,7 @@ use libcgroups::common::CgroupManager;
 use nix::unistd::Pid;
 use oci_spec::runtime::Spec;
 
-use super::{Container, ContainerStatus};
+use super::ContainerStatus;
 use crate::error::{LibcontainerError, MissingSpecError};
 use crate::notify_socket::NotifyListener;
 use crate::process::args::{ContainerArgs, ContainerType};
@@ -17,17 +17,15 @@ use crate::process::{self};
 use crate::syscall::syscall::SyscallType;
 use crate::user_ns::UserNamespaceConfig;
 use crate::workload::Executor;
-use crate::{hooks, utils};
+use crate::hooks;
 
 pub(super) struct ContainerBuilderImpl {
     /// Flag indicating if an init or a tenant container should be created
     pub container_type: ContainerType,
     /// Interface to operating system primitives
     pub syscall: SyscallType,
-    /// Flag indicating if systemd should be used for cgroup management
-    pub use_systemd: bool,
-    /// Id of the container
-    pub container_id: String,
+    /// Interface to operating system primitives
+    pub cgroup_config: libcgroups::common::CgroupConfig,
     /// OCI compliant runtime spec
     pub spec: Rc<Spec>,
     /// Root filesystem of the container
@@ -41,8 +39,6 @@ pub(super) struct ContainerBuilderImpl {
     pub user_ns_config: Option<UserNamespaceConfig>,
     /// Path to the Unix Domain Socket to communicate container start
     pub notify_path: PathBuf,
-    /// Container state
-    pub container: Option<Container>,
     /// File descriptos preserved/passed to the container init process.
     pub preserve_fds: i32,
     /// If the container is to be run in detached mode
@@ -58,7 +54,7 @@ impl ContainerBuilderImpl {
             Err(outer) => {
                 // Only the init container should be cleaned up in the case of
                 // an error.
-                if matches!(self.container_type, ContainerType::InitContainer) {
+                if matches!(self.container_type, ContainerType::InitContainer { .. }) {
                     self.cleanup_container()?;
                 }
 
@@ -68,24 +64,17 @@ impl ContainerBuilderImpl {
     }
 
     fn run_container(&mut self) -> Result<Pid, LibcontainerError> {
-        let linux = self.spec.linux().as_ref().ok_or(MissingSpecError::Linux)?;
-        let cgroups_path = utils::get_cgroup_path(linux.cgroups_path(), &self.container_id);
-        let cgroup_config = libcgroups::common::CgroupConfig {
-            cgroup_path: cgroups_path,
-            systemd_cgroup: self.use_systemd || self.user_ns_config.is_some(),
-            container_name: self.container_id.to_owned(),
-        };
         let process = self
             .spec
             .process()
             .as_ref()
             .ok_or(MissingSpecError::Process)?;
 
-        if matches!(self.container_type, ContainerType::InitContainer) {
+        if let ContainerType::InitContainer { container } = &self.container_type {
             if let Some(hooks) = self.spec.hooks() {
                 hooks::run_hooks(
                     hooks.create_runtime().as_ref(),
-                    self.container.as_ref(),
+                    container,
                     None,
                 )?
             }
@@ -129,6 +118,7 @@ impl ContainerBuilderImpl {
         // 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.
+        let linux = self.spec.linux().as_ref().ok_or(MissingSpecError::Linux)?;
         if linux.namespaces().is_some() {
             prctl::set_dumpable(false).map_err(|e| {
                 LibcontainerError::Other(format!(
@@ -142,16 +132,15 @@ impl ContainerBuilderImpl {
         // therefore we will have to move all the variable by value. Since self
         // is a shared reference, we have to clone these variables here.
         let container_args = ContainerArgs {
-            container_type: self.container_type,
+            container_type: self.container_type.clone(),
             syscall: self.syscall,
             spec: Rc::clone(&self.spec),
             rootfs: self.rootfs.to_owned(),
             console_socket: self.console_socket,
             notify_listener,
             preserve_fds: self.preserve_fds,
-            container: self.container.to_owned(),
             user_ns_config: self.user_ns_config.to_owned(),
-            cgroup_config,
+            cgroup_config: self.cgroup_config.clone(),
             detached: self.detached,
             executor: self.executor.clone(),
         };
@@ -172,7 +161,7 @@ impl ContainerBuilderImpl {
             })?;
         }
 
-        if let Some(container) = &mut self.container {
+        if let ContainerType::InitContainer { container } = &mut self.container_type {
             // update status and pid of the container process
             container
                 .set_status(ContainerStatus::Created)
@@ -186,23 +175,15 @@ impl ContainerBuilderImpl {
     }
 
     fn cleanup_container(&self) -> Result<(), LibcontainerError> {
-        let linux = self.spec.linux().as_ref().ok_or(MissingSpecError::Linux)?;
-        let cgroups_path = utils::get_cgroup_path(linux.cgroups_path(), &self.container_id);
-        let cmanager =
-            libcgroups::common::create_cgroup_manager(libcgroups::common::CgroupConfig {
-                cgroup_path: cgroups_path,
-                systemd_cgroup: self.use_systemd || self.user_ns_config.is_some(),
-                container_name: self.container_id.to_string(),
-            })?;
-
         let mut errors = Vec::new();
 
+        let cmanager = libcgroups::common::create_cgroup_manager(self.cgroup_config.clone())?;
         if let Err(e) = cmanager.remove() {
             tracing::error!(error = ?e, "failed to remove cgroup manager");
             errors.push(e.to_string());
         }
 
-        if let Some(container) = &self.container {
+        if let ContainerType::InitContainer { container } = &self.container_type {
             if let Some(true) = container.clean_up_intel_rdt_subdirectory() {
                 if let Err(e) = delete_resctrl_subdirectory(container.id()) {
                     tracing::error!(id = ?container.id(), error = ?e, "failed to delete resctrl subdirectory");

crates/libcontainer/src/container/container.rs

@@ -121,15 +121,6 @@ impl Container {
         self
     }
 
-    pub fn systemd(&self) -> bool {
-        self.state.use_systemd
-    }
-
-    pub fn set_systemd(&mut self, should_use: bool) -> &mut Self {
-        self.state.use_systemd = should_use;
-        self
-    }
-
     pub fn set_clean_up_intel_rdt_directory(&mut self, clean_up: bool) -> &mut Self {
         self.state.clean_up_intel_rdt_subdirectory = Some(clean_up);
         self

crates/libcontainer/src/container/container_delete.rs

@@ -5,7 +5,6 @@ use libcgroups::{self};
 use nix::sys::signal;
 
 use super::{Container, ContainerStatus};
-use crate::config::YoukiConfig;
 use crate::error::LibcontainerError;
 use crate::hooks;
 use crate::process::intel_rdt::delete_resctrl_subdirectory;
@@ -78,27 +77,21 @@ impl Container {
         }
 
         if self.root.exists() {
-            match YoukiConfig::load(&self.root) {
+            match self.spec() {
                 Ok(config) => {
                     tracing::debug!("config: {:?}", config);
 
                     // remove the cgroup created for the container
                     // check https://man7.org/linux/man-pages/man7/cgroups.7.html
                     // creating and removing cgroups section for more information on cgroups
-                    let cmanager = libcgroups::common::create_cgroup_manager(
-                        libcgroups::common::CgroupConfig {
-                            cgroup_path: config.cgroup_path.to_owned(),
-                            systemd_cgroup: self.systemd(),
-                            container_name: self.id().to_string(),
-                        },
-                    )?;
+                    let cmanager = libcgroups::common::create_cgroup_manager(config.cgroup_config.clone())?;
                     cmanager.remove().map_err(|err| {
-                        tracing::error!(cgroup_path = ?config.cgroup_path, "failed to remove cgroup due to: {err:?}");
+                        tracing::error!(cgroup_config = ?config.cgroup_config, "failed to remove cgroup due to: {err:?}");
                         err
                     })?;
 
                     if let Some(hooks) = config.hooks.as_ref() {
-                        hooks::run_hooks(hooks.poststop().as_ref(), Some(self), None).map_err(
+                        hooks::run_hooks(hooks.poststop().as_ref(), self, None).map_err(
                             |err| {
                                 tracing::error!(err = ?err, "failed to run post stop hooks");
                                 err

crates/libcontainer/src/container/container_events.rs

@@ -35,11 +35,7 @@ impl Container {
         }
 
         let cgroup_manager =
-            libcgroups::common::create_cgroup_manager(libcgroups::common::CgroupConfig {
-                cgroup_path: self.spec()?.cgroup_path,
-                systemd_cgroup: self.systemd(),
-                container_name: self.id().to_string(),
-            })?;
+            libcgroups::common::create_cgroup_manager(self.spec()?.cgroup_config)?;
         match stats {
             true => {
                 let stats = cgroup_manager.stats()?;

crates/libcontainer/src/container/container_kill.rs

@@ -83,11 +83,7 @@ impl Container {
                 libcgroups::common::CgroupSetup::Legacy
                 | libcgroups::common::CgroupSetup::Hybrid => {
                     let cmanager = libcgroups::common::create_cgroup_manager(
-                        libcgroups::common::CgroupConfig {
-                            cgroup_path: self.spec()?.cgroup_path,
-                            systemd_cgroup: self.systemd(),
-                            container_name: self.id().to_string(),
-                        },
+                        self.spec()?.cgroup_config
                     )?;
                     cmanager.freeze(libcgroups::common::FreezerState::Thawed)?;
                 }
@@ -100,11 +96,7 @@ impl Container {
     fn kill_all_processes<S: Into<Signal>>(&self, signal: S) -> Result<(), LibcontainerError> {
         let signal = signal.into().into_raw();
         let cmanager =
-            libcgroups::common::create_cgroup_manager(libcgroups::common::CgroupConfig {
-                cgroup_path: self.spec()?.cgroup_path,
-                systemd_cgroup: self.systemd(),
-                container_name: self.id().to_string(),
-            })?;
+            libcgroups::common::create_cgroup_manager(self.spec()?.cgroup_config)?;
 
         if let Err(e) = cmanager.freeze(libcgroups::common::FreezerState::Frozen) {
             tracing::warn!(

crates/libcontainer/src/container/container_pause.rs

@@ -33,11 +33,7 @@ impl Container {
         }
 
         let cmanager =
-            libcgroups::common::create_cgroup_manager(libcgroups::common::CgroupConfig {
-                cgroup_path: self.spec()?.cgroup_path,
-                systemd_cgroup: self.systemd(),
-                container_name: self.id().to_string(),
-            })?;
+            libcgroups::common::create_cgroup_manager(self.spec()?.cgroup_config)?;
         cmanager.freeze(FreezerState::Frozen)?;
 
         tracing::debug!("saving paused status");

crates/libcontainer/src/container/container_resume.rs

@@ -34,11 +34,7 @@ impl Container {
         }
 
         let cmanager =
-            libcgroups::common::create_cgroup_manager(libcgroups::common::CgroupConfig {
-                cgroup_path: self.spec()?.cgroup_path,
-                systemd_cgroup: self.systemd(),
-                container_name: self.id().to_string(),
-            })?;
+            libcgroups::common::create_cgroup_manager(self.spec()?.cgroup_config)?;
         // resume the frozen container
         cmanager.freeze(FreezerState::Thawed)?;
 

crates/libcontainer/src/container/container_start.rs

@@ -1,7 +1,6 @@
 use nix::sys::signal;
 
 use super::{Container, ContainerStatus};
-use crate::config::YoukiConfig;
 use crate::error::LibcontainerError;
 use crate::hooks;
 use crate::notify_socket::{NotifySocket, NOTIFY_FILE};
@@ -35,7 +34,7 @@ impl Container {
             return Err(LibcontainerError::IncorrectStatus);
         }
 
-        let config = YoukiConfig::load(&self.root).map_err(|err| {
+        let config = self.spec().map_err(|err| {
             tracing::error!(
                 "failed to load runtime spec for container {}: {}",
                 self.id(),
@@ -47,7 +46,7 @@ impl Container {
             // 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), None).map_err(|err| {
+            hooks::run_hooks(hooks.prestart().as_ref(), self, None).map_err(|err| {
                 tracing::error!("failed to run pre start hooks: {}", err);
                 // In the case where prestart hook fails, the runtime must
                 // stop the container before generating an error and exiting.
@@ -69,7 +68,7 @@ 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) = config.hooks.as_ref() {
-            hooks::run_hooks(hooks.poststart().as_ref(), Some(self), Some(&self.root)).map_err(
+            hooks::run_hooks(hooks.poststart().as_ref(), self, Some(&self.root)).map_err(
                 |err| {
                     tracing::error!("failed to run post start hooks: {}", err);
                     err