testcontainers-java
testcontainers-java copied to clipboard
JDBC - MSSQL - getLivenessCheckPortNumbers() shouldn't that be the mapped port and not the internal?
https://github.com/testcontainers/testcontainers-java/blob/600d235d220d5e507405921b4e241aa73c067a2a/modules/mssqlserver/src/main/java/org/testcontainers/containers/MSSQLServerContainer.java#L64-L67
Mysql has the ports mapped https://github.com/testcontainers/testcontainers-java/blob/600d235d220d5e507405921b4e241aa73c067a2a/modules/mysql/src/main/java/org/testcontainers/containers/MySQLContainer.java#L56-L60
If the overrides are removed you would receive mapped ports of the exposedPorts.
Hey @npetzall, thanks for raising this issue. Your observation seems correct. In which scenarios would this lead to issues for you?
If I remember correctly I couldn't use the MSSQLServerContainer and had to extend it. Since it failed the liveliness check.
This was on Windows so testcontainers tried to connect to 1433 or something like that but it was reachable on a different port.
I do also have a a custom wait strategy
@Override
protected void waitUntilContainerStarted() {
new WaitAllStrategy()
.withStrategy(Wait.forListeningPort())
.withStrategy(Wait.forLogMessage("(?s).*default collation was successfully changed.*",1))
.waitUntilReady(this);
super.waitUntilContainerStarted();
}
So it might be related to changes done that affect what is exposed or how the strategies work.
Thanks, this example makes sense.
It's a bit tricky since the JavaDoc only says ports to check if ready, but Wait.forListeningPort seems to assume that it's a published port.
Thank you all for your contributions. This issue should be closed now. It's in the 1.17.4 release