From d9b645eb0d40a9242ade001275a928546a795744 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 3 Jul 2026 15:26:50 +0200 Subject: [PATCH 1/8] feat(ladder): build staggered tranches from allocations via ramp schedule Split each Allocation into a buy-now purchase for the immediate ramp step and future Tranche rows per delayed step, so commitment terms expire staggered. Step amounts use exact big.Rat arithmetic with a last-step remainder and an overshoot clamp, so totals always reconstruct the gap exactly. Tranche rows are fully self-describing (Layer, Term, PaymentOption, amount) and clock plus ID generation are injected. Part of #1334. --- pkg/ladder/ladder.go | 280 ++++++++++++ pkg/ladder/ladder_test.go | 864 ++++++++++++++++++++++++++++++++++++++ pkg/ladder/store.go | 89 +++- pkg/ladder/store_test.go | 64 ++- 4 files changed, 1283 insertions(+), 14 deletions(-) create mode 100644 pkg/ladder/ladder.go create mode 100644 pkg/ladder/ladder_test.go diff --git a/pkg/ladder/ladder.go b/pkg/ladder/ladder.go new file mode 100644 index 000000000..889ccfea7 --- /dev/null +++ b/pkg/ladder/ladder.go @@ -0,0 +1,280 @@ +package ladder + +import ( + "fmt" + "math/big" + "time" +) + +// TrancheInput carries everything BuildTranches needs to turn a set of +// Allocations into immediate purchase actions and future ramp tranches. All +// time and ID generation is injected so BuildTranches is deterministic and +// free of hidden side effects. +type TrancheInput struct { + // Config provides the ramp schedule and is re-validated by BuildTranches. + Config *LadderConfig + // RunID stamps every produced Tranche row for run linkage. Required. + RunID string + // Term is the commitment term (e.g. "1yr", "3yr") passed to each buy-now + // purchase action. The caller selects the term; v1 uses a single term for + // all allocations. + Term string + // PaymentOption is the payment structure (e.g. "no-upfront") passed to + // each buy-now purchase action. + PaymentOption string + // NewID is called once per produced Tranche to assign a unique identifier. + // Callers may inject a UUID function, a sequential counter, or any other + // scheme. Must return a non-empty string on every call. + NewID func() string + // Now is the wall-clock time of the run. BuildTranches never calls + // time.Now internally; callers inject the clock for testability and + // audit-trail accuracy. + Now time.Time + // Allocations is the full set of per-layer sizing decisions from Allocate. + // Each allocation is split across all ramp steps in the schedule. + Allocations []Allocation +} + +// TrancheResult is the output of BuildTranches. +// +// When the ramp schedule contains no step with AfterDays == 0 (a fully- +// delayed ramp), BuyNow will be empty and all commitment activity appears as +// future Tranches. This is a valid configuration; callers should document a +// fully-delayed ramp explicitly so operators are not surprised by the absence +// of an immediate purchase. +type TrancheResult struct { + // BuyNow holds ActionPurchase actions for the immediate (AfterDays == 0) + // ramp step, if one exists in the schedule. + BuyNow []PlannedAction + // Tranches holds future scheduled ramp rows for every step with + // AfterDays > 0. Each row has status TrancheStatusScheduled and a + // FireAfter timestamp set to Now + AfterDays. + Tranches []Tranche +} + +// BuildTranches turns each Allocation into (a) buy-now PlannedActions for the +// ramp step with AfterDays == 0, if present, and (b) future Tranche rows for +// every ramp step with AfterDays > 0, staggered over the ramp schedule so +// commitment terms expire at different times instead of bunching. +// +// Step amounts are computed in exact big.Rat arithmetic. Every step amount is +// clamped to the remaining unallocated gap and the last step receives the +// exact leftover, so the total across all produced items reconstructs the gap +// exactly -- no cent lost, duplicated, or over-allocated -- for every schedule +// that passes RampSchedule.Validate, despite the binary-float representation +// of step fractions. Steps whose computed amount is zero (possible when the +// clamp floors a step after earlier fractions consumed the whole gap) are +// skipped entirely without affecting total exactness. +// +// BuildTranches performs no I/O and never calls time.Now. +func BuildTranches(in *TrancheInput) (*TrancheResult, error) { + if err := validateTrancheInput(in); err != nil { + return nil, err + } + return buildTrancheResult(in) +} + +// validateTrancheInput checks all required fields on TrancheInput. Returns a +// descriptive error naming the offending field on any violation. +func validateTrancheInput(in *TrancheInput) error { + if in == nil { + return fmt.Errorf("tranche input must not be nil") + } + if in.Config == nil { + return fmt.Errorf("config must not be nil") + } + if err := in.Config.Validate(); err != nil { + return fmt.Errorf("config: %w", err) + } + if in.RunID == "" { + return fmt.Errorf("run_id is required") + } + if in.Now.IsZero() { + return fmt.Errorf("now must not be zero (inject the run wall-clock time)") + } + if in.NewID == nil { + return fmt.Errorf("new_id must not be nil (inject an ID generator)") + } + if in.Term == "" { + return fmt.Errorf("term is required (e.g. \"1yr\" or \"3yr\")") + } + if in.PaymentOption == "" { + return fmt.Errorf("payment_option is required (e.g. \"no-upfront\")") + } + return validateInputAllocations(in.Allocations) +} + +// validateInputAllocations checks each allocation for a recognized layer and +// a positive gap. An empty allocation slice is valid (BuildTranches returns an +// empty result) but each element must be well-formed. +func validateInputAllocations(allocs []Allocation) error { + for i, a := range allocs { + if err := a.Layer.Validate(); err != nil { + return fmt.Errorf("allocation[%d]: layer: %w", i, err) + } + if a.GapUSDPerHour == nil || a.GapUSDPerHour.Sign() <= 0 { + return fmt.Errorf("allocation[%d]: gap_usd_per_hour must be positive", i) + } + } + return nil +} + +// buildTrancheResult iterates over all allocations and splits each one across +// the configured ramp schedule steps. +func buildTrancheResult(in *TrancheInput) (*TrancheResult, error) { + steps := in.Config.Ramp.Steps + result := &TrancheResult{} + for _, alloc := range in.Allocations { + if err := processAllocation(in, alloc, steps, result); err != nil { + return nil, err + } + } + return result, nil +} + +// processAllocation splits one allocation across all ramp steps. Each step +// amount is clamped to the remaining unallocated gap (see computeStepAmount) +// and the last step receives the exact leftover, so the sum of all produced +// amounts equals the allocation gap exactly for every Validate-passing +// schedule, regardless of binary-float rounding in the step fractions. Steps +// with a zero computed amount are skipped; priorSum is still updated (adding +// zero is a no-op) to keep the remainder arithmetic consistent. +func processAllocation(in *TrancheInput, alloc Allocation, steps []RampStep, result *TrancheResult) error { + nSteps := len(steps) + priorSum := new(big.Rat) + for i, step := range steps { + isLast := i == nSteps-1 + amount := computeStepAmount(alloc.GapUSDPerHour, step.Fraction, isLast, priorSum) + if !isLast { + // Accumulate before the zero-skip check: adding a zero amount is a + // no-op on priorSum, but always updating keeps the remainder + // arithmetic consistent regardless of which earlier steps were + // skipped. + priorSum.Add(priorSum, amount) + } + if amount.Sign() == 0 { + // Zero amounts are skipped to avoid zero-amount purchases or + // tranches (PlannedAction.Validate would reject them). A zero can + // arise when earlier steps' exact rational fractions already + // consumed the whole gap and the clamp in computeStepAmount + // floored this step at the remaining zero. The clamp keeps the + // grand total exactly equal to the gap regardless of skips. + continue + } + if err := appendStep(in, alloc, step, i, nSteps, amount, result); err != nil { + return err + } + } + return nil +} + +// appendStep routes one non-zero step amount to BuyNow (AfterDays == 0) or +// Tranches (AfterDays > 0). +func appendStep(in *TrancheInput, alloc Allocation, step RampStep, stepIdx, nSteps int, amount *big.Rat, result *TrancheResult) error { + if step.AfterDays == 0 { + action, err := buildBuyNowAction(in, alloc, step, stepIdx, nSteps, amount) + if err != nil { + return err + } + result.BuyNow = append(result.BuyNow, action) + return nil + } + tr, err := buildFutureTranche(in, alloc, step, stepIdx, amount) + if err != nil { + return err + } + result.Tranches = append(result.Tranches, tr) + return nil +} + +// computeStepAmount returns the exact big.Rat amount for one ramp step, +// clamped to the remaining unallocated gap. +// +// For all steps except the last: amount = min(gap * fraction, remaining), +// where remaining = max(gap - priorSum, 0) and fraction is converted to a +// rational via big.Rat.SetFloat64 at the boundary (same discipline as +// ratFromFloat in allocate.go). RampSchedule.Validate ensures the fraction is +// in (0, 1], so NaN/Inf/negative cannot occur here. +// +// For the last step: amount = remaining (the exact leftover, floored at 0). +// +// The clamp matters because RampSchedule.Validate accepts fraction sets whose +// float64 sum is within rampSumEpsilon of 1.0 but whose exact rational sum +// slightly exceeds 1 (e.g. {0.5, 0.5000000008, 1e-10}). Without clamping, the +// last-step remainder would go negative and BuildTranches would reject a +// config its own validator accepted. With the clamp, the sum of all step +// amounts equals the allocation gap exactly for every Validate-passing +// schedule: no cent is ever lost, duplicated, or over-allocated. +func computeStepAmount(gap *big.Rat, fraction float64, isLast bool, priorSum *big.Rat) *big.Rat { + remaining := new(big.Rat).Sub(gap, priorSum) + if remaining.Sign() < 0 { + remaining = new(big.Rat) + } + if isLast { + return remaining + } + amount := new(big.Rat).Mul(gap, new(big.Rat).SetFloat64(fraction)) + if amount.Cmp(remaining) > 0 { + return remaining + } + return amount +} + +// stepRationale returns a human-readable rationale string for a buy-now action +// or future tranche, embedding the step position, fraction percentage, computed +// amount, total gap, and the allocation's original rationale. +func stepRationale(alloc Allocation, stepIdx, totalSteps int, fraction float64, amount, gap *big.Rat) string { + return fmt.Sprintf( + "ramp step %d/%d (%.0f%%): %s of %s total. %s", + stepIdx+1, totalSteps, fraction*100, + fmtRatUSD(amount), fmtRatUSD(gap), + alloc.Rationale, + ) +} + +// buildBuyNowAction creates a validated PlannedAction (ActionPurchase) for a +// ramp step with AfterDays == 0. Returns an error if the produced action fails +// its own Validate check, which would indicate a bug in the caller-supplied +// input (e.g. an empty Term). +func buildBuyNowAction(in *TrancheInput, alloc Allocation, step RampStep, stepIdx, nSteps int, amount *big.Rat) (PlannedAction, error) { + rationale := stepRationale(alloc, stepIdx, nSteps, step.Fraction, amount, alloc.GapUSDPerHour) + action := PlannedAction{ + Action: ActionPurchase, + Layer: alloc.Layer, + AmountUSDPerHour: new(big.Rat).Set(amount), + Term: in.Term, + PaymentOption: in.PaymentOption, + Rationale: rationale, + DataSources: alloc.DataSources, + } + if err := action.Validate(); err != nil { + return PlannedAction{}, fmt.Errorf("buy-now action (layer %s, step %d): %w", alloc.Layer, stepIdx, err) + } + return action, nil +} + +// buildFutureTranche creates a validated Tranche for a ramp step with +// AfterDays > 0. FireAfter is set to Now + AfterDays days. The ID is assigned +// by calling in.NewID(). Layer, Term, and PaymentOption are stamped so the +// tranche is fully self-describing: the executor that fires it must be able +// to place the purchase without consulting the parent run's plan (two +// allocations with equal gaps on different layers stay distinguishable from +// the tranche row alone). Returns an error if the produced tranche fails its +// own Validate check. +func buildFutureTranche(in *TrancheInput, alloc Allocation, step RampStep, stepIdx int, amount *big.Rat) (Tranche, error) { + tr := Tranche{ + ID: in.NewID(), + RunID: in.RunID, + StepIndex: stepIdx, + Status: TrancheStatusScheduled, + FireAfter: in.Now.AddDate(0, 0, step.AfterDays), + AmountUSDPerHour: amount.RatString(), + Layer: alloc.Layer, + Term: in.Term, + PaymentOption: in.PaymentOption, + } + if err := tr.Validate(); err != nil { + return Tranche{}, fmt.Errorf("tranche (layer %s, step %d): %w", alloc.Layer, stepIdx, err) + } + return tr, nil +} diff --git a/pkg/ladder/ladder_test.go b/pkg/ladder/ladder_test.go new file mode 100644 index 000000000..c787273a8 --- /dev/null +++ b/pkg/ladder/ladder_test.go @@ -0,0 +1,864 @@ +package ladder + +import ( + "fmt" + "math/big" + "strings" + "testing" + "time" + + "github.com/LeanerCloud/CUDly/pkg/common" +) + +// --- test helpers --- + +// seqID returns a function that generates IDs sequentially: prefix-1, prefix-2, ... +func seqID(prefix string) func() string { + n := 0 + return func() string { + n++ + return fmt.Sprintf("%s-%d", prefix, n) + } +} + +// baseConfig returns a valid LadderConfig using the provided ramp schedule. +func baseConfig(ramp RampSchedule) *LadderConfig { + return &LadderConfig{ + Scope: Scope{ + Provider: common.ProviderAWS, + AccountID: "123456789012", + }, + Mode: ModeEmailApproval, + Cadence: CadenceWeekly, + Ramp: ramp, + TargetCoveragePct: 100, + BufferFraction: 0, + BaselinePercentile: 5, + LookbackDays: 30, + MaxActionsPerRun: 20, + BufferUtilizationThresholdPct: DefaultBufferUtilizationThresholdPct, + } +} + +// singleStepRamp returns a ramp with one immediate step covering 100% of the gap. +func singleStepRamp() RampSchedule { + return RampSchedule{Steps: []RampStep{{AfterDays: 0, Fraction: 1.0}}} +} + +// threeStepRamp returns a ramp with AfterDays 0/30/60 and fractions 0.4/0.3/0.3. +func threeStepRamp() RampSchedule { + return RampSchedule{Steps: []RampStep{ + {AfterDays: 0, Fraction: 0.4}, + {AfterDays: 30, Fraction: 0.3}, + {AfterDays: 60, Fraction: 0.3}, + }} +} + +// delayedRamp returns a ramp with all steps delayed (no AfterDays == 0). +func delayedRamp() RampSchedule { + return RampSchedule{Steps: []RampStep{ + {AfterDays: 30, Fraction: 0.5}, + {AfterDays: 60, Fraction: 0.5}, + }} +} + +// inexactFracRamp returns a ramp with 0.33/0.33/0.34 fractions (inexact in float64). +func inexactFracRamp() RampSchedule { + return RampSchedule{Steps: []RampStep{ + {AfterDays: 0, Fraction: 0.33}, + {AfterDays: 30, Fraction: 0.33}, + {AfterDays: 60, Fraction: 0.34}, + }} +} + +// mkAlloc builds an Allocation with the given layer and a whole-dollar hourly +// gap as an exact rational. +func mkAlloc(layer LayerType, gapUSD int64) Allocation { + return Allocation{ + Layer: layer, + GapUSDPerHour: new(big.Rat).SetInt64(gapUSD), + Rationale: fmt.Sprintf("test rationale for %s", layer), + DataSources: []string{"test-source"}, + } +} + +// baseInput builds a minimal valid TrancheInput for the given config and allocations. +func baseInput(cfg *LadderConfig, allocs []Allocation, now time.Time) *TrancheInput { + return &TrancheInput{ + Config: cfg, + Allocations: allocs, + RunID: "run-abc", + Term: "1yr", + PaymentOption: "no-upfront", + Now: now, + NewID: seqID("tr"), + } +} + +// totalAmount sums all amounts across BuyNow actions and Tranches. Panics if +// a tranche AmountUSDPerHour fails to parse (the test has already verified +// Validate passes, so this indicates a bug in the test helper). +func totalAmount(result *TrancheResult) *big.Rat { + total := new(big.Rat) + for _, a := range result.BuyNow { + total.Add(total, a.AmountUSDPerHour) + } + for _, tr := range result.Tranches { + r := new(big.Rat) + if _, ok := r.SetString(tr.AmountUSDPerHour); !ok { + panic(fmt.Sprintf("totalAmount: cannot parse tranche amount %q", tr.AmountUSDPerHour)) + } + total.Add(total, r) + } + return total +} + +// assertAllValid calls Validate on every produced action and tranche, reporting +// failures with t.Errorf. +func assertAllValid(t *testing.T, result *TrancheResult) { + t.Helper() + for i, a := range result.BuyNow { + if err := a.Validate(); err != nil { + t.Errorf("BuyNow[%d].Validate() = %v", i, err) + } + } + for i, tr := range result.Tranches { + if err := tr.Validate(); err != nil { + t.Errorf("Tranches[%d].Validate() = %v", i, err) + } + } +} + +// --- tests --- + +// TestBuildTranches_ThreeStepRamp verifies the standard 3-step ramp +// (AfterDays 0/30/60, fractions 0.4/0.3/0.3) over a $5/hr allocation. +// The buy-now step uses the immediate fraction; the two future tranches use the +// remainder logic. The total across all steps must reconstruct the gap exactly. +func TestBuildTranches_ThreeStepRamp(t *testing.T) { + t.Parallel() + now := time.Date(2026, 3, 1, 12, 0, 0, 0, time.UTC) + gap := new(big.Rat).SetInt64(5) // exactly $5/hr + + in := baseInput(baseConfig(threeStepRamp()), []Allocation{ + { + Layer: LayerComputeSP, + GapUSDPerHour: gap, + Rationale: "flex layer gap", + DataSources: []string{"cost-explorer"}, + }, + }, now) + + result, err := BuildTranches(in) + if err != nil { + t.Fatalf("BuildTranches() error = %v", err) + } + + // Immediate step (AfterDays == 0, fraction 0.4): must be positive. + // We assert the total instead of the exact step value because 0.4 is not + // exactly representable in float64; the amount is gap * SetFloat64(0.4), + // which is close but not equal to the rational 2/5. + if len(result.BuyNow) != 1 { + t.Fatalf("BuyNow count = %d, want 1", len(result.BuyNow)) + } + if result.BuyNow[0].AmountUSDPerHour.Sign() <= 0 { + t.Errorf("BuyNow[0].AmountUSDPerHour must be positive, got %s", + result.BuyNow[0].AmountUSDPerHour.RatString()) + } + + // Two future tranches for days 30 and 60. + if len(result.Tranches) != 2 { + t.Fatalf("Tranches count = %d, want 2", len(result.Tranches)) + } + wantDay30 := now.AddDate(0, 0, 30) + if !result.Tranches[0].FireAfter.Equal(wantDay30) { + t.Errorf("Tranches[0].FireAfter = %v, want %v", result.Tranches[0].FireAfter, wantDay30) + } + wantDay60 := now.AddDate(0, 0, 60) + if !result.Tranches[1].FireAfter.Equal(wantDay60) { + t.Errorf("Tranches[1].FireAfter = %v, want %v", result.Tranches[1].FireAfter, wantDay60) + } + + // Grand total must reconstruct the gap exactly. + got := totalAmount(result) + if got.Cmp(gap) != 0 { + t.Errorf("total amount = %s, want %s (gap not reconstructed exactly)", + got.RatString(), gap.RatString()) + } + + // RunID stamped on all tranches. + for i, tr := range result.Tranches { + if tr.RunID != in.RunID { + t.Errorf("Tranches[%d].RunID = %q, want %q", i, tr.RunID, in.RunID) + } + if tr.Status != TrancheStatusScheduled { + t.Errorf("Tranches[%d].Status = %q, want %q", i, tr.Status, TrancheStatusScheduled) + } + } + + assertAllValid(t, result) +} + +// TestBuildTranches_FullyDelayedRamp verifies that a ramp with no AfterDays == 0 +// step produces zero buy-now actions. All commitment activity is deferred. +func TestBuildTranches_FullyDelayedRamp(t *testing.T) { + t.Parallel() + now := time.Date(2026, 3, 1, 0, 0, 0, 0, time.UTC) + gap := new(big.Rat).SetFrac64(10, 1) + + in := baseInput(baseConfig(delayedRamp()), []Allocation{ + { + Layer: LayerComputeSP, + GapUSDPerHour: gap, + Rationale: "delayed ramp test", + DataSources: []string{"cost-explorer"}, + }, + }, now) + + result, err := BuildTranches(in) + if err != nil { + t.Fatalf("BuildTranches() error = %v", err) + } + + if len(result.BuyNow) != 0 { + t.Errorf("BuyNow count = %d, want 0 (fully delayed ramp)", len(result.BuyNow)) + } + if len(result.Tranches) != 2 { + t.Fatalf("Tranches count = %d, want 2", len(result.Tranches)) + } + + // Grand total must still equal the gap exactly. + got := totalAmount(result) + if got.Cmp(gap) != 0 { + t.Errorf("total = %s, want %s", got.RatString(), gap.RatString()) + } + assertAllValid(t, result) +} + +// TestBuildTranches_SingleStep verifies a single-step ramp (fraction 1.0, +// AfterDays 0): everything becomes a buy-now action with no tranches. +func TestBuildTranches_SingleStep(t *testing.T) { + t.Parallel() + now := time.Date(2026, 4, 1, 0, 0, 0, 0, time.UTC) + gap := new(big.Rat).SetFrac64(7, 2) // $3.50/hr + + in := baseInput(baseConfig(singleStepRamp()), []Allocation{ + { + Layer: LayerComputeSP, + GapUSDPerHour: gap, + Rationale: "single step test", + DataSources: []string{"cost-explorer"}, + }, + }, now) + + result, err := BuildTranches(in) + if err != nil { + t.Fatalf("BuildTranches() error = %v", err) + } + + if len(result.BuyNow) != 1 { + t.Fatalf("BuyNow count = %d, want 1", len(result.BuyNow)) + } + if len(result.Tranches) != 0 { + t.Errorf("Tranches count = %d, want 0", len(result.Tranches)) + } + if result.BuyNow[0].AmountUSDPerHour.Cmp(gap) != 0 { + t.Errorf("BuyNow[0].Amount = %s, want %s", + result.BuyNow[0].AmountUSDPerHour.RatString(), gap.RatString()) + } + assertAllValid(t, result) +} + +// TestBuildTranches_MultipleAllocations verifies that multiple allocations +// (base, flex, buffer) are each split correctly and that running BuildTranches +// twice with identical deterministic inputs produces identical output order and +// amounts. +func TestBuildTranches_MultipleAllocations(t *testing.T) { + t.Parallel() + now := time.Date(2026, 5, 1, 0, 0, 0, 0, time.UTC) + cfg := baseConfig(threeStepRamp()) + + allocs := []Allocation{ + mkAlloc(LayerEC2InstanceSP, 3), // $3/hr base + mkAlloc(LayerComputeSP, 5), // $5/hr flex + mkAlloc(LayerConvertibleRI, 2), // $2/hr buffer + } + + buildOnce := func() *TrancheResult { + in := &TrancheInput{ + Config: cfg, + Allocations: allocs, + RunID: "run-multi", + Term: "1yr", + PaymentOption: "no-upfront", + Now: now, + NewID: seqID("id"), + } + res, err := BuildTranches(in) + if err != nil { + t.Fatalf("BuildTranches() error = %v", err) + } + return res + } + + first := buildOnce() + second := buildOnce() + + // Three allocations x one buy-now step = three buy-now actions. + if len(first.BuyNow) != 3 { + t.Errorf("BuyNow count = %d, want 3", len(first.BuyNow)) + } + // Three allocations x two future steps = six tranches. + if len(first.Tranches) != 6 { + t.Errorf("Tranches count = %d, want 6", len(first.Tranches)) + } + + // Determinism: lengths must match first (a shorter second run must fail + // loudly, not pass silently on the shared prefix), then per-element + // amounts must match between runs. + if len(first.BuyNow) != len(second.BuyNow) { + t.Fatalf("BuyNow lengths differ between runs: %d vs %d", len(first.BuyNow), len(second.BuyNow)) + } + if len(first.Tranches) != len(second.Tranches) { + t.Fatalf("Tranches lengths differ between runs: %d vs %d", len(first.Tranches), len(second.Tranches)) + } + for i := range first.BuyNow { + a1 := first.BuyNow[i].AmountUSDPerHour + a2 := second.BuyNow[i].AmountUSDPerHour + if a1.Cmp(a2) != 0 { + t.Errorf("BuyNow[%d] amounts differ between runs: %s vs %s", i, a1.RatString(), a2.RatString()) + } + } + for i := range first.Tranches { + if first.Tranches[i].AmountUSDPerHour != second.Tranches[i].AmountUSDPerHour { + t.Errorf("Tranches[%d] amounts differ: %s vs %s", + i, first.Tranches[i].AmountUSDPerHour, second.Tranches[i].AmountUSDPerHour) + } + } + + // Grand total must reconstruct the sum of all gaps exactly. + // Output ordering: processAllocation iterates allocations in slice order; + // within each allocation steps run in schedule order. So for 3 allocs and + // 3 steps (day0/day30/day60): + // BuyNow: [alloc0-step0, alloc1-step0, alloc2-step0] + // Tranches: [alloc0-step1, alloc0-step2, alloc1-step1, alloc1-step2, alloc2-step1, alloc2-step2] + wantGaps := []*big.Rat{ + new(big.Rat).SetFrac64(3, 1), + new(big.Rat).SetFrac64(5, 1), + new(big.Rat).SetFrac64(2, 1), + } + wantTotal := new(big.Rat).SetFrac64(10, 1) // 3 + 5 + 2 + got := totalAmount(first) + if got.Cmp(wantTotal) != 0 { + t.Errorf("total amount = %s, want %s", got.RatString(), wantTotal.RatString()) + } + + // Per-allocation totals: alloc[i] occupies BuyNow[i] and Tranches[i*2], Tranches[i*2+1]. + for i, wantGap := range wantGaps { + buyNowAmt := first.BuyNow[i].AmountUSDPerHour + tr1Rat := new(big.Rat) + tr2Rat := new(big.Rat) + // errcheck excluded for _test.go; Validate already confirmed these parse. + tr1Rat.SetString(first.Tranches[i*2].AmountUSDPerHour) + tr2Rat.SetString(first.Tranches[i*2+1].AmountUSDPerHour) + layerTotal := new(big.Rat).Add(buyNowAmt, new(big.Rat).Add(tr1Rat, tr2Rat)) + if layerTotal.Cmp(wantGap) != 0 { + t.Errorf("allocation[%d] total = %s, want %s (gap not reconstructed)", + i, layerTotal.RatString(), wantGap.RatString()) + } + } + + assertAllValid(t, first) +} + +// TestBuildTranches_InexactFractionRemainder verifies that when step fractions +// are not exactly representable in float64 (e.g. 0.33/0.33/0.34), the last +// step's remainder ensures the grand total reconstructs the gap exactly. +func TestBuildTranches_InexactFractionRemainder(t *testing.T) { + t.Parallel() + now := time.Date(2026, 6, 1, 0, 0, 0, 0, time.UTC) + // Use a gap of exactly $5/hr expressed as 5/1. + gap := new(big.Rat).SetInt64(5) + + in := baseInput(baseConfig(inexactFracRamp()), []Allocation{ + { + Layer: LayerComputeSP, + GapUSDPerHour: gap, + Rationale: "inexact fraction test", + DataSources: []string{"test"}, + }, + }, now) + + result, err := BuildTranches(in) + if err != nil { + t.Fatalf("BuildTranches() error = %v", err) + } + + // 1 buy-now (step 0, AfterDays==0) + 2 tranches (steps 1 and 2). + if len(result.BuyNow) != 1 { + t.Errorf("BuyNow count = %d, want 1", len(result.BuyNow)) + } + if len(result.Tranches) != 2 { + t.Errorf("Tranches count = %d, want 2", len(result.Tranches)) + } + + // Grand total must be exactly $5 regardless of float64 fraction imprecision. + got := totalAmount(result) + if got.Cmp(gap) != 0 { + t.Errorf("total = %s, want 5/1 (remainder did not reconstruct gap exactly)", + got.RatString()) + } + assertAllValid(t, result) +} + +// TestBuildTranches_TinyGapExactness verifies that even for very small gap +// values (e.g. $0.001/hr) the total across all steps reconstructs the gap +// exactly. With exact big.Rat arithmetic and positive fractions, step amounts +// are never zero for a positive gap. +func TestBuildTranches_TinyGapExactness(t *testing.T) { + t.Parallel() + now := time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC) + // $1/1000 per hour = $0.001/hr, smaller than the $0.01 min-allocatable + // threshold but valid as a raw amount for BuildTranches (threshold is + // enforced by Allocate, not BuildTranches). + gap := new(big.Rat).SetFrac64(1, 1000) + + in := baseInput(baseConfig(inexactFracRamp()), []Allocation{ + { + Layer: LayerComputeSP, + GapUSDPerHour: gap, + Rationale: "tiny gap test", + DataSources: []string{"test"}, + }, + }, now) + + result, err := BuildTranches(in) + if err != nil { + t.Fatalf("BuildTranches() error = %v", err) + } + + got := totalAmount(result) + if got.Cmp(gap) != 0 { + t.Errorf("tiny gap total = %s, want %s", got.RatString(), gap.RatString()) + } + assertAllValid(t, result) +} + +// TestBuildTranches_IDsStamped verifies that produced tranches carry the IDs +// returned by NewID in call order, and that the RunID from the input is +// stamped on every tranche. +func TestBuildTranches_IDsStamped(t *testing.T) { + t.Parallel() + now := time.Date(2026, 8, 1, 0, 0, 0, 0, time.UTC) + + in := &TrancheInput{ + Config: baseConfig(delayedRamp()), + Allocations: []Allocation{ + mkAlloc(LayerComputeSP, 4), + mkAlloc(LayerConvertibleRI, 2), + }, + RunID: "run-id-stamp-test", + Term: "1yr", + PaymentOption: "no-upfront", + Now: now, + NewID: seqID("myid"), + } + + result, err := BuildTranches(in) + if err != nil { + t.Fatalf("BuildTranches() error = %v", err) + } + + // delayedRamp has 2 steps, 2 allocations -> 4 tranches total. + if len(result.Tranches) != 4 { + t.Fatalf("Tranches count = %d, want 4", len(result.Tranches)) + } + // IDs should be myid-1, myid-2, myid-3, myid-4 in order. + wantIDs := []string{"myid-1", "myid-2", "myid-3", "myid-4"} + for i, tr := range result.Tranches { + if tr.ID != wantIDs[i] { + t.Errorf("Tranches[%d].ID = %q, want %q", i, tr.ID, wantIDs[i]) + } + if tr.RunID != in.RunID { + t.Errorf("Tranches[%d].RunID = %q, want %q", i, tr.RunID, in.RunID) + } + } + assertAllValid(t, result) +} + +// TestBuildTranches_ValidationFailures exercises the fail-loud validation path +// for malformed TrancheInput structs. +func TestBuildTranches_ValidationFailures(t *testing.T) { + t.Parallel() + now := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + goodConfig := baseConfig(singleStepRamp()) + goodAlloc := mkAlloc(LayerComputeSP, 5) + + cases := []struct { + build func() *TrancheInput + name string + }{ + { + name: "nil input", + build: func() *TrancheInput { return nil }, + }, + { + name: "nil config", + build: func() *TrancheInput { + in := baseInput(goodConfig, []Allocation{goodAlloc}, now) + in.Config = nil + return in + }, + }, + { + name: "empty RunID", + build: func() *TrancheInput { + in := baseInput(goodConfig, []Allocation{goodAlloc}, now) + in.RunID = "" + return in + }, + }, + { + name: "zero Now", + build: func() *TrancheInput { + in := baseInput(goodConfig, []Allocation{goodAlloc}, now) + in.Now = time.Time{} + return in + }, + }, + { + name: "nil NewID", + build: func() *TrancheInput { + in := baseInput(goodConfig, []Allocation{goodAlloc}, now) + in.NewID = nil + return in + }, + }, + { + name: "empty Term", + build: func() *TrancheInput { + in := baseInput(goodConfig, []Allocation{goodAlloc}, now) + in.Term = "" + return in + }, + }, + { + name: "empty PaymentOption", + build: func() *TrancheInput { + in := baseInput(goodConfig, []Allocation{goodAlloc}, now) + in.PaymentOption = "" + return in + }, + }, + { + name: "allocation with invalid layer", + build: func() *TrancheInput { + bad := Allocation{ + Layer: LayerType("bogus-layer"), + GapUSDPerHour: new(big.Rat).SetInt64(1), + Rationale: "x", + } + return baseInput(goodConfig, []Allocation{bad}, now) + }, + }, + { + name: "allocation with nil gap", + build: func() *TrancheInput { + bad := Allocation{ + Layer: LayerComputeSP, + GapUSDPerHour: nil, + Rationale: "x", + } + return baseInput(goodConfig, []Allocation{bad}, now) + }, + }, + { + name: "allocation with zero gap", + build: func() *TrancheInput { + bad := Allocation{ + Layer: LayerComputeSP, + GapUSDPerHour: new(big.Rat), + Rationale: "x", + } + return baseInput(goodConfig, []Allocation{bad}, now) + }, + }, + { + name: "allocation with negative gap", + build: func() *TrancheInput { + bad := Allocation{ + Layer: LayerComputeSP, + GapUSDPerHour: new(big.Rat).SetInt64(-1), + Rationale: "x", + } + return baseInput(goodConfig, []Allocation{bad}, now) + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + t.Parallel() + _, err := BuildTranches(c.build()) + if err == nil { + t.Errorf("BuildTranches() = nil error, want error for case %q", c.name) + } + }) + } +} + +// TestBuildTranches_StepIndexStamped verifies that each tranche carries the +// correct StepIndex matching its position in the ramp schedule. +func TestBuildTranches_StepIndexStamped(t *testing.T) { + t.Parallel() + now := time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC) + ramp := RampSchedule{Steps: []RampStep{ + {AfterDays: 10, Fraction: 0.5}, + {AfterDays: 20, Fraction: 0.3}, + {AfterDays: 30, Fraction: 0.2}, + }} + in := baseInput(baseConfig(ramp), []Allocation{ + mkAlloc(LayerComputeSP, 6), + }, now) + + result, err := BuildTranches(in) + if err != nil { + t.Fatalf("BuildTranches() error = %v", err) + } + + if len(result.BuyNow) != 0 { + t.Errorf("BuyNow count = %d, want 0 (all steps delayed)", len(result.BuyNow)) + } + if len(result.Tranches) != 3 { + t.Fatalf("Tranches count = %d, want 3", len(result.Tranches)) + } + for i, tr := range result.Tranches { + if tr.StepIndex != i { + t.Errorf("Tranches[%d].StepIndex = %d, want %d", i, tr.StepIndex, i) + } + wantFireAfter := now.AddDate(0, 0, ramp.Steps[i].AfterDays) + if !tr.FireAfter.Equal(wantFireAfter) { + t.Errorf("Tranches[%d].FireAfter = %v, want %v", i, tr.FireAfter, wantFireAfter) + } + } + + // Total must reconstruct gap exactly. + got := totalAmount(result) + want := new(big.Rat).SetInt64(6) + if got.Cmp(want) != 0 { + t.Errorf("total = %s, want %s", got.RatString(), want.RatString()) + } + assertAllValid(t, result) +} + +// TestBuildTranches_EmptyAllocations verifies that an empty allocations slice +// produces a valid empty result without error. +func TestBuildTranches_EmptyAllocations(t *testing.T) { + t.Parallel() + now := time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC) + in := baseInput(baseConfig(singleStepRamp()), nil, now) + + result, err := BuildTranches(in) + if err != nil { + t.Fatalf("BuildTranches() error = %v", err) + } + if len(result.BuyNow) != 0 { + t.Errorf("BuyNow count = %d, want 0 for empty allocations", len(result.BuyNow)) + } + if len(result.Tranches) != 0 { + t.Errorf("Tranches count = %d, want 0 for empty allocations", len(result.Tranches)) + } +} + +// TestBuildTranches_RationaleContents verifies that the rationale string on a +// buy-now action embeds the step index, step count, and the original allocation +// rationale. +func TestBuildTranches_RationaleContents(t *testing.T) { + t.Parallel() + now := time.Date(2026, 11, 1, 0, 0, 0, 0, time.UTC) + allocRationale := "flex layer: low_water=$5.0000/hr, gap=$5.0000/hr" + in := &TrancheInput{ + Config: baseConfig(singleStepRamp()), + Allocations: []Allocation{{ + Layer: LayerComputeSP, + GapUSDPerHour: new(big.Rat).SetInt64(5), + Rationale: allocRationale, + DataSources: []string{"cost-explorer"}, + }}, + RunID: "run-rationale", + Term: "1yr", + PaymentOption: "no-upfront", + Now: now, + NewID: seqID("r"), + } + + result, err := BuildTranches(in) + if err != nil { + t.Fatalf("BuildTranches() error = %v", err) + } + if len(result.BuyNow) != 1 { + t.Fatalf("BuyNow count = %d, want 1", len(result.BuyNow)) + } + + r := result.BuyNow[0].Rationale + checks := []string{"ramp step 1/1", "100%", allocRationale} + for _, want := range checks { + if !strings.Contains(r, want) { + t.Errorf("rationale %q missing expected substring %q", r, want) + } + } +} + +// TestBuildTranches_DataSourcesPropagated verifies that the DataSources from +// each allocation are propagated verbatim to each buy-now PlannedAction. +func TestBuildTranches_DataSourcesPropagated(t *testing.T) { + t.Parallel() + now := time.Date(2026, 12, 1, 0, 0, 0, 0, time.UTC) + wantSources := []string{"cost-explorer", "cloudwatch"} + + in := baseInput(baseConfig(singleStepRamp()), []Allocation{{ + Layer: LayerComputeSP, + GapUSDPerHour: new(big.Rat).SetInt64(3), + Rationale: "ds test", + DataSources: wantSources, + }}, now) + + result, err := BuildTranches(in) + if err != nil { + t.Fatalf("BuildTranches() error = %v", err) + } + if len(result.BuyNow) != 1 { + t.Fatalf("BuyNow count = %d, want 1", len(result.BuyNow)) + } + got := result.BuyNow[0].DataSources + if len(got) != len(wantSources) { + t.Fatalf("DataSources len = %d, want %d", len(got), len(wantSources)) + } + for i, s := range wantSources { + if got[i] != s { + t.Errorf("DataSources[%d] = %q, want %q", i, got[i], s) + } + } +} + +// TestBuildTranches_TranchesSelfDescribing verifies that two allocations with +// identical gaps on different layers produce tranches that are distinguishable +// by their Layer field, and that Term and PaymentOption are stamped on every +// tranche, so a fired tranche is executable without consulting the parent +// run's plan. +func TestBuildTranches_TranchesSelfDescribing(t *testing.T) { + t.Parallel() + now := time.Date(2026, 6, 15, 0, 0, 0, 0, time.UTC) + + // Two allocations with the SAME gap on DIFFERENT layers: without the + // Layer field these would produce identical tranche rows except for ID. + in := baseInput(baseConfig(delayedRamp()), []Allocation{ + mkAlloc(LayerComputeSP, 4), + mkAlloc(LayerConvertibleRI, 4), + }, now) + + result, err := BuildTranches(in) + if err != nil { + t.Fatalf("BuildTranches() error = %v", err) + } + // delayedRamp has 2 delayed steps x 2 allocations = 4 tranches. + if len(result.Tranches) != 4 { + t.Fatalf("Tranches count = %d, want 4", len(result.Tranches)) + } + + // Allocation order is preserved: tranches 0,1 belong to LayerComputeSP, + // tranches 2,3 to LayerConvertibleRI. + wantLayers := []LayerType{LayerComputeSP, LayerComputeSP, LayerConvertibleRI, LayerConvertibleRI} + for i, tr := range result.Tranches { + if tr.Layer != wantLayers[i] { + t.Errorf("Tranches[%d].Layer = %q, want %q", i, tr.Layer, wantLayers[i]) + } + if tr.Term != in.Term { + t.Errorf("Tranches[%d].Term = %q, want %q", i, tr.Term, in.Term) + } + if tr.PaymentOption != in.PaymentOption { + t.Errorf("Tranches[%d].PaymentOption = %q, want %q", i, tr.PaymentOption, in.PaymentOption) + } + } + + // Same StepIndex + same amount across the two layers must still be + // distinguishable via Layer (the whole point of self-description). + if result.Tranches[0].AmountUSDPerHour != result.Tranches[2].AmountUSDPerHour || + result.Tranches[0].StepIndex != result.Tranches[2].StepIndex { + t.Fatalf("test setup expectation broken: tranches 0 and 2 should share amount and step index") + } + if result.Tranches[0].Layer == result.Tranches[2].Layer { + t.Errorf("tranches 0 and 2 are indistinguishable: same amount, step index, and layer %q", result.Tranches[0].Layer) + } + assertAllValid(t, result) +} + +// TestBuildTranches_EpsilonOvershootClamped is the regression test for +// fraction sets whose float64 sum passes RampSchedule.Validate's epsilon +// check but whose exact rational sum exceeds 1. Without the clamp in +// computeStepAmount, the last-step remainder would go negative and +// BuildTranches would reject a config its own validator accepted. +func TestBuildTranches_EpsilonOvershootClamped(t *testing.T) { + t.Parallel() + now := time.Date(2026, 7, 15, 0, 0, 0, 0, time.UTC) + + // Float64 sum is within rampSumEpsilon of 1.0 (passes Validate), but the + // exact rational sum of the first two fractions already exceeds 1. + ramp := RampSchedule{Steps: []RampStep{ + {AfterDays: 0, Fraction: 0.5}, + {AfterDays: 30, Fraction: 0.5000000008}, + {AfterDays: 60, Fraction: 1e-10}, + }} + if err := ramp.Validate(); err != nil { + t.Fatalf("test premise broken: ramp must pass Validate, got %v", err) + } + + gap := new(big.Rat).SetInt64(8) // $8/hr + in := baseInput(baseConfig(ramp), []Allocation{ + { + Layer: LayerComputeSP, + GapUSDPerHour: gap, + Rationale: "epsilon overshoot test", + DataSources: []string{"test"}, + }, + }, now) + + result, err := BuildTranches(in) + if err != nil { + t.Fatalf("BuildTranches() error = %v (validator-accepted schedule must not be rejected)", err) + } + + // Total must reconstruct the gap exactly despite the overshooting fractions. + got := totalAmount(result) + if got.Cmp(gap) != 0 { + t.Errorf("total = %s, want %s (clamp must keep the total exact)", got.RatString(), gap.RatString()) + } + + // No output item may carry a zero or negative amount: the overshot step is + // clamped to the remaining gap and the starved last step is skipped. + for i, a := range result.BuyNow { + if a.AmountUSDPerHour.Sign() <= 0 { + t.Errorf("BuyNow[%d] amount = %s, want > 0", i, a.AmountUSDPerHour.RatString()) + } + } + for i, tr := range result.Tranches { + r := new(big.Rat) + if _, ok := r.SetString(tr.AmountUSDPerHour); !ok { + t.Fatalf("Tranches[%d].AmountUSDPerHour %q does not parse", i, tr.AmountUSDPerHour) + } + if r.Sign() <= 0 { + t.Errorf("Tranches[%d] amount = %s, want > 0", i, tr.AmountUSDPerHour) + } + } + + // Concretely: buy-now consumes gap*0.5 exactly (0.5 is representable); + // step 1 overshoots and is clamped to the remaining gap*0.5; step 2 is + // starved to zero and skipped. + if len(result.BuyNow) != 1 { + t.Errorf("BuyNow count = %d, want 1", len(result.BuyNow)) + } + if len(result.Tranches) != 1 { + t.Errorf("Tranches count = %d, want 1 (starved last step skipped)", len(result.Tranches)) + } + assertAllValid(t, result) +} diff --git a/pkg/ladder/store.go b/pkg/ladder/store.go index 6f2b86cc8..96ec21781 100644 --- a/pkg/ladder/store.go +++ b/pkg/ladder/store.go @@ -3,6 +3,7 @@ package ladder import ( "context" "fmt" + "math/big" "time" ) @@ -137,22 +138,48 @@ func (r *RunRecord) Validate() error { } // Tranche is one ramp step that has been persisted for scheduled firing. +// +// A tranche is fully self-describing: Layer, AmountUSDPerHour, Term, and +// PaymentOption together specify the exact purchase to execute when the +// tranche fires. Executors must never need to reconstruct purchase parameters +// from the parent RunRecord's PlanJSON -- two allocations with equal gaps on +// different layers must remain distinguishable from the tranche row alone. type Tranche struct { // FireAfter is the wall-clock time at or after which this tranche fires. FireAfter time.Time // FiredAt is nil until the tranche actually fires. - FiredAt *time.Time - ID string - RunID string - Status TrancheStatus - StepIndex int + FiredAt *time.Time + // AmountUSDPerHour is the hourly commitment delta for this tranche step, + // encoded as a big.Rat string (via big.Rat.RatString()). The string + // encoding is lossless and round-trips through big.Rat.SetString without + // floating-point precision loss, making it safe for DB persistence and + // rehydration. Must be a positive rational; use big.Rat.RatString() to + // produce the value and big.Rat.SetString to rehydrate it. + AmountUSDPerHour string + ID string + RunID string + Status TrancheStatus + // Layer identifies the commitment layer this tranche purchases into. + // Required so a fired tranche is executable without consulting the parent + // run's plan. + Layer LayerType + // Term is the commitment term for the purchase (e.g. "1yr", "3yr"). + // Required: an empty term would silently default at the provider boundary + // (money-shaping field, same rule as PlannedAction.Term). + Term string + // PaymentOption is the payment structure for the purchase (e.g. + // "no-upfront"). Required for the same reason as Term. + PaymentOption string + StepIndex int } // Validate checks that the tranche is self-consistent: non-empty ID and // RunID (RunID is the single source of run linkage, see // LadderStore.SaveTranches), non-negative step index, a set FireAfter -// timestamp, recognized status, and a FiredAt timestamp only when the -// status implies the tranche fired. +// timestamp, complete purchase-execution fields (recognized Layer, positive +// AmountUSDPerHour encoded as a RatString, non-empty Term and PaymentOption), +// recognized status, and a FiredAt timestamp only when the status implies the +// tranche fired. func (t *Tranche) Validate() error { if t.ID == "" { return fmt.Errorf("tranche ID is required") @@ -169,6 +196,9 @@ func (t *Tranche) Validate() error { if t.FireAfter.IsZero() { return fmt.Errorf("fire_after must be set (zero time would fire immediately)") } + if err := t.validateExecutionFields(); err != nil { + return err + } if err := t.Status.Validate(); err != nil { return fmt.Errorf("status: %w", err) } @@ -178,6 +208,44 @@ func (t *Tranche) Validate() error { return nil } +// validateExecutionFields checks the fields that make a tranche executable as +// a standalone purchase: a recognized Layer, a positive amount, and non-empty +// Term and PaymentOption. Split out of Validate to keep each function's +// cyclomatic complexity within the repo limit. +func (t *Tranche) validateExecutionFields() error { + if err := t.Layer.Validate(); err != nil { + return fmt.Errorf("layer: %w", err) + } + if err := t.validateAmountUSDPerHour(); err != nil { + return err + } + if t.Term == "" { + return fmt.Errorf("term is required (money-shaping field; empty would silently default at the provider boundary)") + } + if t.PaymentOption == "" { + return fmt.Errorf("payment_option is required (money-shaping field; empty would silently default at the provider boundary)") + } + return nil +} + +// validateAmountUSDPerHour checks that AmountUSDPerHour is a non-empty string +// encoding a positive rational value. The RatString encoding (produced by +// big.Rat.RatString and parsed by big.Rat.SetString) is lossless and safe for +// DB round-tripping without floating-point precision loss. +func (t *Tranche) validateAmountUSDPerHour() error { + if t.AmountUSDPerHour == "" { + return fmt.Errorf("amount_usd_per_hour is required (positive rational encoded as big.Rat.RatString, e.g. \"3/2\")") + } + r := new(big.Rat) + if _, ok := r.SetString(t.AmountUSDPerHour); !ok { + return fmt.Errorf("amount_usd_per_hour %q is not a valid rational string (use big.Rat.RatString() encoding)", t.AmountUSDPerHour) + } + if r.Sign() <= 0 { + return fmt.Errorf("amount_usd_per_hour must be positive, got %s", t.AmountUSDPerHour) + } + return nil +} + // LadderStore is the storage contract for the ladder engine. The concrete // implementation lives in internal/ (separate Go module) and is injected // into the engine at startup; pkg/ defines only the interface. @@ -195,8 +263,11 @@ type LadderStore interface { // SaveTranches persists a batch of tranches. Every tranche must carry a // non-empty RunID (Tranche.RunID is the single source of truth for run // linkage); implementations persist tranches exactly as given and must - // not infer linkage from anything else. Callers may call this multiple - // times (e.g., once per ramp step) and implementations should upsert by + // not infer linkage from anything else. Tranches are fully + // self-describing (Layer, AmountUSDPerHour, Term, PaymentOption): + // executors must be able to fire a tranche as a purchase without any + // RunRecord or PlanJSON lookup. Callers may call this multiple times + // (e.g., once per ramp step) and implementations should upsert by // tranche ID. SaveTranches(ctx context.Context, tranches []Tranche) error } diff --git a/pkg/ladder/store_test.go b/pkg/ladder/store_test.go index 395380372..f63cf71ad 100644 --- a/pkg/ladder/store_test.go +++ b/pkg/ladder/store_test.go @@ -151,11 +151,15 @@ func TestTrancheValidate(t *testing.T) { t.Parallel() now := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) valid := Tranche{ - ID: "tranche-1", - RunID: "run-1", - StepIndex: 0, - FireAfter: now, - Status: TrancheStatusScheduled, + ID: "tranche-1", + RunID: "run-1", + StepIndex: 0, + FireAfter: now, + Status: TrancheStatusScheduled, + AmountUSDPerHour: "2/1", + Layer: LayerComputeSP, + Term: "1yr", + PaymentOption: "no-upfront", } cases := []struct { mutate func(tr *Tranche) @@ -239,6 +243,56 @@ func TestTrancheValidate(t *testing.T) { }, wantErr: false, }, + { + name: "empty AmountUSDPerHour is invalid", + mutate: func(tr *Tranche) { tr.AmountUSDPerHour = "" }, + wantErr: true, + }, + { + name: "non-rational AmountUSDPerHour is invalid", + mutate: func(tr *Tranche) { tr.AmountUSDPerHour = "not-a-number" }, + wantErr: true, + }, + { + name: "zero AmountUSDPerHour is invalid", + mutate: func(tr *Tranche) { tr.AmountUSDPerHour = "0" }, + wantErr: true, + }, + { + name: "negative AmountUSDPerHour is invalid", + mutate: func(tr *Tranche) { tr.AmountUSDPerHour = "-3/2" }, + wantErr: true, + }, + { + name: "fractional AmountUSDPerHour is valid", + mutate: func(tr *Tranche) { tr.AmountUSDPerHour = "3/2" }, + wantErr: false, + }, + { + name: "integer AmountUSDPerHour is valid", + mutate: func(tr *Tranche) { tr.AmountUSDPerHour = "5" }, + wantErr: false, + }, + { + name: "unknown Layer is invalid", + mutate: func(tr *Tranche) { tr.Layer = "bogus-layer" }, + wantErr: true, + }, + { + name: "empty Layer is invalid", + mutate: func(tr *Tranche) { tr.Layer = "" }, + wantErr: true, + }, + { + name: "empty Term is invalid", + mutate: func(tr *Tranche) { tr.Term = "" }, + wantErr: true, + }, + { + name: "empty PaymentOption is invalid", + mutate: func(tr *Tranche) { tr.PaymentOption = "" }, + wantErr: true, + }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { From 82589ec277f04ceb4ce513e1958631e9cb398f03 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 3 Jul 2026 15:40:35 +0200 Subject: [PATCH 2/8] fix(ladder): detect duplicate tranche IDs and tighten rationale rendering Reject empty or repeated IDs from the injected NewID across all tranches of a run, since SaveTranches upserts by ID and a collision would silently drop a scheduled purchase. Render step fractions with %.4g so small percentages no longer show as 0% on positive purchases, require a non-empty rationale on every input allocation, and align the tranche amount validation docs with what big.Rat SetString actually enforces. --- pkg/ladder/ladder.go | 53 ++++++++++++++++++++---- pkg/ladder/ladder_test.go | 84 +++++++++++++++++++++++++++++++++++++++ pkg/ladder/store.go | 32 ++++++++------- 3 files changed, 146 insertions(+), 23 deletions(-) diff --git a/pkg/ladder/ladder.go b/pkg/ladder/ladder.go index 889ccfea7..3977985e8 100644 --- a/pkg/ladder/ladder.go +++ b/pkg/ladder/ladder.go @@ -104,9 +104,12 @@ func validateTrancheInput(in *TrancheInput) error { return validateInputAllocations(in.Allocations) } -// validateInputAllocations checks each allocation for a recognized layer and -// a positive gap. An empty allocation slice is valid (BuildTranches returns an -// empty result) but each element must be well-formed. +// validateInputAllocations checks each allocation for a recognized layer, a +// positive gap, and a non-empty rationale. An empty allocation slice is valid +// (BuildTranches returns an empty result) but each element must be +// well-formed. Every Allocation produced by Allocate carries a rationale, so +// the rationale check only catches direct API misuse; it still fails loud +// because the rationale feeds money-path audit trails and approval emails. func validateInputAllocations(allocs []Allocation) error { for i, a := range allocs { if err := a.Layer.Validate(); err != nil { @@ -115,12 +118,16 @@ func validateInputAllocations(allocs []Allocation) error { if a.GapUSDPerHour == nil || a.GapUSDPerHour.Sign() <= 0 { return fmt.Errorf("allocation[%d]: gap_usd_per_hour must be positive", i) } + if a.Rationale == "" { + return fmt.Errorf("allocation[%d]: rationale is required (money-path auditability)", i) + } } return nil } // buildTrancheResult iterates over all allocations and splits each one across -// the configured ramp schedule steps. +// the configured ramp schedule steps, then verifies that the injected NewID +// produced a unique, non-empty ID for every tranche of the run. func buildTrancheResult(in *TrancheInput) (*TrancheResult, error) { steps := in.Config.Ramp.Steps result := &TrancheResult{} @@ -129,9 +136,36 @@ func buildTrancheResult(in *TrancheInput) (*TrancheResult, error) { return nil, err } } + if err := validateTrancheIDs(result.Tranches); err != nil { + return nil, err + } return result, nil } +// validateTrancheIDs verifies that every produced tranche carries a unique, +// non-empty ID. SaveTranches upserts by tranche ID, so a duplicate or empty +// ID from a misbehaving injected NewID would silently collapse scheduled +// purchases at persistence time -- money-path data loss. Fail loud naming +// the offender instead. (An empty ID is already rejected earlier by +// Tranche.Validate inside buildFutureTranche; the check here is kept so this +// function is a self-contained guarantee over the whole batch.) +func validateTrancheIDs(tranches []Tranche) error { + seen := make(map[string]struct{}, len(tranches)) + for i := range tranches { + tr := &tranches[i] + if tr.ID == "" { + return fmt.Errorf("tranche[%d] (layer %s, step %d): NewID returned an empty ID", i, tr.Layer, tr.StepIndex) + } + if _, dup := seen[tr.ID]; dup { + return fmt.Errorf( + "tranche[%d] (layer %s, step %d): NewID returned duplicate ID %q; upsert-by-ID would silently drop a scheduled purchase", + i, tr.Layer, tr.StepIndex, tr.ID) + } + seen[tr.ID] = struct{}{} + } + return nil +} + // processAllocation splits one allocation across all ramp steps. Each step // amount is clamped to the remaining unallocated gap (see computeStepAmount) // and the last step receives the exact leftover, so the sum of all produced @@ -193,8 +227,8 @@ func appendStep(in *TrancheInput, alloc Allocation, step RampStep, stepIdx, nSte // For all steps except the last: amount = min(gap * fraction, remaining), // where remaining = max(gap - priorSum, 0) and fraction is converted to a // rational via big.Rat.SetFloat64 at the boundary (same discipline as -// ratFromFloat in allocate.go). RampSchedule.Validate ensures the fraction is -// in (0, 1], so NaN/Inf/negative cannot occur here. +// ratFromFloat in allocate.go). RampSchedule.Validate rejects NaN and bounds +// fractions to (0, 1], so NaN/Inf/negative cannot occur here. // // For the last step: amount = remaining (the exact leftover, floored at 0). // @@ -222,10 +256,13 @@ func computeStepAmount(gap *big.Rat, fraction float64, isLast bool, priorSum *bi // stepRationale returns a human-readable rationale string for a buy-now action // or future tranche, embedding the step position, fraction percentage, computed -// amount, total gap, and the allocation's original rationale. +// amount, total gap, and the allocation's original rationale. The percentage +// uses %.4g so small fractions render meaningfully (0.4% instead of the +// misleading "0%" that fixed zero-decimal rounding would produce on a +// positive purchase). func stepRationale(alloc Allocation, stepIdx, totalSteps int, fraction float64, amount, gap *big.Rat) string { return fmt.Sprintf( - "ramp step %d/%d (%.0f%%): %s of %s total. %s", + "ramp step %d/%d (%.4g%%): %s of %s total. %s", stepIdx+1, totalSteps, fraction*100, fmtRatUSD(amount), fmtRatUSD(gap), alloc.Rationale, diff --git a/pkg/ladder/ladder_test.go b/pkg/ladder/ladder_test.go index c787273a8..bdbce5a12 100644 --- a/pkg/ladder/ladder_test.go +++ b/pkg/ladder/ladder_test.go @@ -594,6 +594,17 @@ func TestBuildTranches_ValidationFailures(t *testing.T) { return baseInput(goodConfig, []Allocation{bad}, now) }, }, + { + name: "allocation with empty rationale", + build: func() *TrancheInput { + bad := Allocation{ + Layer: LayerComputeSP, + GapUSDPerHour: new(big.Rat).SetInt64(1), + Rationale: "", + } + return baseInput(goodConfig, []Allocation{bad}, now) + }, + }, } for _, c := range cases { @@ -862,3 +873,76 @@ func TestBuildTranches_EpsilonOvershootClamped(t *testing.T) { } assertAllValid(t, result) } + +// TestBuildTranches_DuplicateIDsRejected verifies that a misbehaving injected +// NewID producing repeated IDs is rejected with an explicit error rather than +// silently collapsing tranches at SaveTranches upsert time, while a +// well-behaved sequential generator succeeds. +func TestBuildTranches_DuplicateIDsRejected(t *testing.T) { + t.Parallel() + now := time.Date(2026, 8, 15, 0, 0, 0, 0, time.UTC) + + build := func(newID func() string) (*TrancheResult, error) { + in := baseInput(baseConfig(delayedRamp()), []Allocation{ + mkAlloc(LayerComputeSP, 4), + }, now) + in.NewID = newID + return BuildTranches(in) + } + + // Constant generator: two delayed steps get the same ID -> explicit error. + _, err := build(func() string { return "same-id" }) + if err == nil { + t.Fatalf("BuildTranches() = nil error, want duplicate-ID error for constant NewID") + } + if !strings.Contains(err.Error(), "duplicate ID") || !strings.Contains(err.Error(), "same-id") { + t.Errorf("duplicate-ID error %q must name the offender and the duplication", err) + } + + // Empty generator: rejected loudly (never persisted with a blank key). + _, err = build(func() string { return "" }) + if err == nil { + t.Fatalf("BuildTranches() = nil error, want error for empty-string NewID") + } + + // Sequential generator: unique IDs -> success. + result, err := build(seqID("ok")) + if err != nil { + t.Fatalf("BuildTranches() error = %v, want nil for sequential NewID", err) + } + if len(result.Tranches) != 2 { + t.Errorf("Tranches count = %d, want 2", len(result.Tranches)) + } + assertAllValid(t, result) +} + +// TestBuildTranches_SmallFractionRationale verifies that a small step fraction +// renders with meaningful precision in the rationale (e.g. "0.4%") instead of +// the misleading "(0%)" that zero-decimal rounding would produce on a +// positive purchase. +func TestBuildTranches_SmallFractionRationale(t *testing.T) { + t.Parallel() + now := time.Date(2026, 9, 15, 0, 0, 0, 0, time.UTC) + ramp := RampSchedule{Steps: []RampStep{ + {AfterDays: 0, Fraction: 0.004}, + {AfterDays: 30, Fraction: 0.996}, + }} + in := baseInput(baseConfig(ramp), []Allocation{ + mkAlloc(LayerComputeSP, 100), + }, now) + + result, err := BuildTranches(in) + if err != nil { + t.Fatalf("BuildTranches() error = %v", err) + } + if len(result.BuyNow) != 1 { + t.Fatalf("BuyNow count = %d, want 1", len(result.BuyNow)) + } + r := result.BuyNow[0].Rationale + if !strings.Contains(r, "(0.4%)") { + t.Errorf("rationale %q must render the small fraction as \"(0.4%%)\"", r) + } + if strings.Contains(r, "(0%)") { + t.Errorf("rationale %q renders a positive purchase as \"(0%%)\"", r) + } +} diff --git a/pkg/ladder/store.go b/pkg/ladder/store.go index 96ec21781..1b6103d79 100644 --- a/pkg/ladder/store.go +++ b/pkg/ladder/store.go @@ -149,12 +149,12 @@ type Tranche struct { FireAfter time.Time // FiredAt is nil until the tranche actually fires. FiredAt *time.Time - // AmountUSDPerHour is the hourly commitment delta for this tranche step, - // encoded as a big.Rat string (via big.Rat.RatString()). The string - // encoding is lossless and round-trips through big.Rat.SetString without - // floating-point precision loss, making it safe for DB persistence and - // rehydration. Must be a positive rational; use big.Rat.RatString() to - // produce the value and big.Rat.SetString to rehydrate it. + // AmountUSDPerHour is the hourly commitment delta for this tranche step. + // Validation requires any string that big.Rat.SetString parses as a + // positive rational (e.g. "3/2", "1.5"); producers should emit the + // canonical big.Rat.RatString() form, which is lossless and round-trips + // through big.Rat.SetString without floating-point precision loss, + // making it safe for DB persistence and rehydration. AmountUSDPerHour string ID string RunID string @@ -176,10 +176,10 @@ type Tranche struct { // Validate checks that the tranche is self-consistent: non-empty ID and // RunID (RunID is the single source of run linkage, see // LadderStore.SaveTranches), non-negative step index, a set FireAfter -// timestamp, complete purchase-execution fields (recognized Layer, positive -// AmountUSDPerHour encoded as a RatString, non-empty Term and PaymentOption), -// recognized status, and a FiredAt timestamp only when the status implies the -// tranche fired. +// timestamp, complete purchase-execution fields (recognized Layer, an +// AmountUSDPerHour parsing as a positive rational, non-empty Term and +// PaymentOption), recognized status, and a FiredAt timestamp only when the +// status implies the tranche fired. func (t *Tranche) Validate() error { if t.ID == "" { return fmt.Errorf("tranche ID is required") @@ -229,16 +229,18 @@ func (t *Tranche) validateExecutionFields() error { } // validateAmountUSDPerHour checks that AmountUSDPerHour is a non-empty string -// encoding a positive rational value. The RatString encoding (produced by -// big.Rat.RatString and parsed by big.Rat.SetString) is lossless and safe for -// DB round-tripping without floating-point precision loss. +// that parses as a positive rational via big.Rat.SetString. SetString accepts +// any rational notation ("3/2", "1.5", "2e10"), so enforcement is exactly +// "must parse as a positive rational" -- producers should still emit the +// canonical big.Rat.RatString() form, which is lossless and safe for DB +// round-tripping without floating-point precision loss. func (t *Tranche) validateAmountUSDPerHour() error { if t.AmountUSDPerHour == "" { - return fmt.Errorf("amount_usd_per_hour is required (positive rational encoded as big.Rat.RatString, e.g. \"3/2\")") + return fmt.Errorf("amount_usd_per_hour is required (must parse as a positive rational via big.Rat SetString, e.g. \"3/2\")") } r := new(big.Rat) if _, ok := r.SetString(t.AmountUSDPerHour); !ok { - return fmt.Errorf("amount_usd_per_hour %q is not a valid rational string (use big.Rat.RatString() encoding)", t.AmountUSDPerHour) + return fmt.Errorf("amount_usd_per_hour %q must parse as a positive rational (big.Rat SetString)", t.AmountUSDPerHour) } if r.Sign() <= 0 { return fmt.Errorf("amount_usd_per_hour must be positive, got %s", t.AmountUSDPerHour) From 5ec48c8cb264c0f9433bd1a23ae72038a946bca1 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 3 Jul 2026 15:56:51 +0200 Subject: [PATCH 3/8] fix(ladder): adopt typed term and payment enums in tranche building Switch TrancheInput and Tranche Term/PaymentOption fields from raw strings to the ladder.Term and ladder.PaymentOption enums introduced on feat/ladder-types, validate them via the enum Validate methods instead of non-empty checks, and move all test fixtures to the typed constants. Unknown enum values are now rejected at the tranche-building boundary in addition to empty ones. --- pkg/ladder/ladder.go | 24 +++++++++++++----------- pkg/ladder/ladder_test.go | 32 ++++++++++++++++++++++++-------- pkg/ladder/store.go | 34 ++++++++++++++++++---------------- pkg/ladder/store_test.go | 14 ++++++++++++-- 4 files changed, 67 insertions(+), 37 deletions(-) diff --git a/pkg/ladder/ladder.go b/pkg/ladder/ladder.go index 3977985e8..4982f6357 100644 --- a/pkg/ladder/ladder.go +++ b/pkg/ladder/ladder.go @@ -15,13 +15,15 @@ type TrancheInput struct { Config *LadderConfig // RunID stamps every produced Tranche row for run linkage. Required. RunID string - // Term is the commitment term (e.g. "1yr", "3yr") passed to each buy-now - // purchase action. The caller selects the term; v1 uses a single term for - // all allocations. - Term string - // PaymentOption is the payment structure (e.g. "no-upfront") passed to - // each buy-now purchase action. - PaymentOption string + // Term is the commitment term (Term1Year or Term3Year) passed to each + // buy-now purchase action and future tranche. The caller selects the + // term; v1 uses a single term for all allocations. Must pass + // Term.Validate. + Term Term + // PaymentOption is the payment structure (e.g. PaymentNoUpfront) passed + // to each buy-now purchase action and future tranche. Must pass + // PaymentOption.Validate. + PaymentOption PaymentOption // NewID is called once per produced Tranche to assign a unique identifier. // Callers may inject a UUID function, a sequential counter, or any other // scheme. Must return a non-empty string on every call. @@ -95,11 +97,11 @@ func validateTrancheInput(in *TrancheInput) error { if in.NewID == nil { return fmt.Errorf("new_id must not be nil (inject an ID generator)") } - if in.Term == "" { - return fmt.Errorf("term is required (e.g. \"1yr\" or \"3yr\")") + if err := in.Term.Validate(); err != nil { + return fmt.Errorf("term: %w", err) } - if in.PaymentOption == "" { - return fmt.Errorf("payment_option is required (e.g. \"no-upfront\")") + if err := in.PaymentOption.Validate(); err != nil { + return fmt.Errorf("payment_option: %w", err) } return validateInputAllocations(in.Allocations) } diff --git a/pkg/ladder/ladder_test.go b/pkg/ladder/ladder_test.go index bdbce5a12..b0841ab6b 100644 --- a/pkg/ladder/ladder_test.go +++ b/pkg/ladder/ladder_test.go @@ -88,8 +88,8 @@ func baseInput(cfg *LadderConfig, allocs []Allocation, now time.Time) *TrancheIn Config: cfg, Allocations: allocs, RunID: "run-abc", - Term: "1yr", - PaymentOption: "no-upfront", + Term: Term1Year, + PaymentOption: PaymentNoUpfront, Now: now, NewID: seqID("tr"), } @@ -289,8 +289,8 @@ func TestBuildTranches_MultipleAllocations(t *testing.T) { Config: cfg, Allocations: allocs, RunID: "run-multi", - Term: "1yr", - PaymentOption: "no-upfront", + Term: Term1Year, + PaymentOption: PaymentNoUpfront, Now: now, NewID: seqID("id"), } @@ -458,8 +458,8 @@ func TestBuildTranches_IDsStamped(t *testing.T) { mkAlloc(LayerConvertibleRI, 2), }, RunID: "run-id-stamp-test", - Term: "1yr", - PaymentOption: "no-upfront", + Term: Term1Year, + PaymentOption: PaymentNoUpfront, Now: now, NewID: seqID("myid"), } @@ -550,6 +550,22 @@ func TestBuildTranches_ValidationFailures(t *testing.T) { return in }, }, + { + name: "unknown Term", + build: func() *TrancheInput { + in := baseInput(goodConfig, []Allocation{goodAlloc}, now) + in.Term = "2yr" + return in + }, + }, + { + name: "unknown PaymentOption", + build: func() *TrancheInput { + in := baseInput(goodConfig, []Allocation{goodAlloc}, now) + in.PaymentOption = "monthly" + return in + }, + }, { name: "allocation with invalid layer", build: func() *TrancheInput { @@ -697,8 +713,8 @@ func TestBuildTranches_RationaleContents(t *testing.T) { DataSources: []string{"cost-explorer"}, }}, RunID: "run-rationale", - Term: "1yr", - PaymentOption: "no-upfront", + Term: Term1Year, + PaymentOption: PaymentNoUpfront, Now: now, NewID: seqID("r"), } diff --git a/pkg/ladder/store.go b/pkg/ladder/store.go index 1b6103d79..a055dc4f4 100644 --- a/pkg/ladder/store.go +++ b/pkg/ladder/store.go @@ -163,13 +163,15 @@ type Tranche struct { // Required so a fired tranche is executable without consulting the parent // run's plan. Layer LayerType - // Term is the commitment term for the purchase (e.g. "1yr", "3yr"). - // Required: an empty term would silently default at the provider boundary - // (money-shaping field, same rule as PlannedAction.Term). - Term string + // Term is the commitment term for the purchase (Term1Year or Term3Year). + // Must pass Term.Validate: an unset or unknown term would silently + // default at the provider boundary (money-shaping field, same rule as + // PlannedAction.Term). + Term Term // PaymentOption is the payment structure for the purchase (e.g. - // "no-upfront"). Required for the same reason as Term. - PaymentOption string + // PaymentNoUpfront). Must pass PaymentOption.Validate, for the same + // reason as Term. + PaymentOption PaymentOption StepIndex int } @@ -177,9 +179,9 @@ type Tranche struct { // RunID (RunID is the single source of run linkage, see // LadderStore.SaveTranches), non-negative step index, a set FireAfter // timestamp, complete purchase-execution fields (recognized Layer, an -// AmountUSDPerHour parsing as a positive rational, non-empty Term and -// PaymentOption), recognized status, and a FiredAt timestamp only when the -// status implies the tranche fired. +// AmountUSDPerHour parsing as a positive rational, valid Term and +// PaymentOption enum values), recognized status, and a FiredAt timestamp +// only when the status implies the tranche fired. func (t *Tranche) Validate() error { if t.ID == "" { return fmt.Errorf("tranche ID is required") @@ -209,9 +211,9 @@ func (t *Tranche) Validate() error { } // validateExecutionFields checks the fields that make a tranche executable as -// a standalone purchase: a recognized Layer, a positive amount, and non-empty -// Term and PaymentOption. Split out of Validate to keep each function's -// cyclomatic complexity within the repo limit. +// a standalone purchase: a recognized Layer, a positive amount, and valid +// Term and PaymentOption enum values. Split out of Validate to keep each +// function's cyclomatic complexity within the repo limit. func (t *Tranche) validateExecutionFields() error { if err := t.Layer.Validate(); err != nil { return fmt.Errorf("layer: %w", err) @@ -219,11 +221,11 @@ func (t *Tranche) validateExecutionFields() error { if err := t.validateAmountUSDPerHour(); err != nil { return err } - if t.Term == "" { - return fmt.Errorf("term is required (money-shaping field; empty would silently default at the provider boundary)") + if err := t.Term.Validate(); err != nil { + return fmt.Errorf("term: %w", err) } - if t.PaymentOption == "" { - return fmt.Errorf("payment_option is required (money-shaping field; empty would silently default at the provider boundary)") + if err := t.PaymentOption.Validate(); err != nil { + return fmt.Errorf("payment_option: %w", err) } return nil } diff --git a/pkg/ladder/store_test.go b/pkg/ladder/store_test.go index f63cf71ad..8826898cf 100644 --- a/pkg/ladder/store_test.go +++ b/pkg/ladder/store_test.go @@ -158,8 +158,8 @@ func TestTrancheValidate(t *testing.T) { Status: TrancheStatusScheduled, AmountUSDPerHour: "2/1", Layer: LayerComputeSP, - Term: "1yr", - PaymentOption: "no-upfront", + Term: Term1Year, + PaymentOption: PaymentNoUpfront, } cases := []struct { mutate func(tr *Tranche) @@ -293,6 +293,16 @@ func TestTrancheValidate(t *testing.T) { mutate: func(tr *Tranche) { tr.PaymentOption = "" }, wantErr: true, }, + { + name: "unknown Term is invalid", + mutate: func(tr *Tranche) { tr.Term = "2yr" }, + wantErr: true, + }, + { + name: "unknown PaymentOption is invalid", + mutate: func(tr *Tranche) { tr.PaymentOption = "biannual" }, + wantErr: true, + }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { From 9180e2674cceb332637a315a2358e8be2481352c Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 3 Jul 2026 21:21:03 +0200 Subject: [PATCH 4/8] feat(aws): add ladder capability read side AWSLadder implements the pkg/ladder.LadderCapability read methods: commitment listing merging convertible RIs and savings plans with reservation-total costing (InstanceCount, UsagePrice, amortized upfront); layer states honoring the explicit-zeros contract; nearest-rank percentile usage baseline with strict series validation. Write side stubbed for PR 6. Part of #1335. --- providers/aws/ladder/baseline.go | 158 ++++++ providers/aws/ladder/commitments.go | 144 +++++ providers/aws/ladder/interfaces.go | 133 +++++ providers/aws/ladder/ladder.go | 160 ++++++ providers/aws/ladder/ladder_test.go | 814 +++++++++++++++++++++++++++ providers/aws/ladder/layer_states.go | 342 +++++++++++ 6 files changed, 1751 insertions(+) create mode 100644 providers/aws/ladder/baseline.go create mode 100644 providers/aws/ladder/commitments.go create mode 100644 providers/aws/ladder/interfaces.go create mode 100644 providers/aws/ladder/ladder.go create mode 100644 providers/aws/ladder/ladder_test.go create mode 100644 providers/aws/ladder/layer_states.go diff --git a/providers/aws/ladder/baseline.go b/providers/aws/ladder/baseline.go new file mode 100644 index 000000000..1a6fa461c --- /dev/null +++ b/providers/aws/ladder/baseline.go @@ -0,0 +1,158 @@ +package ladder + +import ( + "context" + "fmt" + "math" + "sort" + + "github.com/LeanerCloud/CUDly/pkg/ladder" +) + +// minBaselineSeriesDays is the shortest daily series AWSLadder will accept for +// baseline computation. Fewer days cannot produce a statistically meaningful +// low-water-mark; GetUsageBaseline returns an error when the series is shorter. +// The caller (engine) should configure LookbackDays >= minBaselineSeriesDays. +const minBaselineSeriesDays = 7 + +// GetUsageBaseline computes a statistical low-water-mark from a daily +// on-demand-equivalent USD/hour series returned by the injected coverageSource. +// +// Series semantics: each element is the average on-demand-equivalent USD/hour +// for one calendar day over the lookback window, ordered oldest-to-newest. +// The series is sourced from coverageSource.GetOnDemandSeries, which is wired +// in a later PR to call CE GetCostAndUsage (Granularity=Daily, on-demand +// usage-type filter). Until that wiring lands, callers receive a data-source +// error from GetOnDemandSeries. +// +// Limitations: +// - The series covers only the on-demand costs reported by the coverage +// source. Until the CE GetCostAndUsage wiring lands, this will be an error. +// - The series is EC2-scoped (the initial coverage source implementation +// covers EC2 on-demand cost only, not RDS/ElastiCache/etc.). +// - LowWaterUSDPerHour is the nearest-rank percentile of the daily series +// (see nearestRankPercentile). +// - StableUSDPerHour is nil: per the pkg/ladder contract (types.go), Stable +// is the post-buffer-fraction estimate — a producer obligation this +// implementation cannot yet meet (no stable-usage estimator exists for +// AWS). Returning nil triggers the engine's documented degradation +// ("stable usage unknown; routing all core gap to flex"), which is honest +// and conservative. Do NOT alias it to LowWater: the engine consumes +// Stable verbatim as the base-layer cap and would over-commit the base. +// +// Error conditions: +// - Series empty: hard error (no data from the coverage source). +// - Series shorter than minBaselineSeriesDays: hard error (insufficient +// history for a reliable percentile estimate). +// - Series containing a non-finite (NaN/Inf) or negative element: hard error +// naming the offending index (a cost series must be finite and >= 0). +// - percentile not in (0, 100]: hard error (out-of-range). +func (a *AWSLadder) GetUsageBaseline(ctx context.Context, scope ladder.Scope, lookbackDays int, percentile float64) (ladder.UsageBaseline, error) { + if err := a.validateScope(scope); err != nil { + return ladder.UsageBaseline{}, err + } + if err := validateBaselineArgs(lookbackDays, percentile); err != nil { + return ladder.UsageBaseline{}, err + } + + series, err := a.coverage.GetOnDemandSeries(ctx, a.cfg.Region, lookbackDays) + if err != nil { + return ladder.UsageBaseline{}, fmt.Errorf("GetUsageBaseline: on-demand series fetch failed: %w", err) + } + if len(series) == 0 { + return ladder.UsageBaseline{}, fmt.Errorf("GetUsageBaseline: on-demand series is empty for region %s (coverage source returned no data)", a.cfg.Region) + } + if len(series) < minBaselineSeriesDays { + return ladder.UsageBaseline{}, fmt.Errorf( + "GetUsageBaseline: series length %d is below minimum %d days; extend the lookback window or check the coverage source", + len(series), minBaselineSeriesDays, + ) + } + if vErr := validateSeries(series); vErr != nil { + return ladder.UsageBaseline{}, fmt.Errorf("GetUsageBaseline: %w", vErr) + } + + lowWater, err := nearestRankPercentile(series, percentile) + if err != nil { + return ladder.UsageBaseline{}, fmt.Errorf("GetUsageBaseline: percentile computation failed: %w", err) + } + + // StableUSDPerHour is intentionally nil: the pkg/ladder contract defines + // Stable as the post-buffer-fraction estimate (a producer obligation), and + // no stable-usage estimator exists for AWS yet. nil triggers the engine's + // documented "stable usage unknown; routing all core gap to flex" + // degradation — honest and conservative until a real estimator lands. + return ladder.UsageBaseline{ + LowWaterUSDPerHour: ptr(lowWater), + StableUSDPerHour: nil, + Series: series, + LookbackDays: lookbackDays, + Percentile: percentile, + }, nil +} + +// validateSeries rejects series containing non-finite (NaN/Inf) or negative +// elements at the trust boundary: the series is injected via coverageSource, +// and a single bad element would silently corrupt the percentile (NaN makes +// the sort order undefined; a negative cost is impossible for on-demand spend). +// The error names the offending index so the data-source bug is traceable. +func validateSeries(series []float64) error { + for i, v := range series { + if math.IsNaN(v) || math.IsInf(v, 0) { + return fmt.Errorf("series element at index %d is not finite (%g); the on-demand series must contain only finite values", i, v) + } + if v < 0 { + return fmt.Errorf("series element at index %d is negative (%g); on-demand cost values must be >= 0", i, v) + } + } + return nil +} + +// validateBaselineArgs checks lookbackDays and percentile for out-of-range +// values. Extracted to keep GetUsageBaseline under the cyclomatic limit. +func validateBaselineArgs(lookbackDays int, percentile float64) error { + if lookbackDays <= 0 { + return fmt.Errorf("GetUsageBaseline: lookbackDays %d must be > 0", lookbackDays) + } + if math.IsNaN(percentile) || !(percentile > 0 && percentile <= 100) { + return fmt.Errorf("GetUsageBaseline: percentile %g must be in (0, 100]", percentile) + } + return nil +} + +// nearestRankPercentile returns the p-th percentile of values using the +// nearest-rank method (NIST definition): +// +// rank = ceil(p/100 * N) (1-indexed, clamped to [1, N]) +// result = sorted_values[rank-1] +// +// Properties: +// - Exact: no interpolation. The result is always a member of the input set. +// - NaN-hostile: if any value in data is NaN, the sort is undefined and +// the result is meaningless. GetUsageBaseline enforces finiteness via +// validateSeries before calling this function; other callers must do the +// same. +// - Empty slice: returns an error (caller must guard before calling). +// - p==100: returns the maximum element (rank=N). +// - p close to 0: rank rounds up to 1, returning the minimum element. +// +// No external statistics package is imported; this keeps the ladder package +// dependency-free for test purposes. +func nearestRankPercentile(data []float64, p float64) (float64, error) { + if len(data) == 0 { + return 0, fmt.Errorf("nearestRankPercentile: empty data slice") + } + sorted := make([]float64, len(data)) + copy(sorted, data) + sort.Float64s(sorted) + + n := float64(len(sorted)) + rank := int(math.Ceil(p / 100.0 * n)) + if rank < 1 { + rank = 1 + } + if rank > len(sorted) { + rank = len(sorted) + } + return sorted[rank-1], nil +} diff --git a/providers/aws/ladder/commitments.go b/providers/aws/ladder/commitments.go new file mode 100644 index 000000000..95c19d8c8 --- /dev/null +++ b/providers/aws/ladder/commitments.go @@ -0,0 +1,144 @@ +package ladder + +import ( + "context" + "fmt" + + "github.com/LeanerCloud/CUDly/pkg/common" + "github.com/LeanerCloud/CUDly/pkg/ladder" + ec2svc "github.com/LeanerCloud/CUDly/providers/aws/services/ec2" +) + +// ListCommitments returns all active commitments for the given scope by merging: +// - Active convertible RIs (riLister.ListConvertibleReservedInstances) +// - Active Savings Plans of type EC2Instance and Compute (spLister.ListActiveSPs) +// +// The scope's AccountID must match Config.AccountID; this implementation is +// single-account and returns an error when the scope targets a different account. +func (a *AWSLadder) ListCommitments(ctx context.Context, scope ladder.Scope) ([]common.Commitment, error) { + if err := a.validateScope(scope); err != nil { + return nil, err + } + + riCommitments, err := a.listRICommitments(ctx) + if err != nil { + return nil, fmt.Errorf("ListCommitments: RI listing failed: %w", err) + } + + spCommitments, err := a.listSPCommitments(ctx) + if err != nil { + return nil, fmt.Errorf("ListCommitments: SP listing failed: %w", err) + } + + result := make([]common.Commitment, 0, len(riCommitments)+len(spCommitments)) + result = append(result, riCommitments...) + result = append(result, spCommitments...) + return result, nil +} + +// validateScope returns an error when scope targets a provider or account that +// does not match this AWSLadder instance. Fails loud rather than silently +// returning data for the wrong account. +func (a *AWSLadder) validateScope(scope ladder.Scope) error { + if scope.Provider != common.ProviderAWS { + return fmt.Errorf("AWSLadder: expected provider %s, got %s", common.ProviderAWS, scope.Provider) + } + if scope.AccountID != a.cfg.AccountID { + return fmt.Errorf("AWSLadder: scope account %s does not match configured account %s", + scope.AccountID, a.cfg.AccountID) + } + return nil +} + +// listRICommitments fetches active convertible RIs and maps them to +// common.Commitment values. Only RIs in "active" state are included; +// payment-pending RIs are excluded because their hourly costs are not +// yet finalized. +func (a *AWSLadder) listRICommitments(ctx context.Context) ([]common.Commitment, error) { + ris, err := a.ris.ListConvertibleReservedInstances(ctx) + if err != nil { + return nil, err + } + out := make([]common.Commitment, 0, len(ris)) + for i := range ris { + out = append(out, riToCommitment(&ris[i], a.cfg.AccountID, a.cfg.Region)) + } + return out, nil +} + +// riToCommitment converts a ConvertibleRI to a common.Commitment. +// ri is taken by pointer to avoid copying the large ConvertibleRI struct (hugeParam). +// +// Cost is the RESERVATION-TOTAL hourly amortized cost, computed by riHourlyCost: +// DescribeReservedInstances pricing fields (RecurringHourlyAmount, UsagePrice, +// FixedPrice) are per-instance, so the per-instance hourly rate is multiplied +// by InstanceCount. See riHourlyCost for the formula and semantics (matches the +// repo's canonical monthlyCostFromConvertibleRI in internal/api/handler_ri_exchange.go). +func riToCommitment(ri *ec2svc.ConvertibleRI, accountID, region string) common.Commitment { + totalHourlyCost := riHourlyCost(ri) + + return common.Commitment{ + Provider: common.ProviderAWS, + Account: accountID, + CommitmentID: ri.ReservedInstanceID, + CommitmentType: common.CommitmentReservedInstance, + Service: common.ServiceEC2, + Region: region, + ResourceType: ri.InstanceType, + Count: int(ri.InstanceCount), + State: ri.State, + StartDate: ri.Start, + EndDate: ri.End, + Cost: totalHourlyCost, + } +} + +// listSPCommitments fetches active EC2Instance and Compute Savings Plans and +// maps them to common.Commitment values. Other plan types (SageMaker, Database) +// are filtered out because they do not belong to any of the three ladder layers. +func (a *AWSLadder) listSPCommitments(ctx context.Context) ([]common.Commitment, error) { + sps, err := a.sps.ListActiveSPs(ctx) + if err != nil { + return nil, err + } + out := make([]common.Commitment, 0, len(sps)) + for i := range sps { + if !isLadderSPType(sps[i].PlanType) { + continue + } + out = append(out, spToCommitment(&sps[i], a.cfg.AccountID)) + } + return out, nil +} + +// isLadderSPType returns true for the two plan types that map to ladder layers. +func isLadderSPType(planType string) bool { + return planType == "EC2Instance" || planType == "Compute" +} + +// spToCommitment converts an ActiveSP to a common.Commitment. +// sp is taken by pointer to avoid copying the large ActiveSP struct (hugeParam). +// Cost is set to HourlyCommitmentUSD, which is the $/hr committed spend. +// The end date carries a zero value when AWS returns an empty End string +// (e.g. for queued plans); callers treat the zero time as "no expiry signal". +func spToCommitment(sp *ActiveSP, accountID string) common.Commitment { + service := common.ServiceSavingsPlansEC2Instance + if sp.PlanType == "Compute" { + service = common.ServiceSavingsPlansCompute + } + + return common.Commitment{ + Provider: common.ProviderAWS, + Account: accountID, + CommitmentID: sp.PlanID, + CommitmentType: common.CommitmentSavingsPlan, + Service: service, + Region: sp.Region, + ResourceType: sp.PlanType, + Count: 1, // Savings Plans are single commitment units + State: sp.State, + StartDate: sp.StartDate, + EndDate: sp.EndDate, + Cost: sp.HourlyCommitmentUSD, + } +} diff --git a/providers/aws/ladder/interfaces.go b/providers/aws/ladder/interfaces.go new file mode 100644 index 000000000..71bb536f0 --- /dev/null +++ b/providers/aws/ladder/interfaces.go @@ -0,0 +1,133 @@ +// Package ladder implements the ladder.LadderCapability READ side for AWS. +// Write-side methods (PurchaseLayer, ReshapeBuffer) return explicit +// not-implemented errors until the write-side PR lands. +package ladder + +import ( + "context" + "time" + + cetypes "github.com/aws/aws-sdk-go-v2/service/costexplorer/types" + + "github.com/LeanerCloud/CUDly/providers/aws/recommendations" + ec2svc "github.com/LeanerCloud/CUDly/providers/aws/services/ec2" +) + +// riLister is the narrow interface for listing active convertible RIs. +// The concrete implementation is ec2svc.Client.ListConvertibleReservedInstances. +type riLister interface { + ListConvertibleReservedInstances(ctx context.Context) ([]ec2svc.ConvertibleRI, error) +} + +// ActiveSP is a minimal view of an active Savings Plan needed by AWSLadder. +// Only EC2Instance and Compute plan types are relevant to the three ladder layers. +// +// Fields are ordered to minimize the GC pointer-scan range (fieldalignment). +type ActiveSP struct { + // PlanID is the AWS Savings Plan ID. + PlanID string + // PlanType is "EC2Instance" or "Compute" (matches the SavingsPlan.SavingsPlanType + // display form from the DescribeSavingsPlans response). + PlanType string + // State mirrors SavingsPlan.State as a string ("active", "pending-return", "queued"). + State string + // StartDate is the SP activation time. + StartDate time.Time + // EndDate is the SP expiry time. + EndDate time.Time + // Region is the AWS region; empty for Compute SPs (which are global). + Region string + // HourlyCommitmentUSD is the committed spend in USD per hour (SavingsPlan.Commitment + // parsed as float64). This is what the engine counts as ExistingUSDPerHour for SP layers. + // Placed last to keep all pointer-containing fields contiguous for the GC scanner. + HourlyCommitmentUSD float64 +} + +// spLister is the narrow interface for listing active Savings Plans relevant +// to the three ladder layers. The real implementation (wired when both PRs +// land) calls DescribeSavingsPlans filtering to Active state and maps the +// Commitment field to HourlyCommitmentUSD. Tests pass a hermetic fake. +type spLister interface { + ListActiveSPs(ctx context.Context) ([]ActiveSP, error) +} + +// coverageSource is the narrow interface for RI coverage data and the +// on-demand daily spend series used by GetUsageBaseline. +// +// GetRICoverageMap returns the per-pool org-wide RI coverage map (keyed by +// "region:instance_type" for EC2) for the given lookback window and regions. +// +// GetOnDemandSeries returns a slice of len(lookbackDays) daily on-demand- +// equivalent USD/hour values for the given region, ordered oldest-to-newest. +// Each element is the average on-demand spend in USD per hour for that +// calendar day. The real implementation sources this from CE GetCostAndUsage +// with Granularity=Daily filtered to on-demand usage types; wiring happens +// when the cost-and-usage collector PR lands. Tests pass a hermetic fake. +type coverageSource interface { + GetRICoverageMap(ctx context.Context, lookbackDays int, regions []string) (recommendations.PoolCoverageMap, error) + GetOnDemandSeries(ctx context.Context, region string, lookbackDays int) ([]float64, error) +} + +// utilizationSource is the narrow interface for RI utilization data. +// The real implementation is RecommendationsClientAdapter.GetRIUtilization. +type utilizationSource interface { + GetRIUtilization(ctx context.Context, lookbackDays int) ([]recommendations.RIUtilization, error) +} + +// SPCoverageSummary carries the Savings Plans coverage result from the CE API. +// The CE GetSavingsPlansCoverage API does not support filtering by plan type, +// so one summary covers all SP layers. When the parallel SP coverage PR +// (PR 4) lands, reconcile this type with the one it defines. +type SPCoverageSummary struct { + // CoveragePct is the percentage (0-100) of eligible compute spend covered + // by Savings Plans. Nil means the CE API returned no data for the scope. + CoveragePct *float64 +} + +// SPUtilizationSummary carries the Savings Plans utilization result from the +// CE API. Per-plan-type utilization is available via GetSavingsPlansUtilization, +// so each SP layer gets its own summary. When PR 4 lands, reconcile this type. +type SPUtilizationSummary struct { + // UtilizationPct is the percentage (0-100) of the committed spend that was + // actually used. Nil means the CE API returned no data for the scope. + UtilizationPct *float64 +} + +// spCoverageSource is the narrow interface for Savings Plans coverage data. +// It is wired when the SP coverage PR (parallel to this one, PR 4) lands. +// Pass nil to skip SP coverage measurement; CoveragePct will be nil for SP layers. +// +// CE API note: GetSavingsPlansCoverage does NOT support plan-type filtering. +// The returned coverage applies to ALL Savings Plan types in the region, so +// AWSLadder sets the same CoveragePct on both EC2Instance and Compute SP layers. +// +// Adapter requirement: Go interface satisfaction needs identical return types, +// and PR 4's concrete implementation returns its own richer +// recommendations.SPCoverageSummary (more fields, e.g. Days). The wiring PR +// must therefore provide a thin adapter mapping +// recommendations.SPCoverageSummary{CoveragePct, ..., Days} -> this package's +// SPCoverageSummary, preserving nil CoveragePct as no-data (PR 4 returns nil +// when Days==0). +type spCoverageSource interface { + GetSPCoverageSummary(ctx context.Context, region string, lookbackDays int) (SPCoverageSummary, error) +} + +// spUtilizationSource is the narrow interface for Savings Plans utilization data. +// It is wired when the SP utilization PR (parallel to this one, PR 4) lands. +// Pass nil to skip SP utilization measurement; UtilizationPct will be nil for SP layers. +// +// planType uses the CE SDK enum (cetypes.SupportedSavingsPlansTypeEc2InstanceSp, +// cetypes.SupportedSavingsPlansTypeComputeSp, etc.). +// region "" = all regions; pass "" for Compute SPs (global) and the configured +// region for EC2 Instance SPs. +// +// Adapter requirement: Go interface satisfaction needs identical return types, +// and PR 4's concrete implementation returns its own richer +// recommendations.SPUtilizationSummary (more fields, e.g. Days). The wiring PR +// must therefore provide a thin adapter mapping +// recommendations.SPUtilizationSummary{UtilizationPct, ..., Days} -> this +// package's SPUtilizationSummary, preserving nil UtilizationPct as no-data +// (PR 4 returns nil when Days==0). +type spUtilizationSource interface { + GetSPUtilization(ctx context.Context, planType cetypes.SupportedSavingsPlansType, region string, lookbackDays int) (SPUtilizationSummary, error) +} diff --git a/providers/aws/ladder/ladder.go b/providers/aws/ladder/ladder.go new file mode 100644 index 000000000..a1274c220 --- /dev/null +++ b/providers/aws/ladder/ladder.go @@ -0,0 +1,160 @@ +package ladder + +import ( + "context" + "errors" + "fmt" + + "github.com/LeanerCloud/CUDly/pkg/common" + "github.com/LeanerCloud/CUDly/pkg/ladder" +) + +// DefaultHorizonDays is the number of days ahead used to classify a +// commitment as "expiring soon" in GetLayerStates.ExpiringUSDPerHour. +// Callers that need a different window pass it via Config.HorizonDays. +const DefaultHorizonDays = 30 + +// DefaultLookbackDays is the number of days used for coverage and +// utilization queries when Config.LookbackDays is zero. +const DefaultLookbackDays = 30 + +// errWriteNotWired is the sentinel returned by PurchaseLayer and ReshapeBuffer +// until the write-side PR (PR 6) lands. It is distinct from +// common.ErrCommitmentPurchaseNotSupported, which signals that this provider +// can NEVER purchase a given layer type programmatically. Here the capability +// WILL be supported once wired; the error is a clear placeholder, not a +// permanent constraint. +var errWriteNotWired = errors.New("write side not yet wired (PR 6): call sites must not invoke PurchaseLayer or ReshapeBuffer until the write PR is merged") + +// Config holds construction-time parameters for AWSLadder. +type Config struct { + // Region is the AWS region this ladder instance operates on. + Region string + // AccountID is the AWS account ID this ladder instance is scoped to. + AccountID string + // HorizonDays is the look-ahead window (in days) used to classify a + // commitment as expiring soon in ExpiringUSDPerHour. When zero, + // DefaultHorizonDays is applied. + HorizonDays int + // LookbackDays is the history window (in days) for coverage and + // utilization queries. When zero, DefaultLookbackDays is applied. + LookbackDays int +} + +// horizonDays returns the effective horizon, applying the default when unset. +func (c Config) horizonDays() int { + if c.HorizonDays > 0 { + return c.HorizonDays + } + return DefaultHorizonDays +} + +// lookbackDays returns the effective lookback, applying the default when unset. +func (c Config) lookbackDays() int { + if c.LookbackDays > 0 { + return c.LookbackDays + } + return DefaultLookbackDays +} + +// AWSLadder implements ladder.LadderCapability for AWS. It provides the READ +// side (ListCommitments, GetLayerStates, GetUsageBaseline); the write side +// (PurchaseLayer, ReshapeBuffer) is wired in PR 6 and returns an explicit +// not-implemented error until then. +// +// All four data-source dependencies are injected via narrow interfaces so that +// unit tests are hermetic (no real AWS calls needed). The caller wires the +// concrete adapters (ec2svc.Client, savingsplans.Client, etc.) at startup. +// +// SP coverage and utilization (spCoverageSource, spUtilizationSource) may be +// nil; when nil, CoveragePct and UtilizationPct for SP layers are nil, which +// the engine treats as "not yet measured." They are wired when the parallel +// SP coverage PR lands. +// +// Fields are ordered to minimize the GC pointer-scan range (fieldalignment): +// interface fields (all-pointer) come before Config (which has trailing int fields). +type AWSLadder struct { + ris riLister + sps spLister + coverage coverageSource + utilization utilizationSource + spCoverage spCoverageSource // nil until parallel SP coverage PR (PR 4) lands + spUtil spUtilizationSource // nil until parallel SP utilization PR (PR 4) lands + cfg Config +} + +// New constructs an AWSLadder. All four required interfaces must be non-nil; +// spCoverage and spUtil may be nil (wired later). +func New( + cfg Config, + ris riLister, + sps spLister, + cov coverageSource, + util utilizationSource, + spCov spCoverageSource, + spUtil spUtilizationSource, +) (*AWSLadder, error) { + if cfg.Region == "" { + return nil, fmt.Errorf("AWSLadder: Config.Region must not be empty") + } + if cfg.AccountID == "" { + return nil, fmt.Errorf("AWSLadder: Config.AccountID must not be empty") + } + if ris == nil { + return nil, fmt.Errorf("AWSLadder: riLister must not be nil") + } + if sps == nil { + return nil, fmt.Errorf("AWSLadder: spLister must not be nil") + } + if cov == nil { + return nil, fmt.Errorf("AWSLadder: coverageSource must not be nil") + } + if util == nil { + return nil, fmt.Errorf("AWSLadder: utilizationSource must not be nil") + } + return &AWSLadder{ + cfg: cfg, + ris: ris, + sps: sps, + coverage: cov, + utilization: util, + spCoverage: spCov, + spUtil: spUtil, + }, nil +} + +// Provider returns common.ProviderAWS to identify this implementation. +func (a *AWSLadder) Provider() common.ProviderType { + return common.ProviderAWS +} + +// SupportedLayers returns the three AWS ladder layers: +// - LayerEC2InstanceSP carries RoleBase (EC2-family-locked SPs for the stable base). +// - LayerComputeSP carries RoleFlex (compute-wide SPs for flexible coverage). +// - LayerConvertibleRI carries RoleBuffer (exchangeable RIs for the reshapeable buffer). +// +// Role-cardinality contract: exactly one RoleFlex (ComputeSP), one RoleBase +// (EC2InstanceSP), one RoleBuffer (ConvertibleRI). No multi-role merges on AWS. +func (a *AWSLadder) SupportedLayers() []ladder.LayerSpec { + return []ladder.LayerSpec{ + {Type: ladder.LayerEC2InstanceSP, Roles: []ladder.LayerRole{ladder.RoleBase}}, + {Type: ladder.LayerComputeSP, Roles: []ladder.LayerRole{ladder.RoleFlex}}, + {Type: ladder.LayerConvertibleRI, Roles: []ladder.LayerRole{ladder.RoleBuffer}}, + } +} + +// PurchaseLayer is not yet wired. It returns an explicit placeholder error +// that is NOT common.ErrCommitmentPurchaseNotSupported (which would signal +// permanent inability to purchase). This error signals that the write-side +// wiring is missing; callers must not invoke this method until PR 6 is merged. +// +//nolint:gocritic // hugeParam: Recommendation is large but the LadderCapability interface contract requires value, not pointer +func (a *AWSLadder) PurchaseLayer(_ context.Context, _ ladder.LayerType, _ common.Recommendation, _ common.PurchaseOptions) (common.PurchaseResult, error) { + return common.PurchaseResult{}, fmt.Errorf("PurchaseLayer: %w", errWriteNotWired) +} + +// ReshapeBuffer is not yet wired. It returns the same placeholder error as +// PurchaseLayer; see that method's comment for the rationale. +func (a *AWSLadder) ReshapeBuffer(_ context.Context, _ ladder.Scope, _ ladder.BufferReshapeConfig) (ladder.ReshapeSummary, error) { + return ladder.ReshapeSummary{}, fmt.Errorf("ReshapeBuffer: %w", errWriteNotWired) +} diff --git a/providers/aws/ladder/ladder_test.go b/providers/aws/ladder/ladder_test.go new file mode 100644 index 000000000..adbeb0ee9 --- /dev/null +++ b/providers/aws/ladder/ladder_test.go @@ -0,0 +1,814 @@ +package ladder + +import ( + "context" + "errors" + "math" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + cetypes "github.com/aws/aws-sdk-go-v2/service/costexplorer/types" + + "github.com/LeanerCloud/CUDly/pkg/common" + "github.com/LeanerCloud/CUDly/pkg/ladder" + "github.com/LeanerCloud/CUDly/providers/aws/recommendations" + ec2svc "github.com/LeanerCloud/CUDly/providers/aws/services/ec2" +) + +// --------------------------------------------------------------------------- +// Fakes +// --------------------------------------------------------------------------- + +// fakeRILister: err field before ris to minimize GC pointer-scan range (fieldalignment). +type fakeRILister struct { + err error + ris []ec2svc.ConvertibleRI +} + +func (f *fakeRILister) ListConvertibleReservedInstances(_ context.Context) ([]ec2svc.ConvertibleRI, error) { + return f.ris, f.err +} + +// fakeSPLister: err field before sps for fieldalignment. +type fakeSPLister struct { + err error + sps []ActiveSP +} + +func (f *fakeSPLister) ListActiveSPs(_ context.Context) ([]ActiveSP, error) { + return f.sps, f.err +} + +// fakeCoverageSource: error fields before slice for fieldalignment +// (all-pointer types before slice whose trailing len/cap are non-pointer). +type fakeCoverageSource struct { + coverageErr error + onDemandErr error + coverageMap recommendations.PoolCoverageMap + onDemandSeries []float64 +} + +func (f *fakeCoverageSource) GetRICoverageMap(_ context.Context, _ int, _ []string) (recommendations.PoolCoverageMap, error) { + return f.coverageMap, f.coverageErr +} + +func (f *fakeCoverageSource) GetOnDemandSeries(_ context.Context, _ string, _ int) ([]float64, error) { + return f.onDemandSeries, f.onDemandErr +} + +// fakeUtilizationSource: err field before utils for fieldalignment. +type fakeUtilizationSource struct { + err error + utils []recommendations.RIUtilization +} + +func (f *fakeUtilizationSource) GetRIUtilization(_ context.Context, _ int) ([]recommendations.RIUtilization, error) { + return f.utils, f.err +} + +// fakeSPCoverageSource is a hermetic fake for the spCoverageSource interface. +type fakeSPCoverageSource struct { + err error + summary SPCoverageSummary +} + +func (f *fakeSPCoverageSource) GetSPCoverageSummary(_ context.Context, _ string, _ int) (SPCoverageSummary, error) { + return f.summary, f.err +} + +// fakeSPUtilizationSource is a hermetic fake for the spUtilizationSource interface. +type fakeSPUtilizationSource struct { + err error + summary SPUtilizationSummary + gotType cetypes.SupportedSavingsPlansType +} + +func (f *fakeSPUtilizationSource) GetSPUtilization(_ context.Context, planType cetypes.SupportedSavingsPlansType, _ string, _ int) (SPUtilizationSummary, error) { + f.gotType = planType + return f.summary, f.err +} + +// --------------------------------------------------------------------------- +// Test helpers +// --------------------------------------------------------------------------- + +func newTestLadder( + t *testing.T, + ris riLister, + sps spLister, + cov coverageSource, + util utilizationSource, +) *AWSLadder { + t.Helper() + a, err := New( + Config{Region: "us-east-1", AccountID: "123456789012", HorizonDays: 30, LookbackDays: 30}, + ris, sps, cov, util, + nil, nil, + ) + require.NoError(t, err) + return a +} + +func testScope() ladder.Scope { + return ladder.Scope{Provider: common.ProviderAWS, AccountID: "123456789012"} +} + +// makeRI constructs a ConvertibleRI for test use. fixedPrice is hardcoded to 0 +// (no-upfront) and duration to one year (31536000 s); tests that need other +// payment options build ec2svc.ConvertibleRI directly. +func makeRI(id, instanceType string, count int32, recurringHourly float64, end time.Time) ec2svc.ConvertibleRI { + return ec2svc.ConvertibleRI{ + ReservedInstanceID: id, + InstanceType: instanceType, + InstanceCount: count, + FixedPrice: 0, + RecurringHourlyAmount: recurringHourly, + Duration: 31536000, // 1 year in seconds + State: "active", + End: end, + } +} + +func makeSP(id, planType string, hourly float64, end time.Time) ActiveSP { + return ActiveSP{ + PlanID: id, + PlanType: planType, + HourlyCommitmentUSD: hourly, + State: "active", + EndDate: end, + } +} + +// --------------------------------------------------------------------------- +// New() constructor +// --------------------------------------------------------------------------- + +func TestNew_RequiredFieldValidation(t *testing.T) { + ri := &fakeRILister{} + sp := &fakeSPLister{} + cov := &fakeCoverageSource{} + util := &fakeUtilizationSource{} + + // cfg is last in the anonymous struct to minimize GC scan range (fieldalignment): + // interface fields (all-pointer) before Config (which has trailing int fields). + tests := []struct { + name string + ri riLister + sp spLister + cov coverageSource + util utilizationSource + wantErr string + cfg Config + }{ + {"empty region", ri, sp, cov, util, "Region must not be empty", Config{AccountID: "1"}}, + {"empty account", ri, sp, cov, util, "AccountID must not be empty", Config{Region: "us-east-1"}}, + {"nil riLister", nil, sp, cov, util, "riLister must not be nil", Config{Region: "us-east-1", AccountID: "1"}}, + {"nil spLister", ri, nil, cov, util, "spLister must not be nil", Config{Region: "us-east-1", AccountID: "1"}}, + {"nil coverageSource", ri, sp, nil, util, "coverageSource must not be nil", Config{Region: "us-east-1", AccountID: "1"}}, + {"nil utilizationSource", ri, sp, cov, nil, "utilizationSource must not be nil", Config{Region: "us-east-1", AccountID: "1"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := New(tt.cfg, tt.ri, tt.sp, tt.cov, tt.util, nil, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + }) + } +} + +// --------------------------------------------------------------------------- +// Provider / SupportedLayers +// --------------------------------------------------------------------------- + +func TestProvider(t *testing.T) { + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}) + assert.Equal(t, common.ProviderAWS, a.Provider()) +} + +func TestSupportedLayers_RoleCardinality(t *testing.T) { + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}) + layers := a.SupportedLayers() + require.Len(t, layers, 3) + + roleSeen := make(map[ladder.LayerRole]int) + for _, l := range layers { + for _, r := range l.Roles { + roleSeen[r]++ + } + } + assert.Equal(t, 1, roleSeen[ladder.RoleFlex], "exactly one RoleFlex") + assert.Equal(t, 1, roleSeen[ladder.RoleBase], "exactly one RoleBase") + assert.Equal(t, 1, roleSeen[ladder.RoleBuffer], "exactly one RoleBuffer") + + // Verify the layer-to-role assignment. + layerRole := make(map[ladder.LayerType]ladder.LayerRole) + for _, l := range layers { + layerRole[l.Type] = l.Roles[0] + } + assert.Equal(t, ladder.RoleBase, layerRole[ladder.LayerEC2InstanceSP]) + assert.Equal(t, ladder.RoleFlex, layerRole[ladder.LayerComputeSP]) + assert.Equal(t, ladder.RoleBuffer, layerRole[ladder.LayerConvertibleRI]) +} + +// --------------------------------------------------------------------------- +// PurchaseLayer / ReshapeBuffer stub errors +// --------------------------------------------------------------------------- + +func TestPurchaseLayer_ReturnsNotWiredError(t *testing.T) { + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}) + _, err := a.PurchaseLayer(context.Background(), ladder.LayerConvertibleRI, common.Recommendation{}, common.PurchaseOptions{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "write side not yet wired") + assert.False(t, errors.Is(err, common.ErrCommitmentPurchaseNotSupported), + "must NOT wrap ErrCommitmentPurchaseNotSupported -- that sentinel means permanent inability, not missing wiring") +} + +func TestReshapeBuffer_ReturnsNotWiredError(t *testing.T) { + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}) + _, err := a.ReshapeBuffer(context.Background(), testScope(), ladder.BufferReshapeConfig{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "write side not yet wired") +} + +// --------------------------------------------------------------------------- +// ListCommitments +// --------------------------------------------------------------------------- + +func TestListCommitments_MergesRIsAndSPs(t *testing.T) { + now := time.Now() + end1yr := now.Add(365 * 24 * time.Hour) + + ris := []ec2svc.ConvertibleRI{ + makeRI("ri-1", "m5.xlarge", 2, 0.50, end1yr), + } + sps := []ActiveSP{ + makeSP("sp-ec2-1", "EC2Instance", 1.00, end1yr), + makeSP("sp-compute-1", "Compute", 2.00, end1yr), + makeSP("sp-sagemaker-1", "SageMaker", 0.50, end1yr), // filtered out + } + + a := newTestLadder(t, &fakeRILister{ris: ris}, &fakeSPLister{sps: sps}, &fakeCoverageSource{}, &fakeUtilizationSource{}) + commitments, err := a.ListCommitments(context.Background(), testScope()) + require.NoError(t, err) + + // 1 RI + 2 SP (EC2Instance + Compute); SageMaker filtered out. + require.Len(t, commitments, 3) + + var riFound, ec2SPFound, computeSPFound bool + for _, c := range commitments { + switch { + case c.CommitmentType == common.CommitmentReservedInstance: + riFound = true + assert.Equal(t, "ri-1", c.CommitmentID) + assert.Equal(t, 2, c.Count) + // Per-instance pricing: no-upfront 0.50/hr recurring x 2 instances + // = 1.00 reservation-total (DescribeReservedInstances fields are + // per-instance). + assert.InDelta(t, 1.00, c.Cost, 1e-9) + case c.CommitmentID == "sp-ec2-1": + ec2SPFound = true + assert.Equal(t, common.ServiceSavingsPlansEC2Instance, c.Service) + case c.CommitmentID == "sp-compute-1": + computeSPFound = true + assert.Equal(t, common.ServiceSavingsPlansCompute, c.Service) + } + } + assert.True(t, riFound) + assert.True(t, ec2SPFound) + assert.True(t, computeSPFound) +} + +func TestListCommitments_EmptySourcesReturnEmptySlice(t *testing.T) { + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}) + commitments, err := a.ListCommitments(context.Background(), testScope()) + require.NoError(t, err) + assert.Empty(t, commitments) +} + +func TestListCommitments_WrongScope_ReturnsError(t *testing.T) { + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}) + wrongScope := ladder.Scope{Provider: common.ProviderAWS, AccountID: "999"} + _, err := a.ListCommitments(context.Background(), wrongScope) + require.Error(t, err) + assert.Contains(t, err.Error(), "does not match configured account") +} + +func TestListCommitments_RIError_Propagates(t *testing.T) { + ri := &fakeRILister{err: errors.New("AWS API error")} + a := newTestLadder(t, ri, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}) + _, err := a.ListCommitments(context.Background(), testScope()) + require.Error(t, err) + assert.Contains(t, err.Error(), "RI listing failed") +} + +// --------------------------------------------------------------------------- +// riHourlyCost +// --------------------------------------------------------------------------- + +func TestRIHourlyCost_PaymentOptions(t *testing.T) { + oneYearSeconds := int64(31536000) + tests := []struct { + name string + fixedPrice float64 + usagePrice float64 + recurringHourly float64 + duration int64 + instanceCount int32 + wantHourly float64 + }{ + { + name: "no-upfront: only recurring", + fixedPrice: 0, + recurringHourly: 0.30, + duration: oneYearSeconds, + instanceCount: 1, + wantHourly: 0.30, + }, + { + name: "all-upfront: only amortized", + fixedPrice: 8760 * 0.20, // $0.20/hr amortized over 1yr + duration: oneYearSeconds, + instanceCount: 1, + wantHourly: 0.20, + }, + { + name: "partial-upfront: both", + fixedPrice: 8760 * 0.10, // $0.10/hr upfront portion + recurringHourly: 0.15, + duration: oneYearSeconds, + instanceCount: 1, + wantHourly: 0.25, + }, + { + name: "legacy usage price included", + usagePrice: 0.05, + recurringHourly: 0.10, + duration: oneYearSeconds, + instanceCount: 1, + wantHourly: 0.15, + }, + { + name: "per-instance semantics: count multiplies the rate", + fixedPrice: 8760 * 0.10, // $0.10/hr upfront portion per instance + usagePrice: 0.02, + recurringHourly: 0.08, + duration: oneYearSeconds, + instanceCount: 4, + wantHourly: 0.80, // (0.10 + 0.02 + 0.08) * 4 + }, + { + name: "zero duration: no panic", + fixedPrice: 1000, + recurringHourly: 0.10, + duration: 0, + instanceCount: 1, + wantHourly: 0.10, // upfront amortized skipped when duration==0 + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ri := ec2svc.ConvertibleRI{ + FixedPrice: tt.fixedPrice, + UsagePrice: tt.usagePrice, + RecurringHourlyAmount: tt.recurringHourly, + Duration: tt.duration, + InstanceCount: tt.instanceCount, + } + got := riHourlyCost(&ri) + assert.InDelta(t, tt.wantHourly, got, 1e-6) + }) + } +} + +// --------------------------------------------------------------------------- +// GetLayerStates - explicit zero contract +// --------------------------------------------------------------------------- + +func TestGetLayerStates_EmptyLayer_ExplicitZeros(t *testing.T) { + // Empty RI and SP lists. The contract requires ExistingUSDPerHour and + // ExpiringUSDPerHour to be explicit zero pointers (not nil), and + // UtilizationPct to be nil (genuinely unmeasured on an empty layer). + a := newTestLadder(t, + &fakeRILister{}, + &fakeSPLister{}, + &fakeCoverageSource{}, + &fakeUtilizationSource{}, + ) + states, err := a.GetLayerStates(context.Background(), testScope()) + require.NoError(t, err) + + for _, layerType := range []ladder.LayerType{ + ladder.LayerConvertibleRI, + ladder.LayerEC2InstanceSP, + ladder.LayerComputeSP, + } { + s, ok := states[layerType] + require.True(t, ok, "layer %s must be present", layerType) + + require.NotNil(t, s.ExistingUSDPerHour, "ExistingUSDPerHour must be non-nil (explicit zero) for empty layer %s", layerType) + assert.Equal(t, 0.0, *s.ExistingUSDPerHour, "ExistingUSDPerHour must be 0 for empty layer %s", layerType) + + require.NotNil(t, s.ExpiringUSDPerHour, "ExpiringUSDPerHour must be non-nil (explicit zero) for empty layer %s", layerType) + assert.Equal(t, 0.0, *s.ExpiringUSDPerHour, "ExpiringUSDPerHour must be 0 for empty layer %s", layerType) + + // UtilizationPct must be nil for an empty layer (genuinely unmeasured). + assert.Nil(t, s.UtilizationPct, "UtilizationPct must be nil (not measured) for empty layer %s", layerType) + } +} + +// --------------------------------------------------------------------------- +// GetLayerStates - expiry horizon boundary +// --------------------------------------------------------------------------- + +func TestGetLayerStates_ExpiryHorizonBoundary(t *testing.T) { + now := time.Now() + horizonDays := 30 + + // RI that expires exactly at the horizon boundary (on or before horizon). + atHorizon := now.Add(time.Duration(horizonDays) * 24 * time.Hour) + justAfter := atHorizon.Add(time.Second) + + riAtHorizon := makeRI("ri-at", "m5.large", 1, 1.00, atHorizon) + riJustAfter := makeRI("ri-after", "m5.large", 1, 1.00, justAfter) + + a, err := New( + Config{Region: "us-east-1", AccountID: "123456789012", HorizonDays: horizonDays, LookbackDays: 30}, + &fakeRILister{ris: []ec2svc.ConvertibleRI{riAtHorizon, riJustAfter}}, + &fakeSPLister{}, + &fakeCoverageSource{}, + &fakeUtilizationSource{}, + nil, nil, + ) + require.NoError(t, err) + + states, err := a.GetLayerStates(context.Background(), testScope()) + require.NoError(t, err) + + ri := states[ladder.LayerConvertibleRI] + require.NotNil(t, ri.ExpiringUSDPerHour) + // Only riAtHorizon is within the horizon; riJustAfter is not. + assert.InDelta(t, 1.00, *ri.ExpiringUSDPerHour, 1e-6, + "only the RI expiring at-or-before the horizon should be counted") +} + +// --------------------------------------------------------------------------- +// GetLayerStates - per-instance RI pricing regression +// --------------------------------------------------------------------------- + +func TestGetLayerStates_RILayer_CountGreaterThanOne_MultipliesCost(t *testing.T) { + // Regression: DescribeReservedInstances pricing fields are per-instance. + // A count=3 RI at 0.40/hr recurring must contribute 1.20/hr to + // ExistingUSDPerHour — understating this by factor InstanceCount would + // make the engine overbuy new commitments. + end1yr := time.Now().Add(365 * 24 * time.Hour) + ris := []ec2svc.ConvertibleRI{ + makeRI("ri-multi", "m5.large", 3, 0.40, end1yr), + } + + a := newTestLadder(t, + &fakeRILister{ris: ris}, + &fakeSPLister{}, + &fakeCoverageSource{}, + &fakeUtilizationSource{}, + ) + states, err := a.GetLayerStates(context.Background(), testScope()) + require.NoError(t, err) + + ri := states[ladder.LayerConvertibleRI] + require.NotNil(t, ri.ExistingUSDPerHour) + assert.InDelta(t, 1.20, *ri.ExistingUSDPerHour, 1e-9, + "reservation-total = per-instance 0.40/hr x 3 instances") + require.NotNil(t, ri.ExpiringUSDPerHour) + assert.InDelta(t, 0.0, *ri.ExpiringUSDPerHour, 1e-9, + "1-year-out expiry is beyond the 30-day horizon") +} + +// --------------------------------------------------------------------------- +// GetLayerStates - coverage and utilization +// --------------------------------------------------------------------------- + +func TestGetLayerStates_RILayer_CoverageAndUtilization(t *testing.T) { + ris := []ec2svc.ConvertibleRI{ + makeRI("ri-1", "m5.large", 1, 0.50, time.Now().Add(365*24*time.Hour)), + } + coverageMap := recommendations.PoolCoverageMap{ + "us-east-1:m5.large": {Pct: 80.0, AvgInstancesPerHour: 10.0}, + "us-east-1:m5.xlarge": {Pct: 60.0, AvgInstancesPerHour: 5.0}, + } + utils := []recommendations.RIUtilization{ + {PurchasedHours: 100, TotalActualHours: 90}, + } + + a := newTestLadder(t, + &fakeRILister{ris: ris}, + &fakeSPLister{}, + &fakeCoverageSource{coverageMap: coverageMap}, + &fakeUtilizationSource{utils: utils}, + ) + states, err := a.GetLayerStates(context.Background(), testScope()) + require.NoError(t, err) + + ri := states[ladder.LayerConvertibleRI] + require.NotNil(t, ri.CoveragePct, "CoveragePct should be non-nil when coverage map is populated") + // Weighted average: (80*10 + 60*5) / (10+5) = (800+300)/15 = 1100/15 ~= 73.33 + assert.InDelta(t, 73.33, *ri.CoveragePct, 0.1) + + require.NotNil(t, ri.UtilizationPct, "UtilizationPct should be non-nil when utilization data is present") + assert.InDelta(t, 90.0, *ri.UtilizationPct, 1e-6) +} + +func TestGetLayerStates_CoverageError_DegradesToNil(t *testing.T) { + ris := []ec2svc.ConvertibleRI{ + makeRI("ri-1", "m5.large", 1, 0.50, time.Now().Add(365*24*time.Hour)), + } + a := newTestLadder(t, + &fakeRILister{ris: ris}, + &fakeSPLister{}, + &fakeCoverageSource{coverageErr: errors.New("CE API error")}, + &fakeUtilizationSource{}, + ) + states, err := a.GetLayerStates(context.Background(), testScope()) + require.NoError(t, err, "coverage error must not fail GetLayerStates") + assert.Nil(t, states[ladder.LayerConvertibleRI].CoveragePct, "CoveragePct must be nil on coverage source error") +} + +func TestGetLayerStates_SPLayers_NilSPInterfaces_GiveNilCovUtil(t *testing.T) { + sps := []ActiveSP{makeSP("sp-1", "Compute", 2.0, time.Now().Add(365*24*time.Hour))} + a := newTestLadder(t, + &fakeRILister{}, + &fakeSPLister{sps: sps}, + &fakeCoverageSource{}, + &fakeUtilizationSource{}, + ) + // spCoverageSource and spUtilizationSource are nil (not wired yet). + states, err := a.GetLayerStates(context.Background(), testScope()) + require.NoError(t, err) + + sp := states[ladder.LayerComputeSP] + require.NotNil(t, sp.ExistingUSDPerHour) + assert.InDelta(t, 2.0, *sp.ExistingUSDPerHour, 1e-9) + assert.Nil(t, sp.CoveragePct, "CoveragePct must be nil when spCoverageSource is not wired") + assert.Nil(t, sp.UtilizationPct, "UtilizationPct must be nil when spUtilizationSource is not wired") +} + +func TestGetLayerStates_SPLayers_SharedCovPct_BothLayersGetSameValue(t *testing.T) { + // When spCoverageSource is wired, both SP layers must share the same CoveragePct + // (CE API limitation: GetSavingsPlansCoverage does not support plan-type filtering). + covPct := 75.0 + spCov := &fakeSPCoverageSource{summary: SPCoverageSummary{CoveragePct: &covPct}} + + a, err := New( + Config{Region: "us-east-1", AccountID: "123456789012", HorizonDays: 30, LookbackDays: 30}, + &fakeRILister{}, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}, + spCov, nil, + ) + require.NoError(t, err) + + states, err := a.GetLayerStates(context.Background(), testScope()) + require.NoError(t, err) + + ec2SP := states[ladder.LayerEC2InstanceSP] + computeSP := states[ladder.LayerComputeSP] + + require.NotNil(t, ec2SP.CoveragePct) + require.NotNil(t, computeSP.CoveragePct) + assert.InDelta(t, 75.0, *ec2SP.CoveragePct, 1e-9, "EC2Instance SP layer coverage") + assert.InDelta(t, 75.0, *computeSP.CoveragePct, 1e-9, "Compute SP layer coverage must equal EC2Instance SP (CE API limitation)") +} + +func TestGetLayerStates_SPUtilization_CorrectCEEnum(t *testing.T) { + // Verify that spLayerState passes the correct CE SDK enum to GetSPUtilization. + utilPct := 85.0 + spUtil := &fakeSPUtilizationSource{summary: SPUtilizationSummary{UtilizationPct: &utilPct}} + + a, err := New( + Config{Region: "us-east-1", AccountID: "123456789012", HorizonDays: 30, LookbackDays: 30}, + &fakeRILister{}, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}, + nil, spUtil, + ) + require.NoError(t, err) + + states, err := a.GetLayerStates(context.Background(), testScope()) + require.NoError(t, err) + + // Both layers should get the same utilization (fake returns same for any planType). + ec2SP := states[ladder.LayerEC2InstanceSP] + computeSP := states[ladder.LayerComputeSP] + + require.NotNil(t, ec2SP.UtilizationPct) + require.NotNil(t, computeSP.UtilizationPct) + assert.InDelta(t, 85.0, *ec2SP.UtilizationPct, 1e-9) + assert.InDelta(t, 85.0, *computeSP.UtilizationPct, 1e-9) +} + +// --------------------------------------------------------------------------- +// toSPUtilPlanType +// --------------------------------------------------------------------------- + +func TestToSPUtilPlanType_MapsCorrectly(t *testing.T) { + got, err := toSPUtilPlanType("EC2Instance") + require.NoError(t, err) + assert.Equal(t, cetypes.SupportedSavingsPlansTypeEc2InstanceSp, got) + + got, err = toSPUtilPlanType("Compute") + require.NoError(t, err) + assert.Equal(t, cetypes.SupportedSavingsPlansTypeComputeSp, got) + + _, err = toSPUtilPlanType("SageMaker") + require.Error(t, err, "unknown plan type must return an error") +} + +// --------------------------------------------------------------------------- +// computeEC2CoveragePct +// --------------------------------------------------------------------------- + +func TestComputeEC2CoveragePct_WeightedAverage(t *testing.T) { + m := recommendations.PoolCoverageMap{ + "us-east-1:m5.large": {Pct: 80.0, AvgInstancesPerHour: 10.0}, + "us-east-1:m5.xlarge": {Pct: 60.0, AvgInstancesPerHour: 5.0}, + "us-west-2:m5.large": {Pct: 50.0, AvgInstancesPerHour: 8.0}, // different region, excluded + } + result := computeEC2CoveragePct(m, "us-east-1") + require.NotNil(t, result) + assert.InDelta(t, 73.33, *result, 0.1) +} + +func TestComputeEC2CoveragePct_NoMatchingPools_ReturnsNil(t *testing.T) { + m := recommendations.PoolCoverageMap{ + "us-west-2:m5.large": {Pct: 80.0}, + } + result := computeEC2CoveragePct(m, "us-east-1") + assert.Nil(t, result) +} + +func TestComputeEC2CoveragePct_AllZeroWeight_UnweightedAverage(t *testing.T) { + m := recommendations.PoolCoverageMap{ + "us-east-1:m5.large": {Pct: 80.0, AvgInstancesPerHour: 0}, + "us-east-1:m5.xlarge": {Pct: 60.0, AvgInstancesPerHour: 0}, + } + result := computeEC2CoveragePct(m, "us-east-1") + require.NotNil(t, result) + assert.InDelta(t, 70.0, *result, 1e-6, "unweighted average of 80 and 60") +} + +// --------------------------------------------------------------------------- +// nearestRankPercentile +// --------------------------------------------------------------------------- + +func TestNearestRankPercentile(t *testing.T) { + tests := []struct { + name string + data []float64 + p float64 + want float64 + wantErr bool + }{ + {"p5 of 20 elements", makeRange(20), 5.0, 1.0, false}, + {"p50 of 10 elements", makeRange(10), 50.0, 5.0, false}, + {"p100 returns max", []float64{3, 1, 4, 1, 5, 9}, 100.0, 9.0, false}, + {"p5 of 1 element", []float64{7.0}, 5.0, 7.0, false}, + {"all equal values", []float64{5, 5, 5, 5, 5}, 25.0, 5.0, false}, + {"empty data", []float64{}, 50.0, 0, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := nearestRankPercentile(tt.data, tt.p) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.InDelta(t, tt.want, got, 1e-9) + }) + } +} + +// --------------------------------------------------------------------------- +// GetUsageBaseline +// --------------------------------------------------------------------------- + +func TestGetUsageBaseline_SingleDaySeries_ReturnsThatValue(t *testing.T) { + // A 7-day series with all same value; p5 should return 3.0. + series := []float64{3.0, 3.0, 3.0, 3.0, 3.0, 3.0, 3.0} + cov := &fakeCoverageSource{onDemandSeries: series} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + + bl, err := a.GetUsageBaseline(context.Background(), testScope(), 7, 5.0) + require.NoError(t, err) + require.NotNil(t, bl.LowWaterUSDPerHour) + assert.InDelta(t, 3.0, *bl.LowWaterUSDPerHour, 1e-9) + // StableUSDPerHour must be nil: no stable-usage estimator exists yet, and + // the pkg/ladder contract defines Stable as post-buffer-fraction (aliasing + // it to LowWater would make the engine over-commit the base layer). nil + // triggers the engine's documented "route all core gap to flex" degradation. + assert.Nil(t, bl.StableUSDPerHour) + assert.Equal(t, 7, bl.LookbackDays) + assert.InDelta(t, 5.0, bl.Percentile, 1e-9) +} + +func TestGetUsageBaseline_P5OfVariedSeries(t *testing.T) { + // 20-element series [1..20]; p5 nearest-rank: ceil(5/100*20)=ceil(1)=1 -> sorted[0]=1. + cov := &fakeCoverageSource{onDemandSeries: makeRange(20)} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + + bl, err := a.GetUsageBaseline(context.Background(), testScope(), 20, 5.0) + require.NoError(t, err) + require.NotNil(t, bl.LowWaterUSDPerHour) + assert.InDelta(t, 1.0, *bl.LowWaterUSDPerHour, 1e-9) +} + +func TestGetUsageBaseline_EmptySeries_ReturnsError(t *testing.T) { + cov := &fakeCoverageSource{onDemandSeries: []float64{}} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + _, err := a.GetUsageBaseline(context.Background(), testScope(), 7, 5.0) + require.Error(t, err) + assert.Contains(t, err.Error(), "empty") +} + +func TestGetUsageBaseline_SeriesTooShort_ReturnsError(t *testing.T) { + cov := &fakeCoverageSource{onDemandSeries: []float64{1.0, 2.0, 3.0}} // < 7 days + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + _, err := a.GetUsageBaseline(context.Background(), testScope(), 7, 5.0) + require.Error(t, err) + assert.Contains(t, err.Error(), "below minimum") +} + +func TestGetUsageBaseline_NaNElement_ReturnsErrorNamingIndex(t *testing.T) { + series := makeRange(10) + series[4] = math.NaN() + cov := &fakeCoverageSource{onDemandSeries: series} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + _, err := a.GetUsageBaseline(context.Background(), testScope(), 10, 5.0) + require.Error(t, err, "a NaN element must be rejected at the boundary") + assert.Contains(t, err.Error(), "index 4") + assert.Contains(t, err.Error(), "not finite") +} + +func TestGetUsageBaseline_InfElement_ReturnsErrorNamingIndex(t *testing.T) { + series := makeRange(10) + series[7] = math.Inf(1) + cov := &fakeCoverageSource{onDemandSeries: series} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + _, err := a.GetUsageBaseline(context.Background(), testScope(), 10, 5.0) + require.Error(t, err, "an Inf element must be rejected at the boundary") + assert.Contains(t, err.Error(), "index 7") + assert.Contains(t, err.Error(), "not finite") +} + +func TestGetUsageBaseline_NegativeElement_ReturnsErrorNamingIndex(t *testing.T) { + series := makeRange(10) + series[2] = -0.5 + cov := &fakeCoverageSource{onDemandSeries: series} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + _, err := a.GetUsageBaseline(context.Background(), testScope(), 10, 5.0) + require.Error(t, err, "a negative cost element must be rejected at the boundary") + assert.Contains(t, err.Error(), "index 2") + assert.Contains(t, err.Error(), "negative") +} + +func TestGetUsageBaseline_OnDemandSourceError_Propagates(t *testing.T) { + cov := &fakeCoverageSource{onDemandErr: errors.New("CE error")} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + _, err := a.GetUsageBaseline(context.Background(), testScope(), 30, 5.0) + require.Error(t, err) + assert.Contains(t, err.Error(), "on-demand series fetch failed") +} + +func TestGetUsageBaseline_InvalidPercentile_ReturnsError(t *testing.T) { + series := makeRange(30) + cov := &fakeCoverageSource{onDemandSeries: series} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + + for _, p := range []float64{0.0, -1.0, 101.0} { + _, err := a.GetUsageBaseline(context.Background(), testScope(), 30, p) + require.Error(t, err, "percentile %g should fail", p) + assert.Contains(t, err.Error(), "percentile") + } +} + +func TestGetUsageBaseline_WrongScope_ReturnsError(t *testing.T) { + cov := &fakeCoverageSource{onDemandSeries: makeRange(30)} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + badScope := ladder.Scope{Provider: common.ProviderAWS, AccountID: "wrong"} + _, err := a.GetUsageBaseline(context.Background(), badScope, 30, 5.0) + require.Error(t, err) +} + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +// makeRange returns a float64 slice [1.0, 2.0, ..., float64(n)]. +func makeRange(n int) []float64 { + out := make([]float64, n) + for i := range out { + out[i] = float64(i + 1) + } + return out +} diff --git a/providers/aws/ladder/layer_states.go b/providers/aws/ladder/layer_states.go new file mode 100644 index 000000000..a59e01806 --- /dev/null +++ b/providers/aws/ladder/layer_states.go @@ -0,0 +1,342 @@ +package ladder + +import ( + "context" + "fmt" + "log" + "strings" + "time" + + cetypes "github.com/aws/aws-sdk-go-v2/service/costexplorer/types" + + "github.com/LeanerCloud/CUDly/pkg/ladder" + "github.com/LeanerCloud/CUDly/providers/aws/recommendations" + ec2svc "github.com/LeanerCloud/CUDly/providers/aws/services/ec2" +) + +// ptr wraps a float64 as a non-nil pointer. Used to convert derived metrics +// into the pointer form required by LayerState. +func ptr(v float64) *float64 { return &v } + +// GetLayerStates returns a point-in-time snapshot for each of the three +// supported AWS ladder layers. The snapshot rules are: +// +// - ExistingUSDPerHour: 0 (explicit zero pointer) when the layer has no +// active commitments; non-nil with the summed hourly amortized cost otherwise. +// - ExpiringUSDPerHour: 0 (explicit zero pointer) when nothing expires within +// Config.HorizonDays; the expiring share otherwise. +// - CoveragePct: nil when not measured (SP layers before the parallel SP +// coverage PR lands); non-nil from CE coverage data for the RI layer. +// - UtilizationPct: nil on an empty layer or when data is unavailable; non-nil +// from CE utilization data for the RI layer. +// +// CE API note: GetSavingsPlansCoverage does not support plan-type filtering, +// so both EC2Instance and Compute SP layers share the same CoveragePct value. +// This is documented on each SP LayerState via the layer type field. +// +// The scope must match Config.AccountID and common.ProviderAWS. +func (a *AWSLadder) GetLayerStates(ctx context.Context, scope ladder.Scope) (map[ladder.LayerType]ladder.LayerState, error) { + if err := a.validateScope(scope); err != nil { + return nil, err + } + + ris, err := a.ris.ListConvertibleReservedInstances(ctx) + if err != nil { + return nil, fmt.Errorf("GetLayerStates: RI listing failed: %w", err) + } + + sps, err := a.sps.ListActiveSPs(ctx) + if err != nil { + return nil, fmt.Errorf("GetLayerStates: SP listing failed: %w", err) + } + + coverageMap, covErr := a.coverage.GetRICoverageMap(ctx, a.cfg.lookbackDays(), []string{a.cfg.Region}) + // covErr is checked per-layer below; a coverage failure does not fail the + // whole snapshot — it degrades CoveragePct to nil. + + utils, utilErr := a.utilization.GetRIUtilization(ctx, a.cfg.lookbackDays()) + // utilErr is handled the same way: degrade UtilizationPct to nil. + + now := time.Now() + horizon := now.Add(time.Duration(a.cfg.horizonDays()) * 24 * time.Hour) + + // SP coverage is fetched once; the CE GetSavingsPlansCoverage API does not + // support filtering by plan type, so both SP layers receive the same value. + spCovPct := a.fetchSPCoveragePct(ctx) + + states := make(map[ladder.LayerType]ladder.LayerState, 3) + states[ladder.LayerConvertibleRI] = a.riLayerState(ris, horizon, coverageMap, covErr, utils, utilErr) + states[ladder.LayerEC2InstanceSP] = a.spLayerState(ctx, ladder.LayerEC2InstanceSP, "EC2Instance", sps, horizon, spCovPct) + states[ladder.LayerComputeSP] = a.spLayerState(ctx, ladder.LayerComputeSP, "Compute", sps, horizon, spCovPct) + return states, nil +} + +// riLayerState builds the LayerState for the ConvertibleRI (buffer) layer. +func (a *AWSLadder) riLayerState( + ris []ec2svc.ConvertibleRI, + horizon time.Time, + coverageMap recommendations.PoolCoverageMap, + covErr error, + utils []recommendations.RIUtilization, + utilErr error, +) ladder.LayerState { + existing := sumRIHourlyCost(ris) + expiring := sumExpiringRIHourlyCost(ris, horizon) + + state := ladder.LayerState{ + Layer: ladder.LayerConvertibleRI, + ExistingUSDPerHour: ptr(existing), + ExpiringUSDPerHour: ptr(expiring), + } + + switch { + case covErr != nil: + // CoveragePct stays nil (unmeasured). Log so a persistently failing CE + // call is visible — silent degradation would quietly disable reshape + // triggering downstream. + log.Printf("WARNING: AWSLadder GetLayerStates: RI coverage degraded to nil (layer=%s, source=GetRICoverageMap, region=%s): %v", + ladder.LayerConvertibleRI, a.cfg.Region, covErr) + case len(coverageMap) > 0: + state.CoveragePct = computeEC2CoveragePct(coverageMap, a.cfg.Region) + } + + if utilErr != nil { + // UtilizationPct stays nil (unmeasured); same observability rationale. + log.Printf("WARNING: AWSLadder GetLayerStates: RI utilization degraded to nil (layer=%s, source=GetRIUtilization, region=%s): %v", + ladder.LayerConvertibleRI, a.cfg.Region, utilErr) + } else { + state.UtilizationPct = computeRIUtilizationPct(utils) + } + + return state +} + +// spLayerState builds the LayerState for an EC2Instance or Compute SP layer. +// +// sharedCovPct is the SP coverage percentage fetched once for both SP layers; +// the CE GetSavingsPlansCoverage API does not support plan-type filtering so +// both layers receive the same value (nil when the source is not yet wired). +// +// UtilizationPct is fetched per-layer via spUtilizationSource, which uses +// GetSavingsPlansUtilization and does support plan-type filtering. +func (a *AWSLadder) spLayerState( + ctx context.Context, + layerType ladder.LayerType, + planType string, + sps []ActiveSP, + horizon time.Time, + sharedCovPct *float64, +) ladder.LayerState { + existing := sumSPHourlyCost(sps, planType) + expiring := sumExpiringSPHourlyCost(sps, planType, horizon) + + state := ladder.LayerState{ + Layer: layerType, + ExistingUSDPerHour: ptr(existing), + ExpiringUSDPerHour: ptr(expiring), + CoveragePct: sharedCovPct, + UtilizationPct: a.fetchSPUtilizationPct(ctx, planType), + } + return state +} + +// fetchSPCoveragePct calls the injected spCoverageSource when wired; returns +// nil (unmeasured) when the interface is nil (PR 4 not yet landed). +// No planType is passed: the CE GetSavingsPlansCoverage API does not support +// filtering by plan type; the result applies to all SP types in the region. +func (a *AWSLadder) fetchSPCoveragePct(ctx context.Context) *float64 { + if a.spCoverage == nil { + return nil + } + summary, err := a.spCoverage.GetSPCoverageSummary(ctx, a.cfg.Region, a.cfg.lookbackDays()) + if err != nil { + // Degrade gracefully (caller treats nil as unmeasured) but log: a + // persistently failing CE call must not silently disable SP coverage. + log.Printf("WARNING: AWSLadder GetLayerStates: SP coverage degraded to nil (layers=%s+%s, source=GetSPCoverageSummary, region=%s): %v", + ladder.LayerEC2InstanceSP, ladder.LayerComputeSP, a.cfg.Region, err) + return nil + } + return summary.CoveragePct +} + +// fetchSPUtilizationPct calls the injected spUtilizationSource when wired; +// returns nil when the interface is nil (PR 4 not yet landed). +// +// Compute SPs are global; their utilization is queried with region="" (all +// regions). EC2 Instance SPs are region-specific; the configured region is used. +func (a *AWSLadder) fetchSPUtilizationPct(ctx context.Context, planType string) *float64 { + if a.spUtil == nil { + return nil + } + cePlanType, err := toSPUtilPlanType(planType) + if err != nil { + // Defensive: unknown plan type -> unmeasured; log for observability. + log.Printf("WARNING: AWSLadder GetLayerStates: SP utilization degraded to nil (planType=%s, source=toSPUtilPlanType): %v", + planType, err) + return nil + } + // Compute SPs are global; EC2 Instance SPs are region-scoped. + region := a.cfg.Region + if planType == "Compute" { + region = "" // "" = all regions in the CE GetSavingsPlansUtilization API + } + summary, err := a.spUtil.GetSPUtilization(ctx, cePlanType, region, a.cfg.lookbackDays()) + if err != nil { + // Degrade gracefully but log: silent CE failures must stay visible. + log.Printf("WARNING: AWSLadder GetLayerStates: SP utilization degraded to nil (planType=%s, source=GetSPUtilization, region=%q): %v", + planType, region, err) + return nil + } + return summary.UtilizationPct +} + +// toSPUtilPlanType maps the DescribeSavingsPlans planType string (from ActiveSP) +// to the CE SDK enum required by GetSavingsPlansUtilization. +func toSPUtilPlanType(planType string) (cetypes.SupportedSavingsPlansType, error) { + switch planType { + case "EC2Instance": + return cetypes.SupportedSavingsPlansTypeEc2InstanceSp, nil + case "Compute": + return cetypes.SupportedSavingsPlansTypeComputeSp, nil + default: + return "", fmt.Errorf("toSPUtilPlanType: unrecognized SP plan type %q", planType) + } +} + +// sumRIHourlyCost returns the total hourly amortized cost across all RIs. +// Index-based range with pointer avoids copying the large ConvertibleRI struct. +func sumRIHourlyCost(ris []ec2svc.ConvertibleRI) float64 { + var total float64 + for i := range ris { + total += riHourlyCost(&ris[i]) + } + return total +} + +// riHourlyCost computes the reservation-total hourly amortized cost for an RI. +// ri is taken by pointer to avoid copying the large ConvertibleRI struct (hugeParam). +// +// DescribeReservedInstances pricing fields are PER-INSTANCE (same semantics as +// the repo's canonical monthlyCostFromConvertibleRI helper in +// internal/api/handler_ri_exchange.go), so the per-instance hourly rate is +// multiplied by InstanceCount: +// +// hourly = (RecurringHourlyAmount + UsagePrice + FixedPrice/(Duration/3600)) * InstanceCount +// +// RecurringHourlyAmount covers the recurring charge (non-zero for no-upfront +// and partial-upfront); UsagePrice is the legacy per-hour usage fee; +// FixedPrice / (Duration / 3600) amortizes the upfront payment over the term +// (Duration is in seconds). Upfront amortization is skipped when Duration is +// zero (defensive; avoids divide-by-zero). +func riHourlyCost(ri *ec2svc.ConvertibleRI) float64 { + var upfrontAmortized float64 + if ri.Duration > 0 { + upfrontAmortized = ri.FixedPrice / (float64(ri.Duration) / 3600.0) + } + perInstance := ri.RecurringHourlyAmount + ri.UsagePrice + upfrontAmortized + return perInstance * float64(ri.InstanceCount) +} + +// sumExpiringRIHourlyCost sums the hourly costs of RIs whose EndDate is +// non-zero and falls on or before horizon. A zero EndDate is treated as +// "no expiry known" and excluded, to avoid misclassifying perpetual-term RIs. +// Index-based range with pointer avoids copying the large ConvertibleRI struct. +func sumExpiringRIHourlyCost(ris []ec2svc.ConvertibleRI, horizon time.Time) float64 { + var total float64 + for i := range ris { + if !ris[i].End.IsZero() && !ris[i].End.After(horizon) { + total += riHourlyCost(&ris[i]) + } + } + return total +} + +// sumSPHourlyCost sums the hourly commitment amounts for SPs of the given +// plan type. +func sumSPHourlyCost(sps []ActiveSP, planType string) float64 { + var total float64 + for _, sp := range sps { + if sp.PlanType == planType { + total += sp.HourlyCommitmentUSD + } + } + return total +} + +// sumExpiringSPHourlyCost sums the hourly commitments of SPs of the given +// plan type that expire on or before horizon. A zero EndDate is excluded. +func sumExpiringSPHourlyCost(sps []ActiveSP, planType string, horizon time.Time) float64 { + var total float64 + for _, sp := range sps { + if sp.PlanType != planType { + continue + } + if !sp.EndDate.IsZero() && !sp.EndDate.After(horizon) { + total += sp.HourlyCommitmentUSD + } + } + return total +} + +// computeEC2CoveragePct derives an aggregate EC2 RI coverage percentage from +// the PoolCoverageMap for the given region. Only EC2 pools (keys matching +// "region:*" with a non-zero AvgInstancesPerHour or Pct) are considered. +// +// Weighting: when any pool has a non-zero AvgInstancesPerHour, coverage is +// a weighted average (weight = AvgInstancesPerHour). When all pools have +// AvgInstancesPerHour == 0 (CE returned coverage % but no running hours -- +// unusual), a simple (unweighted) average is used. Returns nil when no EC2 +// pools for the region are found in the map. +func computeEC2CoveragePct(coverageMap recommendations.PoolCoverageMap, region string) *float64 { + prefix := strings.ToLower(region) + ":" + var weightedSum, totalWeight float64 + var simpleSum float64 + count := 0 + + for key, cov := range coverageMap { + if !strings.HasPrefix(key, prefix) { + continue + } + // Exclude RDS keys (contain extra ":" segments for engine:deployment). + // EC2 pool keys are exactly "region:instance_type" (one colon). + if strings.Count(key, ":") != 1 { + continue + } + count++ + simpleSum += cov.Pct + if cov.AvgInstancesPerHour > 0 { + weightedSum += cov.Pct * cov.AvgInstancesPerHour + totalWeight += cov.AvgInstancesPerHour + } + } + + if count == 0 { + return nil + } + + var result float64 + if totalWeight > 0 { + result = weightedSum / totalWeight + } else { + result = simpleSum / float64(count) + } + return ptr(result) +} + +// computeRIUtilizationPct aggregates per-RI utilization data from the CE +// GetReservationUtilization response into a single percentage. +// +// Method: sum(TotalActualHours) / sum(PurchasedHours) * 100. Returns nil when +// the slice is empty or the sum of PurchasedHours is zero (layer is empty or +// CE returned no hours -- genuinely unmeasured per the LayerState contract). +func computeRIUtilizationPct(utils []recommendations.RIUtilization) *float64 { + var purchased, actual float64 + for _, u := range utils { + purchased += u.PurchasedHours + actual += u.TotalActualHours + } + if purchased == 0 { + return nil + } + return ptr((actual / purchased) * 100.0) +} From 6ce99663cd98269c8bbdf2b38c0df3a141f5fa6d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 3 Jul 2026 21:51:53 +0200 Subject: [PATCH 5/8] feat(aws): add ladder capability write side PurchaseLayer dispatches to the savingsplans/ec2 purchase clients with mandatory idempotency tokens and per-target boundary validation. ReshapeBuffer is a thin RunAutoExchange delegation with fail-loud dry-run rejection pending #1348 and a documented store-wide pending-cancellation warning. Also: SDK-derived plan-type constants and coverage interface split. Part of #1335. --- providers/aws/ladder/baseline.go | 10 +- providers/aws/ladder/commitments.go | 4 +- providers/aws/ladder/interfaces.go | 91 ++++++-- providers/aws/ladder/ladder.go | 88 +++++--- providers/aws/ladder/ladder_test.go | 40 ++-- providers/aws/ladder/layer_states.go | 12 +- providers/aws/ladder/purchase.go | 146 +++++++++++++ providers/aws/ladder/purchase_test.go | 301 ++++++++++++++++++++++++++ providers/aws/ladder/reshape.go | 194 +++++++++++++++++ providers/aws/ladder/reshape_test.go | 265 +++++++++++++++++++++++ 10 files changed, 1073 insertions(+), 78 deletions(-) create mode 100644 providers/aws/ladder/purchase.go create mode 100644 providers/aws/ladder/purchase_test.go create mode 100644 providers/aws/ladder/reshape.go create mode 100644 providers/aws/ladder/reshape_test.go diff --git a/providers/aws/ladder/baseline.go b/providers/aws/ladder/baseline.go index 1a6fa461c..d185199e2 100644 --- a/providers/aws/ladder/baseline.go +++ b/providers/aws/ladder/baseline.go @@ -16,11 +16,11 @@ import ( const minBaselineSeriesDays = 7 // GetUsageBaseline computes a statistical low-water-mark from a daily -// on-demand-equivalent USD/hour series returned by the injected coverageSource. +// on-demand-equivalent USD/hour series returned by the injected onDemandSeriesSource. // // Series semantics: each element is the average on-demand-equivalent USD/hour // for one calendar day over the lookback window, ordered oldest-to-newest. -// The series is sourced from coverageSource.GetOnDemandSeries, which is wired +// The series is sourced from onDemandSeriesSource.GetOnDemandSeries, which is wired // in a later PR to call CE GetCostAndUsage (Granularity=Daily, on-demand // usage-type filter). Until that wiring lands, callers receive a data-source // error from GetOnDemandSeries. @@ -55,12 +55,12 @@ func (a *AWSLadder) GetUsageBaseline(ctx context.Context, scope ladder.Scope, lo return ladder.UsageBaseline{}, err } - series, err := a.coverage.GetOnDemandSeries(ctx, a.cfg.Region, lookbackDays) + series, err := a.onDemand.GetOnDemandSeries(ctx, a.cfg.Region, lookbackDays) if err != nil { return ladder.UsageBaseline{}, fmt.Errorf("GetUsageBaseline: on-demand series fetch failed: %w", err) } if len(series) == 0 { - return ladder.UsageBaseline{}, fmt.Errorf("GetUsageBaseline: on-demand series is empty for region %s (coverage source returned no data)", a.cfg.Region) + return ladder.UsageBaseline{}, fmt.Errorf("GetUsageBaseline: on-demand series is empty for region %s (series source returned no data)", a.cfg.Region) } if len(series) < minBaselineSeriesDays { return ladder.UsageBaseline{}, fmt.Errorf( @@ -92,7 +92,7 @@ func (a *AWSLadder) GetUsageBaseline(ctx context.Context, scope ladder.Scope, lo } // validateSeries rejects series containing non-finite (NaN/Inf) or negative -// elements at the trust boundary: the series is injected via coverageSource, +// elements at the trust boundary: the series is injected via onDemandSeriesSource, // and a single bad element would silently corrupt the percentile (NaN makes // the sort order undefined; a negative cost is impossible for on-demand spend). // The error names the offending index so the data-source bug is traceable. diff --git a/providers/aws/ladder/commitments.go b/providers/aws/ladder/commitments.go index 95c19d8c8..d05f40e08 100644 --- a/providers/aws/ladder/commitments.go +++ b/providers/aws/ladder/commitments.go @@ -113,7 +113,7 @@ func (a *AWSLadder) listSPCommitments(ctx context.Context) ([]common.Commitment, // isLadderSPType returns true for the two plan types that map to ladder layers. func isLadderSPType(planType string) bool { - return planType == "EC2Instance" || planType == "Compute" + return planType == spPlanTypeEC2Instance || planType == spPlanTypeCompute } // spToCommitment converts an ActiveSP to a common.Commitment. @@ -123,7 +123,7 @@ func isLadderSPType(planType string) bool { // (e.g. for queued plans); callers treat the zero time as "no expiry signal". func spToCommitment(sp *ActiveSP, accountID string) common.Commitment { service := common.ServiceSavingsPlansEC2Instance - if sp.PlanType == "Compute" { + if sp.PlanType == spPlanTypeCompute { service = common.ServiceSavingsPlansCompute } diff --git a/providers/aws/ladder/interfaces.go b/providers/aws/ladder/interfaces.go index 71bb536f0..67a0ed38b 100644 --- a/providers/aws/ladder/interfaces.go +++ b/providers/aws/ladder/interfaces.go @@ -1,6 +1,8 @@ -// Package ladder implements the ladder.LadderCapability READ side for AWS. -// Write-side methods (PurchaseLayer, ReshapeBuffer) return explicit -// not-implemented errors until the write-side PR lands. +// Package ladder implements ladder.LadderCapability for AWS: the read side +// (commitment listing, layer states, usage baseline) and the write side +// (layer purchases, buffer reshaping). Write-side methods require the write +// dependencies to be wired via AWSLadder.WithWriteSide; until then they +// return an explicit not-wired error. package ladder import ( @@ -8,11 +10,25 @@ import ( "time" cetypes "github.com/aws/aws-sdk-go-v2/service/costexplorer/types" + sptypes "github.com/aws/aws-sdk-go-v2/service/savingsplans/types" + "github.com/LeanerCloud/CUDly/pkg/common" + "github.com/LeanerCloud/CUDly/pkg/exchange" "github.com/LeanerCloud/CUDly/providers/aws/recommendations" ec2svc "github.com/LeanerCloud/CUDly/providers/aws/services/ec2" ) +// Savings Plan plan-type identifiers, derived from the AWS SDK enum so this +// package can never drift from the vocabulary the savingsplans service client +// uses (its PlanTypeForServiceType / ServiceTypeForPlanType mappings are built +// on sptypes.SavingsPlanType). The string form is needed because ActiveSP. +// PlanType and common.SavingsPlanDetails.PlanType are plain strings; the +// constant conversion keeps these compile-time constants, not vars. +const ( + spPlanTypeEC2Instance = string(sptypes.SavingsPlanTypeEc2Instance) + spPlanTypeCompute = string(sptypes.SavingsPlanTypeCompute) +) + // riLister is the narrow interface for listing active convertible RIs. // The concrete implementation is ec2svc.Client.ListConvertibleReservedInstances. type riLister interface { @@ -51,20 +67,25 @@ type spLister interface { ListActiveSPs(ctx context.Context) ([]ActiveSP, error) } -// coverageSource is the narrow interface for RI coverage data and the -// on-demand daily spend series used by GetUsageBaseline. -// -// GetRICoverageMap returns the per-pool org-wide RI coverage map (keyed by -// "region:instance_type" for EC2) for the given lookback window and regions. -// -// GetOnDemandSeries returns a slice of len(lookbackDays) daily on-demand- -// equivalent USD/hour values for the given region, ordered oldest-to-newest. -// Each element is the average on-demand spend in USD per hour for that -// calendar day. The real implementation sources this from CE GetCostAndUsage -// with Granularity=Daily filtered to on-demand usage types; wiring happens -// when the cost-and-usage collector PR lands. Tests pass a hermetic fake. -type coverageSource interface { +// riCoverageSource is the narrow interface for RI coverage data, consumed by +// GetLayerStates. GetRICoverageMap returns the per-pool org-wide RI coverage +// map (keyed by "region:instance_type" for EC2) for the given lookback window +// and regions. Kept single-method (interface segregation) so implementations +// that only provide coverage need not stub the on-demand series and vice versa; +// one concrete adapter may still implement both. +type riCoverageSource interface { GetRICoverageMap(ctx context.Context, lookbackDays int, regions []string) (recommendations.PoolCoverageMap, error) +} + +// onDemandSeriesSource is the narrow interface for the daily on-demand spend +// series consumed by GetUsageBaseline. GetOnDemandSeries returns a slice of +// len(lookbackDays) daily on-demand-equivalent USD/hour values for the given +// region, ordered oldest-to-newest. Each element is the average on-demand +// spend in USD per hour for that calendar day. The real implementation sources +// this from CE GetCostAndUsage with Granularity=Daily filtered to on-demand +// usage types; wiring happens when the cost-and-usage collector PR lands. +// Tests pass a hermetic fake. +type onDemandSeriesSource interface { GetOnDemandSeries(ctx context.Context, region string, lookbackDays int) ([]float64, error) } @@ -131,3 +152,41 @@ type spCoverageSource interface { type spUtilizationSource interface { GetSPUtilization(ctx context.Context, planType cetypes.SupportedSavingsPlansType, region string, lookbackDays int) (SPUtilizationSummary, error) } + +// riPurchaser is the narrow interface for purchasing EC2 convertible Reserved +// Instances. The concrete implementation is ec2svc.Client.PurchaseCommitment, +// which resolves the offering from the recommendation, enforces the +// idempotency-tag dedupe guard (issue #636: a lookup for an RI already tagged +// with opts.IdempotencyToken short-circuits a re-driven purchase), and tags +// the fresh RI post-purchase. +type riPurchaser interface { + PurchaseCommitment(ctx context.Context, rec common.Recommendation, opts common.PurchaseOptions) (common.PurchaseResult, error) +} + +// spPurchaser is the narrow interface for purchasing Savings Plans. The +// concrete implementation is savingsplans.Client.PurchaseCommitment, which +// resolves the offering (plan type + term + payment option) and calls +// CreateSavingsPlan with opts.IdempotencyToken as the native ClientToken +// (server-side idempotency: a repeated call returns the original plan). +// +// A single spPurchaser serves both SP layers: AWSLadder validates that the +// recommendation's SavingsPlanDetails.PlanType matches the dispatched layer +// (EC2Instance for LayerEC2InstanceSP, Compute for LayerComputeSP) before +// calling, and a plan-type-scoped savingsplans.Client re-validates against +// its own scope (resolveSPPlanType), so a mismatched purchase cannot slip +// through either boundary. +type spPurchaser interface { + PurchaseCommitment(ctx context.Context, rec common.Recommendation, opts common.PurchaseOptions) (common.PurchaseResult, error) +} + +// exchangeRunner is the narrow interface for running the automated RI +// exchange flow. The concrete implementation wraps exchange.RunAutoExchange +// and owns everything ReshapeBuffer must not know about: the exchange store, +// the ExchangeClient, the offering lookup, and the RI/utilization inventory +// conversion (the same wiring internal/server.executeRIExchangeReshape does). +// AWSLadder only supplies the run configuration; injecting the full +// exchange.RunAutoExchangeParams surface here would drag store and exchange +// client dependencies into this package for no benefit. +type exchangeRunner interface { + RunAutoExchange(ctx context.Context, cfg exchange.RIExchangeConfig) (*exchange.AutoExchangeResult, error) +} diff --git a/providers/aws/ladder/ladder.go b/providers/aws/ladder/ladder.go index a1274c220..58dd28b01 100644 --- a/providers/aws/ladder/ladder.go +++ b/providers/aws/ladder/ladder.go @@ -1,7 +1,6 @@ package ladder import ( - "context" "errors" "fmt" @@ -19,12 +18,12 @@ const DefaultHorizonDays = 30 const DefaultLookbackDays = 30 // errWriteNotWired is the sentinel returned by PurchaseLayer and ReshapeBuffer -// until the write-side PR (PR 6) lands. It is distinct from -// common.ErrCommitmentPurchaseNotSupported, which signals that this provider -// can NEVER purchase a given layer type programmatically. Here the capability -// WILL be supported once wired; the error is a clear placeholder, not a -// permanent constraint. -var errWriteNotWired = errors.New("write side not yet wired (PR 6): call sites must not invoke PurchaseLayer or ReshapeBuffer until the write PR is merged") +// when the write-side dependencies have not been wired via WithWriteSide. +// It is distinct from common.ErrCommitmentPurchaseNotSupported, which signals +// that this provider can NEVER purchase a given layer type programmatically. +// Here the capability exists; the instance is just missing its write wiring — +// a configuration error at the call site, not a permanent constraint. +var errWriteNotWired = errors.New("write side not wired: wire riPurchaser, spPurchaser, and exchangeRunner via WithWriteSide before calling PurchaseLayer or ReshapeBuffer") // Config holds construction-time parameters for AWSLadder. type Config struct { @@ -57,15 +56,19 @@ func (c Config) lookbackDays() int { return DefaultLookbackDays } -// AWSLadder implements ladder.LadderCapability for AWS. It provides the READ -// side (ListCommitments, GetLayerStates, GetUsageBaseline); the write side -// (PurchaseLayer, ReshapeBuffer) is wired in PR 6 and returns an explicit -// not-implemented error until then. +// AWSLadder implements ladder.LadderCapability for AWS: the read side +// (ListCommitments, GetLayerStates, GetUsageBaseline) and the write side +// (PurchaseLayer, ReshapeBuffer). // -// All four data-source dependencies are injected via narrow interfaces so that -// unit tests are hermetic (no real AWS calls needed). The caller wires the +// All four read data-source dependencies are injected via narrow interfaces so +// that unit tests are hermetic (no real AWS calls needed). The caller wires the // concrete adapters (ec2svc.Client, savingsplans.Client, etc.) at startup. // +// The write-side dependencies (riPurchase, spPurchase, exchange) are wired via +// WithWriteSide; until then PurchaseLayer and ReshapeBuffer fail loud with +// errWriteNotWired. This keeps read-only wiring (dashboards, analysis) free of +// purchase/exchange infrastructure. +// // SP coverage and utilization (spCoverageSource, spUtilizationSource) may be // nil; when nil, CoveragePct and UtilizationPct for SP layers are nil, which // the engine treats as "not yet measured." They are wired when the parallel @@ -76,20 +79,27 @@ func (c Config) lookbackDays() int { type AWSLadder struct { ris riLister sps spLister - coverage coverageSource + riCoverage riCoverageSource + onDemand onDemandSeriesSource utilization utilizationSource spCoverage spCoverageSource // nil until parallel SP coverage PR (PR 4) lands spUtil spUtilizationSource // nil until parallel SP utilization PR (PR 4) lands + riPurchase riPurchaser // write side; nil until WithWriteSide is called + spPurchase spPurchaser // write side; nil until WithWriteSide is called + exchange exchangeRunner // write side; nil until WithWriteSide is called cfg Config } -// New constructs an AWSLadder. All four required interfaces must be non-nil; -// spCoverage and spUtil may be nil (wired later). +// New constructs an AWSLadder. The five required read-side interfaces must be +// non-nil; spCov and spUtil may be nil (wired later). riCov and odSeries are +// separate single-method interfaces (interface segregation); one concrete +// adapter may satisfy both and be passed for each. func New( cfg Config, ris riLister, sps spLister, - cov coverageSource, + riCov riCoverageSource, + odSeries onDemandSeriesSource, util utilizationSource, spCov spCoverageSource, spUtil spUtilizationSource, @@ -106,8 +116,11 @@ func New( if sps == nil { return nil, fmt.Errorf("AWSLadder: spLister must not be nil") } - if cov == nil { - return nil, fmt.Errorf("AWSLadder: coverageSource must not be nil") + if riCov == nil { + return nil, fmt.Errorf("AWSLadder: riCoverageSource must not be nil") + } + if odSeries == nil { + return nil, fmt.Errorf("AWSLadder: onDemandSeriesSource must not be nil") } if util == nil { return nil, fmt.Errorf("AWSLadder: utilizationSource must not be nil") @@ -116,7 +129,8 @@ func New( cfg: cfg, ris: ris, sps: sps, - coverage: cov, + riCoverage: riCov, + onDemand: odSeries, utilization: util, spCoverage: spCov, spUtil: spUtil, @@ -143,18 +157,26 @@ func (a *AWSLadder) SupportedLayers() []ladder.LayerSpec { } } -// PurchaseLayer is not yet wired. It returns an explicit placeholder error -// that is NOT common.ErrCommitmentPurchaseNotSupported (which would signal -// permanent inability to purchase). This error signals that the write-side -// wiring is missing; callers must not invoke this method until PR 6 is merged. +// WithWriteSide wires the write-side dependencies and returns the same +// instance for chaining. All three must be non-nil: a partially wired write +// side would let one write method work while its sibling fails at call time, +// which is harder to diagnose than failing here at construction. // -//nolint:gocritic // hugeParam: Recommendation is large but the LadderCapability interface contract requires value, not pointer -func (a *AWSLadder) PurchaseLayer(_ context.Context, _ ladder.LayerType, _ common.Recommendation, _ common.PurchaseOptions) (common.PurchaseResult, error) { - return common.PurchaseResult{}, fmt.Errorf("PurchaseLayer: %w", errWriteNotWired) -} - -// ReshapeBuffer is not yet wired. It returns the same placeholder error as -// PurchaseLayer; see that method's comment for the rationale. -func (a *AWSLadder) ReshapeBuffer(_ context.Context, _ ladder.Scope, _ ladder.BufferReshapeConfig) (ladder.ReshapeSummary, error) { - return ladder.ReshapeSummary{}, fmt.Errorf("ReshapeBuffer: %w", errWriteNotWired) +// riP purchases EC2 convertible RIs (LayerConvertibleRI); spP purchases +// Savings Plans (LayerEC2InstanceSP and LayerComputeSP); ex runs the +// automated RI exchange flow backing ReshapeBuffer. +func (a *AWSLadder) WithWriteSide(riP riPurchaser, spP spPurchaser, ex exchangeRunner) (*AWSLadder, error) { + if riP == nil { + return nil, fmt.Errorf("AWSLadder.WithWriteSide: riPurchaser must not be nil") + } + if spP == nil { + return nil, fmt.Errorf("AWSLadder.WithWriteSide: spPurchaser must not be nil") + } + if ex == nil { + return nil, fmt.Errorf("AWSLadder.WithWriteSide: exchangeRunner must not be nil") + } + a.riPurchase = riP + a.spPurchase = spP + a.exchange = ex + return a, nil } diff --git a/providers/aws/ladder/ladder_test.go b/providers/aws/ladder/ladder_test.go index adbeb0ee9..b0932d178 100644 --- a/providers/aws/ladder/ladder_test.go +++ b/providers/aws/ladder/ladder_test.go @@ -95,17 +95,20 @@ func (f *fakeSPUtilizationSource) GetSPUtilization(_ context.Context, planType c // Test helpers // --------------------------------------------------------------------------- +// newTestLadder builds a read-side ladder. cov is the fakeCoverageSource, +// which satisfies both riCoverageSource and onDemandSeriesSource, so it is +// passed for both split interfaces. func newTestLadder( t *testing.T, ris riLister, sps spLister, - cov coverageSource, + cov *fakeCoverageSource, util utilizationSource, ) *AWSLadder { t.Helper() a, err := New( Config{Region: "us-east-1", AccountID: "123456789012", HorizonDays: 30, LookbackDays: 30}, - ris, sps, cov, util, + ris, sps, cov, cov, util, nil, nil, ) require.NoError(t, err) @@ -158,22 +161,24 @@ func TestNew_RequiredFieldValidation(t *testing.T) { name string ri riLister sp spLister - cov coverageSource + riCov riCoverageSource + od onDemandSeriesSource util utilizationSource wantErr string cfg Config }{ - {"empty region", ri, sp, cov, util, "Region must not be empty", Config{AccountID: "1"}}, - {"empty account", ri, sp, cov, util, "AccountID must not be empty", Config{Region: "us-east-1"}}, - {"nil riLister", nil, sp, cov, util, "riLister must not be nil", Config{Region: "us-east-1", AccountID: "1"}}, - {"nil spLister", ri, nil, cov, util, "spLister must not be nil", Config{Region: "us-east-1", AccountID: "1"}}, - {"nil coverageSource", ri, sp, nil, util, "coverageSource must not be nil", Config{Region: "us-east-1", AccountID: "1"}}, - {"nil utilizationSource", ri, sp, cov, nil, "utilizationSource must not be nil", Config{Region: "us-east-1", AccountID: "1"}}, + {"empty region", ri, sp, cov, cov, util, "Region must not be empty", Config{AccountID: "1"}}, + {"empty account", ri, sp, cov, cov, util, "AccountID must not be empty", Config{Region: "us-east-1"}}, + {"nil riLister", nil, sp, cov, cov, util, "riLister must not be nil", Config{Region: "us-east-1", AccountID: "1"}}, + {"nil spLister", ri, nil, cov, cov, util, "spLister must not be nil", Config{Region: "us-east-1", AccountID: "1"}}, + {"nil riCoverageSource", ri, sp, nil, cov, util, "riCoverageSource must not be nil", Config{Region: "us-east-1", AccountID: "1"}}, + {"nil onDemandSeriesSource", ri, sp, cov, nil, util, "onDemandSeriesSource must not be nil", Config{Region: "us-east-1", AccountID: "1"}}, + {"nil utilizationSource", ri, sp, cov, cov, nil, "utilizationSource must not be nil", Config{Region: "us-east-1", AccountID: "1"}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := New(tt.cfg, tt.ri, tt.sp, tt.cov, tt.util, nil, nil) + _, err := New(tt.cfg, tt.ri, tt.sp, tt.riCov, tt.od, tt.util, nil, nil) require.Error(t, err) assert.Contains(t, err.Error(), tt.wantErr) }) @@ -215,14 +220,14 @@ func TestSupportedLayers_RoleCardinality(t *testing.T) { } // --------------------------------------------------------------------------- -// PurchaseLayer / ReshapeBuffer stub errors +// PurchaseLayer / ReshapeBuffer without write-side wiring // --------------------------------------------------------------------------- func TestPurchaseLayer_ReturnsNotWiredError(t *testing.T) { a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}) _, err := a.PurchaseLayer(context.Background(), ladder.LayerConvertibleRI, common.Recommendation{}, common.PurchaseOptions{}) require.Error(t, err) - assert.Contains(t, err.Error(), "write side not yet wired") + assert.Contains(t, err.Error(), "write side not wired") assert.False(t, errors.Is(err, common.ErrCommitmentPurchaseNotSupported), "must NOT wrap ErrCommitmentPurchaseNotSupported -- that sentinel means permanent inability, not missing wiring") } @@ -231,7 +236,7 @@ func TestReshapeBuffer_ReturnsNotWiredError(t *testing.T) { a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}) _, err := a.ReshapeBuffer(context.Background(), testScope(), ladder.BufferReshapeConfig{}) require.Error(t, err) - assert.Contains(t, err.Error(), "write side not yet wired") + assert.Contains(t, err.Error(), "write side not wired") } // --------------------------------------------------------------------------- @@ -436,11 +441,12 @@ func TestGetLayerStates_ExpiryHorizonBoundary(t *testing.T) { riAtHorizon := makeRI("ri-at", "m5.large", 1, 1.00, atHorizon) riJustAfter := makeRI("ri-after", "m5.large", 1, 1.00, justAfter) + cov := &fakeCoverageSource{} a, err := New( Config{Region: "us-east-1", AccountID: "123456789012", HorizonDays: horizonDays, LookbackDays: 30}, &fakeRILister{ris: []ec2svc.ConvertibleRI{riAtHorizon, riJustAfter}}, &fakeSPLister{}, - &fakeCoverageSource{}, + cov, cov, &fakeUtilizationSource{}, nil, nil, ) @@ -562,9 +568,10 @@ func TestGetLayerStates_SPLayers_SharedCovPct_BothLayersGetSameValue(t *testing. covPct := 75.0 spCov := &fakeSPCoverageSource{summary: SPCoverageSummary{CoveragePct: &covPct}} + cov := &fakeCoverageSource{} a, err := New( Config{Region: "us-east-1", AccountID: "123456789012", HorizonDays: 30, LookbackDays: 30}, - &fakeRILister{}, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}, + &fakeRILister{}, &fakeSPLister{}, cov, cov, &fakeUtilizationSource{}, spCov, nil, ) require.NoError(t, err) @@ -586,9 +593,10 @@ func TestGetLayerStates_SPUtilization_CorrectCEEnum(t *testing.T) { utilPct := 85.0 spUtil := &fakeSPUtilizationSource{summary: SPUtilizationSummary{UtilizationPct: &utilPct}} + cov := &fakeCoverageSource{} a, err := New( Config{Region: "us-east-1", AccountID: "123456789012", HorizonDays: 30, LookbackDays: 30}, - &fakeRILister{}, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}, + &fakeRILister{}, &fakeSPLister{}, cov, cov, &fakeUtilizationSource{}, nil, spUtil, ) require.NoError(t, err) diff --git a/providers/aws/ladder/layer_states.go b/providers/aws/ladder/layer_states.go index a59e01806..54c828022 100644 --- a/providers/aws/ladder/layer_states.go +++ b/providers/aws/ladder/layer_states.go @@ -50,7 +50,7 @@ func (a *AWSLadder) GetLayerStates(ctx context.Context, scope ladder.Scope) (map return nil, fmt.Errorf("GetLayerStates: SP listing failed: %w", err) } - coverageMap, covErr := a.coverage.GetRICoverageMap(ctx, a.cfg.lookbackDays(), []string{a.cfg.Region}) + coverageMap, covErr := a.riCoverage.GetRICoverageMap(ctx, a.cfg.lookbackDays(), []string{a.cfg.Region}) // covErr is checked per-layer below; a coverage failure does not fail the // whole snapshot — it degrades CoveragePct to nil. @@ -66,8 +66,8 @@ func (a *AWSLadder) GetLayerStates(ctx context.Context, scope ladder.Scope) (map states := make(map[ladder.LayerType]ladder.LayerState, 3) states[ladder.LayerConvertibleRI] = a.riLayerState(ris, horizon, coverageMap, covErr, utils, utilErr) - states[ladder.LayerEC2InstanceSP] = a.spLayerState(ctx, ladder.LayerEC2InstanceSP, "EC2Instance", sps, horizon, spCovPct) - states[ladder.LayerComputeSP] = a.spLayerState(ctx, ladder.LayerComputeSP, "Compute", sps, horizon, spCovPct) + states[ladder.LayerEC2InstanceSP] = a.spLayerState(ctx, ladder.LayerEC2InstanceSP, spPlanTypeEC2Instance, sps, horizon, spCovPct) + states[ladder.LayerComputeSP] = a.spLayerState(ctx, ladder.LayerComputeSP, spPlanTypeCompute, sps, horizon, spCovPct) return states, nil } @@ -177,7 +177,7 @@ func (a *AWSLadder) fetchSPUtilizationPct(ctx context.Context, planType string) } // Compute SPs are global; EC2 Instance SPs are region-scoped. region := a.cfg.Region - if planType == "Compute" { + if planType == spPlanTypeCompute { region = "" // "" = all regions in the CE GetSavingsPlansUtilization API } summary, err := a.spUtil.GetSPUtilization(ctx, cePlanType, region, a.cfg.lookbackDays()) @@ -194,9 +194,9 @@ func (a *AWSLadder) fetchSPUtilizationPct(ctx context.Context, planType string) // to the CE SDK enum required by GetSavingsPlansUtilization. func toSPUtilPlanType(planType string) (cetypes.SupportedSavingsPlansType, error) { switch planType { - case "EC2Instance": + case spPlanTypeEC2Instance: return cetypes.SupportedSavingsPlansTypeEc2InstanceSp, nil - case "Compute": + case spPlanTypeCompute: return cetypes.SupportedSavingsPlansTypeComputeSp, nil default: return "", fmt.Errorf("toSPUtilPlanType: unrecognized SP plan type %q", planType) diff --git a/providers/aws/ladder/purchase.go b/providers/aws/ladder/purchase.go new file mode 100644 index 000000000..05cd94c8e --- /dev/null +++ b/providers/aws/ladder/purchase.go @@ -0,0 +1,146 @@ +package ladder + +import ( + "context" + "fmt" + "math" + + "github.com/LeanerCloud/CUDly/pkg/common" + "github.com/LeanerCloud/CUDly/pkg/ladder" +) + +// PurchaseLayer buys a commitment for the given layer by dispatching to the +// injected purchase client: +// +// - LayerConvertibleRI -> riPurchaser (EC2 PurchaseReservedInstancesOffering +// with the idempotency-tag dedupe guard) +// - LayerEC2InstanceSP -> spPurchaser with an EC2Instance-plan recommendation +// - LayerComputeSP -> spPurchaser with a Compute-plan recommendation +// +// Boundary validation happens BEFORE any client call (this is a money path; +// nothing is bought on malformed input): +// +// - layer must be one of the three supported AWS layers (unknown -> error); +// - opts.IdempotencyToken must be non-empty: idempotency is mandatory on +// this purchase path so a re-driven execution can never double-buy +// (non-empty guard for idempotency-source fields at the function boundary); +// - rec must carry what the target client needs (see validateRIPurchaseRec / +// validateSPPurchaseRec). +// +// Client errors are wrapped with layer context via %w, so a client that +// returns common.ErrCommitmentPurchaseNotSupported still satisfies +// errors.Is(err, common.ErrCommitmentPurchaseNotSupported) at the engine. +// The client's PurchaseResult is returned alongside the error because the +// concrete clients populate result.Error and partial state on failure. +// +//nolint:gocritic // hugeParam: Recommendation is large but the LadderCapability interface contract requires value, not pointer +func (a *AWSLadder) PurchaseLayer(ctx context.Context, layer ladder.LayerType, rec common.Recommendation, opts common.PurchaseOptions) (common.PurchaseResult, error) { + if a.riPurchase == nil || a.spPurchase == nil { + return common.PurchaseResult{}, fmt.Errorf("PurchaseLayer: %w", errWriteNotWired) + } + + var planType string + switch layer { + case ladder.LayerConvertibleRI: + // planType stays empty; the RI path validates differently below. + case ladder.LayerEC2InstanceSP: + planType = spPlanTypeEC2Instance + case ladder.LayerComputeSP: + planType = spPlanTypeCompute + default: + return common.PurchaseResult{}, fmt.Errorf("PurchaseLayer: layer %q is not a supported AWS ladder layer (want %s, %s, or %s)", + layer, ladder.LayerConvertibleRI, ladder.LayerEC2InstanceSP, ladder.LayerComputeSP) + } + + if opts.IdempotencyToken == "" { + return common.PurchaseResult{}, fmt.Errorf( + "PurchaseLayer(%s): opts.IdempotencyToken must not be empty: idempotency is mandatory on the ladder purchase path so re-driven executions cannot double-buy", + layer) + } + + if layer == ladder.LayerConvertibleRI { + return a.purchaseRI(ctx, &rec, opts) + } + return a.purchaseSP(ctx, layer, planType, &rec, opts) +} + +// purchaseRI validates and executes an EC2 convertible RI purchase. rec is a +// pointer to avoid re-copying the large Recommendation struct internally; the +// client call dereferences it to match the ServiceClient value contract. +func (a *AWSLadder) purchaseRI(ctx context.Context, rec *common.Recommendation, opts common.PurchaseOptions) (common.PurchaseResult, error) { + if err := validateRIPurchaseRec(rec); err != nil { + return common.PurchaseResult{}, fmt.Errorf("PurchaseLayer(%s): %w", ladder.LayerConvertibleRI, err) + } + result, err := a.riPurchase.PurchaseCommitment(ctx, *rec, opts) + if err != nil { + return result, fmt.Errorf("PurchaseLayer(%s): EC2 convertible RI purchase failed: %w", ladder.LayerConvertibleRI, err) + } + return result, nil +} + +// purchaseSP validates and executes a Savings Plan purchase for the given +// layer/plan type pair. rec is a pointer for the same reason as purchaseRI. +func (a *AWSLadder) purchaseSP(ctx context.Context, layer ladder.LayerType, planType string, rec *common.Recommendation, opts common.PurchaseOptions) (common.PurchaseResult, error) { + if err := validateSPPurchaseRec(rec, planType); err != nil { + return common.PurchaseResult{}, fmt.Errorf("PurchaseLayer(%s): %w", layer, err) + } + result, err := a.spPurchase.PurchaseCommitment(ctx, *rec, opts) + if err != nil { + return result, fmt.Errorf("PurchaseLayer(%s): %s Savings Plan purchase failed: %w", layer, planType, err) + } + return result, nil +} + +// validateRIPurchaseRec checks that rec carries everything the EC2 client's +// PurchaseCommitment needs: ComputeDetails (offering lookup uses +// InstanceType/Platform/Tenancy/Scope from it), a positive instance count +// (PurchaseReservedInstancesOffering InstanceCount), and the term/payment +// option strings the offering query converts. +func validateRIPurchaseRec(rec *common.Recommendation) error { + details, ok := rec.Details.(*common.ComputeDetails) + if !ok || details == nil { + return fmt.Errorf("recommendation Details must be *common.ComputeDetails for an EC2 RI purchase, got %T", rec.Details) + } + if rec.Count <= 0 { + return fmt.Errorf("recommendation Count must be > 0 for an EC2 RI purchase, got %d", rec.Count) + } + if details.InstanceType == "" { + return fmt.Errorf("ComputeDetails.InstanceType must not be empty for an EC2 RI purchase") + } + return validateTermAndPayment(rec) +} + +// validateSPPurchaseRec checks that rec carries everything the Savings Plans +// client's PurchaseCommitment needs: SavingsPlanDetails with a positive, +// finite HourlyCommitment (CreateSavingsPlan Commitment) and a PlanType +// matching the dispatched layer, plus the term/payment option strings the +// offering query converts. The plan-type match is enforced here in addition +// to the scoped client's own check so a mislabeled recommendation fails with +// layer context before any AWS call. +func validateSPPurchaseRec(rec *common.Recommendation, wantPlanType string) error { + details, ok := rec.Details.(*common.SavingsPlanDetails) + if !ok || details == nil { + return fmt.Errorf("recommendation Details must be *common.SavingsPlanDetails for a Savings Plan purchase, got %T", rec.Details) + } + if details.PlanType != wantPlanType { + return fmt.Errorf("recommendation plan type %q does not match the dispatched layer's plan type %q", details.PlanType, wantPlanType) + } + if math.IsNaN(details.HourlyCommitment) || math.IsInf(details.HourlyCommitment, 0) || details.HourlyCommitment <= 0 { + return fmt.Errorf("SavingsPlanDetails.HourlyCommitment must be a positive finite value, got %g", details.HourlyCommitment) + } + return validateTermAndPayment(rec) +} + +// validateTermAndPayment checks the two offering-query fields shared by both +// purchase paths. Both clients convert these strings (convertTermToSeconds / +// convertPaymentOption and the EC2 equivalents); empty values would fail +// deeper with a less actionable error. +func validateTermAndPayment(rec *common.Recommendation) error { + if rec.Term == "" { + return fmt.Errorf("recommendation Term must not be empty (offering lookup needs it)") + } + if rec.PaymentOption == "" { + return fmt.Errorf("recommendation PaymentOption must not be empty (offering lookup needs it)") + } + return nil +} diff --git a/providers/aws/ladder/purchase_test.go b/providers/aws/ladder/purchase_test.go new file mode 100644 index 000000000..ed16325d3 --- /dev/null +++ b/providers/aws/ladder/purchase_test.go @@ -0,0 +1,301 @@ +package ladder + +import ( + "context" + "errors" + "fmt" + "math" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/LeanerCloud/CUDly/pkg/common" + "github.com/LeanerCloud/CUDly/pkg/ladder" +) + +// --------------------------------------------------------------------------- +// Write-side fakes +// --------------------------------------------------------------------------- + +// fakePurchaser is a hermetic riPurchaser / spPurchaser double that records +// the last call. Field order minimizes GC pointer-scan range (fieldalignment). +type fakePurchaser struct { + err error + gotRec *common.Recommendation + result common.PurchaseResult + gotOpts common.PurchaseOptions + calls int +} + +func (f *fakePurchaser) PurchaseCommitment(_ context.Context, rec common.Recommendation, opts common.PurchaseOptions) (common.PurchaseResult, error) { + f.calls++ + f.gotRec = &rec + f.gotOpts = opts + return f.result, f.err +} + +// newWiredLadder returns a ladder with the write side wired to the given fakes. +func newWiredLadder(t *testing.T, riP riPurchaser, spP spPurchaser, ex exchangeRunner) *AWSLadder { + t.Helper() + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}) + a, err := a.WithWriteSide(riP, spP, ex) + require.NoError(t, err) + return a +} + +// validRIRec returns a recommendation carrying everything the EC2 RI purchase +// path requires. +func validRIRec() common.Recommendation { + return common.Recommendation{ + ResourceType: "m5.large", + Count: 2, + Term: "1yr", + PaymentOption: "no-upfront", + Details: &common.ComputeDetails{ + InstanceType: "m5.large", + Platform: "linux", + Tenancy: "default", + Scope: "regional", + }, + } +} + +// validSPRec returns a recommendation carrying everything the Savings Plan +// purchase path requires for the given plan type. +func validSPRec(planType string) common.Recommendation { + return common.Recommendation{ + Term: "1yr", + PaymentOption: "no-upfront", + Details: &common.SavingsPlanDetails{ + PlanType: planType, + HourlyCommitment: 1.50, + }, + } +} + +func validPurchaseOpts() common.PurchaseOptions { + return common.PurchaseOptions{ + Source: common.PurchaseSourceWeb, + IdempotencyToken: "ladder-tok-1", + ExecutionID: "exec-1", + } +} + +// --------------------------------------------------------------------------- +// WithWriteSide +// --------------------------------------------------------------------------- + +func TestWithWriteSide_NilArgsRejected(t *testing.T) { + riP := &fakePurchaser{} + spP := &fakePurchaser{} + ex := &fakeExchangeRunner{} + + tests := []struct { + name string + riP riPurchaser + spP spPurchaser + ex exchangeRunner + wantErr string + }{ + {"nil riPurchaser", nil, spP, ex, "riPurchaser must not be nil"}, + {"nil spPurchaser", riP, nil, ex, "spPurchaser must not be nil"}, + {"nil exchangeRunner", riP, spP, nil, "exchangeRunner must not be nil"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}) + _, err := a.WithWriteSide(tt.riP, tt.spP, tt.ex) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + }) + } +} + +// --------------------------------------------------------------------------- +// PurchaseLayer dispatch +// --------------------------------------------------------------------------- + +func TestPurchaseLayer_DispatchConvertibleRI(t *testing.T) { + riP := &fakePurchaser{result: common.PurchaseResult{Success: true, CommitmentID: "ri-new-1"}} + spP := &fakePurchaser{} + a := newWiredLadder(t, riP, spP, &fakeExchangeRunner{}) + + result, err := a.PurchaseLayer(context.Background(), ladder.LayerConvertibleRI, validRIRec(), validPurchaseOpts()) + require.NoError(t, err) + assert.True(t, result.Success) + assert.Equal(t, "ri-new-1", result.CommitmentID) + + assert.Equal(t, 1, riP.calls, "riPurchaser must be called exactly once") + assert.Equal(t, 0, spP.calls, "spPurchaser must not be called for the RI layer") + require.NotNil(t, riP.gotRec) + assert.Equal(t, "m5.large", riP.gotRec.ResourceType) + assert.Equal(t, "ladder-tok-1", riP.gotOpts.IdempotencyToken) +} + +func TestPurchaseLayer_DispatchSPLayers(t *testing.T) { + tests := []struct { + name string + layer ladder.LayerType + wantPlanType string + }{ + {"EC2Instance SP layer", ladder.LayerEC2InstanceSP, spPlanTypeEC2Instance}, + {"Compute SP layer", ladder.LayerComputeSP, spPlanTypeCompute}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + riP := &fakePurchaser{} + spP := &fakePurchaser{result: common.PurchaseResult{Success: true, CommitmentID: "sp-new-1"}} + a := newWiredLadder(t, riP, spP, &fakeExchangeRunner{}) + + result, err := a.PurchaseLayer(context.Background(), tt.layer, validSPRec(tt.wantPlanType), validPurchaseOpts()) + require.NoError(t, err) + assert.True(t, result.Success) + + assert.Equal(t, 1, spP.calls, "spPurchaser must be called exactly once") + assert.Equal(t, 0, riP.calls, "riPurchaser must not be called for SP layers") + require.NotNil(t, spP.gotRec) + details, ok := spP.gotRec.Details.(*common.SavingsPlanDetails) + require.True(t, ok) + assert.Equal(t, tt.wantPlanType, details.PlanType, + "the dispatched recommendation must carry the layer's plan type") + assert.Equal(t, "ladder-tok-1", spP.gotOpts.IdempotencyToken) + }) + } +} + +func TestPurchaseLayer_UnknownLayer_ErrorsWithoutCalling(t *testing.T) { + riP := &fakePurchaser{} + spP := &fakePurchaser{} + a := newWiredLadder(t, riP, spP, &fakeExchangeRunner{}) + + _, err := a.PurchaseLayer(context.Background(), ladder.LayerType("gcp-cud"), validRIRec(), validPurchaseOpts()) + require.Error(t, err) + assert.Contains(t, err.Error(), "not a supported AWS ladder layer") + assert.Equal(t, 0, riP.calls) + assert.Equal(t, 0, spP.calls) +} + +func TestPurchaseLayer_MissingIdempotencyToken_ErrorsWithoutCalling(t *testing.T) { + for _, layer := range []ladder.LayerType{ladder.LayerConvertibleRI, ladder.LayerEC2InstanceSP, ladder.LayerComputeSP} { + t.Run(string(layer), func(t *testing.T) { + riP := &fakePurchaser{} + spP := &fakePurchaser{} + a := newWiredLadder(t, riP, spP, &fakeExchangeRunner{}) + + opts := validPurchaseOpts() + opts.IdempotencyToken = "" + _, err := a.PurchaseLayer(context.Background(), layer, validRIRec(), opts) + require.Error(t, err) + assert.Contains(t, err.Error(), "IdempotencyToken must not be empty") + assert.Equal(t, 0, riP.calls, "no purchase may happen without an idempotency token") + assert.Equal(t, 0, spP.calls, "no purchase may happen without an idempotency token") + }) + } +} + +// --------------------------------------------------------------------------- +// PurchaseLayer recommendation validation +// --------------------------------------------------------------------------- + +func TestPurchaseLayer_RIRecValidation(t *testing.T) { + tests := []struct { + name string + mutate func(*common.Recommendation) + wantErr string + }{ + {"nil details", func(r *common.Recommendation) { r.Details = nil }, "must be *common.ComputeDetails"}, + {"wrong details type", func(r *common.Recommendation) { + r.Details = &common.SavingsPlanDetails{PlanType: spPlanTypeCompute, HourlyCommitment: 1} + }, "must be *common.ComputeDetails"}, + {"zero count", func(r *common.Recommendation) { r.Count = 0 }, "Count must be > 0"}, + {"negative count", func(r *common.Recommendation) { r.Count = -1 }, "Count must be > 0"}, + {"empty instance type", func(r *common.Recommendation) { + r.Details = &common.ComputeDetails{Platform: "linux", Tenancy: "default", Scope: "regional"} + }, "InstanceType must not be empty"}, + {"empty term", func(r *common.Recommendation) { r.Term = "" }, "Term must not be empty"}, + {"empty payment option", func(r *common.Recommendation) { r.PaymentOption = "" }, "PaymentOption must not be empty"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + riP := &fakePurchaser{} + a := newWiredLadder(t, riP, &fakePurchaser{}, &fakeExchangeRunner{}) + + rec := validRIRec() + tt.mutate(&rec) + _, err := a.PurchaseLayer(context.Background(), ladder.LayerConvertibleRI, rec, validPurchaseOpts()) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + assert.Equal(t, 0, riP.calls, "validation failures must prevent the client call") + }) + } +} + +func TestPurchaseLayer_SPRecValidation(t *testing.T) { + tests := []struct { + name string + mutate func(*common.Recommendation) + wantErr string + }{ + {"nil details", func(r *common.Recommendation) { r.Details = nil }, "must be *common.SavingsPlanDetails"}, + {"wrong details type", func(r *common.Recommendation) { + r.Details = &common.ComputeDetails{InstanceType: "m5.large"} + }, "must be *common.SavingsPlanDetails"}, + {"plan type mismatch", func(r *common.Recommendation) { + r.Details = &common.SavingsPlanDetails{PlanType: spPlanTypeCompute, HourlyCommitment: 1} + }, "does not match the dispatched layer's plan type"}, + {"zero hourly commitment", func(r *common.Recommendation) { + r.Details = &common.SavingsPlanDetails{PlanType: spPlanTypeEC2Instance, HourlyCommitment: 0} + }, "HourlyCommitment must be a positive finite value"}, + {"negative hourly commitment", func(r *common.Recommendation) { + r.Details = &common.SavingsPlanDetails{PlanType: spPlanTypeEC2Instance, HourlyCommitment: -0.5} + }, "HourlyCommitment must be a positive finite value"}, + {"NaN hourly commitment", func(r *common.Recommendation) { + r.Details = &common.SavingsPlanDetails{PlanType: spPlanTypeEC2Instance, HourlyCommitment: math.NaN()} + }, "HourlyCommitment must be a positive finite value"}, + {"empty term", func(r *common.Recommendation) { r.Term = "" }, "Term must not be empty"}, + {"empty payment option", func(r *common.Recommendation) { r.PaymentOption = "" }, "PaymentOption must not be empty"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + spP := &fakePurchaser{} + a := newWiredLadder(t, &fakePurchaser{}, spP, &fakeExchangeRunner{}) + + rec := validSPRec(spPlanTypeEC2Instance) + tt.mutate(&rec) + _, err := a.PurchaseLayer(context.Background(), ladder.LayerEC2InstanceSP, rec, validPurchaseOpts()) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + assert.Equal(t, 0, spP.calls, "validation failures must prevent the client call") + }) + } +} + +// --------------------------------------------------------------------------- +// PurchaseLayer error propagation +// --------------------------------------------------------------------------- + +func TestPurchaseLayer_ClientError_WrappedWithLayerContext(t *testing.T) { + clientErr := errors.New("AWS API throttled") + clientResult := common.PurchaseResult{Success: false, Error: clientErr} + riP := &fakePurchaser{err: clientErr, result: clientResult} + a := newWiredLadder(t, riP, &fakePurchaser{}, &fakeExchangeRunner{}) + + result, err := a.PurchaseLayer(context.Background(), ladder.LayerConvertibleRI, validRIRec(), validPurchaseOpts()) + require.Error(t, err) + assert.Contains(t, err.Error(), string(ladder.LayerConvertibleRI)) + assert.Contains(t, err.Error(), "EC2 convertible RI purchase failed") + assert.ErrorIs(t, err, clientErr, "the client error must remain unwrappable") + assert.False(t, result.Success, "the client's result must be passed through for audit") +} + +func TestPurchaseLayer_NotSupportedSentinel_PassesThrough(t *testing.T) { + wrapped := fmt.Errorf("savings plans: %w", common.ErrCommitmentPurchaseNotSupported) + spP := &fakePurchaser{err: wrapped} + a := newWiredLadder(t, &fakePurchaser{}, spP, &fakeExchangeRunner{}) + + _, err := a.PurchaseLayer(context.Background(), ladder.LayerComputeSP, validSPRec(spPlanTypeCompute), validPurchaseOpts()) + require.Error(t, err) + assert.ErrorIs(t, err, common.ErrCommitmentPurchaseNotSupported, + "engine callers detect permanent inability via errors.Is; wrapping must preserve it") +} diff --git a/providers/aws/ladder/reshape.go b/providers/aws/ladder/reshape.go new file mode 100644 index 000000000..a0d3c8e50 --- /dev/null +++ b/providers/aws/ladder/reshape.go @@ -0,0 +1,194 @@ +package ladder + +import ( + "context" + "fmt" + "math" + + "github.com/LeanerCloud/CUDly/pkg/exchange" + "github.com/LeanerCloud/CUDly/pkg/ladder" +) + +// unlimitedCapUSD is the explicit "no cap" value passed to +// exchange.RIExchangeConfig when a BufferReshapeConfig cap is nil. +// +// Why not something cleaner: RIExchangeConfig has no nil/absent +// representation for its float64 caps, and 0 there is maximally RESTRICTIVE +// (RunAutoExchange skips any exchange whose payment exceeds the cap, so a +// zero cap blocks everything) — mapping nil to 0 would silently invert the +// caller's intent. +Inf is not usable either: big.Rat.SetFloat64(+Inf) +// returns nil and the comparison in RunAutoExchange would panic. +// math.MaxFloat64 is finite (exactly representable in big.Rat) and exceeds +// any real exchange payment, making it a faithful "no cap". +const unlimitedCapUSD = math.MaxFloat64 + +// ReshapeBuffer runs the automated RI exchange flow over the buffer layer +// (convertible RIs), delegating to the injected exchangeRunner. AWSLadder +// only maps the configuration and the outcome; the runner owns the exchange +// store, quote/execute client, offering lookup, and RI/utilization inventory +// (the same wiring internal/server.executeRIExchangeReshape performs). +// +// WARNING — store-wide side effect: the underlying exchange.RunAutoExchange +// begins by unconditionally canceling ALL pending exchange records in the +// store (CancelAllPendingExchanges), including pendings created by the +// standalone ri_exchange_reshape scheduled task. Callers MUST NOT run +// ReshapeBuffer concurrently with that task against the same store; the +// pipeline phase that invokes this method must coordinate with (or disable) +// the standalone scheduler. +// +// DryRun is NOT supported: there is no true simulation mode in pkg/exchange +// yet (tracked upstream), and mapping DryRun onto the exchange flow's manual +// mode would be a false simulation — manual mode persists pending +// ExchangeRecords with live approval tokens (actionable money instruments) +// and still triggers the store-wide pending cancellation above. cfg.DryRun +// therefore fails loud; the engine previews reshapes via ActionReshape +// rationales without calling ReshapeBuffer. +// +// Config mapping (BufferReshapeConfig -> exchange.RIExchangeConfig): +// +// - MaxPaymentPerExchangeUSD / MaxPaymentDailyUSD: nil means no cap and maps +// to unlimitedCapUSD (see that constant for why); non-nil values must be +// finite and > 0 — zero is rejected loudly because RunAutoExchange treats +// the cap as a skip threshold and a zero cap would silently block every +// exchange (almost certainly a config bug, not an intent). +// - UtilizationThresholdPct must be in (0, 100]; LookbackDays must be > 0. +// - Mode is always exchange.ExchangeModeAuto: exchanges execute immediately, +// subject to the per-exchange and daily caps. +// +// Outcome mapping (exchange.AutoExchangeResult -> ladder.ReshapeSummary): +// +// - Analyzed = Completed + Pending + Failed + Skipped: the number of reshape +// recommendations processed. NOTE: this is not the total RI inventory size +// (the thin runner seam does not expose it); it counts the commitments the +// exchange analysis flagged and processed. +// - Reshaped = Completed only (exchanges actually executed). +// - Skipped = Skipped only (below threshold, no offering, over cap, ...). +// Failed attempts are not "skipped"; they surface in Details AND as a +// non-nil error (money-path failures must never be silently absorbed +// into a success-looking summary). +// +// Partial failures: when the runner reports failed exchange attempts, the +// populated summary is returned TOGETHER with a non-nil error so callers get +// both the audit detail and a loud failure signal. +func (a *AWSLadder) ReshapeBuffer(ctx context.Context, scope ladder.Scope, cfg ladder.BufferReshapeConfig) (ladder.ReshapeSummary, error) { + if a.exchange == nil { + return ladder.ReshapeSummary{}, fmt.Errorf("ReshapeBuffer: %w", errWriteNotWired) + } + if err := a.validateScope(scope); err != nil { + return ladder.ReshapeSummary{}, err + } + if cfg.DryRun { + // Fail loud beats false simulation: the exchange flow's manual mode + // persists actionable approval records and cancels unrelated pendings + // store-wide — neither is a dry run. See the godoc warning above. + return ladder.ReshapeSummary{}, fmt.Errorf( + "ReshapeBuffer: dry-run is not supported by the AWS exchange flow yet (a true simulation mode in pkg/exchange is tracked upstream); the engine previews reshapes via ActionReshape rationales without calling ReshapeBuffer") + } + + runCfg, err := buildRIExchangeConfig(cfg) + if err != nil { + return ladder.ReshapeSummary{}, fmt.Errorf("ReshapeBuffer: %w", err) + } + + result, err := a.exchange.RunAutoExchange(ctx, runCfg) + if err != nil { + return ladder.ReshapeSummary{}, fmt.Errorf("ReshapeBuffer: auto exchange run failed: %w", err) + } + if result == nil { + return ladder.ReshapeSummary{}, fmt.Errorf("ReshapeBuffer: exchange runner returned a nil result without an error (runner contract violation)") + } + + return summarizeExchangeResult(result) +} + +// buildRIExchangeConfig validates cfg at the boundary and maps it to the +// exchange package's runtime configuration. See ReshapeBuffer's godoc for the +// full mapping rationale. +func buildRIExchangeConfig(cfg ladder.BufferReshapeConfig) (exchange.RIExchangeConfig, error) { + perExchangeCap, err := capOrUnlimited("MaxPaymentPerExchangeUSD", cfg.MaxPaymentPerExchangeUSD) + if err != nil { + return exchange.RIExchangeConfig{}, err + } + dailyCap, err := capOrUnlimited("MaxPaymentDailyUSD", cfg.MaxPaymentDailyUSD) + if err != nil { + return exchange.RIExchangeConfig{}, err + } + if math.IsNaN(cfg.UtilizationThresholdPct) || cfg.UtilizationThresholdPct <= 0 || cfg.UtilizationThresholdPct > 100 { + return exchange.RIExchangeConfig{}, fmt.Errorf( + "UtilizationThresholdPct must be in (0, 100], got %g", cfg.UtilizationThresholdPct) + } + if cfg.LookbackDays <= 0 { + return exchange.RIExchangeConfig{}, fmt.Errorf("LookbackDays must be > 0, got %d", cfg.LookbackDays) + } + + // Mode is always auto: ReshapeBuffer rejects DryRun before this point + // (no true simulation mode exists in pkg/exchange; manual mode is not a + // simulation — see the ReshapeBuffer godoc warning). + return exchange.RIExchangeConfig{ + Mode: string(exchange.ExchangeModeAuto), + UtilizationThreshold: cfg.UtilizationThresholdPct, + MaxPaymentPerExchangeUSD: perExchangeCap, + MaxPaymentDailyUSD: dailyCap, + LookbackDays: cfg.LookbackDays, + }, nil +} + +// capOrUnlimited maps an optional money cap to the float64 the exchange +// config requires: nil -> unlimitedCapUSD (no cap); non-nil values must be +// finite and > 0 (see the unlimitedCapUSD comment for why zero is rejected). +func capOrUnlimited(name string, capUSD *float64) (float64, error) { + if capUSD == nil { + return unlimitedCapUSD, nil + } + v := *capUSD + if math.IsNaN(v) || math.IsInf(v, 0) { + return 0, fmt.Errorf("%s must be finite, got %g", name, v) + } + if v <= 0 { + return 0, fmt.Errorf("%s must be > 0 when set (a zero cap would block every exchange; use nil for no cap), got %g", name, v) + } + return v, nil +} + +// summarizeExchangeResult maps the runner outcome to a ReshapeSummary. +// Index-based loops avoid copying the large outcome structs (rangeValCopy). +func summarizeExchangeResult(result *exchange.AutoExchangeResult) (ladder.ReshapeSummary, error) { + summary := ladder.ReshapeSummary{ + Analyzed: len(result.Completed) + len(result.Pending) + len(result.Failed) + len(result.Skipped), + Reshaped: len(result.Completed), + Skipped: len(result.Skipped), + Details: make([]string, 0, + len(result.Completed)+len(result.Pending)+len(result.Failed)+len(result.Skipped)), + } + + for i := range result.Completed { + o := &result.Completed[i] + summary.Details = append(summary.Details, fmt.Sprintf( + "reshaped: %s (%s) -> %s x%d, payment $%s, exchange %s", + o.SourceRIID, o.SourceInstanceType, o.TargetInstanceType, o.TargetCount, o.PaymentDue, o.ExchangeID)) + } + for i := range result.Pending { + o := &result.Pending[i] + summary.Details = append(summary.Details, fmt.Sprintf( + "pending approval (not executed): %s (%s) -> %s x%d, payment $%s", + o.SourceRIID, o.SourceInstanceType, o.TargetInstanceType, o.TargetCount, o.PaymentDue)) + } + for i := range result.Failed { + o := &result.Failed[i] + summary.Details = append(summary.Details, fmt.Sprintf( + "failed: %s (%s) -> %s: %s", + o.SourceRIID, o.SourceInstanceType, o.TargetInstanceType, o.Error)) + } + for i := range result.Skipped { + s := &result.Skipped[i] + summary.Details = append(summary.Details, fmt.Sprintf( + "skipped: %s (%s): %s", s.SourceRIID, s.SourceInstanceType, s.Reason)) + } + + if len(result.Failed) > 0 { + return summary, fmt.Errorf( + "ReshapeBuffer: %d of %d exchange attempt(s) failed (first: %s: %s); see summary details for the full list", + len(result.Failed), summary.Analyzed, result.Failed[0].SourceRIID, result.Failed[0].Error) + } + return summary, nil +} diff --git a/providers/aws/ladder/reshape_test.go b/providers/aws/ladder/reshape_test.go new file mode 100644 index 000000000..432a3c28c --- /dev/null +++ b/providers/aws/ladder/reshape_test.go @@ -0,0 +1,265 @@ +package ladder + +import ( + "context" + "errors" + "math" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/LeanerCloud/CUDly/pkg/common" + "github.com/LeanerCloud/CUDly/pkg/exchange" + "github.com/LeanerCloud/CUDly/pkg/ladder" +) + +// fakeExchangeRunner is a hermetic exchangeRunner double that records the +// config it received. Field order minimizes GC pointer-scan range. +type fakeExchangeRunner struct { + err error + result *exchange.AutoExchangeResult + gotCfg exchange.RIExchangeConfig + calls int +} + +func (f *fakeExchangeRunner) RunAutoExchange(_ context.Context, cfg exchange.RIExchangeConfig) (*exchange.AutoExchangeResult, error) { + f.calls++ + f.gotCfg = cfg + if f.result == nil && f.err == nil { + return &exchange.AutoExchangeResult{Mode: cfg.Mode}, nil + } + return f.result, f.err +} + +// validReshapeCfg returns a BufferReshapeConfig that passes all boundary checks. +func validReshapeCfg() ladder.BufferReshapeConfig { + return ladder.BufferReshapeConfig{ + MaxPaymentPerExchangeUSD: ptr(100.0), + MaxPaymentDailyUSD: ptr(500.0), + UtilizationThresholdPct: 20.0, + LookbackDays: 30, + DryRun: false, + } +} + +// --------------------------------------------------------------------------- +// Config mapping +// --------------------------------------------------------------------------- + +func TestReshapeBuffer_CapMapping_SetValuesPassedVerbatim(t *testing.T) { + ex := &fakeExchangeRunner{} + a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex) + + _, err := a.ReshapeBuffer(context.Background(), testScope(), validReshapeCfg()) + require.NoError(t, err) + require.Equal(t, 1, ex.calls) + assert.InDelta(t, 100.0, ex.gotCfg.MaxPaymentPerExchangeUSD, 1e-9) + assert.InDelta(t, 500.0, ex.gotCfg.MaxPaymentDailyUSD, 1e-9) + assert.InDelta(t, 20.0, ex.gotCfg.UtilizationThreshold, 1e-9) + assert.Equal(t, 30, ex.gotCfg.LookbackDays) +} + +func TestReshapeBuffer_CapMapping_NilMeansUnlimited(t *testing.T) { + ex := &fakeExchangeRunner{} + a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex) + + cfg := validReshapeCfg() + cfg.MaxPaymentPerExchangeUSD = nil + cfg.MaxPaymentDailyUSD = nil + _, err := a.ReshapeBuffer(context.Background(), testScope(), cfg) + require.NoError(t, err) + assert.Equal(t, unlimitedCapUSD, ex.gotCfg.MaxPaymentPerExchangeUSD, + "nil per-exchange cap must map to the explicit unlimited constant, never to 0 (0 blocks every exchange)") + assert.Equal(t, unlimitedCapUSD, ex.gotCfg.MaxPaymentDailyUSD, + "nil daily cap must map to the explicit unlimited constant, never to 0") +} + +func TestReshapeBuffer_CapValidation_ZeroAndBadValuesRejected(t *testing.T) { + tests := []struct { + name string + mutate func(*ladder.BufferReshapeConfig) + wantErr string + }{ + {"zero per-exchange cap", func(c *ladder.BufferReshapeConfig) { c.MaxPaymentPerExchangeUSD = ptr(0.0) }, + "MaxPaymentPerExchangeUSD must be > 0"}, + {"negative per-exchange cap", func(c *ladder.BufferReshapeConfig) { c.MaxPaymentPerExchangeUSD = ptr(-5.0) }, + "MaxPaymentPerExchangeUSD must be > 0"}, + {"NaN per-exchange cap", func(c *ladder.BufferReshapeConfig) { c.MaxPaymentPerExchangeUSD = ptr(math.NaN()) }, + "MaxPaymentPerExchangeUSD must be finite"}, + {"Inf per-exchange cap", func(c *ladder.BufferReshapeConfig) { c.MaxPaymentPerExchangeUSD = ptr(math.Inf(1)) }, + "MaxPaymentPerExchangeUSD must be finite"}, + {"zero daily cap", func(c *ladder.BufferReshapeConfig) { c.MaxPaymentDailyUSD = ptr(0.0) }, + "MaxPaymentDailyUSD must be > 0"}, + {"negative daily cap", func(c *ladder.BufferReshapeConfig) { c.MaxPaymentDailyUSD = ptr(-1.0) }, + "MaxPaymentDailyUSD must be > 0"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ex := &fakeExchangeRunner{} + a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex) + + cfg := validReshapeCfg() + tt.mutate(&cfg) + _, err := a.ReshapeBuffer(context.Background(), testScope(), cfg) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + assert.Equal(t, 0, ex.calls, "invalid config must never reach the runner") + }) + } +} + +func TestReshapeBuffer_ThresholdAndLookbackValidation(t *testing.T) { + tests := []struct { + name string + mutate func(*ladder.BufferReshapeConfig) + wantErr string + }{ + {"zero threshold", func(c *ladder.BufferReshapeConfig) { c.UtilizationThresholdPct = 0 }, "UtilizationThresholdPct must be in (0, 100]"}, + {"negative threshold", func(c *ladder.BufferReshapeConfig) { c.UtilizationThresholdPct = -5 }, "UtilizationThresholdPct must be in (0, 100]"}, + {"threshold above 100", func(c *ladder.BufferReshapeConfig) { c.UtilizationThresholdPct = 101 }, "UtilizationThresholdPct must be in (0, 100]"}, + {"NaN threshold", func(c *ladder.BufferReshapeConfig) { c.UtilizationThresholdPct = math.NaN() }, "UtilizationThresholdPct must be in (0, 100]"}, + {"zero lookback", func(c *ladder.BufferReshapeConfig) { c.LookbackDays = 0 }, "LookbackDays must be > 0"}, + {"negative lookback", func(c *ladder.BufferReshapeConfig) { c.LookbackDays = -7 }, "LookbackDays must be > 0"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ex := &fakeExchangeRunner{} + a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex) + + cfg := validReshapeCfg() + tt.mutate(&cfg) + _, err := a.ReshapeBuffer(context.Background(), testScope(), cfg) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + assert.Equal(t, 0, ex.calls) + }) + } +} + +func TestReshapeBuffer_DryRun_NotSupported_FailsLoudWithoutCallingRunner(t *testing.T) { + // DryRun must NOT be mapped onto the exchange flow's manual mode: manual + // mode persists pending ExchangeRecords with live approval tokens and + // RunAutoExchange unconditionally cancels ALL pending records store-wide + // first — neither is a simulation. The decided behavior is a loud error. + ex := &fakeExchangeRunner{} + a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex) + + cfg := validReshapeCfg() + cfg.DryRun = true + _, err := a.ReshapeBuffer(context.Background(), testScope(), cfg) + require.Error(t, err) + assert.Contains(t, err.Error(), "not supported") + assert.Equal(t, 0, ex.calls, "a dry run must never reach the runner (it would cancel unrelated pending exchanges)") +} + +func TestReshapeBuffer_LiveRun_AlwaysAutoMode(t *testing.T) { + ex := &fakeExchangeRunner{} + a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex) + + cfg := validReshapeCfg() + cfg.DryRun = false + _, err := a.ReshapeBuffer(context.Background(), testScope(), cfg) + require.NoError(t, err) + require.Equal(t, 1, ex.calls) + assert.Equal(t, string(exchange.ExchangeModeAuto), ex.gotCfg.Mode) +} + +// --------------------------------------------------------------------------- +// Outcome mapping +// --------------------------------------------------------------------------- + +func TestReshapeBuffer_SummaryMapping(t *testing.T) { + ex := &fakeExchangeRunner{result: &exchange.AutoExchangeResult{ + Mode: string(exchange.ExchangeModeAuto), + Completed: []exchange.ExchangeOutcome{ + {SourceRIID: "ri-1", SourceInstanceType: "m5.large", TargetInstanceType: "m5.xlarge", TargetCount: 1, PaymentDue: "12.34", ExchangeID: "ex-1"}, + }, + Pending: []exchange.ExchangeOutcome{ + {SourceRIID: "ri-2", SourceInstanceType: "c5.large", TargetInstanceType: "c5.xlarge", TargetCount: 2, PaymentDue: "0"}, + }, + Skipped: []exchange.SkippedRecommendation{ + {SourceRIID: "ri-3", SourceInstanceType: "r5.large", Reason: "exceeds per-exchange cap"}, + }, + }} + a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex) + + summary, err := a.ReshapeBuffer(context.Background(), testScope(), validReshapeCfg()) + require.NoError(t, err) + + assert.Equal(t, 3, summary.Analyzed, "Analyzed = completed + pending + failed + skipped") + assert.Equal(t, 1, summary.Reshaped, "only executed exchanges count as reshaped") + assert.Equal(t, 1, summary.Skipped) + require.Len(t, summary.Details, 3) + assert.Contains(t, summary.Details[0], "reshaped: ri-1") + assert.Contains(t, summary.Details[0], "ex-1") + assert.Contains(t, summary.Details[1], "pending approval (not executed): ri-2") + assert.Contains(t, summary.Details[2], "skipped: ri-3") + assert.Contains(t, summary.Details[2], "exceeds per-exchange cap") +} + +func TestReshapeBuffer_PartialFailure_SummaryPlusError(t *testing.T) { + ex := &fakeExchangeRunner{result: &exchange.AutoExchangeResult{ + Mode: string(exchange.ExchangeModeAuto), + Completed: []exchange.ExchangeOutcome{ + {SourceRIID: "ri-ok", SourceInstanceType: "m5.large", TargetInstanceType: "m5.xlarge", TargetCount: 1}, + }, + Failed: []exchange.ExchangeOutcome{ + {SourceRIID: "ri-bad", SourceInstanceType: "c5.large", TargetInstanceType: "c5.xlarge", Error: "AWS exchange rejected"}, + }, + }} + a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex) + + summary, err := a.ReshapeBuffer(context.Background(), testScope(), validReshapeCfg()) + require.Error(t, err, "partial failures on a money path must surface as an error, not be absorbed") + assert.Contains(t, err.Error(), "1 of 2 exchange attempt(s) failed") + assert.Contains(t, err.Error(), "ri-bad") + + // The summary is still populated for audit alongside the error. + assert.Equal(t, 2, summary.Analyzed) + assert.Equal(t, 1, summary.Reshaped) + assert.Equal(t, 0, summary.Skipped, "failed attempts are not counted as skipped") + require.Len(t, summary.Details, 2) + assert.Contains(t, summary.Details[1], "failed: ri-bad") +} + +func TestReshapeBuffer_RunnerError_Propagates(t *testing.T) { + runnerErr := errors.New("exchange store unavailable") + ex := &fakeExchangeRunner{err: runnerErr} + a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex) + + _, err := a.ReshapeBuffer(context.Background(), testScope(), validReshapeCfg()) + require.Error(t, err) + assert.ErrorIs(t, err, runnerErr) + assert.Contains(t, err.Error(), "auto exchange run failed") +} + +func TestReshapeBuffer_NilResultWithoutError_IsContractViolation(t *testing.T) { + // The fake returns a synthetic result when both fields are zero, so force + // the nil-result path with a sentinel: result nil, err nil is only + // reachable when the runner violates its contract. + ex := &nilResultRunner{} + a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex) + + _, err := a.ReshapeBuffer(context.Background(), testScope(), validReshapeCfg()) + require.Error(t, err) + assert.Contains(t, err.Error(), "nil result") +} + +// nilResultRunner deliberately violates the runner contract for the guard test. +type nilResultRunner struct{} + +func (n *nilResultRunner) RunAutoExchange(_ context.Context, _ exchange.RIExchangeConfig) (*exchange.AutoExchangeResult, error) { + return nil, nil +} + +func TestReshapeBuffer_WrongScope_ReturnsError(t *testing.T) { + ex := &fakeExchangeRunner{} + a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex) + + badScope := ladder.Scope{Provider: common.ProviderAWS, AccountID: "999"} + _, err := a.ReshapeBuffer(context.Background(), badScope, validReshapeCfg()) + require.Error(t, err) + assert.Contains(t, err.Error(), "does not match configured account") + assert.Equal(t, 0, ex.calls) +} From 49635c3ab4bb3146a6ca4b55ee9c58cb6b940e7d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 3 Jul 2026 22:03:11 +0200 Subject: [PATCH 6/8] fix(aws): address phase-2 review findings on ladder write side Require non-empty Platform/Tenancy/Scope on EC2 RI purchase recs (the ec2 client silently defaults empty Tenancy to "default" and Scope to "Regional", which could buy a default-tenancy RI from a dedicated-tenancy rec). Correct the ReshapeBuffer godoc: daily-cap stops surface as Failed plus an error, not Skipped (upstream reclassification tracked in #1348). Add tests for the new rejections, +Inf HourlyCommitment, and the inverse plan-type mismatch. --- providers/aws/ladder/purchase.go | 15 ++++++++++++++ providers/aws/ladder/purchase_test.go | 28 +++++++++++++++++++++++++++ providers/aws/ladder/reshape.go | 14 ++++++++++---- 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/providers/aws/ladder/purchase.go b/providers/aws/ladder/purchase.go index 05cd94c8e..d1563baaa 100644 --- a/providers/aws/ladder/purchase.go +++ b/providers/aws/ladder/purchase.go @@ -96,6 +96,12 @@ func (a *AWSLadder) purchaseSP(ctx context.Context, layer ladder.LayerType, plan // InstanceType/Platform/Tenancy/Scope from it), a positive instance count // (PurchaseReservedInstancesOffering InstanceCount), and the term/payment // option strings the offering query converts. +// +// Platform, Tenancy, and Scope are REQUIRED non-empty (no-silent-fallback +// rule): the ec2 client silently defaults an empty Tenancy to "default" and +// an empty Scope to "Regional", which could buy a default-tenancy RI from a +// recommendation that meant dedicated tenancy. On this money path the intent +// must be explicit, so empties are rejected here before any AWS call. func validateRIPurchaseRec(rec *common.Recommendation) error { details, ok := rec.Details.(*common.ComputeDetails) if !ok || details == nil { @@ -107,6 +113,15 @@ func validateRIPurchaseRec(rec *common.Recommendation) error { if details.InstanceType == "" { return fmt.Errorf("ComputeDetails.InstanceType must not be empty for an EC2 RI purchase") } + if details.Platform == "" { + return fmt.Errorf("ComputeDetails.Platform must not be empty for an EC2 RI purchase (offering lookup matches on it)") + } + if details.Tenancy == "" { + return fmt.Errorf("ComputeDetails.Tenancy must not be empty for an EC2 RI purchase (the ec2 client would silently default it to %q)", "default") + } + if details.Scope == "" { + return fmt.Errorf("ComputeDetails.Scope must not be empty for an EC2 RI purchase (the ec2 client would silently default it to %q)", "Regional") + } return validateTermAndPayment(rec) } diff --git a/providers/aws/ladder/purchase_test.go b/providers/aws/ladder/purchase_test.go index ed16325d3..0ad223027 100644 --- a/providers/aws/ladder/purchase_test.go +++ b/providers/aws/ladder/purchase_test.go @@ -213,6 +213,18 @@ func TestPurchaseLayer_RIRecValidation(t *testing.T) { {"empty instance type", func(r *common.Recommendation) { r.Details = &common.ComputeDetails{Platform: "linux", Tenancy: "default", Scope: "regional"} }, "InstanceType must not be empty"}, + {"empty platform", func(r *common.Recommendation) { + r.Details = &common.ComputeDetails{InstanceType: "m5.large", Tenancy: "default", Scope: "regional"} + }, "Platform must not be empty"}, + {"empty tenancy", func(r *common.Recommendation) { + // The ec2 client silently defaults empty Tenancy to "default"; a + // dedicated-tenancy rec would buy the wrong product (no-silent-fallback). + r.Details = &common.ComputeDetails{InstanceType: "m5.large", Platform: "linux", Scope: "regional"} + }, "Tenancy must not be empty"}, + {"empty scope", func(r *common.Recommendation) { + // The ec2 client silently defaults empty Scope to "Regional". + r.Details = &common.ComputeDetails{InstanceType: "m5.large", Platform: "linux", Tenancy: "default"} + }, "Scope must not be empty"}, {"empty term", func(r *common.Recommendation) { r.Term = "" }, "Term must not be empty"}, {"empty payment option", func(r *common.Recommendation) { r.PaymentOption = "" }, "PaymentOption must not be empty"}, } @@ -253,6 +265,9 @@ func TestPurchaseLayer_SPRecValidation(t *testing.T) { {"NaN hourly commitment", func(r *common.Recommendation) { r.Details = &common.SavingsPlanDetails{PlanType: spPlanTypeEC2Instance, HourlyCommitment: math.NaN()} }, "HourlyCommitment must be a positive finite value"}, + {"+Inf hourly commitment", func(r *common.Recommendation) { + r.Details = &common.SavingsPlanDetails{PlanType: spPlanTypeEC2Instance, HourlyCommitment: math.Inf(1)} + }, "HourlyCommitment must be a positive finite value"}, {"empty term", func(r *common.Recommendation) { r.Term = "" }, "Term must not be empty"}, {"empty payment option", func(r *common.Recommendation) { r.PaymentOption = "" }, "PaymentOption must not be empty"}, } @@ -271,6 +286,19 @@ func TestPurchaseLayer_SPRecValidation(t *testing.T) { } } +func TestPurchaseLayer_SPPlanTypeMismatch_InverseDirection(t *testing.T) { + // The mismatch table above covers Compute details dispatched to the + // EC2Instance layer; this covers the inverse: EC2Instance details + // dispatched to the Compute layer must be rejected the same way. + spP := &fakePurchaser{} + a := newWiredLadder(t, &fakePurchaser{}, spP, &fakeExchangeRunner{}) + + _, err := a.PurchaseLayer(context.Background(), ladder.LayerComputeSP, validSPRec(spPlanTypeEC2Instance), validPurchaseOpts()) + require.Error(t, err) + assert.Contains(t, err.Error(), "does not match the dispatched layer's plan type") + assert.Equal(t, 0, spP.calls, "a mismatched plan type must never reach the client") +} + // --------------------------------------------------------------------------- // PurchaseLayer error propagation // --------------------------------------------------------------------------- diff --git a/providers/aws/ladder/reshape.go b/providers/aws/ladder/reshape.go index a0d3c8e50..87d391588 100644 --- a/providers/aws/ladder/reshape.go +++ b/providers/aws/ladder/reshape.go @@ -62,10 +62,16 @@ const unlimitedCapUSD = math.MaxFloat64 // (the thin runner seam does not expose it); it counts the commitments the // exchange analysis flagged and processed. // - Reshaped = Completed only (exchanges actually executed). -// - Skipped = Skipped only (below threshold, no offering, over cap, ...). -// Failed attempts are not "skipped"; they surface in Details AND as a -// non-nil error (money-path failures must never be silently absorbed -// into a success-looking summary). +// - Skipped = Skipped only: below the utilization threshold, no matching +// offering, invalid quote, or over the PER-EXCHANGE cap. A DAILY-cap stop +// is NOT in this bucket: pkg/exchange classifies it as Failed +// (saveFailedRecord + result.Failed), so a routine daily-cap policy stop +// currently surfaces from this method as "N of M failed" plus a non-nil +// error. Upstream reclassification of daily-cap stops as skips is +// tracked in #1348. +// Failed attempts are never counted as "skipped"; they surface in +// Details AND as a non-nil error (money-path failures must never be +// silently absorbed into a success-looking summary). // // Partial failures: when the runner reports failed exchange attempts, the // populated summary is returned TOGETHER with a non-nil error so callers get From b79df8ce3e570015aa78c36e522c595d8aed7112 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 3 Jul 2026 22:07:38 +0200 Subject: [PATCH 7/8] chore(aws): address review nitpicks on ladder write side Add direct coverage of the write-side-not-wired guard (errors.Is on both write methods of a New()-built ladder). Fix the stale read-dependency count in the AWSLadder doc comment (four -> five after the coverage interface split) and the stale "coverage source" reference in the short-series error. Denominate the partial-failure message by actual attempts (completed + failed) instead of Analyzed, which includes never-attempted items. --- providers/aws/ladder/baseline.go | 2 +- providers/aws/ladder/ladder.go | 2 +- providers/aws/ladder/purchase_test.go | 18 ++++++++++++++++++ providers/aws/ladder/reshape.go | 6 +++++- providers/aws/ladder/reshape_test.go | 16 ++++++++++++---- 5 files changed, 37 insertions(+), 7 deletions(-) diff --git a/providers/aws/ladder/baseline.go b/providers/aws/ladder/baseline.go index d185199e2..36c474290 100644 --- a/providers/aws/ladder/baseline.go +++ b/providers/aws/ladder/baseline.go @@ -64,7 +64,7 @@ func (a *AWSLadder) GetUsageBaseline(ctx context.Context, scope ladder.Scope, lo } if len(series) < minBaselineSeriesDays { return ladder.UsageBaseline{}, fmt.Errorf( - "GetUsageBaseline: series length %d is below minimum %d days; extend the lookback window or check the coverage source", + "GetUsageBaseline: series length %d is below minimum %d days; extend the lookback window or check the on-demand series source", len(series), minBaselineSeriesDays, ) } diff --git a/providers/aws/ladder/ladder.go b/providers/aws/ladder/ladder.go index 58dd28b01..554760d03 100644 --- a/providers/aws/ladder/ladder.go +++ b/providers/aws/ladder/ladder.go @@ -60,7 +60,7 @@ func (c Config) lookbackDays() int { // (ListCommitments, GetLayerStates, GetUsageBaseline) and the write side // (PurchaseLayer, ReshapeBuffer). // -// All four read data-source dependencies are injected via narrow interfaces so +// All five read data-source dependencies are injected via narrow interfaces so // that unit tests are hermetic (no real AWS calls needed). The caller wires the // concrete adapters (ec2svc.Client, savingsplans.Client, etc.) at startup. // diff --git a/providers/aws/ladder/purchase_test.go b/providers/aws/ladder/purchase_test.go index 0ad223027..b23da0d26 100644 --- a/providers/aws/ladder/purchase_test.go +++ b/providers/aws/ladder/purchase_test.go @@ -112,6 +112,24 @@ func TestWithWriteSide_NilArgsRejected(t *testing.T) { } } +func TestWriteMethods_WithoutWithWriteSide_ReturnErrWriteNotWired(t *testing.T) { + // Direct coverage of the write methods' own nil-dependency guards: a + // New()-built ladder that never had WithWriteSide called must reject + // both write methods with the errWriteNotWired sentinel even when the + // inputs are otherwise fully valid. (No purchaser/runner exists on such + // an instance, so a zero-call assertion is implicit — there is nothing + // wired that could have been invoked.) + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, &fakeCoverageSource{}, &fakeUtilizationSource{}) + + _, err := a.PurchaseLayer(context.Background(), ladder.LayerConvertibleRI, validRIRec(), validPurchaseOpts()) + require.Error(t, err) + assert.ErrorIs(t, err, errWriteNotWired) + + _, err = a.ReshapeBuffer(context.Background(), testScope(), validReshapeCfg()) + require.Error(t, err) + assert.ErrorIs(t, err, errWriteNotWired) +} + // --------------------------------------------------------------------------- // PurchaseLayer dispatch // --------------------------------------------------------------------------- diff --git a/providers/aws/ladder/reshape.go b/providers/aws/ladder/reshape.go index 87d391588..05d6947b7 100644 --- a/providers/aws/ladder/reshape.go +++ b/providers/aws/ladder/reshape.go @@ -192,9 +192,13 @@ func summarizeExchangeResult(result *exchange.AutoExchangeResult) (ladder.Reshap } if len(result.Failed) > 0 { + // Denominate by actual exchange ATTEMPTS (completed + failed), not + // summary.Analyzed: pending and skipped items were never attempted, + // and an inflated denominator would misread during an incident. + attempts := len(result.Completed) + len(result.Failed) return summary, fmt.Errorf( "ReshapeBuffer: %d of %d exchange attempt(s) failed (first: %s: %s); see summary details for the full list", - len(result.Failed), summary.Analyzed, result.Failed[0].SourceRIID, result.Failed[0].Error) + len(result.Failed), attempts, result.Failed[0].SourceRIID, result.Failed[0].Error) } return summary, nil } diff --git a/providers/aws/ladder/reshape_test.go b/providers/aws/ladder/reshape_test.go index 432a3c28c..7430fa5b7 100644 --- a/providers/aws/ladder/reshape_test.go +++ b/providers/aws/ladder/reshape_test.go @@ -199,6 +199,9 @@ func TestReshapeBuffer_SummaryMapping(t *testing.T) { } func TestReshapeBuffer_PartialFailure_SummaryPlusError(t *testing.T) { + // Includes a skipped item so the error's denominator provably counts + // actual ATTEMPTS (completed + failed = 2), not Analyzed (3): skipped + // and pending items were never attempted. ex := &fakeExchangeRunner{result: &exchange.AutoExchangeResult{ Mode: string(exchange.ExchangeModeAuto), Completed: []exchange.ExchangeOutcome{ @@ -207,20 +210,25 @@ func TestReshapeBuffer_PartialFailure_SummaryPlusError(t *testing.T) { Failed: []exchange.ExchangeOutcome{ {SourceRIID: "ri-bad", SourceInstanceType: "c5.large", TargetInstanceType: "c5.xlarge", Error: "AWS exchange rejected"}, }, + Skipped: []exchange.SkippedRecommendation{ + {SourceRIID: "ri-skip", SourceInstanceType: "r5.large", Reason: "exceeds per-exchange cap"}, + }, }} a := newWiredLadder(t, &fakePurchaser{}, &fakePurchaser{}, ex) summary, err := a.ReshapeBuffer(context.Background(), testScope(), validReshapeCfg()) require.Error(t, err, "partial failures on a money path must surface as an error, not be absorbed") - assert.Contains(t, err.Error(), "1 of 2 exchange attempt(s) failed") + assert.Contains(t, err.Error(), "1 of 2 exchange attempt(s) failed", + "denominator must be completed+failed attempts, not Analyzed") assert.Contains(t, err.Error(), "ri-bad") // The summary is still populated for audit alongside the error. - assert.Equal(t, 2, summary.Analyzed) + assert.Equal(t, 3, summary.Analyzed) assert.Equal(t, 1, summary.Reshaped) - assert.Equal(t, 0, summary.Skipped, "failed attempts are not counted as skipped") - require.Len(t, summary.Details, 2) + assert.Equal(t, 1, summary.Skipped, "failed attempts are not counted as skipped") + require.Len(t, summary.Details, 3) assert.Contains(t, summary.Details[1], "failed: ri-bad") + assert.Contains(t, summary.Details[2], "skipped: ri-skip") } func TestReshapeBuffer_RunnerError_Propagates(t *testing.T) { From ed9f25bddbaca80a6582c20c086e2fcd2a4c2011 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 13 Jul 2026 00:06:14 +0300 Subject: [PATCH 8/8] fix(aws): implement SP coverage/utilization on CE test mock mockCostExplorerClient did not implement GetSavingsPlansCoverage or GetSavingsPlansUtilization, which were added to recommendations.CostExplorerAPI when the CE savings-plans queries landed (637c57035). The gap left the root providers/aws test package failing to build (service_client_test.go:41), which only surfaces on `go test`/`go vet`, not `go build`. Bringing in the AWSLadder read side, which exercises those CE queries, makes the stale mock worth fixing now rather than leaving the package un-testable. Add the two missing methods as empty-output stubs, matching the existing mock methods, so mockCostExplorerClient satisfies the full interface again. --- providers/aws/service_client_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/providers/aws/service_client_test.go b/providers/aws/service_client_test.go index 61b5c5b7b..35ae91c18 100644 --- a/providers/aws/service_client_test.go +++ b/providers/aws/service_client_test.go @@ -36,6 +36,14 @@ func (m *mockCostExplorerClient) GetReservationCoverage(ctx context.Context, par return &costexplorer.GetReservationCoverageOutput{}, nil } +func (m *mockCostExplorerClient) GetSavingsPlansCoverage(ctx context.Context, params *costexplorer.GetSavingsPlansCoverageInput, optFns ...func(*costexplorer.Options)) (*costexplorer.GetSavingsPlansCoverageOutput, error) { + return &costexplorer.GetSavingsPlansCoverageOutput{}, nil +} + +func (m *mockCostExplorerClient) GetSavingsPlansUtilization(ctx context.Context, params *costexplorer.GetSavingsPlansUtilizationInput, optFns ...func(*costexplorer.Options)) (*costexplorer.GetSavingsPlansUtilizationOutput, error) { + return &costexplorer.GetSavingsPlansUtilizationOutput{}, nil +} + // newTestRecommendationsClient creates a recommendations client with a mock CE client func newTestRecommendationsClient(ce *mockCostExplorerClient) *recommendations.Client { return recommendations.NewClientWithAPI(ce, "us-east-1")