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 <[email protected]>
opencontainers · Mar 17, 2018 · fd3a6e6 · fd3a6e6
1 parent 03e5859
commit fd3a6e6
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 15 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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{