diff --git a/lib/good_job.rb b/lib/good_job.rb index c880338cb..fa76f8061 100644 --- a/lib/good_job.rb +++ b/lib/good_job.rb @@ -32,8 +32,8 @@ require "good_job/notifier" require "good_job/poller" require "good_job/probe_server" -require "good_job/middleware/catch_all" -require "good_job/middleware/healthcheck" +require "good_job/probe_server/middleware/catchall" +require "good_job/probe_server/middleware/healthcheck" require "good_job/http_server" require "good_job/scheduler" require "good_job/shared_executor" diff --git a/lib/good_job/cli.rb b/lib/good_job/cli.rb index c51690b5f..9d2c8dd06 100644 --- a/lib/good_job/cli.rb +++ b/lib/good_job/cli.rb @@ -104,10 +104,8 @@ def start capsule.start systemd.start - middleware = Rails.application.config.good_job.middleware - port = Rails.application.config.good_job.middleware_port - if middleware && port - probe_server = GoodJob::ProbeServer.new(app: middleware, port: port) + if configuration.probe_port + probe_server = GoodJob::ProbeServer.new(app: configuration.probe_server_app, port: configuration.probe_port) probe_server.start end diff --git a/lib/good_job/configuration.rb b/lib/good_job/configuration.rb index dcef0d706..eabf7ed69 100644 --- a/lib/good_job/configuration.rb +++ b/lib/good_job/configuration.rb @@ -338,6 +338,12 @@ def probe_port env['GOOD_JOB_PROBE_PORT'] end + # Rack compliant application to be run on the ProbeServer + # @return [nil, Class] + def probe_server_app + rails_config[:probe_server_app] + end + def enable_listen_notify return options[:enable_listen_notify] unless options[:enable_listen_notify].nil? return rails_config[:enable_listen_notify] unless rails_config[:enable_listen_notify].nil? diff --git a/lib/good_job/middleware/catch_all.rb b/lib/good_job/middleware/catch_all.rb deleted file mode 100644 index 54e41035e..000000000 --- a/lib/good_job/middleware/catch_all.rb +++ /dev/null @@ -1,9 +0,0 @@ -module GoodJob - module Middleware - class CatchAll - def self.call(env) - [404, {}, ["Not found"]] - end - end - end -end diff --git a/lib/good_job/middleware/healthcheck.rb b/lib/good_job/middleware/healthcheck.rb deleted file mode 100644 index 381ad7335..000000000 --- a/lib/good_job/middleware/healthcheck.rb +++ /dev/null @@ -1,25 +0,0 @@ -module GoodJob - module Middleware - class Healthcheck - 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.rb b/lib/good_job/probe_server.rb index 4c350c4b1..f43c0501c 100644 --- a/lib/good_job/probe_server.rb +++ b/lib/good_job/probe_server.rb @@ -8,7 +8,7 @@ def self.task_observer(time, output, thread_error) # rubocop:disable Lint/Unused GoodJob._on_thread_error(thread_error) if thread_error end - def initialize(app:, port:) + def initialize(port:, app: default_probe_server) @port = port @app = app end @@ -28,5 +28,14 @@ def stop @handler&.stop @future&.value # wait for Future to exit end + + private + + def default_probe_server + Rack::Builder.new do + use Middleware::Healthcheck + run Middleware::Catchall.new + end + end end end diff --git a/lib/good_job/probe_server/middleware/catchall.rb b/lib/good_job/probe_server/middleware/catchall.rb new file mode 100644 index 000000000..19465faac --- /dev/null +++ b/lib/good_job/probe_server/middleware/catchall.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module GoodJob + class ProbeServer + module Middleware + class Catchall + def call(_) + [404, {}, ["Not found"]] + end + end + end + end +end diff --git a/lib/good_job/probe_server/middleware/healthcheck.rb b/lib/good_job/probe_server/middleware/healthcheck.rb new file mode 100644 index 000000000..906d7d1f0 --- /dev/null +++ b/lib/good_job/probe_server/middleware/healthcheck.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module GoodJob + class ProbeServer + module Middleware + class Healthcheck + 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 +end diff --git a/spec/lib/good_job/cli_spec.rb b/spec/lib/good_job/cli_spec.rb index 059a1ebbc..ba13f736d 100644 --- a/spec/lib/good_job/cli_spec.rb +++ b/spec/lib/good_job/cli_spec.rb @@ -56,13 +56,47 @@ 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) + expect(probe_server).to have_received(:start) + expect(probe_server).to have_received(:stop) + end + end + + context 'when a port is and an app is 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_server_app: app_mock, + probe_port: 3838, + options: {}, + daemonize?: false, + shutdown_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) + 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/middleware/catchall_spec.rb b/spec/lib/good_job/probe_server/middleware/catchall_spec.rb new file mode 100644 index 000000000..410c0e8d4 --- /dev/null +++ b/spec/lib/good_job/probe_server/middleware/catchall_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe GoodJob::ProbeServer::Middleware::Catchall do + describe '#call' do + it 'returns "Not Found"' do + response = described_class.new.call("") + expect(response[0]).to eq(404) + end + end +end diff --git a/spec/lib/good_job/middleware/healthcheck_spec.rb b/spec/lib/good_job/probe_server/middleware/healthcheck_spec.rb similarity index 73% rename from spec/lib/good_job/middleware/healthcheck_spec.rb rename to spec/lib/good_job/probe_server/middleware/healthcheck_spec.rb index 14a7474f9..3dfa74374 100644 --- a/spec/lib/good_job/middleware/healthcheck_spec.rb +++ b/spec/lib/good_job/probe_server/middleware/healthcheck_spec.rb @@ -3,13 +3,9 @@ require 'rails_helper' require 'net/http' -RSpec.describe GoodJob::ProbeServer do - let(:healthcheck_rack_app) do - Rack::Builder.app do - use GoodJob::Middleware::Healthcheck - run GoodJob::Middleware::CatchAll - end - end +RSpec.describe GoodJob::ProbeServer::Middleware::Healthcheck do + let(:app) { instance_double(Proc, call: nil) } + let(:healthcheck_middleware) { described_class.new(app) } let(:port) { 3434 } describe '#call' do @@ -20,7 +16,7 @@ let(:path) { '/' } it 'returns "OK"' do - response = healthcheck_rack_app.call(env) + response = healthcheck_middleware.call(env) expect(response[0]).to eq(200) end end @@ -30,7 +26,7 @@ context 'when there are no running schedulers' do it 'returns 503' do - response = healthcheck_rack_app.call(env) + response = healthcheck_middleware.call(env) expect(response[0]).to eq(503) end end @@ -40,7 +36,7 @@ scheduler = instance_double(GoodJob::Scheduler, running?: true, shutdown: true, shutdown?: true) GoodJob::Scheduler.instances << scheduler - response = healthcheck_rack_app.call(env) + response = healthcheck_middleware.call(env) expect(response[0]).to eq(200) end end @@ -51,7 +47,7 @@ context 'when there are no running schedulers or notifiers' do it 'returns 503' do - response = healthcheck_rack_app.call(env) + response = healthcheck_middleware.call(env) expect(response[0]).to eq(503) end end @@ -64,7 +60,7 @@ notifier = instance_double(GoodJob::Notifier, connected?: false, shutdown: true, shutdown?: true) GoodJob::Notifier.instances << notifier - response = healthcheck_rack_app.call(env) + response = healthcheck_middleware.call(env) expect(response[0]).to eq(503) end end @@ -77,10 +73,19 @@ notifier = instance_double(GoodJob::Notifier, connected?: true, shutdown: true, shutdown?: true) GoodJob::Notifier.instances << notifier - response = healthcheck_rack_app.call(env) + 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_spec.rb b/spec/lib/good_job/probe_server_spec.rb index cd35ccaa1..8b35913e7 100644 --- a/spec/lib/good_job/probe_server_spec.rb +++ b/spec/lib/good_job/probe_server_spec.rb @@ -4,42 +4,61 @@ require 'net/http' RSpec.describe GoodJob::ProbeServer do - let(:probe_rack_app) do - Rack::Builder.app do - use GoodJob::Middleware::Healthcheck - run GoodJob::Middleware::CatchAll - end - end - let(:probe_server) { described_class.new(app: probe_rack_app, 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 } + 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) + 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") + 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") + 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", 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/started", port) + expect(response).to eq("Not started") - response = Net::HTTP.get(ip_address, "/status/connected", port) - expect(response).to eq("Not connected") + response = Net::HTTP.get(ip_address, "/status/connected", port) + expect(response).to eq("Not connected") + + response = Net::HTTP.get(ip_address, "/unimplemented_url", port) + expect(response).to eq("Not found") + end end + + probe_server.stop + end + end + + context 'with a provided app' do + it 'starts a http server that binds to all interfaces and returns healthcheck responses' do + app = Proc.new { [ 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