Support for Windows service discovery by msabansal · Pull Request #25987 · moby/moby

msabansal · 2016-08-24T17:33:30Z

- What I did
These changes are required to enable service discovery in Windows.

- How I did it
Enabling service discovery requires the following changes:

Enable service discovery for windows in libnetwork
Enable unqualified name resolution for containers which have endpoints in networks that support service discovery
Enable service discovery for default network (nat) in windows as windows has only one nat network and we fill that this is required.
Allow setting DNS servers on endpoints aswell because in windows DNS servers are associated with network endpoints and not sandboxes.

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

- Libnetwork vendoring:
Fixes #25848
Fixes #25608
Fixes #26075

icecrime · 2016-09-19T23:22:34Z

Thanks @msabansal. Have you submitted the libnetwork related changes to docker/libnetwork? This is a dependency to docker/docker and will be reviewed independently by the networking team.

msabansal · 2016-09-20T00:06:12Z

@icecrime Yes it is being verified at moby/libnetwork#1412

mavenugo · 2016-09-21T04:07:15Z

@msabansal am trying this patch in my setup

Ran 2 containers : docker run --name=testc1 -it nanoserver powershell and docker run --name=testc2 -it nanoserver powershell.

PS C:\DNS-Fix> .\docker.exe ps
CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS              PORTS
NAMES
918039635d46        nanoserver          "powershell"        21 minutes ago      Up 21 minutes
testc2
6933055c1c59        nanoserver          "powershell"        21 minutes ago      Up 21 minutes
testc1

Both are running in default nat network.

Container1's DNS server is populated with Default-gateway ip as designed -

PS C:\> ipconfig /all

Windows IP Configuration

   Host Name . . . . . . . . . . . . : 6933055c1c59
   Primary Dns Suffix  . . . . . . . :
   Node Type . . . . . . . . . . . . : Hybrid
   IP Routing Enabled. . . . . . . . : No
   WINS Proxy Enabled. . . . . . . . : No

Ethernet adapter vEthernet (Temp Nic Name):

   Connection-specific DNS Suffix  . :
   Description . . . . . . . . . . . : Hyper-V Virtual Ethernet Adapter #3
   Physical Address. . . . . . . . . : 00-15-5D-2D-04-CE
   DHCP Enabled. . . . . . . . . . . : No
   Autoconfiguration Enabled . . . . : Yes
   Link-local IPv6 Address . . . . . : fe80::8cb5:8211:7cfa:a758%32(Preferred)
   IPv4 Address. . . . . . . . . . . : 172.24.112.219(Preferred)
   Subnet Mask . . . . . . . . . . . : 255.255.240.0
   Default Gateway . . . . . . . . . : 172.24.112.1
   DNS Servers . . . . . . . . . . . : 172.24.112.1
                                       8.8.8.8
   NetBIOS over Tcpip. . . . . . . . : Disabled

Same with container2 as well

PS C:\> ipconfig /all

Windows IP Configuration

   Host Name . . . . . . . . . . . . : 918039635d46
   Primary Dns Suffix  . . . . . . . :
   Node Type . . . . . . . . . . . . : Hybrid
   IP Routing Enabled. . . . . . . . : No
   WINS Proxy Enabled. . . . . . . . : No

Ethernet adapter vEthernet (Temp Nic Name) 2:

   Connection-specific DNS Suffix  . :
   Description . . . . . . . . . . . : Hyper-V Virtual Ethernet Adapter #4
   Physical Address. . . . . . . . . : 00-15-5D-2D-0C-CE
   DHCP Enabled. . . . . . . . . . . : No
   Autoconfiguration Enabled . . . . : Yes
   Link-local IPv6 Address . . . . . : fe80::3d27:c804:e1a2:ff6d%41(Preferred)
   IPv4 Address. . . . . . . . . . . : 172.24.115.149(Preferred)
   Subnet Mask . . . . . . . . . . . : 255.255.240.0
   Default Gateway . . . . . . . . . : 172.24.112.1
   DNS Servers . . . . . . . . . . . : 172.24.112.1
                                       8.8.8.8
   NetBIOS over Tcpip. . . . . . . . : Disabled

But pinging between the containers using the container-name fails.

PS C:\> ping testc1
Ping request could not find host testc1. Please check the name and try again.
PS C:\> ping testc2
Ping request could not find host testc2. Please check the name and try again.

Whereas if I do nslookup from the host with the gateway-ip as the dns-server, it resolves the container-name to container-ip properly.

PS C:\DNS-Fix> nslookup testc1 172.24.112.1
Server:  UnKnown
Address:  172.24.112.1

Non-authoritative answer:
Name:    testc1
Address:  172.24.112.219

PS C:\DNS-Fix> nslookup testc2 172.24.112.1
Server:  UnKnown
Address:  172.24.112.1

Non-authoritative answer:
Name:    testc2
Address:  172.24.115.149

Do you know why the DNS lookup fails from within the container ?

mavenugo · 2016-09-21T05:48:25Z

@msabansal digged a little bit more and figured out that DNS lookup on testc1. and testc2. (notice the period at the end) makes it work just fine. I know @sanimej was looking into something like this in the past for linux. Can you please check if this is related ? Is this related to the windows client side unqualified name resolution limitation ?

sanimej · 2016-09-21T16:49:32Z

@mavenugo Typically in linux an unqualified name will be sent with a . at the end (ie: unqualified name is treated as a query in the root domain). So we strip the . if present before the lookup. When you see the DNS failure from the container you can check the debug log to see the exact query we are getting.

msabansal · 2016-09-21T16:52:26Z

@mavenugo Yes. We have it fixed but it requires updated images which we are going to ship. Can you please run
reg query HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\Tcpip\Parameters

You should be able to see AllowUnqualifiedDnsQuery key set for Windows contianers. (for HyperV containers it requires image update.)

mavenugo · 2016-09-21T18:56:49Z

api/types/network/network.go

I dont think we should expose the endpoint DNS settings to the docker API just yet.
I think we should reuse the daemon / container --dns configurations as is and can set the endpoint level DNS settings at libnetwork.

When we are ready to expose such functionality at the UX (and possibly across platforms), then we can enable this at the API level. So, my request is to remove this for now and we can consider including it at a later point.

mavenugo · 2016-09-21T18:57:28Z

container/container.go

comment above is applicable here as well.

@mavenugo if I remove epConfig.DNS then we can't support daemon.configStore.DNS without a change in the public API or overwriting container.HostConfig.DNS. What option do you think I should take? Should we drop the support for daemon.configStore.DNS altogether?

Why is that ? Please take a look at buildSandboxOptions and it is exactly the same logic I prefer here.
It will be great if we can not replicate the same code in 2 places. Infact this is the reason for my comment in libnetwork to avoid adding the new endpoint specific option for DNS. But since Sandbox is created after endpoint create, we didnt have a choice. Anyways, that is not a big deal. we can use the same logic as buildSandboxOptions and that should work.

if len(container.HostConfig.DNS) > 0 { dns = container.HostConfig.DNS } else if len(daemon.configStore.DNS) > 0 { dns = daemon.configStore.DNS } for _, d := range dns { sboxOptions = append(sboxOptions, libnetwork.OptionDNS(d)) }

daemon is not available in container.go. I would have to pass it if I have to do that which will require changing the public function BuildCreateEndpointOptions. If that is alright I would do that

Thats fine I guess.

mavenugo · 2016-09-21T18:59:31Z

container/container_windows.go

I was originally thinking of not supporting this. But the only reason why we dont support this in Linux today is for backward compatibility reasons. With this being the first iteration of native docker platform on windows, I think it is appropriate to allow it. 👍

mavenugo · 2016-09-21T19:03:10Z

daemon/container_operations.go

am not sure why we need this ! I think we are mixing SD with --ip ?

wanted to add short id in the default network aswell. But that is not required I guess. Removing

I have moved this to the next line otherwise compose complains to bring up containers in the default nat network in windows.

this however disables lookup by shortid in default nat network

thaJeztah · 2016-09-22T16:44:53Z

ping @mavenugo is "design" ok with you? ok to move to code review?

mavenugo

@msabansal just 1 additional comment from my side. I will request @jhowardmsft to review the AllowUnqualifiedDNSQuery related changes under oci_windows and libcontainerd changes.

