diff --git a/providers/gcp/services/cloudsql/client.go b/providers/gcp/services/cloudsql/client.go index 8f443e3eb..6a102dca0 100644 --- a/providers/gcp/services/cloudsql/client.go +++ b/providers/gcp/services/cloudsql/client.go @@ -455,14 +455,13 @@ func skuMatchesTier(sku *cloudbilling.Sku, tier, region string) bool { return true } -// extractResourceTypeFromContent extracts the last path segment of the first -// non-empty Operation.Resource across all operation groups. Used by all four -// GCP service converters to set rec.ResourceType from Recommender payloads. -func extractResourceTypeFromContent(content *recommenderpb.RecommendationContent) string { - if content == nil || content.OperationGroups == nil { +// extractGCPResourceType returns the last path segment of the first non-empty +// resource field found across all operation groups, or "" if none is present. +func extractGCPResourceType(rec *recommenderpb.Recommendation) string { + if rec.Content == nil || rec.Content.OperationGroups == nil { return "" } - for _, opGroup := range content.OperationGroups { + for _, opGroup := range rec.Content.OperationGroups { for _, op := range opGroup.Operations { if op.Resource == "" { continue @@ -476,13 +475,13 @@ func extractResourceTypeFromContent(content *recommenderpb.RecommendationContent return "" } -// extractEstimatedSavings returns the negative of the PrimaryImpact cost -// projection (GCP encodes savings as a negative cost delta). -func extractEstimatedSavings(gcpRec *recommenderpb.Recommendation) float64 { - if gcpRec.PrimaryImpact == nil { +// extractGCPSavings returns the estimated monthly savings (positive value) +// from the primary cost impact of a GCP recommendation, or 0 if absent. +func extractGCPSavings(rec *recommenderpb.Recommendation) float64 { + if rec.PrimaryImpact == nil { return 0 } - costProj := gcpRec.PrimaryImpact.GetCostProjection() + costProj := rec.PrimaryImpact.GetCostProjection() if costProj == nil || costProj.Cost == nil { return 0 } @@ -510,6 +509,15 @@ func (c *CloudSQLClient) fillSQLPricing(ctx context.Context, rec *common.Recomme } } +// termYearsFromLabel converts a term string such as "1yr" or "3yr" to an +// integer number of years (defaults to 1 for any unrecognized value). +func termYearsFromLabel(term string) int { + if term == "3yr" || term == "3" { + return 3 + } + return 1 +} + // convertGCPRecommendation converts a GCP Recommender recommendation to common format. // It also calls getSQLPricing to fill CommitmentCost/OnDemandCost/SavingsPercentage/ // BreakEvenMonths so the scorer can filter and rank GCP recommendations correctly @@ -536,17 +544,25 @@ func (c *CloudSQLClient) convertGCPRecommendation(ctx context.Context, gcpRec *r PaymentOption: paymentOption, } - rec.ResourceType = extractResourceTypeFromContent(gcpRec.Content) - rec.EstimatedSavings = extractEstimatedSavings(gcpRec) + rec.ResourceType = extractGCPResourceType(gcpRec) + rec.EstimatedSavings = extractGCPSavings(gcpRec) // Thread pricing into the converter so the scorer can rank/filter GCP recs - // correctly (issue #1022 C2). + // correctly (issue #1022 C2). fillSQLPricing performs the single billing + // lookup and populates CommitmentCost; we reuse that value below to derive + // RecurringMonthlyCost rather than issuing a second SKU call. if rec.ResourceType != "" { - termYears := 1 - if rec.Term == "3yr" || rec.Term == "3" { - termYears = 3 - } + termYears := termYearsFromLabel(rec.Term) c.fillSQLPricing(ctx, rec, termYears) + + // Cloud SQL CUDs are monthly-payment commitments, so the per-month + // charge is CommitmentCost / termMonths. When the billing lookup + // failed, CommitmentCost stays 0 and RecurringMonthlyCost remains nil + // so the frontend renders "—" rather than a stale value. + if rec.CommitmentCost > 0 { + monthly := rec.CommitmentCost / float64(termYears*12) + rec.RecurringMonthlyCost = &monthly + } } return rec diff --git a/providers/gcp/services/cloudsql/client_test.go b/providers/gcp/services/cloudsql/client_test.go index 6a93b81ae..98ac60938 100644 --- a/providers/gcp/services/cloudsql/client_test.go +++ b/providers/gcp/services/cloudsql/client_test.go @@ -841,3 +841,119 @@ func TestGetSQLPricing_CommitmentPriceIsTermTotal(t *testing.T) { assert.Less(t, pricing.SavingsPercentage, float64(50), "SavingsPercentage must be realistic (not ~100%)") } + +func TestCloudSQLClient_ConvertGCPRecommendation_RecurringMonthlyCost(t *testing.T) { + ctx := context.Background() + client, _ := NewClient(ctx, "test-project", "us-central1") + + // Inject a billing mock with a known on-demand price and a separate + // commitment SKU. getSQLPricing derives CommitmentPrice from the + // "commitment" SKU (CommitmentPrice = commitmentHourly * 8760), so + // RecurringMonthlyCost must equal CommitmentPrice / 12 (one year = 12 months). + const onDemandHourly = 0.12 // USD/h per vCPU -- representative db-n1-standard-1 value + const commitmentHourly = 0.102 // USD/h -- 1yr CUD rate from the catalog + mockBilling := &MockBillingService{ + skus: &cloudbilling.ListSkusResponse{ + Skus: []*cloudbilling.Sku{ + { + Description: "db-n1-standard-1 Cloud SQL", + ServiceRegions: []string{"us-central1"}, + PricingInfo: []*cloudbilling.PricingInfo{ + { + PricingExpression: &cloudbilling.PricingExpression{ + TieredRates: []*cloudbilling.TierRate{ + { + UnitPrice: &cloudbilling.Money{ + Units: 0, + Nanos: int64(onDemandHourly * 1e9), + CurrencyCode: "USD", + }, + }, + }, + }, + }, + }, + }, + { + // Commitment SKU required by getSQLPricing: without a + // "commitment" SKU it errors and RecurringMonthlyCost stays nil. + Description: "db-n1-standard-1 Cloud SQL commitment 1yr", + ServiceRegions: []string{"us-central1"}, + PricingInfo: []*cloudbilling.PricingInfo{ + { + PricingExpression: &cloudbilling.PricingExpression{ + TieredRates: []*cloudbilling.TierRate{ + { + UnitPrice: &cloudbilling.Money{ + Units: 0, + Nanos: int64(commitmentHourly * 1e9), + CurrencyCode: "USD", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + client.SetBillingService(mockBilling) + + gcpRec := &recommenderpb.Recommendation{ + Name: "test-rec", + PrimaryImpact: &recommenderpb.Impact{ + Category: recommenderpb.Impact_COST, + Projection: &recommenderpb.Impact_CostProjection{ + CostProjection: &recommenderpb.CostProjection{ + Cost: &money.Money{Units: -100, CurrencyCode: "USD"}, + }, + }, + }, + Content: &recommenderpb.RecommendationContent{ + OperationGroups: []*recommenderpb.OperationGroup{ + { + Operations: []*recommenderpb.Operation{ + {Resource: "projects/test/instances/db-n1-standard-1"}, + }, + }, + }, + }, + } + + rec := client.convertGCPRecommendation(ctx, gcpRec, common.RecommendationParams{}) + require.NotNil(t, rec) + + // RecurringMonthlyCost must be a non-nil pointer to a positive value for + // a monthly Cloud SQL CUD recommendation. + require.NotNil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil when billing lookup succeeds") + assert.Greater(t, *rec.RecurringMonthlyCost, 0.0, "RecurringMonthlyCost must be positive for a monthly Cloud SQL CUD") + + // Verify the value matches CommitmentPrice / 12 exactly. CommitmentPrice is + // the commitment SKU's hourly rate scaled to the 1yr term total. + const hoursIn1yr = 8760.0 + expectedMonthly := commitmentHourly * hoursIn1yr / 12 + assert.InDelta(t, expectedMonthly, *rec.RecurringMonthlyCost, 1e-6) +} + +func TestCloudSQLClient_ConvertGCPRecommendation_RecurringMonthlyCost_BillingFailure(t *testing.T) { + ctx := context.Background() + client, _ := NewClient(ctx, "test-project", "us-central1") + + // Inject a billing mock that always errors; RecurringMonthlyCost must + // remain nil (frontend renders "—") rather than a zero or stale value. + client.SetBillingService(&MockBillingService{err: errors.New("billing unavailable")}) + + gcpRec := &recommenderpb.Recommendation{ + Name: "test-rec", + Content: &recommenderpb.RecommendationContent{ + OperationGroups: []*recommenderpb.OperationGroup{ + {Operations: []*recommenderpb.Operation{{Resource: "projects/test/instances/db-n1-standard-1"}}}, + }, + }, + } + + rec := client.convertGCPRecommendation(ctx, gcpRec, common.RecommendationParams{}) + require.NotNil(t, rec) + assert.Nil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be nil when billing lookup fails") +} diff --git a/providers/gcp/services/cloudstorage/client.go b/providers/gcp/services/cloudstorage/client.go index bcae9ae1a..5993132d0 100644 --- a/providers/gcp/services/cloudstorage/client.go +++ b/providers/gcp/services/cloudstorage/client.go @@ -447,14 +447,13 @@ func skuMatchesStorageClass(sku *cloudbilling.Sku, storageClass, region string) return true } -// extractResourceTypeFromContent extracts the last path segment of the first -// non-empty Operation.Resource across all operation groups. Used by all four -// GCP service converters to set rec.ResourceType from Recommender payloads. -func extractResourceTypeFromContent(content *recommenderpb.RecommendationContent) string { - if content == nil || content.OperationGroups == nil { +// extractGCPResourceType returns the last path segment of the first non-empty +// resource field found across all operation groups, or "" if none is present. +func extractGCPResourceType(rec *recommenderpb.Recommendation) string { + if rec.Content == nil || rec.Content.OperationGroups == nil { return "" } - for _, opGroup := range content.OperationGroups { + for _, opGroup := range rec.Content.OperationGroups { for _, op := range opGroup.Operations { if op.Resource == "" { continue @@ -468,13 +467,13 @@ func extractResourceTypeFromContent(content *recommenderpb.RecommendationContent return "" } -// extractEstimatedSavings returns the negative of the PrimaryImpact cost -// projection (GCP encodes savings as a negative cost delta). -func extractEstimatedSavings(gcpRec *recommenderpb.Recommendation) float64 { - if gcpRec.PrimaryImpact == nil { +// extractGCPSavings returns the estimated monthly savings (positive value) +// from the primary cost impact of a GCP recommendation, or 0 if absent. +func extractGCPSavings(rec *recommenderpb.Recommendation) float64 { + if rec.PrimaryImpact == nil { return 0 } - costProj := gcpRec.PrimaryImpact.GetCostProjection() + costProj := rec.PrimaryImpact.GetCostProjection() if costProj == nil || costProj.Cost == nil { return 0 } @@ -502,6 +501,15 @@ func (c *CloudStorageClient) fillStoragePricing(ctx context.Context, rec *common } } +// termYearsFromLabel converts a term string such as "1yr" or "3yr" to an +// integer number of years (defaults to 1 for any unrecognized value). +func termYearsFromLabel(term string) int { + if term == "3yr" || term == "3" { + return 3 + } + return 1 +} + // convertGCPRecommendation converts a GCP Recommender recommendation to common format. // It also calls getStoragePricing to fill CommitmentCost/OnDemandCost/SavingsPercentage/ // BreakEvenMonths so the scorer can filter and rank GCP recommendations correctly @@ -528,17 +536,25 @@ func (c *CloudStorageClient) convertGCPRecommendation(ctx context.Context, gcpRe PaymentOption: paymentOption, } - rec.ResourceType = extractResourceTypeFromContent(gcpRec.Content) - rec.EstimatedSavings = extractEstimatedSavings(gcpRec) + rec.ResourceType = extractGCPResourceType(gcpRec) + rec.EstimatedSavings = extractGCPSavings(gcpRec) // Thread pricing into the converter so the scorer can rank/filter GCP recs - // correctly (issue #1022 C2). + // correctly (issue #1022 C2). fillStoragePricing performs the single billing + // lookup and populates CommitmentCost; we reuse that value below to derive + // RecurringMonthlyCost rather than issuing a second SKU call. if rec.ResourceType != "" { - termYears := 1 - if rec.Term == "3yr" || rec.Term == "3" { - termYears = 3 - } + termYears := termYearsFromLabel(rec.Term) c.fillStoragePricing(ctx, rec, termYears) + + // Cloud Storage committed-use discounts are monthly-payment commitments, + // so the per-month charge is CommitmentCost / termMonths. When the + // billing lookup failed, CommitmentCost stays 0 and RecurringMonthlyCost + // remains nil so the frontend renders "—" rather than a stale value. + if rec.CommitmentCost > 0 { + monthly := rec.CommitmentCost / float64(termYears*12) + rec.RecurringMonthlyCost = &monthly + } } return rec diff --git a/providers/gcp/services/cloudstorage/client_test.go b/providers/gcp/services/cloudstorage/client_test.go index 78b874d0b..c8c1f2058 100644 --- a/providers/gcp/services/cloudstorage/client_test.go +++ b/providers/gcp/services/cloudstorage/client_test.go @@ -856,3 +856,123 @@ func TestSkuMatchesStorageClass_CaseInsensitive(t *testing.T) { } assert.True(t, skuMatchesStorageClass(sku, "standard", "us-central1")) } + +// TestCloudStorageClient_ConvertGCPRecommendation_PopulatesRecurringMonthlyCost verifies +// that convertGCPRecommendation sets a non-nil RecurringMonthlyCost when the billing +// service returns valid pricing. This is the regression test for issue #264: the +// pre-fix state left RecurringMonthlyCost nil, causing the frontend to render "—". +func TestCloudStorageClient_ConvertGCPRecommendation_PopulatesRecurringMonthlyCost(t *testing.T) { + ctx := context.Background() + client, _ := NewClient(ctx, "test-project", "us-central1") + + // Provide a billing mock that returns a non-zero on-demand price so that + // getStoragePricing succeeds and produces a positive CommitmentPrice. + mockBilling := &MockBillingService{ + skus: &cloudbilling.ListSkusResponse{ + Skus: []*cloudbilling.Sku{ + { + Description: "Standard Storage in us-central1", + ServiceRegions: []string{"us-central1"}, + PricingInfo: []*cloudbilling.PricingInfo{ + { + PricingExpression: &cloudbilling.PricingExpression{ + TieredRates: []*cloudbilling.TierRate{ + { + UnitPrice: &cloudbilling.Money{ + Units: 0, + Nanos: 26000000, + CurrencyCode: "USD", + }, + }, + }, + }, + }, + }, + }, + { + // Commitment SKU required by getStoragePricing: without a + // "commitment" SKU it errors and RecurringMonthlyCost stays nil. + Description: "Standard Storage commitment 1yr in us-central1", + ServiceRegions: []string{"us-central1"}, + PricingInfo: []*cloudbilling.PricingInfo{ + { + PricingExpression: &cloudbilling.PricingExpression{ + TieredRates: []*cloudbilling.TierRate{ + { + UnitPrice: &cloudbilling.Money{ + Units: 0, + Nanos: 20000000, + CurrencyCode: "USD", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + client.SetBillingService(mockBilling) + + gcpRec := &recommenderpb.Recommendation{ + Name: "test-rec", + PrimaryImpact: &recommenderpb.Impact{ + Category: recommenderpb.Impact_COST, + Projection: &recommenderpb.Impact_CostProjection{ + CostProjection: &recommenderpb.CostProjection{ + Cost: &money.Money{ + Units: -100, + Nanos: 0, + CurrencyCode: "USD", + }, + }, + }, + }, + Content: &recommenderpb.RecommendationContent{ + OperationGroups: []*recommenderpb.OperationGroup{ + { + Operations: []*recommenderpb.Operation{ + {Resource: "projects/test/buckets/STANDARD"}, + }, + }, + }, + }, + } + + rec := client.convertGCPRecommendation(ctx, gcpRec, common.RecommendationParams{}) + require.NotNil(t, rec) + require.NotNil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil when billing lookup succeeds") + assert.Greater(t, *rec.RecurringMonthlyCost, float64(0)) +} + +// TestCloudStorageClient_ConvertGCPRecommendation_BillingFailure_RecurringMonthlyCostNil +// verifies that convertGCPRecommendation leaves RecurringMonthlyCost nil (rather than +// coercing to 0) when the billing service call fails, matching the sibling services' +// non-fatal-failure contract. +func TestCloudStorageClient_ConvertGCPRecommendation_BillingFailure_RecurringMonthlyCostNil(t *testing.T) { + ctx := context.Background() + client, _ := NewClient(ctx, "test-project", "us-central1") + + mockBilling := &MockBillingService{ + err: errors.New("billing API unavailable"), + } + client.SetBillingService(mockBilling) + + gcpRec := &recommenderpb.Recommendation{ + Name: "test-rec", + Content: &recommenderpb.RecommendationContent{ + OperationGroups: []*recommenderpb.OperationGroup{ + { + Operations: []*recommenderpb.Operation{ + {Resource: "projects/test/buckets/STANDARD"}, + }, + }, + }, + }, + } + + rec := client.convertGCPRecommendation(ctx, gcpRec, common.RecommendationParams{}) + require.NotNil(t, rec) + assert.Nil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must remain nil when billing lookup fails") +} diff --git a/providers/gcp/services/computeengine/client.go b/providers/gcp/services/computeengine/client.go index 9900a863d..506e0c8f6 100644 --- a/providers/gcp/services/computeengine/client.go +++ b/providers/gcp/services/computeengine/client.go @@ -56,6 +56,17 @@ func termPlan(term string) (string, error) { } } +// termYearsFromTerm returns the commitment length in years for a term label, +// defaulting to 1 for any 1-year or unrecognized value and 3 for 3-year labels. +func termYearsFromTerm(term string) int { + switch strings.ToLower(strings.TrimSpace(term)) { + case "3yr", "3", "36mo": + return 3 + default: + return 1 + } +} + // CommitmentsService interface for commitments operations (enables mocking) type CommitmentsService interface { List(ctx context.Context, req *computepb.ListRegionCommitmentsRequest) CommitmentsIterator @@ -945,6 +956,19 @@ func (c *ComputeEngineClient) convertGCPRecommendation(ctx context.Context, gcpR c.enrichRecWithPricing(ctx, rec) + // GCP Compute CUDs are monthly-payment commitments (PaymentOption is forced + // to "monthly" above), so the per-month charge is CommitmentCost / + // termMonths. enrichRecWithPricing performs the single billing lookup and + // populates CommitmentCost; reuse it here rather than issuing a second call. + // When the billing lookup failed CommitmentCost stays 0 and we leave + // RecurringMonthlyCost nil (unknown) so the frontend renders "—" rather than + // an incorrect explicit "$0" (nil means unavailable, 0 means known-zero fee). + if rec.CommitmentCost > 0 { + termYears := termYearsFromTerm(rec.Term) + monthly := rec.CommitmentCost / float64(termYears*12) + rec.RecurringMonthlyCost = &monthly + } + return rec } @@ -958,10 +982,7 @@ func (c *ComputeEngineClient) enrichRecWithPricing(ctx context.Context, rec *com if rec.ResourceType == "" { return } - termYears := 1 - if rec.Term == "3yr" || rec.Term == "3" { - termYears = 3 - } + termYears := termYearsFromTerm(rec.Term) pricing, err := c.getComputePricing(ctx, rec.ResourceType, c.region, termYears) if err != nil { log.Printf("computeengine: pricing unavailable for %s in %s (issue #1020): %v", rec.ResourceType, c.region, err) diff --git a/providers/gcp/services/computeengine/client_test.go b/providers/gcp/services/computeengine/client_test.go index 971cfed87..990694358 100644 --- a/providers/gcp/services/computeengine/client_test.go +++ b/providers/gcp/services/computeengine/client_test.go @@ -900,7 +900,12 @@ func TestComputeEngineClient_ConvertGCPRecommendation(t *testing.T) { assert.Equal(t, "n1-standard-4", rec.ResourceType) assert.Equal(t, 50.5, rec.EstimatedSavings) assert.Equal(t, "monthly", rec.PaymentOption, - "GCP CUDs are billed monthly; PaymentOption must match ValidPaymentOptionsByProvider[\"gcp\"]") + `GCP CUDs are billed monthly; PaymentOption must match ValidPaymentOptionsByProvider["gcp"]`) + // GCP Compute CUDs are monthly-billed and RecurringMonthlyCost is derived + // from CommitmentCost. No billing service is injected here, so the pricing + // lookup fails and the field stays nil (unknown) rather than an incorrect + // explicit 0 (nil means unavailable, 0 means a known-zero recurring fee). + assert.Nil(t, rec.RecurringMonthlyCost) } // infiniteRecommenderIterator never signals iterator.Done, used to exercise diff --git a/providers/gcp/services/memorystore/client.go b/providers/gcp/services/memorystore/client.go index ba6a2946a..f54726824 100644 --- a/providers/gcp/services/memorystore/client.go +++ b/providers/gcp/services/memorystore/client.go @@ -438,14 +438,13 @@ func skuMatchesTier(sku *cloudbilling.Sku, tier, region string) bool { return true } -// extractResourceTypeFromContent extracts the last path segment of the first -// non-empty Operation.Resource across all operation groups. Used by all four -// GCP service converters to set rec.ResourceType from Recommender payloads. -func extractResourceTypeFromContent(content *recommenderpb.RecommendationContent) string { - if content == nil || content.OperationGroups == nil { +// extractGCPResourceType returns the last path segment of the first non-empty +// resource field found across all operation groups, or "" if none is present. +func extractGCPResourceType(rec *recommenderpb.Recommendation) string { + if rec.Content == nil || rec.Content.OperationGroups == nil { return "" } - for _, opGroup := range content.OperationGroups { + for _, opGroup := range rec.Content.OperationGroups { for _, op := range opGroup.Operations { if op.Resource == "" { continue @@ -459,13 +458,13 @@ func extractResourceTypeFromContent(content *recommenderpb.RecommendationContent return "" } -// extractEstimatedSavings returns the negative of the PrimaryImpact cost -// projection (GCP encodes savings as a negative cost delta). -func extractEstimatedSavings(gcpRec *recommenderpb.Recommendation) float64 { - if gcpRec.PrimaryImpact == nil { +// extractGCPSavings returns the estimated monthly savings (positive value) +// from the primary cost impact of a GCP recommendation, or 0 if absent. +func extractGCPSavings(rec *recommenderpb.Recommendation) float64 { + if rec.PrimaryImpact == nil { return 0 } - costProj := gcpRec.PrimaryImpact.GetCostProjection() + costProj := rec.PrimaryImpact.GetCostProjection() if costProj == nil || costProj.Cost == nil { return 0 } @@ -493,6 +492,15 @@ func (c *MemorystoreClient) fillRedisPricing(ctx context.Context, rec *common.Re } } +// termYearsFromLabel converts a term string such as "1yr" or "3yr" to an +// integer number of years (defaults to 1 for any unrecognized value). +func termYearsFromLabel(term string) int { + if term == "3yr" || term == "3" { + return 3 + } + return 1 +} + // convertGCPRecommendation converts a GCP Recommender recommendation to common format. // It also calls getRedisPricing to fill CommitmentCost/OnDemandCost/SavingsPercentage/ // BreakEvenMonths so the scorer can filter and rank GCP recommendations correctly @@ -519,17 +527,25 @@ func (c *MemorystoreClient) convertGCPRecommendation(ctx context.Context, gcpRec PaymentOption: paymentOption, } - rec.ResourceType = extractResourceTypeFromContent(gcpRec.Content) - rec.EstimatedSavings = extractEstimatedSavings(gcpRec) + rec.ResourceType = extractGCPResourceType(gcpRec) + rec.EstimatedSavings = extractGCPSavings(gcpRec) // Thread pricing into the converter so the scorer can rank/filter GCP recs - // correctly (issue #1022 C2). + // correctly (issue #1022 C2). fillRedisPricing performs the single billing + // lookup and populates CommitmentCost; we reuse that value below to derive + // RecurringMonthlyCost rather than issuing a second SKU call. if rec.ResourceType != "" { - termYears := 1 - if rec.Term == "3yr" || rec.Term == "3" { - termYears = 3 - } + termYears := termYearsFromLabel(rec.Term) c.fillRedisPricing(ctx, rec, termYears) + + // Memorystore CUDs are monthly-payment commitments, so the per-month + // charge is CommitmentCost / termMonths. When the billing lookup failed, + // CommitmentCost stays 0 and RecurringMonthlyCost remains nil so the + // frontend renders "—" rather than a stale value. + if rec.CommitmentCost > 0 { + monthly := rec.CommitmentCost / float64(termYears*12) + rec.RecurringMonthlyCost = &monthly + } } return rec diff --git a/providers/gcp/services/memorystore/client_test.go b/providers/gcp/services/memorystore/client_test.go index 81eff3f53..1d3c7e365 100644 --- a/providers/gcp/services/memorystore/client_test.go +++ b/providers/gcp/services/memorystore/client_test.go @@ -775,3 +775,106 @@ func TestMemorystoreClient_SetterMethods(t *testing.T) { client.SetRecommenderClient(mockRecommender) assert.Equal(t, mockRecommender, client.recommenderClient) } + +func TestMemorystoreClient_ConvertGCPRecommendation_RecurringMonthlyCost(t *testing.T) { + ctx := context.Background() + client, _ := NewClient(ctx, "test-project", "us-central1") + + // Inject a billing mock with a known on-demand hourly price and a separate + // commitment SKU. getRedisPricing derives CommitmentPrice from the + // "commitment" SKU (CommitmentPrice = commitmentHourly * 8760), so + // RecurringMonthlyCost must equal CommitmentPrice / 12 (one year = 12 months). + const onDemandHourly = 0.10 // USD/h -- representative basic Memorystore value + const commitmentHourly = 0.07 // USD/h -- 1yr CUD rate from the catalog + client.SetBillingService(&MockBillingService{ + skus: &cloudbilling.ListSkusResponse{ + Skus: []*cloudbilling.Sku{ + { + Description: "redis-basic Cloud Memorystore Redis", + ServiceRegions: []string{"us-central1"}, + PricingInfo: []*cloudbilling.PricingInfo{ + { + PricingExpression: &cloudbilling.PricingExpression{ + TieredRates: []*cloudbilling.TierRate{ + { + UnitPrice: &cloudbilling.Money{ + Units: 0, + Nanos: int64(onDemandHourly * 1e9), + CurrencyCode: "USD", + }, + }, + }, + }, + }, + }, + }, + { + // Commitment SKU required by getRedisPricing: without a + // "commitment" SKU it errors and RecurringMonthlyCost stays nil. + Description: "redis-basic Cloud Memorystore Redis commitment 1yr", + ServiceRegions: []string{"us-central1"}, + PricingInfo: []*cloudbilling.PricingInfo{ + { + PricingExpression: &cloudbilling.PricingExpression{ + TieredRates: []*cloudbilling.TierRate{ + { + UnitPrice: &cloudbilling.Money{ + Units: 0, + Nanos: int64(commitmentHourly * 1e9), + CurrencyCode: "USD", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }) + + gcpRec := &recommenderpb.Recommendation{ + Name: "test-rec", + Content: &recommenderpb.RecommendationContent{ + OperationGroups: []*recommenderpb.OperationGroup{ + { + Operations: []*recommenderpb.Operation{ + {Resource: "projects/test/instances/redis-basic"}, + }, + }, + }, + }, + } + + rec := client.convertGCPRecommendation(ctx, gcpRec, common.RecommendationParams{}) + require.NotNil(t, rec) + + // RecurringMonthlyCost must be a non-nil pointer to a positive value for + // a monthly Memorystore CUD recommendation. + require.NotNil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil when billing lookup succeeds") + assert.Greater(t, *rec.RecurringMonthlyCost, 0.0, "RecurringMonthlyCost must be positive for a monthly Memorystore CUD") + + // Verify the value matches CommitmentPrice / 12 exactly. CommitmentPrice is + // the commitment SKU's hourly rate scaled to the 1yr term total. + const hoursIn1yr = 8760.0 + expectedMonthly := commitmentHourly * hoursIn1yr / 12 + assert.InDelta(t, expectedMonthly, *rec.RecurringMonthlyCost, 1e-6) +} + +func TestMemorystoreClient_ConvertGCPRecommendation_RecurringMonthlyCost_BillingFailure(t *testing.T) { + ctx := context.Background() + client, _ := NewClient(ctx, "test-project", "us-central1") + + // Inject a billing mock that always errors; RecurringMonthlyCost must + // remain nil (frontend renders "—") rather than a zero or stale value. + client.SetBillingService(&MockBillingService{err: errors.New("billing unavailable")}) + + gcpRec := &recommenderpb.Recommendation{ + Name: "test-rec", + Content: &recommenderpb.RecommendationContent{}, + } + + rec := client.convertGCPRecommendation(ctx, gcpRec, common.RecommendationParams{}) + require.NotNil(t, rec) + assert.Nil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be nil when billing lookup fails") +}