diff --git a/TODO.md b/TODO.md index 27bd714..9a7114a 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: 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). diff --git a/internal/docker/client.go b/internal/docker/client.go index 9e6abc5..f348d33 100644 --- a/internal/docker/client.go +++ b/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, diff --git a/internal/service/deploy/deploy.go b/internal/service/deploy/deploy.go index 79343e2..e2e1d30 100644 --- a/internal/service/deploy/deploy.go +++ b/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 - if app.ImageID.Valid && app.ImageID.String != "" { - app.PreviousImageID = app.ImageID - } - - err = svc.updateAppRunning(bgCtx, app, imageID) + err = svc.recordDeployedImage(bgCtx, app, deployment, 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-:, 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, diff --git a/internal/service/deploy/deploy_images_test.go b/internal/service/deploy/deploy_images_test.go new file mode 100644 index 0000000..086ccb3 --- /dev/null +++ b/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") +} diff --git a/internal/service/deploy/export_test.go b/internal/service/deploy/export_test.go index 1d86a25..5b68733 100644 --- a/internal/service/deploy/export_test.go +++ b/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,