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

lib/dash/cli/proxy.rb

@@ -22,7 +22,9 @@ def boot
         execute *DASH.registry.login
 
         # Before anything reads the new container, volume or network: bring a
-        # host still on pre-rename identity across. A no-op once it has been.
+        # host still on pre-rename identity across, and make the apps-config
+        # directory in the same round trip. Nothing but a `test -f` once it has
+        # been - see Dash::Cli::Proxy::LegacyRename.
         Dash::Cli::Proxy::LegacyRename.new(host, self).run
 
         proxy = DASH.proxy(host)
@@ -34,7 +36,8 @@ def boot
         else
           stale_hosts << host.to_s if drift.drifted?
 
-          version = capture_with_info(*proxy.version).strip.presence
+          # The tag off the inspect the drift check already made, not a read of its own.
+          version = drift.version
 
           if version && Dash::Utils.older_version?(version, Dash::Configuration::Proxy::Run::MINIMUM_VERSION)
             raise "dash-proxy version #{version} is too old, run `dash proxy reboot` in order to update to at least #{Dash::Configuration::Proxy::Run::MINIMUM_VERSION}"
@@ -48,7 +51,6 @@ def boot
             execute *proxy.remove_proxy_secrets_file, raise_on_non_zero_exit: false
           end
 
-          execute *proxy.ensure_apps_config_directory
           execute *proxy.start_holder_or_run if proxy.port_holder?
           execute *proxy.start_or_run(digest: drift.expected_digest)
         end
@@ -77,10 +79,10 @@ def boot
           execute *DASH.registry.login
 
           # Bring a pre-rename host across before the container can be created
-          # against an empty volume or a network nothing else joined. A no-op
-          # once it has been.
-          execute *DASH.docker.connect_legacy_network_containers
-          execute *DASH.loadbalancer.copy_legacy_config_volume
+          # against an empty volume or a network nothing else joined, and make
+          # the apps-config directory in the same round trip. Nothing but a
+          # `test -f` once it has been.
+          execute *DASH.loadbalancer.prepare_boot
 
           # The load balancer terminates TLS and owns the cache, so its host
           # needs the proxy secrets (acme credentials, cache store) just like
@@ -92,8 +94,6 @@ def boot
             execute *DASH.loadbalancer.remove_proxy_secrets_file, raise_on_non_zero_exit: false
           end
 
-          execute *DASH.loadbalancer.ensure_apps_config_directory
-
           # TLS terminates at the load balancer, so the TLS material the app
           # hosts get - custom certificates and the mTLS client CA - must
           # reach this host too; the LB container reads it through the same
@@ -105,10 +105,11 @@ def boot
           # The same drift detection the proxy hosts get: a loadbalancer booted
           # with a different config digest reboots below (or warns, when
           # automatic reboot is off) instead of serving a stale config forever.
-          container_id = capture_with_info(*DASH.loadbalancer.container_id, raise_on_non_zero_exit: false).strip
-          current_digest = capture_with_info(*DASH.loadbalancer.config_digest, raise_on_non_zero_exit: false).strip
+          state = Dash::Commands::Proxy::State.parse(
+            capture_with_info(*DASH.loadbalancer.inspect_state, raise_on_non_zero_exit: false)
+          )
 
-          if container_id.present? && current_digest != DASH.loadbalancer_config.run_config_digest
+          if state.exists? && state.digest.to_s != DASH.loadbalancer_config.run_config_digest
             if auto_reboot
               # Leave the old loadbalancer serving until its reboot below.
               lb_drifted << host.to_s

lib/dash/cli/proxy/drift.rb

@@ -7,8 +7,23 @@ def initialize(host, sshkit)
     @sshkit = sshkit
   end
 
+  # One `docker inspect` for everything a boot asks about the running proxy: whether it
+  # exists, which image tag it runs, and the digest it was booted with. Captured once per
+  # instance - `dash proxy boot` reads all three off it, and `dash doctor` only the first.
+  def state
+    @state ||= Dash::Commands::Proxy::State.parse(
+      capture_with_info(*proxy.inspect_state, raise_on_non_zero_exit: false)
+    )
+  end
+
   def container_exists?
-    capture_with_info(*proxy.container_id, raise_on_non_zero_exit: false).strip.present?
+    state.exists?
+  end
+
+  # The tag the running proxy was booted from, for the minimum-version gate. Nil when
+  # nothing is running - a host with no proxy has no version to be too old.
+  def version
+    state.version
   end
 
   # A proxy container has drifted when it was started with a different config
