-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Fix problems with copying files and Docker-Compose on Windows #514
Conversation
For docker socket mounting on Windows, the socket path must be prefixed with an additional "/"
Core will now build on Windows. |
@@ -10,6 +10,7 @@ | |||
import com.google.common.util.concurrent.Uninterruptibles; | |||
import org.apache.commons.lang.StringUtils; | |||
import org.apache.commons.lang.SystemUtils; | |||
import org.jetbrains.annotations.NotNull; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather not use JB's annotations in our code base. Adds another dependency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh that was not intended, wonder how it got in, thanks for spotting it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are already using in in multiple places btw.
➜ testcontainers-java git:(docker-for-windows-compat) ✗ grep -r "import org.jetbrains.annotations.NotNull;" *
core/src/main/java/org/testcontainers/dockerclient/WindowsClientProviderStrategy.java:import org.jetbrains.annotations.NotNull;
core/src/main/java/org/testcontainers/dockerclient/AuditLoggingDockerClient.java:import org.jetbrains.annotations.NotNull;
core/src/main/java/org/testcontainers/dockerclient/UnixSocketClientProviderStrategy.java:import org.jetbrains.annotations.NotNull;
core/src/main/java/org/testcontainers/utility/MountableFile.java:import org.jetbrains.annotations.NotNull;
core/src/main/java/org/testcontainers/utility/AuditLogger.java:import org.jetbrains.annotations.NotNull;
core/src/main/java/org/testcontainers/utility/CommandLine.java:import org.jetbrains.annotations.NotNull;
core/src/main/java/org/testcontainers/utility/ComparableVersion.java:import org.jetbrains.annotations.NotNull;
core/src/main/java/org/testcontainers/containers/GenericContainer.java:import org.jetbrains.annotations.NotNull;
core/src/main/java/org/testcontainers/containers/FixedHostPortGenericContainer.java:import org.jetbrains.annotations.NotNull;
core/src/main/java/org/testcontainers/containers/VncRecordingSidekickContainer.java:import org.jetbrains.annotations.NotNull;
core/src/main/java/org/testcontainers/containers/startupcheck/MinimumDurationRunningStartupCheckStrategy.java:import org.jetbrains.annotations.NotNull;
core/src/test/java/org/testcontainers/junit/wait/HttpWaitStrategyTest.java:import org.jetbrains.annotations.NotNull;
core/src/test/java/org/testcontainers/junit/wait/AbstractWaitStrategyTest.java:import org.jetbrains.annotations.NotNull;
core/src/test/java/org/testcontainers/junit/wait/LogMessageWaitStrategyTest.java:import org.jetbrains.annotations.NotNull;
core/src/test/java/org/testcontainers/junit/wait/HostPortWaitStrategyTest.java:import org.jetbrains.annotations.NotNull;
core/src/test/java/org/testcontainers/junit/BaseDockerComposeTest.java:import org.jetbrains.annotations.NotNull;
core/src/test/java/org/testcontainers/utility/MountableFileTest.java:import org.jetbrains.annotations.NotNull;
modules/selenium/src/main/java/org/testcontainers/containers/BrowserWebDriverContainer.java:import org.jetbrains.annotations.NotNull;
modules/selenium/src/test/java/org/testcontainers/junit/BaseWebDriverContainerTest.java:import org.jetbrains.annotations.NotNull;
modules/postgresql/src/main/java/org/testcontainers/containers/PostgreSQLContainer.java:import org.jetbrains.annotations.NotNull;
modules/mysql/src/main/java/org/testcontainers/containers/MySQLContainer.java:import org.jetbrains.annotations.NotNull;
modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java:import org.jetbrains.annotations.NotNull;
modules/nginx/src/main/java/org/testcontainers/containers/NginxContainer.java:import org.jetbrains.annotations.NotNull;
I don't really care about using it in this place, shall I remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I lose track of what the best No[nt]Null annotation is 😞
I don't think there's any harm in leaving this here, and if we want to get rid of the JetBrain annotation altogether we could do it everywhere at once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed it, since I don't think it added anything useful.
@@ -395,6 +396,10 @@ default void validateFileList(List<File> composeFiles) { | |||
* Use Docker Compose container. | |||
*/ | |||
class ContainerisedDockerCompose extends GenericContainer<ContainerisedDockerCompose> implements DockerCompose { | |||
|
|||
private static final String DOCKER_SOCKET_PATH = "//var/run/docker.sock"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should not start with double /
(you add it in getDockerSocketHostPath
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, still works in Linux interestingly ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, on *nix systems, you can use as many /
as you want :D
$ ls -la /////usr
total 0
drwxr-xr-x@ 10 root wheel 320 Sep 27 08:04 .
drwxr-xr-x 31 root wheel 992 Nov 30 13:02 ..
drwxr-xr-x 976 root wheel 31232 Nov 30 13:02 bin
drwxr-xr-x 263 root wheel 8416 Nov 16 11:38 include
drwxr-xr-x 311 root wheel 9952 Nov 30 13:02 lib
drwxr-xr-x 235 root wheel 7520 Nov 30 13:02 libexec
drwxr-xr-x 15 root wheel 480 Sep 26 22:58 local
drwxr-xr-x 246 root wheel 7872 Nov 30 12:51 sbin
drwxr-xr-x 47 root wheel 1504 Sep 27 08:04 share
drwxr-xr-x 5 root wheel 160 Sep 21 06:32 standalone
@@ -10,6 +10,7 @@ | |||
import com.google.common.util.concurrent.Uninterruptibles; | |||
import org.apache.commons.lang.StringUtils; | |||
import org.apache.commons.lang.SystemUtils; | |||
import org.jetbrains.annotations.NotNull; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I lose track of what the best No[nt]Null annotation is 😞
I don't think there's any harm in leaving this here, and if we want to get rid of the JetBrain annotation altogether we could do it everywhere at once.
For docker socket mounting on Windows, the socket path must be prefixed with an additional "/". Also used Windows style paths when interacting with files for Docker for Windows compatibility.
Bumps [influxdb-java](https://github.com/influxdata/influxdb-java) from 2.10 to 2.14. <details> <summary>Changelog</summary> *Sourced from [influxdb-java's changelog](https://github.com/influxdata/influxdb-java/blob/master/CHANGELOG.md).* > ## 2.14 [2018-10-12] > > ### Fixes > > - Fixed chunked query exception handling [Issue #523](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/523) > - Memory leak in StringBuilder cache for Point.lineprotocol() [Issue #526](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/521) > > ## 2.13 [2018-09-12] > > ### Fixes > - MessagePack queries: Exception during parsing InfluxDB version [macOS] [PR #487](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/487) > - The InfluxDBResultMapper is able to handle results with a different time precision [PR #501](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/501) > - UDP target host address is cached [PR #502](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/502) > - Error messages from server not parsed correctly when using msgpack [PR #506](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/506) > - Response body must be closed properly in case of JSON response [PR #514](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/514) > - Time is serialized not consistently in MsgPack and Json, missing millis and nanos in MsgPack[PR #517](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/517) > > ### Features > > - Support for Basic Authentication [PR #492](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/492) > - Added possibility to reuse client as a core part of [influxdb-java-reactive](https://github.com/bonitoo-io/influxdb-java-reactive) client [PR #493](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/493) > - Retry capability for writing of BatchPoints [PR #503](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/503) > - Added `BiConsumer` with capability to discontinue a streaming query [Issue #515](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/515) > - Added `onComplete` action that is invoked after successfully end of streaming query [Issue #515](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/515) > > ## 2.12 [2018-07-31] > > ### Fixes > > - Remove code which checks for unsupported influxdb versions [PR #474](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/474) > - Unpredictable errors when OkHttpClient.Builder instance is reused [PR #478](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/478) > > ### Features > > - Support for MessagePack [PR #471](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/471) > - Cache version per influxdb instance and reduce ping() calls for every query call [PR #472](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/472) > - FAQ list for influxdb-java [PR #475](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/475) > > ### Improvements > > - Test: Unit test to ensure tags should be sorted by key in line protocol (to reduce db server overheads) [PR #476](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/476) > > ## 2.11 [2018-07-02] > > ### Features > > - Allow write precision of TimeUnit other than Nanoseconds [PR #321](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/321) > - Support dynamic measurement name in InfluxDBResultMapper [PR #423](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/423) > - Debug mode which allows HTTP requests being sent to the database to be logged [PR #450](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/450) > - Fix problem of connecting to the influx api with URL which does not points to the url root (e.g. localhots:80/influx-api/) [PR #400] (https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/400) ></table> ... (truncated) </details> <details> <summary>Commits</summary> - [`91d0f09`](influxdata/influxdb-java@91d0f09) [maven-release-plugin] prepare release influxdb-java-2.14 - [`8ffaeb9`](influxdata/influxdb-java@8ffaeb9) Revert "[maven-release-plugin] prepare release influxdb-java-2.14" - [`2781da2`](influxdata/influxdb-java@2781da2) [maven-release-plugin] prepare release influxdb-java-2.14 - [`19c69ed`](influxdata/influxdb-java@19c69ed) [maven-release-plugin] prepare for next development iteration - [`c6d7f25`](influxdata/influxdb-java@c6d7f25) [maven-release-plugin] prepare release influxdb-java-2.14 - [`2f4c594`](influxdata/influxdb-java@2f4c594) Merge pull request [#531](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/531) from heshengbang/master - [`f653e62`](influxdata/influxdb-java@f653e62) Easy to use try-with-resources, add README.md - [`c7be9b0`](influxdata/influxdb-java@c7be9b0) Easy to use try-with-resources - [`4590d18`](influxdata/influxdb-java@4590d18) - added automated SNAPSHOT publishing to Maven Central repository - [`ce65a41`](influxdata/influxdb-java@ce65a41) - added automated SNAPSHOT publishing to Maven Central repository - Additional commits viewable in [compare view](influxdata/influxdb-java@influxdb-java-2.10...influxdb-java-2.14) </details> <br /> [![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=org.influxdb:influxdb-java&package-manager=gradle&previous-version=2.10&new-version=2.14)](https://dependabot.com/compatibility-score.html?dependency-name=org.influxdb:influxdb-java&package-manager=gradle&previous-version=2.10&new-version=2.14) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- **Note:** This repo was added to Dependabot recently, so you'll receive a maximum of 5 PRs for your first few update runs. Once an update run creates fewer than 5 PRs we'll remove that limit. You can always request more updates by clicking `Bump now` in your [Dependabot dashboard](https://app.dependabot.com). <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme Additionally, you can set the following in the `.dependabot/config.yml` file in this repo: - Update frequency (including time of day and day of week) - Automerge options (never/patch/minor, and dev/runtime dependencies) - Pull request limits (per update run and/or open at any time) - Out-of-range updates (receive only lockfile updates, if desired) - Security updates (receive only security updates, if desired) Finally, you can contact us by mentioning @dependabot. </details>
addEnv("DOCKER_HOST", "unix:///docker.sock"); | ||
setStartupCheckStrategy(new IndefiniteWaitOneShotStartupCheckStrategy()); | ||
setWorkingDirectory(containerPwd); | ||
} | ||
|
||
private String getDockerSocketHostPath() { | ||
return SystemUtils.IS_OS_WINDOWS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is making an assumption that we are only using docker via their sockets interface. However in Docker Machine setups this may not be available It should be able to use the DOCKER_HOST
and DOCKER_CERT_PATH
and DOCKER_TLS
values.
I just checked master
it appears to make an assumption that the socket file does exist.
Copying files into the container did not work on Docker for Windows.
Also the docker socket needs to be mounted differently into the docker-compose container on Windows.
UNIX path separator needs to be used when building docker-compose env file path for containerized docker-compose.