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

Move static config in modules to constructor #2473

Merged
merged 1 commit into from Apr 11, 2020

Conversation

mitchjust
Copy link
Contributor

@mitchjust mitchjust commented Mar 25, 2020

As discussed with @bsideup on slack, static config should be applied in the constructor, not in configure()

@bsideup bsideup added this to the next milestone Apr 11, 2020
@bsideup bsideup merged commit 17b9ffc into testcontainers:master Apr 11, 2020
@rnorth
Copy link
Member

rnorth commented Apr 13, 2020

This was released in https://github.com/testcontainers/testcontainers-java/releases/tag/1.14.0 🎉

Thanks for the contribution!

@aguibert
Copy link
Contributor

This PR seems to have caused a regression after I upgraded to Testcontainers 1.14.0. I see the @bsideup and @rnorth have fixed the regression but then it was reverted later. Will the regression be fixed in a future version? Or is there a workaround that can be used?

Here is an example stack trace I'm seeing after upgrading:

java.lang.IllegalStateException: Container doesn't expose any ports
	at org.testcontainers.containers.ContainerState.lambda$getFirstMappedPort$0(ContainerState.java:119)
	at org.testcontainers.containers.ContainerState$$Lambda$242.000000005E010FF0.get(Unknown Source)
	at java.base/java.util.Optional.orElseThrow(Optional.java:408)
	at org.testcontainers.containers.ContainerState.getFirstMappedPort(ContainerState.java:119)
	at com.ibm.ws.jdbc.fat.oracle.OracleTest.setUp(OracleTest.java:48)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)

Our code is unchanged from the previously working state and looks like this:

static OracleContainer oracle = new OracleContainer("<custom image label>")
                                .withLogConsumer(FATSuite::log);

@bsideup
Copy link
Member

bsideup commented Apr 23, 2020

@aguibert already fixed in today's 1.14.1, sorry! wait...

It seems that we still need to fix it in one of Oracle's constructors :(
As a workaround, you can use new OracleContainer(CompletableFuture.completedFuture("<custom image label>")) (or manually add withExposedPorts)

@aguibert
Copy link
Contributor

@bsideup did you fix the Oracle container in a different commit that I'm not seeing? I only see the MariaDB and MySQL containers updated in the linked commit here: d604f3e

@bsideup
Copy link
Member

bsideup commented Apr 23, 2020

@aguibert yeah, updated my initial answer :(

@aguibert
Copy link
Contributor

I think there is also a 2nd (probably less impactful) regression that we may need to look at. I had a CustomPostgreSQLContainer that extended the PostgreSQLContainer and overrode the configure() method. Previously this worked, but now there seems to be an error because addExposedPort is being called on the same port twice, which appears to cause an error later on.

Perhaps to fix this one GenericContainer.exposedPorts should be converted to an ordered set like LinkedHashSet instead of an ArrayList to prevent duplicate values from going into the exposed ports?

Here is the full stack trace of the error I'm hitting:

org.testcontainers.containers.ContainerLaunchException: Container startup failed
	at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:320)
	at org.testcontainers.containers.GenericContainer.start(GenericContainer.java:300)
	at org.testcontainers.containers.GenericContainer.starting(GenericContainer.java:1011)
	at org.testcontainers.containers.FailureDetectingExternalResource$1.evaluate(FailureDetectingExternalResource.java:29)
	at componenttest.custom.junit.runner.FATRunner$2.evaluate(FATRunner.java:314)
	at componenttest.custom.junit.runner.FATRunner.run(FATRunner.java:167)
Caused by: org.rnorth.ducttape.RetryCountExceededException: Retry limit hit with exception
	at org.rnorth.ducttape.unreliables.Unreliables.retryUntilSuccess(Unreliables.java:83)
	at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:313)
Caused by: org.testcontainers.containers.ContainerLaunchException: Could not create/start container
	at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:488)
	at org.testcontainers.containers.GenericContainer.lambda$doStart$0(GenericContainer.java:315)
	at org.testcontainers.containers.GenericContainer$$Lambda$144.00000000A163DDD0.call(Unknown Source)
	at org.rnorth.ducttape.unreliables.Unreliables.retryUntilSuccess(Unreliables.java:76)
