Remove old images by tag, and test the step after a deploy (closes #216)

Old images are now removed by their upaas-<app>:<N> tag, without force,
so Docker deletes an image only when no other tag (such as another app's
build of the same source) and no container still uses it. Removing by ID
with force could delete another app's rollback image.

The step after a deploy (record the new image, keep the replaced one for
rollback, remove the rest) is its own function, tested against a fake
Docker API, so the test fails if the removal is dropped or runs before
the images are updated.

Model: opus-5-5

TODO.md

@@ -20,6 +20,9 @@ regress.
 
 # Completed Steps
 
+- 2026-09-23: After a successful deploy, upaas removes the app's images other
+  than the running one and the one rollback would use, together with the
+  untagged images they were built on (#216).
 - 2026-09-23: The git clone container is now removed together with its anonymous
   volume (the `alpine/git` image declares one at `/git`), so a deploy no longer
   leaves a Docker volume behind (#215).

internal/docker/client.go

@@ -537,6 +537,55 @@ func (c *Client) RemoveImage(ctx context.Context, imageID ImageID) error {
 	return nil
 }
 
+// ListImageTags returns the tags in the given repository, such as
+// "upaas-myapp:12" in "upaas-myapp", each with the ID of its image.
+// Tags the same image has in other repositories are left out.
+func (c *Client) ListImageTags(
+	ctx context.Context,
+	repository string,
+) (map[string]ImageID, error) {
+	if c.docker == nil {
+		return nil, ErrNotConnected
+	}
+
+	images, err := c.docker.ImageList(ctx, image.ListOptions{
+		Filters: filters.NewArgs(filters.Arg("reference", repository)),
+	})
+	if err != nil {
+		return nil, fmt.Errorf("failed to list images: %w", err)
+	}
+
+	tags := make(map[string]ImageID)
+
+	for _, img := range images {
+		for _, tag := range img.RepoTags {
+			if strings.HasPrefix(tag, repository+":") {
+				tags[tag] = ImageID(img.ID)
+			}
+		}
+	}
+
+	return tags, nil
+}
+
+// RemoveImageTag removes a tag such as "upaas-myapp:12", without force.
+// Docker then deletes the image, and the untagged images it was built on,
+// only if no other tag and no container still uses it.
+func (c *Client) RemoveImageTag(ctx context.Context, tag string) error {
+	if c.docker == nil {
+		return ErrNotConnected
+	}
+
+	_, err := c.docker.ImageRemove(ctx, tag, image.RemoveOptions{
+		PruneChildren: true,
+	})
+	if err != nil && !client.IsErrNotFound(err) {
+		return fmt.Errorf("failed to remove image tag %s: %w", tag, err)
+	}
+
+	return nil
+}
+
 func (c *Client) performBuild(
 	ctx context.Context,
 	opts BuildImageOptions,

internal/service/deploy/deploy.go

@@ -9,8 +9,10 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"maps"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 	"sync"
 	"time"
@@ -524,12 +526,7 @@ func (svc *Service) runBuildAndDeploy(
 		return err
 	}
 
-	// Save current image as previous before updating to new one
+	err = svc.recordDeployedImage(bgCtx, app, deployment, imageID)
-	if app.ImageID.Valid && app.ImageID.String != "" {
-		app.PreviousImageID = app.ImageID
-	}
-
-	err = svc.updateAppRunning(bgCtx, app, imageID)
 	if err != nil {
 		return err
 	}
@@ -714,6 +711,68 @@ func (svc *Service) checkCancelled(
 	return ErrDeployCancelled
 }
 
+// recordDeployedImage runs once the new container has started: it makes
+// imageID the app's current image, keeps the replaced one as the previous
+// image for Rollback, and then removes the app's other images.
+func (svc *Service) recordDeployedImage(
+	ctx context.Context,
+	app *models.App,
+	deployment *models.Deployment,
+	imageID docker.ImageID,
+) error {
+	// Save current image as previous before updating to new one
+	if app.ImageID.Valid && app.ImageID.String != "" {
+		app.PreviousImageID = app.ImageID
+	}
+
+	err := svc.updateAppRunning(ctx, app, imageID)
+	if err != nil {
+		return err
+	}
+
+	svc.removeUnusedImages(ctx, app, deployment)
+
+	return nil
+}
+
+// removeUnusedImages removes the app's tags (upaas-<app>:<deployment>, set by
+// buildImage) except those of the image the running container uses and the
+// one Rollback would start. Docker deletes an image only once no other tag,
+// such as another app's, and no container still uses it.
+func (svc *Service) removeUnusedImages(
+	ctx context.Context,
+	app *models.App,
+	deployment *models.Deployment,
+) {
+	tags, err := svc.docker.ListImageTags(ctx, "upaas-"+app.Name)
+	if err != nil {
+		svc.log.Error("failed to list app images", "error", err, "app", app.Name)
+
+		return
+	}
+
+	for _, tag := range slices.Sorted(maps.Keys(tags)) {
+		imageID := tags[tag].String()
+		if imageID == app.ImageID.String || imageID == app.PreviousImageID.String {
+			continue
+		}
+
+		removeErr := svc.docker.RemoveImageTag(ctx, tag)
+		if removeErr != nil {
+			svc.log.Error("failed to remove old image",
+				"error", removeErr, "app", app.Name, "tag", tag)
+			_ = deployment.AppendLog(
+				ctx,
+				"WARNING: failed to remove old image "+tag+": "+removeErr.Error(),
+			)
+
+			continue
+		}
+
+		_ = deployment.AppendLog(ctx, "Removed old image: "+tag)
+	}
+}
+
 // cleanupCancelledDeploy removes orphan resources left by a cancelled deployment.
 func (svc *Service) cleanupCancelledDeploy(
 	ctx context.Context,

internal/service/deploy/deploy_images_test.go

@@ -0,0 +1,106 @@
+package deploy_test
+
+import (
+	"context"
+	"database/sql"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/fx/fxtest"
+
+	"sneak.berlin/go/upaas/internal/config"
+	"sneak.berlin/go/upaas/internal/database"
+	"sneak.berlin/go/upaas/internal/docker"
+	"sneak.berlin/go/upaas/internal/logger"
+	"sneak.berlin/go/upaas/internal/models"
+	"sneak.berlin/go/upaas/internal/service/deploy"
+)
+
+// TestRecordDeployedImageRemovesOldImages runs the step after a deploy
+// against a fake Docker API. Image one is also tagged for another app,
+// image two was the previous image, three the current one, four is new.
+func TestRecordDeployedImageRemovesOldImages(t *testing.T) {
+	t.Parallel()
+
+	var (
+		mu      sync.Mutex
+		removed []string
+		forced  bool
+	)
+
+	srv := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Content-Type", "application/json")
+
+			switch {
+			case r.Method == http.MethodDelete:
+				_, name, _ := strings.Cut(r.URL.Path, "/images/")
+
+				mu.Lock()
+
+				removed = append(removed, name)
+				forced = forced || r.URL.Query().Get("force") != ""
+				mu.Unlock()
+
+				_, _ = w.Write([]byte(`[]`))
+			case strings.HasSuffix(r.URL.Path, "/images/json"):
+				_, _ = w.Write([]byte(`[
+					{"Id":"sha256:one","RepoTags":["upaas-myapp:1","upaas-otherapp:7"]},
+					{"Id":"sha256:two","RepoTags":["upaas-myapp:2"]},
+					{"Id":"sha256:three","RepoTags":["upaas-myapp:3"]},
+					{"Id":"sha256:four","RepoTags":["upaas-myapp:4"]}
+				]`))
+			default:
+				_, _ = w.Write([]byte(`{}`))
+			}
+		},
+	))
+	t.Cleanup(srv.Close)
+
+	log := slog.New(slog.NewTextHandler(os.Stderr, nil))
+	lifecycle := fxtest.NewLifecycle(t)
+
+	dockerClient, err := docker.New(lifecycle, docker.Params{
+		Logger: logger.NewForTest(log),
+		Config: &config.Config{DockerHost: "tcp://" + srv.Listener.Addr().String()},
+	})
+	require.NoError(t, err)
+
+	lifecycle.RequireStart()
+	t.Cleanup(lifecycle.RequireStop)
+
+	db := database.NewTestDatabase(t)
+	ctx := context.Background()
+
+	app := models.NewApp(db)
+	app.ID = "myapp-id"
+	app.Name = "myapp"
+	app.ImageID = sql.NullString{String: "sha256:three", Valid: true}
+	app.PreviousImageID = sql.NullString{String: "sha256:two", Valid: true}
+	require.NoError(t, app.Save(ctx))
+
+	deployment := models.NewDeployment(db)
+	deployment.AppID = app.ID
+	require.NoError(t, deployment.Save(ctx))
+
+	svc := deploy.NewTestServiceWithConfig(log, &config.Config{}, dockerClient)
+
+	err = svc.RecordDeployedImage(ctx, app, deployment, "sha256:four")
+	require.NoError(t, err)
+
+	assert.Equal(t, "sha256:four", app.ImageID.String)
+	assert.Equal(t, "sha256:three", app.PreviousImageID.String)
+
+	mu.Lock()
+	defer mu.Unlock()
+
+	assert.Equal(t, []string{"upaas-myapp:1", "upaas-myapp:2"}, removed)
+	assert.False(t, forced, "old image tags must be removed without force")
+}

internal/service/deploy/export_test.go

@@ -90,6 +90,16 @@ func (svc *Service) GetBuildDirExported(appName string) string {
 	return svc.GetBuildDir(appName)
 }
 
+// RecordDeployedImage exposes recordDeployedImage for testing.
+func (svc *Service) RecordDeployedImage(
+	ctx context.Context,
+	app *models.App,
+	deployment *models.Deployment,
+	imageID docker.ImageID,
+) error {
+	return svc.recordDeployedImage(ctx, app, deployment, imageID)
+}
+
 // BuildContainerOptionsExported exposes buildContainerOptions for testing.
 func (svc *Service) BuildContainerOptionsExported(
 	ctx context.Context,