Changing scheduling part to manage one single local storage resource #50819

NickrenREN · 2017-08-17T02:46:28Z

What this PR does / why we need it:
Finally decided to manage a single local storage resource

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): part of #50818

Special notes for your reviewer:
Since finally decided to manage a single local storage resource, remove overlay related code in scheduling part and change the name scratch to ephemeral storage.

Release note:

Changing scheduling part of the alpha feature 'LocalStorageCapacityIsolation' to manage one single local ephemeral storage resource

/assign @jingxu97
cc @ddysher

aveshagarwal · 2017-08-17T03:03:28Z

@kubernetes/sig-scheduling-pr-reviews

NickrenREN · 2017-08-17T03:18:46Z

jing's comment on quota pr

NickrenREN · 2017-08-22T07:19:35Z

/retest

jingxu97 · 2017-08-23T21:10:35Z

plugin/pkg/scheduler/algorithm/predicates/predicates_test.go

-			pod:           newResourcePod(schedulercache.Resource{MilliCPU: 1, Memory: 1, StorageOverlay: 10}),
-			emptyDirLimit: 15,
-			storageMedium: v1.StorageMediumDefault,
+			pod: newResourcePod(schedulercache.Resource{EphemeralStorage: 25}),


why remove some of the tests?

we manage a single resource, overlay and emptydir are not considered by predacates now, so remove them.

I think you can still keep most of the tests with some modification. Just change overlay to ephemeralstorage with some modification. This could also help us verify the changes related to emptyDir.

I have already changed overlay to ephemeral storage ,about the test case storage ephemeral local storage request exceeds allocatable
I added a fit test case about ephemeral storage, ptal, thanks

jingxu97 · 2017-08-24T17:42:16Z

cc @dashpole

dashpole

lgtm

NickrenREN · 2017-08-25T02:52:45Z

/retest

jingxu97 · 2017-08-25T06:00:11Z

/lgtm

NickrenREN · 2017-08-25T07:20:45Z

/assign @timothysc

NickrenREN · 2017-08-25T07:21:39Z

ping @timothysc for approval

timothysc · 2017-08-25T13:23:51Z

So this needs sign off from someone from @kubernetes/sig-storage-pr-reviews and @kubernetes/sig-node-pr-reviews to +1 that this is how they plan to reference and manage now, unless there is a spec that outlines these details.

NickrenREN · 2017-08-25T14:10:49Z

ping @jingxu97 @dashpole @vishh

jingxu97 · 2017-08-25T21:43:44Z

/approve

timothysc · 2017-08-25T21:50:43Z

So you are outright removing scratch in favor ephemeral. I'm ok with it so long as it's spelled out in a feature somewhere. But typical policy is to support both for 1 release cycle.

Also what do you do in the case of a 1/2 upgrade.

NickrenREN · 2017-08-25T21:55:27Z

local isolation is alpha feature introduced in 1.7
https://github.com/kubernetes/kubernetes/blob/master/pkg/features/kube_features.go#L110

timothysc · 2017-08-25T21:57:24Z

Could you please update your release note to include that you are removing alpha behavior in favor of the new mechanism?

/approve

k8s-github-robot · 2017-08-25T21:58:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: NickrenREN, jingxu97, timothysc

Associated issue: 50818

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

Needs approval from an approver in each of these OWNERS Files:

~~plugin/pkg/scheduler/OWNERS~~ [timothysc]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2017-08-26T01:52:55Z

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

k8s-github-robot · 2017-08-26T02:40:28Z

Automatic merge from submit-queue (batch tested with PRs 51235, 50819, 51274, 50972, 50504)

liggitt · 2017-09-06T05:30:44Z

plugin/pkg/scheduler/algorithm/predicates/predicates.go

-	// If the storage medium is memory, should exclude the size
-	for _, vol := range pod.Spec.Volumes {
-		if vol.EmptyDir != nil && vol.EmptyDir.Medium != v1.StorageMediumMemory {
-			result.StorageScratch += vol.EmptyDir.SizeLimit.Value()


does this mean the SizeLimit field is getting dropped?

No, one single local ephemeral storage can be set in container's resources,

containers: - name: fooa resources: requests: ephemeral-storage: “10Gi” limits: ephemeral-storage: “10Gi”

predicate functions take that into account and do not consider Emptydir any more.
btw: EmptyDir's sizelimit will be used by eviction manager

k8s-ci-robot assigned jingxu97 Aug 17, 2017

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

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Aug 17, 2017

NickrenREN changed the title ~~Local storage does not manage overlay any more~~ Remove overlay related code in scheduling part Aug 17, 2017

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 17, 2017

NickrenREN force-pushed the remove-overlay-scheduler branch from ef00f47 to 977804c Compare August 17, 2017 03:02

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 17, 2017

k8s-ci-robot added the sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. label Aug 17, 2017

NickrenREN force-pushed the remove-overlay-scheduler branch from 977804c to 13408f3 Compare August 22, 2017 04:59

NickrenREN force-pushed the remove-overlay-scheduler branch 2 times, most recently from 21b6992 to 67bafeb Compare August 22, 2017 06:21

NickrenREN changed the title ~~Remove overlay related code in scheduling part~~ Changing scheduling part to manage one single local storage resource Aug 23, 2017

jingxu97 reviewed Aug 23, 2017

View reviewed changes

NickrenREN force-pushed the remove-overlay-scheduler branch from 67bafeb to 761a455 Compare August 24, 2017 03:22

k8s-github-robot removed the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Aug 24, 2017

jingxu97 requested a review from dashpole August 24, 2017 17:41

dashpole approved these changes Aug 24, 2017

View reviewed changes

Local storage does not manage overlay any more

c95ecbc

NickrenREN force-pushed the remove-overlay-scheduler branch from 761a455 to c95ecbc Compare August 25, 2017 02:00

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

k8s-ci-robot assigned timothysc Aug 25, 2017

k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/node Categorizes an issue or PR as relevant to SIG Node. labels Aug 25, 2017

timothysc added this to the v1.8 milestone Aug 25, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 25, 2017

k8s-github-robot merged commit e923f2b into kubernetes:master Aug 26, 2017

NickrenREN deleted the remove-overlay-scheduler branch August 26, 2017 03:07

liggitt reviewed Sep 6, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changing scheduling part to manage one single local storage resource #50819

Changing scheduling part to manage one single local storage resource #50819

NickrenREN commented Aug 17, 2017 •

edited

Loading

aveshagarwal commented Aug 17, 2017

NickrenREN commented Aug 17, 2017

NickrenREN commented Aug 22, 2017

jingxu97 Aug 23, 2017

NickrenREN Aug 23, 2017 •

edited

Loading

jingxu97 Aug 24, 2017

NickrenREN Aug 25, 2017

jingxu97 commented Aug 24, 2017

dashpole left a comment

NickrenREN commented Aug 25, 2017

jingxu97 commented Aug 25, 2017

NickrenREN commented Aug 25, 2017

NickrenREN commented Aug 25, 2017

timothysc commented Aug 25, 2017

NickrenREN commented Aug 25, 2017

jingxu97 commented Aug 25, 2017

timothysc commented Aug 25, 2017 •

edited

Loading

NickrenREN commented Aug 25, 2017 •

edited

Loading

timothysc commented Aug 25, 2017 •

edited

Loading

k8s-github-robot commented Aug 25, 2017

k8s-github-robot commented Aug 26, 2017

k8s-github-robot commented Aug 26, 2017

liggitt Sep 6, 2017

NickrenREN Sep 6, 2017

Changing scheduling part to manage one single local storage resource #50819

Changing scheduling part to manage one single local storage resource #50819

Conversation

NickrenREN commented Aug 17, 2017 • edited Loading

aveshagarwal commented Aug 17, 2017

NickrenREN commented Aug 17, 2017

NickrenREN commented Aug 22, 2017

jingxu97 Aug 23, 2017

Choose a reason for hiding this comment

NickrenREN Aug 23, 2017 • edited Loading

Choose a reason for hiding this comment

jingxu97 Aug 24, 2017

Choose a reason for hiding this comment

NickrenREN Aug 25, 2017

Choose a reason for hiding this comment

jingxu97 commented Aug 24, 2017

dashpole left a comment

Choose a reason for hiding this comment

NickrenREN commented Aug 25, 2017

jingxu97 commented Aug 25, 2017

NickrenREN commented Aug 25, 2017

NickrenREN commented Aug 25, 2017

timothysc commented Aug 25, 2017

NickrenREN commented Aug 25, 2017

jingxu97 commented Aug 25, 2017

timothysc commented Aug 25, 2017 • edited Loading

NickrenREN commented Aug 25, 2017 • edited Loading

timothysc commented Aug 25, 2017 • edited Loading

k8s-github-robot commented Aug 25, 2017

k8s-github-robot commented Aug 26, 2017

k8s-github-robot commented Aug 26, 2017

liggitt Sep 6, 2017

Choose a reason for hiding this comment

NickrenREN Sep 6, 2017

Choose a reason for hiding this comment

NickrenREN commented Aug 17, 2017 •

edited

Loading

NickrenREN Aug 23, 2017 •

edited

Loading

timothysc commented Aug 25, 2017 •

edited

Loading

NickrenREN commented Aug 25, 2017 •

edited

Loading

timothysc commented Aug 25, 2017 •

edited

Loading