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.

Signed-off-by: Nalin Dahyabhai <[email protected]>
  • Loading branch information
@nalind
nalind committed Apr 12, 2022
1 parent 4203c21 commit cf8644e
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 23 deletions.
65 changes: 55 additions & 10 deletions layers.go
View file
Original file line number Diff line number Diff line change
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
32 changes: 19 additions & 13 deletions tests/idmaps.bats
View file
Original file line number Diff line number Diff line change
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,25 @@ 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)
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 Down

0 comments on commit cf8644e

Please sign in to comment.