perf(deploy): cut a quarter of a deploy's SSH round trips

lib/dash/cli/app.rb

@@ -40,8 +40,8 @@ def boot
 
         # Tag once the app booted on all hosts
         on(DASH.app_hosts) do |host|
-          execute *DASH.auditor.record("Tagging #{DASH.config.absolute_image} as the latest image"), verbosity: :debug
-          execute *DASH.app.tag_latest_image
+          execute *DASH.auditor.record_then("Tagging #{DASH.config.absolute_image} as the latest image",
+            DASH.app.tag_latest_image)
         end
       end
     end

lib/dash/cli/app/boot.rb

@@ -38,22 +38,36 @@ def run
   end
 
   private
+    # Both answers come back from one round trip, which means the running version is read
+    # before any rename happens. When the clashing container IS the running one, the
+    # version to stop later is the name it was renamed to - the name that was read now
+    # belongs to the container this boot is about to start.
     def old_version_renamed_if_clashing
-      if capture_with_info(*app.container_id_for_version(version), raise_on_non_zero_exit: false).present?
+      clashing_container_id, old_version = capture_boot_state
+
+      if clashing_container_id.present?
         renamed_version = "#{version}_replaced_#{SecureRandom.hex(8)}"
         info "Renaming container #{version} to #{renamed_version} as already deployed on #{host}"
-        audit("Renaming container #{version} to #{renamed_version}")
-        execute *app.rename_container(version: version, new_version: renamed_version)
+        execute *auditor.record_then("Renaming container #{version} to #{renamed_version}",
+          app.rename_container(version: version, new_version: renamed_version))
+
+        old_version = renamed_version if old_version == version
       end
 
