From cf39018f4c2a5d6ebd08cb982cfbf5699c9c8a18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Sat, 1 Feb 2025 22:14:16 +0100 Subject: [PATCH 1/4] Fix container cleanup A flake configured to be attached can also be re-started using the same container storage. However, the container was always removed when the command exited. This commit fixes it to avoid removing the container of attach type flakes. In addition a flake option %remove was added to allow removing the container created for resume and attach type flakes --- doc/podman-pilot.rst | 9 ++++++ podman-pilot/src/podman.rs | 60 +++++++++++++++++++++++--------------- 2 files changed, 46 insertions(+), 23 deletions(-) diff --git a/doc/podman-pilot.rst b/doc/podman-pilot.rst index 4b61301..95787db 100644 --- a/doc/podman-pilot.rst +++ b/doc/podman-pilot.rst @@ -163,6 +163,15 @@ OPTIONS terminal or not. This options allows to override the detection. +%remove + + When running in attach or resume mode, podman-pilot keeps the + container created such that the call of the same flake can either + start/attach the container with its entry command or execute + the command in the resume/running container. If the container + should be removed at the end of the process execution, pass + this flake option to the call. + DEBUGGING --------- diff --git a/podman-pilot/src/podman.rs b/podman-pilot/src/podman.rs index c2e0281..0c4f3d0 100644 --- a/podman-pilot/src/podman.rs +++ b/podman-pilot/src/podman.rs @@ -456,11 +456,14 @@ pub fn start(program_name: &str, cid: &str) -> Result<(), FlakeError> { Start container with the given container ID !*/ let RuntimeSection { resume, attach, .. } = config().runtime(); - + + let pilot_options = Lookup::get_pilot_run_options(); let current_user = get_current_username().unwrap(); let user = User::from(current_user.to_str().unwrap()); let is_running = container_running(cid, user)?; + let is_created = container_exists(&cid, user)?; + let mut is_removed = false; if is_running { if attach { @@ -477,6 +480,13 @@ pub fn start(program_name: &str, cid: &str) -> Result<(), FlakeError> { } else { // 4. Startup container call_instance("start", cid, program_name, user)?; + if ! attach || ! is_created { + call_instance("rm_force", cid, program_name, user)?; + is_removed = true + } + }; + + if pilot_options.contains_key("%remove") && ! is_removed { call_instance("rm_force", cid, program_name, user)?; }; Ok(()) @@ -668,6 +678,31 @@ pub fn init_cid_dir() -> Result<(), FlakeError> { Ok(()) } +pub fn container_exists(cid: &str, user: User) -> Result { + /*! + Check if container exists according to the specified cid + !*/ + let mut exists = user.run("podman"); + exists.arg("container").arg("exists").arg(cid); + if Lookup::is_debug() { + debug!("{:?}", exists.get_args()); + } + match exists.status() { + Ok(status) => { + if status.success() { + return Ok(status.success()) + } else { + let _ = Container::podman_setup_permissions(); + exists.status()?; + } + } + Err(error) => { + return Err(error) + } + } + Ok(exists.status()?.success()) +} + pub fn container_running(cid: &str, user: User) -> Result { /*! Check if container with specified cid is running @@ -849,28 +884,7 @@ pub fn gc_cid_file( !*/ let cid = fs::read_to_string(container_cid_file)?; - let mut exists = user.run("podman"); - exists.arg("container") - .arg("exists") - .arg(&cid); - if Lookup::is_debug() { - debug!("{:?}", exists.get_args()); - } - let status = match exists.output() { - Ok(output) => { - if output.status.success() { - output.status - } else { - let _ = Container::podman_setup_permissions(); - exists.output()?.status - } - } - Err(_) => { - let _ = Container::podman_setup_permissions(); - exists.output()?.status - } - }; - if status.success() { + if container_exists(&cid, user)? { Ok(true) } else { fs::remove_file(container_cid_file)?; From 377fe3d2601139386cd74ef5d7e201fbea0392ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Sat, 1 Feb 2025 22:37:25 +0100 Subject: [PATCH 2/4] Fix building runtime arguments Use get_run_cmdline method everywhere --- podman-pilot/src/podman.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/podman-pilot/src/podman.rs b/podman-pilot/src/podman.rs index 0c4f3d0..64bb59f 100644 --- a/podman-pilot/src/podman.rs +++ b/podman-pilot/src/podman.rs @@ -509,8 +509,6 @@ pub fn call_instance( /*! Call container ID based podman commands !*/ - let args: Vec = env::args().collect(); - let RuntimeSection { resume, .. } = config().runtime(); let pilot_options = Lookup::get_pilot_run_options(); @@ -542,10 +540,8 @@ pub fn call_instance( call.arg( get_target_app_path(program_name) ); - for arg in &args[1..] { - if ! arg.starts_with('@') { - call.arg(arg); - } + for arg in Lookup::get_run_cmdline(Vec::new(), false) { + call.arg(arg); } } if Lookup::is_debug() { From 59beb6c3313ae6795b0e4878171acd034a319b89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Sat, 1 Feb 2025 22:46:14 +0100 Subject: [PATCH 3/4] Make clippy happy --- firecracker-pilot/guestvm-tools/sci/src/main.rs | 2 +- podman-pilot/src/podman.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/firecracker-pilot/guestvm-tools/sci/src/main.rs b/firecracker-pilot/guestvm-tools/sci/src/main.rs index 0edaf71..dcf70cb 100644 --- a/firecracker-pilot/guestvm-tools/sci/src/main.rs +++ b/firecracker-pilot/guestvm-tools/sci/src/main.rs @@ -400,7 +400,7 @@ fn main() { if do_exec { // replace ourselves debug(&format!("EXEC: {} -> {:?}", &args[0], call.get_args())); - call.exec(); + let _ = call.exec(); } else { // call a command and keep control debug(&format!( diff --git a/podman-pilot/src/podman.rs b/podman-pilot/src/podman.rs index 64bb59f..e3b8cdd 100644 --- a/podman-pilot/src/podman.rs +++ b/podman-pilot/src/podman.rs @@ -182,9 +182,9 @@ pub fn create( } // create the container with configured runtime arguments + let var_pattern = Regex::new(r"%([A-Z]+)").unwrap(); for arg in podman.iter().flatten().flat_map(|x| x.splitn(2, ' ')) { let mut arg_value = arg.to_string(); - let var_pattern = Regex::new(r"%([A-Z]+)").unwrap(); while var_pattern.captures(&arg_value.clone()).is_some() { for capture in var_pattern.captures_iter(&arg_value.clone()) { // replace %VAR placeholder(s) with the respective @@ -462,7 +462,7 @@ pub fn start(program_name: &str, cid: &str) -> Result<(), FlakeError> { let user = User::from(current_user.to_str().unwrap()); let is_running = container_running(cid, user)?; - let is_created = container_exists(&cid, user)?; + let is_created = container_exists(cid, user)?; let mut is_removed = false; if is_running { From f709fb2176619cd6799366b48bff3b0c573f0050 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcus=20Sch=C3=A4fer?= Date: Sat, 1 Feb 2025 22:47:35 +0100 Subject: [PATCH 4/4] Use actions/upload-artifact: v4 --- .github/workflows/ci-robot-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-robot-tests.yml b/.github/workflows/ci-robot-tests.yml index 74c3d63..cedf8b2 100644 --- a/.github/workflows/ci-robot-tests.yml +++ b/.github/workflows/ci-robot-tests.yml @@ -27,7 +27,7 @@ jobs: run: python3 -m robot robot_tests - name: Upload Log if: always() - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: robot-log path: /home/runner/work/flake-pilot/flake-pilot/log.html