Merge pull request #114605 from mochizuki875/feature_114546

Fix: Pod terminating stuck because of trying to umount not actual mounted dir
This commit is contained in:
Kubernetes Prow Robot 2023-02-01 07:25:08 -08:00 committed by GitHub
commit a94caec59c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 60 additions and 33 deletions

View File

@ -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
}
@ -775,13 +763,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.
@ -789,22 +777,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))
}

View File

@ -26,6 +26,7 @@ import (
"reflect"
"strings"
"testing"
"time"
utilexec "k8s.io/utils/exec"
testexec "k8s.io/utils/exec/testing"
@ -620,3 +621,28 @@ func makeFakeCommandAction(stdout string, err error) testexec.FakeCommandAction
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)
}
}
}