Prune app images correctly

cmd/composectl/cmd/root.go

@@ -160,7 +160,7 @@ func getDockerConfig() (*configfile.ConfigFile, error) {
 // many app operations such as "run", "rm", etc.
 // If `userListedApps` is empty, then return all apps from the local store.
 // Return the list of validated app URIs.
-func checkUserListedApps(ctx context.Context, cfg *compose.Config, userListedApps []string, checkIfExist bool) []string {
+func checkUserListedApps(ctx context.Context, cfg *compose.Config, userListedApps []string, checkIfExist bool, allowMultipleVersions ...bool) []string {
 	// Get the list of all apps in the local store
 	apps, err := compose.ListApps(ctx, cfg)
 	DieNotNil(err)
@@ -173,25 +173,28 @@ func checkUserListedApps(ctx context.Context, cfg *compose.Config, userListedApp
 		}
 	}
 
-	checkedApps := map[string]compose.App{}
+	checkedApps := map[string][]compose.App{}
 	for _, appNameOrURI := range inputAppRefs {
 		var foundName bool
 		var foundURI bool
-		var foundApp compose.App
+		// Keep track of all found apps with the same name,
+		// since more than one version of the same app can be in the local store,
+		// and the user can specify the app by name without version, which is an ambiguous reference to the app.
+		var foundApps []compose.App
 
 		// Search for the app in the local app store by name or by URI
 		for _, app := range apps {
 			if app.Name() == appNameOrURI {
-				if foundName {
+				if (len(allowMultipleVersions) == 0 || !allowMultipleVersions[0]) && foundName {
 					DieNotNil(fmt.Errorf("more than two versions of the same app found in the local app store:"+
-						" %s (%s and %s)", app.Name(), foundApp.Ref().String(), app.Ref().String()))
+						" %s (%s and %s)", app.Name(), foundApps[0].Ref().String(), app.Ref().String()))
 				}
 				foundName = true
-				foundApp = app
+				foundApps = append(foundApps, app)
 				// Continue searching because there might be more than one version of the same app in the store
 			} else if app.Ref().String() == appNameOrURI {
 				foundURI = true
-				foundApp = app
+				foundApps = append(foundApps, app)
 				// No need to continue searching because app URIs are unique
 				break
 			}
@@ -205,16 +208,18 @@ func checkUserListedApps(ctx context.Context, cfg *compose.Config, userListedApp
 			continue
 		}
 
-		if _, exists := checkedApps[foundApp.Name()]; exists {
-			DieNotNil(fmt.Errorf("the same app specified more than once: %s", foundApp.Name()))
+		if _, exists := checkedApps[foundApps[0].Name()]; exists {
+			DieNotNil(fmt.Errorf("the same app specified more than once: %s", foundApps[0].Name()))
 		} else {
-			checkedApps[foundApp.Name()] = foundApp
+			checkedApps[foundApps[0].Name()] = foundApps
 		}
 	}
 
 	var appURIs []string
-	for _, app := range checkedApps {
-		appURIs = append(appURIs, app.Ref().String())
+	for _, allAppVersions := range checkedApps {
+		for _, app := range allAppVersions {
+			appURIs = append(appURIs, app.Ref().String())
+		}
 	}
 	return appURIs
 }

cmd/composectl/cmd/uninstall.go

@@ -1,11 +1,8 @@
 package composectl
 
 import (
-	"fmt"
-	"github.com/docker/docker/api/types/filters"
 	"github.com/foundriesio/composeapp/pkg/compose"
 	"github.com/spf13/cobra"
-	"os"
 )
 
 type (
@@ -18,7 +15,7 @@ type (
 func init() {
 	uninstallCmd := &cobra.Command{
 		Use:   "uninstall",
-		Short: "uninstall <app-name> [<app-name>]",
+		Short: "uninstall <app-name-or-URI> [<app-name-or-URI>]",
 		Long:  ``,
 		Args:  cobra.MinimumNArgs(1),
 	}
@@ -33,23 +30,10 @@ func init() {
 }
 
 func uninstallApps(cmd *cobra.Command, args []string, opts *uninstallOptions) {
-	apps := getAllAppStatuses(cmd.Context(), false)
-	for _, app := range args {
-		if _, ok := apps[app]; ok {
-			DieNotNil(fmt.Errorf("cannot uninstall running app: %s", app))
-		}
-		appComposeDir := config.GetAppComposeDir(app)
-		if !opts.ignoreNonInstalled {
-			if _, err := os.Stat(appComposeDir); os.IsNotExist(err) {
-				DieNotNil(fmt.Errorf("app is not installed: %s", app))
-			}
-		}
-		DieNotNil(os.RemoveAll(appComposeDir))
-	}
+	appURIs := checkUserListedApps(cmd.Context(), config, args, !opts.ignoreNonInstalled, true)
+	pruneType := compose.PruneTypeOnlyAppImages
 	if opts.prune {
-		cli, err := compose.GetDockerClient(dockerHost)
-		DieNotNil(err)
-		_, err = cli.ImagesPrune(cmd.Context(), filters.NewArgs(filters.Arg("dangling", "false")))
-		DieNotNil(err)
+		pruneType = compose.PruneTypeAllUnusedImages
 	}
+	DieNotNil(compose.UninstallApps(cmd.Context(), config, appURIs, compose.WithImagePruning(pruneType)))
 }

pkg/compose/uninstall.go

@@ -3,24 +3,40 @@ package compose
 import (
 	"context"
 	"errors"
-	"github.com/docker/docker/api/types/filters"
+	"fmt"
 	"os"
+
+	"github.com/containerd/containerd/reference/docker"
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/api/types/image"
+	"github.com/docker/docker/client"
 )
 
 type (
 	UninstallOpts struct {
-		Prune bool
+		Prune     bool
+		PruneType PruneType
 	}
 	UninstallOpt func(*UninstallOpts)
+
+	PruneType string
 )
 
 var (
 	ErrUninstallRunningApps = errors.New("failed to uninstall apps: some apps are still running, please stop them first")
+
+	PruneTypeAllUnusedImages PruneType = "all-unused-images"
+	PruneTypeOnlyAppImages   PruneType = "only-app-images"
 )
 
-func WithImagePruning() UninstallOpt {
+func WithImagePruning(pruneType ...PruneType) UninstallOpt {
 	return func(opts *UninstallOpts) {
 		opts.Prune = true
+		opts.PruneType = PruneTypeOnlyAppImages
+		if len(pruneType) > 0 {
+			opts.PruneType = pruneType[0]
+		}
 	}
 }
 
@@ -54,26 +70,115 @@ func UninstallApps(ctx context.Context, cfg *Config, appRefs []string, options .
 	}
 	for _, app := range status.Apps {
 		if appsInStore[app.Name()] > 1 {
-			// Cannot remove compose app dir if there is another version of this app in the store
-			continue
+			// Multiple versions of the same app exist in the store.
+			// If the version being removed is not installed, another version may still be
+			// installed and using the same compose directory. In that case, keep the app
+			// compose directory; otherwise we could remove compose files needed by the
+			// other installed version.
+			if _, isNotInstalled := status.NotInstalledCompose[app.Ref().Digest]; isNotInstalled {
+				continue
+			}
 		}
-		err = os.RemoveAll(cfg.GetAppComposeDir(app.Name()))
-		if err != nil {
+		if err = os.RemoveAll(cfg.GetAppComposeDir(app.Name())); err != nil {
 			return err
 		}
 	}
 
 	if opts.Prune {
 		cli, errClient := GetDockerClient(cfg.DockerHost)
 		if errClient != nil {
-			return errClient
+			return fmt.Errorf("failed to create docker client: %w", errClient)
+		}
+
+		var err error
+		var allImages []image.Summary
+		if allImages, err = cli.ImageList(ctx, types.ImageListOptions{All: true}); err != nil {
+			return fmt.Errorf("failed to list images: %w", err)
+		}
+
+		imagesNotInUse := make(map[string]image.Summary)
+		for _, img := range allImages {
+			imagesNotInUse[img.ID] = img
+		}
+
+		var allContainers []types.Container
+		if allContainers, err = cli.ContainerList(ctx, container.ListOptions{All: true}); err != nil {
+			return fmt.Errorf("failed to list containers: %w", err)
+		}
+		for _, ctr := range allContainers {
+			delete(imagesNotInUse, ctr.ImageID)
+		}
+
+		switch opts.PruneType {
+		case PruneTypeAllUnusedImages:
+			// Remove all images that are not in use by any container.
+			for imgID := range imagesNotInUse {
+				// TODO: print debug message about which image is being removed and any error that occurs during removal.
+				_, _ = cli.ImageRemove(ctx, imgID, types.ImageRemoveOptions{Force: true, PruneChildren: true})
+			}
+		case PruneTypeOnlyAppImages:
+			// Build a map of image refs to image summary for images that are not in use by any container.
+			// We will use this map to check if an image ref related to the uninstalled apps is used by any container before removing it.
+			imageRefsNotInUse := make(map[string]image.Summary)
+			setAllImageRefVariants := func(ref string) {
+				imageRefsNotInUse[ref] = imagesNotInUse[ref]
+				// Make sure to consider all variants of the same image ref, "normalized" and "familiar"
+				if anyRef, err := docker.ParseAnyReference(ref); err == nil {
+					imageRefsNotInUse[anyRef.String()] = imagesNotInUse[ref]
+					if familiarRef := docker.FamiliarString(anyRef); len(familiarRef) > 0 {
+						imageRefsNotInUse[familiarRef] = imagesNotInUse[ref]
+					}
+				}
+			}
+			for _, img := range imagesNotInUse {
+				for _, ref := range img.RepoDigests {
+					setAllImageRefVariants(ref)
+				}
+				for _, ref := range img.RepoTags {
+					setAllImageRefVariants(ref)
+				}
+			}
+			// Remove images that are referenced by the apps being uninstalled and are not in use by any container.
+			removeAppImageRefs(ctx, status.Apps, cli, imageRefsNotInUse)
 		}
-		// Prune only dangling images.
-		// The dangling images are the ones that are not tagged and not referenced by any container.
-		// TODO: consider pruning volumes and networks if needed.
-		// TODO: consider pruning only those images that are related to the uninstalled apps,
-		//       otherwise it prunes all dangling images including those that are not managed by composectl
-		_, err = cli.ImagesPrune(ctx, filters.NewArgs(filters.Arg("dangling", "true")))
 	}
 	return err
 }
+
+func removeAppImageRefs(ctx context.Context, apps []App, cli *client.Client, imageRefsNotInUse map[string]image.Summary) {
+	// Collect image refs related to the uninstalled apps.
+	var imageRefsToPrune []string
+	for _, app := range apps {
+		for _, imageRoot := range app.GetComposeRoot().Children {
+			curImageRoot := imageRoot
+			for {
+				imageRef := curImageRoot.Ref()
+				// Add a digest ref
+				imageRefsToPrune = append(imageRefsToPrune, imageRef)
+				if ref, err := ParseImageRef(imageRef); err == nil {
+					// Add a tag ref
+					imageRefsToPrune = append(imageRefsToPrune, ref.GetTagRef())
+				}
+				if curImageRoot.Type == BlobTypeImageManifest || len(curImageRoot.Children) == 0 {
+					break
+				}
+				// the image root points to an image index, let's add refs that point to the image manifest
+				curImageRoot = curImageRoot.Children[0]
+			}
+		}
+	}
+	// Remove image refs related to the uninstalled apps and images the refs point to are not in use by any container.
+	// If the removed ref is the only ref for the image, the image will also be removed;
+	// if there are other refs for the image, only the removed ref will be removed.
+	// This is the best effort to remove images related to the uninstalled apps without
+	// affecting other apps that may share the same images.
+	// In some case it can remove an image for which there is no container but some other utility reference it
+	// by the same reference as the uninstalled app, but that is an acceptable edge case and best effort
+	// to clean up images related to the uninstalled apps.
+	for _, ref := range imageRefsToPrune {
+		if _, notInUse := imageRefsNotInUse[ref]; notInUse {
+			// TODO: print debug message about which image is being removed and any error that occurs during removal.
+			_, _ = cli.ImageRemove(ctx, ref, types.ImageRemoveOptions{Force: false, PruneChildren: true})
+		}
+	}
+}

test/fixtures/composectl_cmds.go

@@ -188,26 +188,32 @@ func (a *App) Publish(t *testing.T, publishOpts ...func(*PublishOpts)) {
 }
 
 func (a *App) Pull(t *testing.T) {
+	t.Helper()
 	a.runCmd(t, "pull app", "pull", a.PublishedUri, "-u", "90")
 }
 
 func (a *App) Remove(t *testing.T) {
+	t.Helper()
 	a.runCmd(t, "remove app", "rm", a.PublishedUri)
 }
 
 func (a *App) Install(t *testing.T) {
+	t.Helper()
 	a.runCmd(t, "install app", "install", a.PublishedUri)
 }
 
 func (a *App) Uninstall(t *testing.T) {
-	a.runCmd(t, "uninstall app", "uninstall", "--prune=true", a.Name)
+	t.Helper()
+	a.runCmd(t, "uninstall app", "uninstall", "--prune=false", a.Name)
 }
 
 func (a *App) Run(t *testing.T) {
+	t.Helper()
 	a.runCmd(t, "run app", "run", a.Name)
 }
 
 func (a *App) Up(t *testing.T) {
+	t.Helper()
 	t.Run("compose up", func(t *testing.T) {
 		composeRoot := path.Join(AppComposeRootRoot, a.Name)
 
@@ -219,6 +225,7 @@ func (a *App) Up(t *testing.T) {
 }
 
 func (a *App) Stop(t *testing.T) {
+	t.Helper()
 	a.runCmd(t, "stop app", "stop", a.Name)
 }
 

test/integration/update_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"testing"
 
+	"github.com/docker/docker/api/types"
 	"github.com/foundriesio/composeapp/pkg/compose"
 	"github.com/foundriesio/composeapp/pkg/update"
 	f "github.com/foundriesio/composeapp/test/fixtures"
@@ -528,8 +529,8 @@ services:
 	// We need to run "bad" app stop before trying to install a new valid app because one of the bad app containers
 	// could have started before the other failed to start, and it would interfere with the new app installation.
 	badApp.Stop(t)
-	defer badApp.Uninstall(t)
 	defer badApp.Remove(t)
+	defer badApp.Uninstall(t)
 
 	app := f.NewApp(t, appComposeDef)
 	app.Publish(t)
@@ -638,4 +639,17 @@ services:
 	f.Check(t, compose.StopApps(ctx, cfg, oneAppURI))
 	f.Check(t, compose.UninstallApps(ctx, cfg, oneAppURI, compose.WithImagePruning()))
 	f.Check(t, compose.RemoveApps(ctx, cfg, oneAppURI))
+
+	// check if no images are left after uninstalling the only app with image pruning
+	cli, err := compose.GetDockerClient("")
+	f.Check(t, err)
+	defer cli.Close()
+	images, err := cli.ImageList(ctx, types.ImageListOptions{All: true})
+	f.Check(t, err)
+	if len(images) != 0 {
+		for _, img := range images {
+			t.Logf("unexpected image left after pruning: ID=%s, RepoTags=%v, RepoDigests=%v\n", img.ID, img.RepoTags, img.RepoDigests)
+		}
+		t.Fatalf("no images are expected to be left after pruning, found %d", len(images))
+	}
 }