From ec3112a0da890c9e41c1be017ccb936b131df7ab Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 12:18:35 +0200 Subject: [PATCH 1/5] refactor(common): move the EC2 normalization-factor table The table lived in pkg/exchange; the duplicate guard in pkg/recfilter needs it for EC2 size flexibility (#323), as it needs the RDS table from pkg/common/rds_size.go. Export it as common.EC2NormalizationFactor and have pkg/exchange use it. Metal stays absent (#552). Co-Authored-By: Claude Opus 5.5 --- pkg/common/ec2_size.go | 33 +++++++++++++++++++++++++++++++ pkg/common/ec2_size_test.go | 12 ++++++++++++ pkg/exchange/reshape.go | 38 +++++++----------------------------- pkg/exchange/reshape_test.go | 3 ++- 4 files changed, 54 insertions(+), 32 deletions(-) create mode 100644 pkg/common/ec2_size.go create mode 100644 pkg/common/ec2_size_test.go diff --git a/pkg/common/ec2_size.go b/pkg/common/ec2_size.go new file mode 100644 index 00000000..14ae2392 --- /dev/null +++ b/pkg/common/ec2_size.go @@ -0,0 +1,33 @@ +package common //nolint:revive // pkg/common is the module's core domain package, imported by name (common.X) throughout this repo and by consumer-mcp + +// ec2NormalizationFactors maps EC2 instance sizes to their AWS normalization factors. +// "metal" is deliberately absent: its factor equals the family's largest size +// (i3.metal 128, m5.metal 192, m5zn.metal 96), and some families have no metal +// size, so it cannot be a family-independent constant. Callers that know it set +// RIInfo.NormalizationFactor. +// See: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ri-modification-instancemove.html +var ec2NormalizationFactors = map[string]float64{ + "nano": 0.25, + "micro": 0.5, + "small": 1, + "medium": 2, + "large": 4, + "xlarge": 8, + "2xlarge": 16, + "3xlarge": 24, + "4xlarge": 32, + "6xlarge": 48, + "8xlarge": 64, + "9xlarge": 72, + "10xlarge": 80, + "12xlarge": 96, + "16xlarge": 128, + "18xlarge": 144, + "24xlarge": 192, +} + +// EC2NormalizationFactor returns the AWS normalization factor of an EC2 instance size such +// as "xlarge" (8). It returns 0 when the size is not recognized, including "metal". +func EC2NormalizationFactor(size string) float64 { + return ec2NormalizationFactors[size] +} diff --git a/pkg/common/ec2_size_test.go b/pkg/common/ec2_size_test.go new file mode 100644 index 00000000..99c61039 --- /dev/null +++ b/pkg/common/ec2_size_test.go @@ -0,0 +1,12 @@ +package common //nolint:revive // see ec2_size.go + +import "testing" + +func TestEC2NormalizationFactor(t *testing.T) { + t.Parallel() + for size, want := range map[string]float64{"nano": 0.25, "large": 4, "xlarge": 8, "24xlarge": 192, "metal": 0, "bogus": 0, "": 0} { + if got := EC2NormalizationFactor(size); got != want { + t.Errorf("EC2NormalizationFactor(%q) = %v, want %v", size, got, want) + } + } +} diff --git a/pkg/exchange/reshape.go b/pkg/exchange/reshape.go index fc73d645..e0208425 100644 --- a/pkg/exchange/reshape.go +++ b/pkg/exchange/reshape.go @@ -6,6 +6,8 @@ import ( "math" "sort" "strings" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" ) // RIInfo describes a Reserved Instance for reshape analysis. @@ -360,7 +362,7 @@ func nfProximityComponent(off OfferingOption, src RIInfo) float64 { srcNF := src.NormalizationFactor if srcNF <= 0 { _, size := parseInstanceType(src.InstanceType) - srcNF = normalizationFactors[size] + srcNF = common.EC2NormalizationFactor(size) } if srcNF <= 0 || off.NormalizationFactor <= 0 { return 0.5 @@ -373,32 +375,6 @@ func nfProximityComponent(off OfferingOption, src RIInfo) float64 { return 1.0 - deviation } -// normalizationFactors maps EC2 instance sizes to their AWS normalization factors. -// "metal" is deliberately absent: its factor equals the family's largest size -// (i3.metal 128, m5.metal 192, m5zn.metal 96), and some families have no metal -// size, so it cannot be a family-independent constant. Callers that know it set -// RIInfo.NormalizationFactor. -// See: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ri-modification-instancemove.html -var normalizationFactors = map[string]float64{ - "nano": 0.25, - "micro": 0.5, - "small": 1, - "medium": 2, - "large": 4, - "xlarge": 8, - "2xlarge": 16, - "3xlarge": 24, - "4xlarge": 32, - "6xlarge": 48, - "8xlarge": 64, - "9xlarge": 72, - "10xlarge": 80, - "12xlarge": 96, - "16xlarge": 128, - "18xlarge": 144, - "24xlarge": 192, -} - // sizeOrder lists instance sizes from smallest to largest for iteration. var sizeOrder = []string{ "nano", "micro", "small", "medium", "large", "xlarge", @@ -420,7 +396,7 @@ func parseInstanceType(instanceType string) (family, size string) { // NormalizationFactorForSize returns the normalization factor for a given instance size. // Returns 0 if the size is unknown, including "metal" (family-dependent). func NormalizationFactorForSize(size string) float64 { - return normalizationFactors[size] + return common.EC2NormalizationFactor(size) } // AnalyzeReshaping identifies underutilized convertible RIs and suggests optimal @@ -453,7 +429,7 @@ func resolveNormFactor(ri RIInfo, size string) float64 { if ri.NormalizationFactor != 0 { return ri.NormalizationFactor } - return normalizationFactors[size] + return common.EC2NormalizationFactor(size) } // analyzeRI evaluates a single RI and returns a reshape recommendation if it is @@ -730,7 +706,7 @@ func findBestFit(normalizedUsed, normalizedPurchased float64) (size string, coun for i := len(sizeOrder) - 1; i >= 0; i-- { s := sizeOrder[i] - nf := normalizationFactors[s] + nf := common.EC2NormalizationFactor(s) if nf <= 0 || nf > normalizedUsed { continue } @@ -743,7 +719,7 @@ func findBestFit(normalizedUsed, normalizedPurchased float64) (size string, coun // Only usage below the smallest size's factor falls back to that size; // anything else that reached here simply does not fit. s := sizeOrder[0] - nf := normalizationFactors[s] + nf := common.EC2NormalizationFactor(s) if normalizedUsed < nf && nf <= normalizedPurchased { return s, 1 } diff --git a/pkg/exchange/reshape_test.go b/pkg/exchange/reshape_test.go index 95267351..d2352e40 100644 --- a/pkg/exchange/reshape_test.go +++ b/pkg/exchange/reshape_test.go @@ -4,6 +4,7 @@ import ( "math" "testing" + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/stretchr/testify/assert" ) @@ -209,7 +210,7 @@ func TestAnalyzeReshaping_TargetNeverExceedsOwnedCapacity(t *testing.T) { continue } _, tsz := parseInstanceType(rec.TargetInstanceType) - target := normalizationFactors[tsz] * float64(rec.TargetCount) + target := common.EC2NormalizationFactor(tsz) * float64(rec.TargetCount) assert.LessOrEqualf(t, target, rec.NormalizedPurchased, "%dx %s @%.0f%% -> %dx %s", count, ri.InstanceType, util, rec.TargetCount, rec.TargetInstanceType) } From f19b6292c9064a7af33caf028f12bdd811fecaf1 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 12:20:34 +0200 Subject: [PATCH 2/5] fix(recfilter): size-flexible EC2 reservations in the duplicate guard The EC2 dedupe keyed on the exact instance type, so a recent regional Linux/Unix default-tenancy m5.xlarge reservation did not suppress an m5.large recommendation and the guard could buy a duplicate (#323). Such entries now also join a family and region budget in normalized units (common.EC2NormalizationFactor), spent by recommendations that are exactly Linux/Unix and default tenancy. Zonal, other platforms, dedicated tenancy, metal and unknown sizes stay exact-type. Wildcard entries still suppress their exact type but never join the flex budget, and wildcard recommendations never spend it. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 ++ pkg/recfilter/dedupe_ec2.go | 21 ++++++-- pkg/recfilter/dedupe_ec2_flex.go | 58 ++++++++++++++++++++++ pkg/recfilter/dedupe_ec2_flex_test.go | 69 +++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 3 deletions(-) create mode 100644 pkg/recfilter/dedupe_ec2_flex.go create mode 100644 pkg/recfilter/dedupe_ec2_flex_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index a52044c6..e18812e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -199,6 +199,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Fixed - **aws/ec2:** `GetOfferingDetails` always priced the convertible offering and its doc comment wrongly said the class does not change prices. Added `GetOfferingDetailsForClass` to price the class the purchase uses; `GetOfferingDetails` keeps the convertible default (#570). +- **The duplicate-purchase guard now treats regional Linux/Unix default-tenancy + EC2 reservations as size-flexible** (#323): a recent m5.xlarge reservation + suppresses two m5.large recommendations in the same region, counted in + normalized units. Zonal, Windows/RHEL/SUSE, dedicated, wildcard and + unknown-size entries stay exact-type. - **Unbounded AWS pagination loops now stop at a page cap and honour cancellation** (#561): the existing-commitment and valid-resource-type scans for ElastiCache, RDS, OpenSearch, Redshift and EC2 check the context before diff --git a/pkg/recfilter/dedupe_ec2.go b/pkg/recfilter/dedupe_ec2.go index f386be43..9b780aa9 100644 --- a/pkg/recfilter/dedupe_ec2.go +++ b/pkg/recfilter/dedupe_ec2.go @@ -56,6 +56,13 @@ type ec2Entry struct { platform, tenancy string scope scopeState count, wildcards int + // flexUnits is the quarter normalized units of one node when the entry is a + // regional Linux/Unix default-tenancy reservation of a known size, which pays + // for any size of its family (see ec2FlexUnits); 0 keeps the entry exact-type. + flexUnits int + // flexLeft is the unspent budget in the same units. count stays the node count + // the exact-type view sees: flexLeft rounded up to whole nodes. + flexLeft int } func (e *ec2Entry) covers(platform, tenancy string, scope scopeState) bool { @@ -69,8 +76,10 @@ func (e *ec2Entry) covers(platform, tenancy string, scope scopeState) bool { // still suppresses, as for every other service. type ec2Dedupe struct { entries map[string][]*ec2Entry - warned map[string]bool - logf Logf + // families lists the size-flexible entries by family and region, in input order. + families map[string][]*ec2Entry + warned map[string]bool + logf Logf } func isAWSCompute(p common.ProviderType, s common.ServiceType) bool { @@ -82,7 +91,7 @@ func ec2BaseKey(resourceType, region string) string { } func newEC2Dedupe(commitments []common.Commitment, logf Logf) *ec2Dedupe { - d := &ec2Dedupe{entries: map[string][]*ec2Entry{}, warned: map[string]bool{}, logf: logf} + d := &ec2Dedupe{entries: map[string][]*ec2Entry{}, families: map[string][]*ec2Entry{}, warned: map[string]bool{}, logf: logf} for i := range commitments { c := &commitments[i] if !isAWSCompute(c.Provider, c.Service) { @@ -93,6 +102,10 @@ func newEC2Dedupe(commitments []common.Commitment, logf Logf) *ec2Dedupe { e.wildcards = wildcardCount(e) key := ec2BaseKey(c.ResourceType, c.Region) d.entries[key] = append(d.entries[key], e) + if family, units, ok := ec2FlexKey(c.ResourceType, c.Region, platform, tenancy); ok && scope.kind == scopeRegionalKind { + e.flexUnits, e.flexLeft = units, max(e.count, 0)*units + d.families[family] = append(d.families[family], e) + } } for _, list := range d.entries { sort.SliceStable(list, func(i, j int) bool { return list[i].wildcards < list[j].wildcards }) @@ -179,8 +192,10 @@ func (d *ec2Dedupe) adjust(rec common.Recommendation) (common.Recommendation, er } used := min(e.count, remaining) e.count -= used + e.flexLeft = max(e.flexLeft-used*e.flexUnits, 0) remaining -= used } + remaining = d.consumeFamily(rec, platform, tenancy, remaining) if remaining <= 0 { d.logf.printf(" [DuplicateChecker] SKIP %s: recent commitments cover recommended %d", ec2BaseKey(rec.ResourceType, rec.Region), rec.Count) return common.Recommendation{Count: 0}, nil diff --git a/pkg/recfilter/dedupe_ec2_flex.go b/pkg/recfilter/dedupe_ec2_flex.go new file mode 100644 index 00000000..95cd6d41 --- /dev/null +++ b/pkg/recfilter/dedupe_ec2_flex.go @@ -0,0 +1,58 @@ +package recfilter + +import ( + "fmt" + "strings" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" +) + +// ec2FlexKey reports the family key and the quarter normalized units of one node for an +// EC2 instance type whose platform and tenancy are exactly Linux/Unix and default. A +// regional reservation of that kind pays for any size of the same family in its region +// (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/apply_ri.html), so the family +// replaces the exact type in the key and the budget is counted in units (quarters of a +// normalized unit, so nano stays an integer). Anything else returns false and keeps the +// exact-type key: other platforms and dedicated tenancy are not size-flexible, and an +// empty (wildcard) platform or tenancy may not be, so it must never join or spend a +// flex budget. Callers also require a regional scope for entries: zonal +// reservations are not size-flexible. Sizes without a known factor, such as metal, stay exact. +func ec2FlexKey(resourceType, region, platform, tenancy string) (family string, units int, ok bool) { + if platform != "linux/unix" || tenancy != "default" { + return "", 0, false + } + prefix, size, found := strings.Cut(resourceType, ".") + nf := common.EC2NormalizationFactor(size) + if !found || prefix == "" || nf == 0 { + return "", 0, false + } + return fmt.Sprintf("ec2flex|%s|%s", strings.ToLower(prefix), region), int(nf * 4), true +} + +// consumeFamily spends the size-flexible budget of the recommendation's family on what is +// still uncovered and returns the count left. The budget is the unspent units of every regional +// Linux/Unix default-tenancy entry of the family, spent in input order. The covered nodes +// round down and the spent units are exact, so a fractional remainder is never credited. +func (d *ec2Dedupe) consumeFamily(rec common.Recommendation, platform, tenancy string, remaining int) int { + family, recUnits, ok := ec2FlexKey(rec.ResourceType, rec.Region, platform, tenancy) + if !ok || remaining <= 0 { + return remaining + } + entries := d.families[family] + budget := 0 + for _, e := range entries { + budget += e.flexLeft + } + covered := min(budget/recUnits, remaining) + need := covered * recUnits + for _, e := range entries { + if need <= 0 { + break + } + take := min(e.flexLeft, need) + e.flexLeft -= take + e.count = (e.flexLeft + e.flexUnits - 1) / e.flexUnits + need -= take + } + return remaining - covered +} diff --git a/pkg/recfilter/dedupe_ec2_flex_test.go b/pkg/recfilter/dedupe_ec2_flex_test.go new file mode 100644 index 00000000..dd599faa --- /dev/null +++ b/pkg/recfilter/dedupe_ec2_flex_test.go @@ -0,0 +1,69 @@ +package recfilter + +import ( + "testing" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func flexCommitment(instanceType, platform, tenancy, scope, az string, count int) common.Commitment { + c := ec2Commitment(platform, tenancy, scope, az, count) + c.ResourceType = instanceType + return c +} + +func flexRec(instanceType, platform, tenancy string, count int) common.Recommendation { + r := ec2Rec(platform, tenancy, "regional", "", count) + r.ResourceType = instanceType + r.Details.(*common.ComputeDetails).InstanceType = instanceType + return r +} + +func TestEC2SizeFlexDedupe(t *testing.T) { + t.Parallel() + tests := []struct { + name string + commitment common.Commitment + rec common.Recommendation + want int + }{ + {"regional linux default xlarge covers two larges", flexCommitment("m5.xlarge", "linux", "default", "regional", "", 1), flexRec("m5.large", "linux", "default", 2), 0}, + {"covers one of three larges", flexCommitment("m5.xlarge", "linux", "default", "regional", "", 1), flexRec("m5.large", "linux", "default", 3), 1}, + {"small RI does not cover bigger rec", flexCommitment("m5.large", "linux", "default", "regional", "", 1), flexRec("m5.xlarge", "linux", "default", 1), 1}, + {"two larges cover an xlarge", flexCommitment("m5.large", "linux", "default", "regional", "", 2), flexRec("m5.xlarge", "linux", "default", 1), 0}, + {"other family", flexCommitment("m5.xlarge", "linux", "default", "regional", "", 1), flexRec("c5.large", "linux", "default", 1), 1}, + {"zonal RI is exact", flexCommitment("m5.xlarge", "linux", "default", "zonal", "us-east-1a", 1), flexRec("m5.large", "linux", "default", 2), 2}, + {"windows RI is exact", flexCommitment("m5.xlarge", "windows", "default", "regional", "", 1), flexRec("m5.large", "windows", "default", 2), 2}, + {"dedicated RI is exact", flexCommitment("m5.xlarge", "linux", "dedicated", "regional", "", 1), flexRec("m5.large", "linux", "dedicated", 2), 2}, + {"unknown size is exact", flexCommitment("m5.metal", "linux", "default", "regional", "", 1), flexRec("m5.large", "linux", "default", 1), 1}, + {"unknown size same type still dedupes", flexCommitment("m5.metal", "linux", "default", "regional", "", 1), flexRec("m5.metal", "linux", "default", 1), 0}, + {"unknown rec size spends no flex budget", flexCommitment("m5.xlarge", "linux", "default", "regional", "", 1), flexRec("m5.metal", "linux", "default", 1), 1}, + {"wildcard platform RI is exact", flexCommitment("m5.xlarge", "", "default", "regional", "", 1), flexRec("m5.large", "linux", "default", 2), 2}, + {"wildcard platform RI still suppresses its type", flexCommitment("m5.xlarge", "", "default", "regional", "", 1), flexRec("m5.xlarge", "linux", "default", 1), 0}, + {"wildcard scope RI is exact", flexCommitment("m5.xlarge", "linux", "default", "", "", 1), flexRec("m5.large", "linux", "default", 2), 2}, + {"wildcard rec platform spends no flex budget", flexCommitment("m5.xlarge", "linux", "default", "regional", "", 1), flexRec("m5.large", "", "default", 2), 2}, + {"wildcard rec tenancy spends no flex budget", flexCommitment("m5.xlarge", "linux", "default", "regional", "", 1), flexRec("m5.large", "linux", "", 2), 2}, + {"windows rec not covered by linux flex", flexCommitment("m5.xlarge", "linux", "default", "regional", "", 1), flexRec("m5.large", "windows", "default", 2), 2}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + passed := runEC2(t, nil, []common.Commitment{tt.commitment}, tt.rec) + if tt.want == 0 { + assert.Empty(t, passed) + return + } + require.Len(t, passed, 1) + assert.Equal(t, tt.want, passed[0].Count) + }) + } +} + +func TestEC2SizeFlexBudgetSharedAcrossRecs(t *testing.T) { + t.Parallel() + passed := runEC2(t, nil, []common.Commitment{flexCommitment("m5.xlarge", "linux", "default", "regional", "", 1)}, + flexRec("m5.large", "linux", "default", 1), flexRec("m5.large", "linux", "default", 1), flexRec("m5.large", "linux", "default", 1)) + assert.Len(t, passed, 1) +} From c7d7347a7dc436aa2afe407a40301b672aeb2b12 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 12:21:35 +0200 Subject: [PATCH 3/5] fix(recfilter): count Multi-AZ RDS reservations as two units dedupeKey includes the deployment, so a Multi-AZ size-flexible RDS reservation never counted against Single-AZ demand of its family and the guard could buy a duplicate (#323). For size-flexible engines a Multi-AZ node now counts as twice the size units on both sides, and a Single-AZ recommendation also spends the Multi-AZ budget of its family after the Single-AZ one. Single-AZ still does not cover Multi-AZ, and exact-class engines keep their key. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 +++++ pkg/recfilter/dedupe.go | 27 ++++++++++++++++++++------- pkg/recfilter/dedupe_rds.go | 16 ++++++++++++++++ pkg/recfilter/dedupe_rds_test.go | 7 +++++++ 4 files changed, 48 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e18812e1..8669df76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -199,6 +199,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Fixed - **aws/ec2:** `GetOfferingDetails` always priced the convertible offering and its doc comment wrongly said the class does not change prices. Added `GetOfferingDetailsForClass` to price the class the purchase uses; `GetOfferingDetails` keeps the convertible default (#570). +- **A Multi-AZ size-flexible RDS reservation now covers Single-AZ demand in the + duplicate-purchase guard** (#323): it counts as twice the size units of its + family, so a recent Multi-AZ db.r6g.large PostgreSQL reservation suppresses + two Single-AZ db.r6g.large recommendations. Single-AZ never covers Multi-AZ, + and exact-class engines such as SQL Server keep their key. - **The duplicate-purchase guard now treats regional Linux/Unix default-tenancy EC2 reservations as size-flexible** (#323): a recent m5.xlarge reservation suppresses two m5.large recommendations in the same region, counted in diff --git a/pkg/recfilter/dedupe.go b/pkg/recfilter/dedupe.go index ec0ae5b9..04e57528 100644 --- a/pkg/recfilter/dedupe.go +++ b/pkg/recfilter/dedupe.go @@ -217,7 +217,7 @@ func buildExistingCommitmentsMap(commitments []common.Commitment, logf Logf) map normalizedDeployment := common.NormalizeDeploymentName(c.Deployment) resourceType, nodeUnits := c.ResourceType, 1 if family, units, ok := rdsFlexKey(c.Provider, c.Service, normalizedEngine, c.ResourceType); ok { - resourceType, nodeUnits = family, units + resourceType, nodeUnits = family, units*rdsDeploymentFactor(normalizedDeployment) } key := dedupeKey(resourceType, c.Region, normalizedEngine, normalizedDeployment) existingMap[key] += c.Count * nodeUnits @@ -256,6 +256,17 @@ func adjustRecommendationsAgainstExisting(recs []common.Recommendation, existing return passed, filtered, nil } +// spendBudget takes up to want nodes from the budget under key and returns how many it covered. +func spendBudget(existingMap map[string]int, key string, cover elastiCacheCover, want int) int { + available := existingMap[key] + if available <= 0 || want <= 0 { + return 0 + } + covered, spent := cover.consume(available, want) + existingMap[key] -= spent + return covered +} + // adjustSingleRecommendation adjusts a single recommendation based on existing // commitments. A reduced count scales the money fields with it. func adjustSingleRecommendation(rec common.Recommendation, existingMap map[string]int, logf Logf) (common.Recommendation, error) { @@ -266,9 +277,12 @@ func adjustSingleRecommendation(rec common.Recommendation, existingMap map[strin deployment := common.NormalizeDeploymentName(common.DeploymentFromDetails(rec.Details)) resourceType := rec.ResourceType covers := []elastiCacheCover{{engine, 1, 1}} + flexUnits := 0 if family, units, ok := rdsFlexKey(rec.Provider, rec.Service, engine, rec.ResourceType); ok { // A size-flexible budget holds units: one node of this size spends units of them. + units *= rdsDeploymentFactor(deployment) resourceType, covers = family, []elastiCacheCover{{engine, 1, units}} + flexUnits = units } key := dedupeKey(resourceType, rec.Region, engine, deployment) if isElastiCache { @@ -277,12 +291,11 @@ func adjustSingleRecommendation(rec common.Recommendation, existingMap map[strin } remaining := rec.Count for _, cover := range covers { - candidate := dedupeKey(resourceType, rec.Region, cover.engine, deployment) - if available := existingMap[candidate]; available > 0 { - covered, spent := cover.consume(available, remaining) - remaining -= covered - existingMap[candidate] -= spent - } + remaining -= spendBudget(existingMap, dedupeKey(resourceType, rec.Region, cover.engine, deployment), cover, remaining) + } + if flexUnits > 0 && deployment == rdsSingleAZ { + // A Multi-AZ size-flexible reservation also pays for Single-AZ nodes; the reverse never holds. + remaining -= spendBudget(existingMap, dedupeKey(resourceType, rec.Region, engine, rdsMultiAZ), elastiCacheCover{engine, 1, flexUnits}, remaining) } if remaining <= 0 { diff --git a/pkg/recfilter/dedupe_rds.go b/pkg/recfilter/dedupe_rds.go index f7a9a36c..7bea1bcd 100644 --- a/pkg/recfilter/dedupe_rds.go +++ b/pkg/recfilter/dedupe_rds.go @@ -22,3 +22,19 @@ func rdsFlexKey(providerType common.ProviderType, service common.ServiceType, en } return strings.ToLower(common.RDSFamily(resourceType)) + ".*", int(nu * 4), true } + +// Normalized deployment names (common.NormalizeDeploymentName) of the RDS deployment options. +const ( + rdsSingleAZ = "singleaz" + rdsMultiAZ = "multiaz" +) + +// rdsDeploymentFactor is how many times the size units a node of the deployment option +// counts in a size-flexible budget: a Multi-AZ node runs two instances, so it counts +// double: https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_WorkingWithReservedDBInstances.html#USER_WorkingWithReservedDBInstances.SizeFlexible +func rdsDeploymentFactor(normalizedDeployment string) int { + if normalizedDeployment == rdsMultiAZ { + return 2 + } + return 1 +} diff --git a/pkg/recfilter/dedupe_rds_test.go b/pkg/recfilter/dedupe_rds_test.go index 01a23f71..78dead02 100644 --- a/pkg/recfilter/dedupe_rds_test.go +++ b/pkg/recfilter/dedupe_rds_test.go @@ -36,6 +36,13 @@ func TestRDSSizeFlexDedupe(t *testing.T) { {"other engine", "db.r6g.large", "single-az", 1, common.ServiceRDS, "db.r6g.medium", "postgres", "single-az", "us-east-1", 1, 1, ""}, {"other region", "db.r6g.large", "single-az", 1, common.ServiceRDS, "db.r6g.medium", "mysql", "single-az", "us-west-2", 1, 1, ""}, {"other deployment", "db.r6g.large", "single-az", 1, common.ServiceRDS, "db.r6g.medium", "mysql", "multi-az", "us-east-1", 1, 1, ""}, + {"multi-az large covers two single-az larges", "db.r6g.large", "multi-az", 1, common.ServiceRDS, "db.r6g.large", "postgres", "single-az", "us-east-1", 2, 0, "postgres"}, + {"multi-az large covers one of three single-az larges", "db.r6g.large", "multi-az", 1, common.ServiceRDS, "db.r6g.large", "postgres", "single-az", "us-east-1", 3, 1, "postgres"}, + {"multi-az xlarge covers four single-az larges", "db.r6g.xlarge", "Multi-AZ", 1, common.ServiceRDS, "db.r6g.large", "postgres", "single-az", "us-east-1", 4, 0, "postgres"}, + {"multi-az large covers one multi-az large", "db.r6g.large", "multi-az", 1, common.ServiceRDS, "db.r6g.large", "postgres", "multi-az", "us-east-1", 1, 0, "postgres"}, + {"multi-az large covers one of two multi-az larges", "db.r6g.large", "multi-az", 1, common.ServiceRDS, "db.r6g.large", "postgres", "multi-az", "us-east-1", 2, 1, "postgres"}, + {"single-az never covers multi-az", "db.r6g.xlarge", "single-az", 1, common.ServiceRDS, "db.r6g.large", "postgres", "multi-az", "us-east-1", 1, 1, "postgres"}, + {"multi-az sqlserver stays exact", "db.m6i.large", "multi-az", 1, common.ServiceRDS, "db.m6i.large", "sqlserver-se", "single-az", "us-east-1", 2, 2, "sqlserver-se"}, {"unknown size stays exact", "db.r6g.metal", "single-az", 1, common.ServiceRDS, "db.r6g.medium", "mysql", "single-az", "us-east-1", 1, 1, ""}, {"exact class still matches", "db.r6g.metal", "single-az", 1, common.ServiceRDS, "db.r6g.metal", "mysql", "single-az", "us-east-1", 1, 0, ""}, {"postgres family flex", "db.r6g.large", "single-az", 1, common.ServiceRDS, "db.r6g.medium", "", "single-az", "us-east-1", 2, 0, "postgres"}, From aa29db96e26d7c24c8adae14a125787a6162f18c Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 12:30:17 +0200 Subject: [PATCH 4/5] fix(recfilter): keep EC2 flex budgets exact and skip excluded families A flex entry's leftover units were rounded up to a whole node for the exact-type path, so one xlarge reservation suppressed a large and an xlarge recommendation (12 units from 8). The remaining units are now the only state and the exact path spends whole nodes of floor(units / node). AWS excludes G4ad, G4dn, G5, G5g, G6, G6e, G6f, Gr6, Gr6f, hpc7a, P5, Inf1, Inf2, u7i-6tb and u7i-8tb from size flexibility; those families stay exact-type (common.EC2SizeFlexibleFamily). Co-Authored-By: Claude Opus 5.5 --- pkg/common/ec2_size.go | 20 +++++++++ pkg/common/ec2_size_test.go | 13 ++++++ pkg/recfilter/dedupe_ec2.go | 29 ++++++++++--- pkg/recfilter/dedupe_ec2_flex.go | 6 +-- pkg/recfilter/dedupe_ec2_flex_test.go | 62 +++++++++++++++++++++++++++ 5 files changed, 120 insertions(+), 10 deletions(-) diff --git a/pkg/common/ec2_size.go b/pkg/common/ec2_size.go index 14ae2392..ff5cf815 100644 --- a/pkg/common/ec2_size.go +++ b/pkg/common/ec2_size.go @@ -1,5 +1,7 @@ package common //nolint:revive // pkg/common is the module's core domain package, imported by name (common.X) throughout this repo and by consumer-mcp +import "strings" + // ec2NormalizationFactors maps EC2 instance sizes to their AWS normalization factors. // "metal" is deliberately absent: its factor equals the family's largest size // (i3.metal 128, m5.metal 192, m5zn.metal 96), and some families have no metal @@ -31,3 +33,21 @@ var ec2NormalizationFactors = map[string]float64{ func EC2NormalizationFactor(size string) float64 { return ec2NormalizationFactors[size] } + +// ec2NotSizeFlexibleFamilies are the families whose regional Linux/Unix reservations are +// still not size-flexible ("Instance size flexibility > Limitations"): +// https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/apply_ri.html#ri-instance-size-flexibility-limitations +// Re-read that list when AWS adds a family: a missing entry lets the duplicate guard +// credit a reservation to sizes it does not cover. +var ec2NotSizeFlexibleFamilies = map[string]bool{ + "g4ad": true, "g4dn": true, "g5": true, "g5g": true, "g6": true, "g6e": true, "g6f": true, + "gr6": true, "gr6f": true, "hpc7a": true, "p5": true, "inf1": true, "inf2": true, + "u7i-6tb": true, "u7i-8tb": true, +} + +// EC2SizeFlexibleFamily reports whether a regional Linux/Unix default-tenancy reservation of +// the instance family (the part of the type before the dot, such as "m5" or "u7i-6tb") can be +// size-flexible. It is case-insensitive and false for the families AWS excludes. +func EC2SizeFlexibleFamily(family string) bool { + return !ec2NotSizeFlexibleFamilies[strings.ToLower(strings.TrimSpace(family))] +} diff --git a/pkg/common/ec2_size_test.go b/pkg/common/ec2_size_test.go index 99c61039..131218ac 100644 --- a/pkg/common/ec2_size_test.go +++ b/pkg/common/ec2_size_test.go @@ -10,3 +10,16 @@ func TestEC2NormalizationFactor(t *testing.T) { } } } + +func TestEC2SizeFlexibleFamily(t *testing.T) { + t.Parallel() + for family, want := range map[string]bool{ + "m5": true, "c5n": true, "t3": true, "g5": false, "G5": false, "g4dn": false, "g4ad": false, "g5g": false, + "gr6f": false, "p5": false, "p4d": true, "inf1": false, "inf2": false, "hpc7a": false, "hpc7g": true, + "u7i-6tb": false, "u7i-8tb": false, "u7i-12tb": true, "g6e": false, "g4": true, + } { + if got := EC2SizeFlexibleFamily(family); got != want { + t.Errorf("EC2SizeFlexibleFamily(%q) = %v, want %v", family, got, want) + } + } +} diff --git a/pkg/recfilter/dedupe_ec2.go b/pkg/recfilter/dedupe_ec2.go index 9b780aa9..d2d9b884 100644 --- a/pkg/recfilter/dedupe_ec2.go +++ b/pkg/recfilter/dedupe_ec2.go @@ -60,8 +60,8 @@ type ec2Entry struct { // regional Linux/Unix default-tenancy reservation of a known size, which pays // for any size of its family (see ec2FlexUnits); 0 keeps the entry exact-type. flexUnits int - // flexLeft is the unspent budget in the same units. count stays the node count - // the exact-type view sees: flexLeft rounded up to whole nodes. + // flexLeft is the unspent budget in the same units and the single source of truth + // for a flex entry; count is unused for it. flexLeft int } @@ -113,6 +113,24 @@ func newEC2Dedupe(commitments []common.Commitment, logf Logf) *ec2Dedupe { return d } +// nodesLeft is the whole nodes of the entry's own size still unspent. A flex entry whose +// leftover budget is below one node has none, so a partly spent node is never credited twice. +func (e *ec2Entry) nodesLeft() int { + if e.flexUnits > 0 { + return e.flexLeft / e.flexUnits + } + return e.count +} + +func (e *ec2Entry) spend(nodes int) int { + if e.flexUnits > 0 { + e.flexLeft -= nodes * e.flexUnits + } else { + e.count -= nodes + } + return nodes +} + func wildcardCount(e *ec2Entry) int { n := 0 for _, wild := range []bool{e.platform == "", e.tenancy == "", e.scope.kind == scopeUnknown, @@ -187,13 +205,10 @@ func (d *ec2Dedupe) adjust(rec common.Recommendation) (common.Recommendation, er if remaining <= 0 { break } - if e.count <= 0 || !e.covers(platform, tenancy, scope) { + if e.nodesLeft() <= 0 || !e.covers(platform, tenancy, scope) { continue } - used := min(e.count, remaining) - e.count -= used - e.flexLeft = max(e.flexLeft-used*e.flexUnits, 0) - remaining -= used + remaining -= e.spend(min(e.nodesLeft(), remaining)) } remaining = d.consumeFamily(rec, platform, tenancy, remaining) if remaining <= 0 { diff --git a/pkg/recfilter/dedupe_ec2_flex.go b/pkg/recfilter/dedupe_ec2_flex.go index 95cd6d41..d1a2825e 100644 --- a/pkg/recfilter/dedupe_ec2_flex.go +++ b/pkg/recfilter/dedupe_ec2_flex.go @@ -16,14 +16,15 @@ import ( // exact-type key: other platforms and dedicated tenancy are not size-flexible, and an // empty (wildcard) platform or tenancy may not be, so it must never join or spend a // flex budget. Callers also require a regional scope for entries: zonal -// reservations are not size-flexible. Sizes without a known factor, such as metal, stay exact. +// reservations are not size-flexible. Sizes without a known factor, such as metal, and families AWS excludes +// (common.EC2SizeFlexibleFamily) stay exact. func ec2FlexKey(resourceType, region, platform, tenancy string) (family string, units int, ok bool) { if platform != "linux/unix" || tenancy != "default" { return "", 0, false } prefix, size, found := strings.Cut(resourceType, ".") nf := common.EC2NormalizationFactor(size) - if !found || prefix == "" || nf == 0 { + if !found || prefix == "" || nf == 0 || !common.EC2SizeFlexibleFamily(prefix) { return "", 0, false } return fmt.Sprintf("ec2flex|%s|%s", strings.ToLower(prefix), region), int(nf * 4), true @@ -51,7 +52,6 @@ func (d *ec2Dedupe) consumeFamily(rec common.Recommendation, platform, tenancy s } take := min(e.flexLeft, need) e.flexLeft -= take - e.count = (e.flexLeft + e.flexUnits - 1) / e.flexUnits need -= take } return remaining - covered diff --git a/pkg/recfilter/dedupe_ec2_flex_test.go b/pkg/recfilter/dedupe_ec2_flex_test.go index dd599faa..0f5dd8d8 100644 --- a/pkg/recfilter/dedupe_ec2_flex_test.go +++ b/pkg/recfilter/dedupe_ec2_flex_test.go @@ -1,6 +1,8 @@ package recfilter import ( + "math/rand" + "strings" "testing" "github.com/LeanerCloud/cloud-commitments-go/pkg/common" @@ -67,3 +69,63 @@ func TestEC2SizeFlexBudgetSharedAcrossRecs(t *testing.T) { flexRec("m5.large", "linux", "default", 1), flexRec("m5.large", "linux", "default", 1), flexRec("m5.large", "linux", "default", 1)) assert.Len(t, passed, 1) } + +func TestEC2SizeFlexPartialNodeNotCreditedTwice(t *testing.T) { + t.Parallel() + ri := []common.Commitment{flexCommitment("m5.xlarge", "linux", "default", "regional", "", 1)} + // 8 units owned: a large (4) leaves 4, which is below one xlarge (8). + for name, recs := range map[string][]common.Recommendation{ + "large then xlarge": {flexRec("m5.large", "linux", "default", 1), flexRec("m5.xlarge", "linux", "default", 1)}, + "xlarge then large": {flexRec("m5.xlarge", "linux", "default", 1), flexRec("m5.large", "linux", "default", 1)}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + passed := runEC2(t, nil, ri, recs...) + require.Len(t, passed, 1) + assert.Equal(t, 1, passed[0].Count) + }) + } +} + +func TestEC2SizeFlexSuppressedUnitsNeverExceedOwned(t *testing.T) { + t.Parallel() + sizes := []string{"large", "xlarge", "2xlarge"} + units := map[string]int{"large": 4, "xlarge": 8, "2xlarge": 16} + rng := rand.New(rand.NewSource(323)) + for iter := 0; iter < 300; iter++ { + var commitments []common.Commitment + owned := 0 + for i := rng.Intn(3) + 1; i > 0; i-- { + sz, n := sizes[rng.Intn(3)], rng.Intn(3)+1 + commitments = append(commitments, flexCommitment("m5."+sz, "linux", "default", "regional", "", n)) + owned += units[sz] * n + } + var recs []common.Recommendation + demanded := 0 + for i := rng.Intn(5) + 1; i > 0; i-- { + sz, n := sizes[rng.Intn(3)], rng.Intn(3)+1 + recs = append(recs, flexRec("m5."+sz, "linux", "default", n)) + demanded += units[sz] * n + } + kept := 0 + for _, r := range runEC2(t, nil, commitments, recs...) { + kept += units[strings.TrimPrefix(r.ResourceType, "m5.")] * r.Count + } + suppressed := demanded - kept + require.LessOrEqual(t, suppressed, owned, "iter %d: suppressed %d units from %d owned", iter, suppressed, owned) + } +} + +func TestEC2SizeFlexExcludedFamiliesStayExact(t *testing.T) { + t.Parallel() + for _, family := range []string{"g5", "G4dn", "p5", "inf2", "u7i-6tb"} { + t.Run(family, func(t *testing.T) { + t.Parallel() + ri := []common.Commitment{flexCommitment(strings.ToLower(family)+".xlarge", "linux", "default", "regional", "", 1)} + passed := runEC2(t, nil, ri, flexRec(strings.ToLower(family)+".large", "linux", "default", 2)) + require.Len(t, passed, 1) + assert.Equal(t, 2, passed[0].Count) + assert.Empty(t, runEC2(t, nil, ri, flexRec(strings.ToLower(family)+".xlarge", "linux", "default", 1)), "same type still dedupes") + }) + } +} From 7e86db9256077cf3c09f95f4b902f414ab6961eb Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 12:30:24 +0200 Subject: [PATCH 5/5] fix(recfilter): share one RDS family budget across Single-AZ and Multi-AZ Size-flexible benefits apply to both deployment options, so a Single-AZ reservation also pays for Multi-AZ demand at twice the units per node. Single-AZ and Multi-AZ now share one family budget; any other deployment (Aurora's empty one) keeps its own pool, and exact-class engines keep their key. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 11 ++++++++++- pkg/recfilter/dedupe.go | 30 +++++++++--------------------- pkg/recfilter/dedupe_rds.go | 10 ++++++++++ pkg/recfilter/dedupe_rds_test.go | 25 +++++++++++++++++++++++-- 4 files changed, 52 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8669df76..2cac571a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -204,11 +204,20 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). family, so a recent Multi-AZ db.r6g.large PostgreSQL reservation suppresses two Single-AZ db.r6g.large recommendations. Single-AZ never covers Multi-AZ, and exact-class engines such as SQL Server keep their key. +- **Size-flexible RDS reservations now cover Single-AZ and Multi-AZ demand in + the duplicate-purchase guard** (#323): Single-AZ and Multi-AZ share one + family budget in size units, and a Multi-AZ node costs twice the units. A + recent Multi-AZ db.r6g.large PostgreSQL reservation suppresses two Single-AZ + db.r6g.large recommendations, and a Single-AZ db.r6g.xlarge suppresses one + Multi-AZ db.r6g.large. Exact-class engines such as SQL Server keep their key. - **The duplicate-purchase guard now treats regional Linux/Unix default-tenancy EC2 reservations as size-flexible** (#323): a recent m5.xlarge reservation suppresses two m5.large recommendations in the same region, counted in normalized units. Zonal, Windows/RHEL/SUSE, dedicated, wildcard and - unknown-size entries stay exact-type. + unknown-size entries stay exact-type, as do the families AWS excludes from + size flexibility (G4ad, G4dn, G5, G5g, G6, G6e, G6f, Gr6, Gr6f, hpc7a, P5, + Inf1, Inf2, u7i-6tb, u7i-8tb). A partly spent reservation is never credited + as a whole node. - **Unbounded AWS pagination loops now stop at a page cap and honour cancellation** (#561): the existing-commitment and valid-resource-type scans for ElastiCache, RDS, OpenSearch, Redshift and EC2 check the context before diff --git a/pkg/recfilter/dedupe.go b/pkg/recfilter/dedupe.go index 04e57528..07900f9f 100644 --- a/pkg/recfilter/dedupe.go +++ b/pkg/recfilter/dedupe.go @@ -218,6 +218,7 @@ func buildExistingCommitmentsMap(commitments []common.Commitment, logf Logf) map resourceType, nodeUnits := c.ResourceType, 1 if family, units, ok := rdsFlexKey(c.Provider, c.Service, normalizedEngine, c.ResourceType); ok { resourceType, nodeUnits = family, units*rdsDeploymentFactor(normalizedDeployment) + normalizedDeployment = rdsBudgetDeployment(normalizedDeployment) } key := dedupeKey(resourceType, c.Region, normalizedEngine, normalizedDeployment) existingMap[key] += c.Count * nodeUnits @@ -256,17 +257,6 @@ func adjustRecommendationsAgainstExisting(recs []common.Recommendation, existing return passed, filtered, nil } -// spendBudget takes up to want nodes from the budget under key and returns how many it covered. -func spendBudget(existingMap map[string]int, key string, cover elastiCacheCover, want int) int { - available := existingMap[key] - if available <= 0 || want <= 0 { - return 0 - } - covered, spent := cover.consume(available, want) - existingMap[key] -= spent - return covered -} - // adjustSingleRecommendation adjusts a single recommendation based on existing // commitments. A reduced count scales the money fields with it. func adjustSingleRecommendation(rec common.Recommendation, existingMap map[string]int, logf Logf) (common.Recommendation, error) { @@ -277,12 +267,10 @@ func adjustSingleRecommendation(rec common.Recommendation, existingMap map[strin deployment := common.NormalizeDeploymentName(common.DeploymentFromDetails(rec.Details)) resourceType := rec.ResourceType covers := []elastiCacheCover{{engine, 1, 1}} - flexUnits := 0 if family, units, ok := rdsFlexKey(rec.Provider, rec.Service, engine, rec.ResourceType); ok { // A size-flexible budget holds units: one node of this size spends units of them. - units *= rdsDeploymentFactor(deployment) - resourceType, covers = family, []elastiCacheCover{{engine, 1, units}} - flexUnits = units + resourceType, covers = family, []elastiCacheCover{{engine, 1, units * rdsDeploymentFactor(deployment)}} + deployment = rdsBudgetDeployment(deployment) } key := dedupeKey(resourceType, rec.Region, engine, deployment) if isElastiCache { @@ -291,13 +279,13 @@ func adjustSingleRecommendation(rec common.Recommendation, existingMap map[strin } remaining := rec.Count for _, cover := range covers { - remaining -= spendBudget(existingMap, dedupeKey(resourceType, rec.Region, cover.engine, deployment), cover, remaining) - } - if flexUnits > 0 && deployment == rdsSingleAZ { - // A Multi-AZ size-flexible reservation also pays for Single-AZ nodes; the reverse never holds. - remaining -= spendBudget(existingMap, dedupeKey(resourceType, rec.Region, engine, rdsMultiAZ), elastiCacheCover{engine, 1, flexUnits}, remaining) + candidate := dedupeKey(resourceType, rec.Region, cover.engine, deployment) + if available := existingMap[candidate]; available > 0 { + covered, spent := cover.consume(available, remaining) + remaining -= covered + existingMap[candidate] -= spent + } } - if remaining <= 0 { logf.printf(" [DuplicateChecker] SKIP %s: recent commitments cover recommended %d", key, rec.Count) return common.Recommendation{Count: 0}, nil diff --git a/pkg/recfilter/dedupe_rds.go b/pkg/recfilter/dedupe_rds.go index 7bea1bcd..00b9c728 100644 --- a/pkg/recfilter/dedupe_rds.go +++ b/pkg/recfilter/dedupe_rds.go @@ -38,3 +38,13 @@ func rdsDeploymentFactor(normalizedDeployment string) int { } return 1 } + +// rdsBudgetDeployment is the deployment part of a size-flexible budget key. Single-AZ and +// Multi-AZ share one family budget, because the size-flexible benefit applies to both +// configurations and a Multi-AZ node costs twice the units. Any other option keeps its own budget. +func rdsBudgetDeployment(normalizedDeployment string) string { + if normalizedDeployment == rdsSingleAZ || normalizedDeployment == rdsMultiAZ { + return rdsSingleAZ + "|" + rdsMultiAZ + } + return normalizedDeployment +} diff --git a/pkg/recfilter/dedupe_rds_test.go b/pkg/recfilter/dedupe_rds_test.go index 78dead02..b4860694 100644 --- a/pkg/recfilter/dedupe_rds_test.go +++ b/pkg/recfilter/dedupe_rds_test.go @@ -35,13 +35,15 @@ func TestRDSSizeFlexDedupe(t *testing.T) { {"other family", "db.r6g.large", "single-az", 1, common.ServiceRDS, "db.m6g.medium", "mysql", "single-az", "us-east-1", 1, 1, ""}, {"other engine", "db.r6g.large", "single-az", 1, common.ServiceRDS, "db.r6g.medium", "postgres", "single-az", "us-east-1", 1, 1, ""}, {"other region", "db.r6g.large", "single-az", 1, common.ServiceRDS, "db.r6g.medium", "mysql", "single-az", "us-west-2", 1, 1, ""}, - {"other deployment", "db.r6g.large", "single-az", 1, common.ServiceRDS, "db.r6g.medium", "mysql", "multi-az", "us-east-1", 1, 1, ""}, + {"single-az large covers one multi-az medium", "db.r6g.large", "single-az", 1, common.ServiceRDS, "db.r6g.medium", "mysql", "multi-az", "us-east-1", 1, 0, ""}, {"multi-az large covers two single-az larges", "db.r6g.large", "multi-az", 1, common.ServiceRDS, "db.r6g.large", "postgres", "single-az", "us-east-1", 2, 0, "postgres"}, {"multi-az large covers one of three single-az larges", "db.r6g.large", "multi-az", 1, common.ServiceRDS, "db.r6g.large", "postgres", "single-az", "us-east-1", 3, 1, "postgres"}, {"multi-az xlarge covers four single-az larges", "db.r6g.xlarge", "Multi-AZ", 1, common.ServiceRDS, "db.r6g.large", "postgres", "single-az", "us-east-1", 4, 0, "postgres"}, {"multi-az large covers one multi-az large", "db.r6g.large", "multi-az", 1, common.ServiceRDS, "db.r6g.large", "postgres", "multi-az", "us-east-1", 1, 0, "postgres"}, {"multi-az large covers one of two multi-az larges", "db.r6g.large", "multi-az", 1, common.ServiceRDS, "db.r6g.large", "postgres", "multi-az", "us-east-1", 2, 1, "postgres"}, - {"single-az never covers multi-az", "db.r6g.xlarge", "single-az", 1, common.ServiceRDS, "db.r6g.large", "postgres", "multi-az", "us-east-1", 1, 1, "postgres"}, + {"single-az xlarge covers one multi-az large", "db.r6g.xlarge", "single-az", 1, common.ServiceRDS, "db.r6g.large", "postgres", "multi-az", "us-east-1", 1, 0, "postgres"}, + {"single-az large does not cover multi-az large", "db.r6g.large", "single-az", 1, common.ServiceRDS, "db.r6g.large", "postgres", "multi-az", "us-east-1", 1, 1, "postgres"}, + {"single-az sqlserver never covers multi-az", "db.m6i.xlarge", "single-az", 1, common.ServiceRDS, "db.m6i.large", "sqlserver-se", "multi-az", "us-east-1", 1, 1, "sqlserver-se"}, {"multi-az sqlserver stays exact", "db.m6i.large", "multi-az", 1, common.ServiceRDS, "db.m6i.large", "sqlserver-se", "single-az", "us-east-1", 2, 2, "sqlserver-se"}, {"unknown size stays exact", "db.r6g.metal", "single-az", 1, common.ServiceRDS, "db.r6g.medium", "mysql", "single-az", "us-east-1", 1, 1, ""}, {"exact class still matches", "db.r6g.metal", "single-az", 1, common.ServiceRDS, "db.r6g.metal", "mysql", "single-az", "us-east-1", 1, 0, ""}, @@ -81,6 +83,25 @@ func TestRDSSizeFlexDedupe(t *testing.T) { } } +// A recommendation without a deployment (Aurora) is its own pool: it never spends a +// Single-AZ or Multi-AZ budget, and they never spend its budget. +func TestRDSEmptyDeploymentIsSeparatePool(t *testing.T) { + t.Parallel() + for _, deployment := range []string{"multi-az", "single-az"} { + client := &fakeServiceClient{commitments: []common.Commitment{{ + Provider: common.ProviderAWS, Service: common.ServiceRDS, ResourceType: "db.r6g.large", + Region: "us-east-1", Engine: "postgres", Deployment: deployment, Count: 1, + State: common.CommitmentStateActive, StartDate: time.Now(), + }}} + rec := common.Recommendation{Provider: common.ProviderAWS, Service: common.ServiceRDS, ResourceType: "db.r6g.large", + Region: "us-east-1", Count: 2, Details: &common.DatabaseDetails{Engine: "postgres"}} + passed, _, err := NewDuplicateChecker(0).AdjustRecommendationsForExisting(context.Background(), []common.Recommendation{rec}, client) + require.NoError(t, err) + require.Len(t, passed, 1, deployment) + assert.Equal(t, 2, passed[0].Count, deployment) + } +} + func TestRDSSizeFlexBudgetSharedAcrossRecs(t *testing.T) { t.Parallel() client := &fakeServiceClient{commitments: []common.Commitment{{