perf(app): read the version list and the running version in one round trip per host

lib/dash/cli/app.rb

@@ -186,8 +186,8 @@ def stale_containers
     with_lock_if_stopping do
       on_roles(DASH.roles, hosts: DASH.app_hosts) do |host, role|
         app = DASH.app(role: role, host: host)
-        versions = capture_with_info(*app.list_versions, raise_on_non_zero_exit: false).split("\n")
-        versions -= [ capture_with_info(*app.current_running_version, raise_on_non_zero_exit: false).strip ]
+        listed, running = Dash::Commands::App.split_state(capture_with_info(*app.stale_state, raise_on_non_zero_exit: false))
+        versions = listed.strip.split("\n") - [ running.strip ]
 
         versions.each do |version|
           if stop

lib/dash/cli/app/boot.rb

@@ -58,8 +58,7 @@ def old_version_renamed_if_clashing
     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, running = Dash::Commands::App.split_state(capture_with_info(*app.boot_state(version), raise_on_non_zero_exit: false))
 
       [ clashing.strip.presence, running.strip.presence ]
     end

lib/dash/commands/app.rb

@@ -3,10 +3,16 @@ 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.
+  # Separates the two answers #boot_state and #stale_state return. A container id is hex
+  # and a version is a name suffix, so neither can produce this line on its own.
   BOOT_STATE_SEPARATOR = "--%--"
 
+  # The two halves of a #boot_state or #stale_state capture, raw. Callers decide what an
+  # empty half means; the separator line itself is dropped.
+  def self.split_state(output)
+    output.to_s.partition(/^#{Regexp.escape(BOOT_STATE_SEPARATOR)}$/).values_at(0, 2)
+  end
+
   attr_reader :role, :host
 
   delegate :container_name, to: :role
@@ -93,6 +99,16 @@ def boot_state(version)
       current_running_version
   end
 
+  # Everything the stale check needs from a host: every version of the role that has a
+  # container, and the version running now - the difference is what is stale. Same shape
+  # as #boot_state, same separator, same reason for `;` over `&&`.
+  def stale_state
+    chain \
+      list_versions,
+      [ :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}}"'),

test/cli/app_test.rb

@@ -559,34 +559,60 @@ class CliAppTest < CliTestCase
   end
 
   test "stale_containers" do
-    SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
-      .with(:docker, :ps, "--filter", "label=service=app", "--filter", "label=destination=", "--filter", "label=role=web", "--format", "\"{{.Names}}\"", "|", "while read line; do echo ${line#app-web-}; done", raise_on_non_zero_exit: false)
-      .returns("12345678\n87654321\n")
-
-    SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
-      .with(:sh, "-c", "'docker ps --latest --format '\\''{{.Names}}'\\'' --filter label=service=app --filter label=destination= --filter label=role=web --filter status=running --filter status=restarting --filter ancestor=$(docker image ls --filter reference=dhh/app:latest --format '\\''{{.ID}}'\\'') ; docker ps --latest --format '\\''{{.Names}}'\\'' --filter label=service=app --filter label=destination= --filter label=role=web --filter status=running --filter status=restarting'", "|", :head, "-1", "|", "while read line; do echo ${line#app-web-}; done", raise_on_non_zero_exit: false)
-      .returns("12345678\n")
+    stub_stale_state versions: [ "12345678", "87654321" ], running: "12345678"
 
     run_command("stale_containers").tap do |output|
       assert_match /Detected stale container for role web with version 87654321/, output
+      assert_no_match /version 12345678/, output
     end
   end
 
   test "stop stale_containers" do
-    SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
-      .with(:docker, :ps, "--filter", "label=service=app", "--filter", "label=destination=", "--filter", "label=role=web", "--format", "\"{{.Names}}\"", "|", "while read line; do echo ${line#app-web-}; done", raise_on_non_zero_exit: false)
-      .returns("12345678\n87654321\n")
-
-    SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
-      .with(:sh, "-c", "'docker ps --latest --format '\\''{{.Names}}'\\'' --filter label=service=app --filter label=destination= --filter label=role=web --filter status=running --filter status=restarting --filter ancestor=$(docker image ls --filter reference=dhh/app:latest --format '\\''{{.ID}}'\\'') ; docker ps --latest --format '\\''{{.Names}}'\\'' --filter label=service=app --filter label=destination= --filter label=role=web --filter status=running --filter status=restarting'", "|", :head, "-1", "|", "while read line; do echo ${line#app-web-}; done", raise_on_non_zero_exit: false)
-      .returns("12345678\n")
+    stub_stale_state versions: [ "12345678", "87654321" ], running: "12345678"
 
     run_command("stale_containers", "--stop").tap do |output|
       assert_match /Stopping stale container for role web with version 87654321/, output
       assert_match /#{Regexp.escape("docker container ls --all --filter 'name=^app-web-87654321$' --quiet | xargs docker stop")}/, output
     end
   end
 
