Ignore the root container when calculating oom_score_adj for the sandbox.

This is done because the root container for CRI is the infrastructure (pause)
container and always gets a low oom_score_adj. We do this to ensure that only
the oom_score_adj of user containers is used to calculated the sandbox
oom_score_adj.

Implemented in runsc rather than the containerd shim as it's a bit cleaner to
implement here (in the shim it would require overwriting the oomScoreAdj and
re-writing out the config.json again). This processing is Kubernetes(CRI)
specific but we are currently only supporting CRI for multi-container support
anyway.

PiperOrigin-RevId: 267507706

5 files changed, 438 insertions, 17 deletions

diff --git a/test/root/BUILD b/test/root/BUILD
index f130df2c7..d5dd9bca2 100644
--- a/test/root/BUILD
+++ b/test/root/BUILD
@@ -15,6 +15,11 @@ go_test(
         "cgroup_test.go",
         "chroot_test.go",
         "crictl_test.go",
+        "main_test.go",
+        "oom_score_adj_test.go",
+    ],
+    data = [
+        "//runsc",
     ],
     embed = [":root"],
     tags = [
@@ -25,12 +30,15 @@ go_test(
     ],
     visibility = ["//:sandbox"],
     deps = [
+        "//runsc/boot",
         "//runsc/cgroup",
+        "//runsc/container",
         "//runsc/criutil",
         "//runsc/dockerutil",
         "//runsc/specutils",
         "//runsc/testutil",
         "//test/root/testdata",
+        "@com_github_opencontainers_runtime-spec//specs-go:go_default_library",
         "@com_github_syndtr_gocapability//capability:go_default_library",
     ],
 )
diff --git a/test/root/chroot_test.go b/test/root/chroot_test.go
index f47f8e2c2..be0f63d18 100644
--- a/test/root/chroot_test.go
+++ b/test/root/chroot_test.go
@@ -16,19 +16,15 @@
 package root
 
 import (
-	"flag"
 	"fmt"
 	"io/ioutil"
-	"os"
 	"os/exec"
 	"path/filepath"
 	"strconv"
 	"strings"
 	"testing"
 
-	"github.com/syndtr/gocapability/capability"
 	"gvisor.dev/gvisor/runsc/dockerutil"
-	"gvisor.dev/gvisor/runsc/specutils"
 )
 
 // TestChroot verifies that the sandbox is chroot'd and that mounts are cleaned
@@ -144,15 +140,3 @@ func TestChrootGofer(t *testing.T) {
 		}
 	}
 }
-
-func TestMain(m *testing.M) {
-	dockerutil.EnsureSupportedDockerVersion()
-
-	if !specutils.HasCapabilities(capability.CAP_SYS_ADMIN, capability.CAP_DAC_OVERRIDE) {
-		fmt.Println("Test requires sysadmin privileges to run. Try again with sudo.")
-		os.Exit(1)
-	}
-
-	flag.Parse()
-	os.Exit(m.Run())
-}
diff --git a/test/root/main_test.go b/test/root/main_test.go
new file mode 100644
index 000000000..a3a2a91d9
--- /dev/null
+++ b/test/root/main_test.go
@@ -0,0 +1,48 @@
+// Copyright 2018 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package root
+
+import (
+	"flag"
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/syndtr/gocapability/capability"
+	"gvisor.dev/gvisor/runsc/dockerutil"
+	"gvisor.dev/gvisor/runsc/specutils"
+)
+
+// TestMain is the main function for root tests. This function checks the
+// supported docker version, required capabilities, and configures the executable
+// path for runsc.
+func TestMain(m *testing.M) {
+	dockerutil.EnsureSupportedDockerVersion()
+
+	if !specutils.HasCapabilities(capability.CAP_SYS_ADMIN, capability.CAP_DAC_OVERRIDE) {
+		fmt.Println("Test requires sysadmin privileges to run. Try again with sudo.")
+		os.Exit(1)
+	}
+
+	// Configure exe for tests.
+	path, err := dockerutil.RuntimePath()
+	if err != nil {
+		panic(err.Error())
+	}
+	specutils.ExePath = path
+
+	flag.Parse()
+	os.Exit(m.Run())
+}
diff --git a/test/root/oom_score_adj_test.go b/test/root/oom_score_adj_test.go
new file mode 100644
index 000000000..6cd378a1b
--- /dev/null
+++ b/test/root/oom_score_adj_test.go
@@ -0,0 +1,376 @@
+// Copyright 2018 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package root
+
+import (
+	"fmt"
+	"os"
+	"testing"
+
+	specs "github.com/opencontainers/runtime-spec/specs-go"
+	"gvisor.dev/gvisor/runsc/boot"
+	"gvisor.dev/gvisor/runsc/container"
+	"gvisor.dev/gvisor/runsc/specutils"
+	"gvisor.dev/gvisor/runsc/testutil"
+)
+
+var (
+	maxOOMScoreAdj  = 1000
+	highOOMScoreAdj = 500
+	lowOOMScoreAdj  = -500
+	minOOMScoreAdj  = -1000
+)
+
+// Tests for oom_score_adj have to be run as root (rather than in a user
+// namespace) because we need to adjust oom_score_adj for PIDs other than our
+// own and test values below 0.
+
+// TestOOMScoreAdjSingle tests that oom_score_adj is set properly in a
+// single container sandbox.
+func TestOOMScoreAdjSingle(t *testing.T) {
+	ppid, err := specutils.GetParentPid(os.Getpid())
+	if err != nil {
+		t.Fatalf("getting parent pid: %v", err)
+	}
+	parentOOMScoreAdj, err := specutils.GetOOMScoreAdj(ppid)
+	if err != nil {
+		t.Fatalf("getting parent oom_score_adj: %v", err)
+	}
+
+	testCases := []struct {
+		Name string
+
+		// OOMScoreAdj is the oom_score_adj set to the OCI spec. If nil then
+		// no value is set.
+		OOMScoreAdj *int
+	}{
+		{
+			Name:        "max",
+			OOMScoreAdj: &maxOOMScoreAdj,
+		},
+		{
+			Name:        "high",
+			OOMScoreAdj: &highOOMScoreAdj,
+		},
+		{
+			Name:        "low",
+			OOMScoreAdj: &lowOOMScoreAdj,
+		},
+		{
+			Name:        "min",
+			OOMScoreAdj: &minOOMScoreAdj,
+		},
+		{
+			Name:        "nil",
+			OOMScoreAdj: &parentOOMScoreAdj,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.Name, func(t *testing.T) {
+			id := testutil.UniqueContainerID()
+			s := testutil.NewSpecWithArgs("sleep", "1000")
+			s.Process.OOMScoreAdj = testCase.OOMScoreAdj
+
+			conf := testutil.TestConfig()
+			containers, cleanup, err := startContainers(conf, []*specs.Spec{s}, []string{id})
+			if err != nil {
+				t.Fatalf("error starting containers: %v", err)
+			}
+			defer cleanup()
+
+			c := containers[0]
+
+			// Verify the gofer's oom_score_adj
+			if testCase.OOMScoreAdj != nil {
+				goferScore, err := specutils.GetOOMScoreAdj(c.GoferPid)
+				if err != nil {
+					t.Fatalf("error reading gofer oom_score_adj: %v", err)
+				}
+				if goferScore != *testCase.OOMScoreAdj {
+					t.Errorf("gofer oom_score_adj got: %d, want: %d", goferScore, *testCase.OOMScoreAdj)
+				}
+
+				// Verify the sandbox's oom_score_adj.
+				//
+				// The sandbox should be the same for all containers so just use
+				// the first one.
+				sandboxPid := c.Sandbox.Pid
+				sandboxScore, err := specutils.GetOOMScoreAdj(sandboxPid)
+				if err != nil {
+					t.Fatalf("error reading sandbox oom_score_adj: %v", err)
+				}
+				if sandboxScore != *testCase.OOMScoreAdj {
+					t.Errorf("sandbox oom_score_adj got: %d, want: %d", sandboxScore, *testCase.OOMScoreAdj)
+				}
+			}
+		})
+	}
+}
+
+// TestOOMScoreAdjMulti tests that oom_score_adj is set properly in a
+// multi-container sandbox.
+func TestOOMScoreAdjMulti(t *testing.T) {
+	ppid, err := specutils.GetParentPid(os.Getpid())
+	if err != nil {
+		t.Fatalf("getting parent pid: %v", err)
+	}
+	parentOOMScoreAdj, err := specutils.GetOOMScoreAdj(ppid)
+	if err != nil {
+		t.Fatalf("getting parent oom_score_adj: %v", err)
+	}
+
+	testCases := []struct {
+		Name string
+
+		// OOMScoreAdj is the oom_score_adj set to the OCI spec. If nil then
+		// no value is set. One value for each container. The first value is the
+		// root container.
+		OOMScoreAdj []*int
+
+		// Expected is the expected oom_score_adj of the sandbox. If nil, then
+		// this value is ignored.
+		Expected *int
+
+		// Remove is a set of container indexes to remove from the sandbox.
+		Remove []int
+
+		// ExpectedAfterRemove is the expected oom_score_adj of the sandbox
+		// after containers are removed. Ignored if nil.
+		ExpectedAfterRemove *int
+	}{
+		// A single container CRI test case. This should not happen in
+		// practice as there should be at least one container besides the pause
+		// container. However, we include a test case to ensure sane behavior.
+		{
+			Name:        "single",
+			OOMScoreAdj: []*int{&highOOMScoreAdj},
+			Expected:    &parentOOMScoreAdj,
+		},
+		{
+			Name:        "multi_no_value",
+			OOMScoreAdj: []*int{nil, nil, nil},
+			Expected:    &parentOOMScoreAdj,
+		},
+		{
+			Name:        "multi_non_nil_root",
+			OOMScoreAdj: []*int{&minOOMScoreAdj, nil, nil},
+			Expected:    &parentOOMScoreAdj,
+		},
+		{
+			Name:        "multi_value",
+			OOMScoreAdj: []*int{&minOOMScoreAdj, &highOOMScoreAdj, &lowOOMScoreAdj},
+			// The lowest value excluding the root container is expected.
+			Expected: &lowOOMScoreAdj,
+		},
+		{
+			Name:        "multi_min_value",
+			OOMScoreAdj: []*int{&minOOMScoreAdj, &lowOOMScoreAdj},
+			// The lowest value excluding the root container is expected.
+			Expected: &lowOOMScoreAdj,
+		},
+		{
+			Name:        "multi_max_value",
+			OOMScoreAdj: []*int{&minOOMScoreAdj, &maxOOMScoreAdj, &highOOMScoreAdj},
+			// The lowest value excluding the root container is expected.
+			Expected: &highOOMScoreAdj,
+		},
+		{
+			Name:        "remove_adjusted",
+			OOMScoreAdj: []*int{&minOOMScoreAdj, &maxOOMScoreAdj, &highOOMScoreAdj},
+			// The lowest value excluding the root container is expected.
+			Expected: &highOOMScoreAdj,
+			// Remove highOOMScoreAdj container.
+			Remove:              []int{2},
+			ExpectedAfterRemove: &maxOOMScoreAdj,
+		},
+		{
+			// This test removes all non-root sandboxes with a specified oomScoreAdj.
+			Name:        "remove_to_nil",
+			OOMScoreAdj: []*int{&minOOMScoreAdj, nil, &lowOOMScoreAdj},
+			Expected:    &lowOOMScoreAdj,
+			// Remove lowOOMScoreAdj container.
+			Remove: []int{2},
+			// The oom_score_adj expected after remove is that of the parent process.
+			ExpectedAfterRemove: &parentOOMScoreAdj,
+		},
+		{
+			Name:        "remove_no_effect",
+			OOMScoreAdj: []*int{&minOOMScoreAdj, &maxOOMScoreAdj, &highOOMScoreAdj},
+			// The lowest value excluding the root container is expected.
+			Expected: &highOOMScoreAdj,
+			// Remove the maxOOMScoreAdj container.
+			Remove:              []int{1},
+			ExpectedAfterRemove: &highOOMScoreAdj,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.Name, func(t *testing.T) {
+			var cmds [][]string
+			var oomScoreAdj []*int
+			var toRemove []string
+
+			for _, oomScore := range testCase.OOMScoreAdj {
+				oomScoreAdj = append(oomScoreAdj, oomScore)
+				cmds = append(cmds, []string{"sleep", "100"})
+			}
+
+			specs, ids := createSpecs(cmds...)
+			for i, spec := range specs {
+				// Ensure the correct value is set, including no value.
+				spec.Process.OOMScoreAdj = oomScoreAdj[i]
+
+				for _, j := range testCase.Remove {
+					if i == j {
+						toRemove = append(toRemove, ids[i])
+					}
+				}
+			}
+
+			conf := testutil.TestConfig()
+			containers, cleanup, err := startContainers(conf, specs, ids)
+			if err != nil {
+				t.Fatalf("error starting containers: %v", err)
+			}
+			defer cleanup()
+
+			for i, c := range containers {
+				if oomScoreAdj[i] != nil {
+					// Verify the gofer's oom_score_adj
+					score, err := specutils.GetOOMScoreAdj(c.GoferPid)
+					if err != nil {
+						t.Fatalf("error reading gofer oom_score_adj: %v", err)
+					}
+					if score != *oomScoreAdj[i] {
+						t.Errorf("gofer oom_score_adj got: %d, want: %d", score, *oomScoreAdj[i])
+					}
+				}
+			}
+
+			// Verify the sandbox's oom_score_adj.
+			//
+			// The sandbox should be the same for all containers so just use
+			// the first one.
+			sandboxPid := containers[0].Sandbox.Pid
+			if testCase.Expected != nil {
+				score, err := specutils.GetOOMScoreAdj(sandboxPid)
+				if err != nil {
+					t.Fatalf("error reading sandbox oom_score_adj: %v", err)
+				}
+				if score != *testCase.Expected {
+					t.Errorf("sandbox oom_score_adj got: %d, want: %d", score, *testCase.Expected)
+				}
+			}
+
+			if len(toRemove) == 0 {
+				return
+			}
+
+			// Remove containers.
+			for _, removeID := range toRemove {
+				for _, c := range containers {
+					if c.ID == removeID {
+						c.Destroy()
+					}
+				}
+			}
+
+			// Check the new adjusted oom_score_adj.
+			if testCase.ExpectedAfterRemove != nil {
+				scoreAfterRemove, err := specutils.GetOOMScoreAdj(sandboxPid)
+				if err != nil {
+					t.Fatalf("error reading sandbox oom_score_adj: %v", err)
+				}
+				if scoreAfterRemove != *testCase.ExpectedAfterRemove {
+					t.Errorf("sandbox oom_score_adj got: %d, want: %d", scoreAfterRemove, *testCase.ExpectedAfterRemove)
+				}
+			}
+		})
+	}
+}
+
+func createSpecs(cmds ...[]string) ([]*specs.Spec, []string) {
+	var specs []*specs.Spec
+	var ids []string
+	rootID := testutil.UniqueContainerID()
+
+	for i, cmd := range cmds {
+		spec := testutil.NewSpecWithArgs(cmd...)
+		if i == 0 {
+			spec.Annotations = map[string]string{
+				specutils.ContainerdContainerTypeAnnotation: specutils.ContainerdContainerTypeSandbox,
+			}
+			ids = append(ids, rootID)
+		} else {
+			spec.Annotations = map[string]string{
+				specutils.ContainerdContainerTypeAnnotation: specutils.ContainerdContainerTypeContainer,
+				specutils.ContainerdSandboxIDAnnotation:     rootID,
+			}
+			ids = append(ids, testutil.UniqueContainerID())
+		}
+		specs = append(specs, spec)
+	}
+	return specs, ids
+}
+
+func startContainers(conf *boot.Config, specs []*specs.Spec, ids []string) ([]*container.Container, func(), error) {
+	// Setup root dir if one hasn't been provided.
+	if len(conf.RootDir) == 0 {
+		rootDir, err := testutil.SetupRootDir()
+		if err != nil {
+			return nil, nil, fmt.Errorf("error creating root dir: %v", err)
+		}
+		conf.RootDir = rootDir
+	}
+
+	var containers []*container.Container
+	var bundles []string
+	cleanup := func() {
+		for _, c := range containers {
+			c.Destroy()
+		}
+		for _, b := range bundles {
+			os.RemoveAll(b)
+		}
+		os.RemoveAll(conf.RootDir)
+	}
+	for i, spec := range specs {
+		bundleDir, err := testutil.SetupBundleDir(spec)
+		if err != nil {
+			cleanup()
+			return nil, nil, fmt.Errorf("error setting up container: %v", err)
+		}
+		bundles = append(bundles, bundleDir)
+
+		args := container.Args{
+			ID:        ids[i],
+			Spec:      spec,
+			BundleDir: bundleDir,
+		}
+		cont, err := container.New(conf, args)
+		if err != nil {
+			cleanup()
+			return nil, nil, fmt.Errorf("error creating container: %v", err)
+		}
+		containers = append(containers, cont)
+
+		if err := cont.Start(conf); err != nil {
+			cleanup()
+			return nil, nil, fmt.Errorf("error starting container: %v", err)
+		}
+	}
+	return containers, cleanup, nil
+}
diff --git a/test/root/root.go b/test/root/root.go
index 349c752cc..0f1d29faf 100644
--- a/test/root/root.go
+++ b/test/root/root.go
@@ -12,5 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package root is empty. See chroot_test.go for description.
+// Package root is used for tests that requires sysadmin privileges run. First,
+// follow the setup instruction in runsc/test/README.md. You should also have
+// docker, containerd, and crictl installed. To run these tests from the
+// project root directory:
+//
+//     ./scripts/root_tests.sh
 package root