Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement AtomicScaleUp ProvisioningClass #6815

Open
3 of 5 tasks
yaroslava-serdiuk opened this issue May 10, 2024 · 6 comments
Open
3 of 5 tasks

Implement AtomicScaleUp ProvisioningClass #6815

yaroslava-serdiuk opened this issue May 10, 2024 · 6 comments
Assignees
Labels
area/cluster-autoscaler area/core-autoscaler Denotes an issue that is related to the core autoscaler and is not specific to any provider. kind/feature Categorizes issue or PR as related to a new feature.

Comments

@yaroslava-serdiuk
Copy link
Contributor

yaroslava-serdiuk commented May 10, 2024

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

Tasks

@yaroslava-serdiuk
Copy link
Contributor Author

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

@aleksandra-malinowska
Copy link
Contributor

/cc @kisieland

@aleksandra-malinowska
Copy link
Contributor

aleksandra-malinowska commented May 27, 2024

@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

@yaroslava-serdiuk
Copy link
Contributor Author

yaroslava-serdiuk commented May 27, 2024

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

@Shubham82
Copy link
Contributor

/area cluster-autoscaler
/area core-autoscaler

@k8s-ci-robot k8s-ci-robot added area/cluster-autoscaler area/core-autoscaler Denotes an issue that is related to the core autoscaler and is not specific to any provider. labels Jun 3, 2024
@yaroslava-serdiuk
Copy link
Contributor Author

/assign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cluster-autoscaler area/core-autoscaler Denotes an issue that is related to the core autoscaler and is not specific to any provider. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants