diff options
author | Stan Hu <stanhu@gmail.com> | 2018-07-27 12:19:51 -0700 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2018-07-28 14:35:02 -0700 |
commit | 22d44ae9a699c418aba442cd99eec8f58d5fa006 (patch) | |
tree | 035cd854d43f390b3684e123be74482485e25296 | |
parent | eb2bc7d99a99981150f32ac2469bff29eebbfa19 (diff) | |
download | gitlab-ce-22d44ae9a699c418aba442cd99eec8f58d5fa006.tar.gz |
Use /-/health instead of breaking /-/liveness
-rw-r--r-- | app/controllers/health_controller.rb | 5 | ||||
-rw-r--r-- | changelogs/unreleased/sh-simplify-liveness-check.yml | 6 | ||||
-rw-r--r-- | config/application.rb | 2 | ||||
-rw-r--r-- | config/routes.rb | 3 | ||||
-rw-r--r-- | doc/user/admin_area/monitoring/health_check.md | 39 | ||||
-rw-r--r-- | lib/gitlab/middleware/basic_health_check.rb (renamed from lib/gitlab/middleware/liveness_health_check.rb) | 8 | ||||
-rw-r--r-- | spec/controllers/health_controller_spec.rb | 51 | ||||
-rw-r--r-- | spec/lib/gitlab/middleware/basic_health_check_spec.rb (renamed from spec/lib/gitlab/middleware/liveness_health_check_spec.rb) | 8 |
8 files changed, 101 insertions, 21 deletions
diff --git a/app/controllers/health_controller.rb b/app/controllers/health_controller.rb index 72f59693adc..3fedd5bfb29 100644 --- a/app/controllers/health_controller.rb +++ b/app/controllers/health_controller.rb @@ -18,8 +18,9 @@ class HealthController < ActionController::Base end def liveness - # This should never be called; it should be intercepted by LivenessHealthCheck middleware - head :not_found + results = CHECKS.map { |check| [check.name, check.liveness] } + + render_check_results(results) end def storage_check diff --git a/changelogs/unreleased/sh-simplify-liveness-check.yml b/changelogs/unreleased/sh-simplify-liveness-check.yml index 7eb108a1300..225e3dc1378 100644 --- a/changelogs/unreleased/sh-simplify-liveness-check.yml +++ b/changelogs/unreleased/sh-simplify-liveness-check.yml @@ -1,5 +1,5 @@ --- -title: Simplify /-/liveness check to avoid connecting to the database -merge_request: +title: Add /-/health basic health check endpoint +merge_request: 20456 author: -type: changed +type: added diff --git a/config/application.rb b/config/application.rb index ae4ff5dd7c9..a086e860e16 100644 --- a/config/application.rb +++ b/config/application.rb @@ -156,7 +156,7 @@ module Gitlab # This middleware needs to precede ActiveRecord::QueryCache and other middlewares that # connect to the database. - config.middleware.insert_after "Rails::Rack::Logger", "Gitlab::Middleware::LivenessHealthCheck" + config.middleware.insert_after "Rails::Rack::Logger", "Gitlab::Middleware::BasicHealthCheck" config.middleware.insert_after Warden::Manager, Rack::Attack diff --git a/config/routes.rb b/config/routes.rb index 6bf0335a923..aac3a77c4f1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -46,7 +46,8 @@ Rails.application.routes.draw do get 'health_check(/:checks)' => 'health_check#index', as: :health_check scope path: '-' do - get 'liveness' => 'health#liveness' # Intercepted via Gitlab::Middleware::LivenessHealthCheck + # '/-/health' implemented by BasicHealthMiddleware + get 'liveness' => 'health#liveness' get 'readiness' => 'health#readiness' post 'storage_check' => 'health#storage_check' resources :metrics, only: [:index] diff --git a/doc/user/admin_area/monitoring/health_check.md b/doc/user/admin_area/monitoring/health_check.md index 958962fbfbc..6cbca6bd1db 100644 --- a/doc/user/admin_area/monitoring/health_check.md +++ b/doc/user/admin_area/monitoring/health_check.md @@ -20,14 +20,24 @@ To access monitoring resources, the client IP needs to be included in a whitelis [Read how to add IPs to a whitelist for the monitoring endpoints][admin]. -## Using the endpoint +## Using the endpoints With default whitelist settings, the probes can be accessed from localhost: +- `http://localhost/-/health` - `http://localhost/-/readiness` - `http://localhost/-/liveness` -The readiness endpoint will provide a report of system health in JSON format. + +The first endpoint, `/-/health/`, only checks whether the application server is running. It does +-not verify the database or other services are running. A successful response with return +a 200 status code with the following message: + +``` +GitLab OK +``` + +The readiness and liveness probes will provide a report of system health in JSON format. Readiness example output: @@ -57,12 +67,29 @@ Readiness example output: } ``` -The liveness endpoint only checks whether the application server is running. It does -not verify the database or other services are running. A successful response with return -a 200 status code with the following message: +Liveness example output: ``` -GitLab is alive +{ + "fs_shards_check" : { + "status" : "ok" + }, + "cache_check" : { + "status" : "ok" + }, + "db_check" : { + "status" : "ok" + }, + "redis_check" : { + "status" : "ok" + }, + "queues_check" : { + "status" : "ok" + }, + "shared_state_check" : { + "status" : "ok" + } +} ``` ## Status diff --git a/lib/gitlab/middleware/liveness_health_check.rb b/lib/gitlab/middleware/basic_health_check.rb index 404c506a08e..f2a03217098 100644 --- a/lib/gitlab/middleware/liveness_health_check.rb +++ b/lib/gitlab/middleware/basic_health_check.rb @@ -9,20 +9,20 @@ module Gitlab module Middleware - class LivenessHealthCheck + class BasicHealthCheck # This can't be frozen because Rails::Rack::Logger wraps the body # rubocop:disable Style/MutableConstant - OK_RESPONSE = [200, { 'Content-Type' => 'text/plain' }, ["GitLab is alive"]] + OK_RESPONSE = [200, { 'Content-Type' => 'text/plain' }, ["GitLab OK"]] EMPTY_RESPONSE = [404, { 'Content-Type' => 'text/plain' }, [""]] # rubocop:enable Style/MutableConstant - LIVENESS_PATH = '/-/liveness' + HEALTH_PATH = '/-/health' def initialize(app) @app = app end def call(env) - return @app.call(env) unless env['PATH_INFO'] == LIVENESS_PATH + return @app.call(env) unless env['PATH_INFO'] == HEALTH_PATH request = Rack::Request.new(env) diff --git a/spec/controllers/health_controller_spec.rb b/spec/controllers/health_controller_spec.rb index 789b35dc3de..d800ad7c187 100644 --- a/spec/controllers/health_controller_spec.rb +++ b/spec/controllers/health_controller_spec.rb @@ -109,4 +109,55 @@ describe HealthController do end end end + + describe '#liveness' do + shared_context 'endpoint responding with liveness data' do + subject { get :liveness } + + it 'responds with liveness checks data' do + subject + + expect(json_response['db_check']['status']).to eq('ok') + expect(json_response['cache_check']['status']).to eq('ok') + expect(json_response['queues_check']['status']).to eq('ok') + expect(json_response['shared_state_check']['status']).to eq('ok') + end + end + + context 'accessed from whitelisted ip' do + before do + allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip) + end + + it_behaves_like 'endpoint responding with liveness data' + end + + context 'accessed from not whitelisted ip' do + before do + allow(Gitlab::RequestContext).to receive(:client_ip).and_return(not_whitelisted_ip) + end + + it 'responds with resource not found' do + get :liveness + + expect(response.status).to eq(404) + end + + context 'accessed with valid token' do + context 'token passed in request header' do + before do + request.headers['TOKEN'] = token + end + + it_behaves_like 'endpoint responding with liveness data' + end + + context 'token passed as URL param' do + it_behaves_like 'endpoint responding with liveness data' do + subject { get :liveness, token: token } + end + end + end + end + end end diff --git a/spec/lib/gitlab/middleware/liveness_health_check_spec.rb b/spec/lib/gitlab/middleware/basic_health_check_spec.rb index 3dee13b7770..187d903a5e1 100644 --- a/spec/lib/gitlab/middleware/liveness_health_check_spec.rb +++ b/spec/lib/gitlab/middleware/basic_health_check_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Middleware::LivenessHealthCheck do +describe Gitlab::Middleware::BasicHealthCheck do let(:app) { double(:app) } let(:middleware) { described_class.new(app) } let(:env) { {} } @@ -12,7 +12,7 @@ describe Gitlab::Middleware::LivenessHealthCheck do end it 'returns a 404' do - env['PATH_INFO'] = described_class::LIVENESS_PATH + env['PATH_INFO'] = described_class::HEALTH_PATH response = middleware.call(env) @@ -34,7 +34,7 @@ describe Gitlab::Middleware::LivenessHealthCheck do end it 'returns 200 response when endpoint is hit' do - env['PATH_INFO'] = described_class::LIVENESS_PATH + env['PATH_INFO'] = described_class::HEALTH_PATH expect(app).not_to receive(:call) @@ -42,7 +42,7 @@ describe Gitlab::Middleware::LivenessHealthCheck do expect(response[0]).to eq(200) expect(response[1]).to eq({ 'Content-Type' => 'text/plain' }) - expect(response[2]).to eq(['GitLab is alive']) + expect(response[2]).to eq(['GitLab OK']) end it 'forwards the call for other paths' do |