Skip to content

Commit 7dfc605

Browse files
crosbymichaelcrosbymichael
committed
Set shim OOM scores to +1 containerd daemon score
This changes the shim's OOM score from a static max killable of -999 to be +1 of the containerd daemon's score. This should allow the shim's to be killed first in an OOM condition but leave the daemon alone for a bit to help cleanup and manage the containers during this situation. Signed-off-by: Michael Crosby <[email protected]>
1 parent bb9616b commit 7dfc605

6 files changed

Lines changed: 116 additions & 6 deletions

File tree

‎container_linux_test.go‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import (
4242
"github.com/containerd/containerd/plugin"
4343
"github.com/containerd/containerd/runtime/linux/runctypes"
4444
"github.com/containerd/containerd/runtime/v2/runc/options"
45+
"github.com/containerd/containerd/sys"
4546
specs "github.com/opencontainers/runtime-spec/specs-go"
4647
"github.com/pkg/errors"
4748
"golang.org/x/sys/unix"
@@ -1731,3 +1732,75 @@ func TestContainerNoSTDIN(t *testing.T) {
17311732
t.Errorf("expected status 0 from wait but received %d", code)
17321733
}
17331734
}
1735+
1736+
func TestShimOOMScore(t *testing.T) {
1737+
containerdPid := ctrd.cmd.Process.Pid
1738+
containerdScore, err := sys.GetOOMScoreAdj(containerdPid)
1739+
if err != nil {
1740+
t.Fatal(err)
1741+
}
1742+
1743+
client, err := newClient(t, address)
1744+
if err != nil {
1745+
t.Fatal(err)
1746+
}
1747+
defer client.Close()
1748+
1749+
var (
1750+
image Image
1751+
ctx, cancel = testContext()
1752+
id = t.Name()
1753+
)
1754+
defer cancel()
1755+
1756+
path := "/containerd/oomshim"
1757+
cg, err := cgroups.New(cgroups.V1, cgroups.StaticPath(path), &specs.LinuxResources{})
1758+
if err != nil {
1759+
t.Fatal(err)
1760+
}
1761+
defer cg.Delete()
1762+
1763+
image, err = client.GetImage(ctx, testImage)
1764+
if err != nil {
1765+
t.Fatal(err)
1766+
}
1767+
1768+
container, err := client.NewContainer(ctx, id, WithNewSnapshot(id, image), WithNewSpec(oci.WithImageConfig(image), withProcessArgs("sleep", "30")))
1769+
if err != nil {
1770+
t.Fatal(err)
1771+
}
1772+
defer container.Delete(ctx, WithSnapshotCleanup)
1773+
1774+
task, err := container.NewTask(ctx, empty(), WithShimCgroup(path))
1775+
if err != nil {
1776+
t.Fatal(err)
1777+
}
1778+
defer task.Delete(ctx)
1779+
1780+
statusC, err := task.Wait(ctx)
1781+
if err != nil {
1782+
t.Fatal(err)
1783+
}
1784+
1785+
processes, err := cg.Processes(cgroups.Devices, false)
1786+
if err != nil {
1787+
t.Fatal(err)
1788+
}
1789+
expectedScore := containerdScore + 1
1790+
// find the shim's pid
1791+
for _, p := range processes {
1792+
score, err := sys.GetOOMScoreAdj(p.Pid)
1793+
if err != nil {
1794+
t.Fatal(err)
1795+
}
1796+
if score != expectedScore {
1797+
t.Errorf("expected score %d but got %d for shim process", expectedScore, score)
1798+
}
1799+
}
1800+
1801+
if err := task.Kill(ctx, unix.SIGKILL); err != nil {
1802+
t.Fatal(err)
1803+
}
1804+
1805+
<-statusC
1806+
}

‎runtime/v1/shim/client/client.go‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,8 @@ func WithStart(binary, address, daemonAddress, cgroup string, debug bool, exitHa
127127
"address": address,
128128
}).Infof("shim placed in cgroup %s", cgroup)
129129
}
130-
if err = sys.SetOOMScore(cmd.Process.Pid, sys.OOMScoreMaxKillable); err != nil {
131-
return nil, nil, errors.Wrap(err, "failed to set OOM Score on shim")
130+
if err = setupOOMScore(cmd.Process.Pid); err != nil {
131+
return nil, nil, err
132132
}
133133
c, clo, err := WithConnect(address, func() {})(ctx, config)
134134
if err != nil {
@@ -138,6 +138,21 @@ func WithStart(binary, address, daemonAddress, cgroup string, debug bool, exitHa
138138
}
139139
}
140140

141+
// setupOOMScore gets containerd's oom score and adds +1 to it
142+
// to ensure a shim has a lower* score than the daemons
143+
func setupOOMScore(shimPid int) error {
144+
pid := os.Getpid()
145+
score, err := sys.GetOOMScoreAdj(pid)
146+
if err != nil {
147+
return errors.Wrap(err, "get daemon OOM score")
148+
}
149+
shimScore := score + 1
150+
if err := sys.SetOOMScore(shimPid, shimScore); err != nil {
151+
return errors.Wrap(err, "set shim OOM score")
152+
}
153+
return nil
154+
}
155+
141156
func newCommand(binary, daemonAddress string, debug bool, config shim.Config, socket *os.File, stdout, stderr io.Writer) (*exec.Cmd, error) {
142157
selfExe, err := os.Executable()
143158
if err != nil {

‎runtime/v2/runc/v1/service.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,8 @@ func (s *service) StartShim(ctx context.Context, id, containerdBinary, container
190190
}
191191
}
192192
}
193-
if err := shim.SetScore(cmd.Process.Pid); err != nil {
194-
return "", errors.Wrap(err, "failed to set OOM Score on shim")
193+
if err := shim.AdjustOOMScore(cmd.Process.Pid); err != nil {
194+
return "", errors.Wrap(err, "failed to adjust OOM score for shim")
195195
}
196196
return address, nil
197197
}

‎runtime/v2/runc/v2/service.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,8 @@ func (s *service) StartShim(ctx context.Context, id, containerdBinary, container
233233
}
234234
}
235235
}
236-
if err := shim.SetScore(cmd.Process.Pid); err != nil {
237-
return "", errors.Wrap(err, "failed to set OOM Score on shim")
236+
if err := shim.AdjustOOMScore(cmd.Process.Pid); err != nil {
237+
return "", errors.Wrap(err, "failed to adjust OOM score for shim")
238238
}
239239
return address, nil
240240
}

‎runtime/v2/shim/util_unix.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"crypto/sha256"
2424
"fmt"
2525
"net"
26+
"os"
2627
"path/filepath"
2728
"strings"
2829
"syscall"
@@ -46,6 +47,21 @@ func SetScore(pid int) error {
4647
return sys.SetOOMScore(pid, sys.OOMScoreMaxKillable)
4748
}
4849

50+
// AdjustOOMScore sets the OOM score for the process to the parents OOM score +1
51+
// to ensure that they parent has a lower* score than the shim
52+
func AdjustOOMScore(pid int) error {
53+
parent := os.Getppid()
54+
score, err := sys.GetOOMScoreAdj(parent)
55+
if err != nil {
56+
return errors.Wrap(err, "get parent OOM score")
57+
}
58+
shimScore := score + 1
59+
if err := sys.SetOOMScore(pid, shimScore); err != nil {
60+
return errors.Wrap(err, "set shim OOM score")
61+
}
62+
return nil
63+
}
64+
4965
// SocketAddress returns an abstract socket address
5066
func SocketAddress(ctx context.Context, id string) (string, error) {
5167
ns, err := namespaces.NamespaceRequired(ctx)

‎runtime/v2/shim/util_windows.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ func SetScore(pid int) error {
4040
return nil
4141
}
4242

43+
// AdjustOOMScore sets the OOM score for the process to the parents OOM score +1
44+
// to ensure that they parent has a lower* score than the shim
45+
func AdjustOOMScore(pid int) error {
46+
return nil
47+
}
48+
4349
// SocketAddress returns a npipe address
4450
func SocketAddress(ctx context.Context, id string) (string, error) {
4551
ns, err := namespaces.NamespaceRequired(ctx)

0 commit comments

Comments
 (0)