Skip to content

Commit e4c2eb9

Browse files
committed
c8d/prune: Keep the last tagged image instead of creating dangling image
Don't turn images into dangling when they are used by containers created with an image specified by an ID only (e.g. `docker run 82d1e9d`). Keep the last image reference with the same target when all other references would be pruned. If the container was created with a digested and tagged reference (e.g. `docker run alpine:latest@sha256:82d1e9d7ed48a7523bdebc18cf6290bdb97b82302a8a9c27d4fe885949ea94d1`), the `alpine:latest` image won't get untagged. This change makes the behavior consistent with the graphdriver implementation. Signed-off-by: Paweł Gronowski <[email protected]>
1 parent 59fe9e0 commit e4c2eb9

2 files changed

Lines changed: 195 additions & 22 deletions

File tree

daemon/containerd/image_prune.go

Lines changed: 61 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/distribution/reference"
1111
"github.com/docker/docker/api/types/filters"
1212
"github.com/docker/docker/api/types/image"
13+
"github.com/docker/docker/container"
1314
"github.com/docker/docker/errdefs"
1415
"github.com/hashicorp/go-multierror"
1516
"github.com/opencontainers/go-digest"
@@ -60,9 +61,20 @@ func (i *ImageService) ImagesPrune(ctx context.Context, fltrs filters.Args) (*im
6061
return i.pruneUnused(ctx, filterFunc, danglingOnly)
6162
}
6263

64+
// pruneUnused deletes images that are dangling or unused by any container.
65+
// The behavior is controlled by the danglingOnly parameter.
66+
// If danglingOnly is true, only dangling images are deleted.
67+
// Otherwise, all images unused by any container are deleted.
68+
//
69+
// Additionally, the filterFunc parameter is used to filter images that should
70+
// be considered for deletion.
71+
//
72+
// Container created with images specified by an ID only (e.g. `docker run 82d1e9d`)
73+
// will keep at least one image tag with that ID.
74+
//
75+
// In case a digested and tagged reference was used (e.g. `docker run alpine:latest@sha256:82d1e9d7ed48a7523bdebc18cf6290bdb97b82302a8a9c27d4fe885949ea94d1`),
76+
// the alpine:latest image will be kept.
6377
func (i *ImageService) pruneUnused(ctx context.Context, filterFunc imageFilterFunc, danglingOnly bool) (*image.PruneReport, error) {
64-
report := image.PruneReport{}
65-
6678
allImages, err := i.images.List(ctx)
6779
if err != nil {
6880
return nil, err
@@ -85,16 +97,40 @@ func (i *ImageService) pruneUnused(ctx context.Context, filterFunc imageFilterFu
8597
if canBePruned {
8698
imagesToPrune[img.Name] = img
8799
}
100+
}
101+
}
102+
103+
usedDigests := filterImagesUsedByContainers(ctx, i.containers.List(), imagesToPrune)
104+
105+
// Make sure we don't delete the last image of a particular digest used by any container.
106+
for name, img := range imagesToPrune {
107+
dgst := img.Target.Digest
108+
109+
if digestRefCount[dgst] > 1 {
110+
digestRefCount[dgst] -= 1
111+
continue
112+
}
88113

114+
if _, isUsed := usedDigests[dgst]; isUsed {
115+
delete(imagesToPrune, name)
89116
}
90117
}
91118

119+
return i.pruneAll(ctx, imagesToPrune)
120+
}
121+
122+
// filterImagesUsedByContainers removes image names that are used by containers
123+
// and returns a map of used image digests.
124+
func filterImagesUsedByContainers(ctx context.Context,
125+
allContainers []*container.Container,
126+
imagesToPrune map[string]containerdimages.Image,
127+
) (usedDigests map[digest.Digest]struct{}) {
92128
// Image specified by digests that are used by containers.
93-
usedDigests := map[digest.Digest]struct{}{}
129+
usedDigests = map[digest.Digest]struct{}{}
94130

95131
// Exclude images used by existing containers
96-
for _, ctr := range i.containers.List() {
97-
// If the original image was deleted, make sure we don't delete the dangling image
132+
for _, ctr := range allContainers {
133+
// If the original image was force deleted, make sure we don't delete the dangling image
98134
delete(imagesToPrune, danglingImageName(ctr.ImageID.Digest()))
99135

100136
// Config.Image is the image reference passed by user.
@@ -105,41 +141,44 @@ func (i *ImageService) pruneUnused(ctx context.Context, filterFunc imageFilterFu
105141
// but both will have ImageID="sha256:82d1e9d7ed48a7523bdebc18cf6290bdb97b82302a8a9c27d4fe885949ea94d1"
106142
imageDgst := ctr.ImageID.Digest()
107143

108-
// If user didn't specify an explicit image, mark the digest as used.
144+
// If user used an full or truncated ID instead of an explicit image name, mark the digest as used.
109145
normalizedImageID := "sha256:" + strings.TrimPrefix(ctr.Config.Image, "sha256:")
110-
if strings.HasPrefix(imageDgst.String(), normalizedImageID) {
146+
fullOrTruncatedID := strings.HasPrefix(imageDgst.String(), normalizedImageID)
147+
digestedRef := strings.HasSuffix(ctr.Config.Image, "@"+imageDgst.String())
148+
if fullOrTruncatedID || digestedRef {
111149
usedDigests[imageDgst] = struct{}{}
112-
continue
113150
}
114151

115152
ref, err := reference.ParseNormalizedNamed(ctr.Config.Image)
116153
log.G(ctx).WithFields(log.Fields{
117154
"ctr": ctr.ID,
118-
"image": ref,
155+
"imageRef": ref,
156+
"imageID": imageDgst,
119157
"nameParseErr": err,
120158
}).Debug("filtering container's image")
121-
122159
if err == nil {
123160
// If user provided a specific image name, exclude that image.
124161
name := reference.TagNameOnly(ref)
125162
delete(imagesToPrune, name.String())
126-
}
127-
}
128-
129-
// Create dangling images for images that will be deleted but are still in use.
130-
for _, img := range imagesToPrune {
131-
dgst := img.Target.Digest
132163

133-
digestRefCount[dgst] -= 1
134-
if digestRefCount[dgst] == 0 {
135-
if _, isUsed := usedDigests[dgst]; isUsed {
136-
if err := i.ensureDanglingImage(ctx, img); err != nil {
137-
return &report, errors.Wrapf(err, "failed to create ensure dangling image for %s", img.Name)
138-
}
164+
// Also exclude repo:tag image if repo:tag@sha256:digest reference was used.
165+
_, isDigested := name.(reference.Digested)
166+
tagged, isTagged := name.(reference.NamedTagged)
167+
if isDigested && isTagged {
168+
named, _ := reference.ParseNormalizedNamed(tagged.Name())
169+
namedTagged, _ := reference.WithTag(named, tagged.Tag())
170+
delete(imagesToPrune, namedTagged.String())
139171
}
140172
}
141173
}
142174

175+
return usedDigests
176+
}
177+
178+
// pruneAll deletes all images in the imagesToPrune map.
179+
func (i *ImageService) pruneAll(ctx context.Context, imagesToPrune map[string]containerdimages.Image) (*image.PruneReport, error) {
180+
report := image.PruneReport{}
181+
143182
possiblyDeletedConfigs := map[digest.Digest]struct{}{}
144183
var errs error
145184

integration/image/prune_test.go

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,16 @@ import (
44
"strings"
55
"testing"
66

7+
containertypes "github.com/docker/docker/api/types/container"
78
"github.com/docker/docker/api/types/filters"
9+
"github.com/docker/docker/api/types/image"
10+
"github.com/docker/docker/client"
811
"github.com/docker/docker/integration/internal/container"
912
"github.com/docker/docker/internal/testutils/specialimage"
13+
"github.com/docker/docker/testutil"
1014
"github.com/docker/docker/testutil/daemon"
1115
"gotest.tools/v3/assert"
16+
is "gotest.tools/v3/assert/cmp"
1217
"gotest.tools/v3/skip"
1318
)
1419

@@ -47,3 +52,132 @@ func TestPruneDontDeleteUsedDangling(t *testing.T) {
4752
_, _, err = client.ImageInspectWithRaw(ctx, danglingID)
4853
assert.NilError(t, err, "Test dangling image should still exist")
4954
}
55+
56+
// Regression test for https://github.com/moby/moby/issues/48063
57+
func TestPruneDontDeleteUsedImage(t *testing.T) {
58+
skip.If(t, testEnv.DaemonInfo.OSType == "windows", "cannot start multiple daemons on windows")
59+
skip.If(t, testEnv.IsRemoteDaemon, "cannot run daemon when remote daemon")
60+
61+
ctx := setupTest(t)
62+
63+
for _, env := range []struct {
64+
name string
65+
prepare func(t *testing.T, client *daemon.Daemon, apiClient *client.Client) error
66+
check func(t *testing.T, apiClient *client.Client, pruned image.PruneReport)
67+
}{
68+
{
69+
// Container uses the busybox:latest image and it's the only image
70+
// tag with the same target.
71+
name: "single tag",
72+
check: func(t *testing.T, apiClient *client.Client, pruned image.PruneReport) {
73+
assert.Check(t, is.Len(pruned.ImagesDeleted, 0))
74+
75+
_, _, err := apiClient.ImageInspectWithRaw(ctx, "busybox:latest")
76+
assert.NilError(t, err, "Busybox image should still exist")
77+
},
78+
},
79+
{
80+
// Container uses the busybox:latest image and there's also a second
81+
// busybox:other tag pointing to the same image.
82+
name: "two tags",
83+
prepare: func(t *testing.T, d *daemon.Daemon, apiClient *client.Client) error {
84+
return apiClient.ImageTag(ctx, "busybox:latest", "busybox:other")
85+
},
86+
check: func(t *testing.T, apiClient *client.Client, pruned image.PruneReport) {
87+
if assert.Check(t, is.Len(pruned.ImagesDeleted, 1)) {
88+
assert.Check(t, is.Equal(pruned.ImagesDeleted[0].Deleted, ""))
89+
t.Log("Untagged", pruned.ImagesDeleted[0].Untagged)
90+
}
91+
92+
_, _, err1 := apiClient.ImageInspectWithRaw(ctx, "busybox:latest")
93+
_, _, err2 := apiClient.ImageInspectWithRaw(ctx, "busybox:other")
94+
95+
assert.Check(t, err1 != err2 && (err1 == nil || err2 == nil), "One of the images should still exist")
96+
},
97+
},
98+
} {
99+
for _, tc := range []struct {
100+
name string
101+
imageID func(t *testing.T, inspect image.InspectResponse) string
102+
}{
103+
{
104+
name: "full id",
105+
imageID: func(t *testing.T, inspect image.InspectResponse) string {
106+
return inspect.ID
107+
},
108+
},
109+
{
110+
name: "full id without sha256 prefix",
111+
imageID: func(t *testing.T, inspect image.InspectResponse) string {
112+
return strings.TrimPrefix(inspect.ID, "sha256:")
113+
},
114+
},
115+
{
116+
name: "truncated id (without sha256 prefix)",
117+
imageID: func(t *testing.T, inspect image.InspectResponse) string {
118+
return strings.TrimPrefix(inspect.ID, "sha256:")[:8]
119+
},
120+
},
121+
{
122+
name: "repo and digest without tag",
123+
imageID: func(t *testing.T, inspect image.InspectResponse) string {
124+
skip.If(t, !testEnv.UsingSnapshotter())
125+
126+
return "busybox@" + inspect.ID
127+
},
128+
},
129+
{
130+
name: "tagged and digested",
131+
imageID: func(t *testing.T, inspect image.InspectResponse) string {
132+
skip.If(t, !testEnv.UsingSnapshotter())
133+
134+
return "busybox:latest@" + inspect.ID
135+
},
136+
},
137+
{
138+
name: "repo digest",
139+
imageID: func(t *testing.T, inspect image.InspectResponse) string {
140+
// graphdriver won't have a repo digest
141+
skip.If(t, len(inspect.RepoDigests) == 0, "no repo digest")
142+
143+
return inspect.RepoDigests[0]
144+
},
145+
},
146+
} {
147+
tc := tc
148+
t.Run(env.name+"/"+tc.name, func(t *testing.T) {
149+
ctx := testutil.StartSpan(ctx, t)
150+
d := daemon.New(t)
151+
d.Start(t)
152+
defer d.Stop(t)
153+
154+
apiClient := d.NewClientT(t)
155+
defer apiClient.Close()
156+
157+
d.LoadBusybox(ctx, t)
158+
159+
if env.prepare != nil {
160+
err := env.prepare(t, d, apiClient)
161+
assert.NilError(t, err, "prepare failed")
162+
}
163+
164+
inspect, _, err := apiClient.ImageInspectWithRaw(ctx, "busybox:latest")
165+
assert.NilError(t, err)
166+
167+
image := tc.imageID(t, inspect)
168+
t.Log(image)
169+
170+
cid := container.Run(ctx, t, apiClient,
171+
container.WithImage(image),
172+
container.WithCmd("sleep", "60"))
173+
defer container.Remove(ctx, t, apiClient, cid, containertypes.RemoveOptions{Force: true})
174+
175+
// dangling=false also prunes unused images
176+
pruned, err := apiClient.ImagesPrune(ctx, filters.NewArgs(filters.Arg("dangling", "false")))
177+
assert.NilError(t, err)
178+
179+
env.check(t, apiClient, pruned)
180+
})
181+
}
182+
}
183+
}

0 commit comments

Comments
 (0)