containerd: remove unleaseSnapshotsFromDeletedConfigs

Removes workaround for https://github.com/moby/buildkit/issues/3797 now
that the underlying issue is fixed.

Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>

daemon/containerd/image_delete.go

@@ -17,8 +17,6 @@ import (
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/internal/metrics"
 	"github.com/docker/docker/pkg/stringid"
-	"github.com/opencontainers/go-digest"
-	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 // ImageDelete deletes the image referenced by the given imageRef from this
@@ -204,35 +202,6 @@ func (i *ImageService) ImageDelete(ctx context.Context, imageRef string, force,
 // Parent images are removed quietly, and if there is any issue/conflict
 // it is logged but does not halt execution/an error is not returned.
 func (i *ImageService) deleteAll(ctx context.Context, imgID image.ID, all []c8dimages.Image, c conflictType, prune bool) (records []imagetypes.DeleteResponse, _ error) {
-	// Workaround for: https://github.com/moby/buildkit/issues/3797
-	possiblyDeletedConfigs := map[digest.Digest]struct{}{}
-	if len(all) > 0 && i.content != nil {
-		handled := map[digest.Digest]struct{}{}
-		for _, img := range all {
-			if _, ok := handled[img.Target.Digest]; ok {
-				continue
-			} else {
-				handled[img.Target.Digest] = struct{}{}
-			}
-			err := i.walkPresentChildren(ctx, img.Target, func(_ context.Context, d ocispec.Descriptor) error {
-				if c8dimages.IsConfigType(d.MediaType) {
-					possiblyDeletedConfigs[d.Digest] = struct{}{}
-				}
-				return nil
-			})
-			if err != nil {
-				return nil, err
-			}
-		}
-	}
-	defer func() {
-		if len(possiblyDeletedConfigs) > 0 {
-			if err := i.unleaseSnapshotsFromDeletedConfigs(context.WithoutCancel(ctx), possiblyDeletedConfigs); err != nil {
-				log.G(ctx).WithError(err).Warn("failed to unlease snapshots")
-			}
-		}
-	}()
-
 	var parents []c8dimages.Image
 	if prune {
 		// TODO(dmcgowan): Consider using GC labels to walk for deletion

daemon/containerd/image_prune.go

@@ -218,16 +218,7 @@ func (i *ImageService) pruneAll(ctx context.Context, imagesToPrune map[string]c8
 	span.SetAttributes(tracing.Attribute("count", len(imagesToPrune)))
 	defer span.End()
 
-	possiblyDeletedConfigs := map[digest.Digest]struct{}{}
 	var errs error
-
-	// Workaround for https://github.com/moby/buildkit/issues/3797
-	defer func() {
-		if err := i.unleaseSnapshotsFromDeletedConfigs(context.WithoutCancel(ctx), possiblyDeletedConfigs); err != nil {
-			errs = multierror.Append(errs, err)
-		}
-	}()
-
 	for _, img := range imagesToPrune {
 		log.G(ctx).WithField("image", img).Debug("pruning image")
 
@@ -235,9 +226,6 @@ func (i *ImageService) pruneAll(ctx context.Context, imagesToPrune map[string]c8
 
 		err := i.walkPresentChildren(ctx, img.Target, func(_ context.Context, desc ocispec.Descriptor) error {
 			blobs = append(blobs, desc)
-			if c8dimages.IsConfigType(desc.MediaType) {
-				possiblyDeletedConfigs[desc.Digest] = struct{}{}
-			}
 			return nil
 		})
 		if err != nil {
@@ -279,60 +267,3 @@ func (i *ImageService) pruneAll(ctx context.Context, imagesToPrune map[string]c8
 
 	return &report, errs
 }
-
-// unleaseSnapshotsFromDeletedConfigs removes gc.ref.snapshot content label from configs that are not
-// referenced by any of the existing images.
-// This is a temporary solution to the rootfs snapshot not being deleted when there's a buildkit history
-// item referencing an image config.
-func (i *ImageService) unleaseSnapshotsFromDeletedConfigs(ctx context.Context, possiblyDeletedConfigs map[digest.Digest]struct{}) error {
-	all, err := i.images.List(ctx)
-	if err != nil {
-		return errors.Wrap(err, "failed to list images during snapshot lease removal")
-	}
-
-	var errs error
-	for _, img := range all {
-		err := i.walkPresentChildren(ctx, img.Target, func(_ context.Context, desc ocispec.Descriptor) error {
-			if c8dimages.IsConfigType(desc.MediaType) {
-				delete(possiblyDeletedConfigs, desc.Digest)
-			}
-			return nil
-		})
-		if err != nil {
-			errs = multierror.Append(errs, err)
-			if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
-				return errs
-			}
-			continue
-		}
-	}
-
-	// At this point, all configs that are used by any image has been removed from the slice
-	for cfgDigest := range possiblyDeletedConfigs {
-		info, err := i.content.Info(ctx, cfgDigest)
-		if err != nil {
-			if cerrdefs.IsNotFound(err) {
-				log.G(ctx).WithField("config", cfgDigest).Debug("config already gone")
-			} else {
-				errs = multierror.Append(errs, err)
-				if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
-					return errs
-				}
-			}
-			continue
-		}
-
-		label := "containerd.io/gc.ref.snapshot." + i.StorageDriver()
-
-		delete(info.Labels, label)
-		_, err = i.content.Update(ctx, info, "labels."+label)
-		if err != nil {
-			errs = multierror.Append(errs, errors.Wrapf(err, "failed to remove gc.ref.snapshot label from %s", cfgDigest))
-			if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
-				return errs
-			}
-		}
-	}
-
-	return errs
-}

integration/build/build_test.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"slices"
 	"strings"
 	"testing"
 	"time"
@@ -16,6 +17,7 @@ import (
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/events"
 	"github.com/docker/docker/api/types/filters"
+	"github.com/docker/docker/api/types/image"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/docker/docker/testutil"
@@ -791,6 +793,44 @@ func TestBuildEmitsImageCreateEvent(t *testing.T) {
 	}
 }
 
+func TestBuildHistoryDoesNotPreventRemoval(t *testing.T) {
+	skip.If(t, testEnv.DaemonInfo.OSType == "windows", "buildkit is not supported on Windows")
+	skip.If(t, !testEnv.UsingSnapshotter(), "only relevant to c8d integration")
+
+	ctx := setupTest(t)
+
+	dockerfile := "FROM busybox\nRUN echo hello world > /hello"
+	source := fakecontext.New(t, "", fakecontext.WithDockerfile(dockerfile))
+	defer source.Close()
+
+	apiClient := testEnv.APIClient()
+
+	buildImage := func(imgName string) error {
+		resp, err := apiClient.ImageBuild(ctx, source.AsTarReader(t), types.ImageBuildOptions{
+			Remove:      true,
+			ForceRemove: true,
+			Tags:        []string{imgName},
+			Version:     types.BuilderBuildKit,
+		})
+		if err != nil {
+			return err
+		}
+		defer resp.Body.Close()
+
+		_, err = io.Copy(io.Discard, resp.Body)
+		return err
+	}
+
+	err := buildImage("history-a")
+	assert.NilError(t, err)
+
+	resp, err := apiClient.ImageRemove(ctx, "history-a", image.RemoveOptions{})
+	assert.NilError(t, err)
+	assert.Check(t, slices.ContainsFunc(resp, func(r image.DeleteResponse) bool {
+		return r.Deleted != ""
+	}))
+}
+
 func readBuildImageIDs(t *testing.T, rd io.Reader) string {
 	t.Helper()
 	decoder := json.NewDecoder(rd)

integration/image/remove_test.go

@@ -82,7 +82,7 @@ func TestRemoveByDigest(t *testing.T) {
 	assert.Assert(t, id != "")
 
 	_, err = client.ImageRemove(ctx, id, image.RemoveOptions{})
-	assert.NilError(t, err, "error reemoving %s", id)
+	assert.NilError(t, err, "error removing %s", id)
 
 	_, err = client.ImageInspect(ctx, "busybox")
 	assert.NilError(t, err, "busybox image got deleted")