Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions lib/kamal/cli/proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -426,14 +426,7 @@ def loadbalancer(status)
end
when "deploy"
if KAMAL.config.proxy.load_balancing?
targets = []
KAMAL.config.roles.each do |role|
next unless role.running_proxy?

role.hosts.each do |host|
targets << host
end
end
targets = KAMAL.loadbalancer_config.target_hosts

on(KAMAL.config.proxy.effective_loadbalancer) do |host|
Kamal::Cli::Proxy::LoadbalancerClaim.new(host, self).claim_service
Expand Down
46 changes: 46 additions & 0 deletions lib/kamal/cli/proxy/loadbalancer_reboot.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
# loadbalancer keeps its service state in the config volume, which the
# replacement container re-mounts, so every app's routes survive the gap.
class Kamal::Cli::Proxy::LoadbalancerReboot
READY_TIMEOUT = 30

attr_reader :host, :sshkit
delegate :execute, :capture_with_info, :info, :upload!, to: :sshkit

Expand Down Expand Up @@ -33,9 +35,53 @@ def run
Kamal::Cli::Proxy::LoadbalancerClaim.new(host, sshkit).claim_run_config(replace: true)
execute *KAMAL.loadbalancer.run

wait_until_ready
verify_service if re_register_service

# kamal-proxy keeps its service state in the config volume, which the
# replacement container re-mounts - every app's routes survive.
services = capture_with_info(*KAMAL.loadbalancer.list).strip
info "Services registered on the load balancer at #{host} after reboot:\n#{services}"
end

private
def wait_until_ready
deadline = Time.now + READY_TIMEOUT

begin
capture_with_info(*KAMAL.loadbalancer.list, verbosity: :debug)
rescue SSHKit::Command::Failed
raise Kamal::Cli::BootError, "the load balancer on #{host} did not become ready within #{READY_TIMEOUT} seconds" if Time.now >= deadline
sleep 0.5
retry
end
end

# A deploy failure after this reboot must not strand a fresh LB with no
# services: re-register this app's routes now rather than trusting the
# deploy step that hasn't run yet, mirroring the per-host proxy reboot.
#
# Only this app's registration can be rebuilt from this deploy.yml - the
# owner files of other apps sharing the LB record tokens, not deploy
# commands - so anything else rides on the state-volume restore (which
# --recheck-targets-on-restore re-verifies). No owner record, or a foreign
# one, means nothing of ours to restore.
def re_register_service
owner = capture_with_info(*KAMAL.loadbalancer.read_service_owner, raise_on_non_zero_exit: false).strip
return false unless owner == KAMAL.loadbalancer_config.owner_token

info "Re-registering #{KAMAL.config.service} with the load balancer on #{host}..."
execute *KAMAL.loadbalancer.deploy(targets: KAMAL.loadbalancer_config.target_hosts)
true
end

# `list --json` returns {"services": {"<name>": ...}} - exact key
# membership, same as the per-host proxy reboot's verification.
def verify_service
listed = JSON.parse(capture_with_info(*KAMAL.loadbalancer.list(json: true))).fetch("services", {}).keys

unless listed.include?(KAMAL.config.service)
raise Kamal::Cli::BootError, "the load balancer on #{host} is missing service #{KAMAL.config.service} after reboot"
end
end
end
4 changes: 2 additions & 2 deletions lib/kamal/commands/loadbalancer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ def domains(subcommand)
docker :exec, container_name, "kamal-proxy", "domains", subcommand
end

def list
docker :exec, container_name, "kamal-proxy", :list
def list(json: false)
docker :exec, container_name, "kamal-proxy", :list, *("--json" if json)
end

# Cache policy is edge-only under load balancing (see the layering contract),
Expand Down
16 changes: 14 additions & 2 deletions lib/kamal/configuration/loadbalancer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,24 @@ def initialize(config:, proxy_config:, secrets:)

# The load balancer fans a single service out to many targets, so unlike the
# per-app proxy deploy (which takes one target) it takes the full list and
# joins them into a single --target flag, honouring app_port for each.
# joins them into a single --target flag.
#
# Each target is a per-host proxy, reached on its published HTTP port
# (run.http_port, default 80) - the only cross-host surface it exposes.
# Never app_port: that is how a per-host proxy reaches the app container
# inside its own docker network, and nothing listens on it across hosts.
def deploy_command_args(targets:)
target_arg = targets.map { |target| "#{target}:#{app_port}" }.join(",")
target_arg = targets.map { |target| "#{target}:#{run.http_port}" }.join(",")
optionize ({ target: target_arg }).merge(deploy_options), with: "="
end

# The hosts the load balancer forwards to: every host of every role that
# runs a proxy. One source of truth for the deploy step and the reboot
# re-registration, so their target lists cannot diverge.
def target_hosts
config.roles.select(&:running_proxy?).flat_map(&:hosts)
end

def directory
File.join config.run_directory, "loadbalancer"
end
Expand Down
39 changes: 39 additions & 0 deletions test/cli/proxy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1046,6 +1046,45 @@ class CliProxyTest < CliTestCase
end
end

# The LB reboot path must not rely solely on the deploy step that follows
# it: if that step fails, a freshly rebooted LB would be stranded with no
# services and the site down. Re-register this app's routes immediately,
# mirroring the per-host proxy reboot, and verify via the JSON listing.
test "reboot re-registers this apps service on the load balancer and verifies it" do
Kamal::Configuration::Proxy.any_instance.unstub(:load_balancing?)
stub_loadbalancer_registry(service_owner: loadbalancer_owner_token(:with_loadbalancer))
SSHKit::Backend::Abstract.any_instance.stubs(:capture_with_info)
.with(:docker, :exec, "load-balancer", "kamal-proxy", :list, "--json")
.returns({ services: { "app" => { "target" => "1.1.1.1:80" } } }.to_json)

run_command("reboot", "-y", fixture: :with_loadbalancer).tap do |output|
assert_match "Re-registering app with the load balancer on lb.example.com", output
assert_match "docker exec load-balancer kamal-proxy deploy app --target=\"1.1.1.1:80,1.1.1.2:80\"", output
end
end

test "reboot fails when the load balancer is missing this apps service after re-registration" do
Kamal::Configuration::Proxy.any_instance.unstub(:load_balancing?)
stub_loadbalancer_registry(service_owner: loadbalancer_owner_token(:with_loadbalancer))
SSHKit::Backend::Abstract.any_instance.stubs(:capture_with_info)
.with(:docker, :exec, "load-balancer", "kamal-proxy", :list, "--json")
.returns({ services: { "other-app" => {} } }.to_json)

error = assert_raises(SSHKit::Runner::ExecuteError) { run_command("reboot", "-y", fixture: :with_loadbalancer) }
assert_match "missing service app after reboot", error.message
end

# No owner record means this app never deployed through the LB - the
# ordinary deploy step registers it later; there is nothing to restore.
test "reboot skips load balancer re-registration when the service was never registered" do
Kamal::Configuration::Proxy.any_instance.unstub(:load_balancing?)
stub_loadbalancer_registry

run_command("reboot", "-y", fixture: :with_loadbalancer).tap do |output|
assert_no_match(/kamal-proxy deploy/, output)
end
end

test "remove refuses when other apps are installed on the loadbalancer host" do
Thread.report_on_exception = false
Kamal::Configuration::Proxy.any_instance.unstub(:load_balancing?)
Expand Down
26 changes: 24 additions & 2 deletions test/commands/loadbalancer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,23 @@ class CommandsLoadbalancerTest < ActiveSupport::TestCase
new_command.deploy(targets: [ "1.1.1.1" ]).join(" ")
end

test "deploy uses app_port for targets" do
# The LB reaches the per-host proxies over their published HTTP port - the
# only cross-host surface they expose. app_port is how each per-host proxy
# reaches the app container inside its own docker network; nothing listens
# on it across hosts, so leaking it here left targets permanently unhealthy.
test "deploy targets the per-host proxies published http port, not app_port" do
@config[:proxy]["app_port"] = 3000
assert_equal \
"docker exec load-balancer kamal-proxy deploy app --target=\"1.1.1.1:3000,1.1.1.2:3000\" --host=\"app.example.com\" --deploy-timeout=\"30s\" --drain-timeout=\"30s\" --buffer-requests --buffer-responses --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\" --log-request-header=\"User-Agent\"",
"docker exec load-balancer kamal-proxy deploy app --target=\"1.1.1.1:80,1.1.1.2:80\" --host=\"app.example.com\" --deploy-timeout=\"30s\" --drain-timeout=\"30s\" --buffer-requests --buffer-responses --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\" --log-request-header=\"User-Agent\"",
new_command.deploy(targets: [ "1.1.1.1", "1.1.1.2" ]).join(" ")
end

test "deploy honors a custom proxy run http_port for targets" do
@config[:proxy]["run"] = { "http_port" => 8080 }

assert_match "--target=\"1.1.1.1:8080,1.1.1.2:8080\"", new_command.deploy(targets: [ "1.1.1.1", "1.1.1.2" ]).join(" ")
end

test "deploy propagates rich proxy options (healthcheck, response timeout, path prefix)" do
@config[:proxy]["healthcheck"] = { "interval" => 2, "timeout" => 5, "path" => "/healthz" }
@config[:proxy]["response_timeout"] = 10
Expand Down Expand Up @@ -335,6 +345,18 @@ class CommandsLoadbalancerTest < ActiveSupport::TestCase
new_command.list.join(" ")
end

test "list json" do
assert_equal \
"docker exec load-balancer kamal-proxy list --json",
new_command.list(json: true).join(" ")
end

# One source of truth for what the LB fronts: the deploy step and the reboot
# re-registration both draw targets from here, so they cannot diverge.
test "target_hosts collects the hosts of every proxy-running role" do
assert_equal [ "1.1.1.1", "1.1.1.2" ], new_loadbalancer_config.target_hosts
end

test "config_digest" do
assert_equal \
"docker inspect load-balancer --format '{{ index .Config.Labels \"org.kamal.proxy-config-digest\" }}'",
Expand Down