[AWS] Batch CreateFleet calls when possible #740
Conversation
|
✔️ Deploy Preview for karpenter-docs-prod canceled. 🔨 Explore the source changes: d87b66f 🔍 Inspect the deploy log: https://app.netlify.com/sites/karpenter-docs-prod/deploys/61606d7addb0f10007d2a910 |
bwagner5
changed the title
| v1.LabelInstanceTypeStable: instance.Name(), | ||
| for i := 0; i < quantity; i++ { | ||
| name := strings.ToLower(randomdata.SillyName()) | ||
| // Pick first instance type option |
There was a problem hiding this comment.
Could you update this comment - it's pretty clear you're picking the first instance type option, but it's not clear why you're always picking the first one? (And actually is that a good idea, or is there a risk it over-simplifies the tests that might rely on this code?)
There was a problem hiding this comment.
I can remove the comments where it says pick first {instance, zone}. I'm not entirely sure the reason on picking 1 since this was existing code that I just wrapped with the for-loop. I don't think doing anything more complicated would make a difference to tests that depend on this code. If the real provider were used, it would be semi-random (not really, but allocation strategy would pick one on the Fleet backend).
There was a problem hiding this comment.
I'll go ahead and merge this PR (since it's not a new thing introduced to tests). We can evaluate if the tests should be picking a random instance type or maybe sequentially going up the slice and wrapping in the batch case to make the creation more deterministic for the tests, later on.
1. Issue, if available:
#732
2. Description of changes:
The AWS CloudProvider uses the CreateFleet API to provision EC2 Instances as k8s nodes. Before this PR, we were requesting 1 instance per CreateFleet call because we are passing in up to 20 possible instance types that EC2 Fleet could use to fulfill the request.
This PR batches packings that have the same constraints and same instance type options so that we can more efficiently make remote calls to EC2 Fleet and stay within API rate limits.
3. Does this change impact docs?
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.