Merge pull request #1018 from utam0k/feature/exec-wait-pid

Get the result of exec command

crates/libcontainer/src/container/builder_impl.rs

@@ -8,6 +8,7 @@ use crate::{
     utils,
 };
 use anyhow::{bail, Context, Result};
+use nix::unistd::Pid;
 use oci_spec::runtime::Spec;
 use std::{fs, io::Write, os::unix::prelude::RawFd, path::PathBuf};
 
@@ -40,19 +41,19 @@ pub(super) struct ContainerBuilderImpl<'a> {
 }
 
 impl<'a> ContainerBuilderImpl<'a> {
-    pub(super) fn create(&mut self) -> Result<()> {
-        if let Err(outer) = self.run_container().context("failed to create container") {
-            if let Err(inner) = self.cleanup_container() {
-                return Err(outer.context(inner));
+    pub(super) fn create(&mut self) -> Result<Pid> {
+        match self.run_container().context("failed to create container") {
+            Ok(pid) => Ok(pid),
+            Err(outer) => {
+                if let Err(inner) = self.cleanup_container() {
+                    return Err(outer.context(inner));
+                }
+                Err(outer)
             }
-
-            return Err(outer);
         }
-
-        Ok(())
     }
 
-    fn run_container(&mut self) -> Result<()> {
+    fn run_container(&mut self) -> Result<Pid> {
         let linux = self.spec.linux().as_ref().context("no linux in spec")?;
         let cgroups_path = utils::get_cgroup_path(
             linux.cgroups_path(),
@@ -121,7 +122,8 @@ impl<'a> ContainerBuilderImpl<'a> {
             cgroup_manager: cmanager,
         };
 
-        let init_pid = process::container_main_process::container_main_process(&container_args)?;
+        let (intermediate, init_pid) =
+            process::container_main_process::container_main_process(&container_args, !self.init)?;
 
         // if file to write the pid to is specified, write pid of the child
         if let Some(pid_file) = &self.pid_file {
@@ -138,7 +140,7 @@ impl<'a> ContainerBuilderImpl<'a> {
                 .context("Failed to save container state")?;
         }
 
-        Ok(())
+        Ok(intermediate)
     }
 
     fn cleanup_container(&self) -> Result<()> {

crates/libcontainer/src/container/tenant_builder.rs

@@ -1,6 +1,6 @@
 use anyhow::{bail, Context, Result};
 use caps::Capability;
-use nix::unistd;
+use nix::unistd::{self, Pid};
 use oci_spec::runtime::{
     Capabilities as SpecCapabilities, Capability as SpecCapability, LinuxBuilder,
     LinuxCapabilities, LinuxCapabilitiesBuilder, LinuxNamespace, LinuxNamespaceBuilder,
@@ -89,7 +89,7 @@ impl<'a> TenantContainerBuilder<'a> {
     }
 
     /// Joins an existing container
-    pub fn build(self) -> Result<()> {
+    pub fn build(self) -> Result<Pid> {
         let container_dir = self
             .lookup_container_dir()
             .context("failed to look up container dir")?;
@@ -131,11 +131,12 @@ impl<'a> TenantContainerBuilder<'a> {
             preserve_fds: self.base.preserve_fds,
         };
 
-        builder_impl.create()?;
+        let pid = builder_impl.create()?;
 
         let mut notify_socket = NotifySocket::new(notify_path);
         notify_socket.notify_container_start()?;
-        Ok(())
+
+        Ok(pid)
     }
 
     fn lookup_container_dir(&self) -> Result<PathBuf> {

crates/libcontainer/src/process/container_init_process.rs

@@ -412,7 +412,8 @@ pub fn container_init_process(
     }
 
     if proc.args().is_some() {
-        ExecutorManager::exec(spec)
+        ExecutorManager::exec(spec)?;
+        unreachable!("process image should have been replaced after exec");
     } else {
         bail!("on non-Windows, at least one process arg entry is required")
     }

crates/libcontainer/src/process/container_intermediate_process.rs

@@ -14,7 +14,7 @@ pub fn container_intermediate_process(
     intermediate_chan: &mut (channel::IntermediateSender, channel::IntermediateReceiver),
     init_chan: &mut (channel::InitSender, channel::InitReceiver),
     main_sender: &mut channel::MainSender,
-) -> Result<()> {
+) -> Result<Pid> {
     let (inter_sender, inter_receiver) = intermediate_chan;
     let (init_sender, init_receiver) = init_chan;
     let command = &args.syscall;
@@ -95,7 +95,8 @@ pub fn container_intermediate_process(
         inter_sender
             .close()
             .context("failed to close sender in the intermediate process")?;
-        container_init_process(args, main_sender, init_receiver)
+        container_init_process(args, main_sender, init_receiver)?;
+        Ok(0)
     })?;
     // Once we fork the container init process, the job for intermediate process
     // is done. We notify the container main process about the pid we just
@@ -115,7 +116,7 @@ pub fn container_intermediate_process(
         .close()
         .context("failed to close unused init sender")?;
 
-    Ok(())
+    Ok(pid)
 }
 
 fn apply_cgroups<C: CgroupManager + ?Sized>(

crates/libcontainer/src/process/container_main_process.rs

@@ -6,13 +6,16 @@ use crate::{
 };
 use anyhow::{Context, Result};
 use nix::{
-    sys::socket::{self, UnixAddr},
+    sys::{
+        socket::{self, UnixAddr},
+        wait::{waitpid, WaitStatus},
+    },
     unistd::{self, Pid},
 };
 use oci_spec::runtime;
 use std::{io::IoSlice, path::Path};
 
-pub fn container_main_process(container_args: &ContainerArgs) -> Result<Pid> {
+pub fn container_main_process(container_args: &ContainerArgs, wait: bool) -> Result<(Pid, Pid)> {
     // We use a set of channels to communicate between parent and child process.
     // Each channel is uni-directional. Because we will pass these channel to
     // forked process, we have to be deligent about closing any unused channel.
@@ -23,12 +26,22 @@ pub fn container_main_process(container_args: &ContainerArgs) -> Result<Pid> {
     let init_chan = &mut channel::init_channel()?;
 
     let intermediate_pid = fork::container_fork(|| {
-        container_intermediate_process::container_intermediate_process(
+        let container_pid = container_intermediate_process::container_intermediate_process(
             container_args,
             inter_chan,
             init_chan,
             main_sender,
-        )
+        )?;
+
+        if wait {
+            match waitpid(container_pid, None)? {
+                WaitStatus::Exited(_, s) => Ok(s),
+                WaitStatus::Signaled(_, sig, _) => Ok(sig as i32),
+                _ => Ok(0),
+            }
+        } else {
+            Ok(0)
+        }
     })?;
     // Close down unused fds. The corresponding fds are duplicated to the
     // child process during fork.
@@ -90,7 +103,13 @@ pub fn container_main_process(container_args: &ContainerArgs) -> Result<Pid> {
 
     log::debug!("init pid is {:?}", init_pid);
 
-    Ok(init_pid)
+    // here we send both intermediate and init pid, because :
+    // init pid is required for writing it to pid_file (if) given by the high-level runtime
+    // intermediate pid is required in the case when we call exec, as we nned to wait for the
+    // intermediate process to exit, which itself waits for child process (the exec process) to exit
+    // in order to get the proper exit code. We cannot simply wait for the init_pid , that is the actual container
+    // process, as it is not (direect) child of our process
+    Ok((intermediate_pid, init_pid))
 }
 
 fn sync_seccomp(

crates/libcontainer/src/process/fork.rs

@@ -8,15 +8,19 @@ use nix::unistd::Pid;
 // using clone, we would have to manually make sure all the variables are
 // correctly send to the new process, especially Rust borrow checker will be a
 // lot of hassel to deal with every details.
-pub fn container_fork<F: FnOnce() -> Result<()>>(cb: F) -> Result<Pid> {
+pub fn container_fork<F: FnOnce() -> Result<i32>>(cb: F) -> Result<Pid> {
+    // here we return the child's pid in case of parent, the i32 in return signature,
+    // and for child, we run the callback function, and exit with the same exit code
+    // given by it. If there was any error when trying to run callback, exit with -1
     match unsafe { unistd::fork()? } {
         unistd::ForkResult::Parent { child } => Ok(child),
         unistd::ForkResult::Child => {
-            let ret = if let Err(error) = cb() {
-                log::debug!("failed to run fork: {:?}", error);
-                -1
-            } else {
-                0
+            let ret = match cb() {
+                Err(error) => {
+                    log::debug!("failed to run fork: {:?}", error);
+                    -1
+                }
+                Ok(exit_code) => exit_code,
             };
             std::process::exit(ret);
         }
@@ -31,7 +35,7 @@ mod test {
 
     #[test]
     fn test_container_fork() -> Result<()> {
-        let pid = container_fork(|| Ok(()))?;
+        let pid = container_fork(|| Ok(0))?;
         match waitpid(pid, None).expect("wait pid failed.") {
             WaitStatus::Exited(p, status) => {
                 assert_eq!(pid, p);

crates/youki/src/commands/exec.rs

@@ -1,12 +1,13 @@
 use anyhow::Result;
+use nix::sys::wait::{waitpid, WaitStatus};
 use std::path::PathBuf;
 
 use libcontainer::{container::builder::ContainerBuilder, syscall::syscall::create_syscall};
 use liboci_cli::Exec;
 
-pub fn exec(args: Exec, root_path: PathBuf) -> Result<()> {
+pub fn exec(args: Exec, root_path: PathBuf) -> Result<i32> {
     let syscall = create_syscall();
-    ContainerBuilder::new(args.container_id.clone(), syscall.as_ref())
+    let pid = ContainerBuilder::new(args.container_id.clone(), syscall.as_ref())
         .with_root_path(root_path)?
         .with_console_socket(args.console_socket.as_ref())
         .with_pid_file(args.pid_file.as_ref())?
@@ -16,5 +17,11 @@ pub fn exec(args: Exec, root_path: PathBuf) -> Result<()> {
         .with_process(args.process.as_ref())
         .with_no_new_privs(args.no_new_privs)
         .with_container_args(args.command.clone())
-        .build()
+        .build()?;
+
+    match waitpid(pid, None)? {
+        WaitStatus::Exited(_, status) => Ok(status),
+        WaitStatus::Signaled(_, sig, _) => Ok(sig as i32),
+        _ => Ok(0),
+    }
 }

crates/youki/src/main.rs

@@ -112,7 +112,10 @@ fn main() -> Result<()> {
                 commands::checkpoint::checkpoint(checkpoint, root_path)
             }
             CommonCmd::Events(events) => commands::events::events(events, root_path),
-            CommonCmd::Exec(exec) => commands::exec::exec(exec, root_path),
+            CommonCmd::Exec(exec) => {
+                let exit_code = commands::exec::exec(exec, root_path)?;
+                std::process::exit(exit_code)
+            }
             CommonCmd::List(list) => commands::list::list(list, root_path),
             CommonCmd::Pause(pause) => commands::pause::pause(pause, root_path),
             CommonCmd::Ps(ps) => commands::ps::ps(ps, root_path),