Skip to content

Commit

Permalink
Restore original naming
Browse files Browse the repository at this point in the history
This also helps ensure that the existing behavior and API remain intact.
  • Loading branch information
jklina committed Dec 12, 2023
1 parent eb63619 commit ea9a964
Show file tree
Hide file tree
Showing 12 changed files with 178 additions and 87 deletions.
4 changes: 2 additions & 2 deletions lib/good_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 2 additions & 4 deletions lib/good_job/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 6 additions & 0 deletions lib/good_job/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
9 changes: 0 additions & 9 deletions lib/good_job/middleware/catch_all.rb

This file was deleted.

25 changes: 0 additions & 25 deletions lib/good_job/middleware/healthcheck.rb

This file was deleted.

11 changes: 10 additions & 1 deletion lib/good_job/probe_server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
13 changes: 13 additions & 0 deletions lib/good_job/probe_server/middleware/catchall.rb
Original file line number Diff line number Diff line change
@@ -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
29 changes: 29 additions & 0 deletions lib/good_job/probe_server/middleware/healthcheck.rb
Original file line number Diff line number Diff line change
@@ -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
46 changes: 40 additions & 6 deletions spec/lib/good_job/cli_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 12 additions & 0 deletions spec/lib/good_job/probe_server/middleware/catchall_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
73 changes: 46 additions & 27 deletions spec/lib/good_job/probe_server_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit ea9a964

Please sign in to comment.