+  test "stale_containers detects nothing when the host reports no containers" do
+    stub_stale_state versions: [], running: nil
+
+    run_command("stale_containers", "--stop").tap do |output|
+      assert_no_match /stale container/, output
+      assert_no_match /xargs docker stop/, output
+    end
+  end
+
+  # --quiet drops the per-host header, not the finding itself - see puts_by_host.
+  test "stale_containers drops the host header with --quiet" do
+    stub_stale_state versions: [ "12345678", "87654321" ], running: "12345678"
+
+    run_command("stale_containers", "--quiet").tap do |output|
+      assert_match /Detected stale container for role web with version 87654321/, output
+      assert_no_match /App Host: 1\.1\.1\.1/, output
+    end
+  end
+
+  # Counted at the capture layer, not the Printer, for the same reason as the boot test
+  # above: a stubbed capture never reaches execute_command.
+  test "stale_containers reads the version list and the running version in a single round trip per host and role" do
+    captures = []
+    SSHKit::Backend::Abstract.any_instance.stubs(:capture_with_info)
+      .with { |*args| captures << args.join(" "); true }
+      .returns("12345678\n87654321\n#{Dash::Commands::App::BOOT_STATE_SEPARATOR}\n12345678\n")
+
+    run_command("stale_containers", config: :with_roles, host: nil).tap do |output|
+      assert_match /Detected stale container for role web with version 87654321/, output
+      assert_match /Detected stale container for role workers with version 87654321/, output
+    end
+
+    # Two roles over two hosts each - one capture per pair, not two.
+    assert_equal 4, captures.size, captures.inspect
+    assert captures.all? { |capture| capture.include?(Dash::Commands::App::BOOT_STATE_SEPARATOR) }, captures.inspect
+  end
+
   test "details" do
     run_command("details").tap do |output|
       assert_match "docker ps --filter label=service=app --filter label=destination= --filter label=role=web", output
@@ -1130,6 +1156,14 @@ def run_command(*command, config: :with_accessories, host: "1.1.1.1", allow_exec
       end
     end
 
+    # The single capture #stale_containers makes per (host, role): the role's versions,
+    # the separator, then the version running now.
+    def stub_stale_state(versions:, running:)
+      SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
+        .with { |*args| args.join(" ").include?(Dash::Commands::App::BOOT_STATE_SEPARATOR) }
+        .returns([ *versions, Dash::Commands::App::BOOT_STATE_SEPARATOR, running ].compact.join("\n") + "\n")
+    end
+
     def stub_running
       Object.any_instance.stubs(:sleep)
 

test/commands/app_test.rb

@@ -598,6 +598,20 @@ class CommandsAppTest < ActiveSupport::TestCase
       new_command.boot_state("999").join(" ")
   end
 
+  test "stale_state pairs the version list with the running version in one command" do
+    assert_equal \
+      "docker ps --filter label=service=app --filter label=destination= --filter label=role=web --format \"{{.Names}}\" | while read line; do echo ${line#app-web-}; done ; echo --%-- ; " \
+      "sh -c 'docker ps --latest --format '\\''{{.Names}}'\\'' --filter label=service=app --filter label=destination= --filter label=role=web --filter status=running --filter status=restarting --filter ancestor=$(docker image ls --filter reference=dhh/app:latest --format '\\''{{.ID}}'\\'') ; docker ps --latest --format '\\''{{.Names}}'\\'' --filter label=service=app --filter label=destination= --filter label=role=web --filter status=running --filter status=restarting' | head -1 | while read line; do echo ${line#app-web-}; done",
+      new_command.stale_state.join(" ")
+  end
+
+  test "split_state returns the raw halves either side of the separator line" do
+    assert_equal [ "abc\n", "\n123\n" ], Dash::Commands::App.split_state("abc\n--%--\n123\n")
+    assert_equal [ "\n", "\n" ], Dash::Commands::App.split_state("\n--%--\n")
+    assert_equal [ "", "" ], Dash::Commands::App.split_state(nil)
+    assert_equal [ "a--%--b\n", "\n" ], Dash::Commands::App.split_state("a--%--b\n--%--\n"), "only a whole line separates"
+  end
+
   test "list_versions" do
     assert_equal \
       "docker ps --filter label=service=app --filter label=destination= --filter label=role=web --format \"{{.Names}}\" | while read line; do echo ${line#app-web-}; done",