diff --git a/staging/src/k8s.io/mount-utils/mount_linux.go b/staging/src/k8s.io/mount-utils/mount_linux.go index db0beb6d034..f0125fcb489 100644 --- a/staging/src/k8s.io/mount-utils/mount_linux.go +++ b/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 { - 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 checkUmountError(target, command, output, err, mounter.withSafeNotMountedBehavior) } 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 { - klog.V(4).Infof("Unmounting %s", path) +func tryUnmount(target string, withSafeNotMountedBehavior bool, unmountTimeout time.Duration) error { + klog.V(4).Infof("Unmounting %s", target) ctx, cancel := context.WithTimeout(context.Background(), unmountTimeout) defer cancel() - cmd := exec.CommandContext(ctx, "umount", path) - out, cmderr := cmd.CombinedOutput() + command := exec.CommandContext(ctx, "umount", target) + 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 mounter.withSafeNotMountedBehavior && strings.Contains(string(out), errNotMounted) { - klog.V(4).Infof("ignoring 'not mounted' error for %s", path) + if err != nil { + return checkUmountError(target, command, output, err, withSafeNotMountedBehavior) + } + 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 -} - -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)) + if withSafeNotMountedBehavior && strings.Contains(string(output), errNotMounted) { + klog.V(4).Infof("ignoring 'not mounted' error for %s", target) + return nil } - return nil + return fmt.Errorf("unmount failed: %v\nUnmounting arguments: %s\nOutput: %s", err, target, string(output)) } diff --git a/staging/src/k8s.io/mount-utils/mount_linux_test.go b/staging/src/k8s.io/mount-utils/mount_linux_test.go index 0fcb156ee9f..61823abb9f0 100644 --- a/staging/src/k8s.io/mount-utils/mount_linux_test.go +++ b/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