Skip to content
This repository has been archived by the owner on Oct 21, 2020. It is now read-only.

Propagate StorageClass MountOptions to PVs created by nfs-client-provisioner #835

Merged
merged 3 commits into from
Jul 9, 2018
Merged

Propagate StorageClass MountOptions to PVs created by nfs-client-provisioner #835

merged 3 commits into from
Jul 9, 2018

Conversation

pgagnon
Copy link
Contributor

@pgagnon pgagnon commented Jun 27, 2018

This PR will allow StorageClasses to propagate MountOptions to PVs provisioned by nfs-client-provisioner.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 27, 2018
Travis CI tests were failing due to alignment being incorrect in modified VolumeOptions block at line 1037.
@wongma7
Copy link
Contributor

wongma7 commented Jun 28, 2018

/lgtm
thanks for the pr. I may wait a bit to merge it, I want to add #828 on top of this so that when a user puts mount options and the provisioner doesn't know what to do with them the user gets an error instead of the mount options being silently ignored

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 28, 2018
@wongma7
Copy link
Contributor

wongma7 commented Jun 28, 2018

also the test error is unrelated

@brandom
Copy link

brandom commented Jul 3, 2018

@wongma7 Any chance this will be merged soon or is there a resource to help with build issues? I checked out @pgagnon's branch and tried to build but I get:

/go/src/github.com/kubernetes-incubator/external-storage/lib/controller/controller.go:853:53: cannot use claim (type *"github.com/kubernetes-incubator/external-storage/vendor/k8s.io/api/core/v1".PersistentVolumeClaim) as type *"github.com/kubernetes-incubator/external-storage/vendor/k8s.io/kubernetes/vendor/k8s.io/api/core/v1".PersistentVolumeClaim in argument to helper.GetPersistentVolumeClaimClass
/go/src/github.com/kubernetes-incubator/external-storage/lib/controller/controller.go:989:52: cannot use claim (type *"github.com/kubernetes-incubator/external-storage/vendor/k8s.io/api/core/v1".PersistentVolumeClaim) as type *"github.com/kubernetes-incubator/external-storage/vendor/k8s.io/kubernetes/vendor/k8s.io/api/core/v1".PersistentVolumeClaim in argument to helper.GetPersistentVolumeClaimClass

This is my first try at building a go project and I've read all the docs I could find. I installed the deps using glide install -v and then needed to do go get github.com/kubernetes-incubator/external-storage/lib/controller. It is possible I need a specific version of the controller, but I'm stuck.

@k8s-ci-robot k8s-ci-robot removed lgtm Indicates that a PR is ready to be merged. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 7, 2018
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


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.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 7, 2018
# Conflicts:
#	lib/controller/controller.go
@pgagnon
Copy link
Contributor Author

pgagnon commented Jul 7, 2018

@wongma7 I just merged your master branch into my fork in order to restore the ability to merge the PR automatically.

I had to do some history rewriting because my git client was configured with the wrong email address, so my apologies for the extra notifications you may have received.

@wongma7
Copy link
Contributor

wongma7 commented Jul 9, 2018

/lgtm
no problem! Let's just merge it, I still want to add some kind of interface to the Provisioners but it's taking me too long.

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 9, 2018
@wongma7 wongma7 merged commit 43c7070 into kubernetes-retired:master Jul 9, 2018
@pgagnon pgagnon deleted the nfsc-mount-options-propagation branch July 9, 2018 13:25
@pgagnon
Copy link
Contributor Author

pgagnon commented Jul 9, 2018

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/lib area/nfs-client cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants