Improve LowNodeUtilization's consideration of evictable pods

[damemi]

Similar to how we check that a pod fits on other nodes before evicting in strategies like Affinity/TopologySpread, it would be an improvement if LowNodeUtilization checked that a pod from an overutilized node can fit on the intended underutilized node before evicting it (ie, check for taints/affinity). This could improve the performance of the strategy and prevent unnecessary infinite evictions each run.

[RyanDevlin]

I can try this one.

[RyanDevlin]

/assign

[RyanDevlin]

@damemi I've noticed that the Affinity and TopologySpread strategies check for affinity on other nodes, but don't necessarily check for taints. Is checking for taints on other nodes still something that should be included in this feature?

[rustrial]

@RyanDevlin I guess eviction only makes sense if the Pod can be scheduled on another node, therefore it seems reasonable that all scheduling constraints (Affinity, TopologySpread and Taints) should be checked. Otherwise we would just evict a Pod to see it being scheduled to the same node again, which just increases the risk of service outages caused by Pod eviction (restart).

[RyanDevlin]

@rustrial At the bottom of #551 @ingvagabund commented about how, for the purposes of this feature, it would be overkill to take into account every filtering plugin. I'm currently implementing this feature with checks for NodeAffinity, Taints, and NodeSelector. The optimization isn't perfect, but statistically it should improve performance.

[ingvagabund]

Overkill on the code level view (duplicating the code since we can't import kubernetes/kubernetes code).

[ingvagabund]

Otherwise we would just evict a Pod to see it being scheduled to the same node again, which just increases the risk of service outages caused by Pod eviction (restart).

That's responsibility of PDB to take care of disruptions.

[damemi]

@RyanDevlin I guess eviction only makes sense if the Pod can be scheduled on another node

I mentioned this in the other thread (#551 (comment)), sorry for not bringing it up here... but there are use cases where a pod could be evicted regardless of if it fits on another node. For strategies like PodLifetime, you might actually want to evict a pod that will only be recreated on the same node

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[ingvagabund]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[pravarag]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[ingvagabund]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[jecnua]

Still valid

[damemi]

Bumped the NodeFit PR to get that moving again
/remove-lifecycle stale

[ingvagabund]

Fixed in #790
/close

[k8s-ci-robot]

@ingvagabund: Closing this issue.

In response to this:

Fixed in #790
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.