Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions storage/drivers/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ type CreateOpts struct {
MountLabel string
StorageOpt map[string]string
*idtools.IDMappings
ignoreChownErrors bool
}

// MountOpts contains optional arguments for Driver.Get() methods.
Expand Down Expand Up @@ -184,7 +183,7 @@ type DiffDriver interface {
// layer with the specified id and parent, returning the size of the
// new layer in bytes.
// The io.Reader must be an uncompressed stream.
ApplyDiff(id string, parent string, options ApplyDiffOpts) (size int64, err error)
ApplyDiff(id string, options ApplyDiffOpts) (size int64, err error)
// DiffSize calculates the changes between the specified id
// and its parent and returns the size in bytes of the changes
// relative to its base filesystem directory.
Expand Down Expand Up @@ -299,6 +298,18 @@ type DriverWithDiffer interface {
DifferTarget(id string) (string, error)
}

// ApplyDiffStaging is an interface for driver who can apply the diff without holding the main storage lock.
// This API is experimental and can be changed without bumping the major version number.
type ApplyDiffStaging interface {
// StartStagingDiffToApply applies the new layer into a temporary directory.
// It returns a CleanupTempDirFunc which can nil or set regardless if the function return an error or not.
// StagedAddition is only set when there is no error returned and the int64 value returns the size of the layer.
// This can be done without holding the storage lock.
StartStagingDiffToApply(parent string, options ApplyDiffOpts) (tempdir.CleanupTempDirFunc, *tempdir.StagedAddition, int64, error)
// CommitStagedLayer commits the staged layer from StartStagingDiffToApply(). This must be done while the storage lock.
CommitStagedLayer(id string, commit *tempdir.StagedAddition) error
}

// Capabilities defines a list of capabilities a driver may implement.
// These capabilities are not required; however, they do determine how a
// graphdriver can be used.
Expand Down
2 changes: 1 addition & 1 deletion storage/drivers/fsdiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (gdw *NaiveDiffDriver) Changes(id string, idMappings *idtools.IDMappings, p
// ApplyDiff extracts the changeset from the given diff into the
// layer with the specified id and parent, returning the size of the
// new layer in bytes.
func (gdw *NaiveDiffDriver) ApplyDiff(id, parent string, options ApplyDiffOpts) (int64, error) {
func (gdw *NaiveDiffDriver) ApplyDiff(id string, options ApplyDiffOpts) (int64, error) {
driver := gdw.ProtoDriver

if options.Mappings == nil {
Expand Down
2 changes: 1 addition & 1 deletion storage/drivers/graphtest/graphbench_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func DriverBenchDiffApplyN(b *testing.B, fileCount int, drivername string, drive
b.Fatal(err)
}

applyDiffSize, err := driver.ApplyDiff(diff, "", graphdriver.ApplyDiffOpts{})
applyDiffSize, err := driver.ApplyDiff(diff, graphdriver.ApplyDiffOpts{})
if err != nil {
b.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion storage/drivers/graphtest/graphtest_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ func DriverTestDiffApply(t testing.TB, fileCount int, drivername string, driverO
t.Fatal(err)
}

applyDiffSize, err := driver.ApplyDiff(diff, base, graphdriver.ApplyDiffOpts{Diff: bytes.NewReader(buf.Bytes())})
applyDiffSize, err := driver.ApplyDiff(diff, graphdriver.ApplyDiffOpts{Diff: bytes.NewReader(buf.Bytes())})
if err != nil {
t.Fatal(err)
}
Expand Down
153 changes: 124 additions & 29 deletions storage/drivers/overlay/overlay.go
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,49 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) (retErr
return d.create(id, parent, opts, true)
}

// getLayerPermissions returns the base permissions to use for the layer directories.
// The first return value is the idPair to create the possible parent directories with.
// The second return value is the mode how it should be stored on disk.
// The third return value is the mode the layer expects to have which may be stored
// in an xattr when using forceMask, without forceMask both values are the same.
func (d *Driver) getLayerPermissions(parent string, uidMaps, gidMaps []idtools.IDMap) (idtools.IDPair, idtools.Stat, idtools.Stat, error) {
rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps)
if err != nil {
return idtools.IDPair{}, idtools.Stat{}, idtools.Stat{}, err
}

idPair := idtools.IDPair{
UID: rootUID,
GID: rootGID,
}

st := idtools.Stat{IDs: idPair, Mode: defaultPerms}

if parent != "" {
parentBase := d.dir(parent)
parentDiff := filepath.Join(parentBase, "diff")
if xSt, err := idtools.GetContainersOverrideXattr(parentDiff); err == nil {
st = xSt
} else {
systemSt, err := system.Stat(parentDiff)
if err != nil {
return idtools.IDPair{}, idtools.Stat{}, idtools.Stat{}, err
}
st.IDs.UID = int(systemSt.UID())
st.IDs.GID = int(systemSt.GID())
st.Mode = os.FileMode(systemSt.Mode())
}
}

forcedSt := st
if d.options.forceMask != nil {
forcedSt.IDs = idPair
forcedSt.Mode = *d.options.forceMask
}

return idPair, forcedSt, st, nil
}

func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, readOnly bool) (retErr error) {
dir, homedir, _ := d.dir2(id, readOnly)

Expand All @@ -1013,22 +1056,15 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, readOnl
return err
}

rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps)
idPair, forcedSt, st, err := d.getLayerPermissions(parent, uidMaps, gidMaps)
if err != nil {
return err
}

idPair := idtools.IDPair{
UID: rootUID,
GID: rootGID,
}

if err := idtools.MkdirAllAndChownNew(path.Dir(dir), 0o755, idPair); err != nil {
return err
}

st := idtools.Stat{IDs: idPair, Mode: defaultPerms}

if parent != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part was copied into getLayerPermissions, so I think it can be deleted here.

parentBase := d.dir(parent)
parentDiff := filepath.Join(parentBase, "diff")
Expand Down Expand Up @@ -1088,12 +1124,6 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, readOnl
}
}

