From f5f65d6b5f9c940d1eef18918b55ca9db112e41f Mon Sep 17 00:00:00 2001 From: CarloQuick <98628846+CarloQuick@users.noreply.github.com> Date: Tue, 3 Feb 2026 20:06:58 +0900 Subject: [PATCH 1/4] feat(start): Added concurrent operation protection and test. --- src/json.rs | 2 +- src/main.rs | 4 ++-- src/runtime.rs | 28 +++++++++++++++++++++++++++- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/json.rs b/src/json.rs index c0dc95d..8bc7978 100644 --- a/src/json.rs +++ b/src/json.rs @@ -19,7 +19,7 @@ use std::{ #[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] pub struct Container { - dir: String, + pub dir: String, pub state: State, pub pid: Option, } diff --git a/src/main.rs b/src/main.rs index 4476d27..cc8d686 100644 --- a/src/main.rs +++ b/src/main.rs @@ -46,8 +46,8 @@ fn main() -> Result<()> { } Some(Commands::Start { name }) => { match json::check_existing_container(name, &env.bento_containers_env_path) { - Some(_container) => { - if let Err(e) = start(name, &env) { + Some(container) => { + if let Err(e) = start(&container, name, &env) { anyhow::bail!("Starting {} failed! Error: {}.", name, e); } } diff --git a/src/runtime.rs b/src/runtime.rs index 15ed28a..8176caa 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -361,7 +361,10 @@ fn _clean_up(container_dir: &PathBuf) -> Result<()> { Ok(()) } -pub fn start(name: &str, env: &Env) -> Result<()> { +pub fn start(container: &Container, name: &str, env: &Env) -> Result<()> { + if container.state != State::Created || container.state != State::Stopped { + anyhow::bail!("Container failed to start! Check its status."); + } let container_config_path = &env .bento_containers_env_path .join(name) @@ -763,4 +766,27 @@ mod tests { assert_eq!(!containers_dir.exists(), true); Ok(()) } + #[test] + fn container_limited_from_starting_by_state() { + let container: Container = Container { + dir: String::from("temp"), + state: State::Running, + pid: None, + }; + let env = Env { + bento_dir: PathBuf::from("/test_dir"), + bento_image_env_path: PathBuf::from("/test_image_path"), + bento_containers_env_path: PathBuf::from("/test_container_path"), + }; + + let result = start(&container, &"container_name", &env); + + assert!(result.is_err()); + + let error = result.unwrap_err(); + assert_eq!( + error.to_string(), + "Container failed to start! Check its status." + ); + } } From 0b8b7fe4e9ded3d9fa7c0fe0beb64106d8201e46 Mon Sep 17 00:00:00 2001 From: CarloQuick <98628846+CarloQuick@users.noreply.github.com> Date: Tue, 3 Feb 2026 20:16:36 +0900 Subject: [PATCH 2/4] feat(kill): Container must be running to apply kill and test --- src/main.rs | 4 ++-- src/runtime.rs | 44 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/main.rs b/src/main.rs index cc8d686..97d342f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,7 +6,7 @@ use bento::{ bento_cli::{Cli, Commands}, env::Env, json::{self, State}, - runtime::{create, exec, kill_proc, start, stop}, + runtime::{create, exec, kill_container, start, stop}, }; use clap::Parser; use dotenv::dotenv; @@ -92,7 +92,7 @@ fn main() -> Result<()> { } Some(Commands::Kill { name }) => { match json::check_existing_container(name, &env.bento_containers_env_path) { - Some(container) => match kill_proc(&container) { + Some(container) => match kill_container(&container) { Ok(()) => eprintln!("Container {} killed successfully", name), Err(e) => eprintln!("{:?}", e), }, diff --git a/src/runtime.rs b/src/runtime.rs index 8176caa..764dfc2 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -489,6 +489,10 @@ fn apply_signal(pid: Pid, signal: Signal) -> Result<()> { } pub fn stop(name: &str, container: &Container, env: &Env) -> Result<()> { + if container.state != State::Running { + anyhow::bail!("Container not currently running."); + } + if let Some(c_pid) = container.pid { let pid = Pid::from_raw(c_pid); match apply_signal(pid, Signal::SIGTERM) { @@ -522,7 +526,10 @@ pub fn stop(name: &str, container: &Container, env: &Env) -> Result<()> { }; } -pub fn kill_proc(container: &Container) -> Result<()> { +pub fn kill_container(container: &Container) -> Result<()> { + if container.state != State::Running { + anyhow::bail!("Container not currently running."); + } if let Some(c_pid) = container.pid { let pid = Pid::from_raw(c_pid); match apply_signal(pid, Signal::SIGKILL) { @@ -789,4 +796,39 @@ mod tests { "Container failed to start! Check its status." ); } + #[test] + fn container_must_be_running_to_stop() { + let container: Container = Container { + dir: String::from("temp"), + state: State::Stopped, + pid: None, + }; + let env = Env { + bento_dir: PathBuf::from("/test_dir"), + bento_image_env_path: PathBuf::from("/test_image_path"), + bento_containers_env_path: PathBuf::from("/test_container_path"), + }; + + let result = stop(&"container_name", &container, &env); + + assert!(result.is_err()); + + let error = result.unwrap_err(); + assert_eq!(error.to_string(), "Container not currently running."); + } + #[test] + fn container_must_be_running_to_kill() { + let container: Container = Container { + dir: String::from("temp"), + state: State::Stopped, + pid: None, + }; + + let result = kill_container(&container); + + assert!(result.is_err()); + + let error = result.unwrap_err(); + assert_eq!(error.to_string(), "Container not currently running."); + } } From 9bce123c4cf172845dc2ec06d56c96312eed82ea Mon Sep 17 00:00:00 2001 From: CarloQuick <98628846+CarloQuick@users.noreply.github.com> Date: Tue, 3 Feb 2026 20:26:41 +0900 Subject: [PATCH 3/4] fix(create): Refactor --- src/main.rs | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/main.rs b/src/main.rs index 97d342f..91f934d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -27,23 +27,28 @@ fn main() -> Result<()> { mount, current_working_directory, command, - }) => { - let cwd = match current_working_directory { - Some(c) => c, - None => &PathBuf::from("/"), - }; - let mount_dir = match mount { - Some(m) => m, - None => &PathBuf::new(), - }; + }) => match json::check_existing_container(name, &env.bento_containers_env_path) { + Some(_) => { + anyhow::bail!("Container already exists!"); + } + None => { + let cwd = match current_working_directory { + Some(c) => c, + None => &PathBuf::from("/"), + }; + let mount_dir = match mount { + Some(m) => m, + None => &PathBuf::new(), + }; - match create(name, image, mount_dir, cwd, command, &env) { - Ok(_) => { - eprintln!("🍱 Bento Container {} finished", name) - } - Err(e) => return Err(anyhow!("Container not created.\n Error: {}.", e)), - }; - } + match create(name, image, mount_dir, cwd, command, &env) { + Ok(_) => { + eprintln!("🍱 Bento Container {} finished", name) + } + Err(e) => return Err(anyhow!("Container not created.\n Error: {}.", e)), + }; + } + }, Some(Commands::Start { name }) => { match json::check_existing_container(name, &env.bento_containers_env_path) { Some(container) => { From c0b96f03e8930279835f0721356bb31289469b49 Mon Sep 17 00:00:00 2001 From: CarloQuick <98628846+CarloQuick@users.noreply.github.com> Date: Tue, 3 Feb 2026 20:47:53 +0900 Subject: [PATCH 4/4] fix(pr): Better state validation --- src/main.rs | 2 +- src/runtime.rs | 147 +++++++++++++++++++++++++------------------------ 2 files changed, 75 insertions(+), 74 deletions(-) diff --git a/src/main.rs b/src/main.rs index 91f934d..d4d55db 100644 --- a/src/main.rs +++ b/src/main.rs @@ -86,7 +86,7 @@ fn main() -> Result<()> { } Some(Commands::Stop { name }) => { match json::check_existing_container(name, &env.bento_containers_env_path) { - Some(container) => match stop(name, &container, &env) { + Some(container) => match stop(&container, name, &env) { Ok(()) => eprintln!("Container {} stopped successfully", name), Err(e) => eprintln!("{:?}", e), }, diff --git a/src/runtime.rs b/src/runtime.rs index 764dfc2..f0318ae 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -362,42 +362,42 @@ fn _clean_up(container_dir: &PathBuf) -> Result<()> { } pub fn start(container: &Container, name: &str, env: &Env) -> Result<()> { - if container.state != State::Created || container.state != State::Stopped { - anyhow::bail!("Container failed to start! Check its status."); - } - let container_config_path = &env - .bento_containers_env_path - .join(name) - .join("bento_config.json"); + if container.state == State::Created || container.state == State::Stopped { + let container_config_path = &env + .bento_containers_env_path + .join(name) + .join("bento_config.json"); - let bento_config = get_bento_config(&container_config_path).with_context(|| { - format!( - "Container {} failed to load the bento_config.json at {}.", + let bento_config = get_bento_config(&container_config_path).with_context(|| { + format!( + "Container {} failed to load the bento_config.json at {}.", + name, + container_config_path.display() + ) + })?; + unshare_user_namespace().with_context(|| { + format!( + "Container {} failed to go rootless by unsharing user namespace.", + name + ) + })?; + unshare_mount_namespace() + .with_context(|| format!("Container {} failed to unshare mount namespace.", name))?; + if let Err(err) = mount_fs_overlay(&bento_config) { + unmount_and_clean_up(&bento_config) + .context("Failed to unmount and cleantup after failed start.")?; + return Err(anyhow!("Err: {}", err)); + }; + fork_into_namespaces( + &bento_config, name, - container_config_path.display() - ) - })?; - unshare_user_namespace().with_context(|| { - format!( - "Container {} failed to go rootless by unsharing user namespace.", - name + &env.bento_containers_env_path + .join("container_manifest.json"), ) - })?; - unshare_mount_namespace() - .with_context(|| format!("Container {} failed to unshare mount namespace.", name))?; - if let Err(err) = mount_fs_overlay(&bento_config) { - unmount_and_clean_up(&bento_config) - .context("Failed to unmount and cleantup after failed start.")?; - return Err(anyhow!("Err: {}", err)); - }; - fork_into_namespaces( - &bento_config, - name, - &env.bento_containers_env_path - .join("container_manifest.json"), - ) - .with_context(|| format!("Container {} faild to fork the process.", name))?; - + .with_context(|| format!("Container {} faild to fork the process.", name))?; + } else { + anyhow::bail!("Container failed to start! Check its status."); + } Ok(()) } @@ -488,57 +488,58 @@ fn apply_signal(pid: Pid, signal: Signal) -> Result<()> { Ok(()) } -pub fn stop(name: &str, container: &Container, env: &Env) -> Result<()> { - if container.state != State::Running { - anyhow::bail!("Container not currently running."); - } - - if let Some(c_pid) = container.pid { - let pid = Pid::from_raw(c_pid); - match apply_signal(pid, Signal::SIGTERM) { - Ok(()) => { - for i in 1..=10 { - if let Some(c) = - json::check_existing_container(name, &env.bento_containers_env_path) - { - match c.state { - State::Stopped => return Ok(()), - _ => { - if i < 10 { - thread::sleep(time::Duration::from_secs(1)); - } else { - apply_signal(pid, Signal::SIGKILL).with_context(|| { - format!("Failed to apply SIGKILL to pid {}", pid) - })?; - return Ok(()); +pub fn stop(container: &Container, name: &str, env: &Env) -> Result<()> { + if container.state == State::Running { + if let Some(c_pid) = container.pid { + let pid = Pid::from_raw(c_pid); + match apply_signal(pid, Signal::SIGTERM) { + Ok(()) => { + for i in 1..=10 { + if let Some(c) = + json::check_existing_container(name, &env.bento_containers_env_path) + { + match c.state { + State::Stopped => return Ok(()), + _ => { + if i < 10 { + thread::sleep(time::Duration::from_secs(1)); + } else { + apply_signal(pid, Signal::SIGKILL).with_context(|| { + format!("Failed to apply SIGKILL to pid {}", pid) + })?; + return Ok(()); + } } } } } + thread::sleep(Duration::from_millis(200)); + return Ok(()); } - thread::sleep(Duration::from_millis(200)); - return Ok(()); + Err(e) => return Err(e), } - Err(e) => return Err(e), - } + } else { + return Err(anyhow!(ErrorKind::NotFound)); + }; } else { - return Err(anyhow!(ErrorKind::NotFound)); - }; + anyhow::bail!("Container not currently running."); + } } pub fn kill_container(container: &Container) -> Result<()> { - if container.state != State::Running { + if container.state == State::Running { + if let Some(c_pid) = container.pid { + let pid = Pid::from_raw(c_pid); + match apply_signal(pid, Signal::SIGKILL) { + Ok(()) => return Ok(()), + Err(e) => return Err(e), + } + } else { + return Err(anyhow!(ErrorKind::NotFound)); + }; + } else { anyhow::bail!("Container not currently running."); } - if let Some(c_pid) = container.pid { - let pid = Pid::from_raw(c_pid); - match apply_signal(pid, Signal::SIGKILL) { - Ok(()) => return Ok(()), - Err(e) => return Err(e), - } - } else { - return Err(anyhow!(ErrorKind::NotFound)); - }; } pub fn exec( @@ -809,7 +810,7 @@ mod tests { bento_containers_env_path: PathBuf::from("/test_container_path"), }; - let result = stop(&"container_name", &container, &env); + let result = stop(&container, &"container_name", &env); assert!(result.is_err());