Skip to content

Commit

Permalink
Cleanup docker containers on termination (#445)
Browse files Browse the repository at this point in the history
* Cleanup docker containers on termination

The `--rm` switch tells docker to cleanup containers on
termination. Without this an explicit `remove` is required after
`stop` to clean up the container and a `prune` to remove arbitrary
volumes left behind. While these are not difficult actions to perform,
we are often unaware that we need to do them.

I've also changed `kill` to `stop` because `stop` in the drop fn
because `stop` attempts to shutdown the container gracefully.

Since containers will do clean-up on `stop` we no longer need to call `docker rm` ourselves, so removed that as well.

---------

Co-authored-by: tbro <tbro@users.noreply.github.com>
  • Loading branch information
tbro and tbro authored Feb 28, 2024
1 parent 0e92012 commit 797c877
Showing 1 changed file with 2 additions and 13 deletions.
15 changes: 2 additions & 13 deletions src/data_source/storage/sql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1606,6 +1606,7 @@ pub mod testing {

let output = Command::new("docker")
.arg("run")
.arg("--rm")
.arg("-d")
.args(["-p", &format!("{port}:5432")])
.args(["-e", "POSTGRES_PASSWORD=password"])
Expand Down Expand Up @@ -1676,7 +1677,7 @@ pub mod testing {
impl Drop for TmpDb {
fn drop(&mut self) {
let output = Command::new("docker")
.args(["kill", self.container_id.as_str()])
.args(["stop", self.container_id.as_str()])
.output()
.unwrap();
if !output.status.success() {
Expand All @@ -1686,18 +1687,6 @@ pub mod testing {
str::from_utf8(&output.stderr).unwrap()
);
}

let output = Command::new("docker")
.args(["rm", self.container_id.as_str()])
.output()
.unwrap();
if !output.status.success() {
tracing::error!(
"error removing postgres docker {}: {}",
self.container_id,
str::from_utf8(&output.stderr).unwrap()
);
}
}
}
}
Expand Down

0 comments on commit 797c877

Please sign in to comment.