Setup dns servers and search domains for Windows Pods #63905

feiskyer · 2018-05-16T04:36:05Z

What this PR does / why we need it:

Kubelet is depending on docker container's ResolvConfPath (e.g. /var/lib/docker/containers/439efe31d70fc17485fb6810730679404bb5a6d721b10035c3784157966c7e17/resolv.conf) to setup dns servers and search domains. While this is ok for Linux containers, ResolvConfPath is always an empty string for windows containers. So that the DNS setting for windows containers is always not set.

This PR setups DNS for Windows sandboxes. In this way, Windows Pods could also use kubernetes dns policies.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #61579

Special notes for your reviewer:

Requires Docker EE version >= 17.10.0.

Release note:

Setup dns servers and search domains for Windows Pods in dockershim. Docker EE version >= 17.10.0 is required for propagating DNS to containers.

/cc @PatrickLang @taylorb-microsoft @michmike @JiangtianLi

k8s-ci-robot · 2018-05-16T04:36:08Z

@feiskyer: GitHub didn't allow me to request PR reviews from the following users: PatrickLang, taylorb-microsoft.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

What this PR does / why we need it:

Kubelet is depending on docker container's ResolvConfPath (e.g. /var/lib/docker/containers/439efe31d70fc17485fb6810730679404bb5a6d721b10035c3784157966c7e17/resolv.conf) to setup dns servers and search domains. While this is ok for Linux containers, ResolvConfPath is always an empty string for windows containers. So that the DNS setting for windows containers is always not set.

This PR setups DNS for Windows sandboxes. In this way, Windows Pods could also use kubernetes dns policies.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #61579

Special notes for your reviewer:

Requires Docker version >= 17.10.0.

Release note:
Setup dns servers and search domains for Windows Pods in dockershim.
/cc @PatrickLang @taylorb-microsoft @michmike @JiangtianLi

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

PatrickLang · 2018-05-16T23:23:02Z

pkg/kubelet/dockershim/docker_sandbox_windows.go

