fix pod stuck in Terminating issue due to corrupt mnt point in flexvol plugin #75234

andyzhangx · 2019-03-09T11:37:11Z

What type of PR is this?
/kind bug

What this PR does / why we need it:
This PR fixed this issue:
pod could not be terminated when pod volume path is corrupted due to smb server return error status: resource temporarily unavailable, in this condition, pod will be in Terminating forever. In this PR, it would call Unmount if PathExists returns any error.

$ kubectl get po
NAME              READY   STATUS        RESTARTS   AGE
nginx-flex-smb   0/1     Terminating   0          6h46m

Which issue(s) this PR fixes:

Fixes #75233

Special notes for your reviewer:
EAGAIN - APPLICATION_ERROR: "resource temporarily unavailable"
This error happens when SMB server is unavailable due to network issue, password change, smb path change, etc. , in this PR, it would call Unmount if PathExists returns any error.

workaround for this issue

on agent node, run mount | grep cifs to get all cifs mounts
check every cifs mount, e.g.

sudo ls -lt /var/lib/kubelet/pods/5c949781-4c6d-11e9-825d-000d3a0dd47b/volumes/microsoft.com~smb/test
ls: cannot access '/var/lib/kubelet/pods/5c949781-4c6d-11e9-825d-000d3a0dd47b/volumes/microsoft.com~smb/test': Permission denied

if ls -lt ... failed, run

sudo umount /var/lib/kubelet/pods/5c949781-4c6d-11e9-825d-000d3a0dd47b/volumes/microsoft.com~smb/test

And then the pod will be in terminated soon.

Does this PR introduce a user-facing change?:

fix pod stuck issue due to corrupt mnt point in flexvol plugin, call Unmount if PathExists returns any error

/priority important-soon
/assign @jingxu97 @msau42 @davidz627

pkg/kubelet/kubelet_volumes.go

gnufied · 2019-03-09T16:59:30Z

/assign

cc @chakri-nelluri

pkg/util/mount/mount_helper.go

andyzhangx · 2019-03-13T02:00:30Z

thanks guys for the comments, let me hold this PR, and track the root case
/hold

andyzhangx · 2019-03-22T07:06:34Z

PTAL, thanks.

andyzhangx · 2019-05-22T08:55:35Z

hi @jingxu97 @msau42 I have reverted the code change in PathExists func, make this PR only in flexvol function. Change of PathExists func may affect too many plugins, could you take a look? I would also cherry pick this PR to old release, thanks.

jingxu97 · 2019-05-23T01:46:25Z

pkg/volume/flexvolume/unmounter.go

-
-	if pathErr != nil && !mount.IsCorruptedMnt(pathErr) {
-		return fmt.Errorf("Error checking path: %v", pathErr)
+	if pathErr != nil {


how about
if !pathExists & pathErr == nil {
return nil
}
...

then this function would depend on return value of PathExists, if it return <false, error>, it won't proceed, mainly I would like to cherry pick to old release, only for flexvol.TearDownAt func

oh, sorry my mistake. I corrected it.
only if it return (false, nil) which means path is definitely not exist , it will return immediately without any further actions.

Hi @jingxu97 logic of this PR is identical to this your suggested comment:

if pathErr != nil { // only log warning here since plugins should anyways have to deal with errors klog.Warningf("Error checking path: %v", pathErr) } else { if !pathExists { klog.Warningf("Warning: Unmount skipped because path does not exist: %v", dir) return nil } }

yes, the logic is the same. I am thinking to make it easier to read. And I think the other PR #75645 also uses the simpler way.

@jingxu97 shall we go with this PR first, and I would like to cherry pick this PR to old release, thanks.

andyzhangx · 2019-05-30T03:21:29Z

/hold cancel
@jingxu97 PTAL, already addressed your comments, thanks.

andyzhangx · 2019-06-03T14:13:32Z

ping @msau42 @jingxu97

jingxu97 · 2019-06-07T21:25:01Z

/lgtm

andyzhangx · 2019-06-08T01:20:29Z

/lgtm

still needs /approve, and are we going to merge this into v1.15? @jingxu97

jingxu97 · 2019-06-09T23:33:10Z

/approve

…

On Fri, Jun 7, 2019 at 6:20 PM Andy Zhang ***@***.***> wrote: /lgtm still needs /approve, and are we going to merge this into v1.15? @jingxu97 <https://github.com/jingxu97> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#75234?email_source=notifications&email_token=AESI3RNAELPMRYMHJEXCVYTPZMCORA5CNFSM4G42Q7X2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXHKHHI#issuecomment-500081565>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AESI3RJKVH377RY3OJQCPZ3PZMCORANCNFSM4G42Q7XQ> .

-- - Jing

andyzhangx · 2019-06-10T01:16:21Z

/assign @saad-ali @chakri-nelluri @MikaelCluseau

k8s-ci-robot · 2019-06-10T01:16:23Z

@andyzhangx: GitHub didn't allow me to assign the following users: MikaelCluseau.

Note that only kubernetes members and repo collaborators can be assigned and that issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

Details

In response to this:

/assign @saad-ali @chakri-nelluri @MikaelCluseau

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

msau42 · 2019-06-11T00:30:04Z

IIUC, before, Unmount would not be called if PathExists returns non-corrupted error.

Now what this fix does is it always calls Unmount if PathExists returns any error, corrupted or not. Is that the intended behavior @andyzhangx? From the PR description, it sounds like you wanted some change in behavior for corrupted errors?

andyzhangx · 2019-06-11T01:05:13Z

IIUC, before, Unmount would not be called if PathExists returns non-corrupted error.

Now what this fix does is it always calls Unmount if PathExists returns any error, corrupted or not. Is that the intended behavior @andyzhangx? From the PR description, it sounds like you wanted some change in behavior for corrupted errors?

I have changed the PR description, current reality is there is some unexpected error in flexvolume path, and in this PR, it would call Unmount if PathExists returns any error. @msau42

msau42 · 2019-06-11T01:09:17Z

/approve

Thanks for clarifying! Can you also fix the release note?

k8s-ci-robot · 2019-06-11T01:09:28Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~pkg/volume/flexvolume/OWNERS~~ [msau42]

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

andyzhangx · 2019-06-11T01:14:28Z

/approve

Thanks for clarifying! Can you also fix the release note?

done, thanks.

k8s-ci-robot · 2019-06-14T03:52:25Z

@andyzhangx: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-integration	`fb81a28`	link	`/test pull-kubernetes-integration`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

…5234-upstream-release-1.12 Automated cherry pick of #75234: fix flexvol stuck issue due to corrupted mnt point

…5234-upstream-release-1.13 Automated cherry pick of #75234: fix flexvol stuck issue due to corrupted mnt point

…5234-upstream-release-1.14 Automated cherry pick of #75234: fix flexvol stuck issue due to corrupted mnt point

…5234-upstream-release-1.15 Automated cherry pick of #75234: fix flexvol stuck issue due to corrupted mnt point

k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Mar 9, 2019

k8s-ci-robot assigned davidz627, jingxu97 and msau42 Mar 9, 2019

k8s-ci-robot requested review from jingxu97 and yujuhong March 9, 2019 11:38

andyzhangx force-pushed the corrupt-mnt-fix branch from 6b88c84 to cbefeae Compare March 9, 2019 11:45

k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Mar 9, 2019

gnufied reviewed Mar 9, 2019

View reviewed changes

pkg/kubelet/kubelet_volumes.go Outdated Show resolved Hide resolved

k8s-ci-robot assigned gnufied Mar 9, 2019

msau42 reviewed Mar 10, 2019

View reviewed changes

pkg/util/mount/mount_helper.go Outdated Show resolved Hide resolved

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 12, 2019

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 13, 2019

andyzhangx changed the title ~~fix pod stuck issue due to corrupt mnt point~~ fix pod stuck issue due to corrupt mnt point (resource temporarily unavailable) Mar 22, 2019

andyzhangx force-pushed the corrupt-mnt-fix branch from cbefeae to cf69d84 Compare March 22, 2019 07:04

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 22, 2019

jingxu97 reviewed May 23, 2019

View reviewed changes

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 30, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 7, 2019

k8s-ci-robot assigned chakri-nelluri and saad-ali Jun 10, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 11, 2019

k8s-ci-robot merged commit 3a598d7 into kubernetes:master Jun 14, 2019

k8s-ci-robot added a commit that referenced this pull request Jun 20, 2019

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

c440b31

…5234-upstream-release-1.12 Automated cherry pick of #75234: fix flexvol stuck issue due to corrupted mnt point

k8s-ci-robot added a commit that referenced this pull request Jun 20, 2019

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

f2941e2

…5234-upstream-release-1.13 Automated cherry pick of #75234: fix flexvol stuck issue due to corrupted mnt point

k8s-ci-robot added a commit that referenced this pull request Jun 20, 2019

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

6b360e9

…5234-upstream-release-1.14 Automated cherry pick of #75234: fix flexvol stuck issue due to corrupted mnt point

andyzhangx mentioned this pull request Jun 27, 2019

Automated cherry pick of #75234: fix flexvol stuck issue due to corrupted mnt point #79460

Merged

k8s-ci-robot added a commit that referenced this pull request Jun 28, 2019

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

b812eaa

…5234-upstream-release-1.15 Automated cherry pick of #75234: fix flexvol stuck issue due to corrupted mnt point

andyzhangx changed the title ~~fix pod stuck issue due to corrupt mnt point in flexvol plugin~~ fix pod stuck in Terminating issue due to corrupt mnt point in flexvol plugin Jul 3, 2019

fix pod stuck in Terminating issue due to corrupt mnt point in flexvol plugin #75234

fix pod stuck in Terminating issue due to corrupt mnt point in flexvol plugin #75234

Uh oh!

Conversation

andyzhangx commented Mar 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

gnufied commented Mar 9, 2019

Uh oh!

Uh oh!

andyzhangx commented Mar 13, 2019

Uh oh!

andyzhangx commented Mar 22, 2019

Uh oh!

andyzhangx commented May 22, 2019

Uh oh!

jingxu97 May 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andyzhangx May 23, 2019

Choose a reason for hiding this comment

Uh oh!

jingxu97 May 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andyzhangx May 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jingxu97 Jun 3, 2019

Choose a reason for hiding this comment

Uh oh!

andyzhangx Jun 4, 2019

Choose a reason for hiding this comment

Uh oh!

andyzhangx commented May 30, 2019

Uh oh!

andyzhangx commented Jun 3, 2019

Uh oh!

jingxu97 commented Jun 7, 2019

Uh oh!

andyzhangx commented Jun 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jingxu97 commented Jun 9, 2019 via email

Uh oh!

andyzhangx commented Jun 10, 2019

Uh oh!

k8s-ci-robot commented Jun 10, 2019

Uh oh!

msau42 commented Jun 11, 2019

Uh oh!

andyzhangx commented Jun 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

msau42 commented Jun 11, 2019

Uh oh!

k8s-ci-robot commented Jun 11, 2019

Uh oh!

andyzhangx commented Jun 11, 2019

Uh oh!

k8s-ci-robot commented Jun 14, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

andyzhangx commented Mar 9, 2019 •

edited

Loading

jingxu97 May 23, 2019 •

edited

Loading

jingxu97 May 23, 2019 •

edited

Loading

andyzhangx May 23, 2019 •

edited

Loading

andyzhangx commented Jun 8, 2019 •

edited

Loading

andyzhangx commented Jun 11, 2019 •

edited

Loading