fix: reuse container requires name #887
Conversation
Signed-off-by: tison <wander4096@gmail.com>
✅ Deploy Preview for testcontainers-rust ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| 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 { |
There was a problem hiding this comment.
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.
| })?; | ||
|
|
||
| if let Some(container_id) = client | ||
| .get_running_container_id( |
There was a problem hiding this comment.
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;
}There was a problem hiding this comment.
stopped container
This is common when a lasting test container gets stopped during the dev machine reboot.
Signed-off-by: tison <wander4096@gmail.com>
|
Let me fix the format issue. ... also it seems that tests rely on selecting by labels without container name, let me try to fine tune the impl. |
Signed-off-by: tison <wander4096@gmail.com>
|
@DDtKey failure resolved. Please help in triggering the CI. |
|
Besides, we may later drop the "reusable-containers" flag and just deliver it by default. There is no extra dependency requirement. |
|
@tisonkun I need to dig in to confirm when I get back to my desk, but I think there's some overlap here with the fix branch I'd previously asked you to test. Come to think of it actually, I need to actually get that branch up to date with tl;dr I'll block some time out today to give this PR a closer look, and some time this week to get that branch fixed up and get a proper PR opened. |
|
I think this PR can resolve my remaining issue now. If @DDtKey or other maintainer can get it reveiwed, you may put your changes upon this one. That said, I'm glad to learn any concrete suggestion to improve the patch in place. And there is still a pending issue (#887 (comment)) we need to decide before testcontainers-rs can go back to my testkit >< |
2 participants
... also sync
Containershould reuseContainerAsync's drop impl.This resolves #742.
cc @DDtKey @the-wondersmith