Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

glusterfs: retry without auto_unmount only when it's not supported #48709

Merged
merged 1 commit into from
Jul 12, 2017
Merged

glusterfs: retry without auto_unmount only when it's not supported #48709

merged 1 commit into from
Jul 12, 2017

Conversation

jsafrane
Copy link
Member

@jsafrane jsafrane commented Jul 10, 2017 •
edited
Loading

GlusterFS volume plugin should not blindly retry all failed mounts without
auto_unmount, it should retry them only when we are sure that auto_unmount
was the reason the first attempt failed.

@kubernetes/sig-storage-pr-reviews
/assign @humblec

Fixed GlusterFS volumes taking too long to time out

@jsafrane
glusterfs: retry without auto_unmount only when it's not supported
c20b4f7 
GlusterFS volume plugin should not blindly retry all failed mounts without
auto_unmount, it should retry them only when we are sure that auto_unmount
was the reason the first attempt failed.
@k8s-ci-robot
Copy link
Contributor

@jsafrane: GitHub didn't allow me to assign the following users: humblec.

Note that only kubernetes members can be assigned.

In response to this:

GlusterFS volume plugin should not blindly retry all failed mounts without
auto_unmount, it should retry them only when we are sure that auto_unmount
was the reason the first attempt failed.

@kubernetes/sig-storage-pr-reviews
/assign @humblec

NONE

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.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/storage Categorizes an issue or PR as relevant to SIG Storage. labels Jul 10, 2017
@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. labels Jul 10, 2017
msau42
msau42 reviewed Jul 11, 2017
View reviewed changes
pkg/volume/glusterfs/glusterfs.go Outdated
// Give a try without `auto_unmount` mount option, because
// it could be that gluster fuse client is older version and
// mount.glusterfs is unaware of `auto_unmount`.
autoMountOptions := make([]string, len(mountOptions))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable name is kind of misleading. Would something like optionsWithoutAutoMount make more sense?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed to noAutoMountOptions

@jsafrane jsafrane force-pushed the gluster-autumount branch 2 times, most recently from f8f9407 to c20b4f7 Compare July 11, 2017 09:48
@humblec
Copy link
Contributor

humblec commented Jul 11, 2017

@jsafrane Thanks!! LGTM.

@msau42
Copy link
Member

msau42 commented Jul 11, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 11, 2017
@jsafrane
Copy link
Member Author

/approve no-issue
/retest

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jsafrane, msau42

Associated issue requirement bypassed by: jsafrane

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 11, 2017
@fejta
Copy link
Contributor

fejta commented Jul 11, 2017

/test pull-kubernetes-e2e-kops-aws

1 similar comment
@humblec
Copy link
Contributor

humblec commented Jul 12, 2017

/test pull-kubernetes-e2e-kops-aws

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 48672, 47140, 48709, 48786, 48757)

@k8s-github-robot k8s-github-robot merged commit f5f9894 into kubernetes:master Jul 12, 2017
@humblec
Copy link
Contributor

humblec commented Jul 21, 2017

@jsafrane Can we cherry pick this patch to 1.7.1 ?

@jsafrane jsafrane mentioned this pull request Jul 21, 2017
Merged
@jsafrane jsafrane added release-note Denotes a PR that will be considered when it comes time to generate release notes. cherrypick-candidate and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jul 21, 2017
@jsafrane jsafrane modified the milestones: v1.8, v1.7 Jul 21, 2017
@jsafrane jsafrane mentioned this pull request Jul 21, 2017
Merged
@wojtek-t
Copy link
Member

cherrypick approved.

@wojtek-t wojtek-t added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Jul 24, 2017
k8s-github-robot pushed a commit that referenced this pull request Jul 24, 2017
Merge pull request #49381 from jsafrane/automated-cherry-pick-of-#487…
056009b 
…09-upstream-release-1.7

Automatic merge from submit-queue

Automated cherry pick of #48709

Cherry pick of #48709 on release-1.7.

#48709: glusterfs: retry without auto_unmount only when it's not
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.7" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

openshift-merge-robot added a commit to openshift/origin that referenced this pull request Jul 27, 2017
@openshift-merge-robot
Merge pull request #15396 from jsafrane/gluster-auto-unmount-3.6
a691774 
Automatic merge from submit-queue

Gluster timeout patches

There are several upstream patches that shorten gluster timeout when remote gluster servers are unavailable. User gets an error about failed mount in 2 minutes instead of 2 minutes * 2 * nr. of gluster endpoints.

kubernetes/kubernetes#42038
kubernetes/kubernetes#49127
kubernetes/kubernetes#48709
kubernetes/kubernetes#49475

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1473587
@jarrpa jarrpa mentioned this pull request Sep 1, 2017
Closed
@ghost
Copy link

ghost commented Sep 4, 2017 •
edited by ghost
Loading

Hi:

Please, could you confirm that this was fixed in 1.7.4 too? I have same errors in 1.7.4, glusterfs mount with auto_unmount always!!!!

  3m    3m      1       kubelet, s-smartc4-zprei                **Warning FailedMount**     MountVolume.SetUp failed for volume "pvc-89998771-8e37-11e7-9fa1-005056b77c78" : glusterfs: mount failed: mount failed: exit status 1
Mounting command: mount
Mounting arguments: 192.168.133.4:**vol_cd634264e40aa5a24bc21d8b73c3c787** /var/lib/kubelet/pods/9985b31a-8e37-11e7-9fa1-005056b77c78/volumes/kubernetes.io~glusterfs/pvc-89998771-8e37-11e7-9fa1-005056b77c78 glusterfs [auto_unmount log-level=ERROR log-file=/var/lib/kubelet/plugins/kubernetes.io/glusterfs/pvc-89998771-8e37-11e7-9fa1-005056b77c78/nginx-pod-glusterfs.log backup-volfile-servers=192.168.133.2:192.168.133.3:192.168.133.4]
**Output: /usr/bin/fusermount-glusterfs: mount failed: Invalid argument ```

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msau42 msau42 msau42 left review comments

Assignees

@deads2k deads2k

@msau42 msau42

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.7
Development

Successfully merging this pull request may close these issues.
9 participants
@jsafrane @k8s-ci-robot @humblec @msau42 @k8s-github-robot @fejta @wojtek-t @k8s-cherrypick-bot @deads2k