-      capture_with_info(*app.current_running_version, raise_on_non_zero_exit: false).strip.presence
+      old_version
+    end
+
+    def capture_boot_state
+      output = capture_with_info(*app.boot_state(version), raise_on_non_zero_exit: false).to_s
+      clashing, _, running = output.partition(/^#{Regexp.escape(Dash::Commands::App::BOOT_STATE_SEPARATOR)}$/)
+
+      [ clashing.strip.presence, running.strip.presence ]
     end
 
     def start_new_version
-      audit "Booted app version #{version}"
       hostname = "#{host.to_s[0...51].chomp(".")}-#{SecureRandom.hex(6)}"
 
-      execute *app.ensure_env_directory
+      execute *auditor.record_then("Booted app version #{version}", app.ensure_env_directory)
       upload! role.secrets_io(host), role.secrets_path, mode: "0600"
 
       execute *app.run(hostname: hostname)
@@ -161,10 +175,6 @@ def auditor
       @auditor = DASH.auditor(role: role)
     end
 
-    def audit(message)
-      execute *auditor.record(message), verbosity: :debug
-    end
-
     def gatekeeper?
       barrier && barrier_role?
     end

lib/dash/cli/base.rb

@@ -577,10 +577,18 @@ def reset_invocation(cli_class)
         instance_variable_get("@_invocations")[cli_class].pop
       end
 
+      # Every lock acquire wants the run directory to exist, but the sweep is idempotent
+      # and a process only needs it once per host - the deploy lock and the server lock
+      # were paying for it twice.
       def ensure_run_directory
-        on(DASH.hosts) do
+        pending = DASH.hosts.map(&:to_s) - DASH.run_directory_ensured_on
+        return if pending.empty?
+
+        on(pending) do
           execute(*DASH.server.ensure_run_directory)
         end
+
+        DASH.run_directory_ensured_on.concat(pending)
       end
 
       def with_env(env)

lib/dash/cli/build.rb

@@ -240,11 +240,13 @@ def mirror_hosts
       end
     end
 
+    # Audit, clean and pull share one round trip. validate_image keeps its own: folding it
+    # in would put the pull under validate_image's trailing `|| (echo ... && exit 1)`, and
+    # a failed pull would then report a missing service label.
     def pull_on_hosts(hosts)
       on(hosts) do
-        execute *DASH.auditor.record("Pulled image with version #{DASH.config.version}"), verbosity: :debug
-        execute *DASH.builder.clean, raise_on_non_zero_exit: false
-        execute *DASH.builder.pull
+        execute *DASH.auditor.record_then("Pulled image with version #{DASH.config.version}",
+          DASH.builder.clean_then_pull)
         execute *DASH.builder.validate_image
       end
     end

lib/dash/cli/prune.rb

@@ -11,9 +11,7 @@ def all
   def images
     modify(lock: true, server_lock: true) do
       on(DASH.hosts) do
-        execute *DASH.auditor.record("Pruned images"), verbosity: :debug
-        execute *DASH.prune.dangling_images
-        execute *DASH.prune.tagged_images
+        execute *DASH.auditor.record_then("Pruned images", DASH.prune.dangling_images, DASH.prune.tagged_images)
       end
     end
   end
@@ -26,11 +24,10 @@ def containers
 
     modify(lock: true, server_lock: true) do
       on(DASH.hosts) do |host|
-        execute *DASH.auditor.record("Pruned containers"), verbosity: :debug
-
-        DASH.roles_on(host).each do |role|
-          execute *DASH.prune.app_containers(retain: retain, role: role)
-        end
+        # One round trip per host, whatever it runs: a host with no app roles still
+        # records that the sweep reached it.
+        execute *DASH.auditor.record_then("Pruned containers",
+          *DASH.roles_on(host).map { |role| DASH.prune.app_containers(retain: retain, role: role) })
       end
     end
   end

lib/dash/commander.rb

@@ -7,6 +7,12 @@
 class Dash::Commander
   attr_accessor :verbosity, :holding_lock, :holding_server_lock, :connected, :logging, :lock_wait, :lock_wait_timeout, :lock_wait_interval
   attr_reader :specific_roles, :specific_hosts, :timings, :report
+
+  # Hosts whose run directory this process has already swept, so the second lock acquire
+  # of a command does not re-run the migration everywhere. Per host rather than a flag:
+  # `dash upgrade` narrows the host set between acquires, and a host that was never in
+  # scope has never been swept.
+  attr_reader :run_directory_ensured_on
   delegate :hosts, :roles, :primary_host, :primary_role, :roles_on, :app_hosts, :proxy_hosts, :accessory_hosts, to: :specifics
 
   def initialize
@@ -29,6 +35,7 @@ def reset
     @config = @config_kwargs = nil
     @output_logger = nil
     @commands = {}
+    @run_directory_ensured_on = []
   end
 
   def config

lib/dash/commands/app.rb

@@ -3,6 +3,10 @@ class Dash::Commands::App < Dash::Commands::Base
 
   ACTIVE_DOCKER_STATUSES = [ :running, :restarting ]
 
+  # Separates the two answers #boot_state returns. A container id is hex and a version is
+  # a name suffix, so neither can produce this line on its own.
+  BOOT_STATE_SEPARATOR = "--%--"
+
   attr_reader :role, :host
 
   delegate :container_name, to: :role
@@ -75,6 +79,20 @@ def current_running_version
       extract_version_from_name
   end
 
+  # Everything a boot needs to know about a host before it starts anything: whether a
+  # container for the version being deployed already exists (so it can be renamed out of
+  # the way) and which version is running now (so it can be stopped once the new one is
+  # live). Two questions, one round trip, answers split on BOOT_STATE_SEPARATOR.
+  #
+  # Chained with `;` rather than `&&`: an empty answer to either is a normal result, not
+  # a failure, and the second question must be asked whatever the first one said.
+  def boot_state(version)
+    chain \
+      container_id_for_version(version),
+      [ :echo, BOOT_STATE_SEPARATOR ],
+      current_running_version
+  end
+
   def list_versions(*docker_args, statuses: nil)
     pipe \
       docker(:ps, *container_filter_args(statuses: statuses), *docker_args, "--format", '"{{.Names}}"'),

lib/dash/commands/auditor.rb

@@ -14,6 +14,16 @@ def record(line, **details)
       append([ :echo, escape_shell_value(audit_line(line, **details)) ], audit_log_file)
   end
 
+  # The audit line and the action it describes in one round trip, still in that order:
+  # the log is written first, and `&&` means a failed write aborts the action exactly as
+  # a failed standalone audit would have.
+  #
+  # Only ever fold in commands the caller would `execute`. A `capture` folded in here
+  # would come back with nothing to distinguish the audit's own output from the answer.
+  def record_then(line, *commands, **details)
+    combine record(line, **details), *commands
+  end
+
   def reveal
     [ :tail, "-n", 50, audit_log_file ]
   end

lib/dash/commands/builder.rb

@@ -2,7 +2,7 @@
 
 class Dash::Commands::Builder < Dash::Commands::Base
   delegate \
-    :create, :remove, :dev, :push, :clean, :pull, :info, :inspect_builder,
+    :create, :remove, :dev, :push, :clean, :pull, :clean_then_pull, :info, :inspect_builder,
     :validate_image, :first_mirror, :login_to_registry_locally?, :push_env,
     to: :target
 

lib/dash/commands/builder/base.rb

@@ -14,6 +14,19 @@ def clean
     docker :image, :rm, "--force", config.absolute_image
   end
 
+  # Dropping the old image is housekeeping - a host that never had it is not an error -
+  # so it must not short-circuit whatever it shares a round trip with.
+  #
+  # The `|| true` is parenthesised because `&&` and `||` bind equally and associate left:
+  # ungrouped, an `audit && clean || true && pull` chain lets a FAILED audit fall into the
+  # same `|| true` and pull anyway, exit status 0. The group confines it to the clean.
+  #
+  # Composed only, never executed on its own: SSHKit's command map prefixes an unknown
+  # first word with /usr/bin/env, and the first word here is `(`.
+  def clean_then_pull
+    combine [ "(", *any(clean, [ :true ]), ")" ], pull
+  end
+
   def push(export_action = "registry", tag_as_dirty: false, no_cache: false)
     docker :buildx, :build,
       "--output=type=#{export_action}",