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/recommendation_completeness_proxy_test.go b/cmd/recommendation_completeness_proxy_test.go index 55f5e8880..eb2d8b8f5 100644 --- a/cmd/recommendation_completeness_proxy_test.go +++ b/cmd/recommendation_completeness_proxy_test.go @@ -27,16 +27,18 @@ import ( ) type completenessProxy struct { - t *testing.T - mu sync.Mutex - handlers sync.WaitGroup - regions string - details string - service string - requests map[string]int - spRequests []completenessSPRequest - engines []string - cert tls.Certificate + t *testing.T + mu sync.Mutex + handlers sync.WaitGroup + regions string + details string + service string + requests map[string]int + spRequests []completenessSPRequest + engines []string + cert tls.Certificate + expiryCoverage []string + expiryAccounts []string } type completenessSPRequest struct { @@ -60,6 +62,9 @@ func newCompletenessProxy(t *testing.T, dir, regions, details, service string) ( KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, } + if service == "ec2" { + template.DNSNames = append(template.DNSNames, "organizations.us-east-1.amazonaws.com") + } der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) require.NoError(t, err) certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) @@ -79,6 +84,7 @@ func (p *completenessProxy) serveConnect(w http.ResponseWriter, r *http.Request) p.handlers.Add(1) defer p.handlers.Done() allowed := r.Host == "ce.us-east-1.amazonaws.com:443" || r.Host == "ec2.us-east-1.amazonaws.com:443" || r.Host == "rds.us-east-1.amazonaws.com:443" || r.Host == "savingsplans.amazonaws.com:443" + allowed = allowed || (p.service == "ec2" && r.Host == "organizations.us-east-1.amazonaws.com:443") if r.Method != http.MethodConnect || !allowed { p.t.Errorf("unexpected CONNECT %s %s", r.Method, r.Host) http.Error(w, "forbidden", http.StatusForbidden) @@ -133,6 +139,9 @@ func (p *completenessProxy) respond(req *http.Request) *http.Response { if err != nil { p.t.Error(err) } + if p.service == "ec2" { + return p.respondReservationExpiry(req, body) + } op := req.Header.Get("X-Amz-Target") if req.Host == "savingsplans.amazonaws.com" && op == "" { op = strings.TrimPrefix(req.URL.Path, "/") @@ -240,6 +249,10 @@ func (p *completenessProxy) assertRequests(t *testing.T) { p.handlers.Wait() p.mu.Lock() defer p.mu.Unlock() + if p.service == "ec2" { + p.assertReservationExpiryRequests(t) + return + } if p.service != "rds" { p.assertSPRequests(t) return diff --git a/cmd/recommendation_completeness_test.go b/cmd/recommendation_completeness_test.go index f1fa8caba..6ca2ccebb 100644 --- a/cmd/recommendation_completeness_test.go +++ b/cmd/recommendation_completeness_test.go @@ -58,6 +58,7 @@ func runCompletenessScenario(t *testing.T, service, regionMode, details string) "CUDLY_COMPLETENESS_AUDIT=" + filepath.Join(dir, "audit.jsonl"), "CUDLY_COMPLETENESS_REGIONS=" + regionMode, "CUDLY_COMPLETENESS_SERVICE=" + service, + "CUDLY_COMPLETENESS_DETAILS=" + details, "AWS_ACCESS_KEY_ID=synthetic", "AWS_SECRET_ACCESS_KEY=synthetic", "AWS_REGION=us-east-1", "AWS_EC2_METADATA_DISABLED=true", "AWS_MAX_ATTEMPTS=1", "AWS_CONFIG_FILE=" + filepath.Join(dir, "absent-config"), @@ -72,10 +73,13 @@ func runCompletenessScenario(t *testing.T, service, regionMode, details string) for _, failure := range []string{"Could not check", "Failed to query", "request send failed", "certificate", "TLS handshake"} { require.NotContains(t, logs, failure, "unexpected ancillary error") } - if service == "rds" { + switch service { + case "ec2": + assertReservationExpiryCSV(t, output, details, logs) + case "rds": assertCompletenessDiagnostics(t, regionMode, details, stdout.String(), stderr.String()) assertCompletenessCSV(t, output, details, logs) - } else { + default: assertSPCompleteness(t, output, service, details, logs) } } @@ -95,6 +99,9 @@ func runCompletenessCommandChild(t *testing.T) { if os.Getenv("CUDLY_COMPLETENESS_REGIONS") == "explicit" { args = append(args, "--regions", "us-east-1") } + if os.Getenv("CUDLY_COMPLETENESS_SERVICE") == "ec2" { + args = append(args, reservationExpiryArgs(os.Getenv("CUDLY_COMPLETENESS_DETAILS"))...) + } rootCmd.SetArgs(args) require.NoError(t, rootCmd.Execute()) } diff --git a/cmd/reservation_expiry_command_test.go b/cmd/reservation_expiry_command_test.go new file mode 100644 index 000000000..0512eb8ac --- /dev/null +++ b/cmd/reservation_expiry_command_test.go @@ -0,0 +1,289 @@ +package main + +import ( + "bytes" + "encoding/csv" + "encoding/json" + "fmt" + "io" + "math" + "net/http" + "net/url" + "os" + "slices" + "strconv" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +type reservationExpiryScenario struct { + rows, expiry, endDays, count int + average, demand, target, existing float64 + missing, filtered bool +} + +func reservationExpiryCase(name string) reservationExpiryScenario { + s := reservationExpiryScenario{rows: 3, expiry: 18, endDays: 15, count: 12, average: 30, demand: 90, target: 80, existing: 40} + switch name { + case "pool-demand": + case "outside-window": + s.endDays, s.count, s.existing = 180, 6, 60 + 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": + s.count, s.filtered = 18, true + case "exact-boundary", "below-boundary": + s.rows, s.expiry, s.count, s.average, s.demand, s.existing = 2, 2, 4, 15, 30, 160.0/3 + if name == "below-boundary" { + s.target, s.count = math.Nextafter(80, 0), 3 + } + default: + panic("unknown expiry scenario: " + name) + } + return s +} + +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"} { + t.Run(name, func(t *testing.T) { runCompletenessScenario(t, "ec2", "explicit", name) }) + } +} + +func reservationExpiryArgs(name string) []string { + s := reservationExpiryCase(name) + args := []string{"--payment", "partial-upfront", "--target-coverage", strconv.FormatFloat(s.target, 'g', -1, 64), "--rebuy-window-days", "30"} + if s.filtered { + args = append(args, "--exclude-accounts", "expiry-account-3") + } + return args +} + +func assertReservationExpiryCSV(t *testing.T, output, name, logs string) { + t.Helper() + s := reservationExpiryCase(name) + data, err := os.ReadFile(output) + require.NoError(t, err, logs) + rows, err := csv.NewReader(bytes.NewReader(data)).ReadAll() + require.NoError(t, err) + t.Logf("actual synthetic expiry CSV scenario=%s rows=%v", name, rows) + warnings := 0 + if s.missing { + warnings = 1 + require.Contains(t, logs, "3 recommendations because pool demand was unavailable; coverage was left unchanged") + } + require.Equal(t, warnings, strings.Count(logs, "Skipped expiry adjustment for"), logs) + for _, failure := range []string{"Failed to fetch", "incomplete AWS", "Could not fetch", "failed to fetch", "WARNING:", "no signal", "catalog fetch"} { + require.NotContains(t, logs, failure) + } + count := s.rows + avg := s.average + if s.filtered { + count-- + avg = s.demand / float64(count) + } + require.Len(t, rows, count+2, logs) + require.NotContains(t, rows[0], "ExistingCoveragePercentExact") + seen := make(map[string]bool) + for _, row := range rows[1 : len(rows)-1] { + columns := make(map[string]string) + for i, header := range rows[0] { + columns[header] = row[i] + } + account := columns["Account"] + require.False(t, seen[account], "duplicate account %s", account) + seen[account] = true + index := slices.Index([]string{"111111111111", "222222222222", "333333333333"}[:count], account) + require.NotEqual(t, -1, index, "unexpected account %s", account) + want := map[string]string{ + "Service": "ec2", "Region": "us-east-1", "ResourceType": "m5.large", "Term": "1yr", "PaymentOption": "partial-upfront", + "Count": strconv.Itoa(s.count), "RecommendedCount": "30", "AccountName": fmt.Sprintf("expiry-account-%d", index+1), + "Instances": fmt.Sprintf("%.1f", avg), "CoveredInstances": fmt.Sprintf("%.1f", avg*s.existing/100), + "ExistingCoverage": fmt.Sprintf("%.1f", s.existing), "ProjectedCoverage": fmt.Sprintf("%.1f", s.existing+float64(s.count)*100/avg), + "UpfrontPayment": fmt.Sprintf("%.2f", float64(s.count)*100), "EstimatedSavings": fmt.Sprintf("%.2f", float64(s.count)*100), + "RecurringMonthlyCost": fmt.Sprintf("%.2f", float64(s.count)*10), "Success": "true", "Error": "", + } + for field, value := range want { + require.Equal(t, value, columns[field], "scenario=%s account=%s field=%s", name, account, field) + } + } + total := make(map[string]string) + for i, header := range rows[0] { + total[header] = rows[len(rows)-1][i] + } + require.Equal(t, "TOTAL", rows[len(rows)-1][0]) + require.Equal(t, strconv.Itoa(count*s.count), total["Count"]) + for _, field := range []string{"UpfrontPayment", "EstimatedSavings"} { + require.Equal(t, fmt.Sprintf("%.2f", float64(count*s.count)*100), total[field]) + } + require.Equal(t, fmt.Sprintf("%.2f", float64(count*s.count)*10), total["RecurringMonthlyCost"]) +} + +func (p *completenessProxy) respondReservationExpiry(req *http.Request, body []byte) *http.Response { + p.mu.Lock() + defer p.mu.Unlock() + op := req.Header.Get("X-Amz-Target") + values, err := url.ParseQuery(string(body)) + if op == "" { + require.NoError(p.t, err) + op = values.Get("Action") + } + p.requests[req.Host+"/"+op]++ + if req.Method != http.MethodPost || req.URL.Path != "/" || req.URL.RawQuery != "" { + p.t.Errorf("unexpected expiry request %s %s", req.Method, req.URL) + } + status, contentType, payload := p.reservationExpiryResponse(req.Host, op, body, values) + return &http.Response{StatusCode: status, ProtoMajor: 1, ProtoMinor: 1, + Header: http.Header{"Content-Type": []string{contentType}}, + Body: io.NopCloser(strings.NewReader(payload)), ContentLength: int64(len(payload))} +} + +func (p *completenessProxy) reservationExpiryResponse(host, op string, body []byte, values url.Values) (int, string, string) { + s := reservationExpiryCase(p.details) + if host == "ce.us-east-1.amazonaws.com" { + switch op { + case "AWSInsightsIndexService.GetReservationPurchaseRecommendation": + var request map[string]any + require.NoError(p.t, json.Unmarshal(body, &request)) + require.Equal(p.t, map[string]any{"Service": "Amazon Elastic Compute Cloud - Compute", "AccountScope": "LINKED", "TermInYears": "ONE_YEAR", "PaymentOption": "PARTIAL_UPFRONT", "LookbackPeriodInDays": "SEVEN_DAYS"}, request) + var details []map[string]any + for i := 0; i < s.rows; i++ { + details = append(details, map[string]any{ + "AccountId": []string{"111111111111", "222222222222", "333333333333"}[i], + "RecommendedNumberOfInstancesToPurchase": "30", "AverageNumberOfInstancesUsedPerHour": strconv.FormatFloat(s.average, 'g', -1, 64), + "UpfrontCost": "3000", "EstimatedMonthlyOnDemandCost": "6000", "EstimatedMonthlySavingsAmount": "3000", "RecurringStandardMonthlyCost": "300", + "InstanceDetails": map[string]any{"EC2InstanceDetails": map[string]string{"InstanceType": "m5.large", "Region": "us-east-1", "Platform": "Linux/UNIX", "Tenancy": "Shared"}}, + }) + } + encoded, err := json.Marshal(map[string]any{"Recommendations": []any{map[string]any{"RecommendationDetails": details}}}) + require.NoError(p.t, err) + return 200, "application/x-amz-json-1.1", string(encoded) + case "AWSInsightsIndexService.GetReservationCoverage": + return 200, "application/x-amz-json-1.1", p.reservationExpiryCoverage(body) + } + } + if host == "organizations.us-east-1.amazonaws.com" && op == "AWSOrganizationsV20161128.DescribeAccount" { + var request map[string]string + require.NoError(p.t, json.Unmarshal(body, &request)) + index := slices.Index([]string{"111111111111", "222222222222", "333333333333"}[:s.rows], request["AccountId"]) + require.NotEqual(p.t, -1, index) + require.Len(p.t, request, 1) + p.expiryAccounts = append(p.expiryAccounts, request["AccountId"]) + return 200, "application/x-amz-json-1.1", fmt.Sprintf(`{"Account":{"Id":%q,"Name":%q}}`, request["AccountId"], fmt.Sprintf("expiry-account-%d", index+1)) + } + if host == "ec2.us-east-1.amazonaws.com" { + want := url.Values{"Action": {op}, "Version": {"2016-11-15"}} + if op == "DescribeReservedInstances" { + want.Set("Filter.1.Name", "state") + for i, state := range []string{"active", "payment-pending", "queued"} { + want.Set(fmt.Sprintf("Filter.1.Value.%d", i+1), state) + } + } + require.Equal(p.t, want, values) + switch op { + case "DescribeRegions": + return 200, "text/xml", `us-east-1` + 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)) + } + } + if host == "rds.us-east-1.amazonaws.com" && (op == "DescribeDBInstances" || op == "DescribeDBMajorEngineVersions") { + want := url.Values{"Action": {op}, "Version": {"2014-10-31"}} + element := "DBInstances" + if op == "DescribeDBMajorEngineVersions" { + engine := values.Get("Engine") + want.Set("Engine", engine) + p.engines = append(p.engines, engine) + element = "DBMajorEngineVersions" + } + require.Equal(p.t, want, values) + return 200, "text/xml", fmt.Sprintf(`<%sResponse xmlns="http://rds.amazonaws.com/doc/2014-10-31/"><%sResult><%s/>`, op, op, element, op, op) + } + p.t.Errorf("unexpected expiry operation %s %s", host, op) + return 403, "text/plain", "fixture rejected operation" +} + +func (p *completenessProxy) reservationExpiryCoverage(body []byte) string { + var request struct { + TimePeriod map[string]string + GroupBy []map[string]string + Filter struct { + And []struct { + Dimensions struct { + Key string + Values []string + } + } + } + Metrics []string + NextPageToken string + } + require.NoError(p.t, json.Unmarshal(body, &request)) + require.Empty(p.t, request.NextPageToken) + require.Equal(p.t, []string{"Hour"}, request.Metrics) + start, err := time.Parse(time.DateOnly, request.TimePeriod["Start"]) + require.NoError(p.t, err) + end, err := time.Parse(time.DateOnly, request.TimePeriod["End"]) + require.NoError(p.t, err) + require.Equal(p.t, 30*24*time.Hour, end.Sub(start)) + filters := make(map[string][]string) + for _, expression := range request.Filter.And { + require.NotContains(p.t, filters, expression.Dimensions.Key) + filters[expression.Dimensions.Key] = expression.Dimensions.Values + } + require.Equal(p.t, []string{"us-east-1"}, filters["REGION"]) + require.Len(p.t, filters["SERVICE"], 1) + service := filters["SERVICE"][0] + key := service + groups := []map[string]string{{"Type": "DIMENSION", "Key": "INSTANCE_TYPE"}} + if service == "Amazon Relational Database Service" { + require.Len(p.t, filters, 3) + require.Len(p.t, filters["DATABASE_ENGINE"], 1) + key += "/" + filters["DATABASE_ENGINE"][0] + groups = append(groups, map[string]string{"Type": "DIMENSION", "Key": "DEPLOYMENT_OPTION"}) + } else { + require.Len(p.t, filters, 2) + } + require.Equal(p.t, groups, request.GroupBy) + p.expiryCoverage = append(p.expiryCoverage, key) + if service != "Amazon Elastic Compute Cloud - Compute" { + return `{"CoveragesByTime":[]}` + } + s := reservationExpiryCase(p.details) + hours := map[string]string{"CoverageHoursPercentage": "60"} + if !s.missing { + hours["TotalRunningHours"] = strconv.FormatFloat(s.demand*720, 'g', -1, 64) + } + payload, err := json.Marshal(map[string]any{"CoveragesByTime": []any{map[string]any{"Groups": []any{map[string]any{ + "Attributes": map[string]string{"instanceType": "m5.large"}, "Coverage": map[string]any{"CoverageHours": hours}, + }}}}}) + require.NoError(p.t, err) + return string(payload) +} + +func (p *completenessProxy) assertReservationExpiryRequests(t *testing.T) { + t.Helper() + s := reservationExpiryCase(p.details) + want := map[string]int{ + "ce.us-east-1.amazonaws.com/AWSInsightsIndexService.GetReservationPurchaseRecommendation": 1, + "ce.us-east-1.amazonaws.com/AWSInsightsIndexService.GetReservationCoverage": 12, + "organizations.us-east-1.amazonaws.com/AWSOrganizationsV20161128.DescribeAccount": s.rows, + "ec2.us-east-1.amazonaws.com/DescribeRegions": 1, "ec2.us-east-1.amazonaws.com/DescribeInstanceTypes": 1, + "ec2.us-east-1.amazonaws.com/DescribeReservedInstances": 2, + "rds.us-east-1.amazonaws.com/DescribeDBInstances": 1, "rds.us-east-1.amazonaws.com/DescribeDBMajorEngineVersions": 4, + } + require.Equal(t, want, p.requests, "no other operation, including purchases, is permitted") + wantCoverage := make([]string, 0, 12) + wantCoverage = append(wantCoverage, "Amazon Elastic Compute Cloud - Compute", "Amazon ElastiCache", "Amazon OpenSearch Service", "Amazon Redshift", "Amazon MemoryDB") + for _, engine := range []string{"MySQL", "PostgreSQL", "MariaDB", "Oracle", "SQL Server", "Aurora MySQL", "Aurora PostgreSQL"} { + wantCoverage = append(wantCoverage, "Amazon Relational Database Service/"+engine) + } + require.ElementsMatch(t, wantCoverage, p.expiryCoverage) + require.ElementsMatch(t, []string{"111111111111", "222222222222", "333333333333"}[:s.rows], p.expiryAccounts) + require.ElementsMatch(t, []string{"mysql", "postgres", "aurora-mysql", "aurora-postgresql"}, p.engines) + t.Logf("actual root command, SDK and CSV; synthetic expiry operations=%v coverage=%v accounts=%v", p.requests, p.expiryCoverage, p.expiryAccounts) +} 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=