From 0cbb604665cb5fd84ed63d6da12e47d9a56a7d53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Mon, 16 Dec 2024 15:35:22 +0100 Subject: [PATCH] c8d: Implement `RWLayer` and remove `PrepareSnapshot` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement containerd image store backed `RWLayer` and remove the containerd-specific `PrepareSnapshot` method from the ImageService interface. Signed-off-by: Paweł Gronowski --- daemon/containerd/image_snapshot.go | 191 ++++++++++++++++++++++----- daemon/containerd/mount.go | 40 ++---- daemon/containerd/service.go | 14 -- daemon/containerd/service_windows.go | 15 ++- daemon/create.go | 16 +-- daemon/daemon.go | 17 +-- daemon/delete.go | 15 --- daemon/image_service.go | 5 +- daemon/images/image.go | 5 - 9 files changed, 194 insertions(+), 124 deletions(-) diff --git a/daemon/containerd/image_snapshot.go b/daemon/containerd/image_snapshot.go index 3f89539812..69476dc405 100644 --- a/daemon/containerd/image_snapshot.go +++ b/daemon/containerd/image_snapshot.go @@ -4,57 +4,55 @@ import ( "context" "fmt" - "github.com/containerd/containerd" c8dimages "github.com/containerd/containerd/images" "github.com/containerd/containerd/leases" "github.com/containerd/containerd/mount" "github.com/containerd/containerd/snapshots" cerrdefs "github.com/containerd/errdefs" - "github.com/containerd/platforms" + "github.com/containerd/log" + "github.com/docker/docker/container" + "github.com/docker/docker/daemon/snapshotter" "github.com/docker/docker/errdefs" + "github.com/docker/docker/layer" "github.com/opencontainers/image-spec/identity" - ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" ) -// PrepareSnapshot prepares a snapshot from a parent image for a container -func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentImage string, platform *ocispec.Platform, setupInit func(string) error) error { +// CreateLayer creates a new layer for a container. +// TODO(vvoland): Decouple from container +func (i *ImageService) CreateLayer(ctr *container.Container, initFunc layer.MountInit) (container.RWLayer, error) { + ctx := context.TODO() + var parentSnapshot string - if parentImage != "" { - img, err := i.resolveImage(ctx, parentImage) - if err != nil { - return err + if ctr.ImageManifest != nil { + img := c8dimages.Image{ + Target: *ctr.ImageManifest, + } + platformImg, err := i.NewImageManifest(ctx, img, img.Target) + if err != nil { + return nil, err } - - cs := i.content - - matcher := i.matchRequestedOrDefault(platforms.Only, platform) - - platformImg := containerd.NewImageWithPlatform(i.client, img, matcher) unpacked, err := platformImg.IsUnpacked(ctx, i.snapshotter) if err != nil { - return err + return nil, err } if !unpacked { if err := platformImg.Unpack(ctx, i.snapshotter); err != nil { - return err + return nil, err } } - desc, err := c8dimages.Config(ctx, cs, img.Target, matcher) + diffIDs, err := platformImg.RootFS(ctx) if err != nil { - return err - } - - diffIDs, err := c8dimages.RootFS(ctx, cs, desc) - if err != nil { - return err + return nil, err } parentSnapshot = identity.ChainID(diffIDs).String() } + id := ctr.ID + // 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 @@ -63,22 +61,151 @@ func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentIma ls := i.client.LeasesService() lease, err := ls.Create(ctx, leases.WithID(id)) if err != nil { - return err + return nil, err } ctx = leases.WithLease(ctx, lease.ID) - snapshotter := i.client.SnapshotService(i.StorageDriver()) - - if err := i.prepareInitLayer(ctx, id, parentSnapshot, setupInit); err != nil { - return err + if err := i.prepareInitLayer(ctx, id, parentSnapshot, initFunc); err != nil { + return nil, err } + sn := i.client.SnapshotService(i.StorageDriver()) if !i.idMapping.Empty() { - return i.remapSnapshot(ctx, snapshotter, id, id+"-init") + err = i.remapSnapshot(ctx, sn, id, id+"-init") + } else { + _, err = sn.Prepare(ctx, id, id+"-init") } - _, err = snapshotter.Prepare(ctx, id, id+"-init") - return err + if err != nil { + return nil, err + } + + return &rwLayer{ + id: id, + snapshotterName: i.StorageDriver(), + snapshotter: sn, + refCountMounter: i.refCountMounter, + lease: lease, + }, nil +} + +type rwLayer struct { + id string + snapshotter snapshots.Snapshotter + snapshotterName string + refCountMounter snapshotter.Mounter + root string + lease leases.Lease +} + +func (l *rwLayer) mounts(ctx context.Context) ([]mount.Mount, error) { + return l.snapshotter.Mounts(ctx, l.id) +} + +func (l *rwLayer) Mount(mountLabel string) (string, error) { + ctx := context.TODO() + + // TODO: Investigate how we can handle mountLabel + _ = mountLabel + mounts, err := l.mounts(ctx) + if err != nil { + return "", err + } + + var root string + if root, err = l.refCountMounter.Mount(mounts, l.id); err != nil { + return "", fmt.Errorf("failed to mount %s: %w", root, err) + } + l.root = root + + log.G(ctx).WithFields(log.Fields{"container": l.id, "root": root, "snapshotter": l.snapshotterName}).Debug("container mounted via snapshotter") + return root, nil +} + +// GetLayerByID returns a layer by ID +// called from daemon.go Daemon.restore(). +func (i *ImageService) GetLayerByID(cid string) (container.RWLayer, error) { + ctx := context.TODO() + + sn := i.client.SnapshotService(i.StorageDriver()) + if _, err := sn.Stat(ctx, cid); err != nil { + if !cerrdefs.IsNotFound(err) { + return nil, fmt.Errorf("failed to stat snapshot %s: %w", cid, err) + } + return nil, errdefs.NotFound(fmt.Errorf("RW layer for container %s not found", cid)) + } + + ls := i.client.LeasesService() + lss, err := ls.List(ctx, "id=="+cid) + if err != nil { + return nil, err + } + + switch len(lss) { + case 0: + return nil, errdefs.NotFound(errors.New("rw layer lease not found for container " + cid)) + default: + log.G(ctx).WithFields(log.Fields{"container": cid, "leases": lss}).Warn("multiple leases with the same id found, this should not happen") + case 1: + } + + root, err := i.refCountMounter.Mounted(cid) + if err != nil { + log.G(ctx).WithField("container", cid).Warn("failed to determine if container is already mounted") + } + + return &rwLayer{ + id: cid, + snapshotterName: i.StorageDriver(), + snapshotter: sn, + refCountMounter: i.refCountMounter, + lease: lss[0], + root: root, + }, nil + +} + +func (l *rwLayer) Unmount() error { + ctx := context.TODO() + + if l.root == "" { + target, err := l.refCountMounter.Mounted(l.id) + if err != nil { + log.G(ctx).WithField("id", l.id).Warn("failed to determine if container is already mounted") + } + if target == "" { + return errors.New("layer not mounted") + } + l.root = target + } + + if err := l.refCountMounter.Unmount(l.root); err != nil { + log.G(ctx).WithField("container", l.id).WithError(err).Error("error unmounting container") + return fmt.Errorf("failed to unmount %s: %w", l.root, err) + } + + return nil +} + +func (l rwLayer) Metadata() (map[string]string, error) { + return nil, nil +} + +// ReleaseLayer releases a layer allowing it to be removed +// called from delete.go Daemon.cleanupContainer(), and Daemon.containerExport() +func (i *ImageService) ReleaseLayer(rwlayer container.RWLayer) error { + c8dLayer, ok := rwlayer.(*rwLayer) + if !ok { + return fmt.Errorf("invalid layer type %T", rwlayer) + } + + ls := i.client.LeasesService() + if err := ls.Delete(context.Background(), c8dLayer.lease, leases.SynchronousDelete); err != nil { + if !cerrdefs.IsNotFound(err) { + return err + } + } + return nil } func (i *ImageService) prepareInitLayer(ctx context.Context, id string, parent string, setupInit func(string) error) error { diff --git a/daemon/containerd/mount.go b/daemon/containerd/mount.go index d2cf45711d..0c8a079bbe 100644 --- a/daemon/containerd/mount.go +++ b/daemon/containerd/mount.go @@ -11,42 +11,30 @@ import ( // Mount mounts the container filesystem in a temporary location, use defer imageService.Unmount // to unmount the filesystem when calling this -func (i *ImageService) Mount(ctx context.Context, container *container.Container) error { - snapshotter := i.client.SnapshotService(container.Driver) - mounts, err := snapshotter.Mounts(ctx, container.ID) +func (i *ImageService) Mount(ctx context.Context, ctr *container.Container) error { + if ctr.RWLayer == nil { + return errors.New("RWLayer of container " + ctr.ID + " is unexpectedly nil") + } + + root, err := ctr.RWLayer.Mount(ctr.GetMountLabel()) if err != nil { - return err + return fmt.Errorf("failed to mount container %s: %w", ctr.ID, err) } - var root string - if root, err = i.refCountMounter.Mount(mounts, container.ID); err != nil { - return fmt.Errorf("failed to mount %s: %w", root, err) - } + log.G(ctx).WithFields(log.Fields{"ctr": ctr.ID, "root": root, "snapshotter": ctr.Driver}).Debug("ctr mounted via snapshotter") - log.G(ctx).WithFields(log.Fields{"container": container.ID, "root": root, "snapshotter": container.Driver}).Debug("container mounted via snapshotter") - - container.BaseFS = root + ctr.BaseFS = root return nil } // Unmount unmounts the container base filesystem -func (i *ImageService) Unmount(ctx context.Context, container *container.Container) error { - baseFS := container.BaseFS - if baseFS == "" { - target, err := i.refCountMounter.Mounted(container.ID) - if err != nil { - log.G(ctx).WithField("containerID", container.ID).Warn("failed to determine if container is already mounted") - } - if target == "" { - return errors.New("BaseFS is empty") - } - baseFS = target +func (i *ImageService) Unmount(ctx context.Context, ctr *container.Container) error { + if ctr.RWLayer == nil { + return errors.New("RWLayer of container " + ctr.ID + " is unexpectedly nil") } - if err := i.refCountMounter.Unmount(baseFS); err != nil { - log.G(ctx).WithField("container", container.ID).WithError(err).Error("error unmounting container") - return fmt.Errorf("failed to unmount %s: %w", baseFS, err) + if err := ctr.RWLayer.Unmount(); err != nil { + return fmt.Errorf("failed to unmount container %s: %w", ctr.ID, err) } - return nil } diff --git a/daemon/containerd/service.go b/daemon/containerd/service.go index 11bd003559..584b3ef5a6 100644 --- a/daemon/containerd/service.go +++ b/daemon/containerd/service.go @@ -20,7 +20,6 @@ import ( dimages "github.com/docker/docker/daemon/images" "github.com/docker/docker/daemon/snapshotter" "github.com/docker/docker/errdefs" - "github.com/docker/docker/layer" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/registry" "github.com/pkg/errors" @@ -108,13 +107,6 @@ func (i *ImageService) CountImages(ctx context.Context) int { return len(imgs) } -// CreateLayer creates a filesystem layer for a container. -// called from create.go -// TODO: accept an opt struct instead of container? -func (i *ImageService) CreateLayer(container *container.Container, initFunc layer.MountInit) (container.RWLayer, error) { - return nil, errdefs.NotImplemented(errdefs.NotImplemented(errors.New("not implemented"))) -} - // LayerStoreStatus returns the status for each layer store // called from info.go func (i *ImageService) LayerStoreStatus() [][2]string { @@ -143,12 +135,6 @@ func (i *ImageService) StorageDriver() string { return i.snapshotter } -// ReleaseLayer releases a layer allowing it to be removed -// called from delete.go Daemon.cleanupContainer(), and Daemon.containerExport() -func (i *ImageService) ReleaseLayer(rwlayer container.RWLayer) error { - return errdefs.NotImplemented(errors.New("not implemented")) -} - // LayerDiskUsage returns the number of bytes used by layer stores // called from disk_usage.go func (i *ImageService) LayerDiskUsage(ctx context.Context) (int64, error) { diff --git a/daemon/containerd/service_windows.go b/daemon/containerd/service_windows.go index 7a24ee72c4..dd8c1a4309 100644 --- a/daemon/containerd/service_windows.go +++ b/daemon/containerd/service_windows.go @@ -2,6 +2,7 @@ package containerd import ( "context" + "fmt" "github.com/docker/docker/container" "github.com/docker/docker/image" @@ -9,13 +10,17 @@ import ( ) // GetLayerFolders returns the layer folders from an image RootFS. -func (i *ImageService) GetLayerFolders(img *image.Image, rwLayer container.RWLayer, containerID string) ([]string, error) { - if rwLayer != nil { - return nil, errors.New("RWLayer is unexpectedly not nil") +func (i *ImageService) GetLayerFolders(img *image.Image, layer container.RWLayer, containerID string) ([]string, error) { + if layer == nil { + return nil, errors.New("RWLayer is unexpectedly nil") } - snapshotter := i.client.SnapshotService(i.StorageDriver()) - mounts, err := snapshotter.Mounts(context.TODO(), containerID) + c8dLayer, ok := layer.(*rwLayer) + if !ok { + return nil, fmt.Errorf("unexpected layer type: %T", layer) + } + + mounts, err := c8dLayer.mounts(context.TODO()) if err != nil { return nil, errors.Wrapf(err, "snapshotter.Mounts failed: container %s", containerID) } diff --git a/daemon/create.go b/daemon/create.go index 53b949d4db..ba001204f1 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -185,18 +185,12 @@ func (daemon *Daemon) create(ctx context.Context, daemonCfg *config.Config, opts ctr.HostConfig.StorageOpt = opts.params.HostConfig.StorageOpt ctr.ImageManifest = imgManifest - if daemon.UsesSnapshotter() { - if err := daemon.imageService.PrepareSnapshot(ctx, ctr.ID, opts.params.Config.Image, opts.params.Platform, setupInitLayer(daemon.idMapping)); err != nil { - return nil, err - } - } else { - // Set RWLayer for container after mount labels have been set - rwLayer, err := daemon.imageService.CreateLayer(ctr, setupInitLayer(daemon.idMapping)) - if err != nil { - return nil, errdefs.System(err) - } - ctr.RWLayer = rwLayer + // Set RWLayer for container after mount labels have been set + rwLayer, err := daemon.imageService.CreateLayer(ctr, setupInitLayer(daemon.idMapping)) + if err != nil { + return nil, errdefs.System(err) } + ctr.RWLayer = rwLayer current := idtools.CurrentIdentity() if err := idtools.MkdirAndChown(ctr.Root, 0o710, idtools.Identity{UID: current.UID, GID: daemon.IdentityMapping().RootPair().GID}); err != nil { diff --git a/daemon/daemon.go b/daemon/daemon.go index 22d43fbbe7..a9f835bfff 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -202,11 +202,6 @@ func (daemon *Daemon) UsesSnapshotter() bool { return daemon.usesSnapshotter } -// layerAccessor may be implemented by ImageService -type layerAccessor interface { - GetLayerByID(cid string) (container.RWLayer, error) -} - func (daemon *Daemon) restore(cfg *configStore) error { var mapLock sync.Mutex containers := make(map[string]*container.Container) @@ -248,14 +243,12 @@ func (daemon *Daemon) restore(cfg *configStore) error { logger.Debugf("not restoring container because it was created with another storage driver (%s)", c.Driver) return } - if accessor, ok := daemon.imageService.(layerAccessor); ok { - rwlayer, err := accessor.GetLayerByID(c.ID) - if err != nil { - logger.WithError(err).Error("failed to load container mount") - return - } - c.RWLayer = rwlayer + rwlayer, err := daemon.imageService.GetLayerByID(c.ID) + if err != nil { + logger.WithError(err).Error("failed to load container mount") + return } + c.RWLayer = rwlayer logger.WithFields(log.Fields{ "running": c.IsRunning(), "paused": c.IsPaused(), diff --git a/daemon/delete.go b/daemon/delete.go index 7b9d82bb06..9b351c09d1 100644 --- a/daemon/delete.go +++ b/daemon/delete.go @@ -8,8 +8,6 @@ import ( "strings" "time" - "github.com/containerd/containerd/leases" - cerrdefs "github.com/containerd/errdefs" "github.com/containerd/log" "github.com/docker/docker/api/types/backend" containertypes "github.com/docker/docker/api/types/container" @@ -151,19 +149,6 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, config ba container.SetRemovalError(err) return err } - } else { - if daemon.UsesSnapshotter() { - ls := daemon.containerdClient.LeasesService() - lease := leases.Lease{ - ID: container.ID, - } - if err := ls.Delete(context.Background(), lease, leases.SynchronousDelete); err != nil { - if !cerrdefs.IsNotFound(err) { - container.SetRemovalError(err) - return err - } - } - } } // Hold the container lock while deleting the container root directory diff --git a/daemon/image_service.go b/daemon/image_service.go index c1b94fc4ef..3acd308407 100644 --- a/daemon/image_service.go +++ b/daemon/image_service.go @@ -45,14 +45,11 @@ type ImageService interface { SquashImage(id, parent string) (string, error) ImageInspect(ctx context.Context, refOrID string, opts backend.ImageInspectOpts) (*imagetype.InspectResponse, error) - // Containerd related methods - - PrepareSnapshot(ctx context.Context, id string, parentImage string, platform *ocispec.Platform, setupInit func(string) error) error - // Layers GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (builder.Image, builder.ROLayer, error) CreateLayer(container *container.Container, initFunc layer.MountInit) (container.RWLayer, error) + GetLayerByID(cid string) (container.RWLayer, error) LayerStoreStatus() [][2]string GetLayerMountID(cid string) (string, error) ReleaseLayer(rwlayer container.RWLayer) error diff --git a/daemon/images/image.go b/daemon/images/image.go index bf3241759b..cbd69d8517 100644 --- a/daemon/images/image.go +++ b/daemon/images/image.go @@ -45,11 +45,6 @@ type manifest struct { Config ocispec.Descriptor `json:"config"` } -func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentImage string, platform *ocispec.Platform, setupInit func(string) error) error { - // Only makes sense when containerd image store is used - panic("not implemented") -} - func (i *ImageService) manifestMatchesPlatform(ctx context.Context, img *image.Image, platform ocispec.Platform) (bool, error) { ls, err := i.leases.ListResources(ctx, leases.Lease{ID: imageKey(img.ID().String())}) if err != nil {