perf: Improve SimulateScheduling performance by reusing TopologyDomainGroups and InstanceTypes

[RafalSumislawski]

Description

Consolidation generally performs multiple consolidation simulations per cycle. Most significantly SingleNodeConsolidation performs one simulation per node. In my production clusters a call to SimulateScheduling takes ~900ms, meaning that SingleNodeConsolidation needs 18 minutes to scan 1000 nodes (of course there is a timeout before that, but that's not the point).

Below is an example trace of a SimulateScheduling call:

As you can see the scheduling simulation took 930ms, but very little of that computational cost belongs to the actual scheduling simulation. Most of it is the setup, which in principle could be done once and then reused across many simulations (with some challenges). In this PR I'm specifically tackling the getInstanceTypes (65ms) and buildDomainGroups (304ms) calls. The PR puts results of these two operations in a ShedulerFactory which then reuses the data across many SimulateScheduling within a single consolidation cycle.

This is an example SimulateScheduling trace after the change:

Both getInstanceTypes and buildDomainGroups are gone from the trace as they now happen outside of SimulateScheduling. The SimulateScheduling time is reduced from 930ms to 512ms. A 45% reduction.

I think there's more that could be done to reduce that time. But I'd prefere doing it in separate PRs rather than bloating this one.

Note on the test setup:
I run a fork of karpenter 1.9.0 with extra tracing, metrics and other stuff. So I've done the changes and the assessment on a fork of 1.9.0, and then rebased onto main. I haven't seen any differences between the two versions that would meaningfully affect performance of getInstanceTypes or buildDomainGroups, which made me conclude that main needs these improvements as much as 1.9.0 does.

Behaviour changes:
While this PR is meant as a pure performance optimisation, it still leads to subtle behaviour changes:

• Previously GetInstanceTypes was called before each simulation, meaning that within a single consolidation cycle different instance types could have been used by different simulations. The call is now done once for each consolidation method so all simulation that consolidation does use the same instance types info even if it changes at the source while the consolidation cycle is in progress. I'd say that's better, but it is a potential behaviour change.
• aside from the validation delay and the consolidateAfter, karpenter's consolidation doesn't have an explicit pacing mechanism. Computational throughput is the effective consolidation pace regulator. Making the computation faster will make karpenter run more consolidations per unit of time under certain scenarios. This is the whole point of this change, but it may be seen as a regression in cluster stability.

How was this change tested?

• It passes existing unit tests
• I'm running a fork containing this change in production

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

[kubernetes-prow]

Welcome @RafalSumislawski!

It looks like this is your first PR to kubernetes-sigs/karpenter 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/karpenter has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

Details

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