From 718776eb72679b491a93e03eaa4c0a56ee4dfbd4 Mon Sep 17 00:00:00 2001
From: Donal McBreen <donal@basecamp.com>
Date: Wed, 23 Aug 2023 12:03:45 +0100
Subject: [PATCH] Prune healthcheck containers

If a deployment is interrupted it could leave stale healthcheck
containers around that prevent dependent images from being pruned.
---
 lib/kamal/cli/prune.rb            |  3 ++-
 lib/kamal/commands/healthcheck.rb | 10 +++-------
 lib/kamal/commands/prune.rb       | 12 ++++++++++--
 lib/kamal/configuration.rb        |  4 ++++
 test/cli/prune_test.rb            |  3 ++-
 test/commands/prune_test.rb       | 10 ++++++++--
 test/configuration_test.rb        |  4 ++++
 7 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/lib/kamal/cli/prune.rb b/lib/kamal/cli/prune.rb
index d11c39aeb..b918e788d 100644
--- a/lib/kamal/cli/prune.rb
+++ b/lib/kamal/cli/prune.rb
@@ -23,7 +23,8 @@ def containers
     mutating do
       on(KAMAL.hosts) do
         execute *KAMAL.auditor.record("Pruned containers"), verbosity: :debug
-        execute *KAMAL.prune.containers
+        execute *KAMAL.prune.app_containers
+        execute *KAMAL.prune.healthcheck_containers
       end
     end
   end
diff --git a/lib/kamal/commands/healthcheck.rb b/lib/kamal/commands/healthcheck.rb
index fa050b9cb..b68df5bba 100644
--- a/lib/kamal/commands/healthcheck.rb
+++ b/lib/kamal/commands/healthcheck.rb
@@ -7,8 +7,8 @@ def run
       "--detach",
       "--name", container_name_with_version,
       "--publish", "#{exposed_port}:#{config.healthcheck["port"]}",
-      "--label", "service=#{container_name}",
-      "-e", "KAMAL_CONTAINER_NAME=\"#{container_name}\"",
+      "--label", "service=#{config.healthcheck_service}",
+      "-e", "KAMAL_CONTAINER_NAME=\"#{config.healthcheck_service}\"",
       *web.env_args,
       *web.health_check_args(cord: false),
       *config.volume_args,
@@ -38,12 +38,8 @@ def remove
   end
 
   private
-    def container_name
-      [ "healthcheck", config.service, config.destination ].compact.join("-")
-    end
-
     def container_name_with_version
-      "#{container_name}-#{config.version}"
+      "#{config.healthcheck_service}-#{config.version}"
     end
 
     def container_id
diff --git a/lib/kamal/commands/prune.rb b/lib/kamal/commands/prune.rb
index af363e815..326e28322 100644
--- a/lib/kamal/commands/prune.rb
+++ b/lib/kamal/commands/prune.rb
@@ -13,13 +13,17 @@ def tagged_images
       "while read image tag; do docker rmi $tag; done"
   end
 
-  def containers(keep_last: 5)
+  def app_containers(keep_last: 5)
     pipe \
       docker(:ps, "-q", "-a", *service_filter, *stopped_containers_filters),
       "tail -n +#{keep_last + 1}",
       "while read container_id; do docker rm $container_id; done"
   end
 
+  def healthcheck_containers
+    docker :container, :prune, "--force", *healthcheck_service_filter
+  end
+
   private
     def stopped_containers_filters
       [ "created", "exited", "dead" ].flat_map { |status| ["--filter", "status=#{status}"] }
@@ -35,4 +39,8 @@ def active_image_list
     def service_filter
       [ "--filter", "label=service=#{config.service}" ]
     end
-end
+
+    def healthcheck_service_filter
+      [ "--filter", "label=service=#{config.healthcheck_service}" ]
+    end
+  end
diff --git a/lib/kamal/configuration.rb b/lib/kamal/configuration.rb
index af6e46c5f..d56c63f34 100644
--- a/lib/kamal/configuration.rb
+++ b/lib/kamal/configuration.rb
@@ -152,6 +152,10 @@ def healthcheck
     { "path" => "/up", "port" => 3000, "max_attempts" => 7, "exposed_port" => 3999, "cord" => "/tmp/kamal-cord" }.merge(raw_config.healthcheck || {})
   end
 
+  def healthcheck_service
+    [ "healthcheck", service, destination ].compact.join("-")
+  end
+
   def readiness_delay
     raw_config.readiness_delay || 7
   end
diff --git a/test/cli/prune_test.rb b/test/cli/prune_test.rb
index c43fba7a2..2096c6468 100644
--- a/test/cli/prune_test.rb
+++ b/test/cli/prune_test.rb
@@ -18,7 +18,8 @@ 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
-    end
+      assert_match /docker container prune --force --filter label=service=healthcheck-app on 1.1.1.\d/, output
+     end
   end
 
   private
diff --git a/test/commands/prune_test.rb b/test/commands/prune_test.rb
index 939497cd8..c8b9caf43 100644
--- a/test/commands/prune_test.rb
+++ b/test/commands/prune_test.rb
@@ -20,10 +20,16 @@ class CommandsPruneTest < ActiveSupport::TestCase
       new_command.tagged_images.join(" ")
   end
 
-  test "containers" do
+  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.containers.join(" ")
+      new_command.app_containers.join(" ")
+  end
+
+  test "healthcheck containers" do
+    assert_equal \
+      "docker container prune --force --filter label=service=healthcheck-app",
+      new_command.healthcheck_containers.join(" ")
   end
 
   private
diff --git a/test/configuration_test.rb b/test/configuration_test.rb
index f21fdc8d9..27818475d 100644
--- a/test/configuration_test.rb
+++ b/test/configuration_test.rb
@@ -124,6 +124,10 @@ class ConfigurationTest < ActiveSupport::TestCase
     assert_equal "app-missing", @config.service_with_version
   end
 
+  test "healthcheck service" do
+    assert_equal "healthcheck-app", @config.healthcheck_service
+  end
+
   test "env with missing secret" do
     assert_raises(KeyError) do
       config = Kamal::Configuration.new(@deploy.tap { |c| c.merge!({