Implemented batch processing for check capacity provisioning class

[Duke0404]

What type of PR is this?

/kind feature

What this PR does / why we need it:

Implements batch processing such that user can configure CA to process multiple CheckCapacity ProvisioningRequests in a single autoscaling iteration.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Allows CheckCapacity ProvisioningRequests to be processed in batch mode with configurable max batch size and batch timebox enabled by feature flags.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

cc: @yaroslava-serdiuk @aleksandra-malinowska @kawych

[k8s-ci-robot]

Hi @Duke0404. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Duke0404
Once this PR has been reviewed and has the lgtm label, please assign feiskyer for approval. For more information see the Kubernetes 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:

• cluster-autoscaler/OWNERS

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

[yaroslava-serdiuk]

/ok-to-test

[yaroslava-serdiuk]

please, fix tests

[Duke0404]

please, fix tests

I think the tests were failing due to an issue with Github actions, they are passing now.

[aleksandra-malinowska]

FYI @MaciekPytel @mwielgus the change we've been discussing a couple weeks ago.

[MaciekPytel]

I don't like the way that checkcapacity is implemented and I'm not super happy about doubling down on it. Checking capacity is not really similar to scale-up, but it is conceptually pretty much the same as fitting existing pods on upcoming nodes in FilterOutSchedulable. Both cases involve just binpacking on existing nodes and don't require using Estimator, Expander, etc to make scale-up decisions or all the logic related to actuating such decisions.

PodListProcessor interface has been pretty much designed exactly for this use-case, while scale-up orchestrator is intended to do more complex operations that you don't actually need here. The architectural / maintenance downside is inconsistency with the rest of the codebase and related maintenance problems (anyone debugging CA must be aware that provreq works differently from other, similar extensions to CA - our steep learning curve is likely a sum total of small gotchas like that).
In more practical terms - having it in a scale-up orchestrator means you need to use up an entire CA loop to check provreq capacity. If you moved it to a PodListProcessor you could checkcapacity and continue the loop to still do any necessary scale-up. In the type of batch cluster that I expect to be using provreq, loosing CA loops like that is very likely to translate to significant scale-up latency / throughput problem.

I'm not going to block this PR, but I'd really like to look into aligning provisioning request implementation with the expected CA architecture - and migrating the logic to PLP would be an obvious first step here.

[aleksandra-malinowska]

The architectural / maintenance downside is inconsistency with the rest of the codebase and related maintenance problems (anyone debugging CA must be aware that provreq works differently from other, similar extensions to CA - our steep learning curve is likely a sum total of small gotchas like that).

I agree with the general point about CA architecture. I would go even further and say we should probably explicitly group "capacity booking" (CRs, PRs, and possible future overprovisioning API) to make the implementation more consistent - while it'll be slightly different for each case (ex. all-or-nothing for PRs) they're logically very similar and if there's ever a substantial change to how the capacity needs to be booked for CA to recognize it, it'll have to be re-implemented everywhere.

In more practical terms - having it in a scale-up orchestrator means you need to use up an entire CA loop to check provreq capacity. If you moved it to a PodListProcessor you could checkcapacity and continue the loop to still do any necessary scale-up. In the type of batch cluster that I expect to be using provreq, loosing CA loops like that is very likely to translate to significant scale-up latency / throughput problem.

I think this argument essentially boils down to "this could be optimized further". With improvements to frequent loops logic (meaning we'll skip the scan interval), this change will still significantly improve performance compared to what there's now, even in large clusters. Yes, it's possible to improve it even more by skipping refreshing cluster state, but it isn't necessarily an argument against doing partial optimization now.

I'm not going to block this PR, but I'd really like to look into aligning provisioning request implementation with the expected CA architecture - and migrating the logic to PLP would be an obvious first step here.

I expect batch processing to remain an experimental feature for 1.31 (meaning it's turned off by default). I agree we may want to solve this before it becomes enabled by default.