forcedSt := st
if d.options.forceMask != nil {
forcedSt.IDs = idPair
forcedSt.Mode = *d.options.forceMask
}

diff := path.Join(dir, "diff")
if err := idtools.MkdirAs(diff, forcedSt.Mode, forcedSt.IDs.UID, forcedSt.IDs.GID); err != nil {
return err
Expand Down Expand Up @@ -1356,6 +1386,14 @@ func (d *Driver) getTempDirRoot(id string) string {
return filepath.Join(d.home, tempDirName)
}

// getTempDirRootForNewLayer returns the correct temp directory root based on where
// the layer should be created.
//
// This must be kept in sync with GetTempDirRootDirs().
func (d *Driver) getTempDirRootForNewLayer() string {
return filepath.Join(d.homeDirForImageStore(), tempDirName)
}

func (d *Driver) DeferredRemove(id string) (tempdir.CleanupTempDirFunc, error) {
tempDirRoot := d.getTempDirRoot(id)
t, err := tempdir.NewTempDir(tempDirRoot)
Expand Down Expand Up @@ -2369,31 +2407,88 @@ func (d *Driver) DifferTarget(id string) (string, error) {
return d.getDiffPath(id)
}

// ApplyDiff applies the new layer into a root
func (d *Driver) ApplyDiff(id, parent string, options graphdriver.ApplyDiffOpts) (size int64, err error) {
if !d.isParent(id, parent) {
if d.options.ignoreChownErrors {
options.IgnoreChownErrors = d.options.ignoreChownErrors
}
if d.options.forceMask != nil {
options.ForceMask = d.options.forceMask
// StartStagingDiffToApply applies the new layer into a temporary directory.
// It returns a CleanupTempDirFunc which can nil or set regardless if the function return an error or not.
// StagedAddition is only set when there is no error returned and the int64 value returns the size of the layer.
// This can be done without holding the storage lock.
//
// This API is experimental and can be changed without bumping the major version number.
func (d *Driver) StartStagingDiffToApply(parent string, options graphdriver.ApplyDiffOpts) (tempdir.CleanupTempDirFunc, *tempdir.StagedAddition, int64, error) {
tempDirRoot := d.getTempDirRootForNewLayer()
t, err := tempdir.NewTempDir(tempDirRoot)
if err != nil {
return nil, nil, -1, err
}

sa, err := t.StageAddition()
if err != nil {
return t.Cleanup, nil, -1, err
}

_, forcedSt, st, err := d.getLayerPermissions(parent, options.Mappings.UIDs(), options.Mappings.GIDs())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This calls GetContainersOverrideXattr and stat on d.dir(parent), without holding locks.

I think that’s fine if we know that the parent layer existed = was previously fully created — otherwise we might race against Driver.create setting the fields, and see incorrect interim values. And we do know that, due to populateLayerOptions — so that’s all working fine, but it’s a very remote dependency, so worth carefully documenting in the StartStagingDiffToApply function and in callers across the call stack.

(If the layer existed and is later removed, we can fail with an error, but we won’t see invalid data. Failing with an error is semantically correct, creating a layer against a missing parent should fail.)

if err != nil {
return t.Cleanup, nil, -1, err
}

if err := idtools.MkdirAs(sa.Path, forcedSt.Mode, forcedSt.IDs.UID, forcedSt.IDs.GID); err != nil {
return t.Cleanup, nil, -1, err
}

if d.options.forceMask != nil {
st.Mode |= os.ModeDir
if err := idtools.SetContainersOverrideXattr(sa.Path, st); err != nil {
return t.Cleanup, nil, -1, err
}
return d.naiveDiff.ApplyDiff(id, parent, options)
}

idMappings := options.Mappings
if idMappings == nil {
idMappings = &idtools.IDMappings{}
size, err := d.applyDiff(sa.Path, options)
if err != nil {
return t.Cleanup, nil, -1, err
}

return t.Cleanup, sa, size, nil
}

// CommitStagedLayer that was created with StartStagingDiffToApply().
//
// This API is experimental and can be changed without bumping the major version number.
func (d *Driver) CommitStagedLayer(id string, sa *tempdir.StagedAddition) error {
applyDir, err := d.getDiffPath(id)
if err != nil {
return err
}

// The os.Rename() function used by CommitFunc errors when the target directory already
// exists, as such delete the dir. The create() function creates it and it would be more
// complicated to code in a way that it didn't create it.
if err := os.Remove(applyDir); err != nil {
return err
}

return sa.Commit(applyDir)
}

// ApplyDiff applies the new layer into a root
func (d *Driver) ApplyDiff(id string, options graphdriver.ApplyDiffOpts) (size int64, err error) {
applyDir, err := d.getDiffPath(id)
if err != nil {
return 0, err
}
return d.applyDiff(applyDir, options)
}

// ApplyDiff applies the new layer into a root.
// This can run concurrently with any other driver operations, as such it is the
// callers responsibility to ensure the target path passed is safe to use if that is the case.
func (d *Driver) applyDiff(target string, options graphdriver.ApplyDiffOpts) (size int64, err error) {
idMappings := options.Mappings
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Non-blocking: So far, all callers were setting options.Mappings… I’d prefer to be clear and consistent about whether that field is mandatory or optional; in that sense, adapting for nil here, nested inside the driver’s call stack, is a bit unexpected.)

if idMappings == nil {
idMappings = &idtools.IDMappings{}
}

logrus.Debugf("Applying tar in %s", applyDir)
logrus.Debugf("Applying tar in %s", target)
// Overlay doesn't need the parent id to apply the diff
if err := untar(options.Diff, applyDir, &archive.TarOptions{
if err := untar(options.Diff, target, &archive.TarOptions{
UIDMaps: idMappings.UIDs(),
GIDMaps: idMappings.GIDs(),
IgnoreChownErrors: d.options.ignoreChownErrors,
Expand All @@ -2404,7 +2499,7 @@ func (d *Driver) ApplyDiff(id, parent string, options graphdriver.ApplyDiffOpts)
return 0, err
}

return directory.Size(applyDir)
return directory.Size(target)
}

func (d *Driver) getComposefsData(id string) string {
Expand Down
3 changes: 3 additions & 0 deletions storage/drivers/overlay/overlay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import (

const driverName = "overlay"

// check that Driver correctly implements the ApplyDiffTemporary interface
var _ graphdriver.ApplyDiffStaging = &Driver{}

func init() {
// Do not sure chroot to speed run time and allow archive
// errors or hangs to be debugged directly from the test process.
Expand Down
52 changes: 0 additions & 52 deletions storage/drivers/template.go

This file was deleted.

4 changes: 2 additions & 2 deletions storage/drivers/vfs/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,11 @@ func (d *Driver) CreateFromTemplate(id, template string, templateIDMappings *idt
}

// ApplyDiff applies the new layer into a root
func (d *Driver) ApplyDiff(id, parent string, options graphdriver.ApplyDiffOpts) (size int64, err error) {
func (d *Driver) ApplyDiff(id string, options graphdriver.ApplyDiffOpts) (size int64, err error) {
if d.ignoreChownErrors {
options.IgnoreChownErrors = d.ignoreChownErrors
}
return d.naiveDiff.ApplyDiff(id, parent, options)
return d.naiveDiff.ApplyDiff(id, options)
}

// CreateReadWrite creates a layer that is writable for use as a container
Expand Down
Loading
Loading