Browse code

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 <pawel.gronowski@docker.com>

Paweł Gronowski authored on 2024/06/26 22:50:13
Showing 2 changed files
... ...
@@ -10,6 +10,7 @@ import (
10 10
 	"github.com/distribution/reference"
11 11
 	"github.com/docker/docker/api/types/filters"
12 12
 	"github.com/docker/docker/api/types/image"
13
+	"github.com/docker/docker/container"
13 14
 	"github.com/docker/docker/errdefs"
14 15
 	"github.com/hashicorp/go-multierror"
15 16
 	"github.com/opencontainers/go-digest"
... ...
@@ -60,9 +61,20 @@ func (i *ImageService) ImagesPrune(ctx context.Context, fltrs filters.Args) (*im
60 60
 	return i.pruneUnused(ctx, filterFunc, danglingOnly)
61 61
 }
62 62
 
63
+// pruneUnused deletes images that are dangling or unused by any container.
64
+// The behavior is controlled by the danglingOnly parameter.
65
+// If danglingOnly is true, only dangling images are deleted.
66
+// Otherwise, all images unused by any container are deleted.
67
+//
68
+// Additionally, the filterFunc parameter is used to filter images that should
69
+// be considered for deletion.
70
+//
71
+// Container created with images specified by an ID only (e.g. `docker run 82d1e9d`)
72
+// will keep at least one image tag with that ID.
73
+//
74
+// In case a digested and tagged reference was used (e.g. `docker run alpine:latest@sha256:82d1e9d7ed48a7523bdebc18cf6290bdb97b82302a8a9c27d4fe885949ea94d1`),
75
+// the alpine:latest image will be kept.
63 76
 func (i *ImageService) pruneUnused(ctx context.Context, filterFunc imageFilterFunc, danglingOnly bool) (*image.PruneReport, error) {
64
-	report := image.PruneReport{}
65
-
66 77
 	allImages, err := i.images.List(ctx)
67 78
 	if err != nil {
68 79
 		return nil, err
... ...
@@ -85,16 +97,40 @@ func (i *ImageService) pruneUnused(ctx context.Context, filterFunc imageFilterFu
85 85
 			if canBePruned {
86 86
 				imagesToPrune[img.Name] = img
87 87
 			}
88
+		}
89
+	}
90
+
91
+	usedDigests := filterImagesUsedByContainers(ctx, i.containers.List(), imagesToPrune)
92
+
93
+	// Make sure we don't delete the last image of a particular digest used by any container.
94
+	for name, img := range imagesToPrune {
95
+		dgst := img.Target.Digest
96
+
97
+		if digestRefCount[dgst] > 1 {
98
+			digestRefCount[dgst] -= 1
99
+			continue
100
+		}
88 101
 
102
+		if _, isUsed := usedDigests[dgst]; isUsed {
103
+			delete(imagesToPrune, name)
89 104
 		}
90 105
 	}
91 106
 
107
+	return i.pruneAll(ctx, imagesToPrune)
108
+}
109
+
110
+// filterImagesUsedByContainers removes image names that are used by containers
111
+// and returns a map of used image digests.
112
+func filterImagesUsedByContainers(ctx context.Context,
113
+	allContainers []*container.Container,
114
+	imagesToPrune map[string]containerdimages.Image,
115
+) (usedDigests map[digest.Digest]struct{}) {
92 116
 	// Image specified by digests that are used by containers.
93
-	usedDigests := map[digest.Digest]struct{}{}
117
+	usedDigests = map[digest.Digest]struct{}{}
94 118
 
95 119
 	// 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
120
+	for _, ctr := range allContainers {
121
+		// If the original image was force deleted, make sure we don't delete the dangling image
98 122
 		delete(imagesToPrune, danglingImageName(ctr.ImageID.Digest()))
99 123
 
100 124
 		// Config.Image is the image reference passed by user.
... ...
@@ -105,41 +141,44 @@ func (i *ImageService) pruneUnused(ctx context.Context, filterFunc imageFilterFu
105 105
 		// but both will have ImageID="sha256:82d1e9d7ed48a7523bdebc18cf6290bdb97b82302a8a9c27d4fe885949ea94d1"
106 106
 		imageDgst := ctr.ImageID.Digest()
107 107
 
108
-		// If user didn't specify an explicit image, mark the digest as used.
108
+		// If user used an full or truncated ID instead of an explicit image name, mark the digest as used.
109 109
 		normalizedImageID := "sha256:" + strings.TrimPrefix(ctr.Config.Image, "sha256:")
110
-		if strings.HasPrefix(imageDgst.String(), normalizedImageID) {
110
+		fullOrTruncatedID := strings.HasPrefix(imageDgst.String(), normalizedImageID)
111
+		digestedRef := strings.HasSuffix(ctr.Config.Image, "@"+imageDgst.String())
112
+		if fullOrTruncatedID || digestedRef {
111 113
 			usedDigests[imageDgst] = struct{}{}
112
-			continue
113 114
 		}
114 115
 
115 116
 		ref, err := reference.ParseNormalizedNamed(ctr.Config.Image)
116 117
 		log.G(ctx).WithFields(log.Fields{
117 118
 			"ctr":          ctr.ID,
118
-			"image":        ref,
119
+			"imageRef":     ref,
120
+			"imageID":      imageDgst,
119 121
 			"nameParseErr": err,
120 122
 		}).Debug("filtering container's image")
121
-
122 123
 		if err == nil {
123 124
 			// If user provided a specific image name, exclude that image.
124 125
 			name := reference.TagNameOnly(ref)
125 126
 			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
132 127
 
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
-				}
128
+			// Also exclude repo:tag image if repo:tag@sha256:digest reference was used.
129
+			_, isDigested := name.(reference.Digested)
130
+			tagged, isTagged := name.(reference.NamedTagged)
131
+			if isDigested && isTagged {
132
+				named, _ := reference.ParseNormalizedNamed(tagged.Name())
133
+				namedTagged, _ := reference.WithTag(named, tagged.Tag())
134
+				delete(imagesToPrune, namedTagged.String())
139 135
 			}
140 136
 		}
141 137
 	}
142 138
 
139
+	return usedDigests
140
+}
141
+
142
+// pruneAll deletes all images in the imagesToPrune map.
143
+func (i *ImageService) pruneAll(ctx context.Context, imagesToPrune map[string]containerdimages.Image) (*image.PruneReport, error) {
144
+	report := image.PruneReport{}
145
+
143 146
 	possiblyDeletedConfigs := map[digest.Digest]struct{}{}
144 147
 	var errs error
145 148
 
... ...
@@ -4,11 +4,16 @@ import (
4 4
 	"strings"
5 5
 	"testing"
6 6
 
7
+	containertypes "github.com/docker/docker/api/types/container"
7 8
 	"github.com/docker/docker/api/types/filters"
9
+	"github.com/docker/docker/api/types/image"
10
+	"github.com/docker/docker/client"
8 11
 	"github.com/docker/docker/integration/internal/container"
9 12
 	"github.com/docker/docker/internal/testutils/specialimage"
13
+	"github.com/docker/docker/testutil"
10 14
 	"github.com/docker/docker/testutil/daemon"
11 15
 	"gotest.tools/v3/assert"
16
+	is "gotest.tools/v3/assert/cmp"
12 17
 	"gotest.tools/v3/skip"
13 18
 )
14 19
 
... ...
@@ -47,3 +52,132 @@ func TestPruneDontDeleteUsedDangling(t *testing.T) {
47 47
 	_, _, err = client.ImageInspectWithRaw(ctx, danglingID)
48 48
 	assert.NilError(t, err, "Test dangling image should still exist")
49 49
 }
50
+
51
+// Regression test for https://github.com/moby/moby/issues/48063
52
+func TestPruneDontDeleteUsedImage(t *testing.T) {
53
+	skip.If(t, testEnv.DaemonInfo.OSType == "windows", "cannot start multiple daemons on windows")
54
+	skip.If(t, testEnv.IsRemoteDaemon, "cannot run daemon when remote daemon")
55
+
56
+	ctx := setupTest(t)
57
+
58
+	for _, env := range []struct {
59
+		name    string
60
+		prepare func(t *testing.T, client *daemon.Daemon, apiClient *client.Client) error
61
+		check   func(t *testing.T, apiClient *client.Client, pruned image.PruneReport)
62
+	}{
63
+		{
64
+			// Container uses the busybox:latest image and it's the only image
65
+			// tag with the same target.
66
+			name: "single tag",
67
+			check: func(t *testing.T, apiClient *client.Client, pruned image.PruneReport) {
68
+				assert.Check(t, is.Len(pruned.ImagesDeleted, 0))
69
+
70
+				_, _, err := apiClient.ImageInspectWithRaw(ctx, "busybox:latest")
71
+				assert.NilError(t, err, "Busybox image should still exist")
72
+			},
73
+		},
74
+		{
75
+			// Container uses the busybox:latest image and there's also a second
76
+			// busybox:other tag pointing to the same image.
77
+			name: "two tags",
78
+			prepare: func(t *testing.T, d *daemon.Daemon, apiClient *client.Client) error {
79
+				return apiClient.ImageTag(ctx, "busybox:latest", "busybox:other")
80
+			},
81
+			check: func(t *testing.T, apiClient *client.Client, pruned image.PruneReport) {
82
+				if assert.Check(t, is.Len(pruned.ImagesDeleted, 1)) {
83
+					assert.Check(t, is.Equal(pruned.ImagesDeleted[0].Deleted, ""))
84
+					t.Log("Untagged", pruned.ImagesDeleted[0].Untagged)
85
+				}
86
+
87
+				_, _, err1 := apiClient.ImageInspectWithRaw(ctx, "busybox:latest")
88
+				_, _, err2 := apiClient.ImageInspectWithRaw(ctx, "busybox:other")
89
+
90
+				assert.Check(t, err1 != err2 && (err1 == nil || err2 == nil), "One of the images should still exist")
91
+			},
92
+		},
93
+	} {
94
+		for _, tc := range []struct {
95
+			name    string
96
+			imageID func(t *testing.T, inspect image.InspectResponse) string
97
+		}{
98
+			{
99
+				name: "full id",
100
+				imageID: func(t *testing.T, inspect image.InspectResponse) string {
101
+					return inspect.ID
102
+				},
103
+			},
104
+			{
105
+				name: "full id without sha256 prefix",
106
+				imageID: func(t *testing.T, inspect image.InspectResponse) string {
107
+					return strings.TrimPrefix(inspect.ID, "sha256:")
108
+				},
109
+			},
110
+			{
111
+				name: "truncated id (without sha256 prefix)",
112
+				imageID: func(t *testing.T, inspect image.InspectResponse) string {
113
+					return strings.TrimPrefix(inspect.ID, "sha256:")[:8]
114
+				},
115
+			},
116
+			{
117
+				name: "repo and digest without tag",
118
+				imageID: func(t *testing.T, inspect image.InspectResponse) string {
119
+					skip.If(t, !testEnv.UsingSnapshotter())
120
+
121
+					return "busybox@" + inspect.ID
122
+				},
123
+			},
124
+			{
125
+				name: "tagged and digested",
126
+				imageID: func(t *testing.T, inspect image.InspectResponse) string {
127
+					skip.If(t, !testEnv.UsingSnapshotter())
128
+
129
+					return "busybox:latest@" + inspect.ID
130
+				},
131
+			},
132
+			{
133
+				name: "repo digest",
134
+				imageID: func(t *testing.T, inspect image.InspectResponse) string {
135
+					// graphdriver won't have a repo digest
136
+					skip.If(t, len(inspect.RepoDigests) == 0, "no repo digest")
137
+
138
+					return inspect.RepoDigests[0]
139
+				},
140
+			},
141
+		} {
142
+			tc := tc
143
+			t.Run(env.name+"/"+tc.name, func(t *testing.T) {
144
+				ctx := testutil.StartSpan(ctx, t)
145
+				d := daemon.New(t)
146
+				d.Start(t)
147
+				defer d.Stop(t)
148
+
149
+				apiClient := d.NewClientT(t)
150
+				defer apiClient.Close()
151
+
152
+				d.LoadBusybox(ctx, t)
153
+
154
+				if env.prepare != nil {
155
+					err := env.prepare(t, d, apiClient)
156
+					assert.NilError(t, err, "prepare failed")
157
+				}
158
+
159
+				inspect, _, err := apiClient.ImageInspectWithRaw(ctx, "busybox:latest")
160
+				assert.NilError(t, err)
161
+
162
+				image := tc.imageID(t, inspect)
163
+				t.Log(image)
164
+
165
+				cid := container.Run(ctx, t, apiClient,
166
+					container.WithImage(image),
167
+					container.WithCmd("sleep", "60"))
168
+				defer container.Remove(ctx, t, apiClient, cid, containertypes.RemoveOptions{Force: true})
169
+
170
+				// dangling=false also prunes unused images
171
+				pruned, err := apiClient.ImagesPrune(ctx, filters.NewArgs(filters.Arg("dangling", "false")))
172
+				assert.NilError(t, err)
173
+
174
+				env.check(t, apiClient, pruned)
175
+			})
176
+		}
177
+	}
178
+}