From d2e23405bed2f5388b4a6c4f50c94ed2ab580319 Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Wed, 1 Jul 2020 12:04:23 +0200
Subject: [PATCH] Set minimum memory limit to 6M, to account for higher startup
memory use
For some time, we defined a minimum limit for `--memory` limits to account for
overhead during startup, and to supply a reasonable functional container.
Changes in the runtime (runc) introduced a higher memory footprint during container
startup, which now lead to obscure error-messages that are unfriendly for users:
run --rm --memory=4m alpine echo success
docker: Error response from daemon: OCI runtime create failed: container_linux.go:349: starting container process caused "process_linux.go:449: container init caused \"process_linux.go:415: setting cgroup config for procHooks process caused \\\"failed to write \\\\\\\"4194304\\\\\\\" to \\\\\\\"/sys/fs/cgroup/memory/docker/1254c8d63f85442e599b17dff895f4543c897755ee3bd9b56d5d3d17724b38d7/memory.limit_in_bytes\\\\\\\": write /sys/fs/cgroup/memory/docker/1254c8d63f85442e599b17dff895f4543c897755ee3bd9b56d5d3d17724b38d7/memory.limit_in_bytes: device or resource busy\\\"\"": unknown.
ERRO[0000] error waiting for container: context canceled
Containers that fail to start because of this limit, will not be marked as OOMKilled,
which makes it harder for users to find the cause of the failure.
Note that _after_ this memory is only required during startup of the container. After
the container was started, the container may not consume this memory, and limits
could (manually) be lowered, for example, an alpine container running only a shell
can run with 512k of memory;
echo 524288 > /sys/fs/cgroup/memory/docker/acdd326419f0898be63b0463cfc81cd17fb34d2dae6f8aa3768ee6a075ca5c86/memory.limit_in_bytes
However, restarting the container will reset that manual limit to the container's
configuration. While `docker container update` would allow for the updated limit to
be persisted, (re)starting the container after updating produces the same error message
again, so we cannot use different limits for `docker run` / `docker create` and `docker update`.
This patch raises the minimum memory limnit to 6M, so that a better error-message is
produced if a user tries to create a container with a memory-limit that is too low:
docker create --memory=4m alpine echo success
docker: Error response from daemon: Minimum memory limit allowed is 6MB.
Possibly, this constraint could be handled by runc, so that different runtimes
could set a best-matching limit (other runtimes may require less overhead).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
daemon/daemon_unix.go | 6 +++---
integration-cli/docker_api_containers_test.go | 2 +-
integration-cli/docker_cli_run_test.go | 10 ++++------
integration-cli/docker_cli_update_unix_test.go | 2 +-
integration-cli/docker_deprecated_api_v124_test.go | 2 +-
5 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go
index 1a577276ef..58fa4eeb65 100644
--- a/daemon/daemon_unix.go
+++ b/daemon/daemon_unix.go
@@ -68,8 +68,8 @@ const (
linuxMinCPUShares = 2
linuxMaxCPUShares = 262144
platformSupported = true
- // It's not kernel limit, we want this 4M limit to supply a reasonable functional container
- linuxMinMemory = 4194304
+ // It's not kernel limit, we want this 6M limit to account for overhead during startup, and to supply a reasonable functional container
+ linuxMinMemory = 6291456
// constants for remapped root settings
defaultIDSpecifier = "default"
defaultRemappedID = "dockremap"
@@ -433,7 +433,7 @@ func verifyPlatformContainerResources(resources *containertypes.Resources, sysIn
// memory subsystem checks and adjustments
if resources.Memory != 0 && resources.Memory < linuxMinMemory {
- return warnings, fmt.Errorf("Minimum memory limit allowed is 4MB")
+ return warnings, fmt.Errorf("Minimum memory limit allowed is 6MB")
}
if resources.Memory > 0 && !sysInfo.MemoryLimit {
warnings = append(warnings, "Your kernel does not support memory limit capabilities or the cgroup is not mounted. Limitation discarded.")
diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go
index 3a30ea8ab2..88ce5b7b6c 100644
--- a/integration-cli/docker_api_containers_test.go
+++ b/integration-cli/docker_api_containers_test.go
@@ -871,7 +871,7 @@ func (s *DockerSuite) TestCreateWithTooLowMemoryLimit(c *testing.T) {
} else {
assert.Assert(c, res.StatusCode != http.StatusOK)
}
- assert.Assert(c, strings.Contains(string(b), "Minimum memory limit allowed is 4MB"))
+ assert.Assert(c, strings.Contains(string(b), "Minimum memory limit allowed is 6MB"))
}
func (s *DockerSuite) TestContainerAPIRename(c *testing.T) {
diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go
index f1a1dab9fa..f6f0362e1d 100644
--- a/integration-cli/docker_cli_run_test.go
+++ b/integration-cli/docker_cli_run_test.go
@@ -2826,13 +2826,11 @@ func (s *DockerSuite) TestRunPIDHostWithChildIsKillable(c *testing.T) {
}
func (s *DockerSuite) TestRunWithTooSmallMemoryLimit(c *testing.T) {
- // TODO Windows. This may be possible to enable once Windows supports
- // memory limits on containers
+ // TODO Windows. This may be possible to enable once Windows supports memory limits on containers
testRequires(c, DaemonIsLinux)
- // this memory limit is 1 byte less than the min, which is 4MB
- // https://github.com/docker/docker/blob/v1.5.0/daemon/create.go#L22
- out, _, err := dockerCmdWithError("run", "-m", "4194303", "busybox")
- if err == nil || !strings.Contains(out, "Minimum memory limit allowed is 4MB") {
+ // this memory limit is 1 byte less than the min (daemon.linuxMinMemory), which is 6MB (6291456 bytes)
+ out, _, err := dockerCmdWithError("create", "-m", "6291455", "busybox")
+ if err == nil || !strings.Contains(out, "Minimum memory limit allowed is 6MB") {
c.Fatalf("expected run to fail when using too low a memory limit: %q", out)
}
}
diff --git a/integration-cli/docker_cli_update_unix_test.go b/integration-cli/docker_cli_update_unix_test.go
index 8b173ad046..1b3e5089b1 100644
--- a/integration-cli/docker_cli_update_unix_test.go
+++ b/integration-cli/docker_cli_update_unix_test.go
@@ -108,7 +108,7 @@ func (s *DockerSuite) TestUpdateContainerInvalidValue(c *testing.T) {
dockerCmd(c, "run", "-d", "--name", name, "-m", "300M", "busybox", "true")
out, _, err := dockerCmdWithError("update", "-m", "2M", name)
assert.ErrorContains(c, err, "")
- expected := "Minimum memory limit allowed is 4MB"
+ expected := "Minimum memory limit allowed is 6MB"
assert.Assert(c, strings.Contains(out, expected))
}
diff --git a/integration-cli/docker_deprecated_api_v124_test.go b/integration-cli/docker_deprecated_api_v124_test.go
index 943d81b3fd..ea778fdede 100644
--- a/integration-cli/docker_deprecated_api_v124_test.go
+++ b/integration-cli/docker_deprecated_api_v124_test.go
@@ -175,7 +175,7 @@ func (s *DockerSuite) TestDeprecatedStartWithTooLowMemoryLimit(c *testing.T) {
} else {
assert.Equal(c, res.StatusCode, http.StatusBadRequest)
}
- assert.Assert(c, is.Contains(string(b), "Minimum memory limit allowed is 4MB"))
+ assert.Assert(c, is.Contains(string(b), "Minimum memory limit allowed is 6MB"))
}
// #14640