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

feat(healthcheck): added healthcheck test using nc and port detection #36

Closed
wants to merge 2 commits into from

Conversation

gjuchault
Copy link

No description provided.

@stuartpb
Copy link
Collaborator

Do you have an example of other official images doing this?

@stuartpb
Copy link
Collaborator

Closes #34

@gjuchault
Copy link
Author

@stuartpb There is a discussion about wether healthcheck should be provided on official images.
docker-library/postgres#282 (comment)

@stuartpb
Copy link
Collaborator

Yeah, to be honest, I agree with @yosifkit's assessment there:

  • "port is open" isn't a significantly greater indication of healthy operation than "process is running" (it's just as possible for a malfunction to occur without disrupting the socket or process as it would be for a malfunction to occur disrupting the socket but not the process, if not more so)
  • any such more-meaningful health check that could be run would likely depend on the specifics of the use case
  • this introduces a new dependency and (marginally) more load for, as described above, negligible practical benefit

@gjuchault
Copy link
Author

I do agree with all the people that are writing this, but when I started using compose, healthchecks are so much useful. You might close this one if you feel that's settled and let everyone deal with it on their own. At lease they will have an idea if they want to implement a simple solution (the port one)

@stuartpb
Copy link
Collaborator

Closing unmerged per reasoning in #36 (comment).

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.

2 participants