kubernetes-sigs / karpenter

Karpenter is a Kubernetes Node Autoscaler built for flexibility, performance, and simplicity.
Apache License 2.0
630 stars 205 forks source link

NodePool Limit by dollars #1215

Open justinkillen opened 6 months ago

justinkillen commented 6 months ago

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 https://github.com/kubernetes-sigs/karpenter/issues/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:

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.

Bryce-Soghigian commented 6 months ago

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 commented 6 months ago

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:

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 commented 6 months ago

Watch out for assumptions in the request, such as:

sftim commented 6 months ago

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

justinkillen commented 6 months ago

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 commented 6 months ago

/triage accepted