Skip to content

perf(build): fold the registry login into the mirror probe's round trip - #165

Merged
mhenrixon merged 1 commit into
mainfrom
perf/pull-login-mirror-fold
Sep 12, 2026
Merged

perf(build): fold the registry login into the mirror probe's round trip#165
mhenrixon merged 1 commit into
mainfrom
perf/pull-login-mirror-fold

Conversation

@mhenrixon

@mhenrixon mhenrixon commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

dash build pull paid four SSH round trips per app host: docker login, the docker info mirror probe, the audited clean-and-pull (#159), and validate_image. The first two now share one round trip.

  • lib/dash/commands/registry.rb — new Registry#login_then(*commands): composes docker login <server> -u … -p … > /dev/null && <command>. The login's stdout is redirected away so a capture returns the folded command's answer alone. For a local registry (login is nil) it collapses to the command alone. Credentials stay wrapped in sensitive(...) — composition keeps the array elements intact, so the debug log redacts exactly as it does for a standalone login.
  • lib/dash/cli/build.rbmirror_hostslogin_and_mirror_hosts: captures DASH.registry.login_then(DASH.builder.first_mirror) per host. On a single app host there is nothing to seed, so the probe never runs and the plain login sweep stays. The reflect: slice index out of range rescue is unchanged; a rejected login short-circuits the && and raises with docker's own unauthorized/denied text, which the rescue does not match.

This is a fold, not a skip: every command that ran before still runs, on the same hosts, in the same order. Mirror seeding behaves exactly as today.

Closes #161

Before / after

Round trips per dash build pull, measured with recorded_commands on the 4-app-host fixture (deploy_with_accessories.yml, remote registry, no mirror) — the same topology as the deploy report in #161:

Per host 4 hosts
Before (main @ 61c740e) login, probe, pull, validate 16
After login+probe, pull, validate 12

The 16 matches the Pull app image … 16 ssh row the issue was filed from. I did not have the 4-host staging target to re-run a wall-clock deploy, so the number above is the command count from the test harness rather than a re-pasted deploy-report row; the per-host structure is pinned by test "pull issues three round trips per host".

Single host: login, pull, validate (3, unchanged). Local registry, 2 hosts: probe, pull, validate ×2 (6, unchanged).

Test plan

  • test/commands/registry_test.rb — exact composed string, local-registry variant (command alone), credentials remain Dash::Utils::Sensitive
  • test/cli/build_test.rb — composed command appears redacted in output; 3 round trips per host on 4 hosts; single host logs in without probing; local registry probes without logging in; a login failure inside the composed command still raises (SSHKit::Runner::ExecuteError carrying unauthorized); mirror / mirrors seeding tests updated to match the composed capture
  • test/cli/main_test.rb — the four mirror-probe capture expectations match the composed command; the cost-guard sequence is untouched
  • Unit suite: 1885 runs, 0 failures
  • bundle exec rubocop --parallel — no offenses
  • bin/test (full suite, Docker + ghcr.io/zoolutions/dash-proxy:v1.1.0.1): 1904 runs, 6087 assertions, 0 failures — the harness registry has no mirror, so the real deploys took the rescue path through the composed command
  • A real multi-host deploy with a remote registry: confirm the Pull app image row reads 12 ssh on four hosts

Deviations & judgment calls

Deviations

  • None from the plan's shape. The fold lives in Dash::Commands::Registry#login_then (the issue offered that or Builder::Base#login_and_first_mirror); the registry is where login and its sensitive(...) wrapping already are, and the builder has no handle on the registry command object anyway.

Discoveries

  • mirror_hosts carried the app_hosts.many? branch itself, so the login had to move inside it — hence the rename to login_and_mirror_hosts. A helper that only answered "which hosts have mirrors" could not also tell pull whether the login had happened.
  • The login moved from before forward_local_registry_port to inside it. Harmless: the block is only a real port-forward when the registry is local, and a local registry has no login at all.
  • test/cli/main_test.rb had four .with(:docker, :info, "--format …") capture expectations on the mirror probe, not the one the issue anticipated. All four became block matchers. The cost-guard sequence is untouched.
  • A login failure inside the composed command surfaces as SSHKit::Runner::ExecuteError (the parallel runner wraps it), not SSHKit::Command::Failed; the rescue inside the on block correctly declines to match docker's unauthorized text and re-raises.
  • Nothing in docs/ references the mirror probe, so no docs change.

Judgment calls

  • Mocha's .with(exact, args) cannot match the composed command: Dash::Utils::Sensitive defines no ==, so credential arguments compare by identity. Every mirror-probe expectation became a block matcher asserting the command ends with the probe — which also pins that the probe is the last half, i.e. the one whose stdout the capture reads.
  • Two of the new tests (single host, local registry) pass before the change too. They are regression guards for the two cases where only one of the two commands may run.
  • No wall-clock before/after from a staging deploy — see "Before / after". Per .claude/rules/performance.md the gem's metric is round trips, and that count is exact.

Summary by cubic

Folds the docker login into the docker info mirror probe in dash build pull, cutting SSH round trips per app host from four to three (16 to 12 on four hosts) without skipping any command. Adds Registry#login_then to compose both commands and redirects the login's stdout so the capture still returns the mirror probe's answer. Credentials stay redacted, a rejected login still raises, and single-host and local-registry cases are unchanged.

Closes #161.

Written for commit f014956. Summary will update on new commits.

Review in cubic

`dash build pull` paid four SSH round trips per app host: a `docker login`, a
`docker info` mirror probe, the audited clean-and-pull (#159), and
`validate_image`. The first two carried no work the other could not carry — the
probe runs before the pulls it shapes, and nothing between the login and the
probe depends on ordering across hosts.

`Dash::Commands::Registry#login_then` composes them into one command per host:

    docker login <server> -u … -p … > /dev/null && docker info --format '…'

The login's stdout is redirected away so the capture returns the mirror answer
alone. Mirror seeding is unchanged — a host with no mirror still fails the
`docker info` half with docker's index error, which is what "no mirror" means,
and a rejected login short-circuits the `&&` and raises as before. The two
single-command cases stay single: one app host issues only the login (nothing to
seed), a local registry only the probe (`Registry#login` returns nil).

Four app hosts: 16 round trips → 12. This is a fold, not a skip; every command
that ran before still runs, in the same order.

Refs #161
@mhenrixon
mhenrixon enabled auto-merge (squash) September 12, 2026 10:05

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 5 files

Confidence score: 5/5

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

Re-trigger cubic

@mhenrixon
mhenrixon merged commit d93fd8f into main Sep 12, 2026
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant