Open muayyad-alsadi opened 5 years ago
Sorry for my bad Pythonese. Your solution using cmd_quote
is much better.
I tested your latest change and it doesn't handle all cases correctly. Please see the latest commit I pushed to my fork. Do you want a PR for it or can you cherry-pick it?
if my comments in podman
are taken in consideration
https://github.com/containers/libpod/issues/3507#issuecomment-509534722
the code should be like
if is_str(healthcheck_test):
# podman does not add shell to handle command with whitespace
args.extend(['--healthcheck-command', healthcheck_test])
elif is_list(healthcheck_test):
# If it’s a list, first item is either NONE, CMD or CMD-SHELL.
healthcheck_type = healthcheck_test.pop(0)
if healthcheck_type == 'NONE':
args.append("--no-healthcheck")
elif healthcheck_type == 'CMD':
args.extend(['--healthcheck-command', json.dumps(healthcheck_test)])
elif healthcheck_type == 'CMD-SHELL':
if len(healthcheck_test)!=1:
raise ValueError("'CMD_SHELL' takes a single string after it")
args.extend(['--healthcheck-command', healthcheck_test])
Don't know if that error has anything with this:
File "/home/thomas/bin/podman-compose", line 506
args.extend(['--healthcheck-command', '/bin/sh -c {}'.format(cmd_quote(healthcheck_test[0])]))
^
SyntaxError: invalid syntax
on current master (e21932e1de6ad4d3786ab520e14b972b427d1531)
@chpio I noticed that too and fixed it in my patch stefanb2@d1583d5b
@chpio I've fixed the typo, please tell me if it fixed the issue
@stefanb2 I did not like quoting the whole thing as you did
args.extend(['--healthcheck-command', cmd_quote('/bin/sh -c {}'.format(cmd_quote(healthcheck_test)))])
args.extend(['--healthcheck-command', cmd_quote(' '.join([cmd_quote(i) for i in healthcheck_test])
args.extend(['--healthcheck-command', cmd_quote('/bin/sh -c {}'.format(cmd_quote(healthcheck_test[0])))])
because when we pass arrays ["something", "otherthing"]
the string is passed as is, no need for extra escaping.
anyway, I'm following podman
ticket to see what they will do. I hope they solve it in a way I can pass a string or of a list json.dumps(ls)
it's not compose job to work with shell, compose does not even know which shell to use (bash, sh, or zsh), if I want to inspect the image to know the shell, I might need to pull it first, but I can't pull it every time if it's already pulled. (it's a lot of job to do in compose)
on the other hand, podman knows.
@muayyad-alsadi oops I just looked at the generated command lines and missed the fact that your are not passing them to the shell. Just forget my change then.
@chpio I've fixed the typo, please tell me if it fixed the issue
yeah, thank you for that
I am seeing an error here:
Traceback (most recent call last):
File "/opt/homebrew/bin/podman-compose", line 33, in <module>
sys.exit(load_entry_point('podman-compose==1.0.6', 'console_scripts', 'podman-compose')())
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/homebrew/Cellar/podman-compose/1.0.6/libexec/lib/python3.11/site-packages/podman_compose.py", line 2941, in main
podman_compose.run()
File "/opt/homebrew/Cellar/podman-compose/1.0.6/libexec/lib/python3.11/site-packages/podman_compose.py", line 1423, in run
cmd(self, args)
File "/opt/homebrew/Cellar/podman-compose/1.0.6/libexec/lib/python3.11/site-packages/podman_compose.py", line 1754, in wrapped
return func(*args, **kw)
^^^^^^^^^^^^^^^^^
File "/opt/homebrew/Cellar/podman-compose/1.0.6/libexec/lib/python3.11/site-packages/podman_compose.py", line 2067, in compose_up
podman_args = container_to_args(compose, cnt, detached=args.detach)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/homebrew/Cellar/podman-compose/1.0.6/libexec/lib/python3.11/site-packages/podman_compose.py", line 999, in container_to_args
raise ValueError("'CMD_SHELL' takes a single string after it")
ValueError: 'CMD_SHELL' takes a single string after it
The health check in my compose file:
healthcheck:
test: ["CMD-SHELL", "pg_isready", "-U", "$PGUSER", "-d", "$PGDATABASE"]
I was able to get around this by using CMD
and switching the $PGUSER
and $PGDATABASE
strings to reference variables in the compose file. However, it would be nice to have this fixed.
Healthcheck not working on podman-compose (not tested on docker) if /bin/sh not available in container even when using CMD in heathcheck.test (reproduction).
$ podman version
Client: Podman Engine
Version: 4.9.0
API Version: 4.9.0
Go Version: go1.21.6
Built: Wed Jan 24 13:07:27 2024
OS/Arch: linux/amd64
services:
test:
build:
context: .
container_name: healthcheck_test
healthcheck:
test: [
"CMD",
"/healthcheck",
"http://localhost:8080/ping"
]
interval: 5s
timeout: 5s
retries: 5
podman inspect --format "{{json .State.Health }}" healthcheck_test | jq
{
"Status": "unhealthy",
"FailingStreak": 18,
"Log": [
{
"Start": "2024-02-15T10:34:44.046709663+03:00",
"End": "2024-02-15T10:34:44.131872722+03:00",
"ExitCode": 1,
"Output": ""
},
continue on #22
https://docs.docker.com/compose/compose-file/#healthcheck https://docs.docker.com/engine/reference/builder/#healthcheck https://docs.docker.com/engine/reference/run/#healthcheck
and
https://github.com/containers/libpod/issues/3507