diff --git a/lib/kamal/cli/prune.rb b/lib/kamal/cli/prune.rb index 4a2e44f24..7532ade82 100644 --- a/lib/kamal/cli/prune.rb +++ b/lib/kamal/cli/prune.rb @@ -18,16 +18,19 @@ def images end end - desc "containers", "Prune all stopped containers, except the last n (default 5)" - option :retain, type: :numeric, default: nil, desc: "Number of containers to retain" + desc "containers", "Prune all stopped containers, except the last n per role (default 5)" + option :retain, type: :numeric, default: nil, desc: "Number of containers to retain per role" def containers retain = options.fetch(:retain, KAMAL.config.retain_containers) raise "retain must be at least 1" if retain < 1 modify(lock: true) do - on(KAMAL.hosts) do + on(KAMAL.hosts) do |host| execute *KAMAL.auditor.record("Pruned containers"), verbosity: :debug - execute *KAMAL.prune.app_containers(retain: retain) + + KAMAL.roles_on(host).each do |role| + execute *KAMAL.prune.app_containers(retain: retain, role: role) + end end end end diff --git a/lib/kamal/commands/prune.rb b/lib/kamal/commands/prune.rb index 30b6eafd3..0b8b40cff 100644 --- a/lib/kamal/commands/prune.rb +++ b/lib/kamal/commands/prune.rb @@ -13,9 +13,15 @@ def tagged_images "while read image tag; do docker rmi $tag; done" end - def app_containers(retain:) + # Scoped to one role so a busy sibling role cannot push another role's newest + # container past the retain window. That matters beyond disk hygiene: a + # container kamal-proxy has put to sleep is `exited`, so it is a removal + # candidate, and once it is gone every wake 404s. With `retain >= 1` a role's + # newest container always survives, and the slept one is always the newest — + # sleeping happens to the current release. + def app_containers(retain:, role:) pipe \ - docker(:ps, "-q", "-a", *service_filter, *stopped_containers_filters), + docker(:ps, "-q", "-a", *service_filter, *destination_filter, *role_filter(role), *stopped_containers_filters), "tail -n +#{retain + 1}", "while read container_id; do docker rm $container_id; done" end @@ -35,4 +41,12 @@ def active_image_list def service_filter [ "--filter", "label=service=#{config.service}" ] end + + def destination_filter + [ "--filter", "label=destination=#{config.destination}" ] + end + + def role_filter(role) + [ "--filter", "label=role=#{role}" ] + end end diff --git a/lib/kamal/configuration/docs/configuration.yml b/lib/kamal/configuration/docs/configuration.yml index 81d2c42fa..6872417c8 100644 --- a/lib/kamal/configuration/docs/configuration.yml +++ b/lib/kamal/configuration/docs/configuration.yml @@ -136,7 +136,8 @@ allow_empty_roles: false # Retain containers # -# How many old containers and images we retain, defaults to 5: +# How many old containers we retain per role, and how many images we retain, +# defaults to 5: retain_containers: 3 # Minimum version diff --git a/test/cli/proxy_test.rb b/test/cli/proxy_test.rb index ca50b24ea..a83ddb222 100644 --- a/test/cli/proxy_test.rb +++ b/test/cli/proxy_test.rb @@ -247,7 +247,7 @@ class CliProxyTest < CliTestCase assert_match "docker container ls --all --filter 'name=^app-web-12345678$' --quiet | xargs docker stop", output assert_match "docker tag dhh/app:latest dhh/app:latest", output assert_match "/usr/bin/env mkdir -p .kamal", output - assert_match "docker ps -q -a --filter label=service=app --filter status=created --filter status=exited --filter status=dead | tail -n +6 | while read container_id; do docker rm $container_id; done", output + assert_match "docker ps -q -a --filter label=service=app --filter label=destination= --filter label=role=web --filter status=created --filter status=exited --filter status=dead | tail -n +6 | while read container_id; do docker rm $container_id; done", output assert_match "docker image prune --force --filter label=service=app", output assert_match "Upgraded proxy on 1.1.1.1,1.1.1.2,1.1.1.3,1.1.1.4", output end diff --git a/test/cli/prune_test.rb b/test/cli/prune_test.rb index bb4ced51a..3146cc9af 100644 --- a/test/cli/prune_test.rb +++ b/test/cli/prune_test.rb @@ -17,11 +17,11 @@ class CliPruneTest < CliTestCase test "containers" do run_command("containers").tap do |output| - assert_match /docker ps -q -a --filter label=service=app --filter status=created --filter status=exited --filter status=dead | tail -n +6 | while read container_id; do docker rm $container_id; done on 1.1.1.\d/, output + assert_match /docker ps -q -a --filter label=service=app --filter label=destination= --filter label=role=web --filter status=created --filter status=exited --filter status=dead | tail -n \+6 | while read container_id; do docker rm \$container_id; done on 1.1.1.\d/, output end run_command("containers", "--retain", "10").tap do |output| - assert_match /docker ps -q -a --filter label=service=app --filter status=created --filter status=exited --filter status=dead | tail -n +11 | while read container_id; do docker rm $container_id; done on 1.1.1.\d/, output + assert_match /docker ps -q -a --filter label=service=app --filter label=destination= --filter label=role=web --filter status=created --filter status=exited --filter status=dead | tail -n \+11 | while read container_id; do docker rm \$container_id; done on 1.1.1.\d/, output end assert_raises(RuntimeError, "retain must be at least 1") do @@ -29,8 +29,15 @@ class CliPruneTest < CliTestCase end end + test "containers prunes every role on the host separately" do + run_command("containers", config_file: "test/fixtures/deploy_with_roles.yml").tap do |output| + assert_match "--filter label=role=web --filter status=created", output + assert_match "--filter label=role=workers --filter status=created", output + end + end + private - def run_command(*command) - stdouted { Kamal::Cli::Prune.start([ *command, "-c", "test/fixtures/deploy_with_accessories.yml" ]) } + def run_command(*command, config_file: "test/fixtures/deploy_with_accessories.yml") + stdouted { Kamal::Cli::Prune.start([ *command, "-c", config_file ]) } end end diff --git a/test/commands/prune_test.rb b/test/commands/prune_test.rb index 50e852bcb..22e8f0ab3 100644 --- a/test/commands/prune_test.rb +++ b/test/commands/prune_test.rb @@ -22,16 +22,37 @@ class CommandsPruneTest < ActiveSupport::TestCase test "app containers" do assert_equal \ - "docker ps -q -a --filter label=service=app --filter status=created --filter status=exited --filter status=dead | tail -n +6 | while read container_id; do docker rm $container_id; done", - new_command.app_containers(retain: 5).join(" ") + "docker ps -q -a --filter label=service=app --filter label=destination= --filter label=role=web --filter status=created --filter status=exited --filter status=dead | tail -n +6 | while read container_id; do docker rm $container_id; done", + new_command.app_containers(retain: 5, role: role(:web)).join(" ") assert_equal \ - "docker ps -q -a --filter label=service=app --filter status=created --filter status=exited --filter status=dead | tail -n +4 | while read container_id; do docker rm $container_id; done", - new_command.app_containers(retain: 3).join(" ") + "docker ps -q -a --filter label=service=app --filter label=destination= --filter label=role=web --filter status=created --filter status=exited --filter status=dead | tail -n +4 | while read container_id; do docker rm $container_id; done", + new_command.app_containers(retain: 3, role: role(:web)).join(" ") + end + + test "app containers are scoped to the role, so a sibling role's deploys can't push a slept container past the retain window" do + @config[:servers] = { "web" => [ "1.1.1.1" ], "workers" => [ "1.1.1.2" ] } + + assert_match "--filter label=role=workers", new_command.app_containers(retain: 5, role: role(:workers)).join(" ") + assert_no_match(/--filter label=role=web /, new_command.app_containers(retain: 5, role: role(:workers)).join(" ")) + end + + test "app containers are scoped to the destination" do + assert_equal \ + "docker ps -q -a --filter label=service=app --filter label=destination=staging --filter label=role=web --filter status=created --filter status=exited --filter status=dead | tail -n +6 | while read container_id; do docker rm $container_id; done", + new_command(destination: "staging").app_containers(retain: 5, role: role(:web, destination: "staging")).join(" ") end private - def new_command - Kamal::Commands::Prune.new(Kamal::Configuration.new(@config, version: "123")) + def new_command(destination: nil) + Kamal::Commands::Prune.new(config(destination: destination)) + end + + def config(destination: nil) + Kamal::Configuration.new(@config, version: "123", destination: destination) + end + + def role(name, destination: nil) + config(destination: destination).role(name) end end