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

NodePool Limit by dollars #1215

Open
justinkillen opened this issue Apr 26, 2024 · 6 comments
Open

NodePool Limit by dollars #1215

justinkillen opened this issue Apr 26, 2024 · 6 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@justinkillen
Copy link

Description

What problem are you trying to solve?
Currently, I can set a limit on the number of vCPUs allocated. There are proposals to add additional dimensions, for example #732 which proposes a limit bases on node count.

Users typically have limits in place for a few reasons:

  1. They are trying to leverage pre-paid capacity (e.g. AWS Reserved instances, AWS Savings Plans, etc)
  2. They are trying to cap costs. This could be a single number for the entire cluster, or broken down e.g. "application X can use no more than 500 vCPUs"

1a) For Reserved Instances and EC2 Instance Savings Plans, the current implementation is okay. It requires the user to convert from counts/dollars to vCPUs which isn't the most friendly user experience, but it works.
1b) For Compute Savings Plans, the current implementation is unusable. Compute Savings Plans are based on dollars and can be applied to different instance types, which have different vCPU counts. Since there is not a single dollars to vCPU ratio, there is no effective way for a user to specify a NodePool with a vCPU limit.
2) For cost capping, a user ultimately cares about dollars, not CPUs. The vCPU limit is a proxy value to what they actually want to limit.

Proposal: NodePool should have a limit by cost. We already have pricing information, and we know what nodes are running, so we have all the information needed to determine the cost of all the instances in a NodePool, as well as the cost of any instances we might launch.

Revisiting the scenarios above, assuming a "limit by cost" was a feature:
1a) No change
1b) Users could set a limit by cost, and Compute Savings Plans could be utilized
2) Users could set a limit in dollars instead of via proxy values. This is closer to what most users want, and it would be less effort for them

In addition, costs should be added to status:

  • Node.Status should include a cost field that contains the cost of the node
  • NodePool.Status should include a cost field that contains a summation of costs of all provisioned nodes in this pool

How important is this feature to you?

Many users are using AWS Compute Savings Plans, and without this feature there is no effective way to utilize them.

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment
@justinkillen justinkillen added kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Apr 26, 2024
@Bryce-Soghigian
Copy link
Member

Bryce-Soghigian commented Apr 26, 2024

Seems related to global limits as well. A nodepool limit and a global limit would make sense here. Cloud providers for karpenter generally are aware of how much an instance costs, but there is some level of deviation from the actual price since they are cached.

Should we be concerned with users potentially using this as a contract? (We said don't spend more than 1000 dollars but karpenter spends 1020). Karpenter just leverages public cloud apis for pricing, and it's not as granular as I would like for a situation like this where customers are limiting a budget. We can get close, but the real price might be off. Keep in mind cloud pricing is dynamic, and we update the pricing cache every 12 or so hours.

For azure it takes 8 minutes to update the pricing cache because that api is slow. So we can't get real time pricing on demand unfortunately. There would be some level of error in the pricing calculation, which is tricky when you are using something to limit billing. The billing inside of azure is reported at a granularity our apis can't meet.

That being said maybe aws is different.

Node.Status should include a cost field that contains the cost of the node

This is interesting because we also are talking about having cost overrides per instance type. IF you have a special agreement for discounted compute, an instance that may traditionally be more expensive may become cheaper with that discount.

I was thinking rather than surfacing price on the node itself, we would have a crd for instanceTypeOverrides or something of that nature that would allow users or apis to set custom pricing overrides. This is still in design but very relevant to this request.

From what I can see there is a savingsplan sdk that the cloudprovider could use to do instance type overrides in that crd.

There are also pricing apis here for azure so it might make sense to have the cloudprovider implement additional pricing overrides themselves rather than have it flow into a crd.

@justinkillen
Copy link
Author

justinkillen commented Apr 26, 2024

I understand the concern about being exact, and I agree that there should be considerations for the long term. Perhaps though, it is too high of a goal for the immediate future and could be mitigated via careful naming and documentation? E.g. instead of "cost" the field could be "approximateCost" as to imply that it's not meant to be an exact number. It would still provide value to users, and wouldn't have the burden of having to be exact. If/when an exact method is found, it could use "cost" and "approximateCost" could be deprecated.

I was thinking rather than surfacing price on the node itself, we would have a crd for instanceTypeOverrides or something of that nature that would allow users or apis to set custom pricing overrides. This is still in design but very relevant to this request.

A CRD for InstanceTypesOverrides would help, but I don't think it would enable usage of Compute Savings Plans. As an example, say I have a CSP commitment of $1, and that m7i.large costs $0.09 providing 2 vCPUs and r7i.large costs $0.13 providing 2 vCPUs. The only option today is to specify a vCPU count, so I have to take some kind of average in the range of 11 m7i.large providing 22 vCPUs, or 7 r7i.large providing 14 vCPUs. In the two extremes:

  • 14 vCPUs of m7i.large (7 instances), utilizing $0.63 of the CSP - a significant underutilization
  • 22 vCPUs of r7i.large (11 instances), utilizing $1.43 of the CSP - a significant overutilization
    And this is just with 2 instance families. If we increase it to something more flexible like any c/m/r of 5/6/7 intel/amd it makes the potential variance even worse. The issue with solving this with an InstanceTypesOverrides would be that utilization of instance types are not independent i.e. a usage of m7i.large impacts the capacity of r7i.large.

From what I can see there is a savingsplan sdk that the cloudprovider could use to do instance type overrides in that crd.

This would work for EDPs, but not for Private Pricing. Those aren't exposed in APIs anywhere, so you'll definitely need a CRD.

@sftim
Copy link

sftim commented Apr 29, 2024

Watch out for assumptions in the request, such as:

  • bills are paid in dollars
  • bills are in a single currency [but: for any particular NodePool, this is reasonable to assume]
  • nodes are billed by [some small unit of time]
  • nodes are not prepaid
  • node prices are constant over the life of the node
  • costs are allocated at the node level (counterexample: being billed for time on the node, for a resource that each Pod uses on a node, for network traffic incurred, and for electricity use)
  • there is a single payer for the bill
  • all the nodes are in the cloud [but: for a particular NodePool that we care about, this is likely]

@sftim
Copy link

sftim commented Apr 29, 2024

If we propose API changes, that API should try to make as few or no assumptions that won't hold across different clusters.

@justinkillen
Copy link
Author

justinkillen commented Apr 29, 2024

bills are paid in dollars

Agreed. I meant dollars to be generic, but I see now that it is probably read as USD. I probably should have used "units of currency" or something like that.

bills are in a single currency [but: for any particular NodePool, this is reasonable to assume]

I think there's a big distinction between "the bill" (e.g. an invoice), and billable activity. For the purpose of a Node, I think we only need worry about the billable rate for the Node. That billable rate is likely available in multiple currencies, and if we assume a specific NodePool is using a specific currency, it should be fairly straight forward to use the Node rate in that currency. In particular, the currency of "the bill" doesn't really matter.

@jonathan-innis
Copy link
Member

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

5 participants