kubernetes-sigs / descheduler

Descheduler for Kubernetes
https://sigs.k8s.io/descheduler
Apache License 2.0
4.23k stars 645 forks source link

Inter-pod anti-affinity check for `NodeFit` should not check if Pods are on the same Node already #1370

Closed nikimanoledaki closed 3 weeks ago

nikimanoledaki commented 3 months ago

What version of descheduler are you using?

descheduler version: master - not released yet, but would be introduced in 0.30.0

What did you do? What did you expect to see? What did you see instead?

Folow-up to https://github.com/kubernetes-sigs/descheduler/pull/1356

We've been testing the recent addition to NodeFit on our environment to check for inter-pod anti-affinity. I found a bug, which means the feature is not currently doing what it's supposed to be doing. It's not creating any major issues or consequences - just not doing what it should be doing!

It should be assessing whether a Pod fits on another Node based on whether another Pod on this other Node has matching antiaffinities, but it returns early, due to the following issue.

Here, we evaluate whether both Pods are present on the Node. It only makes sense to check this for the plugin where this function originated from, RemovePodsViolatingInterPodAntiAffinity. That plugin checks if both Pods are on the same Node and whether to evict one of them, if they have matching antiaffinities. It does not make sense to do this when we’re evaluating if a Pod could fit on another Node as part of NodeFit.

I can open a PR to fix this as soon as possible on Tuesday next week unless someone else gets to it first. Hopefully we can add this to the descheduler 0.30.0 before it's released.

nikimanoledaki commented 3 months ago

/assign

nikimanoledaki commented 3 weeks ago

After staging this change, I can confirm that #1395 addresses the remaining bug with the descheduler's evaluation of inter-pod anti-affinity policies πŸ‘ πŸŽ‰ Thank you @fanhaouu @ingvagabund for making Kubernetes a little bit (even) more awesome! πŸ˜„