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

special DNS record for host.docker.internal + gateway.docker.internal #2348

Open
wants to merge 6 commits into
base: master
from

Conversation

@0xbad0c0d3
Copy link

commented Mar 8, 2019

I was not able to find "magic" for mac and windows systems about 'host.docker.internal' and all solutions in the Net just workarounds. Here I am with my solution... Maybe it's not the best, but it works just fine.
Yes, this functionality can be implemented with dnsmasq, but not everyone can do it by themselves. That's why i find this PR useful

@GordonTheTurtle

This comment has been minimized.

Copy link

commented Mar 8, 2019

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "master" git@github.com:0xbad0c0d3/libnetwork.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

@0xbad0c0d3 0xbad0c0d3 force-pushed the 0xbad0c0d3:master branch from a9e5507 to 7f85a75 Mar 8, 2019

@GordonTheTurtle GordonTheTurtle removed the dco/no label Mar 8, 2019

@0xbad0c0d3 0xbad0c0d3 force-pushed the 0xbad0c0d3:master branch from ac7573b to 23f1ffd Mar 8, 2019

@0xbad0c0d3 0xbad0c0d3 referenced this pull request Mar 8, 2019
2 of 3 tasks complete
special DNS record for host.docker.internal
Signed-off-by: Daniil Skomarovskiy <0xbad0c0d3@gmail.com>

@0xbad0c0d3 0xbad0c0d3 force-pushed the 0xbad0c0d3:master branch from 8af6b0f to 4fe4569 Mar 12, 2019

special DNS record for host.docker.internal
Signed-off-by: Daniil Skomarovskiy <0xbad0c0d3@gmail.com>
@pbeza

This comment has been minimized.

Copy link

commented Mar 22, 2019

When can we expect it to be merged into master branch?

@nyetwurk

This comment has been minimized.

Copy link

commented Mar 29, 2019

The way docker does DNS by default is fundamentally broken. It should always use dnsmasq and the host's DNS configuration by default (and not use 8.8.8.8). Each container should (by default) resolve DNS by querying the host, which should forward the request to its own resolver, and provide resolution for all of .docker.internal, including host.docker.internal (which should be configurable to allow containers named host). All other accessible containers should also resolve in .docker.internal

@jtaylor100

This comment has been minimized.

Copy link

commented Apr 11, 2019

Any idea when this will be merged? 🙂

@cxyzy1

This comment has been minimized.

Copy link

commented Apr 18, 2019

When will this nice feature be merged?

@Akkadius

This comment has been minimized.

Copy link

commented Apr 19, 2019

I have been watching this one, we have Linux members on the team that have this record broken which seems bizarre that Windows and Mac would have this record and not Linux

@shahzadmasud

This comment has been minimized.

Copy link

commented Apr 19, 2019

Dev team is using this one, and when pushing things on production - we are in a great challenging situation. Any anticipated fix date?

@Bardioc1977

This comment has been minimized.

Copy link

commented Apr 19, 2019

Dev team is using this one, and when pushing things on production - we are in a great challenging situation. Any anticipated fix date?

Hello,

if you really need a feature like this before it has been merged, you can do something like this. In our Dockerfile, we override the entrypoint.sh with something like this:

#!/usr/bin/env bash
set -x
set -e

function fix_linux_internal_host() {
DOCKER_INTERNAL_HOST="host.docker.internal"
if ! grep $DOCKER_INTERNAL_HOST /etc/hosts > /dev/null ; then
DOCKER_INTERNAL_IP='/sbin/ip route | awk '/default/ { print $3 }' | awk '!seen[$0]++'
echo -e "$DOCKER_INTERNAL_IP\t$DOCKER_INTERNAL_HOST" | tee -a /etc/hosts > /dev/null
echo "Added $DOCKER_INTERNAL_HOST to hosts /etc/hosts"
fi
}

fix_linux_internal_host

/usr/local/bin/gosu jboss:jboss bash -c "/opt/jboss/wildfly/bin/standalone.sh $@"

Be aware, that you need to do this as root user, so the change to a specific user needs to be done within entrypoint.sh (in our Wildfly version via gosu).
Afterwards, you have a "host.docker.internal" available within your container.

@nyetwurk

This comment has been minimized.

Copy link

commented Apr 19, 2019

Again, that workaround is known, but not generally practical for complex deployments

@Bardioc1977

This comment has been minimized.

Copy link

commented Apr 19, 2019

Again, that workaround is known, but not generally practical for complex deployments

I know that, thats why its called "workaround". But until the final solution is merged and available, it IS a possibility.

@schildbach
Copy link

left a comment

General advice: if you restrict your PRs to the absolute bare minimum of changes necessary, you increase the chance of being merged. So never reformat any (otherwise untouched) code.

sandbox.go Outdated Show resolved Hide resolved
sandbox.go Outdated
@@ -133,7 +133,8 @@ type containerConfig struct {
}

const (
resolverIPSandbox = "127.0.0.11"
resolverIPSandbox = "127.0.0.11"

This comment has been minimized.

Copy link
@schildbach

schildbach Apr 27, 2019

The additional space is an unrelated change.

This comment has been minimized.

Copy link
@0xbad0c0d3

0xbad0c0d3 May 2, 2019

Author

This space was produced by gofmt, are you sure it should be as you said?

This comment has been minimized.

Copy link
@schildbach

schildbach May 2, 2019

It doesn't matter who or what produced it. It's a change that doesn't belong to the title of this PR. As such, it will distract any reviewers of this PR.

This comment has been minimized.

Copy link
@0xbad0c0d3

0xbad0c0d3 May 2, 2019

Author

If i'll remove it -

🐳 vet
🐳 ineffassign

🐳 fmt
sandbox.go
👹 please format Go code with 'gofmt -s -w'
Makefile:155: recipe for target 'fmt' failed
make: *** [fmt] Error 1
Makefile:114: recipe for target 'check' failed
make: *** [check] Error 2
Exited with code 2

This comment has been minimized.

Copy link
@stativ

stativ May 2, 2019

Another advice, though I don't know whether this applies to docker specifically, but you usually update the pull request, rather than adding more commits that fix the problem in the original pull request.
The reason is to keep the repository history clean and concise.
What I mean, it's better to have a commit that implements the desired functionality (or a logical part of it) without any unrelated changes such as code reformatting, and then another commit that does the reformatting. Rather than having a commit that implements the functionality including unrelated changes, than a commit that reverts the unrelated changes, and finally a commit that adds them back like here.

But to end on a more positive note - I really hope this will get merged soon (whatever version will be accepted by the devs). It's an incredibly useful functionality.

This comment has been minimized.

Copy link
@0xbad0c0d3

0xbad0c0d3 May 2, 2019

Author

Totally agree with you about the history and will keep in mind for future
;)

special DNS record for host.docker.internal - unrelated changes
Signed-off-by: Daniil Skomarovskiy <0xbad0c0d3@gmail.com>
@0xbad0c0d3

This comment has been minimized.

Copy link
Author

commented Apr 27, 2019

@schildbach, done

@Akkadius

This comment has been minimized.

Copy link

commented Apr 27, 2019

Let’s merge that bad boy! 😛

@simpslandyy

This comment has been minimized.

Copy link

commented May 1, 2019

Any day now for this merge! :D

special DNS record for host.docker.internal - gofmt
Signed-off-by: Daniil Skomarovskiy <0xbad0c0d3@gmail.com>
@thaJeztah

This comment has been minimized.

Copy link
Member

commented May 8, 2019

/cc @arkodg

@mavenugo

This comment has been minimized.

Copy link
Contributor

commented May 9, 2019

@0xbad0c0d3 Sorry about the delayed response. couple of questions/comments :

  1. I think this change will not have any effect for the default bridge (docker0) network since it doesn't use the built-in SD resolver. If we need this behavior for all the networks, then we will unfortunately have to tamper with the /etc/hosts of the container in the docker0 bridge networks.
  2. What are the expectations of this fix in d4mac and d4win ? This will ofcourse return the gateway ip of the bridge which is local to the VM hosting the docker daemon. But wont directly point to the mac or win host. Will it satisfy the requirement ?
