-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Healthcheck doc and completion fixes #3455
Conversation
Signed-off-by: Hunor Csomortáni <csomh@redhat.com>
Avoid users having to guess if the issue is with their setup or the way they run the commands. Signed-off-by: Hunor Csomortáni <csomh@redhat.com>
Although this is not compatible with [Docker CLI]. [Docker CLI]: https://docs.docker.com/engine/reference/run/#healthcheck Signed-off-by: Hunor Csomortáni <csomh@redhat.com>
Signed-off-by: Hunor Csomortáni <csomh@redhat.com>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: csomh The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Hi @csomh. Thanks for your PR. I'm waiting for a containers or openshift member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Can one of the admins verify this patch?
|
Our aim is to be compatable with Docker, So I would prefer to change the commands to match Docker commands.
@baude PTAL |
@csomh If you could open a PR to fix podman run commands to use the "Docker" equivalents, and perhaps alias the names podman chose, I think that would be the best fix. |
bot, add author to whitelist |
there is no docker equiv. they dont do any of this outside of the dockerfile as far as I know. |
Sure, will do next week. |
@baude it's possible to run:
and see the container shown as healthy in
The only option missing from the version above is |
cool! that is news to me! |
Fix Docker CLI compatibility issue: the "--healthcheck-command" option value should not be split but instead be passed as single string to "CMD-SHELL", i.e. "/bin/sh -c <opt>". On the other hand implement the same extension as is already available for "--entrypoint", i.e. allow the option value to be a JSON array of strings. This will make life easier for tools like podman-compose. Updated "--healthcheck-command" option values in tests accordingly. Continuation of containers#3455 & containers#3507 Signed-off-by: Stefan Becker <chemobejk@gmail.com>
The
--healthcheck-*
docs and completion has some issues inpodman version 1.4.2
on Fedora 30:--health-*
. Although this would be compatible with Docker CLI (is podman still aiming for this?), the actual implementation is--healthcheck-*
.--healthcheck-command=command
is documented as--healthcheck=command
.--no-healthcheck
has currently(?) no implementation, while it's offered by bash completion.This change attempts to address all the above.
Let me know if any of this needs adjustment.