diff --git a/cmd/helpers.go b/cmd/helpers.go index 309599edd..911708112 100644 --- a/cmd/helpers.go +++ b/cmd/helpers.go @@ -5,7 +5,6 @@ import ( "context" "fmt" "log" - "math" "os" "strings" "sync" @@ -13,6 +12,7 @@ import ( "github.com/LeanerCloud/CUDly/pkg/common" "github.com/LeanerCloud/CUDly/pkg/provider" + "github.com/LeanerCloud/CUDly/pkg/recfilter" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/organizations" "golang.org/x/term" @@ -114,366 +114,23 @@ func CalculateTotalInstances(recs []common.Recommendation) int { return total } -// ApplyCoverage applies coverage percentage to recommendations. -// -// All cost-bearing fields (CommitmentCost, OnDemandCost, EstimatedSavings, -// and for SPs the SavingsPlanDetails.HourlyCommitment) scale by coverage/100 -// so the returned Recommendation represents the sized purchase rather than -// AWS's pre-sized proposal. SavingsPercentage is invariant (savings vs -// on-demand ratio) and stays unscaled. Pre-sizing values can still be -// recovered: RecommendedCount holds AWS's pre-sized count for RIs. +// ApplyCoverage delegates to recfilter.ApplyCoverage, wiring AppLogger as the +// logging sink. Substantive documentation lives on recfilter.ApplyCoverage. func ApplyCoverage(recs []common.Recommendation, coverage float64) []common.Recommendation { - return applyCoverage(recs, coverage, nil) + return recfilter.ApplyCoverage(recs, coverage, AppLogger.Printf, nil) } -// applyCoverage applies legacy percentage sizing and optionally records -// recommendations whose discrete RI count is reduced to zero. +// applyCoverage delegates to recfilter.ApplyCoverage, wiring AppLogger as the +// logging sink. Substantive documentation lives on recfilter.ApplyCoverage. func applyCoverage(recs []common.Recommendation, coverage float64, drops *common.DropSummary) []common.Recommendation { - if coverage >= 100 { - return recs - } - if coverage <= 0 { - return []common.Recommendation{} - } - - ratio := coverage / 100.0 - result := make([]common.Recommendation, 0, len(recs)) - for _rvc := range recs { - rec := recs[_rvc] - adjusted := rec - - // For Savings Plans, reduce the hourly commitment instead of count. - // If Details is the wrong type or a nil pointer (defensive — it - // should always be a non-nil *SavingsPlanDetails for SP recs), - // preserve the recommendation at its original values rather than - // silently dropping it. A missing-Details record is a logged - // anomaly, not a reason to erase coverage from the run. - // - // The nil check matters: an interface holding a typed nil satisfies - // the assertion, so testing ok alone would send an unscalable rec - // down the scaling path. - if common.IsSavingsPlan(rec.Service) { - if details, ok := rec.Details.(*common.SavingsPlanDetails); ok && details != nil { - // ScaleRecommendationCosts scales HourlyCommitment along with - // the cost fields and replaces Details with a scaled copy. - adjusted = common.ScaleRecommendationCosts(adjusted, ratio) - } else { - AppLogger.Printf("WARNING: SP recommendation for service %q has missing or unexpected Details (%T); passing through unscaled\n", rec.Service, rec.Details) - } - result = append(result, adjusted) - continue - } - - // For RIs, reduce the count and scale cost-bearing fields by the - // DISCRETE count ratio (newCount / rec.Count) rather than the - // requested ratio. Truncating newCount to an int then multiplying - // costs by the unrounded ratio desynchronises Count and costs: - // e.g. rec.Count=3 + ratio=0.5 yields newCount=1 (33% of instances) - // but costs would scale to 50%, overstating the sized purchase - // price by ~50%. Mirrors ApplyTargetCoverage / family-NU sizing. - // rec.Count is guaranteed > 0 here because newCount > 0 implies - // rec.Count >= 1 (int(0 * ratio) is 0 for any ratio). - newCount := int(float64(rec.Count) * ratio) - if newCount > 0 { - sizedRatio := float64(newCount) / float64(rec.Count) - adjusted = common.ScaleRecommendationCosts(adjusted, sizedRatio) - adjusted.Count = newCount - result = append(result, adjusted) - } else if drops != nil { - drops.Add(common.DropTargetSizedToZero, 1) - } - } - return result + return recfilter.ApplyCoverage(recs, coverage, AppLogger.Printf, drops) } -// ApplyTargetCoverage sizes RI/SP recommendations so that projected -// post-purchase COVERAGE lands near targetPct, leaving (100-targetPct)% of -// historical demand on-demand as headroom. See ApplyCoverage for the simpler -// rec.Count-scaled coverage flag; the two are dispatched via applySizing. -// -// AWS's recommendation count is sized for ~100% coverage of historical demand -// (average instances used per hour). --target-coverage is the lever the -// operator uses to deliberately under-buy that baseline, accepting more -// on-demand spend in exchange for less idle commitment when demand is bursty -// or trending down. -// -// The flag name says "utilization" because the original framing (issue #338) -// was a utilization floor. In practice operators set values like 70 or 80 -// expecting coverage near that figure (with utilization staying ~100% on the -// commitments actually purchased), not the over-buy semantics that floor -// produces; see the #338 review discussion for the redirect. -// -// RIs (existing-aware, per-pool, strict-target): -// -// gap = targetPct - ExistingCoveragePct (percentage points) -// remaining_gap = 100 - ExistingCoveragePct (percentage points) -// n_target = floor(rec.Count * gap / remaining_gap) -// -// The formula scales AWS's per-account-incremental rec.Count by the -// fraction of the current-to-100% gap we want to fill. For example -// with existing=50% and target=80%: gap=30, remaining_gap=50, so we -// buy 30/50 = 60% of AWS's rec.Count. Anchoring to rec.Count (which -// AWS computed per-linked-account) is more robust in multi-account -// orgs than scaling against avg, since CE's ExistingCoveragePct is -// org-wide averaged and mixes accounts together. -// -// If gap <= 0 (existing already at/above target) → drop with INFO log. -// If n_target == 0 (gap too small to fit one RI) → drop with INFO log. -// If AverageInstancesUsedPerHour <= 0 → pass through (no signal); counted -// in the per-run skip summary. -// Projected coverage = ExistingCoveragePct + n_target/avg * 100 (total -// coverage after the purchase, clamped to 100). Projected utilization = -// avg/n_target * 100 clamped to 100. -// -// ExistingCoveragePct is sourced from CE GetReservationCoverage in the -// same pool; zero means "no signal" and the formula reduces to -// floor(rec.Count * target/100) — i.e. plain target% of AWS's count. -// For RDS the coverage lookup keys by (region, instance_type, engine). -// Floor (rather than ceil or round) gives strict "at-most-target" -// sizing. Pools too small to approximate the target meaningfully -// should be filtered upstream via --min-pool-size; floor will drop -// them as zero-count otherwise. -// -// Pools where CE reports 100% existing coverage but AWS still recommends -// new RIs (typical when existing RIs are near expiry) are dropped here — -// the existing coverage is honored strictly. Use --rebuy-window-days to -// surface those replacements before the cliff. -// -// SPs: -// -// Scale SavingsPlanDetails.HourlyCommitment and EstimatedSavings by -// targetPct/100 (the same lever ApplyCoverage's SP branch uses, but with -// the explicit utilization-target framing). RecommendedUtilization is used -// only as the no-signal guard: when AWS hasn't returned a projected -// utilization figure, we pass the rec through unchanged and count it in -// the skip summary, since we can't sanity-check what the scaled commitment -// would mean. -// If RecommendedUtilization <= 0 → pass through; counted in skip summary. -// -// Recs of any other CommitmentType are passed through unmodified (warned -// once per type per run). -// ApplyTargetCoverage applies the target coverage percentage to a slice of -// recommendations. drops accumulates per-reason drop counts for the -// end-of-run summary; pass nil to skip tracking. +// ApplyTargetCoverage delegates to recfilter.ApplyTargetCoverage, wiring +// AppLogger as the logging sink. Substantive documentation (the RI/SP sizing +// formulas and the #338 flag-name history) lives on recfilter.ApplyTargetCoverage. func ApplyTargetCoverage(recs []common.Recommendation, targetPct float64, drops *common.DropSummary) []common.Recommendation { - if targetPct <= 0 || targetPct > 100 { - // Validation ensures we never get here in production, but be defensive - // so a buggy caller doesn't divide by zero. - AppLogger.Printf("WARNING: ApplyTargetCoverage called with targetPct=%.2f outside (0,100]; returning recs unchanged\n", targetPct) - return recs - } - - result := make([]common.Recommendation, 0, len(recs)) - var skipped int - unsupportedSeen := make(map[common.CommitmentType]bool) - - for i := range recs { - adjusted, kept, missingSignal, dropReason := applyTargetCoverageOne(recs[i], targetPct, unsupportedSeen) - if missingSignal { - skipped++ - } - if kept { - result = append(result, adjusted) - } else if dropReason != "" { - drops.Add(dropReason, 1) - } - } - - if skipped > 0 { - AppLogger.Printf("INFO: --target-coverage=%.1f%% skipped %d of %d recommendations with no utilization signal (passed through unchanged)\n", - targetPct, skipped, len(recs)) - } - - return result -} - -// applyTargetCoverageOne dispatches a single recommendation through the -// appropriate branch. Returns (rec, kept, missingSignal, dropReason): -// - kept=true → caller appends `rec` (the adjusted or pass-through value). -// - kept=false → caller drops the rec (only the RI "target unreachable" -// branches return this; an INFO log already fired). -// - missingSignal=true → counted toward the end-of-run skip summary. -// - dropReason is non-empty when kept=false and the drop has a named category. -// -// Split out of ApplyTargetCoverage to keep that function under gocyclo's -// complexity threshold. -func applyTargetCoverageOne(rec common.Recommendation, targetPct float64, unsupportedSeen map[common.CommitmentType]bool) (result common.Recommendation, kept, missingSignal bool, drop string) { - switch { - case common.IsSavingsPlan(rec.Service): - adjusted, ok := applyTargetCoverageSP(rec, targetPct) - if !ok { - // SP no-signal: pass through unchanged. - return rec, true, true, "" - } - return adjusted, true, false, "" - case rec.CommitmentType == common.CommitmentReservedInstance: - adjusted, ok, dropReason := applyTargetCoverageRI(rec, targetPct) - if !ok { - // Distinguish "no signal" (pass through, count in summary) from - // "target unreachable" (drop with already-fired INFO log). - if rec.AverageInstancesUsedPerHour <= 0 { - return rec, true, true, "" - } - return rec, false, false, dropReason - } - return adjusted, true, false, "" - default: - if !unsupportedSeen[rec.CommitmentType] { - AppLogger.Printf("WARNING: --target-coverage not supported for CommitmentType=%q; passing recommendations through unchanged\n", rec.CommitmentType) - unsupportedSeen[rec.CommitmentType] = true - } - return rec, true, false, "" - } -} - -// applyTargetCoverageRI is the RI branch of ApplyTargetCoverage. Returns -// (adjusted, true, "") on success, (rec, false, dropReason) when the rec -// should be passed through unscaled (no signal) or dropped (target -// unreachable). Caller distinguishes no-signal from drop via -// rec.AverageInstancesUsedPerHour and uses dropReason for the summary. -func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (result common.Recommendation, ok bool, drop string) { - if rec.AverageInstancesUsedPerHour <= 0 { - // No signal — caller will pass through and count in the summary. - return rec, false, "" - } - - avg := rec.AverageInstancesUsedPerHour - // Coverage-anchored under-buy: size linearly off the pool's avg demand - // and the absolute gap to target. Both inputs come from - // GetReservationCoverage (AvgInstancesPerHour from - // TotalRunningHours/window; ExistingCoveragePct from - // CoverageHoursPercentage) so the buy lines up with the AWS console's - // reservations-coverage report: target%-existing% of avg instances. - // - // The previous formula anchored on AWS's rec.Count - // (floor(rec.Count × gap / (100−existing))), which under-bought when - // AWS sized rec.Count for less than full coverage (ROI-curated) and - // when CE's org-wide existing% disagreed with rec.Count's per-account - // derivation. Anchoring on coverage's own avg removes both mismatches. - // rec.Count is retained only for the cost-scaling ratio further down. - // - // Keep the subtraction in percentage units (subtract first, divide - // later) so whole-percent values don't lose precision to float - // rounding at integer boundaries. - gapPct := targetPct - rec.ExistingCoveragePct - if gapPct <= 0 { - // Existing commitments already meet or exceed the target; no purchase - // needed in this pool. Drop with an info log so operators can see what - // the flag did. Returning (_, false) with avg > 0 signals "drop, don't - // pass through". - AppLogger.Printf("INFO: --target-coverage=%.1f%% already met by existing coverage %.1f%% for %s/%s/%s; dropped recommendation\n", - targetPct, rec.ExistingCoveragePct, rec.Service, rec.Region, rec.ResourceType) - return rec, false, common.DropTargetAlreadyMet - } - // Floor so we never over-shoot the target on integer-arithmetic edges. - // Strict-target semantics: 80% means "at most 80% coverage", not "at - // least 80%". Floor under-covers small/odd pools (e.g. avg=2, target=80 - // gives 1 RI = 50% rather than 2 RIs = 100%); pools too small to - // approximate target are best filtered out via --min-pool-size upstream. - nTarget := int(math.Floor(avg * gapPct / 100.0)) - - if nTarget == 0 { - // Floor produces zero when avg × gap% < 100 (small pools or thin - // gaps). Drop — buying 1 RI would over-shoot target and the - // strict-target intent prefers under-cover (run on-demand) over - // over-cover (idle commitment). Use --min-pool-size to filter - // these out earlier so they don't show up as drops in the log. - AppLogger.Printf("INFO: --target-coverage=%.1f%% sizes %s/%s/%s to 0 instances (avg=%.2f, gap=%.2f%% produces <1 RI); dropped recommendation\n", - targetPct, rec.Service, rec.Region, rec.ResourceType, avg, gapPct) - // Returning (_, false) with avg > 0 signals "drop, don't pass through". - // applyTargetCoverageRI's caller branches on - // rec.AverageInstancesUsedPerHour to distinguish drop vs no-signal. - return rec, false, common.DropTargetSizedToZero - } - - // Cost-bearing fields scale by the ratio of sized-to-original count, so the - // returned rec represents the sized purchase rather than AWS's pre-sized - // proposal. SavingsPercentage is invariant (savings vs on-demand ratio). - // rec.Count is the AWS pre-sizing count at this point (parser sets Count - // == RecommendedCount and we haven't mutated either yet). When the - // coverage-anchored nTarget exceeds rec.Count (AWS sized below full - // coverage), the ratio scales costs up linearly — accurate when per-RI - // pricing is constant, which it is within a single pool/term/payment - // combination. Guarded against rec.Count==0 (malformed rec) by falling - // back to nTarget so a zero-cost rec stays zero-cost rather than NaN. - var ratio float64 - if rec.Count > 0 { - ratio = float64(nTarget) / float64(rec.Count) - } else { - ratio = float64(nTarget) - } - adjusted := common.ScaleRecommendationCosts(rec, ratio) - adjusted.Count = nTarget - - // Projection metrics. ProjectedCoverage is TOTAL coverage (existing + - // new) so operators can see the figure they actually targeted. - // ProjectedUtilization stays at the per-purchase fill rate; under-buy - // keeps nTarget <= avg so it always clamps to 100%. - projUtil := avg / float64(nTarget) * 100.0 - if projUtil > 100 { - projUtil = 100 - } - projCov := rec.ExistingCoveragePct + float64(nTarget)/avg*100.0 - if projCov > 100 { - projCov = 100 - } - adjusted.ProjectedUtilization = projUtil - adjusted.ProjectedCoverage = projCov - return adjusted, true, "" -} - -// applyTargetCoverageSP is the SP branch of ApplyTargetCoverage. Returns -// (adjusted, true) when the rec is kept, (rec, false) when it should be -// skipped (caller passes through unscaled and counts in the skip summary). -func applyTargetCoverageSP(rec common.Recommendation, targetPct float64) (common.Recommendation, bool) { - if rec.RecommendedUtilization <= 0 { - return rec, false - } - // If Details isn't a non-nil *SavingsPlanDetails (defensive — it should - // always be one for SP recs), log a warning and pass through UNCHANGED — - // including leaving ProjectedUtilization at zero. Setting projection - // fields on a rec whose commitment fields couldn't be scaled would - // produce a misleading row (projection=target%, savings=full-unscaled). - // - // The nil check must precede the HourlyCommitment read below: an - // interface holding a typed nil satisfies the assertion, so reading the - // field off it would dereference nil. - details, ok := rec.Details.(*common.SavingsPlanDetails) - if !ok || details == nil { - AppLogger.Printf("WARNING: SP recommendation for service %q has missing or unexpected Details (%T); passing through unscaled\n", rec.Service, rec.Details) - return rec, true - } - // Also treat a $0 HourlyCommitment as "no signal" — CE occasionally - // returns placeholder recs with zero commitment. Sizing such a rec - // would produce nonsense ($0 commitment * ratio = $0) while still - // claiming the target coverage is achieved, which is incoherent. - // Pass through unchanged and count in the skip summary. - if details.HourlyCommitment <= 0 { - return rec, false - } - - // Under-buy: scale all cost-bearing fields by target/100 against AWS's - // recommended commitment. This deliberately spends less than AWS suggested, - // leaving (100-target)% of the SP's projected workload on on-demand. - // RecommendedUtilization is consulted only as a no-signal guard above (a - // zero value means we can't sanity-check the result); the scaling itself - // uses targetPct directly rather than a recUtil/target ratio so the flag's - // intent is honored even when AWS already projects above target. - ratio := targetPct / 100.0 - // ScaleRecommendationCosts scales HourlyCommitment along with the cost - // fields and replaces Details with a scaled copy. - adjusted := common.ScaleRecommendationCosts(rec, ratio) - // Shrinking commitment raises projected utilization by 1/ratio - // (used is fixed = orig_commit * RecUtil, bought is orig_commit * ratio). - // Clamp to 100 since utilization caps at full use. - projUtil := rec.RecommendedUtilization / ratio - if projUtil > 100 { - projUtil = 100 - } - adjusted.ProjectedUtilization = projUtil - // ProjectedCoverage stays zero for SPs — CE doesn't expose total-demand-$ - // for a clean coverage figure (see field doc on Recommendation). - return adjusted, true + return recfilter.ApplyTargetCoverage(recs, targetPct, AppLogger.Printf, drops) } // applySizing chooses target-coverage or coverage sizing. diff --git a/pkg/recfilter/sizing.go b/pkg/recfilter/sizing.go new file mode 100644 index 000000000..e7a4562e3 --- /dev/null +++ b/pkg/recfilter/sizing.go @@ -0,0 +1,368 @@ +package recfilter + +import ( + "math" + + "github.com/LeanerCloud/CUDly/pkg/common" +) + +// ApplyCoverage applies coverage percentage to recommendations. +// +// All cost-bearing fields (CommitmentCost, OnDemandCost, EstimatedSavings, +// and for SPs the SavingsPlanDetails.HourlyCommitment) scale by coverage/100 +// so the returned Recommendation represents the sized purchase rather than +// AWS's pre-sized proposal. SavingsPercentage is invariant (savings vs +// on-demand ratio) and stays unscaled. Pre-sizing values can still be +// recovered: RecommendedCount holds AWS's pre-sized count for RIs. +// +// drops accumulates per-reason drop counts for the end-of-run summary; pass +// nil to skip tracking. logf receives WARNING lines for anomalous recs +// (nil-safe; pass nil to disable logging). +func ApplyCoverage(recs []common.Recommendation, coverage float64, logf Logf, drops *common.DropSummary) []common.Recommendation { + if coverage >= 100 { + return recs + } + if coverage <= 0 { + return []common.Recommendation{} + } + + ratio := coverage / 100.0 + result := make([]common.Recommendation, 0, len(recs)) + for _rvc := range recs { + rec := recs[_rvc] + adjusted := rec + + // For Savings Plans, reduce the hourly commitment instead of count. + // If Details is the wrong type or a nil pointer (defensive — it + // should always be a non-nil *SavingsPlanDetails for SP recs), + // preserve the recommendation at its original values rather than + // silently dropping it. A missing-Details record is a logged + // anomaly, not a reason to erase coverage from the run. + // + // The nil check matters: an interface holding a typed nil satisfies + // the assertion, so testing ok alone would send an unscalable rec + // down the scaling path. + if common.IsSavingsPlan(rec.Service) { + if details, ok := rec.Details.(*common.SavingsPlanDetails); ok && details != nil { + // ScaleRecommendationCosts scales HourlyCommitment along with + // the cost fields and replaces Details with a scaled copy. + adjusted = common.ScaleRecommendationCosts(adjusted, ratio) + } else { + logf.printf("WARNING: SP recommendation for service %q has missing or unexpected Details (%T); passing through unscaled\n", rec.Service, rec.Details) + } + result = append(result, adjusted) + continue + } + + // For RIs, reduce the count and scale cost-bearing fields by the + // DISCRETE count ratio (newCount / rec.Count) rather than the + // requested ratio. Truncating newCount to an int then multiplying + // costs by the unrounded ratio desynchronises Count and costs: + // e.g. rec.Count=3 + ratio=0.5 yields newCount=1 (33% of instances) + // but costs would scale to 50%, overstating the sized purchase + // price by ~50%. Mirrors ApplyTargetCoverage / family-NU sizing. + // rec.Count is guaranteed > 0 here because newCount > 0 implies + // rec.Count >= 1 (int(0 * ratio) is 0 for any ratio). + newCount := int(float64(rec.Count) * ratio) + if newCount > 0 { + sizedRatio := float64(newCount) / float64(rec.Count) + adjusted = common.ScaleRecommendationCosts(adjusted, sizedRatio) + adjusted.Count = newCount + result = append(result, adjusted) + } else if drops != nil { + drops.Add(common.DropTargetSizedToZero, 1) + } + } + return result +} + +// ApplyTargetCoverage sizes RI/SP recommendations so that projected +// post-purchase COVERAGE lands near targetPct, leaving (100-targetPct)% of +// historical demand on-demand as headroom. See ApplyCoverage for the simpler +// rec.Count-scaled coverage flag; the two are dispatched via cmd's applySizing. +// +// AWS's recommendation count is sized for ~100% coverage of historical demand +// (average instances used per hour). --target-coverage is the lever the +// operator uses to deliberately under-buy that baseline, accepting more +// on-demand spend in exchange for less idle commitment when demand is bursty +// or trending down. +// +// The flag name says "utilization" because the original framing (issue #338) +// was a utilization floor. In practice operators set values like 70 or 80 +// expecting coverage near that figure (with utilization staying ~100% on the +// commitments actually purchased), not the over-buy semantics that floor +// produces; see the #338 review discussion for the redirect. +// +// RIs (existing-aware, per-pool, strict-target): +// +// gap = targetPct - ExistingCoveragePct (percentage points) +// remaining_gap = 100 - ExistingCoveragePct (percentage points) +// n_target = floor(rec.Count * gap / remaining_gap) +// +// The formula scales AWS's per-account-incremental rec.Count by the +// fraction of the current-to-100% gap we want to fill. For example +// with existing=50% and target=80%: gap=30, remaining_gap=50, so we +// buy 30/50 = 60% of AWS's rec.Count. Anchoring to rec.Count (which +// AWS computed per-linked-account) is more robust in multi-account +// orgs than scaling against avg, since CE's ExistingCoveragePct is +// org-wide averaged and mixes accounts together. +// +// If gap <= 0 (existing already at/above target) → drop with INFO log. +// If n_target == 0 (gap too small to fit one RI) → drop with INFO log. +// If AverageInstancesUsedPerHour <= 0 → pass through (no signal); counted +// in the per-run skip summary. +// Projected coverage = ExistingCoveragePct + n_target/avg * 100 (total +// coverage after the purchase, clamped to 100). Projected utilization = +// avg/n_target * 100 clamped to 100. +// +// ExistingCoveragePct is sourced from CE GetReservationCoverage in the +// same pool; zero means "no signal" and the formula reduces to +// floor(rec.Count * target/100) — i.e. plain target% of AWS's count. +// For RDS the coverage lookup keys by (region, instance_type, engine). +// Floor (rather than ceil or round) gives strict "at-most-target" +// sizing. Pools too small to approximate the target meaningfully +// should be filtered upstream via --min-pool-size; floor will drop +// them as zero-count otherwise. +// +// Pools where CE reports 100% existing coverage but AWS still recommends +// new RIs (typical when existing RIs are near expiry) are dropped here — +// the existing coverage is honored strictly. Use --rebuy-window-days to +// surface those replacements before the cliff. +// +// SPs: +// +// Scale SavingsPlanDetails.HourlyCommitment and EstimatedSavings by +// targetPct/100 (the same lever ApplyCoverage's SP branch uses, but with +// the explicit utilization-target framing). RecommendedUtilization is used +// only as the no-signal guard: when AWS hasn't returned a projected +// utilization figure, we pass the rec through unchanged and count it in +// the skip summary, since we can't sanity-check what the scaled commitment +// would mean. +// If RecommendedUtilization <= 0 → pass through; counted in skip summary. +// +// Recs of any other CommitmentType are passed through unmodified (warned +// once per type per run). +// +// drops accumulates per-reason drop counts for the end-of-run summary; pass +// nil to skip tracking. logf receives WARNING/INFO lines (nil-safe; pass nil +// to disable logging). +func ApplyTargetCoverage(recs []common.Recommendation, targetPct float64, logf Logf, drops *common.DropSummary) []common.Recommendation { + if targetPct <= 0 || targetPct > 100 { + // Validation ensures we never get here in production, but be defensive + // so a buggy caller doesn't divide by zero. + logf.printf("WARNING: ApplyTargetCoverage called with targetPct=%.2f outside (0,100]; returning recs unchanged\n", targetPct) + return recs + } + + result := make([]common.Recommendation, 0, len(recs)) + var skipped int + unsupportedSeen := make(map[common.CommitmentType]bool) + + for i := range recs { + adjusted, kept, missingSignal, dropReason := applyTargetCoverageOne(recs[i], targetPct, unsupportedSeen, logf) + if missingSignal { + skipped++ + } + if kept { + result = append(result, adjusted) + } else if dropReason != "" { + drops.Add(dropReason, 1) + } + } + + if skipped > 0 { + logf.printf("INFO: --target-coverage=%.1f%% skipped %d of %d recommendations with no utilization signal (passed through unchanged)\n", + targetPct, skipped, len(recs)) + } + + return result +} + +// applyTargetCoverageOne dispatches a single recommendation through the +// appropriate branch. Returns (rec, kept, missingSignal, dropReason): +// - kept=true → caller appends `rec` (the adjusted or pass-through value). +// - kept=false → caller drops the rec (only the RI "target unreachable" +// branches return this; an INFO log already fired). +// - missingSignal=true → counted toward the end-of-run skip summary. +// - dropReason is non-empty when kept=false and the drop has a named category. +// +// Split out of ApplyTargetCoverage to keep that function under gocyclo's +// complexity threshold. +func applyTargetCoverageOne(rec common.Recommendation, targetPct float64, unsupportedSeen map[common.CommitmentType]bool, logf Logf) (result common.Recommendation, kept, missingSignal bool, drop string) { + switch { + case common.IsSavingsPlan(rec.Service): + adjusted, ok := applyTargetCoverageSP(rec, targetPct, logf) + if !ok { + // SP no-signal: pass through unchanged. + return rec, true, true, "" + } + return adjusted, true, false, "" + case rec.CommitmentType == common.CommitmentReservedInstance: + adjusted, ok, dropReason := applyTargetCoverageRI(rec, targetPct, logf) + if !ok { + // Distinguish "no signal" (pass through, count in summary) from + // "target unreachable" (drop with already-fired INFO log). + if rec.AverageInstancesUsedPerHour <= 0 { + return rec, true, true, "" + } + return rec, false, false, dropReason + } + return adjusted, true, false, "" + default: + if !unsupportedSeen[rec.CommitmentType] { + logf.printf("WARNING: --target-coverage not supported for CommitmentType=%q; passing recommendations through unchanged\n", rec.CommitmentType) + unsupportedSeen[rec.CommitmentType] = true + } + return rec, true, false, "" + } +} + +// applyTargetCoverageRI is the RI branch of ApplyTargetCoverage. Returns +// (adjusted, true, "") on success, (rec, false, dropReason) when the rec +// should be passed through unscaled (no signal) or dropped (target +// unreachable). Caller distinguishes no-signal from drop via +// rec.AverageInstancesUsedPerHour and uses dropReason for the summary. +func applyTargetCoverageRI(rec common.Recommendation, targetPct float64, logf Logf) (result common.Recommendation, ok bool, drop string) { + if rec.AverageInstancesUsedPerHour <= 0 { + // No signal — caller will pass through and count in the summary. + return rec, false, "" + } + + avg := rec.AverageInstancesUsedPerHour + // Coverage-anchored under-buy: size linearly off the pool's avg demand + // and the absolute gap to target. Both inputs come from + // GetReservationCoverage (AvgInstancesPerHour from + // TotalRunningHours/window; ExistingCoveragePct from + // CoverageHoursPercentage) so the buy lines up with the AWS console's + // reservations-coverage report: target%-existing% of avg instances. + // + // The previous formula anchored on AWS's rec.Count + // (floor(rec.Count × gap / (100−existing))), which under-bought when + // AWS sized rec.Count for less than full coverage (ROI-curated) and + // when CE's org-wide existing% disagreed with rec.Count's per-account + // derivation. Anchoring on coverage's own avg removes both mismatches. + // rec.Count is retained only for the cost-scaling ratio further down. + // + // Keep the subtraction in percentage units (subtract first, divide + // later) so whole-percent values don't lose precision to float + // rounding at integer boundaries. + gapPct := targetPct - rec.ExistingCoveragePct + if gapPct <= 0 { + // Existing commitments already meet or exceed the target; no purchase + // needed in this pool. Drop with an info log so operators can see what + // the flag did. Returning (_, false) with avg > 0 signals "drop, don't + // pass through". + logf.printf("INFO: --target-coverage=%.1f%% already met by existing coverage %.1f%% for %s/%s/%s; dropped recommendation\n", + targetPct, rec.ExistingCoveragePct, rec.Service, rec.Region, rec.ResourceType) + return rec, false, common.DropTargetAlreadyMet + } + // Floor so we never over-shoot the target on integer-arithmetic edges. + // Strict-target semantics: 80% means "at most 80% coverage", not "at + // least 80%". Floor under-covers small/odd pools (e.g. avg=2, target=80 + // gives 1 RI = 50% rather than 2 RIs = 100%); pools too small to + // approximate target are best filtered out via --min-pool-size upstream. + nTarget := int(math.Floor(avg * gapPct / 100.0)) + + if nTarget == 0 { + // Floor produces zero when avg × gap% < 100 (small pools or thin + // gaps). Drop — buying 1 RI would over-shoot target and the + // strict-target intent prefers under-cover (run on-demand) over + // over-cover (idle commitment). Use --min-pool-size to filter + // these out earlier so they don't show up as drops in the log. + logf.printf("INFO: --target-coverage=%.1f%% sizes %s/%s/%s to 0 instances (avg=%.2f, gap=%.2f%% produces <1 RI); dropped recommendation\n", + targetPct, rec.Service, rec.Region, rec.ResourceType, avg, gapPct) + // Returning (_, false) with avg > 0 signals "drop, don't pass through". + // applyTargetCoverageRI's caller branches on + // rec.AverageInstancesUsedPerHour to distinguish drop vs no-signal. + return rec, false, common.DropTargetSizedToZero + } + + // Cost-bearing fields scale by the ratio of sized-to-original count, so the + // returned rec represents the sized purchase rather than AWS's pre-sized + // proposal. SavingsPercentage is invariant (savings vs on-demand ratio). + // rec.Count is the AWS pre-sizing count at this point (parser sets Count + // == RecommendedCount and we haven't mutated either yet). When the + // coverage-anchored nTarget exceeds rec.Count (AWS sized below full + // coverage), the ratio scales costs up linearly — accurate when per-RI + // pricing is constant, which it is within a single pool/term/payment + // combination. Guarded against rec.Count==0 (malformed rec) by falling + // back to nTarget so a zero-cost rec stays zero-cost rather than NaN. + var ratio float64 + if rec.Count > 0 { + ratio = float64(nTarget) / float64(rec.Count) + } else { + ratio = float64(nTarget) + } + adjusted := common.ScaleRecommendationCosts(rec, ratio) + adjusted.Count = nTarget + + // Projection metrics. ProjectedCoverage is TOTAL coverage (existing + + // new) so operators can see the figure they actually targeted. + // ProjectedUtilization stays at the per-purchase fill rate; under-buy + // keeps nTarget <= avg so it always clamps to 100%. + projUtil := avg / float64(nTarget) * 100.0 + if projUtil > 100 { + projUtil = 100 + } + projCov := rec.ExistingCoveragePct + float64(nTarget)/avg*100.0 + if projCov > 100 { + projCov = 100 + } + adjusted.ProjectedUtilization = projUtil + adjusted.ProjectedCoverage = projCov + return adjusted, true, "" +} + +// applyTargetCoverageSP is the SP branch of ApplyTargetCoverage. Returns +// (adjusted, true) when the rec is kept, (rec, false) when it should be +// skipped (caller passes through unscaled and counts in the skip summary). +func applyTargetCoverageSP(rec common.Recommendation, targetPct float64, logf Logf) (common.Recommendation, bool) { + if rec.RecommendedUtilization <= 0 { + return rec, false + } + // If Details isn't a non-nil *SavingsPlanDetails (defensive — it should + // always be one for SP recs), log a warning and pass through UNCHANGED — + // including leaving ProjectedUtilization at zero. Setting projection + // fields on a rec whose commitment fields couldn't be scaled would + // produce a misleading row (projection=target%, savings=full-unscaled). + // + // The nil check must precede the HourlyCommitment read below: an + // interface holding a typed nil satisfies the assertion, so reading the + // field off it would dereference nil. + details, ok := rec.Details.(*common.SavingsPlanDetails) + if !ok || details == nil { + logf.printf("WARNING: SP recommendation for service %q has missing or unexpected Details (%T); passing through unscaled\n", rec.Service, rec.Details) + return rec, true + } + // Also treat a $0 HourlyCommitment as "no signal" — CE occasionally + // returns placeholder recs with zero commitment. Sizing such a rec + // would produce nonsense ($0 commitment * ratio = $0) while still + // claiming the target coverage is achieved, which is incoherent. + // Pass through unchanged and count in the skip summary. + if details.HourlyCommitment <= 0 { + return rec, false + } + + // Under-buy: scale all cost-bearing fields by target/100 against AWS's + // recommended commitment. This deliberately spends less than AWS suggested, + // leaving (100-target)% of the SP's projected workload on on-demand. + // RecommendedUtilization is consulted only as a no-signal guard above (a + // zero value means we can't sanity-check the result); the scaling itself + // uses targetPct directly rather than a recUtil/target ratio so the flag's + // intent is honored even when AWS already projects above target. + ratio := targetPct / 100.0 + // ScaleRecommendationCosts scales HourlyCommitment along with the cost + // fields and replaces Details with a scaled copy. + adjusted := common.ScaleRecommendationCosts(rec, ratio) + // Shrinking commitment raises projected utilization by 1/ratio + // (used is fixed = orig_commit * RecUtil, bought is orig_commit * ratio). + // Clamp to 100 since utilization caps at full use. + projUtil := rec.RecommendedUtilization / ratio + if projUtil > 100 { + projUtil = 100 + } + adjusted.ProjectedUtilization = projUtil + // ProjectedCoverage stays zero for SPs — CE doesn't expose total-demand-$ + // for a clean coverage figure (see field doc on Recommendation). + return adjusted, true +} diff --git a/pkg/recfilter/sizing_test.go b/pkg/recfilter/sizing_test.go new file mode 100644 index 000000000..cca3495a6 --- /dev/null +++ b/pkg/recfilter/sizing_test.go @@ -0,0 +1,383 @@ +package recfilter + +import ( + "fmt" + "math" + "strings" + "testing" + + "github.com/LeanerCloud/CUDly/pkg/common" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// captureLogf returns a Logf that appends every formatted line to *log, for +// tests asserting on warning/info output without touching a global logger. +func captureLogf(log *[]string) Logf { + return func(format string, args ...any) { + *log = append(*log, fmt.Sprintf(format, args...)) + } +} + +// --- T4: ApplyCoverage --- + +// TestApplyCoverage_DiscreteRatioRegression is the most important test in +// this file. Count=3 at coverage=50% must scale money fields to 1/3, NOT +// 0.5: newCount = int(3*0.5) = 1, so the DISCRETE ratio (1/3) governs cost +// scaling, not the requested ratio (0.5). Regressing to the requested ratio +// scales money by 0.5 while Count truncates to 1, overstating the sized +// purchase's cost by ~50% relative to what one instance actually costs. +func TestApplyCoverage_DiscreteRatioRegression(t *testing.T) { + t.Parallel() + rec := common.Recommendation{ + Service: common.ServiceEC2, + Count: 3, + CommitmentCost: 300, + OnDemandCost: 600, + EstimatedSavings: 300, + SavingsPercentage: 50, + } + + out := ApplyCoverage([]common.Recommendation{rec}, 50, nil, nil) + + require.Len(t, out, 1) + assert.Equal(t, 1, out[0].Count) + assert.InDelta(t, 100.0, out[0].CommitmentCost, 0.001, "1/3 of 300, not 0.5*300=150") + assert.InDelta(t, 200.0, out[0].OnDemandCost, 0.001, "1/3 of 600, not 0.5*600=300") + assert.InDelta(t, 100.0, out[0].EstimatedSavings, 0.001, "1/3 of 300, not 0.5*300=150") +} + +func TestApplyCoverage_HundredOrAbove_ReturnsUnchanged(t *testing.T) { + t.Parallel() + recs := []common.Recommendation{ + {Service: common.ServiceEC2, Count: 7, CommitmentCost: 111}, + {Service: common.ServiceSavingsPlansCompute, Details: &common.SavingsPlanDetails{HourlyCommitment: 4}}, + } + + for _, coverage := range []float64{100, 150} { + out := ApplyCoverage(recs, coverage, nil, nil) + require.Len(t, out, len(recs)) + assert.Equal(t, recs, out, "coverage=%.0f must return input unchanged", coverage) + } +} + +func TestApplyCoverage_ZeroOrBelow_ReturnsEmpty(t *testing.T) { + t.Parallel() + recs := []common.Recommendation{{Service: common.ServiceEC2, Count: 5}} + + for _, coverage := range []float64{0, -5} { + out := ApplyCoverage(recs, coverage, nil, nil) + require.NotNil(t, out, "coverage=%.0f must return a non-nil empty slice", coverage) + assert.Len(t, out, 0) + } +} + +func TestApplyCoverage_SavingsPlan_ScalesHourlyCommitment_NotCount(t *testing.T) { + t.Parallel() + rec := common.Recommendation{ + Service: common.ServiceSavingsPlansCompute, + Count: 5, + CommitmentCost: 1000, + OnDemandCost: 2000, + EstimatedSavings: 1000, + Details: &common.SavingsPlanDetails{HourlyCommitment: 10}, + } + + out := ApplyCoverage([]common.Recommendation{rec}, 50, nil, nil) + + require.Len(t, out, 1) + assert.Equal(t, 5, out[0].Count, "SP branch never touches Count") + assert.InDelta(t, 5.0, out[0].Details.(*common.SavingsPlanDetails).HourlyCommitment, 0.001) + assert.InDelta(t, 500.0, out[0].CommitmentCost, 0.001) +} + +func TestApplyCoverage_SPBadDetails_PassThroughUnscaled_WarnsOnce(t *testing.T) { + t.Parallel() + + t.Run("wrong type", func(t *testing.T) { + t.Parallel() + var logs []string + rec := common.Recommendation{ + Service: common.ServiceSavingsPlansCompute, + EstimatedSavings: 1500, + Details: common.ComputeDetails{Platform: "Linux/UNIX"}, + } + out := ApplyCoverage([]common.Recommendation{rec}, 50, captureLogf(&logs), nil) + require.Len(t, out, 1) + assert.Equal(t, 1500.0, out[0].EstimatedSavings, "unscaled") + assert.Len(t, logs, 1, "exactly one warning") + }) + + t.Run("typed nil Details", func(t *testing.T) { + t.Parallel() + var logs []string + rec := common.Recommendation{ + Service: common.ServiceSavingsPlansCompute, + EstimatedSavings: 1500, + Details: (*common.SavingsPlanDetails)(nil), + } + // An interface holding a typed nil satisfies the type assertion with + // ok==true, so this case exercises the `ok && details != nil` guard + // specifically — a plain `ok` check would send this down the + // scaling path and panic on the nil dereference. + out := ApplyCoverage([]common.Recommendation{rec}, 50, captureLogf(&logs), nil) + require.Len(t, out, 1) + assert.Equal(t, 1500.0, out[0].EstimatedSavings, "unscaled") + assert.Len(t, logs, 1, "exactly one warning") + }) +} + +func TestApplyCoverage_RISizedToZero_DropsAndRecords(t *testing.T) { + t.Parallel() + rec := common.Recommendation{Service: common.ServiceEC2, Count: 1, CommitmentCost: 100} + + t.Run("with drops summary", func(t *testing.T) { + t.Parallel() + d := common.NewDropSummary() + out := ApplyCoverage([]common.Recommendation{rec}, 10, nil, d) + assert.Empty(t, out) + assert.Equal(t, 1, d.Total()) + assert.Contains(t, d.FormatOneLine(), common.DropTargetSizedToZero) + }) + + t.Run("with nil drops", func(t *testing.T) { + t.Parallel() + assert.NotPanics(t, func() { + out := ApplyCoverage([]common.Recommendation{rec}, 10, nil, nil) + assert.Empty(t, out) + }) + }) +} + +func TestApplyCoverage_NilLogfSafe(t *testing.T) { + t.Parallel() + rec := common.Recommendation{ + Service: common.ServiceSavingsPlansCompute, + Details: common.ComputeDetails{Platform: "Linux/UNIX"}, // wrong type -> warning path + } + assert.NotPanics(t, func() { + ApplyCoverage([]common.Recommendation{rec}, 50, nil, nil) + }) +} + +// --- T5: ApplyTargetCoverage --- + +func mkRI(count int, avg, existingCov float64) common.Recommendation { + return common.Recommendation{ + Service: common.ServiceEC2, + Region: "us-east-1", + ResourceType: "t3.medium", + Count: count, + CommitmentType: common.CommitmentReservedInstance, + CommitmentCost: 1000, + OnDemandCost: 2000, + EstimatedSavings: 500, + AverageInstancesUsedPerHour: avg, + ExistingCoveragePct: existingCov, + } +} + +func mkSP(recUtil, hourlyCommitment float64) common.Recommendation { + return common.Recommendation{ + Service: common.ServiceSavingsPlansCompute, + CommitmentType: common.CommitmentSavingsPlan, + CommitmentCost: 1000, + OnDemandCost: 5000, + EstimatedSavings: 1500, + RecommendedUtilization: recUtil, + Details: &common.SavingsPlanDetails{HourlyCommitment: hourlyCommitment}, + } +} + +func TestApplyTargetCoverage_GapAlreadyMet_Drops(t *testing.T) { + t.Parallel() + rec := mkRI(5, 8.0, 90) // existing=90 >= target=80 + d := common.NewDropSummary() + out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil, d) + assert.Empty(t, out) + assert.Equal(t, 1, d.Total()) + assert.Contains(t, d.FormatOneLine(), common.DropTargetAlreadyMet) +} + +func TestApplyTargetCoverage_FloorSizesToZero_Drops(t *testing.T) { + t.Parallel() + // avg=0.5, target=80, existing=0: floor(0.5*80/100)=floor(0.4)=0. + rec := mkRI(1, 0.5, 0) + d := common.NewDropSummary() + out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil, d) + assert.Empty(t, out) + assert.Equal(t, 1, d.Total()) + assert.Contains(t, d.FormatOneLine(), common.DropTargetSizedToZero) +} + +func TestApplyTargetCoverage_NoSignal_PassesThroughUnchanged(t *testing.T) { + t.Parallel() + rec := mkRI(5, 0, 0) // AverageInstancesUsedPerHour <= 0 -> no signal + d := common.NewDropSummary() + out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil, d) + require.Len(t, out, 1) + assert.Equal(t, rec, out[0], "passed through unchanged") + assert.True(t, d.IsEmpty(), "no-signal is not a drop") +} + +// TestApplyTargetCoverage_ProjectionsClampTo100 covers the clamp on +// ProjectedUtilization (easy to overflow: avg/nTarget*100 grows unbounded as +// nTarget shrinks relative to avg) and documents why ProjectedCoverage's +// clamp is a defensive boundary check rather than a reachable overflow: floor +// guarantees nTarget/avg*100 <= gapPct, so projCov <= existing+gapPct == +// target <= 100 in exact arithmetic. The target=100 boundary case below +// exercises that bound exactly. +func TestApplyTargetCoverage_ProjectionsClampTo100(t *testing.T) { + t.Parallel() + + t.Run("ProjectedUtilization clamps", func(t *testing.T) { + t.Parallel() + // avg=100, target=10, existing=0: gap=10, nTarget=floor(100*10/100)=10. + // Unclamped projUtil = 100/10*100 = 1000%. + rec := mkRI(200, 100, 0) + out := ApplyTargetCoverage([]common.Recommendation{rec}, 10, nil, nil) + require.Len(t, out, 1) + assert.Equal(t, 100.0, out[0].ProjectedUtilization) + }) + + t.Run("ProjectedCoverage stays at the target boundary", func(t *testing.T) { + t.Parallel() + // avg=10, target=100, existing=0: gap=100, nTarget=floor(10)=10. + // projCov = 0 + 10/10*100 = 100.0 exactly at the clamp boundary. + rec := mkRI(10, 10, 0) + out := ApplyTargetCoverage([]common.Recommendation{rec}, 100, nil, nil) + require.Len(t, out, 1) + assert.LessOrEqual(t, out[0].ProjectedCoverage, 100.0) + assert.Equal(t, 100.0, out[0].ProjectedCoverage) + }) +} + +// TestApplyTargetCoverage_CountZeroNoNaNOrInf covers the rec.Count==0 +// fallback (`ratio = float64(nTarget)` instead of nTarget/rec.Count) that +// guards against a division by zero producing NaN/Inf in every scaled money +// field. +func TestApplyTargetCoverage_CountZeroNoNaNOrInf(t *testing.T) { + t.Parallel() + monthly := 20.0 + rec := mkRI(0, 10, 0) // Count=0, avg=10, existing=0 + rec.RecurringMonthlyCost = &monthly + + out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil, nil) + + require.Len(t, out, 1) + got := out[0] + for name, v := range map[string]float64{ + "CommitmentCost": got.CommitmentCost, + "OnDemandCost": got.OnDemandCost, + "EstimatedSavings": got.EstimatedSavings, + "RecurringMonthlyCost": *got.RecurringMonthlyCost, + "ProjectedUtilization": got.ProjectedUtilization, + "ProjectedCoverage": got.ProjectedCoverage, + } { + assert.False(t, math.IsNaN(v), "%s is NaN", name) + assert.False(t, math.IsInf(v, 0), "%s is Inf", name) + } +} + +func TestApplyTargetCoverage_SPEdgePassthroughs(t *testing.T) { + t.Parallel() + + t.Run("RecommendedUtilization <= 0 passes through unchanged", func(t *testing.T) { + t.Parallel() + rec := mkSP(0, 2.0) + out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil, nil) + require.Len(t, out, 1) + assert.Equal(t, rec, out[0]) + }) + + t.Run("HourlyCommitment <= 0 passes through unchanged", func(t *testing.T) { + t.Parallel() + rec := mkSP(50, 0) + out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil, nil) + require.Len(t, out, 1) + assert.Equal(t, rec, out[0]) + }) + + t.Run("wrong-type Details passes through with warning, ProjectedUtilization stays zero", func(t *testing.T) { + t.Parallel() + var logs []string + rec := mkSP(50, 2.0) + rec.Details = common.ComputeDetails{Platform: "Linux/UNIX"} + out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, captureLogf(&logs), nil) + require.Len(t, out, 1) + assert.Equal(t, rec, out[0]) + assert.Equal(t, 0.0, out[0].ProjectedUtilization, "scaling failed, so projection must not be set") + assert.Len(t, logs, 1) + }) + + t.Run("typed-nil Details passes through with warning, ProjectedUtilization stays zero", func(t *testing.T) { + t.Parallel() + var logs []string + rec := mkSP(50, 2.0) + rec.Details = (*common.SavingsPlanDetails)(nil) + out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, captureLogf(&logs), nil) + require.Len(t, out, 1) + assert.Equal(t, 0.0, out[0].ProjectedUtilization) + assert.Len(t, logs, 1) + }) +} + +// TestApplyTargetCoverage_UnsupportedType_WarnsOncePerType covers a slice +// containing several recs of the same unsupported CommitmentType: the +// warning must fire exactly once per distinct type across the whole slice, +// not once per rec, and every rec passes through unchanged regardless. +func TestApplyTargetCoverage_UnsupportedType_WarnsOncePerType(t *testing.T) { + t.Parallel() + var logs []string + recs := []common.Recommendation{ + {Service: common.ServiceCompute, CommitmentType: common.CommitmentCUD, Count: 1}, + {Service: common.ServiceCompute, CommitmentType: common.CommitmentCUD, Count: 2}, + {Service: common.ServiceCompute, CommitmentType: common.CommitmentReservedCapacity, Count: 3}, + } + + out := ApplyTargetCoverage(recs, 80, captureLogf(&logs), nil) + + require.Len(t, out, 3) + assert.Equal(t, recs, out, "unsupported types pass through unchanged") + + cudWarnings, capacityWarnings := 0, 0 + for _, l := range logs { + switch { + case strings.Contains(l, string(common.CommitmentCUD)): + cudWarnings++ + case strings.Contains(l, string(common.CommitmentReservedCapacity)): + capacityWarnings++ + } + } + assert.Equal(t, 1, cudWarnings, "CommitmentCUD warns exactly once despite 2 recs") + assert.Equal(t, 1, capacityWarnings, "CommitmentReservedCapacity warns exactly once") +} + +func TestApplyTargetCoverage_TargetPctOutOfRange_PassesThroughWithWarning(t *testing.T) { + t.Parallel() + recs := []common.Recommendation{mkRI(5, 8, 0)} + + for _, targetPct := range []float64{0, -1, 101} { + var logs []string + out := ApplyTargetCoverage(recs, targetPct, captureLogf(&logs), nil) + assert.Equal(t, recs, out, "targetPct=%.0f", targetPct) + assert.Len(t, logs, 1, "targetPct=%.0f", targetPct) + } +} + +func TestApplyTargetCoverage_NilLogfSafe(t *testing.T) { + t.Parallel() + recs := []common.Recommendation{ + mkRI(5, 8, 90), // gapPct <= 0 -> INFO log + mkRI(1, 0.5, 0), // sized to zero -> INFO log + mkRI(5, 0, 0), // no signal, no log + mkSP(50, 0), // no signal, no log + {Service: common.ServiceCompute, CommitmentType: common.CommitmentCUD}, // unsupported -> WARNING log + } + assert.NotPanics(t, func() { + ApplyTargetCoverage(recs, 80, nil, nil) + }) + assert.NotPanics(t, func() { + ApplyTargetCoverage(recs, 0, nil, nil) // out-of-range -> WARNING log + }) +}