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
This commit is contained in:
Ian Lewis
2019-09-05 19:21:25 -07:00
committed by gVisor bot
parent 7e94f171f4
commit 0bfffbcb01
7 changed files with 560 additions and 38 deletions
+83 -21
View File
@@ -513,9 +513,16 @@ func (c *Container) Start(conf *boot.Config) error {
return err
}
// Adjust the oom_score_adj for sandbox and gofers. This must be done after
// Adjust the oom_score_adj for sandbox. This must be done after
// save().
return c.adjustOOMScoreAdj(conf)
err = adjustSandboxOOMScoreAdj(c.Sandbox, c.RootContainerDir, false)
if err != nil {
return err
}
// Set container's oom_score_adj to the gofer since it is dedicated to
// the container, in case the gofer uses up too much memory.
return c.adjustGoferOOMScoreAdj()
}
// Restore takes a container and replaces its kernel and file system
@@ -782,6 +789,9 @@ func (c *Container) Destroy() error {
}
defer unlock()
// Stored for later use as stop() sets c.Sandbox to nil.
sb := c.Sandbox
if err := c.stop(); err != nil {
err = fmt.Errorf("stopping container: %v", err)
log.Warningf("%v", err)
@@ -796,6 +806,16 @@ func (c *Container) Destroy() error {
c.changeStatus(Stopped)
// Adjust oom_score_adj for the sandbox. This must be done after the
// container is stopped and the directory at c.Root is removed.
// We must test if the sandbox is nil because Destroy should be
// idempotent.
if sb != nil {
if err := adjustSandboxOOMScoreAdj(sb, c.RootContainerDir, true); err != nil {
errs = append(errs, err.Error())
}
}
// "If any poststop hook fails, the runtime MUST log a warning, but the
// remaining hooks and lifecycle continue as if the hook had succeeded" -OCI spec.
// Based on the OCI, "The post-stop hooks MUST be called after the container is
@@ -1139,33 +1159,80 @@ func runInCgroup(cg *cgroup.Cgroup, fn func() error) error {
return fn()
}
// adjustOOMScoreAdj sets the oom_score_adj for the sandbox and all gofers.
// adjustGoferOOMScoreAdj sets the oom_store_adj for the container's gofer.
func (c *Container) adjustGoferOOMScoreAdj() error {
if c.GoferPid != 0 && c.Spec.Process.OOMScoreAdj != nil {
if err := setOOMScoreAdj(c.GoferPid, *c.Spec.Process.OOMScoreAdj); err != nil {
return fmt.Errorf("setting gofer oom_score_adj for container %q: %v", c.ID, err)
}
}
return nil
}
// adjustSandboxOOMScoreAdj sets the oom_score_adj for the sandbox.
// oom_score_adj is set to the lowest oom_score_adj among the containers
// running in the sandbox.
//
// TODO(gvisor.dev/issue/512): This call could race with other containers being
// created at the same time and end up setting the wrong oom_score_adj to the
// sandbox.
func (c *Container) adjustOOMScoreAdj(conf *boot.Config) error {
// If this container's OOMScoreAdj is nil then we can exit early as no
// change should be made to oom_score_adj for the sandbox.
if c.Spec.Process.OOMScoreAdj == nil {
return nil
}
containers, err := loadSandbox(conf.RootDir, c.Sandbox.ID)
func adjustSandboxOOMScoreAdj(s *sandbox.Sandbox, rootDir string, destroy bool) error {
containers, err := loadSandbox(rootDir, s.ID)
if err != nil {
return fmt.Errorf("loading sandbox containers: %v", err)
}
// Do nothing if the sandbox has been terminated.
if len(containers) == 0 {
return nil
}
// Get the lowest score for all containers.
var lowScore int
scoreFound := false
for _, container := range containers {
if container.Spec.Process.OOMScoreAdj != nil && (!scoreFound || *container.Spec.Process.OOMScoreAdj < lowScore) {
if len(containers) == 1 && len(containers[0].Spec.Annotations[specutils.ContainerdContainerTypeAnnotation]) == 0 {
// This is a single-container sandbox. Set the oom_score_adj to
// the value specified in the OCI bundle.
if containers[0].Spec.Process.OOMScoreAdj != nil {
scoreFound = true
lowScore = *container.Spec.Process.OOMScoreAdj
lowScore = *containers[0].Spec.Process.OOMScoreAdj
}
} else {
for _, container := range containers {
// Special multi-container support for CRI. Ignore the root
// container when calculating oom_score_adj for the sandbox because
// it is the infrastructure (pause) container and always has a very
// low oom_score_adj.
//
// We will use OOMScoreAdj in the single-container case where the
// containerd container-type annotation is not present.
if container.Spec.Annotations[specutils.ContainerdContainerTypeAnnotation] == specutils.ContainerdContainerTypeSandbox {
continue
}
if container.Spec.Process.OOMScoreAdj != nil && (!scoreFound || *container.Spec.Process.OOMScoreAdj < lowScore) {
scoreFound = true
lowScore = *container.Spec.Process.OOMScoreAdj
}
}
}
// If the container is destroyed and remaining containers have no
// oomScoreAdj specified then we must revert to the oom_score_adj of the
// parent process.
if !scoreFound && destroy {
ppid, err := specutils.GetParentPid(s.Pid)
if err != nil {
return fmt.Errorf("getting parent pid of sandbox pid %d: %v", s.Pid, err)
}
pScore, err := specutils.GetOOMScoreAdj(ppid)
if err != nil {
return fmt.Errorf("getting oom_score_adj of parent %d: %v", ppid, err)
}
scoreFound = true
lowScore = pScore
}
// Only set oom_score_adj if one of the containers has oom_score_adj set
@@ -1177,15 +1244,10 @@ func (c *Container) adjustOOMScoreAdj(conf *boot.Config) error {
}
// Set the lowest of all containers oom_score_adj to the sandbox.
if err := setOOMScoreAdj(c.Sandbox.Pid, lowScore); err != nil {
return fmt.Errorf("setting oom_score_adj for sandbox %q: %v", c.Sandbox.ID, err)
if err := setOOMScoreAdj(s.Pid, lowScore); err != nil {
return fmt.Errorf("setting oom_score_adj for sandbox %q: %v", s.ID, err)
}
// Set container's oom_score_adj to the gofer since it is dedicated to the
// container, in case the gofer uses up too much memory.
if err := setOOMScoreAdj(c.GoferPid, *c.Spec.Process.OOMScoreAdj); err != nil {
return fmt.Errorf("setting gofer oom_score_adj for container %q: %v", c.ID, err)
}
return nil
}
+39
View File
@@ -23,6 +23,7 @@ import (
"os"
"path"
"path/filepath"
"strconv"
"strings"
"syscall"
"time"
@@ -503,3 +504,41 @@ func RetryEintr(f func() (uintptr, uintptr, error)) (uintptr, uintptr, error) {
}
}
}
// GetOOMScoreAdj reads the given process' oom_score_adj
func GetOOMScoreAdj(pid int) (int, error) {
data, err := ioutil.ReadFile(fmt.Sprintf("/proc/%d/oom_score_adj", pid))
if err != nil {
return 0, err
}
return strconv.Atoi(strings.TrimSpace(string(data)))
}
// GetParentPid gets the parent process ID of the specified PID.
func GetParentPid(pid int) (int, error) {
data, err := ioutil.ReadFile(fmt.Sprintf("/proc/%d/stat", pid))
if err != nil {
return 0, err
}
var cpid string
var name string
var state string
var ppid int
// Parse after the binary name.
_, err = fmt.Sscanf(string(data),
"%v %v %v %d",
// cpid is ignored.
&cpid,
// name is ignored.
&name,
// state is ignored.
&state,
&ppid)
if err != nil {
return 0, err
}
return ppid, nil
}
+8
View File
@@ -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",
],
)
-16
View File
@@ -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())
}
+48
View File
@@ -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())
}
+376
View File
@@ -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
}
+6 -1
View File
@@ -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