From 4dbb654b794739fc0119400dc9cf10fb01c44081 Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 23 Sep 2026 09:49:43 +0000 Subject: [PATCH] Remove images from earlier deploys after a successful deploy (closes #216) After a successful deploy, upaas now removes the app's images other than the one the running container uses and the previous one, which rollback starts. Removing an image also removes the untagged images it was built on unless another image still needs them. Images left by other stages of a multi-stage build are not tied to the app and stay. Model: opus-5-5 --- TODO.md | 3 ++ internal/docker/client.go | 25 +++++++++ internal/service/deploy/deploy.go | 53 +++++++++++++++++++ internal/service/deploy/deploy_images_test.go | 48 +++++++++++++++++ internal/service/deploy/export_test.go | 5 ++ 5 files changed, 134 insertions(+) create mode 100644 internal/service/deploy/deploy_images_test.go diff --git a/TODO.md b/TODO.md index 72043db..2e86560 100644 --- a/TODO.md +++ b/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: Deployment log files are now stored under `logs//` instead of `logs///`, so downloads keep working after the upaas container is recreated; logs written under an old hostname directory are diff --git a/internal/docker/client.go b/internal/docker/client.go index 168f3fa..2e024fc 100644 --- a/internal/docker/client.go +++ b/internal/docker/client.go @@ -537,6 +537,31 @@ func (c *Client) RemoveImage(ctx context.Context, imageID ImageID) error { return nil } +// ListImageIDs returns the IDs of all images tagged in the given repository, +// such as "upaas-myapp". +func (c *Client) ListImageIDs( + ctx context.Context, + repository 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) + } + + ids := make([]ImageID, 0, len(images)) + for _, img := range images { + ids = append(ids, ImageID(img.ID)) + } + + return ids, nil +} + func (c *Client) performBuild( ctx context.Context, opts BuildImageOptions, diff --git a/internal/service/deploy/deploy.go b/internal/service/deploy/deploy.go index 79343e2..aa14790 100644 --- a/internal/service/deploy/deploy.go +++ b/internal/service/deploy/deploy.go @@ -534,6 +534,8 @@ func (svc *Service) runBuildAndDeploy( return err } + svc.removeUnusedImages(bgCtx, app, deployment) + // Use context.WithoutCancel to ensure health check completes even if // the parent context is cancelled (e.g., HTTP request ends). go svc.checkHealthAfterDelay(bgCtx, app, deployment) @@ -714,6 +716,57 @@ func (svc *Service) checkCancelled( return ErrDeployCancelled } +// removeUnusedImages removes the app's images (tagged upaas-: +// by buildImage) except the one the running container uses and the one +// Rollback would start. Removing an image also removes the untagged images +// it was built on, unless another image still needs them. +func (svc *Service) removeUnusedImages( + ctx context.Context, + app *models.App, + deployment *models.Deployment, +) { + images, err := svc.docker.ListImageIDs(ctx, "upaas-"+app.Name) + if err != nil { + svc.log.Error("failed to list app images", "error", err, "app", app.Name) + + return + } + + for _, imageID := range unusedImages(images, app) { + removeErr := svc.docker.RemoveImage(ctx, imageID) + if removeErr != nil { + svc.log.Error("failed to remove old image", + "error", removeErr, "app", app.Name, "image", imageID) + _ = deployment.AppendLog( + ctx, + "WARNING: failed to remove old image "+ + imageID.String()+": "+removeErr.Error(), + ) + + continue + } + + _ = deployment.AppendLog(ctx, "Removed old image: "+imageID.String()) + } +} + +// unusedImages returns the images that are neither the app's current image +// nor its previous image, which Rollback uses. +func unusedImages(images []docker.ImageID, app *models.App) []docker.ImageID { + var unused []docker.ImageID + + for _, imageID := range images { + if imageID.String() == app.ImageID.String || + imageID.String() == app.PreviousImageID.String { + continue + } + + unused = append(unused, imageID) + } + + return unused +} + // cleanupCancelledDeploy removes orphan resources left by a cancelled deployment. func (svc *Service) cleanupCancelledDeploy( ctx context.Context, diff --git a/internal/service/deploy/deploy_images_test.go b/internal/service/deploy/deploy_images_test.go new file mode 100644 index 0000000..0ece5d0 --- /dev/null +++ b/internal/service/deploy/deploy_images_test.go @@ -0,0 +1,48 @@ +package deploy_test + +import ( + "database/sql" + "testing" + + "github.com/stretchr/testify/assert" + + "sneak.berlin/go/upaas/internal/docker" + "sneak.berlin/go/upaas/internal/models" + "sneak.berlin/go/upaas/internal/service/deploy" +) + +const currentImage = "sha256:current" + +func TestUnusedImages_KeepsCurrentAndRollbackImage(t *testing.T) { + t.Parallel() + + app := &models.App{ + ImageID: sql.NullString{String: currentImage, Valid: true}, + PreviousImageID: sql.NullString{String: "sha256:previous", Valid: true}, + } + images := []docker.ImageID{ + "sha256:oldest", + "sha256:previous", + "sha256:older", + currentImage, + } + + assert.Equal(t, + []docker.ImageID{"sha256:oldest", "sha256:older"}, + deploy.UnusedImages(images, app), + ) +} + +func TestUnusedImages_FirstDeployHasNoRollbackImage(t *testing.T) { + t.Parallel() + + app := &models.App{ + ImageID: sql.NullString{String: currentImage, Valid: true}, + } + images := []docker.ImageID{currentImage, "sha256:failed"} + + assert.Equal(t, + []docker.ImageID{"sha256:failed"}, + deploy.UnusedImages(images, app), + ) +} diff --git a/internal/service/deploy/export_test.go b/internal/service/deploy/export_test.go index 1d86a25..dca8cfe 100644 --- a/internal/service/deploy/export_test.go +++ b/internal/service/deploy/export_test.go @@ -90,6 +90,11 @@ func (svc *Service) GetBuildDirExported(appName string) string { return svc.GetBuildDir(appName) } +// UnusedImages exposes unusedImages for testing. +func UnusedImages(images []docker.ImageID, app *models.App) []docker.ImageID { + return unusedImages(images, app) +} + // BuildContainerOptionsExported exposes buildContainerOptions for testing. func (svc *Service) BuildContainerOptionsExported( ctx context.Context,