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

ScaleIO Volume Plugin - volume attribute updates #49973

Merged
merged 1 commit into from
Aug 22, 2017
Merged

ScaleIO Volume Plugin - volume attribute updates #49973

merged 1 commit into from
Aug 22, 2017

Conversation

vladimirvivien
Copy link
Member

@vladimirvivien vladimirvivien commented Aug 1, 2017 •
edited by jsafrane
Loading

This commit introduces the following updates and fixes:

  • Enable scaleIO volume multip-mapping based on accessMode
  • No longer uses "default" as default values for storagepool & protection domain
  • validates capacity when capacity is zero
  • Better naming for PV and volume
  • make mount ro when accessModes contains ROM

Special notes for your reviewer:

Fixes: #50794

Release note:

ScaleIO: fixed enforcement of fsGroup, enabled multiple-instance volume mapping, adjusted alignment of PVC, PV, and volume names for dynamic provisioning

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 1, 2017
@k8s-ci-robot
Copy link
Contributor

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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

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 the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 1, 2017
@k8s-github-robot k8s-github-robot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Aug 1, 2017
@vladimirvivien
Copy link
Member Author

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 2, 2017
@k8s-ci-robot
Copy link
Contributor

@vladimirvivien: you can't request testing unless you are a kubernetes member.

In response to this:

/ok-to-test

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.

@vladimirvivien
Copy link
Member Author

@jsafrane any help getting this cherry-picked is appreciated. Thanks.

@jsafrane
Copy link
Member

jsafrane commented Aug 3, 2017

@vladimirvivien, what bug(s) does it fix? Why is it so important it should be cherry picked? Btw, that's exactly the reason each fix should get its own PR.

@vladimirvivien
Copy link
Member Author

@jsafrane thanks for looking at this. This PR is not for bugs, but to introduce some enhancements and better admin experience. If cherry-pick is reserved for bugs, let me know.

@vladimirvivien
Copy link
Member Author

@wojtek-t
@jsafrane
I need to re-open this as it actually fix a bug that was brought to my attention. The bug is discussed here #50794. I would like to get this cherry-pick on the next 1.7 branch scheduled out. Let me know what I need to do to get this moving forward. Thank you.

@jsafrane
Copy link
Member

/ok-to-test

@vladimirvivien I updated the release note accordingly, please check.

@jsafrane
Copy link
Member

/approve
I'll lgtm the PR once the tests are green and the code looks OK.

I let @wojtek-t to judge if it's OK to have a cherry pick that has more code than a necessary bugfix. IMO it's harmless and it can break only ScaleIO volume plugin.

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 17, 2017
@vladimirvivien
ScaleIO Volume Plugin - volume attribute updates
02b377c 
This commit introduces the following updates and fixes:
- Enable scaleIO volume multip-mapping based on accessMode
- No longer uses "default" as default values for storagepool & protection domain
- validates capacity when capacity is zero
- Better naming for PV and volume
- make mount ro when accessModes contains ROM
@vladimirvivien
Copy link
Member Author

/test pull-kubernetes-unit

@vladimirvivien
Copy link
Member Author

@jsafrane build is green.

@jsafrane
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 21, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

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

Associated issue: 50794

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

@vladimirvivien
Copy link
Member Author

@jsafrane thank you.

@vladimirvivien
Copy link
Member Author

Hi @wojtek-t please let me know if I need to do anything else to get this to move forward. Thanks.

@wojtek-t wojtek-t added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Aug 21, 2017
@wojtek-t wojtek-t added this to the v1.7 milestone Aug 21, 2017
@vladimirvivien
Copy link
Member Author

@wojtek-t thank you!

@jsafrane
Copy link
Member

Build timed out
/retest

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

1 similar comment
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit a1e16ea into kubernetes:release-1.7 Aug 22, 2017
@vladimirvivien vladimirvivien mentioned this pull request Aug 22, 2017
Closed
18 tasks
@vladimirvivien vladimirvivien mentioned this pull request Oct 24, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jsafrane jsafrane

@derekwaynecarr derekwaynecarr

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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.7
Development

Successfully merging this pull request may close these issues.
7 participants
@vladimirvivien @k8s-ci-robot @jsafrane @k8s-github-robot @fejta-bot @derekwaynecarr @wojtek-t