From c4023a9302c81f04865646d765caf58ccf556cae Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Mon, 6 Jul 2020 14:14:48 -0400 Subject: 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 --- cmd/podman/common/create.go | 3 +-- cmd/podman/common/create_opts.go | 2 +- cmd/podman/common/specgen.go | 29 +++++++---------------------- cmd/podman/containers/create.go | 12 +++++++++--- pkg/specgen/generate/container.go | 16 ++++++++++++++++ 5 files changed, 34 insertions(+), 28 deletions(-) diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go index b7940fb87..ee7f957cc 100644 --- a/cmd/podman/common/create.go +++ b/cmd/podman/common/create.go @@ -329,8 +329,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 f577a0875..08ffa5544 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 Privileged bool diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go index ae61e5283..b4f786da2 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{} @@ -454,7 +433,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 42cee022e..dffa9d7fe 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" @@ -194,13 +195,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, 0, 10) + 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) } -- cgit v1.2.3-54-g00ecf