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

lib/dash/cli/proxy.rb

@@ -414,6 +414,12 @@ def loadbalancer(status)
       if DASH.config.proxy.load_balancing?
         on(DASH.config.proxy.effective_loadbalancer) do |host|
           execute *DASH.registry.login
+          # start_or_run falls through to `docker run` on a host with no
+          # container, so this is a volume-creating path too and gets the same
+          # bridge boot and reboot do (zoolutions/dash#168). It also makes the
+          # apps-config directory the bind mount would otherwise have docker
+          # create root-owned.
+          execute *DASH.loadbalancer.prepare_boot
           execute *DASH.loadbalancer.start_or_run
         end
       else

lib/dash/cli/proxy/loadbalancer_reboot.rb

@@ -21,6 +21,14 @@ def run
     execute *DASH.registry.login
     ensure_network
 
+    # After the network the bridge attaches the legacy network's containers to,
+    # and before anything that could create the new container or let `docker
+    # run --volume` create dash-loadbalancer-config empty: adopt the legacy
+    # volume's routing table, dynamic domains and ACME cache. Carries the
+    # apps-config mkdir this reboot paid a round trip for anyway
+    # (zoolutions/dash#168, see Dash::Commands::Loadbalancer#legacy_rename).
+    execute *DASH.loadbalancer.prepare_boot
+
     info "Stopping and removing #{DASH.loadbalancer.container_name} on #{host}, if running..."
     execute *DASH.loadbalancer.stop, raise_on_non_zero_exit: false
     execute *DASH.loadbalancer.remove_container
@@ -32,7 +40,6 @@ def run
       execute *DASH.loadbalancer.remove_proxy_secrets_file, raise_on_non_zero_exit: false
     end
 
-    execute *DASH.loadbalancer.ensure_apps_config_directory
     Dash::Cli::Proxy::LoadbalancerClaim.new(host, sshkit).claim_run_config(replace: true)
     execute *DASH.loadbalancer.run
 

lib/dash/cli/proxy/reboot.rb

@@ -37,7 +37,12 @@ def pull_image
 
     def replace_container
       execute *proxy.ensure_proxy_directory
-      execute *proxy.ensure_apps_config_directory
+      # Before the new container - and the new config volume `docker run
+      # --volume` would otherwise create empty - exists: bring a host still on
+      # pre-rename identity across. Carries the apps-config mkdir this reboot
+      # paid a round trip for anyway, so the bridge costs none here either
+      # (zoolutions/dash#168, see Dash::Cli::Proxy::LegacyRename).
+      execute *proxy.prepare_boot
       sync_proxy_secrets
 
       if proxy.port_holder?

lib/dash/commands/loadbalancer.rb

@@ -199,18 +199,16 @@ def legacy_rename_marker
     # Verified on the volume existing, not on a container being gone - the loadbalancer
     # replaces no legacy container, so this is the only signal its bridge has. That makes
     # it foolable in one specific way: if `dash-loadbalancer-config` comes to exist before
-    # this bridge ever runs on a host - e.g. `dash proxy reboot` invoked directly against a
-    # dedicated LB host that has never been through `dash proxy boot`, since
-    # Dash::Cli::Proxy::LoadbalancerReboot#run boots the container without calling this
-    # bridge first - the marker is written despite the legacy volume's routing table and
-    # ACME cache never having been copied. That gap predates this fold: on `main`,
-    # `copy_legacy_config_volume`'s own guard (`volume_exists(new) || ...`) already skips
-    # the copy for good in that state, silently, every deploy - this only turns a
-    # per-deploy re-check into a cached one. Closing it means routing `reboot` through the
-    # same bridge, a `Dash::Cli::Proxy::LoadbalancerReboot` change, out of scope here
-    # (zoolutions/dash#160 is `boot`'s round trips) - tracked in zoolutions/dash#168.
-    # Recovery today is the same either way: copy the legacy volume's contents over by
-    # hand, then remove .legacy-renamed under this host's loadbalancer directory so this
+    # this bridge ever runs on a host, the marker is written despite the legacy volume's
+    # routing table and ACME cache never having been copied. The fix is upstream of the
+    # heuristic rather than in it - every path that can create the container, and with it
+    # the volume `docker run --volume` auto-creates empty, runs the bridge first:
+    # `boot`, both reboots (Dash::Cli::Proxy::Reboot, Dash::Cli::Proxy::LoadbalancerReboot)
+    # and `dash proxy loadbalancer start` (zoolutions/dash#168).
+    #
+    # If a host is somehow in that state anyway - an operator's own `docker run`, a
+    # volume created by hand - recovery is to copy the legacy volume's contents over,
+    # then remove .legacy-renamed under this host's loadbalancer directory so this
     # re-evaluates.
     def mark_legacy_renamed
       combine \

