Skip to content

Commit 58a3569

Browse files
committed
remove duplicate --oom-kill-disable warnings on docker run / docker create
This warning was originally added in [moby@3aa70c1], and moved to be printed on both `run` and `create` in commit 7c514a3. However, [moby@57f1305] (docker 19.03, API 1.40) moved such warnings to the daemon side. The patch mentioned this issue: > 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. The CLI does not take this into account currently, and still prints warnings twice; even in cases where the option is not supported by the daemon, and discarded: On a host without OomKillDisable support: docker create --oom-kill-disable alpine WARNING: Disabling the OOM killer on containers without setting a '-m/--memory' limit may be dangerous. WARNING: Your kernel does not support OomKillDisable. OomKillDisable discarded. On a host that supports it: docker create --oom-kill-disable alpine 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. This patch removes the client-side warning, leaving it to the daemon to report if any warnings should produced (and the client to print them). With this patch applied: On a host without OomKillDisable support: docker create --oom-kill-disable alpine WARNING: Your kernel does not support OomKillDisable. OomKillDisable discarded. On a host that supports it: docker create --oom-kill-disable alpine WARNING: OOM killer is disabled for the container, but no memory limit is set, this can result in the system running out of resources. [moby@3aa70c1]: moby/moby@3aa70c1 [moby@57f1305]: moby/moby@57f1305 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
1 parent 2eec746 commit 58a3569

6 files changed

Lines changed: 16 additions & 29 deletions

cli/command/container/create.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,6 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c
207207
hostConfig := containerCfg.HostConfig
208208
networkingConfig := containerCfg.NetworkingConfig
209209

210-
warnOnOomKillDisable(*hostConfig, dockerCli.Err())
211210
warnOnLocalhostDNS(*hostConfig, dockerCli.Err())
212211

213212
var (
@@ -299,12 +298,6 @@ func createContainer(ctx context.Context, dockerCli command.Cli, containerCfg *c
299298
return response.ID, err
300299
}
301300

302-
func warnOnOomKillDisable(hostConfig container.HostConfig, stderr io.Writer) {
303-
if hostConfig.OomKillDisable != nil && *hostConfig.OomKillDisable && hostConfig.Memory == 0 {
304-
_, _ = fmt.Fprintln(stderr, "WARNING: Disabling the OOM killer on containers without setting a '-m/--memory' limit may be dangerous.")
305-
}
306-
}
307-
308301
// check the DNS settings passed via --dns against localhost regexp to warn if
309302
// they are trying to set a DNS to a localhost address
310303
func warnOnLocalhostDNS(hostConfig container.HostConfig, stderr io.Writer) {

cli/command/container/create_test.go

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -270,31 +270,24 @@ func TestNewCreateCommandWithContentTrustErrors(t *testing.T) {
270270

271271
func TestNewCreateCommandWithWarnings(t *testing.T) {
272272
testCases := []struct {
273-
name string
274-
args []string
275-
warning bool
273+
name string
274+
args []string
275+
warnings []string
276+
warning bool
276277
}{
277278
{
278-
name: "container-create-without-oom-kill-disable",
279+
name: "container-create-no-warnings",
279280
args: []string{"image:tag"},
280281
},
281282
{
282-
name: "container-create-oom-kill-disable-false",
283-
args: []string{"--oom-kill-disable=false", "image:tag"},
283+
name: "container-create-daemon-single-warning",
284+
args: []string{"image:tag"},
285+
warnings: []string{"warning from daemon"},
284286
},
285287
{
286-
name: "container-create-oom-kill-without-memory-limit",
287-
args: []string{"--oom-kill-disable", "image:tag"},
288-
warning: true,
289-
},
290-
{
291-
name: "container-create-oom-kill-true-without-memory-limit",
292-
args: []string{"--oom-kill-disable=true", "image:tag"},
293-
warning: true,
294-
},
295-
{
296-
name: "container-create-oom-kill-true-with-memory-limit",
297-
args: []string{"--oom-kill-disable=true", "--memory=100M", "image:tag"},
288+
name: "container-create-daemon-multiple-warnings",
289+
args: []string{"image:tag"},
290+
warnings: []string{"warning from daemon", "another warning from daemon"},
298291
},
299292
{
300293
name: "container-create-localhost-dns",
@@ -316,15 +309,15 @@ func TestNewCreateCommandWithWarnings(t *testing.T) {
316309
platform *specs.Platform,
317310
containerName string,
318311
) (container.CreateResponse, error) {
319-
return container.CreateResponse{}, nil
312+
return container.CreateResponse{Warnings: tc.warnings}, nil
320313
},
321314
})
322315
cmd := NewCreateCommand(fakeCLI)
323316
cmd.SetOut(io.Discard)
324317
cmd.SetArgs(tc.args)
325318
err := cmd.Execute()
326319
assert.NilError(t, err)
327-
if tc.warning {
320+
if tc.warning || len(tc.warnings) > 0 {
328321
golden.Assert(t, fakeCLI.ErrBuffer().String(), tc.name+".golden")
329322
} else {
330323
assert.Equal(t, fakeCLI.ErrBuffer().String(), "")
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
WARNING: warning from daemon
2+
WARNING: another warning from daemon
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
WARNING: warning from daemon

cli/command/container/testdata/container-create-oom-kill-true-without-memory-limit.golden

Lines changed: 0 additions & 1 deletion
This file was deleted.

cli/command/container/testdata/container-create-oom-kill-without-memory-limit.golden

Lines changed: 0 additions & 1 deletion
This file was deleted.

0 commit comments

Comments
 (0)