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

Volume topology aware dynamic provisioning: work based on new API #63193

Merged
merged 2 commits into from
Jun 5, 2018
Merged

Volume topology aware dynamic provisioning: work based on new API #63193

merged 2 commits into from
Jun 5, 2018

Conversation

lichuqiang
Copy link
Contributor

@lichuqiang lichuqiang commented Apr 26, 2018

What this PR does / why we need it:

The PR has been split to 3 parts:

Part1: #63232 for basic scheduler and PV controller plumbing
Part2: #63233 for API change

and the PR itself includes work based on the API change:

  • Dynamic provisioning allowed topologies scheduler work
  • Update provisioning interface to be aware of selected node and topology

Which issue(s) this PR fixes
Feature: kubernetes/enhancements#561
Design: kubernetes/community#2168

Special notes for your reviewer:
/sig storage
/sig scheduling
/assign @msau42 @jsafrane @saad-ali @bsalamat

@kubernetes/sig-storage-pr-reviews
@kubernetes/sig-scheduling-pr-reviews

Release note:

Volume topology aware dynamic provisioning

@k8s-ci-robot k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Apr 26, 2018
@k8s-ci-robot k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 26, 2018
@k8s-github-robot k8s-github-robot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Apr 26, 2018
requested := claim.Spec.Resources.Requests[v1.ResourceStorage]
minVolumeSize := class.Status.Capacity.MinVolumeSize

if !isMinVolumeSizeSatisfied(requested, minVolumeSize) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I misunderstood this part of MinVolumeSize in design, and thought we need to reject it if a user requested a volume under the minimum size.

@msau42
Copy link
Member

msau42 commented Apr 27, 2018

Can you split this PR into 3 PRs, since the API part is still under discussion:

  1. API changes
  2. Basic scheduler and PV controller plumbing
  3. Scheduler work based on the new api

@lichuqiang
Copy link
Contributor Author

lichuqiang commented Apr 27, 2018

@msau42 Sure thing, but the third pr would rely on the above two (in fact this existing PR should be the third one I think), I'll open another two PRs for api change and basic provisioning update

@lichuqiang
Copy link
Contributor Author

/kind feature

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 5, 2018
@lichuqiang
Copy link
Contributor Author

/priority important-soon

@k8s-ci-robot k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Jun 5, 2018
li-ang pushed a commit to li-ang/kubernetes that referenced this pull request Jun 5, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

API change for volume topology aware dynamic provisioning

**What this PR does / why we need it**:

Split PR kubernetes#63193 for better review
part 2: API change

Previous: kubernetes#63232
Next: kubernetes#63193

**Which issue(s) this PR fixes** 
Feature: kubernetes/enhancements#561
Design: kubernetes/community#2168

**Special notes for your reviewer**:
/sig storage
/sig scheduling
/assign @msau42 @jsafrane @thockin 


**Release note**:

```release-note
API change for volume topology aware dynamic provisioning
```
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 5, 2018
@k8s-github-robot k8s-github-robot removed the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Jun 5, 2018
@lichuqiang
Copy link
Contributor Author

After the former PR merged, I still can see the API related commits in this PR.
Not sure whether it matters, and I did another rebase to ensure it's in the right state.

k8s-publishing-bot added a commit to kubernetes/api that referenced this pull request Jun 5, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

API change for volume topology aware dynamic provisioning

**What this PR does / why we need it**:

Split PR kubernetes/kubernetes#63193 for better review
part 2: API change

Previous: kubernetes/kubernetes#63232
Next: kubernetes/kubernetes#63193

**Which issue(s) this PR fixes**
Feature: kubernetes/enhancements#561
Design: kubernetes/community#2168

**Special notes for your reviewer**:
/sig storage
/sig scheduling
/assign @msau42 @jsafrane @thockin

**Release note**:

```release-note
API change for volume topology aware dynamic provisioning
```

Kubernetes-commit: 77d996b27883bed9d8b9015b608f9a0f0c60fd23
@dims
Copy link
Member

dims commented Jun 5, 2018

/assign @lavalamp

@saad-ali
Copy link
Member

saad-ali commented Jun 5, 2018

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 5, 2018
@lavalamp
Copy link
Member

lavalamp commented Jun 5, 2018

/approve

For kube-controller-manager change (I didn't look at anything else).

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lavalamp, lichuqiang, msau42, saad-ali

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 5, 2018
@dims
Copy link
Member

dims commented Jun 5, 2018

/test pull-kubernetes-e2e-gce

@dims
Copy link
Member

dims commented Jun 5, 2018

/priority critical-urgent
/remove-priority important-soon

Bumping priority so the bot does not drop this when we get into code freeze.

@k8s-ci-robot k8s-ci-robot added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. and removed priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Jun 5, 2018
@k8s-github-robot
Copy link

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@bsalamat @jsafrane @lavalamp @lichuqiang @msau42 @saad-ali

Pull Request Labels
  • sig/scheduling sig/storage: Pull Request will be escalated to these SIGs if needed.
  • priority/critical-urgent: Never automatically move pull request out of a release milestone; continually escalate to contributor and SIG through all available channels.
  • kind/feature: New functionality.
Help

@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. If you want to cherry-pick this change to another branch, please follow the instructions here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants