Allow entirely disabling cgroup manipulation

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

crates/libcontainer/src/config.rs

@@ -41,11 +41,11 @@ const YOUKI_CONFIG_NAME: &str = "youki_config.json";
 #[non_exhaustive]
 pub struct YoukiConfig {
     pub hooks: Option<Hooks>,
-    pub cgroup_config: libcgroups::common::CgroupConfig,
+    pub cgroup_config: Option<libcgroups::common::CgroupConfig>,
 }
 
 impl<'a> YoukiConfig {
-    pub fn from_spec(spec: &'a Spec, cgroup_config: libcgroups::common::CgroupConfig) -> Result<Self> {
+    pub fn from_spec(spec: &'a Spec, cgroup_config: Option<libcgroups::common::CgroupConfig>) -> Result<Self> {
         Ok(YoukiConfig {
             hooks: spec.hooks().clone(),
             cgroup_config,

crates/libcontainer/src/container/builder_impl.rs

@@ -25,7 +25,7 @@ pub(super) struct ContainerBuilderImpl {
     /// Interface to operating system primitives
     pub syscall: SyscallType,
     /// Interface to operating system primitives
-    pub cgroup_config: libcgroups::common::CgroupConfig,
+    pub cgroup_config: Option<libcgroups::common::CgroupConfig>,
     /// OCI compliant runtime spec
     pub spec: Rc<Spec>,
     /// Root filesystem of the container
@@ -177,10 +177,12 @@ impl ContainerBuilderImpl {
     fn cleanup_container(&self) -> Result<(), LibcontainerError> {
         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(cc) = &self.cgroup_config {
+            let cmanager = libcgroups::common::create_cgroup_manager(cc.to_owned())?;
+            if let Err(e) = cmanager.remove() {
+                tracing::error!(error = ?e, "failed to remove cgroup manager");
+                errors.push(e.to_string());
+            }
         }
 
         if let ContainerType::InitContainer { container } = &self.container_type {

crates/libcontainer/src/container/container_delete.rs

@@ -84,11 +84,13 @@ impl Container {
                     // 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(config.cgroup_config.clone())?;
-                    cmanager.remove().map_err(|err| {
-                        tracing::error!(cgroup_config = ?config.cgroup_config, "failed to remove cgroup due to: {err:?}");
-                        err
-                    })?;
+                    if let Some(cc) = config.cgroup_config {
+                        let cmanager = libcgroups::common::create_cgroup_manager(cc.clone())?;
+                        cmanager.remove().map_err(|err| {
+                            tracing::error!(cgroup_config = ?cc, "failed to remove cgroup due to: {err:?}");
+                            err
+                        })?;
+                    }
 
                     if let Some(hooks) = config.hooks.as_ref() {
                         hooks::run_hooks(hooks.poststop().as_ref(), self, None).map_err(

crates/libcontainer/src/container/container_events.rs

@@ -34,8 +34,13 @@ impl Container {
             return Err(LibcontainerError::IncorrectStatus);
         }
 
+        let cgroup_config = match self.spec()?.cgroup_config {
+            Some(cc) => cc,
+            None => return Err(LibcontainerError::CgroupsMissing),
+        };
+
         let cgroup_manager =
-            libcgroups::common::create_cgroup_manager(self.spec()?.cgroup_config)?;
+            libcgroups::common::create_cgroup_manager(cgroup_config)?;
         match stats {
             true => {
                 let stats = cgroup_manager.stats()?;

crates/libcontainer/src/container/container_kill.rs

@@ -79,24 +79,33 @@ impl Container {
         // For cgroup V1, a frozon process cannot respond to signals,
         // so we need to thaw it. Only thaw the cgroup for SIGKILL.
         if self.status() == ContainerStatus::Paused && signal == signal::Signal::SIGKILL {
-            match get_cgroup_setup()? {
-                libcgroups::common::CgroupSetup::Legacy
-                | libcgroups::common::CgroupSetup::Hybrid => {
-                    let cmanager = libcgroups::common::create_cgroup_manager(
-                        self.spec()?.cgroup_config
-                    )?;
-                    cmanager.freeze(libcgroups::common::FreezerState::Thawed)?;
+            if let Some(cgroup_config) = self.spec()?.cgroup_config {
+                match get_cgroup_setup()? {
+                    libcgroups::common::CgroupSetup::Legacy
+                    | libcgroups::common::CgroupSetup::Hybrid => {
+                        let cmanager = libcgroups::common::create_cgroup_manager(
+                            cgroup_config
+                        )?;
+                        cmanager.freeze(libcgroups::common::FreezerState::Thawed)?;
+                    }
+                    libcgroups::common::CgroupSetup::Unified => {}
                 }
-                libcgroups::common::CgroupSetup::Unified => {}
+            } else {
+                return Err(LibcontainerError::CgroupsMissing)
             }
         }
         Ok(())
     }
 
     fn kill_all_processes<S: Into<Signal>>(&self, signal: S) -> Result<(), LibcontainerError> {
+        let cgroup_config = match self.spec()?.cgroup_config {
+            Some(cc) => cc,
+            None => return Err(LibcontainerError::CgroupsMissing),
+        };
+
         let signal = signal.into().into_raw();
         let cmanager =
-            libcgroups::common::create_cgroup_manager(self.spec()?.cgroup_config)?;
+            libcgroups::common::create_cgroup_manager(cgroup_config)?;
 
         if let Err(e) = cmanager.freeze(libcgroups::common::FreezerState::Frozen) {
             tracing::warn!(

crates/libcontainer/src/container/container_pause.rs

@@ -32,8 +32,13 @@ impl Container {
             return Err(LibcontainerError::IncorrectStatus);
         }
 
+        let cgroup_config = match self.spec()?.cgroup_config {
+            Some(cc) => cc,
+            None => return Err(LibcontainerError::CgroupsMissing),
+        };
+
         let cmanager =
-            libcgroups::common::create_cgroup_manager(self.spec()?.cgroup_config)?;
+            libcgroups::common::create_cgroup_manager(cgroup_config)?;
         cmanager.freeze(FreezerState::Frozen)?;
 
         tracing::debug!("saving paused status");

crates/libcontainer/src/container/container_resume.rs

@@ -33,8 +33,13 @@ impl Container {
             return Err(LibcontainerError::IncorrectStatus);
         }
 
+        let cgroup_config = match self.spec()?.cgroup_config {
+            Some(cc) => cc,
+            None => return Err(LibcontainerError::CgroupsMissing),
+        };
+
         let cmanager =
-            libcgroups::common::create_cgroup_manager(self.spec()?.cgroup_config)?;
+            libcgroups::common::create_cgroup_manager(cgroup_config)?;
         // resume the frozen container
         cmanager.freeze(FreezerState::Thawed)?;
 

crates/libcontainer/src/container/init_builder.rs

@@ -18,6 +18,7 @@ use crate::{apparmor, tty, user_ns, utils};
 pub struct InitContainerBuilder {
     base: ContainerBuilder,
     bundle: PathBuf,
+    use_cgroups: bool,
     use_systemd: bool,
     detached: bool,
 }
@@ -29,11 +30,18 @@ impl InitContainerBuilder {
         Self {
             base: builder,
             bundle,
+            use_cgroups: true,
             use_systemd: true,
             detached: true,
         }
     }
 
+    /// Sets if cgroups should be used at all (overrides systemd if false)
+    pub fn with_cgroups(mut self, should_use: bool) -> Self {
+        self.use_cgroups = should_use;
+        self
+    }
+
     /// Sets if systemd should be used for managing cgroups
     pub fn with_systemd(mut self, should_use: bool) -> Self {
         self.use_systemd = should_use;
@@ -73,13 +81,16 @@ impl InitContainerBuilder {
 
         let user_ns_config = UserNamespaceConfig::new(&spec)?;
 
-        let linux = spec.linux().as_ref().ok_or(MissingSpecError::Linux)?;
-        let cgroups_path = utils::get_cgroup_path(linux.cgroups_path(), &self.base.container_id);
-        let cgroup_config = libcgroups::common::CgroupConfig {
-            cgroup_path: cgroups_path,
-            systemd_cgroup: self.use_systemd || user_ns_config.is_some(),
-            container_name: self.base.container_id.to_owned(),
-        };
+        let mut cgroup_config = None;
+        if self.use_cgroups {
+            let linux = spec.linux().as_ref().ok_or(MissingSpecError::Linux)?;
+            let cgroups_path = utils::get_cgroup_path(linux.cgroups_path(), &self.base.container_id);
+            cgroup_config = Some(libcgroups::common::CgroupConfig {
+                cgroup_path: cgroups_path,
+                systemd_cgroup: self.use_systemd || user_ns_config.is_some(),
+                container_name: self.base.container_id.to_owned(),
+            });
+        }
 
         let config = YoukiConfig::from_spec(&spec, cgroup_config.clone())?;
         config.save(&container_dir).map_err(|err| {

crates/libcontainer/src/error.rs

@@ -14,6 +14,8 @@ pub enum MissingSpecError {
 pub enum LibcontainerError {
     #[error("failed to perform operation due to incorrect container status")]
     IncorrectStatus,
+    #[error("requested operation requires cgroups to be enabled on the container")]
+    CgroupsMissing,
     #[error("container already exists")]
     Exist,
     #[error("container state directory does not exist")]

crates/libcontainer/src/process/args.rs

@@ -36,7 +36,7 @@ pub struct ContainerArgs {
     /// Options for new namespace creation
     pub user_ns_config: Option<UserNamespaceConfig>,
     /// Cgroup Manager Config
-    pub cgroup_config: CgroupConfig,
+    pub cgroup_config: Option<CgroupConfig>,
     /// If the container is to be run in detached mode
     pub detached: bool,
     /// Manage the functions that actually run on the container

crates/libcontainer/src/process/container_intermediate_process.rs

@@ -49,8 +49,6 @@ pub fn container_intermediate_process(
     let spec = &args.spec;
     let linux = spec.linux().as_ref().ok_or(MissingSpecError::Linux)?;
     let namespaces = Namespaces::try_from(linux.namespaces().as_ref())?;
-    let cgroup_manager = libcgroups::common::create_cgroup_manager(args.cgroup_config.to_owned())
-        .map_err(|e| IntermediateProcessError::Cgroup(e.to_string()))?;
 
     // this needs to be done before we create the init process, so that the init
     // process will already be captured by the cgroup. It also needs to be done
@@ -62,11 +60,15 @@ pub fn container_intermediate_process(
     // In addition this needs to be done before we enter the cgroup namespace as
     // the cgroup of the process will form the root of the cgroup hierarchy in
     // the cgroup namespace.
-    apply_cgroups(
-        &cgroup_manager,
-        linux.resources().as_ref(),
-        matches!(args.container_type, ContainerType::InitContainer { .. }),
-    )?;
+    if let Some(cgroup_config) = &args.cgroup_config {
+        let cgroup_manager = libcgroups::common::create_cgroup_manager(cgroup_config.to_owned())
+            .map_err(|e| IntermediateProcessError::Cgroup(e.to_string()))?;
+        apply_cgroups(
+            &cgroup_manager,
+            linux.resources().as_ref(),
+            matches!(args.container_type, ContainerType::InitContainer { .. }),
+        )?;
+    }
 
     // if new user is specified in specification, this will be true and new
     // namespace will be created, check

crates/liboci-cli/src/lib.rs

@@ -94,4 +94,7 @@ pub struct GlobalOpts {
     /// Enable systemd cgroup manager, rather then use the cgroupfs directly.
     #[clap(short, long)]
     pub systemd_cgroup: bool,
+    /// Entirely disable cgroup manipulation (breaks some operations, e.g. pause)
+    #[clap(short, long)]
+    pub disable_cgroups: bool,
 }

crates/youki/src/commands/create.rs

@@ -13,7 +13,7 @@ use crate::workload::executor::default_executor;
 // can be given impression that is is running on a complete system, but on the system which
 // it is running, it is just another process, and has attributes such as pid, file descriptors, etc.
 // associated with it like any other process.
-pub fn create(args: Create, root_path: PathBuf, systemd_cgroup: bool) -> Result<()> {
+pub fn create(args: Create, root_path: PathBuf, systemd_cgroup: bool, use_cgroups: bool) -> Result<()> {
     ContainerBuilder::new(args.container_id.clone(), SyscallType::default())
         .with_executor(default_executor())
         .with_pid_file(args.pid_file.as_ref())?
@@ -23,6 +23,7 @@ pub fn create(args: Create, root_path: PathBuf, systemd_cgroup: bool) -> Result<
         .validate_id()?
         .as_init(&args.bundle)
         .with_systemd(systemd_cgroup)
+        .with_cgroups(use_cgroups)
         .with_detach(true)
         .build()?;
 

crates/youki/src/commands/mod.rs

@@ -57,5 +57,8 @@ fn create_cgroup_manager<P: AsRef<Path>>(
     container_id: &str,
 ) -> Result<AnyCgroupManager> {
     let container = load_container(root_path, container_id)?;
-    Ok(libcgroups::common::create_cgroup_manager(container.spec()?.cgroup_config)?)
+    match container.spec()?.cgroup_config {
+        Some(cc) => Ok(libcgroups::common::create_cgroup_manager(cc)?),
+        None => bail!("cannot use cgroups on container started without them"),
+    }
 }

crates/youki/src/commands/run.rs

@@ -11,7 +11,7 @@ use nix::unistd::Pid;
 
 use crate::workload::executor::default_executor;
 
-pub fn run(args: Run, root_path: PathBuf, systemd_cgroup: bool) -> Result<i32> {
+pub fn run(args: Run, root_path: PathBuf, systemd_cgroup: bool, use_cgroups: bool) -> Result<i32> {
     let mut container = ContainerBuilder::new(args.container_id.clone(), SyscallType::default())
         .with_executor(default_executor())
         .with_pid_file(args.pid_file.as_ref())?
@@ -21,6 +21,7 @@ pub fn run(args: Run, root_path: PathBuf, systemd_cgroup: bool) -> Result<i32> {
         .validate_id()?
         .as_init(&args.bundle)
         .with_systemd(systemd_cgroup)
+        .with_cgroups(use_cgroups)
         .with_detach(args.detach)
         .build()?;
 

crates/youki/src/main.rs

@@ -101,11 +101,12 @@ fn main() -> Result<()> {
     );
     let root_path = rootpath::determine(opts.global.root)?;
     let systemd_cgroup = opts.global.systemd_cgroup;
+    let use_cgroups = !opts.global.disable_cgroups;
 
     let cmd_result = match opts.subcmd {
         SubCommand::Standard(cmd) => match *cmd {
             StandardCmd::Create(create) => {
-                commands::create::create(create, root_path, systemd_cgroup)
+                commands::create::create(create, root_path, systemd_cgroup, use_cgroups)
             }
             StandardCmd::Start(start) => commands::start::start(start, root_path),
             StandardCmd::Kill(kill) => commands::kill::kill(kill, root_path),
@@ -130,7 +131,7 @@ fn main() -> Result<()> {
             CommonCmd::Pause(pause) => commands::pause::pause(pause, root_path),
             CommonCmd::Ps(ps) => commands::ps::ps(ps, root_path),
             CommonCmd::Resume(resume) => commands::resume::resume(resume, root_path),
-            CommonCmd::Run(run) => match commands::run::run(run, root_path, systemd_cgroup) {
+            CommonCmd::Run(run) => match commands::run::run(run, root_path, systemd_cgroup, use_cgroups) {
                 Ok(exit_code) => std::process::exit(exit_code),
                 Err(e) => {
                     tracing::error!("error in executing command: {:?}", e);