From fd3a6e6c8323b318ce5c6bb1a8a2f690ccd3f40a Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Fri, 16 Mar 2018 11:54:47 +1100 Subject: [PATCH] libcontainer: handle unset oomScoreAdj corectly Previously if oomScoreAdj was not set in config.json we would implicitly set oom_score_adj to 0. This is not allowed according to the spec: > If oomScoreAdj is not set, the runtime MUST NOT change the value of > oom_score_adj. Change this so that we do not modify oom_score_adj if oomScoreAdj is not present in the configuration. While this modifies our internal configuration types, the on-disk format is still compatible. Signed-off-by: Aleksa Sarai --- libcontainer/configs/config.go | 5 +++-- libcontainer/container_linux.go | 12 +++++++----- libcontainer/integration/exec_test.go | 6 +++--- libcontainer/integration/execin_test.go | 6 +++--- libcontainer/integration/utils_test.go | 4 ++++ libcontainer/specconv/spec_linux.go | 4 ++-- 6 files changed, 22 insertions(+), 15 deletions(-) diff --git a/libcontainer/configs/config.go b/libcontainer/configs/config.go index 3cae4fd8d96..b1c4762fe20 100644 --- a/libcontainer/configs/config.go +++ b/libcontainer/configs/config.go @@ -141,9 +141,10 @@ type Config struct { // OomScoreAdj specifies the adjustment to be made by the kernel when calculating oom scores // for a process. Valid values are between the range [-1000, '1000'], where processes with - // higher scores are preferred for being killed. + // higher scores are preferred for being killed. If it is unset then we don't touch the current + // value. // More information about kernel oom score calculation here: https://lwn.net/Articles/317814/ - OomScoreAdj int `json:"oom_score_adj"` + OomScoreAdj *int `json:"oom_score_adj,omitempty"` // UidMappings is an array of User ID mappings for User Namespaces UidMappings []IDMap `json:"uid_mappings"` diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index db2242e2696..eec9f6bcbf9 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -1818,11 +1818,13 @@ func (c *linuxContainer) bootstrapData(cloneFlags uintptr, nsMaps map[configs.Na } } - // write oom_score_adj - r.AddData(&Bytemsg{ - Type: OomScoreAdjAttr, - Value: []byte(fmt.Sprintf("%d", c.config.OomScoreAdj)), - }) + if c.config.OomScoreAdj != nil { + // write oom_score_adj + r.AddData(&Bytemsg{ + Type: OomScoreAdjAttr, + Value: []byte(fmt.Sprintf("%d", *c.config.OomScoreAdj)), + }) + } // write rootless r.AddData(&Boolmsg{ diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 4fbd760fd3a..0ef425be9fc 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -1075,7 +1075,7 @@ func TestOomScoreAdj(t *testing.T) { defer remove(rootfs) config := newTemplateConfig(rootfs) - config.OomScoreAdj = 200 + config.OomScoreAdj = ptrInt(200) factory, err := libcontainer.New(root, libcontainer.Cgroupfs) ok(t, err) @@ -1100,8 +1100,8 @@ func TestOomScoreAdj(t *testing.T) { outputOomScoreAdj := strings.TrimSpace(string(stdout.Bytes())) // Check that the oom_score_adj matches the value that was set as part of config. - if outputOomScoreAdj != strconv.Itoa(config.OomScoreAdj) { - t.Fatalf("Expected oom_score_adj %d; got %q", config.OomScoreAdj, outputOomScoreAdj) + if outputOomScoreAdj != strconv.Itoa(*config.OomScoreAdj) { + t.Fatalf("Expected oom_score_adj %d; got %q", *config.OomScoreAdj, outputOomScoreAdj) } } diff --git a/libcontainer/integration/execin_test.go b/libcontainer/integration/execin_test.go index 8ef19947797..56fa3c75c76 100644 --- a/libcontainer/integration/execin_test.go +++ b/libcontainer/integration/execin_test.go @@ -497,7 +497,7 @@ func TestExecInOomScoreAdj(t *testing.T) { ok(t, err) defer remove(rootfs) config := newTemplateConfig(rootfs) - config.OomScoreAdj = 200 + config.OomScoreAdj = ptrInt(200) container, err := newContainer(config) ok(t, err) defer container.Destroy() @@ -532,8 +532,8 @@ func TestExecInOomScoreAdj(t *testing.T) { waitProcess(process, t) out := buffers.Stdout.String() - if oomScoreAdj := strings.TrimSpace(out); oomScoreAdj != strconv.Itoa(config.OomScoreAdj) { - t.Fatalf("expected oomScoreAdj to be %d, got %s", config.OomScoreAdj, oomScoreAdj) + if oomScoreAdj := strings.TrimSpace(out); oomScoreAdj != strconv.Itoa(*config.OomScoreAdj) { + t.Fatalf("expected oomScoreAdj to be %d, got %s", *config.OomScoreAdj, oomScoreAdj) } } diff --git a/libcontainer/integration/utils_test.go b/libcontainer/integration/utils_test.go index e209c9bcfde..25500032c93 100644 --- a/libcontainer/integration/utils_test.go +++ b/libcontainer/integration/utils_test.go @@ -19,6 +19,10 @@ import ( "github.com/opencontainers/runc/libcontainer/configs" ) +func ptrInt(v int) *int { + return &v +} + func newStdBuffers() *stdBuffers { return &stdBuffers{ Stdin: bytes.NewBuffer(nil), diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go index 98fd2e63214..0222d3927fd 100644 --- a/libcontainer/specconv/spec_linux.go +++ b/libcontainer/specconv/spec_linux.go @@ -243,8 +243,8 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) { if spec.Process.SelinuxLabel != "" { config.ProcessLabel = spec.Process.SelinuxLabel } - if spec.Process != nil && spec.Process.OOMScoreAdj != nil { - config.OomScoreAdj = *spec.Process.OOMScoreAdj + if spec.Process != nil { + config.OomScoreAdj = spec.Process.OOMScoreAdj } if spec.Process.Capabilities != nil { config.Capabilities = &configs.Capabilities{