Skip to content
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.

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

Possible race condition with JUnit 5 in 1.16.1 and 1.16.2 #4679

Closed
SanityResort opened this issue Nov 16, 2021 · 7 comments
Closed

Possible race condition with JUnit 5 in 1.16.1 and 1.16.2 #4679

SanityResort opened this issue Nov 16, 2021 · 7 comments
Labels
resolution/waiting-for-info Waiting for more information of the issue author or another 3rd party. type/bug

Comments

@SanityResort
Copy link

When trying to upgrade from 1.15.3 to 1.16.2 we encountered an issue breaking our tests that might be a race condition.

It seems that connecting to the exposed port is not possible yet when the test cases are executed. Adding timeouts to the @BeforeEach method does reduce these errors which would suggest a race condition here.

I did have a look at PR 4597 and the linked issues. Maybe these could be related but cannot say for sure.

So far I have not been able to create a local reproducer but adding debug logging produces the following output:

[main] INFO 🐳 [rabbitmq:3.8] - Creating container for image: rabbitmq:3.8
[main] INFO 🐳 [rabbitmq:3.8] - Starting container with ID: 633fc3b2f14fb048fd089b805c2596cba8d36358e5802af6e78075f50cfecc96
[main] INFO 🐳 [rabbitmq:3.8] - Container rabbitmq:3.8 is starting: 633fc3b2f14fb048fd089b805c2596cba8d36358e5802af6e78075f50cfecc96
[main] INFO 🐳 [rabbitmq:3.8] - Container rabbitmq:3.8 started in PT4.845S
[AMQP Connection 127.0.0.1:49668] WARN com.rabbitmq.client.impl.ForgivingExceptionHandler - An unexpected connection driver error occurred (Exception message: Connection reset)
[main] INFO 🐳 [rabbitmq:3.8] - Creating container for image: rabbitmq:3.8
[main] INFO 🐳 [rabbitmq:3.8] - Starting container with ID: 57d3b35f8cba584142d5efc0c69c1864802b3c977cefddf815cd415b972af66f
[main] INFO 🐳 [rabbitmq:3.8] - Container rabbitmq:3.8 is starting: 57d3b35f8cba584142d5efc0c69c1864802b3c977cefddf815cd415b972af66f
[main] INFO 🐳 [rabbitmq:3.8] - Container rabbitmq:3.8 started in PT5.483S
[main] INFO 🐳 [rabbitmq:3.8] - Creating container for image: rabbitmq:3.8
[main] INFO 🐳 [rabbitmq:3.8] - Starting container with ID: 364373afe9fed9056c0314b75709b76a5013c1f7d375089c2329d51686cba14e
[main] INFO 🐳 [rabbitmq:3.8] - Container rabbitmq:3.8 is starting: 364373afe9fed9056c0314b75709b76a5013c1f7d375089c2329d51686cba14e
[main] INFO 🐳 [rabbitmq:3.8] - Container rabbitmq:3.8 started in PT5.912S

This shows a run of three tests starting a container with a rabbitmq:3.8 image, in our tests we try to connect to that container during the @BeforeEach setup method (Moving this call to the actual test method did not solve the issue). The first test results in an error where as the other two complete successfully. We have also seen runs with all three tests failing.

From the log output it would seem the container is up and running before the test gets executed, the actual behaviour suggests differently. With 1.16.0 or 1.15.3 we did not observe any similar issues.

Connection reset could also imply that the container was shutting down again already but that would be unexpected during the test setup and adding timeout at the start of the setup method should not have an effect on it.

Changing to a shared container instance for all tests did also not solve the issue.

Please let me know if you require any further information.

@kiview
Copy link
Member

kiview commented Nov 16, 2021

Hey @SanityResort, thanks for reporting. Did you only observe this for RabbitMQ, or also other images? And can you reproduce it when manually starting the container, instead of using the JUnit5 extension?

1.16.2 will have code for performing the port checks faster which, in your case, might lead to another race condition (just an assumption now).

@SanityResort
Copy link
Author

Thanks for the quick reply.

Currently we are only using testcontainers for tests with RabbitMQ so I can't say anything about other images. All I can say that simply upgrading the testcontainers dependency introduce the error even at the first run.

I will have a look if I can change the tests to run without the JUnit5 extension and will get back at you ASAP.

@kiview kiview added the resolution/waiting-for-info Waiting for more information of the issue author or another 3rd party. label Nov 16, 2021
@kiview
Copy link
Member

kiview commented Nov 16, 2021

You are using RabbitMQContainer, correct? This container waits for a certain log message. If this log message is not a sufficiently stable criterion for assuming the application has started, we need to reconsider the wait strategy:
https://github.com/testcontainers/testcontainers-java/blob/master/modules/rabbitmq/src/main/java/org/testcontainers/containers/RabbitMQContainer.java#L68

@SanityResort
Copy link
Author

Acutally no. We are creating a GenericContainer like so:

public static final GenericContainer<?> MESSAGING_CONTAINER = new GenericContainer<>(DockerImageName.parse("rabbitmq:3.8")) .withExposedPorts(5672);

From what you write this could very well be the issue as the the GenericContainer probably does not use the same waiting strategy. Sorry for not mentioning this sooner, I was not aware that there is a dedicated abstraction class for RabbitMQ.

I will try to change our test accordingly.

@kiview
Copy link
Member

kiview commented Nov 16, 2021

In this case, you are using the default host port wait strategy. This might very well not be a good indicator for RabbitMQ being actually started. Since we improved the performance of this check, it might have indeed led to your race condition. However, this just highlighted an existing race condition based on wrong assumptions.

Let me know if using the dedicated module or switching the wait strategies helps with your problem 🙂

@SanityResort
Copy link
Author

Indeed using RabbitMQContainer seems to solve the issue. It is interesting that this has never had an impact before.

Thanks a lot for your help and sorry for this inconvenience.

@kiview
Copy link
Member

kiview commented Nov 16, 2021

Awesome, great that it works for you now 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolution/waiting-for-info Waiting for more information of the issue author or another 3rd party. type/bug
Projects
None yet
Development

No branches or pull requests

2 participants