diff --git a/pkg/util/oom/oom_linux.go b/pkg/util/oom/oom_linux.go index 90a753c1695..c054682bdd7 100644 --- a/pkg/util/oom/oom_linux.go +++ b/pkg/util/oom/oom_linux.go @@ -20,7 +20,6 @@ package oom import ( "fmt" - "io/ioutil" "os" "path" "strconv" @@ -49,16 +48,6 @@ func getPids(cgroupName string) ([]int, error) { return fsManager.GetPids() } -func syscallNotExists(err error) bool { - if err == nil { - return false - } - if e, ok := err.(*os.SyscallError); ok && os.IsNotExist(e) { - return true - } - return false -} - // Writes 'value' to /proc//oom_score_adj. PID = 0 means self // Returns os.ErrNotExist if the `pid` does not exist. func applyOOMScoreAdj(pid int, oomScoreAdj int) error { @@ -78,12 +67,19 @@ func applyOOMScoreAdj(pid int, oomScoreAdj int) error { value := strconv.Itoa(oomScoreAdj) var err error for i := 0; i < maxTries; i++ { - if err = ioutil.WriteFile(oomScoreAdjPath, []byte(value), 0700); err != nil { - if syscallNotExists(err) { + f, err := os.Open(oomScoreAdjPath) + if err != nil { + if os.IsNotExist(err) { return os.ErrNotExist } err = fmt.Errorf("failed to apply oom-score-adj to pid %d (%v)", pid, err) + continue } + if _, err := f.Write([]byte(value)); err != nil { + err = fmt.Errorf("failed to apply oom-score-adj to pid %d (%v)", pid, err) + continue + } + return nil } return err } @@ -96,20 +92,26 @@ func (oomAdjuster *OOMAdjuster) applyOOMScoreAdjContainer(cgroupName string, oom continueAdjusting := false pidList, err := oomAdjuster.pidLister(cgroupName) if err != nil { - if syscallNotExists(err) { + if os.IsNotExist(err) { // Nothing to do since the container doesn't exist anymore. return os.ErrNotExist } continueAdjusting = true - glog.Errorf("Error getting process list for cgroup %s: %+v", cgroupName, err) + glog.V(10).Infof("Error getting process list for cgroup %s: %+v", cgroupName, err) } else if len(pidList) == 0 { + glog.V(10).Infof("Pid list is empty") continueAdjusting = true } else { for _, pid := range pidList { if !adjustedProcessSet[pid] { - continueAdjusting = true + glog.V(10).Infof("pid %d needs to be set", pid) if err = oomAdjuster.ApplyOOMScoreAdj(pid, oomScoreAdj); err == nil { adjustedProcessSet[pid] = true + } else if err == os.ErrNotExist { + continue + } else { + glog.V(10).Infof("cannot adjust oom score for pid %d - %v", pid, err) + continueAdjusting = true } // Processes can come and go while we try to apply oom score adjust value. So ignore errors here. } diff --git a/pkg/util/oom/oom_linux_test.go b/pkg/util/oom/oom_linux_test.go index c3a850d91be..10e9f3d9ec6 100644 --- a/pkg/util/oom/oom_linux_test.go +++ b/pkg/util/oom/oom_linux_test.go @@ -19,7 +19,10 @@ limitations under the License. package oom import ( + "os" "testing" + + "github.com/stretchr/testify/assert" ) // Converts a sequence of PID lists into a PID lister. @@ -62,10 +65,9 @@ func applyOOMScoreAdjContainerTester(pidListSequence [][]int, maxTries int, appl } else if err != nil { return } - // Check that OOM scores were applied to the right processes. if len(appliedPids) != len(pidOOMs) { - t.Errorf("Applied OOM scores to incorrect number of processes") + t.Errorf("Applied OOM scores to incorrect number of processes - %+v vs %v", appliedPids, pidOOMs) return } for _, pid := range appliedPids { @@ -82,29 +84,21 @@ func TestOOMScoreAdjContainer(t *testing.T) { pidListSequence1 := [][]int{ {1, 2}, } - applyOOMScoreAdjContainerTester(pidListSequence1, 1, nil, true, t) - applyOOMScoreAdjContainerTester(pidListSequence1, 2, []int{1, 2}, false, t) - applyOOMScoreAdjContainerTester(pidListSequence1, 3, []int{1, 2}, false, t) - - pidListSequence3 := [][]int{ - {1, 2}, - {1, 2, 4, 5}, - {2, 1, 4, 5, 3}, - } - applyOOMScoreAdjContainerTester(pidListSequence3, 1, nil, true, t) - applyOOMScoreAdjContainerTester(pidListSequence3, 2, nil, true, t) - applyOOMScoreAdjContainerTester(pidListSequence3, 3, nil, true, t) - applyOOMScoreAdjContainerTester(pidListSequence3, 4, []int{1, 2, 3, 4, 5}, false, t) + applyOOMScoreAdjContainerTester(pidListSequence1, 1, []int{1, 2}, false, t) pidListSequenceLag := [][]int{ {}, {}, {}, {1, 2, 4}, - {1, 2, 4, 5}, } - for i := 1; i < 5; i++ { + for i := 1; i < 4; i++ { applyOOMScoreAdjContainerTester(pidListSequenceLag, i, nil, true, t) } - applyOOMScoreAdjContainerTester(pidListSequenceLag, 6, []int{1, 2, 4, 5}, false, t) + applyOOMScoreAdjContainerTester(pidListSequenceLag, 4, []int{1, 2, 4}, false, t) +} + +func TestPidListerFailure(t *testing.T) { + _, err := getPids("/does/not/exist") + assert.True(t, os.IsNotExist(err), "expected getPids to return not exists error. Got %v", err) } diff --git a/pkg/util/procfs/procfs.go b/pkg/util/procfs/procfs.go index cc432255fb2..d0c38bf21e7 100644 --- a/pkg/util/procfs/procfs.go +++ b/pkg/util/procfs/procfs.go @@ -49,7 +49,7 @@ func (pfs *ProcFS) GetFullContainerName(pid int) (string, error) { filePath := path.Join("/proc", strconv.Itoa(pid), "cgroup") content, err := ioutil.ReadFile(filePath) if err != nil { - if e, ok := err.(*os.SyscallError); ok && os.IsNotExist(e) { + if os.IsNotExist(err) { return "", os.ErrNotExist } return "", err