fix: set "host is down" as corrupted mount #101398

andyzhangx · 2021-04-23T02:55:06Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

fix: set "host is down" as corrupted mount

When SMB server is down, there is no way to terminate pod which is using SMB mount, would get following error. This PR regard host is down as corrupted mount dir.

Apr 20 11:11:52 aks-nonzone-17963928-vmss000000 kubelet[8516]: E0420 11:11:52.618206    8516 nestedpendingoperations.go:301] Operation for "{volumeName:kubernetes.io/csi/smb.csi.k8s.io^pvc-da830ab0-e8d2-4e0e-89f4-906a4e4398a4 podName:90ee5993-7f80-4fe3-9d88-7bc89b19cbfe nodeName:}" failed. No retries permitted until 2021-04-20 11:11:56.61815874 +0000 UTC m=+337.879942743 (durationBeforeRetry 4s). Error: "UnmountVolume.TearDown failed for volume \"persistent-storage\" (UniqueName: \"kubernetes.io/csi/smb.csi.k8s.io^pvc-da830ab0-e8d2-4e0e-89f4-906a4e4398a4\") pod \"90ee5993-7f80-4fe3-9d88-7bc89b19cbfe\" (UID: \"90ee5993-7f80-4fe3-9d88-7bc89b19cbfe\") : kubernetes.io/csi: mounter.TearDownAt failed to clean mount dir [/var/lib/kubelet/pods/90ee5993-7f80-4fe3-9d88-7bc89b19cbfe/volumes/kubernetes.io~csi/pvc-da830ab0-e8d2-4e0e-89f4-906a4e4398a4/mount]: stat /var/lib/kubelet/pods/90ee5993-7f80-4fe3-9d88-7bc89b19cbfe/volumes/kubernetes.io~csi/pvc-da830ab0-e8d2-4e0e-89f4-906a4e4398a4/mount: host is down"

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

fix: set "host is down" as corrupted mount

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

fix: set "host is down" as corrupted mount

andyzhangx · 2021-04-23T02:56:28Z

/kind bug
/assign @msau42
/priority important-soon
/sig cloud-provider
/area provider/azure
/triage accepted

andyzhangx · 2021-04-23T03:47:40Z

/retest

andyzhangx · 2021-04-23T03:51:33Z

/test pull-kubernetes-e2e-aks-engine-azure-file
/test pull-kubernetes-e2e-aks-engine-azure-disk-vmss

andyzhangx · 2021-04-23T08:20:04Z

/retest

andyzhangx · 2021-04-23T14:42:57Z

/retest

gnufied · 2021-04-23T14:50:24Z

@andyzhangx The follow up PR will be in the CSI driver repo right? So as on NodeUnpublish the CSI driver removes the publish directory entirely and hence that operation will be a NO-OP in kubelet.

andyzhangx · 2021-04-23T14:56:47Z

@andyzhangx The follow up PR will be in the CSI driver repo right? So as on NodeUnpublish the CSI driver removes the publish directory entirely and hence that operation will be a NO-OP in kubelet.

@gnufied I found this should be the only fix in CSI driver repo since SMB CSI driver also uses this IsCorruptedMnt func, so with this PR merged, and after mount-utils vendor updated in CSI driver, SMB CSI driver could unmount host is down mount point correctly.

gnufied · 2021-04-23T15:02:38Z

/lgtm

gnufied · 2021-04-23T15:05:32Z

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

@@ -52,7 +52,7 @@ func IsCorruptedMnt(err error) bool {
 		underlyingError = pe.Err
 	}

-	return underlyingError == syscall.ENOTCONN || underlyingError == syscall.ESTALE || underlyingError == syscall.EIO || underlyingError == syscall.EACCES
+	return underlyingError == syscall.ENOTCONN || underlyingError == syscall.ESTALE || underlyingError == syscall.EIO || underlyingError == syscall.EACCES || underlyingError == syscall.EHOSTDOWN


I wonder though - if we should separate corrupted mounts from socket/host not connected errors. But may be we could consider them same. cc @chakri-nelluri

When the host is down does that actually mean the mount is gone and cleaned up on the client side? Wondering if this will cause stale/leaking mounts and we need to do something like a force unmount instead.

