diff --git a/cmd/multi_service_helpers.go b/cmd/multi_service_helpers.go index 387508d00..8a04076a3 100644 --- a/cmd/multi_service_helpers.go +++ b/cmd/multi_service_helpers.go @@ -537,10 +537,13 @@ 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 := recommendations.AdjustExistingCoverageForExpiringCommitments(recs, expiringCommitments, cfg.RebuyWindowDays) + n, missingDemand := recommendations.AdjustExistingCoverageForExpiringCommitmentsWithCoverage(recs, expiringCommitments, cfg.RebuyWindowDays, coverageMap) if n > 0 { AppLogger.Printf(" ⏰ Treating %d recs as partially uncovered (RIs expiring within %d days)\n", n, cfg.RebuyWindowDays) } + if missingDemand > 0 { + AppLogger.Printf(" ⚠️ Skipped expiry adjustment for %d recommendations because pool demand was unavailable; coverage was left unchanged\n", missingDemand) + } } // Family-NU sizing for RDS recs: AWS rec API already bundles size-flex // demand within a family into one rec at one size, so per-pool sizing diff --git a/cmd/reservation_expiry_test.go b/cmd/reservation_expiry_test.go new file mode 100644 index 000000000..e469dbbc3 --- /dev/null +++ b/cmd/reservation_expiry_test.go @@ -0,0 +1,136 @@ +package main + +import ( + "bytes" + "fmt" + "math" + "strings" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestReservationExpirySizing(t *testing.T) { + for _, tc := range []struct { + name string + averages []float64 + wantCounts []int + wantAverages []float64 + demand, target, existing float64 + expiry, endDays int + exclude []string + }{ + {"equal-shares", []float64{30, 30, 30}, []int{12, 12, 12}, []float64{30, 30, 30}, 90, 80, 40, 18, 15, nil}, + {"unequal-shares", []float64{15, 30, 45}, []int{6, 12, 18}, []float64{15, 30, 45}, 90, 80, 40, 18, 15, nil}, + {"zero-raw-shares", []float64{0, 0, 0}, []int{12, 12, 12}, []float64{30, 30, 30}, 90, 80, 40, 18, 15, nil}, + {"filtered-two", []float64{30, 30, 30}, []int{18, 18}, []float64{45, 45}, 90, 80, 40, 18, 15, []string{"expiry-account-3"}}, + {"filtered-one", []float64{30, 30, 30}, []int{36}, []float64{90}, 90, 80, 40, 18, 15, []string{"expiry-account-2", "expiry-account-3"}}, + {"outside-window", []float64{30, 30, 30}, []int{6, 6, 6}, []float64{30, 30, 30}, 90, 80, 60, 18, 180, nil}, + {"exact-boundary", []float64{15, 15}, []int{4, 4}, []float64{15, 15}, 30, 80, 160.0 / 3, 2, 15, nil}, + {"below-boundary", []float64{15, 15}, []int{3, 3}, []float64{15, 15}, 30, math.Nextafter(80, 0), 160.0 / 3, 2, 15, nil}, + } { + t.Run(tc.name, func(t *testing.T) { + recs := reservationExpiryRecommendations(tc.averages) + cfg := Config{TargetCoverage: tc.target, RebuyWindowDays: 30, IncludeExtendedSupport: true, ExcludeAccounts: tc.exclude} + filtered := applyRegionFilters(recs, engineVersionData{}, "us-east-1", cfg, nil) + got := applyCoverageAndOverrides(filtered, cfg, recommendations.PoolCoverageMap{ + "us-east-1:m5.large": {Pct: 60, AvgInstancesPerHour: tc.demand}, + }, reservationExpiryCommitments(tc.expiry, tc.endDays), nil) + require.Len(t, got, len(tc.wantCounts)) + for i := range got { + assertReservationExpirySizedRow(t, got[i], tc.wantCounts[i], tc.wantAverages[i], tc.existing) + require.NotNil(t, recs[i].RecurringMonthlyCost) + assert.Equal(t, 300.0, *recs[i].RecurringMonthlyCost, "input monthly pointer must remain unmodified") + } + }) + } +} + +func TestReservationExpiryMissingDemand(t *testing.T) { + for _, tc := range []struct { + name string + coverage recommendations.PoolCoverageMap + }{ + {"nil-map", nil}, + {"missing-key", recommendations.PoolCoverageMap{"us-west-2:m5.large": {Pct: 60, AvgInstancesPerHour: 90}}}, + {"zero", recommendations.PoolCoverageMap{"us-east-1:m5.large": {Pct: 60}}}, + {"negative", recommendations.PoolCoverageMap{"us-east-1:m5.large": {Pct: 60, AvgInstancesPerHour: -1}}}, + {"nan", recommendations.PoolCoverageMap{"us-east-1:m5.large": {Pct: 60, AvgInstancesPerHour: math.NaN()}}}, + {"infinite", recommendations.PoolCoverageMap{"us-east-1:m5.large": {Pct: 60, AvgInstancesPerHour: math.Inf(1)}}}, + } { + t.Run(tc.name, func(t *testing.T) { + var output bytes.Buffer + previous := AppLogger.Writer() + AppLogger.SetOutput(&output) + t.Cleanup(func() { AppLogger.SetOutput(previous) }) + cfg := Config{TargetCoverage: 80, RebuyWindowDays: 30} + baseline := applyCoverageAndOverrides(reservationExpiryRecommendations([]float64{10, 10, 10}), cfg, tc.coverage, nil, nil) + output.Reset() + got := applyCoverageAndOverrides(reservationExpiryRecommendations([]float64{10, 10, 10}), cfg, tc.coverage, reservationExpiryCommitments(18, 15), nil) + require.Len(t, got, len(baseline)) + for i := range got { + assert.Equal(t, baseline[i].Count, got[i].Count) + assert.Equal(t, baseline[i].ExistingCoveragePct, got[i].ExistingCoveragePct) + assert.Equal(t, baseline[i].CommitmentCost, got[i].CommitmentCost) + assert.Equal(t, baseline[i].OnDemandCost, got[i].OnDemandCost) + assert.Equal(t, baseline[i].EstimatedSavings, got[i].EstimatedSavings) + assert.Equal(t, baseline[i].RecurringMonthlyCost, got[i].RecurringMonthlyCost) + } + assert.Equal(t, 1, strings.Count(output.String(), "Skipped expiry adjustment for 3 recommendations because pool demand was unavailable; coverage was left unchanged")) + }) + } +} + +func TestReservationExpiryZeroDemandRow(t *testing.T) { + recs := reservationExpiryRecommendations([]float64{30, 0, 60}) + got := applyCoverageAndOverrides(recs, Config{TargetCoverage: 80, RebuyWindowDays: 30}, recommendations.PoolCoverageMap{ + "us-east-1:m5.large": {Pct: 60, AvgInstancesPerHour: 90}, + }, reservationExpiryCommitments(18, 15), nil) + require.Len(t, got, 3) + assertReservationExpirySizedRow(t, got[0], 12, 30, 40) + assertReservationExpirySizedRow(t, got[2], 24, 60, 40) + assert.Equal(t, 30, got[1].Count) + assert.Equal(t, 60.0, got[1].ExistingCoveragePct) + assert.Equal(t, 3000.0, got[1].CommitmentCost) +} + +func reservationExpiryRecommendations(averages []float64) []common.Recommendation { + recs := make([]common.Recommendation, len(averages)) + for i, average := range averages { + monthly := 300.0 + recs[i] = common.Recommendation{ + Provider: common.ProviderAWS, Service: common.ServiceEC2, CommitmentType: common.CommitmentReservedInstance, + ResourceType: "m5.large", Region: "us-east-1", AccountName: fmt.Sprintf("expiry-account-%d", i+1), + Count: 30, RecommendedCount: 30, AverageInstancesUsedPerHour: average, ExistingCoveragePct: 60, + CommitmentCost: 3000, OnDemandCost: 6000, EstimatedSavings: 3000, RecurringMonthlyCost: &monthly, + } + } + return recs +} + +func reservationExpiryCommitments(count, endDays int) []common.Commitment { + return []common.Commitment{{ + Provider: common.ProviderAWS, Service: common.ServiceEC2, CommitmentType: common.CommitmentReservedInstance, + ResourceType: "m5.large", Region: "us-east-1", Count: count, State: common.CommitmentStateActive, + StartDate: time.Now().AddDate(-1, 0, 0), EndDate: time.Now().AddDate(0, 0, endDays), + }} +} + +func assertReservationExpirySizedRow(t *testing.T, got common.Recommendation, count int, average, existing float64) { + t.Helper() + assert.Equal(t, count, got.Count) + assert.Equal(t, 30, got.RecommendedCount) + assert.InDelta(t, average, got.AverageInstancesUsedPerHour, 1e-12) + assert.InDelta(t, existing, got.ExistingCoveragePct, 1e-12) + assert.InDelta(t, existing+float64(count)*100/average, got.ProjectedCoverage, 1e-10) + assert.InDelta(t, 100, got.ProjectedUtilization, 1e-10) + assert.InDelta(t, float64(count)*100, got.CommitmentCost, 1e-10) + assert.InDelta(t, float64(count)*200, got.OnDemandCost, 1e-10) + assert.InDelta(t, float64(count)*100, got.EstimatedSavings, 1e-10) + require.NotNil(t, got.RecurringMonthlyCost) + assert.InDelta(t, float64(count)*10, *got.RecurringMonthlyCost, 1e-10) +} diff --git a/go.mod b/go.mod index c973ecf9e..a1d5968b2 100644 --- a/go.mod +++ b/go.mod @@ -82,8 +82,8 @@ 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-20260929105827-b3b4cb5e3d80 - github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261003204812-9962786e0695 + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261004235520-de46f760cdcf + github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261005004231-945a4045d11f github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20260928214714-ce9513612901 github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20260928214714-ce9513612901 github.com/aws/aws-sdk-go-v2/service/organizations v1.45.3 diff --git a/go.sum b/go.sum index 86de8e6cb..897bb4644 100644 --- a/go.sum +++ b/go.sum @@ -78,10 +78,10 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0.54.0/go.mod h1:vB2GH9GAYYJTO3mEn8oYwzEdhlayZIdQz6zdzgUIRvA= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0 h1:s0WlVbf9qpvkh1c/uDAPElam0WrL7fHRIidgZJ7UqZI= 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-20260929105827-b3b4cb5e3d80 h1:wVKlMokfaME/Lw525Qz3F155R3nY4VmuaIyh+6d4sCU= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20260929105827-b3b4cb5e3d80/go.mod h1:ApWBliDXe099f3oDXBz41K/I9v4bHvn1dG/BGoRmHlw= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261003204812-9962786e0695 h1:DfNEBzFS7/MaBZljLGRRRUaarfnXXZyBE/6iozzp9/k= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261003204812-9962786e0695/go.mod h1:d4nsy61/Ptib0SxqMg0ble3yksVm3+QbtUtmGja82PA= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261004235520-de46f760cdcf h1:Q2MH8FZXpD9MdS/ZSExcjF7mTKxeQpoj96cY65ust1k= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261004235520-de46f760cdcf/go.mod h1:ApWBliDXe099f3oDXBz41K/I9v4bHvn1dG/BGoRmHlw= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261005004231-945a4045d11f h1:c3K60juwiY6t25Gr0HWAxajIDsXH4LzDO4e3ENGAsAM= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261005004231-945a4045d11f/go.mod h1:wpW9/TvGFUUOqBEleJL639loh6Ue5aVIcZKdubRnYPQ= github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20260928214714-ce9513612901 h1:iSdHYdmGUjcjtSmgGZxptBzDuLrJ9KR2mh1/x7FNvSY= github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20260928214714-ce9513612901/go.mod h1:zgCL/ozOkcZUDbEC7a2UwW+6MPcLoBlEU2WUZ0TORUs= github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20260928214714-ce9513612901 h1:i6OwXLUheudN3GfwnYXdKuEq8vPdE9qkNJ+r71LRLKA=