podman-run/docker-run --pids-limit inconsistency when unlimited

[jerboaa]

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind bug

Description
podman run --pids-limit=<unlimited> is inconsistent with docker-run. man docker-run mentions this for --pids-limit:

       --pids-limit=""
          Tune the container's pids (process IDs) limit. Set to -1 to have unlimited pids for the container.

Contrast this to what man podman-run states:

   --pids-limit=limit
       Tune the container's pids limit. Set to 0 to have unlimited pids for the container. The default is 4096 on systems that support "pids" cgroup controller.

Note the -1 vs. 0 difference for unlimited.

Steps to reproduce the issue:

# whoami
root
# podman run --rm -ti --pids-limit=-1 fedora:34
Error: OCI runtime error: writing file `pids.max`: Invalid argument

Describe the results you received:

Error: OCI runtime error: writing file `pids.max`: Invalid argument

Describe the results you expected:
Starts the container with unlimited pids-limit just like docker run does.

# docker run --rm -ti --pids-limit=-1 fedora:34
[root@35fb9551de8c /]# cat /sys/fs/cgroup/pids/pids.max 
38006

Additional information you deem important (e.g. issue happens only occasionally):
--pids-limit=0 works fine, but seems inconsistent and arbitrary comparing to docker. Isn't it supposed to be a drop-in replacement?

Output of podman version:

podman version 3.3.1

Output of podman info --debug:

host:
  arch: amd64
  buildahVersion: 1.22.3
  cgroupControllers:
  - cpuset
  - cpu
  - cpuacct
  - blkio
  - memory
  - devices
  - freezer
  - net_cls
  - perf_event
  - net_prio
  - hugetlb
  - pids
  - misc
  cgroupManager: systemd
  cgroupVersion: v1
  conmon:
    package: conmon-2.0.29-2.fc34.x86_64
    path: /usr/bin/conmon
    version: 'conmon version 2.0.29, commit: '
  cpus: 8
  distribution:
    distribution: fedora
    version: "34"
  eventLogger: journald
  hostname: t580-laptop
  idMappings:
    gidmap: null
    uidmap: null
  kernel: 5.13.19-200.fc34.x86_64
  linkmode: dynamic
  memFree: 13222797312
  memTotal: 33255153664
  ociRuntime:
    name: crun
    package: crun-1.0-1.fc34.x86_64
    path: /usr/bin/crun
    version: |-
      crun version 1.0
      commit: 139dc6971e2f1d931af520188763e984d6cdfbf8
      spec: 1.0.0
      +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +CRIU +YAJL
  os: linux
  remoteSocket:
    path: /run/podman/podman.sock
  security:
    apparmorEnabled: false
    capabilities: CAP_CHOWN,CAP_DAC_OVERRIDE,CAP_FOWNER,CAP_FSETID,CAP_KILL,CAP_NET_BIND_SERVICE,CAP_SETFCAP,CAP_SETGID,CAP_SETPCAP,CAP_SETUID,CAP_SYS_CHROOT
    rootless: false
    seccompEnabled: true
    seccompProfilePath: /usr/share/containers/seccomp.json
    selinuxEnabled: true
  serviceIsRemote: false
  slirp4netns:
    executable: /usr/bin/slirp4netns
    package: slirp4netns-1.1.12-2.fc34.x86_64
    version: |-
      slirp4netns version 1.1.12
      commit: 7a104a101aa3278a2152351a082a6df71f57c9a3
      libslirp: 4.4.0
      SLIRP_CONFIG_VERSION_MAX: 3
      libseccomp: 2.5.0
  swapFree: 25767698432
  swapTotal: 25767698432
  uptime: 3h 49m 37.09s (Approximately 0.12 days)
registries:
  search:
  - registry.fedoraproject.org
  - registry.access.redhat.com
  - docker.io
  - quay.io
store:
  configFile: /etc/containers/storage.conf
  containerStore:
    number: 25
    paused: 0
    running: 0
    stopped: 25
  graphDriverName: overlay
  graphOptions:
    overlay.mountopt: nodev,metacopy=on
  graphRoot: /var/lib/containers/storage
  graphStatus:
    Backing Filesystem: extfs
    Native Overlay Diff: "false"
    Supports d_type: "true"
    Using metacopy: "true"
  imageStore:
    number: 5
  runRoot: /run/containers/storage
  volumePath: /var/lib/containers/storage/volumes
version:
  APIVersion: 3.3.1
  Built: 1630356396
  BuiltTime: Mon Aug 30 22:46:36 2021
  GitCommit: ""
  GoVersion: go1.16.6
  OsArch: linux/amd64
  Version: 3.3.1

Package info (e.g. output of rpm -q podman or apt list podman):

podman-3.3.1-1.fc34.x86_64

Have you tested with the latest version of Podman and have you checked the Podman Troubleshooting Guide? (https://github.com/containers/podman/blob/master/troubleshooting.md)

Yes

Additional environment details (AWS, VirtualBox, physical, etc.):

physical

[umohnani8]

We currently use -1 to set the limits to the server defaults Tune container pids limit (set 0 for unlimited, -1 for server defaults). We can switch the values, @mheon @rhatdan wdyt?

[rhatdan]

Easiest fix is to allow -1 and then just set it to 0.

[umohnani8]

@rhatdan but -1 is being used for server defaults, not exactly sure what server defaults are, but that would cause an issue if we just convert -1 to 0 for the unlimited case.

[rhatdan]

Man page says.

   --pids-limit=limit
       Tune  the  container's  pids limit. Set to 0 to have unlimited pids for
       the container. The default is  4096  on  systems  that  support  "pids"
       cgroup controller.

No mention of -1.

[umohnani8]

I am seeing the -1 here in the flag definition https://github.com/containers/podman/blob/main/cmd/podman/common/create.go#L424

pidsLimitFlagName := "pids-limit"
		createFlags.Int64(
			pidsLimitFlagName, pidsLimit(),
			"Tune container pids limit (set 0 for unlimited, -1 for server defaults)",
		)
		_ = cmd.RegisterFlagCompletionFunc(pidsLimitFlagName, completion.AutocompleteNone)

The flag definition might be wrong or the man page, but we should make it consistent. It fails with -1, so no idea what "server defaults" are supposed to be.

[mheon]

I think that description might just be wrong - it doesn't look like -1 is handled properly, and the manpage says it isn't a valid value.

[jerboaa]

Thanks for the quick fix! Much appreciated.