perf(proxy): stop paying the stage-3c bridge on a host that has already migrated by mhenrixon · Pull Request #167 · zoolutions/dash

mhenrixon · 2026-09-12T16:58:26Z

Summary

Ensure dash-proxy is the most expensive row of a deploy by round trips, and on a host that has already been through the stage-3c rename most of what it does is checking, again, that it has been through the stage-3c rename.

Two changes, both folds or skips — no new SSH or docker command is added anywhere, and nothing changes for a host still running kamal-proxy.

1. The bridge is one command a migrated host skips inside

Dash::Commands::Proxy#legacy_rename composes the four existing builders behind a marker:

test -f .dash/proxy/.legacy-renamed || ( ( bridge_network ) && ( copy_config_volume ) && ( remove_legacy_container ) && ( remove_legacy_holder ) && ( mark || true ) )

Each step is wrapped in its own subshell because every one of them already mixes && and || at a single precedence level — composing them flat re-associates across the volume copy's guard, which is the chain 4.0.0 got wrong. The bodies are untouched, so the /usr/bin/env and precedence lessons in their comments still hold.

The marker is written only on verified absence of both legacy containers, never on the chain's exit status: the two removals end in || true, so a host whose docker container stop failed would otherwise record itself as migrated and never retry. Its own || true keeps that failure as quiet as it is today. A failed volume copy still exits non-zero through the && chain and aborts the boot, exactly as it does now.

Dash::Commands::Proxy#prepare_boot then carries the apps-config mkdir -p in the same round trip (a || b && c is (a || b) && c, so it runs either way), which is the round trip the host pays anyway. A migrated host now pays zero round trips for the bridge.

Same shape for the load-balancer branch, with its own marker under .dash/loadbalancer/. It replaces no legacy container, so its marker is verified on the volume instead: the new one exists, or there was never a legacy one to adopt.

2. One inspect for what were three reads

container_id, config_digest and version were three docker invocations against the same container. inspect_state returns id, image tag and digest in one --format, parsed by Dash::Commands::Proxy::State. Dash::Cli::Proxy::Drift owns the capture and exposes #version, so the minimum-version gate reads the tag off the inspect the drift check already made. dash doctor's drift check gets the 2 → 1 reduction for free.

Drift#drifted?, #expected_digest and the minimum-version error message keep their semantics.

Before / after

Measured with the unit harness (recorded_commands plus a recording stub on capture_with_info), not a real fleet — see Deviations. Executes and captures both counted, since both are SSH round trips.

One proxy host, migrated, running a non-drifted proxy (deploy_simple):

	`main`	this PR
`docker login`	1	1
`connect_legacy_network_containers`	1	—	skip (marker)
`copy_legacy_config_volume`	1	—	skip
`remove_legacy_container`	1	—	skip
`remove_legacy_holder_container`	1	—	skip
`ensure_apps_config_directory`	1	1	fold — now carries the guarded bridge
`container_id` (capture)	1	—	fold
`config_digest` (capture)	1	1	fold — one inspect returns all three
`version` (capture)	1	—	fold
`boot_config` (capture)	1	1	unchanged (legacy boot path only)
secrets file	1	1
`start_or_run`	1	1
total	12	6

Plus the one shared docker network create dash per host, unchanged.

Dedicated load-balancer host (deploy_with_loadbalancer): 10 → 7 — the two legacy commands become one that folds into the apps-config mkdir (3 → 1), and container_id + config_digest become one inspect (2 → 1).

Extrapolated to the 4-host topology in the issue (3 web + 1 job, LB auto-activated), the Ensure dash-proxy row's 54 round trips fall to roughly half. This is not a measured wall-clock number — no staging fleet was available in this session.

The per-host sequence is pinned exactly in test/cli/proxy_test.rb (PROXY_BOOT_ROUND_TRIPS_PER_HOST), the way #159 pinned its reduction, so a regression cannot land unnoticed.

Test plan

bundle exec rubocop --parallel — clean
bin/test — 1939 runs, 0 failures (unit + integration, Docker + published proxy image; MINIMUM_VERSION does not move)
The cost-guard sequence in test/cli/main_test.rb is unchanged
Generated shell exercised against a real sh and bash with a fake docker on PATH:
- fresh 4.x host (nothing legacy) → exit 0, marker written, apps-config created
- still-legacy host, everything succeeds → exit 0, marker written
- volume copy fails → exit 1, no marker, boot aborts
- container stop fails → exit 0, no marker (retries next deploy)
- marker present, docker removed from PATH entirely → exit 0, apps-config created, zero docker calls
- the same five for the loadbalancer's two-step variant

Still to verify on a real fleet (the issue's step 4)

A host still on kamal-proxy migrates and ends with the marker
The next deploy on that host issues no legacy docker commands (-v output) and leaves the marker untouched
A fresh host gets the marker on first boot
reboot_on_deploy: false with a drifted proxy still prints the stale warning
An older proxy image still trips the minimum-version error

Closes #160

Deviations & judgment calls

Deviations

No real-fleet before/after deploy table. The issue asks for a baseline from a real multi-host deploy on main. This session has no staging fleet, so the numbers above are measured with the unit harness and pinned as assertions. Commands and captures counted, not wall seconds — the deploy-table extrapolation is stated as such.
Step 4 (folding docker network create dash into the per-host prepare command) not done. The issue makes it conditional on the measured table still justifying it. It is one round trip per host against four for the bridge, and it would have to branch on "does this host get another proxy-boot command or not". Left for a follow-up, since there is no real table to justify it.

Discoveries

Composing the four bridge builders with && cannot be done flat: each already mixes && and || at one precedence level, so BRIDGE && COPY && … re-associates across their internals. Every step is wrapped in its own ( … ). A group helper was extracted in Dash::Commands::Base and copy_legacy_volume's inline subshell migrated onto it.
The outer command has to start with test, not (: SSHKit's command map passes test through and prefixes everything else with /usr/bin/env, and /usr/bin/env ( is exit 127. So prepare_boot relies on a || b && c == (a || b) && c rather than parenthesising the guard. This is called out in the method comment.
Dash::Commands::Proxy#version masked a missing container by accident: the | awk pipeline made the exit status awk's, so capture_with_info with the default raise_on_non_zero_exit: true never raised on a host with no proxy. inspect_state makes that explicit with raise_on_non_zero_exit: false; the observable behaviour (no container → no version → gate skipped) is identical.

Judgment calls

Dash::Cli::Proxy::LegacyRename#run now executes one command that also carries the apps-config mkdir -p. The class name covers slightly more than the rename; its header comment says so, and stage 3d keeps the mkdir and deletes the rest.
The load balancer's marker is verified on the volume, not on container absence — its branch has no legacy-container removal step, so the volume is the only thing its bridge can verify.
Drift owns the single inspect_state capture and exposes #version, rather than boot capturing the state and injecting it. Fewer moving parts, and dash doctor gets the reduction for free.
The apps-config mkdir -p now runs on every proxy host, including one that is drifted and about to reboot — it used to sit inside the not-drifted branch. It costs nothing (it rides in the bridge round trip) and Dash::Cli::Proxy::Reboot makes the directory anyway, so this is strictly fewer conditionals for the same result.
Dash::Commands::Proxy#config_digest and the loadbalancer's container_id / config_digest lose their last caller but stay: they are one-line reads of the same labels, and deleting them plus their tests is cleanup unrelated to this diff's story. Same call the issue makes for version, which doctor still uses.

Everything added here is bridge code marked Stage 3c … 3d deletes, per CLAUDE.md. No MINIMUM_VERSION change, nothing in ../kamal-proxy, and the proxy image, container, network and volume names are untouched.

Summary by cubic

Reduces proxy boot round trips by having already-migrated hosts skip the stage-3c legacy bridge entirely, and by merging three container reads into one inspect. For hosts still running kamal-proxy, the bridge runs exactly as before.

Changes

The four bridge commands are now one guarded command (marker file .legacy-renamed) that also carries the apps-config mkdir -p, so a migrated host pays zero bridge round trips.
The marker is written only after both legacy containers are verified absent via docker container ls; a docker error fails closed rather than reading as confirmed absence, keeping a failed stop retryable, while a failed volume copy still aborts the boot.
container_id, config_digest, and version become a single docker inspect parsed by Dash::Commands::Proxy::State; Drift owns that capture and exposes #version.
No new SSH or Docker commands are added, and the load balancer host gets the same treatment with its own marker.
The load balancer's marker is verified on volume existence; the reboot-before-bridge edge case that gap creates is documented and tracked in Loadbalancer reboot never routes through the stage-3c bridge, so it can create the new volume before it is copied #168.
Per proxy host round trips drop from 12 to 6; load balancer host from 10 to 7.

^{Written for commit 1014e5a. Summary will update on new commits.}

…dy migrated ## Summary `Ensure dash-proxy` is the most expensive row of a deploy by round trips, and on a host that has already been through the 3c rename most of what it did was checking, again, that it had. The four bridge commands become one, guarded on a marker in the run directory: `test -f .dash/proxy/.legacy-renamed || ( bridge && copy && replace && mark )`. A migrated host — and a host installed fresh on 4.x that never had a kamal-proxy — runs no docker command for the bridge at all. The command is folded into the apps-config `mkdir -p` the host pays anyway, so the round trip goes too. The marker is written only on verified absence of both legacy containers, never on the chain's exit status: the removals end in `|| true`, and a host whose stop failed must retry next deploy rather than record itself as done. A failed volume copy still aborts the boot through the && chain, exactly as today. `container_id`, `config_digest` and `version` were three docker invocations against the same container; one `docker inspect --format` returns all three, parsed by Dash::Commands::Proxy::State. Drift owns the capture and the minimum-version gate reads the tag off it. `dash doctor`'s drift check gets the same 2 -> 1 for free. Nothing changes for a host still running kamal-proxy: the same three steps, in the same documented order, before anything reads the new container, volume or network. Per proxy host, with a running proxy: 12 -> 6 round trips. On the load balancer host: 10 -> 7. ## Test Coverage - commands/proxy_test: the marker guard, the documented step order, per-step subshells, the verified-absence marker write, the copy left free to fail, the apps-config fold, the inspect format, and State's parsing (tag past a registry port, no container, unlabelled container) - commands/loadbalancer_test: the same for the two-step loadbalancer variant, whose marker is verified on the volume instead - cli/proxy_test: the per-host round-trip sequence pinned exactly, and a still-legacy host getting the whole bridge in order ## Verification - [x] bundle exec rubocop --parallel passes - [x] bin/test (unit + integration) passes - [x] generated shell exercised against real sh and bash with a fake docker Refs #160

cubic-dev-ai

All reported issues were addressed across 11 files

_{Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic}

- Marker check fails closed on a docker error: `mark_legacy_renamed` used a negated `docker container inspect`, whose exit code cannot distinguish "no such container" from "the daemon couldn't be asked." A transient docker error read as confirmed absence and would permanently mark a host migrated with the legacy container still running. Replaced with `confirmed_empty?`, a new Dash::Commands::Base helper built on the existing list-based `container_id_for` (which exits 0 on no match and non-zero only on a genuine failure): `result=$(list) && [ -z "$result" ]` propagates the list's own exit status, verified against real sh and bash. - Documented, and opened #168 for, the loadbalancer's narrower risk: its marker is verified on volume existence (no legacy container to check there), which a `dash proxy reboot` run before the bridge has ever executed on a host could satisfy without the legacy volume ever being copied. Root cause is LoadbalancerReboot#run (and the per-host Reboot) never routing through the bridge - pre-existing on `main`, and a fix belongs in a PR that touches those classes, not this one (scoped to `boot`'s round trips per #160). - Extracted `recorded_commands_and_captures` into CliTestCase, shared by the Printer/capture_with_info stubbing `recorded_commands` and `recorded_captures` already do separately; `recorded_proxy_round_trips` now composes it and keeps only the proxy-specific redaction. Addresses cubic-dev-ai review on PR #167.

cubic-dev-ai

0 issues found across 6 files (changes from recent commits).

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{Re-trigger cubic}

…t order `on` runs the proxy hosts in parallel threads, and the recorder behind "boot issues no round trip beyond the pinned per-host sequence" appended from both. The pin then spelled out host 1's sequence followed by host 2's, which held only while the two threads happened not to overlap. CI seed 59404 interleaved them (login, bridge, login, bridge, inspect, ...) - same commands, same count per host, different scheduling - and the test failed on a run that issued exactly what it pins. It reproduces standalone here too: 1 in 150. Tag every recorded round trip with the host it went to (the Printer command carries it; a capture reads it off SSHKit::Backend.current, the thread-local the backend sets for its run) and assert each host's own sequence. That is the claim the test was making - the count and the order the gem chooses - minus the one it never meant to: which thread the scheduler ran first. 0 in 300 after. Refs #167

…bridge (#169) * fix(proxy): route every container-creating path through the stage-3c bridge `Dash::Cli::Proxy::Reboot`, `Dash::Cli::Proxy::LoadbalancerReboot` and `dash proxy loadbalancer start` all create the renamed container without ever running the stage-3c bridge. `docker run --volume dash-loadbalancer-config:...` auto-creates the named volume empty when it does not exist, so a `dash proxy reboot` against a host that has never been through `dash proxy boot` brings the new volume into existence before the bridge has had any chance to copy the legacy routing table and ACME cache into it. The next boot then finds the new volume already there and skips the copy for good, via `copy_legacy_config_volume`'s own guard - silently. Since #167 that state also writes the `.legacy-renamed` marker, so recovery needs the marker deleted as well as the volume fixed. Fix it at the source rather than making the marker's heuristic smarter: every path that can create the container, the volume or the network now runs `prepare_boot` first. On both reboot paths this is round-trip neutral - they already spent a round trip on `ensure_apps_config_directory`, which `prepare_boot` carries. Refs #168 * docs(proxy): record why the bridge copies the config volume while it is live Both cubic and a human reader will ask whether `cp -a` over a volume the legacy container still mounts can capture a half-written routing table or certificate. It cannot: dash-proxy renames into place on every writer - the routing table via writeFileAtomic, the dynamic domain and redirect state via their own temp + rename, the response cache via CreateTemp + Rename, and the ACME cache via autocert.DirCache. Written at the shared copy rather than at one caller, since `boot` and both reboots all reach it. Refs #168 * test: stop a --quiet CLI test deciding whether later tests see SSHKit output `dash app stale_containers --quiet` in test/cli/app_test.rb leaves :error on both the DASH singleton and SSHKit's global output_verbosity: Cli::Base#initialize_commander sets the commander's verbosity and Commander#configure_sshkit_with mirrors it into SSHKit. Nothing restores either between tests — Commander#reset would, but only `dash alias` calls it — so from that point on every SSHKit.config.output.info in the process is dropped. Whether that mattered depended on the seed. CI run 34709935073 put the quiet test ahead of test/cli/healthcheck/progress_reporter_test.rb on Ruby 3.2 (seed 36230) and three of its assertions saw "", while Ruby 3.3, 3.4 and 4.0 drew seeds that passed the same commit. Reproduced locally with `bin/test --seed 36230`, three failures, same three tests. Pin both to :info in the suite's global setup, beside the Docker pins that answer the same class of problem — a test that wants another verbosity still sets it itself. Refs #166 * test: pin the proxy boot's round trips per host, not in one cross-host order `on` runs the proxy hosts in parallel threads, and the recorder behind "boot issues no round trip beyond the pinned per-host sequence" appended from both. The pin then spelled out host 1's sequence followed by host 2's, which held only while the two threads happened not to overlap. CI seed 59404 interleaved them (login, bridge, login, bridge, inspect, ...) - same commands, same count per host, different scheduling - and the test failed on a run that issued exactly what it pins. It reproduces standalone here too: 1 in 150. Tag every recorded round trip with the host it went to (the Printer command carries it; a capture reads it off SSHKit::Backend.current, the thread-local the backend sets for its run) and assert each host's own sequence. That is the claim the test was making - the count and the order the gem chooses - minus the one it never meant to: which thread the scheduler ran first. 0 in 300 after. Refs #167

cubic-dev-ai Bot reviewed Sep 12, 2026

View reviewed changes

Comment thread lib/dash/commands/loadbalancer.rb

Comment thread lib/dash/commands/proxy.rb Outdated

Comment thread test/cli/proxy_test.rb Outdated

mhenrixon mentioned this pull request Sep 12, 2026

Loadbalancer reboot never routes through the stage-3c bridge, so it can create the new volume before it is copied #168

Closed

cubic-dev-ai Bot approved these changes Sep 12, 2026

View reviewed changes

mhenrixon merged commit fbdd7bb into main Sep 12, 2026
10 checks passed

mhenrixon deleted the fix/proxy-boot-migrated-hosts branch September 12, 2026 17:44

This was referenced Sep 12, 2026

fix(proxy): route every container-creating path through the stage-3c bridge #169

Merged

Deploy report: measure every phase, build step and dash's own overhead, and turn it into advice #154

Closed

Navigation Menu

Uh oh!

Uh oh!

perf(proxy): stop paying the stage-3c bridge on a host that has already migrated - #167

perf(proxy): stop paying the stage-3c bridge on a host that has already migrated#167
mhenrixon merged 2 commits into
mainfrom
fix/proxy-boot-migrated-hosts

mhenrixon commented Sep 12, 2026 •

edited by cubic-dev-ai Bot

Loading

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

mhenrixon commented Sep 12, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

1. The bridge is one command a migrated host skips inside

2. One inspect for what were three reads

Before / after

Test plan

Still to verify on a real fleet (the issue's step 4)

Deviations & judgment calls

Summary by cubic

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mhenrixon commented Sep 12, 2026 •

edited by cubic-dev-ai Bot

Loading

cubic-dev-ai Bot left a comment •

edited

Loading