Add documentation for disk based eviction #1196

derekwaynecarr · 2016-09-09T16:47:25Z

Documents new features in eviction.

This change is

derekwaynecarr · 2016-09-09T16:47:35Z

/cc @vishh @ronnielai

devin-donnelly · 2016-09-09T19:10:49Z

Review status: 0 of 1 files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

timstclair · 2016-09-09T22:39:46Z

docs/admin/out-of-resource.md


-In future releases, the `kubelet` will support the ability to trigger eviction decisions based on disk pressure.
+Each of the above signals support either a literal or percentage based value.  The percentage based value
+is calculated relative to the total capacity associated with each signal.


Does this hold for memory.available too?

yes. it holds for all the things.

timstclair · 2016-09-09T22:57:28Z

Review pass complete.

yujuhong · 2016-09-12T19:40:42Z

docs/admin/out-of-resource.md

+### How kubelet ranks pods for eviction in response to inode exhaustion
+
+At this time, it is not possible to know how many inodes were consumed by a particular container.  If the `kubelet` observes
+inode exhaustion, it will evict pods by ranking them by quality of service.  The following issue has been opened in cadvisor


How does it rank pods in the same QoS class?

this is addressed under "Known issues" section "How kubelet ranks pods for eviction in response to inode exhaustion", in the future, we want it to rank by consumption when we can know it.

to be clear, it does not differentiate within a class, so its random.

vishh · 2016-09-12T23:13:25Z

Review status: 0 of 1 files reviewed at latest revision, 12 unresolved discussions.

docs/admin/out-of-resource.md, line 40 [r1] (raw file):

Previously, timstclair (Tim St. Clair) wrote…

Does this hold for memory.available too?

