Fix jailer --parent-cgroup parameter when no cgroups specified

CHANGELOG.md

@@ -48,6 +48,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- [#4309](https://github.com/firecracker-microvm/firecracker/pull/4309): The
+  jailer's option `--parent-cgroup` will move the process to that cgroup if no
+  `cgroup` options are provided.
 - Simplified and clarified the removal policy of deprecated API elements
   to follow semantic versioning 2.0.0. For more information, please refer to
   [this GitHub discussion](https://github.com/firecracker-microvm/firecracker/discussions/4135).

docs/jailer.md

@@ -46,6 +46,8 @@ jailer --id <id> \
   the jailer will write all cgroup parameters specified through `--cgroup` in
   `/sys/fs/cgroup/<controller_name>/all_uvms/external_uvms/<id>`. By default, the
   parent cgroup is `exec-file`.
+  If there are no `--cgroup` parameters specified and `--group-version=2` was
+  passed, then the jailer will move the process to the specified cgroup.
 - `cgroup-version` is used to select which type of cgroup hierarchy to use for
   the creation of cgroups. The default value is "1" which means that cgroups
   specified with the `cgroup` argument will be created within a v1 hierarchy.

src/jailer/src/cgroup.rs

@@ -161,6 +161,16 @@ impl CgroupBuilder {
             }
         }
     }
+
+    // Returns the path to the root of the hierarchy
+    pub fn get_v2_hierarchy_path(&mut self) -> Result<&PathBuf, JailerError> {
+        match self.hierarchies.entry("unified".to_string()) {
+            Occupied(entry) => Ok(entry.into_mut()),
+            Vacant(_entry) => Err(JailerError::CgroupHierarchyMissing(
+                "cgroupsv2 hierarchy missing".to_string(),
+            )),
+        }
+    }
 }
 
 #[derive(Debug)]

src/jailer/src/env.rs

@@ -233,11 +233,24 @@ impl Env {
             .parse::<u8>()
             .map_err(|_| JailerError::CgroupInvalidVersion(cgroup_ver.to_string()))?;
 
-        let mut cgroup_builder = None;
+        let cgroups_args: &[String] = arguments.multiple_values("cgroup").unwrap_or_default();
+
+        // If the --parent-cgroup exists, and we have no other cgroups,
+        // then the intent is to move the process to that cgroup.
+        // Only applies to cgroupsv2 since it's a unified hierarchy
+        if cgroups_args.is_empty() && cgroup_ver == 2 {
+            let mut builder = CgroupBuilder::new(cgroup_ver)?;
+            let cg_parent = builder.get_v2_hierarchy_path()?.join(parent_cgroup);
+            let cg_parent_procs = cg_parent.join("cgroup.procs");
+            if cg_parent.exists() {
+                fs::write(cg_parent_procs, std::process::id().to_string())
+                    .map_err(|_| JailerError::CgroupWrite(io::Error::last_os_error()))?;
+            }
+        }
 
         // cgroup format: <cgroup_controller>.<cgroup_property>=<value>,...
         if let Some(cgroups_args) = arguments.multiple_values("cgroup") {
-            let builder = cgroup_builder.get_or_insert(CgroupBuilder::new(cgroup_ver)?);
+            let mut builder = CgroupBuilder::new(cgroup_ver)?;
             for cg in cgroups_args {
                 let aux: Vec<&str> = cg.split('=').collect();
                 if aux.len() != 2 || aux[1].is_empty() {

src/jailer/src/main.rs

@@ -32,8 +32,6 @@ pub enum JailerError {
     CgroupLineNotFound(String, String),
     #[error("Cgroup invalid file: {0}")]
     CgroupInvalidFile(String),
-    #[error("Expected value {0} for {2}. Current value: {1}")]
-    CgroupWrite(String, String, String),
     #[error("Invalid format for cgroups: {0}")]
     CgroupFormat(String),
     #[error("Hierarchy not found: {0}")]
@@ -44,6 +42,8 @@ pub enum JailerError {
     CgroupInvalidVersion(String),
     #[error("Parent cgroup path is invalid. Path should not be absolute or contain '..' or '.'")]
     CgroupInvalidParentPath(),
+    #[error("Failed to write to cgroups file: {0}")]
+    CgroupWrite(io::Error),
     #[error("Failed to change owner for {0:?}: {1}")]
     ChangeFileOwner(PathBuf, io::Error),
     #[error("Failed to chdir into chroot directory: {0}")]

tests/framework/jailer.py

@@ -72,7 +72,7 @@ def __init__(
         self.new_pid_ns = new_pid_ns
         self.extra_args = extra_args
         self.api_socket_name = DEFAULT_USOCKET_NAME
-        self.cgroups = cgroups
+        self.cgroups = cgroups or []
         self.resource_limits = resource_limits
         self.cgroup_ver = cgroup_ver
         self.parent_cgroup = parent_cgroup
@@ -112,7 +112,7 @@ def construct_param_list(self):
             jailer_param_list.extend(["--parent-cgroup", str(self.parent_cgroup)])
         if self.cgroup_ver:
             jailer_param_list.extend(["--cgroup-version", str(self.cgroup_ver)])
-        if self.cgroups is not None:
+        if self.cgroups:
             for cgroup in self.cgroups:
                 jailer_param_list.extend(["--cgroup", str(cgroup)])
         if self.resource_limits is not None: