Merge pull request #378 from Luap99/staged-layer-creation

Staged layer creation

Author: mtrmac

storage/drivers/driver.go

@@ -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.
@@ -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.
@@ -299,6 +298,19 @@ 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 be 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, if a parent is given the caller must check for existence
+	// beforehand while holding a lock.
+	StartStagingDiffToApply(parent string, options ApplyDiffOpts) (tempdir.CleanupTempDirFunc, *tempdir.StagedAddition, int64, error)
+	// CommitStagedLayer commits the staged layer from StartStagingDiffToApply(). This must be done while holding 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.

storage/drivers/fsdiff.go

@@ -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 {

storage/drivers/graphtest/graphbench_unix.go

@@ -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)
 		}

storage/drivers/graphtest/graphtest_unix.go

@@ -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)
 	}

storage/drivers/overlay/overlay.go

@@ -995,38 +995,22 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) (retErr
 	return d.create(id, parent, opts, true)
 }
 
-func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, readOnly bool) (retErr error) {
-	dir, homedir, _ := d.dir2(id, readOnly)
-
-	disableQuota := readOnly
-
-	var uidMaps []idtools.IDMap
-	var gidMaps []idtools.IDMap
-
-	if opts != nil && opts.IDMappings != nil {
-		uidMaps = opts.IDMappings.UIDs()
-		gidMaps = opts.IDMappings.GIDs()
-	}
-
-	// Make the link directory if it does not exist
-	if err := idtools.MkdirAllAs(path.Join(homedir, linkDir), 0o755, 0, 0); err != nil {
-		return err
-	}
-
+// 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 err
+		return idtools.IDPair{}, idtools.Stat{}, idtools.Stat{}, 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 != "" {
@@ -1037,14 +1021,50 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, readOnl
 		} else {
 			systemSt, err := system.Stat(parentDiff)
 			if err != nil {
-				return err
+				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)
+
+	disableQuota := readOnly
+
+	var uidMaps []idtools.IDMap
+	var gidMaps []idtools.IDMap
+
+	if opts != nil && opts.IDMappings != nil {
+		uidMaps = opts.IDMappings.UIDs()
+		gidMaps = opts.IDMappings.GIDs()
+	}
+
+	// Make the link directory if it does not exist
+	if err := idtools.MkdirAllAs(path.Join(homedir, linkDir), 0o755, 0, 0); err != nil {
+		return err
+	}
+
+	idPair, forcedSt, st, err := d.getLayerPermissions(parent, uidMaps, gidMaps)
+	if err != nil {
+		return err
+	}
+
+	if err := idtools.MkdirAllAndChownNew(path.Dir(dir), 0o755, idPair); err != nil {
+		return err
+	}
+
 	if err := fileutils.Lexists(dir); err == nil {
 		logrus.Warnf("Trying to create a layer %#v while directory %q already exists; removing it first", id, dir)
 		// Don’t just os.RemoveAll(dir) here; d.Remove also removes the link in linkDir,
@@ -1088,12 +1108,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
@@ -1356,6 +1370,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)
@@ -2369,31 +2391,94 @@ 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
+// StartStagingDiffToApply applies the new layer into a temporary directory.
+// It returns a CleanupTempDirFunc which can be 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, if a parent is given the caller must check for existence
+// beforehand while holding a 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())
+	if err != nil {
+		// If we have a ENOENT it means the parent was removed which can happen as we are unlocked here.
+		// In this case also wrap ErrLayerUnknown which some callers can handle to retry after recreating the parent.
+		if errors.Is(err, fs.ErrNotExist) {
+			err = fmt.Errorf("parent layer %q: %w: %w", parent, graphdriver.ErrLayerUnknown, err)
 		}
-		if d.options.forceMask != nil {
-			options.ForceMask = d.options.forceMask
+		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
+	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,
@@ -2404,7 +2489,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 {

storage/drivers/overlay/overlay_test.go

@@ -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.

storage/drivers/template.go

@@ -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