RemovePodsViolatingNodeTaints: list only pods that are not failed/suceeded

[ingvagabund]

Listing pods was incorrectly changed to listing all pods during code refactoring in 0ff8ecb#diff-494cd6260fda05699f68c7da810f7814dea797a006afee5380728a6433206fefL82.

[a7i]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a7i

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [a7i]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[a7i]

@ingvagabund thoughts on adding a unit test to avoid similar regression in the future?

[ingvagabund]

Field selectors are not properly implemented in the fake clientset, resp. informer built on top of a fake clientset. There would be no difference on the level of listing pods. You'd need to simulate the field selector filtering by injecting the right list of pods :(. Instead of relying on the functionality of the field selector to exclude the pods. Which is what is required here to properly test the code. The field selector filtering is kube-apiserver functionality.

[googs1025]

/lgtm

I will add a case where the pod is in failed success phase and the eviction count is 0 in the unit test of each called plugin.
After this PRs are merged, the only plugins that use the ListAllPodsOnANode method are: PodLifeTime RemoveFailedPods RemovePodsHavingTooManyRestarts plugins, because these plugins will filter internally or use the pod.Status.Phase status. 😄

[googs1025]

I will add a case where the pod is in failed success phase and the eviction count is 0 in the unit test of each called plugin.

Since I currently have limited bandwidth, I may have to wait for the weekend to work on this.