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.
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
Update ScaleIO volume plugin default readOnly value #42854
Update ScaleIO volume plugin default readOnly value #42854
Conversation
Hi @vladimirvivien. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with 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. |
This PR is not for the master branch but does not have the |
@vladimirvivien redirect the PR against master branch |
@rootfs I am trying to get this in 1.6 do I still base against master ? What's the workflow to get changes before code freeze end ? |
Open a PR to master. Once merged and not in release-1.6, then request a cherry pick. |
@rootfs thanks for the info. |
@k8s-bot ok to test |
@vladimirvivien: you can't request testing unless you are a kubernetes member. In response to this comment:
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-bot ok to test |
@k8s-bot gce etcd3 e2e test this |
examples/volumes/scaleio/README.md
Outdated
| volumeName| the name of an existing volume in ScaleIO (required)| | ||
| secretRef:name| reference to a configuered Secret object (required, see Secret earlier)| | ||
| readOnly| specifies the access mode to the mounted volume (default `false`)| | ||
| fsType| the file system to use for the volume (default `xfs`)| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(also for rest for the patch) this is inconsistent with api doc https://github.com/kubernetes/kubernetes/blob/master/pkg/api/types.go#L1091.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rootfs Hi, I am not sure what you mean that desc for fsType
is inconsistent with the api doc? Are you saying I should copy what's in the API doc in the examples doc ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, please make sure the API doc is consistent with README. API doc states ext4 is the default while this patch switches to xfs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rootfs the doc and the api is sync'd
@rootfs can I get a LGTM on this PR so it can be cherry picked for 1.6 please ? Thank you. |
@vladimirvivien please address the comment and ping me back. |
This commit updates the code to set the default value of the readOnly attribute to false. It also updates the example docs to add full list of supported plugin attributes and doc.
@rootfs ready for another look for a LGTM. |
/lgtm |
[APPROVALNOTIFIER] This PR is NOT APPROVED The following people have approved this PR: rootfs, vladimirvivien Needs approval from an approver in each of these OWNERS Files: We suggest the following people: |
@vladimirvivien need @thockin approve to get it merged |
Minor isolated bug fix should be ok for 1.6 post-code freeze merge. |
Automatic merge from submit-queue (batch tested with PRs 42854, 43105, 43090) |
This commit updates the code to set readOnly attribute to be set to false.
What this PR does / why we need it:
This PR is a minor fix that updates the default value of
readOnly
attribute tofalse
.Release note: