Remove old images after building new one

.env

@@ -9,4 +9,5 @@ CYBER_DOJO_DIFFER_PORT=4567
 CYBER_DOJO_NGINX_PORT=80
 CYBER_DOJO_RUNNER_PORT=4597
 CYBER_DOJO_SAVER_PORT=4537
+CYBER_DOJO_SPOOLER_PORT=4539
 CYBER_DOJO_WEB_PORT=3000

bin/build_image.sh

@@ -60,8 +60,6 @@ build_image()
   export DOCKER_DEFAULT_PLATFORM=linux/amd64
 
   if [ "${CI:-}" != 'true' ]; then
-    # In CI workflow, don't remove image pulled in the 'Download docker image' CI workflow jobs.
-    remove_old_images
     # Locally, client and server tests both need a server
     docker --log-level=ERROR compose build server
   fi
@@ -81,9 +79,16 @@ build_image()
     exit_non_zero
   fi
 
-  # Tag image-name for local development where differs name comes from echo-versioner-env-vars
   if [ "${type}" == 'server' ]; then
+    # Create latest tag for image build cache
+    docker tag "${image_name}" "${CYBER_DOJO_DIFFER_IMAGE}:latest"
+    # Tag image-name for local development where differs name comes from echo-versioner-env-vars
     docker tag "${image_name}" "cyberdojo/differ:${CYBER_DOJO_DIFFER_TAG}"
+    # After tagging, so removing an earlier build's tags takes its last tag with
+    # them and the image itself goes, rather than being left dangling when
+    # :latest moves to this build. check_args rejects 'server' inside CI, so the
+    # image pulled by the 'Download docker image' CI job is never at risk here.
+    remove_old_images
     echo
     echo "  echo CYBER_DOJO_DIFFER_SHA=${CYBER_DOJO_DIFFER_SHA}"
     echo "  echo CYBER_DOJO_DIFFER_TAG=${CYBER_DOJO_DIFFER_TAG}"

bin/lib.sh

@@ -92,22 +92,32 @@ strip_known_warning()
 remove_old_images()
 {
   echo Removing old images
-  local -r dil=$(docker image ls --format "{{.Repository}}:{{.Tag}}" | grep differ)
-  remove_all_but_latest "${dil}" "${CYBER_DOJO_DIFFER_CLIENT_IMAGE}"
-  remove_all_but_latest "${dil}" "${CYBER_DOJO_DIFFER_IMAGE}"
-  remove_all_but_latest "${dil}" cyberdojo/differ
+  # A stopped container still references its image, so clear them first to let
+  # the removals below actually take effect.
+  docker container prune --force
+  # grep exits non-zero when the machine holds no differ image, eg one whose
+  # images have just been cleared, so an empty list must not end the build.
+  local -r dil=$(docker image ls --format "{{.Repository}}:{{.Tag}}" | grep differ || true)
+  remove_all_but_current "${dil}" "${CYBER_DOJO_DIFFER_CLIENT_IMAGE}"
+  remove_all_but_current "${dil}" "${CYBER_DOJO_DIFFER_IMAGE}"
+  remove_all_but_current "${dil}" cyberdojo/differ
 }
 
-remove_all_but_latest()
+# Keeps :latest, which preserves the image-layer build cache, and this commit's
+# tag, which names the build just made. Every older tag goes, and an earlier
+# build whose last tag was one of those goes with it.
+remove_all_but_current()
 {
-  # Keep latest in the cache
   local -r docker_image_ls="${1}"
   local -r name="${2}"
-  docker container prune --force
-  for image_name in $(echo "${docker_image_ls}" | grep "${name}:")
+  # Its own name, not image_name: bash locals are dynamically scoped, and
+  # build_image declares image_name readonly before calling this.
+  local tagged_name
+  for tagged_name in $(echo "${docker_image_ls}" | grep "${name}:" || true)
   do
-    if [ "${image_name}" != "${name}:latest" ]; then
-      docker image rm --force "${image_name}" || echo "  skipped ${image_name} (in use)"
+    if [ "${tagged_name}" != "${name}:latest" ] \
+    && [ "${tagged_name}" != "${name}:${CYBER_DOJO_DIFFER_TAG}" ]; then
+      docker image rm --force "${tagged_name}" || echo "  skipped ${tagged_name} (in use)"
     fi
   done
 }