lib/dash/commands/proxy.rb

@@ -101,6 +101,16 @@ def legacy_rename
   # first deploy. The subshell groups create-and-copy because `&&` and `||`
   # share precedence and associate left — without it a host that already has
   # the new volume would still run the copy over live state.
+  #
+  # The source volume is still mounted by the legacy container while this runs -
+  # deliberately, on every path (see Dash::Cli::Proxy::LegacyRename's step order), and
+  # safe because every writer into it renames into place: the routing table through
+  # writeFileAtomic, the dynamic domain and redirect state through their own temp +
+  # rename, the response cache through CreateTemp + Rename, and the ACME cache through
+  # autocert.DirCache. `cp -a` reads a complete file either way. Skew across files is
+  # possible and harmless - an unused certificate, or a route whose certificate reissues -
+  # and --recheck-targets-on-restore re-verifies the targets on the way back up
+  # (zoolutions/dash#169 review).
   def copy_legacy_config_volume(volume: Dash::Configuration::Proxy::CONFIG_VOLUME, legacy: Dash::Configuration::Proxy::LEGACY_CONFIG_VOLUME)
     copy_legacy_volume(legacy: legacy, volume: volume, image: proxy_image)
   end

test/cli/cli_test_case.rb

@@ -66,11 +66,16 @@ def recorded_captures
     # array between both stubs instead. Formats captures without their trailing options
     # hash (`raise_on_non_zero_exit: false` and friends), which reads as noise next to a
     # shell command; #recorded_captures keeps the hash for its own callers.
+    # Each round trip is tagged with the host it went to. `on` runs hosts in parallel
+    # threads, so the order ACROSS hosts is whatever the scheduler chose that run - a pin
+    # that spells out host A's sequence and then host B's held only while the threads
+    # happened not to overlap (CI seed 59404 interleaved them). Only the order within one
+    # host's list is the gem's to promise; group by host and assert there.
     def recorded_commands_and_captures
       round_trips = []
-      SSHKit::Backend::Printer.any_instance.stubs(:execute_command).with { |cmd| round_trips << cmd.to_command; true }
+      SSHKit::Backend::Printer.any_instance.stubs(:execute_command).with { |cmd| round_trips << [ cmd.host.to_s, cmd.to_command ]; true }
       SSHKit::Backend::Abstract.any_instance.stubs(:capture_with_info)
-        .with { |*args| round_trips << args.reject { |arg| arg.is_a?(Hash) }.join(" "); false }
+        .with { |*args| round_trips << [ SSHKit::Backend.current.host.to_s, args.reject { |arg| arg.is_a?(Hash) }.join(" ") ]; false }
 
       begin
         yield

test/cli/proxy_test.rb

@@ -78,6 +78,34 @@ class CliProxyTest < CliTestCase
     end
   end
 
+  # zoolutions/dash#168, the loadbalancer half: a dedicated LB host reached by `reboot`
+  # before it was ever booted would get `dash-loadbalancer-config` created empty by
+  # `docker run`, and its routing table, dynamic domains and ACME cache would never be
+  # adopted from `kamal-loadbalancer-config`.
+  test "reboot routes the loadbalancer host through the stage-3c bridge before creating the container" do
+    Dash::Configuration::Proxy.any_instance.unstub(:load_balancing?)
+
+    run_command("reboot", "-y", fixture: :with_loadbalancer).tap do |output|
+      create = output.index("docker network create dash on lb.example.com")
+      bridge = output.index("test -f .dash/loadbalancer/.legacy-renamed || (")
+      copy = output.index("docker volume inspect dash-loadbalancer-config > /dev/null 2>&1 || ! docker volume inspect kamal-loadbalancer-config")
+      run = output.index("docker run --name load-balancer")
+
+      assert bridge, "reboot must send the loadbalancer host the bridge: #{output}"
+      assert create && copy && run, output
+      assert create < bridge, "the bridge attaches containers to the dash network, which has to exist first"
+      assert bridge < run, "the volume copy must land before docker run can create the volume empty"
+    end
+  end
+
+  test "reboot carries the loadbalancer bridge in the apps-config round trip it already paid" do
+    Dash::Configuration::Proxy.any_instance.unstub(:load_balancing?)
+
+    run_command("reboot", "-y", fixture: :with_loadbalancer).tap do |output|
+      assert_match(/test -f \.dash\/loadbalancer\/\.legacy-renamed \|\| \(.*\) && mkdir -p \.dash\/proxy\/apps-config on lb\.example\.com/, output)
+    end
+  end
+
   # SSHKit prefixes the first word of every command with /usr/bin/env, and
   # `env !` is exit 127 — so no chain the gem emits may start with `!`.
   test "no command sent to a host starts with shell negation" do
@@ -115,8 +143,11 @@ class CliProxyTest < CliTestCase
 
     round_trips = recorded_proxy_round_trips { run_command("boot", fixture: :simple) }
 
-    assert_equal [ "docker network create dash" ] * 2 + PROXY_BOOT_ROUND_TRIPS_PER_HOST * 2, round_trips,
-      "deploy_simple has two proxy hosts: the network sweep, then the pinned sequence twice"
+    assert_equal %w[ 1.1.1.1 1.1.1.2 ], round_trips.keys.sort, "deploy_simple has two proxy hosts"
+    round_trips.each do |host, sequence|
+      assert_equal [ "docker network create dash" ] + PROXY_BOOT_ROUND_TRIPS_PER_HOST, sequence,
+        "#{host}: the network sweep, then the pinned sequence"
+    end
   end
 
   # Nothing changes for a host still running kamal-proxy: the three bridge steps travel
@@ -136,6 +167,32 @@ class CliProxyTest < CliTestCase
     end
   end
 
+  # zoolutions/dash#168. `reboot` creates the new container, and `docker run --volume`
+  # auto-creates the new config volume empty if it is not there yet - so a host rebooted
+  # before it was ever booted would take the bridge's volume-existence guard out from
+  # under it, adopting nothing and recording itself migrated. The bridge belongs on this
+  # path too, ahead of anything that can create the new identity.
+  test "reboot routes a host through the stage-3c bridge before replacing the container" do
+    run_command("reboot", "-y").tap do |output|
+      bridge = output.index("test -f .dash/proxy/.legacy-renamed || (")
+      stop = output.index("docker container stop --time 40 dash-proxy on 1.1.1.1")
+      run = output.index("| xargs docker run --name dash-proxy")
+
+      assert bridge, "reboot must send the bridge: #{output}"
+      assert stop && run, output
+      assert bridge < stop, "the bridge decides before the running proxy is taken down: #{output}"
+      assert bridge < run, "the new volume must not exist before the legacy one is copied into it"
+    end
+  end
+
+  # The bridge rides the apps-config mkdir the reboot already paid a round trip for, so
+  # closing the gap costs none - the same fold boot got in zoolutions/dash#167.
+  test "reboot carries the bridge in the apps-config round trip it already paid" do
+    run_command("reboot", "-y").tap do |output|
+      assert_match(/test -f \.dash\/proxy\/\.legacy-renamed \|\| \(.*\) && mkdir -p \.dash\/proxy\/apps-config on 1\.1\.1\.1/, output)
+    end
+  end
+
   test "boot takes the server lock so concurrent destinations serialise on the shared proxy" do
     run_command("boot").tap do |output|
       assert_match "Acquiring the server lock...", output
