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

Don't evict static pods #39059

Merged
merged 1 commit into from
Dec 23, 2016
Merged

Don't evict static pods #39059

merged 1 commit into from
Dec 23, 2016
+11 −0

Conversation

bprashanth
Copy link
Contributor

@bprashanth bprashanth commented Dec 21, 2016 •
edited by dchen1107
Loading

A follow up to #38914, the desired behavior is to:

  1. deprioritize critical pods
  2. never evict static pods

I don't think 1 needs a cherrypick if 2 goes in.

partial fix for ##38322

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 21, 2016
@k8s-reviewable
Copy link

This change is Reviewable

@bprashanth bprashanth mentioned this pull request Dec 21, 2016
Closed
@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-label-needed labels Dec 21, 2016
@bprashanth bprashanth added cherrypick-candidate release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Dec 21, 2016
@bprashanth bprashanth added this to the v1.5 milestone Dec 21, 2016
@bprashanth bprashanth mentioned this pull request Dec 22, 2016
Closed
@bprashanth
Copy link
Contributor Author

Filed #39124 and added a TODO

@bprashanth
Copy link
Contributor Author

@k8s-bot gci gke e2e test this

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GKE smoke e2e failed for commit 7a28d4b1a09129d5ca75b29b9df90b387a3e285d. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci 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 k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 22, 2016
ant-caichu
ant-caichu reviewed Dec 22, 2016
View reviewed changes
pkg/kubelet/eviction/eviction_manager.go
// --config directory.
// TODO(39124): This is a short term fix, we can't assume static pods
// are always well behaved.
glog.V(4).Infof("eviction manager: NOT evicting static pod %v", pod.Name)
Copy link

Choose a reason for hiding this comment

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

Is glog level is higher? i think 3 is ok.

ant-caichu
ant-caichu reviewed Dec 22, 2016
View reviewed changes
pkg/kubelet/eviction/eviction_manager.go
// --config directory.
// TODO(39124): This is a short term fix, we can't assume static pods
// are always well behaved.
glog.V(4).Infof("eviction manager: NOT evicting static pod %v", pod.Name)
Copy link

Choose a reason for hiding this comment

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

Is glog level is higher? i think 3 is ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I agree, I want this to show up in production logs

@k8s-ci-robot
Copy link
Contributor

Jenkins CRI GCE Node e2e failed for commit 18eba97ce854d50224ed359a67c2cc6dc892a39a. Full PR test history.

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

@dchen1107
Copy link
Member

Between #38914 and this pr, I prefer this one to patch both 1.4 and 1.5. We are going to fix the priority inversion issue in a proper way in 1.6.

LGTM

@dchen1107 dchen1107 added lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/P2 labels Dec 22, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins Bazel Build failed for commit 18eba97ce854d50224ed359a67c2cc6dc892a39a. Full PR test history.

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

@dchen1107 dchen1107 mentioned this pull request Dec 22, 2016
Merged
@dchen1107
Copy link
Member

@bprashanth bazel build failed, you need to run hack/verify-bazel.sh

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 23, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins GCE etcd3 e2e failed for commit 18eba97ce854d50224ed359a67c2cc6dc892a39a. Full PR test history.

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

@bprashanth bprashanth added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 23, 2016
@bprashanth
Copy link
Contributor Author

Dawn, re-applied lgtm after rebase

@k8s-github-robot
Copy link

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

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit d04fd1b into kubernetes:master Dec 23, 2016
@saad-ali saad-ali added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Jan 7, 2017
saad-ali added a commit that referenced this pull request Jan 10, 2017
@saad-ali
Merge pull request #39562 from dchen1107/automated-cherry-pick-of-#38…
2f67f8c 
…836-#39114-#39059-upstream-release-1.5

Automated cherry pick of #38836 #39114 #39059 upstream release 1.5
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.5" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

jessfraz added a commit that referenced this pull request Jan 11, 2017
@jessfraz
Merge pull request #39563 from dchen1107/automated-cherry-pick-of-#38…
64dcab3 
…836-#39114-#39059-upstream-release-1.4

Automated cherry pick of #38836 #39114 #39059 upstream release 1.4
@vishh vishh mentioned this pull request Jan 12, 2017
Closed
derekwaynecarr added a commit to derekwaynecarr/kubernetes that referenced this pull request Jan 17, 2017
@derekwaynecarr
Revert "Merge pull request kubernetes#39059 from bprashanth/static_ev…
02c1673 
…ict"

This reverts commit d04fd1b, reversing
changes made to ae4db79.
@derekwaynecarr derekwaynecarr mentioned this pull request Jan 17, 2017
Closed
@bprashanth bprashanth deleted the static_evict branch January 26, 2017 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ant-caichu ant-caichu ant-caichu left review comments

Assignees

@dchen1107 dchen1107

Labels
cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. 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. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.
8 participants
@bprashanth @k8s-reviewable @k8s-ci-robot @dchen1107 @k8s-github-robot @k8s-cherrypick-bot @ant-caichu @saad-ali