Skip to content

Commit

Permalink
Merge pull request #58 from OSInside/fix_container_cleanup
Browse files Browse the repository at this point in the history
Fix container cleanup
  • Loading branch information
schaefi authored Feb 1, 2025
2 parents 228b712 + f709fb2 commit 740b01f
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 32 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci-robot-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions doc/podman-pilot.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
---------

Expand Down
2 changes: 1 addition & 1 deletion firecracker-pilot/guestvm-tools/sci/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
70 changes: 40 additions & 30 deletions podman-pilot/src/podman.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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(())
Expand All @@ -499,8 +509,6 @@ pub fn call_instance(
/*!
Call container ID based podman commands
!*/
let args: Vec<String> = env::args().collect();

let RuntimeSection { resume, .. } = config().runtime();

let pilot_options = Lookup::get_pilot_run_options();
Expand Down Expand Up @@ -532,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() {
Expand Down Expand Up @@ -668,6 +674,31 @@ pub fn init_cid_dir() -> Result<(), FlakeError> {
Ok(())
}

pub fn container_exists(cid: &str, user: User) -> Result<bool, std::io::Error> {
/*!
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<bool, CommandError> {
/*!
Check if container with specified cid is running
Expand Down Expand Up @@ -849,28 +880,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)?;
Expand Down

0 comments on commit 740b01f

Please sign in to comment.