Merge pull request #60 from Random-Liu/always-add-etc-hosts

Add sandbox /etc/hosts.

pkg/os/os.go

@@ -32,6 +32,7 @@ type OS interface {
 	RemoveAll(path string) error
 	OpenFifo(ctx context.Context, fn string, flag int, perm os.FileMode) (io.ReadWriteCloser, error)
 	Stat(name string) (os.FileInfo, error)
+	CopyFile(src, dest string, perm os.FileMode) error
 }
 
 // RealOS is used to dispatch the real system level operations.
@@ -56,3 +57,21 @@ func (RealOS) OpenFifo(ctx context.Context, fn string, flag int, perm os.FileMod
 func (RealOS) Stat(name string) (os.FileInfo, error) {
 	return os.Stat(name)
 }
+
+// CopyFile copys src file to dest file
+func (RealOS) CopyFile(src, dest string, perm os.FileMode) error {
+	in, err := os.Open(src)
+	if err != nil {
+		return err
+	}
+	defer in.Close()
+
+	out, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, perm)
+	if err != nil {
+		return err
+	}
+	defer out.Close()
+
+	_, err = io.Copy(out, in)
+	return err
+}

pkg/os/testing/fake_os.go

@@ -34,7 +34,8 @@ type FakeOS struct {
 	MkdirAllFn  func(string, os.FileMode) error
 	RemoveAllFn func(string) error
 	OpenFifoFn  func(context.Context, string, int, os.FileMode) (io.ReadWriteCloser, error)
-	StatFn      func(name string) (os.FileInfo, error)
+	StatFn      func(string) (os.FileInfo, error)
+	CopyFileFn  func(string, string, os.FileMode) error
 	errors      map[string]error
 }
 
@@ -118,7 +119,7 @@ func (f *FakeOS) OpenFifo(ctx context.Context, fn string, flag int, perm os.File
 	return nil, nil
 }
 
