agent: Drop the Option for LinuxContainer.cgroup_manager

Cgroup manager for a container will always be created.
Thus, dropping the option for LinuxContainer.cgroup_manager
is feasible and could simplify the code.

Fixes: #5778

Signed-off-by: Yuan-Zhuo <yuanzhuo0118@outlook.com>

src/agent/rustjail/src/container.rs

@@ -109,7 +109,6 @@ impl Default for ContainerStatus {
 }
 
 // We might want to change this to thiserror in the future
-const MissingCGroupManager: &str = "failed to get container's cgroup Manager";
 const MissingLinux: &str = "no linux config";
 const InvalidNamespace: &str = "invalid namespace type";
 
@@ -243,7 +242,7 @@ pub struct LinuxContainer {
     pub id: String,
     pub root: String,
     pub config: Config,
-    pub cgroup_manager: Option<Box<dyn Manager + Send + Sync>>,
+    pub cgroup_manager: Box<dyn Manager + Send + Sync>,
     pub init_process_pid: pid_t,
     pub init_process_start_time: u64,
     pub uid_map_path: String,
@@ -292,16 +291,11 @@ impl Container for LinuxContainer {
             ));
         }
 
-        if self.cgroup_manager.is_some() {
-            self.cgroup_manager
-                .as_ref()
-                .unwrap()
-                .freeze(FreezerState::Frozen)?;
+        self.cgroup_manager.as_ref().freeze(FreezerState::Frozen)?;
 
-            self.status.transition(ContainerState::Paused);
-            return Ok(());
-        }
-        Err(anyhow!(MissingCGroupManager))
+        self.status.transition(ContainerState::Paused);
+
+        Ok(())
     }
 
     fn resume(&mut self) -> Result<()> {
@@ -310,16 +304,11 @@ impl Container for LinuxContainer {
             return Err(anyhow!("container status is: {:?}, not paused", status));
         }
 
-        if self.cgroup_manager.is_some() {
-            self.cgroup_manager
-                .as_ref()
-                .unwrap()
-                .freeze(FreezerState::Thawed)?;
+        self.cgroup_manager.as_ref().freeze(FreezerState::Thawed)?;
 
-            self.status.transition(ContainerState::Running);
-            return Ok(());
-        }
-        Err(anyhow!(MissingCGroupManager))
+        self.status.transition(ContainerState::Running);
+
+        Ok(())
     }
 }
 
@@ -847,22 +836,17 @@ impl BaseContainer for LinuxContainer {
     }
 
     fn stats(&self) -> Result<StatsContainerResponse> {
-        let mut r = StatsContainerResponse::default();
-
-        if self.cgroup_manager.is_some() {
-            r.cgroup_stats =
-                SingularPtrField::some(self.cgroup_manager.as_ref().unwrap().get_stats()?);
-        }
-
         // what about network interface stats?
 
-        Ok(r)
+        Ok(StatsContainerResponse {
+            cgroup_stats: SingularPtrField::some(self.cgroup_manager.as_ref().get_stats()?),
+            ..Default::default()
+        })
     }
 
     fn set(&mut self, r: LinuxResources) -> Result<()> {
-        if self.cgroup_manager.is_some() {
-            self.cgroup_manager.as_ref().unwrap().set(&r, true)?;
-        }
+        self.cgroup_manager.as_ref().set(&r, true)?;
+
         self.config
             .spec
             .as_mut()
@@ -1035,7 +1019,7 @@ impl BaseContainer for LinuxContainer {
             &logger,
             spec,
             &p,
-            self.cgroup_manager.as_ref().unwrap().as_ref(),
+            self.cgroup_manager.as_ref(),
             self.config.use_systemd_cgroup,
             &st,
             &mut pipe_w,
@@ -1114,19 +1098,19 @@ impl BaseContainer for LinuxContainer {
         )?;
         fs::remove_dir_all(&self.root)?;
 
-        if let Some(cgm) = self.cgroup_manager.as_mut() {
-            // Kill all of the processes created in this container to prevent
-            // the leak of some daemon process when this container shared pidns
-            // with the sandbox.
-            let pids = cgm.get_pids().context("get cgroup pids")?;
-            for i in pids {
-                if let Err(e) = signal::kill(Pid::from_raw(i), Signal::SIGKILL) {
-                    warn!(self.logger, "kill the process {} error: {:?}", i, e);
-                }
+        let cgm = self.cgroup_manager.as_mut();
+        // Kill all of the processes created in this container to prevent
+        // the leak of some daemon process when this container shared pidns
+        // with the sandbox.
+        let pids = cgm.get_pids().context("get cgroup pids")?;
+        for i in pids {
+            if let Err(e) = signal::kill(Pid::from_raw(i), Signal::SIGKILL) {
+                warn!(self.logger, "kill the process {} error: {:?}", i, e);
             }
-
-            cgm.destroy().context("destroy cgroups")?;
         }
+
+        cgm.destroy().context("destroy cgroups")?;
+
         Ok(())
     }
 
@@ -1514,7 +1498,7 @@ impl LinuxContainer {
         Ok(LinuxContainer {
             id: id.clone(),
             root,
-            cgroup_manager: Some(cgroup_manager),
+            cgroup_manager,
             status: ContainerStatus::new(),
             uid_map_path: String::from(""),
             gid_map_path: "".to_string(),
@@ -1980,24 +1964,12 @@ mod tests {
         assert!(format!("{:?}", ret).contains("failed to pause container"))
     }
 
-    #[test]
-    fn test_linuxcontainer_pause_cgroupmgr_is_none() {
-        let ret = new_linux_container_and_then(|mut c: LinuxContainer| {
-            c.cgroup_manager = None;
-            c.pause().map_err(|e| anyhow!(e))
-        });
-
-        assert!(ret.is_err(), "Expecting error, Got {:?}", ret);
-    }
-
     #[test]
     fn test_linuxcontainer_pause() {
         let ret = new_linux_container_and_then(|mut c: LinuxContainer| {
-            let cgroup_manager: Box<dyn Manager + Send + Sync> =
-                Box::new(FsManager::new("").map_err(|e| {
-                    anyhow!(format!("fail to create cgroup manager with path: {:}", e))
-                })?);
-            c.cgroup_manager = Some(cgroup_manager);
+            c.cgroup_manager = Box::new(FsManager::new("").map_err(|e| {
+                anyhow!(format!("fail to create cgroup manager with path: {:}", e))
+            })?);
             c.pause().map_err(|e| anyhow!(e))
         });
 
@@ -2016,25 +1988,12 @@ mod tests {
         assert!(format!("{:?}", ret).contains("not paused"))
     }
 
-    #[test]
-    fn test_linuxcontainer_resume_cgroupmgr_is_none() {
-        let ret = new_linux_container_and_then(|mut c: LinuxContainer| {
-            c.status.transition(ContainerState::Paused);
-            c.cgroup_manager = None;
-            c.resume().map_err(|e| anyhow!(e))
-        });
-
-        assert!(ret.is_err(), "Expecting error, Got {:?}", ret);
-    }
-
     #[test]
     fn test_linuxcontainer_resume() {
         let ret = new_linux_container_and_then(|mut c: LinuxContainer| {
-            let cgroup_manager: Box<dyn Manager + Send + Sync> =
-                Box::new(FsManager::new("").map_err(|e| {
-                    anyhow!(format!("fail to create cgroup manager with path: {:}", e))
-                })?);
-            c.cgroup_manager = Some(cgroup_manager);
+            c.cgroup_manager = Box::new(FsManager::new("").map_err(|e| {
+                anyhow!(format!("fail to create cgroup manager with path: {:}", e))
+            })?);
             // Change status to paused, this way we can resume it
             c.status.transition(ContainerState::Paused);
             c.resume().map_err(|e| anyhow!(e))

src/agent/src/rpc.rs

@@ -271,14 +271,13 @@ impl AgentService {
         }
 
         // start oom event loop
-        if let Some(ref ctr) = ctr.cgroup_manager {
-            let cg_path = ctr.get_cgroup_path("memory");
 
-            if let Ok(cg_path) = cg_path {
-                let rx = notifier::notify_oom(cid.as_str(), cg_path.to_string()).await?;
+        let cg_path = ctr.cgroup_manager.as_ref().get_cgroup_path("memory");
 
-                s.run_oom_event_monitor(rx, cid.clone()).await;
-            }
+        if let Ok(cg_path) = cg_path {
+            let rx = notifier::notify_oom(cid.as_str(), cg_path.to_string()).await?;
+
+            s.run_oom_event_monitor(rx, cid.clone()).await;
         }
 
         Ok(())
@@ -465,11 +464,7 @@ impl AgentService {
         let ctr = sandbox
             .get_container(cid)
             .ok_or_else(|| anyhow!("Invalid container id {}", cid))?;
-        let cm = ctr
-            .cgroup_manager
-            .as_ref()
-            .ok_or_else(|| anyhow!("cgroup manager not exist"))?;
-        cm.freeze(state)?;
+        ctr.cgroup_manager.as_ref().freeze(state)?;
         Ok(())
     }
 
@@ -479,11 +474,7 @@ impl AgentService {
         let ctr = sandbox
             .get_container(cid)
             .ok_or_else(|| anyhow!("Invalid container id {}", cid))?;
-        let cm = ctr
-            .cgroup_manager
-            .as_ref()
-            .ok_or_else(|| anyhow!("cgroup manager not exist"))?;
-        let pids = cm.get_pids()?;
+        let pids = ctr.cgroup_manager.as_ref().get_pids()?;
         Ok(pids)
     }
 

src/agent/src/sandbox.rs

@@ -296,7 +296,6 @@ impl Sandbox {
             info!(self.logger, "updating {}", ctr.id.as_str());
             ctr.cgroup_manager
                 .as_ref()
-                .unwrap()
                 .update_cpuset_path(guest_cpuset.as_str(), container_cpust)?;
         }
 

src/tools/runk/libcontainer/src/container.rs

@@ -337,7 +337,6 @@ impl ContainerLauncher {
             self.runner
                 .cgroup_manager
                 .as_ref()
-                .unwrap()
                 .as_any()?
                 .downcast_ref::<CgroupManager>()
                 .unwrap()