Skip to content

fix(loadbalancer): target per-host proxies on their published http port + re-register services after LB reboot - #107

Merged
mhenrixon merged 2 commits into
dashfrom
fix/issue-104-lb-target-port
Aug 3, 2026
Merged

fix(loadbalancer): target per-host proxies on their published http port + re-register services after LB reboot#107
mhenrixon merged 2 commits into
dashfrom
fix/issue-104-lb-target-port

Conversation

@mhenrixon

Copy link
Copy Markdown
Collaborator

Closes #104

What

Two fixes for the 3.0.0 loadbalancer deploy regression:

  1. Target portKamal::Configuration::Loadbalancer#deploy_command_args built targets as <host>:<app_port>. 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 ("target failed to become healthy"). Targets now use run.http_port (default 80): the LB's run config is the same shared proxy/run surface the per-host proxies publish with (publish maps host http_port → container 80), so a custom proxy.run.http_port is honoured automatically.

  2. Reboot re-registration safety net — the LB reboot path (drift-detected on boot, or kamal proxy reboot) previously relied entirely on the state-volume restore plus 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 (bug 1) the LB was stranded with an empty service table — site down. Kamal::Cli::Proxy::LoadbalancerReboot now mirrors the per-host proxy reboot: wait until the replacement container answers kamal-proxy list (30s deadline), re-register this app's service with the current targets, and verify it via list --json exact key membership, raising Kamal::Cli::BootError if missing.

Supporting changes:

  • Kamal::Configuration::Loadbalancer#target_hosts — target collection (hosts of every proxy-running role) extracted from Kamal::Cli::Proxy#loadbalancer so the deploy step and the reboot re-registration draw from one source of truth. Same semantics (order, no dedup).
  • Kamal::Commands::Loadbalancer#list gained a json: param, mirroring Kamal::Commands::Proxy#list.

Why re-register only this app's service

The issue suggests re-registering everything recorded under .kamal/loadbalancer/services/*, but those files record owner tokens, not deploy commands — another app's targets and options cannot be rebuilt from this app's deploy.yml. Other apps' routes survive through the kamal-loadbalancer-config state volume (re-verified by --recheck-targets-on-restore); this app's registration is additionally re-registered fresh and verified, gated on the owner file recording this app. No owner record (never deployed) or a foreign owner skips the step silently.

Test plan

  • test/commands/loadbalancer_test.rb
    • replaced the test that codified the regression ("deploy uses app_port for targets") with "deploy targets the per-host proxies published http port, not app_port" (app_port: 3000 → targets stay :80)
    • new: custom proxy.run.http_port: 8080 → targets :8080
    • new: list --json, target_hosts
  • test/cli/proxy_test.rb
    • reboot re-registers this app's service and verifies it via the JSON listing (asserts the kamal-proxy deploy app --target="1.1.1.1:80,1.1.1.2:80" exec on the LB)
    • reboot raises when the service is missing from list --json after re-registration
    • reboot skips re-registration when no owner record exists
  • bundle exec rubocop --parallel — clean (218 files)
  • Full unit suite: 1427 runs, 4405 assertions — green except the two known Apple-Silicon arch-dependent builder tests (amd64/arm64 platform ordering; pass in CI; no builder code touched)
  • Integration suite not run locally (needs Docker + the published proxy image)

Deviations & judgment calls

  • Part 1 uses run.http_port (default 80), not a hardcoded 80: KAMAL.loadbalancer_config is built from the root proxy config, so its run is the same proxy/run block the per-host proxies publish with — a custom proxy.run.http_port is respected automatically, and apps sharing an LB already must agree on proxy/run (run-config digest claim), so the value cannot diverge per host.
  • The existing test "deploy uses app_port for targets" codified the regression; replaced with an assertion that app_port does NOT leak into LB targets (it still applies to the per-host proxy → app container hop).
  • Part 2 re-registers only THIS app's service, not every file in .kamal/loadbalancer/services/* as the issue literally suggests: those files record owner tokens only — another app's deploy options/targets cannot be rebuilt from this app's deploy.yml. Other apps' routes survive via the state volume; ours is additionally re-registered fresh so a later app-deploy failure can't strand the LB without it.
  • Re-registration is gated on the owner file matching this app's owner_token — absent (first deploy) or foreign owner skips it silently; the ordinary deploy step still registers the service later in the flow.
  • Added wait_until_ready (retry kamal-proxy list for up to 30s) before re-registration, mirroring Kamal::Cli::Proxy::Reboot::READY_TIMEOUT — the old code listed services immediately after docker run and could race a slow container start.
  • If the re-registration deploy itself fails (e.g. targets unhealthy) the reboot now fails loudly instead of silently leaving an empty LB — deliberate: with part 1 fixed the targets are the per-host proxies' :80, which were (re)booted earlier in both the boot and reboot flows.
  • Extracted target collection into Kamal::Configuration::Loadbalancer#target_hosts; preserved exact old semantics (no uniq/dedup, same ordering).
  • Out of scope, noted: proxy.run.publish: false makes the per-host proxies unreachable from a dedicated LB host entirely; that topology error pre-dates this fix and is not newly validated here.
  • Docs (lib/kamal/configuration/docs/proxy.yml) never claimed the LB targets app_port, so no doc change needed.

@mhenrixon mhenrixon self-assigned this Aug 3, 2026
@mhenrixon mhenrixon added the bugfix Fixes a defect in fork code label Aug 3, 2026
…ot 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 <host>:<app_port>. 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
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
@mhenrixon
mhenrixon force-pushed the fix/issue-104-lb-target-port branch from e296f24 to 9926121 Compare August 3, 2026 20:46
@mhenrixon
mhenrixon merged commit 9dabb42 into dash Aug 3, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fixes a defect in fork code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3.0.0 regression: loadbalancer deploy targets host:app_port, but per-host proxies only publish 80/443 — targets can never become healthy

1 participant