Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get the result of exec command #1018

Merged
merged 11 commits into from
Sep 20, 2022
24 changes: 13 additions & 11 deletions crates/libcontainer/src/container/builder_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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) =
YJDoc2 marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand All @@ -138,7 +140,7 @@ impl<'a> ContainerBuilderImpl<'a> {
.context("Failed to save container state")?;
}

Ok(())
Ok(intermediate)
}

fn cleanup_container(&self) -> Result<()> {
Expand Down
9 changes: 5 additions & 4 deletions crates/libcontainer/src/container/tenant_builder.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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")?;
Expand Down Expand Up @@ -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> {
Expand Down
3 changes: 2 additions & 1 deletion crates/libcontainer/src/process/container_init_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -115,7 +116,7 @@ pub fn container_intermediate_process(
.close()
.context("failed to close unused init sender")?;

Ok(())
return Ok(pid);
}

fn apply_cgroups<C: CgroupManager + ?Sized>(
Expand Down
29 changes: 24 additions & 5 deletions crates/libcontainer/src/process/container_main_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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(
Expand Down
18 changes: 11 additions & 7 deletions crates/libcontainer/src/process/fork.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
YJDoc2 marked this conversation as resolved.
Show resolved Hide resolved
// 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);
}
Expand All @@ -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);
Expand Down
13 changes: 10 additions & 3 deletions crates/youki/src/commands/exec.rs
Original file line number Diff line number Diff line change
@@ -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())?
Expand All @@ -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),
}
}
5 changes: 4 additions & 1 deletion crates/youki/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down