diff --git a/daemon/containerd/image_changes.go b/daemon/containerd/image_changes.go index 9d2802520b..d51de8a06f 100644 --- a/daemon/containerd/image_changes.go +++ b/daemon/containerd/image_changes.go @@ -2,68 +2,35 @@ package containerd import ( "context" - "encoding/json" - "github.com/containerd/containerd/content" "github.com/containerd/containerd/log" "github.com/containerd/containerd/mount" "github.com/docker/docker/container" "github.com/docker/docker/pkg/archive" - "github.com/google/uuid" - "github.com/opencontainers/image-spec/identity" - ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) func (i *ImageService) Changes(ctx context.Context, container *container.Container) ([]archive.Change, error) { - cs := i.client.ContentStore() - - imageManifest, err := getContainerImageManifest(container) - if err != nil { - return nil, err - } - - imageManifestBytes, err := content.ReadBlob(ctx, cs, imageManifest) - if err != nil { - return nil, err - } - var manifest ocispec.Manifest - if err := json.Unmarshal(imageManifestBytes, &manifest); err != nil { - return nil, err - } - - imageConfigBytes, err := content.ReadBlob(ctx, cs, manifest.Config) - if err != nil { - return nil, err - } - var image ocispec.Image - if err := json.Unmarshal(imageConfigBytes, &image); err != nil { - return nil, err - } - - rnd, err := uuid.NewRandom() - if err != nil { - return nil, err - } - snapshotter := i.client.SnapshotService(container.Driver) - - diffIDs := image.RootFS.DiffIDs - parent, err := snapshotter.View(ctx, rnd.String(), identity.ChainID(diffIDs).String()) + info, err := snapshotter.Stat(ctx, container.ID) if err != nil { return nil, err } + + imageMounts, _ := snapshotter.View(ctx, container.ID+"-parent-view", info.Parent) + defer func() { - if err := snapshotter.Remove(ctx, rnd.String()); err != nil { - log.G(ctx).WithError(err).WithField("key", rnd.String()).Warn("remove temporary snapshot") + if err := snapshotter.Remove(ctx, container.ID+"-parent-view"); err != nil { + log.G(ctx).WithError(err).Warn("error removing the parent view snapshot") } }() var changes []archive.Change - err = i.PerformWithBaseFS(ctx, container, func(containerRootfs string) error { - return mount.WithReadonlyTempMount(ctx, parent, func(parentRootfs string) error { - changes, err = archive.ChangesDirs(containerRootfs, parentRootfs) + err = i.PerformWithBaseFS(ctx, container, func(containerRoot string) error { + return mount.WithReadonlyTempMount(ctx, imageMounts, func(imageRoot string) error { + changes, err = archive.ChangesDirs(containerRoot, imageRoot) return err }) }) + return changes, err } diff --git a/daemon/containerd/image_snapshot.go b/daemon/containerd/image_snapshot.go index 40eeccdd0c..190c2e0321 100644 --- a/daemon/containerd/image_snapshot.go +++ b/daemon/containerd/image_snapshot.go @@ -8,6 +8,7 @@ import ( cerrdefs "github.com/containerd/containerd/errdefs" containerdimages "github.com/containerd/containerd/images" "github.com/containerd/containerd/leases" + "github.com/containerd/containerd/mount" "github.com/containerd/containerd/platforms" "github.com/containerd/containerd/snapshots" "github.com/docker/docker/errdefs" @@ -19,7 +20,7 @@ import ( const remapSuffix = "-remap" // 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) error { +func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentImage string, platform *ocispec.Platform, setupInit func(string) error) error { var parentSnapshot string if parentImage != "" { img, err := i.resolveImage(ctx, parentImage) @@ -59,27 +60,42 @@ func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentIma parentSnapshot = identity.ChainID(diffIDs).String() } - // Add a lease so that containerd doesn't garbage collect our snapshot ls := i.client.LeasesService() lease, err := ls.Create(ctx, leases.WithID(id)) if err != nil { return err } - if err := ls.AddResource(ctx, lease, leases.Resource{ - ID: id, - Type: "snapshots/" + i.StorageDriver(), + 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 !i.idMapping.Empty() { + return i.remapSnapshot(ctx, snapshotter, id, id+"-init") + } + + _, err = snapshotter.Prepare(ctx, id, id+"-init") + return err +} + +func (i *ImageService) prepareInitLayer(ctx context.Context, id string, parent string, setupInit func(string) error) error { + snapshotter := i.client.SnapshotService(i.StorageDriver()) + + mounts, err := snapshotter.Prepare(ctx, id+"-init-key", parent) + if err != nil { + return err + } + + if err := mount.WithTempMount(ctx, mounts, func(root string) error { + return setupInit(root) }); err != nil { return err } - snapshotter := i.client.SnapshotService(i.StorageDriver()) - - if !i.idMapping.Empty() { - return i.remapSnapshot(ctx, snapshotter, id, parentSnapshot, lease) - } - - _, err = snapshotter.Prepare(ctx, id, parentSnapshot) - return err + return snapshotter.Commit(ctx, id+"-init", id+"-init-key") } // calculateSnapshotParentUsage returns the usage of all ancestors of the diff --git a/daemon/containerd/image_snapshot_unix.go b/daemon/containerd/image_snapshot_unix.go index d5e2977601..ee85bc8839 100644 --- a/daemon/containerd/image_snapshot_unix.go +++ b/daemon/containerd/image_snapshot_unix.go @@ -9,15 +9,13 @@ import ( "path/filepath" "syscall" - "github.com/containerd/containerd/leases" "github.com/containerd/containerd/log" "github.com/containerd/containerd/mount" "github.com/containerd/containerd/snapshots" "github.com/docker/docker/pkg/idtools" ) -func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots.Snapshotter, id string, parentSnapshot string, lease leases.Lease) error { - ls := i.client.LeasesService() +func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots.Snapshotter, id string, parentSnapshot string) error { rootPair := i.idMapping.RootPair() usernsID := fmt.Sprintf("%s-%d-%d", parentSnapshot, rootPair.UID, rootPair.GID) remappedID := usernsID + remapSuffix @@ -28,19 +26,6 @@ func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots. return err } - if err := ls.AddResource(ctx, lease, leases.Resource{ - ID: remappedID, - Type: "snapshots/" + i.StorageDriver(), - }); err != nil { - return err - } - if err := ls.AddResource(ctx, lease, leases.Resource{ - ID: usernsID, - Type: "snapshots/" + i.StorageDriver(), - }); err != nil { - return err - } - mounts, err := snapshotter.Prepare(ctx, remappedID, parentSnapshot) if err != nil { return err diff --git a/daemon/containerd/image_snapshot_windows.go b/daemon/containerd/image_snapshot_windows.go index 7330efd17f..bb4ed3ac12 100644 --- a/daemon/containerd/image_snapshot_windows.go +++ b/daemon/containerd/image_snapshot_windows.go @@ -3,10 +3,9 @@ package containerd import ( "context" - "github.com/containerd/containerd/leases" "github.com/containerd/containerd/snapshots" ) -func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots.Snapshotter, id string, parentSnapshot string, lease leases.Lease) error { +func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots.Snapshotter, id string, parentSnapshot string) error { return nil } diff --git a/daemon/containerd/service.go b/daemon/containerd/service.go index c751c81f96..0fee95df9f 100644 --- a/daemon/containerd/service.go +++ b/daemon/containerd/service.go @@ -21,7 +21,6 @@ import ( "github.com/docker/docker/layer" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/registry" - ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" ) @@ -186,14 +185,3 @@ func (i *ImageService) GetContainerLayerSize(ctx context.Context, containerID st // TODO(thaJeztah): include content-store size for the image (similar to "GET /images/json") return rwLayerUsage.Size, rwLayerUsage.Size + unpackedUsage.Size, nil } - -// getContainerImageManifest safely dereferences ImageManifest. -// ImageManifest can be nil for containers created with Docker Desktop with old -// containerd image store integration enabled which didn't set this field. -func getContainerImageManifest(ctr *container.Container) (ocispec.Descriptor, error) { - if ctr.ImageManifest == nil { - return ocispec.Descriptor{}, errdefs.InvalidParameter(errors.New("container is missing ImageManifest (probably created on old version), please recreate it")) - } - - return *ctr.ImageManifest, nil -} diff --git a/daemon/create.go b/daemon/create.go index 682b2b2a65..834dcb0109 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -195,7 +195,7 @@ func (daemon *Daemon) create(ctx context.Context, daemonCfg *config.Config, opts ctr.ImageManifest = imgManifest if daemon.UsesSnapshotter() { - if err := daemon.imageService.PrepareSnapshot(ctx, ctr.ID, opts.params.Config.Image, opts.params.Platform); err != nil { + if err := daemon.imageService.PrepareSnapshot(ctx, ctr.ID, opts.params.Config.Image, opts.params.Platform, setupInitLayer(daemon.idMapping)); err != nil { return nil, err } } else { diff --git a/daemon/image_service.go b/daemon/image_service.go index 9945d8ad32..5b28fbe8ef 100644 --- a/daemon/image_service.go +++ b/daemon/image_service.go @@ -47,7 +47,7 @@ type ImageService interface { // Containerd related methods - PrepareSnapshot(ctx context.Context, id string, image string, platform *ocispec.Platform) error + PrepareSnapshot(ctx context.Context, id string, parentImage string, platform *ocispec.Platform, setupInit func(string) error) error GetImageManifest(ctx context.Context, refOrID string, options imagetype.GetImageOpts) (*ocispec.Descriptor, error) // Layers diff --git a/daemon/images/image.go b/daemon/images/image.go index 4ac75c7592..6c53cda35d 100644 --- a/daemon/images/image.go +++ b/daemon/images/image.go @@ -46,7 +46,7 @@ type manifest struct { Config ocispec.Descriptor `json:"config"` } -func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, image string, platform *ocispec.Platform) error { +func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentImage string, platform *ocispec.Platform, setupInit func(string) error) error { // Only makes sense when conatinerd image store is used panic("not implemented") } diff --git a/integration/container/diff_test.go b/integration/container/diff_test.go index e01a90e5c3..29068c2513 100644 --- a/integration/container/diff_test.go +++ b/integration/container/diff_test.go @@ -12,22 +12,38 @@ import ( ) func TestDiff(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType == "windows", "cannot diff a running container on Windows") + ctx := setupTest(t) + apiClient := testEnv.APIClient() + + cID := container.Run(ctx, t, apiClient, container.WithCmd("sh", "-c", `mkdir /foo; echo xyzzy > /foo/bar`)) + + expected := []containertypes.FilesystemChange{ + {Kind: containertypes.ChangeAdd, Path: "/foo"}, + {Kind: containertypes.ChangeAdd, Path: "/foo/bar"}, + } + + items, err := apiClient.ContainerDiff(ctx, cID) + assert.NilError(t, err) + assert.DeepEqual(t, expected, items) +} + +func TestDiffStoppedContainer(t *testing.T) { + // There's no way in Windows to differentiate between an Add or a Modify, + // and all files are under a "Files/" prefix. skip.If(t, testEnv.DaemonInfo.OSType == "windows", "FIXME") ctx := setupTest(t) apiClient := testEnv.APIClient() cID := container.Run(ctx, t, apiClient, container.WithCmd("sh", "-c", `mkdir /foo; echo xyzzy > /foo/bar`)) - // Wait for it to exit as cannot diff a running container on Windows, and - // it will take a few seconds to exit. Also there's no way in Windows to - // differentiate between an Add or a Modify, and all files are under - // a "Files/" prefix. + poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "exited"), poll.WithDelay(100*time.Millisecond), poll.WithTimeout(60*time.Second)) + expected := []containertypes.FilesystemChange{ {Kind: containertypes.ChangeAdd, Path: "/foo"}, {Kind: containertypes.ChangeAdd, Path: "/foo/bar"}, } if testEnv.DaemonInfo.OSType == "windows" { - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "exited"), poll.WithDelay(100*time.Millisecond), poll.WithTimeout(60*time.Second)) expected = []containertypes.FilesystemChange{ {Kind: containertypes.ChangeModify, Path: "Files/foo"}, {Kind: containertypes.ChangeModify, Path: "Files/foo/bar"},