From 371f98d67f543f72509ca0f6c1d46d92660c4eca Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Wed, 22 Feb 2023 19:04:23 +0100 Subject: [PATCH] Start before stopping and longer timeouts --- lib/mrsk/cli/app.rb | 8 ++++++-- lib/mrsk/cli/healthcheck.rb | 2 +- lib/mrsk/cli/main.rb | 7 +++++-- lib/mrsk/commands/app.rb | 10 ++++++++-- lib/mrsk/configuration/role.rb | 2 +- test/cli/app_test.rb | 24 +++++++++++++++++++----- test/commands/app_test.rb | 6 +++--- test/configuration/role_test.rb | 4 ++-- 8 files changed, 45 insertions(+), 18 deletions(-) diff --git a/lib/mrsk/cli/app.rb b/lib/mrsk/cli/app.rb index f4ff7c67..0f16e5a7 100644 --- a/lib/mrsk/cli/app.rb +++ b/lib/mrsk/cli/app.rb @@ -10,13 +10,17 @@ class Mrsk::Cli::App < Mrsk::Cli::Base execute *MRSK.auditor.record("Booted app version #{version}"), verbosity: :debug begin - execute *MRSK.app.stop, raise_on_non_zero_exit: false + old_version = capture_with_info(*MRSK.app.current_running_version).strip execute *MRSK.app.run(role: role.name) + sleep 10 + execute *MRSK.app.stop(version: old_version), raise_on_non_zero_exit: false if old_version.present? + rescue SSHKit::Command::Failed => e if e.message =~ /already in use/ - error "Rebooting container with same version already deployed on #{host}" + error "Rebooting container with same version #{version} already deployed on #{host}" execute *MRSK.auditor.record("Rebooted app version #{version}"), verbosity: :debug + execute *MRSK.app.stop(version: version) execute *MRSK.app.remove_container(version: version) execute *MRSK.app.run(role: role.name) else diff --git a/lib/mrsk/cli/healthcheck.rb b/lib/mrsk/cli/healthcheck.rb index eb7bb9ab..3fda7754 100644 --- a/lib/mrsk/cli/healthcheck.rb +++ b/lib/mrsk/cli/healthcheck.rb @@ -1,5 +1,5 @@ class Mrsk::Cli::Healthcheck < Mrsk::Cli::Base - MAX_ATTEMPTS = 5 + MAX_ATTEMPTS = 7 class HealthcheckError < StandardError; end diff --git a/lib/mrsk/cli/main.rb b/lib/mrsk/cli/main.rb index 352e7f30..52020092 100644 --- a/lib/mrsk/cli/main.rb +++ b/lib/mrsk/cli/main.rb @@ -55,11 +55,14 @@ class Mrsk::Cli::Main < Mrsk::Cli::Base MRSK.version = version if container_name_available?(MRSK.config.service_with_version) - say "Stop current version, then start version #{version}...", :magenta + say "Start version #{version}, then stop the old version...", :magenta on(MRSK.hosts) do |host| - execute *MRSK.app.stop, raise_on_non_zero_exit: false + old_version = capture_with_info(*MRSK.app.current_running_version).strip.presence + execute *MRSK.app.start + sleep 10 + execute *MRSK.app.stop(version: old_version), raise_on_non_zero_exit: false end audit_broadcast "Rolled back app to version #{version}" diff --git a/lib/mrsk/commands/app.rb b/lib/mrsk/commands/app.rb index 4c8f0c7e..4b1de91a 100644 --- a/lib/mrsk/commands/app.rb +++ b/lib/mrsk/commands/app.rb @@ -18,8 +18,10 @@ class Mrsk::Commands::App < Mrsk::Commands::Base docker :start, service_with_version end - def stop - pipe current_container_id, xargs(docker(:stop)) + def stop(version: nil) + pipe \ + version ? container_id_for_version(version) : current_container_id, + xargs(docker(:stop)) end def info @@ -132,6 +134,10 @@ class Mrsk::Commands::App < Mrsk::Commands::Base end end + def container_id_for_version(version) + container_id_for(container_name: service_with_version(version)) + end + def service_filter [ "--filter", "label=service=#{config.service}" ] end diff --git a/lib/mrsk/configuration/role.rb b/lib/mrsk/configuration/role.rb index 628299c0..1795649b 100644 --- a/lib/mrsk/configuration/role.rb +++ b/lib/mrsk/configuration/role.rb @@ -61,7 +61,7 @@ class Mrsk::Configuration::Role "traefik.http.routers.#{config.service}.rule" => "PathPrefix(`/`)", "traefik.http.services.#{config.service}.loadbalancer.healthcheck.path" => config.healthcheck["path"], "traefik.http.services.#{config.service}.loadbalancer.healthcheck.interval" => "1s", - "traefik.http.middlewares.#{config.service}.retry.attempts" => "3", + "traefik.http.middlewares.#{config.service}.retry.attempts" => "5", "traefik.http.middlewares.#{config.service}.retry.initialinterval" => "500ms" } else diff --git a/test/cli/app_test.rb b/test/cli/app_test.rb index f8b5da05..76f59a80 100644 --- a/test/cli/app_test.rb +++ b/test/cli/app_test.rb @@ -2,7 +2,16 @@ require_relative "cli_test_case" class CliAppTest < CliTestCase test "boot" do - assert_match /Running docker run --detach --restart unless-stopped/, run_command("boot") + # Stub current version fetch + SSHKit::Backend::Abstract.any_instance.stubs(:capture) + .returns("999") # new version + .then + .returns("123") # old version + + run_command("boot").tap do |output| + assert_match /docker run --detach --restart unless-stopped/, output + assert_match /docker container ls --all --filter name=app-123 --quiet | xargs docker stop/, output + end end test "boot will reboot if same version is already running" do @@ -11,12 +20,17 @@ class CliAppTest < CliTestCase # Prevent expected failures from outputting to terminal Thread.report_on_exception = false - MRSK.app.stubs(:run).raises(SSHKit::Command::Failed.new("already in use")).then.returns([ :docker, :run ]) + MRSK.app.stubs(:run) + .raises(SSHKit::Command::Failed.new("already in use")) + .then + .raises(SSHKit::Command::Failed.new("already in use")) + .then + .returns([ :docker, :run ]) run_command("boot").tap do |output| - assert_match /Rebooting container with same version already deployed/, output # Can't start what's already running - assert_match /docker ps --quiet --filter label=service=app \| xargs docker stop/, output # Stop what's running - assert_match /docker container ls --all --filter name=app-999 --quiet \| xargs docker container rm/, output # Remove old container + assert_match /Rebooting container with same version 999 already deployed/, output # Can't start what's already running + assert_match /docker container ls --all --filter name=app-999 --quiet | xargs docker container rm/, output # Stop old running + assert_match /docker container ls --all --filter name=app-999 --quiet | xargs docker container rm/, output # Remove old container assert_match /docker run/, output # Start new container end ensure diff --git a/test/commands/app_test.rb b/test/commands/app_test.rb index adce57ae..a62ab80c 100644 --- a/test/commands/app_test.rb +++ b/test/commands/app_test.rb @@ -14,7 +14,7 @@ class CommandsAppTest < ActiveSupport::TestCase test "run" do assert_equal \ - "docker run --detach --restart unless-stopped --log-opt max-size=10m --name app-999 -e RAILS_MASTER_KEY=\"456\" --label service=\"app\" --label role=\"web\" --label traefik.http.routers.app.rule=\"PathPrefix(\\`/\\`)\" --label traefik.http.services.app.loadbalancer.healthcheck.path=\"/up\" --label traefik.http.services.app.loadbalancer.healthcheck.interval=\"1s\" --label traefik.http.middlewares.app.retry.attempts=\"3\" --label traefik.http.middlewares.app.retry.initialinterval=\"500ms\" dhh/app:999", + "docker run --detach --restart unless-stopped --log-opt max-size=10m --name app-999 -e RAILS_MASTER_KEY=\"456\" --label service=\"app\" --label role=\"web\" --label traefik.http.routers.app.rule=\"PathPrefix(\\`/\\`)\" --label traefik.http.services.app.loadbalancer.healthcheck.path=\"/up\" --label traefik.http.services.app.loadbalancer.healthcheck.interval=\"1s\" --label traefik.http.middlewares.app.retry.attempts=\"5\" --label traefik.http.middlewares.app.retry.initialinterval=\"500ms\" dhh/app:999", @app.run.join(" ") end @@ -22,7 +22,7 @@ class CommandsAppTest < ActiveSupport::TestCase @config[:volumes] = ["/local/path:/container/path" ] assert_equal \ - "docker run --detach --restart unless-stopped --log-opt max-size=10m --name app-999 -e RAILS_MASTER_KEY=\"456\" --volume /local/path:/container/path --label service=\"app\" --label role=\"web\" --label traefik.http.routers.app.rule=\"PathPrefix(\\`/\\`)\" --label traefik.http.services.app.loadbalancer.healthcheck.path=\"/up\" --label traefik.http.services.app.loadbalancer.healthcheck.interval=\"1s\" --label traefik.http.middlewares.app.retry.attempts=\"3\" --label traefik.http.middlewares.app.retry.initialinterval=\"500ms\" dhh/app:999", + "docker run --detach --restart unless-stopped --log-opt max-size=10m --name app-999 -e RAILS_MASTER_KEY=\"456\" --volume /local/path:/container/path --label service=\"app\" --label role=\"web\" --label traefik.http.routers.app.rule=\"PathPrefix(\\`/\\`)\" --label traefik.http.services.app.loadbalancer.healthcheck.path=\"/up\" --label traefik.http.services.app.loadbalancer.healthcheck.interval=\"1s\" --label traefik.http.middlewares.app.retry.attempts=\"5\" --label traefik.http.middlewares.app.retry.initialinterval=\"500ms\" dhh/app:999", @app.run.join(" ") end @@ -30,7 +30,7 @@ class CommandsAppTest < ActiveSupport::TestCase @config[:healthcheck] = { "path" => "/healthz" } assert_equal \ - "docker run --detach --restart unless-stopped --log-opt max-size=10m --name app-999 -e RAILS_MASTER_KEY=\"456\" --label service=\"app\" --label role=\"web\" --label traefik.http.routers.app.rule=\"PathPrefix(\\`/\\`)\" --label traefik.http.services.app.loadbalancer.healthcheck.path=\"/healthz\" --label traefik.http.services.app.loadbalancer.healthcheck.interval=\"1s\" --label traefik.http.middlewares.app.retry.attempts=\"3\" --label traefik.http.middlewares.app.retry.initialinterval=\"500ms\" dhh/app:999", + "docker run --detach --restart unless-stopped --log-opt max-size=10m --name app-999 -e RAILS_MASTER_KEY=\"456\" --label service=\"app\" --label role=\"web\" --label traefik.http.routers.app.rule=\"PathPrefix(\\`/\\`)\" --label traefik.http.services.app.loadbalancer.healthcheck.path=\"/healthz\" --label traefik.http.services.app.loadbalancer.healthcheck.interval=\"1s\" --label traefik.http.middlewares.app.retry.attempts=\"5\" --label traefik.http.middlewares.app.retry.initialinterval=\"500ms\" dhh/app:999", @app.run.join(" ") end diff --git a/test/configuration/role_test.rb b/test/configuration/role_test.rb index b1916a52..1017221f 100644 --- a/test/configuration/role_test.rb +++ b/test/configuration/role_test.rb @@ -42,7 +42,7 @@ class ConfigurationRoleTest < ActiveSupport::TestCase end test "special label args for web" do - assert_equal [ "--label", "service=\"app\"", "--label", "role=\"web\"", "--label", "traefik.http.routers.app.rule=\"PathPrefix(\\`/\\`)\"", "--label", "traefik.http.services.app.loadbalancer.healthcheck.path=\"/up\"", "--label", "traefik.http.services.app.loadbalancer.healthcheck.interval=\"1s\"", "--label", "traefik.http.middlewares.app.retry.attempts=\"3\"", "--label", "traefik.http.middlewares.app.retry.initialinterval=\"500ms\""], @config.role(:web).label_args + assert_equal [ "--label", "service=\"app\"", "--label", "role=\"web\"", "--label", "traefik.http.routers.app.rule=\"PathPrefix(\\`/\\`)\"", "--label", "traefik.http.services.app.loadbalancer.healthcheck.path=\"/up\"", "--label", "traefik.http.services.app.loadbalancer.healthcheck.interval=\"1s\"", "--label", "traefik.http.middlewares.app.retry.attempts=\"5\"", "--label", "traefik.http.middlewares.app.retry.initialinterval=\"500ms\""], @config.role(:web).label_args end test "custom labels" do @@ -66,7 +66,7 @@ class ConfigurationRoleTest < ActiveSupport::TestCase c[:servers]["beta"] = { "traefik" => "true", "hosts" => [ "1.1.1.5" ] } }) - assert_equal [ "--label", "service=\"app\"", "--label", "role=\"beta\"", "--label", "traefik.http.routers.app.rule=\"PathPrefix(\\`/\\`)\"", "--label", "traefik.http.services.app.loadbalancer.healthcheck.path=\"/up\"", "--label", "traefik.http.services.app.loadbalancer.healthcheck.interval=\"1s\"", "--label", "traefik.http.middlewares.app.retry.attempts=\"3\"", "--label", "traefik.http.middlewares.app.retry.initialinterval=\"500ms\"" ], config.role(:beta).label_args + assert_equal [ "--label", "service=\"app\"", "--label", "role=\"beta\"", "--label", "traefik.http.routers.app.rule=\"PathPrefix(\\`/\\`)\"", "--label", "traefik.http.services.app.loadbalancer.healthcheck.path=\"/up\"", "--label", "traefik.http.services.app.loadbalancer.healthcheck.interval=\"1s\"", "--label", "traefik.http.middlewares.app.retry.attempts=\"5\"", "--label", "traefik.http.middlewares.app.retry.initialinterval=\"500ms\"" ], config.role(:beta).label_args end test "env overwritten by role" do