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

Add healthcheck #77

Closed
wants to merge 1 commit into from
Closed

Conversation

vincentchalamon
Copy link

@ImreSamu
Copy link
Member

ImreSamu commented Dec 28, 2017

@vincentchalamon :
imho: it would be better if the official postgres image will support this feature ( for compatibility reason)

@vincentchalamon
Copy link
Author

@ImreSamu That would be awesome (not just on postgres). But if docker-library/postgres#282 is rejected, current PR will improve current Docker image.

Status: waiting for docker-library/postgres#282 to be accepted or rejected.

Copy link
Contributor

@md5 md5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please make these changes in the Dockerfile.template file as well?

Also, the Makefile needs to be updated to copy the healthcheck.sh script from a shared source of truth, like the other scripts.

@@ -15,3 +15,6 @@ RUN mkdir -p /docker-entrypoint-initdb.d
COPY ./initdb-postgis.sh /docker-entrypoint-initdb.d/postgis.sh
COPY ./update-postgis.sh /usr/local/bin

COPY ./healthcheck.sh /usr/local/bin/docker-healthcheck
RUN chmod +x /usr/local/bin/docker-healthcheck
HEALTHCHECK CMD ["docker-healthcheck"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this line and add a section to the README.md telling users how to enable the health check command using either HEALTHCHECK CMD or the --health-cmd command line switch (I think it would make sense to link to https://docs.docker.com/engine/reference/run/#healthcheck).

The README.md section should describe what docker-healthcheck does and perhaps possible reasons why someone would want a more specific healthcheck for their application, such as the fact that it can't test connectivity from outside the container.

@@ -15,3 +15,6 @@ RUN mkdir -p /docker-entrypoint-initdb.d
COPY ./initdb-postgis.sh /docker-entrypoint-initdb.d/postgis.sh
COPY ./update-postgis.sh /usr/local/bin

COPY ./healthcheck.sh /usr/local/bin/docker-healthcheck
RUN chmod +x /usr/local/bin/docker-healthcheck
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you chmod the actual file in the repository, this line should not be necessary.

@md5 md5 closed this Oct 3, 2018
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.

3 participants