Test Coverage for Deprovisioning

[njtran]

Tell us about your request

Multi node consolidation is a core deprovisioner in the deprovisioning controller, but only has 3 suite tests. I also made changes to the core logic and make presubmit didn't notice the change was made, which we should be catching.

Tests

Multi-Node Consolidation

• Consolidation of "n" (maybe 4) nodes that have pods scheduled and can be deleted onto another node
• Consolidation converging on the ideal number of machines to consolidate (say 25 machines) on a cluster with a larger number of node (say 50)

Single Node Consolidation

• Consolidation using TopologySpreadConstraint and "ScheduleAnyway" (Maintaining Preferences)
• Consolidation using PodAntiAffinity and "PreferredDuringSchedulingIgnoredDuringExecution" (Maintaining Preferences)

Metrics

• karpenter_deprovisioning_evaluation_duration Should be Fired During Consolidation
• karpenter_deprovisioning_machine_initialized_seconds Should be Fired During Deprovisoining Replacement
• karpenter_deprovisioning_actions_performed Should Be Fired correctly for each deprovisioner

Candidate Filtering and Generation (see https://github.com/aws/karpenter-core/blob/main/pkg/controllers/deprovisioning/helpers.go#L180)

• Should Filter on Initialization
• Should Filter on Provisioner, Capacity Type, Zone Label
• Should Filter on Nomination
• Should Filter on Instance Type Resolution
• Should Generate the Correct Candidate when pods are scheduled

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

[khareyash05]

I would like to add unit tests for the functions

[khareyash05]

we are talking about singlemachineconsolidation.go and multimachineconsolidation.go right? @jonathan-innis

[jonathan-innis]

Yes, the tests for these files lives inside of suite_test.go right now. There's handling for multi-node consolidation here and single-node consolidation is all over the file

[Bryce-Soghigian]

@khareyash05 are you still working on this?

[khareyash05]

@khareyash05 are you still working on this?

Please go ahead with this

[jonathan-innis]

/unassign @khareyash05

I'm going to unassign since it looks like there's been no progress on this since it was assigned.

[Sanskarzz]

/assign

[asmit27rai]

/assign