Skip to content

Commit

Permalink
Merge pull request moby#46502 from rumpl/c8d-fix-diff
Browse files Browse the repository at this point in the history
c8d: Fix `docker diff`
thaJeztah authored Sep 20, 2023
2 parents d8a51d2 + 207c4d5 commit 4dbfe7e
Showing 9 changed files with 61 additions and 90 deletions.
51 changes: 9 additions & 42 deletions daemon/containerd/image_changes.go
Original file line number Diff line number Diff line change
@@ -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()
snapshotter := i.client.SnapshotService(container.Driver)
info, err := snapshotter.Stat(ctx, container.ID)
if err != nil {
return nil, err
}

snapshotter := i.client.SnapshotService(container.Driver)
imageMounts, _ := snapshotter.View(ctx, container.ID+"-parent-view", info.Parent)

diffIDs := image.RootFS.DiffIDs
parent, err := snapshotter.View(ctx, rnd.String(), identity.ChainID(diffIDs).String())
if err != nil {
return nil, err
}
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
}
36 changes: 26 additions & 10 deletions daemon/containerd/image_snapshot.go
Original file line number Diff line number Diff line change
@@ -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,29 +60,44 @@ 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(),
}); err != nil {
return 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 !i.idMapping.Empty() {
return i.remapSnapshot(ctx, snapshotter, id, parentSnapshot, lease)
return i.remapSnapshot(ctx, snapshotter, id, id+"-init")
}

_, err = snapshotter.Prepare(ctx, id, parentSnapshot)
_, 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
}

return snapshotter.Commit(ctx, id+"-init", id+"-init-key")
}

// calculateSnapshotParentUsage returns the usage of all ancestors of the
// provided snapshot. It doesn't include the size of the snapshot itself.
func calculateSnapshotParentUsage(ctx context.Context, snapshotter snapshots.Snapshotter, snapshotID string) (snapshots.Usage, error) {
17 changes: 1 addition & 16 deletions daemon/containerd/image_snapshot_unix.go
Original file line number Diff line number Diff line change
@@ -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
3 changes: 1 addition & 2 deletions daemon/containerd/image_snapshot_windows.go
Original file line number Diff line number Diff line change
@@ -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
}
12 changes: 0 additions & 12 deletions daemon/containerd/service.go
Original file line number Diff line number Diff line change
@@ -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
}
2 changes: 1 addition & 1 deletion daemon/create.go
Original file line number Diff line number Diff line change
@@ -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 {
2 changes: 1 addition & 1 deletion daemon/image_service.go
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion daemon/images/image.go
Original file line number Diff line number Diff line change
@@ -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")
}
26 changes: 21 additions & 5 deletions integration/container/diff_test.go
Original file line number Diff line number Diff line change
@@ -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"},

0 comments on commit 4dbfe7e

Please sign in to comment.