Skip to content

Commit

Permalink
Add channel message when exec fails
Browse files Browse the repository at this point in the history
Signed-off-by: Yashodhan Joshi <[email protected]>
  • Loading branch information
YJDoc2 committed Feb 23, 2024
1 parent 919ff4a commit 7f8422f
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 12 deletions.
4 changes: 4 additions & 0 deletions crates/libcontainer/src/process/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@ impl MainReceiver {
})?;
match msg {
Message::InitReady => Ok(()),
// this case in unique and known enough to have a special error format
Message::ExecFailed(err) => Err(ChannelError::ExecError(format!(
"error in executing process : {err}"
))),
msg => Err(ChannelError::UnexpectedMessage {
expected: Message::InitReady,
received: msg,
Expand Down
6 changes: 6 additions & 0 deletions crates/libcontainer/src/process/container_init_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,12 @@ pub fn container_init_process(
})?;
}

// As we have chagned the root mount, from here on
// logs are no longer visible in journalctl
// so make sure that you bubble up any errors
// and do not call unwrap as any panics would not be correctly logged


rootfs.adjust_root_mount_propagation(linux).map_err(|err| {
tracing::error!(?err, "failed to adjust root mount propagation");
InitProcessError::RootFS(err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,19 +122,19 @@ pub fn container_intermediate_process(
match container_init_process(&args, &mut main_sender, &mut init_receiver) {
Ok(_) => 0,
Err(e) => {
tracing::error!("failed to initialize container process: {e}");
if let Err(err) = main_sender.exec_failed(e.to_string()) {
tracing::error!(?err, "failed sending error to main sender");
}
if let ContainerType::TenantContainer { exec_notify_fd } = args.container_type {
let buf = format!("{e}");
if let Err(err) = write(exec_notify_fd, buf.as_bytes()) {
tracing::error!(?err, "failed to write to exec notify fd");
return -1;
}
if let Err(err) = close(exec_notify_fd) {
tracing::error!(?err, "failed to close exec notify fd");
return -1;
}
}

tracing::error!("failed to initialize container process: {e}");
-1
}
}
Expand Down
31 changes: 25 additions & 6 deletions crates/libcontainer/src/workload/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,13 @@ impl Executor for DefaultExecutor {
.collect();
unistd::execvp(&cstring_path, &a).map_err(|err| {
tracing::error!(?err, filename = ?cstring_path, args = ?a, "failed to execvp");
ExecutorError::Execution(err.into())
ExecutorError::Execution(
format!(
"error '{}' executing '{:?}' with args '{:?}'",
err, cstring_path, a
)
.into(),
)
})?;

// After execvp is called, the process is replaced with the container
Expand All @@ -46,14 +52,18 @@ impl Executor for DefaultExecutor {
let proc = spec
.process()
.as_ref()
.ok_or(ExecutorValidationError::InvalidArg)?;
.ok_or(ExecutorValidationError::ArgValidationError(
"spec did not contain process".into(),
))?;

if let Some(args) = proc.args() {
let envs: Vec<String> = proc.env().as_ref().unwrap_or(&vec![]).clone();
let path_vars: Vec<&String> = envs.iter().filter(|&e| e.starts_with("PATH=")).collect();
if path_vars.is_empty() {
tracing::error!("PATH environment variable is not set");
Err(ExecutorValidationError::InvalidArg)?;
Err(ExecutorValidationError::ArgValidationError(
"PATH environment variable is not set".into(),
))?;
}
let path_var = path_vars[0].trim_start_matches("PATH=");
match get_executable_path(&args[0], path_var) {
Expand All @@ -62,7 +72,10 @@ impl Executor for DefaultExecutor {
executable = ?args[0],
"executable for container process not found in PATH",
);
Err(ExecutorValidationError::InvalidArg)?;
Err(ExecutorValidationError::ArgValidationError(format!(
"executable '{}' not found in $PATH",
args[0]
)))?;
}
Some(path) => match is_executable(&path) {
Ok(true) => {
Expand All @@ -73,15 +86,21 @@ impl Executor for DefaultExecutor {
executable = ?path,
"executable does not have the correct permission set",
);
Err(ExecutorValidationError::InvalidArg)?;
Err(ExecutorValidationError::ArgValidationError(format!(
"executable '{}' at path '{:?}' does not have correct permissions",
args[0], path
)))?;
}
Err(err) => {
tracing::error!(
executable = ?path,
?err,
"failed to check permissions for executable",
);
Err(ExecutorValidationError::InvalidArg)?;
Err(ExecutorValidationError::ArgValidationError(format!(
"failed to check permissions for executable '{}' at path '{:?}' : {}",
args[0], path, err
)))?;
}
},
}
Expand Down
4 changes: 2 additions & 2 deletions crates/libcontainer/src/workload/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ pub enum ExecutorError {
pub enum ExecutorValidationError {
#[error("{0} executor can't handle spec")]
CantHandle(&'static str),
#[error("invalid argument")]
InvalidArg,
#[error("{0}")]
ArgValidationError(String),
}

// Here is an explanation about the complexity below regarding to
Expand Down
3 changes: 3 additions & 0 deletions crates/youki/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ fn main() -> Result<()> {
CommonCmd::Exec(exec) => match commands::exec::exec(exec, root_path) {
Ok(exit_code) => std::process::exit(exit_code),
Err(e) => {
tracing::error!("error in executing command: {:?}", e);
eprintln!("exec failed : {e}");
std::process::exit(-1);
}
Expand All @@ -135,6 +136,7 @@ fn main() -> Result<()> {
CommonCmd::Run(run) => match commands::run::run(run, root_path, systemd_cgroup) {
Ok(exit_code) => std::process::exit(exit_code),
Err(e) => {
tracing::error!("error in executing command: {:?}", e);
eprintln!("run failed : {e}");
std::process::exit(-1);
}
Expand All @@ -151,6 +153,7 @@ fn main() -> Result<()> {

if let Err(ref e) = cmd_result {
tracing::error!("error in executing command: {:?}", e);
eprintln!("error in executing command: {:?}", e);
}
cmd_result
}

0 comments on commit 7f8422f

Please sign in to comment.