out of resource killing (memory) #21274

derekwaynecarr · 2016-02-15T18:10:15Z

Adds the core framework for low-resource killing in the kubelet.

Implements support for out of memory killing.

Related:
#18724

This change is

derekwaynecarr · 2016-03-23T17:00:53Z

pkg/kubelet/stability/manager_linux.go

+		return
+	}
+	glog.Infof("stability manager: kill pod %s", pod.Name)
+	err = podLifecycleManager.KillPod(pod)


recording a note here so when i get time to get back to this PR, but I will look to use kubelet.HandlePodDeletions

derekwaynecarr · 2016-04-04T20:22:23Z

pkg/kubelet/stability/manager_linux.go

+}
+
+// PodUsageInfo returns usage information for each container in a pod.
+func (r *resourceAccountantImpl) PodUsageInfo(pod *api.Pod) (api.ResourceList, error) {


Need to move this to use stats.PodStats

https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/server/stats/summary.go#L185

vishh · 2016-05-13T21:40:03Z

pkg/kubelet/eviction/helpers.go

+
+		// disk is best effort, so we don't measure relative to a request.
+		// TODO: add disk as a guaranteed resource
+		p1Disk := p1Usage[api.ResourceStorage]


Resource storage is pretty generic and weirdly it only applies to volumes as per API comments. Should local-disk be a first class resource? For the purposes of this PR, this can be a kubelet internal type too..

Yeah, I was unsure what resource to really use. Fine with that.

added internal type "disk".

vishh · 2016-05-13T22:56:30Z

I added a few comments. Overall this PR LGTM. @derekwaynecarr Thanks for coming up with super readable code :)

derekwaynecarr · 2016-05-14T15:36:13Z

@vishh - updated code per your comments.

/cc @kubernetes/rh-cluster-infra @kubernetes/sig-node - if anyone wants to take a pass at memory eviction.

vishh · 2016-05-17T00:20:05Z

@derekwaynecarr I hope at-least one of the memory eviction PRs (ideally the PR that adds flags) has a release-note flag.

vishh · 2016-05-17T00:22:13Z

@derekwaynecarr Next step is to add a node e2e to test all this logic. The test might need some changes to the framework - setting eviction flags.

derekwaynecarr · 2016-05-17T01:24:49Z

I have a follow on PR to enable the max pod eviction period and will have a release note for that PR that covers the feature.

derekwaynecarr · 2016-05-17T14:06:26Z

@k8s-bot test this issue #25652

k8s-bot · 2016-05-17T14:49:13Z

GCE e2e build/test passed for commit edc76f6.

k8s-github-robot · 2016-05-17T16:31:48Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-05-17T17:12:05Z

GCE e2e build/test passed for commit edc76f6.

k8s-github-robot · 2016-05-17T17:15:32Z

Automatic merge from submit-queue

smarterclayton · 2016-05-18T13:50:58Z

pkg/kubelet/eviction/types.go

+type signalObservations map[Signal]resource.Quantity
+
+// thresholdsObservedAt maps a threshold to a time that it was observed
+type thresholdsObservedAt map[Threshold]time.Time


I think this is broken, because you're assuming that resource.Quantity contributes to the uniqueness of the the threshold. It does not, because it's a pointer to an inf.Dec, which is only compared on pointer equality, not actual equality.

If you are depending on uniqueness of Quantity to signal value, you'll need to change your map keys.

Basically, map[Threshold1{Quantity: NewQuantity("1m")}] is not guaranteed to return the same value if you call it twice.

@derekwaynecarr

…to-scheduler Automatic merge from submit-queue Introduce node memory pressure condition to scheduler Following the work done by @derekwaynecarr at #21274, introducing memory pressure predicate for scheduler. Missing: * write down unit-test * test the implementation At the moment this is a heads up for further discussion how the new node's memory pressure condition should be handled in the generic scheduler. **Additional info** * Based on [1], only best effort pods are subject to filtering. * Based on [2], best effort pods are those pods "iff requests & limits are not specified for any resource across all containers". [1] https://github.com/derekwaynecarr/kubernetes/blob/542668cc7998fe0acb315a43731e1f45ecdcc85b/docs/proposals/kubelet-eviction.md#scheduler [2] #14943

@derekwaynecarr

…to-scheduler Automatic merge from submit-queue Introduce node memory pressure condition to scheduler Following the work done by @derekwaynecarr at kubernetes/kubernetes#21274, introducing memory pressure predicate for scheduler. Missing: * write down unit-test * test the implementation At the moment this is a heads up for further discussion how the new node's memory pressure condition should be handled in the generic scheduler. **Additional info** * Based on [1], only best effort pods are subject to filtering. * Based on [2], best effort pods are those pods "iff requests & limits are not specified for any resource across all containers". [1] https://github.com/derekwaynecarr/kubernetes/blob/542668cc7998fe0acb315a43731e1f45ecdcc85b/docs/proposals/kubelet-eviction.md#scheduler [2] kubernetes/kubernetes#14943

