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.

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

CreatePVC: correct setting of read/only access #249

Merged
merged 1 commit into from
Nov 3, 2016

Conversation

dustymabe
Copy link
Contributor

Fixes #237

@janetkuo janetkuo self-assigned this Oct 28, 2016
@@ -175,7 +175,7 @@ func CreatePVC(name string, mode string) *api.PersistentVolumeClaim {
}

if mode == "ro" {
pvc.Spec.AccessModes = []api.PersistentVolumeAccessMode{"ReadWriteOnce"}
pvc.Spec.AccessModes = []api.PersistentVolumeAccessMode{"ReadOnlyMany"}
Copy link
Member

Choose a reason for hiding this comment

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

nit: use api.ReadOnlyMany and api.ReadWriteMany instead

Copy link
Member

Choose a reason for hiding this comment

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

Update: I meant api.ReadWriteOnce (for the else case)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should be fixed now

@janetkuo
Copy link
Member

janetkuo commented Oct 28, 2016

Except for the nit this LGTM

Fixes kubernetes#237

Also update the code to use the constant values for api.ReadWriteOnce
and api.ReadOnlyMany.
@dustymabe
Copy link
Contributor Author

@janetkuo sorry about the delay. Have been away from the computer mostly since Friday.

@ngtuna ngtuna added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 2, 2016
@janetkuo janetkuo merged commit c9ed84f into kubernetes:master Nov 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. review needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants