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

HEALTHCHECK directive in Dockerfile #76

Closed
wants to merge 1 commit into from

Conversation

augi
Copy link

@augi augi commented Sep 5, 2016

It's a standard way how to tell if the container is healthy. It helps to wait for initialization in Docker Compose and similar use-cases.

@augi
Copy link
Author

augi commented Sep 5, 2016

Btw. the integration in TravisCI is failing on this (which is not affected by changes in this PR):
E: Failed to fetch http://httpredir.debian.org/debian/pool/main/l/lksctp-tools/lksctp-tools_1.0.16+dfsg-2_amd64.deb Error reading from server. Remote end closed connection [IP: 128.31.0.66 80]

@yosifkit
Copy link
Member

yosifkit commented Sep 9, 2016

I do not feel that generalized healthchecks on the official images are really that useful.

  • users will have their own idea of what is "healthy"
  • it does not actually test that the service is listening to connections outside of localhost (see https://github.com/docker-library/healthcheck for some examples that do more than what's proposed here, including attempting to check whether the service is listening remotely)
    • some of the Official Images even purposely start in a localhost only mode for database initialization and then kill and start the main service with full network availability
  • 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

@augi
Copy link
Author

augi commented Sep 10, 2016

Thanks for the feedback! What about publishing more tags, so having 3.7-healthchecked tag? Would it be the way? Or do you have any other idea how to provide images with healthchecks safely?

I'm asking because our main use-case is usage in integration tests (we are using https://github.com/avast/docker-compose-gradle-plugin) and it's very useful to have a reliable mechanism how to wait till the DB is ready to be used. Now, we are waiting for exposed ports opening but I feel it like a hack (although it works like a charm).

@amq
Copy link

amq commented Apr 10, 2017

HEALTHCHECK is not about eliminating the deployment risk, but about reducing it. I think it serves the purpose well.

@rturk
Copy link

rturk commented Jun 21, 2017

@yosifkit humbly disagree with your comment, any service should report if can perform what is expected/required to do.

@rturk
Copy link

rturk commented Jun 21, 2017

@augi this can be really helpful in HA environments like Kubernetes

@LandonSchropp
Copy link

I was pointed to this repository from docker-library/postgres/#282. This seems to be the place where the larger discussion of whether to include health checks in base images is happening, so I thought I'd comment here. Please let me know if there's a better place to post this and I'll move it.

The addition of health checks to Docker was really enticing to us. Prior to adding health checks, running commands via docker-compose run would occasionally result in race conditions between our application and our postgres container. We resolved these issues with custom scripts that checked that Postgres was running before executing our commands, but we disliked the level of code duplication that caused. The health checks allowed us to remove all of this duplicate code and let docker-compose orchestrate the health checks for us.

After reading the CodeShip documentation, we adopted the healthcheck/postgres and healthcheck/redis images into our infrastructure. I learned today from @wglambert that these are intended to be examples, and the suggested approach is to write our own health check Dockerfiles on top of the official images.

The office Docker images are extremely well maintained and provide an easy way to use services without needing to own the complexity of configuring and maintaining their environments. The enticing part of adding a health check to an official image is that it can be done in one place, and everyone who uses that image benefits from it.

If we create our own versions of health check containers, then our team is responsible for keeping those images up to date and maintaining them. If there's a security patch and the base image is updated, we'd have to be aware of that issue and push an updated image ourselves. Given some of the security issues with images hosted on DockerHub recently, we're feeling wary of maintaining the security details of these images. We'd much rather tie our infrastructure to a major version tag of an official image and let you guys, who are much more qualified, make sure those tags are kept up-to-date and secure.

My team is a small team with limited resources. Every team with production software has to manage dev ops at some level, but our goal is to focus as much as possible on writing code that delivers value to our customers. I'd be willing to bet that many teams using Docker are like ours, without specialists who can focus on ops and security. We like using Docker because it's focused on security and convenience, and we can lean on the official images.

I understand the argument about different people having different definitions of what constitutes "healthy", but I'd be willing to be that the vast majority of people would agree with @rnavarropiris that this should be defined as "the server was successfully started and is ready." It's true that this might not work for everyone, but I feel like the general case is what should be considered, with the ability to override as needed.

I'd really appreciate if you guys would reconsider your stance on this issue. I know health checks in the official images would make our lives a lot easier, and judging by the response the other health check tickets, it seems like others agree.

Thanks for taking the time to read and consider this.

@yosifkit
Copy link
Member

yosifkit commented Oct 3, 2018

After 2 years I still don't think healthchecks should be added to official images.

@codefromthecrypt
Copy link

I read this issue and noticed it started in 2016.

Probably between then and kubernetes/kubernetes#50796 docker-layer health checks were interfering with restart logic and k8s. This is no longer an issue even if probably there are some orchestrators out there who either need to tighten stages or otherwise reduce fragility in worst case disabling the healthcheck on the image.

Meanwhile healthcheck trips lint on certified images. DockerHub UTs use it. End users are better off seeing if something is healthy still, but this part was known years ago.

Main question is, are there any facts that can change to make it ok to have a HEALTHCHECK hosted here? Or is it basically blocked for another reason that hasn't been mentioned? What would change externally over the next few years and make it a decision to re-evaluate?

@PrivatePuffin
Copy link

PrivatePuffin commented Nov 1, 2020

Main question is, are there any facts that can change to make it ok to have a HEALTHCHECK hosted here

There are no "facts" that are preventing it, it's the personal OPINION of @yosifkit to not do this.

Meanwhile CIS Docker Security benchmark version 1.2.0 advices including it, which I think is a lot more relevant than the argumentative errors spewed by @yosifkit .

All of those arguments are so easy to crush i'm not even going to care (actually, it's only one argument, rephrased multiple times to look like more than it actually is) . All thats left is "his personal opinion", which isn't worth that much.

Be aware:
Disabling a healthcheck, is easier than creating one from scratch. All of his arguments fall apart by the fact that healthchecks can always be changed or disabled, but not everyone has the skill to write one from scratch.

Actually, I think it would even be possible to include a healthcheck and disable it by default. Which leave precisely 0 of the arguments against inclusion left.


edit

What I want to add is the fact his opinion is now used by MANY repository owners as a reputable sources as an official Docker Inc. viewpoint because he is a maintainer here. I think it's important to point out the fact that while it his (and possibly Docker Inc.'s) decision how to maintain this repo, that does not mean it's an official guideline of how to use Healthchecks.

@LandonSchropp
Copy link

@Ornias1993 I don't feel like your tone is constructive. It's one thing to disagree with someone, but it's another to attack them over their opinion. It's important to remember there's a person at the other end of these things.

@yosifkit While I don't agree with your position on this issue, I would like to express some appreciation for the work you do in maintaining the Docker images we use. Docker is a tool that my team relies on heavily at work, at it's partially due to your effort that we're able to do that. Thanks for all of your hard work.

@PrivatePuffin
Copy link

@LandonSchropp If pointing out someones writhing is an opinion and not a fact combined with pointing out it contained basic and fundemental argumentative errors, is a personal attack. i don't think we'll ever agree on anything.

Also: this whole discussion was far from constructive. Sometimes is better NOT to be constructive, because if an issue is caused by having someone in a position of power, the solution might be removing said person from said position of power. Which I do not thing should be done and I fully support the fact @yosifkit has the right to manage this repo the way he sees with.

I just do not agree with how some people are abusing his opinion to nuke any discussion about inclusion of health checks or make it seem like a stronger argument than it actually is.

To be clear: My post is not supposed to personally attack @yosifkit , he never said it his post was objective or facts-based or that his opinion represented the official stance of Docker. Thats why I wanted to highlight the errors in his argument and added an edit explaining how people are using (abusing?) his argument.

Btw:
A personal attack is an attack on the person, not the argument. I never said anything about @yosifkit personally (for all I care he is a unicorn-zebra hybrid), I only fierce pointed out the utter baselessness of his arguments.

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.

7 participants