diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index b26750c9f..1ab3c5ae2 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -2425,6 +2425,7 @@ const PAYMENT_DISPLAY_LABELS: Record = { 'all-upfront': 'All Upfront', 'partial-upfront': 'Partial Upfront', 'no-upfront': 'No Upfront', + 'upfront': 'Upfront', 'monthly': 'Monthly', }; diff --git a/providers/azure/internal/recommendations/converter.go b/providers/azure/internal/recommendations/converter.go index 565fcd812..69d52f8b7 100644 --- a/providers/azure/internal/recommendations/converter.go +++ b/providers/azure/internal/recommendations/converter.go @@ -25,6 +25,7 @@ package recommendations import ( "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption" + "github.com/LeanerCloud/CUDly/pkg/common" "github.com/LeanerCloud/CUDly/pkg/logging" ) @@ -246,3 +247,64 @@ func strDeref(s *string) string { } return *s } + +// termToMonths maps a normalised term string ("1yr", "3yr") to the number of +// months it spans. Unknown terms default to 12 so the monthly-cost arithmetic +// stays valid rather than dividing by zero. +func termToMonths(term string) int { + switch term { + case "3yr": + return 36 + default: + return 12 + } +} + +// ExpandPaymentVariants fans out a single Azure reservation recommendation +// into two variants that differ only in payment schedule: +// +// - "upfront" — the full reservation cost is paid today; no monthly +// recurring charge (RecurringMonthlyCost = pointer to 0). +// - "monthly" — nothing is paid today; the same total reservation cost +// is spread evenly across the term months (RecurringMonthlyCost = +// CommitmentCost / termMonths). +// +// Azure charges the same total reservation price for both billing plans +// (unlike AWS, which prices partial-upfront separately), so EstimatedSavings +// and SavingsPercentage vs on-demand are identical between the two variants; +// only the cashflow split changes. +// +// The base recommendation must already have PaymentOption set to "upfront" +// and a valid CommitmentCost (total reservation price) and OnDemandCost (total +// on-demand cost over the same period). If OnDemandCost is zero the savings +// fields are forced to zero to avoid a divide-by-zero; if CommitmentCost is +// zero both variants are still emitted with zero costs (caller's responsibility +// to validate upstream). +func ExpandPaymentVariants(base common.Recommendation) []common.Recommendation { + totalReservation := base.CommitmentCost + totalOnDemand := base.OnDemandCost + + var savingsPct float64 + var savings float64 + if totalOnDemand != 0 { + savings = totalOnDemand - totalReservation + savingsPct = savings / totalOnDemand * 100 + } + + months := termToMonths(base.Term) + recurringMonthly := totalReservation / float64(months) + + allUpfront := base + allUpfront.PaymentOption = "upfront" + allUpfront.EstimatedSavings = savings + allUpfront.SavingsPercentage = savingsPct + allUpfront.RecurringMonthlyCost = float64Ptr(0) + + noUpfront := base + noUpfront.PaymentOption = "monthly" + noUpfront.EstimatedSavings = savings + noUpfront.SavingsPercentage = savingsPct + noUpfront.RecurringMonthlyCost = float64Ptr(recurringMonthly) + + return []common.Recommendation{allUpfront, noUpfront} +} diff --git a/providers/azure/internal/recommendations/converter_test.go b/providers/azure/internal/recommendations/converter_test.go index 20fdc0c03..f0ebd162c 100644 --- a/providers/azure/internal/recommendations/converter_test.go +++ b/providers/azure/internal/recommendations/converter_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/LeanerCloud/CUDly/pkg/common" "github.com/LeanerCloud/CUDly/providers/azure/mocks" ) @@ -239,3 +240,126 @@ func TestExtract_Modern_RecurringMonthlyCostIsZeroPointer(t *testing.T) { require.NotNil(t, f.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil for Azure reservations") assert.InDelta(t, 0.0, *f.RecurringMonthlyCost, 1e-9) } + +// --- ExpandPaymentVariants -------------------------------------------------- + +func baseRec(service common.ServiceType, term string, onDemand, commitment float64) common.Recommendation { + return common.Recommendation{ + Provider: common.ProviderAzure, + Service: service, + Region: "eastus", + ResourceType: "Standard_D2s_v3", + CommitmentType: common.CommitmentReservedInstance, + Term: term, + PaymentOption: "upfront", + OnDemandCost: onDemand, + CommitmentCost: commitment, + } +} + +func TestExpandPaymentVariants_ReturnsTwoVariants(t *testing.T) { + variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 100, 70)) + require.Len(t, variants, 2, "must return exactly two variants") +} + +func TestExpandPaymentVariants_PaymentOptionValues(t *testing.T) { + variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 100, 70)) + assert.Equal(t, "upfront", variants[0].PaymentOption) + assert.Equal(t, "monthly", variants[1].PaymentOption) +} + +func TestExpandPaymentVariants_AllUpfrontCashflow(t *testing.T) { + // all-upfront: RecurringMonthlyCost must be a non-nil pointer to 0. + variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 100, 70)) + allUpfront := variants[0] + require.NotNil(t, allUpfront.RecurringMonthlyCost) + assert.InDelta(t, 0.0, *allUpfront.RecurringMonthlyCost, 1e-9) +} + +func TestExpandPaymentVariants_NoUpfront1yrCashflow(t *testing.T) { + // no-upfront 1yr: RecurringMonthlyCost = CommitmentCost / 12. + variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 100, 72)) + noUpfront := variants[1] + require.NotNil(t, noUpfront.RecurringMonthlyCost) + assert.InDelta(t, 72.0/12.0, *noUpfront.RecurringMonthlyCost, 1e-9) +} + +func TestExpandPaymentVariants_NoUpfront3yrCashflow(t *testing.T) { + // no-upfront 3yr: RecurringMonthlyCost = CommitmentCost / 36. + variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "3yr", 200, 120)) + noUpfront := variants[1] + require.NotNil(t, noUpfront.RecurringMonthlyCost) + assert.InDelta(t, 120.0/36.0, *noUpfront.RecurringMonthlyCost, 1e-9) +} + +func TestExpandPaymentVariants_SavingsIdenticalAcrossVariants(t *testing.T) { + // EstimatedSavings and SavingsPercentage must be the same for both + // variants — Azure's total reservation price is unchanged between billing + // plans; only cashflow splits. + variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 100, 70)) + assert.InDelta(t, variants[0].EstimatedSavings, variants[1].EstimatedSavings, 1e-9) + assert.InDelta(t, variants[0].SavingsPercentage, variants[1].SavingsPercentage, 1e-9) +} + +func TestExpandPaymentVariants_SavingsValues(t *testing.T) { + variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 100, 70)) + assert.InDelta(t, 30.0, variants[0].EstimatedSavings, 1e-9) + assert.InDelta(t, 30.0, variants[1].EstimatedSavings, 1e-9) + assert.InDelta(t, 30.0, variants[0].SavingsPercentage, 1e-9) + assert.InDelta(t, 30.0, variants[1].SavingsPercentage, 1e-9) +} + +func TestExpandPaymentVariants_ZeroOnDemand_NoSavings(t *testing.T) { + // Guard: avoid divide-by-zero when OnDemandCost is 0. + variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 0, 0)) + for _, v := range variants { + assert.InDelta(t, 0.0, v.EstimatedSavings, 1e-9) + assert.InDelta(t, 0.0, v.SavingsPercentage, 1e-9) + } +} + +func TestExpandPaymentVariants_ZeroOnDemand_NonZeroCommitment(t *testing.T) { + // Regression: when OnDemandCost == 0 but CommitmentCost > 0, the guard + // must also force EstimatedSavings to 0. Without it, savings would be + // computed as 0 - CommitmentCost and emit negative savings. + variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 0, 10)) + require.Len(t, variants, 2) + for _, v := range variants { + assert.InDelta(t, 0.0, v.EstimatedSavings, 1e-9) + assert.InDelta(t, 0.0, v.SavingsPercentage, 1e-9) + } +} + +func TestExpandPaymentVariants_ZeroCommitmentCost(t *testing.T) { + // Zero reservation total: both variants still emitted; no-upfront monthly = 0. + variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 50, 0)) + require.Len(t, variants, 2) + require.NotNil(t, variants[1].RecurringMonthlyCost) + assert.InDelta(t, 0.0, *variants[1].RecurringMonthlyCost, 1e-9) +} + +func TestExpandPaymentVariants_SharedFieldsCarriedThrough(t *testing.T) { + // Service, Region, ResourceType, CommitmentType, Term, Account must be + // identical between the two returned variants (only payment schedule changes). + base := baseRec(common.ServiceRelationalDB, "3yr", 200, 140) + base.Account = "sub-123" + variants := ExpandPaymentVariants(base) + for _, v := range variants { + assert.Equal(t, common.ServiceRelationalDB, v.Service) + assert.Equal(t, "eastus", v.Region) + assert.Equal(t, "Standard_D2s_v3", v.ResourceType) + assert.Equal(t, common.CommitmentReservedInstance, v.CommitmentType) + assert.Equal(t, "3yr", v.Term) + assert.Equal(t, "sub-123", v.Account) + } +} + +func TestExpandPaymentVariants_RecurringMonthlyCostPointersAreIndependent(t *testing.T) { + // The two variants must hold independent pointer values — mutating one + // must not affect the other. + variants := ExpandPaymentVariants(baseRec(common.ServiceCompute, "1yr", 100, 60)) + require.NotNil(t, variants[0].RecurringMonthlyCost) + require.NotNil(t, variants[1].RecurringMonthlyCost) + assert.True(t, variants[0].RecurringMonthlyCost != variants[1].RecurringMonthlyCost, + "each variant must own an independent pointer to its RecurringMonthlyCost") +} diff --git a/providers/azure/recommendations.go b/providers/azure/recommendations.go index ea0c007a5..4019c46ec 100644 --- a/providers/azure/recommendations.go +++ b/providers/azure/recommendations.go @@ -14,6 +14,7 @@ import ( "github.com/LeanerCloud/CUDly/pkg/common" "github.com/LeanerCloud/CUDly/pkg/concurrency" "github.com/LeanerCloud/CUDly/pkg/logging" + azrecs "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations" "github.com/LeanerCloud/CUDly/providers/azure/services/cache" "github.com/LeanerCloud/CUDly/providers/azure/services/compute" "github.com/LeanerCloud/CUDly/providers/azure/services/cosmosdb" @@ -224,21 +225,41 @@ func (r *RecommendationsClientAdapter) getAdvisorRecommendations(ctx context.Con logging.Warnf("Azure Advisor pagination error (partial results may be returned): %v", err) break } + recommendations = r.appendAdvisorPageRecs(params, page, recommendations) + } - for _, advisorRec := range page.Value { - if advisorRec.Properties == nil { - continue - } + return recommendations, nil +} - // Convert Azure Advisor recommendation to our common format - rec := r.convertAdvisorRecommendation(advisorRec) - if rec != nil && shouldIncludeService(params, rec.Service) { - recommendations = append(recommendations, *rec) +// appendAdvisorPageRecs converts one Advisor pager page into common recommendations +// and appends them to the provided slice. Pulled out of getAdvisorRecommendations +// to keep that function under the cyclomatic limit. +func (r *RecommendationsClientAdapter) appendAdvisorPageRecs( + params common.RecommendationParams, + page armadvisor.RecommendationsClientListResponse, + recommendations []common.Recommendation, +) []common.Recommendation { + for _, advisorRec := range page.Value { + if advisorRec.Properties == nil { + continue + } + // Convert Azure Advisor recommendation to our common format + rec := r.convertAdvisorRecommendation(advisorRec) + if rec != nil && shouldIncludeService(params, rec.Service) { + // Preserve Advisor-provided EstimatedSavings when both OnDemandCost + // and CommitmentCost are unset (zero), since ExpandPaymentVariants + // would otherwise overwrite it with zero (OnDemandCost - CommitmentCost). + advisorSavings := rec.EstimatedSavings + variants := azrecs.ExpandPaymentVariants(*rec) + if rec.OnDemandCost == 0 && rec.CommitmentCost == 0 && advisorSavings != 0 { + for i := range variants { + variants[i].EstimatedSavings = advisorSavings + } } + recommendations = append(recommendations, variants...) } } - - return recommendations, nil + return recommendations } // resolveAdvisorRegion picks the region for an Advisor recommendation. diff --git a/providers/azure/services/cache/client.go b/providers/azure/services/cache/client.go index e7fd1921f..32d9b55cb 100644 --- a/providers/azure/services/cache/client.go +++ b/providers/azure/services/cache/client.go @@ -21,7 +21,7 @@ import ( "github.com/LeanerCloud/CUDly/pkg/logging" "github.com/LeanerCloud/CUDly/providers/azure/internal/httpclient" "github.com/LeanerCloud/CUDly/providers/azure/internal/pricing" - "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations" + azrecs "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations" "github.com/LeanerCloud/CUDly/providers/azure/services/internal/reservations" ) @@ -175,7 +175,7 @@ func (c *CacheClient) GetRecommendations(ctx context.Context, params common.Reco for _, rec := range page.Value { converted := c.convertAzureRedisRecommendation(ctx, rec) if converted != nil { - recommendations = append(recommendations, *converted) + recommendations = append(recommendations, azrecs.ExpandPaymentVariants(*converted)...) } } } @@ -574,7 +574,7 @@ func extractRedisPricing(items []CacheRetailPriceItem, termYears int) (onDemand, // Premium-tier SKU; otherwise stays 0 (zero means "unknown", not // "definitely zero shards" — see the redisSKUEntry godoc). func (c *CacheClient) convertAzureRedisRecommendation(ctx context.Context, azureRec armconsumption.ReservationRecommendationClassification) *common.Recommendation { - f := recommendations.Extract(azureRec) + f := azrecs.Extract(azureRec) if f == nil { return nil } diff --git a/providers/azure/services/compute/client.go b/providers/azure/services/compute/client.go index e016e963f..c32be7761 100644 --- a/providers/azure/services/compute/client.go +++ b/providers/azure/services/compute/client.go @@ -22,7 +22,7 @@ import ( "github.com/LeanerCloud/CUDly/pkg/logging" "github.com/LeanerCloud/CUDly/providers/azure/internal/httpclient" "github.com/LeanerCloud/CUDly/providers/azure/internal/pricing" - "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations" + azrecs "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations" "github.com/LeanerCloud/CUDly/providers/azure/services/internal/reservations" ) @@ -194,7 +194,7 @@ func (c *ComputeClient) GetRecommendations(ctx context.Context, params common.Re for _, rec := range page.Value { converted := c.convertAzureVMRecommendation(ctx, rec) if converted != nil { - recommendations = append(recommendations, *converted) + recommendations = append(recommendations, azrecs.ExpandPaymentVariants(*converted)...) } } } @@ -698,7 +698,7 @@ func extractVMPricing(items []AzureRetailPriceItem, termYears int) (onDemand, re // sources (consumption usage records, dedicated-host inventory) and // remain unpopulated — out of scope for this issue. func (c *ComputeClient) convertAzureVMRecommendation(ctx context.Context, azureRec armconsumption.ReservationRecommendationClassification) *common.Recommendation { - f := recommendations.Extract(azureRec) + f := azrecs.Extract(azureRec) if f == nil { return nil } diff --git a/providers/azure/services/compute/client_test.go b/providers/azure/services/compute/client_test.go index fb9b2076b..740c384ba 100644 --- a/providers/azure/services/compute/client_test.go +++ b/providers/azure/services/compute/client_test.go @@ -15,6 +15,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -218,6 +219,59 @@ func TestComputeClient_GetRecommendations_WithMock(t *testing.T) { assert.Empty(t, recommendations) } +// TestComputeClient_GetRecommendations_EmitsBothPaymentVariants asserts that +// a single Azure reservation recommendation from the API is expanded into two +// entries — "upfront" and "monthly" — with correct cashflow split and +// identical savings figures. +func TestComputeClient_GetRecommendations_EmitsBothPaymentVariants(t *testing.T) { + ctx := context.Background() + client := NewClient(nil, "test-subscription", "eastus") + + // Inject a single recommendation via the mock pager. + apiRec := mocks.BuildLegacyReservationRecommendation( + mocks.WithRegion("eastus"), + mocks.WithScope("Shared"), + mocks.WithTerm("P1Y"), + mocks.WithQuantity(1), + mocks.WithNormalizedSize("Standard_D2s_v3"), + mocks.WithCosts(120, 84, 36), // onDemand=120, commitment=84, savings=36 + ) + mockPager := &mocks.MockRecommendationsPager{ + Results: []armconsumption.ReservationRecommendationClassification{apiRec}, + HasMore: true, + } + client.SetRecommendationsPager(mockPager) + + recs, err := client.GetRecommendations(ctx, common.RecommendationParams{}) + require.NoError(t, err) + require.Len(t, recs, 2, "one API rec must expand to two payment-variant entries") + + payments := make(map[string]common.Recommendation) + for _, r := range recs { + payments[r.PaymentOption] = r + } + require.Contains(t, payments, "upfront", "upfront variant must be present") + require.Contains(t, payments, "monthly", "monthly variant must be present") + + allUp := payments["upfront"] + noUp := payments["monthly"] + + // Cashflow: upfront has zero recurring; monthly spreads over 12 months. + require.NotNil(t, allUp.RecurringMonthlyCost) + assert.InDelta(t, 0.0, *allUp.RecurringMonthlyCost, 1e-9) + require.NotNil(t, noUp.RecurringMonthlyCost) + assert.InDelta(t, 84.0/12.0, *noUp.RecurringMonthlyCost, 1e-9) + + // Savings must be identical across variants. + assert.InDelta(t, allUp.EstimatedSavings, noUp.EstimatedSavings, 1e-9) + assert.InDelta(t, allUp.SavingsPercentage, noUp.SavingsPercentage, 1e-9) + + // Shared fields. + assert.Equal(t, "test-subscription", allUp.Account) + assert.Equal(t, common.ServiceCompute, allUp.Service) + assert.Equal(t, "1yr", allUp.Term) +} + func TestComputeClient_GetExistingCommitments_WithMock(t *testing.T) { ctx := context.Background() client := NewClient(nil, "test-subscription", "eastus") diff --git a/providers/azure/services/cosmosdb/client.go b/providers/azure/services/cosmosdb/client.go index b77ad88f7..c60d77ddc 100644 --- a/providers/azure/services/cosmosdb/client.go +++ b/providers/azure/services/cosmosdb/client.go @@ -22,7 +22,7 @@ import ( "github.com/LeanerCloud/CUDly/pkg/logging" "github.com/LeanerCloud/CUDly/providers/azure/internal/httpclient" "github.com/LeanerCloud/CUDly/providers/azure/internal/pricing" - "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations" + azrecs "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations" "github.com/LeanerCloud/CUDly/providers/azure/services/internal/reservations" ) @@ -169,7 +169,7 @@ func (c *CosmosDBClient) GetRecommendations(ctx context.Context, params common.R for _, rec := range page.Value { converted := c.convertAzureCosmosRecommendation(ctx, rec) if converted != nil { - recommendations = append(recommendations, *converted) + recommendations = append(recommendations, azrecs.ExpandPaymentVariants(*converted)...) } } } @@ -577,7 +577,7 @@ func calculateCosmosSavingsPercentage(onDemandPrice, hoursInTerm, reservationPri // subscription has zero Cosmos accounts, multiple Cosmos accounts with // different API types (ambiguous), or the listing fails. func (c *CosmosDBClient) convertAzureCosmosRecommendation(ctx context.Context, azureRec armconsumption.ReservationRecommendationClassification) *common.Recommendation { - f := recommendations.Extract(azureRec) + f := azrecs.Extract(azureRec) if f == nil { return nil } diff --git a/providers/azure/services/database/client.go b/providers/azure/services/database/client.go index f0238c00c..fb564fdaa 100644 --- a/providers/azure/services/database/client.go +++ b/providers/azure/services/database/client.go @@ -21,7 +21,7 @@ import ( "github.com/LeanerCloud/CUDly/pkg/logging" "github.com/LeanerCloud/CUDly/providers/azure/internal/httpclient" "github.com/LeanerCloud/CUDly/providers/azure/internal/pricing" - "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations" + azrecs "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations" "github.com/LeanerCloud/CUDly/providers/azure/services/internal/reservations" ) @@ -177,7 +177,7 @@ func (c *DatabaseClient) GetRecommendations(ctx context.Context, params common.R for _, rec := range page.Value { converted := c.convertAzureSQLRecommendation(ctx, rec) if converted != nil { - recommendations = append(recommendations, *converted) + recommendations = append(recommendations, azrecs.ExpandPaymentVariants(*converted)...) } } } @@ -575,7 +575,7 @@ func extractSQLPricing(items []DatabaseRetailPriceItem, termYears int) (onDemand // otherwise stays empty. AZConfig/Deployment still need additional // signals (per-server config) and remain deferred. func (c *DatabaseClient) convertAzureSQLRecommendation(ctx context.Context, azureRec armconsumption.ReservationRecommendationClassification) *common.Recommendation { - f := recommendations.Extract(azureRec) + f := azrecs.Extract(azureRec) if f == nil { return nil } diff --git a/providers/azure/services/search/client.go b/providers/azure/services/search/client.go index 7573577be..372a6ac6f 100644 --- a/providers/azure/services/search/client.go +++ b/providers/azure/services/search/client.go @@ -19,6 +19,7 @@ import ( "github.com/LeanerCloud/CUDly/pkg/common" "github.com/LeanerCloud/CUDly/providers/azure/internal/httpclient" + azrecs "github.com/LeanerCloud/CUDly/providers/azure/internal/recommendations" "github.com/LeanerCloud/CUDly/providers/azure/services/internal/reservations" ) @@ -149,7 +150,7 @@ func (c *SearchClient) GetRecommendations(ctx context.Context, params common.Rec for _, rec := range page.Value { converted := c.convertAzureSearchRecommendation(ctx, rec) if converted != nil { - recommendations = append(recommendations, *converted) + recommendations = append(recommendations, azrecs.ExpandPaymentVariants(*converted)...) } } } @@ -539,15 +540,33 @@ func calculateSearchSavingsPercentage(onDemandPrice, hoursInTerm, reservationPri // convertAzureSearchRecommendation converts Azure Search reservation recommendation to common format func (c *SearchClient) convertAzureSearchRecommendation(ctx context.Context, azureRec armconsumption.ReservationRecommendationClassification) *common.Recommendation { + // Extract fields from Azure recommendation using the shared converter + extracted := azrecs.Extract(azureRec) + if extracted == nil { + return nil + } + rec := &common.Recommendation{ Provider: common.ProviderAzure, Service: common.ServiceOther, Account: c.subscriptionID, - Region: c.region, CommitmentType: common.CommitmentReservedInstance, Timestamp: time.Now(), - Term: "1yr", - PaymentOption: "upfront", + // Populate fields from Azure API response + Region: extracted.Region, + ResourceType: extracted.ResourceType, + Count: extracted.Count, + OnDemandCost: extracted.OnDemandCost, + CommitmentCost: extracted.CommitmentCost, + EstimatedSavings: extracted.EstimatedSavings, + Term: extracted.Term, + RecurringMonthlyCost: extracted.RecurringMonthlyCost, + PaymentOption: "upfront", // Default, will be expanded by ExpandPaymentVariants + } + + // Override region with client region if extraction didn't find one + if rec.Region == "" { + rec.Region = c.region } return rec diff --git a/providers/azure/services/search/client_test.go b/providers/azure/services/search/client_test.go index bfee43ce7..6f2e91f12 100644 --- a/providers/azure/services/search/client_test.go +++ b/providers/azure/services/search/client_test.go @@ -19,6 +19,7 @@ import ( "github.com/stretchr/testify/require" "github.com/LeanerCloud/CUDly/pkg/common" + "github.com/LeanerCloud/CUDly/providers/azure/mocks" ) // MockRecommendationsPager mocks the RecommendationsPager interface @@ -587,16 +588,37 @@ func TestSearchClient_GetCommonSKUs(t *testing.T) { assert.Contains(t, skus, "storage_optimized_l2") } -func TestSearchClient_ConvertAzureSearchRecommendation(t *testing.T) { - ctx := context.Background() +// TestSearchClient_ConvertAzureSearchRecommendation_NilGuards pins the contract: +// unusable SDK payloads (nil or nil Properties) produce a nil *Recommendation. +func TestSearchClient_ConvertAzureSearchRecommendation_NilGuards(t *testing.T) { client := NewClient(nil, "test-subscription", "eastus") + assert.Nil(t, client.convertAzureSearchRecommendation(context.Background(), nil)) +} - rec := client.convertAzureSearchRecommendation(ctx, nil) +// TestSearchClient_ConvertAzureSearchRecommendation_PopulatesAllFields asserts +// the converter forwards every helper-extracted field plus the Search-service +// constants (Provider, Service, CommitmentType, PaymentOption). +func TestSearchClient_ConvertAzureSearchRecommendation_PopulatesAllFields(t *testing.T) { + client := NewClient(nil, "test-subscription", "eastus") + + azRec := mocks.BuildLegacyReservationRecommendation( + mocks.WithRegion("eastus"), + mocks.WithTerm("P1Y"), + mocks.WithQuantity(2), + mocks.WithNormalizedSize("standard2"), + mocks.WithCosts(120, 80, 40), + ) + rec := client.convertAzureSearchRecommendation(context.Background(), azRec) require.NotNil(t, rec) assert.Equal(t, common.ProviderAzure, rec.Provider) assert.Equal(t, common.ServiceOther, rec.Service) assert.Equal(t, "test-subscription", rec.Account) assert.Equal(t, "eastus", rec.Region) + assert.Equal(t, "standard2", rec.ResourceType) + assert.Equal(t, 2, rec.Count) + assert.InDelta(t, 120.0, rec.OnDemandCost, 1e-9) + assert.InDelta(t, 80.0, rec.CommitmentCost, 1e-9) + assert.InDelta(t, 40.0, rec.EstimatedSavings, 1e-9) assert.Equal(t, common.CommitmentReservedInstance, rec.CommitmentType) assert.Equal(t, "1yr", rec.Term) assert.Equal(t, "upfront", rec.PaymentOption)