Retry unmount on EBUSY and return errors

This is another WIP to fix #1785.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>

container_opts_unix.go

@@ -24,7 +24,6 @@ import (
 	"github.com/opencontainers/image-spec/identity"
 	"github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
-	"golang.org/x/sys/unix"
 )
 
 // WithCheckpoint allows a container to be created from the checkpointed information
@@ -199,8 +198,11 @@ func remapRootFS(mounts []mount.Mount, uid, gid uint32) error {
 			return err
 		}
 	}
-	defer unix.Unmount(root, 0)
-	return filepath.Walk(root, incrementFS(root, uid, gid))
+	err = filepath.Walk(root, incrementFS(root, uid, gid))
+	if uerr := mount.Unmount(root, 0); err == nil {
+		err = uerr
+	}
+	return err
 }
 
 func incrementFS(root string, uidInc, gidInc uint32) filepath.WalkFunc {

linux/runtime.go

@@ -22,6 +22,7 @@ import (
 	shim "github.com/containerd/containerd/linux/shim/v1"
 	"github.com/containerd/containerd/log"
 	"github.com/containerd/containerd/metadata"
+	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/namespaces"
 	"github.com/containerd/containerd/platforms"
 	"github.com/containerd/containerd/plugin"
@@ -474,7 +475,7 @@ func (r *Runtime) terminate(ctx context.Context, bundle *bundle, ns, id string)
 	}); err != nil {
 		log.G(ctx).WithError(err).Warnf("delete runtime state %s", id)
 	}
-	if err := unix.Unmount(filepath.Join(bundle.path, "rootfs"), 0); err != nil {
+	if err := mount.Unmount(filepath.Join(bundle.path, "rootfs"), 0); err != nil {
 		log.G(ctx).WithError(err).WithFields(logrus.Fields{
 			"path": bundle.path,
 			"id":   id,

linux/shim/local.go

@@ -7,8 +7,8 @@ import (
 	"path/filepath"
 
 	shimapi "github.com/containerd/containerd/linux/shim/v1"
+	"github.com/containerd/containerd/mount"
 	ptypes "github.com/gogo/protobuf/types"
-	"golang.org/x/sys/unix"
 )
 
 // NewLocal returns a shim client implementation for issue commands to a shim
@@ -32,7 +32,7 @@ func (c *local) Start(ctx context.Context, in *shimapi.StartRequest) (*shimapi.S
 
 func (c *local) Delete(ctx context.Context, in *ptypes.Empty) (*shimapi.DeleteResponse, error) {
 	// make sure we unmount the containers rootfs for this local
-	if err := unix.Unmount(filepath.Join(c.s.config.Path, "rootfs"), 0); err != nil {
+	if err := mount.Unmount(filepath.Join(c.s.config.Path, "rootfs"), 0); err != nil {
 		return nil, err
 	}
 	return c.s.Delete(ctx, in)

mount/mount_linux.go

@@ -2,7 +2,9 @@ package mount
 
 import (
 	"strings"
+	"time"
 
+	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
 )
 
@@ -42,8 +44,27 @@ func (m *Mount) Mount(target string) error {
 }
 
 // Unmount the provided mount path with the flags
-func Unmount(mount string, flags int) error {
-	return unix.Unmount(mount, flags)
+func Unmount(target string, flags int) error {
+	if err := unmount(target, flags); err != nil && err != unix.EINVAL {
+		return err
+	}
+	return nil
+}
+
+func unmount(target string, flags int) error {
+	for i := 0; i < 50; i++ {
+		if err := unix.Unmount(target, flags); err != nil {
+			switch err {
+			case unix.EBUSY:
+				time.Sleep(50 * time.Millisecond)
+				continue
+			default:
+				return err
+			}
+		}
+		return nil
+	}
+	return errors.Wrapf(unix.EBUSY, "failed to unmount target %s", target)
 }
 
 // UnmountAll repeatedly unmounts the given mount point until there
@@ -51,7 +72,7 @@ func Unmount(mount string, flags int) error {
 // useful for undoing a stack of mounts on the same mount point.
 func UnmountAll(mount string, flags int) error {
 	for {
-		if err := Unmount(mount, flags); err != nil {
+		if err := unmount(mount, flags); err != nil {
 			// EINVAL is returned if the target is not a
 			// mount point, indicating that we are
 			// done. It can also indicate a few other

oci/spec_opts_unix.go

@@ -12,12 +12,11 @@ import (
 	"strconv"
 	"strings"
 
-	"golang.org/x/sys/unix"
-
 	"github.com/containerd/containerd/containers"
 	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/fs"
 	"github.com/containerd/containerd/images"
+	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/namespaces"
 	"github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/opencontainers/runc/libcontainer/user"
@@ -260,7 +259,7 @@ func WithUIDGID(uid, gid uint32) SpecOpts {
 // or uid is not found in /etc/passwd, it sets gid to be the same with
 // uid, and not returns error.
 func WithUserID(uid uint32) SpecOpts {
-	return func(ctx context.Context, client Client, c *containers.Container, s *specs.Spec) error {
+	return func(ctx context.Context, client Client, c *containers.Container, s *specs.Spec) (err error) {
 		if c.Snapshotter == "" {
 			return errors.Errorf("no snapshotter set for container")
 		}
@@ -282,7 +281,13 @@ func WithUserID(uid uint32) SpecOpts {
 				return err
 			}
 		}
-		defer unix.Unmount(root, 0)
+		defer func() {
+			if uerr := mount.Unmount(root, 0); uerr != nil {
+				if err == nil {
+					err = uerr
+				}
+			}
+		}()
 		ppath, err := fs.RootPath(root, "/etc/passwd")
 		if err != nil {
 			return err
@@ -317,7 +322,7 @@ func WithUserID(uid uint32) SpecOpts {
 // does not exist, or the username is not found in /etc/passwd,
 // it returns error.
 func WithUsername(username string) SpecOpts {
-	return func(ctx context.Context, client Client, c *containers.Container, s *specs.Spec) error {
+	return func(ctx context.Context, client Client, c *containers.Container, s *specs.Spec) (err error) {
 		if c.Snapshotter == "" {
 			return errors.Errorf("no snapshotter set for container")
 		}
@@ -339,7 +344,13 @@ func WithUsername(username string) SpecOpts {
 				return err
 			}
 		}
-		defer unix.Unmount(root, 0)
+		defer func() {
+			if uerr := mount.Unmount(root, 0); uerr != nil {
+				if err == nil {
+					err = uerr
+				}
+			}
+		}()
 		ppath, err := fs.RootPath(root, "/etc/passwd")
 		if err != nil {
 			return err