Browse code

Merge pull request #50799 from thaJeztah/touchup_delete

image delete: inline some variables, and touch-up TODOs

Austin Vazquez authored on 2025/09/05 04:27:30
Showing 2 changed files
... ...
@@ -136,8 +136,8 @@ func (i *ImageService) ImageDelete(ctx context.Context, imageRef string, options
136 136
 			return i.untagReferences(ctx, sameRef)
137 137
 		} else if len(all) > 1 && !force {
138 138
 			// Since only a single used reference, remove all active
139
-			// TODO: Consider keeping the conflict and changing active
140
-			// reference calculation in image checker.
139
+			//
140
+			// TODO(dmcgowan): Consider keeping the conflict and changing active reference calculation in image checker; https://github.com/moby/moby/pull/46840
141 141
 			c &= ^conflictActiveReference
142 142
 		}
143 143
 
... ...
@@ -166,23 +166,18 @@ func (i *ImageService) ImageDelete(ctx context.Context, imageRef string, options
166 166
 
167 167
 			return false
168 168
 		}
169
-		// TODO: Should this also check parentage here?
170
-		ctr := i.containers.First(using)
171
-		if ctr != nil {
172
-			familiarRef := reference.FamiliarString(parsedRef)
169
+		// TODO(dmcgowan): Should this also check parentage here? https://github.com/moby/moby/pull/46840
170
+		if ctr := i.containers.First(using); ctr != nil {
173 171
 			if !force {
174 172
 				// If we removed the repository reference then
175 173
 				// this image would remain "dangling" and since
176 174
 				// we really want to avoid that the client must
177 175
 				// explicitly force its removal.
178
-				err := &imageDeleteConflict{
179
-					reference: familiarRef,
176
+				return nil, &imageDeleteConflict{
177
+					reference: reference.FamiliarString(parsedRef),
180 178
 					used:      true,
181
-					message: fmt.Sprintf("container %s is using its referenced image %s",
182
-						stringid.TruncateID(ctr.ID),
183
-						stringid.TruncateID(imgID.String())),
179
+					message:   fmt.Sprintf("container %s is using its referenced image %s", stringid.TruncateID(ctr.ID), stringid.TruncateID(imgID.String())),
184 180
 				}
185
-				return nil, err
186 181
 			}
187 182
 
188 183
 			if len(options.Platforms) > 0 {
... ...
@@ -190,14 +185,13 @@ func (i *ImageService) ImageDelete(ctx context.Context, imageRef string, options
190 190
 			}
191 191
 
192 192
 			// Delete all images
193
-			err := i.softImageDelete(ctx, *img, all)
194
-			if err != nil {
193
+			if err := i.softImageDelete(ctx, *img, all); err != nil {
195 194
 				return nil, err
196 195
 			}
197 196
 
197
+			familiarRef := reference.FamiliarString(parsedRef)
198 198
 			i.logImageEvent(*img, familiarRef, events.ActionUnTag)
199
-			records := []imagetypes.DeleteResponse{{Untagged: familiarRef}}
200
-			return records, nil
199
+			return []imagetypes.DeleteResponse{{Untagged: familiarRef}}, nil
201 200
 		}
202 201
 	}
203 202
 
... ...
@@ -248,8 +242,7 @@ func (i *ImageService) deleteImagePlatformByImageID(ctx context.Context, img *c8
248 248
 		return nil, err
249 249
 	}
250 250
 
251
-	// TODO: Check if these are not used by other images with different
252
-	// target root images.
251
+	// TODO(vvoland): Check if these are not used by other images with different target root images; https://github.com/moby/moby/pull/49982
253 252
 	// The same manifest can be referenced by different image indexes.
254 253
 	var response []imagetypes.DeleteResponse
255 254
 	for _, d := range toDelete {
... ...
@@ -273,7 +266,7 @@ func (i *ImageService) deleteImagePlatformByImageID(ctx context.Context, img *c8
273 273
 func (i *ImageService) deleteAll(ctx context.Context, imgID image.ID, all []c8dimages.Image, c conflictType, prune bool) (records []imagetypes.DeleteResponse, _ error) {
274 274
 	var parents []c8dimages.Image
275 275
 	if prune {
276
-		// TODO(dmcgowan): Consider using GC labels to walk for deletion
276
+		// TODO(dmcgowan): Consider using GC labels to walk for deletion; https://github.com/moby/moby/pull/46840
277 277
 		var err error
278 278
 		parents, err = i.parents(ctx, imgID)
279 279
 		if err != nil {
... ...
@@ -437,7 +430,7 @@ func (i *ImageService) imageDeleteHelper(ctx context.Context, img c8dimages.Imag
437 437
 		}
438 438
 	}
439 439
 
440
-	// TODO: Add target option
440
+	// TODO(dmcgowan): Add with target; https://github.com/moby/moby/pull/46840
441 441
 	err = i.images.Delete(ctx, img.Name, c8dimages.SynchronousDelete())
442 442
 	if err != nil {
443 443
 		return err
... ...
@@ -479,7 +472,7 @@ func (*imageDeleteConflict) Conflict() {}
479 479
 func (i *ImageService) untagReferences(ctx context.Context, refs []c8dimages.Image) ([]imagetypes.DeleteResponse, error) {
480 480
 	var records []imagetypes.DeleteResponse
481 481
 	for _, ref := range refs {
482
-		// TODO: Add with target
482
+		// TODO(dmcgowan): Add with target; https://github.com/moby/moby/pull/46840
483 483
 		err := i.images.Delete(ctx, ref.Name)
484 484
 		if err != nil {
485 485
 			return nil, err
... ...
@@ -527,7 +520,7 @@ func (i *ImageService) checkImageDeleteConflict(ctx context.Context, imgID image
527 527
 	}
528 528
 
529 529
 	if mask&conflictActiveReference != 0 {
530
-		// TODO: Count unexpired references...
530
+		// TODO(dmcgowan): Count unexpired references; https://github.com/moby/moby/pull/46840
531 531
 		if len(all) > 1 {
532 532
 			return &imageDeleteConflict{
533 533
 				reference: stringid.TruncateID(imgID.String()),
... ...
@@ -140,8 +140,8 @@ func TestRemoveWithPlatform(t *testing.T) {
140 140
 		platform *ocispec.Platform
141 141
 		deleted  ocispec.Descriptor
142 142
 	}{
143
-		{&platformHost, descs[0]},
144
-		{&someOtherPlatform, descs[3]},
143
+		{platform: &platformHost, deleted: descs[0]},
144
+		{platform: &someOtherPlatform, deleted: descs[3]},
145 145
 	} {
146 146
 		resp, err := apiClient.ImageRemove(ctx, imgName, client.ImageRemoveOptions{
147 147
 			Platforms: []ocispec.Platform{*tc.platform},
... ...
@@ -162,7 +162,7 @@ func TestRemoveWithPlatform(t *testing.T) {
162 162
 	assert.Check(t, is.Len(resp, 2))
163 163
 	assert.Check(t, is.Equal(resp[0].Untagged, imgName))
164 164
 	assert.Check(t, is.Equal(resp[1].Deleted, imageIdx.Manifests[0].Digest.String()))
165
-	// TODO: Should it also include platform-specific manifests?
165
+	// TODO(vvoland): Should it also include platform-specific manifests? https://github.com/moby/moby/pull/49982
166 166
 }
167 167
 
168 168
 func checkPlatformDeleted(t *testing.T, imageIdx *ocispec.Index, resp []image.DeleteResponse, mfstDesc ocispec.Descriptor) {