fix(aws): round fractional RI quantities down and scale their costs

CHANGELOG.md

@@ -279,6 +279,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ### Fixed
 
+- **`aws` RI recommendations:** a fractional Cost Explorer quantity now rounds
+  down instead of to the nearest integer, so a purchase never exceeds CE's
+  quantity. The upfront, recurring monthly cost, monthly savings and savings
+  percentage scale by `count / quantity`, so they describe the instances
+  actually bought. Before, 2.5 became 3 with the upfront for 2.5, and the
+  approved-upfront check refused the purchase. The on-demand baseline and
+  break-even months are unchanged (#635).
 - **`azure`:** an error from a reservation purchase retry (attempt 2 or later) now also wraps `common.ErrOutcomeUnknown`. That covers a currency refusal from the retry's calculatePrice, a failed idempotency re-check, a non-retryable purchase failure, a cancelled retry wait and exhausted attempts. A retry runs only after a purchase that failed retryably and may have committed, and the re-check can miss that order, so none of these errors may read as "nothing was bought". The cause stays in the chain, so `errors.Is(err, common.ErrCurrencyNotAccepted)` still matches. The `ErrCurrencyNotAccepted` doc now says it is pre-purchase only without `ErrOutcomeUnknown` (#671).
 - **Azure purchases check the recommendation currency.** Azure reservation purchases
   compare the calculatePrice billing currency with the recommendation currency

providers/aws/recommendations/parser_ri.go

@@ -73,7 +73,7 @@ func (c *Client) parseRecommendationDetail(ctx context.Context, details *types.R
 	// count so the CSV can show what AWS proposed alongside what --coverage /
 	// --target-coverage chose; Count is the working value the sizing step
 	// mutates.
-	count, roundedFromFraction, err := c.parseRecommendedQuantity(details)
+	count, qty, roundedFromFraction, err := c.parseRecommendedQuantity(details)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse recommended quantity: %w", err)
 	}
@@ -107,6 +107,8 @@ func (c *Client) parseRecommendationDetail(ctx context.Context, details *types.R
 		return nil, fmt.Errorf("failed to parse RI utilization signals: %w", err)
 	}
 
+	scaleToRoundedCount(rec, count, qty)
+
 	// Parse service-specific details
 	if err := c.parseServiceSpecificDetails(ctx, rec, details, params.Service); err != nil {
 		return nil, err
@@ -143,35 +145,67 @@ func (c *Client) parseRIUtilizationSignals(rec *common.Recommendation, details *
 	return nil
 }
 
-// parseRecommendedQuantity extracts the recommended quantity from details. The
-// bool reports that a positive fractional quantity rounded down to 0; the
-// caller warns once the recommendation's identity is known.
-func (c *Client) parseRecommendedQuantity(details *types.ReservationPurchaseRecommendationDetail) (rounded int, fractionRoundedToZero bool, err error) {
+// scaleToRoundedCount makes the figures CE reports for its (fractional)
+// quantity agree with the rounded-down count that is actually bought (go#635
+// option C): the upfront, the recurring monthly charge, the monthly savings and
+// the savings percentage scale by count/qty. The on-demand baseline, break-even
+// (upfront over savings) and the usage signals do not depend on the quantity.
+// A quantity that rounds to 0 scales every cost to 0, so the row never carries
+// an upfront or savings for instances it does not buy; the caller warns.
+func scaleToRoundedCount(rec *common.Recommendation, count int, qty float64) {
+	if float64(count) == qty {
+		return
+	}
+	f := float64(count) / qty
+	rec.CommitmentCost *= f
+	if rec.RecurringMonthlyCost != nil {
+		v := *rec.RecurringMonthlyCost * f
+		rec.RecurringMonthlyCost = &v
+	}
+	rec.EstimatedSavings *= f
+	rec.SavingsPercentage *= f
+}
+
+// parseRecommendedQuantity extracts the recommended quantity from details,
+// rounded down so a purchase never exceeds CE's quantity (go#635 option C),
+// and also returns the parsed quantity for scaling. The bool reports that a
+// positive fractional quantity rounded down to 0; the caller warns once the
+// recommendation's identity is known.
+func (c *Client) parseRecommendedQuantity(details *types.ReservationPurchaseRecommendationDetail) (rounded int, qty float64, fractionRoundedToZero bool, err error) {
 	if details.RecommendedNumberOfInstancesToPurchase == nil {
-		return 0, false, fmt.Errorf("recommended quantity not found")
+		return 0, 0, false, fmt.Errorf("recommended quantity not found")
 	}
 
-	qty := *details.RecommendedNumberOfInstancesToPurchase
+	raw := *details.RecommendedNumberOfInstancesToPurchase
 
 	var count float64
-	if _, err = fmt.Sscanf(qty, "%f", &count); err != nil {
-		if intCount, atoiErr := strconv.Atoi(qty); atoiErr == nil {
-			if intCount < 0 {
-				return 0, false, fmt.Errorf("recommended quantity %q is negative", qty)
-			}
-			return intCount, false, nil
+	if _, scanErr := fmt.Sscanf(raw, "%f", &count); scanErr != nil {
+		intCount, atoiErr := strconv.Atoi(raw)
+		if atoiErr != nil {
+			return 0, 0, false, fmt.Errorf("failed to parse quantity '%s' as float or int", raw)
 		}
-		return 0, false, fmt.Errorf("failed to parse quantity '%s' as float or int", qty)
+		count = float64(intCount)
 	}
-	// Sscanf %f accepts "NaN"/"Inf"; a non-finite quantity would corrupt the
-	// purchase count (int(math.Round(NaN)) is undefined). A negative count is
-	// likewise invalid for a purchase quantity. Fail loud on both.
-	if math.IsNaN(count) || math.IsInf(count, 0) || count < 0 {
-		return 0, false, fmt.Errorf("recommended quantity %q is not a finite non-negative number", qty)
+	if err := checkQuantityRange(raw, count); err != nil {
+		return 0, 0, false, err
 	}
 
-	rounded = int(math.Round(count))
-	return rounded, rounded == 0 && count > 0, nil
+	rounded = int(math.Floor(count))
+	return rounded, count, rounded == 0 && count > 0, nil
+}
+
+// checkQuantityRange refuses a quantity that cannot be a purchase count.
+// Sscanf %f accepts "NaN"/"Inf" (int(math.Floor(NaN)) is undefined), negatives
+// are meaningless, and purchase APIs take an int32 count, so a larger value
+// would saturate or truncate.
+func checkQuantityRange(raw string, count float64) error {
+	if math.IsNaN(count) || math.IsInf(count, 0) || count < 0 {
+		return fmt.Errorf("recommended quantity %q is not a finite non-negative number", raw)
+	}
+	if count > math.MaxInt32 {
+		return fmt.Errorf("recommended quantity %q exceeds %d", raw, math.MaxInt32)
+	}
+	return nil
 }
 
 // parseCostInformation extracts cost and savings information.

providers/aws/recommendations/parser_ri_test.go

@@ -36,11 +36,11 @@ func TestParseRecommendedQuantity(t *testing.T) {
 			expectError: false,
 		},
 		{
-			name: "Float quantity rounds to nearest",
+			name: "Float quantity rounds down (go#635 option C)",
 			details: &types.ReservationPurchaseRecommendationDetail{
 				RecommendedNumberOfInstancesToPurchase: aws.String("3.8"),
 			},
-			expected:    4, // math.Round(3.8) = 4
+			expected:    3, // never buy above CE's quantity
 			expectError: false,
 		},
 		{
@@ -107,6 +107,30 @@ func TestParseRecommendedQuantity(t *testing.T) {
 			expected:    0,
 			expectError: true,
 		},
+		{
+			name: "float quantity above MaxInt32 rejected",
+			details: &types.ReservationPurchaseRecommendationDetail{
+				RecommendedNumberOfInstancesToPurchase: aws.String("1e10"),
+			},
+			expected:    0,
+			expectError: true,
+		},
+		{
+			name: "int quantity above MaxInt32 rejected",
+			details: &types.ReservationPurchaseRecommendationDetail{
+				RecommendedNumberOfInstancesToPurchase: aws.String("3000000000"),
+			},
+			expected:    0,
+			expectError: true,
+		},
+		{
+			name: "MaxInt32 accepted",
+			details: &types.ReservationPurchaseRecommendationDetail{
+				RecommendedNumberOfInstancesToPurchase: aws.String("2147483647"),
+			},
+			expected:    2147483647,
+			expectError: false,
+		},
 		{
 			name: "negative int quantity rejected",
 			details: &types.ReservationPurchaseRecommendationDetail{
@@ -119,7 +143,7 @@ func TestParseRecommendedQuantity(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result, _, err := client.parseRecommendedQuantity(tt.details)
+			result, _, _, err := client.parseRecommendedQuantity(tt.details)
 
 			if tt.expectError {
 				assert.Error(t, err)
@@ -141,7 +165,9 @@ func TestParseRecommendationDetail_WarnsWhenFractionRoundsToZero(t *testing.T) {
 		{"0.05", 0, true},
 		{"0.4", 0, true},
 		{"0.4999", 0, true},
-		{"0.5", 1, false}, // math.Round rounds half away from zero
+		{"0.5", 0, true},
+		{"0.9999", 0, true}, // rounds down, never up (go#635)
+		{"1.5", 1, false},
 		{"2.4", 2, false},
 		{"0", 0, false},
 		{"0.0", 0, false},
@@ -829,3 +855,59 @@ func TestParseRecommendations_InvalidAverageInstancesIsReportedNotZeroed(t *test
 		assert.Zero(t, recs[0].AverageInstancesUsedPerHour)
 	})
 }
+
+// go#635 option C: a fractional quantity rounds down, and the figures CE gives
+// for that quantity scale to the bought count, so Count and the costs agree.
+func TestParseRecommendationDetail_FloorsFractionalQuantityAndScalesCosts(t *testing.T) {
+	client := &Client{}
+	for _, tt := range []struct {
+		qty   string
+		count int
+		scale float64
+	}{
+		{"2.5", 2, 2 / 2.5},
+		{"2.4", 2, 2 / 2.4},
+		{"3.99", 3, 3 / 3.99},
+		{"3", 3, 1},
+		{"3.0", 3, 1},
+		// Rounds to 0: option C scales the costs to 0 as well, so a Count-0
+		// row never carries a full upfront or counts toward savings.
+		{"0.4", 0, 0},
+		{"0.5", 0, 0},
+		{"0.6", 0, 0},
+	} {
+		t.Run(tt.qty, func(t *testing.T) {
+			rec, err := client.parseRecommendationDetail(context.Background(), &types.ReservationPurchaseRecommendationDetail{
+				RecommendedNumberOfInstancesToPurchase: aws.String(tt.qty),
+				AccountId:                              aws.String("123456789012"),
+				CurrencyCode:                           aws.String("USD"),
+				UpfrontCost:                            aws.String("2500"),
+				RecurringStandardMonthlyCost:           aws.String("100"),
+				EstimatedMonthlySavingsAmount:          aws.String("50"),
+				EstimatedMonthlySavingsPercentage:      aws.String("40"),
+				EstimatedMonthlyOnDemandCost:           aws.String("125"),
+				EstimatedBreakEvenInMonths:             aws.String("7"),
+				AverageNumberOfInstancesUsedPerHour:    aws.String("2.7"),
+				InstanceDetails: &types.InstanceDetails{
+					EC2InstanceDetails: &types.EC2InstanceDetails{
+						InstanceType: aws.String("m5.large"),
+						Platform:     aws.String("Linux/UNIX"),
+						Region:       aws.String("us-east-1"),
+						Tenancy:      aws.String("shared"),
+					},
+				},
+			}, common.RecommendationParams{Service: common.ServiceEC2, PaymentOption: "partial-upfront", Term: "1yr", LookbackPeriod: "7d"})
+			require.NoError(t, err)
+			assert.Equal(t, tt.count, rec.Count)
+			assert.Equal(t, tt.count, rec.RecommendedCount)
+			assert.InDelta(t, 2500*tt.scale, rec.CommitmentCost, 1e-9, "upfront")
+			require.NotNil(t, rec.RecurringMonthlyCost)
+			assert.InDelta(t, 100*tt.scale, *rec.RecurringMonthlyCost, 1e-9, "recurring")
+			assert.InDelta(t, 50*tt.scale, rec.EstimatedSavings, 1e-9, "savings")
+			assert.InDelta(t, 40*tt.scale, rec.SavingsPercentage, 1e-9, "savings %% of an unchanged on-demand baseline")
+			assert.InDelta(t, 125, rec.OnDemandCost, 1e-9, "on-demand spend does not depend on the quantity bought")
+			assert.InDelta(t, 7, rec.BreakEvenMonths, 1e-9, "upfront/savings is unchanged by scaling")
+			assert.InDelta(t, 2.7, rec.AverageInstancesUsedPerHour, 1e-9, "demand is not scaled")
+		})
+	}
+}

providers/aws/services/ec2/purchase_price_guard_test.go

@@ -86,3 +86,30 @@ func TestCheckApprovedUpfront_RefusesMissingPriceAndNonUSDOffering(t *testing.T)
 	eur.CurrencyCode = types.CurrencyCodeValues("EUR")
 	require.ErrorContains(t, checkApprovedUpfront(pricedRec(), &eur), "USD only")
 }
+
+// go#635: CE recommends 2.5 instances at a $2500 upfront for that quantity. The
+// parser floors it to 2 and scales the upfront to $2000 (pinned in
+// recommendations.TestParseRecommendationDetail_FloorsFractionalQuantityAndScalesCosts),
+// so a $1000 offering (2 x 1000) is bought, and one above the 5% tolerance is
+// still refused.
+func TestPurchaseCommitment_FlooredFractionalQuantity(t *testing.T) {
+	t.Parallel()
+	floored := func() common.Recommendation {
+		r := pricedRec()
+		r.Count = 2
+		r.CommitmentCost = 2500 * 2 / 2.5
+		return r
+	}
+	m := offeringsMock(pricedOffering(1000))
+	res, err := (&Client{client: m, region: "us-east-1"}).PurchaseCommitment(context.Background(), floored(), common.PurchaseOptions{})
+	require.NoError(t, err)
+	assert.True(t, res.Success)
+	m.AssertCalled(t, "PurchaseReservedInstancesOffering", mock.Anything, mock.MatchedBy(func(in *ec2.PurchaseReservedInstancesOfferingInput) bool {
+		return in.InstanceCount != nil && *in.InstanceCount == 2
+	}), mock.Anything)
+
+	m = offeringsMock(pricedOffering(1051)) // 2102 > 2000 x 1.05
+	_, err = (&Client{client: m, region: "us-east-1"}).PurchaseCommitment(context.Background(), floored(), common.PurchaseOptions{})
+	require.ErrorContains(t, err, "exceeds the approved upfront")
+	m.AssertNotCalled(t, "PurchaseReservedInstancesOffering", mock.Anything, mock.Anything)
+}