diff --git a/components/engine/daemon/execdriver/lxc/driver.go b/components/engine/daemon/execdriver/lxc/driver.go index 30b36e775d..12793bd01f 100644 --- a/components/engine/daemon/execdriver/lxc/driver.go +++ b/components/engine/daemon/execdriver/lxc/driver.go @@ -324,24 +324,20 @@ func (d *Driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, hooks execd c.ContainerPid = pid - oomKill := false - oomKillNotification, err := notifyOnOOM(cgroupPaths) - if hooks.Start != nil { logrus.Debugf("Invoking startCallback") - hooks.Start(&c.ProcessConfig, pid, oomKillNotification) - + chOOM := make(chan struct{}) + close(chOOM) + hooks.Start(&c.ProcessConfig, pid, chOOM) } + oomKillNotification := notifyChannelOOM(cgroupPaths) + <-waitLock exitCode := getExitCode(c) - if err == nil { - _, oomKill = <-oomKillNotification - logrus.Debugf("oomKill error: %v, waitErr: %v", oomKill, waitErr) - } else { - logrus.Warnf("Your kernel does not support OOM notifications: %s", err) - } + _, oomKill := <-oomKillNotification + logrus.Debugf("oomKill error: %v, waitErr: %v", oomKill, waitErr) // check oom error if oomKill { @@ -351,6 +347,17 @@ func (d *Driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, hooks execd return execdriver.ExitStatus{ExitCode: exitCode, OOMKilled: oomKill}, waitErr } +func notifyChannelOOM(paths map[string]string) <-chan struct{} { + oom, err := notifyOnOOM(paths) + if err != nil { + logrus.Warnf("Your kernel does not support OOM notifications: %s", err) + c := make(chan struct{}) + close(c) + return c + } + return oom +} + // copy from libcontainer func notifyOnOOM(paths map[string]string) (<-chan struct{}, error) { dir := paths["memory"] @@ -386,11 +393,13 @@ func notifyOnOOM(paths map[string]string) (<-chan struct{}, error) { buf := make([]byte, 8) for { if _, err := eventfd.Read(buf); err != nil { + logrus.Warn(err) return } // When a cgroup is destroyed, an event is sent to eventfd. // So if the control path is gone, return instead of notifying. if _, err := os.Lstat(eventControlPath); os.IsNotExist(err) { + logrus.Warn(err) return } ch <- struct{}{} @@ -424,6 +433,11 @@ func cgroupPaths(containerID string) (map[string]string, error) { //unsupported subystem continue } + // if we are running dind + dockerPathIdx := strings.LastIndex(cgroupDir, "docker") + if dockerPathIdx != -1 { + cgroupDir = cgroupDir[:dockerPathIdx-1] + } path := filepath.Join(cgroupRoot, cgroupDir, "lxc", containerID) paths[subsystem] = path } diff --git a/components/engine/daemon/execdriver/lxc/lxc_template.go b/components/engine/daemon/execdriver/lxc/lxc_template.go index 975c5f19e0..6b7996f04c 100644 --- a/components/engine/daemon/execdriver/lxc/lxc_template.go +++ b/components/engine/daemon/execdriver/lxc/lxc_template.go @@ -91,9 +91,9 @@ lxc.mount.entry = {{$value.Source}} {{escapeFstabSpaces $ROOTFS}}/{{escapeFstabS {{if .Resources}} {{if .Resources.Memory}} lxc.cgroup.memory.limit_in_bytes = {{.Resources.Memory}} -{{with $memSwap := getMemorySwap .Resources}} -lxc.cgroup.memory.memsw.limit_in_bytes = {{$memSwap}} {{end}} +{{if gt .Resources.MemorySwap 0}} +lxc.cgroup.memory.memsw.limit_in_bytes = {{.Resources.MemorySwap}} {{end}} {{if gt .Resources.MemoryReservation 0}} lxc.cgroup.memory.soft_limit_in_bytes = {{.Resources.MemoryReservation}} @@ -209,15 +209,6 @@ func isDirectory(source string) string { return "file" } -func getMemorySwap(v *execdriver.Resources) int64 { - // By default, MemorySwap is set to twice the size of RAM. - // If you want to omit MemorySwap, set it to `-1'. - if v.MemorySwap < 0 { - return 0 - } - return v.Memory * 2 -} - func getLabel(c map[string][]string, name string) string { label := c["label"] for _, l := range label { @@ -242,7 +233,6 @@ func getHostname(env []string) string { func init() { var err error funcMap := template.FuncMap{ - "getMemorySwap": getMemorySwap, "escapeFstabSpaces": escapeFstabSpaces, "formatMountLabel": label.FormatMountLabel, "isDirectory": isDirectory, diff --git a/components/engine/daemon/execdriver/lxc/lxc_template_unit_test.go b/components/engine/daemon/execdriver/lxc/lxc_template_unit_test.go index afb5b1eb6b..01bc3eaef3 100644 --- a/components/engine/daemon/execdriver/lxc/lxc_template_unit_test.go +++ b/components/engine/daemon/execdriver/lxc/lxc_template_unit_test.go @@ -34,6 +34,7 @@ func TestLXCConfig(t *testing.T) { memMin = 33554432 memMax = 536870912 mem = memMin + r.Intn(memMax-memMin) + swap = memMax cpuMin = 100 cpuMax = 10000 cpu = cpuMin + r.Intn(cpuMax-cpuMin) @@ -46,8 +47,9 @@ func TestLXCConfig(t *testing.T) { command := &execdriver.Command{ ID: "1", Resources: &execdriver.Resources{ - Memory: int64(mem), - CPUShares: int64(cpu), + Memory: int64(mem), + MemorySwap: int64(swap), + CPUShares: int64(cpu), }, Network: &execdriver.Network{ Mtu: 1500, @@ -63,7 +65,7 @@ func TestLXCConfig(t *testing.T) { fmt.Sprintf("lxc.cgroup.memory.limit_in_bytes = %d", mem)) grepFile(t, p, - fmt.Sprintf("lxc.cgroup.memory.memsw.limit_in_bytes = %d", mem*2)) + fmt.Sprintf("lxc.cgroup.memory.memsw.limit_in_bytes = %d", swap)) } func TestCustomLxcConfig(t *testing.T) { diff --git a/components/engine/daemon/execdriver/native/driver.go b/components/engine/daemon/execdriver/native/driver.go index 09f84a37bb..94f200a311 100644 --- a/components/engine/daemon/execdriver/native/driver.go +++ b/components/engine/daemon/execdriver/native/driver.go @@ -167,7 +167,6 @@ func (d *Driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, hooks execd oom := notifyOnOOM(cont) if hooks.Start != nil { - pid, err := p.Pid() if err != nil { p.Signal(os.Kill) diff --git a/components/engine/integration-cli/check_test.go b/components/engine/integration-cli/check_test.go index 575f8ea71f..030b07f0e5 100644 --- a/components/engine/integration-cli/check_test.go +++ b/components/engine/integration-cli/check_test.go @@ -31,6 +31,7 @@ func (s *DockerSuite) TearDownTest(c *check.C) { deleteAllContainers() deleteAllImages() deleteAllVolumes() + deleteAllNetworks() } func init() { diff --git a/components/engine/integration-cli/docker_cli_diff_test.go b/components/engine/integration-cli/docker_cli_diff_test.go index 60eff132ca..42f1d89fb1 100644 --- a/components/engine/integration-cli/docker_cli_diff_test.go +++ b/components/engine/integration-cli/docker_cli_diff_test.go @@ -61,6 +61,7 @@ func (s *DockerSuite) TestDiffEnsureOnlyKmsgAndPtmx(c *check.C) { "C /dev": true, "A /dev/full": true, // busybox "C /dev/ptmx": true, // libcontainer + "A /dev/mqueue": true, // lxc "A /dev/kmsg": true, // lxc "A /dev/fd": true, "A /dev/fuse": true, diff --git a/components/engine/integration-cli/docker_cli_events_unix_test.go b/components/engine/integration-cli/docker_cli_events_unix_test.go index ca7c092214..9115809b87 100644 --- a/components/engine/integration-cli/docker_cli_events_unix_test.go +++ b/components/engine/integration-cli/docker_cli_events_unix_test.go @@ -56,6 +56,7 @@ func (s *DockerSuite) TestEventsRedirectStdout(c *check.C) { func (s *DockerSuite) TestEventsOOMDisableFalse(c *check.C) { testRequires(c, DaemonIsLinux) + testRequires(c, NativeExecDriver) testRequires(c, oomControl) errChan := make(chan error) @@ -103,6 +104,7 @@ func (s *DockerSuite) TestEventsOOMDisableFalse(c *check.C) { func (s *DockerSuite) TestEventsOOMDisableTrue(c *check.C) { testRequires(c, DaemonIsLinux) + testRequires(c, NativeExecDriver) testRequires(c, oomControl) errChan := make(chan error) diff --git a/components/engine/integration-cli/docker_cli_run_test.go b/components/engine/integration-cli/docker_cli_run_test.go index 0ceb768e95..53361dfdf2 100644 --- a/components/engine/integration-cli/docker_cli_run_test.go +++ b/components/engine/integration-cli/docker_cli_run_test.go @@ -1197,7 +1197,7 @@ func (s *DockerSuite) TestRunNonRootUserResolvName(c *check.C) { // uses the host's /etc/resolv.conf and does not have any dns options provided. func (s *DockerSuite) TestRunResolvconfUpdate(c *check.C) { // Not applicable on Windows as testing unix specific functionality - testRequires(c, SameHostDaemon, DaemonIsLinux) + testRequires(c, SameHostDaemon, DaemonIsLinux, NativeExecDriver) tmpResolvConf := []byte("search pommesfrites.fr\nnameserver 12.34.56.78\n") tmpLocalhostResolvConf := []byte("nameserver 127.0.0.1") @@ -3425,13 +3425,10 @@ func (s *DockerSuite) TestContainersInUserDefinedNetwork(c *check.C) { dockerCmd(c, "run", "-d", "--net=testnetwork", "--name=first", "busybox", "top") c.Assert(waitRun("first"), check.IsNil) dockerCmd(c, "run", "-t", "--net=testnetwork", "--name=second", "busybox", "ping", "-c", "1", "first") - dockerCmd(c, "stop", "first") - dockerCmd(c, "stop", "second") - dockerCmd(c, "network", "rm", "testnetwork") } func (s *DockerSuite) TestContainersInMultipleNetworks(c *check.C) { - testRequires(c, DaemonIsLinux, NotUserNamespace) + testRequires(c, DaemonIsLinux, NotUserNamespace, NativeExecDriver) // Create 2 networks using bridge driver dockerCmd(c, "network", "create", "-d", "bridge", "testnetwork1") dockerCmd(c, "network", "create", "-d", "bridge", "testnetwork2") @@ -3447,14 +3444,10 @@ func (s *DockerSuite) TestContainersInMultipleNetworks(c *check.C) { dockerCmd(c, "network", "connect", "testnetwork2", "second") // Check connectivity between containers dockerCmd(c, "exec", "second", "ping", "-c", "1", "first.testnetwork2") - dockerCmd(c, "stop", "first") - dockerCmd(c, "stop", "second") - dockerCmd(c, "network", "rm", "testnetwork1") - dockerCmd(c, "network", "rm", "testnetwork2") } func (s *DockerSuite) TestContainersNetworkIsolation(c *check.C) { - testRequires(c, DaemonIsLinux, NotUserNamespace) + testRequires(c, DaemonIsLinux, NotUserNamespace, NativeExecDriver) // Create 2 networks using bridge driver dockerCmd(c, "network", "create", "-d", "bridge", "testnetwork1") dockerCmd(c, "network", "create", "-d", "bridge", "testnetwork2") @@ -3478,11 +3471,6 @@ func (s *DockerSuite) TestContainersNetworkIsolation(c *check.C) { // ping must fail again _, _, err = dockerCmdWithError("exec", "first", "ping", "-c", "1", "second") c.Assert(err, check.NotNil) - - dockerCmd(c, "stop", "first") - dockerCmd(c, "stop", "second") - dockerCmd(c, "network", "rm", "testnetwork1") - dockerCmd(c, "network", "rm", "testnetwork2") } func (s *DockerSuite) TestNetworkRmWithActiveContainers(c *check.C) { @@ -3501,17 +3489,14 @@ func (s *DockerSuite) TestNetworkRmWithActiveContainers(c *check.C) { dockerCmd(c, "stop", "first") _, _, err = dockerCmdWithError("network", "rm", "testnetwork1") c.Assert(err, check.NotNil) - - dockerCmd(c, "stop", "second") - // Network delete must succeed after all the connected containers are inactive - dockerCmd(c, "network", "rm", "testnetwork1") } func (s *DockerSuite) TestContainerRestartInMultipleNetworks(c *check.C) { - testRequires(c, DaemonIsLinux, NotUserNamespace) + testRequires(c, DaemonIsLinux, NotUserNamespace, NativeExecDriver) // Create 2 networks using bridge driver dockerCmd(c, "network", "create", "-d", "bridge", "testnetwork1") dockerCmd(c, "network", "create", "-d", "bridge", "testnetwork2") + // Run and connect containers to testnetwork1 dockerCmd(c, "run", "-d", "--net=testnetwork1", "--name=first", "busybox", "top") c.Assert(waitRun("first"), check.IsNil) @@ -3536,11 +3521,6 @@ func (s *DockerSuite) TestContainerRestartInMultipleNetworks(c *check.C) { dockerCmd(c, "start", "second") dockerCmd(c, "exec", "first", "ping", "-c", "1", "second.testnetwork1") dockerCmd(c, "exec", "second", "ping", "-c", "1", "first.testnetwork2") - - dockerCmd(c, "stop", "first") - dockerCmd(c, "stop", "second") - dockerCmd(c, "network", "rm", "testnetwork1") - dockerCmd(c, "network", "rm", "testnetwork2") } func (s *DockerSuite) TestContainerWithConflictingHostNetworks(c *check.C) { @@ -3555,8 +3535,6 @@ func (s *DockerSuite) TestContainerWithConflictingHostNetworks(c *check.C) { // Connecting to the user defined network must fail _, _, err := dockerCmdWithError("network", "connect", "testnetwork1", "first") c.Assert(err, check.NotNil) - dockerCmd(c, "stop", "first") - dockerCmd(c, "network", "rm", "testnetwork1") } func (s *DockerSuite) TestContainerWithConflictingSharedNetwork(c *check.C) { @@ -3574,10 +3552,6 @@ func (s *DockerSuite) TestContainerWithConflictingSharedNetwork(c *check.C) { out, _, err := dockerCmdWithError("network", "connect", "testnetwork1", "second") c.Assert(err, check.NotNil) c.Assert(out, checker.Contains, runconfig.ErrConflictSharedNetwork.Error()) - - dockerCmd(c, "stop", "first") - dockerCmd(c, "stop", "second") - dockerCmd(c, "network", "rm", "testnetwork1") } func (s *DockerSuite) TestContainerWithConflictingNoneNetwork(c *check.C) { @@ -3600,10 +3574,6 @@ func (s *DockerSuite) TestContainerWithConflictingNoneNetwork(c *check.C) { // Connect second container to none network. it must fail as well _, _, err = dockerCmdWithError("network", "connect", "none", "second") c.Assert(err, check.NotNil) - - dockerCmd(c, "stop", "first") - dockerCmd(c, "stop", "second") - dockerCmd(c, "network", "rm", "testnetwork1") } // #11957 - stdin with no tty does not exit if stdin is not closed even though container exited diff --git a/components/engine/integration-cli/docker_cli_run_unix_test.go b/components/engine/integration-cli/docker_cli_run_unix_test.go index 785aad4108..694890271c 100644 --- a/components/engine/integration-cli/docker_cli_run_unix_test.go +++ b/components/engine/integration-cli/docker_cli_run_unix_test.go @@ -17,6 +17,7 @@ import ( "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/parsers" "github.com/docker/docker/pkg/sysinfo" + "github.com/docker/docker/pkg/units" "github.com/go-check/check" "github.com/kr/pty" ) @@ -419,7 +420,7 @@ func (s *DockerSuite) TestRunInvalidCpusetMemsFlagValue(c *check.C) { } func (s *DockerSuite) TestRunInvalidCPUShares(c *check.C) { - testRequires(c, cpuShare) + testRequires(c, cpuShare, NativeExecDriver) out, _, err := dockerCmdWithError("run", "--cpu-shares", "1", "busybox", "echo", "test") c.Assert(err, check.NotNil, check.Commentf(out)) expected := "The minimum allowed cpu-shares is 2" @@ -435,3 +436,22 @@ func (s *DockerSuite) TestRunInvalidCPUShares(c *check.C) { expected = "The maximum allowed cpu-shares is" c.Assert(out, checker.Contains, expected) } + +func (s *DockerSuite) TestRunWithCorrectMemorySwapOnLXC(c *check.C) { + testRequires(c, memoryLimitSupport) + testRequires(c, swapMemorySupport) + testRequires(c, SameHostDaemon) + + out, _ := dockerCmd(c, "run", "-d", "-m", "16m", "--memory-swap", "64m", "busybox", "top") + if _, err := os.Stat("/sys/fs/cgroup/memory/lxc"); err != nil { + c.Skip("Excecution driver must be LXC for this test") + } + id := strings.TrimSpace(out) + memorySwap, err := ioutil.ReadFile(fmt.Sprintf("/sys/fs/cgroup/memory/lxc/%s/memory.memsw.limit_in_bytes", id)) + c.Assert(err, check.IsNil) + cgSwap, err := strconv.ParseInt(strings.TrimSpace(string(memorySwap)), 10, 64) + c.Assert(err, check.IsNil) + swap, err := units.RAMInBytes("64m") + c.Assert(err, check.IsNil) + c.Assert(cgSwap, check.Equals, swap) +} diff --git a/components/engine/integration-cli/docker_utils.go b/components/engine/integration-cli/docker_utils.go index 7ade4706cc..caba24ab49 100644 --- a/components/engine/integration-cli/docker_utils.go +++ b/components/engine/integration-cli/docker_utils.go @@ -506,6 +506,42 @@ func deleteAllContainers() error { return nil } +func deleteAllNetworks() error { + networks, err := getAllNetworks() + if err != nil { + return err + } + var errors []string + for _, n := range networks { + if n.Name != "bridge" { + status, b, err := sockRequest("DELETE", "/networks/"+n.Name, nil) + if err != nil { + errors = append(errors, err.Error()) + continue + } + if status != http.StatusNoContent { + errors = append(errors, fmt.Sprintf("error deleting network %s: %s", n.Name, string(b))) + } + } + } + if len(errors) > 0 { + return fmt.Errorf(strings.Join(errors, "\n")) + } + return nil +} + +func getAllNetworks() ([]types.NetworkResource, error) { + var networks []types.NetworkResource + _, b, err := sockRequest("GET", "/networks", nil) + if err != nil { + return nil, err + } + if err := json.Unmarshal(b, &networks); err != nil { + return nil, err + } + return networks, nil +} + func deleteAllVolumes() error { volumes, err := getAllVolumes() if err != nil {