add a docker healthcheck #98

Closed
chrisns wants to merge 2 commits from docker-healthcheck into master
chrisns commented 2018-09-26 13:53:49 +00:00 (Migrated from github.com)

in relation to https://github.com/localtunnel/localtunnel/issues/215#issuecomment-423824738

Ideally the port would be an env var rather than a process argument which would make the healthcheck a lot less ugly.

What I first have to do is figure out what port its listening on, and then curl it.

I'll assume if it responds to a curl on that port the server is healthy

in relation to https://github.com/localtunnel/localtunnel/issues/215#issuecomment-423824738 Ideally the port would be an env var rather than a process argument which would make the healthcheck a lot less ugly. What I first have to do is figure out what port its listening on, and then curl it. I'll assume if it responds to a curl on that port the server is healthy
defunctzombie commented 2018-09-28 22:02:07 +00:00 (Migrated from github.com)

Few thoughts:

  • I would prefer adding the env var to avoid the obtuse shell concoction to get the port.
  • I don't think this health-check will really impact the issues folks see around subdomains. The container already is set to restart when it dies - are you saying there are instances you have noticed where it "stalls" - i.e. is not responding to any request but also not restarting?
Few thoughts: * I would prefer adding the env var to avoid the obtuse shell concoction to get the port. * I don't think this health-check will really impact the issues folks see around subdomains. The container already is set to restart when it dies - are you saying there are instances you have noticed where it "stalls" - i.e. is not responding to any request but also not restarting?
chrisns commented 2018-09-28 23:20:43 +00:00 (Migrated from github.com)

Hey,
Happy to make the changes to allow for it to be specified as an env var, I'll make a separate PR for that.

The downside would be that the healthcheck would always fail for any folk starting it with the --port format so effectively a breaking change.

I'm only basing it on the observations of the service you run on localtunnel.me I can't simulate the same level of load you're getting there to cause it to fail.

The point of the healthcheck here was so that docker would know the service is unhealthy if it stops responding to HTTP GETs which is conceivable that the node process might not have exited causing docker to restart, but could have failed or slowed down so much that its not responding in a timely fashion.

Hey, Happy to make the changes to allow for it to be specified as an env var, I'll make a separate PR for that. The downside would be that the healthcheck would always fail for any folk starting it with the `--port` format so effectively a breaking change. I'm only basing it on the observations of the service you run on localtunnel.me I can't simulate the same level of load you're getting there to cause it to fail. The point of the healthcheck here was so that docker would know the service is unhealthy if it stops responding to HTTP GETs which is conceivable that the node process might not have exited causing docker to restart, but could have failed or slowed down so much that its not responding in a timely fashion.

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: hoelee/server#98