Merge pull request moby#25105 from hqhq/fix_kmem_test

Fix TestUpdateKernelMemoryUninitialized on new kernel version

Author: thaJeztah

daemon/daemon_unix.go

@@ -194,17 +194,6 @@ func getBlkioThrottleDevices(devs []*blkiodev.ThrottleDevice) ([]specs.ThrottleD
 	return throttleDevices, nil
 }
 
-func checkKernelVersion(k, major, minor int) bool {
-	if v, err := kernel.GetKernelVersion(); err != nil {
-		logrus.Warnf("error getting kernel version: %s", err)
-	} else {
-		if kernel.CompareKernelVersion(*v, kernel.VersionInfo{Kernel: k, Major: major, Minor: minor}) < 0 {
-			return false
-		}
-	}
-	return true
-}
-
 func checkKernel() error {
 	// Check for unsupported kernel versions
 	// FIXME: it would be cleaner to not test for specific versions, but rather
@@ -215,7 +204,7 @@ func checkKernel() error {
 	// For details see https://github.com/docker/docker/issues/407
 	// Docker 1.11 and above doesn't actually run on kernels older than 3.4,
 	// due to containerd-shim usage of PR_SET_CHILD_SUBREAPER (introduced in 3.4).
-	if !checkKernelVersion(3, 10, 0) {
+	if !kernel.CheckKernelVersion(3, 10, 0) {
 		v, _ := kernel.GetKernelVersion()
 		if os.Getenv("DOCKER_NOWARN_KERNEL_VERSION") == "" {
 			logrus.Fatalf("Your Linux kernel version %s is not supported for running docker. Please upgrade your kernel to 3.10.0 or newer.", v.String())
@@ -317,7 +306,7 @@ func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysi
 	if resources.KernelMemory > 0 && resources.KernelMemory < linuxMinMemory {
 		return warnings, fmt.Errorf("Minimum kernel memory limit allowed is 4MB")
 	}
-	if resources.KernelMemory > 0 && !checkKernelVersion(4, 0, 0) {
+	if resources.KernelMemory > 0 && !kernel.CheckKernelVersion(4, 0, 0) {
 		warnings = append(warnings, "You specified a kernel memory limit on a kernel older than 4.0. Kernel memory limits are experimental on older kernels, it won't work as expected and can cause your system to be unstable.")
 		logrus.Warn("You specified a kernel memory limit on a kernel older than 4.0. Kernel memory limits are experimental on older kernels, it won't work as expected and can cause your system to be unstable.")
 	}

docs/reference/commandline/update.md

@@ -37,9 +37,9 @@ limits on a single container or on many. To specify more than one container,
 provide space-separated list of container names or IDs.
 
 With the exception of the `--kernel-memory` option, you can specify these
-options on a running or a stopped container. You can only update
-`--kernel-memory` on a stopped container or on a running container with
-kernel memory initialized.
+options on a running or a stopped container. On kernel version older than
+4.6, you can only update `--kernel-memory` on a stopped container or on
+a running container with kernel memory initialized.
 
 ## EXAMPLES
 
@@ -66,9 +66,10 @@ $ docker update --cpu-shares 512 -m 300M abebf7571666 hopeful_morse
 ### Update a container's kernel memory constraints
 
 You can update a container's kernel memory limit using the `--kernel-memory`
-option. This option can be updated on a running container only if the container
-was started with `--kernel-memory`. If the container was started *without*
-`--kernel-memory` you need to stop the container before updating kernel memory.
+option. On kernel version older than 4.6, this option can be updated on a
+running container only if the container was started with `--kernel-memory`.
+If the container was started *without* `--kernel-memory` you need to stop
+the container before updating kernel memory.
 
 For example, if you started a container with this command:
 
@@ -92,6 +93,8 @@ Update kernel memory of running container `test2` will fail. You need to stop
 the container before updating the `--kernel-memory` setting. The next time you
 start it, the container uses the new value.
 
+Kernel version newer than (include) 4.6 does not have this limitation, you
+can use `--kernel-memory` the same way as other options.
 
 ### Update a container's restart policy
 

integration-cli/docker_cli_update_unix_test.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/docker/docker/pkg/integration/checker"
+	"github.com/docker/docker/pkg/parsers/kernel"
 	"github.com/docker/engine-api/types"
 	"github.com/go-check/check"
 )
@@ -132,26 +133,36 @@ func (s *DockerSuite) TestUpdateKernelMemory(c *check.C) {
 func (s *DockerSuite) TestUpdateKernelMemoryUninitialized(c *check.C) {
 	testRequires(c, DaemonIsLinux, kernelMemorySupport)
 
+	isNewKernel := kernel.CheckKernelVersion(4, 6, 0)
 	name := "test-update-container"
 	dockerCmd(c, "run", "-d", "--name", name, "busybox", "top")
 	_, _, err := dockerCmdWithError("update", "--kernel-memory", "100M", name)
-	// Update kernel memory to a running container without kernel memory initialized is not allowed.
-	c.Assert(err, check.NotNil)
+	// Update kernel memory to a running container without kernel memory initialized
+	// is not allowed before kernel version 4.6.
+	if !isNewKernel {
+		c.Assert(err, check.NotNil)
+	} else {
+		c.Assert(err, check.IsNil)
+	}
 
 	dockerCmd(c, "pause", name)
-	_, _, err = dockerCmdWithError("update", "--kernel-memory", "100M", name)
-	c.Assert(err, check.NotNil)
+	_, _, err = dockerCmdWithError("update", "--kernel-memory", "200M", name)
+	if !isNewKernel {
+		c.Assert(err, check.NotNil)
+	} else {
+		c.Assert(err, check.IsNil)
+	}
 	dockerCmd(c, "unpause", name)
 
 	dockerCmd(c, "stop", name)
-	dockerCmd(c, "update", "--kernel-memory", "100M", name)
+	dockerCmd(c, "update", "--kernel-memory", "300M", name)
 	dockerCmd(c, "start", name)
 
-	c.Assert(inspectField(c, name, "HostConfig.KernelMemory"), checker.Equals, "104857600")
+	c.Assert(inspectField(c, name, "HostConfig.KernelMemory"), checker.Equals, "314572800")
 
 	file := "/sys/fs/cgroup/memory/memory.kmem.limit_in_bytes"
 	out, _ := dockerCmd(c, "exec", name, "cat", file)
-	c.Assert(strings.TrimSpace(out), checker.Equals, "104857600")
+	c.Assert(strings.TrimSpace(out), checker.Equals, "314572800")
 }
 
 func (s *DockerSuite) TestUpdateSwapMemoryOnly(c *check.C) {

man/docker-update.1.md

@@ -29,9 +29,9 @@ limits on a single container or on many. To specify more than one container,
 provide space-separated list of container names or IDs.
 
 With the exception of the **--kernel-memory** option, you can specify these
-options on a running or a stopped container. You can only update
-**--kernel-memory** on a stopped container or on a running container with
-kernel memory initialized.
+options on a running or a stopped container. On kernel version older than
+4.6, You can only update **--kernel-memory** on a stopped container or on
+a running container with kernel memory initialized.
 
 # OPTIONS
 
@@ -59,9 +59,10 @@ kernel memory initialized.
 **--kernel-memory**=""
    Kernel memory limit (format: `<number>[<unit>]`, where unit = b, k, m or g)
 
-   Note that you can not update kernel memory on a running container if the container
-   is started without kernel memory initialized, in this case, it can only be updated
-   after it's stopped. The new setting takes effect when the container is started.
+   Note that on kernel version older than 4.6, you can not update kernel memory on
+   a running container if the container is started without kernel memory initialized,
+   in this case, it can only be updated after it's stopped. The new setting takes
+   effect when the container is started.
 
 **-m**, **--memory**=""
    Memory limit (format: <number><optional unit>, where unit = b, k, m or g)
@@ -100,9 +101,10 @@ $ docker update --cpu-shares 512 -m 300M abebf7571666 hopeful_morse
 ### Update a container's kernel memory constraints
 
 You can update a container's kernel memory limit using the **--kernel-memory**
-option. This option can be updated on a running container only if the container
-was started with **--kernel-memory**. If the container was started *without*
-**--kernel-memory** you need to stop the container before updating kernel memory.
+option. On kernel version older than 4.6, this option can be updated on a
+running container only if the container was started with **--kernel-memory**.
+If the container was started *without* **--kernel-memory** you need to stop
+the container before updating kernel memory.
 
 For example, if you started a container with this command:
 
@@ -126,6 +128,9 @@ Update kernel memory of running container `test2` will fail. You need to stop
 the container before updating the **--kernel-memory** setting. The next time you
 start it, the container uses the new value.
 
+Kernel version newer than (include) 4.6 does not have this limitation, you
+can use `--kernel-memory` the same way as other options.
+
 ### Update a container's restart policy
 
 You can change a container's restart policy on a running container. The new

pkg/parsers/kernel/kernel_unix.go

@@ -6,6 +6,8 @@ package kernel
 
 import (
 	"bytes"
+
+	"github.com/Sirupsen/logrus"
 )
 
 // GetKernelVersion gets the current kernel version.
@@ -28,3 +30,16 @@ func GetKernelVersion() (*VersionInfo, error) {
 
 	return ParseRelease(string(release))
 }
+
+// CheckKernelVersion checks if current kernel is newer than (or equal to)
+// the given version.
+func CheckKernelVersion(k, major, minor int) bool {
+	if v, err := GetKernelVersion(); err != nil {
+		logrus.Warnf("error getting kernel version: %s", err)
+	} else {
+		if CompareKernelVersion(*v, VersionInfo{Kernel: k, Major: major, Minor: minor}) < 0 {
+			return false
+		}
+	}
+	return true
+}