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

Always run the podGC controller. #35476

Merged
merged 1 commit into from
Oct 25, 2016

Conversation

foxish
Copy link
Contributor

@foxish foxish commented Oct 25, 2016

What this PR does / why we need it: The podGC controller has evolved to do more than just GC of terminated pods beyond a threshold number. It no longer makes sense to gate running it with the terminated-pod-gc-threshold flag. We still ensure that it only runs the terminatedPodsGC if the threshold specified in the argument to the controller manager is > 0.

Related discussion: #34160 (comment)

Release note:

The podGC controller will now always run, irrespective of the value supplied to the "terminated-pod-gc-threshold" flag supplied to the controller manager. 
The specific behavior of the podGC controller to clean up terminated pods is still governed by the flag, but the podGC's responsibilities have evolved beyond just cleaning up terminated pods.

This change is Reviewable

@foxish foxish added the release-note-none Denotes a PR that doesn't merit a release note. label Oct 25, 2016
@foxish foxish added this to the v1.5 milestone Oct 25, 2016
@k8s-github-robot k8s-github-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 25, 2016
@foxish
Copy link
Contributor Author

foxish commented Oct 25, 2016

/cc @erictune @davidopp @smarterclayton

@smarterclayton
Copy link
Contributor

LGTM but I would recommend adding a release note to document the change of behavior and why it will always run. Once you have that feel free to self apply.

@smarterclayton smarterclayton added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Oct 25, 2016
@gmarek gmarek added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 25, 2016
@foxish foxish added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Oct 25, 2016
@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 ff57f58 into kubernetes:master Oct 25, 2016
@foxish foxish deleted the always-run-pod-gc branch October 25, 2016 18:04
foxish added a commit to foxish/kubernetes that referenced this pull request Oct 25, 2016
This should be a NOP because we're just moving functionality
around and thanks to kubernetes#35476, the podGC controller should always
run anyway.
foxish added a commit to foxish/kubernetes that referenced this pull request Oct 27, 2016
This should be a NOP because we're just moving functionality
around and thanks to kubernetes#35476, the podGC controller should always
run anyway.
k8s-github-robot pushed a commit that referenced this pull request Oct 28, 2016
Automatic merge from submit-queue

Moving some force deletion logic from the NC into the PodGC

**What this PR does / why we need it**: Moves some pod force-deletion behavior into the PodGC, which is a better place for these.

This should be a NOP because we're just moving functionality
around and thanks to #35476, the podGC controller should always
run.

Related: #34160, #35145

cc @gmarek @kubernetes/sig-apps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/backlog Higher priority than priority/awaiting-more-evidence. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants