fix: delegate on-demand pricing to truffle instead of fabricating a rate (#533) - #536
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…ate (#533) --cost-limit was enforced against a fabricated price, not the real on-demand rate. spawn's own hand-rolled AWS Pricing API lookup (LookupEC2OnDemandPrice) had no cache and, on failure — which it always did for a principal onboarded via `spawn onboard`, whose policy granted no pricing:* action — fell back to a static per-family-size guess table missing every current-gen instance family. Measured: g6e.xlarge guessed at $0.20/hr against a real $1.861/hr; p5.48xlarge guessed at $9.60/hr against a real $55.04/hr. Pricing at launch now delegates to truffle's OnDemandPriceWithSource, the suite's pricing authority (live Price List + 24h cache, static fallback that errors rather than guesses). A launch that requested --cost-limit and can't be priced at all now fails with a clear error instead of silently leaving the cap unenforced; without --cost-limit an unpriceable instance still launches with PricePerHour left unset. Also: - Add pricing:GetProducts to spawn onboard's IAM policy (required for the in-process truffle lookup to succeed for onboarded principals). - Bump github.com/spore-host/truffle v0.49.0 -> v0.53.0 (pricing.go is byte-identical between the two; no signature changes to the pricing or GetCapabilities APIs spawn calls). - cmd/status.go's worst-case-cost display now reads the spawn:price-per-hour tag instead of re-deriving a rate, so it can't drift from what --cost-limit is actually enforced against.
scttfrdmn
force-pushed
the
fix/533-delegate-pricing-to-truffle
branch
from
August 19, 2026 22:24
5847693 to
75007eb
Compare
lambda/alert-handler, lambda/autoscale-orchestrator, lambda/sweep-orchestrator, and lambda/ttl-reaper each pin their own go.mod (replace ../..) and had drifted out of sync with the root module's transitive AWS SDK versions from other merges landing on main, failing govulncheck/Test in CI with "updates to go.mod needed" without naming which module. Same nested-module trap as lagotto#43 and the earlier x/mod bump on this same PR's rebase.
6 tasks
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
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
--cost-limitwas enforced against a fabricated price, not the real on-demand rate. At launch, whenPricePerHourwasn't already known, spawn called its own hand-rolled, uncached AWS Pricing API lookup (LookupEC2OnDemandPrice) and, on failure — which it always did for any principal onboarded viaspawn onboard, since that policy granted nopricing:*action — fell back to a static per-family-size guess table (libs/pricing.GetEC2HourlyRate) missing every current-gen instance family (g6, g6e, p5, p5e, c7g, c8g, ...). Measured:g6e.xlargeguessed at $0.20/hr vs a real $1.861/hr (9.3x under);p5.48xlargeguessed at $9.60/hr vs a real $55.04/hr (5.7x under). That fabricated number is what gets written tospawn:price-per-hourand is what spored enforces--cost-limitagainst.OnDemandPriceWithSource(github.com/spore-host/truffle/pkg/aws) — the suite's pricing authority, with a real 24h cache and a static fallback that errors rather than guesses (truffle#114/test(agent): guard --on-complete fires regardless of job origin (#105) #115). spawn keeps no rate table of its own for enforcement anymore.--cost-limit, the launch now fails with an error naming the instance type, region, and limit — instead of silently leaving the cap unenforced. Without--cost-limit, an unpriceable instance still launches withPricePerHourleft unset (spored's cap-check already treats 0 as "no cap enforced," never fabricated).LookupEC2OnDemandPrice(dead code after the above).libs/pricing.GetEC2HourlyRateis intentionally left in place for the separate display-only estimate paths (--estimate-only's "On-demand:" line incmd/launch_single.go,spawn costincmd/service.go/pkg/cost) — that's a companion display bug tracked separately, out of scope here per the issue.pricing:GetProductstospawn onboard's IAM policy (cmd/onboard.go) — a global, read-only action with no resource scoping — since the pricing lookup runs in-process under the launching principal's own credentials via truffle. Operational note: anyone onboarded before this change needs to re-runspawn onboardto pick this up.github.com/spore-host/trufflev0.49.0 → v0.53.0. Diffedpkg/aws/pricing.gobetween the two versions — byte-identical, no signature changes toOnDemandPrice/OnDemandPriceWithSource/GetCapabilities. Skimmed truffle's CHANGELOG for v0.50.0–v0.53.0: additive fields (Capabilities), new discovery-path error-handling fixes,find/searchUX changes — nothing affecting the call sites spawn already uses (cmd/slurm.go,cmd/task.go,cmd/launch_single.go,cmd/launch_sweep_quota.go,cmd/launch_preflight.go).cmd/status.go's worst-case-cost display now reads thespawn:price-per-hourtag (via the existingpricePerHourhelper) instead of independently re-deriving a rate, so the displayed ceiling can't drift from what--cost-limitis actually enforced against.Notes on searches performed
pricing_test.go:69-70(unknown.xlarge→expected: 0.2). That test lives in the separategithub.com/spore-host/libsmodule'spkg/pricing/pricing_test.go(a different repo/module), not in spawn — spawn itself had nopricing_test.gobefore this PR. Nothing to delete in this repo;libs/pricing.GetEC2HourlyRateand its test are untouched, since that package is still legitimately used for the display-only estimate paths noted above.Test plan
pkg/aws/pricing_test.go(TestResolvePricePerHour_*) mock the truffle pricing call via a smallonDemandPricerinterface (no real AWS calls):PricePerHourwith the real rate--cost-limitrequested → error naming instance type/region/limit--cost-limit→ launch proceeds,PricePerHourstays 0PricePerHouris never overriddencfg.CostLimit > 0→cfg.CostLimit > 999999), confirmed 2 of the new tests failed, reverted, confirmed all pass.make checkclean (fmt, vet, staticcheck skip — not installed locally —, full short test suite across all packages).go build ./...andgo vet ./...clean after the truffle bump.