Skip to content

feat: Added support for listing custom mountOptions#142

Closed
pentago wants to merge 1 commit intoopenebs-archive:developfrom
pentago:develop
Closed

feat: Added support for listing custom mountOptions#142
pentago wants to merge 1 commit intoopenebs-archive:developfrom
pentago:develop

Conversation

@pentago
Copy link
Copy Markdown

@pentago pentago commented Aug 15, 2022

Signed-off-by: pentago purplertiza@gmail.com

Pull Request template

It fixes mounting of a provisioned NFS server pod in situations where the following issue occurs:

Warning  FailedMount  1s (x3 over 3s)  kubelet   MountVolume.SetUp failed for volume "pvc-79743abd-d5c9-4939-a48d-d3038aa30d2a" : mount failed: exit status 255
Mounting command: mount
Mounting arguments: -t nfs 10.0.244.196:/ /var/lib/kubelet/pods/14af0af3-da2e-4276-92e6-eb39d0d57341/volumes/kubernetes.io~nfs/pvc-79743abd-d5c9-4939-a48d-d3038aa30d2a
Output: mount: mounting 10.0.244.196:/ on /var/lib/kubelet/pods/14af0af3-da2e-4276-92e6-eb39d0d57341/volumes/kubernetes.io~nfs/pvc-79743abd-d5c9-4939-a48d-d3038aa30d2a failed: Not supported

What this PR does?:
Enables user to add custom NFS mount options

Does this PR require any upgrade changes?:
No to my knowledge, maintainers please advise.

If the changes in this PR are manually verified, list down the scenarios covered::
Performed helm template to ensure proper output and tested deployment - worked fine.

Any additional information for your reviewer? :
No.

Checklist:

  • Fixes nfs mount failed Not supported #140
  • PR Title follows the convention of <type>(<scope>): <subject>
  • Has the change log section been updated?
  • Commit has unit tests
  • Commit has integration tests
  • (Optional) Does this PR change require updating NFS-Provisioner Chart? If yes, mention the Helm Chart PR #
  • (Optional) Are upgrade changes included in this PR? If not, mention the issue/PR to track:
  • (Optional) If documentation changes are required, which issue on https://github.com/openebs/openebs-docs is used to track them

@pawanpraka1 here's the new PR as I wasn't able to fix the previous one by force-pushing with the signoff message, hopefully, it gets merged soon.

Signed-off-by: pentago <purplertiza@gmail.com>
isDefaultClass: false
backendStorageClass: ""
mountOptions:
- vers=4.1
Copy link
Copy Markdown
Contributor

@dsharma-dc dsharma-dc Aug 23, 2023

Choose a reason for hiding this comment

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

Keeping a provision for using mountOptions is ok, but I'd say enforcing a particular default like vers 4.1 won't be desired. A user deploying nfs server for a performant workload may prefer vers 3 for example, or some clients(like rhel) by default start negotiating highest supported vers i.e. 4.2.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@pentago Please revert the changes done in values.yaml based on above comment.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@dsharma-dc This doesn't seem to move forward. Would you be open to a separate PR for this? I've released a patch version on my fork and it works perfectly. 18e41e0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@beschoenen I have created #164 to apply this change.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great, thanks!

@dsharma-dc
Copy link
Copy Markdown
Contributor

Closing this PR as the changes from this PR are picked up and applied under #164

@dsharma-dc dsharma-dc closed this Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nfs mount failed Not supported

4 participants