Merge pull request #28138 from dmcgowan/handle-overlay2-copy-up-bug

Use naive diff for overlay2 when opaque copy up bug present

daemon/graphdriver/overlay2/check.go

@@ -0,0 +1,79 @@
+// +build linux
+
+package overlay2
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path"
+	"path/filepath"
+	"syscall"
+
+	"github.com/Sirupsen/logrus"
+	"github.com/docker/docker/pkg/system"
+	"github.com/pkg/errors"
+)
+
+// hasOpaqueCopyUpBug checks whether the filesystem has a bug
+// which copies up the opaque flag when copying up an opaque
+// directory. When this bug exists naive diff should be used.
+func hasOpaqueCopyUpBug(d string) error {
+	td, err := ioutil.TempDir(d, "opaque-bug-check")
+	if err != nil {
+		return err
+	}
+	defer func() {
+		if err := os.RemoveAll(td); err != nil {
+			logrus.Warnf("Failed to remove check directory %v: %v", td, err)
+		}
+	}()
+
+	// Make directories l1/d, l2/d, l3, work, merged
+	if err := os.MkdirAll(filepath.Join(td, "l1", "d"), 0755); err != nil {
+		return err
+	}
+	if err := os.MkdirAll(filepath.Join(td, "l2", "d"), 0755); err != nil {
+		return err
+	}
+	if err := os.Mkdir(filepath.Join(td, "l3"), 0755); err != nil {
+		return err
+	}
+	if err := os.Mkdir(filepath.Join(td, "work"), 0755); err != nil {
+		return err
+	}
+	if err := os.Mkdir(filepath.Join(td, "merged"), 0755); err != nil {
+		return err
+	}
+
+	// Mark l2/d as opaque
+	if err := system.Lsetxattr(filepath.Join(td, "l2", "d"), "trusted.overlay.opaque", []byte("y"), 0); err != nil {
+		return errors.Wrap(err, "failed to set opaque flag on middle layer")
+	}
+
+	opts := fmt.Sprintf("lowerdir=%s:%s,upperdir=%s,workdir=%s", path.Join(td, "l2"), path.Join(td, "l1"), path.Join(td, "l3"), path.Join(td, "work"))
+	if err := syscall.Mount("overlay", filepath.Join(td, "merged"), "overlay", 0, opts); err != nil {
+		return errors.Wrap(err, "failed to mount overlay")
+	}
+	defer func() {
+		if err := syscall.Unmount(filepath.Join(td, "merged"), 0); err != nil {
+			logrus.Warnf("Failed to unmount check directory %v: %v", filepath.Join(td, "merged"), err)
+		}
+	}()
+
+	// Touch file in d to force copy up of opaque directory "d" from "l2" to "l3"
+	if err := ioutil.WriteFile(filepath.Join(td, "merged", "d", "f"), []byte{}, 0644); err != nil {
+		return errors.Wrap(err, "failed to write to merged directory")
+	}
+
+	// Check l3/d does not have opaque flag
+	xattrOpaque, err := system.Lgetxattr(filepath.Join(td, "l3", "d"), "trusted.overlay.opaque")
+	if err != nil {
+		return errors.Wrap(err, "failed to read opaque flag on upper layer")
+	}
+	if string(xattrOpaque) == "y" {
+		return errors.New("opaque flag erroneously copied up, consider update to kernel 4.8 or later to fix")
+	}
+
+	return nil
+}

daemon/graphdriver/overlay2/overlay.go

@@ -14,6 +14,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
 	"syscall"
 
 	"github.com/Sirupsen/logrus"
@@ -102,6 +103,9 @@ type Driver struct {
 var (
 	backingFs             = "<unknown>"
 	projectQuotaSupported = false
+
+	useNaiveDiffLock sync.Once
+	useNaiveDiffOnly bool
 )
 
 func init() {
@@ -235,6 +239,16 @@ func supportsOverlay() error {
 	return graphdriver.ErrNotSupported
 }
 
+func useNaiveDiff(home string) bool {
+	useNaiveDiffLock.Do(func() {
+		if err := hasOpaqueCopyUpBug(home); err != nil {
+			logrus.Warnf("Not using native diff for overlay2: %v", err)
+			useNaiveDiffOnly = true
+		}
+	})
+	return useNaiveDiffOnly
+}
+
 func (d *Driver) String() string {
 	return driverName
 }
@@ -245,6 +259,7 @@ func (d *Driver) Status() [][2]string {
 	return [][2]string{
 		{"Backing Filesystem", backingFs},
 		{"Supports d_type", strconv.FormatBool(d.supportsDType)},
+		{"Native Overlay Diff", strconv.FormatBool(!useNaiveDiff(d.home))},
 	}
 }
 
@@ -606,7 +621,7 @@ func (d *Driver) getDiffPath(id string) string {
 // and its parent and returns the size in bytes of the changes
 // relative to its base filesystem directory.
 func (d *Driver) DiffSize(id, parent string) (size int64, err error) {
-	if !d.isParent(id, parent) {
+	if useNaiveDiff(d.home) || !d.isParent(id, parent) {
 		return d.naiveDiff.DiffSize(id, parent)
 	}
 	return directory.Size(d.getDiffPath(id))
@@ -615,7 +630,7 @@ func (d *Driver) DiffSize(id, parent string) (size int64, err error) {
 // Diff produces an archive of the changes between the specified
 // layer and its parent layer which may be "".
 func (d *Driver) Diff(id, parent string) (io.ReadCloser, error) {
-	if !d.isParent(id, parent) {
+	if useNaiveDiff(d.home) || !d.isParent(id, parent) {
 		return d.naiveDiff.Diff(id, parent)
 	}
 
@@ -632,7 +647,7 @@ func (d *Driver) Diff(id, parent string) (io.ReadCloser, error) {
 // Changes produces a list of changes between the specified layer
 // and its parent layer. If parent is "", then all changes will be ADD changes.
 func (d *Driver) Changes(id, parent string) ([]archive.Change, error) {
-	if !d.isParent(id, parent) {
+	if useNaiveDiff(d.home) || !d.isParent(id, parent) {
 		return d.naiveDiff.Changes(id, parent)
 	}
 	// Overlay doesn't have snapshots, so we need to get changes from all parent

integration-cli/docker_cli_build_test.go

@@ -7267,3 +7267,23 @@ func (s *DockerSuite) TestBuildContChar(c *check.C) {
 	c.Assert(out, checker.Contains, "Step 1/2 : FROM busybox")
 	c.Assert(out, checker.Contains, "Step 2/2 : RUN echo hi \\\\\n")
 }
+
+// TestBuildOpaqueDirectory tests that a build succeeds which
+// creates opaque directories.
+// See https://github.com/docker/docker/issues/25244
+func (s *DockerSuite) TestBuildOpaqueDirectory(c *check.C) {
+	testRequires(c, DaemonIsLinux)
+
+	dockerFile := `
+		FROM busybox
+		RUN mkdir /dir1 && touch /dir1/f1
+		RUN rm -rf /dir1 && mkdir /dir1 && touch /dir1/f2
+		RUN touch /dir1/f3
+		RUN [ -f /dir1/f2 ]
+		`
+
+	// Test that build succeeds, last command fails if opaque directory
+	// was not handled correctly
+	_, err := buildImage("testopaquedirectory", dockerFile, false)
+	c.Assert(err, checker.IsNil)
+}