-// Stat is a fake call that invokes Stat or just return nil.
+// Stat is a fake call that invokes StatFn or just return nil.
 func (f *FakeOS) Stat(name string) (os.FileInfo, error) {
 	if err := f.getError("Stat"); err != nil {
 		return nil, err
@@ -129,3 +130,15 @@ func (f *FakeOS) Stat(name string) (os.FileInfo, error) {
 	}
 	return nil, nil
 }
+
+// CopyFile is a fake call that invokes CopyFileFn or just return nil.
+func (f *FakeOS) CopyFile(src, dest string, perm os.FileMode) error {
+	if err := f.getError("CopyFile"); err != nil {
+		return err
+	}
+
+	if f.CopyFileFn != nil {
+		return f.CopyFileFn(src, dest, perm)
+	}
+	return nil
+}

pkg/server/container_start.go

@@ -22,6 +22,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 
 	"github.com/containerd/containerd/api/services/execution"
@@ -119,7 +120,10 @@ func (c *criContainerdService) startContainer(ctx context.Context, id string, me
 	if err != nil {
 		return fmt.Errorf("failed to get container image %q: %v", meta.ImageRef, err)
 	}
-	spec, err := c.generateContainerSpec(id, sandboxPid, config, sandboxConfig, imageMeta.Config)
+
+	mounts := c.generateContainerMounts(getSandboxRootDir(c.rootDir, sandboxID), config)
+
+	spec, err := c.generateContainerSpec(id, sandboxPid, config, sandboxConfig, imageMeta.Config, mounts)
 	if err != nil {
 		return fmt.Errorf("failed to generate container %q spec: %v", id, err)
 	}
@@ -218,7 +222,7 @@ func (c *criContainerdService) startContainer(ctx context.Context, id string, me
 }
 
 func (c *criContainerdService) generateContainerSpec(id string, sandboxPid uint32, config *runtime.ContainerConfig,
-	sandboxConfig *runtime.PodSandboxConfig, imageConfig *imagespec.ImageConfig) (*runtimespec.Spec, error) {
+	sandboxConfig *runtime.PodSandboxConfig, imageConfig *imagespec.ImageConfig, extraMounts []*runtime.Mount) (*runtimespec.Spec, error) {
 	// Creates a spec Generator with the default spec.
 	// TODO(random-liu): [P2] Move container runtime spec generation into a helper function.
 	g := generate.New()
@@ -246,7 +250,8 @@ func (c *criContainerdService) generateContainerSpec(id string, sandboxPid uint3
 		g.AddProcessEnv(e.GetKey(), e.GetValue())
 	}
 
-	addOCIBindMounts(&g, config.GetMounts())
+	// Add extra mounts first so that CRI specified mounts can override.
+	addOCIBindMounts(&g, append(extraMounts, config.GetMounts()...))
 
 	// TODO(random-liu): [P1] Set device mapping.
 	// Ref https://github.com/moby/moby/blob/master/oci/devices_linux.go.
@@ -294,6 +299,21 @@ func (c *criContainerdService) generateContainerSpec(id string, sandboxPid uint3
 	return g.Spec(), nil
 }
 
+// generateContainerMounts sets up necessary container mounts including /dev/shm, /etc/hosts
+// and /etc/resolv.conf.
+func (c *criContainerdService) generateContainerMounts(sandboxRootDir string, config *runtime.ContainerConfig) []*runtime.Mount {
+	var mounts []*runtime.Mount
+	securityContext := config.GetLinux().GetSecurityContext()
+	mounts = append(mounts, &runtime.Mount{
+		ContainerPath: etcHosts,
+		HostPath:      getSandboxHosts(sandboxRootDir),
+		Readonly:      securityContext.ReadonlyRootfs,
+	})
+	// TODO(random-liu): [P0] Mount sandbox resolv.config.
+	// TODO(random-liu): [P0] Mount sandbox /dev/shm.
+	return mounts
+}
+
 // setOCIProcessArgs sets process args. It returns error if the final arg list
 // is empty.
 func setOCIProcessArgs(g *generate.Generator, config *runtime.ContainerConfig, imageConfig *imagespec.ImageConfig) error {
@@ -315,6 +335,19 @@ func setOCIProcessArgs(g *generate.Generator, config *runtime.ContainerConfig, i
 	return nil
 }
 
+// addImageEnvs adds environment variables from image config. It returns error if
+// an invalid environment variable is encountered.
+func addImageEnvs(g *generate.Generator, imageEnvs []string) error {
+	for _, e := range imageEnvs {
+		kv := strings.Split(e, "=")
+		if len(kv) != 2 {
+			return fmt.Errorf("invalid environment variable %q", e)
+		}
+		g.AddProcessEnv(kv[0], kv[1])
+	}
+	return nil
+}
+
 // addOCIBindMounts adds bind mounts.
 func addOCIBindMounts(g *generate.Generator, mounts []*runtime.Mount) {
 	for _, mount := range mounts {

pkg/server/container_start_test.go

@@ -176,7 +176,7 @@ func TestGeneralContainerSpec(t *testing.T) {
 	testPid := uint32(1234)
 	config, sandboxConfig, imageConfig, specCheck := getStartContainerTestData()
 	c := newTestCRIContainerdService()
-	spec, err := c.generateContainerSpec(testID, testPid, config, sandboxConfig, imageConfig)
+	spec, err := c.generateContainerSpec(testID, testPid, config, sandboxConfig, imageConfig, nil)
 	assert.NoError(t, err)
 	specCheck(t, testID, testPid, spec)
 }
@@ -188,7 +188,7 @@ func TestContainerSpecTty(t *testing.T) {
 	c := newTestCRIContainerdService()
 	for _, tty := range []bool{true, false} {
 		config.Tty = tty
-		spec, err := c.generateContainerSpec(testID, testPid, config, sandboxConfig, imageConfig)
+		spec, err := c.generateContainerSpec(testID, testPid, config, sandboxConfig, imageConfig, nil)
 		assert.NoError(t, err)
 		specCheck(t, testID, testPid, spec)
 		assert.Equal(t, tty, spec.Process.Terminal)
@@ -202,13 +202,46 @@ func TestContainerSpecReadonlyRootfs(t *testing.T) {
 	c := newTestCRIContainerdService()
 	for _, readonly := range []bool{true, false} {
 		config.Linux.SecurityContext.ReadonlyRootfs = readonly
-		spec, err := c.generateContainerSpec(testID, testPid, config, sandboxConfig, imageConfig)
+		spec, err := c.generateContainerSpec(testID, testPid, config, sandboxConfig, imageConfig, nil)
 		assert.NoError(t, err)
 		specCheck(t, testID, testPid, spec)
 		assert.Equal(t, readonly, spec.Root.Readonly)
 	}
 }
 
+func TestContainerSpecWithExtraMounts(t *testing.T) {
+	testID := "test-id"
+	testPid := uint32(1234)
+	config, sandboxConfig, imageConfig, specCheck := getStartContainerTestData()
+	c := newTestCRIContainerdService()
+	mountInConfig := &runtime.Mount{
+		ContainerPath: "test-container-path",
+		HostPath:      "test-host-path",
+		Readonly:      false,
+	}
+	config.Mounts = append(config.Mounts, mountInConfig)
+	extraMount := &runtime.Mount{
+		ContainerPath: "test-container-path",
+		HostPath:      "test-host-path-extra",
+		Readonly:      true,
+	}
+	spec, err := c.generateContainerSpec(testID, testPid, config, sandboxConfig, imageConfig, []*runtime.Mount{extraMount})
+	assert.NoError(t, err)
+	specCheck(t, testID, testPid, spec)
+	var mounts []runtimespec.Mount
+	for _, m := range spec.Mounts {
+		if m.Destination == "test-container-path" {
+			mounts = append(mounts, m)
+		}
+	}
+	t.Logf("Extra mounts should come first")
+	require.Len(t, mounts, 2)
+	assert.Equal(t, "test-host-path-extra", mounts[0].Source)
+	assert.Contains(t, mounts[0].Options, "ro")
+	assert.Equal(t, "test-host-path", mounts[1].Source)
+	assert.Contains(t, mounts[1].Options, "rw")
+}
+
 func TestContainerSpecCommand(t *testing.T) {
 	for desc, test := range map[string]struct {
 		criEntrypoint   []string
@@ -270,6 +303,46 @@ func TestContainerSpecCommand(t *testing.T) {
 	}
 }
 
+func TestGenerateContainerMounts(t *testing.T) {
+	testSandboxRootDir := "test-sandbox-root"
+	for desc, test := range map[string]struct {
+		securityContext *runtime.LinuxContainerSecurityContext
+		expectedMounts  []*runtime.Mount
+	}{
+		"should setup ro /etc/hosts mount when rootfs is read-only": {
+			securityContext: &runtime.LinuxContainerSecurityContext{
+				ReadonlyRootfs: true,
+			},
+			expectedMounts: []*runtime.Mount{{
+				ContainerPath: "/etc/hosts",
+				HostPath:      testSandboxRootDir + "/hosts",
+				Readonly:      true,
+			}},
+		},
+		"should setup rw /etc/hosts mount when rootfs is read-write": {
+			securityContext: &runtime.LinuxContainerSecurityContext{},
+			expectedMounts: []*runtime.Mount{{
+				ContainerPath: "/etc/hosts",
+				HostPath:      testSandboxRootDir + "/hosts",
+				Readonly:      false,
+			}},
+		},
+	} {
+		config := &runtime.ContainerConfig{
+			Metadata: &runtime.ContainerMetadata{
+				Name:    "test-name",
+				Attempt: 1,
+			},
+			Linux: &runtime.LinuxContainerConfig{
+				SecurityContext: test.securityContext,
+			},
+		}
+		c := newTestCRIContainerdService()
+		mounts := c.generateContainerMounts(testSandboxRootDir, config)
+		assert.Equal(t, test.expectedMounts, mounts, desc)
+	}
+}
+
 func TestStartContainer(t *testing.T) {
 	testID := "test-id"
 	testSandboxID := "test-sandbox-id"

pkg/server/helpers.go

@@ -85,6 +85,8 @@ const (
 	utsNSFormat = "/proc/%v/ns/uts"
 	// pidNSFormat is the format of pid namespace of a process.
 	pidNSFormat = "/proc/%v/ns/pid"
+	// etcHosts is the default path of /etc/hosts file.
+	etcHosts = "/etc/hosts"
 )
 
 // generateID generates a random unique id.
@@ -133,7 +135,8 @@ func getContainerRootDir(rootDir, id string) string {
 	return filepath.Join(rootDir, containersDir, id)
 }
 
-// getStreamingPipes returns the stdin/stdout/stderr pipes path in the root.
+// getStreamingPipes returns the stdin/stdout/stderr pipes path in the
+// container/sandbox root.
 func getStreamingPipes(rootDir string) (string, string, string) {
 	stdin := filepath.Join(rootDir, stdinNamedPipe)
 	stdout := filepath.Join(rootDir, stdoutNamedPipe)
@@ -141,6 +144,11 @@ func getStreamingPipes(rootDir string) (string, string, string) {
 	return stdin, stdout, stderr
 }
 
+// getSandboxHosts returns the hosts file path inside the sandbox root directory.
+func getSandboxHosts(sandboxRootDir string) string {
+	return filepath.Join(sandboxRootDir, "hosts")
+}
+
 // prepareStreamingPipes prepares stream named pipe for container. returns nil
 // streaming handler if corresponding stream path is empty.
 func (c *criContainerdService) prepareStreamingPipes(ctx context.Context, stdin, stdout, stderr string) (

pkg/server/sandbox_run.go

@@ -19,7 +19,6 @@ package server
 import (
 	"encoding/json"
 	"fmt"
-	"strings"
 	"time"
 
 	"github.com/containerd/containerd/api/services/execution"
@@ -133,6 +132,13 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run
 		return nil, fmt.Errorf("failed to start sandbox stderr logger: %v", err)
 	}
 
+	// Setup sandbox /dev/shm, /etc/hosts and /etc/resolv.conf.
+	if err = c.setupSandboxFiles(sandboxRootDir, config); err != nil {
+		return nil, fmt.Errorf("failed to setup sandbox files: %v", err)
+	}
+	// No need to cleanup on error, because the whole sandbox root directory will be removed
+	// on error.
+
 	// Start sandbox container.
 	spec, err := c.generateSandboxContainerSpec(id, config, imageMeta.Config)
 	if err != nil {
@@ -239,8 +245,6 @@ func (c *criContainerdService) generateSandboxContainerSpec(id string, config *r
 	// Set hostname.
 	g.SetHostname(config.GetHostname())
 
-	// TODO(random-liu): [P0] Set DNS options. Maintain a resolv.conf for the sandbox.
-
 	// TODO(random-liu): [P0] Add NamespaceGetter and PortMappingGetter to initialize network plugin.
 
 	// TODO(random-liu): [P0] Add annotation to identify the container is managed by cri-containerd.
@@ -270,8 +274,6 @@ func (c *criContainerdService) generateSandboxContainerSpec(id string, config *r
 		g.RemoveLinuxNamespace(string(runtimespec.PIDNamespace)) // nolint: errcheck
 	}
 
-	// TODO(random-liu): [P0] Deal with /dev/shm. Use host for HostIpc, and create and mount for
-	// non-HostIpc. What about mqueue?
 	if nsOptions.GetHostIpc() {
 		g.RemoveLinuxNamespace(string(runtimespec.IPCNamespace)) // nolint: errcheck
 	}
@@ -293,15 +295,16 @@ func (c *criContainerdService) generateSandboxContainerSpec(id string, config *r
 	return g.Spec(), nil
 }
 
-// addImageEnvs adds environment variables from image config. It returns error if
+// setupSandboxFiles sets up necessary sandbox files including /dev/shm, /etc/hosts
-// an invalid environment variable is encountered.
+// and /etc/resolv.conf.
-func addImageEnvs(g *generate.Generator, imageEnvs []string) error {
+func (c *criContainerdService) setupSandboxFiles(rootDir string, config *runtime.PodSandboxConfig) error {
-	for _, e := range imageEnvs {
+	// TODO(random-liu): Consider whether we should maintain /etc/hosts and /etc/resolv.conf in kubelet.
-		kv := strings.Split(e, "=")
+	sandboxEtcHosts := getSandboxHosts(rootDir)
-		if len(kv) != 2 {
+	if err := c.os.CopyFile(etcHosts, sandboxEtcHosts, 0666); err != nil {
-			return fmt.Errorf("invalid environment variable %q", e)
+		return fmt.Errorf("failed to generate sandbox hosts file %q: %v", sandboxEtcHosts, err)
-		}
-		g.AddProcessEnv(kv[0], kv[1])
 	}
+	// TODO(random-liu): [P0] Set DNS options. Maintain a resolv.conf for the sandbox.
+	// TODO(random-liu): [P0] Deal with /dev/shm. Use host for HostIpc, and create and mount for
+	// non-HostIpc. What about mqueue?
 	return nil
 }

pkg/server/sandbox_run_test.go

@@ -157,6 +157,21 @@ func TestGenerateSandboxContainerSpec(t *testing.T) {
 	}
 }
 
+func TestSetupSandboxFiles(t *testing.T) {
+	testRootDir := "test-sandbox-root"
+	expectedCopys := [][]interface{}{
+		{"/etc/hosts", testRootDir + "/hosts", os.FileMode(0666)},
+	}
+	c := newTestCRIContainerdService()
+	var copys [][]interface{}
+	c.os.(*ostesting.FakeOS).CopyFileFn = func(src string, dest string, perm os.FileMode) error {
+		copys = append(copys, []interface{}{src, dest, perm})
+		return nil
+	}
+	c.setupSandboxFiles(testRootDir, nil)
+	assert.Equal(t, expectedCopys, copys, "should copy /etc/hosts for sandbox")
+}
+
 func TestRunPodSandbox(t *testing.T) {
 	config, imageConfig, specCheck := getRunPodSandboxTestData()
 	c := newTestCRIContainerdService()