diff --git a/CHANGELOG.md b/CHANGELOG.md index 93b1a00..f29383e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Added +- **`ladder.AllocateResult.NetExistingUSDPerHour`:** the net existing + commitment (existing minus expiring, summed over layers) that `Allocate` + sized the gap against, set on every successful result including the + hold-only exits. Callers can report it instead of recomputing a gross + figure from the layer states, so a plan's existing figure reconciles with + its gap. `Allocate` now checks the snapshot before the baseline-unavailable + hold, so expiring above existing is an error there too (#83). - **`exchange.WithOfferedInstanceTypes` / `RunAutoExchangeParams.OfferedInstanceTypes`:** restrict RI reshape targets to the instance types the region offers. `AnalyzeReshaping` and `AnalyzeReshapingWithRecs` take it as a variadic diff --git a/pkg/ladder/allocate.go b/pkg/ladder/allocate.go index e3ec56a..9bd26c2 100644 --- a/pkg/ladder/allocate.go +++ b/pkg/ladder/allocate.go @@ -103,10 +103,18 @@ type Allocation struct { // no-op if and only if Allocations and Reshapes are both empty (see IsNoOp); // in that case Holds carries the explanation (baseline unavailable, gap below // the minimum threshold, or every split amount below the minimum). +// +// NetExistingUSDPerHour is the net existing commitment E that Allocate sized +// the gap against: the sum over layers of ExistingUSDPerHour minus +// ExpiringUSDPerHour, in the same on-demand-equivalent USD/hr unit as the +// inputs. It is set on every successful result, including the hold-only +// early exits, so callers report the figure the engine used instead of +// recomputing a gross one from the layer states. type AllocateResult struct { - Allocations []Allocation - Reshapes []PlannedAction - Holds []PlannedAction + NetExistingUSDPerHour *big.Rat + Allocations []Allocation + Reshapes []PlannedAction + Holds []PlannedAction } // IsNoOp reports whether the result proposes no state-changing actions: @@ -121,7 +129,10 @@ func (r *AllocateResult) IsNoOp() bool { // arithmetic; float64 inputs are converted at the boundary via ratFromFloat. // // Steps (see inline comments): -// 1. Validate config, layers (enums + topology), and layer states. +// 1. Validate config, layers (enums + topology), and layer states, then +// compute net existing commitment (reported on every result as +// NetExistingUSDPerHour; an inconsistent snapshot is an error even when +// the baseline is unavailable). // 2. Detect under-utilized buffer layers (reshape, or informational hold // when utilization is unknown on a non-empty buffer layer). This runs // INDEPENDENTLY of the purchase gap: an over-committed account is exactly @@ -148,6 +159,23 @@ func Allocate(in *AllocationInput) (*AllocateResult, error) { if err := validateAllocateInput(in); err != nil { return nil, err } + nets, existingTotal, err := computeNetExisting(in.Layers, in.LayerStates) + if err != nil { + return nil, err + } + result, err := allocate(in, nets, existingTotal) + if err != nil { + return nil, err + } + result.NetExistingUSDPerHour = new(big.Rat).Set(existingTotal) + return result, nil +} + +// allocate runs steps 2-8 of Allocate on validated input. nets and +// existingTotal are the per-layer and total net existing commitment from +// computeNetExisting, computed by Allocate before any early exit so every +// result can report the E the gap is sized against. +func allocate(in *AllocationInput, nets map[LayerType]*big.Rat, existingTotal *big.Rat) (*AllocateResult, error) { rates, err := parseCommitmentRates(in.CommitmentRates) if err != nil { return nil, err @@ -160,10 +188,6 @@ func Allocate(in *AllocationInput) (*AllocateResult, error) { if in.Baseline.LowWaterUSDPerHour == nil { return withMaintenance(baselineUnavailableHold(in), reshapes, maintHolds), nil } - nets, existingTotal, err := computeNetExisting(in.Layers, in.LayerStates) - if err != nil { - return nil, err - } lowWater, target, err := computeBaselineAndTarget(in) if err != nil { return nil, err diff --git a/pkg/ladder/allocate_net_existing_test.go b/pkg/ladder/allocate_net_existing_test.go new file mode 100644 index 0000000..4e60520 --- /dev/null +++ b/pkg/ladder/allocate_net_existing_test.go @@ -0,0 +1,120 @@ +package ladder + +import ( + "math/big" + "strings" + "testing" +) + +// netExistingStates returns AWS layer states with $10/hr existing on the +// ComputeSP layer, $4/hr of which expires this cadence window, plus $2/hr on +// the EC2InstanceSP layer with nothing expiring. Gross existing is $12/hr and +// net existing (what Allocate subtracts from the target) is $8/hr. +func netExistingStates(layers []LayerSpec) map[LayerType]LayerState { + states := zeroStates(layers) + states = withExisting(states, LayerComputeSP, 10.0) + states = withExpiring(states, LayerComputeSP, 4.0) + return withExisting(states, LayerEC2InstanceSP, 2.0) +} + +// TestAllocate_ReportsNetExisting verifies the result carries the net +// existing commitment (existing - expiring, summed over layers) that Allocate +// used to size the gap, on every successful return path, so callers can +// report the same figure instead of recomputing a gross one (go#83). +func TestAllocate_ReportsNetExisting(t *testing.T) { + t.Parallel() + cases := []struct { + lowWater *float64 + name string + wantAlloc bool + }{ + // target $20 > net $8: purchases are planned. + {name: "allocation", lowWater: ptr(20.0), wantAlloc: true}, + // target $8 == net $8: target-met hold, nothing bought. + {name: "target met hold", lowWater: ptr(8.0)}, + // no baseline: baseline-unavailable hold. + {name: "baseline unavailable hold", lowWater: nil}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + layers := awsLayers() + result, err := Allocate(&AllocationInput{ + Config: validConfigAWS(), + Layers: layers, + Baseline: UsageBaseline{LowWaterUSDPerHour: tc.lowWater}, + LayerStates: netExistingStates(layers), + Now: nowFixed(), + InFlight: noInFlight(), + CommitmentRates: unitRates(), + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result.NetExistingUSDPerHour == nil { + t.Fatal("NetExistingUSDPerHour is nil; want the net existing commitment Allocate used") + } + ratEq(t, "NetExistingUSDPerHour", result.NetExistingUSDPerHour, 8, 1) + if got := len(result.Allocations) > 0; got != tc.wantAlloc { + t.Fatalf("allocations present = %v, want %v (holds: %+v)", got, tc.wantAlloc, result.Holds) + } + }) + } +} + +// TestAllocate_NetExistingReconcilesWithGap verifies that the reported net +// existing figure is the E the gap was sized against: with unit rates and +// 100% coverage, target - NetExistingUSDPerHour equals the sum of the +// planned on-demand-equivalent gaps. A gross figure ($12/hr here) would not +// reconcile, which is the approval-email inconsistency go#83 describes. +func TestAllocate_NetExistingReconcilesWithGap(t *testing.T) { + t.Parallel() + layers := awsLayers() + result, err := Allocate(&AllocationInput{ + Config: validConfigAWS(), + Layers: layers, + Baseline: UsageBaseline{LowWaterUSDPerHour: ptr(20.0)}, + LayerStates: netExistingStates(layers), + Now: nowFixed(), + InFlight: noInFlight(), + CommitmentRates: unitRates(), + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result.NetExistingUSDPerHour == nil { + t.Fatal("NetExistingUSDPerHour is nil") + } + planned := new(big.Rat) + for _, a := range result.Allocations { + planned.Add(planned, a.OnDemandEquivalentUSDPerHour) + } + reconciled := new(big.Rat).Add(planned, result.NetExistingUSDPerHour) + ratEq(t, "planned gap + NetExistingUSDPerHour", reconciled, 20, 1) +} + +// TestAllocate_NilBaselineInconsistentSnapshot_Error verifies that net +// existing is computed (and an inconsistent snapshot rejected) before the +// baseline-unavailable hold, so the hold never reports an existing figure +// derived from expiring > existing. +func TestAllocate_NilBaselineInconsistentSnapshot_Error(t *testing.T) { + t.Parallel() + layers := awsLayers() + states := withExisting(zeroStates(layers), LayerConvertibleRI, 2.0) + states = withExpiring(states, LayerConvertibleRI, 5.0) + _, err := Allocate(&AllocationInput{ + Config: validConfigAWS(), + Layers: layers, + Baseline: UsageBaseline{}, + LayerStates: states, + Now: nowFixed(), + InFlight: noInFlight(), + CommitmentRates: unitRates(), + }) + if err == nil { + t.Fatal("expected an error for expiring > existing with a nil baseline, got nil") + } + if !strings.Contains(err.Error(), "inconsistent provider snapshot") { + t.Errorf("error missing %q: %v", "inconsistent provider snapshot", err) + } +} diff --git a/pkg/ladder/capability.go b/pkg/ladder/capability.go index e4a4b76..07f762b 100644 --- a/pkg/ladder/capability.go +++ b/pkg/ladder/capability.go @@ -86,11 +86,12 @@ type LadderCapability interface { // is needed; NaN/Inf/non-positive values are rejected wherever these caps // are validated. type BufferReshapeConfig struct { //nolint:govet // fieldalignment: file owned by open PR #441 - // MaxPaymentPerExchangeUSD caps the payment for a single exchange. nil - // means no per-exchange cap is applied. + // MaxPaymentPerExchangeUSD caps the payment for a single exchange. It is + // required: nil is an error, never "no cap" (reshape executes exchanges + // immediately, so an omitted cap must not mean unlimited spend). MaxPaymentPerExchangeUSD *float64 // MaxPaymentDailyUSD caps the total exchange payments for the current day. - // nil means no daily cap is applied. + // It is required on the same terms as MaxPaymentPerExchangeUSD. MaxPaymentDailyUSD *float64 // UtilizationThresholdPct triggers a reshape when commitment utilization // drops below this percentage.