kubelet manages /etc/hosts file #16052

ArtfulCoder · 2015-10-21T20:10:00Z

This implementation relies on moby/moby#14613

ArtfulCoder · 2015-10-21T20:11:59Z

ArtfulCoder · 2015-10-21T20:12:20Z

thockin · 2015-10-21T20:17:27Z

pkg/kubelet/kubelet.go

+}
+
+func makeHostsMount(podDir, podIP, podName string) (*kubecontainer.Mount, error) {
+	hostsFilePath := path.Join(podDir, "hosts")


can we call the file "etc-hosts" ?

fixed, added container name to it as well

thockin · 2015-10-21T20:19:01Z

nice fix. Waiting for e2e

k8s-github-robot · 2015-10-21T20:28:43Z

Labelling this PR as size/M

k8s-bot · 2015-10-21T20:36:03Z

GCE e2e test build/test passed for commit 77d170ac19a9d09d307da242e896d45a23039731.

bgrant0607 · 2015-10-21T20:42:17Z

pkg/kubelet/kubelet.go

@@ -955,8 +958,11 @@ func (kl *Kubelet) syncNodeStatus() {
 	}
 }

-func makeMounts(container *api.Container, podVolumes kubecontainer.VolumeMap) (mounts []kubecontainer.Mount) {
+func (kl *Kubelet) makeMounts(pod *api.Pod, container *api.Container, podVolumes kubecontainer.VolumeMap) ([]kubecontainer.Mount, error) {
+	mountEtcHostsFile := !pod.Spec.SecurityContext.HostNetwork


What does Docker do with /etc/hosts in the case of net=host?

It mounts the the host's /etc/hosts

On Wed, Oct 21, 2015 at 1:42 PM, Brian Grant notifications@github.com
wrote:

In pkg/kubelet/kubelet.go
#16052 (comment)
:

@@ -955,8 +958,11 @@ func (kl *Kubelet) syncNodeStatus() {
}
}

-func makeMounts(container *api.Container, podVolumes kubecontainer.VolumeMap) (mounts []kubecontainer.Mount) {
+func (kl *Kubelet) makeMounts(pod *api.Pod, container *api.Container, podVolumes kubecontainer.VolumeMap) ([]kubecontainer.Mount, error) {

mountEtcHostsFile := !pod.Spec.SecurityContext.HostNetwork

What does Docker do with /etc/hosts in the case of net=host?

—
Reply to this email directly or view it on GitHub
https://github.com/kubernetes/kubernetes/pull/16052/files#r42679420.

ArtfulCoder · 2015-10-21T21:07:44Z

etchosts is per-container
added e2e
adding more e2e

dchen1107 · 2015-10-21T21:10:04Z

The current implementation is like this way: for a given pod, we re-write pod's /etc/hosts file completely when every container start / restart. Also at network container creation stage, there is no PodIP information yet. I am not sure this is the best solution.

What I proposed yesterday:

Create pod based etc_hosts file at /var/lib/kubelet/pods/$pod_id directory and write whatever static content there including localhost.
Every container creation stage, mount above file as volume
After network container is creation, update pod's etcd_hosts file with pod ip information.

dchen1107 · 2015-10-21T21:13:06Z

Ok, talked to @ArtfulCoder offline. He changed this to per container based, not pod based, so that Kubelet won't overwrite the single file every container start / restart. The side effect is that we create an extra file for each container. I guess this should be ok.

k8s-github-robot · 2015-10-21T21:32:02Z

Labelling this PR as size/L

k8s-bot · 2015-10-21T21:35:03Z

GCE e2e test build/test passed for commit c9420095e3a2844db8591d3d02fb2f8c6620116f.

ArtfulCoder · 2015-10-21T22:01:28Z

e2e added
Please review

thockin · 2015-10-21T22:13:17Z

pkg/kubelet/kubelet.go

+}
+
+func makeHostsMount(podDir, podIP, podName, containerName string) (*kubecontainer.Mount, error) {
+	hostsFilePath := path.Join(podDir, fmt.Sprintf("%s-etc-hosts", containerName))


we should not need a file per container - can we make the hosts file once after the pod is created and then use that for all of the container mounts?

thockin · 2015-10-21T22:23:21Z

A few small points, then LGTM

k8s-bot · 2015-10-21T22:36:08Z

GCE e2e test build/test passed for commit 716ea07b90fc4513edbdebb0783e4ed5ddec4c5d.

ArtfulCoder · 2015-10-22T00:17:59Z

feedback incorporated.
PTAL

k8s-bot · 2015-10-22T00:51:58Z

GCE e2e test build/test passed for commit 5b2f28d35ee913ad4541ee023b027c8f6c7d62c4.

ArtfulCoder · 2015-10-22T16:32:20Z

fixing test issue found by shippable

dchen1107 · 2015-10-22T16:35:09Z

pkg/kubelet/kubelet.go

+
+func ensureHostsFile(fileName string, hostIP, hostName string) error {
+	if _, err := os.Stat(fileName); os.IsExist(err) {
+		glog.Errorf("File exits: %q", fileName)


nit: Shouldn't be Errorf here, right?

s/exits/exists

but I agree with dawn - is this log line useful? It seems like V(4) at best

dchen1107 · 2015-10-22T17:01:59Z

pkg/kubelet/kubelet.go

@@ -955,8 +958,12 @@ func (kl *Kubelet) syncNodeStatus() {
 	}
 }

-func makeMounts(container *api.Container, podVolumes kubecontainer.VolumeMap) (mounts []kubecontainer.Mount) {
+func (kl *Kubelet) makeMounts(pod *api.Pod, container *api.Container, podVolumes kubecontainer.VolumeMap) ([]kubecontainer.Mount, error) {
+	mountEtcHostsFile := !pod.Spec.SecurityContext.HostNetwork && len(pod.Status.PodIP) > 0


Please comment here: You only create etc_hosts file for containers meeting both requirements:

not using host network. This is pretty straight-forward. But worth a comment to make sure other maintainer won't break this logic in the future.

podIP is available already. This means network infrastructure container's etc_hosts file is different from rest of containers. It should be ok since that is a hidden container, and not used by the users anyway. But it is confusing. I did stair at it for a couple of minutes to figure it out. :-)

dchen1107 · 2015-10-22T17:02:39Z

LGTM overall.Just some nits and comments.

dchen1107 · 2015-10-22T17:24:10Z

cc/ @yifan-gu on rkt side. Now Kubelet takes over container's /etc/hosts file, not sure if it breaks rkt's logic here.

ArtfulCoder · 2015-10-22T20:29:41Z

feedback addressed.
PTAL

yifan-gu · 2015-10-22T20:31:57Z

cc @jonboulle

k8s-bot · 2015-10-22T20:55:05Z

GCE e2e test build/test passed for commit ba6469d.

yifan-gu · 2015-10-22T21:05:44Z

LGTM, shouldn't break rkt

dchen1107 · 2015-10-22T22:45:29Z

LGTM

kubelet manages /etc/hosts file

ArtfulCoder · 2015-10-23T20:15:04Z

waiting on #16174 before cherry-picking

https://github.com/artfulcoder/kubernetes into today

pmorie · 2015-10-28T16:32:10Z

pkg/kubelet/dockertools/manager.go

@@ -312,22 +312,6 @@ type containerStatusResult struct {

 const podIPDownwardAPISelector = "status.podIP"

-// podDependsOnIP returns whether any containers in a pod depend on using the pod IP via


@ArtfulCoder why did this change?

because all pods depend on IP now.

pmorie · 2015-10-29T16:50:40Z

@dchen1107

was the change to the downward API logic part of this PR? i don't understand.

ArtfulCoder · 2015-10-30T21:08:13Z

@pmorie
There is no change in downward API
Kubelet now manages the /etc/hosts file for each pod.
Before this PR, /etc/hosts was managed by docker for each container.

We need the container's hostname and ip so that it can be included in the /etc/hosts file.
As a result, we now need to figure out the ip.

Hope that answers your question.

pmorie · 2015-10-31T15:40:19Z

Thanks for the clarification @thockin

googlebot added the cla: yes label Oct 21, 2015

ArtfulCoder added the sig/network Categorizes an issue or PR as relevant to SIG Network. label Oct 21, 2015

ArtfulCoder assigned dchen1107 Oct 21, 2015

thockin reviewed Oct 21, 2015
View reviewed changes

k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 21, 2015

bgrant0607 reviewed Oct 21, 2015
View reviewed changes

ArtfulCoder force-pushed the mount_etc_hosts branch from 77d170a to c942009 Compare October 21, 2015 21:07

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 21, 2015

ArtfulCoder force-pushed the mount_etc_hosts branch from c942009 to 716ea07 Compare October 21, 2015 22:01

thockin reviewed Oct 21, 2015
View reviewed changes

ArtfulCoder force-pushed the mount_etc_hosts branch from 716ea07 to 5b2f28d Compare October 22, 2015 00:17

thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 22, 2015

ArtfulCoder added the cherrypick-candidate label Oct 22, 2015

dchen1107 reviewed Oct 22, 2015
View reviewed changes

thockin removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 22, 2015

dchen1107 reviewed Oct 22, 2015
View reviewed changes

kubelet manages /etc/hosts file

ba6469d

ArtfulCoder force-pushed the mount_etc_hosts branch from 5b2f28d to ba6469d Compare October 22, 2015 20:27

dchen1107 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 22, 2015

fgrzadkowski added a commit that referenced this pull request Oct 23, 2015

Merge pull request #16052 from ArtfulCoder/mount_etc_hosts

165169a

kubelet manages /etc/hosts file

fgrzadkowski merged commit 165169a into kubernetes:master Oct 23, 2015

dchen1107 mentioned this pull request Oct 23, 2015

Validate docker 1.8.3 #15563

Closed

ArtfulCoder mentioned this pull request Oct 26, 2015

cherrypick for release 1.1 - kubelet manages /etc/hosts file #16291

Closed

ArtfulCoder added a commit to ArtfulCoder/kubernetes that referenced this pull request Oct 26, 2015

Merge branch 'automated-cherry-pick-of-kubernetes#16052-release-1.1' of

c4a2aad

https://github.com/artfulcoder/kubernetes into today

ArtfulCoder mentioned this pull request Oct 26, 2015

kubelet manages /etc/hosts file - cherrypick to release-1.1 #16314

Merged

pmorie reviewed Oct 28, 2015
View reviewed changes

borqosky mentioned this pull request Mar 2, 2016

SPEC: volumes as single files appc/spec#144

Open

eparis removed the cherrypick-candidate label Mar 8, 2016

danwinship mentioned this pull request May 5, 2024

Kubelet should not add IPv6 entries to /etc/hosts on systems where IPv6 is disabled #124686

Open

		@@ -312,22 +312,6 @@ type containerStatusResult struct {

		const podIPDownwardAPISelector = "status.podIP"

		// podDependsOnIP returns whether any containers in a pod depend on using the pod IP via

kubelet manages /etc/hosts file #16052

kubelet manages /etc/hosts file #16052

Conversation

ArtfulCoder commented Oct 21, 2015

ArtfulCoder commented Oct 21, 2015

ArtfulCoder commented Oct 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin commented Oct 21, 2015

k8s-github-robot commented Oct 21, 2015

k8s-bot commented Oct 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ArtfulCoder commented Oct 21, 2015

dchen1107 commented Oct 21, 2015

dchen1107 commented Oct 21, 2015

k8s-github-robot commented Oct 21, 2015

k8s-bot commented Oct 21, 2015

ArtfulCoder commented Oct 21, 2015

Choose a reason for hiding this comment

thockin commented Oct 21, 2015

k8s-bot commented Oct 21, 2015

ArtfulCoder commented Oct 22, 2015

k8s-bot commented Oct 22, 2015

ArtfulCoder commented Oct 22, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dchen1107 commented Oct 22, 2015

dchen1107 commented Oct 22, 2015

ArtfulCoder commented Oct 22, 2015

yifan-gu commented Oct 22, 2015

k8s-bot commented Oct 22, 2015

yifan-gu commented Oct 22, 2015

dchen1107 commented Oct 22, 2015

ArtfulCoder commented Oct 23, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmorie commented Oct 29, 2015

ArtfulCoder commented Oct 30, 2015

pmorie commented Oct 31, 2015