@@ -1053,6 +1110,22 @@ class CliProxyTest < CliTestCase
     end
   end
 
+  # `start_or_run` falls through to `docker run` on a host with no container, which is
+  # the same volume-creating path `reboot` takes - so this admin command gets the bridge
+  # too, the last thing that can create dash-loadbalancer-config (zoolutions/dash#168).
+  test "loadbalancer start routes through the stage-3c bridge before it can run" do
+    Dash::Configuration::Proxy.any_instance.unstub(:load_balancing?)
+
+    run_command("loadbalancer", "start", fixture: :with_loadbalancer).tap do |output|
+      bridge = output.index("test -f .dash/loadbalancer/.legacy-renamed || (")
+      start = output.index("docker container start load-balancer || docker run --name load-balancer")
+
+      assert bridge, "loadbalancer start must send the bridge: #{output}"
+      assert start, output
+      assert bridge < start, "the volume copy has to land before docker run could create the volume empty"
+    end
+  end
+
   test "loadbalancer stop" do
     Dash::Configuration::Proxy.any_instance.unstub(:load_balancing?)
 
@@ -1535,13 +1608,16 @@ def stub_proxy_drift(version: Dash::Configuration::Proxy::Run::MINIMUM_VERSION)
       stub_proxy_state("dash-proxy", "abc123 ghcr.io/zoolutions/dash-proxy:#{version} stale-digest")
     end
 
-    # Every round trip a proxy boot spends, executes and captures interleaved in issue
-    # order, with the deploy's own run-directory and lock commands filtered out and the
-    # long ones elided - the count and the order are what this pins, not the shell.
+    # Every round trip a proxy boot spends, per host: executes and captures interleaved in
+    # the order that host issued them, with the deploy's own run-directory and lock commands
+    # filtered out and the long ones elided - the count and the order are what this pins,
+    # not the shell. Keyed by host because hosts run in parallel and only each host's own
+    # order is deterministic (see CliTestCase#recorded_commands_and_captures).
     def recorded_proxy_round_trips
       recorded_commands_and_captures { yield }
-        .reject { |command| command.match?(/\.dash\/lock-|mv \.kamal \.dash/) }
-        .map { |command| elide(command) }
+        .reject { |_host, command| command.match?(/\.dash\/lock-|mv \.kamal \.dash/) }
+        .group_by(&:first)
+        .transform_values { |pairs| pairs.map { |_host, command| elide(command) } }
     end
 
     # Collapses the parts that carry a digest, a boot-config read or the whole stage-3c

test/test_helper.rb

@@ -74,6 +74,21 @@ class ActiveSupport::TestCase
     # when it is on PATH, so a developer who has it installed would see different advice
     # than CI does. Pin it off; test/dockerfile/hadolint_test.rb turns it back on.
     Dash::Dockerfile::Hadolint.stubs(:available?).returns(false)
+
+    # Fourth path, this one internal to the suite: a CLI test that runs a command with
+    # --quiet or -v leaves that verbosity behind on BOTH the DASH singleton and SSHKit's
+    # global output_verbosity (Cli::Base#initialize_commander, then
+    # Commander#configure_sshkit_with). Nothing restores either between tests -
+    # Commander#reset would, but only `dash alias` calls it - so
+    # `dash app stale_containers --quiet` in test/cli/app_test.rb silences every later
+    # SSHKit.config.output.info in the process. Whether that mattered depended on the
+    # seed: CI seed 36230 put it ahead of
+    # test/cli/healthcheck/progress_reporter_test.rb and took three of its assertions
+    # down to "", while the other three Ruby versions' seeds passed the same commit.
+    # Pin the default so the order cannot decide; a test that wants another verbosity
+    # still sets it itself.
+    DASH.verbosity = :info
+    SSHKit.config.output_verbosity = :info
   end
 
   # Dash::Commands::Base#ensure_run_directory — the one-shot .kamal -> .dash