Skip to content

fix: reuse container requires name #887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+21 −15
Open

fix: reuse container requires name #887

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e4e346c
fix: reuse container requires name
tisonkun Dec 1, 2025
8cd16da
fixup cargo fmt
tisonkun Dec 1, 2025
499d5c6
fixup config checks
tisonkun Dec 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 7 additions & 14 deletions testcontainers/src/core/containers/sync_container.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
use std::{fmt, io::BufRead, net::IpAddr, sync::Arc};

use crate::{
core::{
copy::CopyFileFromContainer, env, error::Result, ports::Ports, ContainerPort, ExecCommand,
},
core::{copy::CopyFileFromContainer, error::Result, ports::Ports, ContainerPort, ExecCommand},
ContainerAsync, Image,
};

Expand Down Expand Up @@ -257,17 +255,12 @@ where

impl<I: Image> Drop for Container<I> {
fn drop(&mut self) {
if let Some(active) = self.inner.take() {
active.runtime.block_on(async {
match active.async_impl.docker_client().config.command() {
env::Command::Remove => {
if let Err(e) = active.async_impl.rm().await {
Copy link
Author

@tisonkun tisonkun Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is why containers are always removed as reported in #742 (comment)

I'm using sync Containers and its Drop impl isn't properly forwarded to the async impl.

log::error!("Failed to remove container on drop: {}", e);
}
}
env::Command::Keep => {}
}
});
if let Some(ActiveContainer {
runtime,
async_impl,
}) = self.inner.take()
{
runtime.block_on(async { drop(async_impl) });
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions testcontainers/src/core/env/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ pub enum ConfigurationError {
#[cfg(feature = "properties-config")]
#[error("failed to load testcontainers properties: {0}")]
WrongPropertiesFormat(#[from] serde_java_properties::de::Error),
#[cfg(feature = "reusable-containers")]
#[error("container name or labels must be provided when reusing containers")]
MissingContainerNameAndLabels,
}

/// The default path to the Docker configuration file.
Expand Down
12 changes: 11 additions & 1 deletion testcontainers/src/runners/async_runner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,19 @@ where

#[cfg(feature = "reusable-containers")]
{
use crate::ReuseDirective::{Always, CurrentSession};
use crate::{
core::env::ConfigurationError,
ReuseDirective::{Always, CurrentSession},
TestcontainersError,
};

if matches!(container_req.reuse(), Always | CurrentSession) {
if labels.is_empty() && container_req.container_name().is_none() {
return Err(TestcontainersError::Client(ClientError::Configuration(
ConfigurationError::MissingContainerNameAndLabels,
)));
}

if let Some(container_id) = client
.get_running_container_id(
Copy link
Author

@tisonkun tisonkun Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, this can't handle the case where an existing container is stopped. It would then failed with:

called `Result::unwrap()` on an `Err` value: Client(CreateContainer(DockerResponseServerError { status_code: 409, message: "Conflict. The container name \"/postgres\" is already in use by container \"5d2940fb2bd042cf859598932aae34aa5004b19d38797165806282b4ada66157\". You have to remove (or rename) that container to be able to reuse that name." }))
thread 'test_simple_pubsub' panicked at tests/toolkit/src/container.rs:150:10:
called `Result::unwrap()` on an `Err` value: Client(CreateContainer(DockerResponseServerError { status_code: 409, message: "Conflict. The container name \"/postgres\" is already in use by container \"5d2940fb2bd042cf859598932aae34aa5004b19d38797165806282b4ada66157\". You have to remove (or rename) that container to be able to reuse that name." }))

I resolve this issue in my bollard based solution by removing a stopped container with the same name, but not sure how to make it more smoothly in testcontainers.

Anyway, this can be a follow-up to improved:

async fn maybe_remove_container(&self, docker: &Docker, container_name: &str) {
    let options = Some(RemoveContainerOptions {
        v: true,
        force: true,
        link: false,
    });

    // best effort to remove existing container; ignore errors
    let _ = docker.remove_container(container_name, options).await;
}

Copy link
Author

@tisonkun tisonkun Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stopped container

This is common when a lasting test container gets stopped during the dev machine reboot.

container_req.container_name().as_deref(),
Expand Down