Merge pull request #115769 from mochizuki875/revert-115732-revert-root-test

Revert "Revert #114605: its unit test requires root permission"

staging/src/k8s.io/mount-utils/mount_linux.go

@@ -363,19 +363,7 @@ func (mounter *Mounter) Unmount(target string) error {
 	command := exec.Command("umount", target)
 	output, err := command.CombinedOutput()
 	if err != nil {
-		if err.Error() == errNoChildProcesses {
+		return checkUmountError(target, command, output, err, mounter.withSafeNotMountedBehavior)
-			if command.ProcessState.Success() {
-				// We don't consider errNoChildProcesses an error if the process itself succeeded (see - k/k issue #103753).
-				return nil
-			}
-			// Rewrite err with the actual exit error of the process.
-			err = &exec.ExitError{ProcessState: command.ProcessState}
-		}
-		if mounter.withSafeNotMountedBehavior && strings.Contains(string(output), errNotMounted) {
-			klog.V(4).Infof("ignoring 'not mounted' error for %s", target)
-			return nil
-		}
-		return fmt.Errorf("unmount failed: %v\nUnmounting arguments: %s\nOutput: %s", err, target, string(output))
 	}
 	return nil
 }
@@ -383,11 +371,11 @@ func (mounter *Mounter) Unmount(target string) error {
 // UnmountWithForce unmounts given target but will retry unmounting with force option
 // after given timeout.
 func (mounter *Mounter) UnmountWithForce(target string, umountTimeout time.Duration) error {
-	err := tryUnmount(mounter, target, umountTimeout)
+	err := tryUnmount(target, mounter.withSafeNotMountedBehavior, umountTimeout)
 	if err != nil {
 		if err == context.DeadlineExceeded {
 			klog.V(2).Infof("Timed out waiting for unmount of %s, trying with -f", target)
-			err = forceUmount(target)
+			err = forceUmount(target, mounter.withSafeNotMountedBehavior)
 		}
 		return err
 	}
@@ -799,13 +787,13 @@ func (mounter *Mounter) IsMountPoint(file string) (bool, error) {
 }
 
 // tryUnmount calls plain "umount" and waits for unmountTimeout for it to finish.
-func tryUnmount(mounter *Mounter, path string, unmountTimeout time.Duration) error {
+func tryUnmount(target string, withSafeNotMountedBehavior bool, unmountTimeout time.Duration) error {
-	klog.V(4).Infof("Unmounting %s", path)
+	klog.V(4).Infof("Unmounting %s", target)
 	ctx, cancel := context.WithTimeout(context.Background(), unmountTimeout)
 	defer cancel()
 
-	cmd := exec.CommandContext(ctx, "umount", path)
+	command := exec.CommandContext(ctx, "umount", target)
-	out, cmderr := cmd.CombinedOutput()
+	output, err := command.CombinedOutput()
 
 	// CombinedOutput() does not return DeadlineExceeded, make sure it's
 	// propagated on timeout.
@@ -813,22 +801,35 @@ func tryUnmount(mounter *Mounter, path string, unmountTimeout time.Duration) err
 		return ctx.Err()
 	}
 
-	if cmderr != nil {
+	if err != nil {
-		if mounter.withSafeNotMountedBehavior && strings.Contains(string(out), errNotMounted) {
+		return checkUmountError(target, command, output, err, withSafeNotMountedBehavior)
-			klog.V(4).Infof("ignoring 'not mounted' error for %s", path)
+	}
+	return nil
+}
+
+func forceUmount(target string, withSafeNotMountedBehavior bool) error {
+	command := exec.Command("umount", "-f", target)
+	output, err := command.CombinedOutput()
+
+	if err != nil {
+		return checkUmountError(target, command, output, err, withSafeNotMountedBehavior)
+	}
+	return nil
+}
+
+// checkUmountError checks a result of umount command and determine a return value.
+func checkUmountError(target string, command *exec.Cmd, output []byte, err error, withSafeNotMountedBehavior bool) error {
+	if err.Error() == errNoChildProcesses {
+		if command.ProcessState.Success() {
+			// We don't consider errNoChildProcesses an error if the process itself succeeded (see - k/k issue #103753).
 			return nil
 		}
-		return fmt.Errorf("unmount failed: %v\nUnmounting arguments: %s\nOutput: %s", cmderr, path, string(out))
+		// Rewrite err with the actual exit error of the process.
+		err = &exec.ExitError{ProcessState: command.ProcessState}
 	}
-	return nil
+	if withSafeNotMountedBehavior && strings.Contains(string(output), errNotMounted) {
-}
+		klog.V(4).Infof("ignoring 'not mounted' error for %s", target)
-
+		return nil
-func forceUmount(path string) error {
-	cmd := exec.Command("umount", "-f", path)
-	out, cmderr := cmd.CombinedOutput()
-
-	if cmderr != nil {
-		return fmt.Errorf("unmount failed: %v\nUnmounting arguments: %s\nOutput: %s", cmderr, path, string(out))
 	}
-	return nil
+	return fmt.Errorf("unmount failed: %v\nUnmounting arguments: %s\nOutput: %s", err, target, string(output))
 }

staging/src/k8s.io/mount-utils/mount_linux_test.go

@@ -24,6 +24,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"os/exec"
 	"reflect"
 	"strings"
 	"sync"
@@ -607,6 +608,47 @@ func TestDetectSafeNotMountedBehavior(t *testing.T) {
 	}
 }
 
+func TestCheckUmountError(t *testing.T) {
+	target := "/test/path"
+	withSafeNotMountedBehavior := true
+	command := exec.Command("uname", "-r") // dummy command return status 0
+
+	if err := command.Run(); err != nil {
+		t.Errorf("Faild to exec dummy command. err: %s", err)
+	}
+
+	testcases := []struct {
+		output   []byte
+		err      error
+		expected bool
+	}{
+		{
+			err:      errors.New("wait: no child processes"),
+			expected: true,
+		},
+		{
+			output:   []byte("umount: /test/path: not mounted."),
+			err:      errors.New("exit status 1"),
+			expected: true,
+		},
+		{
+			output:   []byte("umount: /test/path: No such file or directory"),
+			err:      errors.New("exit status 1"),
+			expected: false,
+		},
+	}
+
+	for _, v := range testcases {
+		if err := checkUmountError(target, command, v.output, v.err, withSafeNotMountedBehavior); (err == nil) != v.expected {
+			if v.expected {
+				t.Errorf("Expected to return nil, but did not. err: %s", err)
+			} else {
+				t.Errorf("Expected to return error, but did not.")
+			}
+		}
+	}
+}
+
 func TestFormat(t *testing.T) {
 	const (
 		formatCount    = 5