Fix default handling of pids-limit

Add test to verify that updates without a pids-limit specified no longer
overwrite the previous value.

Also fixes erroneous warning generated by remote clients:

"Resource limits are not supported and ignored on cgroups V1 rootless
systems"

Signed-off-by: Jason T. Greene <[email protected]>

Author: n1hility

cmd/podman/common/create.go

@@ -893,8 +893,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
 		_ = cmd.RegisterFlagCompletionFunc(deviceWriteIopsFlagName, completion.AutocompleteDefault)
 
 		pidsLimitFlagName := "pids-limit"
-		createFlags.Int64Var(
-			cf.PIDsLimit,
+		createFlags.Int64(
 			pidsLimitFlagName, pidsLimit(),
 			"Tune container pids limit (set -1 for unlimited)",
 		)

cmd/podman/common/create_opts.go

@@ -92,5 +92,4 @@ func DefineCreateDefaults(opts *entities.ContainerCreateOptions) {
 	opts.Ulimit = ulimits()
 	opts.SeccompPolicy = "default"
 	opts.Volume = volumes()
-	opts.PIDsLimit = &podmanConfig.ContainersConf.Containers.PidsLimit
 }

cmd/podman/containers/create.go

@@ -196,6 +196,23 @@ func replaceContainer(name string) error {
 	return removeContainers([]string{name}, rmOptions, false)
 }
 
+func createOrUpdateFlags(cmd *cobra.Command, vals *entities.ContainerCreateOptions) error {
+	if cmd.Flags().Changed("pids-limit") {
+		val := cmd.Flag("pids-limit").Value.String()
+		// Convert -1 to 0, so that -1 maps to unlimited pids limit
+		if val == "-1" {
+			val = "0"
+		}
+		pidsLimit, err := strconv.ParseInt(val, 10, 32)
+		if err != nil {
+			return err
+		}
+		vals.PIDsLimit = &pidsLimit
+	}
+
+	return nil
+}
+
 func CreateInit(c *cobra.Command, vals entities.ContainerCreateOptions, isInfra bool) (entities.ContainerCreateOptions, error) {
 	if len(vals.UIDMap) > 0 || len(vals.GIDMap) > 0 || vals.SubUIDName != "" || vals.SubGIDName != "" {
 		if c.Flag("userns").Changed {
@@ -255,18 +272,11 @@ func CreateInit(c *cobra.Command, vals entities.ContainerCreateOptions, isInfra
 			}
 			vals.OOMScoreAdj = &val
 		}
-		if c.Flags().Changed("pids-limit") {
-			val := c.Flag("pids-limit").Value.String()
-			// Convert -1 to 0, so that -1 maps to unlimited pids limit
-			if val == "-1" {
-				val = "0"
-			}
-			pidsLimit, err := strconv.ParseInt(val, 10, 32)
-			if err != nil {
-				return vals, err
-			}
-			vals.PIDsLimit = &pidsLimit
+
+		if err := createOrUpdateFlags(c, &vals); err != nil {
+			return vals, err
 		}
+
 		if c.Flags().Changed("env") {
 			env, err := c.Flags().GetStringArray("env")
 			if err != nil {

cmd/podman/containers/update.go

@@ -65,6 +65,11 @@ func update(cmd *cobra.Command, args []string) error {
 	s := &specgen.SpecGenerator{}
 	s.ResourceLimits = &specs.LinuxResources{}
 
+	err = createOrUpdateFlags(cmd, &updateOpts)
+	if err != nil {
+		return err
+	}
+
 	// we need to pass the whole specgen since throttle devices are parsed later due to cross compat.
 	s.ResourceLimits, err = specgenutil.GetResources(s, &updateOpts)
 	if err != nil {

test/e2e/update_test.go

@@ -98,6 +98,33 @@ var _ = Describe("Podman update", func() {
 
 	})
 
+	It("podman update container unspecified pid limit", func() {
+		SkipIfCgroupV1("testing flags that only work in cgroup v2")
+		SkipIfRootless("many of these handlers are not enabled while rootless in CI")
+		session := podmanTest.Podman([]string{"run", "-dt", "--pids-limit", "-1", ALPINE})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		ctrID := session.OutputToString()
+
+		commonArgs := []string{
+			"update",
+			"--cpus", "5",
+			ctrID}
+
+		session = podmanTest.Podman(commonArgs)
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		ctrID = session.OutputToString()
+
+		// checking pids-limit was not changed after update when not specified as an option
+		session = podmanTest.Podman([]string{"exec", "-it", ctrID, "cat", "/sys/fs/cgroup/pids.max"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		Expect(session.OutputToString()).Should(ContainSubstring("max"))
+	})
+
 	It("podman update container all options v2", func() {
 		SkipIfCgroupV1("testing flags that only work in cgroup v2")
 		SkipIfRootless("many of these handlers are not enabled while rootless in CI")