From f09346578021c12069b6deb9487a1462b8d28a83 Mon Sep 17 00:00:00 2001

From: Nalin Dahyabhai <nalin@redhat.com>

Date: Thu, 21 Nov 2019 15:32:41 -0500

Subject: [PATCH 1/3] bind: don't complain about missing mountpoints

When we go to unmount a tree of mounts, if one of the directories isn't

there, instead of returning an error as before, log a debug message and

keep going.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>

---

 bind/mount.go | 4 ++++

 1 file changed, 4 insertions(+)

diff --git a/bind/mount.go b/bind/mount.go

index e1ae323b9..adde901fd 100644

--- a/bind/mount.go

+++ b/bind/mount.go

@@ -264,6 +264,10 @@ func UnmountMountpoints(mountpoint string, mountpointsToRemove []string) error {

 		mount := getMountByID(id)

 		// check if this mountpoint is mounted

 		if err := unix.Lstat(mount.Mountpoint, &st); err != nil {

+			if os.IsNotExist(err) {

+				logrus.Debugf("mountpoint %q is not present(?), skipping", mount.Mountpoint)

+				continue

+			}

 			return errors.Wrapf(err, "error checking if %q is mounted", mount.Mountpoint)

 		}

 		if mount.Major != int(unix.Major(st.Dev)) || mount.Minor != int(unix.Minor(st.Dev)) {

From c5fb681a6082b78c422eb3531667dc6d607a9355 Mon Sep 17 00:00:00 2001

From: Nalin Dahyabhai <nalin@redhat.com>

Date: Fri, 22 Nov 2019 14:22:26 -0500

Subject: [PATCH 2/3] chroot: Unmount with MNT_DETACH instead of

 UnmountMountpoints()

Unmounting the rootfs with MNT_DETACH should unmount everything below

it, so we don't need to use the more exhaustive method that our bind

package uses for its bind mounts.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>

---

 chroot/run.go | 25 +++++++++++++++----------

 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/chroot/run.go b/chroot/run.go

index fbccbcdb0..76ac78d1f 100644

--- a/chroot/run.go

+++ b/chroot/run.go

@@ -15,6 +15,7 @@ import (

 	"strings"

 	"sync"

 	"syscall"

+	"time"

 	"unsafe"

 	"github.com/containers/buildah/bind"

@@ -1002,12 +1003,19 @@ func isDevNull(dev os.FileInfo) bool {

 // callback that will clean up its work.

 func setupChrootBindMounts(spec *specs.Spec, bundlePath string) (undoBinds func() error, err error) {

 	var fs unix.Statfs_t

-	removes := []string{}

 	undoBinds = func() error {

-		if err2 := bind.UnmountMountpoints(spec.Root.Path, removes); err2 != nil {

-			logrus.Warnf("pkg/chroot: error unmounting %q: %v", spec.Root.Path, err2)

-			if err == nil {

-				err = err2

+		if err2 := unix.Unmount(spec.Root.Path, unix.MNT_DETACH); err2 != nil {

+			retries := 0

+			for (err2 == unix.EBUSY || err2 == unix.EAGAIN) && retries < 50 {

+				time.Sleep(50 * time.Millisecond)

+				err2 = unix.Unmount(spec.Root.Path, unix.MNT_DETACH)

+				retries++

+			}

+			if err2 != nil {

+				logrus.Warnf("pkg/chroot: error unmounting %q (retried %d times): %v", spec.Root.Path, retries, err2)

+				if err == nil {

+					err = err2

+				}

 			}

 		}

 		return err

@@ -1096,6 +1104,7 @@ func setupChrootBindMounts(spec *specs.Spec, bundlePath string) (undoBinds func(

 	// Add /sys/fs/selinux to the set of masked paths, to ensure that we don't have processes

 	// attempting to interact with labeling, when they aren't allowed to do so.

 	spec.Linux.MaskedPaths = append(spec.Linux.MaskedPaths, "/sys/fs/selinux")

+

 	// Bind mount in everything we've been asked to mount.

 	for _, m := range spec.Mounts {

 		// Skip anything that we just mounted.

@@ -1141,13 +1150,11 @@ func setupChrootBindMounts(spec *specs.Spec, bundlePath string) (undoBinds func(

 			if !os.IsNotExist(err) {

 				return undoBinds, errors.Wrapf(err, "error examining %q for mounting in mount namespace", target)

 			}

-			// The target isn't there yet, so create it, and make a

-			// note to remove it later.

+			// The target isn't there yet, so create it.

 			if srcinfo.IsDir() {

 				if err = os.MkdirAll(target, 0111); err != nil {

 					return undoBinds, errors.Wrapf(err, "error creating mountpoint %q in mount namespace", target)

 				}

-				removes = append(removes, target)

 			} else {

 				if err = os.MkdirAll(filepath.Dir(target), 0111); err != nil {

 					return undoBinds, errors.Wrapf(err, "error ensuring parent of mountpoint %q (%q) is present in mount namespace", target, filepath.Dir(target))

@@ -1157,7 +1164,6 @@ func setupChrootBindMounts(spec *specs.Spec, bundlePath string) (undoBinds func(

 					return undoBinds, errors.Wrapf(err, "error creating mountpoint %q in mount namespace", target)

 				}

 				file.Close()

-				removes = append(removes, target)

 			}

 		}

 		requestFlags := bindFlags

@@ -1266,7 +1272,6 @@ func setupChrootBindMounts(spec *specs.Spec, bundlePath string) (undoBinds func(

 		if err := os.Mkdir(roEmptyDir, 0700); err != nil {

 			return undoBinds, errors.Wrapf(err, "error creating empty directory %q", roEmptyDir)

 		}

-		removes = append(removes, roEmptyDir)

 	}

 	// Set up any masked paths that we need to.  If we're running inside of

From ec1be6a51941e10b5316c911ef97c88940f7c095 Mon Sep 17 00:00:00 2001

From: Nalin Dahyabhai <nalin@redhat.com>

Date: Fri, 22 Nov 2019 14:52:25 -0500

Subject: [PATCH 3/3] overlay.bats typo: fuse-overlays should be fuse-overlayfs

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>

---

 tests/overlay.bats | 4 ++--

 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/overlay.bats b/tests/overlay.bats

index 04056f680..7cc2d0c62 100644

--- a/tests/overlay.bats

+++ b/tests/overlay.bats

@@ -3,14 +3,14 @@

 load helpers

 @test "overlay specific level" {

-  if test \! -e /usr/bin/fuse-overlays -a  "$BUILDAH_ISOLATION" = "rootless"; then

+  if test \! -e /usr/bin/fuse-overlayfs -a "$BUILDAH_ISOLATION" = "rootless"; then

     skip "BUILDAH_ISOLATION = $BUILDAH_ISOLATION" and no /usr/bin/fuse-overlayfs present

   fi

   image=alpine

   mkdir ${TESTDIR}/lower

   touch ${TESTDIR}/lower/foo

-cid=$(buildah --log-level=error from -v ${TESTDIR}/lower:/lower:O --quiet --signature-policy ${TESTSDIR}/policy.json $image)

+  cid=$(buildah --log-level=error from -v ${TESTDIR}/lower:/lower:O --quiet --signature-policy ${TESTSDIR}/policy.json $image)

   # This should succeed

   run_buildah --log-level=error run $cid ls /lower/foo