Add ensureState helper for checking container state #4355
Conversation
We have a lot of checks for container state scattered throughout libpod. Many of these need to ensure the container is in one of a given set of states so an operation may safely proceed. Previously there was no set way of doing this, so we'd use unique boolean logic for each one. Introduce a helper to standardize state checks. Note that this is only intended to replace checks for multiple states. A simple check for one state (ContainerStateRunning, for example) should remain a straight equality, and not use this new helper. Signed-off-by: Matthew Heon <mheon@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
approved
|
LGTM, Nice fix. |
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
|
/lgtm |
| } | ||
| } | ||
|
|
||
| // TODO: Is killing a paused container OK? |
There was a problem hiding this comment.
A paused container shouldn't be running, so I'm not sure how you'd stop the process there. If you could, I don't think you should unless there was some kind of --force option added to the kill command.
There was a problem hiding this comment.
Paused processes can still receive signals, which has some interesting applications - you can pause a container, SIGKILL every process in it, and unpause it, to cause it to immediately exit without any possibility of a race condition around fork()
There was a problem hiding this comment.
Thanks for the 411, I wasn't aware that paused containers could receive signals still.
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
nice little convenience function!
|
LGTM, but some test unhappiness is sprouting. |
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
github-actions
bot
added
the
locked - please file new issue/PR
6 participants
We have a lot of checks for container state scattered throughout libpod. Many of these need to ensure the container is in one of a given set of states so an operation may safely proceed.
Previously there was no set way of doing this, so we'd use unique boolean logic for each one. Introduce a helper to standardize state checks.
Note that this is only intended to replace checks for multiple states. A simple check for one state (ContainerStateRunning, for example) should remain a straight equality, and not use this new helper.