Pids-limit should only be set if the user set it

Currently we are sending over pids-limits from the user even if they
never modified the defaults.  The pids limit should be set at the server
side unless modified by the user.

This issue has led to failures on systems that were running with cgroups V1.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

6 files changed, 43 insertions, 28 deletions

diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go
index f6fbe8e10..a26bbf718 100644
--- a/cmd/podman/common/create.go
+++ b/cmd/podman/common/create.go
@@ -330,8 +330,7 @@ func GetCreateFlags(cf *ContainerCLIOpts) *pflag.FlagSet {
 		"pid", "",
 		"PID namespace to use",
 	)
-	createFlags.Int64Var(
-		&cf.PIDsLimit,
+	createFlags.Int64(
 		"pids-limit", containerConfig.PidsLimit(),
 		"Tune container pids limit (set 0 for unlimited, -1 for server defaults)",
 	)
diff --git a/cmd/podman/common/create_opts.go b/cmd/podman/common/create_opts.go
index eafe7f090..a544846aa 100644
--- a/cmd/podman/common/create_opts.go
+++ b/cmd/podman/common/create_opts.go
@@ -66,7 +66,7 @@ type ContainerCLIOpts struct {
 	OverrideArch      string
 	OverrideOS        string
 	PID               string
-	PIDsLimit         int64
+	PIDsLimit         *int64
 	Pod               string
 	PodIDFile         string
 	PreserveFDs       uint
diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go
index eca0da32b..8d051ead7 100644
--- a/cmd/podman/common/specgen.go
+++ b/cmd/podman/common/specgen.go
@@ -7,14 +7,12 @@ import (
 	"strings"
 	"time"
 
-	"github.com/containers/common/pkg/config"
 	"github.com/containers/image/v5/manifest"
 	"github.com/containers/libpod/v2/cmd/podman/parse"
 	"github.com/containers/libpod/v2/libpod/define"
 	ann "github.com/containers/libpod/v2/pkg/annotations"
 	envLib "github.com/containers/libpod/v2/pkg/env"
 	ns "github.com/containers/libpod/v2/pkg/namespaces"
-	"github.com/containers/libpod/v2/pkg/rootless"
 	"github.com/containers/libpod/v2/pkg/specgen"
 	systemdGen "github.com/containers/libpod/v2/pkg/systemd/generate"
 	"github.com/containers/libpod/v2/pkg/util"
@@ -127,25 +125,6 @@ func getIOLimits(s *specgen.SpecGenerator, c *ContainerCLIOpts) (*specs.LinuxBlo
 	return io, nil
 }
 
-func getPidsLimits(c *ContainerCLIOpts) *specs.LinuxPids {
-	pids := &specs.LinuxPids{}
-	if c.CGroupsMode == "disabled" && c.PIDsLimit != 0 {
-		return nil
-	}
-	if c.PIDsLimit < 0 {
-		if rootless.IsRootless() && containerConfig.Engine.CgroupManager != config.SystemdCgroupsManager {
-			return nil
-		}
-		pids.Limit = containerConfig.PidsLimit()
-		return pids
-	}
-	if c.PIDsLimit > 0 {
-		pids.Limit = c.PIDsLimit
-		return pids
-	}
-	return nil
-}
-
 func getMemoryLimits(s *specgen.SpecGenerator, c *ContainerCLIOpts) (*specs.LinuxMemory, error) {
 	var err error
 	memory := &specs.LinuxMemory{}
@@ -457,7 +436,13 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string
 	if err != nil {
 		return err
 	}
-	s.ResourceLimits.Pids = getPidsLimits(c)
+	if c.PIDsLimit != nil {
+		pids := specs.LinuxPids{
+			Limit: *c.PIDsLimit,
+		}
+
+		s.ResourceLimits.Pids = &pids
+	}
 	s.ResourceLimits.CPU = getCPULimits(c)
 	if s.ResourceLimits.CPU == nil && s.ResourceLimits.Pids == nil && s.ResourceLimits.BlockIO == nil && s.ResourceLimits.Memory == nil {
 		s.ResourceLimits = nil
diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go
index a44c0406f..9c9edb14f 100644
--- a/cmd/podman/containers/create.go
+++ b/cmd/podman/containers/create.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"strconv"
 	"strings"
 
 	"github.com/containers/common/pkg/config"
@@ -195,13 +196,18 @@ func createInit(c *cobra.Command) error {
 	cliVals.UTS = c.Flag("uts").Value.String()
 	cliVals.PID = c.Flag("pid").Value.String()
 	cliVals.CGroupsNS = c.Flag("cgroupns").Value.String()
-	if !c.Flag("pids-limit").Changed {
-		cliVals.PIDsLimit = -1
-	}
 	if c.Flag("entrypoint").Changed {
 		val := c.Flag("entrypoint").Value.String()
 		cliVals.Entrypoint = &val
 	}
+	if c.Flags().Changed("pids-limit") {
+		val := c.Flag("pids-limit").Value.String()
+		pidsLimit, err := strconv.ParseInt(val, 10, 32)
+		if err != nil {
+			return err
+		}
+		cliVals.PIDsLimit = &pidsLimit
+	}
 	if c.Flags().Changed("env") {
 		env, err := c.Flags().GetStringArray("env")
 		if err != nil {
diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go
index dee79cf67..f0d52d0c3 100644
--- a/pkg/specgen/generate/container.go
+++ b/pkg/specgen/generate/container.go
@@ -10,6 +10,7 @@ import (
 	envLib "github.com/containers/libpod/v2/pkg/env"
 	"github.com/containers/libpod/v2/pkg/signal"
 	"github.com/containers/libpod/v2/pkg/specgen"
+	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
 )
@@ -169,6 +170,21 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 		}
 	}
 
+	// If caller did not specify Pids Limits load default
+	if s.ResourceLimits == nil || s.ResourceLimits.Pids == nil {
+		if s.CgroupsMode != "disabled" {
+			limit := rtc.PidsLimit()
+			if limit != 0 {
+				if s.ResourceLimits == nil {
+					s.ResourceLimits = &spec.LinuxResources{}
+				}
+				s.ResourceLimits.Pids = &spec.LinuxPids{
+					Limit: limit,
+				}
+			}
+		}
+	}
+
 	return verifyContainerResources(s)
 }
 
diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go
index 6cbedb457..9d48f1540 100644
--- a/test/e2e/run_test.go
+++ b/test/e2e/run_test.go
@@ -1072,4 +1072,13 @@ USER mail`
 		Expect(session.OutputToString()).To(ContainSubstring(h))
 
 	})
+
+	It("podman run verify pids-limit", func() {
+		SkipIfCgroupV1()
+		limit := "4321"
+		session := podmanTest.Podman([]string{"run", "--pids-limit", limit, "--rm", ALPINE, "cat", "/sys/fs/cgroup/pids.max"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(ContainSubstring(limit))
+	})
 })