From 9bf7ffd51a587598eba0d8a2023399c769a53d10 Mon Sep 17 00:00:00 2001 From: Crazykev Date: Wed, 24 May 2017 15:38:42 +0800 Subject: [PATCH 1/2] generate and maintain resolv.conf for sandbox Signed-off-by: Crazykev --- pkg/os/os.go | 7 +++++ pkg/os/testing/fake_os.go | 13 ++++++++ pkg/server/container_start.go | 16 ++++++---- pkg/server/container_start_test.go | 12 ++++--- pkg/server/helpers.go | 10 ++++++ pkg/server/sandbox_run.go | 50 +++++++++++++++++++++++++++++- pkg/server/sandbox_run_test.go | 3 +- 7 files changed, 99 insertions(+), 12 deletions(-) diff --git a/pkg/os/os.go b/pkg/os/os.go index 0f695ec72..1caed5eeb 100644 --- a/pkg/os/os.go +++ b/pkg/os/os.go @@ -18,6 +18,7 @@ package os import ( "io" + "io/ioutil" "os" "golang.org/x/net/context" @@ -33,6 +34,7 @@ type OS interface { 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 + WriteFile(filename string, data []byte, perm os.FileMode) error } // RealOS is used to dispatch the real system level operations. @@ -75,3 +77,8 @@ func (RealOS) CopyFile(src, dest string, perm os.FileMode) error { _, err = io.Copy(out, in) return err } + +// WriteFile will call ioutil.WriteFile to write data into a file. +func (RealOS) WriteFile(filename string, data []byte, perm os.FileMode) error { + return ioutil.WriteFile(filename, data, perm) +} diff --git a/pkg/os/testing/fake_os.go b/pkg/os/testing/fake_os.go index 4a3def8f2..10709915a 100644 --- a/pkg/os/testing/fake_os.go +++ b/pkg/os/testing/fake_os.go @@ -36,6 +36,7 @@ type FakeOS struct { OpenFifoFn func(context.Context, string, int, os.FileMode) (io.ReadWriteCloser, error) StatFn func(string) (os.FileInfo, error) CopyFileFn func(string, string, os.FileMode) error + WriteFileFn func(string, []byte, os.FileMode) error errors map[string]error } @@ -142,3 +143,15 @@ func (f *FakeOS) CopyFile(src, dest string, perm os.FileMode) error { } return nil } + +// WriteFile is a fake call that invokes WriteFileFn or just return nil. +func (f *FakeOS) WriteFile(filename string, data []byte, perm os.FileMode) error { + if err := f.getError("WriteFile"); err != nil { + return err + } + + if f.WriteFileFn != nil { + return f.WriteFileFn(filename, data, perm) + } + return nil +} diff --git a/pkg/server/container_start.go b/pkg/server/container_start.go index 458ac87b3..d5ad04fb8 100644 --- a/pkg/server/container_start.go +++ b/pkg/server/container_start.go @@ -292,10 +292,6 @@ func (c *criContainerdService) generateContainerSpec(id string, sandboxPid uint3 // TODO(random-liu): [P2] Add apparmor and seccomp. - // TODO(random-liu): [P1] Bind mount sandbox /dev/shm. - - // TODO(random-liu): [P0] Bind mount sandbox resolv.conf. - return g.Spec(), nil } @@ -307,9 +303,17 @@ func (c *criContainerdService) generateContainerMounts(sandboxRootDir string, co mounts = append(mounts, &runtime.Mount{ ContainerPath: etcHosts, HostPath: getSandboxHosts(sandboxRootDir), - Readonly: securityContext.ReadonlyRootfs, + Readonly: securityContext.GetReadonlyRootfs(), }) - // TODO(random-liu): [P0] Mount sandbox resolv.config. + + // Mount sandbox resolv.config. + // TODO: Need to figure out whether we should always mount it as read-only + mounts = append(mounts, &runtime.Mount{ + ContainerPath: resolvConfPath, + HostPath: getResolvPath(sandboxRootDir), + Readonly: securityContext.GetReadonlyRootfs(), + }) + // TODO(random-liu): [P0] Mount sandbox /dev/shm. return mounts } diff --git a/pkg/server/container_start_test.go b/pkg/server/container_start_test.go index 5fce69496..8d410ecfc 100644 --- a/pkg/server/container_start_test.go +++ b/pkg/server/container_start_test.go @@ -173,22 +173,24 @@ func getStartContainerTestData() (*runtime.ContainerConfig, *runtime.PodSandboxC func TestGeneralContainerSpec(t *testing.T) { testID := "test-id" + testPodID := "test-pod-id" testPid := uint32(1234) config, sandboxConfig, imageConfig, specCheck := getStartContainerTestData() c := newTestCRIContainerdService() - spec, err := c.generateContainerSpec(testID, testPid, config, sandboxConfig, imageConfig, nil) + spec, err := c.generateContainerSpec(testID, testPodID, testPid, config, sandboxConfig, imageConfig, nil) assert.NoError(t, err) specCheck(t, testID, testPid, spec) } func TestContainerSpecTty(t *testing.T) { testID := "test-id" + testPodID := "test-pod-id" testPid := uint32(1234) config, sandboxConfig, imageConfig, specCheck := getStartContainerTestData() c := newTestCRIContainerdService() for _, tty := range []bool{true, false} { config.Tty = tty - spec, err := c.generateContainerSpec(testID, testPid, config, sandboxConfig, imageConfig, nil) + spec, err := c.generateContainerSpec(testID, testPodID, testPid, config, sandboxConfig, imageConfig, nil) assert.NoError(t, err) specCheck(t, testID, testPid, spec) assert.Equal(t, tty, spec.Process.Terminal) @@ -197,12 +199,13 @@ func TestContainerSpecTty(t *testing.T) { func TestContainerSpecReadonlyRootfs(t *testing.T) { testID := "test-id" + testPodID := "test-pod-id" testPid := uint32(1234) config, sandboxConfig, imageConfig, specCheck := getStartContainerTestData() c := newTestCRIContainerdService() for _, readonly := range []bool{true, false} { config.Linux.SecurityContext.ReadonlyRootfs = readonly - spec, err := c.generateContainerSpec(testID, testPid, config, sandboxConfig, imageConfig, nil) + spec, err := c.generateContainerSpec(testID, testPodID, testPid, config, sandboxConfig, imageConfig, nil) assert.NoError(t, err) specCheck(t, testID, testPid, spec) assert.Equal(t, readonly, spec.Root.Readonly) @@ -211,6 +214,7 @@ func TestContainerSpecReadonlyRootfs(t *testing.T) { func TestContainerSpecWithExtraMounts(t *testing.T) { testID := "test-id" + testPodID := "test-pod-id" testPid := uint32(1234) config, sandboxConfig, imageConfig, specCheck := getStartContainerTestData() c := newTestCRIContainerdService() @@ -225,7 +229,7 @@ func TestContainerSpecWithExtraMounts(t *testing.T) { HostPath: "test-host-path-extra", Readonly: true, } - spec, err := c.generateContainerSpec(testID, testPid, config, sandboxConfig, imageConfig, []*runtime.Mount{extraMount}) + spec, err := c.generateContainerSpec(testID, testPodID, testPid, config, sandboxConfig, imageConfig, []*runtime.Mount{extraMount}) assert.NoError(t, err) specCheck(t, testID, testPid, spec) var mounts []runtimespec.Mount diff --git a/pkg/server/helpers.go b/pkg/server/helpers.go index b07beffa3..32d70bf2e 100644 --- a/pkg/server/helpers.go +++ b/pkg/server/helpers.go @@ -69,6 +69,9 @@ const ( sandboxesDir = "sandboxes" // containersDir contains all container root. containersDir = "containers" + // According to http://man7.org/linux/man-pages/man5/resolv.conf.5.html: + // "The search list is currently limited to six domains with a total of 256 characters." + maxDNSSearches = 6 // stdinNamedPipe is the name of stdin named pipe. stdinNamedPipe = "stdin" // stdoutNamedPipe is the name of stdout named pipe. @@ -87,6 +90,8 @@ const ( pidNSFormat = "/proc/%v/ns/pid" // etcHosts is the default path of /etc/hosts file. etcHosts = "/etc/hosts" + // resolvConfPath is the abs path of resolv.conf on host or container. + resolvConfPath = "/etc/resolv.conf" ) // generateID generates a random unique id. @@ -149,6 +154,11 @@ func getSandboxHosts(sandboxRootDir string) string { return filepath.Join(sandboxRootDir, "hosts") } +// getResolvPath returns resolv.conf filepath for specified sandbox. +func getResolvPath(sandboxRoot string) string { + return filepath.Join(sandboxRoot, "resolv.conf") +} + // 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) ( diff --git a/pkg/server/sandbox_run.go b/pkg/server/sandbox_run.go index b79eee59e..8a8a5f23a 100644 --- a/pkg/server/sandbox_run.go +++ b/pkg/server/sandbox_run.go @@ -19,6 +19,7 @@ package server import ( "encoding/json" "fmt" + "strings" "time" "github.com/containerd/containerd/api/services/execution" @@ -303,8 +304,55 @@ func (c *criContainerdService) setupSandboxFiles(rootDir string, config *runtime if err := c.os.CopyFile(etcHosts, sandboxEtcHosts, 0666); err != nil { return fmt.Errorf("failed to generate sandbox hosts file %q: %v", sandboxEtcHosts, err) } - // TODO(random-liu): [P0] Set DNS options. Maintain a resolv.conf for the sandbox. + + // Set DNS options. Maintain a resolv.conf for the sandbox. + var err error + resolvContent := "" + if dnsConfig := config.GetDnsConfig(); dnsConfig != nil { + resolvContent, err = parseDNSOptions(dnsConfig.Servers, dnsConfig.Searches, dnsConfig.Options) + if err != nil { + return fmt.Errorf("failed to parse sandbox DNSConfig %+v: %v", dnsConfig, err) + } + } + resolvPath := getResolvPath(rootDir) + if resolvContent == "" { + // copy host's resolv.conf to resolvPath + err = c.os.CopyFile(resolvConfPath, resolvPath, 0644) + if err != nil { + return fmt.Errorf("failed to copy host's resolv.conf to %q: %v", resolvPath, err) + } + } else { + err = c.os.WriteFile(resolvPath, []byte(resolvContent), 0644) + if err != nil { + return fmt.Errorf("failed to write resolv content to %q: %v", resolvPath, err) + } + } + // TODO(random-liu): [P0] Deal with /dev/shm. Use host for HostIpc, and create and mount for // non-HostIpc. What about mqueue? return nil } + +// parseDNSOptions parse DNS options into resolv.conf format content, +// if none option is specified, will return empty with no error. +func parseDNSOptions(servers, searches, options []string) (string, error) { + resolvContent := "" + + if len(searches) > maxDNSSearches { + return "", fmt.Errorf("DNSOption.Searches has more than 6 domains") + } + + if len(searches) > 0 { + resolvContent += fmt.Sprintf("search %s\n", strings.Join(searches, " ")) + } + + if len(servers) > 0 { + resolvContent += fmt.Sprintf("nameserver %s\n", strings.Join(servers, "\nnameserver ")) + } + + if len(options) > 0 { + resolvContent += fmt.Sprintf("options %s\n", strings.Join(options, " ")) + } + + return resolvContent, nil +} diff --git a/pkg/server/sandbox_run_test.go b/pkg/server/sandbox_run_test.go index f4f81dbbe..acd07724a 100644 --- a/pkg/server/sandbox_run_test.go +++ b/pkg/server/sandbox_run_test.go @@ -161,6 +161,7 @@ func TestSetupSandboxFiles(t *testing.T) { testRootDir := "test-sandbox-root" expectedCopys := [][]interface{}{ {"/etc/hosts", testRootDir + "/hosts", os.FileMode(0666)}, + {"/etc/resolv.conf", testRootDir + "/resolv.conf", os.FileMode(0644)}, } c := newTestCRIContainerdService() var copys [][]interface{} @@ -169,7 +170,7 @@ func TestSetupSandboxFiles(t *testing.T) { return nil } c.setupSandboxFiles(testRootDir, nil) - assert.Equal(t, expectedCopys, copys, "should copy /etc/hosts for sandbox") + assert.Equal(t, expectedCopys, copys, "should copy expected files for sandbox") } func TestRunPodSandbox(t *testing.T) { From 62d1e5dc10a248daba8bda9e677193a131657d46 Mon Sep 17 00:00:00 2001 From: Crazykev Date: Tue, 6 Jun 2017 19:33:28 +0800 Subject: [PATCH 2/2] add unit test Signed-off-by: Crazykev --- pkg/server/container_start_test.go | 50 ++++++++++++++++++------------ pkg/server/sandbox_run_test.go | 43 +++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 20 deletions(-) diff --git a/pkg/server/container_start_test.go b/pkg/server/container_start_test.go index 8d410ecfc..101cfceb7 100644 --- a/pkg/server/container_start_test.go +++ b/pkg/server/container_start_test.go @@ -173,24 +173,22 @@ func getStartContainerTestData() (*runtime.ContainerConfig, *runtime.PodSandboxC func TestGeneralContainerSpec(t *testing.T) { testID := "test-id" - testPodID := "test-pod-id" testPid := uint32(1234) config, sandboxConfig, imageConfig, specCheck := getStartContainerTestData() c := newTestCRIContainerdService() - spec, err := c.generateContainerSpec(testID, testPodID, testPid, config, sandboxConfig, imageConfig, nil) + spec, err := c.generateContainerSpec(testID, testPid, config, sandboxConfig, imageConfig, nil) assert.NoError(t, err) specCheck(t, testID, testPid, spec) } func TestContainerSpecTty(t *testing.T) { testID := "test-id" - testPodID := "test-pod-id" testPid := uint32(1234) config, sandboxConfig, imageConfig, specCheck := getStartContainerTestData() c := newTestCRIContainerdService() for _, tty := range []bool{true, false} { config.Tty = tty - spec, err := c.generateContainerSpec(testID, testPodID, testPid, config, sandboxConfig, imageConfig, nil) + 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) @@ -199,13 +197,12 @@ func TestContainerSpecTty(t *testing.T) { func TestContainerSpecReadonlyRootfs(t *testing.T) { testID := "test-id" - testPodID := "test-pod-id" testPid := uint32(1234) config, sandboxConfig, imageConfig, specCheck := getStartContainerTestData() c := newTestCRIContainerdService() for _, readonly := range []bool{true, false} { config.Linux.SecurityContext.ReadonlyRootfs = readonly - spec, err := c.generateContainerSpec(testID, testPodID, testPid, config, sandboxConfig, imageConfig, nil) + 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) @@ -214,7 +211,6 @@ func TestContainerSpecReadonlyRootfs(t *testing.T) { func TestContainerSpecWithExtraMounts(t *testing.T) { testID := "test-id" - testPodID := "test-pod-id" testPid := uint32(1234) config, sandboxConfig, imageConfig, specCheck := getStartContainerTestData() c := newTestCRIContainerdService() @@ -229,7 +225,7 @@ func TestContainerSpecWithExtraMounts(t *testing.T) { HostPath: "test-host-path-extra", Readonly: true, } - spec, err := c.generateContainerSpec(testID, testPodID, testPid, config, sandboxConfig, imageConfig, []*runtime.Mount{extraMount}) + 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 @@ -313,23 +309,37 @@ func TestGenerateContainerMounts(t *testing.T) { securityContext *runtime.LinuxContainerSecurityContext expectedMounts []*runtime.Mount }{ - "should setup ro /etc/hosts mount when rootfs is read-only": { + "should setup ro mount when rootfs is read-only": { securityContext: &runtime.LinuxContainerSecurityContext{ ReadonlyRootfs: true, }, - expectedMounts: []*runtime.Mount{{ - ContainerPath: "/etc/hosts", - HostPath: testSandboxRootDir + "/hosts", - Readonly: true, - }}, + expectedMounts: []*runtime.Mount{ + { + ContainerPath: "/etc/hosts", + HostPath: testSandboxRootDir + "/hosts", + Readonly: true, + }, + { + ContainerPath: resolvConfPath, + HostPath: testSandboxRootDir + "/resolv.conf", + Readonly: true, + }, + }, }, - "should setup rw /etc/hosts mount when rootfs is read-write": { + "should setup rw mount when rootfs is read-write": { securityContext: &runtime.LinuxContainerSecurityContext{}, - expectedMounts: []*runtime.Mount{{ - ContainerPath: "/etc/hosts", - HostPath: testSandboxRootDir + "/hosts", - Readonly: false, - }}, + expectedMounts: []*runtime.Mount{ + { + ContainerPath: "/etc/hosts", + HostPath: testSandboxRootDir + "/hosts", + Readonly: false, + }, + { + ContainerPath: resolvConfPath, + HostPath: getResolvPath(testSandboxRootDir), + Readonly: false, + }, + }, }, } { config := &runtime.ContainerConfig{ diff --git a/pkg/server/sandbox_run_test.go b/pkg/server/sandbox_run_test.go index acd07724a..c45a4f6c5 100644 --- a/pkg/server/sandbox_run_test.go +++ b/pkg/server/sandbox_run_test.go @@ -270,5 +270,48 @@ func TestRunPodSandbox(t *testing.T) { assert.Equal(t, expectedPluginArgument, pluginArgument, "SetUpPod should be called with correct arguments") } +func TestParseDNSOption(t *testing.T) { + for desc, test := range map[string]struct { + servers []string + searches []string + options []string + expectedContent string + expectErr bool + }{ + "empty dns options should return empty content": {}, + "non-empty dns options should return correct content": { + servers: []string{"8.8.8.8", "server.google.com"}, + searches: []string{"114.114.114.114"}, + options: []string{"timeout:1"}, + expectedContent: `search 114.114.114.114 +nameserver 8.8.8.8 +nameserver server.google.com +options timeout:1 +`, + }, + "should return error if dns search exceeds limit(6)": { + searches: []string{ + "server0.google.com", + "server1.google.com", + "server2.google.com", + "server3.google.com", + "server4.google.com", + "server5.google.com", + "server6.google.com", + }, + expectErr: true, + }, + } { + t.Logf("TestCase %q", desc) + resolvContent, err := parseDNSOptions(test.servers, test.searches, test.options) + if test.expectErr { + assert.Error(t, err) + continue + } + assert.NoError(t, err) + assert.Equal(t, resolvContent, test.expectedContent) + } +} + // TODO(random-liu): [P1] Add unit test for different error cases to make sure // the function cleans up on error properly.