Merge pull request #19927 from giuseppe/move-oom-clamp-at-start-time

libpod: move oom_score_adj clamp to init

libpod/container_internal_common.go

@@ -633,6 +633,13 @@ func (c *Container) generateSpec(ctx context.Context) (s *spec.Spec, cleanupFunc
 	nprocSet := false
 	isRootless := rootless.IsRootless()
 	if isRootless {
+		if g.Config.Process != nil && g.Config.Process.OOMScoreAdj != nil {
+			var err error
+			*g.Config.Process.OOMScoreAdj, err = maybeClampOOMScoreAdj(*g.Config.Process.OOMScoreAdj)
+			if err != nil {
+				return nil, nil, err
+			}
+		}
 		for _, rlimit := range c.config.Spec.Process.Rlimits {
 			if rlimit.Type == "RLIMIT_NOFILE" {
 				nofileSet = true
@@ -2938,3 +2945,19 @@ func (c *Container) umask() (uint32, error) {
 	}
 	return uint32(decVal), nil
 }
+
+func maybeClampOOMScoreAdj(oomScoreValue int) (int, error) {
+	v, err := os.ReadFile("/proc/self/oom_score_adj")
+	if err != nil {
+		return oomScoreValue, err
+	}
+	currentValue, err := strconv.Atoi(strings.TrimRight(string(v), "\n"))
+	if err != nil {
+		return oomScoreValue, err
+	}
+	if currentValue > oomScoreValue {
+		logrus.Warnf("Requested oom_score_adj=%d is lower than the current one, changing to %d", oomScoreValue, currentValue)
+		return currentValue, nil
+	}
+	return oomScoreValue, nil
+}

pkg/specgen/generate/oci_linux.go

@@ -4,9 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"os"
 	"path"
-	"strconv"
 	"strings"
 
 	"github.com/containers/common/libimage"
@@ -18,7 +16,6 @@ import (
 	"github.com/containers/podman/v4/pkg/specgen"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/runtime-tools/generate"
-	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
 
@@ -81,25 +78,6 @@ func getCgroupPermissions(unmask []string) string {
 	return ro
 }
 
-func maybeClampOOMScoreAdj(oomScoreValue int, isRootless bool) (int, error) {
-	if !isRootless {
-		return oomScoreValue, nil
-	}
-	v, err := os.ReadFile("/proc/self/oom_score_adj")
-	if err != nil {
-		return oomScoreValue, err
-	}
-	currentValue, err := strconv.Atoi(strings.TrimRight(string(v), "\n"))
-	if err != nil {
-		return oomScoreValue, err
-	}
-	if currentValue > oomScoreValue {
-		logrus.Warnf("Requested oom_score_adj=%d is lower than the current one, changing to %d", oomScoreValue, currentValue)
-		return currentValue, nil
-	}
-	return oomScoreValue, nil
-}
-
 // SpecGenToOCI returns the base configuration for the container.
 func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runtime, rtc *config.Config, newImage *libimage.Image, mounts []spec.Mount, pod *libpod.Pod, finalCmd []string, compatibleOptions *libpod.InfraInherit) (*spec.Spec, error) {
 	cgroupPerm := getCgroupPermissions(s.Unmask)
@@ -343,12 +321,9 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt
 	}
 
 	if s.OOMScoreAdj != nil {
-		score, err := maybeClampOOMScoreAdj(*s.OOMScoreAdj, isRootless)
+		g.SetProcessOOMScoreAdj(*s.OOMScoreAdj)
-		if err != nil {
-			return nil, err
-		}
-		g.SetProcessOOMScoreAdj(score)
 	}
+
 	setProcOpts(s, &g)
 	if s.ReadOnlyFilesystem && !s.ReadWriteTmpfs {
 		setDevOptsReadOnly(&g)

test/e2e/run_test.go

@@ -648,10 +648,17 @@ USER bin`, BB)
 
 		currentOOMScoreAdj, err := os.ReadFile("/proc/self/oom_score_adj")
 		Expect(err).ToNot(HaveOccurred())
-		session = podmanTest.Podman([]string{"run", "--rm", fedoraMinimal, "cat", "/proc/self/oom_score_adj"})
+		name := "ctr-with-oom-score"
+		session = podmanTest.Podman([]string{"create", "--name", name, fedoraMinimal, "cat", "/proc/self/oom_score_adj"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
-		Expect(session.OutputToString()).To(Equal(strings.TrimRight(string(currentOOMScoreAdj), "\n")))
+
+		for i := 0; i < 2; i++ {
+			session = podmanTest.Podman([]string{"start", "-a", name})
+			session.WaitWithDefaultTimeout()
+			Expect(session).Should(Exit(0))
+			Expect(session.OutputToString()).To(Equal(strings.TrimRight(string(currentOOMScoreAdj), "\n")))
+		}
 	})
 
 	It("podman run limits host test", func() {