Merge pull request #1197 from nalind/mapped-layer-sizes

Fully populate layer records for mapped image top layers

drivers/overlay/overlay.go

@@ -39,7 +39,6 @@ import (
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-	"github.com/vbatts/tar-split/tar/storage"
 	"golang.org/x/sys/unix"
 )
 
@@ -1743,11 +1742,24 @@ func (d *Driver) getWhiteoutFormat() archive.WhiteoutFormat {
 	return whiteoutFormat
 }
 
-type fileGetNilCloser struct {
-	storage.FileGetter
+type overlayFileGetter struct {
+	diffDirs []string
 }
 
-func (f fileGetNilCloser) Close() error {
+func (g *overlayFileGetter) Get(path string) (io.ReadCloser, error) {
+	for _, d := range g.diffDirs {
+		f, err := os.Open(filepath.Join(d, path))
+		if err == nil {
+			return f, nil
+		}
+	}
+	if len(g.diffDirs) > 0 {
+		return os.Open(filepath.Join(g.diffDirs[0], path))
+	}
+	return nil, fmt.Errorf("%s: %w", path, os.ErrNotExist)
+}
+
+func (g *overlayFileGetter) Close() error {
 	return nil
 }
 
@@ -1756,13 +1768,18 @@ func (d *Driver) getStagingDir() string {
 }
 
 // DiffGetter returns a FileGetCloser that can read files from the directory that
-// contains files for the layer differences. Used for direct access for tar-split.
+// contains files for the layer differences, either for this layer, or one of our
+// lowers if we're just a template directory. Used for direct access for tar-split.
 func (d *Driver) DiffGetter(id string) (graphdriver.FileGetCloser, error) {
 	p, err := d.getDiffPath(id)
 	if err != nil {
 		return nil, err
 	}
-	return fileGetNilCloser{storage.NewPathFileGetter(p)}, nil
+	paths, err := d.getLowerDiffPaths(id)
+	if err != nil {
+		return nil, err
+	}
+	return &overlayFileGetter{diffDirs: append([]string{p}, paths...)}, nil
 }
 
 // CleanupStagingDirectory cleanups the staging directory.

layers.go

@@ -725,12 +725,32 @@ func (r *layerStore) Put(id string, parentLayer *Layer, names []string, mountLab
 		parent = parentLayer.ID
 	}
 	var parentMappings, templateIDMappings, oldMappings *idtools.IDMappings
+	var (
+		templateMetadata           string
+		templateCompressedDigest   digest.Digest
+		templateCompressedSize     int64
+		templateUncompressedDigest digest.Digest
+		templateUncompressedSize   int64
+		templateCompressionType    archive.Compression
+		templateUIDs, templateGIDs []uint32
+		templateTSdata             []byte
+	)
 	if moreOptions.TemplateLayer != "" {
+		var tserr error
 		templateLayer, ok := r.lookup(moreOptions.TemplateLayer)
 		if !ok {
 			return nil, -1, ErrLayerUnknown
 		}
+		templateMetadata = templateLayer.Metadata
 		templateIDMappings = idtools.NewIDMappingsFromMaps(templateLayer.UIDMap, templateLayer.GIDMap)
+		templateCompressedDigest, templateCompressedSize = templateLayer.CompressedDigest, templateLayer.CompressedSize
+		templateUncompressedDigest, templateUncompressedSize = templateLayer.UncompressedDigest, templateLayer.UncompressedSize
+		templateCompressionType = templateLayer.CompressionType
+		templateUIDs, templateGIDs = append([]uint32{}, templateLayer.UIDs...), append([]uint32{}, templateLayer.GIDs...)
+		templateTSdata, tserr = ioutil.ReadFile(r.tspath(templateLayer.ID))
+		if tserr != nil && !os.IsNotExist(tserr) {
+			return nil, -1, tserr
+		}
 	} else {
 		templateIDMappings = &idtools.IDMappings{}
 	}
@@ -775,17 +795,43 @@ func (r *layerStore) Put(id string, parentLayer *Layer, names []string, mountLab
 			return nil, -1, err
 		}
 	}
+	if len(templateTSdata) > 0 {
+		if err := os.MkdirAll(filepath.Dir(r.tspath(id)), 0o700); err != nil {
+			// We don't have a record of this layer, but at least
+			// try to clean it up underneath us.
+			if err2 := r.driver.Remove(id); err2 != nil {
+				logrus.Errorf("While recovering from a failure creating in UpdateLayerIDMap, error deleting layer %#v: %v", id, err2)
+			}
+			return nil, -1, err
+		}
+		if err = ioutils.AtomicWriteFile(r.tspath(id), templateTSdata, 0o600); err != nil {
+			// We don't have a record of this layer, but at least
+			// try to clean it up underneath us.
+			if err2 := r.driver.Remove(id); err2 != nil {
+				logrus.Errorf("While recovering from a failure creating in UpdateLayerIDMap, error deleting layer %#v: %v", id, err2)
+			}
+			return nil, -1, err
+		}
+	}
 	if err == nil {
 		layer = &Layer{
-			ID:           id,
-			Parent:       parent,
-			Names:        names,
-			MountLabel:   mountLabel,
-			Created:      time.Now().UTC(),
-			Flags:        make(map[string]interface{}),
-			UIDMap:       copyIDMap(moreOptions.UIDMap),
-			GIDMap:       copyIDMap(moreOptions.GIDMap),
-			BigDataNames: []string{},
+			ID:                 id,
+			Parent:             parent,
+			Names:              names,
+			MountLabel:         mountLabel,
+			Metadata:           templateMetadata,
+			Created:            time.Now().UTC(),
+			CompressedDigest:   templateCompressedDigest,
+			CompressedSize:     templateCompressedSize,
+			UncompressedDigest: templateUncompressedDigest,
+			UncompressedSize:   templateUncompressedSize,
+			CompressionType:    templateCompressionType,
+			UIDs:               templateUIDs,
+			GIDs:               templateGIDs,
+			Flags:              make(map[string]interface{}),
+			UIDMap:             copyIDMap(moreOptions.UIDMap),
+			GIDMap:             copyIDMap(moreOptions.GIDMap),
+			BigDataNames:       []string{},
 		}
 		r.layers = append(r.layers, layer)
 		r.idindex.Add(id)
@@ -872,7 +918,6 @@ func (r *layerStore) Mounted(id string) (int, error) {
 }
 
 func (r *layerStore) Mount(id string, options drivers.MountOpts) (string, error) {
-
 	// check whether options include ro option
 	hasReadOnlyOpt := func(opts []string) bool {
 		for _, item := range opts {

tests/idmaps.bats

@@ -560,10 +560,12 @@ load helpers
 	esac
 	n=5
 	host=2
+	filelist=
 	# Create some temporary files.
 	for i in $(seq $n) ; do
 		createrandom "$TESTDIR"/file$i
 		chown ${i}:${i} "$TESTDIR"/file$i
+		filelist="$filelist file$i"
 	done
 	# Select some ID ranges.
 	for i in $(seq $n) ; do
@@ -576,24 +578,18 @@ load helpers
 	[ "$status" -eq 0 ]
 	[ "$output" != "" ]
 	baselayer="$output"
+	# Create an empty layer blob and apply it to the layer.
+	dd if=/dev/zero bs=1k count=1 of="$TESTDIR"/layer.empty
+	run storage --debug=false applydiff -f "$TESTDIR"/layer.empty $baselayer
 	# Create a layer using the host's mappings.
 	run storage --debug=false create-layer --hostuidmap --hostgidmap $baselayer
 	echo "$output"
 	[ "$status" -eq 0 ]
 	[ "$output" != "" ]
 	lowerlayer="$output"
-	# Mount the layer.
-	run storage --debug=false mount $lowerlayer
-	echo "$output"
-	[ "$status" -eq 0 ]
-	[ "$output" != "" ]
-	lowermount="$output"
-	# Copy the files in (host mapping, so it's fine), and set ownerships on them.
-	cp -p "$TESTDIR"/file1 ${lowermount}
-	cp -p "$TESTDIR"/file2 ${lowermount}
-	cp -p "$TESTDIR"/file3 ${lowermount}
-	cp -p "$TESTDIR"/file4 ${lowermount}
-	cp -p "$TESTDIR"/file5 ${lowermount}
+	# Create a layer blob containing the files and apply it to the layer.
+	tar --directory "$TESTDIR" -cvf "$TESTDIR"/layer.tar $filelist
+	run storage --debug=false applydiff -f "$TESTDIR"/layer.tar $lowerlayer
 	# Create an image record for this layer.
 	run storage --debug=false create-image $lowerlayer
 	echo "$output"
@@ -679,15 +675,30 @@ load helpers
 	echo nparents:$nparents
 	[ $nparents -eq $n ]
 
-	# The image should have five top layers at this point.
+	# The image should have five top layers at this point, they should all
+	# have known sizes, and we should be able to diff them all.
 	run storage --debug=false image $image
 	echo "$output"
 	[ "$status" -eq 0 ]
 	[ "$output" != "" ]
 	tops=$(grep '^Top Layer:' <<< "$output" | sed 's,^Top Layer: ,,g')
+	echo tops: "$tops"
 	ntops=$(for p in $tops; do echo $p ; done | sort -u | wc -l)
 	echo ntops:$ntops
 	[ $ntops -eq $n ]
+	for p in $tops; do
+		rm -f "$TESTDIR"/diff.tar
+		storage --debug=false diff -u -f "$TESTDIR"/diff.tar "$p"
+		test -s "$TESTDIR"/diff.tar
+		expected=$(storage --debug=false layer --json $p | sed -r -e 's|.*"diff-size":([^",]*).*|\1|g')
+		actual=$(stat -c %s "$TESTDIR"/diff.tar)
+		echo expected diff size "$expected", got "$actual"
+		test $actual = $expected
+		expected=$(storage --debug=false layer --json $p | sed -r -e 's|.*"diff-digest":"?([^",]*).*|\1|g')
+		actual=sha256:$(sha256sum "$TESTDIR"/diff.tar | sed -e 's, .*,,g')
+		echo expected diff digest "$expected", got "$actual"
+		test $actual = $expected
+	done
 
 	# Remove the containers and image and check that all of the layers we used got removed.
 	for container in "${containers[@]}" ; do
@@ -702,10 +713,10 @@ load helpers
 
 @test "idmaps-create-mapped-container" {
 	case "$STORAGE_DRIVER" in
-	btrfs|devicemapper|overlay*|vfs|zfs)
+	overlay*|vfs)
 		;;
 	*)
-		skip "not supported by driver $STORAGE_DRIVER"
+		skip "${STORAGE_DRIVER}.imagestore option not supported by driver ${STORAGE_DRIVER}"
 		;;
 	esac
 	case "$STORAGE_OPTION" in