From 5eb592095bb33feaa6ab54b248a50dd26c660f1f Mon Sep 17 00:00:00 2001 From: mhenrixon Date: Mon, 3 Aug 2026 22:39:48 +0200 Subject: [PATCH 1/2] fix(loadbalancer): target the per-host proxies published http port, not app_port 3.0.0 moved the LB target construction from Kamal::Commands::Loadbalancer (which used :80) into Configuration::Loadbalancer#deploy_command_args, which built :. app_port is how a per-host proxy reaches the app container inside its own docker network - nothing listens on it cross-host, the per-host proxies only publish 80/443 - so every LB deploy timed out with targets that could never become healthy. Use run.http_port (default 80) instead: the LB run config is the same shared proxy/run surface the per-host proxies publish with, so a custom http_port is honoured automatically. Refs #104 --- lib/kamal/configuration/loadbalancer.rb | 9 +++++++-- test/commands/loadbalancer_test.rb | 14 ++++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/lib/kamal/configuration/loadbalancer.rb b/lib/kamal/configuration/loadbalancer.rb index 001d4a501..3e57b83ed 100644 --- a/lib/kamal/configuration/loadbalancer.rb +++ b/lib/kamal/configuration/loadbalancer.rb @@ -12,9 +12,14 @@ 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 diff --git a/test/commands/loadbalancer_test.rb b/test/commands/loadbalancer_test.rb index c8788e781..a2dfd847d 100644 --- a/test/commands/loadbalancer_test.rb +++ b/test/commands/loadbalancer_test.rb @@ -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 From 9926121d7ff1f40d284f9e1f6fb1dbc47339e5f3 Mon Sep 17 00:00:00 2001 From: mhenrixon Date: Mon, 3 Aug 2026 22:40:02 +0200 Subject: [PATCH 2/2] fix(proxy): re-register this apps service after a loadbalancer reboot A rebooted LB starts from whatever the state volume restored; until 3.0.0 the deploy flow only re-registered services in the LB deploy step that runs AFTER app boot. On the first v3 deploy the drift-digest schema change forces an LB reboot, and when the subsequent deploy step failed (see the target port regression) the LB was stranded with an empty service table - site down. Mirror the per-host proxy reboot instead: once the replacement container is ready, re-register this apps service with the current targets and verify it via kamal-proxy list --json, raising if it is missing. Only this apps registration can be rebuilt from this deploy.yml, so the safety net is gated on the service owner file recording this app; other apps sharing the LB still ride on the state-volume restore. Target collection moves to Configuration::Loadbalancer#target_hosts so the deploy step and the reboot re-registration draw from one source of truth. Refs #104 --- lib/kamal/cli/proxy.rb | 9 +---- lib/kamal/cli/proxy/loadbalancer_reboot.rb | 46 ++++++++++++++++++++++ lib/kamal/commands/loadbalancer.rb | 4 +- lib/kamal/configuration/loadbalancer.rb | 7 ++++ test/cli/proxy_test.rb | 39 ++++++++++++++++++ test/commands/loadbalancer_test.rb | 12 ++++++ 6 files changed, 107 insertions(+), 10 deletions(-) diff --git a/lib/kamal/cli/proxy.rb b/lib/kamal/cli/proxy.rb index d58eb5459..4c45896d4 100644 --- a/lib/kamal/cli/proxy.rb +++ b/lib/kamal/cli/proxy.rb @@ -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 diff --git a/lib/kamal/cli/proxy/loadbalancer_reboot.rb b/lib/kamal/cli/proxy/loadbalancer_reboot.rb index c9daed776..599345167 100644 --- a/lib/kamal/cli/proxy/loadbalancer_reboot.rb +++ b/lib/kamal/cli/proxy/loadbalancer_reboot.rb @@ -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 @@ -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": {"": ...}} - 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 diff --git a/lib/kamal/commands/loadbalancer.rb b/lib/kamal/commands/loadbalancer.rb index 44ff13673..15ce17117 100644 --- a/lib/kamal/commands/loadbalancer.rb +++ b/lib/kamal/commands/loadbalancer.rb @@ -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), diff --git a/lib/kamal/configuration/loadbalancer.rb b/lib/kamal/configuration/loadbalancer.rb index 3e57b83ed..fee5076fc 100644 --- a/lib/kamal/configuration/loadbalancer.rb +++ b/lib/kamal/configuration/loadbalancer.rb @@ -23,6 +23,13 @@ def deploy_command_args(targets:) 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 diff --git a/test/cli/proxy_test.rb b/test/cli/proxy_test.rb index 3c274b8f7..081870c1c 100644 --- a/test/cli/proxy_test.rb +++ b/test/cli/proxy_test.rb @@ -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?) diff --git a/test/commands/loadbalancer_test.rb b/test/commands/loadbalancer_test.rb index a2dfd847d..2e3c1553c 100644 --- a/test/commands/loadbalancer_test.rb +++ b/test/commands/loadbalancer_test.rb @@ -345,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\" }}'",