Refactor the libcgroups interface

crates/libcgroups/src/common.rs

@@ -316,65 +316,70 @@ pub enum CreateCgroupSetupError {
     Systemd(#[from] systemd::manager::SystemdManagerError),
 }
 
-pub fn create_cgroup_manager<P: Into<PathBuf>>(
-    cgroup_path: P,
-    systemd_cgroup: bool,
-    container_name: &str,
+#[derive(Clone)]
+pub struct CgroupConfig {
+    pub cgroup_path: PathBuf,
+    pub systemd_cgroup: bool,
+    pub container_name: String,
+}
+
+pub fn create_cgroup_manager(
+    config: CgroupConfig,
 ) -> Result<AnyCgroupManager, CreateCgroupSetupError> {
     let cgroup_setup = get_cgroup_setup().map_err(|err| match err {
         GetCgroupSetupError::WrappedIo(err) => CreateCgroupSetupError::WrappedIo(err),
         GetCgroupSetupError::NonDefault => CreateCgroupSetupError::NonDefault,
         GetCgroupSetupError::FailedToDetect => CreateCgroupSetupError::FailedToDetect,
     })?;
-    let cgroup_path = cgroup_path.into();
+    let cgroup_path = config.cgroup_path.as_path();
 
     match cgroup_setup {
         CgroupSetup::Legacy | CgroupSetup::Hybrid => {
             Ok(create_v1_cgroup_manager(cgroup_path)?.any())
         }
         CgroupSetup::Unified => {
             // ref https://github.com/opencontainers/runtime-spec/blob/main/config-linux.md#cgroups-path
-            if cgroup_path.is_absolute() || !systemd_cgroup {
+            if cgroup_path.is_absolute() || !config.systemd_cgroup {
                 return Ok(create_v2_cgroup_manager(cgroup_path)?.any());
             }
-            Ok(create_systemd_cgroup_manager(cgroup_path, container_name)?.any())
+            Ok(create_systemd_cgroup_manager(cgroup_path, config.container_name.as_str())?.any())
         }
     }
 }
 
 #[cfg(feature = "v1")]
 fn create_v1_cgroup_manager(
-    cgroup_path: PathBuf,
+    cgroup_path: &Path,
 ) -> Result<v1::manager::Manager, v1::manager::V1ManagerError> {
     tracing::info!("cgroup manager V1 will be used");
     v1::manager::Manager::new(cgroup_path)
 }
 
 #[cfg(not(feature = "v1"))]
 fn create_v1_cgroup_manager(
-    _cgroup_path: PathBuf,
+    _cgroup_path: &Path,
 ) -> Result<v1::manager::Manager, v1::manager::V1ManagerError> {
     Err(v1::manager::V1ManagerError::NotEnabled)
 }
 
 #[cfg(feature = "v2")]
 fn create_v2_cgroup_manager(
-    cgroup_path: PathBuf,
+    cgroup_path: &Path,
 ) -> Result<v2::manager::Manager, v2::manager::V2ManagerError> {
     tracing::info!("cgroup manager V2 will be used");
-    v2::manager::Manager::new(DEFAULT_CGROUP_ROOT.into(), cgroup_path)
+    v2::manager::Manager::new(DEFAULT_CGROUP_ROOT.into(), cgroup_path.to_owned())
 }
 
 #[cfg(not(feature = "v2"))]
 fn create_v2_cgroup_manager(
-    _cgroup_path: PathBuf,
+    _cgroup_path: &Path,
 ) -> Result<v2::manager::Manager, v2::manager::V2ManagerError> {
     Err(v2::manager::V2ManagerError::NotEnabled)
 }
 
 #[cfg(feature = "systemd")]
 fn create_systemd_cgroup_manager(
-    cgroup_path: PathBuf,
+    cgroup_path: &Path,
     container_name: &str,
 ) -> Result<systemd::manager::Manager, systemd::manager::SystemdManagerError> {
     if !systemd::booted() {
@@ -391,15 +396,15 @@ fn create_systemd_cgroup_manager(
     );
     systemd::manager::Manager::new(
         DEFAULT_CGROUP_ROOT.into(),
-        cgroup_path,
+        cgroup_path.to_owned(),
         container_name.into(),
         use_system,
     )
 }
 
 #[cfg(not(feature = "systemd"))]
 fn create_systemd_cgroup_manager(
-    _cgroup_path: PathBuf,
+    _cgroup_path: &Path,
     _container_name: &str,
 ) -> Result<systemd::manager::Manager, systemd::manager::SystemdManagerError> {
     Err(systemd::manager::SystemdManagerError::NotEnabled)

crates/libcgroups/src/systemd/manager.rs

@@ -180,7 +180,7 @@ impl Manager {
         container_name: String,
         use_system: bool,
     ) -> Result<Self, SystemdManagerError> {
-        let mut destructured_path = cgroups_path.as_path().try_into()?;
+        let mut destructured_path: CgroupsPath = cgroups_path.as_path().try_into()?;
         ensure_parent_unit(&mut destructured_path, use_system);
 
         let client = match use_system {

crates/libcgroups/src/v1/manager.rs

@@ -82,10 +82,10 @@ pub enum V1ManagerError {
 
 impl Manager {
     /// Constructs a new cgroup manager with cgroups_path being relative to the root of the subsystem
-    pub fn new(cgroup_path: PathBuf) -> Result<Self, V1ManagerError> {
+    pub fn new(cgroup_path: &Path) -> Result<Self, V1ManagerError> {
         let mut subsystems = HashMap::<CtrlType, PathBuf>::new();
         for subsystem in CONTROLLERS {
-            if let Ok(subsystem_path) = Self::get_subsystem_path(&cgroup_path, subsystem) {
+            if let Ok(subsystem_path) = Self::get_subsystem_path(cgroup_path, subsystem) {
                 subsystems.insert(*subsystem, subsystem_path);
             } else {
                 tracing::warn!("cgroup {} not supported on this system", subsystem);

crates/libcontainer/src/container/builder_impl.rs

@@ -73,11 +73,11 @@ impl<'a> ContainerBuilderImpl<'a> {
             &self.container_id,
             self.rootless.is_some(),
         );
-        let cmanager = libcgroups::common::create_cgroup_manager(
-            cgroups_path,
-            self.use_systemd || self.rootless.is_some(),
-            &self.container_id,
-        )?;
+        let cgroup_config = libcgroups::common::CgroupConfig {
+            cgroup_path: cgroups_path,
+            systemd_cgroup: self.use_systemd || self.rootless.is_some(),
+            container_name: self.container_id.to_owned(),
+        };
         let process = self
             .spec
             .process()
@@ -93,8 +93,10 @@ impl<'a> ContainerBuilderImpl<'a> {
         // Need to create the notify socket before we pivot root, since the unix
         // domain socket used here is outside of the rootfs of container. During
         // exec, need to create the socket before we enter into existing mount
-        // namespace.
-        let notify_socket: NotifyListener = NotifyListener::new(&self.notify_path)?;
+        // namespace. We also need to create to socket before entering into the
+        // user namespace in the case that the path is located in paths only
+        // root can access.
+        let notify_listener = NotifyListener::new(&self.notify_path)?;
 
         // If Out-of-memory score adjustment is set in specification.  set the score
         // value for the current process check
@@ -139,11 +141,11 @@ impl<'a> ContainerBuilderImpl<'a> {
             spec: self.spec,
             rootfs: &self.rootfs,
             console_socket: self.console_socket,
-            notify_socket,
+            notify_listener,
             preserve_fds: self.preserve_fds,
             container: &self.container,
             rootless: &self.rootless,
-            cgroup_manager: cmanager,
+            cgroup_config,
             detached: self.detached,
             executor_manager: &self.executor_manager,
         };
@@ -184,11 +186,12 @@ impl<'a> ContainerBuilderImpl<'a> {
             &self.container_id,
             self.rootless.is_some(),
         );
-        let cmanager = libcgroups::common::create_cgroup_manager(
-            cgroups_path,
-            self.use_systemd || self.rootless.is_some(),
-            &self.container_id,
-        )?;
+        let cmanager =
+            libcgroups::common::create_cgroup_manager(libcgroups::common::CgroupConfig {
+                cgroup_path: cgroups_path,
+                systemd_cgroup: self.use_systemd || self.rootless.is_some(),
+                container_name: self.container_id.to_string(),
+            })?;
 
         let mut errors = Vec::new();
 

crates/libcontainer/src/container/container_delete.rs

@@ -81,11 +81,12 @@ 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 use_systemd = self.systemd();
                     let cmanager = libcgroups::common::create_cgroup_manager(
-                        &config.cgroup_path,
-                        use_systemd,
-                        self.id(),
+                        libcgroups::common::CgroupConfig {
+                            cgroup_path: config.cgroup_path.to_owned(),
+                            systemd_cgroup: self.systemd(),
+                            container_name: self.id().to_string(),
+                        },
                     )?;
                     cmanager.remove().map_err(|err| {
                         tracing::error!(cgroup_path = ?config.cgroup_path, "failed to remove cgroup due to: {err:?}");

crates/libcontainer/src/container/container_events.rs

@@ -33,11 +33,12 @@ impl Container {
             return Err(LibcontainerError::IncorrectStatus);
         }
 
-        let cgroups_path = self.spec()?.cgroup_path;
-        let use_systemd = self.systemd();
-
         let cgroup_manager =
-            libcgroups::common::create_cgroup_manager(cgroups_path, use_systemd, self.id())?;
+            libcgroups::common::create_cgroup_manager(libcgroups::common::CgroupConfig {
+                cgroup_path: self.spec()?.cgroup_path,
+                systemd_cgroup: self.systemd(),
+                container_name: self.id().to_string(),
+            })?;
         match stats {
             true => {
                 let stats = cgroup_manager.stats()?;

crates/libcontainer/src/container/container_kill.rs

@@ -1,6 +1,6 @@
 use super::{Container, ContainerStatus};
 use crate::{error::LibcontainerError, signal::Signal};
-use libcgroups::common::{create_cgroup_manager, get_cgroup_setup, CgroupManager};
+use libcgroups::common::{get_cgroup_setup, CgroupManager};
 use nix::sys::signal::{self};
 
 impl Container {
@@ -78,10 +78,14 @@ impl Container {
             match get_cgroup_setup()? {
                 libcgroups::common::CgroupSetup::Legacy
                 | libcgroups::common::CgroupSetup::Hybrid => {
-                    let cgroups_path = self.spec()?.cgroup_path;
-                    let use_systemd = self.systemd();
-                    let cmanger = create_cgroup_manager(cgroups_path, use_systemd, self.id())?;
-                    cmanger.freeze(libcgroups::common::FreezerState::Thawed)?;
+                    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(),
+                        },
+                    )?;
+                    cmanager.freeze(libcgroups::common::FreezerState::Thawed)?;
                 }
                 libcgroups::common::CgroupSetup::Unified => {}
             }
@@ -91,19 +95,22 @@ impl Container {
 
     fn kill_all_processes<S: Into<Signal>>(&self, signal: S) -> Result<(), LibcontainerError> {
         let signal = signal.into().into_raw();
-        let cgroups_path = self.spec()?.cgroup_path;
-        let use_systemd = self.systemd();
-        let cmanger = create_cgroup_manager(cgroups_path, use_systemd, self.id())?;
+        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(),
+            })?;
 
-        if let Err(e) = cmanger.freeze(libcgroups::common::FreezerState::Frozen) {
+        if let Err(e) = cmanager.freeze(libcgroups::common::FreezerState::Frozen) {
             tracing::warn!(
                 err = ?e,
                 id = ?self.id(),
                 "failed to freeze container",
             );
         }
 
-        let pids = cmanger.get_all_pids()?;
+        let pids = cmanager.get_all_pids()?;
         pids.iter()
             .try_for_each(|&pid| {
                 tracing::debug!("kill signal {} to {}", signal, pid);
@@ -117,7 +124,7 @@ impl Container {
                 }
             })
             .map_err(LibcontainerError::OtherSyscall)?;
-        if let Err(err) = cmanger.freeze(libcgroups::common::FreezerState::Thawed) {
+        if let Err(err) = cmanager.freeze(libcgroups::common::FreezerState::Thawed) {
             tracing::warn!(
                 err = ?err,
                 id = ?self.id(),

crates/libcontainer/src/container/container_pause.rs

@@ -32,10 +32,12 @@ impl Container {
             return Err(LibcontainerError::IncorrectStatus);
         }
 
-        let cgroups_path = self.spec()?.cgroup_path;
-        let use_systemd = self.systemd();
         let cmanager =
-            libcgroups::common::create_cgroup_manager(cgroups_path, use_systemd, self.id())?;
+            libcgroups::common::create_cgroup_manager(libcgroups::common::CgroupConfig {
+                cgroup_path: self.spec()?.cgroup_path,
+                systemd_cgroup: self.systemd(),
+                container_name: self.id().to_string(),
+            })?;
         cmanager.freeze(FreezerState::Frozen)?;
 
         tracing::debug!("saving paused status");

crates/libcontainer/src/container/container_resume.rs

@@ -34,10 +34,12 @@ impl Container {
             return Err(LibcontainerError::IncorrectStatus);
         }
 
-        let cgroups_path = self.spec()?.cgroup_path;
-        let use_systemd = self.systemd();
         let cmanager =
-            libcgroups::common::create_cgroup_manager(cgroups_path, use_systemd, self.id())?;
+            libcgroups::common::create_cgroup_manager(libcgroups::common::CgroupConfig {
+                cgroup_path: self.spec()?.cgroup_path,
+                systemd_cgroup: self.systemd(),
+                container_name: self.id().to_string(),
+            })?;
         // resume the frozen container
         cmanager.freeze(FreezerState::Thawed)?;