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

Update priority eviction docs #1162

Merged
merged 1 commit into from
Oct 6, 2017
Merged

Update priority eviction docs #1162

merged 1 commit into from
Oct 6, 2017

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Oct 6, 2017

After discussion with @dchen1107 and @bsalamat I think it would be simpler to start with a tiered eviction sorting rather than by using a function. See #846 (review) for some of the rationale.

This PR makes two changes:

  1. Changes the release at which changes take effect to 1.9 (since implementation missed 1.8)
  2. Changes the strategy from (usage > requests, func(priority, usage - requests)) to (usage > requests, priority, usage - requests)

cc @dchen1107 @derekwaynecarr @vishh

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 6, 2017
@dashpole dashpole assigned dchen1107 and bsalamat and unassigned thockin and wojtek-t Oct 6, 2017
@derekwaynecarr
Copy link
Member

/approve
/lgtm

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

Automatic merge from submit-queue.

@k8s-github-robot k8s-github-robot merged commit 9979d37 into kubernetes:master Oct 6, 2017
@dashpole dashpole deleted the priority_eviction branch October 6, 2017 20:06
@dchen1107
Copy link
Member

/lgtm

k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request Oct 13, 2017
Automatic merge from submit-queue (batch tested with PRs 51840, 53542, 53857, 53831, 53702). 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>.

Kubelet Evictions take Priority into account

Issue: #22212
This implements the eviction strategy documented here: kubernetes/community#1162, and discussed here: kubernetes/community#846.
When priority is not enabled, all pods are treated as equal priority.

This PR makes the following changes:

1. Changes the eviction ordering strategy to (usage < requests, priority, usage - requests)
2. Changes unit testing to account for this change in eviction strategy (including tests where priority is disabled).
3. Adds a node e2e test which tests the eviction ordering of pods with different priorities.

/assign @dchen1107 @vishh 
cc @bsalamat @derekwaynecarr 

```release-note
Kubelet evictions take pod priority into account
```
MadhavJivrajani pushed a commit to MadhavJivrajani/community that referenced this pull request Nov 30, 2021
Automatic merge from submit-queue.

Update priority eviction docs

After discussion with @dchen1107 and @bsalamat I think it would be simpler to start with a tiered eviction sorting rather than by using a function.  See kubernetes#846 (review) for some of the rationale.

This PR makes two changes:

1. Changes the release at which changes take effect to 1.9 (since implementation missed 1.8)
2. Changes the strategy from (usage > requests, func(priority, usage - requests)) to (usage > requests, priority, usage - requests)

cc @dchen1107 @derekwaynecarr @vishh
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants