podman unpause error output inconsistent

[srcshelton]

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

/kind bug

Description

$ sudo podman ps -a
Password:
CONTAINER ID  IMAGE                                            COMMAND               CREATED       STATUS         PORTS       NAMES
b66eefb501f7  localhost/service.app-admin.metalog:20200113-r1  -p /var/run/metal...  3 days ago    Up 3 days ago              openrc-metalog-20200113-r1
2e00f2032603  localhost/service.net-misc.openntpd:6.2_p3-r2    -d -f /etc/opennt...  3 days ago    Up 3 days ago              openrc-openntpd-6.2_p3-r2
8a8297f95023  localhost/gentoo-build:latest                    =sys-devel/gcc-10...  17 hours ago  paused                     buildpkg.hostpkgs.gcc.update

$ sudo podman unpause -a
8a8297f9502356fc1bc3032ba675640c6d3556e32ae03c59a45a5a902b77e11d
"2e00f2032603f4e5af2097cdf197abd8a3875d88887ee7f1c318dedcd8cd71d7" is not paused, can't unpause: container state improper
Error: "b66eefb501f793d59a2586e09dd44be489cbaf2493953b97133eefa6825a71c6" is not paused, can't unpause: container state improper

So the issues here appear to be:

1. The "Error: " prefix from the second line of the unpause sub-command output seems to have got lost (or was correctly removed from second line but not the third?);
2. Is it actually an error to have unpaused containers when you run unpause --all? It feels as if this is perhaps a warning at most - although since the specified containers are arguably already in the desired state, do they even need to be mentioned at all?

(I also have a nagging feeling that errors from other podman sub-commands don't enclose the container ID in double-quotes, but I've not verified this!)

Describe the results you expected:

No error messages should be produced when when no errors have occurred (this may be more nuanced when using the --all argument than when supplied with specific parameters).

Output of podman version:

Version:      3.2.3
API Version:  3.2.3
Go Version:   go1.16.6
Git Commit:   1e6fd46e91b21342f9454cf8105a92b90e398c52
Built:        Thu Jul 22 13:02:47 2021
OS/Arch:      linux/amd64

Output of podman info --debug:

host:
  arch: amd64
  buildahVersion: 1.21.3
  cgroupControllers:
  - cpuset
  - cpu
  - io
  - memory
  - hugetlb
  - pids
  - rdma
  cgroupManager: cgroupfs
  cgroupVersion: v2
  conmon:
    package: app-emulation/conmon-2.0.29
    path: /usr/bin/conmon
    version: 'conmon version 2.0.29, commit: 7e6de6678f6ed8a18661e1d5721b81ccee293b9b'
  cpus: 4
  distribution:
    distribution: gentoo
    version: unknown
  eventLogger: file
  hostname: apu2e
  idMappings:
    gidmap: null
    uidmap: null
  kernel: 5.12.0-gentoo
  linkmode: dynamic
  memFree: 959692800
  memTotal: 4116377600
  ociRuntime:
    name: crun
    package: app-emulation/crun-0.20.1
    path: /usr/bin/crun
    version: |-
      crun version 0.20.1
      commit: 38271d1c8d9641a2cdc70acfa3dcb6996d124b3d
      spec: 1.0.0
      +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +YAJL
  os: linux
  remoteSocket:
    path: /run/podman/podman.sock
  security:
    apparmorEnabled: false
    capabilities: CAP_AUDIT_WRITE,CAP_CHOWN,CAP_DAC_OVERRIDE,CAP_FOWNER,CAP_FSETID,CAP_KILL,CAP_MKNOD,CAP_NET_BIND_SERVICE,CAP_NET_RAW,CAP_SETFCAP,CAP_SETGID,CAP_SETPCAP,CAP_SETUID,CAP_SYS_CHROOT
    rootless: false
    seccompEnabled: true
    seccompProfilePath: /usr/share/containers/seccomp.json
    selinuxEnabled: false
  serviceIsRemote: false
  slirp4netns:
    executable: ""
    package: ""
    version: ""
  swapFree: 2449694720
  swapTotal: 2684350464
  uptime: 794h 45m 20.28s (Approximately 33.08 days)
registries:
  localhost:5000:
    Blocked: false
    Insecure: true
    Location: localhost:5000
    MirrorByDigestOnly: false
    Mirrors: []
    Prefix: localhost:5000
  search:
  - docker.io
  - docker.pkg.github.com
  - quay.io
  - public.ecr.aws
store:
  configFile: /etc/containers/storage.conf
  containerStore:
    number: 3
    paused: 0
    running: 3
    stopped: 0
  graphDriverName: overlay
  graphOptions:
    overlay.ignore_chown_errors: "false"
  graphRoot: /storage/containers/podman/storage
  graphStatus:
    Backing Filesystem: extfs
    Native Overlay Diff: "false"
    Supports d_type: "true"
    Using metacopy: "true"
  imageStore:
    number: 10
  runRoot: /storage/containers/podman/run
  volumePath: /storage/containers/podman/volumes
version:
  APIVersion: 3.2.3
  Built: 1626955367
  BuiltTime: Thu Jul 22 13:02:47 2021
  GitCommit: 1e6fd46e91b21342f9454cf8105a92b90e398c52
  GoVersion: go1.16.6
  OsArch: linux/amd64
  Version: 3.2.3

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

[mheon]

The second line isn't really having any output dropped, so much as the last line is having Error: prepended - if you had 6 containers error, only the last one would have an Error prefix. We can change this over to have every container error printed the same and print a trailing Error: 5 containers failed to unpause line, I suppose?

Otherwise, the unpause --all bit does sound reasonable - makes sense that it would not act on containers that are not paused, similar to stop --all does not error if containers are stopped already.

[jwhonce]

@mheon The pre-pending behavior is a holdover from podman v1. A number of commands print n-1 errors without prefix and then the "last" error gets decorated. I am fine with all errors getting the prefix, but we should do it for all commands not just this one.

[srcshelton]

Ahh, I wondered if that'd be the case.

I'd +1 adding 'Error: ' to all relevant lines, because if you're string-parsing the command output (which I'm not, but it sounds like a use-case) then it's much easier to find all of the potential errors you might want to respond to that way.

But also yes, like stop -a it'd be helpful if containers already in the desired state didn't generate warnings/errors.