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

WIP: proper healthchecks support #23

Open
muayyad-alsadi opened this issue Jul 8, 2019 · 9 comments
Open

WIP: proper healthchecks support #23

muayyad-alsadi opened this issue Jul 8, 2019 · 9 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@muayyad-alsadi
Copy link
Collaborator

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

containers/podman#3507

@stefanb2
Copy link
Contributor

stefanb2 commented Jul 9, 2019

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?

@muayyad-alsadi
Copy link
Collaborator Author

if my comments in podman are taken in consideration
containers/podman#3507 (comment)

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])

@chpio
Copy link

chpio commented Jul 9, 2019

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 (e21932e)

@stefanb2
Copy link
Contributor

stefanb2 commented Jul 9, 2019

@chpio I noticed that too and fixed it in my patch stefanb2/podman-compose@d1583d5b

muayyad-alsadi added a commit that referenced this issue Jul 9, 2019
@muayyad-alsadi
Copy link
Collaborator Author

@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.

@stefanb2
Copy link
Contributor

stefanb2 commented Jul 9, 2019

@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
Copy link

chpio commented Jul 10, 2019

@chpio I've fixed the typo, please tell me if it fixed the issue

yeah, thank you for that

@davidjayb
Copy link

davidjayb commented May 24, 2023

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.

@x-user
Copy link

x-user commented Feb 15, 2024

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": ""
    },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants