fix(quotas): CanLaunch's Spot path now tracks current Spot usage - #133
Merged
Conversation
QuotaInfo gains SpotUsage, populated by splitting getCurrentUsage's DescribeInstances scan by InstanceLifecycle (spot vs on-demand) instead of summing both lifecycles into one map. CanLaunch's Spot branch now computes real headroom (quota - SpotUsage) instead of only confirming a request fits the FULL quota with no usage signal, mirroring the on-demand branch's existing pattern. This also fixes a second, previously undiagnosed bug in the same code path: on-demand Usage included running Spot instances' vCPUs too (getCurrentUsage summed every running/pending instance regardless of lifecycle), so on-demand headroom was understated by however many vCPUs were running as Spot. Real-world case: an account with a 64-vCPU G/VT Spot quota already fully saturated by 8 running g7e.2xlarge instances got a false "fits" for 2 more shards, since 16 <= 64 (the full quota) in isolation. The actual RunInstances call then failed with MaxSpotInstanceCountExceeded with zero prior warning. Fixes #132.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
QuotaInfogainsSpotUsage, populated by splittinggetCurrentUsage'sDescribeInstancesscan byInstanceLifecycle(spot vs on-demand) instead of summing both into one map.CanLaunch's Spot branch now computes real headroom (quota - SpotUsage) instead of only confirming a request fits the full quota with zero usage signal — mirroring the on-demand branch's existingavailable := quota - usagepattern exactly.Usagepreviously included running Spot instances' vCPUs too (the oldgetCurrentUsagesummed every running/pending instance regardless of lifecycle), so on-demand headroom was understated by however many vCPUs were running as Spot. Splitting by lifecycle fixes both directions.Real-world case (from the issue): an account with a 64-vCPU G/VT Spot quota already fully saturated by 8 running
g7e.2xlargeinstances got a false "fits" for 2 more shards, since 16 ≤ 64 (the full quota) in isolation — the actualRunInstancescall then failed withMaxSpotInstanceCountExceededwith zero prior warning.Fixes #132.
Test plan
TestCanLaunch_SpotTracksCurrentUsagereproduces the exact calque scenario (quota fully saturated, partially used within headroom, partially used exceeding headroom, and the no-SpotUsage/nil-map fallback behaving as before)TestCanLaunchunchanged and passing.Usage(pkg/aws/obtainability.go'sOnDemandQuotaHeadroom) is unaffected/correctly improved — it was already documented as on-demand-only and now actually gets on-demand-only datago build ./...,go vet ./...,go test ./...(full repo) greenmake check-fmt,golangci-lint run(0 issues),make check-docs(no drift),./scripts/verify-pins.sh(15/15) all clean