diff --git a/daemon/containerd/image_delete.go b/daemon/containerd/image_delete.go index 3ed0b768de..8308150eff 100644 --- a/daemon/containerd/image_delete.go +++ b/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 diff --git a/daemon/containerd/image_prune.go b/daemon/containerd/image_prune.go index bf61138b57..8986bf42a5 100644 --- a/daemon/containerd/image_prune.go +++ b/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 -} diff --git a/integration/build/build_test.go b/integration/build/build_test.go index 786341acd9..748224f6ae 100644 --- a/integration/build/build_test.go +++ b/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) diff --git a/integration/image/remove_test.go b/integration/image/remove_test.go index a909a8d48c..99111bd67d 100644 --- a/integration/image/remove_test.go +++ b/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")