Browse code
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 <github@gone.nl>
Sebastiaan van Stijn authored on 2019/02/24 23:36:45Showing 3 changed files
- api/server/router/container/container_routes.go
- daemon/daemon_unix.go
- integration/container/update_linux_test.go
| ... | ... |
@@ -428,6 +428,13 @@ func (s *containerRouter) postContainerUpdate(ctx context.Context, w http.Respon |
| 428 | 428 |
if versions.LessThan(httputils.VersionFromContext(ctx), "1.40") {
|
| 429 | 429 |
updateConfig.PidsLimit = nil |
| 430 | 430 |
} |
| 431 |
+ if updateConfig.PidsLimit != nil && *updateConfig.PidsLimit <= 0 {
|
|
| 432 |
+ // Both `0` and `-1` are accepted to set "unlimited" when updating. |
|
| 433 |
+ // Historically, any negative value was accepted, so treat them as |
|
| 434 |
+ // "unlimited" as well. |
|
| 435 |
+ var unlimited int64 |
|
| 436 |
+ updateConfig.PidsLimit = &unlimited |
|
| 437 |
+ } |
|
| 431 | 438 |
|
| 432 | 439 |
hostConfig := &container.HostConfig{
|
| 433 | 440 |
Resources: updateConfig.Resources, |
| ... | ... |
@@ -481,6 +488,14 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo |
| 481 | 481 |
hostConfig.Capabilities = nil |
| 482 | 482 |
} |
| 483 | 483 |
|
| 484 |
+ if hostConfig != nil && hostConfig.PidsLimit != nil && *hostConfig.PidsLimit <= 0 {
|
|
| 485 |
+ // Don't set a limit if either no limit was specified, or "unlimited" was |
|
| 486 |
+ // explicitly set. |
|
| 487 |
+ // Both `0` and `-1` are accepted as "unlimited", and historically any |
|
| 488 |
+ // negative value was accepted, so treat those as "unlimited" as well. |
|
| 489 |
+ hostConfig.PidsLimit = nil |
|
| 490 |
+ } |
|
| 491 |
+ |
|
| 484 | 492 |
ccr, err := s.backend.ContainerCreate(types.ContainerCreateConfig{
|
| 485 | 493 |
Name: name, |
| 486 | 494 |
Config: config, |
| ... | ... |
@@ -119,16 +119,16 @@ func getMemoryResources(config containertypes.Resources) *specs.LinuxMemory {
|
| 119 | 119 |
} |
| 120 | 120 |
|
| 121 | 121 |
func getPidsLimit(config containertypes.Resources) *specs.LinuxPids {
|
| 122 |
- limit := &specs.LinuxPids{}
|
|
| 123 |
- if config.PidsLimit != nil {
|
|
| 124 |
- limit.Limit = *config.PidsLimit |
|
| 125 |
- if limit.Limit == 0 {
|
|
| 126 |
- // docker API allows 0 to unset this to be consistent with default values. |
|
| 127 |
- // when updating values, runc requires -1 |
|
| 128 |
- limit.Limit = -1 |
|
| 129 |
- } |
|
| 122 |
+ if config.PidsLimit == nil {
|
|
| 123 |
+ return nil |
|
| 130 | 124 |
} |
| 131 |
- return limit |
|
| 125 |
+ if *config.PidsLimit <= 0 {
|
|
| 126 |
+ // docker API allows 0 and negative values to unset this to be consistent |
|
| 127 |
+ // with default values. When updating values, runc requires -1 to unset |
|
| 128 |
+ // the previous limit. |
|
| 129 |
+ return &specs.LinuxPids{Limit: -1}
|
|
| 130 |
+ } |
|
| 131 |
+ return &specs.LinuxPids{Limit: *config.PidsLimit}
|
|
| 132 | 132 |
} |
| 133 | 133 |
|
| 134 | 134 |
func getCPUResources(config containertypes.Resources) (*specs.LinuxCPU, error) {
|
| ... | ... |
@@ -466,10 +466,11 @@ func verifyPlatformContainerResources(resources *containertypes.Resources, sysIn |
| 466 | 466 |
if resources.OomKillDisable != nil && *resources.OomKillDisable && resources.Memory == 0 {
|
| 467 | 467 |
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.") |
| 468 | 468 |
} |
| 469 |
- if resources.PidsLimit != nil && *resources.PidsLimit != 0 && !sysInfo.PidsLimit {
|
|
| 470 |
- warnings = append(warnings, "Your kernel does not support pids limit capabilities or the cgroup is not mounted. PIDs limit discarded.") |
|
| 471 |
- var limit int64 |
|
| 472 |
- resources.PidsLimit = &limit |
|
| 469 |
+ if resources.PidsLimit != nil && !sysInfo.PidsLimit {
|
|
| 470 |
+ if *resources.PidsLimit > 0 {
|
|
| 471 |
+ warnings = append(warnings, "Your kernel does not support PIDs limit capabilities or the cgroup is not mounted. PIDs limit discarded.") |
|
| 472 |
+ } |
|
| 473 |
+ resources.PidsLimit = nil |
|
| 473 | 474 |
} |
| 474 | 475 |
|
| 475 | 476 |
// cpu subsystem checks and adjustments |
| ... | ... |
@@ -132,6 +132,10 @@ func TestUpdatePidsLimit(t *testing.T) {
|
| 132 | 132 |
{desc: "update lower", update: intPtr(16), expect: 16, expectCg: "16"},
|
| 133 | 133 |
{desc: "update on old api ignores value", oldAPI: true, update: intPtr(10), expect: 16, expectCg: "16"},
|
| 134 | 134 |
{desc: "unset limit", update: intPtr(0), expect: 0, expectCg: "max"},
|
| 135 |
+ {desc: "reset", update: intPtr(32), expect: 32, expectCg: "32"},
|
|
| 136 |
+ {desc: "unset limit with minus one", update: intPtr(-1), expect: 0, expectCg: "max"},
|
|
| 137 |
+ {desc: "reset", update: intPtr(32), expect: 32, expectCg: "32"},
|
|
| 138 |
+ {desc: "unset limit with minus two", update: intPtr(-2), expect: 0, expectCg: "max"},
|
|
| 135 | 139 |
} {
|
| 136 | 140 |
c := apiClient |
| 137 | 141 |
if test.oldAPI {
|