Merge pull request #1204 from cyyzero/fix_kill

Thaw a paused container in cgroup v1 when it is deleted with --force.

crates/libcontainer/src/container/container_delete.rs

@@ -28,9 +28,7 @@ impl Container {
         self.refresh_status()
             .context("failed to refresh container status")?;
         if self.can_kill() && force {
-            let sig = signal::Signal::SIGKILL;
-            log::debug!("kill signal {} to {}", sig, self.pid().unwrap());
-            signal::kill(self.pid().unwrap(), sig)?;
+            self.do_kill(signal::Signal::SIGKILL, true)?;
             self.set_status(ContainerStatus::Stopped).save()?;
         }
         log::debug!("container status: {:?}", self.status());
@@ -68,7 +66,7 @@ impl Container {
                         .with_context(|| "failed to run post stop hooks")?;
                 }
             }
-            std::process::exit(0)
+            Ok(())
         } else {
             bail!(
                 "{} could not be deleted because it was {:?}",

crates/libcontainer/src/container/container_kill.rs

@@ -1,7 +1,7 @@
 use super::{Container, ContainerStatus};
 use crate::signal::Signal;
 use anyhow::{bail, Context, Result};
-use libcgroups::common::create_cgroup_manager;
+use libcgroups::common::{create_cgroup_manager, get_cgroup_setup};
 use nix::sys::signal::{self};
 
 impl Container {
@@ -24,37 +24,87 @@ impl Container {
     /// # }
     /// ```
     pub fn kill<S: Into<Signal>>(&mut self, signal: S, all: bool) -> Result<()> {
-        let signal = signal.into().into_raw();
-
-        let pids = if all {
-            let cgroups_path = self.spec()?.cgroup_path;
-            let use_systemd = self
-                .systemd()
-                .context("container state does not contain cgroup manager")?;
-            let cmanger = create_cgroup_manager(&cgroups_path, use_systemd, self.id())?;
-            cmanger.get_all_pids()?
-        } else {
-            vec![self
-                .pid()
-                .context("failed to get the pid of the container")?]
-        };
-
         self.refresh_status()
             .context("failed to refresh container status")?;
         if self.can_kill() {
-            pids.into_iter().try_for_each(|pid| {
-                log::debug!("kill signal {} to {}", signal, pid);
-                signal::kill(pid, signal)
-            })?;
-
-            self.set_status(ContainerStatus::Stopped).save()?;
-            std::process::exit(0)
+            self.do_kill(signal, all)?;
         } else {
-            bail!(
-                "{} could not be killed because it was {:?}",
-                self.id(),
-                self.status()
-            )
+            // just like runc, allow kill --all even if the container is stopped
+            if all && self.status() == ContainerStatus::Stopped {
+                self.do_kill(signal, all)?;
+            } else {
+                bail!(
+                    "{} could not be killed because it was {:?}",
+                    self.id(),
+                    self.status()
+                )
+            }
+        }
+        self.set_status(ContainerStatus::Stopped).save()?;
+        Ok(())
+    }
+
+    pub(crate) fn do_kill<S: Into<Signal>>(&self, signal: S, all: bool) -> Result<()> {
+        if all {
+            self.kill_all_processes(signal)
+        } else {
+            self.kill_one_process(signal)
         }
     }
+
+    fn kill_one_process<S: Into<Signal>>(&self, signal: S) -> Result<()> {
+        let signal = signal.into().into_raw();
+        let pid = self.pid().unwrap();
+
+        log::debug!("kill signal {} to {}", signal, pid);
+        signal::kill(pid, signal)?;
+        // 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 cgroups_path = self.spec()?.cgroup_path;
+                    let use_systemd = self
+                        .systemd()
+                        .context("container state does not contain cgroup manager")?;
+                    let cmanger = create_cgroup_manager(&cgroups_path, use_systemd, self.id())?;
+                    cmanger.freeze(libcgroups::common::FreezerState::Thawed)?;
+                }
+                libcgroups::common::CgroupSetup::Unified => {}
+            }
+        }
+        Ok(())
+    }
+
+    fn kill_all_processes<S: Into<Signal>>(&self, signal: S) -> Result<()> {
+        let signal = signal.into().into_raw();
+        let cgroups_path = self.spec()?.cgroup_path;
+        let use_systemd = self
+            .systemd()
+            .context("container state does not contain cgroup manager")?;
+        let cmanger = create_cgroup_manager(&cgroups_path, use_systemd, self.id())?;
+        let ret = cmanger.freeze(libcgroups::common::FreezerState::Frozen);
+        if ret.is_err() {
+            log::warn!(
+                "failed to freeze container {}, error: {}",
+                self.id(),
+                ret.unwrap_err()
+            );
+        }
+        let pids = cmanger.get_all_pids()?;
+        pids.iter().try_for_each(|&pid| {
+            log::debug!("kill signal {} to {}", signal, pid);
+            signal::kill(pid, signal)
+        })?;
+        let ret = cmanger.freeze(libcgroups::common::FreezerState::Thawed);
+        if ret.is_err() {
+            log::warn!(
+                "failed to thaw container {}, error: {}",
+                self.id(),
+                ret.unwrap_err()
+            );
+        }
+        Ok(())
+    }
 }