-
Notifications
You must be signed in to change notification settings - Fork 40.7k
Prioritize deleting the non-running pods when reducing replicas #6992
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
Conversation
// Unassigned < assigned | ||
if s[i].Spec.Host == "" && s[j].Spec.Host != "" { | ||
return true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. You could also check readiness (not ready < ready). The endpoints controller check could be factored out into a function:
https://github.com/GoogleCloudPlatform/kubernetes/blob/e1b76b922b3f7aec73715cc53cb6f8e9180e5c26/pkg/service/endpoints_controller.go#L99
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Factored out the pod ready logic to api.IsPodReady. Added this new condition to pod sorting.
3fa44ac
to
dd4527a
Compare
Comments addressed in the new patch. Thanks for the suggestion! |
|
||
func (s activePods) Less(i, j int) bool { | ||
// Not ready < ready | ||
if !api.IsPodReady(&s[i]) && api.IsPodReady(&s[j]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would order the checks as follows:
- host
- phase
- readiness
but it shouldn't make a lot of difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
LGTM |
This changes instructs the replication controller to delete replicas in the order of "unscheduled (pending)", "scheduled (pending)", and "scheduled (running)" pods. This is less disruptive than deleting random pods.
Addressed the comments and rebased. |
Thanks. LGTM. |
Prioritize deleting the non-running pods when reducing replicas
This changes instructs the replication controller to delete replicas in the
order of "unscheduled (pending)", "scheduled (pending)", and "scheduled
(running)" pods. This is less disruptive than deleting random pods.
This fixes #3948.