+
+// applySandboxPlatformOptions applies platform specific options to dockercontainer.HostConfig and dockercontainer.ContainerCreateConfig.
+func (ds *dockerService) applySandboxPlatformOptions(hc *dockercontainer.HostConfig, config *runtimeapi.PodSandboxConfig, createConfig *dockertypes.ContainerCreateConfig, image string, separator rune) error {
+	dnsConfig := config.GetDnsConfig()


Where is this config initially set on Windows? Does it already have valid data, or does it need to be set up in another PR?

@jstarks any thoughts here?

It is from CRI, specifically from kuberuntime. There's already valid data, but just not set into docker container's config.

akochnev

@feiskyer - the code changes look fairly straightforward and isolated; however, this doesn't seem to be enough to resolve the issue (at least in server 2016). The container of the actual workload doesn't get the correct settings . If I understand correctly, the assumption is that if the dns configuration is set up in the HostConfig, then when the containers are started, they will get the correct settings.

Unfortunately, my testing on server 2016 shows that although the "sandbox container" ( in this case, the "apprenda/pause" container) does get the correct DNS settings, the container of the workload in the same pod, doesn't.

In my example app :

PS C:\Users\vagrant> docker ps
CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS
     NAMES
b78da7582bfd        4dba31379dad        "powershell.exe -c..."   36 minutes ago      Up 36 minutes
     k8s_test-container_dapi-envars-fieldref-win_default_30371bdd-5abf-11e8-aa21-080027ee1df7_0
f4f40e1025cc        apprenda/pause      "cmd /S /C 'powers..."   36 minutes ago      Up 36 minutes
     k8s_POD_dapi-envars-fieldref-win_default_30371bdd-5abf-11e8-aa21-080027ee1df7_0

That shows both containers sitting in the pod. When I exec into the "apprenda/pause" container, it gets the correct DNS settings and the names of services are resolved by kube-dns:

PS C:\> ipconfig /all

Windows IP Configuration

   Host Name . . . . . . . . . . . . : dapi-envars-fieldref-win
   Primary Dns Suffix  . . . . . . . :
   Node Type . . . . . . . . . . . . : Hybrid
   IP Routing Enabled. . . . . . . . : No
   WINS Proxy Enabled. . . . . . . . : No
   DNS Suffix Search List. . . . . . : default.svc.cluster.local
                                       svc.cluster.local
                                       cluster.local

Ethernet adapter vEthernet (Container NIC d470fc15):

   Connection-specific DNS Suffix  . : apprenda.local
   Description . . . . . . . . . . . : Hyper-V Virtual Ethernet Adapter #6
   Physical Address. . . . . . . . . : 00-15-5D-91-C1-FF
   DHCP Enabled. . . . . . . . . . . : No
   Autoconfiguration Enabled . . . . : Yes
   Link-local IPv6 Address . . . . . : fe80::b9f9:9e1f:fc8d:2534%33(Preferred)
   IPv4 Address. . . . . . . . . . . : 172.24.24.33(Preferred)
   Subnet Mask . . . . . . . . . . . : 255.255.240.0
   Default Gateway . . . . . . . . . : 172.24.16.1
   DNS Servers . . . . . . . . . . . : 172.24.16.1
                                       172.16.1.10
   NetBIOS over Tcpip. . . . . . . . : Disabled
PS C:\> ping my-nginx

Pinging my-nginx.default.svc.cluster.local [172.16.1.46]

However, in the container of the actual workload, that's not the case:

PS C:\> ipconfig /all

Windows IP Configuration

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

Ethernet adapter vEthernet (Container NIC 828752cc):

   Connection-specific DNS Suffix  . :
   Description . . . . . . . . . . . : Hyper-V Virtual Ethernet Adapter #8
   Physical Address. . . . . . . . . : 00-15-5D-C8-85-92
   DHCP Enabled. . . . . . . . . . . : No
   Autoconfiguration Enabled . . . . : Yes
   Link-local IPv6 Address . . . . . : fe80::59bc:3470:cd58:8fc4%40(Preferred)
   IPv4 Address. . . . . . . . . . . : 192.168.2.53(Preferred)
   Subnet Mask . . . . . . . . . . . : 255.255.255.0
   Default Gateway . . . . . . . . . : 192.168.2.1
   DNS Servers . . . . . . . . . . . : fec0:0:0:ffff::1%1
                                       fec0:0:0:ffff::2%1
                                       fec0:0:0:ffff::3%1
   NetBIOS over Tcpip. . . . . . . . : Disabled
PS C:\> ping my-nginx
Ping request could not find host my-nginx. Please check the name and try again.

I was able to observe the same behavior if I "kubectl exec" into the pod, as it works only with the "workload container" (and not the infrastructure/sandbox/pause container), and the ipconfig settings do not have the correct DNS settings.

Does this behavior differ from what you have tested with ?

My windows kubelet was built from your PR. The rest of the cluster is a 1.10.0 cluster , with ovs/ovn CNI (I don't believe that to be relevant to this issue)

PS C:\k> .\kubelet --version
Kubernetes v1.11.0-alpha.2.541+228ba8e9090bf0-dirty

feiskyer · 2018-05-21T03:06:11Z

@akochnev I think it's a bug of old docker releases. As stated in PR description, it requires Docker version >= 17.10.0.

feiskyer · 2018-05-21T03:06:32Z

/sig windows
/sig node

feiskyer · 2018-05-21T03:06:42Z

/assign @Random-Liu

feiskyer · 2018-05-21T03:07:08Z

@Random-Liu Could you also take a look at the PR?

PatrickLang · 2018-05-23T01:36:51Z

I emailed some MS networking devs asking for a review. They've been looking at other DNS related issues in the Azure CNI plugin so they're the best to review this for Windows.

feiskyer · 2018-05-23T06:40:07Z

/milestone v1.11
/status approved-for-milestone

PatrickLang · 2018-05-25T17:45:24Z

/lgtm

PatrickLang · 2018-05-25T22:10:46Z

/test pull-kubernetes-kubemark-e2e-gce-big
/test pull-kubernetes-local-e2e-containerized

alinbalutoiu · 2018-05-29T12:23:36Z

I am encountering the same issue that @akochnev was describing. It doesn't seem that this commit fixes the issue in that case.

I've tested on Windows Server 2016 version 1709 and 1803, docker version 17.10 and 17.11 from https://download.docker.com/win/static/test/x86_64/
Kubelet was built on top of master with this patch added to it.

More details about this issue can be found here: ovn-org/ovn-kubernetes#336 (comment)

@feiskyer Any ideas what the issue could be? The infra container gets everything set up properly, but the other containers that are sharing the network compartment do not have the DNS set.

dims · 2018-06-01T16:51:17Z

/test pull-kubernetes-local-e2e-containerized

feiskyer · 2018-06-07T07:19:29Z

But we do need windows test soon... There is no test, we don't know whether this PR works or not.

Yep, @PatrickLang is working on this. Hoping we could make it within v1.12.

derekwaynecarr · 2018-06-07T15:26:07Z

I agree w/ @Random-Liu about the need for testing.

/approve
/lgtm

k8s-github-robot · 2018-06-07T15:26:45Z

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@PatrickLang @Random-Liu @dchen1107 @derekwaynecarr @feiskyer @yujuhong

Pull Request Labels

sig/node sig/windows: Pull Request will be escalated to these SIGs if needed.
priority/critical-urgent: Never automatically move pull request out of a release milestone; continually escalate to contributor and SIG through all available channels.
kind/bug: Fixes a bug discovered during the current release.

Help

derekwaynecarr · 2018-06-07T15:33:11Z

/test all

dchen1107 · 2018-06-07T15:48:08Z

/approve

k8s-ci-robot · 2018-06-07T15:48:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dchen1107, derekwaynecarr, feiskyer, PatrickLang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/kubelet/OWNERS~~ [dchen1107,derekwaynecarr]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-github-robot · 2018-06-07T18:40:10Z

Automatic merge from submit-queue (batch tested with PRs 63905, 64855). If you want to cherry-pick this change to another branch, please follow the instructions here.

dineshgovindasamy · 2018-06-07T19:46:01Z

@madhanrm can you review these changes?

madhanrm · 2018-06-12T19:43:30Z

I don't think this change does anything on Windows. On windows, the network endpoint configuration is taken care of completely by CNI. If you would like to pass on the custom dns polices from the pod spec, it should be dynamically going to the cni configuration that gets passed to CNI. From there, it would be passed down to platform and would be taken care of appropriately by HNS.

etc\resolve.conf is very specific to linux and that should remain linux speicfic implementation. We should be trying to move away from platform specific code in Kubelet.
Docker is not managing the networking here for windows. So it doens't really care about any network settings. So passing it to docker shim's hostconfig also doens;t make sense here.

michmike · 2018-06-13T23:25:10Z

@madhanrm, should we be reverting the changes with this PR then?

…dows Pods

Automatic merge from submit-queue (batch tested with PRs 66491, 66587, 66856, 66657, 66923). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Revert #63905: Setup dns servers and search domains for Windows Pods **What this PR does / why we need it**: From #63905 (comment): > I don't think this change does anything on Windows. On windows, the network endpoint configuration is taken care of completely by CNI. If you would like to pass on the custom dns polices from the pod spec, it should be dynamically going to the cni configuration that gets passed to CNI. From there, it would be passed down to platform and would be taken care of appropriately by HNS. > etc\resolve.conf is very specific to linux and that should remain linux speicfic implementation. We should be trying to move away from platform specific code in Kubelet. Docker is not managing the networking here for windows. So it doens't really care about any network settings. So passing it to docker shim's hostconfig also doens;t make sense here. DNS for Windows containers will be set by CNI plugins. And this change also introduced two endpoints for sandbox container. So this PR reverts #63905 . **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Fixes # **Special notes for your reviewer**: The PR should also be cherry-picked to release-1.11. Also, #66588 is opened to track the process of pushing this to CNI. **Release note**: ```release-note Revert #63905: Setup dns servers and search domains for Windows Pods. DNS for Windows containers will be set by CNI plugins. ``` /sig windows /sig node /kind bug

…dows Pods

…87-upstream-release-1.11 Automatic merge from submit-queue. Automated cherry pick of #66587: Revert #63905: Setup dns servers and search domains for Cherry pick of #66587 on release-1.11. #66587: Revert #63905: Setup dns servers and search domains for

…dows Pods

Setup dns servers and search domains for Windows Pods

228ba8e

k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 16, 2018

k8s-ci-robot requested review from michmike and JiangtianLi May 16, 2018 04:36

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 16, 2018

PatrickLang reviewed May 16, 2018

View reviewed changes

michmike approved these changes May 18, 2018

View reviewed changes

akochnev suggested changes May 18, 2018

View reviewed changes

k8s-ci-robot added sig/windows Categorizes an issue or PR as relevant to SIG Windows. sig/node Categorizes an issue or PR as relevant to SIG Node. labels May 21, 2018

k8s-ci-robot assigned Random-Liu May 21, 2018

feiskyer added kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels May 23, 2018

k8s-ci-robot added the status/approved-for-milestone label May 23, 2018

k8s-ci-robot added this to the v1.11 milestone May 23, 2018

PatrickLang approved these changes May 25, 2018

View reviewed changes

k8s-ci-robot assigned PatrickLang May 25, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 25, 2018

feiskyer mentioned this pull request Jun 7, 2018

Setup docker container's dns directly instead of rewriting resolv.conf #64864

Closed

k8s-ci-robot assigned derekwaynecarr Jun 7, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 7, 2018

alinbalutoiu mentioned this pull request Jun 7, 2018

Revert #64189: Fix Windows CNI for the sandbox case #64862

Merged

k8s-github-robot merged commit 38beee6 into kubernetes:master Jun 7, 2018

feiskyer deleted the win-dns branch June 7, 2018 20:52

feiskyer restored the win-dns branch July 20, 2018 06:59

feiskyer deleted the win-dns branch July 25, 2018 00:14

feiskyer added a commit to feiskyer/kubernetes that referenced this pull request Jul 25, 2018

Revert kubernetes#63905: Setup dns servers and search domains for Win…

cfb776d

…dows Pods

This was referenced Jul 25, 2018

Revert #63905: Setup dns servers and search domains for Windows Pods #66587

Merged

Setup DNS for Windows containers #66588

Closed

PatrickLang mentioned this pull request Jul 30, 2018

Pod Dns Policy not implemented on Windows, leaves incomplete suffix list #66801

Closed

feiskyer mentioned this pull request Aug 15, 2018

Automated cherry pick of #66587: Revert #63905: Setup dns servers and search domains for #67408

Merged

feiskyer added a commit to feiskyer/kubernetes that referenced this pull request Aug 15, 2018

Revert kubernetes#63905: Setup dns servers and search domains for Win…

150d166

…dows Pods

feiskyer mentioned this pull request Aug 15, 2018

Add dns capabilities for Windows CNI plugins #67435

Merged

aniket-s-kulkarni pushed a commit to aniket-s-kulkarni/kubernetes that referenced this pull request Aug 24, 2018

Revert kubernetes#63905: Setup dns servers and search domains for Win…

0ae332e

…dows Pods

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setup dns servers and search domains for Windows Pods #63905

Setup dns servers and search domains for Windows Pods #63905

feiskyer commented May 16, 2018 •

edited

k8s-ci-robot commented May 16, 2018

PatrickLang May 16, 2018

PatrickLang May 16, 2018

feiskyer May 17, 2018

akochnev left a comment •

edited

feiskyer commented May 21, 2018

feiskyer commented May 21, 2018

feiskyer commented May 21, 2018

feiskyer commented May 21, 2018

PatrickLang commented May 23, 2018

feiskyer commented May 23, 2018

PatrickLang commented May 25, 2018

PatrickLang commented May 25, 2018

alinbalutoiu commented May 29, 2018

dims commented Jun 1, 2018

feiskyer commented Jun 7, 2018

derekwaynecarr commented Jun 7, 2018

k8s-github-robot commented Jun 7, 2018

derekwaynecarr commented Jun 7, 2018

dchen1107 commented Jun 7, 2018

k8s-ci-robot commented Jun 7, 2018

k8s-github-robot commented Jun 7, 2018

dineshgovindasamy commented Jun 7, 2018

madhanrm commented Jun 12, 2018

michmike commented Jun 13, 2018

Setup dns servers and search domains for Windows Pods #63905

Setup dns servers and search domains for Windows Pods #63905

Conversation

feiskyer commented May 16, 2018 • edited

k8s-ci-robot commented May 16, 2018

PatrickLang May 16, 2018

Choose a reason for hiding this comment

PatrickLang May 16, 2018

Choose a reason for hiding this comment

feiskyer May 17, 2018

Choose a reason for hiding this comment

akochnev left a comment • edited

Choose a reason for hiding this comment

feiskyer commented May 21, 2018

feiskyer commented May 21, 2018

feiskyer commented May 21, 2018

feiskyer commented May 21, 2018

PatrickLang commented May 23, 2018

feiskyer commented May 23, 2018

PatrickLang commented May 25, 2018

PatrickLang commented May 25, 2018

alinbalutoiu commented May 29, 2018

dims commented Jun 1, 2018

feiskyer commented Jun 7, 2018

derekwaynecarr commented Jun 7, 2018

k8s-github-robot commented Jun 7, 2018

derekwaynecarr commented Jun 7, 2018

dchen1107 commented Jun 7, 2018

k8s-ci-robot commented Jun 7, 2018

k8s-github-robot commented Jun 7, 2018

dineshgovindasamy commented Jun 7, 2018

madhanrm commented Jun 12, 2018

michmike commented Jun 13, 2018

feiskyer commented May 16, 2018 •

edited

akochnev left a comment •

edited