From eec1f4b3d69fc55618430f09ada3d6d0b0e693cf Mon Sep 17 00:00:00 2001 From: Josh Klina Date: Tue, 23 Jan 2024 17:18:49 -0500 Subject: [PATCH] Make health probe server more general purpose (#1079) * Make health probe server more general purpose This removes the health check logic from the ProbeServer and renames the ProbeServer to UtilityServer that accepts any Rack based app. The health check and catchall logic are moved into simple Rack middleware that can be composed by users however they like and be used to preserve existing health check behavior while transitioning to a more general purpose utility server. All and all this pattern will allow users to add whatever functionality they like to GoodJob's web server by composing Rack apps and using GoodJob's configuration to pass in users' Rack apps. IE: ``` config.good_job.middleware = Rack::Builder.app do use GoodJob::Middleware::MyCustomMiddleware use GoodJob::Middleware::PrometheusExporter use GoodJob::Middleware::Healthcheck run GoodJob::Middleware::CatchAll end config.good_job.middleware_port = 7001 ``` This could help resolve: * https://github.com/bensheldon/good_job/issues/750 * https://github.com/bensheldon/good_job/issues/532 * Use new API * Revert server name change We decided to leave the original ProbeServer name better sets expectations. See: https://github.com/bensheldon/good_job/pull/1079#pullrequestreview-1631284009 This also splits out middleware testing into separate specs. * Restore original naming This also helps ensure that the existing behavior and API remain intact. * Appease linters * Add required message for mock * Make test description relevant * Allow for handler to be injected into ProbeServer * Add WEBrick WEBrick handler * Add WEBrick as a development dependency * Add WEBrick tests and configuration * Add idle_timeout method to mock * Namespace server handlers * Warn and fallback when WEBrick isn't loadable Since the probe server has the option to use WEBrick as a server handler, but this library doesn't have WEBrick as a dependency, we want to throw a warning when WEBrick is configured, but not in the load path. This will also gracefully fallback to the built in HTTP server. * inspect load path * Account for multiple webrick entries in $LOAD_PATH * try removing load path test * For error on require to initiate test As opposed to manipulating the load path. * Handle explicit nils in intialization * Allow probe_handler to be set in configuration * Add documentation for probe server customization * Appease linter * retrigger CI * Rename `probe_server_app` to `probe_app`; make handler name a symbol; rename Rack middleware/app for clarity * Update readme to have relevant app example * Fix readme grammar --------- Co-authored-by: Ben Sheldon [he/him] --- Gemfile.lock | 1 + README.md | 98 +++++++- good_job.gemspec | 1 + lib/good_job.rb | 5 +- lib/good_job/cli.rb | 5 +- lib/good_job/configuration.rb | 20 +- lib/good_job/http_server.rb | 77 ------- lib/good_job/probe_server.rb | 37 ++-- .../probe_server/healthcheck_middleware.rb | 27 +++ lib/good_job/probe_server/not_found_app.rb | 11 + lib/good_job/probe_server/simple_handler.rb | 83 +++++++ lib/good_job/probe_server/webrick_handler.rb | 28 +++ spec/lib/good_job/cli_spec.rb | 68 +++++- .../healthcheck_middleware_spec.rb | 91 ++++++++ .../probe_server/not_found_app_spec.rb | 12 + spec/lib/good_job/probe_server_spec.rb | 209 ++++++++++++------ 16 files changed, 587 insertions(+), 186 deletions(-) delete mode 100644 lib/good_job/http_server.rb create mode 100644 lib/good_job/probe_server/healthcheck_middleware.rb create mode 100644 lib/good_job/probe_server/not_found_app.rb create mode 100644 lib/good_job/probe_server/simple_handler.rb create mode 100644 lib/good_job/probe_server/webrick_handler.rb create mode 100644 spec/lib/good_job/probe_server/healthcheck_middleware_spec.rb create mode 100644 spec/lib/good_job/probe_server/not_found_app_spec.rb diff --git a/Gemfile.lock b/Gemfile.lock index 344ef5223..0f85d538a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -542,6 +542,7 @@ DEPENDENCIES spoom stackprof tapioca + webrick yard yard-activesupport-concern diff --git a/README.md b/README.md index 5d42c6098..c683b8c3c 100644 --- a/README.md +++ b/README.md @@ -177,18 +177,19 @@ Usage: good_job start Options: - [--queues=QUEUE_LIST] # Queues or pools to work from. (env var: GOOD_JOB_QUEUES, default: *) - [--max-threads=COUNT] # Default number of threads per pool to use for working jobs. (env var: GOOD_JOB_MAX_THREADS, default: 5) - [--poll-interval=SECONDS] # Interval between polls for available jobs in seconds (env var: GOOD_JOB_POLL_INTERVAL, default: 10) - [--max-cache=COUNT] # Maximum number of scheduled jobs to cache in memory (env var: GOOD_JOB_MAX_CACHE, default: 10000) - [--shutdown-timeout=SECONDS] # Number of seconds to wait for jobs to finish when shutting down before stopping the thread. (env var: GOOD_JOB_SHUTDOWN_TIMEOUT, default: -1 (forever)) - [--enable-cron] # Whether to run cron process (default: false) - [--enable-listen-notify] # Whether to enqueue and read jobs with Postgres LISTEN/NOTIFY (default: true) - [--idle-timeout=SECONDS] # Exit process when no jobs have been performed for this many seconds (env var: GOOD_JOB_IDLE_TIMEOUT, default: nil) - [--daemonize] # Run as a background daemon (default: false) - [--pidfile=PIDFILE] # Path to write daemonized Process ID (env var: GOOD_JOB_PIDFILE, default: tmp/pids/good_job.pid) - [--probe-port=PORT] # Port for http health check (env var: GOOD_JOB_PROBE_PORT, default: nil) - [--queue-select-limit=COUNT] # The number of queued jobs to select when polling for a job to run. (env var: GOOD_JOB_QUEUE_SELECT_LIMIT, default: nil)" + [--queues=QUEUE_LIST] # Queues or pools to work from. (env var: GOOD_JOB_QUEUES, default: *) + [--max-threads=COUNT] # Default number of threads per pool to use for working jobs. (env var: GOOD_JOB_MAX_THREADS, default: 5) + [--poll-interval=SECONDS] # Interval between polls for available jobs in seconds (env var: GOOD_JOB_POLL_INTERVAL, default: 10) + [--max-cache=COUNT] # Maximum number of scheduled jobs to cache in memory (env var: GOOD_JOB_MAX_CACHE, default: 10000) + [--shutdown-timeout=SECONDS] # Number of seconds to wait for jobs to finish when shutting down before stopping the thread. (env var: GOOD_JOB_SHUTDOWN_TIMEOUT, default: -1 (forever)) + [--enable-cron] # Whether to run cron process (default: false) + [--enable-listen-notify] # Whether to enqueue and read jobs with Postgres LISTEN/NOTIFY (default: true) + [--idle-timeout=SECONDS] # Exit process when no jobs have been performed for this many seconds (env var: GOOD_JOB_IDLE_TIMEOUT, default: nil) + [--daemonize] # Run as a background daemon (default: false) + [--pidfile=PIDFILE] # Path to write daemonized Process ID (env var: GOOD_JOB_PIDFILE, default: tmp/pids/good_job.pid) + [--probe-port=PORT] # Port for http health check (env var: GOOD_JOB_PROBE_PORT, default: nil) + [--probe-handler=PROBE_HANDLER] # Use 'webrick' to use WEBrick to handle probe server requests which is Rack compliant, otherwise default server that is not Rack compliant is used. + [--queue-select-limit=COUNT] # The number of queued jobs to select when polling for a job to run. (env var: GOOD_JOB_QUEUE_SELECT_LIMIT, default: nil)" Executes queued jobs. @@ -304,6 +305,18 @@ Available configuration options are: config.good_job.on_thread_error = -> (exception) { Rails.error.report(exception) } ``` +- `probe_server_app` (Rack application) allows you to specify a Rack application to be used for the probe server. Defaults to `nil` which uses the default probe server. Example: + + ```ruby + config.good_job.probe_app = -> (env) { [200, {}, ["OK"]] } + ``` + +- `probe_handler` (string) allows you to use WEBrick, a fully Rack compliant webserver instead of the simple default server. **Note:** You'll need to ensure WEBrick is in your load path as GoodJob doesn't have WEBrick as a dependency. Example: + + ```ruby + config.good_job.probe_handler = 'webrick' + ``` + By default, GoodJob configures the following execution modes per environment: ```ruby @@ -1321,6 +1334,8 @@ A workaround to this limitation is to make a direct database connection availabl ### CLI HTTP health check probes +#### Default configuration + GoodJob's CLI offers an http health check probe to better manage process lifecycle in containerized environments like Kubernetes: ```bash @@ -1374,6 +1389,65 @@ spec: periodSeconds: 10 ``` +#### Custom configuration + +The CLI health check probe server can be customized to serve additional information. Two things to note when customizing the probe server: + +- By default, the probe server uses a homespun single thread, blocking server so your custom app should be very simple and lightly used and could affect job performance. +- The default probe server is not fully Rack compliant. Rack specifies various mandatory fields and some Rack apps assume those fields exist. If you do need to use a Rack app that depends on being fully Rack compliant, you can configure GoodJob to [use WEBrick as the server](#using-webrick) + +To customize the probe server, set `config.good_job.probe_app` to a Rack app or a Rack builder: + +```ruby +# config/initializers/good_job.rb OR config/application.rb OR config/environments/{RAILS_ENV}.rb + +Rails.application.configure do + config.good_job.probe_app = Rack::Builder.new do + # Add your custom middleware + use Custom::AuthorizationMiddleware + use Custom::PrometheusExporter + + # This is the default middleware + use GoodJob::ProbeServer::HealthcheckMiddleware + run GoodJob::ProbeServer::NotFoundApp # will return 404 for all other requests + end +end +``` + +##### Using WEBrick + +If your custom app requires a fully Rack compliant server, you can configure GoodJob to use WEBrick as the server: + +```ruby +# config/initializers/good_job.rb OR config/application.rb OR config/environments/{RAILS_ENV}.rb + +Rails.application.configure do + config.good_job.probe_handler = :webrick +end + +``` + +You can also enable WEBrick through the command line: + +```bash +good_job start --probe-handler=webrick +``` + +or via an environment variable: + +```bash +GOOD_JOB_PROBE_HANDLER=webrick good_job start +``` + +Note that GoodJob doesn't include WEBrick as a dependency, so you'll need to add it to your Gemfile: + +```ruby +# Gemfile +gem 'webrick' +``` + +If WEBrick is configured to be used, but the dependency is not found, GoodJob will log a warning and fallback to the default probe server. + ## Contribute diff --git a/good_job.gemspec b/good_job.gemspec index 5161aed3f..849e31529 100644 --- a/good_job.gemspec +++ b/good_job.gemspec @@ -66,6 +66,7 @@ Gem::Specification.new do |spec| spec.add_development_dependency "puma", "~> 5.6" # waiting on Capybara support for Puma v6 spec.add_development_dependency "rspec-rails" spec.add_development_dependency "selenium-webdriver" + spec.add_development_dependency "webrick" spec.add_development_dependency "yard" spec.add_development_dependency "yard-activesupport-concern" end diff --git a/lib/good_job.rb b/lib/good_job.rb index 1287e57f4..3b678b669 100644 --- a/lib/good_job.rb +++ b/lib/good_job.rb @@ -32,8 +32,11 @@ require "good_job/multi_scheduler" require "good_job/notifier" require "good_job/poller" -require "good_job/http_server" require "good_job/probe_server" +require "good_job/probe_server/healthcheck_middleware" +require "good_job/probe_server/not_found_app" +require "good_job/probe_server/simple_handler" +require "good_job/probe_server/webrick_handler" require "good_job/scheduler" require "good_job/shared_executor" require "good_job/systemd_service" diff --git a/lib/good_job/cli.rb b/lib/good_job/cli.rb index ffb5931ee..2c7b8f218 100644 --- a/lib/good_job/cli.rb +++ b/lib/good_job/cli.rb @@ -94,6 +94,9 @@ def exit_on_failure? type: :numeric, banner: 'PORT', desc: "Port for http health check (env var: GOOD_JOB_PROBE_PORT, default: nil)" + method_option :probe_handler, + type: :string, + desc: "Use 'webrick' to use WEBrick to handle probe server requests which is Rack compliant, otherwise default server that is not Rack compliant is used. (env var: GOOD_JOB_PROBE_HANDLER, default: nil)" method_option :queue_select_limit, type: :numeric, banner: 'COUNT', @@ -112,7 +115,7 @@ def start systemd.start if configuration.probe_port - probe_server = GoodJob::ProbeServer.new(port: configuration.probe_port) + probe_server = GoodJob::ProbeServer.new(app: configuration.probe_app, port: configuration.probe_port, handler: configuration.probe_handler) probe_server.start end diff --git a/lib/good_job/configuration.rb b/lib/good_job/configuration.rb index fa655bf6c..6ce375934 100644 --- a/lib/good_job/configuration.rb +++ b/lib/good_job/configuration.rb @@ -342,10 +342,26 @@ def pidfile end # Port of the probe server - # @return [nil,Integer] + # @return [nil, Integer] def probe_port - options[:probe_port] || + (options[:probe_port] || env['GOOD_JOB_PROBE_PORT'] + )&.to_i + end + + # Probe server handler + # @return [nil, Symbol] + def probe_handler + (options[:probe_handler] || + rails_config[:probe_handler] || + env['GOOD_JOB_PROBE_HANDLER'] + )&.to_sym + end + + # Rack compliant application to be run on the ProbeServer + # @return [nil, Class] + def probe_app + rails_config[:probe_app] end def enable_listen_notify diff --git a/lib/good_job/http_server.rb b/lib/good_job/http_server.rb deleted file mode 100644 index ba4f69cdb..000000000 --- a/lib/good_job/http_server.rb +++ /dev/null @@ -1,77 +0,0 @@ -# frozen_string_literal: true - -module GoodJob - class HttpServer - SOCKET_READ_TIMEOUT = 5 # in seconds - - def initialize(app, options = {}) - @app = app - @port = options[:port] - @logger = options[:logger] - - @running = Concurrent::AtomicBoolean.new(false) - end - - def run - @running.make_true - start_server - handle_connections if @running.true? - rescue StandardError => e - @logger.error "Server encountered an error: #{e}" - ensure - stop - end - - def stop - @running.make_false - @server&.close - end - - def running? - @running.true? - end - - private - - def start_server - @server = TCPServer.new('0.0.0.0', @port) - rescue StandardError => e - @logger.error "Failed to start server: #{e}" - @running.make_false - end - - def handle_connections - while @running.true? - begin - ready_sockets, = IO.select([@server], nil, nil, SOCKET_READ_TIMEOUT) - next unless ready_sockets - - client = @server.accept_nonblock - request = client.gets - - if request - status, headers, body = @app.call(parse_request(request)) - respond(client, status, headers, body) - end - - client.close - rescue IO::WaitReadable, Errno::EINTR, Errno::EPIPE - retry - end - end - end - - def parse_request(request) - method, full_path = request.split - path, query = full_path.split('?') - { 'REQUEST_METHOD' => method, 'PATH_INFO' => path, 'QUERY_STRING' => query || '' } - end - - def respond(client, status, headers, body) - client.write "HTTP/1.1 #{status}\r\n" - headers.each { |key, value| client.write "#{key}: #{value}\r\n" } - client.write "\r\n" - body.each { |part| client.write part.to_s } - end - end -end diff --git a/lib/good_job/probe_server.rb b/lib/good_job/probe_server.rb index 0e1553636..c92229ca2 100644 --- a/lib/good_job/probe_server.rb +++ b/lib/good_job/probe_server.rb @@ -8,13 +8,20 @@ def self.task_observer(time, output, thread_error) # rubocop:disable Lint/Unused GoodJob._on_thread_error(thread_error) if thread_error end - def initialize(port:) - @port = port + def self.default_app + ::Rack::Builder.new do + use GoodJob::ProbeServer::HealthcheckMiddleware + run GoodJob::ProbeServer::NotFoundApp + end + end + + def initialize(port:, handler: nil, app: nil) + app ||= self.class.default_app + @handler = build_handler(port: port, handler: handler, app: app) end def start - @handler = HttpServer.new(self, port: @port, logger: GoodJob.logger) - @future = Concurrent::Future.new { @handler.run } + @future = @handler.build_future @future.add_observer(self.class, :task_observer) @future.execute end @@ -28,19 +35,17 @@ def stop @future&.value # wait for Future to exit end - def call(env) - case Rack::Request.new(env).path - when '/', '/status' - [200, {}, ["OK"]] - when '/status/started' - started = GoodJob::Scheduler.instances.any? && GoodJob::Scheduler.instances.all?(&:running?) - started ? [200, {}, ["Started"]] : [503, {}, ["Not started"]] - when '/status/connected' - connected = GoodJob::Scheduler.instances.any? && GoodJob::Scheduler.instances.all?(&:running?) && - GoodJob::Notifier.instances.any? && GoodJob::Notifier.instances.all?(&:connected?) - connected ? [200, {}, ["Connected"]] : [503, {}, ["Not connected"]] + def build_handler(port:, handler:, app:) + if handler == :webrick + begin + require 'webrick' + WebrickHandler.new(app, port: port, logger: GoodJob.logger) + rescue LoadError + GoodJob.logger.warn("WEBrick was requested as the probe server handler, but it's not in the load path. GoodJob doesn't keep WEBrick as a dependency, so you'll have to make sure its added to your Gemfile to make use of it. GoodJob will fallback to its own webserver in the meantime.") + SimpleHandler.new(app, port: port, logger: GoodJob.logger) + end else - [404, {}, ["Not found"]] + SimpleHandler.new(app, port: port, logger: GoodJob.logger) end end end diff --git a/lib/good_job/probe_server/healthcheck_middleware.rb b/lib/good_job/probe_server/healthcheck_middleware.rb new file mode 100644 index 000000000..87cc2356e --- /dev/null +++ b/lib/good_job/probe_server/healthcheck_middleware.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module GoodJob + class ProbeServer + class HealthcheckMiddleware + def initialize(app) + @app = app + end + + def call(env) + case Rack::Request.new(env).path + when '/', '/status' + [200, {}, ["OK"]] + when '/status/started' + started = GoodJob::Scheduler.instances.any? && GoodJob::Scheduler.instances.all?(&:running?) + started ? [200, {}, ["Started"]] : [503, {}, ["Not started"]] + when '/status/connected' + connected = GoodJob::Scheduler.instances.any? && GoodJob::Scheduler.instances.all?(&:running?) && + GoodJob::Notifier.instances.any? && GoodJob::Notifier.instances.all?(&:connected?) + connected ? [200, {}, ["Connected"]] : [503, {}, ["Not connected"]] + else + @app.call(env) + end + end + end + end +end diff --git a/lib/good_job/probe_server/not_found_app.rb b/lib/good_job/probe_server/not_found_app.rb new file mode 100644 index 000000000..6da83cc6b --- /dev/null +++ b/lib/good_job/probe_server/not_found_app.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module GoodJob + class ProbeServer + module NotFoundApp + def self.call(_env) + [404, {}, ["Not found"]] + end + end + end +end diff --git a/lib/good_job/probe_server/simple_handler.rb b/lib/good_job/probe_server/simple_handler.rb new file mode 100644 index 000000000..fef108d00 --- /dev/null +++ b/lib/good_job/probe_server/simple_handler.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module GoodJob + class ProbeServer + class SimpleHandler + SOCKET_READ_TIMEOUT = 5 # in seconds + + def initialize(app, options = {}) + @app = app + @port = options[:port] + @logger = options[:logger] + + @running = Concurrent::AtomicBoolean.new(false) + end + + def stop + @running.make_false + @server&.close + end + + def running? + @running.true? + end + + def build_future + Concurrent::Future.new { run } + end + + private + + def run + @running.make_true + start_server + handle_connections if @running.true? + rescue StandardError => e + @logger.error "Server encountered an error: #{e}" + ensure + stop + end + + def start_server + @server = TCPServer.new('0.0.0.0', @port) + rescue StandardError => e + @logger.error "Failed to start server: #{e}" + @running.make_false + end + + def handle_connections + while @running.true? + begin + ready_sockets, = IO.select([@server], nil, nil, SOCKET_READ_TIMEOUT) + next unless ready_sockets + + client = @server.accept_nonblock + request = client.gets + + if request + status, headers, body = @app.call(parse_request(request)) + respond(client, status, headers, body) + end + + client.close + rescue IO::WaitReadable, Errno::EINTR, Errno::EPIPE + retry + end + end + end + + def parse_request(request) + method, full_path = request.split + path, query = full_path.split('?') + { 'REQUEST_METHOD' => method, 'PATH_INFO' => path, 'QUERY_STRING' => query || '' } + end + + def respond(client, status, headers, body) + client.write "HTTP/1.1 #{status}\r\n" + headers.each { |key, value| client.write "#{key}: #{value}\r\n" } + client.write "\r\n" + body.each { |part| client.write part.to_s } + end + end + end +end diff --git a/lib/good_job/probe_server/webrick_handler.rb b/lib/good_job/probe_server/webrick_handler.rb new file mode 100644 index 000000000..68e667bf5 --- /dev/null +++ b/lib/good_job/probe_server/webrick_handler.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module GoodJob + class ProbeServer + class WebrickHandler + def initialize(app, options = {}) + @app = app + @port = options[:port] + @logger = options[:logger] + @handler = ::Rack::Handler.get('webrick') + end + + def stop + @handler&.shutdown + end + + def running? + @handler&.instance_variable_get(:@server)&.status == :Running + end + + def build_future + Concurrent::Future.new(args: [@handler, @port, GoodJob.logger]) do |thr_handler, thr_port, thr_logger| + thr_handler.run(@app, Port: thr_port, Host: '0.0.0.0', Logger: thr_logger, AccessLog: []) + end + end + end + end +end diff --git a/spec/lib/good_job/cli_spec.rb b/spec/lib/good_job/cli_spec.rb index 09ca7f55f..4d47ad713 100644 --- a/spec/lib/good_job/cli_spec.rb +++ b/spec/lib/good_job/cli_spec.rb @@ -60,6 +60,26 @@ end end + describe 'probe-handler' do + let(:probe_server) { instance_double GoodJob::ProbeServer, start: nil, stop: nil } + + before do + allow(Kernel).to receive(:loop) + allow(GoodJob::ProbeServer).to receive(:new).and_return probe_server + end + + context 'when a port and handler are specified' do + it 'starts a ProbeServer with the specified port and a "nil" app' do + cli = described_class.new([], { probe_port: 3838, probe_handler: "webrick" }, {}) + cli.start + + expect(GoodJob::ProbeServer).to have_received(:new).with(app: nil, port: 3838, handler: :webrick) + expect(probe_server).to have_received(:start) + expect(probe_server).to have_received(:stop) + end + end + end + describe 'probe-port' do let(:probe_server) { instance_double GoodJob::ProbeServer, start: nil, stop: nil } @@ -68,13 +88,49 @@ allow(GoodJob::ProbeServer).to receive(:new).and_return probe_server end - it 'starts a ProbeServer' do - cli = described_class.new([], { probe_port: 3838 }, {}) - cli.start + context 'when a port is specified' do + it 'starts a ProbeServer with the specified port and a "nil" app' do + cli = described_class.new([], { probe_port: 3838 }, {}) + cli.start + + expect(GoodJob::ProbeServer).to have_received(:new).with(app: nil, port: 3838, handler: nil) + expect(probe_server).to have_received(:start) + expect(probe_server).to have_received(:stop) + end + end + + context 'when a port and an app are set in the Rails configuration' do + it 'starts a ProbesServer with the configured port and app' do + app_mock = instance_double(Proc, call: nil) + configuration_mock = instance_double( + GoodJob::Configuration, + probe_app: app_mock, + probe_port: 3838, + probe_handler: nil, + options: {}, + daemonize?: false, + shutdown_timeout: 100, + idle_timeout: 100 + ) + allow(GoodJob).to receive_messages(configuration: configuration_mock) + cli = described_class.new([], [], {}) + cli.start + + expect(GoodJob::ProbeServer).to have_received(:new).with(app: app_mock, port: 3838, handler: nil) + expect(probe_server).to have_received(:start) + expect(probe_server).to have_received(:stop) + end + end + + context 'when a port is not specified' do + it 'does not start a ProbeServer' do + cli = described_class.new([], {}, {}) + cli.start - expect(GoodJob::ProbeServer).to have_received(:new).with(port: 3838) - expect(probe_server).to have_received(:start) - expect(probe_server).to have_received(:stop) + expect(GoodJob::ProbeServer).not_to have_received(:new) + expect(probe_server).not_to have_received(:start) + expect(probe_server).not_to have_received(:stop) + end end end diff --git a/spec/lib/good_job/probe_server/healthcheck_middleware_spec.rb b/spec/lib/good_job/probe_server/healthcheck_middleware_spec.rb new file mode 100644 index 000000000..36333e2e9 --- /dev/null +++ b/spec/lib/good_job/probe_server/healthcheck_middleware_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'net/http' + +RSpec.describe GoodJob::ProbeServer::HealthcheckMiddleware do + let(:app) { instance_double(Proc, call: nil) } + let(:healthcheck_middleware) { described_class.new(app) } + let(:port) { 3434 } + + describe '#call' do + let(:path) { nil } + let(:env) { Rack::MockRequest.env_for("http://127.0.0.1:#{port}#{path}") } + + describe '/' do + let(:path) { '/' } + + it 'returns "OK"' do + response = healthcheck_middleware.call(env) + expect(response[0]).to eq(200) + end + end + + describe '/status/started' do + let(:path) { '/status/started' } + + context 'when there are no running schedulers' do + it 'returns 503' do + response = healthcheck_middleware.call(env) + expect(response[0]).to eq(503) + end + end + + context 'when there are running schedulers' do + it 'returns 200' do + scheduler = instance_double(GoodJob::Scheduler, running?: true, shutdown: true, shutdown?: true) + GoodJob::Scheduler.instances << scheduler + + response = healthcheck_middleware.call(env) + expect(response[0]).to eq(200) + end + end + end + + describe '/status/connected' do + let(:path) { '/status/connected' } + + context 'when there are no running schedulers or notifiers' do + it 'returns 503' do + response = healthcheck_middleware.call(env) + expect(response[0]).to eq(503) + end + end + + context 'when there are running schedulers but disconnected notifiers' do + it 'returns 200' do + scheduler = instance_double(GoodJob::Scheduler, running?: true, shutdown: true, shutdown?: true) + GoodJob::Scheduler.instances << scheduler + + notifier = instance_double(GoodJob::Notifier, connected?: false, shutdown: true, shutdown?: true) + GoodJob::Notifier.instances << notifier + + response = healthcheck_middleware.call(env) + expect(response[0]).to eq(503) + end + end + + context 'when there are running schedulers and connected notifiers' do + it 'returns 200' do + scheduler = instance_double(GoodJob::Scheduler, running?: true, shutdown: true, shutdown?: true) + GoodJob::Scheduler.instances << scheduler + + notifier = instance_double(GoodJob::Notifier, connected?: true, shutdown: true, shutdown?: true) + GoodJob::Notifier.instances << notifier + + response = healthcheck_middleware.call(env) + expect(response[0]).to eq(200) + end + end + end + + describe 'forwarding unknown requests to given app' do + let(:path) { '/unhandled_path' } + + it 'passes the request to the given app' do + healthcheck_middleware.call(env) + expect(app).to have_received(:call).with(env) + end + end + end +end diff --git a/spec/lib/good_job/probe_server/not_found_app_spec.rb b/spec/lib/good_job/probe_server/not_found_app_spec.rb new file mode 100644 index 000000000..c469aee51 --- /dev/null +++ b/spec/lib/good_job/probe_server/not_found_app_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe GoodJob::ProbeServer::NotFoundApp do + describe '#call' do + it 'returns "Not Found"' do + response = described_class.call("") + expect(response[0]).to eq(404) + end + end +end diff --git a/spec/lib/good_job/probe_server_spec.rb b/spec/lib/good_job/probe_server_spec.rb index 9faf5adb8..950dc4914 100644 --- a/spec/lib/good_job/probe_server_spec.rb +++ b/spec/lib/good_job/probe_server_spec.rb @@ -4,107 +4,174 @@ require 'net/http' RSpec.describe GoodJob::ProbeServer do - let(:probe_server) { described_class.new(port: port) } let(:port) { 3434 } - after do - probe_server.stop - end - - describe '#start' do - it 'starts a http server that binds to all interfaces' do - probe_server.start - wait_until(max: 1) { expect(probe_server).to be_running } + describe 'default rack app' do + include Rack::Test::Methods + let(:app) { described_class.default_app } - ip_addresses = Socket.ip_address_list.select(&:ipv4?).map(&:ip_address) - expect(ip_addresses.size).to be >= 2 - expect(ip_addresses).to include("127.0.0.1") + it "responds to the expected routes" do + get "/" + expect(last_response.status).to eq 200 + expect(last_response.body).to eq("OK") - aggregate_failures do - ip_addresses.each do |ip_address| - response = Net::HTTP.get(ip_address, "/", port) - expect(response).to eq("OK") + get "/status" + expect(last_response.body).to eq("OK") - response = Net::HTTP.get(ip_address, "/status", port) - expect(response).to eq("OK") + get "/status/started" + expect(last_response.body).to eq("Not started") - response = Net::HTTP.get(ip_address, "/status/started", port) - expect(response).to eq("Not started") + get "/status/connected" + expect(last_response.body).to eq("Not connected") - response = Net::HTTP.get(ip_address, "/status/connected", port) - expect(response).to eq("Not connected") - end - end + get "/unimplemented_url" + expect(last_response.status).to eq 404 end end - describe '#call' do - let(:path) { nil } - let(:env) { Rack::MockRequest.env_for("http://127.0.0.1:#{port}#{path}") } + describe '#start' do + context "with default http server" do + context 'with the default healthcheck app' do + it 'starts a http server that binds to all interfaces and returns healthcheck responses' do + probe_server = described_class.new(port: port, app: nil) + probe_server.start + wait_until(max: 1) { expect(probe_server).to be_running } - describe '/' do - let(:path) { '/' } + ip_addresses = Socket.ip_address_list.select(&:ipv4?).map(&:ip_address) + expect(ip_addresses.size).to be >= 2 + expect(ip_addresses).to include("127.0.0.1") - it 'returns "OK"' do - response = probe_server.call(env) - expect(response[0]).to eq(200) - end - end + aggregate_failures do + ip_addresses.each do |ip_address| + response = Net::HTTP.get(ip_address, "/", port) + expect(response).to eq("OK") + + response = Net::HTTP.get(ip_address, "/status", port) + expect(response).to eq("OK") + + response = Net::HTTP.get(ip_address, "/status/started", port) + expect(response).to eq("Not started") - describe '/status/started' do - let(:path) { '/status/started' } + response = Net::HTTP.get(ip_address, "/status/connected", port) + expect(response).to eq("Not connected") - context 'when there are no running schedulers' do - it 'returns 503' do - response = probe_server.call(env) - expect(response[0]).to eq(503) + response = Net::HTTP.get(ip_address, "/unimplemented_url", port) + expect(response).to eq("Not found") + end + end + + probe_server.stop end end - context 'when there are running schedulers' do - it 'returns 200' do - scheduler = instance_double(GoodJob::Scheduler, running?: true, shutdown: true, shutdown?: true) - GoodJob::Scheduler.instances << scheduler - - response = probe_server.call(env) - expect(response[0]).to eq(200) + context 'with a provided app' do + it 'starts a http server that binds to all interfaces and uses the supplied app' do + app = proc { [200, { "Content-Type" => "text/plain" }, ["Hello World"]] } + probe_server = described_class.new(app: app, port: port) + probe_server.start + wait_until(max: 1) { expect(probe_server).to be_running } + + ip_addresses = Socket.ip_address_list.select(&:ipv4?).map(&:ip_address) + expect(ip_addresses.size).to be >= 2 + expect(ip_addresses).to include("127.0.0.1") + + aggregate_failures do + ip_addresses.each do |ip_address| + response = Net::HTTP.get(ip_address, "/", port) + expect(response).to eq("Hello World") + end + end + + probe_server.stop end end end - describe '/status/connected' do - let(:path) { '/status/connected' } + context "with WEBrick" do + context 'with the default healthcheck app' do + it 'starts a WEBrick http server' do + probe_server = described_class.new(port: port, handler: :webrick) + probe_server.start + wait_until(max: 1) { expect(probe_server).to be_running } + + ip_address = Socket.ip_address_list.select(&:ipv4?).map(&:ip_address).first + response = Net::HTTP.get_response(ip_address, "/", port) - context 'when there are no running schedulers or notifiers' do - it 'returns 503' do - response = probe_server.call(env) - expect(response[0]).to eq(503) + expect(response["server"]).to match(/WEBrick/) + + probe_server.stop end - end - context 'when there are running schedulers but disconnected notifiers' do - it 'returns 200' do - scheduler = instance_double(GoodJob::Scheduler, running?: true, shutdown: true, shutdown?: true) - GoodJob::Scheduler.instances << scheduler + it 'server binds to all interfaces and returns healthcheck responses' do + probe_server = described_class.new(port: port, handler: :webrick) + probe_server.start + wait_until(max: 1) { expect(probe_server).to be_running } + + ip_addresses = Socket.ip_address_list.select(&:ipv4?).map(&:ip_address) + expect(ip_addresses.size).to be >= 2 + expect(ip_addresses).to include("127.0.0.1") + + aggregate_failures do + ip_addresses.each do |ip_address| + response = Net::HTTP.get(ip_address, "/", port) + expect(response).to eq("OK") + + response = Net::HTTP.get(ip_address, "/status", port) + expect(response).to eq("OK") + + response = Net::HTTP.get(ip_address, "/status/started", port) + expect(response).to eq("Not started") + + response = Net::HTTP.get(ip_address, "/status/connected", port) + expect(response).to eq("Not connected") - notifier = instance_double(GoodJob::Notifier, connected?: false, shutdown: true, shutdown?: true) - GoodJob::Notifier.instances << notifier + response = Net::HTTP.get(ip_address, "/unimplemented_url", port) + expect(response).to eq("Not found") + end + end - response = probe_server.call(env) - expect(response[0]).to eq(503) + probe_server.stop end - end - context 'when there are running schedulers and connected notifiers' do - it 'returns 200' do - scheduler = instance_double(GoodJob::Scheduler, running?: true, shutdown: true, shutdown?: true) - GoodJob::Scheduler.instances << scheduler + context "when WEBrick isn't in the load path" do + it 'sends out a warning and falls back to the built in server' do + allow_any_instance_of(described_class).to receive(:require).with("webrick").and_raise(LoadError) + allow(GoodJob.logger).to receive(:warn) + + probe_server = described_class.new(port: port, handler: :webrick) + probe_server.start + wait_until(max: 1) { expect(probe_server).to be_running } + + ip_address = Socket.ip_address_list.select(&:ipv4?).map(&:ip_address).first + response = Net::HTTP.get(ip_address, "/", port) - notifier = instance_double(GoodJob::Notifier, connected?: true, shutdown: true, shutdown?: true) - GoodJob::Notifier.instances << notifier + expect(GoodJob.logger).to have_received(:warn).with(/WEBrick was requested/) + expect(response).to eq("OK") + + probe_server.stop + end + end + end - response = probe_server.call(env) - expect(response[0]).to eq(200) + context 'with a provided app' do + it 'starts a http server that binds to all interfaces and uses the supplied app' do + app = proc { [200, { "Content-Type" => "text/plain" }, ["Hello World"]] } + probe_server = described_class.new(app: app, port: port) + probe_server.start + wait_until(max: 1) { expect(probe_server).to be_running } + + ip_addresses = Socket.ip_address_list.select(&:ipv4?).map(&:ip_address) + expect(ip_addresses.size).to be >= 2 + expect(ip_addresses).to include("127.0.0.1") + + aggregate_failures do + ip_addresses.each do |ip_address| + response = Net::HTTP.get(ip_address, "/", port) + expect(response).to eq("Hello World") + end + end + + probe_server.stop end end end