Browse code

Update cleanup logic to use resolve all images

Ensure that when removing an image, an image is checked consistently
against the images with the same target digest. Add unit testing around
delete.

Signed-off-by: Derek McGowan <derek@mcg.dev>

Derek McGowan authored on 2023/11/17 14:55:54
Showing 10 changed files
... ...
@@ -12,8 +12,7 @@ import (
12 12
 // walkPresentChildren is a simple wrapper for containerdimages.Walk with presentChildrenHandler.
13 13
 // This is only a convenient helper to reduce boilerplate.
14 14
 func (i *ImageService) walkPresentChildren(ctx context.Context, target ocispec.Descriptor, f func(context.Context, ocispec.Descriptor) error) error {
15
-	store := i.client.ContentStore()
16
-	return containerdimages.Walk(ctx, presentChildrenHandler(store, containerdimages.HandlerFunc(
15
+	return containerdimages.Walk(ctx, presentChildrenHandler(i.content, containerdimages.HandlerFunc(
17 16
 		func(ctx context.Context, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
18 17
 			return nil, f(ctx, desc)
19 18
 		})), target)
... ...
@@ -43,8 +43,6 @@ func (i *ImageService) GetImage(ctx context.Context, refOrID string, options ima
43 43
 		platform = cplatforms.OnlyStrict(*options.Platform)
44 44
 	}
45 45
 
46
-	cs := i.client.ContentStore()
47
-
48 46
 	var presentImages []imagespec.DockerOCIImage
49 47
 	err = i.walkImageManifests(ctx, desc, func(img *ImageManifest) error {
50 48
 		conf, err := img.Config(ctx)
... ...
@@ -59,7 +57,7 @@ func (i *ImageService) GetImage(ctx context.Context, refOrID string, options ima
59 59
 		}
60 60
 
61 61
 		var ociimage imagespec.DockerOCIImage
62
-		if err := readConfig(ctx, cs, conf, &ociimage); err != nil {
62
+		if err := readConfig(ctx, i.content, conf, &ociimage); err != nil {
63 63
 			if cerrdefs.IsNotFound(err) {
64 64
 				log.G(ctx).WithFields(log.Fields{
65 65
 					"manifestDescriptor": img.Target(),
... ...
@@ -101,7 +99,7 @@ func (i *ImageService) GetImage(ctx context.Context, refOrID string, options ima
101 101
 			return nil, err
102 102
 		}
103 103
 
104
-		tagged, err := i.client.ImageService().List(ctx, "target.digest=="+desc.Target.Digest.String())
104
+		tagged, err := i.images.List(ctx, "target.digest=="+desc.Target.Digest.String())
105 105
 		if err != nil {
106 106
 			return nil, err
107 107
 		}
... ...
@@ -266,11 +264,9 @@ func (i *ImageService) resolveImage(ctx context.Context, refOrID string) (contai
266 266
 		return containerdimages.Image{}, errdefs.InvalidParameter(err)
267 267
 	}
268 268
 
269
-	is := i.client.ImageService()
270
-
271 269
 	digested, ok := parsed.(reference.Digested)
272 270
 	if ok {
273
-		imgs, err := is.List(ctx, "target.digest=="+digested.Digest().String())
271
+		imgs, err := i.images.List(ctx, "target.digest=="+digested.Digest().String())
274 272
 		if err != nil {
275 273
 			return containerdimages.Image{}, errors.Wrap(err, "failed to lookup digest")
276 274
 		}
... ...
@@ -300,7 +296,7 @@ func (i *ImageService) resolveImage(ctx context.Context, refOrID string) (contai
300 300
 	}
301 301
 
302 302
 	ref := reference.TagNameOnly(parsed.(reference.Named)).String()
303
-	img, err := is.Get(ctx, ref)
303
+	img, err := i.images.Get(ctx, ref)
304 304
 	if err == nil {
305 305
 		return img, nil
306 306
 	} else {
... ...
@@ -317,7 +313,7 @@ func (i *ImageService) resolveImage(ctx context.Context, refOrID string) (contai
317 317
 			fmt.Sprintf("name==%q", ref), // Or it could just look like one.
318 318
 			"target.digest~=" + strconv.Quote(fmt.Sprintf(`^sha256:%s[0-9a-fA-F]{%d}$`, regexp.QuoteMeta(idWithoutAlgo), 64-len(idWithoutAlgo))),
319 319
 		}
320
-		imgs, err := is.List(ctx, filters...)
320
+		imgs, err := i.images.List(ctx, filters...)
321 321
 		if err != nil {
322 322
 			return containerdimages.Image{}, err
323 323
 		}
... ...
@@ -17,7 +17,7 @@ import (
17 17
 
18 18
 // Children returns a slice of image IDs that are children of the `id` image
19 19
 func (i *ImageService) Children(ctx context.Context, id image.ID) ([]image.ID, error) {
20
-	imgs, err := i.client.ImageService().List(ctx, "labels."+imageLabelClassicBuilderParent+"=="+string(id))
20
+	imgs, err := i.images.List(ctx, "labels."+imageLabelClassicBuilderParent+"=="+string(id))
21 21
 	if err != nil {
22 22
 		return []image.ID{}, errdefs.System(errors.Wrap(err, "failed to list all images"))
23 23
 	}
... ...
@@ -88,16 +88,14 @@ func (i *ImageService) parents(ctx context.Context, id image.ID) ([]imageWithRoo
88 88
 		return nil, errors.Wrap(err, "failed to get child image")
89 89
 	}
90 90
 
91
-	cs := i.client.ContentStore()
92
-
93
-	allPlatforms, err := containerdimages.Platforms(ctx, cs, target)
91
+	allPlatforms, err := containerdimages.Platforms(ctx, i.content, target)
94 92
 	if err != nil {
95 93
 		return nil, errdefs.System(errors.Wrap(err, "failed to list platforms supported by image"))
96 94
 	}
97 95
 
98 96
 	var childRootFS []ocispec.RootFS
99 97
 	for _, platform := range allPlatforms {
100
-		rootfs, err := platformRootfs(ctx, cs, target, platform)
98
+		rootfs, err := platformRootfs(ctx, i.content, target, platform)
101 99
 		if err != nil {
102 100
 			if cerrdefs.IsNotFound(err) {
103 101
 				continue
... ...
@@ -108,7 +106,7 @@ func (i *ImageService) parents(ctx context.Context, id image.ID) ([]imageWithRoo
108 108
 		childRootFS = append(childRootFS, rootfs)
109 109
 	}
110 110
 
111
-	imgs, err := i.client.ImageService().List(ctx)
111
+	imgs, err := i.images.List(ctx)
112 112
 	if err != nil {
113 113
 		return nil, errdefs.System(errors.Wrap(err, "failed to list all images"))
114 114
 	}
... ...
@@ -117,7 +115,7 @@ func (i *ImageService) parents(ctx context.Context, id image.ID) ([]imageWithRoo
117 117
 	for _, img := range imgs {
118 118
 	nextImage:
119 119
 		for _, platform := range allPlatforms {
120
-			rootfs, err := platformRootfs(ctx, cs, img.Target, platform)
120
+			rootfs, err := platformRootfs(ctx, i.content, img.Target, platform)
121 121
 			if err != nil {
122 122
 				if cerrdefs.IsNotFound(err) {
123 123
 					continue
... ...
@@ -158,7 +156,7 @@ func (i *ImageService) getParentsByBuilderLabel(ctx context.Context, img contain
158 158
 		return nil, nil
159 159
 	}
160 160
 
161
-	return i.client.ImageService().List(ctx, "target.digest=="+dgst.String())
161
+	return i.images.List(ctx, "target.digest=="+dgst.String())
162 162
 }
163 163
 
164 164
 type imageWithRootfs struct {
... ...
@@ -53,123 +53,161 @@ import (
53 53
 //
54 54
 // TODO(thaJeztah): image delete should send prometheus counters; see https://github.com/moby/moby/issues/45268
55 55
 func (i *ImageService) ImageDelete(ctx context.Context, imageRef string, force, prune bool) ([]imagetypes.DeleteResponse, error) {
56
-	parsedRef, err := reference.ParseNormalizedNamed(imageRef)
57
-	if err != nil {
58
-		return nil, err
56
+	var c conflictType
57
+	if !force {
58
+		c |= conflictSoft
59 59
 	}
60 60
 
61
-	img, err := i.resolveImage(ctx, imageRef)
61
+	img, all, err := i.resolveAllReferences(ctx, imageRef)
62 62
 	if err != nil {
63 63
 		return nil, err
64 64
 	}
65 65
 
66
-	imgID := image.ID(img.Target.Digest)
67
-
68
-	explicitDanglingRef := strings.HasPrefix(imageRef, imageNameDanglingPrefix) && isDanglingImage(img)
69
-	if isImageIDPrefix(imgID.String(), imageRef) || explicitDanglingRef {
70
-		return i.deleteAll(ctx, img, force, prune)
71
-	}
72
-
73
-	singleRef, err := i.isSingleReference(ctx, img)
74
-	if err != nil {
75
-		return nil, err
76
-	}
77
-	if !singleRef {
78
-		err := i.client.ImageService().Delete(ctx, img.Name)
66
+	var imgID image.ID
67
+	if img == nil {
68
+		if len(all) == 0 {
69
+			parsed, _ := reference.ParseAnyReference(imageRef)
70
+			return nil, dimages.ErrImageDoesNotExist{Ref: parsed}
71
+		}
72
+		imgID = image.ID(all[0].Target.Digest)
73
+		sameRef, err := i.getSameReferences(ctx, nil, all)
79 74
 		if err != nil {
80 75
 			return nil, err
81 76
 		}
82
-		i.LogImageEvent(imgID.String(), imgID.String(), events.ActionUnTag)
83
-		records := []imagetypes.DeleteResponse{{Untagged: reference.FamiliarString(reference.TagNameOnly(parsedRef))}}
84
-		return records, nil
85
-	}
86 77
 
87
-	using := func(c *container.Container) bool {
88
-		return c.ImageID == imgID
89
-	}
90
-	ctr := i.containers.First(using)
91
-	if ctr != nil {
92
-		if !force {
93
-			// If we removed the repository reference then
94
-			// this image would remain "dangling" and since
95
-			// we really want to avoid that the client must
96
-			// explicitly force its removal.
97
-			refString := reference.FamiliarString(reference.TagNameOnly(parsedRef))
98
-			err := &imageDeleteConflict{
99
-				reference: refString,
100
-				used:      true,
101
-				message: fmt.Sprintf("container %s is using its referenced image %s",
102
-					stringid.TruncateID(ctr.ID),
103
-					stringid.TruncateID(imgID.String())),
104
-			}
78
+		if len(sameRef) == len(all) && !force {
79
+			c &= ^conflictActiveReference
80
+		}
81
+	} else {
82
+		imgID = image.ID(img.Target.Digest)
83
+		explicitDanglingRef := strings.HasPrefix(imageRef, imageNameDanglingPrefix) && isDanglingImage(*img)
84
+		if isImageIDPrefix(imgID.String(), imageRef) || explicitDanglingRef {
85
+			return i.deleteAll(ctx, imgID, all, c, prune)
86
+		}
87
+		parsedRef, err := reference.ParseNormalizedNamed(img.Name)
88
+		if err != nil {
105 89
 			return nil, err
106 90
 		}
107 91
 
108
-		err := i.softImageDelete(ctx, img)
92
+		sameRef, err := i.getSameReferences(ctx, img, all)
109 93
 		if err != nil {
110 94
 			return nil, err
111 95
 		}
96
+		if len(sameRef) != len(all) {
97
+			var records []imagetypes.DeleteResponse
98
+			for _, ref := range sameRef {
99
+				// TODO: Add with target
100
+				err := i.images.Delete(ctx, ref.Name)
101
+				if err != nil {
102
+					return nil, err
103
+				}
104
+				if nn, err := reference.ParseNormalizedNamed(ref.Name); err == nil {
105
+					familiarRef := reference.FamiliarString(nn)
106
+					i.logImageEvent(ref, familiarRef, events.ActionUnTag)
107
+					records = append(records, imagetypes.DeleteResponse{Untagged: familiarRef})
108
+				}
109
+			}
110
+			return records, nil
111
+		} else if !force {
112
+			// Since only a single used reference, remove all active
113
+			// TODO: Consider keeping the conflict and changing active
114
+			// reference calculation in image checker.
115
+			c &= ^conflictActiveReference
116
+		}
117
+
118
+		using := func(c *container.Container) bool {
119
+			return c.ImageID == imgID
120
+		}
121
+		ctr := i.containers.First(using)
122
+		if ctr != nil {
123
+			familiarRef := reference.FamiliarString(parsedRef)
124
+			if !force {
125
+				// If we removed the repository reference then
126
+				// this image would remain "dangling" and since
127
+				// we really want to avoid that the client must
128
+				// explicitly force its removal.
129
+				err := &imageDeleteConflict{
130
+					reference: familiarRef,
131
+					used:      true,
132
+					message: fmt.Sprintf("container %s is using its referenced image %s",
133
+						stringid.TruncateID(ctr.ID),
134
+						stringid.TruncateID(imgID.String())),
135
+				}
136
+				return nil, err
137
+			}
138
+
139
+			// Delete all images
140
+			err := i.softImageDelete(ctx, *img, all)
141
+			if err != nil {
142
+				return nil, err
143
+			}
112 144
 
113
-		i.LogImageEvent(imgID.String(), imgID.String(), events.ActionUnTag)
114
-		records := []imagetypes.DeleteResponse{{Untagged: reference.FamiliarString(reference.TagNameOnly(parsedRef))}}
115
-		return records, nil
145
+			i.logImageEvent(*img, familiarRef, events.ActionUnTag)
146
+			records := []imagetypes.DeleteResponse{{Untagged: familiarRef}}
147
+			return records, nil
148
+		}
116 149
 	}
117 150
 
118
-	return i.deleteAll(ctx, img, force, prune)
151
+	return i.deleteAll(ctx, imgID, all, c, prune)
119 152
 }
120 153
 
121 154
 // deleteAll deletes the image from the daemon, and if prune is true,
122 155
 // also deletes dangling parents if there is no conflict in doing so.
123 156
 // Parent images are removed quietly, and if there is any issue/conflict
124 157
 // it is logged but does not halt execution/an error is not returned.
125
-func (i *ImageService) deleteAll(ctx context.Context, img images.Image, force, prune bool) ([]imagetypes.DeleteResponse, error) {
126
-	var records []imagetypes.DeleteResponse
127
-
158
+func (i *ImageService) deleteAll(ctx context.Context, imgID image.ID, all []images.Image, c conflictType, prune bool) (records []imagetypes.DeleteResponse, err error) {
128 159
 	// Workaround for: https://github.com/moby/buildkit/issues/3797
129 160
 	possiblyDeletedConfigs := map[digest.Digest]struct{}{}
130
-	err := i.walkPresentChildren(ctx, img.Target, func(_ context.Context, d ocispec.Descriptor) error {
131
-		if images.IsConfigType(d.MediaType) {
132
-			possiblyDeletedConfigs[d.Digest] = struct{}{}
161
+	if len(all) > 0 && i.content != nil {
162
+		handled := map[digest.Digest]struct{}{}
163
+		for _, img := range all {
164
+			if _, ok := handled[img.Target.Digest]; ok {
165
+				continue
166
+			} else {
167
+				handled[img.Target.Digest] = struct{}{}
168
+			}
169
+			err := i.walkPresentChildren(ctx, img.Target, func(_ context.Context, d ocispec.Descriptor) error {
170
+				if images.IsConfigType(d.MediaType) {
171
+					possiblyDeletedConfigs[d.Digest] = struct{}{}
172
+				}
173
+				return nil
174
+			})
175
+			if err != nil {
176
+				return nil, err
177
+			}
133 178
 		}
134
-		return nil
135
-	})
136
-	if err != nil {
137
-		return nil, err
138 179
 	}
139 180
 	defer func() {
140
-		if err := i.unleaseSnapshotsFromDeletedConfigs(compatcontext.WithoutCancel(ctx), possiblyDeletedConfigs); err != nil {
141
-			log.G(ctx).WithError(err).Warn("failed to unlease snapshots")
181
+		if len(possiblyDeletedConfigs) > 0 {
182
+			if err := i.unleaseSnapshotsFromDeletedConfigs(compatcontext.WithoutCancel(ctx), possiblyDeletedConfigs); err != nil {
183
+				log.G(ctx).WithError(err).Warn("failed to unlease snapshots")
184
+			}
142 185
 		}
143 186
 	}()
144 187
 
145
-	imgID := img.Target.Digest.String()
146
-
147 188
 	var parents []imageWithRootfs
148 189
 	if prune {
149
-		parents, err = i.parents(ctx, image.ID(imgID))
190
+		// TODO(dmcgowan): Consider using GC labels to walk for deletion
191
+		parents, err = i.parents(ctx, imgID)
150 192
 		if err != nil {
151 193
 			log.G(ctx).WithError(err).Warn("failed to get image parents")
152 194
 		}
153 195
 		sortParentsByAffinity(parents)
154 196
 	}
155 197
 
156
-	imageRefs, err := i.client.ImageService().List(ctx, "target.digest=="+imgID)
157
-	if err != nil {
158
-		return nil, err
159
-	}
160
-	for _, imageRef := range imageRefs {
161
-		if err := i.imageDeleteHelper(ctx, imageRef, &records, force); err != nil {
198
+	for _, imageRef := range all {
199
+		if err := i.imageDeleteHelper(ctx, imageRef, all, &records, c); err != nil {
162 200
 			return records, err
163 201
 		}
164 202
 	}
165
-	i.LogImageEvent(imgID, imgID, events.ActionDelete)
166
-	records = append(records, imagetypes.DeleteResponse{Deleted: imgID})
203
+	i.LogImageEvent(imgID.String(), imgID.String(), events.ActionDelete)
204
+	records = append(records, imagetypes.DeleteResponse{Deleted: imgID.String()})
167 205
 
168 206
 	for _, parent := range parents {
169 207
 		if !isDanglingImage(parent.img) {
170 208
 			break
171 209
 		}
172
-		err = i.imageDeleteHelper(ctx, parent.img, &records, false)
210
+		err = i.imageDeleteHelper(ctx, parent.img, all, &records, conflictSoft)
173 211
 		if err != nil {
174 212
 			log.G(ctx).WithError(err).Warn("failed to remove image parent")
175 213
 			break
... ...
@@ -205,19 +243,71 @@ func sortParentsByAffinity(parents []imageWithRootfs) {
205 205
 	})
206 206
 }
207 207
 
208
-// isSingleReference returns true if there are no other images in the
209
-// daemon targeting the same content as `img` that are not dangling.
210
-func (i *ImageService) isSingleReference(ctx context.Context, img images.Image) (bool, error) {
211
-	refs, err := i.client.ImageService().List(ctx, "target.digest=="+img.Target.Digest.String())
212
-	if err != nil {
213
-		return false, err
208
+// getSameReferences returns the set of images which are the same as:
209
+// - the provided img if non-nil
210
+// - OR the first named image found in the provided image set
211
+// - OR the full set of provided images if no named references in the set
212
+//
213
+// References are considered the same if:
214
+// - Both contain the same name and tag
215
+// - Both contain the same name, one is untagged and no other differing tags in set
216
+// - One is dangling
217
+//
218
+// Note: All imgs should have the same target, only the image name will be considered
219
+// for determining whether images are the same.
220
+func (i *ImageService) getSameReferences(ctx context.Context, img *images.Image, imgs []images.Image) ([]images.Image, error) {
221
+	var (
222
+		named      reference.Named
223
+		tag        string
224
+		sameRef    []images.Image
225
+		digestRefs = []images.Image{}
226
+	)
227
+	if img != nil {
228
+		repoRef, err := reference.ParseNamed(img.Name)
229
+		if err != nil {
230
+			return nil, err
231
+		}
232
+		named = repoRef
233
+		if tagged, ok := named.(reference.Tagged); ok {
234
+			tag = tagged.Tag()
235
+		}
214 236
 	}
215
-	for _, ref := range refs {
216
-		if !isDanglingImage(ref) && ref.Name != img.Name {
217
-			return false, nil
237
+	for _, ref := range imgs {
238
+		if !isDanglingImage(ref) {
239
+			if repoRef, err := reference.ParseNamed(ref.Name); err == nil {
240
+				if named == nil {
241
+					named = repoRef
242
+					if tagged, ok := named.(reference.Tagged); ok {
243
+						tag = tagged.Tag()
244
+					}
245
+				} else if named.Name() != repoRef.Name() {
246
+					continue
247
+				} else if tagged, ok := repoRef.(reference.Tagged); ok {
248
+					if tag == "" {
249
+						tag = tagged.Tag()
250
+					} else if tag != tagged.Tag() {
251
+						// Same repo, different tag, do not include digest refs
252
+						digestRefs = nil
253
+						continue
254
+					}
255
+				} else {
256
+					if digestRefs != nil {
257
+						digestRefs = append(digestRefs, ref)
258
+					}
259
+					// Add digest refs at end if no other tags in the same name
260
+					continue
261
+				}
262
+			} else {
263
+				// Ignore names which do not parse
264
+				log.G(ctx).WithError(err).WithField("image", ref.Name).Info("failed to parse image name, ignoring")
265
+			}
218 266
 		}
267
+		sameRef = append(sameRef, ref)
268
+	}
269
+	if digestRefs != nil {
270
+		sameRef = append(sameRef, digestRefs...)
219 271
 	}
220
-	return true, nil
272
+	return sameRef, nil
221 273
 }
222 274
 
223 275
 type conflictType int
... ...
@@ -238,17 +328,14 @@ const (
238 238
 // images and untagged references are appended to the given records. If any
239 239
 // error or conflict is encountered, it will be returned immediately without
240 240
 // deleting the image.
241
-func (i *ImageService) imageDeleteHelper(ctx context.Context, img images.Image, records *[]imagetypes.DeleteResponse, force bool) error {
241
+func (i *ImageService) imageDeleteHelper(ctx context.Context, img images.Image, all []images.Image, records *[]imagetypes.DeleteResponse, extra conflictType) error {
242 242
 	// First, determine if this image has any conflicts. Ignore soft conflicts
243 243
 	// if force is true.
244
-	c := conflictHard
245
-	if !force {
246
-		c |= conflictSoft
247
-	}
244
+	c := conflictHard | extra
248 245
 
249 246
 	imgID := image.ID(img.Target.Digest)
250 247
 
251
-	err := i.checkImageDeleteConflict(ctx, imgID, c)
248
+	err := i.checkImageDeleteConflict(ctx, imgID, all, c)
252 249
 	if err != nil {
253 250
 		return err
254 251
 	}
... ...
@@ -257,13 +344,15 @@ func (i *ImageService) imageDeleteHelper(ctx context.Context, img images.Image,
257 257
 	if err != nil {
258 258
 		return err
259 259
 	}
260
-	err = i.client.ImageService().Delete(ctx, img.Name, images.SynchronousDelete())
260
+
261
+	// TODO: Add target option
262
+	err = i.images.Delete(ctx, img.Name, images.SynchronousDelete())
261 263
 	if err != nil {
262 264
 		return err
263 265
 	}
264 266
 
265 267
 	if !isDanglingImage(img) {
266
-		i.LogImageEvent(imgID.String(), imgID.String(), events.ActionUnTag)
268
+		i.logImageEvent(img, reference.FamiliarString(untaggedRef), events.ActionUnTag)
267 269
 		*records = append(*records, imagetypes.DeleteResponse{Untagged: reference.FamiliarString(untaggedRef)})
268 270
 	}
269 271
 
... ...
@@ -299,7 +388,7 @@ func (imageDeleteConflict) Conflict() {}
299 299
 // nil if there are none. It takes a bitmask representing a
300 300
 // filter for which conflict types the caller cares about,
301 301
 // and will only check for these conflict types.
302
-func (i *ImageService) checkImageDeleteConflict(ctx context.Context, imgID image.ID, mask conflictType) error {
302
+func (i *ImageService) checkImageDeleteConflict(ctx context.Context, imgID image.ID, all []images.Image, mask conflictType) error {
303 303
 	if mask&conflictRunningContainer != 0 {
304 304
 		running := func(c *container.Container) bool {
305 305
 			return c.ImageID == imgID && c.IsRunning()
... ...
@@ -328,11 +417,8 @@ func (i *ImageService) checkImageDeleteConflict(ctx context.Context, imgID image
328 328
 	}
329 329
 
330 330
 	if mask&conflictActiveReference != 0 {
331
-		refs, err := i.client.ImageService().List(ctx, "target.digest=="+imgID.String())
332
-		if err != nil {
333
-			return err
334
-		}
335
-		if len(refs) > 1 {
331
+		// TODO: Count unexpired references...
332
+		if len(all) > 1 {
336 333
 			return &imageDeleteConflict{
337 334
 				reference: stringid.TruncateID(imgID.String()),
338 335
 				message:   "image is referenced in multiple repositories",
339 336
new file mode 100644
... ...
@@ -0,0 +1,218 @@
0
+package containerd
1
+
2
+import (
3
+	"context"
4
+	"testing"
5
+
6
+	"github.com/containerd/containerd/images"
7
+	"github.com/containerd/containerd/metadata"
8
+	"github.com/containerd/containerd/namespaces"
9
+	"github.com/containerd/log/logtest"
10
+	"github.com/docker/docker/container"
11
+	daemonevents "github.com/docker/docker/daemon/events"
12
+	dimages "github.com/docker/docker/daemon/images"
13
+	"gotest.tools/v3/assert"
14
+	is "gotest.tools/v3/assert/cmp"
15
+)
16
+
17
+func TestImageDelete(t *testing.T) {
18
+	ctx := namespaces.WithNamespace(context.TODO(), "testing")
19
+
20
+	for _, tc := range []struct {
21
+		ref       string
22
+		starting  []images.Image
23
+		remaining []images.Image
24
+		err       error
25
+		// TODO: Records
26
+		// TODO: Containers
27
+		// TODO: Events
28
+	}{
29
+		{
30
+			ref: "nothingthere",
31
+			err: dimages.ErrImageDoesNotExist{Ref: nameTag("nothingthere", "latest")},
32
+		},
33
+		{
34
+			ref: "justoneimage",
35
+			starting: []images.Image{
36
+				{
37
+					Name:   "docker.io/library/justoneimage:latest",
38
+					Target: desc(10),
39
+				},
40
+			},
41
+		},
42
+		{
43
+			ref: "justoneref",
44
+			starting: []images.Image{
45
+				{
46
+					Name:   "docker.io/library/justoneref:latest",
47
+					Target: desc(10),
48
+				},
49
+				{
50
+					Name:   "docker.io/library/differentrepo:latest",
51
+					Target: desc(10),
52
+				},
53
+			},
54
+			remaining: []images.Image{
55
+				{
56
+					Name:   "docker.io/library/differentrepo:latest",
57
+					Target: desc(10),
58
+				},
59
+			},
60
+		},
61
+		{
62
+			ref: "hasdigest",
63
+			starting: []images.Image{
64
+				{
65
+					Name:   "docker.io/library/hasdigest:latest",
66
+					Target: desc(10),
67
+				},
68
+				{
69
+					Name:   "docker.io/library/hasdigest@" + digestFor(10).String(),
70
+					Target: desc(10),
71
+				},
72
+			},
73
+		},
74
+		{
75
+			ref: digestFor(11).String(),
76
+			starting: []images.Image{
77
+				{
78
+					Name:   "docker.io/library/byid:latest",
79
+					Target: desc(11),
80
+				},
81
+				{
82
+					Name:   "docker.io/library/byid@" + digestFor(11).String(),
83
+					Target: desc(11),
84
+				},
85
+			},
86
+		},
87
+		{
88
+			ref: "bydigest@" + digestFor(12).String(),
89
+			starting: []images.Image{
90
+				{
91
+					Name:   "docker.io/library/bydigest:latest",
92
+					Target: desc(12),
93
+				},
94
+				{
95
+					Name:   "docker.io/library/bydigest@" + digestFor(12).String(),
96
+					Target: desc(12),
97
+				},
98
+			},
99
+		},
100
+		{
101
+			ref: "onerefoftwo",
102
+			starting: []images.Image{
103
+				{
104
+					Name:   "docker.io/library/onerefoftwo:latest",
105
+					Target: desc(12),
106
+				},
107
+				{
108
+					Name:   "docker.io/library/onerefoftwo:other",
109
+					Target: desc(12),
110
+				},
111
+				{
112
+					Name:   "docker.io/library/onerefoftwo@" + digestFor(12).String(),
113
+					Target: desc(12),
114
+				},
115
+			},
116
+			remaining: []images.Image{
117
+				{
118
+					Name:   "docker.io/library/onerefoftwo:other",
119
+					Target: desc(12),
120
+				},
121
+				{
122
+					Name:   "docker.io/library/onerefoftwo@" + digestFor(12).String(),
123
+					Target: desc(12),
124
+				},
125
+			},
126
+		},
127
+		{
128
+			ref: "otherreporemaining",
129
+			starting: []images.Image{
130
+				{
131
+					Name:   "docker.io/library/otherreporemaining:latest",
132
+					Target: desc(12),
133
+				},
134
+				{
135
+					Name:   "docker.io/library/otherreporemaining@" + digestFor(12).String(),
136
+					Target: desc(12),
137
+				},
138
+				{
139
+					Name:   "docker.io/library/someotherrepo:latest",
140
+					Target: desc(12),
141
+				},
142
+			},
143
+			remaining: []images.Image{
144
+				{
145
+					Name:   "docker.io/library/someotherrepo:latest",
146
+					Target: desc(12),
147
+				},
148
+			},
149
+		},
150
+	} {
151
+		tc := tc
152
+		t.Run(tc.ref, func(t *testing.T) {
153
+			t.Parallel()
154
+			ctx := logtest.WithT(ctx, t)
155
+			mdb := newTestDB(ctx, t)
156
+			service := &ImageService{
157
+				images:        metadata.NewImageStore(mdb),
158
+				containers:    emptyTestContainerStore(),
159
+				eventsService: daemonevents.New(),
160
+			}
161
+			for _, img := range tc.starting {
162
+				if _, err := service.images.Create(ctx, img); err != nil {
163
+					t.Fatalf("failed to create image %q: %v", img.Name, err)
164
+				}
165
+			}
166
+
167
+			_, err := service.ImageDelete(ctx, tc.ref, false, false)
168
+			if tc.err == nil {
169
+				assert.NilError(t, err)
170
+			} else {
171
+				assert.Error(t, err, tc.err.Error())
172
+			}
173
+
174
+			all, err := service.images.List(ctx)
175
+			if err != nil {
176
+				t.Fatal(err)
177
+			}
178
+			assert.Equal(t, len(tc.remaining), len(all))
179
+
180
+			// Order should match
181
+			for i := range all {
182
+				assert.Check(t, is.Equal(all[i].Name, tc.remaining[i].Name), "image[%d]", i)
183
+				assert.Check(t, is.Equal(all[i].Target.Digest, tc.remaining[i].Target.Digest), "image[%d]", i)
184
+				// TODO: Check labels too
185
+			}
186
+		})
187
+	}
188
+
189
+}
190
+
191
+type testContainerStore struct{}
192
+
193
+func emptyTestContainerStore() container.Store {
194
+	return &testContainerStore{}
195
+}
196
+
197
+func (*testContainerStore) Add(string, *container.Container) {}
198
+
199
+func (*testContainerStore) Get(string) *container.Container {
200
+	return nil
201
+}
202
+
203
+func (*testContainerStore) Delete(string) {}
204
+
205
+func (*testContainerStore) List() []*container.Container {
206
+	return []*container.Container{}
207
+}
208
+
209
+func (*testContainerStore) Size() int {
210
+	return 0
211
+}
212
+
213
+func (*testContainerStore) First(container.StoreFilter) *container.Container {
214
+	return nil
215
+}
216
+
217
+func (*testContainerStore) ApplyAll(container.StoreReducer) {}
... ...
@@ -3,6 +3,7 @@ package containerd
3 3
 import (
4 4
 	"context"
5 5
 
6
+	"github.com/containerd/containerd/images"
6 7
 	"github.com/docker/docker/api/types/events"
7 8
 	imagetypes "github.com/docker/docker/api/types/image"
8 9
 )
... ...
@@ -27,6 +28,18 @@ func (i *ImageService) LogImageEvent(imageID, refName string, action events.Acti
27 27
 	})
28 28
 }
29 29
 
30
+// logImageEvent generates an event related to an image with only name attribute.
31
+func (i *ImageService) logImageEvent(img images.Image, refName string, action events.Action) {
32
+	attributes := map[string]string{}
33
+	if refName != "" {
34
+		attributes["name"] = refName
35
+	}
36
+	i.eventsService.Log(action, events.ImageEventType, events.Actor{
37
+		ID:         img.Target.Digest.String(),
38
+		Attributes: attributes,
39
+	})
40
+}
41
+
30 42
 // copyAttributes guarantees that labels are not mutated by event triggers.
31 43
 func copyAttributes(attributes, labels map[string]string) {
32 44
 	if labels == nil {
... ...
@@ -64,10 +64,8 @@ func (i *ImageService) ImagesPrune(ctx context.Context, fltrs filters.Args) (*ty
64 64
 
65 65
 func (i *ImageService) pruneUnused(ctx context.Context, filterFunc imageFilterFunc, danglingOnly bool) (*types.ImagesPruneReport, error) {
66 66
 	report := types.ImagesPruneReport{}
67
-	is := i.client.ImageService()
68
-	store := i.client.ContentStore()
69 67
 
70
-	allImages, err := i.client.ImageService().List(ctx)
68
+	allImages, err := i.images.List(ctx)
71 69
 	if err != nil {
72 70
 		return nil, err
73 71
 	}
... ...
@@ -173,7 +171,7 @@ func (i *ImageService) pruneUnused(ctx context.Context, filterFunc imageFilterFu
173 173
 			}
174 174
 			continue
175 175
 		}
176
-		err = is.Delete(ctx, img.Name, containerdimages.SynchronousDelete())
176
+		err = i.images.Delete(ctx, img.Name, containerdimages.SynchronousDelete())
177 177
 		if err != nil && !cerrdefs.IsNotFound(err) {
178 178
 			errs = multierror.Append(errs, err)
179 179
 			if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
... ...
@@ -190,7 +188,7 @@ func (i *ImageService) pruneUnused(ctx context.Context, filterFunc imageFilterFu
190 190
 
191 191
 		// Check which blobs have been deleted and sum their sizes
192 192
 		for _, blob := range blobs {
193
-			_, err := store.ReaderAt(ctx, blob)
193
+			_, err := i.content.ReaderAt(ctx, blob)
194 194
 
195 195
 			if cerrdefs.IsNotFound(err) {
196 196
 				report.ImagesDeleted = append(report.ImagesDeleted,
... ...
@@ -211,10 +209,7 @@ func (i *ImageService) pruneUnused(ctx context.Context, filterFunc imageFilterFu
211 211
 // This is a temporary solution to the rootfs snapshot not being deleted when there's a buildkit history
212 212
 // item referencing an image config.
213 213
 func (i *ImageService) unleaseSnapshotsFromDeletedConfigs(ctx context.Context, possiblyDeletedConfigs map[digest.Digest]struct{}) error {
214
-	is := i.client.ImageService()
215
-	store := i.client.ContentStore()
216
-
217
-	all, err := is.List(ctx)
214
+	all, err := i.images.List(ctx)
218 215
 	if err != nil {
219 216
 		return errors.Wrap(err, "failed to list images during snapshot lease removal")
220 217
 	}
... ...
@@ -238,7 +233,7 @@ func (i *ImageService) unleaseSnapshotsFromDeletedConfigs(ctx context.Context, p
238 238
 
239 239
 	// At this point, all configs that are used by any image has been removed from the slice
240 240
 	for cfgDigest := range possiblyDeletedConfigs {
241
-		info, err := store.Info(ctx, cfgDigest)
241
+		info, err := i.content.Info(ctx, cfgDigest)
242 242
 		if err != nil {
243 243
 			if cerrdefs.IsNotFound(err) {
244 244
 				log.G(ctx).WithField("config", cfgDigest).Debug("config already gone")
... ...
@@ -254,7 +249,7 @@ func (i *ImageService) unleaseSnapshotsFromDeletedConfigs(ctx context.Context, p
254 254
 		label := "containerd.io/gc.ref.snapshot." + i.StorageDriver()
255 255
 
256 256
 		delete(info.Labels, label)
257
-		_, err = store.Update(ctx, info, "labels."+label)
257
+		_, err = i.content.Update(ctx, info, "labels."+label)
258 258
 		if err != nil {
259 259
 			errs = multierror.Append(errs, errors.Wrapf(err, "failed to remove gc.ref.snapshot label from %s", cfgDigest))
260 260
 			if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
... ...
@@ -2,6 +2,7 @@ package containerd
2 2
 
3 3
 import (
4 4
 	"context"
5
+	"fmt"
5 6
 
6 7
 	cerrdefs "github.com/containerd/containerd/errdefs"
7 8
 	containerdimages "github.com/containerd/containerd/images"
... ...
@@ -34,9 +35,11 @@ func (i *ImageService) TagImage(ctx context.Context, imageID image.ID, newTag re
34 34
 			return errdefs.System(errors.Wrapf(err, "failed to create image with name %s and target %s", newImg.Name, newImg.Target.Digest.String()))
35 35
 		}
36 36
 
37
-		replacedImg, err := is.Get(ctx, newImg.Name)
37
+		replacedImg, all, err := i.resolveAllReferences(ctx, newImg.Name)
38 38
 		if err != nil {
39 39
 			return errdefs.Unknown(errors.Wrapf(err, "creating image %s failed because it already exists, but accessing it also failed", newImg.Name))
40
+		} else if replacedImg == nil {
41
+			return errdefs.Unknown(fmt.Errorf("creating image %s failed because it already exists, but failed to resolve", newImg.Name))
40 42
 		}
41 43
 
42 44
 		// Check if image we would replace already resolves to the same target.
... ...
@@ -47,7 +50,7 @@ func (i *ImageService) TagImage(ctx context.Context, imageID image.ID, newTag re
47 47
 		}
48 48
 
49 49
 		// If there already exists an image with this tag, delete it
50
-		if err := i.softImageDelete(ctx, replacedImg); err != nil {
50
+		if err := i.softImageDelete(ctx, *replacedImg, all); err != nil {
51 51
 			return errors.Wrapf(err, "failed to delete previous image %s", replacedImg.Name)
52 52
 		}
53 53
 
... ...
@@ -6,6 +6,7 @@ import (
6 6
 	"sync/atomic"
7 7
 
8 8
 	"github.com/containerd/containerd"
9
+	"github.com/containerd/containerd/content"
9 10
 	cerrdefs "github.com/containerd/containerd/errdefs"
10 11
 	"github.com/containerd/containerd/images"
11 12
 	"github.com/containerd/containerd/plugin"
... ...
@@ -15,7 +16,7 @@ import (
15 15
 	"github.com/distribution/reference"
16 16
 	"github.com/docker/docker/container"
17 17
 	daemonevents "github.com/docker/docker/daemon/events"
18
-	daemonimages "github.com/docker/docker/daemon/images"
18
+	dimages "github.com/docker/docker/daemon/images"
19 19
 	"github.com/docker/docker/daemon/snapshotter"
20 20
 	"github.com/docker/docker/errdefs"
21 21
 	"github.com/docker/docker/image"
... ...
@@ -29,6 +30,7 @@ import (
29 29
 type ImageService struct {
30 30
 	client          *containerd.Client
31 31
 	images          images.Store
32
+	content         content.Store
32 33
 	containers      container.Store
33 34
 	snapshotter     string
34 35
 	registryHosts   docker.RegistryHosts
... ...
@@ -62,6 +64,7 @@ func NewService(config ImageServiceConfig) *ImageService {
62 62
 	return &ImageService{
63 63
 		client:          config.Client,
64 64
 		images:          config.Client.ImageService(),
65
+		content:         config.Client.ContentStore(),
65 66
 		containers:      config.Containers,
66 67
 		snapshotter:     config.Snapshotter,
67 68
 		registryHosts:   config.RegistryHosts,
... ...
@@ -73,8 +76,8 @@ func NewService(config ImageServiceConfig) *ImageService {
73 73
 }
74 74
 
75 75
 // DistributionServices return services controlling daemon image storage.
76
-func (i *ImageService) DistributionServices() daemonimages.DistributionServices {
77
-	return daemonimages.DistributionServices{}
76
+func (i *ImageService) DistributionServices() dimages.DistributionServices {
77
+	return dimages.DistributionServices{}
78 78
 }
79 79
 
80 80
 // CountImages returns the number of images stored by ImageService
... ...
@@ -17,23 +17,13 @@ const imageNameDanglingPrefix = "moby-dangling@"
17 17
 // softImageDelete deletes the image, making sure that there are other images
18 18
 // that reference the content of the deleted image.
19 19
 // If no other image exists, a dangling one is created.
20
-func (i *ImageService) softImageDelete(ctx context.Context, img containerdimages.Image) error {
21
-	is := i.client.ImageService()
22
-
23
-	// If the image already exists, persist it as dangling image
24
-	// but only if no other image has the same target.
25
-	dgst := img.Target.Digest.String()
26
-	imgs, err := is.List(ctx, "target.digest=="+dgst)
27
-	if err != nil {
28
-		return errdefs.System(errors.Wrapf(err, "failed to check if there are images targeting digest %s", dgst))
29
-	}
30
-
20
+func (i *ImageService) softImageDelete(ctx context.Context, img containerdimages.Image, imgs []containerdimages.Image) error {
31 21
 	// From this point explicitly ignore the passed context
32 22
 	// and don't allow to interrupt operation in the middle.
33 23
 
34 24
 	// Create dangling image if this is the last image pointing to this target.
35 25
 	if len(imgs) == 1 {
36
-		err = i.ensureDanglingImage(compatcontext.WithoutCancel(ctx), img)
26
+		err := i.ensureDanglingImage(compatcontext.WithoutCancel(ctx), img)
37 27
 
38 28
 		// Error out in case we couldn't persist the old image.
39 29
 		if err != nil {
... ...
@@ -43,7 +33,8 @@ func (i *ImageService) softImageDelete(ctx context.Context, img containerdimages
43 43
 	}
44 44
 
45 45
 	// Free the target name.
46
-	err = is.Delete(compatcontext.WithoutCancel(ctx), img.Name)
46
+	// TODO: Add with target option
47
+	err := i.images.Delete(compatcontext.WithoutCancel(ctx), img.Name)
47 48
 	if err != nil {
48 49
 		if !cerrdefs.IsNotFound(err) {
49 50
 			return errdefs.System(errors.Wrapf(err, "failed to delete image %s which existed a moment before", img.Name))
... ...
@@ -67,7 +58,7 @@ func (i *ImageService) ensureDanglingImage(ctx context.Context, from containerdi
67 67
 	}
68 68
 	danglingImage.Name = danglingImageName(from.Target.Digest)
69 69
 
70
-	_, err := i.client.ImageService().Create(compatcontext.WithoutCancel(ctx), danglingImage)
70
+	_, err := i.images.Create(compatcontext.WithoutCancel(ctx), danglingImage)
71 71
 	// If it already exists, then just continue.
72 72
 	if cerrdefs.IsAlreadyExists(err) {
73 73
 		return nil
... ...
@@ -81,5 +72,6 @@ func danglingImageName(digest digest.Digest) string {
81 81
 }
82 82
 
83 83
 func isDanglingImage(image containerdimages.Image) bool {
84
+	// TODO: Also check for expired
84 85
 	return image.Name == danglingImageName(image.Target.Digest)
85 86
 }