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

Testcontainer started twice since 3.4 #43253

Open
marcantoine-bibeau opened this issue Nov 21, 2024 · 8 comments
Open

Testcontainer started twice since 3.4 #43253

marcantoine-bibeau opened this issue Nov 21, 2024 · 8 comments
Labels
type: bug A general bug
Milestone

Comments

@marcantoine-bibeau
Copy link

marcantoine-bibeau commented Nov 21, 2024

ContainerConnectionDetailsFactory.getContainer() is doing

if (this.container instanceof Startable startable) {
				startable.start();
			}

While TestcontainersLifecycleBeanPostProcessor.initializeStartables(...) already started the MockServer

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 21, 2024
@philwebb
Copy link
Member

Is this causing an issue? Can you provide a sample application showing the actual problem you're facing?

@philwebb philwebb added the status: waiting-for-feedback We need additional information before we can continue label Nov 21, 2024
@marcantoine-bibeau
Copy link
Author

I do not think it creates a big problem but still created a regression on my side because I'm wrapping the MockServer and override the start() method to perform additional stuff. I can fix by verifying if the TestContainer is running but would be ideal to not start twice?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 22, 2024
@snicoll
Copy link
Member

snicoll commented Nov 22, 2024

Can you please share the sample we've requested? See also https://github.com/spring-projects/spring-boot/wiki/How-To-Get-Help

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Nov 22, 2024
@wilkinsona
Copy link
Member

We could perhaps use a non-null response from ContainerState#getContainerId() to determine that a container's already been started (as ContainerState#getMappedPort(int) does) but that won't work for every Startable implementation. Given this, it appears to be impossible for us to tell with 100% certainty if a container's already been started. @marcantoine-bibeau, I'd encourage you to implement start() such that it is idempotent as it is in GenericContainer.

@nosan
Copy link
Contributor

nosan commented Nov 22, 2024

@wilkinsona
Is it possible to add the following condition?

protected final C getContainer() {
	Assert.state(this.container != null,
			"Container cannot be obtained before the connection details bean has been initialized");
	if (!this.container.isRunning() && this.container instanceof Startable startable) {
		startable.start();
	}
	return this.container;
}

org.testcontainers.containers.ContainerState contains the following default methods:

    /**
     * @return is the container currently running?
     */
    default boolean isRunning() {
        if (getContainerId() == null) {
            return false;
        }

        try {
            Boolean running = getCurrentContainerInfo().getState().getRunning();
            return Boolean.TRUE.equals(running);
        } catch (DockerException e) {
            return false;
        }
    }

    /**
     * @return is the container created?
     */
    default boolean isCreated() {
        if (getContainerId() == null) {
            return false;
        }

        try {
            String status = getCurrentContainerInfo().getState().getStatus();
            return "created".equalsIgnoreCase(status) || isRunning();
        } catch (DockerException e) {
            return false;
        }
    }

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 22, 2024
@wilkinsona
Copy link
Member

Yes, that's what I was (poorly) describing above when talking about the container ID. It'll certainly help, but I'm not sure that it'll address the problem 100% of the time as there appears to be an implied assumption in Testcontainers that start() will be idempotent. If you've only got a Startable and start() isn't idempotent, I think it's possible that something will be started twice either by Boot or by a combination of Boot and Testcontainers.

@wilkinsona wilkinsona changed the title TestContainer started twice since 3.4 Testcontainer started twice since 3.4 Nov 22, 2024
@marcantoine-bibeau
Copy link
Author

That I was not aware, I thought that start() would only be called once. I'll definitely do that on my side! Feel free to improve on Spring side as you describe :) Thanks a lot for your help and quick response! I really appreciate! Cheers!

@philwebb
Copy link
Member

We might be able to track beans that we've started ourselves rather than relying on the container ID

@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Nov 22, 2024
@philwebb philwebb added this to the 3.4.x milestone Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

6 participants