My understanding is yes. Relevant code [here](https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/eviction/helpers.go#L183)

docs/admin/out-of-resource.md, line 49 [r1] (raw file):

`imagefs` is optional. `kubelet` auto-discovers these filesystems using cAdvisor.  `kubelet` does not care about any 
other filesystems. Any other types of configurations are not currently supported by the kubelet. For example, it is
*not OK* to store volumes and logs in a dedicated `imagefs`.

nit: dedicated filesystem instead of imagefs.

docs/admin/out-of-resource.md, line 52 [r1] (raw file):

Previously, yujuhong (Yu-Ju Hong) wrote…

Garbage collection (for both image and containers) is more aggressive than the disk eviction AFAIK. For example, it deletes containers associated with deleted pods, and it also ensure we kept no more than N containers per (pod, container) tuple. Is the plan to fold garbage collection into eviction?

Yes. The plan is to only expose evictions as the feature to end users. GC will be an internal implementation detail.

docs/admin/out-of-resource.md, line 72 [r1] (raw file):

Previously, timstclair (Tim St. Clair) wrote…

Where are these specified? I assume there's a flag / config for it somewhere? What are the defaults?

--eviction-hard is defaulted [here](https://github.com/kubernetes/kubernetes/blob/7523669699be96c49faddc3e7be83d2eae56360e/pkg/apis/componentconfig/v1alpha1/defaults.go#L331). In v1.5 disk evictions will also become opt-out in upstream. Meanwhile, k8s clusters on GCE/GKE already have disk evictions as opt-opt.

docs/admin/out-of-resource.md, line 181 [r1] (raw file):

The `kubelet` responds differently to disk based eviction thresholds if the machine has a
dedicatd `imagefs` configured for the container runtime.

nit: typo - dedicated

docs/admin/out-of-resource.md, line 223 [r1] (raw file):

Previously, timstclair (Tim St. Clair) wrote…

Is this specific to disk-based evictions?

Nope. +1 for moving to a separate section

docs/admin/out-of-resource.md, line 247 [r1] (raw file):

The default `eviction-minimum-reclaim` is `0` for all resources.

What about documentation on inodes based eviction behavior?

docs/admin/out-of-resource.md, line 328 [r1] (raw file):

Previously, yujuhong (Yu-Ju Hong) wrote…

I like simplifying the configuration, just one question. Will eviction only happen when the threshold is met? More aggressive, periodic cleanup, seems desirable IMO.

I assume you are referring to container GC. I wish we had this discussion on a k8s issue given that this feature is already in place. Container GC will eventually happen all the time once logs are pulled out of dead containers and kubelet can checkpoint outside of containers. If you were referring to image GC, there is really no need to GC images unless there is disk pressure. Image locality is helpful for pod startup latency.

docs/admin/out-of-resource.md, line 336 [r1] (raw file):

Previously, timstclair (Tim St. Clair) wrote…

(nit) I don't think this is a proposal anymore?

I don't think the rationale column is necessary here for the documentation.

docs/admin/out-of-resource.md, line 346 [r1] (raw file):

increases within that window rapidly, the `kubelet` may not observe `MemoryPressure` fast enough, and the `OOMKiller`
will still be invoked.  We intend to integrate with the `memcg` notification API in a future release to reduce this
latency, and instead have the kernel tell us when a threshold has been crossed immmediately.

It might be worth mentioning a workaround. For example, set eviction thresholds to <= 75% for example to reduce the occurrence of this issue.

Comments from Reviewable

devin-donnelly · 2016-09-15T21:32:40Z

@derekwaynecarr , have you had a chance to address the comments from @vishh and @timstclair yet?

derekwaynecarr · 2016-09-16T19:15:23Z

@devin-donnelly - I am addressing feedback now.

derekwaynecarr · 2016-09-16T19:25:07Z

docs/admin/out-of-resource.md, line 72 [r1] (raw file):

Previously, derekwaynecarr (Derek Carr) wrote…

they are specified as flags on the kubelet that are defined later in this doc.

this document does not define defaults.

defaults right now are specific to a commercial offering imo.

actually, we do default eviction on memory. let me update this.

Comments from Reviewable

derekwaynecarr · 2016-09-16T19:34:12Z

Review status: 0 of 1 files reviewed at latest revision, 12 unresolved discussions.

docs/admin/out-of-resource.md, line 49 [r1] (raw file):

Previously, vishh (Vish Kannan) wrote…

nit: dedicated filesystem instead of imagefs.

fixing

docs/admin/out-of-resource.md, line 247 [r1] (raw file):

Previously, vishh (Vish Kannan) wrote…

What about documentation on inodes based eviction behavior?

i give details under known issues.

docs/admin/out-of-resource.md, line 346 [r1] (raw file):

Previously, vishh (Vish Kannan) wrote…

It might be worth mentioning a workaround. For example, set eviction thresholds to <= 75% for example to reduce the occurrence of this issue.

added text.

Comments from Reviewable

derekwaynecarr · 2016-09-16T19:36:00Z

@vishh @timstclair -- all comments addressed. PTAL, apologies this is 30 min late.

derekwaynecarr · 2016-09-16T19:37:45Z

I am also confused by Reviewable... do you know if we are going to turn it off by default now with github reviews being a thing?

devin-donnelly · 2016-09-16T21:08:30Z

I would prefer to use GitHub's PR Review system from here on out; it's cleaner than Reviewable.

devin-donnelly · 2016-09-16T21:08:51Z

Thanks for getting to this, @derekwaynecarr .

timstclair · 2016-09-16T21:12:11Z

Review status: 0 of 1 files reviewed at latest revision, 12 unresolved discussions.

docs/admin/out-of-resource.md, line 223 [r1] (raw file):

Previously, derekwaynecarr (Derek Carr) wrote…

no, its supported for any resource under pressure, but it was new to 1.4 so got collapsed into this PR. the heading depth confuses this, so i updated it, thanks.

I think the heading depth might still be wrong? Raise `Ranking pods for eviction` and the 2 subsections 1 level?

Comments from Reviewable

vishh · 2016-09-17T00:01:07Z

LGTM

devin-donnelly · 2016-09-19T17:45:09Z

Review status: 0 of 1 files reviewed at latest revision, 12 unresolved discussions.

docs/admin/out-of-resource.md, line 223 [r1] (raw file):

Previously, timstclair (Tim St. Clair) wrote…

I think the heading depth might still be wrong? Raise Ranking pods for eviction and the 2 subsections 1 level?

I think @timstclair is right--should be ### Ranking pods for eviction and #### With Imagefs and #### Without Imagefs .

Comments from Reviewable

devin-donnelly · 2016-09-20T20:22:28Z

@derekwaynecarr , would you mind fixing this last comment in Reviewable about the header levels? Then I'll merge.

derekwaynecarr · 2016-09-21T20:40:22Z

@devin-donnelly -- restructured, headers should be good.

devin-donnelly · 2016-09-21T22:07:43Z

Looks good. Thanks! I'll merge it into 1.4.

* add emojivoto policy manifest This PR adds a new file under `run.linkerd.io` to have a `emojivoto` policy manifest, that is maintained and updated based on the changes from `emojivoto.yml` manifest here. This will make it possible to access the policy manifest from `run.linkerd.io/emojivoto-policy.yml` Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>

googlebot added the cla: yes label Sep 9, 2016

derekwaynecarr mentioned this pull request Sep 9, 2016

kubelet evicts resources when observing disk pressure kubernetes/enhancements#39

Closed

22 tasks

k8s-github-robot assigned timstclair Sep 9, 2016

timstclair added Needs Tech Review labels Sep 9, 2016

devin-donnelly added the Docs LGTM label Sep 9, 2016

devin-donnelly removed the Needs Docs Review label Sep 9, 2016

timstclair reviewed Sep 9, 2016
View reviewed changes

devin-donnelly added Tech Review: Open Issues and removed Needs Tech Review labels Sep 9, 2016

yujuhong reviewed Sep 12, 2016
View reviewed changes

devin-donnelly added this to the 1.4 milestone Sep 14, 2016

derekwaynecarr removed the Tech Review: Open Issues label Sep 16, 2016

vishh added the Tech Review LGTM label Sep 17, 2016

vishh self-assigned this Sep 17, 2016

devin-donnelly added Docs Review: Open Issues and removed Docs LGTM labels Sep 19, 2016

Add documentation for disk based eviction

e26aa12

devin-donnelly added Docs LGTM and removed Docs Review: Open Issues labels Sep 21, 2016

devin-donnelly merged commit f5e6c1d into kubernetes:release-1.4 Sep 21, 2016

sftim mentioned this pull request Feb 19, 2020

Typo - The example code doesn't match to the explantation. #19186

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add documentation for disk based eviction #1196

Add documentation for disk based eviction #1196

derekwaynecarr commented Sep 9, 2016 •

edited by thockin

Loading

derekwaynecarr commented Sep 9, 2016

devin-donnelly commented Sep 9, 2016

timstclair Sep 9, 2016

derekwaynecarr Sep 16, 2016

timstclair commented Sep 9, 2016

yujuhong Sep 12, 2016

derekwaynecarr Sep 16, 2016

derekwaynecarr Sep 16, 2016

vishh commented Sep 12, 2016

devin-donnelly commented Sep 15, 2016

derekwaynecarr commented Sep 16, 2016

derekwaynecarr commented Sep 16, 2016

derekwaynecarr commented Sep 16, 2016

derekwaynecarr commented Sep 16, 2016

derekwaynecarr commented Sep 16, 2016

devin-donnelly commented Sep 16, 2016

devin-donnelly commented Sep 16, 2016

timstclair commented Sep 16, 2016

vishh commented Sep 17, 2016

devin-donnelly commented Sep 19, 2016

devin-donnelly commented Sep 20, 2016

derekwaynecarr commented Sep 21, 2016

devin-donnelly commented Sep 21, 2016

Add documentation for disk based eviction #1196

Add documentation for disk based eviction #1196

Conversation

derekwaynecarr commented Sep 9, 2016 • edited by thockin Loading

derekwaynecarr commented Sep 9, 2016

devin-donnelly commented Sep 9, 2016

timstclair Sep 9, 2016

Choose a reason for hiding this comment

derekwaynecarr Sep 16, 2016

Choose a reason for hiding this comment

timstclair commented Sep 9, 2016

yujuhong Sep 12, 2016

Choose a reason for hiding this comment

derekwaynecarr Sep 16, 2016

Choose a reason for hiding this comment

derekwaynecarr Sep 16, 2016

Choose a reason for hiding this comment

vishh commented Sep 12, 2016

devin-donnelly commented Sep 15, 2016

derekwaynecarr commented Sep 16, 2016

derekwaynecarr commented Sep 16, 2016

derekwaynecarr commented Sep 16, 2016

derekwaynecarr commented Sep 16, 2016

derekwaynecarr commented Sep 16, 2016

devin-donnelly commented Sep 16, 2016

devin-donnelly commented Sep 16, 2016

timstclair commented Sep 16, 2016

vishh commented Sep 17, 2016

devin-donnelly commented Sep 19, 2016

devin-donnelly commented Sep 20, 2016

derekwaynecarr commented Sep 21, 2016

devin-donnelly commented Sep 21, 2016

derekwaynecarr commented Sep 9, 2016 •

edited by thockin

Loading