Skip to content

Commit

Permalink
Merge #922
Browse files Browse the repository at this point in the history
922: Adds numerous bug fixes for remote cross. r=Emilgardis a=Alexhuszagh

Adds simpler container and volume cleanup, and avoiding keeping anonymous volumes for `/cargo/bin` present after the container exited. It also ensures containers are stopped even if a signal is sent to the `cross` command (except `SIGKILL`), and ensures that containers exit quicker if possible. This is done by using the presence of a TTY, and if it is present, running the default entrypoint, which responds to signals. If we do not have a TTY, the image will not respond to signals, so we send the container a `SIGKILL` via `docker stop` using a an instant timeout (0).

This also fixes an issue with stale fingerprint data: if a persistent data volume was deleted and then recreated, previously the project data would not be copied over. This meant that the project could not be found, and it would require users to manually delete the fingerprint data or use single-used volumes.

This also checks if the container has been scheduled for deletion (the drop may not have completed), and errors out early if more calls are made to the container.

This also properly handles `docker rm` calls if the `docker stop` command worked without needing a `SIGKILL`: it silences the output of the error if the container wasn't found.

Co-authored-by: Alex Huszagh <ahuszagh@gmail.com>
  • Loading branch information
bors[bot] and Alexhuszagh authored Jul 8, 2022
2 parents ca16d4a + 2e5755c commit d43dc25
Show file tree
Hide file tree
Showing 4 changed files with 229 additions and 78 deletions.
21 changes: 15 additions & 6 deletions src/bin/commands/containers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ pub fn create_persistent_volume(
let state = docker::remote::container_state(engine, &container, msg_info)?;
if !state.is_stopped() {
msg_info.warn("container {container} was running.")?;
docker::remote::container_stop(engine, &container, msg_info)?;
docker::remote::container_stop_default(engine, &container, msg_info)?;
}
if state.exists() {
msg_info.warn("container {container} was exited.")?;
Expand All @@ -407,14 +407,24 @@ pub fn create_persistent_volume(
let mount_prefix = docker::remote::MOUNT_PREFIX;
let mut docker = docker::subcommand(engine, "run");
docker.args(&["--name", &container]);
docker.arg("--rm");
docker.args(&["-v", &format!("{}:{}", volume, mount_prefix)]);
docker.arg("-d");
if io::Stdin::is_atty() && io::Stdout::is_atty() && io::Stderr::is_atty() {
let is_tty = io::Stdin::is_atty() && io::Stdout::is_atty() && io::Stderr::is_atty();
if is_tty {
docker.arg("-t");
}
docker.arg(docker::UBUNTU_BASE);
// ensure the process never exits until we stop it
docker.args(&["sh", "-c", "sleep infinity"]);
if !is_tty {
// ensure the process never exits until we stop it
// we only need this infinite loop if we don't allocate
// a TTY. this has a few issues though: now, the
// container no longer responds to signals, so the
// container will need to be sig-killed.
docker.args(&["sh", "-c", "sleep infinity"]);
}
// store first, since failing to non-existing container is fine
docker::remote::create_container_deleter(engine.clone(), container.clone());
docker.run_and_get_status(msg_info, false)?;

docker::remote::copy_volume_container_xargo(
Expand Down Expand Up @@ -443,8 +453,7 @@ pub fn create_persistent_volume(
msg_info,
)?;

docker::remote::container_stop(engine, &container, msg_info)?;
docker::remote::container_rm(engine, &container, msg_info)?;
docker::remote::drop_container(is_tty, msg_info);

Ok(())
}
Expand Down
Loading

0 comments on commit d43dc25

Please sign in to comment.