@@ -30,7 +45,7 @@ def expected_digest
 
   private
     def current_digest
-      capture_with_info(*proxy.config_digest, raise_on_non_zero_exit: false).strip
+      state.digest.to_s
     end
 
     def proxy

lib/dash/cli/proxy/legacy_rename.rb

@@ -23,6 +23,13 @@
 # so a second deploy is a no-op. Nothing here removes the legacy network or
 # volume: an operator who wants them gone removes them by hand, and stage 3d
 # deletes this class outright.
+#
+# All three travel as one command (Dash::Commands::Proxy#prepare_boot), guarded on a
+# marker the host writes once it is verifiably past the rename - so a migrated host, and
+# a host installed fresh on 4.x that never had a kamal-proxy, run no docker command here
+# at all. The round trip itself is one the host already pays: the command carries the
+# apps-config `mkdir -p` too, which reads nothing the bridge writes. Stage 3d keeps the
+# mkdir and deletes the rest.
 class Dash::Cli::Proxy::LegacyRename
   attr_reader :host, :sshkit
   delegate :execute, to: :sshkit
@@ -33,26 +40,6 @@ def initialize(host, sshkit)
   end
 
   def run
-    bridge_network
-    adopt_config_volume
-    replace_legacy_container
+    execute *DASH.proxy(host).prepare_boot
   end
-
-  private
-    def bridge_network
-      execute *DASH.docker.connect_legacy_network_containers
-    end
-
-    def adopt_config_volume
-      execute *DASH.proxy(host).copy_legacy_config_volume
-    end
-
-    # The drain timeout the proxy is configured with, so a busy host is not cut
-    # off mid-request any more abruptly than a normal reboot would.
-    def replace_legacy_container
-      proxy = DASH.proxy(host)
-
-      execute *proxy.remove_legacy_container(timeout: DASH.config.drain_timeout)
-      execute *proxy.remove_legacy_holder_container
-    end
 end

lib/dash/commands/base.rb

@@ -114,6 +114,13 @@ def chain(*commands)
         combine *commands, by: ";"
       end
 
+      # One subshell around an && chain. Composing two builders that each mix && and ||
+      # cannot be done flat - the operators share precedence and associate left, so the
+      # second builder's guards re-associate across the first one's.
+      def group(*commands)
+        [ "(", *combine(*commands), ")" ]
+      end
+
       def pipe(*commands)
         combine *commands, by: "|"
       end
@@ -150,13 +157,19 @@ def copy_legacy_volume(legacy:, volume:, image:)
         any \
           volume_exists(volume),
           negate(volume_exists(legacy)),
-          [ "(", *combine(docker(:volume, :create, volume), copy_between_volumes(legacy, volume, image: image)), ")" ]
+          group(docker(:volume, :create, volume), copy_between_volumes(legacy, volume, image: image))
       end
 
       def negate(command)
         [ "!", *command ]
       end
 
+      # The docker builders (network create, the stage-3c network bridge) for callers that
+      # compose them into a command of their own rather than executing them on their own.
+      def docker_commands
+        @docker_commands ||= Dash::Commands::Docker.new(config)
+      end
+
       def volume_exists(name)
         docker :volume, :inspect, name, ">", "/dev/null", "2>&1"
       end

lib/dash/commands/loadbalancer.rb

@@ -47,6 +47,28 @@ def copy_legacy_config_volume
     copy_legacy_volume(legacy: legacy_config_volume_name, volume: config_volume_name, image: loadbalancer_config.run.image)
   end
 
+  # Everything this host needs before anything reads its container, volume or network,
+  # in the one round trip it already pays for the apps-config directory. Same shape as
+  # Dash::Commands::Proxy#prepare_boot, including why the guard has to be the first word.
+  def prepare_boot
+    combine legacy_rename, ensure_apps_config_directory
+  end
+
+  # The loadbalancer's half of the stage-3c bridge - network, then volume - skipped
+  # outright by a host that has already been through it. No legacy container is replaced
+  # here (a dedicated loadbalancer host never ran one under a name this gem knows), so
+  # the marker is verified on the volume instead: the new one exists, or there was never
+  # a legacy one to adopt. Stage 3d deletes this with the rest of the bridge.
+  def legacy_rename
+    any \
+      [ :test, "-f", legacy_rename_marker ],
+      group(
+        group(docker_commands.connect_legacy_network_containers),
+        group(copy_legacy_config_volume),
+        group(any(mark_legacy_renamed, [ :true ]))
+      )
+  end
+
   def deploy(targets: [])
     docker :exec, container_name, "dash-proxy", "deploy", loadbalancer_config.config.service,
       *loadbalancer_config.deploy_command_args(targets: targets)
@@ -76,6 +98,12 @@ def config_digest
     docker :inspect, container_name, "--format", Dash::Commands::Proxy::CONFIG_DIGEST_FORMAT
   end
 
+  # One read for container id, image tag and config digest - parsed by
+  # Dash::Commands::Proxy::State, same as the per-host proxy's.
+  def inspect_state
+    docker :inspect, container_name, "--format", Dash::Commands::Proxy::STATE_FORMAT
+  end
+
   def container_id(only_running: false)
     container_id_for(container_name: container_name, only_running: only_running)
   end
@@ -163,6 +191,18 @@ def container_name
   end
 
   private
+    # Stage 3c. 3d deletes both of these with the rest of the bridge.
+    def legacy_rename_marker
+      File.join loadbalancer_config.directory, Dash::Configuration::Proxy::LEGACY_RENAME_MARKER
+    end
+
+    def mark_legacy_renamed
+      combine \
+        group(any(volume_exists(config_volume_name), negate(volume_exists(legacy_config_volume_name)))),
+        make_directory(loadbalancer_config.directory),
+        [ :touch, legacy_rename_marker ]
+    end
+
     def run_args
       loadbalancer_config.run_args
     end

lib/dash/commands/proxy.rb

@@ -12,8 +12,13 @@ class Dash::Commands::Proxy < Dash::Commands::Base
   # Both the legacy constant and the fallback go away in stage 3d.
   LEGACY_CONFIG_DIGEST_LABEL = "org.kamal.proxy-config-digest"
 
-  CONFIG_DIGEST_FORMAT = "'{{ with index .Config.Labels \"#{CONFIG_DIGEST_LABEL}\" }}{{ . }}" \
-    "{{ else }}{{ index .Config.Labels \"#{LEGACY_CONFIG_DIGEST_LABEL}\" }}{{ end }}'"
+  CONFIG_DIGEST_TEMPLATE = "{{ with index .Config.Labels \"#{CONFIG_DIGEST_LABEL}\" }}{{ . }}" \
+    "{{ else }}{{ index .Config.Labels \"#{LEGACY_CONFIG_DIGEST_LABEL}\" }}{{ end }}"
+
+  CONFIG_DIGEST_FORMAT = "'#{CONFIG_DIGEST_TEMPLATE}'"
+
+  # Everything Dash::Cli::Proxy::Drift and the minimum-version gate need, in one format.
+  STATE_FORMAT = "'{{.Id}} {{.Config.Image}} #{CONFIG_DIGEST_TEMPLATE}'"
 
   def initialize(config, host:)
     super(config)
@@ -42,6 +47,41 @@ def run(digest: nil, name: nil)
   # destination not existing, so a second deploy is a no-op. Stage 3d deletes
   # them along with the legacy constants they read.
 
+  # Everything a proxy host needs before anything reads its container, volume or
+  # network, in the one round trip it already pays for the apps-config directory.
+  #
+  # `a || b && c` is `(a || b) && c`, so the mkdir runs whichever way the guard went -
+  # and the guard has to be the first word rather than a parenthesised group, because
+  # SSHKit prefixes the first word with /usr/bin/env and passes only `test` through.
+  # Stage 3d drops the legacy_rename half and leaves the mkdir.
+  def prepare_boot
+    combine legacy_rename, ensure_apps_config_directory
+  end
+
+  # The whole stage-3c bridge as one command, skipped outright by a host that has
+  # already been through it - or was installed fresh on 4.x and never had a kamal-proxy.
+  # The three steps keep their own bodies and their documented order (see
+  # Dash::Cli::Proxy::LegacyRename); each is wrapped in its own subshell because they
+  # all mix && and || at one precedence level, and composing them flat would
+  # re-associate across the volume copy's guard - the chain 4.0.0 got wrong.
+  #
+  # The marker is written on verified absence of both legacy containers, never on the
+  # chain's exit status: the two removals end in `|| true`, so a host whose stop failed
+  # would otherwise record itself as migrated and never retry. Its own `|| true` keeps
+  # that failure as quiet as it is today, while a failed volume copy still exits
+  # non-zero through the && chain and aborts the boot exactly as it does now.
+  def legacy_rename
+    any \
+      [ :test, "-f", legacy_rename_marker ],
+      group(
+        group(docker_commands.connect_legacy_network_containers),
+        group(copy_legacy_config_volume),
+        group(remove_legacy_container(timeout: config.drain_timeout)),
+        group(remove_legacy_holder_container),
+        group(any(mark_legacy_renamed, [ :true ]))
+      )
+  end
+
   # Copies the pre-rename config volume into the new one, before anything
   # starts. The volume holds the routing table and the ACME account and
   # certificate cache; losing it means re-issuing every certificate and
@@ -116,6 +156,13 @@ def config_digest
     docker :inspect, container_name, "--format", CONFIG_DIGEST_FORMAT
   end
 
+  # One read for container id, image tag and config digest - parsed by
+  # Dash::Commands::Proxy::State. Capture it with raise_on_non_zero_exit: false;
+  # a host with no proxy container inspects to nothing, which is an answer.
+  def inspect_state
+    docker :inspect, container_name, "--format", STATE_FORMAT
+  end
+
   def container_id(only_running: false)
     container_id_for(container_name: container_name, only_running: only_running)
   end
@@ -327,6 +374,19 @@ def container_exists(name)
       docker :container, :inspect, name, ">", "/dev/null", "2>&1"
     end
 
+    # Stage 3c. 3d deletes both of these with the rest of the bridge.
+    def legacy_rename_marker
+      File.join config.proxy_boot.host_directory, Dash::Configuration::Proxy::LEGACY_RENAME_MARKER
+    end
+
+    def mark_legacy_renamed
+      combine \
+        negate(container_exists(Dash::Configuration::Proxy::LEGACY_CONTAINER_NAME)),
+        negate(container_exists(Dash::Configuration::Proxy::LEGACY_HOLDER_CONTAINER_NAME)),
+        make_directory(config.proxy_boot.host_directory),
+        [ :touch, legacy_rename_marker ]
+    end
+
     # The image the volume copy borrows. The proxy this gem is pinned to is
     # already pulled by the time the copy runs, and `rake release` gates on
     # MINIMUM_VERSION being published, so this is always resolvable — unlike

lib/dash/commands/proxy/state.rb

@@ -0,0 +1,31 @@
+# What one `docker inspect` of a proxy container tells a boot: whether it exists, which
+# image tag it runs, and the config digest it was booted with. Three questions that used
+# to cost three round trips each (`container_id`, `version`, `config_digest`).
+#
+# Produced by Dash::Commands::Proxy#inspect_state and its loadbalancer twin, both of which
+# are captured with raise_on_non_zero_exit: false - a host with no container inspects to
+# empty output, which parses to a state that simply does not exist.
+class Dash::Commands::Proxy::State
+  attr_reader :id, :image, :digest
+
+  def self.parse(output)
+    id, image, digest = output.to_s.strip.split(" ", 3)
+    new(id: id, image: image, digest: digest)
+  end
+
+  def initialize(id: nil, image: nil, digest: nil)
+    @id = id.presence
+    @image = image.presence
+    @digest = digest.presence
+  end
+
+  def exists?
+    id.present?
+  end
+
+  # The tag, read the way Dash::Commands::Proxy#version reads it - everything past the
+  # LAST colon, so a registry host carrying a port does not get mistaken for the version.
+  def version
+    image&.split(":")&.last
+  end
+end

lib/dash/configuration/proxy.rb

@@ -23,6 +23,9 @@ class Dash::Configuration::Proxy
   LEGACY_LOADBALANCER_CONTAINER_NAME = "kamal-loadbalancer"
   LEGACY_HOLDER_CONTAINER_NAME = "kamal-proxy-net"
   LEGACY_NETWORK = "kamal"
+  # Written into the run directory once a host is verifiably past the 3c rename, so the
+  # bridge costs it nothing but a `test -f` on every deploy after. Deleted in stage 3d.
+  LEGACY_RENAME_MARKER = ".legacy-renamed"
   LEGACY_CONFIG_VOLUME = "kamal-proxy-config"
   LEGACY_LOADBALANCER_CONFIG_VOLUME = "kamal-loadbalancer-config"
   LEGACY_IMAGE_TITLE = "kamal-proxy"