WIP: Staged layer creation

storage/drivers/driver.go

@@ -299,6 +299,15 @@ 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 layer in a temporary directory. This can be done without holding the storage lock.
+	StartStagingDiffToApply(options ApplyDiffOpts) (tempdir.CleanupTempDirFunc, *tempdir.StageAddition, int64, error)
+	// CommitStagedLayer commits the staged layer from StartStagingDiffToApply(). This must be done while the storage lock.
+	CommitStagedLayer(id string, commit *tempdir.StageAddition) 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/overlay/overlay.go

@@ -2369,31 +2369,91 @@ 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 nil or set regardless if the function return an error or not.
+// CommitFunc is only set when there is no error returned and the int64 value returns the size of the layer.
+//
+// This API is experimental and can be changed without bumping the major version number.
+func (d *Driver) StartStagingDiffToApply(options graphdriver.ApplyDiffOpts) (tempdir.CleanupTempDirFunc, *tempdir.StageAddition, int64, error) {
+	// FIXME: how to consolidate with d.getTempDirRoot(id) if we don't have the id?
+	tempDirRoot := filepath.Join(d.homeDirForImageStore(), tempDirName)
+	t, err := tempdir.NewTempDir(tempDirRoot)
+	if err != nil {
+		return nil, nil, -1, err
+	}
+
+	sa, err := t.StageAddition()
+	if err != nil {
+		return nil, nil, -1, err
+	}
+
+	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.StageAddition) error {
+	applyDir, err := d.getDiffPath(id)
+	if err != nil {
+		return err
+	}
+
+	// FIXME: Is there a better way to do this?
+	stat, err := system.Stat(applyDir)
+	if err != nil {
+		return err
+	}
+	if err := os.Chmod(sa.Path, os.FileMode(stat.Mode())); err != nil {
+		return err
+	}
+
+	if err := os.Chown(sa.Path, int(stat.UID()), int(stat.GID())); err != nil {
+		return err
+	}
+	if d.options.forceMask != nil {
+		st, err := idtools.GetContainersOverrideXattr(applyDir)
+		if err != nil {
+			return err
 		}
-		if d.options.forceMask != nil {
-			options.ForceMask = d.options.forceMask
+		if err := idtools.SetContainersOverrideXattr(sa.Path, st); err != nil {
+			return err
 		}
-		return d.naiveDiff.ApplyDiff(id, parent, options)
 	}
 
-	idMappings := options.Mappings
-	if idMappings == nil {
-		idMappings = &idtools.IDMappings{}
+	// The os.Rename() function used by CommitFunc errors when the target directory already
+	// exists, as such delete the dir.
+	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, parent 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
+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 +2464,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/internal/tempdir/tempdir.go

@@ -91,6 +91,30 @@ type TempDir struct {
 	counter uint64
 }
 
+// StageAddition is a temporary object which holds the information of where to
+// put the data into and then use Commit() to move the data into the final location.
+type StageAddition struct {
+	// Path is the temporary path. The path is not created so caller must create
+	// a file or directory on it in order to use Commit(). The path is only valid
+	// until Commit() is called or until the TempDir instance Cleanup() method is used.
+	Path string
+}
+
+// CommitFunc is a function type that can be returned by operations
+// which need to perform the commit operation later.
+type CommitFunc func(destination string) error
+
+// Commit the staged content into its final destination by using os.Rename().
+// That means the dest must be on the same on the same fs as the root directory
+// that was given to NewTempDir() and the dest must not exist yet.
+// Commit must only be called once per instance returned from the
+// StageAddition() call.
+func (s *StageAddition) Commit(destination string) error {
+	err := os.Rename(s.Path, destination)
+	s.Path = "" // invalidate Path to avoid reuse
+	return err
+}
+
 // CleanupTempDirFunc is a function type that can be returned by operations
 // which need to perform cleanup actions later.
 type CleanupTempDirFunc func() error
@@ -190,6 +214,23 @@ func NewTempDir(rootDir string) (*TempDir, error) {
 	return td, nil
 }
 
+// StageAddition creates a new temporary path that is returned as field in the StageAddition
+// struct. The returned type has a type a the Commit() function to move the content from
+// the temporary location to the final one.
+//
+// The caller MUST ensure .Cleanup() is called after Commit() otherwise the staged content
+// will be deleted and the move will fail.
+// If the TempDir has been cleaned up already, this method will return an error.
+func (td *TempDir) StageAddition() (*StageAddition, error) {
+	if td.tempDirLock == nil {
+		return nil, fmt.Errorf("temp dir instance not initialized or already cleaned up")
+	}
+	fileName := fmt.Sprintf("%d-", td.counter) + "addition"
+	tmpAddPath := filepath.Join(td.tempDirPath, fileName)
+	td.counter++
+	return &StageAddition{Path: tmpAddPath}, nil
+}
+
 // StageDeletion moves the specified file into the instance's temporary directory.
 // The temporary directory must already exist (created during NewTempDir).
 // Files are renamed with a counter-based prefix (e.g., "0-filename", "1-filename") to ensure uniqueness.

storage/internal/tempdir/tempdir_test.go

@@ -271,3 +271,63 @@ func TestTempDirFileNaming(t *testing.T) {
 		assert.True(t, found, "Expected file %s not found", expectedName)
 	}
 }
+
+func TestStageAddition(t *testing.T) {
+	rootDir := t.TempDir()
+	td, err := NewTempDir(rootDir)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		assert.NoError(t, td.Cleanup())
+	})
+
+	sa1, err := td.StageAddition()
+	require.NoError(t, err)
+	// Path should not be created by StageAddition.
+	assert.NoFileExists(t, sa1.Path)
+
+	// ensure we can create file
+	f, err := os.Create(sa1.Path)
+	require.NoError(t, err)
+	require.NoError(t, f.Close())
+
+	// need to use a dest file which does not exist yet
+	dest := filepath.Join(t.TempDir(), "file1")
+
+	err = sa1.Commit(dest)
+	require.NoError(t, err)
+	assert.FileExists(t, dest)
+	assert.NoFileExists(t, sa1.Path)
+
+	// now test the same with a directory
+	sa2, err := td.StageAddition()
+	require.NoError(t, err)
+	// Path should not be created by StageAddition.
+	assert.NoDirExists(t, sa2.Path)
+
+	// ensure we can create a directory
+	err = os.Mkdir(sa2.Path, 0o755)
+	require.NoError(t, err)
+
+	// need to use a dest which does not exist yet
+	dest = filepath.Join(t.TempDir(), "dir")
+
+	err = sa2.Commit(dest)
+	require.NoError(t, err)
+	assert.DirExists(t, dest)
+	assert.NoDirExists(t, sa2.Path)
+
+	// Commit the same stage addition struct again should error
+	err = sa2.Commit(dest)
+	require.Error(t, err)
+
+	// Cleanup() should cleanup the temp paths from StageAddition
+	sa3, err := td.StageAddition()
+	require.NoError(t, err)
+
+	err = os.Mkdir(sa3.Path, 0o755)
+	require.NoError(t, err)
+
+	err = td.Cleanup()
+	require.NoError(t, err)
+	assert.NoDirExists(t, sa3.Path)
+}