mavenugo · 2016-09-22T18:29:15Z

container/container.go

It would be good to honor container.NetworkSettings.IsAnonymousEndpoint regardless if windows supports SD on default network. That will make it consistent with all platforms for all networks.

mavenugo · 2016-09-22T19:16:52Z

@msabansal also pls rebase to resolve the conflicts.

msabansal · 2016-09-22T19:19:57Z

@mavenugo Working on it.

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

mavenugo · 2016-09-22T19:52:24Z

LGTM

ping @jhowardmsft

icecrime · 2016-09-23T00:51:26Z

Changes on docker/docker are very limited, so if it's good for Madhu it's good for me: LGTM!

Leaving it to @jhowardmsft to give it a final eye and merge 👍

mavenugo · 2016-09-23T03:55:37Z

Since this PR is blocking other PRs (#26844) which closes a bunch of issue, am merging it. Also the libcontainerd changes seems correct to me.
If required, @jhowardmsft can comment on the libcontainerd changes offline.

lowenna · 2016-09-23T06:28:51Z

Yup LGTM too

drnybble · 2017-02-15T19:58:08Z

docker run --rm --name test microsoft/windowsservercore ping test
Works -> see output showing that the ping functions
docker run -it --rm --name test microsoft/windowsservercore
Try 'ping test' at the PowerShell command line -> fails

Something I am missing?

Trying nslookup test inside the container returns the correct address.

msabansal · 2017-02-16T00:25:18Z

@drnybble What does the following commands show in container?

ipconfig /all
reg query /s HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\Dnscache\Parameters
reg query HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\Tcpip\Parameters

drnybble · 2017-02-16T15:00:12Z

C:>ping test
Ping request could not find host test. Please check the name and try again.

C:>nslookup test
Server: UnKnown
Address: 172.19.144.1

Non-authoritative answer:
Name: test
Address: 172.19.159.253

C:>ipconfig /all

Windows IP Configuration

Host Name . . . . . . . . . . . . : 024743456d0f
Primary Dns Suffix . . . . . . . :
Node Type . . . . . . . . . . . . : Hybrid
IP Routing Enabled. . . . . . . . : No
WINS Proxy Enabled. . . . . . . . : No
DNS Suffix Search List. . . . . . : ottriv.can.ibm.com

Ethernet adapter vEthernet (Container NIC afa9cae1):

Connection-specific DNS Suffix . : ottriv.can.ibm.com
Description . . . . . . . . . . . : Hyper-V Virtual Ethernet Adapter #2
Physical Address. . . . . . . . . : 00-15-5D-D2-92-83
DHCP Enabled. . . . . . . . . . . : No
Autoconfiguration Enabled . . . . : Yes
Link-local IPv6 Address . . . . . : fe80::cc79:1108:eb34:6897%26(Preferred)
IPv4 Address. . . . . . . . . . . : 172.19.159.253(Preferred)
Subnet Mask . . . . . . . . . . . : 255.255.240.0
Default Gateway . . . . . . . . . : 172.19.144.1
DNS Servers . . . . . . . . . . . : 172.19.144.1
9.0.130.50
9.0.128.50
NetBIOS over Tcpip. . . . . . . . : Disabled

C:>reg query HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\services\Dnscache\Parameters /s

HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\services\Dnscache\Parameters
ServerPriorityTimeLimity REG_DWORD 0x0
ServiceDll REG_EXPAND_SZ %SystemRoot%\System32\dnsrslvr.dll
ServiceDllUnloadOnStop REG_DWORD 0x1

HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\services\Dnscache\Parameters\Probe

C:>reg query HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\Tcpip\Parameters

HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\Tcpip\Parameters
NV HostName REG_SZ 024743456d0f
AllowUnqualifiedQuery REG_DWORD 0x1
Hostname REG_SZ 024743456d0f
DataBasePath REG_EXPAND_SZ %SystemRoot%\System32\drivers\etc
Domain REG_SZ
ForwardBroadcasts REG_DWORD 0x0
ICSDomain REG_SZ mshome.net
IPEnableRouter REG_DWORD 0x0
NameServer REG_SZ
SyncDomainWithMembership REG_DWORD 0x1

HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Adapters
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\DNSRegisteredAdapters
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\NsiObjectSecurity
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\PersistentRoutes
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Winsock

PS C:\Users\Administrator> docker version
Client:
Version: 1.14.0-dev
API version: 1.26
Go version: go1.7.4
Git commit: fa5e097
Built: Fri Jan 27 17:05:28 2017
OS/Arch: windows/amd64

Server:
Version: 1.14.0-dev
API version: 1.26 (minimum version 1.24)
Go version: go1.7.4
Git commit: fa5e097
Built: Fri Jan 27 17:05:28 2017
OS/Arch: windows/amd64
Experimental: false

msabansal · 2017-02-16T17:38:09Z

@drnybble All this information looks good. One more question. What does ping test. give in the container (there is a . appended to the query) Can you please open an issue with this info and ping me on it

drnybble · 2017-02-16T18:10:28Z

Yes "ping test." works while "ping test" does not.
But I must say it is very inconsistent. I try a container with the name "testing" instead and I can ping it directly with no dot. I then relaunch the container and try again -- and it fails. Launch it again and it works....

Logged a new issue, see #31093

vdemeester added the status/1-design-review label Aug 24, 2016

cpuguy83 added the area/networking Networking label Aug 26, 2016

msabansal mentioned this pull request Sep 7, 2016

Networking doesn't work with Windows containers due to disconnecting containers from networks docker/compose#3252

Closed

thaJeztah added this to the 1.13.0 milestone Sep 20, 2016

mavenugo reviewed Sep 21, 2016

View reviewed changes

mavenugo changed the title ~~Dnssupport~~ Support for Windows service discovery Sep 22, 2016

mavenugo added status/2-code-review and removed status/1-design-review labels Sep 22, 2016

mavenugo reviewed Sep 22, 2016

View reviewed changes

msabansal added 2 commits September 22, 2016 12:21

Changes required to support windows service discovery

d1e0a78

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

Vendoring dependencies

522930c

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

mavenugo mentioned this pull request Sep 23, 2016

Vendoring libnetwork @6caf9022fa09 #26844

Merged

mavenugo merged commit d3139fc into moby:master Sep 23, 2016

friism mentioned this pull request Sep 28, 2016

Windows proposal: implement single-host service discovery with c:\Windows\System32\drivers\etc\hosts rewrites #23068

Closed

StefanScherer added a commit to StefanScherer/dockerfiles-windows that referenced this pull request Sep 28, 2016

Playing with moby/moby#25987

74604bd

jbhurat mentioned this pull request Oct 6, 2016

DNS support for Windows Containers kubernetes/kubernetes#34263

Closed

drnybble mentioned this pull request Feb 16, 2017

Windows service discovery does not work consistently #31093

Closed

Conversation

msabansal commented Aug 24, 2016 • edited by aboch Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

icecrime commented Sep 19, 2016

Uh oh!

msabansal commented Sep 20, 2016

Uh oh!

mavenugo commented Sep 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mavenugo commented Sep 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sanimej commented Sep 21, 2016

Uh oh!

msabansal commented Sep 21, 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!

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!

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!

thaJeztah commented Sep 22, 2016

Uh oh!

mavenugo left a comment

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!

mavenugo commented Sep 22, 2016

Uh oh!

msabansal commented Sep 22, 2016

Uh oh!

mavenugo commented Sep 22, 2016

Uh oh!

icecrime commented Sep 23, 2016

Uh oh!

mavenugo commented Sep 23, 2016

Uh oh!

lowenna commented Sep 23, 2016

Uh oh!

drnybble commented Feb 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

msabansal commented Feb 16, 2017

Uh oh!

drnybble commented Feb 16, 2017

Uh oh!

msabansal commented Feb 16, 2017

Uh oh!

drnybble commented Feb 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

msabansal commented Aug 24, 2016 •

edited by aboch

Loading

mavenugo commented Sep 21, 2016 •

edited

Loading

mavenugo commented Sep 21, 2016 •

edited

Loading

drnybble commented Feb 15, 2017 •

edited

Loading

drnybble commented Feb 16, 2017 •

edited

Loading