From ffa1728d4bc7c9c662c6090e48d438bc728df5dc Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 24 Feb 2019 15:36:45 +0100 Subject: [PATCH] Normalize values for pids-limit - Don't set `PidsLimit` when creating a container and no limit was set (or the limit was set to "unlimited") - Don't set `PidsLimit` if the host does not have pids-limit support (previously "unlimited" was set). - Do not generate a warning if the host does not have pids-limit support, but pids-limit was set to unlimited (having no limit set, or the limit set to "unlimited" is equivalent, so no warning is nescessary in that case). - When updating a container, convert `0`, and `-1` to "unlimited" (`0`). Signed-off-by: Sebastiaan van Stijn --- .../router/container/container_routes.go | 15 +++++++++++ daemon/daemon_unix.go | 27 ++++++++++--------- integration/container/update_linux_test.go | 4 +++ 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index 5b76d6208d..549d1c1c3a 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -428,6 +428,13 @@ func (s *containerRouter) postContainerUpdate(ctx context.Context, w http.Respon if versions.LessThan(httputils.VersionFromContext(ctx), "1.40") { updateConfig.PidsLimit = nil } + if updateConfig.PidsLimit != nil && *updateConfig.PidsLimit <= 0 { + // Both `0` and `-1` are accepted to set "unlimited" when updating. + // Historically, any negative value was accepted, so treat them as + // "unlimited" as well. + var unlimited int64 + updateConfig.PidsLimit = &unlimited + } hostConfig := &container.HostConfig{ Resources: updateConfig.Resources, @@ -481,6 +488,14 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo hostConfig.Capabilities = nil } + if hostConfig != nil && hostConfig.PidsLimit != nil && *hostConfig.PidsLimit <= 0 { + // Don't set a limit if either no limit was specified, or "unlimited" was + // explicitly set. + // Both `0` and `-1` are accepted as "unlimited", and historically any + // negative value was accepted, so treat those as "unlimited" as well. + hostConfig.PidsLimit = nil + } + ccr, err := s.backend.ContainerCreate(types.ContainerCreateConfig{ Name: name, Config: config, diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index 810f451f76..ec53ab06d4 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -119,16 +119,16 @@ func getMemoryResources(config containertypes.Resources) *specs.LinuxMemory { } func getPidsLimit(config containertypes.Resources) *specs.LinuxPids { - limit := &specs.LinuxPids{} - if config.PidsLimit != nil { - limit.Limit = *config.PidsLimit - if limit.Limit == 0 { - // docker API allows 0 to unset this to be consistent with default values. - // when updating values, runc requires -1 - limit.Limit = -1 - } + if config.PidsLimit == nil { + return nil } - return limit + if *config.PidsLimit <= 0 { + // docker API allows 0 and negative values to unset this to be consistent + // with default values. When updating values, runc requires -1 to unset + // the previous limit. + return &specs.LinuxPids{Limit: -1} + } + return &specs.LinuxPids{Limit: *config.PidsLimit} } func getCPUResources(config containertypes.Resources) (*specs.LinuxCPU, error) { @@ -466,10 +466,11 @@ func verifyPlatformContainerResources(resources *containertypes.Resources, sysIn if resources.OomKillDisable != nil && *resources.OomKillDisable && resources.Memory == 0 { warnings = append(warnings, "OOM killer is disabled for the container, but no memory limit is set, this can result in the system running out of resources.") } - if resources.PidsLimit != nil && *resources.PidsLimit != 0 && !sysInfo.PidsLimit { - warnings = append(warnings, "Your kernel does not support pids limit capabilities or the cgroup is not mounted. PIDs limit discarded.") - var limit int64 - resources.PidsLimit = &limit + if resources.PidsLimit != nil && !sysInfo.PidsLimit { + if *resources.PidsLimit > 0 { + warnings = append(warnings, "Your kernel does not support PIDs limit capabilities or the cgroup is not mounted. PIDs limit discarded.") + } + resources.PidsLimit = nil } // cpu subsystem checks and adjustments diff --git a/integration/container/update_linux_test.go b/integration/container/update_linux_test.go index 1b753e28f9..5efda6d411 100644 --- a/integration/container/update_linux_test.go +++ b/integration/container/update_linux_test.go @@ -132,6 +132,10 @@ func TestUpdatePidsLimit(t *testing.T) { {desc: "update lower", update: intPtr(16), expect: 16, expectCg: "16"}, {desc: "update on old api ignores value", oldAPI: true, update: intPtr(10), expect: 16, expectCg: "16"}, {desc: "unset limit", update: intPtr(0), expect: 0, expectCg: "max"}, + {desc: "reset", update: intPtr(32), expect: 32, expectCg: "32"}, + {desc: "unset limit with minus one", update: intPtr(-1), expect: 0, expectCg: "max"}, + {desc: "reset", update: intPtr(32), expect: 32, expectCg: "32"}, + {desc: "unset limit with minus two", update: intPtr(-2), expect: 0, expectCg: "max"}, } { c := apiClient if test.oldAPI {