diff --git a/lib/dash/cli/app.rb b/lib/dash/cli/app.rb index 51c6bc59..f1b036cd 100644 --- a/lib/dash/cli/app.rb +++ b/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 diff --git a/lib/dash/cli/app/boot.rb b/lib/dash/cli/app/boot.rb index 74e15c37..2d863a6c 100644 --- a/lib/dash/cli/app/boot.rb +++ b/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 diff --git a/lib/dash/commands/app.rb b/lib/dash/commands/app.rb index c624c5c8..037d4de4 100644 --- a/lib/dash/commands/app.rb +++ b/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}}"'), diff --git a/test/cli/app_test.rb b/test/cli/app_test.rb index bf06deb6..fa0f4857 100644 --- a/test/cli/app_test.rb +++ b/test/cli/app_test.rb @@ -559,27 +559,16 @@ 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 @@ -587,6 +576,43 @@ class CliAppTest < CliTestCase 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) diff --git a/test/commands/app_test.rb b/test/commands/app_test.rb index ea58c5f8..1aa6f69c 100644 --- a/test/commands/app_test.rb +++ b/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",