diff --git a/src/agent/rustjail/src/container.rs b/src/agent/rustjail/src/container.rs index de92adf4ceac..31dbeda707fd 100644 --- a/src/agent/rustjail/src/container.rs +++ b/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>, + pub cgroup_manager: Box, 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 { - 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 = - 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 = - 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)) diff --git a/src/agent/src/rpc.rs b/src/agent/src/rpc.rs index d06f986e54fe..3b0b0ab65c4d 100644 --- a/src/agent/src/rpc.rs +++ b/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) } diff --git a/src/agent/src/sandbox.rs b/src/agent/src/sandbox.rs index 922d9e11419c..384a8e523d20 100644 --- a/src/agent/src/sandbox.rs +++ b/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)?; } diff --git a/src/tools/runk/libcontainer/src/container.rs b/src/tools/runk/libcontainer/src/container.rs index 20bad5d82655..fbb990e02cb9 100644 --- a/src/tools/runk/libcontainer/src/container.rs +++ b/src/tools/runk/libcontainer/src/container.rs @@ -337,7 +337,6 @@ impl ContainerLauncher { self.runner .cgroup_manager .as_ref() - .unwrap() .as_any()? .downcast_ref::() .unwrap()