Pull app image: fold the registry login and the mirror probe into one round trip per host

[mhenrixon]

Pull app image: fold the registry login and the mirror probe into one round trip per host

Follow-up to #154 (PR 5, #159, folded clean + pull but left the login and the mirror probe as they were). Sibling of #160 (proxy boot), the stale-container issue and the boot issue filed from the same deploy report.

Problem / Goal

On a real 4-host deploy on dash 4.1.0 (3 web, 1 job, prebuilt image, remote registry):

  Pull app image                         25.1s  16 ssh    94.1s

Reading Dash::Cli::Build#pull for four app hosts, the 16 round trips are exactly:

Per app host	Round trips	Origin
docker login	1	login_to_registry_remotely (on(DASH.app_hosts))
docker info --format '{{index .RegistryConfig.Mirrors 0}}'	1	mirror_hosts (only when DASH.app_hosts.many?)
audit + clean_then_pull	1	pull_on_hosts (#159)
validate_image	1	pull_on_hosts (kept separate on purpose, see #159)

The mirror probe exists to pull once per registry mirror before the rest of the fleet, so a mirror is seeded by one host instead of hammered by all of them. Most fleets configure no mirror, and on those docker info fails with error calling index: reflect: slice index out of range, which the code rescues. Either way it is a full SSH round trip per host per deploy that carries no other work, and it runs before the pulls it is meant to shape. The login is also its own round trip on every host even though nothing between it and the probe depends on ordering across hosts.

Done looks like: one round trip per host does the login and the probe, mirror seeding behaves exactly as today (seeded when a mirror is configured, plain fan-out when not), and the row reads 12 ssh on the same topology, with before/after in the PR.

Context (read these first)

• lib/dash/cli/build.rb — pull (login unless local registry → forward_local_registry_port → mirror_hosts → pull_on_hosts), mirror_hosts (the capture_with_info(*DASH.builder.first_mirror) per host and the reflect: slice index out of range rescue that means "no mirror configured"), login_to_registry_remotely (on(DASH.app_hosts) { execute *DASH.registry.login }), pull_on_hosts (the perf(deploy): cut a quarter of a deploy's SSH round trips #159 fold: auditor.record_then(..., DASH.builder.clean_then_pull) then validate_image).
• lib/dash/commands/registry.rb — login returns nil for a local registry and otherwise docker login <server> -u <sensitive> -p <sensitive>; the sensitive(...) wrapper is what keeps the credentials out of logs and must survive any composition. docker login prints Login Succeeded to stdout, which a capture would otherwise return.
• lib/dash/commands/builder/base.rb — first_mirror (docker info --format '{{index .RegistryConfig.Mirrors 0}}'), clean_then_pull and its comment on why the composed command is wrapped in ( … ) (SSHKit prefixes an unknown first word with /usr/bin/env).
• lib/dash/commands/auditor.rb — record_then (perf(deploy): cut a quarter of a deploy's SSH round trips #159): the pattern for folding independent commands into one round trip in the Commands layer; lib/dash/commands/base.rb — combine, chain, pipe, any.
• lib/dash/commander.rb — app_hosts, registry.
• Tests: test/cli/build_test.rb (the pull assertions, and the "one round trip per host" test near line 355 that uses recorded_commands from test/cli/cli_test_case.rb), test/commands/registry_test.rb, test/commands/builder_test.rb, test/cli/main_test.rb (the cost-guard sequence; build:pull is a stubbed subcommand there, so pin the reduction in build_test).
• Rules: .claude/rules/performance.md (round trips are the metric; before/after from the deploy table), .claude/rules/coding-style.md (shell composed in Dash::Commands::*, never inline in a CLI command), .claude/rules/testing.md.

Decision

One capture per host that logs in and probes the mirror in the same shell string; the login's own stdout is discarded so the capture is the mirror answer alone.

Shape, built in the Commands layer (name open, e.g. Dash::Commands::Registry#login_then(*command) or a Builder::Base#login_and_first_mirror):

docker login <server> -u … -p … > /dev/null && docker info --format '{{index .RegistryConfig.Mirrors 0}}'

• mirror_hosts captures this instead of first_mirror, and login_to_registry_remotely is no longer called separately when the probe runs. The existing rescue keeps working: a host without a mirror still fails on the docker info half with the reflect: slice index out of range message, and a login failure fails with docker's own unauthorized/denied text, which the rescue does not match and so re-raises, as today.
• The two cases where today only one of the two commands runs stay as they are: a local registry (DASH.registry.local?) issues only the probe, since Registry#login returns nil; a single app host (app_hosts.many? false) issues only the login, since there is nothing to seed.
• Ordering across hosts is unchanged: all hosts log in and answer the probe in one parallel on, then the seed pulls, then the rest, exactly as now. Within a host, login still precedes anything that pulls.
• Credentials stay wrapped in sensitive(...) inside the composed command so the debug log redacts them exactly as it does for the standalone login.

Alternatives considered

• Probe only behind a new registry: mirrors: true key. Cheapest, but a fleet with a daemon-side mirror silently loses seeding until an operator learns the key exists; the mirror is daemon config, not deploy config, so the gem cannot know. Rejected in interview.
• Fold the probe into the pull command. The pull order depends on the probe's answer, so it cannot share a round trip with the thing it decides. Rejected.
• Fold validate_image into the pull. Already rejected in perf(deploy): cut a quarter of a deploy's SSH round trips #159: a failed pull would report a missing label.

Settled in interview:

• Fold login + mirror probe into one capture per host. No new config key; mirror seeding keeps its current behaviour.

Design decisions the executor must not reopen

• No new SSH or docker command; the change is a fold. The audit line still precedes the pull it describes, and validate_image stays its own round trip.
• The login's stdout must be redirected inside the composed command; the capture must return only the mirror (or fail).
• Secrets never appear in test fixtures or assertions unredacted; use the existing sensitive expectations in test/commands/registry_test.rb as the model.

Implementation steps

One PR (perf/pull-login-mirror-fold off fresh main). Baseline first: a real multi-host deploy with a remote registry, paste the Pull app image row.

1. lib/dash/commands/registry.rb (or builder/base.rb, whichever reads better) — the composed login-then-probe command; unit-test the exact string, the local-registry variant (probe only) and that credentials are sensitive.
2. lib/dash/cli/build.rb — pull: when the probe will run, use the composed capture and skip the separate login sweep; when it will not (single host), keep the plain login. Keep the reflect: slice index out of range rescue, add a test that a login failure inside the composed command still raises.
3. Tests RED first: test/cli/build_test.rb — pull on deploy_with_roles.yml (multi-host) issues 3 round trips per host, one of which is the composed login+probe; single-host fixture issues login, pull, validate; local registry fixture issues probe, pull, validate with no login. test/commands/registry_test.rb, test/commands/builder_test.rb for the builders.
4. Real deploy through the integration harness (the harness registry has no mirror: confirm the rescue path) and against a staging target; paste before/after rows.

Verification gates

• bundle exec ruby -Itest -e 'Dir["test/**/*_test.rb"].grep_v(/integration/).each { |f| require File.expand_path(f) }' — green
• bundle exec rubocop --parallel — no offenses
• bin/test — full suite (Docker + published proxy image; MINIMUM_VERSION does not move)
• PR description shows the Pull app image row before and after on the same topology (expected 16 → 12 on four hosts) and states the reduction is a fold, not a skip.
• Cost-guard sequence in test/cli/main_test.rb unchanged.

Out of scope

• The pull itself (the 94 s in SSH are real image transfers) and the first-connection cost (5.5 s connect across four hosts is the SSH handshake the phase pays for being first to touch the hosts).
• Any change to mirror seeding semantics or a config key for it.
• validate_image, the local registry port forwarding, and the build/push path.
• No direct pushes to main, no manual lib/dash/version.rb bumps, no MINIMUM_VERSION change, nothing in ../kamal-proxy, no frozen-artifact renames.

Execution

Hand to a fresh implementation session on the sonnet tier. Baseline table first, then steps 1–4.