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

Docker for Windows: Use command-based port checker #309

Merged
merged 2 commits into from
Mar 12, 2017

Conversation

bsideup
Copy link
Member

@bsideup bsideup commented Mar 6, 2017

It seems like Docker for Windows has the same port forwarding issue as Docker for Mac

@bsideup bsideup requested a review from rnorth March 6, 2017 10:24
@bsideup bsideup added this to the 1.1.10 milestone Mar 6, 2017
private boolean isUsingSocketProxyOnMac() {
return DockerClientFactory.instance().isUsing(ProxiedUnixSocketClientProviderStrategy.class)
&& System.getProperty("os.name").toLowerCase().contains("mac");
private boolean isApplicable() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this could be named more clearly? Something like doesDockerPrematurelyListen or shouldApplyProxyWorkaround?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to shouldCheckWithCommand, but still not sure :) Does it sound fine to you? )

Copy link
Member

@rnorth rnorth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one comment, but LGTM otherwise

@rnorth rnorth merged commit 68d292d into master Mar 12, 2017
@rnorth rnorth deleted the docker_for_windows_port_check branch March 12, 2017 20:40
rnorth added a commit that referenced this pull request Mar 12, 2017
## [1.2.0] - 2017-03-12
### Fixed
- Fix various escaping issues that may arise when paths contain spaces (#263, #279)
- General documentation fixes/improvements (#300, #303, #304)
- Improve reliability of `ResourceReaper` when there are a large number of containers returned by `docker ps -a` (#295)

### Changed
- Support Docker for Windows via TCP socket connection (#291, #297, #309). _Note that Docker Compose is not yet supported under Docker for Windows (see #306)
- Expose `docker-java`'s `CreateContainerCmd` API for low-level container tweaking (#301)
- Shade `org.newsclub` and Guava dependencies (#299, #292)
- Add `org.testcontainers` label to all containers created by Testcontainers (#294)
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

2 participants