Merge pull request #18721 from Cydox/fix-ulimit-pr

fix ulimit issue

libpod/container_internal_common.go

@@ -56,6 +56,7 @@ import (
 	"github.com/opencontainers/selinux/go-selinux"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/sirupsen/logrus"
+	"golang.org/x/sys/unix"
 )
 
 func parseOptionIDs(ctrMappings []idtools.IDMap, option string) ([]idtools.IDMap, error) {
@@ -624,6 +625,50 @@ func (c *Container) generateSpec(ctx context.Context) (s *spec.Spec, cleanupFunc
 		}
 	}
 
+	// setup rlimits
+	nofileSet := false
+	nprocSet := false
+	isRootless := rootless.IsRootless()
+	if isRootless {
+		for _, rlimit := range c.config.Spec.Process.Rlimits {
+			if rlimit.Type == "RLIMIT_NOFILE" {
+				nofileSet = true
+			} else if rlimit.Type == "RLIMIT_NPROC" {
+				nprocSet = true
+			}
+		}
+		if !nofileSet {
+			max := rlimT(define.RLimitDefaultValue)
+			current := rlimT(define.RLimitDefaultValue)
+			var rlimit unix.Rlimit
+			if err := unix.Getrlimit(unix.RLIMIT_NOFILE, &rlimit); err != nil {
+				logrus.Warnf("Failed to return RLIMIT_NOFILE ulimit %q", err)
+			}
+			if rlimT(rlimit.Cur) < current {
+				current = rlimT(rlimit.Cur)
+			}
+			if rlimT(rlimit.Max) < max {
+				max = rlimT(rlimit.Max)
+			}
+			g.AddProcessRlimits("RLIMIT_NOFILE", uint64(max), uint64(current))
+		}
+		if !nprocSet {
+			max := rlimT(define.RLimitDefaultValue)
+			current := rlimT(define.RLimitDefaultValue)
+			var rlimit unix.Rlimit
+			if err := unix.Getrlimit(unix.RLIMIT_NPROC, &rlimit); err != nil {
+				logrus.Warnf("Failed to return RLIMIT_NPROC ulimit %q", err)
+			}
+			if rlimT(rlimit.Cur) < current {
+				current = rlimT(rlimit.Cur)
+			}
+			if rlimT(rlimit.Max) < max {
+				max = rlimT(rlimit.Max)
+			}
+			g.AddProcessRlimits("RLIMIT_NPROC", uint64(max), uint64(current))
+		}
+	}
+
 	return g.Config, cleanupFunc, nil
 }
 

libpod/rlimit_int64.go

@@ -1,6 +1,6 @@
 //go:build freebsd
 // +build freebsd
 
-package generate
+package libpod
 
 type rlimT int64

libpod/rlimit_uint64.go

@@ -1,6 +1,6 @@
 //go:build linux || darwin
 // +build linux darwin
 
-package generate
+package libpod
 
 type rlimT uint64

pkg/specgen/container_validate.go

