Browse code

c8d/commit: Don't produce an empty layer

If the diff is empty and don't produce an empty layer.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>

Paweł Gronowski authored on 2023/08/21 23:50:07
Showing 2 changed files
... ...
@@ -21,6 +21,7 @@ import (
21 21
 	"github.com/containerd/containerd/snapshots"
22 22
 	"github.com/docker/docker/api/types/backend"
23 23
 	"github.com/docker/docker/image"
24
+	"github.com/docker/docker/pkg/archive"
24 25
 	"github.com/opencontainers/go-digest"
25 26
 	"github.com/opencontainers/image-spec/identity"
26 27
 	"github.com/opencontainers/image-spec/specs-go"
... ...
@@ -38,28 +39,27 @@ func (i *ImageService) CommitImage(ctx context.Context, cc backend.CommitConfig)
38 38
 	container := i.containers.Get(cc.ContainerID)
39 39
 	cs := i.client.ContentStore()
40 40
 
41
-	imageManifest, err := getContainerImageManifest(container)
42
-	if err != nil {
43
-		return "", err
44
-	}
41
+	var parentManifest ocispec.Manifest
42
+	var parentImage ocispec.Image
45 43
 
46
-	imageManifestBytes, err := content.ReadBlob(ctx, cs, imageManifest)
47
-	if err != nil {
48
-		return "", err
49
-	}
44
+	// ImageManifest can be nil when committing an image with base FROM scratch
45
+	if container.ImageManifest != nil {
46
+		imageManifestBytes, err := content.ReadBlob(ctx, cs, *container.ImageManifest)
47
+		if err != nil {
48
+			return "", err
49
+		}
50 50
 
51
-	var manifest ocispec.Manifest
52
-	if err := json.Unmarshal(imageManifestBytes, &manifest); err != nil {
53
-		return "", err
54
-	}
51
+		if err := json.Unmarshal(imageManifestBytes, &parentManifest); err != nil {
52
+			return "", err
53
+		}
55 54
 
56
-	imageConfigBytes, err := content.ReadBlob(ctx, cs, manifest.Config)
57
-	if err != nil {
58
-		return "", err
59
-	}
60
-	var ociimage ocispec.Image
61
-	if err := json.Unmarshal(imageConfigBytes, &ociimage); err != nil {
62
-		return "", err
55
+		imageConfigBytes, err := content.ReadBlob(ctx, cs, parentManifest.Config)
56
+		if err != nil {
57
+			return "", err
58
+		}
59
+		if err := json.Unmarshal(imageConfigBytes, &parentImage); err != nil {
60
+			return "", err
61
+		}
63 62
 	}
64 63
 
65 64
 	var (
... ...
@@ -78,15 +78,19 @@ func (i *ImageService) CommitImage(ctx context.Context, cc backend.CommitConfig)
78 78
 	if err != nil {
79 79
 		return "", fmt.Errorf("failed to export layer: %w", err)
80 80
 	}
81
+	imageConfig := generateCommitImageConfig(parentImage, diffID, cc)
82
+
83
+	layers := parentManifest.Layers
84
+	if diffLayerDesc != nil {
85
+		rootfsID := identity.ChainID(imageConfig.RootFS.DiffIDs).String()
81 86
 
82
-	imageConfig := generateCommitImageConfig(ociimage, diffID, cc)
87
+		if err := applyDiffLayer(ctx, rootfsID, parentImage, sn, differ, *diffLayerDesc); err != nil {
88
+			return "", fmt.Errorf("failed to apply diff: %w", err)
89
+		}
83 90
 
84
-	rootfsID := identity.ChainID(imageConfig.RootFS.DiffIDs).String()
85
-	if err := applyDiffLayer(ctx, rootfsID, ociimage, sn, differ, diffLayerDesc); err != nil {
86
-		return "", fmt.Errorf("failed to apply diff: %w", err)
91
+		layers = append(layers, *diffLayerDesc)
87 92
 	}
88 93
 
89
-	layers := append(manifest.Layers, diffLayerDesc)
90 94
 	commitManifestDesc, err := writeContentsForImage(ctx, container.Driver, cs, imageConfig, layers)
91 95
 	if err != nil {
92 96
 		return "", err
... ...
@@ -130,6 +134,12 @@ func generateCommitImageConfig(baseConfig ocispec.Image, diffID digest.Digest, o
130 130
 		log.G(context.TODO()).Warnf("assuming os=%q", os)
131 131
 	}
132 132
 	log.G(context.TODO()).Debugf("generateCommitImageConfig(): arch=%q, os=%q", arch, os)
133
+
134
+	diffIds := baseConfig.RootFS.DiffIDs
135
+	if diffID != "" {
136
+		diffIds = append(diffIds, diffID)
137
+	}
138
+
133 139
 	return ocispec.Image{
134 140
 		Platform: ocispec.Platform{
135 141
 			Architecture: arch,
... ...
@@ -140,7 +150,7 @@ func generateCommitImageConfig(baseConfig ocispec.Image, diffID digest.Digest, o
140 140
 		Config:  containerConfigToOciImageConfig(opts.Config),
141 141
 		RootFS: ocispec.RootFS{
142 142
 			Type:    "layers",
143
-			DiffIDs: append(baseConfig.RootFS.DiffIDs, diffID),
143
+			DiffIDs: diffIds,
144 144
 		},
145 145
 		History: append(baseConfig.History, ocispec.History{
146 146
 			Created:   &createdTime,
... ...
@@ -217,28 +227,43 @@ func writeContentsForImage(ctx context.Context, snName string, cs content.Store,
217 217
 }
218 218
 
219 219
 // createDiff creates a layer diff into containerd's content store.
220
-func createDiff(ctx context.Context, name string, sn snapshots.Snapshotter, cs content.Store, comparer diff.Comparer) (ocispec.Descriptor, digest.Digest, error) {
220
+// If the diff is empty it returns nil empty digest and no error.
221
+func createDiff(ctx context.Context, name string, sn snapshots.Snapshotter, cs content.Store, comparer diff.Comparer) (*ocispec.Descriptor, digest.Digest, error) {
221 222
 	newDesc, err := rootfs.CreateDiff(ctx, name, sn, comparer)
222 223
 	if err != nil {
223
-		return ocispec.Descriptor{}, "", err
224
+		return nil, "", err
225
+	}
226
+
227
+	ra, err := cs.ReaderAt(ctx, newDesc)
228
+	if err != nil {
229
+		return nil, "", fmt.Errorf("failed to read diff archive: %w", err)
230
+	}
231
+	defer ra.Close()
232
+
233
+	empty, err := archive.IsEmpty(content.NewReader(ra))
234
+	if err != nil {
235
+		return nil, "", fmt.Errorf("failed to check if archive is empty: %w", err)
236
+	}
237
+	if empty {
238
+		return nil, "", nil
224 239
 	}
225 240
 
226 241
 	info, err := cs.Info(ctx, newDesc.Digest)
227 242
 	if err != nil {
228
-		return ocispec.Descriptor{}, "", err
243
+		return nil, "", fmt.Errorf("failed to get content info: %w", err)
229 244
 	}
230 245
 
231 246
 	diffIDStr, ok := info.Labels["containerd.io/uncompressed"]
232 247
 	if !ok {
233
-		return ocispec.Descriptor{}, "", fmt.Errorf("invalid differ response with no diffID")
248
+		return nil, "", fmt.Errorf("invalid differ response with no diffID")
234 249
 	}
235 250
 
236 251
 	diffID, err := digest.Parse(diffIDStr)
237 252
 	if err != nil {
238
-		return ocispec.Descriptor{}, "", err
253
+		return nil, "", err
239 254
 	}
240 255
 
241
-	return ocispec.Descriptor{
256
+	return &ocispec.Descriptor{
242 257
 		MediaType: ocispec.MediaTypeImageLayerGzip,
243 258
 		Digest:    newDesc.Digest,
244 259
 		Size:      info.Size,
... ...
@@ -254,7 +279,7 @@ func applyDiffLayer(ctx context.Context, name string, baseImg ocispec.Image, sn
254 254
 
255 255
 	mount, err := sn.Prepare(ctx, key, parent)
256 256
 	if err != nil {
257
-		return err
257
+		return fmt.Errorf("failed to prepare snapshot: %w", err)
258 258
 	}
259 259
 
260 260
 	defer func() {
... ...
@@ -224,6 +224,25 @@ func ApplyUncompressedLayer(dest string, layer io.Reader, options *TarOptions) (
224 224
 	return applyLayerHandler(dest, layer, options, false)
225 225
 }
226 226
 
227
+// IsEmpty checks if the tar archive is empty (doesn't contain any entries).
228
+func IsEmpty(rd io.Reader) (bool, error) {
229
+	decompRd, err := DecompressStream(rd)
230
+	if err != nil {
231
+		return true, fmt.Errorf("failed to decompress archive: %v", err)
232
+	}
233
+	defer decompRd.Close()
234
+
235
+	tarReader := tar.NewReader(decompRd)
236
+	if _, err := tarReader.Next(); err != nil {
237
+		if err == io.EOF {
238
+			return true, nil
239
+		}
240
+		return false, fmt.Errorf("failed to read next archive header: %v", err)
241
+	}
242
+
243
+	return false, nil
244
+}
245
+
227 246
 // do the bulk load of ApplyLayer, but allow for not calling DecompressStream
228 247
 func applyLayerHandler(dest string, layer io.Reader, options *TarOptions, decompress bool) (int64, error) {
229 248
 	dest = filepath.Clean(dest)