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

Support host.docker.internal in dockerd on Linux #40007

Open
wants to merge 2 commits into
base: master
from

Conversation

@arkodg
Copy link
Contributor

arkodg commented Sep 29, 2019

Signed-off-by: Arko Dasgupta arko.dasgupta@docker.com

Please provide the following information:
-->

- What I did
This PR allows containers to connect to Linux hosts
by appending a special string "host-gateway" to --add-host
e.g. "--add-host=host.docker.internal:host-gateway" which adds
host.docker.internal DNS entry in /etc/hosts and maps it to host-gateway-ip
This PR also add a daemon flag call host-gateway-ip which defaults to
the default bridge IP
Docker Desktop will need to set this field to the Host Proxy IP
so DNS requests for host.docker.internal can be routed to VPNkit

- How to verify it

dockerd &
docker run -it --add-host=host.docker.internal:host-gateway alpine cat /etc/hosts
127.0.0.1	localhost
::1	localhost ip6-localhost ip6-loopback
fe00::0	ip6-localnet
ff00::0	ip6-mcastprefix
ff02::1	ip6-allnodes
ff02::2	ip6-allrouters
172.18.0.1	host.docker.internal
172.18.0.2	9f74a81028b7
dockerd --host-gateway-ip=1.2.3.4 &
docker run -it --add-host=host.docker.internal:host-gateway alpine cat /etc/hosts
127.0.0.1	localhost
::1	localhost ip6-localhost ip6-loopback
fe00::0	ip6-localnet
ff00::0	ip6-mcastprefix
ff02::1	ip6-allnodes
ff02::2	ip6-allrouters
1.2.3.4	host.docker.internal
172.18.0.2	a5f53cc81d24

Addresses : docker/for-linux#264

@tao12345666333

This comment has been minimized.

Copy link
Contributor

tao12345666333 commented Sep 29, 2019

Looks Good. But I don't really understand the intent of this feature.

Just to save the IP information of docker0? Is it the same effect when using environment variable records?

If I understand wrong, please tell me.

@arkodg

This comment has been minimized.

Copy link
Contributor Author

arkodg commented Sep 29, 2019

@tao12345666333 updated the PR with a link to the issue

@arkodg

This comment has been minimized.

Copy link
Contributor Author

arkodg commented Sep 29, 2019

@Dwood15

This comment has been minimized.

Copy link

Dwood15 commented Sep 29, 2019

Yes, please 👀

@djs55

This comment has been minimized.

Copy link

djs55 commented Sep 30, 2019

@arkodg looks good to me. So on Docker Desktop we would add --dns-resolve-docker-host=false to the command line and this will disable the feature so that queries for host.docker.internal (and gateway) will be resolved by our host software as before.

}
if len(gateway) > 0 {
sboxOptions = append(sboxOptions, libnetwork.OptionExtraHost(network.DockerHostInternalDNSName, gateway))
sboxOptions = append(sboxOptions, libnetwork.OptionExtraHost(network.DockerGatewayInternalDNSName, gateway))

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Sep 30, 2019

Member

If the container is running with the default (bridge) network, does libnetwork.OptionExtraHost also add entries to /etc/hosts ? (i.o.w. does this also use when not connected to a custom network (because the default bridge network doesn't currently use the embedded DNS)

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Sep 30, 2019

Member

Actually; I see you mention the reverse;

This PR allows containers to connect to Linux hosts by setting a Daemon flag called dns-resolve-docker-host which adds a host.docker.internal entry in /etc/hosts and maps it to docker0's IP

/etc/hosts is only used on the default (bridge) network, but isn't used for DNS resolution on other networks (well, I guess it's used, but it would be different from resolution of other entries we set in the embedded DNS)?

edit: hm.. so we actually do use it for the custom hosts (--add-host), so this would be consistent. Would it be better to use the embedded DNS?

docker network create foo 

docker run --rm --network=foo --add-host foo.example.com:123.123.123.1 busybox cat /etc/hosts

127.0.0.1	localhost
::1	localhost ip6-localhost ip6-loopback
fe00::0	ip6-localnet
ff00::0	ip6-mcastprefix
ff02::1	ip6-allnodes
ff02::2	ip6-allrouters
123.123.123.1	foo.example.com
172.19.0.2	de9b9822b00f

This comment has been minimized.

Copy link
@cpuguy83

cpuguy83 Oct 7, 2019

Contributor

What happens if this is, for example, a macvlan interface? Wouldn't the gateway be different (ie. not necessarily the "host" ip) in such a case?

@@ -64,6 +64,7 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error {
flags.Var(opts.NewListOptsRef(&conf.DNS, opts.ValidateIPAddress), "dns", "DNS server to use")
flags.Var(opts.NewNamedListOptsRef("dns-opts", &conf.DNSOptions, nil), "dns-opt", "DNS options to use")
flags.Var(opts.NewListOptsRef(&conf.DNSSearch, opts.ValidateDNSSearch), "dns-search", "DNS search domains to use")
flags.BoolVar(&conf.DNSResolveDockerHost, "dns-resolve-docker-host", true, "dockerd will resolve the host.docker.internal DNS name if set to true")

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Sep 30, 2019

Member

Not a big fan of making this a boolean. Perhaps we could make this flag (that can be specified multiple times) to add custom entries to the resolver (either the embedded DNS or (see my other comment) entries in /etc/hosts, depending on the situation).

For this particular case, we may need a "special" value for the gateway address (which I think is dynamically set), so that users can (haven't thought of the name of the flag yet);

--custom-dns-entry host.docker.internal=gateway \
--custom-dns-entry gateway.docker.internal=gateway

or, e.g. (same naming for the flag as for docker run);

--add-host host.docker.internal=gateway \
--add-host gateway.docker.internal=gateway

With that (perhaps) Docker Desktop could also use that option?

This comment has been minimized.

Copy link
@scottgeary

scottgeary Sep 30, 2019

This would also make it compatible using Compose's extra_hosts: field.

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Sep 30, 2019

Member

that option is per container/service, not at the daemon level

This comment has been minimized.

Copy link
@arkodg

arkodg Sep 30, 2019

Author Contributor

@thaJeztah for this case, the community prefers some automagic

This comment has been minimized.

Copy link
@arkodg

arkodg Sep 30, 2019

Author Contributor

also if we use embedded DNS it won't support this DNS name for containers attached to the default bridge/docker0 network

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Oct 7, 2019

Member

@thaJeztah for this case, the community prefers some automagic

While I understand that some magic could be nice to have, I think the reason we originally didn't add it on Linux was specifically because hard-coding could cause issues in some situations;

For reference, the host.docker.internal was added on Docker Desktop to get parity with Linux (see #22753: "No docker0 on docker for mac?").

Related discussions use-cases / issues were

  • #1143 Document how to connect to Docker host from container
  • #23177 Add dockerhost as an entry in /etc/hosts in all containers
  • (slightly related) #15086 "Document how to get real remote client ip for service running in container"

I think that;

  • For the first issue; on Docker Desktop, docker has (almost) full control over how networking is set-up, so implementing host.docker.internal could be done because of that, whereas on Linux, networking configuration can widely differ, so hard-coding values may cause issues in some setups
  • For the other issues, expectations "how to connect to the host" widely varied;
    • some people were looking at connecting to a (non-containerised) service running on the same host
    • some people were looking at "how to connect to a service running on the same host" (either containerised or non-containerised) on a "interface", and how to get that IP-address?
    • "public interface" / IP-address is ambiguous in setups where the docker host is behind a proxy (and the docker host itself does not know the public address

To get "magic", docker could still decide to set these options in the default configuration (systemd unit?), but hard-coding could potentially (see above) limit the usefulness if users cannot customise these settings, and if the hard-coded values don't work.

This comment has been minimized.

Copy link
@arkodg

arkodg Oct 7, 2019

Author Contributor

@thaJeztah the hard coding can always be removed by setting dns-resolve-docker-host=false if someone wants to be change this

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Oct 7, 2019

Member

But for the situations mentioned above, there would be no way to change host.docker.internal=<hard-coded-ip> to host.docker.internal=<correct IP> to be included in all containers.

This comment has been minimized.

Copy link
@arkodg

arkodg Oct 7, 2019

Author Contributor

dns-resolve-docker-host=false (In Daemon config) && --add-host host.docker.internal=<some-IP> would address your concern

There is no ideal answer (I have raised the same questions), but apparently many developers want parity across OSs with the same Compose file and this PR attempts to do that

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Oct 7, 2019

Member

it would have to be set on every container, every service, so not the same as this feature implements

@thaJeztah

This comment has been minimized.

Copy link
Member

thaJeztah commented Sep 30, 2019

Seeing some failures;

s390x and powerpc integration tests (looks legit); https://ci.docker.com/public/job/moby/job/PR-40007/1/execution/node/217/log/

00:17:27.053  === RUN   TestLinksEtcHostsContentMatch
00:17:27.943  --- FAIL: TestLinksEtcHostsContentMatch (0.65s)
00:17:27.943      links_linux_test.go:33: assertion failed: 
00:17:27.943          --- ←
00:17:27.943          +++ →
00:17:27.943          @@ -5,4 +5,5 @@
00:17:27.943           ff02::1	ip6-allnodes
00:17:27.943           ff02::2	ip6-allrouters
00:17:27.943          -172.17.0.2	6d642f0c99aa
00:17:27.943          +172.18.0.1	host.docker.internal
00:17:27.943          +172.18.0.1	gateway.docker.internal
00:17:27.943           


....


00:19:01.889  === FAIL: s390x.integration.container TestDaemonRestartKillContainers/live-restore=false/container_without_restart_policy/kill-daemon (1.97s)
00:19:01.889  === PAUSE TestDaemonRestartKillContainers/live-restore=false/container_without_restart_policy/kill-daemon
00:19:01.889  === CONT  TestDaemonRestartKillContainers/live-restore=false/container_without_restart_policy/kill-daemon
00:19:01.889      --- FAIL: TestDaemonRestartKillContainers/live-restore=false/container_without_restart_policy/kill-daemon (1.97s)
00:19:01.889          restart_test.go:67: Creating a new daemon at: "/go/src/github.com/docker/docker/bundles/test-integration/TestDaemonRestartKillContainers/live-restore=false/container_without_restart_policy/kill-daemon"
00:19:01.889          restart_test.go:89: failed to start daemon with arguments [--iptables=false] : [d3521c6b8cec2] Daemon exited during startup: exit status 2
00:19:01.889          panic.go:563: Daemon d3521c6b8cec2 is not started
00:19:01.889  
00:19:01.889  === FAIL: s390x.integration.container TestDaemonRestartKillContainers/live-restore=true/container_with_restart=always/stop-daemon (2.14s)
00:19:01.889  === PAUSE TestDaemonRestartKillContainers/live-restore=true/container_with_restart=always/stop-daemon
00:19:01.889  === CONT  TestDaemonRestartKillContainers/live-restore=true/container_with_restart=always/stop-daemon
00:19:01.889      --- FAIL: TestDaemonRestartKillContainers/live-restore=true/container_with_restart=always/stop-daemon (2.14s)
00:19:01.889          restart_test.go:67: Creating a new daemon at: "/go/src/github.com/docker/docker/bundles/test-integration/TestDaemonRestartKillContainers/live-restore=true/container_with_restart=always/stop-daemon"
00:19:01.889          restart_test.go:89: failed to start daemon with arguments [--iptables=false --live-restore] : [d88a76ccc6bfb] Daemon exited during startup: exit status 2
00:19:01.889          panic.go:563: Daemon d88a76ccc6bfb is not started
00:19:01.889  
00:19:01.889  === FAIL: s390x.integration.container TestDaemonRestartKillContainers/live-restore=true/container_with_restart=always/kill-daemon (1.93s)
00:19:01.889  === PAUSE TestDaemonRestartKillContainers/live-restore=true/container_with_restart=always/kill-daemon
00:19:01.889  === CONT  TestDaemonRestartKillContainers/live-restore=true/container_with_restart=always/kill-daemon
00:19:01.889      --- FAIL: TestDaemonRestartKillContainers/live-restore=true/container_with_restart=always/kill-daemon (1.93s)
00:19:01.889          restart_test.go:67: Creating a new daemon at: "/go/src/github.com/docker/docker/bundles/test-integration/TestDaemonRestartKillContainers/live-restore=true/container_with_restart=always/kill-daemon"
00:19:01.889          restart_test.go:89: failed to start daemon with arguments [--iptables=false --live-restore] : [d2ec219ca71f4] Daemon exited during startup: exit status 2
00:19:01.889          panic.go:563: Daemon d2ec219ca71f4 is not started
00:19:01.889  
00:19:01.889  === FAIL: s390x.integration.container TestDaemonRestartKillContainers/live-restore=true/container_without_restart_policy/stop-daemon (1.95s)
00:19:01.889  === PAUSE TestDaemonRestartKillContainers/live-restore=true/container_without_restart_policy/stop-daemon
00:19:01.889  === CONT  TestDaemonRestartKillContainers/live-restore=true/container_without_restart_policy/stop-daemon
00:19:01.889      --- FAIL: TestDaemonRestartKillContainers/live-restore=true/container_without_restart_policy/stop-daemon (1.95s)
00:19:01.889          restart_test.go:67: Creating a new daemon at: "/go/src/github.com/docker/docker/bundles/test-integration/TestDaemonRestartKillContainers/live-restore=true/container_without_restart_policy/stop-daemon"
00:19:01.889          restart_test.go:89: failed to start daemon with arguments [--iptables=false --live-restore] : [de1b3c56d6a31] Daemon exited during startup: exit status 2
00:19:01.889          panic.go:563: Daemon de1b3c56d6a31 is not started
00:19:01.889  
00:19:01.889  === FAIL: s390x.integration.container TestDaemonRestartKillContainers/live-restore=false/container_with_restart=always/kill-daemon (1.92s)
00:19:01.889  === PAUSE TestDaemonRestartKillContainers/live-restore=false/container_with_restart=always/kill-daemon
00:19:01.889  === CONT  TestDaemonRestartKillContainers/live-restore=false/container_with_restart=always/kill-daemon
00:19:01.889      --- FAIL: TestDaemonRestartKillContainers/live-restore=false/container_with_restart=always/kill-daemon (1.92s)
00:19:01.889          restart_test.go:67: Creating a new daemon at: "/go/src/github.com/docker/docker/bundles/test-integration/TestDaemonRestartKillContainers/live-restore=false/container_with_restart=always/kill-daemon"
00:19:01.889          restart_test.go:89: failed to start daemon with arguments [--iptables=false] : [df42d55b97be8] Daemon exited during startup: exit status 2
00:19:01.889          panic.go:563: Daemon df42d55b97be8 is not started
00:19:01.889  
00:19:01.889  === FAIL: s390x.integration.container TestDaemonRestartKillContainers/live-restore=true/container_without_restart_policy/kill-daemon (1.94s)
00:19:01.889  === PAUSE TestDaemonRestartKillContainers/live-restore=true/container_without_restart_policy/kill-daemon
00:19:01.889  === CONT  TestDaemonRestartKillContainers/live-restore=true/container_without_restart_policy/kill-daemon
00:19:01.889      --- FAIL: TestDaemonRestartKillContainers/live-restore=true/container_without_restart_policy/kill-daemon (1.94s)
00:19:01.889          restart_test.go:67: Creating a new daemon at: "/go/src/github.com/docker/docker/bundles/test-integration/TestDaemonRestartKillContainers/live-restore=true/container_without_restart_policy/kill-daemon"
00:19:01.889          restart_test.go:89: failed to start daemon with arguments [--iptables=false --live-restore] : [d392e42874177] Daemon exited during startup: exit status 2
00:19:01.889          panic.go:563: Daemon d392e42874177 is not started
00:19:01.889  
00:19:01.889  === FAIL: s390x.integration.container TestDaemonRestartKillContainers (0.00s)

amd64 integration-cli tests: https://ci.docker.com/public/job/moby/job/PR-40007/1/execution/node/196/log/

00:58:54.789  === FAIL: amd64.integration-cli TestDockerDaemonSuite/TestCleanupMountsAfterDaemonCrash (2.46s)
00:58:54.789      --- FAIL: TestDockerDaemonSuite/TestCleanupMountsAfterDaemonCrash (2.46s)
00:58:54.789          daemon.go:26: Creating a new daemon at: "/go/src/github.com/docker/docker/bundles/test-integration/TestDockerDaemonSuite/TestCleanupMountsAfterDaemonCrash"
00:58:54.789          docker_cli_daemon_test.go:2016: failed to start daemon with arguments [--live-restore] : [dc5633eb3befd] Daemon exited during startup: exit status 2
00:58:54.789          check_test.go:309: Daemon dc5633eb3befd is not started
00:58:54.789  
00:58:54.789  === FAIL: amd64.integration-cli TestDockerDaemonSuite/TestDaemonRestartRestoreBridgeNetwork (2.14s)
00:58:54.789      --- FAIL: TestDockerDaemonSuite/TestDaemonRestartRestoreBridgeNetwork (2.14s)
00:58:54.789          daemon.go:26: Creating a new daemon at: "/go/src/github.com/docker/docker/bundles/test-integration/TestDockerDaemonSuite/TestDaemonRestartRestoreBridgeNetwork"
00:58:54.789          docker_cli_network_unix_test.go:1644: failed to start daemon with arguments [--live-restore] : [d1dab7dde500b] Daemon exited during startup: exit status 2
00:58:54.789          panic.go:563: Daemon d1dab7dde500b is not started
00:58:54.789          check_test.go:309: Daemon d1dab7dde500b is not started
00:58:54.789  
00:58:54.789  === FAIL: amd64.integration-cli TestDockerDaemonSuite/TestDaemonRestartWithUnpausedRunningContainer (2.12s)
00:58:54.789      --- FAIL: TestDockerDaemonSuite/TestDaemonRestartWithUnpausedRunningContainer (2.12s)
00:58:54.789          daemon.go:26: Creating a new daemon at: "/go/src/github.com/docker/docker/bundles/test-integration/TestDockerDaemonSuite/TestDaemonRestartWithUnpausedRunningContainer"
00:58:54.789          docker_cli_daemon_test.go:2081: failed to start daemon with arguments [--live-restore] : [d2a14381f4c28] Daemon exited during startup: exit status 2
00:58:54.789          panic.go:563: Daemon d2a14381f4c28 is not started
00:58:54.789          check_test.go:309: Daemon d2a14381f4c28 is not started
00:58:54.789  
00:58:54.789  === FAIL: amd64.integration-cli TestDockerDaemonSuite/TestExecWithUserAfterLiveRestore (2.28s)
00:58:54.789      --- FAIL: TestDockerDaemonSuite/TestExecWithUserAfterLiveRestore (2.28s)
00:58:54.789          daemon.go:26: Creating a new daemon at: "/go/src/github.com/docker/docker/bundles/test-integration/TestDockerDaemonSuite/TestExecWithUserAfterLiveRestore"
00:58:54.789          docker_cli_daemon_test.go:2720: failed to start daemon with arguments [--live-restore] : [d245f49a55601] Daemon exited during startup: exit status 2
00:58:54.789          check_test.go:309: Daemon d245f49a55601 is not started
00:58:54.789  
00:58:54.789  === FAIL: amd64.integration-cli TestDockerDaemonSuite/TestRemoveContainerAfterLiveRestore (2.69s)
00:58:54.789      --- FAIL: TestDockerDaemonSuite/TestRemoveContainerAfterLiveRestore (2.69s)
00:58:54.789          daemon.go:26: Creating a new daemon at: "/go/src/github.com/docker/docker/bundles/test-integration/TestDockerDaemonSuite/TestRemoveContainerAfterLiveRestore"
00:58:54.789          docker_cli_daemon_test.go:2739: failed to start daemon with arguments [--live-restore --storage-driver overlay] : [d8664dd6e9cf8] Daemon exited during startup: exit status 2
00:58:54.789          check_test.go:309: Daemon d8664dd6e9cf8 is not started
00:58:54.789  
00:58:54.789  === FAIL: amd64.integration-cli TestDockerDaemonSuite/TestRestartPolicyWithLiveRestore (1.94s)
00:58:54.789      --- FAIL: TestDockerDaemonSuite/TestRestartPolicyWithLiveRestore (1.94s)
00:58:54.789          daemon.go:26: Creating a new daemon at: "/go/src/github.com/docker/docker/bundles/test-integration/TestDockerDaemonSuite/TestRestartPolicyWithLiveRestore"
00:58:54.789          docker_cli_daemon_test.go:2782: failed to start daemon with arguments [--live-restore] : [d21d61da22ae8] Daemon exited during startup: exit status 2
00:58:54.789          check_test.go:309: Daemon d21d61da22ae8 is not started

Looks like there's a NPE:
docker.log

time="2019-09-29T08:12:20.166341466Z" level=debug msg="container mounted via layerStore: &{/go/src/github.com/docker/docker/bundles/test-integration/TestDockerDaemonSuite/TestCleanupMountsAfterDaemonCrash/dc5633eb3befd/root/overlay2/b27cbc0c172353a0deb8cde0698da92bdaefbb7a374ac3c64a2e69cf1d3c2173/merged 0x556347a54840 0x556347a54840}"
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x58 pc=0x55634536af90]
@arkodg arkodg force-pushed the arkodg:add-host-docker-internal branch from 4d494b6 to ac434cd Sep 30, 2019
@arkodg

This comment has been minimized.

Copy link
Contributor Author

arkodg commented Oct 1, 2019

CI failed due to #39966 (comment)

@arkodg arkodg force-pushed the arkodg:add-host-docker-internal branch from ac434cd to 2c34caf Oct 4, 2019
@cpuguy83

This comment has been minimized.

Copy link
Contributor

cpuguy83 commented Oct 7, 2019

There seems to be a fair number of edge cases this has a lot more edge cases than are accounted for here.
For example, what happens on a macvlan network where the "gateway" is not necessarily (not even likely) the host IP.

In Docker4Mac there is a very specific and controlled network stack to get at the "host" IP.

@arkodg

This comment has been minimized.

Copy link
Contributor Author

arkodg commented Oct 7, 2019

@cpuguy83 the above PR attempts to route host-docker-internal to the Host, and so the default network's IP is a convenient way to do it, AFAIK default network is always based off the bridge driver and not macvlan . In case the default bridge is disabled, these entries will not be added

@arkodg arkodg force-pushed the arkodg:add-host-docker-internal branch from 61b4e98 to d6e00a9 Oct 7, 2019
@cpuguy83

This comment has been minimized.

Copy link
Contributor

cpuguy83 commented Oct 15, 2019

Ok, SGTM.

@tiborvass

This comment has been minimized.

Copy link
Collaborator

tiborvass commented Oct 24, 2019

This should be on docker run and not a daemon flag, because non-portable properties are in HostConfig and we should not add more non-portable properties to the daemon. We already have --add-host and the only problem is the IP is not portable. I like @thaJeztah's approach in #40007 (comment) for add-host.

For swarmkit consideraitons: this would be opaque to the managers and they would pass it down to workers and each worker would resolve it to its own IP.

@tiborvass

This comment has been minimized.

Copy link
Collaborator

tiborvass commented Oct 28, 2019

Here's a conclusion of my IRL talk with @arkodg:

  • we should not break existing Desktop users
  • instead of adding yet another knob to the daemon that makes all applications running on it less portable, users should be encouraged to opt-in to non-portability per application.

Suggested implementation:

  • --add-host accepts as an IP, a magic gateway string (open to suggestions for naming) that, by default is replaced by the bridge IP (as configued by dockerd --bip)
  • a new --hosts-gateway-default-ip flag (again: i'm sure we can find a better name) is added to dockerd which allows docker desktop to configure the IP that the magic gateway string should be replaced to.
  • Docs are updated to deprecate (but not remove) the automatically added hosts in Desktop, and instead encourage users to add manually the magic host if needed.

As someone pointed it out, this would work automatically with compose using the gateway string in extra_hosts.

@thaJeztah

This comment has been minimized.

Copy link
Member

thaJeztah commented Oct 28, 2019

I think in addition to the above, we could add a configuration on the CLI that allows setting these as a default, similar to what we did with proxy configuration; docker/cli#93

@arkodg

This comment has been minimized.

Copy link
Contributor Author

arkodg commented Oct 28, 2019

@tibor
so what you are suggesting is a per container opt in strategy rather than a daemon level option since this usage is not the common-case
Now desktop will need to spawn dockerd with --hosts-gateway-default-ip host-proxy-ip.

Because in dockerd the code would look like
if gateway in add-hosts
get IP from hosts-gateway-default-ip. (Desktop case)
if hosts-gateway-default-ip == nil
get IP from default bridge (dockerd running on host OS case)

Will also not break older versions of Desktop

WDYT @djs55

@tiborvass

This comment has been minimized.

Copy link
Collaborator

tiborvass commented Oct 28, 2019

@arkodg Almost: there is no conditional logic for getting the gateway IP: it's always from the hosts-gateway-default-ip config. That config defaults to the bridge IP upon daemon start.

@djs55

This comment has been minimized.

Copy link

djs55 commented Oct 31, 2019

This sounds reasonable to me. Adding an extra argument to dockerd is fine and if the per-container opt-in looks like docker run --add-host gateway then that's ok from my PoV.

@arkodg arkodg force-pushed the arkodg:add-host-docker-internal branch from d6e00a9 to dd6d54b Nov 2, 2019
@arkodg

This comment has been minimized.

Copy link
Contributor Author

arkodg commented Nov 2, 2019

updated the PR , would appreciate suggestions for a better name, picked host-gateway as a placeholder

docker run -it --add-host=host.docker.internal:host-gateway alpine cat /etc/hosts
127.0.0.1	localhost
::1	localhost ip6-localhost ip6-loopback
fe00::0	ip6-localnet
ff00::0	ip6-mcastprefix
ff02::1	ip6-allnodes
ff02::2	ip6-allrouters
172.18.0.1	host.docker.internal
172.18.0.2	e25baec2305e
@arkodg arkodg force-pushed the arkodg:add-host-docker-internal branch 2 times, most recently from b566735 to ad495a2 Nov 4, 2019
@arkodg

This comment has been minimized.

Copy link
Contributor Author

arkodg commented Nov 6, 2019

@tiborvass PTAL

Docker Desktop (on MAC and Windows hosts) allows containers
running inside a Linux VM to connect to the host using
the host.docker.internal DNS name, which is implemented by
VPNkit (DNS proxy on the host)

This PR allows containers to connect to Linux hosts
by appending a special string "host-gateway" to --add-host
e.g. "--add-host=host.docker.internal:host-gateway" which adds
host.docker.internal DNS entry in /etc/hosts and maps it to host-gateway-ip

This PR also add a daemon flag call host-gateway-ip which defaults to
the default bridge IP
Docker Desktop will need to set this field to the Host Proxy IP
so DNS requests for host.docker.internal can be routed to VPNkit

Addresses: docker/for-linux#264

Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com>
@arkodg arkodg force-pushed the arkodg:add-host-docker-internal branch from ad495a2 to 1d52cb3 Nov 7, 2019
@@ -64,6 +64,8 @@ func installCommonConfigFlags(conf *config.Config, flags *pflag.FlagSet) error {
flags.Var(opts.NewListOptsRef(&conf.DNS, opts.ValidateIPAddress), "dns", "DNS server to use")
flags.Var(opts.NewNamedListOptsRef("dns-opts", &conf.DNSOptions, nil), "dns-opt", "DNS options to use")
flags.Var(opts.NewListOptsRef(&conf.DNSSearch, opts.ValidateDNSSearch), "dns-search", "DNS search domains to use")
flags.StringVar(&conf.HostGatewayIP, "host-gateway-ip", "", "dockerd will resolve the IP Address to this IP "+

This comment has been minimized.

Copy link
@tiborvass

tiborvass Nov 8, 2019

Collaborator

maybe something like IP address that the special 'host-gateway' string in --add-host resolves to. Defaults to bridge IP value set by --bip

This comment has been minimized.

Copy link
@tiborvass

tiborvass Nov 8, 2019

Collaborator

Maybe use opts.NewIPOpt to have the flags package validate the IP format early on. Check BridgeConfig.DefaultGatewayIPv4 for example

This comment has been minimized.

Copy link
@arkodg

arkodg Nov 8, 2019

Author Contributor

ack

if gateway := daemon.configStore.HostGatewayIP; net.ParseIP(gateway) != nil {
parts[1] = gateway
} else {
logrus.Warnf("HostGatewayIP value %s is invalid", gateway)

This comment has been minimized.

Copy link
@tiborvass

tiborvass Nov 8, 2019

Collaborator

This should never happen if the config is validated on startup and HostGatewayIP is already a net.IP. See https://github.com/moby/moby/pull/40007/files#r344312751

This comment has been minimized.

Copy link
@arkodg

arkodg Nov 8, 2019

Author Contributor

there can be a case where default bridge is disabled and the HostGatewayIP is not set, in that case might be best to bail out ?

@tiborvass

This comment has been minimized.

Copy link
Collaborator

tiborvass commented Nov 8, 2019

Apart from minor nits, I'm fine with the PR. I don't have opinions on the names, so I'm fine with host-gateway magic string and --host-gateway-ip flag. If @djs55 @thaJeztah @tonistiigi or more opinionated people dislike the current names, speak up now.

Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com>
@perlun perlun mentioned this pull request Nov 13, 2019
2 of 3 tasks complete
Copy link
Member

thaJeztah left a comment

LGTM

one question; are host.docker.internal and gateway.docker.internal always expected to have the same "magic" value, or would there be situations where we'd wanted different values for both?

@arkodg

This comment has been minimized.

Copy link
Contributor Author

arkodg commented Nov 14, 2019

I'm not sure what the true difference between gateway.docker.internal and host.docker.internal is ? @djs55 might now

Sidenote - We need a similar fix in docker-cli in opts/hosts.go once this PR is approved

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