fix(aws): credit replacement RIs bought inside the coverage lookback

CHANGELOG.md

@@ -13,6 +13,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ### Fixed
 
+- Savings Plans purchases now send the hourly commitment truncated to 5 decimals
+  (at least 2 shown) instead of rounded to cents, and accept amounts from 0.001
+  to 1,000,000, so a 0.001 to 0.004 commitment is no longer rejected (library
+  bump, #2235).
+- `--target-coverage` with `--rebuy-window-days` no longer buys replacement EC2 and
+  other non-RDS RIs again when they were already bought inside the coverage
+  lookback: the expiring RIs are netted against those replacements (#2235).
 - Recommendations with no account ID or name are now reported in the drop summary
   when an account filter (`--include-accounts` or `--exclude-accounts`) removes
   them (#1613).

cmd/ec2_zonal_az_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"net/url"
 	"path/filepath"
+	"strings"
 	"sync"
 	"testing"
 
@@ -90,7 +91,7 @@ func TestPurchaseSingleRec_RefusesInconsistentEC2ScopeBeforeAnyAWSCall(t *testin
 			result, recorder := purchaseEC2WithRecorder(t, rec)
 
 			require.Error(t, result.Error)
-			assert.Contains(t, result.Error.Error(), "AvailabilityZone")
+			assert.Contains(t, strings.ToLower(result.Error.Error()), "availability zone")
 			assert.Empty(t, recorder.actions, "no AWS call may precede the refusal")
 		})
 	}

cmd/multi_service_helpers.go

@@ -577,8 +577,9 @@ func applyRegionFilters(
 //
 // expiringCommitments (when non-empty and cfg.RebuyWindowDays > 0) reduces
 // ExistingCoveragePct further by the share of pool demand attributable to
-// RIs expiring within the window, so --target-coverage recommends
-// replacements before the cliff. Doesn't run if either input is empty.
+// RIs expiring within the window, net of replacement RIs started inside the
+// coverage lookback (so a re-run does not buy them again), so --target-coverage
+// recommends replacements before the cliff. Doesn't run if either input is empty.
 // RDS recs sized by the family-NU pass (target-coverage mode) ignore that
 // per-rec adjustment and instead get the same commitments and window through
 // ApplyFamilyNUSizingRDSWithExpiryOptions, which deducts expiring normalized units
@@ -592,7 +593,9 @@ func applyRegionFilters(
 func applyCoverageAndOverrides(recs []common.Recommendation, cfg Config, coverageMap recommendations.PoolCoverageMap, expiringCommitments []common.Commitment, drops *common.DropSummary) []common.Recommendation {
 	recommendations.ApplyCoverageMapToRecommendations(recs, coverageMap)
 	if cfg.RebuyWindowDays > 0 && len(expiringCommitments) > 0 {
-		n, missingDemand := recommendations.AdjustExistingCoverageForExpiringCommitmentsWithCoverage(recs, expiringCommitments, cfg.RebuyWindowDays, coverageMap)
+		n, missingDemand := recommendations.AdjustExistingCoverageForExpiringCommitmentsWithCoverageOptions(
+			recs, expiringCommitments, coverageMap,
+			recommendations.PoolExpiryOptions{WindowDays: cfg.RebuyWindowDays, CoverageLookbackDays: coverageLookbackDays(cfg)})
 		if n > 0 {
 			AppLogger.Printf("  ⏰ Treating %d recs as partially uncovered (RIs expiring within %d days)\n", n, cfg.RebuyWindowDays)
 		}

cmd/reservation_expiry_command_test.go

@@ -23,6 +23,7 @@ type reservationExpiryScenario struct {
 	rows, expiry, endDays, count      int
 	average, demand, target, existing float64
 	missing, filtered                 bool
+	replacedDays, replacedCount       int
 }
 
 func reservationExpiryCase(name string) reservationExpiryScenario {
@@ -31,6 +32,11 @@ func reservationExpiryCase(name string) reservationExpiryScenario {
 	case "pool-demand":
 	case "outside-window":
 		s.endDays, s.count, s.existing = 180, 6, 60
+	case "replaced-inside-lookback":
+		// Replacements bought 5 days ago are inside the 30-day coverage
+		// lookback, so the expiring RIs net to zero and the run must not
+		// re-buy them (cli#2235).
+		s.count, s.existing, s.replacedDays, s.replacedCount = 6, 60, 5, 18
 	case "missing-demand-two-each-six-total":
 		s.average, s.demand, s.count, s.existing, s.missing = 10, 0, 2, 60, true
 	case "filtered-subset":
@@ -47,7 +53,7 @@ func reservationExpiryCase(name string) reservationExpiryScenario {
 }
 
 func TestReservationExpiryCommand(t *testing.T) {
-	for _, name := range []string{"pool-demand", "outside-window", "missing-demand-two-each-six-total", "filtered-subset", "exact-boundary", "below-boundary"} {
+	for _, name := range []string{"pool-demand", "outside-window", "replaced-inside-lookback", "missing-demand-two-each-six-total", "filtered-subset", "exact-boundary", "below-boundary"} {
 		t.Run(name, func(t *testing.T) { runCompletenessScenario(t, "ec2", "explicit", name) })
 	}
 }
@@ -189,7 +195,7 @@ func (p *completenessProxy) reservationExpiryResponse(host, op string, body []by
 		case "DescribeInstanceTypes":
 			return 200, "text/xml", `<DescribeInstanceTypesResponse xmlns="http://ec2.amazonaws.com/doc/2016-11-15/"><instanceTypeSet><item><instanceType>m5.large</instanceType><vCpuInfo><defaultVCpus>2</defaultVCpus></vCpuInfo><memoryInfo><sizeInMiB>8192</sizeInMiB></memoryInfo></item></instanceTypeSet></DescribeInstanceTypesResponse>`
 		case "DescribeReservedInstances":
-			return 200, "text/xml", fmt.Sprintf(`<DescribeReservedInstancesResponse xmlns="http://ec2.amazonaws.com/doc/2016-11-15/"><reservedInstancesSet><item><reservedInstancesId>expiry-fixture</reservedInstancesId><instanceType>m5.large</instanceType><instanceCount>%d</instanceCount><state>active</state><start>%s</start><end>%s</end></item></reservedInstancesSet></DescribeReservedInstancesResponse>`, s.expiry, time.Now().AddDate(-1, 0, 0).UTC().Format(time.RFC3339), time.Now().AddDate(0, 0, s.endDays).UTC().Format(time.RFC3339))
+			return 200, "text/xml", reservationExpiryReservedInstances(s)
 		}
 	}
 	if host == "rds.us-east-1.amazonaws.com" && (op == "DescribeDBInstances" || op == "DescribeDBMajorEngineVersions") {
@@ -208,6 +214,23 @@ func (p *completenessProxy) reservationExpiryResponse(host, op string, body []by
 	return 403, "text/plain", "fixture rejected operation"
 }
 
+// reservationExpiryReservedInstances renders the owned EC2 RIs the fixture
+// reports: the expiring RI that --rebuy-window-days acts on, plus a
+// replacement bought inside the coverage lookback when the scenario asks for
+// one.
+func reservationExpiryReservedInstances(s reservationExpiryScenario) string {
+	item := func(id string, count int, start, end time.Time) string {
+		return fmt.Sprintf(`<item><reservedInstancesId>%s</reservedInstancesId><instanceType>m5.large</instanceType><instanceCount>%d</instanceCount><state>active</state><start>%s</start><end>%s</end></item>`,
+			id, count, start.UTC().Format(time.RFC3339), end.UTC().Format(time.RFC3339))
+	}
+	now := time.Now()
+	items := item("expiry-fixture", s.expiry, now.AddDate(-1, 0, 0), now.AddDate(0, 0, s.endDays))
+	if s.replacedCount > 0 {
+		items += item("replacement-fixture", s.replacedCount, now.AddDate(0, 0, -s.replacedDays), now.AddDate(0, 0, 400))
+	}
+	return fmt.Sprintf(`<DescribeReservedInstancesResponse xmlns="http://ec2.amazonaws.com/doc/2016-11-15/"><reservedInstancesSet>%s</reservedInstancesSet></DescribeReservedInstancesResponse>`, items)
+}
+
 func (p *completenessProxy) reservationExpiryCoverage(body []byte) string {
 	var request struct {
 		TimePeriod map[string]string

cmd/reservation_expiry_test.go

@@ -50,6 +50,41 @@ func TestReservationExpirySizing(t *testing.T) {
 	}
 }
 
+// A --target-coverage run must not re-buy replacements already bought inside
+// the coverage lookback (cli#2235): the lagged coverage average only partly
+// holds them, so the expiring RIs are netted against the replacements.
+func TestReservationExpiryCreditsReplacementsInsideLookback(t *testing.T) {
+	coverage := recommendations.PoolCoverageMap{"us-east-1:m5.large": {Pct: 60, AvgInstancesPerHour: 90}}
+	replacement := func(startDaysAgo int) common.Commitment {
+		c := reservationExpiryCommitments(18, 400)[0]
+		c.StartDate = time.Now().AddDate(0, 0, -startDaysAgo)
+		return c
+	}
+	for _, tc := range []struct {
+		name         string
+		lookbackDays int
+		startDaysAgo int
+		wantCount    int
+		wantExisting float64
+	}{
+		{"replaced-inside-lookback", 30, 5, 6, 60},
+		{"default-lookback-when-unset", 0, 5, 6, 60},
+		{"replaced-outside-lookback", 30, 45, 12, 40},
+		{"lookback-narrower-than-purchase", 3, 5, 12, 40},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := Config{TargetCoverage: 80, RebuyWindowDays: 30, CoverageLookbackDays: tc.lookbackDays}
+			commitments := append(reservationExpiryCommitments(18, 15), replacement(tc.startDaysAgo))
+			got := applyCoverageAndOverrides(reservationExpiryRecommendations([]float64{30, 30, 30}), cfg, coverage, commitments, nil)
+			require.Len(t, got, 3)
+			for _, rec := range got {
+				assert.Equal(t, tc.wantCount, rec.Count)
+				assert.InDelta(t, tc.wantExisting, rec.ExistingCoveragePct, 1e-12)
+			}
+		})
+	}
+}
+
 func TestReservationExpiryMissingDemand(t *testing.T) {
 	for _, tc := range []struct {
 		name     string

cmd/savings_plan_purchase.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"fmt"
+	"math"
 	"strconv"
 	"strings"
 	"time"
@@ -17,11 +18,15 @@ import (
 // regions and are not supported for Savings Plan purchases.
 const spAPIRegion = "us-east-1"
 
-// spCommitmentFormat is how the library formats HourlyCommitment for the
-// CreateSavingsPlan call (savingsplans/client.go). A positive value that
-// formats as "0.00" would be sent as a zero commitment, so preconditions
-// reject it.
-const spCommitmentFormat = "%.2f"
+// Hourly commitment bounds accepted by the library's CreateSavingsPlan path.
+// Cross-repo mirror: providers/aws/internal/spcommitment.Normalize in
+// cloud-commitments-go is the source of truth (it also truncates the sent
+// amount to 5 decimals). The CLI repeats the bounds so a dry run and the
+// confirmation prompt reject a bad amount before any purchase starts.
+const (
+	spMinHourlyCommitment = 0.001
+	spMaxHourlyCommitment = 1000000.0
+)
 
 // savingsPlanRowKey identifies one Savings Plan purchase. Two rows with the
 // same key buy the same plan twice.
@@ -146,11 +151,11 @@ func validateSavingsPlanPreconditions(rec common.Recommendation) error {
 	if d.PlanType == "" {
 		return fmt.Errorf("%s recommendation is missing the plan type", rec.Service)
 	}
-	if d.HourlyCommitment <= 0 {
-		return fmt.Errorf("%s recommendation has no positive hourly commitment (got %v)", rec.Service, d.HourlyCommitment)
+	if math.IsNaN(d.HourlyCommitment) || math.IsInf(d.HourlyCommitment, 0) || d.HourlyCommitment <= 0 {
+		return fmt.Errorf("%s recommendation has no positive finite hourly commitment (got %v)", rec.Service, d.HourlyCommitment)
 	}
-	if fmt.Sprintf(spCommitmentFormat, d.HourlyCommitment) == "0.00" {
-		return fmt.Errorf("%s recommendation's hourly commitment %v would be sent as 0.00; it must be at least 0.005", rec.Service, d.HourlyCommitment)
+	if d.HourlyCommitment < spMinHourlyCommitment || d.HourlyCommitment > spMaxHourlyCommitment {
+		return fmt.Errorf("%s recommendation's hourly commitment %v must be between %g and %g", rec.Service, d.HourlyCommitment, spMinHourlyCommitment, spMaxHourlyCommitment)
 	}
 	if rec.Service == common.ServiceSavingsPlansEC2Instance {
 		return validateEC2InstanceSPOffering(d)

cmd/savings_plan_purchase_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/csv"
 	"io"
+	"math"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -298,21 +299,18 @@ func TestValidatePurchasePreconditions(t *testing.T) {
 	}
 }
 
-// TestValidatePurchasePreconditions_RejectsCommitmentThatRoundsToZero covers
-// issue #2163: the library sends the hourly commitment as "%.2f", so a positive
-// value below 0.005 would reach CreateSavingsPlan as "0.00".
-func TestValidatePurchasePreconditions_RejectsCommitmentThatRoundsToZero(t *testing.T) {
+// TestValidatePurchasePreconditions_CommitmentBounds mirrors the library's
+// spcommitment.Normalize bounds (0.001..1e6, positive, finite).
+func TestValidatePurchasePreconditions_CommitmentBounds(t *testing.T) {
 	withCommitment := func(c float64) common.Recommendation {
 		rec := parserShapedSP()
 		rec.Details = &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: c}
 		return rec
 	}
-	for _, c := range []float64{0.001, 0.004, 0.00499} {
-		err := validatePurchasePreconditions(withCommitment(c), "", defaultTestAccount)
-		require.Error(t, err, "commitment %v rounds to 0.00 and must be rejected", c)
-		assert.Contains(t, err.Error(), "0.00", "commitment %v", c)
+	for _, c := range []float64{0.0009, 0.00099, 0, -1, 1000000.1, math.NaN(), math.Inf(1)} {
+		assert.Error(t, validatePurchasePreconditions(withCommitment(c), "", defaultTestAccount), "commitment %v", c)
 	}
-	for _, c := range []float64{0.005, 0.01, 1.5} {
+	for _, c := range []float64{0.001, 0.004, 0.005, 1.239, 1.2345678, 1000000} {
 		assert.NoError(t, validatePurchasePreconditions(withCommitment(c), "", defaultTestAccount), "commitment %v", c)
 	}
 }

go.mod

@@ -83,7 +83,7 @@ require (
 require (
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2 v2.2.0
 	github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261016175124-570d168c6bc7
-	github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261013212804-9b35837e78d1
+	github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261016165653-512e42fe37c6
 	github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261013212804-9b35837e78d1
 	github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261013212804-9b35837e78d1
 	github.com/aws/aws-sdk-go-v2/service/organizations v1.45.3

go.sum

@@ -80,8 +80,8 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapp
 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0/go.mod h1:Mf6O40IAyB9zR/1J8nGDDPirZQQPbYJni8Yisy7NTMc=
 github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261016175124-570d168c6bc7 h1:mAvqQSMkTuw4P8m8o+/wdyoxGE9L6eiEzZFanGXqYcI=
 github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261016175124-570d168c6bc7/go.mod h1:SCDEgHG6evjPh0RsoXof+J3aMmGXNFMrwbJIle7rMpQ=
-github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261013212804-9b35837e78d1 h1:c4JuGObXDFP+oqC7CD+0Sn9ljzlUciVikW7iMDCNNa4=
-github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261013212804-9b35837e78d1/go.mod h1:no7i/7i+W37+qhEOVU8F/0dP5FhO3wtzvKIN0+tvL3A=
+github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261016165653-512e42fe37c6 h1:nxcsaVj7BKH0/I9wiTvraQPivKABf/GzE7Ecwm47u9U=
+github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261016165653-512e42fe37c6/go.mod h1:uOUuTVRjTm0Fhu/Vra2M2NES0w4pvZUSfZLsGyCd18s=
 github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261013212804-9b35837e78d1 h1:aYYu7JDJQcoriUKg/m+IgTJruc2ugfBLXafAC2LQy00=
 github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261013212804-9b35837e78d1/go.mod h1:8vqO2ZlDzjX/Z4bESZVp/IenhTifkySeF0VJwQKihLA=
 github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261013212804-9b35837e78d1 h1:LPI8x3lOnrvr4w3oX8cHsCLoXuCSLSUhcHbe4WCMUSQ=