Move "OOM Kill disable" warning to the daemon
Disabling the oom-killer for a container without setting a memory limit
is dangerous, as it can result in the container consuming unlimited memory,
without the kernel being able to kill it. A check for this situation is curently
done in the CLI, but other consumers of the API won't receive this warning.
This patch adds a check for this situation to the daemon, so that all consumers
of the API will receive this warning.
This patch will have one side-effect; docker cli's that also perform this check
client-side will print the warning twice; this can be addressed by disabling
the cli-side check for newer API versions, but will generate a bit of extra
noise when using an older CLI.
With this patch applied (and a cli that does not take the new warning into account);
```
docker create --oom-kill-disable busybox
WARNING: OOM killer is disabled for the container, but no memory limit is set, this can result in the system running out of resources.
669933b9b237fa27da699483b5cf15355a9027050825146587a0e5be0d848adf
docker run --rm --oom-kill-disable busybox
WARNING: Disabling the OOM killer on containers without setting a '-m/--memory' limit may be dangerous.
WARNING: OOM killer is disabled for the container, but no memory limit is set, this can result in the system running out of resources.
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn authored on 2018/12/17 19:23:41Showing 2 changed files
| ... | ... |
@@ -422,7 +422,10 @@ func verifyContainerResources(resources *containertypes.Resources, sysInfo *sysi |
| 422 | 422 |
} |
| 423 | 423 |
resources.OomKillDisable = nil |
| 424 | 424 |
} |
| 425 |
- |
|
| 425 |
+ if resources.OomKillDisable != nil && *resources.OomKillDisable && resources.Memory == 0 {
|
|
| 426 |
+ 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.") |
|
| 427 |
+ logrus.Warn("OOM killer is disabled for the container, but no memory limit is set, this can result in the system running out of resources.")
|
|
| 428 |
+ } |
|
| 426 | 429 |
if resources.PidsLimit != 0 && !sysInfo.PidsLimit {
|
| 427 | 430 |
warnings = append(warnings, "Your kernel does not support pids limit capabilities or the cgroup is not mounted. PIDs limit discarded.") |
| 428 | 431 |
logrus.Warn("Your kernel does not support pids limit capabilities or the cgroup is not mounted. PIDs limit discarded.")
|
| ... | ... |
@@ -11,6 +11,9 @@ import ( |
| 11 | 11 |
containertypes "github.com/docker/docker/api/types/container" |
| 12 | 12 |
"github.com/docker/docker/container" |
| 13 | 13 |
"github.com/docker/docker/daemon/config" |
| 14 |
+ "github.com/docker/docker/pkg/sysinfo" |
|
| 15 |
+ "gotest.tools/assert" |
|
| 16 |
+ is "gotest.tools/assert/cmp" |
|
| 14 | 17 |
) |
| 15 | 18 |
|
| 16 | 19 |
type fakeContainerGetter struct {
|
| ... | ... |
@@ -266,3 +269,110 @@ func TestNetworkOptions(t *testing.T) {
|
| 266 | 266 |
t.Fatal("Expected networkOptions error, got nil")
|
| 267 | 267 |
} |
| 268 | 268 |
} |
| 269 |
+ |
|
| 270 |
+func TestVerifyContainerResources(t *testing.T) {
|
|
| 271 |
+ t.Parallel() |
|
| 272 |
+ var ( |
|
| 273 |
+ no = false |
|
| 274 |
+ yes = true |
|
| 275 |
+ ) |
|
| 276 |
+ |
|
| 277 |
+ withMemoryLimit := func(si *sysinfo.SysInfo) {
|
|
| 278 |
+ si.MemoryLimit = true |
|
| 279 |
+ } |
|
| 280 |
+ withSwapLimit := func(si *sysinfo.SysInfo) {
|
|
| 281 |
+ si.SwapLimit = true |
|
| 282 |
+ } |
|
| 283 |
+ withOomKillDisable := func(si *sysinfo.SysInfo) {
|
|
| 284 |
+ si.OomKillDisable = true |
|
| 285 |
+ } |
|
| 286 |
+ |
|
| 287 |
+ tests := []struct {
|
|
| 288 |
+ name string |
|
| 289 |
+ resources containertypes.Resources |
|
| 290 |
+ sysInfo sysinfo.SysInfo |
|
| 291 |
+ update bool |
|
| 292 |
+ expectedWarnings []string |
|
| 293 |
+ }{
|
|
| 294 |
+ {
|
|
| 295 |
+ name: "no-oom-kill-disable", |
|
| 296 |
+ resources: containertypes.Resources{},
|
|
| 297 |
+ sysInfo: sysInfo(t, withMemoryLimit), |
|
| 298 |
+ expectedWarnings: []string{},
|
|
| 299 |
+ }, |
|
| 300 |
+ {
|
|
| 301 |
+ name: "oom-kill-disable-disabled", |
|
| 302 |
+ resources: containertypes.Resources{
|
|
| 303 |
+ OomKillDisable: &no, |
|
| 304 |
+ }, |
|
| 305 |
+ sysInfo: sysInfo(t, withMemoryLimit), |
|
| 306 |
+ expectedWarnings: []string{},
|
|
| 307 |
+ }, |
|
| 308 |
+ {
|
|
| 309 |
+ name: "oom-kill-disable-not-supported", |
|
| 310 |
+ resources: containertypes.Resources{
|
|
| 311 |
+ OomKillDisable: &yes, |
|
| 312 |
+ }, |
|
| 313 |
+ sysInfo: sysInfo(t, withMemoryLimit), |
|
| 314 |
+ expectedWarnings: []string{
|
|
| 315 |
+ "Your kernel does not support OomKillDisable. OomKillDisable discarded.", |
|
| 316 |
+ }, |
|
| 317 |
+ }, |
|
| 318 |
+ {
|
|
| 319 |
+ name: "oom-kill-disable-without-memory-constraints", |
|
| 320 |
+ resources: containertypes.Resources{
|
|
| 321 |
+ OomKillDisable: &yes, |
|
| 322 |
+ Memory: 0, |
|
| 323 |
+ }, |
|
| 324 |
+ sysInfo: sysInfo(t, withMemoryLimit, withOomKillDisable, withSwapLimit), |
|
| 325 |
+ expectedWarnings: []string{
|
|
| 326 |
+ "OOM killer is disabled for the container, but no memory limit is set, this can result in the system running out of resources.", |
|
| 327 |
+ }, |
|
| 328 |
+ }, |
|
| 329 |
+ {
|
|
| 330 |
+ name: "oom-kill-disable-with-memory-constraints-but-no-memory-limit-support", |
|
| 331 |
+ resources: containertypes.Resources{
|
|
| 332 |
+ OomKillDisable: &yes, |
|
| 333 |
+ Memory: linuxMinMemory, |
|
| 334 |
+ }, |
|
| 335 |
+ sysInfo: sysInfo(t, withOomKillDisable), |
|
| 336 |
+ expectedWarnings: []string{
|
|
| 337 |
+ "Your kernel does not support memory limit capabilities or the cgroup is not mounted. Limitation discarded.", |
|
| 338 |
+ "OOM killer is disabled for the container, but no memory limit is set, this can result in the system running out of resources.", |
|
| 339 |
+ }, |
|
| 340 |
+ }, |
|
| 341 |
+ {
|
|
| 342 |
+ name: "oom-kill-disable-with-memory-constraints", |
|
| 343 |
+ resources: containertypes.Resources{
|
|
| 344 |
+ OomKillDisable: &yes, |
|
| 345 |
+ Memory: linuxMinMemory, |
|
| 346 |
+ }, |
|
| 347 |
+ sysInfo: sysInfo(t, withMemoryLimit, withOomKillDisable, withSwapLimit), |
|
| 348 |
+ expectedWarnings: []string{},
|
|
| 349 |
+ }, |
|
| 350 |
+ } |
|
| 351 |
+ for _, tc := range tests {
|
|
| 352 |
+ t.Run(tc.name, func(t *testing.T) {
|
|
| 353 |
+ t.Parallel() |
|
| 354 |
+ warnings, err := verifyContainerResources(&tc.resources, &tc.sysInfo, tc.update) |
|
| 355 |
+ assert.NilError(t, err) |
|
| 356 |
+ for _, w := range tc.expectedWarnings {
|
|
| 357 |
+ assert.Assert(t, is.Contains(warnings, w)) |
|
| 358 |
+ } |
|
| 359 |
+ }) |
|
| 360 |
+ } |
|
| 361 |
+} |
|
| 362 |
+ |
|
| 363 |
+func sysInfo(t *testing.T, opts ...func(*sysinfo.SysInfo)) sysinfo.SysInfo {
|
|
| 364 |
+ t.Helper() |
|
| 365 |
+ si := sysinfo.SysInfo{}
|
|
| 366 |
+ |
|
| 367 |
+ for _, opt := range opts {
|
|
| 368 |
+ opt(&si) |
|
| 369 |
+ } |
|
| 370 |
+ |
|
| 371 |
+ if si.OomKillDisable {
|
|
| 372 |
+ t.Log(t.Name(), "OOM disable supported") |
|
| 373 |
+ } |
|
| 374 |
+ return si |
|
| 375 |
+} |