-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix podman unpause to work like podman stop #11113
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 |
@@ -69,13 +69,17 @@ func (ic *ContainerEngine) ContainerPause(ctx context.Context, namesOrIds []stri | |||
} | |||
|
|||
func (ic *ContainerEngine) ContainerUnpause(ctx context.Context, namesOrIds []string, options entities.PauseUnPauseOptions) ([]*entities.PauseUnpauseReport, error) { | |||
reports := []*entities.PauseUnpauseReport{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why move this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe because the len(ctrs)
is not needed anymore so it can move up for readability?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved it to match containersstop, but if it offends I can move it back
One nit otherwise LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM, maybe a new integration test to test explicitly and other |
c2a64f2
to
a6a0846
Compare
Currently if you execute podman unpause --all, podman pause --all Podman shows attempts to unpause containers that are not paused and prints an error. This PR catches this error and only prints errors if a paused container was not able to be unpaused. Currently if you execute podman pause --all or podman kill --all, Podman Podman shows attempts to pause or kill containers that are not running and prints an error. This PR catches this error and only prints errors if a running container was not able to be paused or killed. Also change printing of multiple errors to go to stderr and to prefix "Error: " in front to match the output of the last error. Fixes: containers#11098 Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
/lgtm |
/hold cancel |
Currently if you execute podman unpause --all, it
shows attempts to unpause containers that are not paused and prints
an error. This PR catches this error and only prints errors if
a paused container was not able to be unpaused.
Also change printing of multiple errors to go to stderr and to prefix
"Error: " in front to match the output of the last error.
Fixes: #11098
Signed-off-by: Daniel J Walsh dwalsh@redhat.com