Browse code

c8d: Fix `docker diff`

Diffing a container yielded some extra changes that come from the
files/directories that we mount inside the container (/etc/resolv.conf
for example). To avoid that we create an intermediate snapshot that has
these files, with this we can now diff the container fs with its parent
and only get the differences that were made inside the container.

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>

Djordje Lukic authored on 2023/09/17 03:06:12
Showing 9 changed files
... ...
@@ -2,68 +2,35 @@ package containerd
2 2
 
3 3
 import (
4 4
 	"context"
5
-	"encoding/json"
6 5
 
7
-	"github.com/containerd/containerd/content"
8 6
 	"github.com/containerd/containerd/log"
9 7
 	"github.com/containerd/containerd/mount"
10 8
 	"github.com/docker/docker/container"
11 9
 	"github.com/docker/docker/pkg/archive"
12
-	"github.com/google/uuid"
13
-	"github.com/opencontainers/image-spec/identity"
14
-	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
15 10
 )
16 11
 
17 12
 func (i *ImageService) Changes(ctx context.Context, container *container.Container) ([]archive.Change, error) {
18
-	cs := i.client.ContentStore()
19
-
20
-	imageManifest, err := getContainerImageManifest(container)
21
-	if err != nil {
22
-		return nil, err
23
-	}
24
-
25
-	imageManifestBytes, err := content.ReadBlob(ctx, cs, imageManifest)
26
-	if err != nil {
27
-		return nil, err
28
-	}
29
-	var manifest ocispec.Manifest
30
-	if err := json.Unmarshal(imageManifestBytes, &manifest); err != nil {
31
-		return nil, err
32
-	}
33
-
34
-	imageConfigBytes, err := content.ReadBlob(ctx, cs, manifest.Config)
35
-	if err != nil {
36
-		return nil, err
37
-	}
38
-	var image ocispec.Image
39
-	if err := json.Unmarshal(imageConfigBytes, &image); err != nil {
40
-		return nil, err
41
-	}
42
-
43
-	rnd, err := uuid.NewRandom()
13
+	snapshotter := i.client.SnapshotService(container.Driver)
14
+	info, err := snapshotter.Stat(ctx, container.ID)
44 15
 	if err != nil {
45 16
 		return nil, err
46 17
 	}
47 18
 
48
-	snapshotter := i.client.SnapshotService(container.Driver)
19
+	imageMounts, _ := snapshotter.View(ctx, container.ID+"-parent-view", info.Parent)
49 20
 
50
-	diffIDs := image.RootFS.DiffIDs
51
-	parent, err := snapshotter.View(ctx, rnd.String(), identity.ChainID(diffIDs).String())
52
-	if err != nil {
53
-		return nil, err
54
-	}
55 21
 	defer func() {
56
-		if err := snapshotter.Remove(ctx, rnd.String()); err != nil {
57
-			log.G(ctx).WithError(err).WithField("key", rnd.String()).Warn("remove temporary snapshot")
22
+		if err := snapshotter.Remove(ctx, container.ID+"-parent-view"); err != nil {
23
+			log.G(ctx).WithError(err).Warn("error removing the parent view snapshot")
58 24
 		}
59 25
 	}()
60 26
 
61 27
 	var changes []archive.Change
62
-	err = i.PerformWithBaseFS(ctx, container, func(containerRootfs string) error {
63
-		return mount.WithReadonlyTempMount(ctx, parent, func(parentRootfs string) error {
64
-			changes, err = archive.ChangesDirs(containerRootfs, parentRootfs)
28
+	err = i.PerformWithBaseFS(ctx, container, func(containerRoot string) error {
29
+		return mount.WithReadonlyTempMount(ctx, imageMounts, func(imageRoot string) error {
30
+			changes, err = archive.ChangesDirs(containerRoot, imageRoot)
65 31
 			return err
66 32
 		})
67 33
 	})
34
+
68 35
 	return changes, err
69 36
 }
... ...
@@ -8,6 +8,7 @@ import (
8 8
 	cerrdefs "github.com/containerd/containerd/errdefs"
9 9
 	containerdimages "github.com/containerd/containerd/images"
10 10
 	"github.com/containerd/containerd/leases"
11
+	"github.com/containerd/containerd/mount"
11 12
 	"github.com/containerd/containerd/platforms"
12 13
 	"github.com/containerd/containerd/snapshots"
13 14
 	"github.com/docker/docker/errdefs"
... ...
@@ -19,7 +20,7 @@ import (
19 19
 const remapSuffix = "-remap"
20 20
 
21 21
 // PrepareSnapshot prepares a snapshot from a parent image for a container
22
-func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentImage string, platform *ocispec.Platform) error {
22
+func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentImage string, platform *ocispec.Platform, setupInit func(string) error) error {
23 23
 	var parentSnapshot string
24 24
 	if parentImage != "" {
25 25
 		img, err := i.resolveImage(ctx, parentImage)
... ...
@@ -59,29 +60,44 @@ func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentIma
59 59
 		parentSnapshot = identity.ChainID(diffIDs).String()
60 60
 	}
61 61
 
62
-	// Add a lease so that containerd doesn't garbage collect our snapshot
63 62
 	ls := i.client.LeasesService()
64 63
 	lease, err := ls.Create(ctx, leases.WithID(id))
65 64
 	if err != nil {
66 65
 		return err
67 66
 	}
68
-	if err := ls.AddResource(ctx, lease, leases.Resource{
69
-		ID:   id,
70
-		Type: "snapshots/" + i.StorageDriver(),
71
-	}); err != nil {
72
-		return err
73
-	}
67
+	ctx = leases.WithLease(ctx, lease.ID)
74 68
 
75 69
 	snapshotter := i.client.SnapshotService(i.StorageDriver())
76 70
 
71
+	if err := i.prepareInitLayer(ctx, id, parentSnapshot, setupInit); err != nil {
72
+		return err
73
+	}
74
+
77 75
 	if !i.idMapping.Empty() {
78
-		return i.remapSnapshot(ctx, snapshotter, id, parentSnapshot, lease)
76
+		return i.remapSnapshot(ctx, snapshotter, id, id+"-init")
79 77
 	}
80 78
 
81
-	_, err = snapshotter.Prepare(ctx, id, parentSnapshot)
79
+	_, err = snapshotter.Prepare(ctx, id, id+"-init")
82 80
 	return err
83 81
 }
84 82
 
83
+func (i *ImageService) prepareInitLayer(ctx context.Context, id string, parent string, setupInit func(string) error) error {
84
+	snapshotter := i.client.SnapshotService(i.StorageDriver())
85
+
86
+	mounts, err := snapshotter.Prepare(ctx, id+"-init-key", parent)
87
+	if err != nil {
88
+		return err
89
+	}
90
+
91
+	if err := mount.WithTempMount(ctx, mounts, func(root string) error {
92
+		return setupInit(root)
93
+	}); err != nil {
94
+		return err
95
+	}
96
+
97
+	return snapshotter.Commit(ctx, id+"-init", id+"-init-key")
98
+}
99
+
85 100
 // calculateSnapshotParentUsage returns the usage of all ancestors of the
86 101
 // provided snapshot. It doesn't include the size of the snapshot itself.
87 102
 func calculateSnapshotParentUsage(ctx context.Context, snapshotter snapshots.Snapshotter, snapshotID string) (snapshots.Usage, error) {
... ...
@@ -9,15 +9,13 @@ import (
9 9
 	"path/filepath"
10 10
 	"syscall"
11 11
 
12
-	"github.com/containerd/containerd/leases"
13 12
 	"github.com/containerd/containerd/log"
14 13
 	"github.com/containerd/containerd/mount"
15 14
 	"github.com/containerd/containerd/snapshots"
16 15
 	"github.com/docker/docker/pkg/idtools"
17 16
 )
18 17
 
19
-func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots.Snapshotter, id string, parentSnapshot string, lease leases.Lease) error {
20
-	ls := i.client.LeasesService()
18
+func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots.Snapshotter, id string, parentSnapshot string) error {
21 19
 	rootPair := i.idMapping.RootPair()
22 20
 	usernsID := fmt.Sprintf("%s-%d-%d", parentSnapshot, rootPair.UID, rootPair.GID)
23 21
 	remappedID := usernsID + remapSuffix
... ...
@@ -28,19 +26,6 @@ func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots.
28 28
 		return err
29 29
 	}
30 30
 
31
-	if err := ls.AddResource(ctx, lease, leases.Resource{
32
-		ID:   remappedID,
33
-		Type: "snapshots/" + i.StorageDriver(),
34
-	}); err != nil {
35
-		return err
36
-	}
37
-	if err := ls.AddResource(ctx, lease, leases.Resource{
38
-		ID:   usernsID,
39
-		Type: "snapshots/" + i.StorageDriver(),
40
-	}); err != nil {
41
-		return err
42
-	}
43
-
44 31
 	mounts, err := snapshotter.Prepare(ctx, remappedID, parentSnapshot)
45 32
 	if err != nil {
46 33
 		return err
... ...
@@ -3,10 +3,9 @@ package containerd
3 3
 import (
4 4
 	"context"
5 5
 
6
-	"github.com/containerd/containerd/leases"
7 6
 	"github.com/containerd/containerd/snapshots"
8 7
 )
9 8
 
10
-func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots.Snapshotter, id string, parentSnapshot string, lease leases.Lease) error {
9
+func (i *ImageService) remapSnapshot(ctx context.Context, snapshotter snapshots.Snapshotter, id string, parentSnapshot string) error {
11 10
 	return nil
12 11
 }
... ...
@@ -21,7 +21,6 @@ import (
21 21
 	"github.com/docker/docker/layer"
22 22
 	"github.com/docker/docker/pkg/idtools"
23 23
 	"github.com/docker/docker/registry"
24
-	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
25 24
 	"github.com/pkg/errors"
26 25
 )
27 26
 
... ...
@@ -186,14 +185,3 @@ func (i *ImageService) GetContainerLayerSize(ctx context.Context, containerID st
186 186
 	// TODO(thaJeztah): include content-store size for the image (similar to "GET /images/json")
187 187
 	return rwLayerUsage.Size, rwLayerUsage.Size + unpackedUsage.Size, nil
188 188
 }
189
-
190
-// getContainerImageManifest safely dereferences ImageManifest.
191
-// ImageManifest can be nil for containers created with Docker Desktop with old
192
-// containerd image store integration enabled which didn't set this field.
193
-func getContainerImageManifest(ctr *container.Container) (ocispec.Descriptor, error) {
194
-	if ctr.ImageManifest == nil {
195
-		return ocispec.Descriptor{}, errdefs.InvalidParameter(errors.New("container is missing ImageManifest (probably created on old version), please recreate it"))
196
-	}
197
-
198
-	return *ctr.ImageManifest, nil
199
-}
... ...
@@ -195,7 +195,7 @@ func (daemon *Daemon) create(ctx context.Context, daemonCfg *config.Config, opts
195 195
 	ctr.ImageManifest = imgManifest
196 196
 
197 197
 	if daemon.UsesSnapshotter() {
198
-		if err := daemon.imageService.PrepareSnapshot(ctx, ctr.ID, opts.params.Config.Image, opts.params.Platform); err != nil {
198
+		if err := daemon.imageService.PrepareSnapshot(ctx, ctr.ID, opts.params.Config.Image, opts.params.Platform, setupInitLayer(daemon.idMapping)); err != nil {
199 199
 			return nil, err
200 200
 		}
201 201
 	} else {
... ...
@@ -47,7 +47,7 @@ type ImageService interface {
47 47
 
48 48
 	// Containerd related methods
49 49
 
50
-	PrepareSnapshot(ctx context.Context, id string, image string, platform *ocispec.Platform) error
50
+	PrepareSnapshot(ctx context.Context, id string, parentImage string, platform *ocispec.Platform, setupInit func(string) error) error
51 51
 	GetImageManifest(ctx context.Context, refOrID string, options imagetype.GetImageOpts) (*ocispec.Descriptor, error)
52 52
 
53 53
 	// Layers
... ...
@@ -46,7 +46,7 @@ type manifest struct {
46 46
 	Config ocispec.Descriptor `json:"config"`
47 47
 }
48 48
 
49
-func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, image string, platform *ocispec.Platform) error {
49
+func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentImage string, platform *ocispec.Platform, setupInit func(string) error) error {
50 50
 	// Only makes sense when conatinerd image store is used
51 51
 	panic("not implemented")
52 52
 }
... ...
@@ -12,22 +12,38 @@ import (
12 12
 )
13 13
 
14 14
 func TestDiff(t *testing.T) {
15
+	skip.If(t, testEnv.DaemonInfo.OSType == "windows", "cannot diff a running container on Windows")
16
+	ctx := setupTest(t)
17
+	apiClient := testEnv.APIClient()
18
+
19
+	cID := container.Run(ctx, t, apiClient, container.WithCmd("sh", "-c", `mkdir /foo; echo xyzzy > /foo/bar`))
20
+
21
+	expected := []containertypes.FilesystemChange{
22
+		{Kind: containertypes.ChangeAdd, Path: "/foo"},
23
+		{Kind: containertypes.ChangeAdd, Path: "/foo/bar"},
24
+	}
25
+
26
+	items, err := apiClient.ContainerDiff(ctx, cID)
27
+	assert.NilError(t, err)
28
+	assert.DeepEqual(t, expected, items)
29
+}
30
+
31
+func TestDiffStoppedContainer(t *testing.T) {
32
+	// There's no way in Windows to differentiate between an Add or a Modify,
33
+	// and all files are under a "Files/" prefix.
15 34
 	skip.If(t, testEnv.DaemonInfo.OSType == "windows", "FIXME")
16 35
 	ctx := setupTest(t)
17 36
 	apiClient := testEnv.APIClient()
18 37
 
19 38
 	cID := container.Run(ctx, t, apiClient, container.WithCmd("sh", "-c", `mkdir /foo; echo xyzzy > /foo/bar`))
20 39
 
21
-	// Wait for it to exit as cannot diff a running container on Windows, and
22
-	// it will take a few seconds to exit. Also there's no way in Windows to
23
-	// differentiate between an Add or a Modify, and all files are under
24
-	// a "Files/" prefix.
40
+	poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "exited"), poll.WithDelay(100*time.Millisecond), poll.WithTimeout(60*time.Second))
41
+
25 42
 	expected := []containertypes.FilesystemChange{
26 43
 		{Kind: containertypes.ChangeAdd, Path: "/foo"},
27 44
 		{Kind: containertypes.ChangeAdd, Path: "/foo/bar"},
28 45
 	}
29 46
 	if testEnv.DaemonInfo.OSType == "windows" {
30
-		poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "exited"), poll.WithDelay(100*time.Millisecond), poll.WithTimeout(60*time.Second))
31 47
 		expected = []containertypes.FilesystemChange{
32 48
 			{Kind: containertypes.ChangeModify, Path: "Files/foo"},
33 49
 			{Kind: containertypes.ChangeModify, Path: "Files/foo/bar"},