fix(prune): scope container pruning to a role and destination

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

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

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

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

test/cli/prune_test.rb

@@ -17,20 +17,27 @@ 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
       run_command("containers", "--retain", "0")
     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

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