Here is the way how this host is down mount point is unmounted in CSI driver, SMB CSI driver invoke CleanupMountPoint func to unmount mount point:

without this PR, PathExists return false, err directly, and then CleanupMountPoint returns Warning: Unmount skipped because path does not exist (actually do nothing)

with this PR, PathExists return true, err, and finally invoke doCleanupMountPoint to unmount, that will fix this issue.

So by this PR, CSI driver should unmount corrupted mount point if using CleanupMountPoint func. @jingxu97

kubernetes/staging/src/k8s.io/mount-utils/mount_helper_common.go

Lines 137 to 147 in ea07644

func PathExists(path string) (bool, error) {

_, err := os.Stat(path)

if err == nil {

return true, nil

} else if os.IsNotExist(err) {

return false, nil

} else if IsCorruptedMnt(err) {

return true, err

}

return false, err

}

kubernetes/staging/src/k8s.io/mount-utils/mount_helper_common.go

Lines 31 to 41 in ea07644

func CleanupMountPoint(mountPath string, mounter Interface, extensiveMountPointCheck bool) error {

pathExists, pathErr := PathExists(mountPath)

if !pathExists && pathErr == nil {

klog.Warningf("Warning: Unmount skipped because path does not exist: %v", mountPath)

return nil

}

corruptedMnt := IsCorruptedMnt(pathErr)

if pathErr != nil && !corruptedMnt {

return fmt.Errorf("Error checking path: %v", pathErr)

}

return doCleanupMountPoint(mountPath, mounter, extensiveMountPointCheck, corruptedMnt)

how about Windows case? Is windows version of function can already handle this case?

@jingxu97 good point, I added windows fix.
While this specific SMB unmount issue actually does not exists on Windows since CleanupMountPoint on Windows just removes directly. Anyway, should also fix IsCorruptedMnt issue on Windows.

func CleanupMountPoint(m *mount.SafeFormatAndMount, target string, extensiveMountCheck bool) error { proxy, ok := m.Interface.(*mounter.CSIProxyMounter) if !ok { return fmt.Errorf("could not cast to csi proxy class") } return proxy.Rmdir(target) }

add HOSTDOWN code for Windows

andyzhangx · 2021-04-24T14:03:56Z

/retest

msau42 · 2021-04-27T03:36:54Z

/lgtm

/assign @jingxu97

msau42 · 2021-04-27T03:37:05Z

/retest

jingxu97 · 2021-04-27T05:11:04Z

/approve

k8s-ci-robot · 2021-04-27T05:11:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx, jingxu97

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~staging/src/k8s.io/mount-utils/OWNERS~~ [jingxu97]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

fejta-bot · 2021-04-27T07:33:53Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

…101398-upstream-release-1.21 Automated cherry pick of #101398: fix: set "host is down" as corrupted mount

…101398-upstream-release-1.20 Automated cherry pick of #101398: fix: set "host is down" as corrupted mount

andyzhangx · 2022-07-21T00:57:27Z

this PR is cherry-picked to 1.20.7, 1.21.1, 1.22.0 releases

k8s-ci-robot requested review from gnufied and saad-ali April 23, 2021 02:56

andyzhangx mentioned this pull request Apr 23, 2021

fix unmount failure for SMB volume in host is down state #101305

Closed

k8s-ci-robot assigned msau42 Apr 23, 2021

andyzhangx mentioned this pull request Apr 23, 2021

fix: set "host is down" as corrupted mount kubernetes/mount-utils#3

Closed

k8s-ci-robot assigned gnufied Apr 23, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 23, 2021

gnufied reviewed Apr 23, 2021

View reviewed changes

fix: set "host is down" as corrupted mount

4dde743

add HOSTDOWN code for Windows

andyzhangx force-pushed the EHOSTDOWN branch from 2c889bf to 4dde743 Compare April 24, 2021 12:48

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 24, 2021

k8s-ci-robot assigned jingxu97 Apr 27, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 27, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 27, 2021

k8s-ci-robot merged commit fbaf5ee into kubernetes:master Apr 27, 2021

k8s-ci-robot added this to the v1.22 milestone Apr 27, 2021

k8s-ci-robot added a commit that referenced this pull request May 7, 2021

Merge pull request #101518 from andyzhangx/automated-cherry-pick-of-#…

26c1fbd

…101398-upstream-release-1.21 Automated cherry pick of #101398: fix: set "host is down" as corrupted mount

k8s-ci-robot added a commit that referenced this pull request May 7, 2021

Merge pull request #101519 from andyzhangx/automated-cherry-pick-of-#…

30d205f

…101398-upstream-release-1.20 Automated cherry pick of #101398: fix: set "host is down" as corrupted mount

This was referenced May 19, 2021

fix: set "host is down" as corrupted mount kubernetes-csi/csi-driver-smb#268

Merged

fix: set "host is down" as corrupted mount kubernetes-sigs/azurefile-csi-driver#662

Merged

fix: set "host is down" as corrupted mount kubernetes-sigs/blob-csi-driver#431

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: set "host is down" as corrupted mount #101398

fix: set "host is down" as corrupted mount #101398

andyzhangx commented Apr 23, 2021 •

edited

Loading

andyzhangx commented Apr 23, 2021

andyzhangx commented Apr 23, 2021

andyzhangx commented Apr 23, 2021

andyzhangx commented Apr 23, 2021

andyzhangx commented Apr 23, 2021

gnufied commented Apr 23, 2021

andyzhangx commented Apr 23, 2021 •

edited

Loading

gnufied commented Apr 23, 2021

gnufied Apr 23, 2021

msau42 Apr 23, 2021

andyzhangx Apr 24, 2021 •

edited

Loading

jingxu97 Apr 24, 2021

andyzhangx Apr 24, 2021

andyzhangx commented Apr 24, 2021

msau42 commented Apr 27, 2021

msau42 commented Apr 27, 2021

jingxu97 commented Apr 27, 2021

k8s-ci-robot commented Apr 27, 2021

fejta-bot commented Apr 27, 2021

andyzhangx commented Jul 21, 2022

	func PathExists(path string) (bool, error) {
	_, err := os.Stat(path)
	if err == nil {
	return true, nil
	} else if os.IsNotExist(err) {
	return false, nil
	} else if IsCorruptedMnt(err) {
	return true, err
	}
	return false, err
	}

	func CleanupMountPoint(mountPath string, mounter Interface, extensiveMountPointCheck bool) error {
	pathExists, pathErr := PathExists(mountPath)
	if !pathExists && pathErr == nil {
	klog.Warningf("Warning: Unmount skipped because path does not exist: %v", mountPath)
	return nil
	}
	corruptedMnt := IsCorruptedMnt(pathErr)
	if pathErr != nil && !corruptedMnt {
	return fmt.Errorf("Error checking path: %v", pathErr)
	}
	return doCleanupMountPoint(mountPath, mounter, extensiveMountPointCheck, corruptedMnt)

fix: set "host is down" as corrupted mount #101398

fix: set "host is down" as corrupted mount #101398

Conversation

andyzhangx commented Apr 23, 2021 • edited Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

andyzhangx commented Apr 23, 2021

andyzhangx commented Apr 23, 2021

andyzhangx commented Apr 23, 2021

andyzhangx commented Apr 23, 2021

andyzhangx commented Apr 23, 2021

gnufied commented Apr 23, 2021

andyzhangx commented Apr 23, 2021 • edited Loading

gnufied commented Apr 23, 2021

gnufied Apr 23, 2021

Choose a reason for hiding this comment

msau42 Apr 23, 2021

Choose a reason for hiding this comment

andyzhangx Apr 24, 2021 • edited Loading

Choose a reason for hiding this comment

jingxu97 Apr 24, 2021

Choose a reason for hiding this comment

andyzhangx Apr 24, 2021

Choose a reason for hiding this comment

andyzhangx commented Apr 24, 2021

msau42 commented Apr 27, 2021

msau42 commented Apr 27, 2021

jingxu97 commented Apr 27, 2021

k8s-ci-robot commented Apr 27, 2021

fejta-bot commented Apr 27, 2021

andyzhangx commented Jul 21, 2022

andyzhangx commented Apr 23, 2021 •

edited

Loading

andyzhangx commented Apr 23, 2021 •

edited

Loading

andyzhangx Apr 24, 2021 •

edited

Loading