diff --git a/internal/api/handler_ri_exchange.go b/internal/api/handler_ri_exchange.go index 4304a8932..0a4644552 100644 --- a/internal/api/handler_ri_exchange.go +++ b/internal/api/handler_ri_exchange.go @@ -1605,18 +1605,47 @@ func parseThresholdParam(params map[string]string) (float64, error) { // monthly = (FixedPrice / hours_per_term + UsagePrice + recurring_hourly) × 730 // // 730 is AWS's canonical hours-per-month constant. Returns an error when -// Duration is non-positive: a zero cost would read as a free RI and skip -// reshape's dollar-units pricing gate, so callers must exclude the RI. +// Duration is non-positive or when the computed cost is not positive (for +// example FixedPrice, UsagePrice and RecurringHourlyAmount all zero, which +// means AWS returned no pricing, not a free RI): a zero cost would read as a +// free RI and skip reshape's dollar-units pricing gate, so callers must +// exclude the RI. The error wraps errRINonPositiveDuration or errRINoPricing. // // Used to populate exchange.RIInfo.MonthlyCost so the cross-family // dollar-units pre-filter can compare against per-target offering // costs computed with the same formula. +// +// Keep in sync with the copy in internal/server/handler_ri_exchange.go +// (auto-exchange and ladder path), which applies the same checks but does +// not wrap the sentinels. func monthlyCostFromConvertibleRI(ri ec2svc.ConvertibleRI) (float64, error) { if ri.Duration <= 0 { - return 0, fmt.Errorf("convertible RI %s has non-positive duration %d: monthly cost is not computable", ri.ReservedInstanceID, ri.Duration) + return 0, fmt.Errorf("convertible RI %s has %w %d: monthly cost is not computable", ri.ReservedInstanceID, errRINonPositiveDuration, ri.Duration) } hoursPerTerm := float64(ri.Duration) / 3600 - return ((ri.FixedPrice / hoursPerTerm) + ri.UsagePrice + ri.RecurringHourlyAmount) * 730, nil + cost := ((ri.FixedPrice / hoursPerTerm) + ri.UsagePrice + ri.RecurringHourlyAmount) * 730 + // !(cost > 0) also rejects NaN, which is no usable price either. + if !(cost > 0) { + return 0, fmt.Errorf("convertible RI %s has %w (fixed %v, usage %v, recurring hourly %v): monthly cost is not computable", + ri.ReservedInstanceID, errRINoPricing, ri.FixedPrice, ri.UsagePrice, ri.RecurringHourlyAmount) + } + return cost, nil +} + +// Sentinel causes for monthlyCostFromConvertibleRI, used to pick the reason +// shown for an RI excluded from reshape analysis. +var ( + errRINonPositiveDuration = errors.New("non-positive duration") + errRINoPricing = errors.New("no positive price") +) + +// excludedRIReason is the user-facing reason for an RI left out of reshape +// analysis because monthlyCostFromConvertibleRI returned err. +func excludedRIReason(err error) string { + if errors.Is(err, errRINoPricing) { + return "AWS returned no pricing (fixed, usage and recurring charges total zero or less), so its monthly cost cannot be computed" + } + return "AWS returned no term length (duration), so its monthly cost cannot be computed" } // convertToExchangeTypes converts provider-specific types to the exchange package types. @@ -1630,7 +1659,7 @@ func convertToExchangeTypes(instances []ec2svc.ConvertibleRI, utilData []recomme monthlyCost, err := monthlyCostFromConvertibleRI(inst) if err != nil { logging.Warnf("convertToExchangeTypes: excluding RI from reshape: %v", err) - excluded = append(excluded, ExcludedRI{RIID: inst.ReservedInstanceID, Reason: "AWS returned no term length (duration), so its monthly cost cannot be computed"}) + excluded = append(excluded, ExcludedRI{RIID: inst.ReservedInstanceID, Reason: excludedRIReason(err)}) continue } riInfos = append(riInfos, exchange.RIInfo{ diff --git a/internal/api/handler_ri_exchange_integration_test.go b/internal/api/handler_ri_exchange_integration_test.go index 13b71e06a..b920875c4 100644 --- a/internal/api/handler_ri_exchange_integration_test.go +++ b/internal/api/handler_ri_exchange_integration_test.go @@ -109,6 +109,12 @@ func seedRecsForRegion(ctx context.Context, t *testing.T, store *config.Postgres "seeding recommendations into the test container failed") } +// reshapeIntegrationFixedPrice gives fixture RIs a real price (an unpriced +// RI is excluded, issue #1188) small enough that the dollar-units gate keeps +// the seeded cross-family recs: 100 over 1y is ~8.33/month, and NF 8 x 8.33 +// stays under the c5.large / r5.large recs' NF 4 x 50 and NF 4 x 60. +const reshapeIntegrationFixedPrice = 100 + // TestReshapeRecommendations_Integration_EndToEnd exercises the full // path: auth → cache cold-fetch → AnalyzeReshapingWithRecs (recs-driven // alternatives) → JSON response. Asserts the response carries @@ -135,8 +141,9 @@ func TestReshapeRecommendations_Integration_EndToEnd(t *testing.T) { ec2Fake := &fakeReshapeEC2{ instances: []ec2svc.ConvertibleRI{ - {ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, Duration: 31536000}, + {ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, Duration: 31536000, FixedPrice: reshapeIntegrationFixedPrice}, {ReservedInstanceID: "ri-noterm", InstanceType: "m5.xlarge", InstanceCount: 1}, + {ReservedInstanceID: "ri-unpriced", InstanceType: "m5.xlarge", InstanceCount: 1, Duration: 31536000}, }, } recsFake := &fakeReshapeRecs{ @@ -154,8 +161,11 @@ func TestReshapeRecommendations_Integration_EndToEnd(t *testing.T) { rec := body.Recommendations[0] require.Equal(t, "ri-1", rec.SourceRIID) require.Equal(t, "m5.large", rec.TargetInstanceType) - require.Len(t, body.ExcludedRIs, 1, "an RI with no term length must be reported, not silently skipped") + require.Len(t, body.ExcludedRIs, 2, "RIs with no term length or no pricing must be reported, not silently skipped") require.Equal(t, "ri-noterm", body.ExcludedRIs[0].RIID) + require.Contains(t, body.ExcludedRIs[0].Reason, "term length") + require.Equal(t, "ri-unpriced", body.ExcludedRIs[1].RIID) + require.Contains(t, body.ExcludedRIs[1].Reason, "no pricing") // Same-family m5 stripped; c5 + r5 surface ascending by cost. require.Len(t, rec.AlternativeTargets, 2, @@ -200,7 +210,7 @@ func TestReshapeRecommendations_Integration_SecondCallHitsCache(t *testing.T) { ec2Fake := &fakeReshapeEC2{ instances: []ec2svc.ConvertibleRI{ - {ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, Duration: 31536000}, + {ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, Duration: 31536000, FixedPrice: reshapeIntegrationFixedPrice}, }, } recsFake := &fakeReshapeRecs{ @@ -236,7 +246,7 @@ func TestReshapeRecommendations_Integration_NoCachedRecsReturnsPrimaryOnly(t *te ec2Fake := &fakeReshapeEC2{ instances: []ec2svc.ConvertibleRI{ - {ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, Duration: 31536000}, + {ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, Duration: 31536000, FixedPrice: reshapeIntegrationFixedPrice}, }, } recsFake := &fakeReshapeRecs{ @@ -337,7 +347,7 @@ func TestReshapeRecommendations_Integration_ScopedAccount_FiltersToAccount(t *te ec2Fake := &fakeReshapeEC2{ instances: []ec2svc.ConvertibleRI{ - {ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, Duration: 31536000}, + {ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, Duration: 31536000, FixedPrice: reshapeIntegrationFixedPrice}, }, } recsFake := &fakeReshapeRecs{ diff --git a/internal/api/handler_ri_exchange_test.go b/internal/api/handler_ri_exchange_test.go index 499923dd2..1a733e73e 100644 --- a/internal/api/handler_ri_exchange_test.go +++ b/internal/api/handler_ri_exchange_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "math" "math/big" "strings" "testing" @@ -974,7 +975,7 @@ func TestGetReshapeRecommendations_EmptyRegionUsesConfigRegion(t *testing.T) { reshapeEC2Factory: func(_ aws.Config) reshapeEC2Client { return &fakeReshapeEC2Stub{ instances: []ec2svc.ConvertibleRI{ - {ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, CurrencyCode: "USD", Duration: 31536000}, + {ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, CurrencyCode: "USD", Duration: 31536000, FixedPrice: 876}, }, } }, @@ -1059,8 +1060,11 @@ func TestGetReshapeRecommendations_AlternativeCurrencyFromRIs(t *testing.T) { config: mockStore, auth: &mockAuthForExchange{}, reshapeEC2Factory: func(_ aws.Config) reshapeEC2Client { + // FixedPrice 100 over 1y is ~8.33/month; NF 8 x 8.33 stays + // under the c6i.large rec's NF 4 x 40, so the dollar-units + // gate keeps the alternative this test is about. return &fakeReshapeEC2Stub{instances: []ec2svc.ConvertibleRI{ - {ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, CurrencyCode: tc.riCurrency, Duration: 31536000}, + {ReservedInstanceID: "ri-1", InstanceType: "m5.xlarge", InstanceCount: 1, CurrencyCode: tc.riCurrency, Duration: 31536000, FixedPrice: 100}, }} }, reshapeRecsFactory: func(_ aws.Config) reshapeRecsClient { @@ -2803,3 +2807,101 @@ func TestMonthlyCostFromConvertibleRI_NonPositiveDurationErrors(t *testing.T) { require.NoError(t, err) assert.InDelta(t, 730.0, cost, 1e-6) } + +// A convertible RI whose FixedPrice, UsagePrice and RecurringHourlyAmount are +// all zero has no pricing data; it is not free. MonthlyCost=0 would make +// reshape's pricing gate pass every alternative and score the RI as neutral, +// so it must be excluded (issue #1188). A No Upfront RI (FixedPrice 0 with an +// hourly charge) is real pricing and must be kept. +func TestConvertToExchangeTypes_ExcludesRIWithoutPricing(t *testing.T) { + const oneYear = 31536000 + instances := []ec2svc.ConvertibleRI{ + {ReservedInstanceID: "ri-unpriced", InstanceType: "m5.large", InstanceCount: 1, Duration: oneYear}, + {ReservedInstanceID: "ri-no-upfront", InstanceType: "m5.large", InstanceCount: 1, RecurringHourlyAmount: 0.05, Duration: oneYear}, + {ReservedInstanceID: "ri-usage-only", InstanceType: "m5.large", InstanceCount: 1, UsagePrice: 0.04, Duration: oneYear}, + {ReservedInstanceID: "ri-all-upfront", InstanceType: "m5.large", InstanceCount: 1, FixedPrice: 876, Duration: oneYear}, + {ReservedInstanceID: "ri-negative", InstanceType: "c5.large", InstanceCount: 1, FixedPrice: -876, Duration: oneYear}, + } + + riInfos, _, excluded := convertToExchangeTypes(instances, nil) + + require.Len(t, riInfos, 3) + want := map[string]float64{"ri-no-upfront": 0.05 * 730, "ri-usage-only": 0.04 * 730, "ri-all-upfront": 876.0 / 8760 * 730} + for _, ri := range riInfos { + cost, ok := want[ri.ID] + require.True(t, ok, "unexpected RI %s kept", ri.ID) + assert.InDelta(t, cost, ri.MonthlyCost, 1e-6, ri.ID) + assert.Greater(t, ri.MonthlyCost, 0.0, ri.ID) + } + require.Len(t, excluded, 2) + assert.Equal(t, "ri-unpriced", excluded[0].RIID) + assert.Equal(t, "ri-negative", excluded[1].RIID) + for _, e := range excluded { + assert.Contains(t, e.Reason, "no pricing") + assert.NotContains(t, e.Reason, "term length") + } +} + +// End to end through the reshape handler: an unpriced RI is reported in +// excluded_ris with the pricing reason and gets no recommendation, while a +// priced RI in the same account still does. +func TestGetReshapeRecommendations_ReportsUnpricedRIs(t *testing.T) { + mockStore := &MockConfigStore{} + mockStore.On("ListStoredRecommendations", mock.Anything, mock.Anything). + Return([]config.RecommendationRecord(nil), nil) + h := &Handler{ + config: mockStore, + auth: &mockAuthForExchange{}, + reshapeEC2Factory: func(_ aws.Config) reshapeEC2Client { + return &fakeReshapeEC2Stub{instances: []ec2svc.ConvertibleRI{ + {ReservedInstanceID: "ri-unpriced", InstanceType: "m5.xlarge", InstanceCount: 1, Duration: 31536000, CurrencyCode: "USD"}, + {ReservedInstanceID: "ri-ok", InstanceType: "m5.xlarge", InstanceCount: 1, FixedPrice: 8760, Duration: 31536000, CurrencyCode: "USD"}, + }} + }, + reshapeRecsFactory: func(_ aws.Config) reshapeRecsClient { + return &fakeReshapeRecsStub{utilization: []recommendations.RIUtilization{ + {ReservedInstanceID: "ri-unpriced", UtilizationPercent: 50.0}, + {ReservedInstanceID: "ri-ok", UtilizationPercent: 50.0}, + }} + }, + reshapeAccountResolver: func(_ context.Context) (string, error) { return "", nil }, + } + h.awsCfgOnce.Do(func() { h.awsCfg = aws.Config{Region: "us-east-1"} }) + + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"authorization": "Bearer test-token"}, + QueryStringParameters: map[string]string{"lookback_days": "30", "threshold": "95.0"}, + } + out, err := h.getReshapeRecommendations(context.Background(), req) + require.NoError(t, err) + resp, ok := out.(*ReshapeRecommendationsResponse) + require.True(t, ok) + + require.Len(t, resp.ExcludedRIs, 1) + assert.Equal(t, "ri-unpriced", resp.ExcludedRIs[0].RIID) + assert.Contains(t, resp.ExcludedRIs[0].Reason, "no pricing") + require.NotEmpty(t, resp.Recommendations) + for _, r := range resp.Recommendations { + assert.NotEqual(t, "ri-unpriced", r.SourceRIID) + } +} + +func TestMonthlyCostFromConvertibleRI_NoPositivePriceErrors(t *testing.T) { + for name, ri := range map[string]ec2svc.ConvertibleRI{ + "all zero": {ReservedInstanceID: "ri-x", Duration: 31536000}, + "negative sum": {ReservedInstanceID: "ri-x", FixedPrice: -876, Duration: 31536000}, + "NaN": {ReservedInstanceID: "ri-x", UsagePrice: math.NaN(), Duration: 31536000}, + } { + cost, err := monthlyCostFromConvertibleRI(ri) + require.Error(t, err, name) + assert.Zero(t, cost, name) + assert.Contains(t, err.Error(), "ri-x", name) + assert.ErrorIs(t, err, errRINoPricing, name) + assert.NotErrorIs(t, err, errRINonPositiveDuration, name) + } + _, err := monthlyCostFromConvertibleRI(ec2svc.ConvertibleRI{ReservedInstanceID: "ri-y"}) + assert.ErrorIs(t, err, errRINonPositiveDuration, "duration is checked before pricing") + cost, err := monthlyCostFromConvertibleRI(ec2svc.ConvertibleRI{RecurringHourlyAmount: 0.05, Duration: 31536000}) + require.NoError(t, err) + assert.InDelta(t, 36.5, cost, 1e-6) +} diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index 2ba2afd0d..6865a67d3 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -1341,9 +1341,12 @@ paths: excluded_ris: type: array description: > - Convertible RIs left out of cost-based analysis (for - example AWS returned no term length), so they have no - recommendation. Omitted when empty. + Convertible RIs left out of cost-based analysis + because their monthly cost cannot be computed (AWS + returned no term length, or no pricing: fixed, usage + and recurring charges total zero or less), so they have + no recommendation. reason says which. Omitted when + empty. items: type: object properties: diff --git a/internal/server/handler_ri_exchange.go b/internal/server/handler_ri_exchange.go index 86d18605f..b79760e58 100644 --- a/internal/server/handler_ri_exchange.go +++ b/internal/server/handler_ri_exchange.go @@ -244,22 +244,38 @@ func buildExchangeNotificationData(result *exchange.AutoExchangeResult, dashboar // // monthly = (FixedPrice / hours_per_term + UsagePrice + recurring_hourly) × 730 // -// Returns an error when Duration is non-positive: a zero cost would read -// as a free RI and skip the pricing gate, so callers must exclude the RI. +// Returns an error when Duration is non-positive or when the computed cost +// is not positive (for example FixedPrice, UsagePrice and +// RecurringHourlyAmount all zero, which means AWS returned no pricing, not a +// free RI): a zero cost would read as a free RI and skip the pricing gate, +// so callers must exclude the RI. // // Used to populate exchange.RIInfo.MonthlyCost so the cross-family // dollar-units pre-filter compares apples-to-apples against per-target // offering costs. +// +// Keep in sync with the copy in internal/api/handler_ri_exchange.go. That +// copy also wraps errRINonPositiveDuration / errRINoPricing so the API can +// report a per-RI reason; this one does not, because the callers here only +// log and drop the RI. func monthlyCostFromConvertibleRI(ri ec2svc.ConvertibleRI) (float64, error) { if ri.Duration <= 0 { return 0, fmt.Errorf("convertible RI %s has non-positive duration %d: monthly cost is not computable", ri.ReservedInstanceID, ri.Duration) } hoursPerTerm := float64(ri.Duration) / 3600 - return ((ri.FixedPrice / hoursPerTerm) + ri.UsagePrice + ri.RecurringHourlyAmount) * 730, nil + cost := ((ri.FixedPrice / hoursPerTerm) + ri.UsagePrice + ri.RecurringHourlyAmount) * 730 + // !(cost > 0) also rejects NaN, which is no usable price either. + if !(cost > 0) { + return 0, fmt.Errorf("convertible RI %s has no positive price (fixed %v, usage %v, recurring hourly %v): monthly cost is not computable", + ri.ReservedInstanceID, ri.FixedPrice, ri.UsagePrice, ri.RecurringHourlyAmount) + } + return cost, nil } // convertForAutoExchange converts provider-specific types to exchange package types, -// including RI metadata needed for offering lookup. +// including RI metadata needed for offering lookup. It serves both the +// auto-exchange run and the ladder write path (ladder_write.go); an RI whose +// monthly cost is not computable is logged and dropped on both. func convertForAutoExchange(instances []ec2svc.ConvertibleRI, utilData []recommendations.RIUtilization) ([]exchange.RIInfo, []exchange.UtilizationInfo, map[string]exchange.RIMetadataInfo) { riInfos := make([]exchange.RIInfo, 0, len(instances)) riMetadata := make(map[string]exchange.RIMetadataInfo, len(instances)) @@ -268,7 +284,7 @@ func convertForAutoExchange(instances []ec2svc.ConvertibleRI, utilData []recomme inst := instances[i] monthlyCost, err := monthlyCostFromConvertibleRI(inst) if err != nil { - logging.Warnf("convertForAutoExchange: excluding RI from auto-exchange: %v", err) + logging.Warnf("convertForAutoExchange: excluding RI from auto-exchange and ladder reshaping: %v", err) continue } riInfos = append(riInfos, exchange.RIInfo{ diff --git a/internal/server/handler_ri_exchange_reservation_integration_test.go b/internal/server/handler_ri_exchange_reservation_integration_test.go index 5bb5a7609..dece66e9e 100644 --- a/internal/server/handler_ri_exchange_reservation_integration_test.go +++ b/internal/server/handler_ri_exchange_reservation_integration_test.go @@ -63,7 +63,7 @@ func TestRIExchangeAutoWorkers_ConcreteStoreAndFixtureClient(t *testing.T) { } ri := func(id string) []ec2svc.ConvertibleRI { return []ec2svc.ConvertibleRI{{ReservedInstanceID: id, InstanceType: "c5.xlarge", InstanceCount: 1, NormalizationFactor: 8, - ProductDescription: "Linux/UNIX", InstanceTenancy: "default", Scope: "Region", Duration: 31536000}} + ProductDescription: "Linux/UNIX", InstanceTenancy: "default", Scope: "Region", Duration: 31536000, FixedPrice: 876}} } util := func(id string) []recommendations.RIUtilization { return []recommendations.RIUtilization{{ReservedInstanceID: id, UtilizationPercent: 30}} diff --git a/internal/server/handler_ri_exchange_test.go b/internal/server/handler_ri_exchange_test.go index 6d4d998bc..b5bd1890c 100644 --- a/internal/server/handler_ri_exchange_test.go +++ b/internal/server/handler_ri_exchange_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "math" "math/big" "strings" "testing" @@ -140,6 +141,7 @@ func TestConvertForAutoExchange(t *testing.T) { ProductDescription: "Linux/UNIX", InstanceTenancy: "default", Scope: "Region", + FixedPrice: 876, Duration: 31536000, }, { @@ -150,6 +152,7 @@ func TestConvertForAutoExchange(t *testing.T) { ProductDescription: "Windows", InstanceTenancy: "dedicated", Scope: "Availability Zone", + FixedPrice: 876, Duration: 94608000, }, } @@ -568,6 +571,7 @@ func TestExecuteRIExchangeReshape_ManualMode(t *testing.T) { ProductDescription: "Linux/UNIX", InstanceTenancy: "default", Scope: "Region", + FixedPrice: 876, Duration: 31536000, }, }, nil @@ -675,6 +679,7 @@ func TestExecuteRIExchangeReshape_AutoMode(t *testing.T) { ProductDescription: "Linux/UNIX", InstanceTenancy: "default", Scope: "Region", + FixedPrice: 876, Duration: 31536000, }, }, nil @@ -783,6 +788,7 @@ func TestExecuteRIExchangeReshape_DailyCapHitMidRun(t *testing.T) { ProductDescription: "Linux/UNIX", InstanceTenancy: "default", Scope: "Region", + FixedPrice: 876, Duration: 31536000, }, { @@ -793,6 +799,7 @@ func TestExecuteRIExchangeReshape_DailyCapHitMidRun(t *testing.T) { ProductDescription: "Linux/UNIX", InstanceTenancy: "default", Scope: "Region", + FixedPrice: 876, Duration: 31536000, }, }, nil @@ -1106,3 +1113,45 @@ func TestMonthlyCostFromConvertibleRI_NonPositiveDurationErrors(t *testing.T) { t.Fatalf("valid RI: cost=%v err=%v", cost, err) } } + +// A convertible RI whose FixedPrice, UsagePrice and RecurringHourlyAmount are +// all zero has no pricing data; it is not free. It must be excluded like a +// non-positive duration (issue #1188), while a No Upfront RI (FixedPrice 0 +// with an hourly charge) is real pricing and must be kept. +func TestConvertForAutoExchange_ExcludesRIWithoutPricing(t *testing.T) { + const oneYear = 31536000 + instances := []ec2svc.ConvertibleRI{ + {ReservedInstanceID: "ri-unpriced", InstanceType: "m5.large", InstanceCount: 1, Duration: oneYear}, + {ReservedInstanceID: "ri-no-upfront", InstanceType: "m5.large", InstanceCount: 1, RecurringHourlyAmount: 0.05, Duration: oneYear}, + {ReservedInstanceID: "ri-negative", InstanceType: "c5.large", InstanceCount: 1, FixedPrice: -876, Duration: oneYear}, + } + + riInfos, _, meta := convertForAutoExchange(instances, nil) + + if len(riInfos) != 1 || riInfos[0].ID != "ri-no-upfront" { + t.Fatalf("want only ri-no-upfront, got %+v", riInfos) + } + if want := 0.05 * 730; riInfos[0].MonthlyCost < want-1e-6 || riInfos[0].MonthlyCost > want+1e-6 { + t.Fatalf("MonthlyCost = %v, want %v", riInfos[0].MonthlyCost, want) + } + if _, ok := meta["ri-no-upfront"]; !ok || len(meta) != 1 { + t.Fatalf("metadata must cover only ri-no-upfront, got %+v", meta) + } +} + +func TestMonthlyCostFromConvertibleRI_NoPositivePriceErrors(t *testing.T) { + for name, ri := range map[string]ec2svc.ConvertibleRI{ + "all zero": {ReservedInstanceID: "ri-x", Duration: 31536000}, + "negative sum": {ReservedInstanceID: "ri-x", FixedPrice: -876, Duration: 31536000}, + "NaN": {ReservedInstanceID: "ri-x", UsagePrice: math.NaN(), Duration: 31536000}, + } { + cost, err := monthlyCostFromConvertibleRI(ri) + if err == nil || cost != 0 || !strings.Contains(err.Error(), "ri-x") || !strings.Contains(err.Error(), "no positive price") { + t.Fatalf("%s: want error naming ri-x and the missing price, got cost=%v err=%v", name, cost, err) + } + } + cost, err := monthlyCostFromConvertibleRI(ec2svc.ConvertibleRI{RecurringHourlyAmount: 0.05, Duration: 31536000}) + if err != nil || cost < 36.5-1e-6 || cost > 36.5+1e-6 { + t.Fatalf("No Upfront RI: cost=%v err=%v", cost, err) + } +} diff --git a/internal/server/ri_exchange_zone_test.go b/internal/server/ri_exchange_zone_test.go index 1515fd590..d98888695 100644 --- a/internal/server/ri_exchange_zone_test.go +++ b/internal/server/ri_exchange_zone_test.go @@ -58,6 +58,7 @@ func zoneTestRI(scope ec2types.Scope, az string) ec2types.ReservedInstances { InstanceTenancy: ec2types.TenancyDefault, Scope: scope, Duration: aws.Int64(31536000), + FixedPrice: aws.Float32(876), OfferingClass: ec2types.OfferingClassTypeConvertible, CurrencyCode: ec2types.CurrencyCodeValuesUsd, } @@ -179,8 +180,8 @@ func TestNewConvertibleOfferingLookup_MapsEveryField(t *testing.T) { func TestConvertForAutoExchange_CarriesAvailabilityZone(t *testing.T) { _, _, meta := convertForAutoExchange([]ec2svc.ConvertibleRI{ - {ReservedInstanceID: "ri-zonal", Scope: "Availability Zone", AvailabilityZone: "us-east-1b", Duration: 31536000}, - {ReservedInstanceID: "ri-regional", Scope: "Region", Duration: 31536000}, + {ReservedInstanceID: "ri-zonal", Scope: "Availability Zone", AvailabilityZone: "us-east-1b", Duration: 31536000, FixedPrice: 876}, + {ReservedInstanceID: "ri-regional", Scope: "Region", Duration: 31536000, FixedPrice: 876}, }, nil) assert.Equal(t, "us-east-1b", meta["ri-zonal"].AvailabilityZone) assert.Empty(t, meta["ri-regional"].AvailabilityZone)