Caused by: java.lang.RuntimeException: org.testcontainers.shaded.com.fasterxml.jackson.databind.JsonMappingException: Failed to getValue() with method com.github.dockerjava.api.model.ExposedPorts#toPrimitive(0 params): null (through reference chain: com.github.dockerjava.core.command.CreateContainerCmdImpl["ExposedPorts"]->com.github.dockerjava.api.model.ExposedPorts["toPrimitive()"])
	at com.github.dockerjava.okhttp.OkHttpInvocationBuilder.post(OkHttpInvocationBuilder.java:134)
	at com.github.dockerjava.core.exec.CreateContainerCmdExec.execute(CreateContainerCmdExec.java:33)
	at com.github.dockerjava.core.exec.CreateContainerCmdExec.execute(CreateContainerCmdExec.java:13)
	at com.github.dockerjava.core.exec.AbstrSyncDockerCmdExec.exec(AbstrSyncDockerCmdExec.java:21)
	at com.github.dockerjava.core.command.AbstrDockerCmd.exec(AbstrDockerCmd.java:35)
	at com.github.dockerjava.core.command.CreateContainerCmdImpl.exec(CreateContainerCmdImpl.java:595)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at org.testcontainers.dockerclient.AuditLoggingDockerClient.lambda$wrappedCommand$14(AuditLoggingDockerClient.java:102)
	at org.testcontainers.dockerclient.AuditLoggingDockerClient$$Lambda$104.00000000A14EF810.invoke(Unknown Source)
	at com.sun.proxy.$Proxy23.exec(Unknown Source)
	at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:397)
Caused by: org.testcontainers.shaded.com.fasterxml.jackson.databind.JsonMappingException: Failed to getValue() with method com.github.dockerjava.api.model.ExposedPorts#toPrimitive(0 params): null (through reference chain: com.github.dockerjava.core.command.CreateContainerCmdImpl["ExposedPorts"]->com.github.dockerjava.api.model.ExposedPorts["toPrimitive()"])
	at org.testcontainers.shaded.com.fasterxml.jackson.databind.JsonMappingException.wrapWithPath(JsonMappingException.java:397)
	at org.testcontainers.shaded.com.fasterxml.jackson.databind.JsonMappingException.wrapWithPath(JsonMappingException.java:356)
	at org.testcontainers.shaded.com.fasterxml.jackson.databind.ser.std.StdSerializer.wrapAndThrow(StdSerializer.java:316)
	at org.testcontainers.shaded.com.fasterxml.jackson.databind.ser.std.JsonValueSerializer.serialize(JsonValueSerializer.java:183)
	at org.testcontainers.shaded.com.fasterxml.jackson.databind.ser.BeanPropertyWriter.serializeAsField(BeanPropertyWriter.java:727)
	at org.testcontainers.shaded.com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeFields(BeanSerializerBase.java:722)
	at org.testcontainers.shaded.com.fasterxml.jackson.databind.ser.BeanSerializer.serialize(BeanSerializer.java:166)
	at org.testcontainers.shaded.com.fasterxml.jackson.databind.ser.DefaultSerializerProvider._serialize(DefaultSerializerProvider.java:480)
	at org.testcontainers.shaded.com.fasterxml.jackson.databind.ser.DefaultSerializerProvider.serializeValue(DefaultSerializerProvider.java:319)
	at org.testcontainers.shaded.com.fasterxml.jackson.databind.ObjectMapper._configAndWriteValue(ObjectMapper.java:4110)
	at org.testcontainers.shaded.com.fasterxml.jackson.databind.ObjectMapper.writeValueAsBytes(ObjectMapper.java:3437)
	at com.github.dockerjava.okhttp.OkHttpInvocationBuilder.post(OkHttpInvocationBuilder.java:126)
Caused by: java.lang.IllegalArgumentException: Failed to getValue() with method com.github.dockerjava.api.model.ExposedPorts#toPrimitive(0 params): null
	at org.testcontainers.shaded.com.fasterxml.jackson.databind.introspect.AnnotatedMethod.getValue(AnnotatedMethod.java:186)
	at org.testcontainers.shaded.com.fasterxml.jackson.databind.ser.std.JsonValueSerializer.serialize(JsonValueSerializer.java:166)
Caused by: java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at org.testcontainers.shaded.com.fasterxml.jackson.databind.introspect.AnnotatedMethod.getValue(AnnotatedMethod.java:183)
Caused by: java.lang.IllegalStateException: Duplicate key 5432/tcp (attempted merging values java.lang.Object@f203e3d8 and java.lang.Object@54902a24)
	at java.base/java.util.stream.Collectors.duplicateKeyException(Collectors.java:133)
	at java.base/java.util.stream.Collectors.lambda$uniqKeysMapAccumulator$1(Collectors.java:180)
	at java.util.stream.Collectors$$Lambda$112.00000000A1332450.accept(Unknown Source)
	at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:497)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:487)
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:239)
	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
	at com.github.dockerjava.api.model.ExposedPorts.toPrimitive(ExposedPorts.java:38)

@bsideup
Copy link
Member

bsideup commented Apr 24, 2020

@aguibert fixed the Oracle one in #2612

@bsideup
Copy link
Member

bsideup commented Apr 24, 2020

And the ports: #2613

quincy pushed a commit to quincy/testcontainers-java that referenced this pull request May 28, 2020
quincy pushed a commit to quincy/testcontainers-java that referenced this pull request May 28, 2020
quincy pushed a commit to quincy/testcontainers-java that referenced this pull request May 28, 2020
mrotteveel added a commit to FirebirdSQL/firebird-testcontainers-java that referenced this pull request Aug 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants