Report errors when trying to pause rootless containers #4006
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
approved
pkg/varlinkapi/containers.go
Outdated
| @@ -454,6 +456,15 @@ func (i *LibpodAPI) KillContainer(call iopodman.VarlinkCall, name string, signal | |||
|
|
|||
| // PauseContainer ... | |||
| func (i *LibpodAPI) PauseContainer(call iopodman.VarlinkCall, name string) error { | |||
| if rootless.IsRootless() { | |||
There was a problem hiding this comment.
Do we want to put this after the LookupContainer call? I think we'd want to report "No such container" before "Rootless doesn't support pause"?
|
We could put the check in |
|
@mheon My concern there would be the error message might not be good, |
|
LGTM |
libpod/pod_api.go
Outdated
| return nil, errors.Wrap(err, "failed to determine cgroupversion") | ||
| } | ||
| if !cgroupv2 { | ||
| return nil, errors.Wrap(define.ErrNoCgroups, "can not pause pods on rootless containers with cgroup V1") |
There was a problem hiding this comment.
should this be: "can not pause pods containing rootless containers with cgroup V1" ? s/containing rootless/on rootless/
|
One possible grammar change, otherwise LGTM. |
|
Fixes: #3970 |
If you are running a rootless container on cgroupV1 you can not pause the container. We need to report the proper error if this happens. Signed-off-by: Daniel J Walsh <[email protected]>
| return errors.Wrap(err, "failed to determine cgroupversion") | ||
| } | ||
| if !cgroupv2 { | ||
| return errors.Wrap(define.ErrNoCgroups, "can not pause containers on rootless containers with cgroup V1") |
There was a problem hiding this comment.
We should discard pause here and warn instead of erroring. We can't break existing rootless installs.
There was a problem hiding this comment.
Wrong PR, I think this one should get in and then we open a specific one for podman cp.
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
If you are running a rootless container on cgroupV1
you can not pause the container. We need to report the proper error
if this happens.
Signed-off-by: Daniel J Walsh [email protected]