From 079d9538bbef4bc09e69ad77cdb8269c70522445 Mon Sep 17 00:00:00 2001
From: Donal McBreen <donal@basecamp.com>
Date: Wed, 31 May 2023 09:43:50 +0100
Subject: [PATCH] Improve image pruning robustness

If you different images with the same git SHA, on the second deploy the
tag is moved and the first image becomes untagged. It may however still
be attached to an existing container.

To handle this:
1. Initially prune dangling images - this will remove any untagged
images that are not attached to an existing image
2. Then filter out the untagged images when deleting tagged images - any
that remain will be attached to a container.

The second issue is that `docker container ls -a --format '{{.Image}}`
will sometimes return the image id rather than a tag. This means that
the image doesn't get filtered out when we grep to remove the active
images.

To fix that we'll grep against both the image id and repo:tag.
---
 lib/mrsk/cli/prune.rb       |  3 ++-
 lib/mrsk/commands/prune.rb  | 15 +++++++++++----
 test/cli/prune_test.rb      |  3 ++-
 test/commands/prune_test.rb | 12 +++++++++---
 4 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/lib/mrsk/cli/prune.rb b/lib/mrsk/cli/prune.rb
index bcfdd5bf..de227e08 100644
--- a/lib/mrsk/cli/prune.rb
+++ b/lib/mrsk/cli/prune.rb
@@ -12,7 +12,8 @@ class Mrsk::Cli::Prune < Mrsk::Cli::Base
     with_lock do
       on(MRSK.hosts) do
         execute *MRSK.auditor.record("Pruned images"), verbosity: :debug
-        execute *MRSK.prune.images
+        execute *MRSK.prune.dangling_images
+        execute *MRSK.prune.tagged_images
       end
     end
   end
diff --git a/lib/mrsk/commands/prune.rb b/lib/mrsk/commands/prune.rb
index 3b68fe3f..92f80bab 100644
--- a/lib/mrsk/commands/prune.rb
+++ b/lib/mrsk/commands/prune.rb
@@ -2,11 +2,15 @@ require "active_support/duration"
 require "active_support/core_ext/numeric/time"
 
 class Mrsk::Commands::Prune < Mrsk::Commands::Base
-  def images
+  def dangling_images
+    docker :image, :prune, "--force", "--filter", "label=service=#{config.service}", "--filter", "dangling=true"
+  end
+
+  def tagged_images
     pipe \
-      docker(:image, :ls, *service_filter, "--format", "'{{.Repository}}:{{.Tag}}'"),
+      docker(:image, :ls, *service_filter, "--format", "'{{.ID}} {{.Repository}}:{{.Tag}}'"),
       "grep -v -w \"#{active_image_list}\"",
-      "while read tag; do docker rmi $tag; done"
+      "while read image tag; do docker rmi $tag; done"
   end
 
   def containers(keep_last: 5)
@@ -22,7 +26,10 @@ class Mrsk::Commands::Prune < Mrsk::Commands::Base
     end
 
     def active_image_list
-      "$(docker container ls -a --format '{{.Image}}\\|' --filter label=service=#{config.service} | tr -d '\\n')#{config.latest_image}"
+      # Pull the images that are used by any containers
+      # Append repo:latest - to avoid deleting the latest tag
+      # Append repo:<none> - to avoid deleting dangling images that are in use. Unused dangling images are deleted separately
+      "$(docker container ls -a --format '{{.Image}}\\|' --filter label=service=#{config.service} | tr -d '\\n')#{config.latest_image}\\|#{config.repository}:<none>"
     end
 
     def service_filter
diff --git a/test/cli/prune_test.rb b/test/cli/prune_test.rb
index 85cdf159..fdf6c66f 100644
--- a/test/cli/prune_test.rb
+++ b/test/cli/prune_test.rb
@@ -10,7 +10,8 @@ class CliPruneTest < CliTestCase
 
   test "images" do
     run_command("images").tap do |output|
-      assert_match "docker image ls --filter label=service=app --format '{{.Repository}}:{{.Tag}}' | grep -v -w \"$(docker container ls -a --format '{{.Image}}\\|' --filter label=service=app | tr -d '\\n')dhh/app:latest\" | while read tag; do docker rmi $tag; done on 1.1.1.", output
+      assert_match "docker image prune --force --filter label=service=app --filter dangling=true on 1.1.1.", output
+      assert_match "docker image ls --filter label=service=app --format '{{.ID}} {{.Repository}}:{{.Tag}}' | grep -v -w \"$(docker container ls -a --format '{{.Image}}\\|' --filter label=service=app | tr -d '\\n')dhh/app:latest\\|dhh/app:<none>\" | while read image tag; do docker rmi $tag; done on 1.1.1.", output
     end
   end
 
diff --git a/test/commands/prune_test.rb b/test/commands/prune_test.rb
index f4025893..7fbb24a5 100644
--- a/test/commands/prune_test.rb
+++ b/test/commands/prune_test.rb
@@ -8,10 +8,16 @@ class CommandsPruneTest < ActiveSupport::TestCase
     }
   end
 
-  test "images" do
+  test "dangling images" do
     assert_equal \
-      "docker image ls --filter label=service=app --format '{{.Repository}}:{{.Tag}}' | grep -v -w \"$(docker container ls -a --format '{{.Image}}\\|' --filter label=service=app | tr -d '\\n')dhh/app:latest\" | while read tag; do docker rmi $tag; done",
-      new_command.images.join(" ")
+      "docker image prune --force --filter label=service=app --filter dangling=true",
+      new_command.dangling_images.join(" ")
+  end
+
+  test "tagged images" do
+    assert_equal \
+      "docker image ls --filter label=service=app --format '{{.ID}} {{.Repository}}:{{.Tag}}' | grep -v -w \"$(docker container ls -a --format '{{.Image}}\\|' --filter label=service=app | tr -d '\\n')dhh/app:latest\\|dhh/app:<none>\" | while read image tag; do docker rmi $tag; done",
+      new_command.tagged_images.join(" ")
   end
 
   test "containers" do