UPSTREAM: 69890: Run static pods before bootstrap Origin-commit: db65ebbf04b82a420e88b7f870a3ec667dd69998

googlebot added the cla: yes label Feb 15, 2016

derekwaynecarr force-pushed the kubelet_eviction branch from 837719b to ccb9ce6 Compare February 15, 2016 18:12

k8s-github-robot assigned brendandburns Feb 15, 2016

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 15, 2016

derekwaynecarr force-pushed the kubelet_eviction branch 3 times, most recently from b80c763 to 8bb4fed Compare February 15, 2016 22:33

brendandburns assigned dchen1107 and unassigned brendandburns Feb 17, 2016

k8s-github-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 20, 2016

derekwaynecarr reviewed Mar 23, 2016
View reviewed changes

k8s-github-robot added the release-note-label-needed label Mar 31, 2016

derekwaynecarr force-pushed the kubelet_eviction branch from 8bb4fed to 7bdf4bd Compare April 4, 2016 20:10

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

derekwaynecarr reviewed Apr 4, 2016
View reviewed changes

derekwaynecarr force-pushed the kubelet_eviction branch from 7bdf4bd to f5d3de7 Compare April 13, 2016 21:10

k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 13, 2016

derekwaynecarr force-pushed the kubelet_eviction branch 2 times, most recently from c03d24e to 9715e29 Compare April 18, 2016 21:42

derekwaynecarr mentioned this pull request Apr 19, 2016

Define interfaces for kubelet pod admission and eviction #24344

Merged

derekwaynecarr force-pushed the kubelet_eviction branch 2 times, most recently from 173ad77 to f482830 Compare April 25, 2016 17:00

vishh reviewed May 13, 2016
View reviewed changes

derekwaynecarr force-pushed the kubelet_eviction branch from 5d2922b to 97eeeb5 Compare May 14, 2016 15:33

out of resource killing (memory)

edc76f6

derekwaynecarr force-pushed the kubelet_eviction branch from 97eeeb5 to edc76f6 Compare May 14, 2016 15:34

derekwaynecarr added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels May 16, 2016

vishh added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 17, 2016

k8s-github-robot merged commit 7b4851b into kubernetes:master May 17, 2016

smarterclayton reviewed May 18, 2016
View reviewed changes

euank mentioned this pull request May 26, 2016

rkt: KillPod grace period override support #26370

Closed

dashpole mentioned this pull request Feb 22, 2017

Guaranteed admission for Critical Pods #40952

Merged

openshift-publish-robot pushed a commit to openshift/kubernetes that referenced this pull request Nov 5, 2018

Merge pull request kubernetes#21274 from smarterclayton/test_upstream

bb810aa

UPSTREAM: 69890: Run static pods before bootstrap Origin-commit: db65ebbf04b82a420e88b7f870a3ec667dd69998

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

out of resource killing (memory) #21274

out of resource killing (memory) #21274

derekwaynecarr commented Feb 15, 2016 •

edited

Loading

derekwaynecarr Mar 23, 2016

derekwaynecarr Apr 4, 2016

vishh May 13, 2016 •

edited

Loading

derekwaynecarr May 13, 2016

derekwaynecarr May 14, 2016

vishh commented May 13, 2016

derekwaynecarr commented May 14, 2016

vishh commented May 17, 2016

vishh commented May 17, 2016

derekwaynecarr commented May 17, 2016

derekwaynecarr commented May 17, 2016

k8s-bot commented May 17, 2016

k8s-github-robot commented May 17, 2016

k8s-bot commented May 17, 2016

k8s-github-robot commented May 17, 2016

smarterclayton May 18, 2016

smarterclayton May 18, 2016

out of resource killing (memory) #21274

out of resource killing (memory) #21274

Conversation

derekwaynecarr commented Feb 15, 2016 • edited Loading

derekwaynecarr Mar 23, 2016

Choose a reason for hiding this comment

derekwaynecarr Apr 4, 2016

Choose a reason for hiding this comment

vishh May 13, 2016 • edited Loading

Choose a reason for hiding this comment

derekwaynecarr May 13, 2016

Choose a reason for hiding this comment

derekwaynecarr May 14, 2016

Choose a reason for hiding this comment

vishh commented May 13, 2016

derekwaynecarr commented May 14, 2016

vishh commented May 17, 2016

vishh commented May 17, 2016

derekwaynecarr commented May 17, 2016

derekwaynecarr commented May 17, 2016

k8s-bot commented May 17, 2016

k8s-github-robot commented May 17, 2016

k8s-bot commented May 17, 2016

k8s-github-robot commented May 17, 2016

smarterclayton May 18, 2016

Choose a reason for hiding this comment

smarterclayton May 18, 2016

Choose a reason for hiding this comment

derekwaynecarr commented Feb 15, 2016 •

edited

Loading

vishh May 13, 2016 •

edited

Loading