Merge pull request #49140 from rickypai/rpai/hostnetwork_etc_hosts

Automatic merge from submit-queue (batch tested with PRs 50094, 48966, 49478, 50593, 49140)

Kubelet manage hosts file for HostNetwork Pods instead of Docker

**What this PR does / why we need it**: Currently, Docker manages the hosts file for containers inside Pods using hostNetwork. It creates discrepancy between how we treat hostNetwork and non-hostNetwork Pods. Kubelet should manage the file regardless of the network setup.

**Which issue this PR fixes**: fixes #48397 more context in https://github.com/kubernetes/kubernetes/issues/43632#issuecomment-304376441

**Special notes for your reviewer**: Because the new logic relies on reading the node filesystem, I'm not sure how to write a proper unit test. I was thinking about using a node e2e test to cover the case, but suggestions are greatly welcomed.

**Release note**:
```release-note
Kubelet now manages `/etc/hosts` file for both hostNetwork Pods and non-hostNetwork Pods.
```

/kind feature
/sig node

@yujuhong @hongchaodeng @thockin
@kubernetes/sig-network-feature-requests @kubernetes/sig-node-feature-requests

pkg/kubelet/kubelet_pods.go

@@ -110,15 +110,16 @@ func (kl *Kubelet) makeDevices(pod *v1.Pod, container *v1.Container) ([]kubecont
 // makeMounts determines the mount points for the given container.
 func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, hostDomain, podIP string, podVolumes kubecontainer.VolumeMap) ([]kubecontainer.Mount, error) {
 	// Kubernetes only mounts on /etc/hosts if:
-	// - container does not use hostNetwork and
 	// - container is not an infrastructure (pause) container
 	// - container is not already mounting on /etc/hosts
-	// When the pause container is being created, its IP is still unknown. Hence, PodIP will not have been set.
+	// - OS is not Windows
-	// OS is not Windows
+	// Kubernetes will not mount /etc/hosts if:
-	mountEtcHostsFile := !pod.Spec.HostNetwork && len(podIP) > 0 && runtime.GOOS != "windows"
+	// - when the Pod sandbox is being created, its IP is still unknown. Hence, PodIP will not have been set.
+	mountEtcHostsFile := len(podIP) > 0 && runtime.GOOS != "windows"
 	glog.V(3).Infof("container: %v/%v/%v podIP: %q creating hosts mount: %v", pod.Namespace, pod.Name, container.Name, podIP, mountEtcHostsFile)
 	mounts := []kubecontainer.Mount{}
 	for _, mount := range container.VolumeMounts {
+		// do not mount /etc/hosts if container is already mounting on the path
 		mountEtcHostsFile = mountEtcHostsFile && (mount.MountPath != etcHostsPath)
 		vol, ok := podVolumes[mount.Name]
 		if !ok || vol.Mounter == nil {
@@ -197,7 +198,7 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h
 	}
 	if mountEtcHostsFile {
 		hostAliases := pod.Spec.HostAliases
-		hostsMount, err := makeHostsMount(podDir, podIP, hostName, hostDomain, hostAliases)
+		hostsMount, err := makeHostsMount(podDir, podIP, hostName, hostDomain, hostAliases, pod.Spec.HostNetwork)
 		if err != nil {
 			return nil, err
 		}
@@ -208,9 +209,9 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h
 
 // makeHostsMount makes the mountpoint for the hosts file that the containers
 // in a pod are injected with.
-func makeHostsMount(podDir, podIP, hostName, hostDomainName string, hostAliases []v1.HostAlias) (*kubecontainer.Mount, error) {
+func makeHostsMount(podDir, podIP, hostName, hostDomainName string, hostAliases []v1.HostAlias, useHostNetwork bool) (*kubecontainer.Mount, error) {
 	hostsFilePath := path.Join(podDir, "etc-hosts")
-	if err := ensureHostsFile(hostsFilePath, podIP, hostName, hostDomainName, hostAliases); err != nil {
+	if err := ensureHostsFile(hostsFilePath, podIP, hostName, hostDomainName, hostAliases, useHostNetwork); err != nil {
 		return nil, err
 	}
 	return &kubecontainer.Mount{
@@ -224,13 +225,34 @@ func makeHostsMount(podDir, podIP, hostName, hostDomainName string, hostAliases
 
 // ensureHostsFile ensures that the given host file has an up-to-date ip, host
 // name, and domain name.
-func ensureHostsFile(fileName, hostIP, hostName, hostDomainName string, hostAliases []v1.HostAlias) error {
+func ensureHostsFile(fileName, hostIP, hostName, hostDomainName string, hostAliases []v1.HostAlias, useHostNetwork bool) error {
-	content := hostsFileContent(hostIP, hostName, hostDomainName, hostAliases)
+	var hostsFileContent []byte
-	return ioutil.WriteFile(fileName, content, 0644)
+	var err error
+
+	if useHostNetwork {
+		// if Pod is using host network, read hosts file from the node's filesystem.
+		// `etcHostsPath` references the location of the hosts file on the node.
+		// `/etc/hosts` for *nix systems.
+		hostsFileContent, err = nodeHostsFileContent(etcHostsPath)
+		if err != nil {
+			return err
+		}
+	} else {
+		// if Pod is not using host network, create a managed hosts file with Pod IP and other information.
+		hostsFileContent = managedHostsFileContent(hostIP, hostName, hostDomainName, hostAliases)
 	}
 
-// hostsFileContent is the content of the managed etc hosts
+	return ioutil.WriteFile(fileName, hostsFileContent, 0644)
-func hostsFileContent(hostIP, hostName, hostDomainName string, hostAliases []v1.HostAlias) []byte {
+}
+
+// nodeHostsFileContent reads the content of node's hosts file.
+func nodeHostsFileContent(hostsFilePath string) ([]byte, error) {
+	return ioutil.ReadFile(hostsFilePath)
+}
+
+// managedHostsFileContent generates the content of the managed etc hosts based on Pod IP and other
+// information.
+func managedHostsFileContent(hostIP, hostName, hostDomainName string, hostAliases []v1.HostAlias) []byte {
 	var buffer bytes.Buffer
 	buffer.WriteString("# Kubernetes-managed hosts file.\n")
 	buffer.WriteString("127.0.0.1\tlocalhost\n")                      // ipv4 localhost

pkg/kubelet/kubelet_pods_test.go

@@ -20,7 +20,10 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"io/ioutil"
 	"net"
+	"os"
+	"path/filepath"
 	"sort"
 	"testing"
 
@@ -179,7 +182,59 @@ func TestMakeMounts(t *testing.T) {
 	}
 }
 
-func TestHostsFileContent(t *testing.T) {
+func TestNodeHostsFileContent(t *testing.T) {
+	testCases := []struct {
+		hostsFileName   string
+		expectedContent string
+	}{
+		{
+			"hosts_test_file1",
+			`# hosts file for testing.
+127.0.0.1	localhost
+::1	localhost ip6-localhost ip6-loopback
+fe00::0	ip6-localnet
+fe00::0	ip6-mcastprefix
+fe00::1	ip6-allnodes
+fe00::2	ip6-allrouters
+123.45.67.89	some.domain
+`,
+		},
+		{
+			"hosts_test_file2",
+			`# another hosts file for testing.
+127.0.0.1	localhost
+::1	localhost ip6-localhost ip6-loopback
+fe00::0	ip6-localnet
+fe00::0	ip6-mcastprefix
+fe00::1	ip6-allnodes
+fe00::2	ip6-allrouters
+12.34.56.78	another.domain
+`,
+		},
+	}
+
+	for _, testCase := range testCases {
+		tmpdir, err := writeHostsFile(testCase.hostsFileName, testCase.expectedContent)
+		require.NoError(t, err, "could not create a temp hosts file")
+		defer os.RemoveAll(tmpdir)
+
+		actualContent, fileReadErr := nodeHostsFileContent(filepath.Join(tmpdir, testCase.hostsFileName))
+		require.NoError(t, fileReadErr, "could not create read hosts file")
+		assert.Equal(t, testCase.expectedContent, string(actualContent), "hosts file content not expected")
+	}
+}
+
+// writeHostsFile will write a hosts file into a temporary dir, and return that dir.
+// Caller is responsible for deleting the dir and its contents.
+func writeHostsFile(filename string, cfg string) (string, error) {
+	tmpdir, err := ioutil.TempDir("", "kubelet=kubelet_pods_test.go=")
+	if err != nil {
+		return "", err
+	}
+	return tmpdir, ioutil.WriteFile(filepath.Join(tmpdir, filename), []byte(cfg), 0644)
+}
+
+func TestManagedHostsFileContent(t *testing.T) {
 	testCases := []struct {
 		hostIP          string
 		hostName        string
@@ -264,8 +319,8 @@ fe00::2	ip6-allrouters
 	}
 
 	for _, testCase := range testCases {
-		actualContent := string(hostsFileContent(testCase.hostIP, testCase.hostName, testCase.hostDomainName, testCase.hostAliases))
+		actualContent := managedHostsFileContent(testCase.hostIP, testCase.hostName, testCase.hostDomainName, testCase.hostAliases)
-		assert.Equal(t, testCase.expectedContent, actualContent, "hosts file content not expected")
+		assert.Equal(t, testCase.expectedContent, string(actualContent), "hosts file content not expected")
 	}
 }