diff --git a/changelogs/unreleased/219046-fix-readiness-probe-500-on-db-down.yml b/changelogs/unreleased/219046-fix-readiness-probe-500-on-db-down.yml new file mode 100644 index 0000000000000000000000000000000000000000..9765e2635e444dfe36caba4270ab05e0c556df71 --- /dev/null +++ b/changelogs/unreleased/219046-fix-readiness-probe-500-on-db-down.yml @@ -0,0 +1,5 @@ +--- +title: Expand healtchecks `500`s when DB is not available +merge_request: 34844 +author: +type: fixed diff --git a/doc/user/admin_area/monitoring/health_check.md b/doc/user/admin_area/monitoring/health_check.md index 91e29118e3e87ce24e2e4e9b71d64285d0a8da03..329b6ff5bb0118a89cb0fe63c2e55716ef026493 100644 --- a/doc/user/admin_area/monitoring/health_check.md +++ b/doc/user/admin_area/monitoring/health_check.md @@ -137,8 +137,8 @@ This check is being exempt from Rack Attack. ## Access token (Deprecated) -> NOTE: **Note:** -> Access token has been deprecated in GitLab 9.4 in favor of [IP whitelist](#ip-whitelist). +NOTE: **Note:** +Access token has been deprecated in GitLab 9.4 in favor of [IP whitelist](#ip-whitelist). An access token needs to be provided while accessing the probe endpoints. The current accepted token can be found under the **Admin Area > Monitoring > Health check** @@ -152,6 +152,10 @@ The access token can be passed as a URL parameter: https://gitlab.example.com/-/readiness?token=ACCESS_TOKEN ``` +NOTE: **Note:** +In case the database or Redis service are unaccessible, the probe endpoints response is not guaranteed to be correct. +You should switch to [IP whitelist](#ip-whitelist) from deprecated access token to avoid it. + <!-- ## Troubleshooting Include any troubleshooting steps that you can foresee. If you know beforehand what issues diff --git a/lib/gitlab/health_checks/probes/collection.rb b/lib/gitlab/health_checks/probes/collection.rb index db3ef4834c2d8470c95da77de1882e49c9507658..08b6d82291ee2ba3c2a42960a28403f44655c639 100644 --- a/lib/gitlab/health_checks/probes/collection.rb +++ b/lib/gitlab/health_checks/probes/collection.rb @@ -20,6 +20,12 @@ def execute success ? 200 : 503, status(success).merge(payload(readiness)) ) + rescue => e + exception_payload = { message: "#{e.class} : #{e.message}" } + + Probes::Status.new( + 500, + status(false).merge(exception_payload)) end private diff --git a/spec/lib/gitlab/health_checks/probes/collection_spec.rb b/spec/lib/gitlab/health_checks/probes/collection_spec.rb index 3371f3986b6c62ff279eb4091663ea87b1c34c42..03138e936aab571007bcbe9d02445badf4986792 100644 --- a/spec/lib/gitlab/health_checks/probes/collection_spec.rb +++ b/spec/lib/gitlab/health_checks/probes/collection_spec.rb @@ -47,6 +47,20 @@ status: 'failed', message: 'check error') end end + + context 'when check raises exception not handled inside the check' do + before do + expect(Gitlab::HealthChecks::Redis::RedisCheck).to receive(:readiness).and_raise( + ::Redis::CannotConnectError, 'Redis down') + end + + it 'responds with failure including the exception info' do + expect(subject.http_status).to eq(500) + + expect(subject.json[:status]).to eq('failed') + expect(subject.json[:message]).to eq('Redis::CannotConnectError : Redis down') + end + end end context 'without checks' do diff --git a/spec/requests/health_controller_spec.rb b/spec/requests/health_controller_spec.rb index 8b0492a27b95bc7bef74a7df909dadc298a99660..592a57bc63796f9e2f1ae503e648edd152a5dba2 100644 --- a/spec/requests/health_controller_spec.rb +++ b/spec/requests/health_controller_spec.rb @@ -129,6 +129,40 @@ expect(response).to have_gitlab_http_status(:service_unavailable) expect(response.headers['X-GitLab-Custom-Error']).to eq(1) end + + context 'when DB is not accessible and connection raises an exception' do + before do + expect(Gitlab::HealthChecks::DbCheck) + .to receive(:readiness) + .and_raise(PG::ConnectionBad, 'could not connect to server') + end + + it 'responds with 500 including the exception info' do + subject + + expect(response).to have_gitlab_http_status(:internal_server_error) + expect(response.headers['X-GitLab-Custom-Error']).to eq(1) + expect(json_response).to eq( + { 'status' => 'failed', 'message' => 'PG::ConnectionBad : could not connect to server' }) + end + end + + context 'when any exception happens during the probing' do + before do + expect(Gitlab::HealthChecks::Redis::RedisCheck) + .to receive(:readiness) + .and_raise(::Redis::CannotConnectError, 'Redis down') + end + + it 'responds with 500 including the exception info' do + subject + + expect(response).to have_gitlab_http_status(:internal_server_error) + expect(response.headers['X-GitLab-Custom-Error']).to eq(1) + expect(json_response).to eq( + { 'status' => 'failed', 'message' => 'Redis::CannotConnectError : Redis down' }) + end + end end end