Merge pull request #271 from basecamp/app-boot-for-rollback

Call app:boot to rollback

lib/mrsk/cli/app.rb

@@ -2,6 +2,7 @@ class Mrsk::Cli::App < Mrsk::Cli::Base
   desc "boot", "Boot app on servers (or reboot app if already running)"
   def boot
     with_lock do
+      hold_lock_on_error do
         say "Get most recent version available as an image...", :magenta unless options[:version]
         using_version(version_or_latest) do |version|
           say "Start container with version #{version} using a #{MRSK.config.readiness_delay}s readiness delay (or reboot if already running)...", :magenta
@@ -18,17 +19,17 @@ class Mrsk::Cli::App < Mrsk::Cli::Base
               app = MRSK.app(role: role)
               auditor = MRSK.auditor(role: role)
 
-            execute *auditor.record("Booted app version #{version}"), verbosity: :debug
+              if capture_with_info(*app.container_id_for_version(version, only_running: true), raise_on_non_zero_exit: false).present?
-
-            if capture_with_info(*app.container_id_for_version(version), raise_on_non_zero_exit: false).present?
                 tmp_version = "#{version}_replaced_#{SecureRandom.hex(8)}"
                 info "Renaming container #{version} to #{tmp_version} as already deployed on #{host}"
                 execute *auditor.record("Renaming container #{version} to #{tmp_version}"), verbosity: :debug
                 execute *app.rename_container(version: version, new_version: tmp_version)
               end
 
+              execute *auditor.record("Booted app version #{version}"), verbosity: :debug
+
               old_version = capture_with_info(*app.current_running_version, raise_on_non_zero_exit: false).strip
-            execute *app.run
+              execute *app.start_or_run
 
               Mrsk::Utils::HealthcheckPoller.wait_for_healthy(pause_after_ready: true) { capture_with_info(*app.status(version: version)) }
 
@@ -38,6 +39,7 @@ class Mrsk::Cli::App < Mrsk::Cli::Base
         end
       end
     end
+  end
 
   desc "start", "Start existing app container on servers"
   def start

lib/mrsk/cli/main.rb

@@ -37,9 +37,7 @@ class Mrsk::Cli::Main < Mrsk::Cli::Base
         say "Detect stale containers...", :magenta
         invoke "mrsk:cli:app:stale_containers", [], invoke_options
 
-        hold_lock_on_error do
         invoke "mrsk:cli:app:boot", [], invoke_options
-        end
 
         say "Prune old containers and images...", :magenta
         invoke "mrsk:cli:prune:all", [], invoke_options
@@ -70,11 +68,9 @@ class Mrsk::Cli::Main < Mrsk::Cli::Base
         say "Detect stale containers...", :magenta
         invoke "mrsk:cli:app:stale_containers", [], invoke_options
 
-        hold_lock_on_error do
         invoke "mrsk:cli:app:boot", [], invoke_options
       end
     end
-    end
 
     audit_broadcast "Redeployed #{service_version} in #{runtime.round} seconds" unless options[:skip_broadcast]
   end
@@ -84,34 +80,11 @@ class Mrsk::Cli::Main < Mrsk::Cli::Base
     with_lock do
       invoke_options = deploy_options
 
-      hold_lock_on_error do
       MRSK.config.version = version
       old_version = nil
 
       if container_available?(version)
-          say "Start version #{version}, then wait #{MRSK.config.readiness_delay}s for app to boot before stopping the old version...", :magenta
+        invoke "mrsk:cli:app:boot", [], invoke_options.merge(version: version)
-
-          on(MRSK.hosts) do
-            execute *MRSK.auditor.record("Tagging #{MRSK.config.absolute_image} as the latest image"), verbosity: :debug
-            execute *MRSK.app.tag_current_as_latest
-          end
-
-          on(MRSK.hosts) do |host|
-            roles = MRSK.roles_on(host)
-
-            roles.each do |role|
-              app = MRSK.app(role: role)
-              old_version = capture_with_info(*app.current_running_version).strip.presence
-
-              execute *app.start
-
-              if old_version
-                sleep MRSK.config.readiness_delay
-
-                execute *app.stop(version: old_version), raise_on_non_zero_exit: false
-              end
-            end
-          end
 
         audit_broadcast "Rolled back #{service_version(Mrsk::Utils.abbreviate_version(old_version))} to #{service_version}" unless options[:skip_broadcast]
       else
@@ -119,7 +92,6 @@ class Mrsk::Cli::Main < Mrsk::Cli::Base
       end
     end
   end
-  end
 
   desc "details", "Show details about all containers"
   def details

lib/mrsk/commands/app.rb

@@ -6,6 +6,10 @@ class Mrsk::Commands::App < Mrsk::Commands::Base
     @role = role
   end
 
+  def start_or_run
+    combine start, run, by: "||"
+  end
+
   def run
     role = config.role(self.role)
 
@@ -91,8 +95,8 @@ class Mrsk::Commands::App < Mrsk::Commands::Base
     docker :ps, "--quiet", *filter_args(status: :running), "--latest"
   end
 
-  def container_id_for_version(version)
+  def container_id_for_version(version, only_running: false)
-    container_id_for(container_name: container_name(version))
+    container_id_for(container_name: container_name(version), only_running: only_running)
   end
 
   def current_running_version

lib/mrsk/commands/base.rb

@@ -18,8 +18,8 @@ module Mrsk::Commands
       end
     end
 
-    def container_id_for(container_name:)
+    def container_id_for(container_name:, only_running: false)
-      docker :container, :ls, "--all", "--filter", "name=^#{container_name}$", "--quiet"
+      docker :container, :ls, *("--all" unless only_running), "--filter", "name=^#{container_name}$", "--quiet"
     end
 
     private

test/cli/app_test.rb

@@ -19,7 +19,7 @@ class CliAppTest < CliTestCase
     run_command("details") # Preheat MRSK const
 
     SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
-      .with(:docker, :container, :ls, "--all", "--filter", "name=^app-web-latest$", "--quiet", raise_on_non_zero_exit: false)
+      .with(:docker, :container, :ls, "--filter", "name=^app-web-latest$", "--quiet", raise_on_non_zero_exit: false)
       .returns("12345678") # running version
 
     SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
@@ -50,6 +50,18 @@ class CliAppTest < CliTestCase
     run_command("boot", config: :with_boot_strategy)
   end
 
+  test "boot errors leave lock in place" do
+    invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "skip_broadcast" => false, "version" => "999" }
+
+    Mrsk::Cli::App.any_instance.expects(:using_version).raises(RuntimeError)
+
+    assert !MRSK.holding_lock?
+    assert_raises(RuntimeError) do
+      stderred { run_command("boot") }
+    end
+    assert MRSK.holding_lock?
+  end
+
   test "start" do
     run_command("start").tap do |output|
       assert_match "docker start app-web-999", output

test/cli/main_test.rb

@@ -80,23 +80,6 @@ class CliMainTest < CliTestCase
     end
   end
 
-  test "deploy errors during critical section leave lock in place" do
-    invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "skip_broadcast" => false, "version" => "999" }
-
-    Mrsk::Cli::Main.any_instance.expects(:invoke).with("mrsk:cli:registry:login", [], invoke_options)
-    Mrsk::Cli::Main.any_instance.expects(:invoke).with("mrsk:cli:build:deliver", [], invoke_options)
-    Mrsk::Cli::Main.any_instance.expects(:invoke).with("mrsk:cli:app:stale_containers", [], invoke_options)
-    Mrsk::Cli::Main.any_instance.expects(:invoke).with("mrsk:cli:traefik:boot", [], invoke_options)
-    Mrsk::Cli::Main.any_instance.expects(:invoke).with("mrsk:cli:healthcheck:perform", [], invoke_options)
-    Mrsk::Cli::Main.any_instance.expects(:invoke).with("mrsk:cli:app:boot", [], invoke_options).raises(RuntimeError)
-
-    assert !MRSK.holding_lock?
-    assert_raises(RuntimeError) do
-      stderred { run_command("deploy") }
-    end
-    assert MRSK.holding_lock?
-  end
-
   test "deploy errors during outside section leave remove lock" do
     invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "skip_broadcast" => false, "version" => "999" }
 
@@ -151,22 +134,24 @@ class CliMainTest < CliTestCase
   end
 
   test "rollback good version" do
+    [ "web", "workers" ].each do |role|
       SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
-      .with(:docker, :container, :ls, "--all", "--filter", "name=^app-web-123$", "--quiet")
+        .with(:docker, :container, :ls, "--filter", "name=^app-#{role}-123$", "--quiet", raise_on_non_zero_exit: false)
+        .returns("").at_least_once
+      SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
+        .with(:docker, :container, :ls, "--all", "--filter", "name=^app-#{role}-123$", "--quiet")
         .returns("version-to-rollback\n").at_least_once
       SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
-      .with(:docker, :container, :ls, "--all", "--filter", "name=^app-workers-123$", "--quiet")
+        .with(:docker, :ps, "--filter", "label=service=app", "--filter", "label=role=#{role}", "--filter", "status=running", "--latest", "--format", "\"{{.Names}}\"", "|", "grep -oE \"\\-[^-]+$\"", "|", "cut -c 2-", raise_on_non_zero_exit: false)
         .returns("version-to-rollback\n").at_least_once
       SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
-      .with(:docker, :ps, "--filter", "label=service=app", "--filter", "label=role=web", "--filter", "status=running", "--latest", "--format", "\"{{.Names}}\"", "|", "grep -oE \"\\-[^-]+$\"", "|", "cut -c 2-")
+        .with(:docker, :container, :ls, "--all", "--filter", "name=^app-#{role}-123$", "--quiet", "|", :xargs, :docker, :inspect, "--format", "'{{if .State.Health}}{{.State.Health.Status}}{{else}}{{.State.Status}}{{end}}'")
-      .returns("version-to-rollback\n").at_least_once
+        .returns("running").at_least_once # health check
-    SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
+    end
-      .with(:docker, :ps, "--filter", "label=service=app", "--filter", "label=role=workers", "--filter", "status=running", "--latest", "--format", "\"{{.Names}}\"", "|", "grep -oE \"\\-[^-]+$\"", "|", "cut -c 2-")
-      .returns("version-to-rollback\n").at_least_once
 
 
     run_command("rollback", "123", config_file: "deploy_with_accessories").tap do |output|
-      assert_match "Start version 123", output
+      assert_match "Start container with version 123", output
       assert_match "docker tag dhh/app:123 dhh/app:latest", output
       assert_match "docker start app-web-123", output
       assert_match "docker container ls --all --filter name=^app-web-version-to-rollback$ --quiet | xargs docker stop", output, "Should stop the container that was previously running"
@@ -175,11 +160,22 @@ class CliMainTest < CliTestCase
 
   test "rollback without old version" do
     Mrsk::Cli::Main.any_instance.stubs(:container_available?).returns(true)
-    SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info).with(:docker, :ps, "--filter", "label=service=app", "--filter", "label=role=web", "--filter", "status=running", "--latest", "--format", "\"{{.Names}}\"", "|", "grep -oE \"\\-[^-]+$\"", "|", "cut -c 2-").returns("").at_least_once
+
+    Mrsk::Utils::HealthcheckPoller.stubs(:sleep)
+
+    SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
+      .with(:docker, :container, :ls, "--filter", "name=^app-web-123$", "--quiet", raise_on_non_zero_exit: false)
+      .returns("").at_least_once
+    SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
+      .with(:docker, :ps, "--filter", "label=service=app", "--filter", "label=role=web", "--filter", "status=running", "--latest", "--format", "\"{{.Names}}\"", "|", "grep -oE \"\\-[^-]+$\"", "|", "cut -c 2-", raise_on_non_zero_exit: false)
+      .returns("").at_least_once
+    SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
+      .with(:docker, :container, :ls, "--all", "--filter", "name=^app-web-123$", "--quiet", "|", :xargs, :docker, :inspect, "--format", "'{{if .State.Health}}{{.State.Health.Status}}{{else}}{{.State.Status}}{{end}}'")
+      .returns("running").at_least_once # health check
 
     run_command("rollback", "123").tap do |output|
-      assert_match "Start version 123", output
+      assert_match "Start container with version 123", output
-      assert_match "docker start app-web-123", output
+      assert_match "docker start app-web-123 || docker run --detach --restart unless-stopped --name app-web-123", output
       assert_no_match "docker stop", output
     end
   end