autoscaler icon indicating copy to clipboard operation
autoscaler copied to clipboard

Implement AtomicScaleUp ProvisioningClass

Open yaroslava-serdiuk opened this issue 1 year ago • 6 comments

The Atomic-scale-up Provisioning Class aims to provision the resources required for the specified pods in an atomic way.

- [x] Update API to support AtomicScaleUp (the implementation of the method is a cloud provider responsibility, if the method is not implemented, fallback to IncreaseSize)
- [x] Implement ScaleUp for Provisioning Class
- [x] Update ProvisioningRequest Injector
- [x] Update ProvisioningRequest Processor
- [ ] Fix https://github.com/kubernetes/autoscaler/issues/6517
- [ ] Backport to 1.30

yaroslava-serdiuk avatar May 10 '24 11:05 yaroslava-serdiuk

@aleksandra-malinowska is already working on ScaleUp implementation. /assign @aleksandra-malinowska

yaroslava-serdiuk avatar May 10 '24 12:05 yaroslava-serdiuk

/cc @kisieland

aleksandra-malinowska avatar May 14 '24 10:05 aleksandra-malinowska

@yaroslava-serdiuk regarding the 'update ProvisioningRequest injector', I assume you meant adding the new class to supported classes list. But it doesn't seem like this list isn't used anywhere (we don't validate if the request is of the supported class in injector). Is this WAI?

I can see why we wouldn't validate class here - we should take into account the requests that were provisioned and will cause a workload to be admitted - but I'm not sure if this was a conscious decision. LMK if I should add validation or just clean up this var.

Relevant code: https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/processors/provreq/provisioning_request_injector.go#L41C5-L41C33

aleksandra-malinowska avatar May 27 '24 15:05 aleksandra-malinowska

I think supported classes list is actually a leftover. We validate provisioning class separately in ScaleUp loop, so it's ok to remove the const to me. I think this additional validation is not needed (and it's easy forget to update the list if we add another prov class).

we should take into account the requests that were provisioned

If ProvReq is failed or Provisioned we do not inject pods: https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/processors/provreq/provisioning_request_injector.go#L60

yaroslava-serdiuk avatar May 27 '24 18:05 yaroslava-serdiuk

/area cluster-autoscaler /area core-autoscaler

Shubham82 avatar Jun 03 '24 09:06 Shubham82

/assign

yaroslava-serdiuk avatar Jun 06 '24 11:06 yaroslava-serdiuk

  • need to be cherry picked (can't create cherry-pick at the moment): https://github.com/kubernetes/autoscaler/pull/6873, https://github.com/kubernetes/autoscaler/pull/6880, https://github.com/kubernetes/autoscaler/pull/6874

yaroslava-serdiuk avatar Jul 09 '24 11:07 yaroslava-serdiuk

cc: @x13n

yaroslava-serdiuk avatar Jul 09 '24 11:07 yaroslava-serdiuk