-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Various fixes to health check scripts #3002
Various fixes to health check scripts #3002
Conversation
request for comment: Should this PR contain all suggestion in #2766? |
@kkimurak Yes, this would be nice. However, do you have an idea to adress all issues? |
@sachilles Ok, I'll try to implement it by the next release. As far as I checked, the suggestion contains everything to do. |
@kkimurak Thank you! But please bear in mind that the reverse proxy can also play an important role in the choice of protocol in the curl command (depending on whether the reverse proxy terminates the SSL connection or not). |
When the healthcheck feature introduced, the script were generated on build time and url was fixed to "http://localhost/-/liveness". See pull request sameersbn#2102 nginx is configured to redirect all http traffic to https when `GITLAB_HTTPS` is enabled. (see https://github.com/sameersbn/docker-gitlab/blob/ac9e1fe/assets/runtime/config/nginx/gitlab-ssl#L41-L54), `--location` option is set to follow the redirection. See pull request sameersbn#2165 Health check script generation has been ported to the runtime, allowing us to dynamically generate health check URLs while referencing configuration parameters. See sameersbn#2338 If configured correctly, the redirect will not occur and the option can be removed. Original removal suggestion by @Gaibhne , additional (historical) research by @kkimurak. Co-authored-by: Kazunori Kimura <kkimura@ims.ac.jp>
On upstream, expected default value is `127.0.0.1/8` and it is already listed in corresponding configuration. `GITLAB_MONITORING_IP_WHITELIST` is used to allow monitoring from hosts other than loopback (localhost). So just unset default value for it. If the value is not set, the line specifying this "additional" IP range will be removed.
Access to health check resources such as /-/liveness is restricted to IPs specified in gitlab.monitoring.ip_whitelist (`GITLAB_MONITORING_IP_WHITELIST`). The name `localhost` is solved to IPv6 loopback address (::1) that is not listed in the whitelist by default. Possible alternate designs: - Add IPv6 loopback to whitelist - Disable IPv6 for gitlab container by specifying `net.ipv6.conf.all.disable_ipv6=1` in docker-compose.yml for example See sameersbn#2766 (comment)
9c7e016
to
e5dc2dd
Compare
I've implemented all (4) changes and rebased on the current master (v17.3.3), but I haven't been able to test them yet as my local environment has become very slow. It's taking over 10 hours to build the image.. |
CI failed due to timeout. |
Now CI looks succeed. It seems like it was automatically restarted after it failed. |
@kkimurak I did the restart of the CI. However, in my setup the healthcheck is working. I'll backport the healthcheck into the 17.x branches and the 16.11.x branch as well. |
Backport is done by cherry-picking the commits. |
@sachilles Thank you. |
You're welcome. |
close #2992, partially implement #2766
Access to health check resources such as /-/liveness is restricted to IPs specified in gitlab.monitoring.ip_whitelist (
GITLAB_MONITORING_IP_WHITELIST
).Currently healthcheck may report unhealthy because the name
localhost
is solved to IPv6 loopback address (::1) that is not listed in the whitelist by default.This PR fix the issue by using 127.0.0.1 (IPv4 loopback address) instead of
localhost
for monitoring endpoint url.Possible alternate designs:
net.ipv6.conf.all.disable_ipv6=1
in docker-compose.yml for example See Multiple parts of healthcheck broken/bugged #2766 (comment)/cc @Gaibhne I have create commit based on your suggestion in #2766 so I have set you an author of the commit. Let me know if you don't like it.