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.

Sign up for GitHub

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

Jump to bottom

kubelet: eviction: allow minimum reclaim as percentage #33392

Merged
merged 1 commit into from
Oct 6, 2016
Merged

kubelet: eviction: allow minimum reclaim as percentage #33392

merged 1 commit into from
Oct 6, 2016
+129 −40

Conversation

sjenning
Copy link
Contributor

@sjenning sjenning commented Sep 23, 2016 •
edited
Loading

Fixes #33354

xref #32537

Release note:

The kubelet --eviction-minimum-reclaim option can now take precentages as well as absolute values for resources quantities

@derekwaynecarr @vishh @mtaufen

This change is Reviewable

@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Sep 23, 2016
@sjenning sjenning changed the title kubelet: eviction: allow minreclaim as percentage kubelet: eviction: allow minimum reclaim as percentage Sep 23, 2016
@derekwaynecarr derekwaynecarr added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Sep 26, 2016
@derekwaynecarr derekwaynecarr added this to the v1.5 milestone Sep 26, 2016
derekwaynecarr
derekwaynecarr suggested changes Sep 26, 2016
View reviewed changes
Copy link
Member

@derekwaynecarr derekwaynecarr left a comment

Choose a reason for hiding this comment

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

A few things

  1. the kubelet -h text needs updating
  2. this should have a release-note
  3. it would be good if you can update kubelet-eviction.md design doc
  4. we will need a doc follow-on.

pkg/kubelet/eviction/helpers_test.go
}{
"BothMet": {
thresholds: specifiecThresholds,
enforceMinRelaim: false,
thresholds: specifiecThresholds,
Copy link
Member

Choose a reason for hiding this comment

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

typo: specificThresholds

@sjenning
Copy link
Contributor Author

@derekwaynecarr I pushed the typo fix and updated the PR description with a release note.

From a documentation perspective (-h, design doc, kube docs) I don't think anything needs to be updated. The soft and hard thresholds don't call out explicitly that they support absolute values and percentages either.

If anything the docs should have said that --eviction-minimum-reclaim didn't support percentages before. With this PR, they are all uniform, accepting both percentages and absolute values.

@derekwaynecarr
Copy link
Member

@sjenning - squash then lgtm, @ronnielai fyi

@sjenning
Copy link
Contributor Author

@derekwaynecarr rebased and squashed

@sjenning
Copy link
Contributor Author

flaked #33838

@derekwaynecarr
Copy link
Member

@k8s-bot unit test this

@derekwaynecarr
Copy link
Member

@k8s-bot gke e2e test this

@derekwaynecarr derekwaynecarr added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 30, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins unit/integration failed for commit f52dce9. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@mtaufen
Copy link
Contributor

mtaufen commented Oct 3, 2016

I think this is the fix for the TestNamespaceController unit test flake: #33866

@sjenning
Copy link
Contributor Author

sjenning commented Oct 6, 2016

@k8s-bot unit test this

@k8s-ci-robot
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit f52dce9. Full PR test history.

The magic incantation to run this job again is @k8s-bot gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 07eba4c into kubernetes:master Oct 6, 2016
@mtaufen
Copy link
Contributor

mtaufen commented Oct 6, 2016

Yay!

@sjenning sjenning deleted the min-reclaim-percent branch November 21, 2016 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derekwaynecarr derekwaynecarr derekwaynecarr approved these changes

Assignees

@derekwaynecarr derekwaynecarr

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.
6 participants
@sjenning @derekwaynecarr @k8s-ci-robot @mtaufen @k8s-github-robot @googlebot