Repository: moby/moby (language: go)

Issue:
Containerd snapshotter saves layers that can't be cleared easily
### Description

I have the following enabled on my machine for docker:

  "features": {
    "containerd-snapshotter": true
  }

If I do a pull and cancel it part way through it will save the layers.
If I never complete this pull the data will persist under /contianerd/io.containerd.snapshotter.v1.overlayfs and /containerd/io.containerd.content.v1.content
Running commands like docker system prune -a or docker image prune -a do not work for removing this data and there doesn't seem to be any way to manually trigger garbage collection of these files.
Additionally I tried to see if I could find anything related to these files through ctr images, ctr leases, and ctr snapshots with no luck.

Was told this is the best place for this bug here:
https://github.com/containerd/containerd/issues/10548
https://github.com/docker/cli/issues/5315

### Reproduce

1. Enable snapshotter for docker with code block from description.
2. Note reported usage of docker images with docker system df
3. docker pull <any_image>
4. Cancel the pull before it completes, but after some layers have been pulled and extracted.
5. Confirm with docker system df (or du) that the usage has increased.
6. Confirm new data is unable to be removed through normal commands (or even through gc over time)

### Expected behavior

Have some way of clearing this data without running an rm command on the directory.
Either through ctr or docker; or if garbage collection cleans it up then a way to manually force that data to be removed.

### docker version

```bash
Client: Docker Engine - Community
Version:           27.1.1
API version:       1.46
Go version:        go1.21.12
Git commit:        6312585
Built:             Tue Jul 23 19:57:01 2024
OS/Arch:           linux/amd64
Context:           default

Server: Docker Engine - Community
Engine:
 Version:          27.1.1
 API version:      1.46 (minimum version 1.24)
 Go version:       go1.21.12
 Git commit:       cc13f95
 Built:            Tue Jul 23 19:57:01 2024
 OS/Arch:          linux/amd64
 Experimental:     true
containerd:
 Version:          1.7.19
 GitCommit:        2bf793ef6dc9a18e00cb12efb64355c2c9d5eb41
runc:
 Version:          1.7.19
 GitCommit:        v1.1.13-0-g58aa920
docker-init:
 Version:          0.19.0
 GitCommit:        de40ad0
```

### docker info

```bash
Client: Docker Engine - Community
 Version:    27.1.1
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc.)
    Version:  v0.16.1
    Path:     /usr/libexec/docker/cli-plugins/docker-buildx
  compose: Docker Compose (Docker Inc.)
    Version:  v2.29.1
    Path:     /usr/libexec/docker/cli-plugins/docker-compose

Server:
 Containers: 0
  Running: 0
  Paused: 0
  Stopped: 0
 Images: 0
 Server Version: 27.1.1
 Storage Driver: overlayfs
  driver-type: io.containerd.snapshotter.v1
 Logging Driver: json-file
 Cgroup Driver: systemd
 Cgroup Version: 2
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 nvidia runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 2bf793ef6dc9a18e00cb12efb64355c2c9d5eb41
 runc version: v1.1.13-0-g58aa920
 init version: de40ad0
 Security Options:
  apparmor
  seccomp
   Profile: builtin
  cgroupns
 Kernel Version: 6.5.0-44-generic
 Operating System: Ubuntu 22.04.4 LTS
 OSType: linux
 Architecture: x86_64
 CPUs: 20
 Total Memory: 62.46GiB
 Name:
 ID: 797409c4-1056-4a1f-946d-93f3a7004068
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 Experimental: true
 Insecure Registries:
  localhost:5000
 Live Restore Enabled: false
```

### Additional Info

_No response_


Discussion / hints:
/cc @dmcgowan 
> Confirm new data is unable to be removed through normal commands (or even through gc over time)

There is a lease that I see is being left behind after cancel with a 24 hour expiration on it. That time could likely be much shorter and we should definitely update prune to clean up those leases. As to whether to leave behind the partial progress or not, that is debatable. The advantage is upon resuming the content that was previously downloaded is still there to use.

Fix patch:
diff --git a/daemon/containerd/image_builder.go b/daemon/containerd/image_builder.go
index 42bd84ee9b3f1..c5c331cb04db4 100644
--- a/daemon/containerd/image_builder.go
+++ b/daemon/containerd/image_builder.go
@@ -106,10 +106,11 @@ func (i *ImageService) GetImageAndReleasableLayer(ctx context.Context, refOrID s
 		}
 	}
 
-	ctx, _, err := i.client.WithLease(ctx, leases.WithRandomID(), leases.WithExpiration(1*time.Hour))
+	ctx, release, err := i.withLease(ctx, true)
 	if err != nil {
 		return nil, nil, fmt.Errorf("failed to create lease for commit: %w", err)
 	}
+	defer release()
 
 	// TODO(laurazard): do we really need a new method here to pull the image?
 	imgDesc, err := i.pullForBuilder(ctx, refOrID, opts.AuthConfig, opts.Output, opts.Platform)
@@ -234,9 +235,9 @@ func newROLayerForImage(ctx context.Context, imgDesc *ocispec.Descriptor, i *Ima
 
 func createLease(ctx context.Context, lm leases.Manager) (context.Context, leases.Lease, error) {
 	lease, err := lm.Create(ctx,
-		leases.WithExpiration(time.Hour*24),
+		leases.WithExpiration(leaseExpireDuration),
 		leases.WithLabels(map[string]string{
-			"org.mobyproject.lease.classicbuilder": "true",
+			pruneLeaseLabel: "true",
 		}),
 	)
 	if err != nil {
@@ -493,17 +494,11 @@ func (i *ImageService) createImageOCI(ctx context.Context, imgToCreate imagespec
 	parentDigest digest.Digest, layers []ocispec.Descriptor,
 	containerConfig container.Config,
 ) (dimage.ID, error) {
-	// Necessary to prevent the contents from being GC'd
-	// between writing them here and creating an image
-	ctx, release, err := i.client.WithLease(ctx, leases.WithRandomID(), leases.WithExpiration(1*time.Hour))
+	ctx, release, err := i.withLease(ctx, false)
 	if err != nil {
 		return "", err
 	}
-	defer func() {
-		if err := release(context.WithoutCancel(ctx)); err != nil {
-			log.G(ctx).WithError(err).Warn("failed to release lease created for create")
-		}
-	}()
+	defer release()
 
 	manifestDesc, ccDesc, err := writeContentsForImage(ctx, i.snapshotter, i.content, imgToCreate, layers, containerConfig)
 	if err != nil {
diff --git a/daemon/containerd/image_commit.go b/daemon/containerd/image_commit.go
index 2e86d4829ab74..878232193647e 100644
--- a/daemon/containerd/image_commit.go
+++ b/daemon/containerd/image_commit.go
@@ -12,7 +12,6 @@ import (
 
 	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/diff"
-	"github.com/containerd/containerd/leases"
 	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/pkg/cleanup"
 	"github.com/containerd/containerd/snapshots"
@@ -67,16 +66,11 @@ func (i *ImageService) CommitImage(ctx context.Context, cc backend.CommitConfig)
 		sn     = i.client.SnapshotService(container.Driver)
 	)
 
-	// Don't gc me and clean the dirty data after 1 hour!
-	ctx, release, err := i.client.WithLease(ctx, leases.WithRandomID(), leases.WithExpiration(1*time.Hour))
+	ctx, release, err := i.withLease(ctx, false)
 	if err != nil {
 		return "", fmt.Errorf("failed to create lease for commit: %w", err)
 	}
-	defer func() {
-		if err := release(context.WithoutCancel(ctx)); err != nil {
-			log.G(ctx).WithError(err).Warn("failed to release lease created for commit")
-		}
-	}()
+	defer release()
 
 	diffLayerDesc, diffID, err := i.createDiff(ctx, cc.ContainerID, sn, cs, differ)
 	if err != nil {
diff --git a/daemon/containerd/image_exporter.go b/daemon/containerd/image_exporter.go
index c3808aed0f872..e05b1394673cb 100644
--- a/daemon/containerd/image_exporter.go
+++ b/daemon/containerd/image_exporter.go
@@ -67,19 +67,14 @@ func (i *ImageService) ExportImage(ctx context.Context, names []string, outStrea
 		archive.WithSkipMissing(i.content),
 	}
 
-	leasesManager := i.client.LeasesService()
-	lease, err := leasesManager.Create(ctx, leases.WithRandomID())
+	ctx, done, err := i.withLease(ctx, false)
 	if err != nil {
 		return errdefs.System(err)
 	}
-	defer func() {
-		if err := leasesManager.Delete(ctx, lease); err != nil {
-			log.G(ctx).WithError(err).Warn("cleaning up lease")
-		}
-	}()
+	defer done()
 
 	addLease := func(ctx context.Context, target ocispec.Descriptor) error {
-		return leaseContent(ctx, i.content, leasesManager, lease, target)
+		return i.leaseContent(ctx, i.content, target)
 	}
 
 	exportImage := func(ctx context.Context, target ocispec.Descriptor, ref reference.Named) error {
@@ -206,7 +201,13 @@ func (i *ImageService) ExportImage(ctx context.Context, names []string, outStrea
 
 // leaseContent will add a resource to the lease for each child of the descriptor making sure that it and
 // its children won't be deleted while the lease exists
-func leaseContent(ctx context.Context, store content.Store, leasesManager leases.Manager, lease leases.Lease, desc ocispec.Descriptor) error {
+func (i *ImageService) leaseContent(ctx context.Context, store content.Store, desc ocispec.Descriptor) error {
+	lid, ok := leases.FromContext(ctx)
+	if !ok {
+		return nil
+	}
+	lease := leases.Lease{ID: lid}
+	leasesManager := i.client.LeasesService()
 	return containerdimages.Walk(ctx, containerdimages.HandlerFunc(func(ctx context.Context, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
 		_, err := store.Info(ctx, desc.Digest)
 		if err != nil {
diff --git a/daemon/containerd/image_prune.go b/daemon/containerd/image_prune.go
index 071d651fc8b6a..e3c924152c916 100644
--- a/daemon/containerd/image_prune.go
+++ b/daemon/containerd/image_prune.go
@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	containerdimages "github.com/containerd/containerd/images"
+	"github.com/containerd/containerd/leases"
 	"github.com/containerd/containerd/tracing"
 	cerrdefs "github.com/containerd/errdefs"
 	"github.com/containerd/log"
@@ -60,6 +61,22 @@ func (i *ImageService) ImagesPrune(ctx context.Context, fltrs filters.Args) (*im
 		return nil, err
 	}
 
+	// Prune leases
+	leaseManager := i.client.LeasesService()
+	pullLeases, err := leaseManager.List(ctx, pruneLeaseFilter)
+	if err != nil {
+		return nil, err
+	}
+	for i, lease := range pullLeases {
+		var opts []leases.DeleteOpt
+		if i == len(pullLeases)-1 {
+			opts = append(opts, leases.SynchronousDelete)
+		}
+		if err := leaseManager.Delete(ctx, lease, opts...); err != nil {
+			return nil, err
+		}
+	}
+
 	return i.pruneUnused(ctx, filterFunc, danglingOnly)
 }
 
diff --git a/daemon/containerd/image_pull.go b/daemon/containerd/image_pull.go
index b70d1dd071a14..69204d02e0702 100644
--- a/daemon/containerd/image_pull.go
+++ b/daemon/containerd/image_pull.go
@@ -10,7 +10,6 @@ import (
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/images"
-	"github.com/containerd/containerd/leases"
 	"github.com/containerd/containerd/pkg/snapshotters"
 	"github.com/containerd/containerd/remotes/docker"
 	cerrdefs "github.com/containerd/errdefs"
@@ -40,6 +39,12 @@ func (i *ImageService) PullImage(ctx context.Context, baseRef reference.Named, p
 	}()
 	out := streamformatter.NewJSONProgressOutput(outStream, false)
 
+	ctx, done, err := i.withLease(ctx, true)
+	if err != nil {
+		return err
+	}
+	defer done()
+
 	if !reference.IsNameOnly(baseRef) {
 		return i.pullTag(ctx, baseRef, platform, metaHeaders, authConfig, out)
 	}
@@ -89,14 +94,7 @@ func (i *ImageService) pullTag(ctx context.Context, ref reference.Named, platfor
 	var outNewImg containerd.Image
 
 	if oldImage.Target.Digest != "" {
-		// Lease the old image content to prevent it from being garbage collected until we keep it as dangling image.
-		lm := i.client.LeasesService()
-		lease, err := lm.Create(ctx, leases.WithRandomID())
-		if err != nil {
-			return errdefs.System(fmt.Errorf("failed to create lease: %w", err))
-		}
-
-		err = leaseContent(ctx, i.content, lm, lease, oldImage.Target)
+		err = i.leaseContent(ctx, i.content, oldImage.Target)
 		if err != nil {
 			return errdefs.System(fmt.Errorf("failed to lease content: %w", err))
 		}
@@ -110,9 +108,6 @@ func (i *ImageService) pullTag(ctx context.Context, ref reference.Named, platfor
 					}
 				}
 			}
-			if err := lm.Delete(ctx, lease); err != nil {
-				log.G(ctx).WithError(err).Warn("failed to delete lease")
-			}
 		}()
 	}
 
diff --git a/daemon/containerd/image_snapshot.go b/daemon/containerd/image_snapshot.go
index 9e72192eb0676..b404113f140b0 100644
--- a/daemon/containerd/image_snapshot.go
+++ b/daemon/containerd/image_snapshot.go
@@ -58,6 +58,11 @@ func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentIma
 		parentSnapshot = identity.ChainID(diffIDs).String()
 	}
 
+	// TODO: Consider a better way to do this. It is better to have a container directly
+	// reference a snapshot, however, that is not done today because a container may
+	// removed and recreated with nothing holding the snapshot in between. Consider
+	// removing this lease and only temporarily holding a lease on re-create, using
+	// non-expiring leases introduces the possibility of leaking resources.
 	ls := i.client.LeasesService()
 	lease, err := ls.Create(ctx, leases.WithID(id))
 	if err != nil {
diff --git a/daemon/containerd/leases.go b/daemon/containerd/leases.go
new file mode 100644
index 0000000000000..e2a38e5b579d7
--- /dev/null
+++ b/daemon/containerd/leases.go
@@ -0,0 +1,52 @@
+package containerd
+
+import (
+	"context"
+	"time"
+
+	"github.com/containerd/containerd/leases"
+	"github.com/containerd/log"
+)
+
+const (
+	leaseExpireDuration = 8 * time.Hour
+	expireLabel         = "containerd.io/gc.expire" // Copied from containerd
+	pruneLeaseLabel     = "moby/prune.images"
+	pruneLeaseFilter    = `labels."moby/prune.images"`
+)
+
+// withLease is used to prevent content or snapshots from being eligible for
+// garbage collection while they are being used. Leases should always be
+// released when complete to make the resources eligible again.
+// If cancellable is set to true, then the lease will remain if the context
+// is canceled until its expiration or deletion via prune.
+func (i *ImageService) withLease(ctx context.Context, cancellable bool) (context.Context, func(), error) {
+	_, ok := leases.FromContext(ctx)
+	if ok {
+		return ctx, func() {}, nil
+	}
+
+	ls := i.client.LeasesService()
+
+	expireAt := time.Now().Add(leaseExpireDuration)
+	l, err := ls.Create(ctx,
+		leases.WithRandomID(),
+		leases.WithLabels(map[string]string{
+			pruneLeaseLabel: "true",
+			expireLabel:     expireAt.Format(time.RFC3339),
+		}))
+	if err != nil {
+		return ctx, func() {}, err
+	}
+
+	ctx = leases.WithLease(ctx, l.ID)
+	return ctx, func() {
+		if ctx.Err() != nil && cancellable {
+			log.G(ctx).WithFields(log.Fields{"lease": l.ID, "expires_at": expireAt}).Info("Cancel with lease, leased resources will remain until expiration")
+			return
+		}
+		if err := ls.Delete(context.WithoutCancel(ctx), l); err != nil {
+			log.G(ctx).WithError(err).WithFields(log.Fields{"lease": l.ID, "expires_at": expireAt}).Warn("Error deleting lease, leased resources will remain until expiration")
+		}
+	}, nil
+}


Test patch: