Skip to content

Commit 1edab60

Browse files
mxpvmxpv
authored
Merge pull request #5253 from thaJeztah/refactor_oom
sys: refactor OOM utilities
2 parents 88880f0 + 7bb73da commit 1edab60

7 files changed

Lines changed: 68 additions & 67 deletions

File tree

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,10 +182,7 @@ func setupOOMScore(shimPid int) error {
182182
return errors.Wrap(err, "get daemon OOM score")
183183
}
184184
shimScore := score + 1
185-
if shimScore > sys.OOMScoreAdjMax {
186-
shimScore = sys.OOMScoreAdjMax
187-
}
188-
if err := sys.SetOOMScore(shimPid, shimScore); err != nil {
185+
if err := sys.AdjustOOMScore(shimPid, shimScore); err != nil {
189186
return errors.Wrap(err, "set shim OOM score")
190187
}
191188
return nil

‎runtime/v2/shim/util_unix.go‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,6 @@ func getSysProcAttr() *syscall.SysProcAttr {
4646
}
4747
}
4848

49-
// SetScore sets the oom score for a process
50-
func SetScore(pid int) error {
51-
return sys.SetOOMScore(pid, sys.OOMScoreMaxKillable)
52-
}
53-
5449
// AdjustOOMScore sets the OOM score for the process to the parents OOM score +1
5550
// to ensure that they parent has a lower* score than the shim
5651
// if not already at the maximum OOM Score
@@ -61,10 +56,7 @@ func AdjustOOMScore(pid int) error {
6156
return errors.Wrap(err, "get parent OOM score")
6257
}
6358
shimScore := score + 1
64-
if shimScore > sys.OOMScoreAdjMax {
65-
shimScore = sys.OOMScoreAdjMax
66-
}
67-
if err := sys.SetOOMScore(pid, shimScore); err != nil {
59+
if err := sys.AdjustOOMScore(pid, shimScore); err != nil {
6860
return errors.Wrap(err, "set shim OOM score")
6961
}
7062
return nil

‎sys/env.go‎

Lines changed: 0 additions & 33 deletions
This file was deleted.

‎sys/mount_linux_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import (
3232
type fMountatCaseFunc func(t *testing.T, root string)
3333

3434
func TestFMountat(t *testing.T) {
35-
if RunningUnprivileged() {
35+
if !runningPrivileged() {
3636
t.Skip("Needs to be run as root")
3737
return
3838
}

‎sys/oom_linux.go‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,25 +24,40 @@ import (
2424
"strings"
2525

2626
"github.com/containerd/containerd/pkg/userns"
27+
"golang.org/x/sys/unix"
2728
)
2829

2930
const (
30-
// OOMScoreMaxKillable is the maximum score keeping the process killable by the oom killer
31-
OOMScoreMaxKillable = -999
32-
// OOMScoreAdjMax is from OOM_SCORE_ADJ_MAX https://github.com/torvalds/linux/blob/master/include/uapi/linux/oom.h
31+
// OOMScoreAdjMin is from OOM_SCORE_ADJ_MIN https://github.com/torvalds/linux/blob/v5.10/include/uapi/linux/oom.h#L9
32+
OOMScoreAdjMin = -1000
33+
// OOMScoreAdjMax is from OOM_SCORE_ADJ_MAX https://github.com/torvalds/linux/blob/v5.10/include/uapi/linux/oom.h#L10
3334
OOMScoreAdjMax = 1000
3435
)
3536

37+
// AdjustOOMScore sets the oom score for the provided pid. If the provided score
38+
// is out of range (-1000 - 1000), it is clipped to the min/max value.
39+
func AdjustOOMScore(pid, score int) error {
40+
if score > OOMScoreAdjMax {
41+
score = OOMScoreAdjMax
42+
} else if score < OOMScoreAdjMin {
43+
score = OOMScoreAdjMin
44+
}
45+
return SetOOMScore(pid, score)
46+
}
47+
3648
// SetOOMScore sets the oom score for the provided pid
3749
func SetOOMScore(pid, score int) error {
50+
if score > OOMScoreAdjMax || score < OOMScoreAdjMin {
51+
return fmt.Errorf("value out of range (%d): OOM score must be between %d and %d", score, OOMScoreAdjMin, OOMScoreAdjMax)
52+
}
3853
path := fmt.Sprintf("/proc/%d/oom_score_adj", pid)
3954
f, err := os.OpenFile(path, os.O_WRONLY, 0)
4055
if err != nil {
4156
return err
4257
}
4358
defer f.Close()
4459
if _, err = f.WriteString(strconv.Itoa(score)); err != nil {
45-
if os.IsPermission(err) && (userns.RunningInUserNS() || RunningUnprivileged()) {
60+
if os.IsPermission(err) && (!runningPrivileged() || userns.RunningInUserNS()) {
4661
return nil
4762
}
4863
return err
@@ -59,3 +74,9 @@ func GetOOMScoreAdj(pid int) (int, error) {
5974
}
6075
return strconv.Atoi(strings.TrimSpace(string(data)))
6176
}
77+
78+
// runningPrivileged returns true if the effective user ID of the
79+
// calling process is 0
80+
func runningPrivileged() bool {
81+
return unix.Geteuid() == 0
82+
}

‎sys/oom_linux_test.go‎

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,52 +18,68 @@ package sys
1818

1919
import (
2020
"errors"
21+
"fmt"
2122
"os"
2223
"os/exec"
2324
"testing"
2425
"time"
2526

27+
"github.com/containerd/containerd/pkg/userns"
2628
"gotest.tools/v3/assert"
2729
is "gotest.tools/v3/assert/cmp"
2830
)
2931

3032
func TestSetPositiveOomScoreAdjustment(t *testing.T) {
33+
// Setting a *positive* OOM score adjust does not require privileged
3134
_, adjustment, err := adjustOom(123)
32-
if err != nil {
33-
t.Error(err)
34-
return
35-
}
35+
assert.NilError(t, err)
3636
assert.Check(t, is.Equal(adjustment, 123))
3737
}
3838

3939
func TestSetNegativeOomScoreAdjustmentWhenPrivileged(t *testing.T) {
40-
if RunningUnprivileged() {
41-
t.Skip("Needs to be run as root")
40+
if !runningPrivileged() || userns.RunningInUserNS() {
41+
t.Skip("requires root and not running in user namespace")
4242
return
4343
}
4444

4545
_, adjustment, err := adjustOom(-123)
46-
if err != nil {
47-
t.Error(err)
48-
return
49-
}
46+
assert.NilError(t, err)
5047
assert.Check(t, is.Equal(adjustment, -123))
5148
}
5249

5350
func TestSetNegativeOomScoreAdjustmentWhenUnprivilegedHasNoEffect(t *testing.T) {
54-
if RunningPrivileged() {
55-
t.Skip("Needs to be run as non-root")
51+
if runningPrivileged() && !userns.RunningInUserNS() {
52+
t.Skip("needs to be run as non-root or in user namespace")
5653
return
5754
}
5855

5956
initial, adjustment, err := adjustOom(-123)
60-
if err != nil {
61-
t.Error(err)
62-
return
63-
}
57+
assert.NilError(t, err)
6458
assert.Check(t, is.Equal(adjustment, initial))
6559
}
6660

61+
func TestSetOOMScoreBoundaries(t *testing.T) {
62+
err := SetOOMScore(0, OOMScoreAdjMax+1)
63+
assert.ErrorContains(t, err, fmt.Sprintf("value out of range (%d): OOM score must be between", OOMScoreAdjMax+1))
64+
65+
err = SetOOMScore(0, OOMScoreAdjMin-1)
66+
assert.ErrorContains(t, err, fmt.Sprintf("value out of range (%d): OOM score must be between", OOMScoreAdjMin-1))
67+
68+
_, adjustment, err := adjustOom(OOMScoreAdjMax)
69+
assert.NilError(t, err)
70+
assert.Check(t, is.Equal(adjustment, OOMScoreAdjMax))
71+
72+
score, err := GetOOMScoreAdj(os.Getpid())
73+
assert.NilError(t, err)
74+
if score == 0 || score == OOMScoreAdjMin {
75+
// we won't be able to set the score lower than the parent process,
76+
// so only test if parent process does not have a oom-score-adj
77+
_, adjustment, err = adjustOom(OOMScoreAdjMin)
78+
assert.NilError(t, err)
79+
assert.Check(t, is.Equal(adjustment, OOMScoreAdjMin))
80+
}
81+
}
82+
6783
func adjustOom(adjustment int) (int, int, error) {
6884
cmd := exec.Command("sleep", "100")
6985
if err := cmd.Start(); err != nil {

‎sys/oom_unsupported.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ const (
2525
OOMScoreAdjMax = 0
2626
)
2727

28+
// AdjustOOMScore sets the oom score for the provided pid. If the provided score
29+
// is out of range (-1000 - 1000), it is clipped to the min/max value.
30+
//
31+
// Not implemented on Windows
32+
func AdjustOOMScore(pid, score int) error {
33+
return nil
34+
}
35+
2836
// SetOOMScore sets the oom score for the process
2937
//
3038
// Not implemented on Windows

0 commit comments

Comments
 (0)