Skip to content

Commit 9320876

Browse files
committed
fix(compose): improve image pruning during uninstall
Either remove all unused images (when the "dangling" filter is false), or remove only images referenced by apps being uninstalled, unless they are still used by one or more containers. Update the integration test to verify that no images remain after uninstall with pruning enabled. Signed-off-by: Mike Sul <mike.sul@foundries.io>
1 parent 7aec748 commit 9320876

2 files changed

Lines changed: 83 additions & 14 deletions

File tree

pkg/compose/uninstall.go

Lines changed: 64 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,19 @@ package compose
33
import (
44
"context"
55
"errors"
6-
"github.com/docker/docker/api/types/filters"
6+
"fmt"
77
"os"
8+
9+
"github.com/docker/docker/api/types"
10+
"github.com/docker/docker/api/types/container"
11+
"github.com/docker/docker/api/types/filters"
12+
dockerClient "github.com/docker/docker/client"
813
)
914

1015
type (
1116
UninstallOpts struct {
12-
Prune bool
17+
Prune bool
18+
RemoveAllUnusedImages bool
1319
}
1420
UninstallOpt func(*UninstallOpts)
1521
)
@@ -18,9 +24,12 @@ var (
1824
ErrUninstallRunningApps = errors.New("failed to uninstall apps: some apps are still running, please stop them first")
1925
)
2026

21-
func WithImagePruning() UninstallOpt {
27+
func WithImagePruning(allUnused ...bool) UninstallOpt {
2228
return func(opts *UninstallOpts) {
2329
opts.Prune = true
30+
if len(allUnused) > 0 {
31+
opts.RemoveAllUnusedImages = allUnused[0]
32+
}
2433
}
2534
}
2635

@@ -64,16 +73,61 @@ func UninstallApps(ctx context.Context, cfg *Config, appRefs []string, options .
6473
}
6574

6675
if opts.Prune {
67-
cli, errClient := GetDockerClient(cfg.DockerHost)
76+
dockerClient, errClient := GetDockerClient(cfg.DockerHost)
6877
if errClient != nil {
6978
return errClient
7079
}
71-
// Prune only dangling images.
72-
// The dangling images are the ones that are not tagged and not referenced by any container.
73-
// TODO: consider pruning volumes and networks if needed.
74-
// TODO: consider pruning only those images that are related to the uninstalled apps,
75-
// otherwise it prunes all dangling images including those that are not managed by composectl
76-
_, err = cli.ImagesPrune(ctx, filters.NewArgs(filters.Arg("dangling", "true")))
80+
81+
if opts.RemoveAllUnusedImages {
82+
// Prune all unused images, including dangling and non-dangling ones.
83+
// The non-dangling images are the ones that are not referenced by any container, but they can still be tagged.
84+
_, err = dockerClient.ImagesPrune(ctx, filters.NewArgs(filters.Arg("dangling", "false")))
85+
return err
86+
}
87+
88+
// Get all containers to find out which images are still referenced by the containers, so that we don't prune those images.
89+
allContainers, errCtrList := dockerClient.ContainerList(ctx, container.ListOptions{})
90+
if errCtrList != nil {
91+
return fmt.Errorf("failed to list containers: %w", errCtrList)
92+
}
93+
imagesWithContainer := make(map[string]types.Container)
94+
for _, container := range allContainers {
95+
imagesWithContainer[container.Image] = container
96+
}
97+
98+
// Remove or untag images that thar are referenced by the compose apps to be uninstalled, but not referenced by any container.
99+
// We cannot even untag images that are still referenced by the containers, if we do then
100+
// composectl will think the app that uses that image is uninstalled.
101+
for _, app := range status.Apps {
102+
for _, imageRoot := range app.GetComposeRoot().Children {
103+
if _, hasContainer := imagesWithContainer[imageRoot.Ref()]; !hasContainer {
104+
removeImage(ctx, dockerClient, imageRoot)
105+
}
106+
}
107+
}
108+
// Prune dangling images, which are the ones that are not tagged and not referenced by any container.
109+
_, err = dockerClient.ImagesPrune(ctx, filters.NewArgs(filters.Arg("dangling", "true")))
77110
}
78111
return err
79112
}
113+
114+
func removeImage(ctx context.Context, client *dockerClient.Client, imageRoot *TreeNode) {
115+
imageNode := imageRoot
116+
for {
117+
// remove image (untag if more than 2 references) referenced by the URI with a digest
118+
// Ignore error because the image may have already been removed as a child of another image,
119+
// or the image may be referenced by other compose apps that are running or not uninstalled.
120+
_, _ = client.ImageRemove(ctx, imageNode.Ref(), types.ImageRemoveOptions{Force: false, PruneChildren: true})
121+
if imageRef, refParseErr := ParseImageRef(imageNode.Ref()); refParseErr == nil {
122+
// remove image (untag if more than 2 references) referenced by the URI with a tag
123+
// Ignore error because the image may have already been removed as a child of another image,
124+
// or the image may be referenced by other compose apps that are running or not uninstalled.
125+
_, _ = client.ImageRemove(ctx, imageRef.GetTagRef(), types.ImageRemoveOptions{Force: false, PruneChildren: true})
126+
}
127+
if imageNode.Type == BlobTypeImageManifest || len(imageNode.Children) == 0 {
128+
break
129+
}
130+
// The image root points to the image index, which was removed, now remove the image manifest
131+
imageNode = imageNode.Children[0]
132+
}
133+
}

test/integration/update_test.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package e2e_tests
22

33
import (
44
"context"
5+
"github.com/docker/docker/api/types"
56
"testing"
67

78
"github.com/foundriesio/composeapp/pkg/compose"
@@ -621,21 +622,35 @@ services:
621622
f.Check(t, updateRunner.Start(ctx))
622623
// Complete with pruning to remove the second app
623624
f.Check(t, updateRunner.Complete(ctx, update.CompleteWithPruning()))
624-
625625
appsStatus, err = compose.CheckAppsStatus(ctx, cfg, nil)
626626
f.Check(t, err)
627-
if !appsStatus.AreRunning() {
628-
t.Fatal("app is expected to be running")
629-
}
630627
if len(appsStatus.Apps) > 1 || len(appsStatus.Apps) == 0 {
631628
t.Fatalf("only one app is expected to be running, found %d", len(appsStatus.Apps))
632629
}
633630
if appsStatus.Apps[0].Ref().String() != appURIs[0] {
634631
t.Fatalf("expected app URI %s, found %s", appURIs[0], appsStatus.Apps[0].Ref().String())
635632
}
633+
appsStatus, err = compose.CheckAppsStatus(ctx, cfg, oneAppURI)
634+
f.Check(t, err)
635+
if !appsStatus.AreRunning() {
636+
t.Fatalf("app is expected to be running; uri=%s", oneAppURI[0])
637+
}
636638

637639
// stop, uninstall, and remove all apps
638640
f.Check(t, compose.StopApps(ctx, cfg, oneAppURI))
639641
f.Check(t, compose.UninstallApps(ctx, cfg, oneAppURI, compose.WithImagePruning()))
640642
f.Check(t, compose.RemoveApps(ctx, cfg, oneAppURI))
643+
644+
// check if no images are left after pruning in the docker storage
645+
cli, err := compose.GetDockerClient("")
646+
f.Check(t, err)
647+
defer cli.Close()
648+
images, err := cli.ImageList(ctx, types.ImageListOptions{All: true})
649+
f.Check(t, err)
650+
if len(images) != 0 {
651+
for _, img := range images {
652+
t.Logf("unexpected image left after pruning: ID=%s, RepoTags=%v, RepoDigests=%v\n", img.ID, img.RepoTags, img.RepoDigests)
653+
}
654+
t.Fatalf("no images are expected to be left after pruning, found %d", len(images))
655+
}
641656
}

0 commit comments

Comments
 (0)