diff --git a/runtime/v1/shim/client/client.go b/runtime/v1/shim/client/client.go index 9211c0052..a8afb0e48 100644 --- a/runtime/v1/shim/client/client.go +++ b/runtime/v1/shim/client/client.go @@ -182,10 +182,7 @@ func setupOOMScore(shimPid int) error { return errors.Wrap(err, "get daemon OOM score") } shimScore := score + 1 - if shimScore > sys.OOMScoreAdjMax { - shimScore = sys.OOMScoreAdjMax - } - if err := sys.SetOOMScore(shimPid, shimScore); err != nil { + if err := sys.AdjustOOMScore(shimPid, shimScore); err != nil { return errors.Wrap(err, "set shim OOM score") } return nil diff --git a/runtime/v2/shim/util_unix.go b/runtime/v2/shim/util_unix.go index f4c7cfa55..f956b0986 100644 --- a/runtime/v2/shim/util_unix.go +++ b/runtime/v2/shim/util_unix.go @@ -46,11 +46,6 @@ func getSysProcAttr() *syscall.SysProcAttr { } } -// SetScore sets the oom score for a process -func SetScore(pid int) error { - return sys.SetOOMScore(pid, sys.OOMScoreMaxKillable) -} - // AdjustOOMScore sets the OOM score for the process to the parents OOM score +1 // to ensure that they parent has a lower* score than the shim // if not already at the maximum OOM Score @@ -61,10 +56,7 @@ func AdjustOOMScore(pid int) error { return errors.Wrap(err, "get parent OOM score") } shimScore := score + 1 - if shimScore > sys.OOMScoreAdjMax { - shimScore = sys.OOMScoreAdjMax - } - if err := sys.SetOOMScore(pid, shimScore); err != nil { + if err := sys.AdjustOOMScore(pid, shimScore); err != nil { return errors.Wrap(err, "set shim OOM score") } return nil diff --git a/sys/env.go b/sys/env.go deleted file mode 100644 index 8450d6275..000000000 --- a/sys/env.go +++ /dev/null @@ -1,33 +0,0 @@ -// +build !windows - -/* - Copyright The containerd 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 sys - -import "golang.org/x/sys/unix" - -// RunningPrivileged returns true if the effective user ID of the -// calling process is 0 -func RunningPrivileged() bool { - return unix.Geteuid() == 0 -} - -// RunningUnprivileged returns true if the effective user ID of the -// calling process is not 0 -func RunningUnprivileged() bool { - return !RunningPrivileged() -} diff --git a/sys/mount_linux_test.go b/sys/mount_linux_test.go index 8210c837e..38745f497 100644 --- a/sys/mount_linux_test.go +++ b/sys/mount_linux_test.go @@ -32,7 +32,7 @@ import ( type fMountatCaseFunc func(t *testing.T, root string) func TestFMountat(t *testing.T) { - if RunningUnprivileged() { + if !runningPrivileged() { t.Skip("Needs to be run as root") return } diff --git a/sys/oom_linux.go b/sys/oom_linux.go index f24e9af97..6e759a0b1 100644 --- a/sys/oom_linux.go +++ b/sys/oom_linux.go @@ -24,17 +24,32 @@ import ( "strings" "github.com/containerd/containerd/pkg/userns" + "golang.org/x/sys/unix" ) const ( - // OOMScoreMaxKillable is the maximum score keeping the process killable by the oom killer - OOMScoreMaxKillable = -999 - // OOMScoreAdjMax is from OOM_SCORE_ADJ_MAX https://github.com/torvalds/linux/blob/master/include/uapi/linux/oom.h + // OOMScoreAdjMin is from OOM_SCORE_ADJ_MIN https://github.com/torvalds/linux/blob/v5.10/include/uapi/linux/oom.h#L9 + OOMScoreAdjMin = -1000 + // OOMScoreAdjMax is from OOM_SCORE_ADJ_MAX https://github.com/torvalds/linux/blob/v5.10/include/uapi/linux/oom.h#L10 OOMScoreAdjMax = 1000 ) +// AdjustOOMScore sets the oom score for the provided pid. If the provided score +// is out of range (-1000 - 1000), it is clipped to the min/max value. +func AdjustOOMScore(pid, score int) error { + if score > OOMScoreAdjMax { + score = OOMScoreAdjMax + } else if score < OOMScoreAdjMin { + score = OOMScoreAdjMin + } + return SetOOMScore(pid, score) +} + // SetOOMScore sets the oom score for the provided pid func SetOOMScore(pid, score int) error { + if score > OOMScoreAdjMax || score < OOMScoreAdjMin { + return fmt.Errorf("value out of range (%d): OOM score must be between %d and %d", score, OOMScoreAdjMin, OOMScoreAdjMax) + } path := fmt.Sprintf("/proc/%d/oom_score_adj", pid) f, err := os.OpenFile(path, os.O_WRONLY, 0) if err != nil { @@ -42,7 +57,7 @@ func SetOOMScore(pid, score int) error { } defer f.Close() if _, err = f.WriteString(strconv.Itoa(score)); err != nil { - if os.IsPermission(err) && (userns.RunningInUserNS() || RunningUnprivileged()) { + if os.IsPermission(err) && (!runningPrivileged() || userns.RunningInUserNS()) { return nil } return err @@ -59,3 +74,9 @@ func GetOOMScoreAdj(pid int) (int, error) { } return strconv.Atoi(strings.TrimSpace(string(data))) } + +// runningPrivileged returns true if the effective user ID of the +// calling process is 0 +func runningPrivileged() bool { + return unix.Geteuid() == 0 +} diff --git a/sys/oom_linux_test.go b/sys/oom_linux_test.go index 2b824eb70..bcbb1bc04 100644 --- a/sys/oom_linux_test.go +++ b/sys/oom_linux_test.go @@ -18,52 +18,68 @@ package sys import ( "errors" + "fmt" "os" "os/exec" "testing" "time" + "github.com/containerd/containerd/pkg/userns" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" ) func TestSetPositiveOomScoreAdjustment(t *testing.T) { + // Setting a *positive* OOM score adjust does not require privileged _, adjustment, err := adjustOom(123) - if err != nil { - t.Error(err) - return - } + assert.NilError(t, err) assert.Check(t, is.Equal(adjustment, 123)) } func TestSetNegativeOomScoreAdjustmentWhenPrivileged(t *testing.T) { - if RunningUnprivileged() { - t.Skip("Needs to be run as root") + if !runningPrivileged() || userns.RunningInUserNS() { + t.Skip("requires root and not running in user namespace") return } _, adjustment, err := adjustOom(-123) - if err != nil { - t.Error(err) - return - } + assert.NilError(t, err) assert.Check(t, is.Equal(adjustment, -123)) } func TestSetNegativeOomScoreAdjustmentWhenUnprivilegedHasNoEffect(t *testing.T) { - if RunningPrivileged() { - t.Skip("Needs to be run as non-root") + if runningPrivileged() && !userns.RunningInUserNS() { + t.Skip("needs to be run as non-root or in user namespace") return } initial, adjustment, err := adjustOom(-123) - if err != nil { - t.Error(err) - return - } + assert.NilError(t, err) assert.Check(t, is.Equal(adjustment, initial)) } +func TestSetOOMScoreBoundaries(t *testing.T) { + err := SetOOMScore(0, OOMScoreAdjMax+1) + assert.ErrorContains(t, err, fmt.Sprintf("value out of range (%d): OOM score must be between", OOMScoreAdjMax+1)) + + err = SetOOMScore(0, OOMScoreAdjMin-1) + assert.ErrorContains(t, err, fmt.Sprintf("value out of range (%d): OOM score must be between", OOMScoreAdjMin-1)) + + _, adjustment, err := adjustOom(OOMScoreAdjMax) + assert.NilError(t, err) + assert.Check(t, is.Equal(adjustment, OOMScoreAdjMax)) + + score, err := GetOOMScoreAdj(os.Getpid()) + assert.NilError(t, err) + if score == 0 || score == OOMScoreAdjMin { + // we won't be able to set the score lower than the parent process, + // so only test if parent process does not have a oom-score-adj + _, adjustment, err = adjustOom(OOMScoreAdjMin) + assert.NilError(t, err) + assert.Check(t, is.Equal(adjustment, OOMScoreAdjMin)) + } +} + func adjustOom(adjustment int) (int, int, error) { cmd := exec.Command("sleep", "100") if err := cmd.Start(); err != nil { diff --git a/sys/oom_unsupported.go b/sys/oom_unsupported.go index 558340790..f5d7e9786 100644 --- a/sys/oom_unsupported.go +++ b/sys/oom_unsupported.go @@ -25,6 +25,14 @@ const ( OOMScoreAdjMax = 0 ) +// AdjustOOMScore sets the oom score for the provided pid. If the provided score +// is out of range (-1000 - 1000), it is clipped to the min/max value. +// +// Not implemented on Windows +func AdjustOOMScore(pid, score int) error { + return nil +} + // SetOOMScore sets the oom score for the process // // Not implemented on Windows