@@ -3,13 +3,11 @@ package specgen
 import (
 	"errors"
 	"fmt"
-	"strconv"
 	"strings"
 
 	"github.com/containers/common/pkg/util"
 	"github.com/containers/podman/v4/libpod/define"
 	"github.com/containers/podman/v4/pkg/rootless"
-	"github.com/opencontainers/runtime-spec/specs-go"
 )
 
 var (
@@ -135,38 +133,6 @@ func (s *SpecGenerator) Validate() error {
 	// default:
 	//	return errors.New("unrecognized option for cgroups; supported are 'default', 'disabled', 'no-conmon'")
 	// }
-	invalidUlimitFormatError := errors.New("invalid default ulimit definition must be form of type=soft:hard")
-	// set ulimits if not rootless
-	if len(s.ContainerResourceConfig.Rlimits) < 1 && !rootless.IsRootless() {
-		// Containers common defines this as something like nproc=4194304:4194304
-		tmpnproc := containerConfig.Ulimits()
-		var posixLimits []specs.POSIXRlimit
-		for _, limit := range tmpnproc {
-			limitSplit := strings.SplitN(limit, "=", 2)
-			if len(limitSplit) < 2 {
-				return fmt.Errorf("missing = in %s: %w", limit, invalidUlimitFormatError)
-			}
-			valueSplit := strings.SplitN(limitSplit[1], ":", 2)
-			if len(valueSplit) < 2 {
-				return fmt.Errorf("missing : in %s: %w", limit, invalidUlimitFormatError)
-			}
-			hard, err := strconv.Atoi(valueSplit[0])
-			if err != nil {
-				return err
-			}
-			soft, err := strconv.Atoi(valueSplit[1])
-			if err != nil {
-				return err
-			}
-			posixLimit := specs.POSIXRlimit{
-				Type: limitSplit[0],
-				Hard: uint64(hard),
-				Soft: uint64(soft),
-			}
-			posixLimits = append(posixLimits, posixLimit)
-		}
-		s.ContainerResourceConfig.Rlimits = posixLimits
-	}
 	// Namespaces
 	if err := s.UtsNS.validate(); err != nil {
 		return err

pkg/specgen/generate/oci.go

@@ -7,72 +7,17 @@ import (
 	"github.com/containers/common/libimage"
 	"github.com/containers/common/pkg/config"
 	"github.com/containers/podman/v4/libpod/define"
-	"github.com/containers/podman/v4/pkg/rootless"
 	"github.com/containers/podman/v4/pkg/specgen"
 	"github.com/opencontainers/runtime-tools/generate"
-	"github.com/sirupsen/logrus"
-	"golang.org/x/sys/unix"
 )
 
 func addRlimits(s *specgen.SpecGenerator, g *generate.Generator) {
-	var (
+	g.Config.Process.Rlimits = nil
-		isRootless = rootless.IsRootless()
-		nofileSet  = false
-		nprocSet   = false
-	)
-
-	if s.Rlimits == nil {
-		g.Config.Process.Rlimits = nil
-		return
-	}
 
 	for _, u := range s.Rlimits {
 		name := "RLIMIT_" + strings.ToUpper(u.Type)
-		if name == "RLIMIT_NOFILE" {
-			nofileSet = true
-		} else if name == "RLIMIT_NPROC" {
-			nprocSet = true
-		}
 		g.AddProcessRlimits(name, u.Hard, u.Soft)
 	}
-
-	// If not explicitly overridden by the user, default number of open
-	// files and number of processes to the maximum they can be set to
-	// (without overriding a sysctl)
-	if !nofileSet {
-		max := rlimT(define.RLimitDefaultValue)
-		current := rlimT(define.RLimitDefaultValue)
-		if isRootless {
-			var rlimit unix.Rlimit
-			if err := unix.Getrlimit(unix.RLIMIT_NOFILE, &rlimit); err != nil {
-				logrus.Warnf("Failed to return RLIMIT_NOFILE ulimit %q", err)
-			}
-			if rlimT(rlimit.Cur) < current {
-				current = rlimT(rlimit.Cur)
-			}
-			if rlimT(rlimit.Max) < max {
-				max = rlimT(rlimit.Max)
-			}
-		}
-		g.AddProcessRlimits("RLIMIT_NOFILE", uint64(max), uint64(current))
-	}
-	if !nprocSet {
-		max := rlimT(define.RLimitDefaultValue)
-		current := rlimT(define.RLimitDefaultValue)
-		if isRootless {
-			var rlimit unix.Rlimit
-			if err := unix.Getrlimit(unix.RLIMIT_NPROC, &rlimit); err != nil {
-				logrus.Warnf("Failed to return RLIMIT_NPROC ulimit %q", err)
-			}
-			if rlimT(rlimit.Cur) < current {
-				current = rlimT(rlimit.Cur)
-			}
-			if rlimT(rlimit.Max) < max {
-				max = rlimT(rlimit.Max)
-			}
-		}
-		g.AddProcessRlimits("RLIMIT_NPROC", uint64(max), uint64(current))
-	}
 }
 
 // Produce the final command for the container.

test/e2e/inspect_test.go

@@ -489,6 +489,34 @@ var _ = Describe("Podman inspect", func() {
 		Expect(found).To(BeTrue(), "found RLIMIT_CORE")
 	})
 
+	It("Container inspect Ulimit test", func() {
+		SkipIfNotRootless("Only applicable to rootless")
+		ctrName := "testctr"
+		session := podmanTest.Podman([]string{"create", "--name", ctrName, ALPINE, "top"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		inspect := podmanTest.Podman([]string{"inspect", ctrName})
+		inspect.WaitWithDefaultTimeout()
+		Expect(inspect).Should(Exit(0))
+
+		dataCreate := inspect.InspectContainerToJSON()
+		ulimitsCreate := dataCreate[0].HostConfig.Ulimits
+		Expect(ulimitsCreate).To(BeEmpty())
+
+		start := podmanTest.Podman([]string{"start", ctrName})
+		start.WaitWithDefaultTimeout()
+		Expect(start).Should(Exit(0))
+
+		inspect2 := podmanTest.Podman([]string{"inspect", ctrName})
+		inspect2.WaitWithDefaultTimeout()
+		Expect(inspect2).Should(Exit(0))
+
+		dataStart := inspect2.InspectContainerToJSON()
+		ulimitsStart := dataStart[0].HostConfig.Ulimits
+		Expect(ulimitsStart).ToNot(BeEmpty())
+	})
+
 	It("Dropped capabilities are sorted", func() {
 		ctrName := "testCtr"
 		session := podmanTest.Podman([]string{"run", "-d", "--cap-drop", "SETUID", "--cap-drop", "SETGID", "--cap-drop", "CAP_NET_BIND_SERVICE", "--name", ctrName, ALPINE, "top"})

test/system/045-start.bats

@@ -99,4 +99,43 @@ load helpers
     run_podman rm -t 0 -f $ctrID $cname
 }
 
+@test "podman start again with lower ulimit -u" {
+    skip_if_not_rootless "tests ulimit -u changes in the rootless scenario"
+    skip_if_remote "test relies on control of ulimit -u (not possible for remote)"
+    # get current ulimit -u
+    nrpoc_limit=$(ulimit -Hu)
+
+    # create container
+    run_podman create $IMAGE echo "hello"
+    ctrID="$output"
+
+    # inspect
+    run_podman inspect $ctrID
+    assert "$output" =~ '"Ulimits": \[\]' "Ulimits has to be empty after create"
+
+    # start container for the first time
+    run_podman start $ctrID
+    is "$output" "$ctrID"
+
+    # inspect
+    run_podman inspect $ctrID --format '{{range .HostConfig.Ulimits}}{{if eq .Name "RLIMIT_NPROC" }}{{.Soft}}:{{.Hard}}{{end}}{{end}}'
+    assert "$output" == "${nrpoc_limit}:${nrpoc_limit}" "Ulimit has to match ulimit -Hu"
+
+    # lower ulimit -u by one
+    ((nrpoc_limit--))
+
+    # set new ulimit -u
+    ulimit -u $nrpoc_limit
+
+    # start container for the second time
+    run_podman start $ctrID
+    is "$output" "$ctrID"
+
+    # inspect
+    run_podman inspect $ctrID --format '{{range .HostConfig.Ulimits}}{{if eq .Name "RLIMIT_NPROC" }}{{.Soft}}:{{.Hard}}{{end}}{{end}}'
+    assert "$output" == "${nrpoc_limit}:${nrpoc_limit}" "Ulimit has to match new ulimit -Hu"
+
+    run_podman rm -t 0 -f $ctrID $cname
+}
+
 # vim: filetype=sh