perf: respect multinode consolidation timeout in all cases

[rschalo]

Fixes #N/A

Description
Scheduling for 50 nodes in multinode consolidation can take a long time, especially in large clusters where a scheduling decision for a node can take 20 seconds or longer. This can cause multinode consolidation to block drift, emptiness, and single node consolidation for longer than intended.

How was this change tested?
Deployed with a 5 second timeout and saw multinode consolidation bail before exhausting the list of candidates.

disruption/multinodeconsolidation.go:83","message":"failed to find a multi-node consolidation after timeout, last considered batch had 8 candidates

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rschalo
Once this PR has been reviewed and has the lgtm label, please assign tzneal for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[coveralls]

Pull Request Test Coverage Report for Build 13532586135

Details

• 12 of 21 (57.14%) changed or added relevant lines in 3 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+0.08%) to 81.615%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/controllers/provisioning/provisioner.go	1	3	33.33%
pkg/controllers/disruption/multinodeconsolidation.go	8	11	72.73%
pkg/controllers/provisioning/scheduling/scheduler.go	3	7	42.86%

Files with Coverage Reduction	New Missed Lines	%
pkg/controllers/disruption/multinodeconsolidation.go	1	86.86%

Totals
Change from base Build 13509538416:	0.08%
Covered Lines:	9296
Relevant Lines:	11390

---

💛 - Coveralls

[tzneal]

Can we just check for if ctx.Err() != nil here, and does that make this a bit cleaner? (no select effectively and a smaller diff)

[tzneal]

I'm probably missing something, but what is this for? Shouldn't we normally fail scheduling if there is a pod error?

[rschalo]

This is probably unnecessary. Was using it to error on line 142 after calling SimulateScheduling in computeConsolidation but we do check if pods failed to schedule on line 146. We'd then return empty results, no command nil error, and continue the loop.

The loop then either breaks because min > max and returns the last saved items or the timeout bails out of consolidation and returns the last saved items plus an error if context deadline was exceeded.