dokku / docker-container-healthchecker

Runs healthchecks against local docker containers
BSD 3-Clause "New" or "Revised" License
16 stars 4 forks source link

`command` in `app.json` does not respect the `ENTRYPOINT` for a docker image. #92

Closed IgnisDa closed 7 months ago

IgnisDa commented 1 year ago

Description of problem

As the title says.

Steps to reproduce

FROM ubuntu as builder
RUN touch config.yaml

FROM ghcr.io/teamhanko/hanko:v0.8.3
COPY --from=builder config.yaml ./config/config.yaml
{
    "healthchecks": {
        "web": [
            {
                "type": "startup",
                "name": "Command check",
                "description": "Checking if a command in the container runs as expected",
                "initialDelay": 5,
                "attempts": 5,
                "wait": 3,
                "command": ["hanko", "isready"] // this should be possible without the `hanko`
            }
        ]
    }
}

docker-container-healthchecker version

0.6.4

Output of failing command

No response

josegonzalez commented 1 year ago

This just runs docker exec as shown here. Do we need to respecify the entrypoint when doing that?

IgnisDa commented 1 year ago

Maybe. Because the following procfile works for me (which means entrypoint is respected for Procfiles)

release: migrate up # dont need to specify hanko
web: serve public
IgnisDa commented 1 year ago

ENTRYPOINT for base image: https://github.com/teamhanko/hanko/blob/a4288afcf4426dc02e8f7825ea7c76454402028b/backend/Dockerfile#L41

josegonzalez commented 1 year ago

I don't think docker exec respects the entrypoint, though this is definitely what I'd expect, behavior-wise, from healthchecking.

josegonzalez commented 12 months ago

How should we handle shell-based entrypoints? These don't allow command-line arguments at all, so healthchecks using them wouldn't work...

josegonzalez commented 12 months ago

One interesting thing I've noted is that none of the herokuish containers have an entrypoint, which means that we're re-implementing that on our end in Dokku.

I think we can change that in Dokku for the builder-herokuish builder by:

The annoying thing is that we wouldn't be able to support && in release commands anymore - they are invoked differently in dokku - though that might actually be good from a parity perspective.


I think for this issue, we should use the /start command for herokuish-based images, and otherwise respecting the entrypoint. I don't see a need to re-implement everything else.

IgnisDa commented 12 months ago

I think for this issue, we should use the /start command for herokuish-based images, and otherwise respecting the entrypoint. I don't see a need to re-implement everything else.

Yep that sounds good.

However, I don't really use herokuish based images so I can't comment on those.

josegonzalez commented 12 months ago

Can you give me a valid config.yaml for this hanko test? The container doesn't run as is, even with passing args:

% docker run -it --rm hanko serve all
2023/10/01 06:22:05 Using config file: ./config/config.yaml
2023/10/01 06:22:05 failed to validate config: failed to validate passcode settings: failed to validate smtp settings: smtp host must not be empty
IgnisDa commented 12 months ago

Here is a docker compose dev container: https://github.com/IgnisDa/codefarem/blob/main/.devcontainer/docker-compose.yml