Przeglądaj źródła

Merge pull request #46502 from rumpl/c8d-fix-diff

c8d: Fix `docker diff`
Sebastiaan van Stijn 1 rok temu
rodzic
commit
4dbfe7e17e

+ 9 - 42
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()
+	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
 }

+ 26 - 10
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,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) {

+ 1 - 16
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

+ 1 - 2
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
 }

+ 0 - 12
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
-}

+ 1 - 1
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 {

+ 1 - 1
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

+ 1 - 1
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")
 }

+ 21 - 5
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"},