sandbox.go Outdated Show resolved Hide resolved
@arkodg

This comment has been minimized.

Copy link
Contributor

commented May 9, 2019

@0xbad0c0d3 have not tried this out but AFAIK this will break Docker Desktop (Mac and Windows) . Today this works on Desktop through a magic proxy called vpnkit running on OSX host ( ps -e | grep com.docker.vpnkit ) for more info which allows you a way to reach the OSX host via host.docker.internal (the DNS proxy in vpnkit sends this A record to the VM)
By adding a SRV Record in the DNS Server in the dockerd daemon that maps host.docker.internal to the Gateway-IP, containers running in a native Linux environment will be able to reach the Linux Host , but on MAC, containers will be sending traffic to the Linux VM and not the OSX host

If users were able to insert A records into dockerd's DNS server through cmdline/config options and so custom mappings such as host.docker.internal could be mapped to the Host IP / or interface, this functionality could be achieved (Note: this would not work for default docker0 bridge, since for this case, the host's resolv.conf is mounted so the nameserver does not point to dockerd). I will bring up this feature with the team .

@0xbad0c0d3

This comment has been minimized.

Copy link
Author

commented May 9, 2019

@0xbad0c0d3 have not tried this out but AFAIK this will break Docker Desktop (Mac and Windows) . Today this works on Desktop through a magic proxy called vpnkit running on OSX host ( ps -e | grep com.docker.vpnkit ) for more info which allows you a way to reach the OSX host via host.docker.internal (the DNS proxy in vpnkit sends this A record to the VM)
By adding a SRV Record in the DNS Server in the dockerd daemon that maps host.docker.internal to the Gateway-IP, containers running in a native Linux environment will be able to reach the Linux Host , but on MAC, containers will be sending traffic to the Linux VM and not the OSX host

If users were able to insert A records into dockerd's DNS server through cmdline/config options and so custom mappings such as host.docker.internal could be mapped to the Host IP / or interface, this functionality could be achieved (Note: this would not work for default docker0 bridge, since for this case, the host's resolv.conf is mounted so the nameserver does not point to dockerd). I will bring up this feature with the team .

I was thinking about the possibility to manage this feature with cmdline option, but, I am new in go, and do not have time to implement it, more because i could not test it on WIN/MAC systems... But, i think it's easy to detect OS family and apply my "fix" for linux only. Also, i realized that it would not work with docker0 because of the resolv.conf.

@arkodg

This comment has been minimized.

Copy link
Contributor

commented May 10, 2019

@0xbad0c0d3 dockerd for both the scenarios (Mac and Linux) is running on Linux 😄 , for the Mac case it happens to be a VM .
https://docs.docker.com/engine/reference/commandline/run/#add-entries-to-container-hosts-file---add-host documents how you can use the --add-host argument to add entries to your container's /etc/hosts so something like docker run --add-host=host.docker.internal:172.17.0.1 --rm -it alpine ping host.docker.internal should help you reach the Linux host, but please do not add this argument while running containers on Mac or Windows (https://docs.docker.com/docker-for-mac/networking/)

@0xbad0c0d3

This comment has been minimized.

Copy link
Author

commented May 10, 2019

@0xbad0c0d3 dockerd for both the scenarios (Mac and Linux) is running on Linux , for the Mac case it happens to be a VM .
https://docs.docker.com/engine/reference/commandline/run/#add-entries-to-container-hosts-file---add-host documents how you can use the --add-host argument to add entries to your container's /etc/hosts so something like docker run --add-host=host.docker.internal:172.17.0.1 --rm -it alpine ping host.docker.internal should help you reach the Linux host, but please do not add this argument while running containers on Mac or Windows (https://docs.docker.com/docker-for-mac/networking/)

--add-host is requiring extra work, you have to configure interface first (add alias IP), then modify all your containers to be run with --add-host, or install dnsmasq, configure it...rather than just enjoying the feature "out of the box".

0xbad0c0d3 added some commits May 10, 2019

special DNS record for host.docker.internal + gateway.docker.internal
Signed-off-by: Daniil Skomarovskiy <0xbad0c0d3@gmail.com>
special DNS record for host.docker.internal + gateway.docker.internal…
… - gofmt

Signed-off-by: Daniil Skomarovskiy <0xbad0c0d3@gmail.com>

@0xbad0c0d3 0xbad0c0d3 changed the title special DNS record for host.docker.internal special DNS record for host.docker.internal + gateway.docker.internal May 10, 2019

@0xbad0c0d3

This comment has been minimized.

Copy link
Author

commented May 13, 2019

@mavenugo as you suggested i'e wrapped this functionality with if ep.needResolver() and, also, with runtime.GOOS == "linux"

@dsesami

This comment has been minimized.

Copy link

commented Jun 3, 2019

what's the deal with what's left on this in order to merge? :) just wondering.

@nyetwurk

This comment has been minimized.

Copy link

commented Jun 3, 2019

It seems there are a lot of platform dependent details that, if not addressed, may cause some bad side effects. In general, the name resolution in docker seems poorly designed and fragile.

@Guttz Guttz referenced this pull request Jun 4, 2019
1 of 2 tasks complete
@Shankar1598

This comment has been minimized.

Copy link

commented Jul 3, 2019

Any updates on this yet? Waiting for it to merge!

@gaui

This comment has been minimized.

Copy link

commented Jul 5, 2019

Me too!

@selansen

This comment has been minimized.

Copy link
Contributor

commented Jul 7, 2019

PTAL @arkodg

@arkodg

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2019

@0xbad0c0d3 want to reiterate that this code will break Docker Desktop on Mac and Windows, what is needed is

  1. a dockerd configuration flag called --dns-resolve-docker-host or something similar (such as --iptables) which needs to be added to https://github.com/moby/moby and enabled by default
  2. Based on that flag we can programtically add a DNS entry (either in moby/moby or libnetowrk) that maps host.docker.internal to docker0's IP
  3. Docker Desktop internally can then override this flag when creating a VM and spawning dockerd so that all these DNS requests go to its special proxy outside the VM

cc: @thaJeztah @justincormack @djs55

@0xbad0c0d3

This comment has been minimized.

Copy link
Author

commented Jul 10, 2019

@0xbad0c0d3 want to reiterate that this code will break Docker Desktop on Mac and Windows, what is needed is

  1. a dockerd configuration flag called --dns-resolve-docker-host or something similar (such as --iptables) which needs to be added to https://github.com/moby/moby and enabled by default
  2. Based on that flag we can programtically add a DNS entry (either in moby/moby or libnetowrk) that maps host.docker.internal to docker0's IP
  3. Docker Desktop internally can then override this flag when creating a VM and spawning dockerd so that all these DNS requests go to its special proxy outside the VM

cc: @thaJeztah @justincormack @djs55

@arkodg, I doubt about windows and not pretty sure about Mac... because of:
if ep.needResolver() && runtime.GOOS == "linux"
If on windows you have GOOS=linux - i think you need to reinstall windows :D

@arkodg

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2019

For Desktop for Mac Docker spawns a Linux VM and creates dockerd inside that VM so runtime.GOOS == "linux" will evaluate to true, and I believe in Windows too there's an option to create a Linux VM for Linux containers which will also evaluate the above cmd to true

@djs55

This comment has been minimized.

Copy link

commented Jul 11, 2019

(I work on the Docker Desktop Mac + Win DNS proxy in vpnkit). @arkodg is correct -- on both Mac and Windows we run Linux containers inside a Linux VM where runtime.GOOS == "linux" so adding a DNS record conditionally on that would break host.docker.internal because it would point inside the VM rather than on the host as expected.

If this feature is conditional based on a command-line argument (e.g. --dns-resolve-docker-host) then in Desktop we can disable this feature when we start dockerd and fall back to our existing vpnkit proxy. We could then have the same feature on Mac, Windows and Linux.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.