Repository navigation
Add a HEALTHCHECK to the images #1237
Description
Activity
Some of this is discussed in #920
Indeed, see https://github.com/docker-library/faq#healthcheck and the resulting non-triviality of https://github.com/docker-library/healthcheck/blob/master/postgres/docker-healthcheck
Why not include healthcheck script but not enable it by default to make things a bit easier?
IMHO included healthcheck should have these features:
- ensures all init scripts are done
- ensures db can be queried (
SELECT 1) - allows user to provide custom query via env variable
I was also struggling with a health check until I discovered that
pg_readywas not what I was looking for. Here I posted my solution on stackoverflow and explain why this is the case.My healthcheck:
test: ["CMD-SHELL", "psql -U ${DB_USER} -d ${DB_MAIN} -c 'SELECT 1' || exit 1"]See #920 (comment), especially:
Using
--host 127.0.0.1withpg_isreadyorpsqlfor checking the status of the container from within it is going to be the safed/best supported option (see https://github.com/docker-library/healthcheck/blob/master/postgres/docker-healthcheck for an example that does exactly that).(Your example will report "healthy" before the container is actually ready for external connections because it will happily connect to the unix socket of the initialization instance.)
pg_isready --host=127.0.0.1 || exit 1seems like a sufficiently good default, no? For more specific use cases, users of the image can override theHEALTHCHECKinstruction to change the command, the options, or to disable it altogether.
Rebuttal of the FAQ below…
- many users will have their own idea of what "healthy" means and credentials change over time making generic health checks hard to define
As mentioned above, users can customize the instruction as they wish. Checking whether the server is ready to accept a connection seems like a reasonable default.
after upgrading their images, current users will have extra unexpected load on their systems for healthchecks they don't necessarily need/want and may be unaware of
Yes, this is a small breaking change. But we should also be open to breaking changes; otherwise, we will be stuck with less-than-ideal solutions. Besides, in what use cases would it make sense not to add the
HEALTHCHECKinstruction?Kubernetes does not use Docker's heath checks (opting instead for separate liveness and readiness probes)
Kubernetes disables Docker’s
HEALTHCHECKinstruction (like anyone else can!), so adding the instruction to the image does not impact Kubernetes.sometimes things like databases will take too long to initialize, and a defined health check will often cause the orchestration system to prematurely kill the container (docker-library/mysql#439)
The root cause here is that the configured timeout is too small. The solution is to increase the timeout, not to disable the
HEALTHCHECKinstruction.Funnily enough, that has never been proven a robust health check for Testcontainers usage, given the reasons in #1237 (comment). Hence, we always relied on checking for a specific sequence of log messages 🤷
Reacted by Tianon GraviRebuttal of the FAQ below…
- many users will have their own idea of what "healthy" means and credentials change over time making generic health checks hard to define
As mentioned above, users can customize the instruction as they wish. Checking whether the server is ready to accept a connection seems like a reasonable default.
after upgrading their images, current users will have extra unexpected load on their systems for healthchecks they don't necessarily need/want and may be unaware of
Yes, this is a small breaking change. But we should also be open to breaking changes; otherwise, we will be stuck with less-than-ideal solutions. Besides, in what use cases would it make sense not to add the
HEALTHCHECKinstruction?I wanted to add some extra thoughts here, as I agree with the rebuttal, but it seems nothing has moved in a few years:
Many, many, many containers include a default healthcheck. Users who find that the definition of healthy does not suit their needs can change it, but I think that this is a rare case. It makes the most sense for maintainers to make a call at what is an appropriate definition. For a default healthcheck for postgres in particular, it's reasonable to stop at "can we execute a query using the 'external' connection". You could even add env vars to support a particular database, particular query, etc.
Extra unexpected load? A single extra command/query every 30-60s won't even register...
At the same time, I appreciate the persistent maintainers' concern of extra support burden. That being said, I just don't think a default healthcheck is likely to increase support requests in a significant way.
Reacted by fumoboy007, Marcin Wisnicki and Miriam Müller
When helping people on StackOverflow, I often see people having problems with Docker Compose and applications that try to connect to the database immediately on startup. When using a simple docker compose 'depends_on' directive, compose only waits for the database to be started before starting the application. The application then fails because the database isn't ready to accept connections yet.
The solution is to add a healthcheck to the database container in docker compose and have the application wait for the database to enter a healthy state.
It would be nice if the healthcheck was included in the Postgres image by default. A simple
HEALTHCHECK ["pg_isready"]should be enough.