DNS support for Windows by msabansal · Pull Request #1412 · moby/libnetwork

msabansal · 2016-08-23T22:42:53Z

This PR adds support for DNS for windows. In Windows the DNS resolver will listen on the gateway for the network which is routable from each container. If the name can be resolved we reply back with a positive answer. If the name can't be resolved we reply with a negative answer and the dns client in the container takes care of forwarding the queries to other DNS servers

Signed-off-by: msabansal [email protected]

sanimej · 2016-08-29T22:00:46Z

controller.go

 	}

+	n.startResolver()
+


@msabansal Can a container be connected to only one network in Windows ? Even if that is the case today for nat networks I think we should consider the possibility in future a container can be connected to multiple networks. Hence the resolver instance should be associated with the sandbox and not per network. We already have platform specific files, sandbox_dns_unix.go and sandbox_dns_windows.go to abstract out the implementation details of startResolver.

The name/ip lookup functions are not platform specific and the service DB svcRecords is already keyed by the network ID. So the lookup functions ResolveName etc. can remain in sandbox.go and doesn't have to be refactored.

The reason we have the resolver associated with the network is we can't distinguish a sandbox based on the request. There is a single resolver listening on the gateway IP of the network. The IP is routable from the container. In windows DNS servers are associated with a network adapter (endpoint). If there are multiple networks we will have multiple endpoints each with it's own list of dns servers (which will be the gateway IP plus any other user supplied dns servers).

A container connected to multiple networks should be able to resolve the service names in any network its connected to. This is done by walking through all the end-points in resolveName and lookup the service DB of the network to which the EP belongs.

For Windows containers if the query from the container can be associated with only one of the networks (depending on which interface the client sends the query on) is there any way to search the other networks as well ?

Yes. Windows DNS resolver inside the container would send out a new query for every endpoint the container is connected to so this should not be an issue. Every network connected to the container will get a query and will be able to resolve the name.

sanimej · 2016-08-29T22:05:31Z

docker run has the following dns specific config options; --dns, --dns-opt and --dns-search. Will any of this be used for Windows containers ? If they are not relevant will the config be blocked ?

sanimej · 2016-08-29T22:08:48Z

endpoint.go

+		ep.generic[netlabel.DNSServers] = dns
+	}
+}
+


External nameservers are not a property of the endpoints. Since the windows DNS server will only try to resolve the container names and not doing the proxying (ie: forwarding queries to external servers) why is this necessary ?

We have added this to support the --dns server options. These servers will be passed on to HNS for configuring the container. In windows dns servers are specific to network adapters and are not universal as in linux

@msabansal its fine to have windows network adapters to have specific dns servers. But from Docker POV, the configuration is per container and this configuration is passed to libnetwork when the sandbox is created. Shouldn't we reuse that and use it per-endpoint instead of adding a new option per endpoint ?

@mavenugo With the current libnetwork model you can have an endpoint created before the sandbox is created. We don't allow updating endpoints after they have been created. We need the DNS information before endpoint creation and the current model doesn't guarantee it.

sanimej · 2016-08-29T22:24:23Z

One of the use cases we have had is to be able to run a DNS server in a container and use that as the resolver for other containers. Can you confirm that it will work for Windows as well ?

sanimej · 2016-08-29T22:39:09Z

In Windows the DNS resolver will listen on the gateway for the network which is routable from each container.

If the host is already running a DNS server and using the default config it will bind port 53 on all IPs. In that case setting up the internal DNS on the gateway IP will fail. Will this be a limitation or are there any alternatives ? In Linux we explicitly setup the listener in the container namespace on a random port and do the port translation from port 53 to this random port to not break this use case.

msabansal · 2016-08-30T00:17:02Z

One of the use cases we have had is to be able to run a DNS server in a container and use that as the resolver for other containers. Can you confirm that it will work for Windows as well ?
This cannot be supported with our current implementation. There is no way for us to execute a query in the context of another container (particularly for Hyperv containers which are default on developer machines)

If the host is already running a DNS server and using the default config it will bind port 53 on all IPs. In that case setting up the internal DNS on the gateway IP will fail. Will this be a limitation or are there any alternatives ? In Linux we explicitly setup the listener in the container namespace on a random port and do the port translation from port 53 to this random port to not break this use case.

Yes that is a limitation which can't be avoided. We have no port translation facility available on windows to enable this.

sanimej · 2016-08-30T23:44:36Z

We currently support the concept of container scoped aliases; ie: one container can add an alias for another container's name using --link option. In the example below only for c2, web is an alias for c1.

vagrant@net-1:~$ docker run -d --net docker --name c1 busybox top
83006320f67d176ff29a9d07405e2433c759a45149650039bb8930dcae915f73
vagrant@net-1:~$ docker run -d --net docker --name c2 --link c1:web busybox top
09156b7efd49196048fd5b9093107d3da4b0c58edd4a8157a2513d0f7a2cdc24
vagrant@net-1:~$ docker exec -it c2 sh
/ # nslookup c1
Server:    127.0.0.11
Address 1: 127.0.0.11

Name:      c1
Address 1: 172.18.0.2 c1.docker
/ # nslookup web
Server:    127.0.0.11
Address 1: 127.0.0.11

Name:      web
Address 1: 172.18.0.2 c1.docker
/ #

I think this can't be supported in Windows because of resolver being at the network level and not per container. This should also be mentioned in the caveats with other limitations we have identified.

msabansal · 2016-09-02T01:23:14Z

@sanimej Incorporated the changes.

Not sure why this test is failing:
--- FAIL: TestNetworkRequest (0.02s)
utils_test.go:226: exected 172.17.0.0/16. got 172.18.0.0/16

Don't think I have any changes that should impact this. I am trying to figure it out

sanimej · 2016-09-02T13:14:27Z

--- FAIL: TestNetworkRequest (0.02s)

Yes, this failure is not related to your changes. This has been fixed in the master. If you rebase to the latest master it should pass.

mavenugo

Overall the code looks simple & clean. 1 minor comment regarding a change to public endpoint API which I think we can avoid.

mavenugo · 2016-09-19T18:17:51Z

@msabansal can you please rebase & resolve the conflicts ?

msabansal · 2016-09-19T18:19:34Z

@mavenugo Working on it. Getting it done by EOD

sanimej · 2016-09-19T18:59:53Z

network.go

+	// ResolveService returns all the backend details about the containers or hosts
+	// backing a service. Its purpose is to satisfy an SRV query
+	ResolveService(name string) ([]*net.SRV, []net.IP)
+


These Resolve methods are defined in the DNSBackend interface. Please remove it from the Network interface.

Its not necessary in the Sandbox interface either where its currently defined. That can be removed as well.

msabansal · 2016-09-20T00:26:32Z

@mavenugo Done. There is one test failing but I don't think its related. I will take a look at the test soon.

sanimej · 2016-09-20T01:12:30Z

network_windows.go

+
+func (n *network) startResolver() {
+	n.resolverOnce.Do(func() {
+		log.Warnf("Launching DNS server for network", n.Name())


This should be log.Debugf

Accepted. Will fix all the logging related stuff asap.

sanimej · 2016-09-20T01:13:43Z

network_windows.go

+		for _, subnet := range hnsresponse.Subnets {
+			if subnet.GatewayAddress != "" {
+				resolver := NewResolver(subnet.GatewayAddress, false, "", n)
+				log.Warnf("Binding a resolver on network %s gateway %s", n.Name(), subnet.GatewayAddress)


This should be log.Debugf

sanimej · 2016-09-20T01:28:54Z

@msabansal Other than couple of nits the changes LGTM. I will try with the changes to confirm there is no collateral for Linux. Will update by tomorrow morning.

mavenugo · 2016-09-20T01:39:11Z

Thanks @msabansal

LGTM

sanimej · 2016-09-20T16:07:41Z

Verified on docker master with these changes. Looks ok for the linux name resolution. I think we are good to merge if you can address couple of comments on changing Warn messages to Debug.

Signed-off-by: msabansal <[email protected]>

mavenugo · 2016-09-21T00:35:19Z

LGTM

GordonTheTurtle added status/0-triage group/windows labels Aug 23, 2016

sanimej reviewed Aug 29, 2016
View reviewed changes

sanimej mentioned this pull request Sep 7, 2016

Vlan and VSID support and adding some missing parameters #1397

Closed

mavenugo reviewed Sep 19, 2016

View reviewed changes

sanimej reviewed Sep 19, 2016

View reviewed changes

msabansal mentioned this pull request Sep 20, 2016

Support for Windows service discovery moby/moby#25987

Merged

sanimej reviewed Sep 20, 2016

View reviewed changes

msabansal added 2 commits September 20, 2016 13:02

DNS support

52d17c5

Signed-off-by: msabansal <[email protected]>

Vendoring dependencies

c35192a

Signed-off-by: msabansal <[email protected]>

mavenugo merged commit e669011 into moby:master Sep 21, 2016

sanimej mentioned this pull request Sep 21, 2016

Check for ExecFunc failure #1459

Merged

jbhurat mentioned this pull request Oct 6, 2016

DNS support for Windows Containers kubernetes/kubernetes#34263

Closed

corhere mentioned this pull request Nov 9, 2023

External DNS forwarding is broken on Windows container NAT networks moby/moby#46792

Closed

Conversation

msabansal commented Aug 23, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanimej commented Aug 29, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanimej commented Aug 29, 2016

Uh oh!

sanimej commented Aug 29, 2016

Uh oh!

msabansal commented Aug 30, 2016

Uh oh!

sanimej commented Aug 30, 2016

Uh oh!

msabansal commented Sep 2, 2016

Uh oh!

sanimej commented Sep 2, 2016

Uh oh!

mavenugo left a comment

Choose a reason for hiding this comment

Uh oh!

mavenugo commented Sep 19, 2016

Uh oh!

msabansal commented Sep 19, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msabansal commented Sep 20, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanimej commented Sep 20, 2016

Uh oh!

mavenugo commented Sep 20, 2016

Uh oh!

sanimej commented Sep 20, 2016

Uh oh!

mavenugo commented Sep 21, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants