Skip to content
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

Support healthcheck associative array #22

Merged

Conversation

stefanb2
Copy link
Contributor

@stefanb2 stefanb2 commented Jul 8, 2019

Map keys to corresponding --healthcheck-XXX option

@stefanb2
Copy link
Contributor Author

stefanb2 commented Jul 8, 2019

I needed to add healthcheck section to my docker-compose.yml because of containers/podman#3507

@muayyad-alsadi
Copy link
Collaborator

Thank you very much for this kind contribution. I'll look into it very soon

@stefanb2
Copy link
Contributor Author

stefanb2 commented Jul 8, 2019

Hmm, looks like ['CMD-SHELL', 'echo with whitespace'] does not work correctly, i.e. podman tries to find the executable echo with whitespace instead of executing a shell command line.

@muayyad-alsadi
Copy link
Collaborator

there are two things here

  1. build-time (in Dockerfile) specification of shell and healthchecks
  2. run-time (in docker-compose.yml) specification of healthchecks (no shell)

build-time issues should be reported to buildah
run-time issues should be reported to podman

from the compose.yml reference

test: ["CMD-SHELL", "curl -f http://localhost || exit 1"]

which should be interpreted as

test: ["CMD", "/bin/sh", "-c", "curl -f http://localhost || exit 1"]

assuming default shell ['/bin/sh', '-c']

this means that podman should interpret ['CMD-SHELL', 'echo with whitespace']
as ['CMD', '/bin/sh', '-c', 'echo with whitespace']

https://docs.docker.com/compose/compose-file/#healthcheck
https://docs.docker.com/engine/reference/builder/#healthcheck

@muayyad-alsadi
Copy link
Collaborator

I've seen the diff, it's not podman fault, we should pass it the the arguments in wrong way,

no worries, I'll fixes it ASAP.

Map keys to corresponding --healthcheck-XXX option

Unfortunately --healthcheck-command only accepts a string, not a list of
strings. Furthermore it splits the string on whitespace. Therefore the
mapping of all allowed Docker Compose values is not possible.
@stefanb2
Copy link
Contributor Author

stefanb2 commented Jul 8, 2019

Unfortunately things aren't that easy. It turns out that --healthcheck-command really only accepts a string, not a list of strings. Furthermore the string gets split on whitespace, i.e.

--healthcheck-command "stat /etc/passwd || exit 1"

ends up in the container configuration as

"Healthcheck": {
    "Test": [
        "stat",
        "/etc/passwd",
        "||",
        "exit",
        "1"
    ],

My updated patch tries to approximate as best as possible, but not all valid Docker Compose values will be able to be mapped correctly

@muayyad-alsadi muayyad-alsadi merged commit 2f4da3f into containers:master Jul 8, 2019
@muayyad-alsadi
Copy link
Collaborator

I've used

from shlex import quote as cmd_quote

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants