Skip to content

Commit

Permalink
Fully populate layer records for mapped image top layers
Browse files Browse the repository at this point in the history
When we create a copy of an image's top layer that's intended to be
identical to the top layer, except for having some set of ID mappings
already applied to it, copy over the template layer's compressed and
uncompressed digest and size information, compression information,
tar-split data, and lists of used UIDs and GIDs, if we have them.

The lack of sizing information was forcing ImageSize() to regenerate the
diffs to determine the size of the mapped layers, which shouldn't have
been necessary.

Teach the overlay DiffGetter to look for files in the diff directories
of lower layers if we can't find them in the current layer, so that
tar-split can retrieve content that we didn't have to pull up.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
  • Loading branch information
nalind committed Apr 12, 2022
1 parent 4203c21 commit d76b360
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 31 deletions.
29 changes: 23 additions & 6 deletions drivers/overlay/overlay.go
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}

Expand All @@ -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.
Expand Down
65 changes: 55 additions & 10 deletions layers.go
Expand Up @@ -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{}
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
41 changes: 26 additions & 15 deletions tests/idmaps.bats
Expand Up @@ -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
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit d76b360

Please sign in to comment.