diff --git a/providers/aws/ladder/baseline.go b/providers/aws/ladder/baseline.go index 1a6fa461c..36c474290 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,16 +55,16 @@ 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( - "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, ) } @@ -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..554760d03 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 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. // +// 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..d1563baaa --- /dev/null +++ b/providers/aws/ladder/purchase.go @@ -0,0 +1,161 @@ +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. +// +// 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 { + 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") + } + 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) +} + +// 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..b23da0d26 --- /dev/null +++ b/providers/aws/ladder/purchase_test.go @@ -0,0 +1,347 @@ +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) + }) + } +} + +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 +// --------------------------------------------------------------------------- + +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 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"}, + } + 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"}, + {"+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"}, + } + 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") + }) + } +} + +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 +// --------------------------------------------------------------------------- + +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..05d6947b7 --- /dev/null +++ b/providers/aws/ladder/reshape.go @@ -0,0 +1,204 @@ +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 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 +// 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 { + // 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), 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 new file mode 100644 index 000000000..7430fa5b7 --- /dev/null +++ b/providers/aws/ladder/reshape_test.go @@ -0,0 +1,273 @@ +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) { + // 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{ + {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"}, + }, + 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", + "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, 3, summary.Analyzed) + assert.Equal(t, 1, summary.Reshaped) + 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) { + 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) +}