From 1371798b8083b37ccdfb3e1b01ce7e4ce0e04b9a Mon Sep 17 00:00:00 2001 From: "A. Wilcox" Date: Mon, 1 Dec 2025 16:29:23 -0600 Subject: [PATCH] app: Convert health checks to use OKComputer * The Alma patron fetch has been converted to a custom check. * ActionMailer is checked for connectivity. Note that the test is marked pending until emmahsax/okcomputer#21 is merged, because the `:test` delivery method isn't recognised for the ActionMailer check. Once it is merged, we can update the Gemfile to use the new version and then the tests will pass. Ref: AP-508 --- Gemfile | 1 + Gemfile.lock | 4 +++ app/controllers/home_controller.rb | 8 ----- app/lib/health/check.rb | 57 ------------------------------ app/lib/health/result.rb | 28 --------------- app/lib/health/status.rb | 23 ------------ config/initializers/okcomputer.rb | 28 +++++++++++++++ config/routes.rb | 2 +- lib/tasks/health.rake | 9 ----- spec/lib/health/status_spec.rb | 28 --------------- spec/request/home_request_spec.rb | 35 ------------------ spec/request/okcomputer_spec.rb | 29 +++++++++++++++ spec/system/home_system_spec.rb | 8 ----- 13 files changed, 63 insertions(+), 197 deletions(-) delete mode 100644 app/lib/health/check.rb delete mode 100644 app/lib/health/result.rb delete mode 100644 app/lib/health/status.rb create mode 100644 config/initializers/okcomputer.rb delete mode 100644 lib/tasks/health.rake delete mode 100644 spec/lib/health/status_spec.rb create mode 100644 spec/request/okcomputer_spec.rb diff --git a/Gemfile b/Gemfile index a37234bd..d092ef9d 100644 --- a/Gemfile +++ b/Gemfile @@ -27,6 +27,7 @@ gem 'lograge', '>=0.11.2' gem 'mutex_m' gem 'netaddr', '~> 1.5', '>= 1.5.1' gem 'net-ssh' +gem 'okcomputer', '~> 1.19' gem 'omniauth', '~> 1.9', '>= 1.9.2' gem 'omniauth-cas', '~> 2.0' gem 'pg', '~> 1.2' diff --git a/Gemfile.lock b/Gemfile.lock index 4ded878c..dcfbc325 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -74,6 +74,7 @@ GEM execjs (~> 2) awesome_print (1.9.2) base64 (0.3.0) + benchmark (0.5.0) berkeley_library-alma (0.1.1) berkeley_library-logging (~> 0.2) berkeley_library-marc (~> 0.3.1) @@ -258,6 +259,8 @@ GEM oj (3.16.11) bigdecimal (>= 3.0) ostruct (>= 0.2) + okcomputer (1.19.1) + benchmark omniauth (1.9.2) hashie (>= 3.4.6) rack (>= 1.6.2, < 3) @@ -500,6 +503,7 @@ DEPENDENCIES mutex_m net-ssh netaddr (~> 1.5, >= 1.5.1) + okcomputer (~> 1.19) omniauth (~> 1.9, >= 1.9.2) omniauth-cas (~> 2.0) pg (~> 1.2) diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index 66a9d2ef..da7909fb 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -3,14 +3,6 @@ # We'll likely be asked to remove this at some point, but for now it's useful # to have in development. class HomeController < ApplicationController - - # TODO: Move this to a HealthController that extends ActionController::API - # - note: may involve extracting some of ApplicationController into a mixin - def health - check = Health::Check.new - render json: check, status: check.http_status_code - end - def admin render :admin if require_framework_admin! end diff --git a/app/lib/health/check.rb b/app/lib/health/check.rb deleted file mode 100644 index a7a2779e..00000000 --- a/app/lib/health/check.rb +++ /dev/null @@ -1,57 +0,0 @@ -module Health - - # Checks on the health of critical application dependencies - # - # @see https://tools.ietf.org/id/draft-inadarei-api-health-check-01.html JSON Format - # @see https://www.consul.io/docs/agent/checks.html StatusCode based on Consul - # - # @todo We could improve on this by adding a few additional checks, namely: - # - That we can actually send emails - # - That we can add notes to patrons (SSH access to the millennium scripts) - class Check - - TEST_PATRON_ID = '012158720'.freeze - - attr_reader :status - attr_reader :details - - def initialize - status = Status::PASS - details = {} - all_checks.each do |name, check_method| - result = check_method.call - details[name] = result.as_json - status &= result.status - end - - @status = status - @details = details - end - - def as_json(*) - { status:, details: } - end - - def http_status_code - passing? ? 200 : 429 - end - - private - - def all_checks - { 'patron_api:find' => method(:try_find_patron) } - end - - def passing? - status == Status::PASS - end - - def try_find_patron - Alma::User.find(TEST_PATRON_ID) - Result.pass - rescue StandardError => e - Result.warn(e.class.name) - end - - end -end diff --git a/app/lib/health/result.rb b/app/lib/health/result.rb deleted file mode 100644 index dfd93faf..00000000 --- a/app/lib/health/result.rb +++ /dev/null @@ -1,28 +0,0 @@ -module Health - # Encapsulates a health check result - class Result - attr_reader :status - attr_reader :output - - def initialize(status:, output: nil) - @status = status - @output = output - end - - def as_json(*) - json = { status: status.as_json } - json[:output] = output if output - json - end - - class << self - def pass - Result.new(status: Status::PASS) - end - - def warn(output) - Result.new(status: Status::WARN, output:) - end - end - end -end diff --git a/app/lib/health/status.rb b/app/lib/health/status.rb deleted file mode 100644 index 131f45ac..00000000 --- a/app/lib/health/status.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'typesafe_enum' - -module Health - # Enumerated list of health states - class Status < TypesafeEnum::Base - new :PASS # NOTE: states should be ordered from least to most severe - new :WARN - - # Concatenates health states, returning the more severe state. - # @return [Status] the more severe status - def &(other) - return self unless other - - [self, other].max - end - - # Returns the status as a string, suitable for use as a JSON value. - # @return [String] the name of the status, in lower case - def as_json(*) - value.downcase - end - end -end diff --git a/config/initializers/okcomputer.rb b/config/initializers/okcomputer.rb new file mode 100644 index 00000000..b61c407a --- /dev/null +++ b/config/initializers/okcomputer.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# Health check configuration + +OkComputer.logger = Rails.logger +OkComputer.check_in_parallel = true + +class AlmaPatronCheck < OkComputer::Check + TEST_PATRON_ID = '000311@lbl.gov' + + def check + Alma::User.find(TEST_PATRON_ID) + mark_message 'Success' + rescue StandardError => e + mark_failure + Rails.logger.warn "Couldn't connect to Alma API during health check: #{e}" + mark_message 'Failed' + end +end + +# Ensure Alma API is working. +OkComputer::Registry.register 'alma-patron-lookup', AlmaPatronCheck.new + +# Ensure database migrations have been run. +OkComputer::Registry.register 'database-migrations', OkComputer::ActiveRecordMigrationsCheck.new + +# Ensure connectivity to the mail system. +OkComputer::Registry.register 'action-mailer', OkComputer::ActionMailerCheck.new diff --git a/config/routes.rb b/config/routes.rb index e8f7bbdd..be450bd1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -2,7 +2,7 @@ root to: redirect('/forms/altmedia/new') get 'admin', to: 'home#admin' - get 'health', to: 'home#health' + get 'health', to: 'ok_computer/ok_computer#index', defaults: { format: :json } get 'home', to: 'home#index' get 'build_info', to: 'home#build_info' diff --git a/lib/tasks/health.rake b/lib/tasks/health.rake deleted file mode 100644 index bb8796e9..00000000 --- a/lib/tasks/health.rake +++ /dev/null @@ -1,9 +0,0 @@ -namespace :health do - desc 'Query the healthcheck endpoint' - task check: :environment do - session = ActionDispatch::Integration::Session.new(Rails.application) - session.get(Rails.application.routes.url_helpers.health_path) - puts session.response.body - exit 1 if session.response.server_error? - end -end diff --git a/spec/lib/health/status_spec.rb b/spec/lib/health/status_spec.rb deleted file mode 100644 index 116fe8f6..00000000 --- a/spec/lib/health/status_spec.rb +++ /dev/null @@ -1,28 +0,0 @@ -require 'rails_helper' - -module Health - - describe Status do - describe '&' do - it 'handles nil' do - expect(Status::PASS & nil).to eq(Status::PASS) - end - - it 'handles self' do - expect(Status::PASS & Status::PASS).to eq(Status::PASS) - expect(Status::WARN & Status::WARN).to eq(Status::WARN) - end - - it 'respects order' do - expect(Status::WARN & Status::PASS).to eq(Status::WARN) - expect(Status::PASS & Status::WARN).to eq(Status::WARN) - end - - it 'supports &=' do - status = Status::PASS - status &= Status::WARN - expect(status).to eq(Status::WARN) - end - end - end -end diff --git a/spec/request/home_request_spec.rb b/spec/request/home_request_spec.rb index 4225d35c..ef64c7d6 100644 --- a/spec/request/home_request_spec.rb +++ b/spec/request/home_request_spec.rb @@ -1,41 +1,6 @@ require 'calnet_helper' describe HomeController, type: :request do - describe :health do - it 'returns OK for a successful patron lookup' do - patron = Alma::User.new - - expect(Alma::User).to receive(:find).with(Health::Check::TEST_PATRON_ID).and_return(patron) - get health_path - expect(response).to have_http_status(:ok) - expected_body = { - 'status' => 'pass', - 'details' => { - 'patron_api:find' => { - 'status' => 'pass' - } - } - } - expect(JSON.parse(response.body)).to eq(expected_body) - end - - it 'returns 429 Too Many Requests for a failure' do - expect(Alma::User).to receive(:find).and_raise('Something went wrong') - get health_path - expect(response).to have_http_status(:too_many_requests) - expected_body = { - 'status' => 'warn', - 'details' => { - 'patron_api:find' => { - 'status' => 'warn', - 'output' => 'RuntimeError' - } - } - } - expect(JSON.parse(response.body)).to eq(expected_body) - end - end - RSpec.shared_examples 'allow admin' do |page:| before do diff --git a/spec/request/okcomputer_spec.rb b/spec/request/okcomputer_spec.rb new file mode 100644 index 00000000..fbf8d418 --- /dev/null +++ b/spec/request/okcomputer_spec.rb @@ -0,0 +1,29 @@ +require 'rails_helper' + +RSpec.describe 'OKComputer', type: :request do + before { allow(Alma::User).to receive(:find).and_return(Alma::User.new) } + + it 'is mounted at /okcomputer' do + get '/okcomputer' + expect(response).to have_http_status :ok + end + + it 'returns all checks to /health' do + get '/health' + expect(response.parsed_body.keys).to match_array %w[ + action-mailer + alma-patron-lookup + default + database + database-migrations + ] + pending 'https://github.com/emmahsax/okcomputer/pull/21' + expect(response).to have_http_status :ok + end + + it 'fails when Alma lookups fail' do + expect(Alma::User).to receive(:find).and_raise('Uh oh!') + get '/health' + expect(response).not_to have_http_status :ok + end +end diff --git a/spec/system/home_system_spec.rb b/spec/system/home_system_spec.rb index d79839b9..81581ec3 100644 --- a/spec/system/home_system_spec.rb +++ b/spec/system/home_system_spec.rb @@ -3,14 +3,6 @@ require 'support/build_info_context' describe :home, type: :system do - describe :health do - it 'does something sensible for a general error' do - expect(Health::Check).to receive(:new).and_raise('Something went wrong') - visit health_path - expect(page).to have_content('Internal Server Error') - end - end - describe :admin do context 'without login' do it 'redirects to login' do