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..dec91c72050 100644 --- a/staging/src/k8s.io/mount-utils/mount_linux_test.go +++ b/staging/src/k8s.io/mount-utils/mount_linux_test.go @@ -705,3 +705,28 @@ func makeFakeCommandAction(stdout string, err error, cmdFn func()) testexec.Fake return testexec.InitFakeCmd(&c, cmd, args...) } } + +func TestNotMountedBehaviorOfUnmount(t *testing.T) { + target, err := ioutil.TempDir("", "kubelet-umount") + if err != nil { + t.Errorf("Cannot create temp dir: %v", err) + } + + defer os.RemoveAll(target) + + m := Mounter{withSafeNotMountedBehavior: true} + if err = m.Unmount(target); err != nil { + t.Errorf(`Expect complete Unmount(), but it dose not: %v`, err) + } + + if err = tryUnmount(target, m.withSafeNotMountedBehavior, time.Minute); err != nil { + t.Errorf(`Expect complete tryUnmount(), but it does not: %v`, err) + } + + // forceUmount exec "umount -f", so skip this case if user is not root. + if os.Getuid() == 0 { + if err = forceUmount(target, m.withSafeNotMountedBehavior); err != nil { + t.Errorf(`Expect complete forceUnmount(), but it does not: %v`, err) + } + } +}