Skip to content

Commit

Permalink
Merge pull request #17355 from openshift-cherrypick-robot/cherry-pick…
Browse files Browse the repository at this point in the history
…-17262-to-v4.4

[v4.4] Fix default handling of pids-limit
  • Loading branch information
openshift-merge-robot authored Feb 3, 2023
2 parents 3988540 + 322802e commit 9c5b249
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 14 deletions.
3 changes: 1 addition & 2 deletions cmd/podman/common/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
)
Expand Down
1 change: 0 additions & 1 deletion cmd/podman/common/create_opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,5 +92,4 @@ func DefineCreateDefaults(opts *entities.ContainerCreateOptions) {
opts.Ulimit = ulimits()
opts.SeccompPolicy = "default"
opts.Volume = volumes()
opts.PIDsLimit = &podmanConfig.ContainersConf.Containers.PidsLimit
}
32 changes: 21 additions & 11 deletions cmd/podman/containers/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions cmd/podman/containers/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
27 changes: 27 additions & 0 deletions test/e2e/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

0 comments on commit 9c5b249

Please sign in to comment.