diff --git a/CHANGELOG.md b/CHANGELOG.md
index 68d8361c5..3623a02b6 100644
--- a/CHANGELOG.md
+++ b/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).
diff --git a/cmd/ec2_zonal_az_test.go b/cmd/ec2_zonal_az_test.go
index abb8d4207..f74f62d78 100644
--- a/cmd/ec2_zonal_az_test.go
+++ b/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")
})
}
diff --git a/cmd/multi_service_helpers.go b/cmd/multi_service_helpers.go
index ec1f8acc8..a8bb73d9a 100644
--- a/cmd/multi_service_helpers.go
+++ b/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)
}
diff --git a/cmd/reservation_expiry_command_test.go b/cmd/reservation_expiry_command_test.go
index 0306cf900..8a9f795ea 100644
--- a/cmd/reservation_expiry_command_test.go
+++ b/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", `- m5.large28192
`
case "DescribeReservedInstances":
- return 200, "text/xml", fmt.Sprintf(`- expiry-fixturem5.large%dactive%s%s
`, 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(`- %sm5.large%dactive%s%s
`,
+ 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(`%s`, items)
+}
+
func (p *completenessProxy) reservationExpiryCoverage(body []byte) string {
var request struct {
TimePeriod map[string]string
diff --git a/cmd/reservation_expiry_test.go b/cmd/reservation_expiry_test.go
index e469dbbc3..b17174e58 100644
--- a/cmd/reservation_expiry_test.go
+++ b/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
diff --git a/cmd/savings_plan_purchase.go b/cmd/savings_plan_purchase.go
index 1d37727b1..7ad0e28aa 100644
--- a/cmd/savings_plan_purchase.go
+++ b/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)
diff --git a/cmd/savings_plan_purchase_test.go b/cmd/savings_plan_purchase_test.go
index af4f05e68..998bf24f1 100644
--- a/cmd/savings_plan_purchase_test.go
+++ b/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)
}
}
diff --git a/go.mod b/go.mod
index 7fb6388f5..6c7b481cc 100644
--- a/go.mod
+++ b/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
diff --git a/go.sum b/go.sum
index 9ae81a29c..83477ee9e 100644
--- a/go.sum
+++ b/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=