-
Notifications
You must be signed in to change notification settings - Fork 885
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 command #1257
Comments
Hi there! 👋🏼 As you're new to this repo, we'd like to suggest that you read our code of conduct as well as our contribution guidelines. Thanks a bunch for opening your first issue! 🙏 |
The thing is that since the watchtower container only contains a single binary, which is the entrypoint, it either a) is still running or b) the container has stopped (and restarted if that is configured). |
My reasoning is that to motivate adding additional complexity to the docker image, it needs to do something worthwhile, ie. actually contributing to the operability or observability of the workload. Adding a health check just to make it look nicer is not a sufficient argument to me. |
I get your point. It's stupidly simple, build a binary that returns |
Health checks are simply good practice. |
Aside from that, this has made maintaining my server so much easier, so thank you all for your wonderful work. |
A workaround would be to e. g. mount static wget/curl into container, enable http-api-metrics and check api endpoint
|
@bugficks I think you are missing the point a little bit here. Since the container only has a single binary, which is the sole execution target, the application is either running or has crashed, taking the container down with it. There is no known state where the container would be up, but not responding to a metrics-request (unless you disable it of course). |
@piksel Well not really, I am just saying instead of modifying/fork image it might be better to simply mount e. g. wget as it |
@bugficks Yeah, sorry, it was a bit aggressive. It's a good point, that doing this instead of maintaining a fork seems like a much easier solution. |
If by fork you mean my image, it’s no fork, it’s just a inherent Image with another binary. It’s build automatically every day based on the latest watchtower, so there is no need for maintenance. |
It's been said that a healthcheck is pointless because the container is either running or not. I agree from watchtower's perspective: that is absolutely true. But in a large infrastructure of many containers and services, on different servers, you'll use automation tools - and a standard way to monitor services is with healthchecks. (e.g. our ansible infrastructure uses healthchecks to determine container state and thus decide what actions to take.) So although it adds zero benefit to watchtower's operation, the watchtower container is part of a larger system, and healthchecks are expected. |
@lonix1 I have the same opinion. But is there another endpoint to use? I don't use metrics so I don't want to enable just to abuse it. Another idea is to use http api endpoint with bad credentials then it will respond 401, and your calling code can assume "401 is good". |
@denisz1 Well that would work for the healthcheck, but then you'd flood your logs with 401 errors. I think those are the only two endpoints right now, but I'm unsure. |
Yes true I didn't think of that. Actually I just give up on this - mounting wget just feels bad. And you must "mount" a shell, and dependencies... it's get ugly fast. The image needs its own healthcheck and for that it must be base on at least busybox or alpine, not scratch. |
I think it would be fine to add a command line argument to the watchtower binary that (optionally, just checks if there is another watchtower process running and then) returns Then we could add that as the This way, we still fulfill the contract of "when is the container considered healthy", despite the actual work the HEALTHCHECK command does being none. The reasoning stated above as to why we only do that should of course be added to the docs |
What a clever idea! That would solve the issue without expanding the image's attack surface. (Do you think maybe the issue could be reopened and put on the backlog then? ....so it's not forgotten.) |
Woohoo! Thank you! |
Here's a simple sample:
services:
watchtower:
image: containrrr/watchtower
restart: unless-stopped
container_name: watchtower
# Slightly adjusted resource limits based on typical production loads
deploy:
resources:
limits:
cpus: '1'
memory: 1G
reservations:
cpus: '0.5'
memory: 512M
volumes:
- /var/run/docker.sock:/var/run/docker.sock
healthcheck:
test: ["CMD", "/watchtower", "--health-check"]
interval: 30s
timeout: 10s
retries: 3
start_period: 5s
cap_drop:
- ALL
cap_add:
- CHOWN # Only if you need to change file ownership
- SETGID # Only if you need to change group ID
- SETUID # Only if you need to change user ID
- DAC_OVERRIDE # Only if you need to bypass file permissions
# Add logging limits
logging:
driver: "json-file"
options:
max-size: "100m"
max-file: "5"
environment:
- REPO_USER=[ENTER YOUR DOCKER HUB USER]
- REPO_PASS=[ENTER YOUR DOCKER HUB TOKEN]
command:
--interval 15
--include-restarting
--include-stopped |
Is your feature request related to a problem? Please describe.
No problem
Describe the solution you'd like
Add a healthcheck command to the image.
Describe alternatives you've considered
reopen #972 :)
Additional context
maybe something like that :
healthcheck.sh
And for the dockerfile :
HEALTHCHECK --interval=5m --timeout=2s --start-period=30s CMD sh healthcheck.sh
it's just an example to improve and adjust with env var. The watchtower job has just to make a touch on the file to update it
The text was updated successfully, but these errors were encountered: